> On Jan. 28, 2015, 8: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? > > Adam B wrote: > 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). > > Till Toenshoff wrote: > Can we make this a TODO at this spot as well and create a JIRA? I'ld also > be happy to work on a follow up patch, covering more finegrained > authentication result-triggers. > > Adam B wrote: > We should create a JIRA for the authentication backoff TODO, and perhaps > another JIRA for retrying without authentication. > I would like to point out that the behavior after this patch matches the > AuthenticationErrorMessage behavior all the way back to 0.19 or earlier, so > we can certainly leave the TODOs alone for this release and just create JIRAs > for proper tracking in the future.
sgtm. please resolve this once you add the TODO. > On Jan. 28, 2015, 8: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? > > Adam B wrote: > 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") > > Till Toenshoff wrote: > Thanks Adam for your explanations. Vinod, please let me know if this part > was matching your expectations. yea. that makes sense. can you add comments in the code mentioning the same, for future readers? you can resolve this then. - Vinod ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/27760/#review70051 ----------------------------------------------------------- On Feb. 18, 2015, 3:57 a.m., Till Toenshoff wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/27760/ > ----------------------------------------------------------- > > (Updated Feb. 18, 2015, 3:57 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 > > > 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 > ----- > > include/mesos/authentication/authenticator.hpp f66217a > src/Makefile.am d372404 > src/authentication/cram_md5/authenticator.hpp 7578ea1 > src/authentication/cram_md5/authenticator.cpp PRE-CREATION > src/authentication/cram_md5/auxprop.hpp d036b11 > src/authentication/cram_md5/auxprop.cpp 5ff9755 > src/master/master.hpp 6a39df0 > src/master/master.cpp f10a3cf > src/tests/cram_md5_authentication_tests.cpp dd102dc > > Diff: https://reviews.apache.org/r/27760/diff/ > > > Testing > ------- > > make check > > > Thanks, > > Till Toenshoff > >
