> On Jan. 28, 2015, 12:33 p.m., Vinod Kone wrote: > > Can you expand description on why you designed the authenticator inteface > > this way? Particularly, expand on "The initial design and implementation of > > the authenticator module interface caused issues and was not optimal for > > heavy lifting setup of external dependencies" and "The new design also gets > > us back on track to the goal of makeing SASL a soft dependency of mesos". > > > > An issue with the new design is that now users have to implement an > > Authenticator and an AuthenticatorSession to implement a new type of > > authenticator. Also, we lost the symmetry between authenticatee and > > authenticator. An Authenticatee now communicates with AuthenticatorSession > > instead of Authenticator, which is confusing. Is the plan to refactor the > > Authenticatee similarly?
We need to load/initialize the auxprop plugin at most once per linux process, so we need the authenticator to live for the lifetime of the master, but we need the sasl connection to be created/disposed with the lifetime of a per-authenticatee auth session. We could probably get away with hiding the Session variable underneath the Authenticator interface, and just call Authenticator::authenticate(pid) which will spin up a new session/connection and Authenticator::cancel/complete(pid) which erases/disposes the session/connection. The sasl-based authenticators will then manage the pid->session map internally, and other non-sasl authenticators don't have to know about that implementation detail. (I think I mentioned this before, but Till will have to remind me why we decided against it.) > On Jan. 28, 2015, 12:33 p.m., Vinod Kone wrote: > > src/master/master.cpp, lines 4173-4190 > > <https://reviews.apache.org/r/27760/diff/12/?file=834181#file834181line4173> > > > > Hmm. I think this is a serious enough error that the master should fail > > during initilization rather than sending an error message here. Do you have > > use cases in mind that warranted this? The default authenticator is CRAM-MD5 rather than none, and that authenticator fails to initialize if no credentials are provided (or if auxprops cannot be loaded). Since the default parameters specify CRAM-MD5 authenticator, no required authentication, and no credentials, we must support this starting successfully, although we do log a warning ("Only non-authenticating frameworks and slaves are allowed to connect. Authentication is disabled: error_message"). In this case, we must allow non-authenticating frameworks/slaves to register without authentication, but we will return an AuthenticationError if they actually try to authenticate. This is the same behavior as before. For scenarios where the authenticator fails to load, but authentication is explicitly enabled for frameworks/slaves, we do exit the master during initialization "Cannot start master with authentication enabled: error_message") > On Jan. 28, 2015, 12:33 p.m., Vinod Kone wrote: > > src/master/master.cpp, lines 4193-4203 > > <https://reviews.apache.org/r/27760/diff/12/?file=834181#file834181line4193> > > > > IIRC, sending an AuthenticationErrorMessage here causes the driver to > > simply retry the authentication and thus fall into a retry loop? Truth. There is a TODO about at least adding a backoff, but maybe we should also attempt registering without authentication? Maybe authentication shouldn't even cause retries under certain scenarios where it will never eventually succeed (bad credentials, no authenticator). - Adam ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/27760/#review70051 ----------------------------------------------------------- On Jan. 26, 2015, 12:49 a.m., Till Toenshoff wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/27760/ > ----------------------------------------------------------- > > (Updated Jan. 26, 2015, 12:49 a.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/authentication/cram_md5/auxprop.hpp b894386 > src/authentication/cram_md5/auxprop.cpp cf503a2 > src/master/master.hpp 1d342e5 > src/master/master.cpp bda8fda > src/tests/cram_md5_authentication_tests.cpp a356aa1 > > Diff: https://reviews.apache.org/r/27760/diff/ > > > Testing > ------- > > make check > > > Thanks, > > Till Toenshoff > >
