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




src/master/master.hpp
Line 593 (original), 593 (patched)
<https://reviews.apache.org/r/64515/#comment272266>

    I wonder how come we didn't do this before?



src/master/master.cpp
Lines 6138-6141 (original), 6169-6173 (patched)
<https://reviews.apache.org/r/64515/#comment272268>

    How about:
    
    _Not authorized to register agent with(out) principal XXX providing the 
resrouces YYYY_



src/master/master.cpp
Lines 6478-6480 (original), 6512-6515 (patched)
<https://reviews.apache.org/r/64515/#comment272269>

    ditto.



src/tests/master_authorization_tests.cpp
Lines 2515 (patched)
<https://reviews.apache.org/r/64515/#comment272273>

    `s/slaveFlags/agentFlags/`



src/tests/master_authorization_tests.cpp
Lines 2518 (patched)
<https://reviews.apache.org/r/64515/#comment272274>

    `s/slave/agent/` here and below.



src/tests/master_authorization_tests.cpp
Lines 2538-2539 (patched)
<https://reviews.apache.org/r/64515/#comment272270>

    Instead of the things in the parenthesis you can add:
    
    ```c++
    {
      mesos::ACL::ReserveResources* acl = acls.add_reserve_resources();
      acl->mutable_principals()->add_values(DEFAULT_CREDENTIAL.principal());
      acl->mutable_roles()->set_type(ACL::Entity::NONE);
    }
    ```
    
    And add a comment when starting the agent indicating that the agent is 
registering with `DEFAULT_CREDENTIAL.principal()`



src/tests/master_authorization_tests.cpp
Lines 2564 (patched)
<https://reviews.apache.org/r/64515/#comment272271>

    `s/slaveFlags/agentFlags/`



src/tests/master_authorization_tests.cpp
Lines 2567 (patched)
<https://reviews.apache.org/r/64515/#comment272272>

    `s/slave/agent` here and below.



src/tests/master_authorization_tests.cpp
Lines 2571 (patched)
<https://reviews.apache.org/r/64515/#comment272275>

    Please add a test where the agent tries to register with 
`high-security-role` and succeeds.



src/tests/master_authorization_tests.cpp
Lines 2577 (patched)
<https://reviews.apache.org/r/64515/#comment272276>

    this test can be merged with 
`MasterAuthorizationTest.UnauthorizedToStaticallyReserveResources` and you save 
a restart of a master.


- Alexander Rojas


On Dec. 12, 2017, 1:30 a.m., Jiang Yan Xu wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/64515/
> -----------------------------------------------------------
> 
> (Updated Dec. 12, 2017, 1:30 a.m.)
> 
> 
> Review request for mesos, Alexander Rojas and James Peach.
> 
> 
> Bugs: MESOS-8306
>     https://issues.apache.org/jira/browse/MESOS-8306
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Used `reserve_resources` ACL for static reservations.
> 
> 
> Diffs
> -----
> 
>   include/mesos/authorizer/acls.proto 
> 40a1425ca51c5bb70f7af2e17d605f2125dcb4cb 
>   src/master/master.hpp 232cc3758f240db626c4fdaf852163fa48af4dd7 
>   src/master/master.cpp b10d0341276090bfa70aaa4fd6317a560e3334ea 
>   src/tests/master_authorization_tests.cpp 
> 676543a5ad1bb5d47011fc2a8b05dfaaeef18c64 
> 
> 
> Diff: https://reviews.apache.org/r/64515/diff/1/
> 
> 
> Testing
> -------
> 
> make check.
> 
> 
> Thanks,
> 
> Jiang Yan Xu
> 
>

Reply via email to