> 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
>
>
