mesos-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Adam B" <a...@mesosphere.io>
Subject Re: Review Request 22284: Authorized roles.
Date Tue, 10 Jun 2014 08:37:38 GMT

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

Ship it!


Good stuff. I'm just a bit confused about how the ReceiveOffers Message uses ANY/NONE principals
in the master ACL vs. a specific request.


src/master/flags.hpp
<https://reviews.apache.org/r/22284/#comment79922>

    Deprecate this in favor of '--acls' too?



src/master/master.cpp
<https://reviews.apache.org/r/22284/#comment79923>

    Why is this wrapped now? I only count 77 chars, and it doesn't look modified otherwise.



src/master/master.cpp
<https://reviews.apache.org/r/22284/#comment79925>

    Please correct me if I'm misinterpreting the ReceiveOffers Message.
    In the master ACL:
    global-permissive: ANY Principal can ReceiveOffers for Role foo
    global-restrictive: NONE Principal can ReceiveOffers for Role foo
    But when calling authorize(ReceiveOffers request),
    Use ANY Principal when the framework has No Principal (framework authentication disabled?),
since that's the only time this framework will still be authorized.
    Use NONE Principal never?!?



src/master/master.cpp
<https://reviews.apache.org/r/22284/#comment79926>

    These Option<Error>'s read weird to me. I would expect future.get().isSome() to
be a positive case, not an error case.
    Using Try<Nothing>'s instead gives "if(future.get().isError()) { //handle error
}", which reads a lot better to me.



src/master/master.cpp
<https://reviews.apache.org/r/22284/#comment79927>

    Log 'from'?



src/master/master.cpp
<https://reviews.apache.org/r/22284/#comment79928>

    Log frameworkInfo.id()?



src/tests/master_authorization_tests.cpp
<https://reviews.apache.org/r/22284/#comment79929>

    Why confirm the first rereg with an EXPECT_CALL(sched, reregistered) and the second rereg
with FUTURE_PROTOBUF(FrameworkReregisteredMessage)?


- Adam B


On June 9, 2014, 3:09 p.m., Vinod Kone wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/22284/
> -----------------------------------------------------------
> 
> (Updated June 9, 2014, 3:09 p.m.)
> 
> 
> Review request for mesos, Benjamin Hindman and Ben Mahler.
> 
> 
> Bugs: MESOS-1307
>     https://issues.apache.org/jira/browse/MESOS-1307
> 
> 
> Repository: mesos-git
> 
> 
> Description
> -------
> 
> Added authorization for roles during framework (re-)registration.
> 
> 
> Diffs
> -----
> 
>   src/master/flags.hpp 486335970ef05b345c5584ac012dde63437ef149 
>   src/master/master.hpp e2448310aa714b5fae49b9bf4fc95859ae9d7ec3 
>   src/master/master.cpp 89f426c14de365369b900864f1983b1f9260953f 
>   src/tests/master_authorization_tests.cpp PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/22284/diff/
> 
> 
> Testing
> -------
> 
> make check
> 
> 
> Thanks,
> 
> Vinod Kone
> 
>


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