You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@mesos.apache.org by Alexander Rojas <al...@mesosphere.io> on 2015/12/07 14:33:26 UTC
Re: Review Request 39043: Added support for HTTP Authentication in
Mesos.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/39043/
-----------------------------------------------------------
(Updated Dec. 7, 2015, 2:33 p.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 (updated)
-----
src/master/constants.hpp 51d477444c4d1a2c9dd2f32164ebffbb1fd5c8c2
src/master/constants.cpp 2b66b27783f18930010ee912e9977ea1647eba09
src/master/flags.hpp 5fd5d502697b2edc22ae98a5a8e361bf62bf8bb6
src/master/flags.cpp 806e2da6ad37a6acf76818d4c6b3c462175fd09d
src/master/http.cpp 1c4f1406f5d917f5d655a7d61d311365f8999ce0
src/master/master.hpp 5e5a575dc7dd49324f3c837028df8a7f75cd1f80
src/master/master.cpp 39a0e730b230cee73e30d831ee67d9207359ae28
Diff: https://reviews.apache.org/r/39043/diff/
Testing
-------
make check
Thanks,
Alexander Rojas
Re: Review Request 39043: Added support for HTTP Authentication in
Mesos.
Posted by Mesos ReviewBot <re...@mesos.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/39043/#review109249
-----------------------------------------------------------
Bad patch!
Reviews applied: [40539, 37999, 38000, 38094, 38950]
Failed command: ./support/apply-review.sh -n -r 38950
Error:
2015-12-08 01:54:53 URL:https://reviews.apache.org/r/38950/diff/raw/ [19796/19796] -> "38950.patch" [1]
Successfully applied: Http Authenticators can be loaded as modules from mesos.
Adds support for modularization of HTTP Authenticators.
It includes an example of how to do it with the Basic HTTP Authenticator.
Review: https://reviews.apache.org/r/38950
src/examples/test_http_authenticator_module.cpp:1: A license header should appear on the file's first line starting with '// Licensed'.: /**
src/authentication/http/basic_authenticator_factory.cpp:1: A license header should appear on the file's first line starting with '// Licensed'.: /**
include/mesos/authentication/http/basic_authenticator_factory.hpp:1: A license header should appear on the file's first line starting with '// Licensed'.: /**
include/mesos/module/http_authenticator.hpp:1: A license header should appear on the file's first line starting with '// Licensed'.: /**
src/tests/http_authentication_tests.cpp:1: A license header should appear on the file's first line starting with '// Licensed'.: /**
Total errors found: 5
Checking 10 files
Failed to commit patch
- Mesos ReviewBot
On Dec. 7, 2015, 2:22 p.m., Alexander Rojas wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39043/
> -----------------------------------------------------------
>
> (Updated Dec. 7, 2015, 2:22 p.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
>
>
Re: Review Request 39043: Added support for HTTP Authentication in
Mesos.
Posted by Alexander Rukletsov <ru...@gmail.com>.
> On Dec. 8, 2015, 2: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?
>
> Alexander Rojas wrote:
> 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.
It looks like `Optional<Credentials> credentials` has been introduced in https://github.com/apache/mesos/commit/bb8375975e92ee722befb478ddc3b2541d1ccaa9 and factored out in https://github.com/apache/mesos/commit/a5cc9b435aad080a79230f0366a6ce77116c95a4. I think we do not need `master->credentials` any longer.
- Alexander
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/39043/#review109318
-----------------------------------------------------------
On Dec. 8, 2015, 2:38 p.m., Alexander Rojas wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39043/
> -----------------------------------------------------------
>
> (Updated Dec. 8, 2015, 2:38 p.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
>
>
Re: Review Request 39043: Added support for HTTP Authentication in
Mesos.
Posted by Alexander Rojas <al...@mesosphere.io>.
> 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
>
>
Re: Review Request 39043: Added support for HTTP Authentication in
Mesos.
Posted by Alexander Rukletsov <ru...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/39043/#review109318
-----------------------------------------------------------
src/master/constants.cpp (line 49)
<https://reviews.apache.org/r/39043/#comment168779>
I though it should be capitlized, according to [RFC2617](https://tools.ietf.org/html/rfc2617).
src/master/http.cpp
<https://reviews.apache.org/r/39043/#comment168789>
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?
src/master/master.cpp (lines 29 - 30)
<https://reviews.apache.org/r/39043/#comment168782>
Blank line, please
src/master/master.cpp (lines 121 - 122)
<https://reviews.apache.org/r/39043/#comment168783>
Blank line, please
src/master/master.cpp (lines 475 - 478)
<https://reviews.apache.org/r/39043/#comment168788>
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?
src/master/master.cpp (line 501)
<https://reviews.apache.org/r/39043/#comment168785>
We agreed to use backticks instead of single quotes.
src/master/master.cpp (line 503)
<https://reviews.apache.org/r/39043/#comment168784>
No periods at the end of the log statements. Here and below.
src/master/master.cpp (lines 510 - 511)
<https://reviews.apache.org/r/39043/#comment168787>
We usually insert a blank line if a previous statement spans multiple lines. Here and below.
src/master/master.cpp (lines 519 - 551)
<https://reviews.apache.org/r/39043/#comment168786>
For allocator modules, we created a factory method in `allocator.cpp` in order to be more concise and keep master code small and clean.
- Alexander Rukletsov
On Dec. 8, 2015, 10: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, 10: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
>
>
Re: Review Request 39043: Added support for HTTP Authentication in
Mesos.
Posted by Mesos ReviewBot <re...@mesos.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/39043/#review109799
-----------------------------------------------------------
Patch looks great!
Reviews applied: [40539, 37999, 38000, 38094, 38950, 39043]
Passed command: export OS=ubuntu:14.04;export CONFIGURATION="--verbose";export COMPILER=gcc; ./support/docker_build.sh
- Mesos ReviewBot
On Dec. 10, 2015, 2:34 p.m., Alexander Rojas wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39043/
> -----------------------------------------------------------
>
> (Updated Dec. 10, 2015, 2:34 p.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 5d33138d60e684b23f07e1781de7991209d3e161
> src/master/master.hpp 4683fa542a740f9a0b80fff7fbe0e63ec66266f2
> src/master/master.cpp 7acdc0a5d23a606eac2f37f4b7dd021c5a4fceb7
>
> Diff: https://reviews.apache.org/r/39043/diff/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Alexander Rojas
>
>
Re: Review Request 39043: Added support for HTTP Authentication in
Mesos.
Posted by Alexander Rojas <al...@mesosphere.io>.
> On Dec. 11, 2015, 2:41 p.m., Bernd Mathiske wrote:
> > src/master/master.cpp, line 509
> > <https://reviews.apache.org/r/39043/diff/19/?file=1158817#file1158817line509>
> >
> > Why do we have a vector then?
> > Why do we have plural variable names then?
> > I suspect this is an MVP decision.
> > Then it needs a TODO hinting
> > that plural will make sense later.
> >
> > Personally, I would find it cleaner to use singular and change it to plural (and vector) only at the time when the post-MVP actually gets worked upon.
It is the same pattern used for non http authenticators and authorizers modules. Where we support only one but the infrastructure is there for the rest.
- Alexander
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/39043/#review109955
-----------------------------------------------------------
On Dec. 11, 2015, 3:06 p.m., Alexander Rojas wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39043/
> -----------------------------------------------------------
>
> (Updated Dec. 11, 2015, 3:06 p.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 37fbcb93074fe189133161afaa28046dc2ad0731
> src/master/master.hpp 9aa548aa6e159046c94e4ec96f631ea8b3bfd5d8
> src/master/master.cpp 0d1482279c68f2a4a27dabaf28774769a5d515c4
> src/master/quota_handler.cpp 11167879b2480d9c8dd6398ca39c479089ec2272
>
> Diff: https://reviews.apache.org/r/39043/diff/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Alexander Rojas
>
>
Re: Review Request 39043: Added support for HTTP Authentication in
Mesos.
Posted by Bernd Mathiske <be...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/39043/#review109955
-----------------------------------------------------------
src/master/flags.cpp (line 438)
<https://reviews.apache.org/r/39043/#comment169694>
s/doing/handling
src/master/master.cpp (line 119)
<https://reviews.apache.org/r/39043/#comment169702>
auth is ambiguousm could mean authn or authz. So this should be authn. But we do not really need to abbreviate here. The code below that uses this is more readable if we don't IMHO. And there aren't many uses.
src/master/master.cpp (line 504)
<https://reviews.apache.org/r/39043/#comment169697>
Grammar-wise, there is something missing here, maybe:
s/is/this is
src/master/master.cpp (line 509)
<https://reviews.apache.org/r/39043/#comment169699>
Why do we have a vector then?
Why do we have plural variable names then?
I suspect this is an MVP decision.
Then it needs a TODO hinting
that plural will make sense later.
Personally, I would find it cleaner to use singular and change it to plural (and vector) only at the time when the post-MVP actually gets worked upon.
- Bernd Mathiske
On Dec. 10, 2015, 6:34 a.m., Alexander Rojas wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39043/
> -----------------------------------------------------------
>
> (Updated Dec. 10, 2015, 6:34 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 5d33138d60e684b23f07e1781de7991209d3e161
> src/master/master.hpp 4683fa542a740f9a0b80fff7fbe0e63ec66266f2
> src/master/master.cpp 7acdc0a5d23a606eac2f37f4b7dd021c5a4fceb7
>
> Diff: https://reviews.apache.org/r/39043/diff/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Alexander Rojas
>
>
Re: Review Request 39043: Added support for HTTP Authentication in
Mesos.
Posted by Alexander Rojas <al...@mesosphere.io>.
> On Jan. 5, 2016, 1:57 p.m., Bernd Mathiske wrote:
> > src/master/master.cpp, line 528
> > <https://reviews.apache.org/r/39043/diff/22/?file=1181826#file1181826line528>
> >
> > Isn't there a version of BasicAuthenticatorFactory::create() that takes credentials instead of parameters? Then we can omit the packaging up here.
There wasn't but now there is.
- Alexander
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/39043/#review112830
-----------------------------------------------------------
On Jan. 5, 2016, 12:18 p.m., Alexander Rojas wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39043/
> -----------------------------------------------------------
>
> (Updated Jan. 5, 2016, 12:18 p.m.)
>
>
> Review request for mesos, Adam B, Bernd Mathiske, Isabel Jimenez, Jan Schlicht, 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 f1624e1ed5091f08f57d4382b344cab8b05ba840
> src/master/constants.cpp 589f2ee55a24d7e8b9352a4c8f94a3785fd1bef4
> src/master/flags.hpp 9af6c68eef6bcf39d5776809fab6c66dc95da6b2
> src/master/flags.cpp 800e54a302386f26f9b2f21edacad8840abbb42d
> src/master/http.cpp deb0c8f9852dc0eec1e8c0ff35c684f982e8b110
> src/master/master.hpp 1cc5531de70bdc0bdce9835c4930dc774406fac9
> src/master/master.cpp 40ce3e17fca88da689128bcf5d35fdddc396c011
> src/master/quota_handler.cpp 1d84ef5767b3cb7cf870dd184aa2d9f030919498
> src/tests/mesos.cpp f4b0f82449c4b6a2b4b7b7f14518714485d5a13a
>
> Diff: https://reviews.apache.org/r/39043/diff/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Alexander Rojas
>
>
Re: Review Request 39043: Added support for HTTP Authentication in
Mesos.
Posted by Bernd Mathiske <be...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/39043/#review112830
-----------------------------------------------------------
src/master/http.cpp (line 1661)
<https://reviews.apache.org/r/39043/#comment173316>
I suggest this deleting this rather redundant and slightly misplaced comment. Here, the code readability improves in its absense, IMHO.
src/master/master.cpp (line 503)
<https://reviews.apache.org/r/39043/#comment173321>
Please correct the spelling mistakes in this comment.
src/master/master.cpp (line 528)
<https://reviews.apache.org/r/39043/#comment173328>
Isn't there a version of BasicAuthenticatorFactory::create() that takes credentials instead of parameters? Then we can omit the packaging up here.
src/master/master.cpp (line 561)
<https://reviews.apache.org/r/39043/#comment173326>
This somment is rather cryptic. Ownership of what? Why? How?
src/tests/mesos.cpp (line 430)
<https://reviews.apache.org/r/39043/#comment173314>
Singular, plural? "s" behind "lifetime", not "Authenticator"?
What mesos process? "the Mesos master process"?
src/tests/mesos.cpp (line 431)
<https://reviews.apache.org/r/39043/#comment173315>
s/c/C
- Bernd Mathiske
On Jan. 5, 2016, 3:18 a.m., Alexander Rojas wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39043/
> -----------------------------------------------------------
>
> (Updated Jan. 5, 2016, 3:18 a.m.)
>
>
> Review request for mesos, Adam B, Bernd Mathiske, Isabel Jimenez, Jan Schlicht, 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 f1624e1ed5091f08f57d4382b344cab8b05ba840
> src/master/constants.cpp 589f2ee55a24d7e8b9352a4c8f94a3785fd1bef4
> src/master/flags.hpp 9af6c68eef6bcf39d5776809fab6c66dc95da6b2
> src/master/flags.cpp 800e54a302386f26f9b2f21edacad8840abbb42d
> src/master/http.cpp deb0c8f9852dc0eec1e8c0ff35c684f982e8b110
> src/master/master.hpp 1cc5531de70bdc0bdce9835c4930dc774406fac9
> src/master/master.cpp 40ce3e17fca88da689128bcf5d35fdddc396c011
> src/master/quota_handler.cpp 1d84ef5767b3cb7cf870dd184aa2d9f030919498
> src/tests/mesos.cpp f4b0f82449c4b6a2b4b7b7f14518714485d5a13a
>
> Diff: https://reviews.apache.org/r/39043/diff/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Alexander Rojas
>
>
Re: Review Request 39043: Added support for HTTP Authentication in
Mesos.
Posted by Mesos ReviewBot <re...@mesos.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/39043/#review112860
-----------------------------------------------------------
Patch looks great!
Reviews applied: [40539, 37999, 38000, 38094, 38950, 39043]
Passed command: export OS=ubuntu:14.04;export CONFIGURATION="--verbose";export COMPILER=gcc; ./support/docker_build.sh
- Mesos ReviewBot
On Jan. 5, 2016, 11:18 a.m., Alexander Rojas wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39043/
> -----------------------------------------------------------
>
> (Updated Jan. 5, 2016, 11:18 a.m.)
>
>
> Review request for mesos, Adam B, Bernd Mathiske, Isabel Jimenez, Jan Schlicht, 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 f1624e1ed5091f08f57d4382b344cab8b05ba840
> src/master/constants.cpp 589f2ee55a24d7e8b9352a4c8f94a3785fd1bef4
> src/master/flags.hpp 9af6c68eef6bcf39d5776809fab6c66dc95da6b2
> src/master/flags.cpp 800e54a302386f26f9b2f21edacad8840abbb42d
> src/master/http.cpp deb0c8f9852dc0eec1e8c0ff35c684f982e8b110
> src/master/master.hpp 1cc5531de70bdc0bdce9835c4930dc774406fac9
> src/master/master.cpp 40ce3e17fca88da689128bcf5d35fdddc396c011
> src/master/quota_handler.cpp 1d84ef5767b3cb7cf870dd184aa2d9f030919498
> src/tests/mesos.cpp f4b0f82449c4b6a2b4b7b7f14518714485d5a13a
>
> Diff: https://reviews.apache.org/r/39043/diff/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Alexander Rojas
>
>
Re: Review Request 39043: Added support for HTTP Authentication in
Mesos.
Posted by Bernd Mathiske <be...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/39043/#review112877
-----------------------------------------------------------
src/master/master.cpp (line 505)
<https://reviews.apache.org/r/39043/#comment173351>
s/consider/considered
src/tests/mesos.cpp (line 430)
<https://reviews.apache.org/r/39043/#comment173354>
s/lifetime/lifetimes
- Bernd Mathiske
On Jan. 5, 2016, 8:20 a.m., Alexander Rojas wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39043/
> -----------------------------------------------------------
>
> (Updated Jan. 5, 2016, 8:20 a.m.)
>
>
> Review request for mesos, Adam B, Bernd Mathiske, Isabel Jimenez, Jan Schlicht, 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
> -----
>
> include/mesos/authentication/http/basic_authenticator_factory.hpp PRE-CREATION
> src/authentication/http/basic_authenticator_factory.cpp PRE-CREATION
> src/master/constants.hpp f1624e1ed5091f08f57d4382b344cab8b05ba840
> src/master/constants.cpp 589f2ee55a24d7e8b9352a4c8f94a3785fd1bef4
> src/master/flags.hpp 9af6c68eef6bcf39d5776809fab6c66dc95da6b2
> src/master/flags.cpp 800e54a302386f26f9b2f21edacad8840abbb42d
> src/master/http.cpp deb0c8f9852dc0eec1e8c0ff35c684f982e8b110
> src/master/master.hpp 1cc5531de70bdc0bdce9835c4930dc774406fac9
> src/master/master.cpp 40ce3e17fca88da689128bcf5d35fdddc396c011
> src/master/quota_handler.cpp 1d84ef5767b3cb7cf870dd184aa2d9f030919498
> src/tests/mesos.cpp f4b0f82449c4b6a2b4b7b7f14518714485d5a13a
>
> Diff: https://reviews.apache.org/r/39043/diff/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Alexander Rojas
>
>
Re: Review Request 39043: Added support for HTTP Authentication in
Mesos.
Posted by Jan Schlicht <ja...@mesosphere.io>.
> On Jan. 6, 2016, 3:45 p.m., Jan Schlicht wrote:
> > src/master/http.cpp, line 606
> > <https://reviews.apache.org/r/39043/diff/25/?file=1184416#file1184416line606>
> >
> > You're changing this to `Forbidden()` here and in `Master::Http::destroyVolumes`, which has to do with RFC-2616. If this is the right way to respond to a failed authorization, you should also implement it that way for the other code paths that deal with authorization. It might be better to leave it be for now and create a separate JIRA for that?
>
> Alexander Rojas wrote:
> I updated them all right now. Let's see what the author thinks.
`Master::Http::reserve`, `Master::Http::teardown` and `Master::Http::unreserve` use authorization and would also be affected. Also `Master::QuotaHandler::remove` will get an authorization soon-ish and might be affected after a rebase.
- Jan
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/39043/#review113065
-----------------------------------------------------------
On Jan. 6, 2016, 4:27 p.m., Alexander Rojas wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39043/
> -----------------------------------------------------------
>
> (Updated Jan. 6, 2016, 4:27 p.m.)
>
>
> Review request for mesos, Adam B, Bernd Mathiske, Isabel Jimenez, Jan Schlicht, 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
> -----
>
> include/mesos/authentication/http/basic_authenticator_factory.hpp PRE-CREATION
> src/authentication/http/basic_authenticator_factory.cpp PRE-CREATION
> src/master/constants.hpp f1624e1ed5091f08f57d4382b344cab8b05ba840
> src/master/constants.cpp 589f2ee55a24d7e8b9352a4c8f94a3785fd1bef4
> src/master/flags.hpp 9af6c68eef6bcf39d5776809fab6c66dc95da6b2
> src/master/flags.cpp 800e54a302386f26f9b2f21edacad8840abbb42d
> src/master/http.cpp d7afa2af989eebfc9039b3681f087ce570f601d5
> src/master/master.hpp 2936d32c5e7b5b944bf16a1f261c7c54179900ec
> src/master/master.cpp 40ce3e17fca88da689128bcf5d35fdddc396c011
> src/master/quota_handler.cpp ac4da74686f524a66745b4a4e038c3d703ea1ee9
> src/tests/master_quota_tests.cpp 81f0386d072d0176cb28eb229b274e424004f54c
> src/tests/mesos.cpp 082e57bc73fad02de77e16e4b34451e6c0903038
> src/tests/persistent_volume_endpoints_tests.cpp f0cce190abc90f0fae84d6c3db20e8215c2d8132
>
> Diff: https://reviews.apache.org/r/39043/diff/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Alexander Rojas
>
>
Re: Review Request 39043: Added support for HTTP Authentication in
Mesos.
Posted by Alexander Rojas <al...@mesosphere.io>.
> On Jan. 6, 2016, 3:45 p.m., Jan Schlicht wrote:
> > src/master/constants.hpp, line 137
> > <https://reviews.apache.org/r/39043/diff/25/?file=1184412#file1184412line137>
> >
> > s/mesos/Mesos
The string there is the actual value of the string, which is "mesos". That is the pattern in this file. I'll drop it now, but if you feel like it should be otherwise, please reopen.
> On Jan. 6, 2016, 3:45 p.m., Jan Schlicht wrote:
> > src/master/http.cpp, line 606
> > <https://reviews.apache.org/r/39043/diff/25/?file=1184416#file1184416line606>
> >
> > You're changing this to `Forbidden()` here and in `Master::Http::destroyVolumes`, which has to do with RFC-2616. If this is the right way to respond to a failed authorization, you should also implement it that way for the other code paths that deal with authorization. It might be better to leave it be for now and create a separate JIRA for that?
I updated them all right now. Let's see what the author thinks.
> On Jan. 6, 2016, 3:45 p.m., Jan Schlicht wrote:
> > src/master/quota_handler.cpp, line 50
> > <https://reviews.apache.org/r/39043/diff/25/?file=1184419#file1184419line50>
> >
> > `Forbidden` isn't used in this file.
It is now.
> On Jan. 6, 2016, 3:45 p.m., Jan Schlicht wrote:
> > src/master/quota_handler.cpp, line 349
> > <https://reviews.apache.org/r/39043/diff/25/?file=1184419#file1184419line349>
> >
> > See my comment re `Unauthorized` & `Forbidden` above. This would have to be changed to `Forbidden` to be consistent with the change in `Master::Http::createVolumes`.
I changed to `Forbidden`. Though if the authors insist, I will change it back. It is my opinion that only the authorizer should return `Unauthorized`.
- Alexander
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/39043/#review113065
-----------------------------------------------------------
On Jan. 6, 2016, 3:44 p.m., Alexander Rojas wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39043/
> -----------------------------------------------------------
>
> (Updated Jan. 6, 2016, 3:44 p.m.)
>
>
> Review request for mesos, Adam B, Bernd Mathiske, Isabel Jimenez, Jan Schlicht, 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
> -----
>
> include/mesos/authentication/http/basic_authenticator_factory.hpp PRE-CREATION
> src/authentication/http/basic_authenticator_factory.cpp PRE-CREATION
> src/master/constants.hpp f1624e1ed5091f08f57d4382b344cab8b05ba840
> src/master/constants.cpp 589f2ee55a24d7e8b9352a4c8f94a3785fd1bef4
> src/master/flags.hpp 9af6c68eef6bcf39d5776809fab6c66dc95da6b2
> src/master/flags.cpp 800e54a302386f26f9b2f21edacad8840abbb42d
> src/master/http.cpp d7afa2af989eebfc9039b3681f087ce570f601d5
> src/master/master.hpp 2936d32c5e7b5b944bf16a1f261c7c54179900ec
> src/master/master.cpp 40ce3e17fca88da689128bcf5d35fdddc396c011
> src/master/quota_handler.cpp ac4da74686f524a66745b4a4e038c3d703ea1ee9
> src/tests/master_quota_tests.cpp 81f0386d072d0176cb28eb229b274e424004f54c
> src/tests/mesos.cpp 082e57bc73fad02de77e16e4b34451e6c0903038
> src/tests/persistent_volume_endpoints_tests.cpp f0cce190abc90f0fae84d6c3db20e8215c2d8132
>
> Diff: https://reviews.apache.org/r/39043/diff/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Alexander Rojas
>
>
Re: Review Request 39043: Added support for HTTP Authentication in
Mesos.
Posted by Bernd Mathiske <be...@mesosphere.io>.
> On Jan. 6, 2016, 6:45 a.m., Jan Schlicht wrote:
> > src/master/constants.hpp, line 137
> > <https://reviews.apache.org/r/39043/diff/25/?file=1184412#file1184412line137>
> >
> > s/mesos/Mesos
>
> Alexander Rojas wrote:
> The string there is the actual value of the string, which is "mesos". That is the pattern in this file. I'll drop it now, but if you feel like it should be otherwise, please reopen.
Aha. Then I'd suggest writing either of these:
- "mesos"
- Mesos
- Bernd
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/39043/#review113065
-----------------------------------------------------------
On Jan. 6, 2016, 7:27 a.m., Alexander Rojas wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39043/
> -----------------------------------------------------------
>
> (Updated Jan. 6, 2016, 7:27 a.m.)
>
>
> Review request for mesos, Adam B, Bernd Mathiske, Isabel Jimenez, Jan Schlicht, 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
> -----
>
> include/mesos/authentication/http/basic_authenticator_factory.hpp PRE-CREATION
> src/authentication/http/basic_authenticator_factory.cpp PRE-CREATION
> src/master/constants.hpp f1624e1ed5091f08f57d4382b344cab8b05ba840
> src/master/constants.cpp 589f2ee55a24d7e8b9352a4c8f94a3785fd1bef4
> src/master/flags.hpp 9af6c68eef6bcf39d5776809fab6c66dc95da6b2
> src/master/flags.cpp 800e54a302386f26f9b2f21edacad8840abbb42d
> src/master/http.cpp d7afa2af989eebfc9039b3681f087ce570f601d5
> src/master/master.hpp 2936d32c5e7b5b944bf16a1f261c7c54179900ec
> src/master/master.cpp 40ce3e17fca88da689128bcf5d35fdddc396c011
> src/master/quota_handler.cpp ac4da74686f524a66745b4a4e038c3d703ea1ee9
> src/tests/master_quota_tests.cpp 81f0386d072d0176cb28eb229b274e424004f54c
> src/tests/mesos.cpp 082e57bc73fad02de77e16e4b34451e6c0903038
> src/tests/persistent_volume_endpoints_tests.cpp f0cce190abc90f0fae84d6c3db20e8215c2d8132
>
> Diff: https://reviews.apache.org/r/39043/diff/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Alexander Rojas
>
>
Re: Review Request 39043: Added support for HTTP Authentication in
Mesos.
Posted by Jan Schlicht <ja...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/39043/#review113065
-----------------------------------------------------------
include/mesos/authentication/http/basic_authenticator_factory.hpp (line 41)
<https://reviews.apache.org/r/39043/#comment173575>
4 spaces instead of 2.
src/master/constants.hpp (line 137)
<https://reviews.apache.org/r/39043/#comment173578>
s/mesos/Mesos
src/master/http.cpp (line 598)
<https://reviews.apache.org/r/39043/#comment173584>
You're changing this to `Forbidden()` here and in `Master::Http::destroyVolumes`, which has to do with RFC-2616. If this is the right way to respond to a failed authorization, you should also implement it that way for the other code paths that deal with authorization. It might be better to leave it be for now and create a separate JIRA for that?
src/master/quota_handler.cpp (line 50)
<https://reviews.apache.org/r/39043/#comment173586>
`Forbidden` isn't used in this file.
src/master/quota_handler.cpp (line 339)
<https://reviews.apache.org/r/39043/#comment173587>
See my comment re `Unauthorized` & `Forbidden` above. This would have to be changed to `Forbidden` to be consistent with the change in `Master::Http::createVolumes`.
- Jan Schlicht
On Jan. 6, 2016, 3:44 p.m., Alexander Rojas wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39043/
> -----------------------------------------------------------
>
> (Updated Jan. 6, 2016, 3:44 p.m.)
>
>
> Review request for mesos, Adam B, Bernd Mathiske, Isabel Jimenez, Jan Schlicht, 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
> -----
>
> include/mesos/authentication/http/basic_authenticator_factory.hpp PRE-CREATION
> src/authentication/http/basic_authenticator_factory.cpp PRE-CREATION
> src/master/constants.hpp f1624e1ed5091f08f57d4382b344cab8b05ba840
> src/master/constants.cpp 589f2ee55a24d7e8b9352a4c8f94a3785fd1bef4
> src/master/flags.hpp 9af6c68eef6bcf39d5776809fab6c66dc95da6b2
> src/master/flags.cpp 800e54a302386f26f9b2f21edacad8840abbb42d
> src/master/http.cpp d7afa2af989eebfc9039b3681f087ce570f601d5
> src/master/master.hpp 2936d32c5e7b5b944bf16a1f261c7c54179900ec
> src/master/master.cpp 40ce3e17fca88da689128bcf5d35fdddc396c011
> src/master/quota_handler.cpp ac4da74686f524a66745b4a4e038c3d703ea1ee9
> src/tests/master_quota_tests.cpp 81f0386d072d0176cb28eb229b274e424004f54c
> src/tests/mesos.cpp 082e57bc73fad02de77e16e4b34451e6c0903038
> src/tests/persistent_volume_endpoints_tests.cpp f0cce190abc90f0fae84d6c3db20e8215c2d8132
>
> Diff: https://reviews.apache.org/r/39043/diff/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Alexander Rojas
>
>
Re: Review Request 39043: Added support for HTTP Authentication in
Mesos.
Posted by Bernd Mathiske <be...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/39043/#review113244
-----------------------------------------------------------
Ship it!
Ship It!
- Bernd Mathiske
On Jan. 7, 2016, 2:30 a.m., Alexander Rojas wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39043/
> -----------------------------------------------------------
>
> (Updated Jan. 7, 2016, 2:30 a.m.)
>
>
> Review request for mesos, Adam B, Bernd Mathiske, Isabel Jimenez, Jan Schlicht, 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
> -----
>
> include/mesos/authentication/http/basic_authenticator_factory.hpp PRE-CREATION
> src/authentication/http/basic_authenticator_factory.cpp PRE-CREATION
> src/master/constants.hpp f1624e1ed5091f08f57d4382b344cab8b05ba840
> src/master/constants.cpp 589f2ee55a24d7e8b9352a4c8f94a3785fd1bef4
> src/master/flags.hpp 9af6c68eef6bcf39d5776809fab6c66dc95da6b2
> src/master/flags.cpp f864419a6276356c97a0a4fe29e5cfce6c4660c4
> src/master/http.cpp d7afa2af989eebfc9039b3681f087ce570f601d5
> src/master/master.hpp f764915ff8bbf74b99a617e3c4735d38fc13e5f0
> src/master/master.cpp 40ce3e17fca88da689128bcf5d35fdddc396c011
> src/master/quota_handler.cpp 93960f3728f9e68c085e6009fe975542d29d2551
> src/tests/master_quota_tests.cpp bc8a117982b994279e0df7639b21abaeb308b76d
> src/tests/mesos.cpp 3867ed2fcb197f98f0a19ca37ed81392db27e8a0
> src/tests/persistent_volume_endpoints_tests.cpp f0cce190abc90f0fae84d6c3db20e8215c2d8132
>
> Diff: https://reviews.apache.org/r/39043/diff/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Alexander Rojas
>
>
Re: Review Request 39043: Added support for HTTP Authentication in
Mesos.
Posted by Till Toenshoff <to...@me.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/39043/#review113377
-----------------------------------------------------------
This looks ready Alexander. Let me fix most issues marked here while landing this patch. I left them marked as unsolved for your consideration. Note that some of the below issues still need you to take actions!
src/master/flags.cpp (lines 447 - 454)
<https://reviews.apache.org/r/39043/#comment173973>
s/Authenticator/authenticator/g
We need to update configuration.md with this - adding that to your patch while committing.
src/master/flags.cpp (line 454)
<https://reviews.apache.org/r/39043/#comment174002>
s/thre's/there is/
s/Authenticators/authenticators/
We should consider making this remark for both, "old"- and http authenticators. In any case, this should be made consistent. Please follow up, thanks!
src/master/master.cpp (lines 500 - 501)
<https://reviews.apache.org/r/39043/#comment174006>
I believe we agreed to have a tracking JIRA for this one. Please create it, thanks Alexander!
src/master/master.cpp (line 501)
<https://reviews.apache.org/r/39043/#comment173996>
s/Authenticator/authenticator/
s/allocator/the allocator/
src/master/master.cpp (line 512)
<https://reviews.apache.org/r/39043/#comment173998>
As the flag name suggests otherwise, we obviously intend to allow multiple authenticators in the future. Can you please add a JIRA that captures this? Thanks Alexander!
src/master/master.cpp (line 517)
<https://reviews.apache.org/r/39043/#comment173997>
s/HTTP Authenticator/HTTP authenticator/
src/master/master.cpp (line 528)
<https://reviews.apache.org/r/39043/#comment173987>
s/.//
src/tests/mesos.cpp (line 436)
<https://reviews.apache.org/r/39043/#comment173989>
As discussed and commented by Alex, that addition about the risks in multi-master configurations are a good idea. Adding that while committing.
- Till Toenshoff
On Jan. 7, 2016, 4:41 p.m., Alexander Rojas wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39043/
> -----------------------------------------------------------
>
> (Updated Jan. 7, 2016, 4:41 p.m.)
>
>
> Review request for mesos, Adam B, Bernd Mathiske, Isabel Jimenez, Jan Schlicht, and Till Toenshoff.
>
>
> Bugs: MESOS-3756 and MESOS-4149
> https://issues.apache.org/jira/browse/MESOS-3756
> https://issues.apache.org/jira/browse/MESOS-4149
>
>
> 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
> -----
>
> include/mesos/authentication/http/basic_authenticator_factory.hpp PRE-CREATION
> src/authentication/http/basic_authenticator_factory.cpp PRE-CREATION
> src/master/constants.hpp f1624e1ed5091f08f57d4382b344cab8b05ba840
> src/master/constants.cpp 589f2ee55a24d7e8b9352a4c8f94a3785fd1bef4
> src/master/flags.hpp 9af6c68eef6bcf39d5776809fab6c66dc95da6b2
> src/master/flags.cpp f864419a6276356c97a0a4fe29e5cfce6c4660c4
> src/master/http.cpp d7afa2af989eebfc9039b3681f087ce570f601d5
> src/master/master.hpp f764915ff8bbf74b99a617e3c4735d38fc13e5f0
> src/master/master.cpp 40ce3e17fca88da689128bcf5d35fdddc396c011
> src/master/quota_handler.cpp 93960f3728f9e68c085e6009fe975542d29d2551
> src/tests/master_quota_tests.cpp bc8a117982b994279e0df7639b21abaeb308b76d
> src/tests/mesos.cpp 3867ed2fcb197f98f0a19ca37ed81392db27e8a0
>
> Diff: https://reviews.apache.org/r/39043/diff/
>
>
> Testing
> -------
>
> ```bash
> # On OS X 10.11.2 and Ubuntu 14.04.3
> $ make check
> $ ./bin/mesos-tests.sh --gtest_repeat=50 --gtest_shuffle --gtest_break_on_failure
>
> # On Ubuntu 14.04.3 (Disabling known flaky tests)
> $ sudo ./bin/mesos-tests.sh --gtest_repeat=50 --gtest_shuffle --gtest_break_on_failure \
> --gtest_filter="-*.ROOT_IncreaseRSS:*.ROOT_CGROUPS_Listen"
> ```
>
>
> Thanks,
>
> Alexander Rojas
>
>
Re: Review Request 39043: Added support for HTTP Authentication in
Mesos.
Posted by Greg Mann <gr...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/39043/#review113276
-----------------------------------------------------------
Ship it!
Ship It!
- Greg Mann
On Jan. 7, 2016, 4:41 p.m., Alexander Rojas wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39043/
> -----------------------------------------------------------
>
> (Updated Jan. 7, 2016, 4:41 p.m.)
>
>
> Review request for mesos, Adam B, Bernd Mathiske, Isabel Jimenez, Jan Schlicht, and Till Toenshoff.
>
>
> Bugs: MESOS-3756 and MESOS-4149
> https://issues.apache.org/jira/browse/MESOS-3756
> https://issues.apache.org/jira/browse/MESOS-4149
>
>
> 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
> -----
>
> include/mesos/authentication/http/basic_authenticator_factory.hpp PRE-CREATION
> src/authentication/http/basic_authenticator_factory.cpp PRE-CREATION
> src/master/constants.hpp f1624e1ed5091f08f57d4382b344cab8b05ba840
> src/master/constants.cpp 589f2ee55a24d7e8b9352a4c8f94a3785fd1bef4
> src/master/flags.hpp 9af6c68eef6bcf39d5776809fab6c66dc95da6b2
> src/master/flags.cpp f864419a6276356c97a0a4fe29e5cfce6c4660c4
> src/master/http.cpp d7afa2af989eebfc9039b3681f087ce570f601d5
> src/master/master.hpp f764915ff8bbf74b99a617e3c4735d38fc13e5f0
> src/master/master.cpp 40ce3e17fca88da689128bcf5d35fdddc396c011
> src/master/quota_handler.cpp 93960f3728f9e68c085e6009fe975542d29d2551
> src/tests/master_quota_tests.cpp bc8a117982b994279e0df7639b21abaeb308b76d
> src/tests/mesos.cpp 3867ed2fcb197f98f0a19ca37ed81392db27e8a0
>
> Diff: https://reviews.apache.org/r/39043/diff/
>
>
> Testing
> -------
>
> ```bash
> # On OS X 10.11.2 and Ubuntu 14.04.3
> $ make check
> $ ./bin/mesos-tests.sh --gtest_repeat=50 --gtest_shuffle --gtest_break_on_failure
>
> # On Ubuntu 14.04.3 (Disabling known flaky tests)
> $ sudo ./bin/mesos-tests.sh --gtest_repeat=50 --gtest_shuffle --gtest_break_on_failure \
> --gtest_filter="-*.ROOT_IncreaseRSS:*.ROOT_CGROUPS_Listen"
> ```
>
>
> Thanks,
>
> Alexander Rojas
>
>
Re: Review Request 39043: Added support for HTTP Authentication in
Mesos.
Posted by Alexander Rojas <al...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/39043/
-----------------------------------------------------------
(Updated Jan. 7, 2016, 5:41 p.m.)
Review request for mesos, Adam B, Bernd Mathiske, Isabel Jimenez, Jan Schlicht, and Till Toenshoff.
Changes
-------
Updated test done.
Bugs: MESOS-3756 and MESOS-4149
https://issues.apache.org/jira/browse/MESOS-3756
https://issues.apache.org/jira/browse/MESOS-4149
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
-----
include/mesos/authentication/http/basic_authenticator_factory.hpp PRE-CREATION
src/authentication/http/basic_authenticator_factory.cpp PRE-CREATION
src/master/constants.hpp f1624e1ed5091f08f57d4382b344cab8b05ba840
src/master/constants.cpp 589f2ee55a24d7e8b9352a4c8f94a3785fd1bef4
src/master/flags.hpp 9af6c68eef6bcf39d5776809fab6c66dc95da6b2
src/master/flags.cpp f864419a6276356c97a0a4fe29e5cfce6c4660c4
src/master/http.cpp d7afa2af989eebfc9039b3681f087ce570f601d5
src/master/master.hpp f764915ff8bbf74b99a617e3c4735d38fc13e5f0
src/master/master.cpp 40ce3e17fca88da689128bcf5d35fdddc396c011
src/master/quota_handler.cpp 93960f3728f9e68c085e6009fe975542d29d2551
src/tests/master_quota_tests.cpp bc8a117982b994279e0df7639b21abaeb308b76d
src/tests/mesos.cpp 3867ed2fcb197f98f0a19ca37ed81392db27e8a0
Diff: https://reviews.apache.org/r/39043/diff/
Testing (updated)
-------
```bash
# On OS X 10.11.2 and Ubuntu 14.04.3
$ make check
$ ./bin/mesos-tests.sh --gtest_repeat=50 --gtest_shuffle --gtest_break_on_failure
# On Ubuntu 14.04.3 (Disabling known flaky tests)
$ sudo ./bin/mesos-tests.sh --gtest_repeat=50 --gtest_shuffle --gtest_break_on_failure \
--gtest_filter="-*.ROOT_IncreaseRSS:*.ROOT_CGROUPS_Listen"
```
Thanks,
Alexander Rojas
Re: Review Request 39043: Added support for HTTP Authentication in
Mesos.
Posted by Alexander Rojas <al...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/39043/
-----------------------------------------------------------
(Updated Jan. 7, 2016, 5:03 p.m.)
Review request for mesos, Adam B, Bernd Mathiske, Isabel Jimenez, Jan Schlicht, and Till Toenshoff.
Changes
-------
Also solves MESOS-4149
Bugs: MESOS-3756 and MESOS-4149
https://issues.apache.org/jira/browse/MESOS-3756
https://issues.apache.org/jira/browse/MESOS-4149
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
-----
include/mesos/authentication/http/basic_authenticator_factory.hpp PRE-CREATION
src/authentication/http/basic_authenticator_factory.cpp PRE-CREATION
src/master/constants.hpp f1624e1ed5091f08f57d4382b344cab8b05ba840
src/master/constants.cpp 589f2ee55a24d7e8b9352a4c8f94a3785fd1bef4
src/master/flags.hpp 9af6c68eef6bcf39d5776809fab6c66dc95da6b2
src/master/flags.cpp f864419a6276356c97a0a4fe29e5cfce6c4660c4
src/master/http.cpp d7afa2af989eebfc9039b3681f087ce570f601d5
src/master/master.hpp f764915ff8bbf74b99a617e3c4735d38fc13e5f0
src/master/master.cpp 40ce3e17fca88da689128bcf5d35fdddc396c011
src/master/quota_handler.cpp 93960f3728f9e68c085e6009fe975542d29d2551
src/tests/master_quota_tests.cpp bc8a117982b994279e0df7639b21abaeb308b76d
src/tests/mesos.cpp 3867ed2fcb197f98f0a19ca37ed81392db27e8a0
Diff: https://reviews.apache.org/r/39043/diff/
Testing
-------
make check
Thanks,
Alexander Rojas
Re: Review Request 39043: Added support for HTTP Authentication in
Mesos.
Posted by Alexander Rojas <al...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/39043/
-----------------------------------------------------------
(Updated Jan. 7, 2016, 5:02 p.m.)
Review request for mesos, Adam B, Bernd Mathiske, Isabel Jimenez, Jan Schlicht, and Till Toenshoff.
Changes
-------
Adresses Alex's issues.
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 (updated)
-----
include/mesos/authentication/http/basic_authenticator_factory.hpp PRE-CREATION
src/authentication/http/basic_authenticator_factory.cpp PRE-CREATION
src/master/constants.hpp f1624e1ed5091f08f57d4382b344cab8b05ba840
src/master/constants.cpp 589f2ee55a24d7e8b9352a4c8f94a3785fd1bef4
src/master/flags.hpp 9af6c68eef6bcf39d5776809fab6c66dc95da6b2
src/master/flags.cpp f864419a6276356c97a0a4fe29e5cfce6c4660c4
src/master/http.cpp d7afa2af989eebfc9039b3681f087ce570f601d5
src/master/master.hpp f764915ff8bbf74b99a617e3c4735d38fc13e5f0
src/master/master.cpp 40ce3e17fca88da689128bcf5d35fdddc396c011
src/master/quota_handler.cpp 93960f3728f9e68c085e6009fe975542d29d2551
src/tests/master_quota_tests.cpp bc8a117982b994279e0df7639b21abaeb308b76d
src/tests/mesos.cpp 3867ed2fcb197f98f0a19ca37ed81392db27e8a0
Diff: https://reviews.apache.org/r/39043/diff/
Testing
-------
make check
Thanks,
Alexander Rojas
Re: Review Request 39043: Added support for HTTP Authentication in
Mesos.
Posted by Alexander Rojas <al...@mesosphere.io>.
> On Jan. 7, 2016, 4:28 p.m., Alexander Rukletsov wrote:
> > src/tests/mesos.cpp, lines 435-438
> > <https://reviews.apache.org/r/39043/diff/29/?file=1186135#file1186135line435>
> >
> > Since `setAuthenticator` is called in `Master::initialize()` won't it be a problem if we have multiple masters in a test case? Is `unsetAuthenticator()` idempotent? In tests multiple masters (and agents) share the same libprocess context (which is not true in production scenarios), I wonder whether this can be a problem.
That is exactly the reason it is a TODO, the global state of libprocess is rather inconvenient in this case.
> On Jan. 7, 2016, 4:28 p.m., Alexander Rukletsov wrote:
> > src/tests/persistent_volume_endpoints_tests.cpp, lines 900-902
> > <https://reviews.apache.org/r/39043/diff/29/?file=1186136#file1186136line900>
> >
> > I suggest to make this change in a separate patch, but this fits one line now.
Done, moved to r/42027/
- Alexander
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/39043/#review113242
-----------------------------------------------------------
On Jan. 7, 2016, 5:02 p.m., Alexander Rojas wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39043/
> -----------------------------------------------------------
>
> (Updated Jan. 7, 2016, 5:02 p.m.)
>
>
> Review request for mesos, Adam B, Bernd Mathiske, Isabel Jimenez, Jan Schlicht, 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
> -----
>
> include/mesos/authentication/http/basic_authenticator_factory.hpp PRE-CREATION
> src/authentication/http/basic_authenticator_factory.cpp PRE-CREATION
> src/master/constants.hpp f1624e1ed5091f08f57d4382b344cab8b05ba840
> src/master/constants.cpp 589f2ee55a24d7e8b9352a4c8f94a3785fd1bef4
> src/master/flags.hpp 9af6c68eef6bcf39d5776809fab6c66dc95da6b2
> src/master/flags.cpp f864419a6276356c97a0a4fe29e5cfce6c4660c4
> src/master/http.cpp d7afa2af989eebfc9039b3681f087ce570f601d5
> src/master/master.hpp f764915ff8bbf74b99a617e3c4735d38fc13e5f0
> src/master/master.cpp 40ce3e17fca88da689128bcf5d35fdddc396c011
> src/master/quota_handler.cpp 93960f3728f9e68c085e6009fe975542d29d2551
> src/tests/master_quota_tests.cpp bc8a117982b994279e0df7639b21abaeb308b76d
> src/tests/mesos.cpp 3867ed2fcb197f98f0a19ca37ed81392db27e8a0
>
> Diff: https://reviews.apache.org/r/39043/diff/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Alexander Rojas
>
>
Re: Review Request 39043: Added support for HTTP Authentication in
Mesos.
Posted by Till Toenshoff <to...@me.com>.
> On Jan. 7, 2016, 3:28 p.m., Alexander Rukletsov wrote:
> > src/master/master.hpp, lines 1187-1190
> > <https://reviews.apache.org/r/39043/diff/29/?file=1186131#file1186131line1187>
> >
> > Let's remove this in a patch chained to this review (and fix MESOS-4149 as a consequence) in order not to forget.
For the future, please point our your solution if it is not following the proposed one - otherwise it gets harder to confirm resolved issues.
For this specific case, we agreed to have the ticket to be added to this RR.
- Till
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/39043/#review113242
-----------------------------------------------------------
On Jan. 7, 2016, 4:41 p.m., Alexander Rojas wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39043/
> -----------------------------------------------------------
>
> (Updated Jan. 7, 2016, 4:41 p.m.)
>
>
> Review request for mesos, Adam B, Bernd Mathiske, Isabel Jimenez, Jan Schlicht, and Till Toenshoff.
>
>
> Bugs: MESOS-3756 and MESOS-4149
> https://issues.apache.org/jira/browse/MESOS-3756
> https://issues.apache.org/jira/browse/MESOS-4149
>
>
> 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
> -----
>
> include/mesos/authentication/http/basic_authenticator_factory.hpp PRE-CREATION
> src/authentication/http/basic_authenticator_factory.cpp PRE-CREATION
> src/master/constants.hpp f1624e1ed5091f08f57d4382b344cab8b05ba840
> src/master/constants.cpp 589f2ee55a24d7e8b9352a4c8f94a3785fd1bef4
> src/master/flags.hpp 9af6c68eef6bcf39d5776809fab6c66dc95da6b2
> src/master/flags.cpp f864419a6276356c97a0a4fe29e5cfce6c4660c4
> src/master/http.cpp d7afa2af989eebfc9039b3681f087ce570f601d5
> src/master/master.hpp f764915ff8bbf74b99a617e3c4735d38fc13e5f0
> src/master/master.cpp 40ce3e17fca88da689128bcf5d35fdddc396c011
> src/master/quota_handler.cpp 93960f3728f9e68c085e6009fe975542d29d2551
> src/tests/master_quota_tests.cpp bc8a117982b994279e0df7639b21abaeb308b76d
> src/tests/mesos.cpp 3867ed2fcb197f98f0a19ca37ed81392db27e8a0
>
> Diff: https://reviews.apache.org/r/39043/diff/
>
>
> Testing
> -------
>
> ```bash
> # On OS X 10.11.2 and Ubuntu 14.04.3
> $ make check
> $ ./bin/mesos-tests.sh --gtest_repeat=50 --gtest_shuffle --gtest_break_on_failure
>
> # On Ubuntu 14.04.3 (Disabling known flaky tests)
> $ sudo ./bin/mesos-tests.sh --gtest_repeat=50 --gtest_shuffle --gtest_break_on_failure \
> --gtest_filter="-*.ROOT_IncreaseRSS:*.ROOT_CGROUPS_Listen"
> ```
>
>
> Thanks,
>
> Alexander Rojas
>
>
Re: Review Request 39043: Added support for HTTP Authentication in
Mesos.
Posted by Alexander Rukletsov <ru...@gmail.com>.
> On Jan. 7, 2016, 3:28 p.m., Alexander Rukletsov wrote:
> > src/tests/mesos.cpp, lines 435-438
> > <https://reviews.apache.org/r/39043/diff/29/?file=1186135#file1186135line435>
> >
> > Since `setAuthenticator` is called in `Master::initialize()` won't it be a problem if we have multiple masters in a test case? Is `unsetAuthenticator()` idempotent? In tests multiple masters (and agents) share the same libprocess context (which is not true in production scenarios), I wonder whether this can be a problem.
>
> Alexander Rojas wrote:
> That is exactly the reason it is a TODO, the global state of libprocess is rather inconvenient in this case.
If multiple masters in tests wuth authz enabled may crash tests, mind leaving a comment about that for the guy who will be so unlucky to debug it : )?
- Alexander
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/39043/#review113242
-----------------------------------------------------------
On Jan. 7, 2016, 4:41 p.m., Alexander Rojas wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39043/
> -----------------------------------------------------------
>
> (Updated Jan. 7, 2016, 4:41 p.m.)
>
>
> Review request for mesos, Adam B, Bernd Mathiske, Isabel Jimenez, Jan Schlicht, and Till Toenshoff.
>
>
> Bugs: MESOS-3756 and MESOS-4149
> https://issues.apache.org/jira/browse/MESOS-3756
> https://issues.apache.org/jira/browse/MESOS-4149
>
>
> 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
> -----
>
> include/mesos/authentication/http/basic_authenticator_factory.hpp PRE-CREATION
> src/authentication/http/basic_authenticator_factory.cpp PRE-CREATION
> src/master/constants.hpp f1624e1ed5091f08f57d4382b344cab8b05ba840
> src/master/constants.cpp 589f2ee55a24d7e8b9352a4c8f94a3785fd1bef4
> src/master/flags.hpp 9af6c68eef6bcf39d5776809fab6c66dc95da6b2
> src/master/flags.cpp f864419a6276356c97a0a4fe29e5cfce6c4660c4
> src/master/http.cpp d7afa2af989eebfc9039b3681f087ce570f601d5
> src/master/master.hpp f764915ff8bbf74b99a617e3c4735d38fc13e5f0
> src/master/master.cpp 40ce3e17fca88da689128bcf5d35fdddc396c011
> src/master/quota_handler.cpp 93960f3728f9e68c085e6009fe975542d29d2551
> src/tests/master_quota_tests.cpp bc8a117982b994279e0df7639b21abaeb308b76d
> src/tests/mesos.cpp 3867ed2fcb197f98f0a19ca37ed81392db27e8a0
>
> Diff: https://reviews.apache.org/r/39043/diff/
>
>
> Testing
> -------
>
> ```bash
> # On OS X 10.11.2 and Ubuntu 14.04.3
> $ make check
> $ ./bin/mesos-tests.sh --gtest_repeat=50 --gtest_shuffle --gtest_break_on_failure
>
> # On Ubuntu 14.04.3 (Disabling known flaky tests)
> $ sudo ./bin/mesos-tests.sh --gtest_repeat=50 --gtest_shuffle --gtest_break_on_failure \
> --gtest_filter="-*.ROOT_IncreaseRSS:*.ROOT_CGROUPS_Listen"
> ```
>
>
> Thanks,
>
> Alexander Rojas
>
>
Re: Review Request 39043: Added support for HTTP Authentication in
Mesos.
Posted by Alexander Rukletsov <ru...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/39043/#review113242
-----------------------------------------------------------
Great stuff, happy to see it landing! Some general suggestions, not necessarily for immediate execution, but for posterity (and maybe follow-up JIRAs ; ):
1. Smaller, self-contained reviews. For example, you can update `Unauthorized` -> `Forbidden` in a separate review for clarity and to keep this one smaller.
2. When updating endpoints code, always check and update appropriate documentation. For example `Unauthorized` -> `Forbidden` must be reflected in reservation and persistent volumes docs.
3. Spooky places like blobs that are hard to read should ideally be prepended with a TODO saying you plan to refactor it. If it does not look nice and slick to you, chances are the next guy will feel the same. Let's show we care about that and are not leaving broken windows.
4. Changes touching libprocess are vulnerable to races, especially in tests, where the libprocess context may be shared among multiple masters and agents. Please extend the testing (and mention this in the "Testing Done" section) with different platforms, and goodies like `--gtest_shuffle` and `--gtest-repeat`.
src/authentication/http/basic_authenticator_factory.cpp (line 40)
<https://reviews.apache.org/r/39043/#comment173751>
#include <stout/foreach.hpp>?
src/master/flags.cpp (lines 447 - 449)
<https://reviews.apache.org/r/39043/#comment173769>
The variable name implies there can be many authenticators, while the comment says it may just be one. What's right? I assume the former, because you do a split on comma later in the patch. Mind mentioning that?
src/master/http.cpp (lines 1671 - 1672)
<https://reviews.apache.org/r/39043/#comment173770>
I like how the scope is narrowed now and we do not need to carry around the unused secret!
src/master/master.hpp (lines 1179 - 1182)
<https://reviews.apache.org/r/39043/#comment173764>
Let's remove this in a patch chained to this review (and fix MESOS-4149 as a consequence) in order not to forget.
src/master/master.cpp (lines 500 - 508)
<https://reviews.apache.org/r/39043/#comment173778>
Let's add a TODO to refactor lines 500-556 into a factory so people see we consider such blobs a tech debt.
src/tests/mesos.cpp (lines 435 - 438)
<https://reviews.apache.org/r/39043/#comment173768>
Since `setAuthenticator` is called in `Master::initialize()` won't it be a problem if we have multiple masters in a test case? Is `unsetAuthenticator()` idempotent? In tests multiple masters (and agents) share the same libprocess context (which is not true in production scenarios), I wonder whether this can be a problem.
src/tests/mesos.cpp (line 438)
<https://reviews.apache.org/r/39043/#comment173765>
4 spaces, please.
src/tests/persistent_volume_endpoints_tests.cpp (lines 900 - 902)
<https://reviews.apache.org/r/39043/#comment173743>
I suggest to make this change in a separate patch, but this fits one line now.
- Alexander Rukletsov
On Jan. 7, 2016, 3:25 p.m., Alexander Rojas wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39043/
> -----------------------------------------------------------
>
> (Updated Jan. 7, 2016, 3:25 p.m.)
>
>
> Review request for mesos, Adam B, Bernd Mathiske, Isabel Jimenez, Jan Schlicht, 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
> -----
>
> include/mesos/authentication/http/basic_authenticator_factory.hpp PRE-CREATION
> src/authentication/http/basic_authenticator_factory.cpp PRE-CREATION
> src/master/constants.hpp f1624e1ed5091f08f57d4382b344cab8b05ba840
> src/master/constants.cpp 589f2ee55a24d7e8b9352a4c8f94a3785fd1bef4
> src/master/flags.hpp 9af6c68eef6bcf39d5776809fab6c66dc95da6b2
> src/master/flags.cpp f864419a6276356c97a0a4fe29e5cfce6c4660c4
> src/master/http.cpp d7afa2af989eebfc9039b3681f087ce570f601d5
> src/master/master.hpp f764915ff8bbf74b99a617e3c4735d38fc13e5f0
> src/master/master.cpp 40ce3e17fca88da689128bcf5d35fdddc396c011
> src/master/quota_handler.cpp 93960f3728f9e68c085e6009fe975542d29d2551
> src/tests/master_quota_tests.cpp bc8a117982b994279e0df7639b21abaeb308b76d
> src/tests/mesos.cpp 3867ed2fcb197f98f0a19ca37ed81392db27e8a0
>
> Diff: https://reviews.apache.org/r/39043/diff/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Alexander Rojas
>
>
Re: Review Request 39043: Added support for HTTP Authentication in
Mesos.
Posted by Jan Schlicht <ja...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/39043/#review113263
-----------------------------------------------------------
Ship it!
Ship It!
- Jan Schlicht
On Jan. 7, 2016, 4:25 p.m., Alexander Rojas wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39043/
> -----------------------------------------------------------
>
> (Updated Jan. 7, 2016, 4:25 p.m.)
>
>
> Review request for mesos, Adam B, Bernd Mathiske, Isabel Jimenez, Jan Schlicht, 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
> -----
>
> include/mesos/authentication/http/basic_authenticator_factory.hpp PRE-CREATION
> src/authentication/http/basic_authenticator_factory.cpp PRE-CREATION
> src/master/constants.hpp f1624e1ed5091f08f57d4382b344cab8b05ba840
> src/master/constants.cpp 589f2ee55a24d7e8b9352a4c8f94a3785fd1bef4
> src/master/flags.hpp 9af6c68eef6bcf39d5776809fab6c66dc95da6b2
> src/master/flags.cpp f864419a6276356c97a0a4fe29e5cfce6c4660c4
> src/master/http.cpp d7afa2af989eebfc9039b3681f087ce570f601d5
> src/master/master.hpp f764915ff8bbf74b99a617e3c4735d38fc13e5f0
> src/master/master.cpp 40ce3e17fca88da689128bcf5d35fdddc396c011
> src/master/quota_handler.cpp 93960f3728f9e68c085e6009fe975542d29d2551
> src/tests/master_quota_tests.cpp bc8a117982b994279e0df7639b21abaeb308b76d
> src/tests/mesos.cpp 3867ed2fcb197f98f0a19ca37ed81392db27e8a0
>
> Diff: https://reviews.apache.org/r/39043/diff/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Alexander Rojas
>
>
Re: Review Request 39043: Added support for HTTP Authentication in
Mesos.
Posted by Alexander Rojas <al...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/39043/
-----------------------------------------------------------
(Updated Jan. 7, 2016, 4:25 p.m.)
Review request for mesos, Adam B, Bernd Mathiske, Isabel Jimenez, Jan Schlicht, and Till Toenshoff.
Changes
-------
Added TODO.
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 (updated)
-----
include/mesos/authentication/http/basic_authenticator_factory.hpp PRE-CREATION
src/authentication/http/basic_authenticator_factory.cpp PRE-CREATION
src/master/constants.hpp f1624e1ed5091f08f57d4382b344cab8b05ba840
src/master/constants.cpp 589f2ee55a24d7e8b9352a4c8f94a3785fd1bef4
src/master/flags.hpp 9af6c68eef6bcf39d5776809fab6c66dc95da6b2
src/master/flags.cpp f864419a6276356c97a0a4fe29e5cfce6c4660c4
src/master/http.cpp d7afa2af989eebfc9039b3681f087ce570f601d5
src/master/master.hpp f764915ff8bbf74b99a617e3c4735d38fc13e5f0
src/master/master.cpp 40ce3e17fca88da689128bcf5d35fdddc396c011
src/master/quota_handler.cpp 93960f3728f9e68c085e6009fe975542d29d2551
src/tests/master_quota_tests.cpp bc8a117982b994279e0df7639b21abaeb308b76d
src/tests/mesos.cpp 3867ed2fcb197f98f0a19ca37ed81392db27e8a0
Diff: https://reviews.apache.org/r/39043/diff/
Testing
-------
make check
Thanks,
Alexander Rojas
Re: Review Request 39043: Added support for HTTP Authentication in
Mesos.
Posted by Alexander Rojas <al...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/39043/
-----------------------------------------------------------
(Updated Jan. 7, 2016, 4:01 p.m.)
Review request for mesos, Adam B, Bernd Mathiske, Isabel Jimenez, Jan Schlicht, and Till Toenshoff.
Changes
-------
Removes changes from `Unauthorized` to `Forbidden`. These changes are now covered by [MESOS-4305](https://issues.apache.org/jira/browse/MESOS-4305) and dealt with on their own review.
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 (updated)
-----
include/mesos/authentication/http/basic_authenticator_factory.hpp PRE-CREATION
src/authentication/http/basic_authenticator_factory.cpp PRE-CREATION
src/master/constants.hpp f1624e1ed5091f08f57d4382b344cab8b05ba840
src/master/constants.cpp 589f2ee55a24d7e8b9352a4c8f94a3785fd1bef4
src/master/flags.hpp 9af6c68eef6bcf39d5776809fab6c66dc95da6b2
src/master/flags.cpp f864419a6276356c97a0a4fe29e5cfce6c4660c4
src/master/http.cpp d7afa2af989eebfc9039b3681f087ce570f601d5
src/master/master.hpp f764915ff8bbf74b99a617e3c4735d38fc13e5f0
src/master/master.cpp 40ce3e17fca88da689128bcf5d35fdddc396c011
src/master/quota_handler.cpp 93960f3728f9e68c085e6009fe975542d29d2551
src/tests/master_quota_tests.cpp bc8a117982b994279e0df7639b21abaeb308b76d
src/tests/mesos.cpp 3867ed2fcb197f98f0a19ca37ed81392db27e8a0
Diff: https://reviews.apache.org/r/39043/diff/
Testing
-------
make check
Thanks,
Alexander Rojas
Re: Review Request 39043: Added support for HTTP Authentication in
Mesos.
Posted by Alexander Rojas <al...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/39043/
-----------------------------------------------------------
(Updated Jan. 7, 2016, 11:30 a.m.)
Review request for mesos, Adam B, Bernd Mathiske, Isabel Jimenez, Jan Schlicht, and Till Toenshoff.
Changes
-------
Rebase.
Jan's changes.
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 (updated)
-----
include/mesos/authentication/http/basic_authenticator_factory.hpp PRE-CREATION
src/authentication/http/basic_authenticator_factory.cpp PRE-CREATION
src/master/constants.hpp f1624e1ed5091f08f57d4382b344cab8b05ba840
src/master/constants.cpp 589f2ee55a24d7e8b9352a4c8f94a3785fd1bef4
src/master/flags.hpp 9af6c68eef6bcf39d5776809fab6c66dc95da6b2
src/master/flags.cpp f864419a6276356c97a0a4fe29e5cfce6c4660c4
src/master/http.cpp d7afa2af989eebfc9039b3681f087ce570f601d5
src/master/master.hpp f764915ff8bbf74b99a617e3c4735d38fc13e5f0
src/master/master.cpp 40ce3e17fca88da689128bcf5d35fdddc396c011
src/master/quota_handler.cpp 93960f3728f9e68c085e6009fe975542d29d2551
src/tests/master_quota_tests.cpp bc8a117982b994279e0df7639b21abaeb308b76d
src/tests/mesos.cpp 3867ed2fcb197f98f0a19ca37ed81392db27e8a0
src/tests/persistent_volume_endpoints_tests.cpp f0cce190abc90f0fae84d6c3db20e8215c2d8132
Diff: https://reviews.apache.org/r/39043/diff/
Testing
-------
make check
Thanks,
Alexander Rojas
Re: Review Request 39043: Added support for HTTP Authentication in
Mesos.
Posted by Mesos ReviewBot <re...@mesos.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/39043/#review113108
-----------------------------------------------------------
Bad patch!
Reviews applied: [40539, 37999, 38000, 38094, 38950, 39043]
Failed command: ./support/apply-review.sh -n -r 39043
Error:
2016-01-06 18:42:26 URL:https://reviews.apache.org/r/39043/diff/raw/ [29169/29169] -> "39043.patch" [1]
error: patch failed: src/master/quota_handler.cpp:337
error: src/master/quota_handler.cpp: patch does not apply
error: patch failed: src/tests/master_quota_tests.cpp:1341
error: src/tests/master_quota_tests.cpp: patch does not apply
- Mesos ReviewBot
On Jan. 6, 2016, 3:27 p.m., Alexander Rojas wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39043/
> -----------------------------------------------------------
>
> (Updated Jan. 6, 2016, 3:27 p.m.)
>
>
> Review request for mesos, Adam B, Bernd Mathiske, Isabel Jimenez, Jan Schlicht, 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
> -----
>
> include/mesos/authentication/http/basic_authenticator_factory.hpp PRE-CREATION
> src/authentication/http/basic_authenticator_factory.cpp PRE-CREATION
> src/master/constants.hpp f1624e1ed5091f08f57d4382b344cab8b05ba840
> src/master/constants.cpp 589f2ee55a24d7e8b9352a4c8f94a3785fd1bef4
> src/master/flags.hpp 9af6c68eef6bcf39d5776809fab6c66dc95da6b2
> src/master/flags.cpp 800e54a302386f26f9b2f21edacad8840abbb42d
> src/master/http.cpp d7afa2af989eebfc9039b3681f087ce570f601d5
> src/master/master.hpp 2936d32c5e7b5b944bf16a1f261c7c54179900ec
> src/master/master.cpp 40ce3e17fca88da689128bcf5d35fdddc396c011
> src/master/quota_handler.cpp ac4da74686f524a66745b4a4e038c3d703ea1ee9
> src/tests/master_quota_tests.cpp 81f0386d072d0176cb28eb229b274e424004f54c
> src/tests/mesos.cpp 082e57bc73fad02de77e16e4b34451e6c0903038
> src/tests/persistent_volume_endpoints_tests.cpp f0cce190abc90f0fae84d6c3db20e8215c2d8132
>
> Diff: https://reviews.apache.org/r/39043/diff/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Alexander Rojas
>
>
Re: Review Request 39043: Added support for HTTP Authentication in
Mesos.
Posted by Alexander Rojas <al...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/39043/
-----------------------------------------------------------
(Updated Jan. 6, 2016, 4:27 p.m.)
Review request for mesos, Adam B, Bernd Mathiske, Isabel Jimenez, Jan Schlicht, and Till Toenshoff.
Changes
-------
Jan's review.
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 (updated)
-----
include/mesos/authentication/http/basic_authenticator_factory.hpp PRE-CREATION
src/authentication/http/basic_authenticator_factory.cpp PRE-CREATION
src/master/constants.hpp f1624e1ed5091f08f57d4382b344cab8b05ba840
src/master/constants.cpp 589f2ee55a24d7e8b9352a4c8f94a3785fd1bef4
src/master/flags.hpp 9af6c68eef6bcf39d5776809fab6c66dc95da6b2
src/master/flags.cpp 800e54a302386f26f9b2f21edacad8840abbb42d
src/master/http.cpp d7afa2af989eebfc9039b3681f087ce570f601d5
src/master/master.hpp 2936d32c5e7b5b944bf16a1f261c7c54179900ec
src/master/master.cpp 40ce3e17fca88da689128bcf5d35fdddc396c011
src/master/quota_handler.cpp ac4da74686f524a66745b4a4e038c3d703ea1ee9
src/tests/master_quota_tests.cpp 81f0386d072d0176cb28eb229b274e424004f54c
src/tests/mesos.cpp 082e57bc73fad02de77e16e4b34451e6c0903038
src/tests/persistent_volume_endpoints_tests.cpp f0cce190abc90f0fae84d6c3db20e8215c2d8132
Diff: https://reviews.apache.org/r/39043/diff/
Testing
-------
make check
Thanks,
Alexander Rojas
Re: Review Request 39043: Added support for HTTP Authentication in
Mesos.
Posted by Alexander Rojas <al...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/39043/
-----------------------------------------------------------
(Updated Jan. 6, 2016, 3:44 p.m.)
Review request for mesos, Adam B, Bernd Mathiske, Isabel Jimenez, Jan Schlicht, and Till Toenshoff.
Changes
-------
Remove use of deprecated `Unauthorized` constructor. Unauthorized request should return `Forbidden`.
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 (updated)
-----
include/mesos/authentication/http/basic_authenticator_factory.hpp PRE-CREATION
src/authentication/http/basic_authenticator_factory.cpp PRE-CREATION
src/master/constants.hpp f1624e1ed5091f08f57d4382b344cab8b05ba840
src/master/constants.cpp 589f2ee55a24d7e8b9352a4c8f94a3785fd1bef4
src/master/flags.hpp 9af6c68eef6bcf39d5776809fab6c66dc95da6b2
src/master/flags.cpp 800e54a302386f26f9b2f21edacad8840abbb42d
src/master/http.cpp d7afa2af989eebfc9039b3681f087ce570f601d5
src/master/master.hpp 2936d32c5e7b5b944bf16a1f261c7c54179900ec
src/master/master.cpp 40ce3e17fca88da689128bcf5d35fdddc396c011
src/master/quota_handler.cpp ac4da74686f524a66745b4a4e038c3d703ea1ee9
src/tests/master_quota_tests.cpp 81f0386d072d0176cb28eb229b274e424004f54c
src/tests/mesos.cpp 082e57bc73fad02de77e16e4b34451e6c0903038
src/tests/persistent_volume_endpoints_tests.cpp f0cce190abc90f0fae84d6c3db20e8215c2d8132
Diff: https://reviews.apache.org/r/39043/diff/
Testing
-------
make check
Thanks,
Alexander Rojas
Re: Review Request 39043: Added support for HTTP Authentication in
Mesos.
Posted by Alexander Rojas <al...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/39043/
-----------------------------------------------------------
(Updated Jan. 6, 2016, 2:47 p.m.)
Review request for mesos, Adam B, Bernd Mathiske, Isabel Jimenez, Jan Schlicht, and Till Toenshoff.
Changes
-------
Rebase.
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 (updated)
-----
include/mesos/authentication/http/basic_authenticator_factory.hpp PRE-CREATION
src/authentication/http/basic_authenticator_factory.cpp PRE-CREATION
src/master/constants.hpp f1624e1ed5091f08f57d4382b344cab8b05ba840
src/master/constants.cpp 589f2ee55a24d7e8b9352a4c8f94a3785fd1bef4
src/master/flags.hpp 9af6c68eef6bcf39d5776809fab6c66dc95da6b2
src/master/flags.cpp 800e54a302386f26f9b2f21edacad8840abbb42d
src/master/http.cpp d7afa2af989eebfc9039b3681f087ce570f601d5
src/master/master.hpp 2936d32c5e7b5b944bf16a1f261c7c54179900ec
src/master/master.cpp 40ce3e17fca88da689128bcf5d35fdddc396c011
src/master/quota_handler.cpp ac4da74686f524a66745b4a4e038c3d703ea1ee9
src/tests/master_quota_tests.cpp 81f0386d072d0176cb28eb229b274e424004f54c
src/tests/mesos.cpp 082e57bc73fad02de77e16e4b34451e6c0903038
src/tests/persistent_volume_endpoints_tests.cpp f0cce190abc90f0fae84d6c3db20e8215c2d8132
Diff: https://reviews.apache.org/r/39043/diff/
Testing
-------
make check
Thanks,
Alexander Rojas
Re: Review Request 39043: Added support for HTTP Authentication in
Mesos.
Posted by Alexander Rojas <al...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/39043/
-----------------------------------------------------------
(Updated Jan. 6, 2016, 2:35 p.m.)
Review request for mesos, Adam B, Bernd Mathiske, Isabel Jimenez, Jan Schlicht, and Till Toenshoff.
Changes
-------
Rebase
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 (updated)
-----
include/mesos/authentication/http/basic_authenticator_factory.hpp PRE-CREATION
src/authentication/http/basic_authenticator_factory.cpp PRE-CREATION
src/master/constants.hpp f1624e1ed5091f08f57d4382b344cab8b05ba840
src/master/constants.cpp 589f2ee55a24d7e8b9352a4c8f94a3785fd1bef4
src/master/flags.hpp 9af6c68eef6bcf39d5776809fab6c66dc95da6b2
src/master/flags.cpp 800e54a302386f26f9b2f21edacad8840abbb42d
src/master/http.cpp d7afa2af989eebfc9039b3681f087ce570f601d5
src/master/master.hpp 2936d32c5e7b5b944bf16a1f261c7c54179900ec
src/master/master.cpp 40ce3e17fca88da689128bcf5d35fdddc396c011
src/master/quota_handler.cpp ac4da74686f524a66745b4a4e038c3d703ea1ee9
src/tests/master_quota_tests.cpp 81f0386d072d0176cb28eb229b274e424004f54c
src/tests/mesos.cpp 082e57bc73fad02de77e16e4b34451e6c0903038
src/tests/persistent_volume_endpoints_tests.cpp f0cce190abc90f0fae84d6c3db20e8215c2d8132
Diff: https://reviews.apache.org/r/39043/diff/
Testing
-------
make check
Thanks,
Alexander Rojas
Re: Review Request 39043: Added support for HTTP Authentication in
Mesos.
Posted by Mesos ReviewBot <re...@mesos.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/39043/#review113062
-----------------------------------------------------------
Bad patch!
Reviews applied: [40539, 37999, 38000, 38094, 38950, 39043]
Failed command: ./support/apply-review.sh -n -r 39043
Error:
2016-01-06 13:09:59 URL:https://reviews.apache.org/r/39043/diff/raw/ [24594/24594] -> "39043.patch" [1]
error: patch failed: src/master/master.hpp:951
error: src/master/master.hpp: patch does not apply
- Mesos ReviewBot
On Jan. 6, 2016, 8:03 a.m., Alexander Rojas wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39043/
> -----------------------------------------------------------
>
> (Updated Jan. 6, 2016, 8:03 a.m.)
>
>
> Review request for mesos, Adam B, Bernd Mathiske, Isabel Jimenez, Jan Schlicht, 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
> -----
>
> include/mesos/authentication/http/basic_authenticator_factory.hpp PRE-CREATION
> src/authentication/http/basic_authenticator_factory.cpp PRE-CREATION
> src/master/constants.hpp f1624e1ed5091f08f57d4382b344cab8b05ba840
> src/master/constants.cpp 589f2ee55a24d7e8b9352a4c8f94a3785fd1bef4
> src/master/flags.hpp 9af6c68eef6bcf39d5776809fab6c66dc95da6b2
> src/master/flags.cpp 800e54a302386f26f9b2f21edacad8840abbb42d
> src/master/http.cpp deb0c8f9852dc0eec1e8c0ff35c684f982e8b110
> src/master/master.hpp 1cc5531de70bdc0bdce9835c4930dc774406fac9
> src/master/master.cpp 40ce3e17fca88da689128bcf5d35fdddc396c011
> src/master/quota_handler.cpp 1d84ef5767b3cb7cf870dd184aa2d9f030919498
> src/tests/mesos.cpp f4b0f82449c4b6a2b4b7b7f14518714485d5a13a
>
> Diff: https://reviews.apache.org/r/39043/diff/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Alexander Rojas
>
>
Re: Review Request 39043: Added support for HTTP Authentication in
Mesos.
Posted by Alexander Rojas <al...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/39043/
-----------------------------------------------------------
(Updated Jan. 6, 2016, 9:03 a.m.)
Review request for mesos, Adam B, Bernd Mathiske, Isabel Jimenez, Jan Schlicht, and Till Toenshoff.
Changes
-------
Ortographic corrections.
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 (updated)
-----
include/mesos/authentication/http/basic_authenticator_factory.hpp PRE-CREATION
src/authentication/http/basic_authenticator_factory.cpp PRE-CREATION
src/master/constants.hpp f1624e1ed5091f08f57d4382b344cab8b05ba840
src/master/constants.cpp 589f2ee55a24d7e8b9352a4c8f94a3785fd1bef4
src/master/flags.hpp 9af6c68eef6bcf39d5776809fab6c66dc95da6b2
src/master/flags.cpp 800e54a302386f26f9b2f21edacad8840abbb42d
src/master/http.cpp deb0c8f9852dc0eec1e8c0ff35c684f982e8b110
src/master/master.hpp 1cc5531de70bdc0bdce9835c4930dc774406fac9
src/master/master.cpp 40ce3e17fca88da689128bcf5d35fdddc396c011
src/master/quota_handler.cpp 1d84ef5767b3cb7cf870dd184aa2d9f030919498
src/tests/mesos.cpp f4b0f82449c4b6a2b4b7b7f14518714485d5a13a
Diff: https://reviews.apache.org/r/39043/diff/
Testing
-------
make check
Thanks,
Alexander Rojas
Re: Review Request 39043: Added support for HTTP Authentication in
Mesos.
Posted by Mesos ReviewBot <re...@mesos.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/39043/#review112944
-----------------------------------------------------------
Patch looks great!
Reviews applied: [40539, 37999, 38000, 38094, 38950, 39043]
Passed command: export OS=ubuntu:14.04;export CONFIGURATION="--verbose";export COMPILER=gcc; ./support/docker_build.sh
- Mesos ReviewBot
On Jan. 5, 2016, 4:20 p.m., Alexander Rojas wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39043/
> -----------------------------------------------------------
>
> (Updated Jan. 5, 2016, 4:20 p.m.)
>
>
> Review request for mesos, Adam B, Bernd Mathiske, Isabel Jimenez, Jan Schlicht, 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
> -----
>
> include/mesos/authentication/http/basic_authenticator_factory.hpp PRE-CREATION
> src/authentication/http/basic_authenticator_factory.cpp PRE-CREATION
> src/master/constants.hpp f1624e1ed5091f08f57d4382b344cab8b05ba840
> src/master/constants.cpp 589f2ee55a24d7e8b9352a4c8f94a3785fd1bef4
> src/master/flags.hpp 9af6c68eef6bcf39d5776809fab6c66dc95da6b2
> src/master/flags.cpp 800e54a302386f26f9b2f21edacad8840abbb42d
> src/master/http.cpp deb0c8f9852dc0eec1e8c0ff35c684f982e8b110
> src/master/master.hpp 1cc5531de70bdc0bdce9835c4930dc774406fac9
> src/master/master.cpp 40ce3e17fca88da689128bcf5d35fdddc396c011
> src/master/quota_handler.cpp 1d84ef5767b3cb7cf870dd184aa2d9f030919498
> src/tests/mesos.cpp f4b0f82449c4b6a2b4b7b7f14518714485d5a13a
>
> Diff: https://reviews.apache.org/r/39043/diff/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Alexander Rojas
>
>
Re: Review Request 39043: Added support for HTTP Authentication in
Mesos.
Posted by Alexander Rojas <al...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/39043/
-----------------------------------------------------------
(Updated Jan. 5, 2016, 5:20 p.m.)
Review request for mesos, Adam B, Bernd Mathiske, Isabel Jimenez, Jan Schlicht, and Till Toenshoff.
Changes
-------
Bernd's review.
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 (updated)
-----
include/mesos/authentication/http/basic_authenticator_factory.hpp PRE-CREATION
src/authentication/http/basic_authenticator_factory.cpp PRE-CREATION
src/master/constants.hpp f1624e1ed5091f08f57d4382b344cab8b05ba840
src/master/constants.cpp 589f2ee55a24d7e8b9352a4c8f94a3785fd1bef4
src/master/flags.hpp 9af6c68eef6bcf39d5776809fab6c66dc95da6b2
src/master/flags.cpp 800e54a302386f26f9b2f21edacad8840abbb42d
src/master/http.cpp deb0c8f9852dc0eec1e8c0ff35c684f982e8b110
src/master/master.hpp 1cc5531de70bdc0bdce9835c4930dc774406fac9
src/master/master.cpp 40ce3e17fca88da689128bcf5d35fdddc396c011
src/master/quota_handler.cpp 1d84ef5767b3cb7cf870dd184aa2d9f030919498
src/tests/mesos.cpp f4b0f82449c4b6a2b4b7b7f14518714485d5a13a
Diff: https://reviews.apache.org/r/39043/diff/
Testing
-------
make check
Thanks,
Alexander Rojas
Re: Review Request 39043: Added support for HTTP Authentication in
Mesos.
Posted by Alexander Rojas <al...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/39043/
-----------------------------------------------------------
(Updated Jan. 5, 2016, 12:18 p.m.)
Review request for mesos, Adam B, Bernd Mathiske, Isabel Jimenez, Jan Schlicht, and Till Toenshoff.
Changes
-------
Rebase.
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 (updated)
-----
src/master/constants.hpp f1624e1ed5091f08f57d4382b344cab8b05ba840
src/master/constants.cpp 589f2ee55a24d7e8b9352a4c8f94a3785fd1bef4
src/master/flags.hpp 9af6c68eef6bcf39d5776809fab6c66dc95da6b2
src/master/flags.cpp 800e54a302386f26f9b2f21edacad8840abbb42d
src/master/http.cpp deb0c8f9852dc0eec1e8c0ff35c684f982e8b110
src/master/master.hpp 1cc5531de70bdc0bdce9835c4930dc774406fac9
src/master/master.cpp 40ce3e17fca88da689128bcf5d35fdddc396c011
src/master/quota_handler.cpp 1d84ef5767b3cb7cf870dd184aa2d9f030919498
src/tests/mesos.cpp f4b0f82449c4b6a2b4b7b7f14518714485d5a13a
Diff: https://reviews.apache.org/r/39043/diff/
Testing
-------
make check
Thanks,
Alexander Rojas
Re: Review Request 39043: Added support for HTTP Authentication in
Mesos.
Posted by Alexander Rojas <al...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/39043/
-----------------------------------------------------------
(Updated Dec. 14, 2015, 4:02 p.m.)
Review request for mesos, Adam B, Bernd Mathiske, Isabel Jimenez, and Till Toenshoff.
Changes
-------
Bernd's review.
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 (updated)
-----
src/master/constants.hpp cc38dfcf57ecbc8555379084acbf2bfa4b3fc759
src/master/constants.cpp 98ea7c8f1e7c3057cfe941f82f1b71f7731a8f32
src/master/flags.hpp 9af6c68eef6bcf39d5776809fab6c66dc95da6b2
src/master/flags.cpp f9223edb33483cb5619e7dd75221f735306462c6
src/master/http.cpp 37fbcb93074fe189133161afaa28046dc2ad0731
src/master/master.hpp 9aa548aa6e159046c94e4ec96f631ea8b3bfd5d8
src/master/master.cpp 0d1482279c68f2a4a27dabaf28774769a5d515c4
src/master/quota_handler.cpp 11167879b2480d9c8dd6398ca39c479089ec2272
Diff: https://reviews.apache.org/r/39043/diff/
Testing
-------
make check
Thanks,
Alexander Rojas
Re: Review Request 39043: Added support for HTTP Authentication in
Mesos.
Posted by Alexander Rojas <al...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/39043/
-----------------------------------------------------------
(Updated Dec. 11, 2015, 3:06 p.m.)
Review request for mesos, Adam B, Bernd Mathiske, Isabel Jimenez, and Till Toenshoff.
Changes
-------
Rebase.
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 (updated)
-----
src/master/constants.hpp cc38dfcf57ecbc8555379084acbf2bfa4b3fc759
src/master/constants.cpp 98ea7c8f1e7c3057cfe941f82f1b71f7731a8f32
src/master/flags.hpp 9af6c68eef6bcf39d5776809fab6c66dc95da6b2
src/master/flags.cpp f9223edb33483cb5619e7dd75221f735306462c6
src/master/http.cpp 37fbcb93074fe189133161afaa28046dc2ad0731
src/master/master.hpp 9aa548aa6e159046c94e4ec96f631ea8b3bfd5d8
src/master/master.cpp 0d1482279c68f2a4a27dabaf28774769a5d515c4
src/master/quota_handler.cpp 11167879b2480d9c8dd6398ca39c479089ec2272
Diff: https://reviews.apache.org/r/39043/diff/
Testing
-------
make check
Thanks,
Alexander Rojas
Re: Review Request 39043: Added support for HTTP Authentication in
Mesos.
Posted by Alexander Rojas <al...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/39043/
-----------------------------------------------------------
(Updated Dec. 10, 2015, 3:34 p.m.)
Review request for mesos, Adam B, Bernd Mathiske, Isabel Jimenez, and Till Toenshoff.
Changes
-------
Updates for changes in previous reviews.
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 (updated)
-----
src/master/constants.hpp cc38dfcf57ecbc8555379084acbf2bfa4b3fc759
src/master/constants.cpp 98ea7c8f1e7c3057cfe941f82f1b71f7731a8f32
src/master/flags.hpp 9af6c68eef6bcf39d5776809fab6c66dc95da6b2
src/master/flags.cpp f9223edb33483cb5619e7dd75221f735306462c6
src/master/http.cpp 5d33138d60e684b23f07e1781de7991209d3e161
src/master/master.hpp 4683fa542a740f9a0b80fff7fbe0e63ec66266f2
src/master/master.cpp 7acdc0a5d23a606eac2f37f4b7dd021c5a4fceb7
Diff: https://reviews.apache.org/r/39043/diff/
Testing
-------
make check
Thanks,
Alexander Rojas
Re: Review Request 39043: Added support for HTTP Authentication in
Mesos.
Posted by Mesos ReviewBot <re...@mesos.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/39043/#review109358
-----------------------------------------------------------
Patch looks great!
Reviews applied: [40539, 37999, 38000, 38094, 38950, 39043]
Passed command: export OS=ubuntu:14.04;export CONFIGURATION="--verbose";export COMPILER=gcc; ./support/docker_build.sh
- Mesos ReviewBot
On Dec. 8, 2015, 3:53 p.m., Alexander Rojas wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39043/
> -----------------------------------------------------------
>
> (Updated Dec. 8, 2015, 3:53 p.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
>
>
Re: Review Request 39043: Added support for HTTP Authentication in
Mesos.
Posted by Alexander Rojas <al...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/39043/
-----------------------------------------------------------
(Updated Dec. 8, 2015, 4:53 p.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 (updated)
-----
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
Re: Review Request 39043: Added support for HTTP Authentication in
Mesos.
Posted by Alexander Rojas <al...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/39043/
-----------------------------------------------------------
(Updated Dec. 8, 2015, 3:38 p.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 (updated)
-----
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
Re: Review Request 39043: Added support for HTTP Authentication in
Mesos.
Posted by Alexander Rojas <al...@mesosphere.io>.
-----------------------------------------------------------
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.
Changes
-------
Forcing buildbot rebuild.
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 (updated)
-----
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
Re: Review Request 39043: Added support for HTTP Authentication in
Mesos.
Posted by Alexander Rojas <al...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/39043/
-----------------------------------------------------------
(Updated Dec. 7, 2015, 3:22 p.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 (updated)
-----
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