> 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
>
>
|