----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36927/#review93532 -----------------------------------------------------------
src/master/master.cpp (lines 1574 - 1576) <https://reviews.apache.org/r/36927/#comment147918> so this line will be presented twice if principal is not set (which will be for most frameworks) because this function is called twice. that is kinda unfortunate and likely confusing to users. not sure what we could do here yet. src/master/master.cpp (lines 1720 - 1742) <https://reviews.apache.org/r/36927/#comment147917> checking validationError.isNone() in each if statement looks a bit weird. how about doing these in an else if instead? if (frameworkInfo.has_id() && !(frameworkInfo.id() == "")) { } else if (!roles.contains(frameworkInfo.role()) { } else if (frameworkInfo.user() == "root" && !flags.root_submissions) { } else { } i think the above is easier to read? src/master/master.cpp (lines 1731 - 1732) <https://reviews.apache.org/r/36927/#comment147916> why did you use 2 if loops here instead of &&? anyway, see above comment to make it simpler. src/master/master.cpp (lines 1866 - 1882) <https://reviews.apache.org/r/36927/#comment147919> ditto. please use else if. - Vinod Kone On July 29, 2015, 11:52 p.m., Ben Mahler wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/36927/ > ----------------------------------------------------------- > > (Updated July 29, 2015, 11:52 p.m.) > > > Review request for mesos and Vinod Kone. > > > Repository: mesos > > > Description > ------- > > Framework authorization is now done through `authorizeFramework` which is > consistent with `authorizeTask`. > Also, `validateFrameworkAuthentication` captures the validation for > authentication. > > I also made registration and re-registration consistent: > > * Both perform the check for root submission, rather than just registration. > * Authentication checks in `_registerFramework` and `_reregisterFramework` > are now comprehensive (thanks to `validateFrameworkAuthentication`). > > > Diffs > ----- > > src/master/master.hpp 1135049d34f8ed4d56c795ad28c9ca1d07237b63 > src/master/master.cpp 2bfec2f69375444925252480142ee409b8474761 > > Diff: https://reviews.apache.org/r/36927/diff/ > > > Testing > ------- > > make check > > > Thanks, > > Ben Mahler > >
