> On July 5, 2018, 9:55 p.m., Chun-Hung Hsiao wrote: > > src/master/http.cpp > > Lines 3821 (patched) > > <https://reviews.apache.org/r/67693/diff/3/?file=2044470#file2044470line3821> > > > > Do we want to: > > 1. Simply skip such operations, as suggested in the code here, or > > 2. Return an `InternalServerError` eventually, or > > 3. Just `CHECK` that this shouldn't happen? > > Ditto below. > > > > Please feel free to drop this if you think skipping is good enough. > > Benjamin Bannier wrote: > Thanks for bringing this up. > > Since we here only are concerned with authorizing operations, I still > think that skipping is a fair solution (we cannot show the operation without > authorizing, but also do not need to render the response not useful with > `InternalServerError`; we do not need to `CHECK` either I believe as that > would introduce very hard coupling). > > I added a `WARNING` log with some details. Does that work?
Sounds good. - Chun-Hung ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/67693/#review205755 ----------------------------------------------------------- On July 6, 2018, 7:37 p.m., Benjamin Bannier wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/67693/ > ----------------------------------------------------------- > > (Updated July 6, 2018, 7:37 p.m.) > > > Review request for mesos, Chun-Hung Hsiao and Jan Schlicht. > > > Bugs: MESOS-8473 > https://issues.apache.org/jira/browse/MESOS-8473 > > > Repository: mesos > > > Description > ------- > > This patch adds filtering of operations reported for `GET_OPERATIONS` > calls. A principal is allowed to view see all operations for which it > is allowed to see the role of the underlying resources. > > > Diffs > ----- > > src/master/http.cpp 0492b979e4657a489ca3428e6f8022ef20cb05f5 > src/slave/http.cpp aa9cdc5f58f73323958a6872e2721c83317f354c > src/tests/api_tests.cpp f343991a5d23ac665429456471ac06a5315fc692 > > > Diff: https://reviews.apache.org/r/67693/diff/4/ > > > Testing > ------- > > `make check` > > The added tests fails without the corresponding changes to the master and > agent code. > > > Thanks, > > Benjamin Bannier > >
