mesos-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Benjamin Bannier <benjamin.bann...@mesosphere.io>
Subject Re: Review Request 56178: Enabled the authorizer to work with MULTI_ROLE frameworks.
Date Thu, 09 Feb 2017 08:59:59 GMT


> On Feb. 9, 2017, 8:53 a.m., Adam B wrote:
> > src/master/master.cpp, lines 2178-2179
> > <https://reviews.apache.org/r/56178/diff/6/?file=1626414#file1626414line2178>
> >
> >     Why not check `if(protobuf::framework::getRoles(frameworkInfo).size() <=
1)` instead of checking the capability? A legacy authorizer could still authorize a multi-role-capable
framework if it's only trying to register with a single role.

This cannot be used instead of the current condition; what you propose would amount to an
additional branch to set `value` to `roles[0]` for single role, `MULTI_ROLE` frameworks.

I am not sure of the value this would bring; we might effectively allow users to run multirole
frameworks, but these frameworks could only be in a single role, and all this for authorizers
already relying on a deprecated field. Currently a framework's `role` cannot be changed (not
enforced, only per documentation); with the upcoming possibility to change framework `roles`,
`MULTI_ROLE` frameworks we would enable here to work with these legacy authorizers would only
fail to authz if they added another role.

I do see value in disallowing use of `MULTI_ROLE` features in these legacy setups as early
as possible, instead of letting them fail later. It would also help us avoid introducing new,
but already deprecated behavior.

Anything else to keep in mind here?


- Benjamin


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


On Feb. 9, 2017, 9:59 a.m., Benjamin Bannier wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/56178/
> -----------------------------------------------------------
> 
> (Updated Feb. 9, 2017, 9:59 a.m.)
> 
> 
> Review request for mesos, Adam B, Alexander Rojas, and Benjamin Mahler.
> 
> 
> Bugs: MESOS-7022
>     https://issues.apache.org/jira/browse/MESOS-7022
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> This updates the local authorizer so that MULTI_ROLE frameworks can be
> authorized.
> 
> For non-MULTI_ROLE frameworks we continue to support use of the
> deprecated 'value' field in the authorization request's 'Object';
> however for MULTI_ROLE frameworks the 'value' field will not be set,
> and authorizers still relying on it should be updated to instead use
> the object's 'framework_info' field to extract roles to authorize
> against from.
> 
> 
> Diffs
> -----
> 
>   src/authorizer/local/authorizer.cpp b98e1fcdf2ee5ec1f6ac0be6f8accdefaa390a09 
>   src/master/master.cpp 620919ecfe85367b5c1281afc5216cc20e5e2e3c 
> 
> Diff: https://reviews.apache.org/r/56178/diff/
> 
> 
> Testing
> -------
> 
> Tested on various configurations in internal CI.
> 
> 
> Thanks,
> 
> Benjamin Bannier
> 
>


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