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 53541: WIP: Added authorization actions for debug API.
Date Thu, 17 Nov 2016 09:44:48 GMT

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



Nice work with the subclassed NestedContainerObjectApprover.
I'd really like to see at least LaunchNestedContainer wired up in the existing v1 API Calls,
so we can see how the approver will be called in context.


include/mesos/authorizer/acls.proto (lines 260 - 261)
<https://reviews.apache.org/r/53541/#comment226346>

    s/running with/running as/



include/mesos/authorizer/acls.proto (lines 266 - 267)
<https://reviews.apache.org/r/53541/#comment226345>

    Ojbects: The operating system users (e.g. linux users) to run the nested container command
as.



include/mesos/authorizer/acls.proto (lines 271 - 272)
<https://reviews.apache.org/r/53541/#comment226343>

    "Which principals are authorized to launch nested containers under a top-level container
running as the given user."



include/mesos/authorizer/acls.proto (lines 277 - 278)
<https://reviews.apache.org/r/53541/#comment226344>

    "Objects: The operating system users (e.g. linux users) of the top-level containers under
which the principal may launch a nested container."



include/mesos/authorizer/acls.proto (lines 282 - 283)
<https://reviews.apache.org/r/53541/#comment226347>

    "Which principals are authorized to launch nested container sessions running as the given
users."



include/mesos/authorizer/acls.proto (lines 293 - 294)
<https://reviews.apache.org/r/53541/#comment226348>

    "Which principals are authorized to launch nested container sessions under a top-level
container running as the given user."
    
    or "... top-level container whose executor was launched with the given user."?



include/mesos/authorizer/acls.proto (lines 386 - 387)
<https://reviews.apache.org/r/53541/#comment226349>

    Is this really proper indentation for a Mesos protobuf? Doesn't look like we've had to
wrap before..



include/mesos/authorizer/authorizer.proto (lines 158 - 159)
<https://reviews.apache.org/r/53541/#comment226341>

    "This action will always set `ExecutorInfo` [and `FrameworkInfo`] in the object, and optionally
`CommandInfo` if available."



include/mesos/authorizer/authorizer.proto (line 160)
<https://reviews.apache.org/r/53541/#comment226342>

    Since LNC, KillNC, and WaitNC already exist, could you wire up the authorization check
for those Calls as a separate patch, so we can see how these fields will be filled in?



include/mesos/authorizer/authorizer.proto (line 166)
<https://reviews.apache.org/r/53541/#comment226340>

    with `ExecutorInfo` and `FrameworkInfo` set, in case the authorizer wants the FrameworkInfo.role?
VIEW_EXECUTOR and ACCESS_SANDBOX both set both.



src/authorizer/local/authorizer.cpp (line 502)
<https://reviews.apache.org/r/53541/#comment226350>

    Maybe just call it a LocalNestedContainerObjectApprover? (to match `getNestedContainerObjectApprover()`)



src/authorizer/local/authorizer.cpp (line 528)
<https://reviews.apache.org/r/53541/#comment226351>

    Remind me, are we guaranteed to have an executor_info->command.user at this point?
Or do we need to check FrameworkInfo.user as a backup?



src/authorizer/local/authorizer.cpp (line 999)
<https://reviews.apache.org/r/53541/#comment226353>

    "specialized"


- Adam B


On Nov. 16, 2016, 7:32 a.m., Alexander Rojas wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/53541/
> -----------------------------------------------------------
> 
> (Updated Nov. 16, 2016, 7:32 a.m.)
> 
> 
> Review request for mesos, Adam B, Kapil Arya, Kevin Klues, and Till Toenshoff.
> 
> 
> Bugs: MESOS-6474
>     https://issues.apache.org/jira/browse/MESOS-6474
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> **WIP: Do not commit**
> 
> Added authorization actions for debug API.
> 
> 
> Diffs
> -----
> 
>   include/mesos/authorizer/acls.proto e3fd6a4a1b617a75714ebd6e08ab10cffa1a7d1b 
>   include/mesos/authorizer/authorizer.hpp 7217600351bb089dbd5728ce015d962be7498665 
>   include/mesos/authorizer/authorizer.proto 0696a629ac2d2b9950e20708f0c3666b58ff7ca0

>   src/authorizer/local/authorizer.cpp 77e05dd2475d6e7511e7c7eeea578ec31ff3d198 
>   src/tests/authorization_tests.cpp d23f551c2caa454da0c0f6cb7d77a8c2bd75a474 
> 
> Diff: https://reviews.apache.org/r/53541/diff/
> 
> 
> Testing
> -------
> 
> make check
> 
> 
> Thanks,
> 
> Alexander Rojas
> 
>


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