> On Nov. 6, 2014, 6:30 a.m., Adam B wrote: > > src/messages/messages.proto, line 375 > > <https://reviews.apache.org/r/27494/diff/3/?file=750907#file750907line375> > > > > A little more clarity on this change: 'string' and 'bytes' are exactly > > the same on the wire, and both are represented as std::string in C++, so > > that shouldn't be a problem. > > https://www.mail-archive.com/protobuf@googlegroups.com/msg01478.html > > Java and Python represent UTF-8 strings ('string') differently from > > byte arrays ('bytes'), but the framework scheduler would send this message > > from the C++ libmesos authenticatee. > > Some pure language bindings may have to change the datatype used to > > support framework authentication after this change. This may need to be > > documented in upgrade notes.
Awesome comment. I have just proposed a new RR that intends to cover the documentation of this change; https://reviews.apache.org/r/27675/ > On Nov. 6, 2014, 6:30 a.m., Adam B wrote: > > src/slave/slave.cpp, lines 620-621 > > <https://reviews.apache.org/r/27494/diff/3/?file=750914#file750914line620> > > > > This feels a little awkward. Now, we try to authenticate if > > --authenticatee is set (to non-default) or --credential is set; otherwise > > register directly? > > Maybe it's time to have an explicit --authenticate flag, so > > --credential isn't both a path to a credential file and a switch to enable > > authentication. Thoughts? > > > > Or maybe just check to see if there's a viable authenticatee, and if > > not, try to register without authenticating. The master/authenticator would > > reject an unauthenticated slave/framework if the master has enabled authn, > > so the authenticatee doesn't have to be so strict about exiting instead of > > registering without authentication. Entirely true as written Adam. I had the same feelings when implementing it this way but hesitated to revise the existing logic too much. My first hunch is to follow your second option - but let's see what others say about that. I will put up a comment on https://issues.apache.org/jira/browse/MESOS-2040 to open up a quick discussion, inviting everyone for input. - Till ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/27494/#review60125 ----------------------------------------------------------- On Nov. 6, 2014, 12:19 a.m., Till Toenshoff wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/27494/ > ----------------------------------------------------------- > > (Updated Nov. 6, 2014, 12:19 a.m.) > > > Review request for mesos, Adam B and Vinod Kone. > > > Bugs: MESOS-2001 > https://issues.apache.org/jira/browse/MESOS-2001 > > > Repository: mesos-git > > > Description > ------- > > Also fixes messages.proto to use a raw bytestream instead of a string for > AuthenticationStartMessage as non CRAM-MD5 authentication may transmit binary > data. > > Note that this patch covers modularized slave authentication only. Framework > authentication currently is currently covered by the default (built-in) > implementation. There will be a subsequent patch for modularized framework > authentication. > > > Diffs > ----- > > src/messages/messages.proto de0e2a2 > src/sched/sched.cpp e5f828d > src/scheduler/scheduler.cpp c74187c > src/slave/constants.hpp 701dd89 > src/slave/constants.cpp d6ad78c > src/slave/flags.hpp efbd35d > src/slave/slave.hpp 5b082fc > src/slave/slave.cpp dbfd1a8 > > Diff: https://reviews.apache.org/r/27494/diff/ > > > Testing > ------- > > make check > > NOTE all three CRAM-MD5 authenticatee module related RRs need to get applied > before running make check. > > > Thanks, > > Till Toenshoff > >
