mesos-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Greg Mann <g...@mesosphere.io>
Subject Re: Review Request 58254: Added implicit executor authorization to the agent operator API.
Date Fri, 07 Apr 2017 23:25:51 GMT


> On April 7, 2017, 11:40 a.m., Alexander Rojas wrote:
> > src/authorizer/local/authorizer.cpp
> > Lines 654-657 (original), 683-690 (patched)
> > <https://reviews.apache.org/r/58254/diff/1/?file=1686361#file1686361line683>
> >
> >     I'm not so sure returning a `RejectingObjectApprover()` is the right thing to
do. It looks to me that the request is in an invalid state and at the very least should log
that, but this if sounds like a precondition to me: If it has claims, it needs to be one of
the three actions. Probably chang it for a `CHECK`.
> >     
> >     If I'm wrong, at least a comment mentioning when is it a valid case having a
request that fails the check would help.

There's nothing stopping a client from providing a token with claims but no value in an operator
API request with some other action, so we need to fail authorization for those cases somewhere.
However, it's probably better to do it in the body of `getObjectApprover`, rather than here.
Will update and add a comment.


> On April 7, 2017, 11:40 a.m., Alexander Rojas wrote:
> > src/authorizer/local/authorizer.cpp
> > Lines 692-699 (patched)
> > <https://reviews.apache.org/r/58254/diff/1/?file=1686361#file1686361line692>
> >
> >     Refer to my review of previous patch.

I'm not sure precisely what you mean to say here, could you elaborate?


- Greg


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


On April 7, 2017, 11:25 p.m., Greg Mann wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/58254/
> -----------------------------------------------------------
> 
> (Updated April 7, 2017, 11:25 p.m.)
> 
> 
> Review request for mesos, Adam B, Alexander Rojas, Till Toenshoff, and Vinod Kone.
> 
> 
> Bugs: MESOS-7014
>     https://issues.apache.org/jira/browse/MESOS-7014
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> This patch updates the agent handlers for the LAUNCH_, WAIT_,
> and KILL_NESTED_CONTAINER calls of the operator API to set the
> `container_id` field within the authorization object,
> facilitating implicit executor authorization.
> 
> 
> Diffs
> -----
> 
>   include/mesos/authorizer/authorizer.proto 736f76d552956f2351ffd40fc51d088dff83f8c8

>   src/authorizer/local/authorizer.cpp e241edf4afa48d35dbbbb94d72e8e8690f5bedfc 
>   src/slave/http.cpp b07ce7c73a90ef297d980806ebba9530d86f25ae 
> 
> 
> Diff: https://reviews.apache.org/r/58254/diff/2/
> 
> 
> Testing
> -------
> 
> Testing details can be found at the end of this chain.
> 
> 
> Thanks,
> 
> Greg Mann
> 
>


Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message