mesos-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Adam B <a...@mesosphere.io>
Subject Re: Review Request 46969: Added (Framework/Executor/Command}Info to authorizer object message.
Date Thu, 05 May 2016 07:47:43 GMT

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



Good start, but I have some basic questions.


include/mesos/authorizer/authorizer.proto (lines 38 - 43)
<https://reviews.apache.org/r/46969/#comment195828>

    Is your intention that only one of these optional fields will be set for a particular
request, or can multiple of them be set?
    If only-one, then you should add a Type enum so modules don't have to guess which field
to use.



include/mesos/authorizer/authorizer.proto (lines 40 - 42)
<https://reviews.apache.org/r/46969/#comment195826>

    Can you give an example of why we would want/need each of these? ExecutorInfo.name is
insufficient



src/authorizer/local/authorizer.cpp (line 269)
<https://reviews.apache.org/r/46969/#comment195831>

    ExecutorInfo.name is a silly thing to authorize on, since it's arbitrarily set by the
user, and there's no control over what values it can be set to.
    FrameworkInfo.role and CommandInfo.user are both the subjects of creation-time authorization
checks, so authorizing read access by these fields is sensible.



src/authorizer/local/authorizer.cpp (line 274)
<https://reviews.apache.org/r/46969/#comment195829>

    s/name field/user field/
    s/ExecutorInfo/CommandInfo/



src/authorizer/local/authorizer.cpp (line 276)
<https://reviews.apache.org/r/46969/#comment195830>

    If not set, use FrameworkInfo.user


- Adam B


On May 4, 2016, 5:58 a.m., Joerg Schad wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/46969/
> -----------------------------------------------------------
> 
> (Updated May 4, 2016, 5:58 a.m.)
> 
> 
> Review request for mesos, Adam B and Alexander Rojas.
> 
> 
> Bugs: MESOS-5169
>     https://issues.apache.org/jira/browse/MESOS-5169
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> This allows us to use common protobug messages for authorization.
> 
> 
> Diffs
> -----
> 
>   include/mesos/authorizer/authorizer.proto 32492a59ad95df3bb673ec42321518f86c11af59

>   src/authorizer/local/authorizer.cpp e59c11269670a7ed72b780913971b421ee17f33f 
> 
> Diff: https://reviews.apache.org/r/46969/diff/
> 
> 
> Testing
> -------
> 
> tested entire chain (see upcoming patches)
> 
> 
> Thanks,
> 
> Joerg Schad
> 
>


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