> On Dec. 18, 2014, 2:43 a.m., Adam B wrote: > > src/authentication/authenticator.hpp, line 59 > > <https://reviews.apache.org/r/27760/diff/7/?file=766647#file766647line59> > > > > Do we really want to reference the 'master' here? Authenticator could > > conceivably be used by anything that wants to authenticate an > > authenticatee. Perhaps slave will want to authenticate its executors at > > some point. > > s/by the master// > > Same applies to other 'master' references in this file.
Very good point indeed. > On Dec. 18, 2014, 2:43 a.m., Adam B wrote: > > src/authentication/cram_md5/authenticator.hpp, line 494 > > <https://reviews.apache.org/r/27760/diff/7/?file=766648#file766648line494> > > > > What makes these "registration" credentials? Couldn't these > > authenticators be used for anything requiring authentication? > > Maybe just remove the comment. That comment I inherited from master.cpp where it was previously located. Anyway, I think you are right :) > On Dec. 18, 2014, 2:43 a.m., Adam B wrote: > > src/authentication/cram_md5/authenticator.hpp, line 495 > > <https://reviews.apache.org/r/27760/diff/7/?file=766648#file766648line495> > > > > Does this call have a return status we should be checking? Nope, never did. > On Dec. 18, 2014, 2:43 a.m., Adam B wrote: > > src/master/master.cpp, line 432 > > <https://reviews.apache.org/r/27760/diff/7/?file=766650#file766650line432> > > > > Maybe preface with "Cannot start master with authentication enabled: " Nice, yes that makes sense. > On Dec. 18, 2014, 2:43 a.m., Adam B wrote: > > src/master/master.cpp, line 3959 > > <https://reviews.apache.org/r/27760/diff/7/?file=766650#file766650line3959> > > > > !!! send(from, message); ? Whooops, yes - thank you! > On Dec. 18, 2014, 2:43 a.m., Adam B wrote: > > src/master/master.cpp, line 3980 > > <https://reviews.apache.org/r/27760/diff/7/?file=766650#file766650line3980> > > > > I wonder if authenticatorSessions should put/erase `from` instead of > > `pid`, since the session is actually with the authenticatee (`from`). What > > would be the effect of such a change? Good idea and I see the point but it would take more changes as we dont always have the from available later when needing to index that session (e.g. in _authenticate) - Till ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/27760/#review65425 ----------------------------------------------------------- On Dec. 19, 2014, 6:22 p.m., Till Toenshoff wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/27760/ > ----------------------------------------------------------- > > (Updated Dec. 19, 2014, 6:22 p.m.) > > > Review request for mesos, Adam B, Kapil Arya, Niklas Nielsen, and Vinod Kone. > > > Bugs: MESOS-2050 > https://issues.apache.org/jira/browse/MESOS-2050 > > > Repository: mesos-git > > > Description > ------- > > The initial design and implementation of the authenticator module interface > caused issues and was not optimal for heavy lifting setup of external > dependencies. By introducing a two fold design, this has been decoupled from > the authentication message processing. The new design also gets us back on > track to the goal of makeing SASL a soft dependency of mesos. > > > Diffs > ----- > > src/authentication/authenticator.hpp 460494a > src/authentication/cram_md5/authenticator.hpp d739a02 > src/master/master.hpp 26116af > src/master/master.cpp 0c98b51 > src/tests/cram_md5_authentication_tests.cpp a356aa1 > > Diff: https://reviews.apache.org/r/27760/diff/ > > > Testing > ------- > > make check > > > Thanks, > > Till Toenshoff > >
