> On Dec. 14, 2015, 4:52 p.m., Till Toenshoff wrote: > > src/tests/authorization_tests.cpp, lines 591-592 > > <https://reviews.apache.org/r/39520/diff/7/?file=1158400#file1158400line591> > > > > Why would we check a request with multiple principals in this > > integration test? > > Greg Mann wrote: > I would say this is closer to a unit test, since we're not testing > realistic attempts at authorization, but are rather just exercising the > authorization of ACLs without the extra bits for communication with a > framework or operator. While it's true that we don't have any current use > cases for multiple principals in a request, the ACLs do allow it, so perhaps > it's reasonable to test this? I don't have strong feelings either way; maybe > best to just split them into two requests to make the test closer to > real-world usage patterns? > > Till Toenshoff wrote: > Greg, you are right - it is meant to be a unit test. > > Right now my assumption is that our API allows multiple principals for > requests as a result of the implementation details -- streamlined re-use of > that `Entity` concept making that matrix check rather slick. I do not think > that this feature was/is a goal for that API. So I agree that testing this > option can be done in a dedicated unit-test even if it was only for > documenting this option. I also think repeating that test over and over for > all kinds of endpoints / ACLs should be avoided as it seems of little value > and even confusing to the reader that tries to see how this would ever happen > in reality.
The tests now only use a single principal in the request. > On Dec. 14, 2015, 4:52 p.m., Till Toenshoff wrote: > > src/tests/master_quota_tests.cpp, line 1014 > > <https://reviews.apache.org/r/39520/diff/7/?file=1158401#file1158401line1014> > > > > Can you please also add a test that handles a missing principle in the > > request? The new test fixture `AuthorizedQuotaSetRequestWithoutPrincipal` now does that. - Jan ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/39520/#review110223 ----------------------------------------------------------- On Dec. 18, 2015, 10:12 a.m., Jan Schlicht wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/39520/ > ----------------------------------------------------------- > > (Updated Dec. 18, 2015, 10:12 a.m.) > > > Review request for mesos, Alexander Rukletsov and Till Toenshoff. > > > Bugs: MESOS-4082 > https://issues.apache.org/jira/browse/MESOS-4082 > > > Repository: mesos > > > Description > ------- > > Quota: Added authentication, authorization tests. > > > Diffs > ----- > > src/tests/authorization_tests.cpp dcf348acc75b143e3a854bde17e6042dc90599d5 > src/tests/master_quota_tests.cpp 0473869783a714766ed26fff61d7f8c56342df74 > > Diff: https://reviews.apache.org/r/39520/diff/ > > > Testing > ------- > > make check > > > Thanks, > > Jan Schlicht > >
