> On Feb. 20, 2015, 10:19 p.m., Vinod Kone wrote: > > src/authentication/cram_md5/authenticator.cpp, line 449 > > <https://reviews.apache.org/r/27760/diff/17/?file=869278#file869278line449> > > > > Shouldn't this be protected by once() to avoid 2 different threads > > loading secrets at the same time? > > Till Toenshoff wrote: > That would render our tests broken (those that reset the credentials). > The concurrency is covered by a Lock within that SASL aux plugin code.
Let me know if this is ok, please. > On Feb. 20, 2015, 10:19 p.m., Vinod Kone wrote: > > src/master/master.cpp, lines 466-467 > > <https://reviews.apache.org/r/27760/diff/17/?file=869282#file869282line466> > > > > why this if condition for logging? > > Till Toenshoff wrote: > Not sure I understand. Did you see the comment directly above those lines? > ``` > // A failure to initialize the authenticator does lead to > // unusable authentication but still allows non authenticating > // frameworks and slaves to connect. > ``` > > Adam B wrote: > I think (a few revisions back), the thought was that the default > parameters for a master are authenticate_frameworks/slaves=false && > credentials=none && authenticator=default(cram), at which point it seems > unnecessary to warn somebody that we didn't load the authenticator. Upon > returning to this decision, I see no reason not to log this message anytime > authentication is disabled, even if it's the default setting. Maybe it would > seem less harsh as an INFO in the default case, but a single WARN message on > master startup could be a gentle nudge to try out authentication. Let's try to get this sorted out as well. Any actions needed? - Till ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/27760/#review73310 ----------------------------------------------------------- On Feb. 22, 2015, 10:03 p.m., Till Toenshoff wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/27760/ > ----------------------------------------------------------- > > (Updated Feb. 22, 2015, 10:03 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 > > > 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 a466f92 > 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 > >
