> On Feb. 7, 2017, 1:16 a.m., Adam B wrote: > > src/master/master.cpp, line 2178 > > <https://reviews.apache.org/r/56178/diff/5/?file=1626265#file1626265line2178> > > > > What does the framework's capability have to do with whether a custom > > authorizer can support multi-role authorization? > > If I have an old authorizer module, and multi-role (phase 1) capable > > frameworks, I will only be able to authorize frameworks with a single role > > (regardless of capability). For a multi-role framework, I could authorize a > > single role from the list, but that essentially provides a back-door for > > those frameworks to use other roles. Or I could call the authorizer > > multiple times, one for each role. But we wouldn't want to do that for > > multi-role capable authorizers. Maybe we need the concept of "module > > capabilites" now too? > > Am I overthinking this? > > Benjamin Bannier wrote: > There is only a weak relation between this framework capability and the > capabilities of authorizers. My thinking here was roughly: > > * The way authorization works ("send at most a single authorization > request per authorizable action") all information required for authorization > needs to be packed into a single authorization request. > * There are authorizers relying only on the `value` field to determine > the role of a framework; this approach is deprecated. > * In the current authorization approach there is no meaningful value for > `value` for multirole frameworks. > * Multirole frameworks are a new feature so not updating deprecated > features to work with them does not break backwards compatibility. > > With this change we > > * maintain the current, single-request authorization approach, > * avoid breaking authorizers assuming frameworks can only have a single > role, > * avoid breaking authorizers using `value` and assuming a single-role > world, and > * enable authorizers using `framework_info` to authorize multirole > frameworks. > > This is very much in line with how e.g., `FrameworkInfo` has both a > (deprecated) `role` and a `roles` field, and the correct action is taken > depending on framework capabilities. > > Would we instead e.g., send an authorization request for each role, we > wouldn't just introduce new code to use a deprecated field (`value`), but we > would also introduce a lot of potential overhead, all while ignoring the > existence of `framework_info` which already now bundles all this information > and can be sent as part of an authorization request.
Thanks for the explanation. I agree that sending an authz request for each role is overkill just to support a deprecated field. Forget about that. My only concern is that authz decisions generally prefer to deny access when they don't know what to do, and an old authorizer that only looks at the `value` field will try to authorize a Framework registering with no role. Is that the same as registering with `*`? If not, they probably already handle that as an error. If so, they would probably authorize based on `*` (allow), when they should be authorizing based on `foo` and `bar` (deny). If that's a serious concern, I'd rather remove the `value` field or otherwise break their compilation so they are forced to review the API changes. - Adam ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/56178/#review164477 ----------------------------------------------------------- On Feb. 7, 2017, 2:26 a.m., Benjamin Bannier wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/56178/ > ----------------------------------------------------------- > > (Updated Feb. 7, 2017, 2:26 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 98c39b279e7b9830d02efc8ec6a4469afc15d62a > > Diff: https://reviews.apache.org/r/56178/diff/ > > > Testing > ------- > > Tested on various configurations in internal CI. > > > Thanks, > > Benjamin Bannier > >
