> On Jan. 22, 2016, 2:44 a.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'
>     ```

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.


- Maxim


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


On Jan. 20, 2016, 8: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, 8: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