----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/68104/#review206637 -----------------------------------------------------------
src/slave/http.cpp Lines 1818 (patched) <https://reviews.apache.org/r/68104/#comment289695> Conventionally we only indent this by 4 extra spaces aligning with "return". src/slave/http.cpp Lines 1821-1822 (patched) <https://reviews.apache.org/r/68104/#comment289696> The following can be fit into 80 characters: ``` [this, acceptType](const Owned<ObjectApprover>& approvers) -> Response { ``` Or for here it seems harmless to just use `[=]`. I'm fine with either though. src/slave/http.cpp Lines 1824 (patched) <https://reviews.apache.org/r/68104/#comment289698> Should we return an empty list of resource provider infos or return a 403 Forbidden? src/tests/api_tests.cpp Lines 7002-7004 (original), 7002-7021 (patched) <https://reviews.apache.org/r/68104/#comment289697> How about the following: ``` slave::Flags slaveFlags = CreateSlaveFlags(); slaveFlags.authenticate_http_readwrite = true; { // `DEFAULT_CREDENTIAL_2` is not allowed to view any resource provider. mesos::ACL::ViewResourceProvider* acl = slaveFlags.acls->add_view_resource_providers(); acl->mutable_principals()->add_values(DEFAULT_CREDENTIAL_2.principal()); acl->mutable_resource_providers()->set_type(mesos::ACL::Entity::NONE); { Try<Owned<cluster::Slave>> slave = StartSlave(&detector, slaveFlags); ``` src/tests/api_tests.cpp Lines 7046-7049 (patched) <https://reviews.apache.org/r/68104/#comment289699> I'm a bit against checking content of the failure string since it's slightly hard to maintain. Can we avoid this? - Chun-Hung Hsiao On July 30, 2018, 8:57 a.m., Benjamin Bannier wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/68104/ > ----------------------------------------------------------- > > (Updated July 30, 2018, 8:57 a.m.) > > > Review request for mesos, Chun-Hung Hsiao and Jan Schlicht. > > > Bugs: MESOS-8314 > https://issues.apache.org/jira/browse/MESOS-8314 > > > Repository: mesos > > > Description > ------- > > Implemented authorization for agent `GET_RESOURCE_PROVIDER` calls. > > > Diffs > ----- > > src/slave/http.cpp ab5864d9fd2fde478ed7da2ca7ed8abedc72c7c5 > src/tests/api_tests.cpp 182622a62d350ebefc891a385de3f2d35a7c0243 > > > Diff: https://reviews.apache.org/r/68104/diff/1/ > > > Testing > ------- > > `make check` > > > Thanks, > > Benjamin Bannier > >
