> On Dec. 8, 2015, 3:07 p.m., Alexander Rukletsov wrote: > > src/master/master.cpp, lines 475-478 > > <https://reviews.apache.org/r/39043/diff/16/?file=1155921#file1155921line475> > > > > I'm slightly confused. Now we have two different authenticators: one > > for frameworks and one for endpoints? And they are both called > > `Authenticator`? Am I missing something?
Well, the old authenticator which is not even secure, will be deprecated, but we cannot just delete it, can we? moreover, the new authenticator exists at libprocess layer of abstraction. In fact, the `Credentials` protobuf predates the use of `Master::Http::authenticate()` > On Dec. 8, 2015, 3:07 p.m., Alexander Rukletsov wrote: > > src/master/constants.cpp, line 49 > > <https://reviews.apache.org/r/39043/diff/16/?file=1155916#file1155916line49> > > > > I though it should be capitlized, according to > > [RFC2617](https://tools.ietf.org/html/rfc2617). There was a good reason to do so, but right now I don't remember why. > On Dec. 8, 2015, 3:07 p.m., Alexander Rukletsov wrote: > > src/master/http.cpp, lines 2276-2282 > > <https://reviews.apache.org/r/39043/diff/16/?file=1155919#file1155919line2276> > > > > Looks like the scope of the `Credential` protobuf is reduced. It looks > > like now it's only used to load credentials on startup and for framework > > authentication. Do we need to store `credentials` in the master then? I isn't reduced, It hasn't change at all. And as far as I'm concerned, as long as framework/slave authentication uses the procedure we have, it needs to stay there. In fact the method `authenticate` isn't very old at all. I think the first implementation of these patches actually predates the inclusion of that method. > On Dec. 8, 2015, 3:07 p.m., Alexander Rukletsov wrote: > > src/master/master.cpp, lines 519-551 > > <https://reviews.apache.org/r/39043/diff/16/?file=1155921#file1155921line519> > > > > For allocator modules, we created a factory method in `allocator.cpp` > > in order to be more concise and keep master code small and clean. You are right, but the rationale behind it is that the other authenticator's initialization code is already here, so one can notice the difference between them both while having them here. Once this lands I will open a ticket to move them both to their own factories. > On Dec. 8, 2015, 3:07 p.m., Alexander Rukletsov wrote: > > src/master/master.cpp, line 501 > > <https://reviews.apache.org/r/39043/diff/16/?file=1155921#file1155921line501> > > > > We agreed to use backticks instead of single quotes. Here is when you realized how long has this code waited, that it actually predates that rule. - Alexander ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/39043/#review109318 ----------------------------------------------------------- On Dec. 8, 2015, 11:24 a.m., Alexander Rojas wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/39043/ > ----------------------------------------------------------- > > (Updated Dec. 8, 2015, 11:24 a.m.) > > > Review request for mesos, Adam B, Bernd Mathiske, Isabel Jimenez, and Till > Toenshoff. > > > Bugs: MESOS-3756 > https://issues.apache.org/jira/browse/MESOS-3756 > > > Repository: mesos > > > Description > ------- > > 1. Adds a flag to load an HTTP Authenticator module from the flags. > 2. If provided, uses the credentials file to initialize the default HTTP > Authenticator. > 3. Updates the existing endpoints which implement their own basic HTTP > authenticator with the libprocess one. > 4. Updates one test which expected the wrong results since now credentials > are checked before the body of the request. > > > Diffs > ----- > > src/master/constants.hpp cc38dfcf57ecbc8555379084acbf2bfa4b3fc759 > src/master/constants.cpp 98ea7c8f1e7c3057cfe941f82f1b71f7731a8f32 > src/master/flags.hpp 9af6c68eef6bcf39d5776809fab6c66dc95da6b2 > src/master/flags.cpp f9223edb33483cb5619e7dd75221f735306462c6 > src/master/http.cpp 9097eda47558fb5306af5a61b464937d7ab34e83 > src/master/master.hpp 4683fa542a740f9a0b80fff7fbe0e63ec66266f2 > src/master/master.cpp 953fa4f14929581b226a7e27d30aea7a5aa1fd7c > > Diff: https://reviews.apache.org/r/39043/diff/ > > > Testing > ------- > > make check > > > Thanks, > > Alexander Rojas > >
