> 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' > ``` > > 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 > > Bill Farner wrote: > Ping?
To John's question: we actually use a separate custom gradle project linking to the original one to build an internal version. We don't change to mainClassName yet but it's certainly possible. I am fine with this patch as is but would highly appreciate adding a brief doc summary on the new recommended way to use custom modules. - 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 > >
