> On Feb. 3, 2015, 12:16 a.m., Bill Farner wrote: > > src/main/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterface.java, > > line 1535 > > <https://reviews.apache.org/r/30325/diff/3/?file=841928#file841928line1535> > > > > This is too restrictive. It means that _only_ the update coordinator > > role may call this RPC, and that a user cannot build something to pulse > > their own updates. > > > > You should instead do what `isAdmin` does and fall back to the > > coordinator role if direct auth fails. > > > > This is an unfortunate state of affairs, and hopefully the move to > > shiro dramatically improves all this.
I don't see how it's necessarily better. Pulsing can always be done under UPDATE_COORDINATOR membership, which is specifically covering heartbeats only. The isAdmin() requires ROOT that opens up everything else, including the ability to kill anyone's tasks in the claster. Isn't that more restrictive in real life? We don't expect regular users having ROOT access, meaning they will unlikely to get to write their own pulse routine due to that. Or maybe you suggesting an approach similar to killTasks(), where an admin check followed by a role validation? The problem here is that we don't have a job key to extract the role for authorization. Perhaps we can change the RPC to accept a job key instead but that would open up for a race we don't want to see (e.g. late pulse arrives for a wrong update ID). We could probably get away with both updateId and jobKey in the API to avoid ambiguity, or perhpas just updateId and role? I am open to suggestions. - Maxim ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30325/#review70655 ----------------------------------------------------------- On Jan. 30, 2015, 5:23 p.m., Maxim Khutornenko wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/30325/ > ----------------------------------------------------------- > > (Updated Jan. 30, 2015, 5:23 p.m.) > > > Review request for Aurora, David McLaughlin, Joshua Cohen, and Bill Farner. > > > Bugs: AURORA-1009 > https://issues.apache.org/jira/browse/AURORA-1009 > > > Repository: aurora > > > Description > ------- > > Implemented the `pulseJobUpdate` RPC. Also, moved it into AuroraAdmin > interface to support AOP capability validation. > > The RB is diffed against https://reviews.apache.org/r/30225/ > > > Diffs > ----- > > api/src/main/thrift/org/apache/aurora/gen/api.thrift > 4f603f9e7ed004e53937a45ea8edf7241e15f5cf > src/main/java/org/apache/aurora/auth/CapabilityValidator.java > 45ef643ebe57c1517cdae373574331ea302a8b74 > > src/main/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterface.java > 8c19f3b08135eb5f3098591ebf9931b42a086318 > > src/test/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterfaceTest.java > 03d1fba76c23570c2c4102a48daf5ce035ecaaa3 > > Diff: https://reviews.apache.org/r/30325/diff/ > > > Testing > ------- > > ./gradlew -Pq build > > > Thanks, > > Maxim Khutornenko > >
