> On Dec. 16, 2015, 2:35 p.m., Alexander Rukletsov wrote: > > include/mesos/authorizer/authorizer.proto, line 96 > > <https://reviews.apache.org/r/40167/diff/4/?file=1156029#file1156029line96> > > > > I see that you ensure this when you create requests in > > `authorizeCreateVolume`, but I'm not sure it's validated for ACLs. Do you > > think it makes sense to add validation in the > > `LocalAuthorizer::initialize()`? > > > > After a second thought, this note relates to the default > > implementation, because the master does not really validate it, right? > > Which means a 3rdparty authorizer may react to particular types.
You're correct that a 3rd party authorizer could handle values other than ANY and NONE, but the current default implementation enforces this constraint by always setting `volume_types` equal to ANY in the request ACL. I altered this comment to clarify that point a bit; feel free to re-open the issue if you think we should do more. - Greg ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/40167/#review110641 ----------------------------------------------------------- On Dec. 17, 2015, 6:58 a.m., Greg Mann wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/40167/ > ----------------------------------------------------------- > > (Updated Dec. 17, 2015, 6:58 a.m.) > > > Review request for mesos, Jie Yu, Michael Park, and Neil Conway. > > > Bugs: MESOS-4178 > https://issues.apache.org/jira/browse/MESOS-4178 > > > Repository: mesos > > > Description > ------- > > Added ACL protobuf messages 'CreateVolume' and 'DestroyVolume'. > > > Diffs > ----- > > include/mesos/authorizer/authorizer.proto > 74fcc86d3c92cb3aa27e45b647b1653705b3201c > > Diff: https://reviews.apache.org/r/40167/diff/ > > > Testing > ------- > > This is the second in a chain of 7 patches. `make check` was used to test > after all patches were applied. > > Note that this chain of patches touches many of the same files as another > chain beginning with Review #39985 and ending with Review #39989, which is > currently in review as well. To avoid conflicts, the beginning of this chain > begins on top of Review #39989. > > > Thanks, > > Greg Mann > >