> On Jan. 21, 2016, 6:44 p.m., Zameer Manji wrote:
> > src/main/java/org/apache/aurora/scheduler/http/api/security/HttpSecurityModule.java,
> >  line 80
> > <https://reviews.apache.org/r/42565/diff/3/?file=1203749#file1203749line80>
> >
> >     Just to be clear, this command line argument will only accept two 
> > modules, the ini realm module and the kerberos realm module?
> >     
> >     This means users who need to setup a custom realm (probably any medium 
> > to large cluster operator), must create a custom main.
> >     
> >     I'm unsure if this is the ideal approach to take w.r.t security. I 
> > think this requires some discussion on the mailing list. For example, an 
> > alternative to what you have done here would be to remove this argument 
> > entirely, and request users specify realms in the ini file (see 
> > http://shiro.apache.org/configuration.html section "INI Sections").
> 
> Bill Farner wrote:
>     In case the concern about a custom main is that it sounds dautning, 
> here's an illustration:
>     
>     ```
>     public class CustomMain {
>       public static void main(String[] args) {
>         SchedulerMain.applyStaticArgumentValues(args);
>         SchedulerMain.publicMain(new CustomRealmModule());
>       }
>     }
>     ```
>     
>     (It may be worth building in the `applyStaticArgumentValues` step, so we 
> can reduce even further.)
>     
>     I see 2 upsides with this:
>     - The API for customization (whether security or other arbitrary feature 
> addition) is uniform.
>     - The `CustomMain` can use the same commane line arguments system as the 
> rest of the scheduler (this will work with the forthcoming replacement as 
> well).
>     
>     I believe your suggestion to use an INI section falls apart in 2 ways:
>     - the `[main]` section only applies when using pure INI configuration for 
> shiro. I believe we fall under the programmatic configuration section, with 
> shiro-guice doing the programmatic bits.  The ini file aurora accepts is read 
> by `org.apache.shiro.realm.text.IniRealm` and it only uses the `users` and 
> `roles` sections (see 
> https://shiro.apache.org/static/1.2.3/apidocs/org/apache/shiro/realm/text/IniRealm.html)
>     - i don't believe classes configured with pure shiro INI configuration 
> would participate in guice injection
>     
>     I'm open to ideas here.  Ultimately i'm trying to preserve integration of 
> custom secruity controls with features that exist today:
>     1. support for participation in guice injection
>     2. support for participation in the same configuration system as the rest 
> of the scheduler
>     
>     If (1) is not necessary, that simplifies things.  An alternative to (2) 
> is to force custom code to handle its own configuration (e.g. with 
> environment variables).
> 
> Maxim Khutornenko wrote:
>     Perhaps a script example will help evaluating this approach? Bill, would 
> you mind giving a simple example of how aurora-scheduler.conf looked had we 
> added a custom module in our e2e tests?
> 
> Bill Farner wrote:
>     `aurora-scheduler.conf` would not change, as you would only alter the 
> main class.  The change would be in `build.gradle`:
>     
>     From
>     ```
>     mainClassName = 'org.apache.aurora.scheduler.app.SchedulerMain'
>     ```
>     
>     to
>     ```
>     mainClassName = 'com.my.package.CustomScheduler'
>     ```
> 
> Maxim Khutornenko wrote:
>     This means anyone who wants to add custom modules would be forced to fork 
> Aurora codebase instead of a purely pluggable model that we have now? I am 
> not sure I like it. This is error and merge conflict prone.
>     
>     Perhaps having a well documented gradle option taking a path to 
> CustomScheduler would be a better middle ground? At least that would not 
> require forking Aurora.
> 
> John Sirois wrote:
>     Wait - how do you extend aurora today?  My understanding is you need to 
> muck with start scripts to ammend the classpath.  Under that assumption, I 
> saw mucking with classpath + changing main class name as not an undue burden 
> over and above the status quo.
> 
> John Sirois wrote:
>     Basically - if you actually want to claim the label pluggable, you need 
> an out-of-packaging way to ammend the classpath.  IE: an optional file in - 
> say ubuntu - /etc/default/aurora.d/my_plugins.conf - that adds a, say 
> 'my_extensions/*' entry to the claaspath.  The need to write a main could be 
> converted to using a requirement to implement a ServiceLoader interface that 
> would allow the standard scheduler main to scan for service implementations - 
> that service might just be a SAM that returns an Iterable of Modules or else 
> it could be a more rigid service interface with 1 method (defaulted) per new 
> extension point that comes up.
>     
>     I think this sort of puggability would be great to have, but it seems to 
> me that should be decoupled from the new args system if this change is 
> good-enough, ie: maintains the status quo of having to 1.) compile your own 
> jars out-of-band, 2.) deliver them to aurora clusters where the aurora start 
> scripts are modified to add them to the classpath and - now - change the name 
> of the main in the 'java ...' command line.
> 
> John Sirois wrote:
>     And even then - Aurora won't be pluggable by any reasonable definition 
> unless it publishes the extension APIs as a library.  It looks like that was 
> attempted once but is not kept up2date: 
> http://search.maven.org/#artifactdetails%7Corg.apache.aurora%7Caurora-api%7C0.8.0%7Cjar

Ping?


- Bill


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/42565/#review115765
-----------------------------------------------------------


On Jan. 20, 2016, 12:21 p.m., Bill Farner wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42565/
> -----------------------------------------------------------
> 
> (Updated Jan. 20, 2016, 12:21 p.m.)
> 
> 
> Review request for Aurora, John Sirois and Zameer Manji.
> 
> 
> Repository: aurora
> 
> 
> Description
> -------
> 
> This patch proposes that users installing custom modules do so via a custom 
> main.
> 
> Specifying custom modules on the command line has proven troublesome for 
> replacing the command line args system with one where all arguments are 
> injected and explicitly-defined.  It also adds complexity to the scheduler 
> application by unnecessarily punching holes at specific places.
> 
> If this proposal is agreeable, i will add a NEWS entry and document how one 
> might implement a custom main to add modules.  The tl;dr, however, is to 
> invoke `SchedulerMain.publicMain(customModule)`
> 
> 
> Diffs
> -----
> 
>   src/main/java/org/apache/aurora/scheduler/app/MoreModules.java 
> d5f96543d1068bf30b9d173a247c2806faf35578 
>   src/main/java/org/apache/aurora/scheduler/app/SchedulerMain.java 
> 0659c358479283756179c2cabebc8416730cc3e3 
>   
> src/main/java/org/apache/aurora/scheduler/http/api/security/HttpSecurityModule.java
>  e32862034a1ad47dae8fff89cb04deb34ccd90e2 
>   
> src/main/java/org/apache/aurora/scheduler/http/api/security/ModuleParser.java 
> ccd9a20e8b18831458cba2d53e6b8b84fef06162 
>   src/test/java/org/apache/aurora/scheduler/app/MoreModulesTest.java 
> b2fb3c9dcba64f69a05894f506ba43766f74ddaa 
>   
> src/test/java/org/apache/aurora/scheduler/http/api/security/HttpSecurityIT.java
>  3e811a4f4d2c82892217ca1f950ac792303fbcf3 
>   
> src/test/java/org/apache/aurora/scheduler/http/api/security/ModuleParserTest.java
>  baaeb2390a909de1a92e4328d35a49f7b74c36cb 
> 
> Diff: https://reviews.apache.org/r/42565/diff/
> 
> 
> Testing
> -------
> 
> end-to-end tests, `./gradlew run`
> 
> 
> Thanks,
> 
> Bill Farner
> 
>

Reply via email to