> 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. > > Maxim Khutornenko wrote: > 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. > > Kevin Sweeney wrote: > In Shiro this will look something like > > subject.checkPermission("update:resume:mesos:prod:labrat"); > > With role update_coordinator having: > ``` > update:* > ``` > > role root having: > ``` > * > ``` > > and user mesos having: > ``` > *:*:mesos > ``` > > So they'd all be allowed.
> Or maybe you suggesting an approach similar to killTasks(), where an admin > check followed by a role validation? Yeah, this. I'm not asking for update heartbeats to require ROOT. > The problem here is that we don't have a job key to extract the role for > authorization. We have an association between {{updateId}} and role owning the job, right? /** * Fetches a read-only view of a job update. * * @param updateId Update ID to fetch. * @return A read-only view of job update. */ Optional<IJobUpdate> fetchJobUpdate(String updateId); - Bill ----------------------------------------------------------- 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 > >