> On April 29, 2020, 2:58 p.m., Andrei Sekretenko wrote:
> > src/master/master.cpp
> > Lines 12264 (patched)
> > <https://reviews.apache.org/r/72448/diff/1/?file=2229246#file2229246line12264>
> >
> > As implemented now, `false` returned by `ObjectApprovers::approved()`
> > is not necessarily an error case. This might just be a declined
> > authorization, i.e. the subscriber is not even allowed to see the object
> > in question due to the missing permissions. Events for such objects should
> > just be not sent to the subscriber.
> >
> > To distinguish between **declined authorization** and **authorization
> > error** here, you will have to change/extend the interface provided by
> > `ObjectApprovers` (plural) so that it doesn't hide `Error`s returned by
> > `approved()` method of `ObjectApprover`(singular).
>
> Dong Zhu wrote:
> Why will we need to distinguish those two circumstances ?
> `ObjectApprovers::approved()` returned with `false` when **declined
> authorization** or **authorization error** happens. The purpose of this patch
> just handling the `false` part by sending a notification to the subscriber.
>
> Andrei Sekretenko wrote:
> The `false` part (i.e. declined authorization) is not an issue.
> Subscriber does not need to be informed about the fact that somewhere on
> the cluster there are some objects (tasks/frameworks/whatever) that the
> subscriber is not allowed to access.
> It is frequently the case that there are, and there is nothing wrong with
> it.
>
> Declined authorization is pretty much expected to occur, and existing
> subscribers (such as DC/OS UI) are designed not to expect any information
> about the objects their principal is not authorized to access.
>
> Probably https://issues.apache.org/jira/browse/MESOS-7785 points to more
> detailed explanations why this decision was made.
>
>
> On the other hand, subscribers expect to see all events for all objects
> they are authorized to see.
> Here lies the issue described in MESOS-10085: in case of an authorization
> failure, the event will be silently dropped, but the subscriber will have no
> knowledge of that and will continue working as if there was no event.
In summary:
- for declined authorization circumstance:
There is no need to inform the subscriber just drop the message. eg, for
`FRAMEWORK_ADDED` event, if there is no proper permission like `VIEW_FRAMEWORK`
configured just drop the message.
- for authorization error circumstance:
Send an event to subscriber to inform the error. It fixed the issue
intorducted in MESOS-10085 **"in case of an authorization failure, the event
will be silently dropped, but the subscriber will have no knowledge of that and
will continue working as if there was no event."**
Am I correct ?
- Dong
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/72448/#review220548
-----------------------------------------------------------
On April 29, 2020, 6:18 a.m., Dong Zhu wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/72448/
> -----------------------------------------------------------
>
> (Updated April 29, 2020, 6:18 a.m.)
>
>
> Review request for mesos, Andrei Sekretenko and Benjamin Mahler.
>
>
> Repository: mesos
>
>
> Description
> -------
>
> This patch intends to fix issue MESOS-10085.
>
> When the authorization failed happens master return nothing to the
> subscriber, subscriber isn't aware of what is happening, this issue
> can lead to inconsistencies in Event stream.
>
>
> Diffs
> -----
>
> include/mesos/master/master.proto 021dadcea026da41347b3aaee5ddd12f4f14fa29
> include/mesos/v1/master/master.proto
> 488fe294e8bfe8e0c6fc23c88f06c0d41169b96d
> src/master/master.cpp a8cca622ff0bd172300b9a2717b4860ed06b620c
> src/tests/master/mock_master_api_subscriber.cpp
> 893d3e366164ccebd2847ed4c2874ab00e0e5b7b
>
>
> Diff: https://reviews.apache.org/r/72448/diff/1/
>
>
> Testing
> -------
>
> - Manually tested
> - make check
>
>
> Thanks,
>
> Dong Zhu
>
>
