----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/70549/#review215484 -----------------------------------------------------------
include/mesos/authorizer/authorizer.proto Line 139 (original), 141-147 (patched) <https://reviews.apache.org/r/70549/#comment302197> How about something like: ``` // TODO(mzhu): Remove this action after associated API calls `SET_QUOTA` and `REMOVE_QUOTA` are no longer supported. // // NOTE: We cannot reuse this action for the `UPDATE_QUOTA` API call, because the associated `QuotaConfig` message contains more information than `QuotaInfo`. ``` include/mesos/authorizer/authorizer.proto Lines 149-150 (patched) <https://reviews.apache.org/r/70549/#comment302198> 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. - Alexander Rukletsov On May 22, 2019, 12:48 p.m., Meng Zhu wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/70549/ > ----------------------------------------------------------- > > (Updated May 22, 2019, 12:48 p.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 > >
