> On Sept. 16, 2015, 12:53 a.m., Joseph Wu wrote: > > include/mesos/authorizer/authorizer.proto, lines 78-79 > > <https://reviews.apache.org/r/38399/diff/1/?file=1073651#file1073651line78> > > > > Consider renaming to `machine_ids`. > > > > You should also consider a string representation of the MachineID > > protobuf. > > (https://github.com/apache/mesos/blob/master/include/mesos/v1/mesos.proto#L164-L167) > > > > Both fields are important for identifying a machine. The hostname is > > not enough.
Thank you. I think you mean something like this: machine_id = hostname + ':' + ip, it's a little complex for user to set the MaintenanceMachine ACL, and in most datacenters the machine hostnames usually different. > On Sept. 16, 2015, 12:53 a.m., Joseph Wu wrote: > > include/mesos/authorizer/authorizer.proto, line 74 > > <https://reviews.apache.org/r/38399/diff/1/?file=1073651#file1073651line74> > > > > I think you should reconsider which ACL goes where. There are 4 > > maintenance endpoints (currently): > > > > * `/maintenance/schedule` schedules machines. This probably belongs in > > an ACL on it's own. (Like this new one.) > > * `/machine/down` and `/machine/up` bring machines up and down. I > > think this falls either in it's own ACL or together with > > `ShutdownFramework`. Also, it's important to note that these two actions > > are *not* maintenance specific. (Implementation-wise, they are restricted > > to maintenance for now though.) > > * `/maintenance/status` is read-only. So it might not need to be > > authenticated. Thank you for your comments, I am working on next patchset and will upload it soon. - Zhiwei ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/38399/#review99042 ----------------------------------------------------------- On Sept. 16, 2015, 9:52 a.m., Zhiwei Chen wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/38399/ > ----------------------------------------------------------- > > (Updated Sept. 16, 2015, 9:52 a.m.) > > > Review request for mesos, Artem Harutyunyan, Joris Van Remoortere, and Joseph > Wu. > > > Bugs: mesos-2222 > https://issues.apache.org/jira/browse/mesos-2222 > > > Repository: mesos > > > Description > ------- > > Add ACLs for the maintenance HTTP endpoints > > > Diffs > ----- > > include/mesos/authorizer/authorizer.hpp > d667a52f90f970a313580446a5a006cec4b5e25b > include/mesos/authorizer/authorizer.proto > 86bbb45f9d91b4098a262e3e50a793f3bb39497e > src/authorizer/local/authorizer.hpp > 32de102fd588f029882ef2222121ca83a7410c65 > src/authorizer/local/authorizer.cpp > 6d7da87731a438c2180cf91003e09d4aa5a1c773 > src/master/flags.cpp 80879611fbcfd764c9fc8f60a31613a9c8fc2364 > > Diff: https://reviews.apache.org/r/38399/diff/ > > > Testing > ------- > > > Thanks, > > Zhiwei Chen > >
