> On May 23, 2019, 4:23 a.m., Alexander Rukletsov wrote: > > include/mesos/authorizer/authorizer.proto > > Lines 149-150 (patched) > > <https://reviews.apache.org/r/70549/diff/3/?file=2146379#file2146379line149> > > > > I would like us to challenge the necessity of passing `QuotaConfig` > > here. The built-in authorizer only looks at `role`, ignoring any > > information about resources. One might say that a custom authorizer might > > utilize that extra information, however, that extra information might not > > be enough to make a decision, because it does not include the current state > > or the state change, e.g., resource delta. > > > > Imagine an authorizer that allows decreasing quota for a number of > > principals, but only a few are allowed to increase. Passing `QuotaConfig` > > does not really help that authorizer to make a decision. Note that > > authorizer cannot tract previous requests to deduce the current quota state > > because previous request could have been dropped after successful > > authorization. > > > > I tend to keep it simple and use just the role. We should consult Till > > Tönshoff and Jan-Philip Gehrcke as maintainers of DC/OS authorizer for more > > input.
Thanks for the comment. Yes, the two authorizers that I am aware of only looks into the role field. I choose to pass `QuotaConfig` mainly to try to be parity with the legacy `QuotaInfo`, i.e. pass info regarding the requested quota setting. For your example of needing more information (e.g. to also pass a role's current old quota config), I think if that need ever arise, we could add another optional object field. The Object message states that (https://github.com/apache/mesos/blob/a8c411d3f8d2895ff5e95c412ef2f3e94713520f/include/mesos/authorizer/authorizer.proto#L40-L46) > An `Object` has a number of optional fields of which depending on the action, > one or more fields must be set. So we can use that as a wrapper for adding extra fields if needed. - Meng ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/70549/#review215484 ----------------------------------------------------------- On May 22, 2019, 5:48 a.m., Meng Zhu wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/70549/ > ----------------------------------------------------------- > > (Updated May 22, 2019, 5:48 a.m.) > > > Review request for mesos, Alexander Rukletsov, Andrei Sekretenko, and > Benjamin Mahler. > > > Bugs: MESOS-9640 > https://issues.apache.org/jira/browse/MESOS-9640 > > > Repository: mesos > > > Description > ------- > > A new authorizable action `UPDATE_QUOTA_WITH_CONFIG` is added. > This disambiguates with the old action `UPDATE_QUOTA` which > are used for the old `SetQuota` and `RemoveQuota` calls. > `UPDATE_QUOTA` action requires `QuotaInfo` as the object while > the new `UpdatedQuota` call uses `QuotaConfig`. To keep it compatible > with any external authorization modules, a new action is introduced. > > > Diffs > ----- > > include/mesos/authorizer/authorizer.proto > e2740c402732bb37db991ec92b9301e58b33215b > src/master/master.hpp 5ad128d00d3cdf0dca89eb637ae196987bdce412 > src/master/quota_handler.cpp a18d8bafda5604d1844f7f7ed31d4ea80fbf6d04 > src/tests/master_authorization_tests.cpp > ee69910a34416728bf14ed23f4a6faae6c1204a0 > > > Diff: https://reviews.apache.org/r/70549/diff/3/ > > > Testing > ------- > > make check > > > Thanks, > > Meng Zhu > >
