> On Sept. 8, 2020, 7:09 p.m., Benjamin Mahler wrote: > > src/master/http.cpp > > Lines 2368-2370 (original), 2368-2370 (patched) > > <https://reviews.apache.org/r/72843/diff/1/?file=2239304#file2239304line2368> > > > > A bit hard for me to understand how the comment relates to the code, I > > guess what I should be getting out of this comment is that the object > > approvers are always functionally equivalent now? > > > > Seems like the comment belongs up above in the de-duplication search? > > > > ``` > > // NOTE: This is not a general-purpose request comparison, but > > // specific to the batched requests which are always members of > > // `ReadOnlyHandler`, since we rely on the response only > > depending > > // on query parameters and the current master state. > > // > > // We don't need to check that the ObjectApprovers are the same, > > // since ...? > > return handler == batchedRequest.handler && > > principal == batchedRequest.principal && > > outputContentType == batchedRequest.outputContentType && > > queryParameters == batchedRequest.queryParameters; > > ``` > > > > Down here it seems like there's nothing related to object approvers to > > comment on.
Good point! While moving the comment, realized how fragile the apporovers reuse actually is: regardless of synchronous authorization, this code heavily relies on the fact that each caller always provides `approvers` for the same set of actions for the same handler. Makes me think about moving `ObjectApprovers` creation into this method (not 100% trivial thanks to that <VIEW_ROLE> specialization in `ObjectApprovers`). Added a TODO. - Andrei ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/72843/#review221817 ----------------------------------------------------------- On Sept. 9, 2020, 5:38 p.m., Andrei Sekretenko wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/72843/ > ----------------------------------------------------------- > > (Updated Sept. 9, 2020, 5:38 p.m.) > > > Review request for mesos and Benjamin Mahler. > > > Repository: mesos > > > Description > ------- > > Now that synchronous authorization requires the authorizer to keep > object approvers up-to-date by the authorizer, batched processing > of readonly requests in the Master no longer intorduces an additional > delay into propagation of permission changes. > > > Diffs > ----- > > src/master/http.cpp f34ea54ec48065f526327252aa10c6d917a96601 > > > Diff: https://reviews.apache.org/r/72843/diff/2/ > > > Testing > ------- > > > Thanks, > > Andrei Sekretenko > >
