Re: Review Request 44515: Added agent flags for HTTP authentication.

[Greg Mann]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/44515/
---

(Updated March 23, 2016, 9:38 a.m.)


Review request for mesos, Adam B, Alexander Rojas, Joerg Schad, and Till 
Toenshoff.


Changes
---

Addressed comment, cleaned up `if ... else` block.


Bugs: MESOS-4849
https://issues.apache.org/jira/browse/MESOS-4849


Repository: mesos


Description
---

Added agent flags for HTTP authentication.

Three command-line flags have been added to the agent to enable HTTP 
authentication: `--authenticate_http`, `--http_credentials`, and 
`--http_authenticators`.


Diffs (updated)
-

  src/slave/constants.hpp 4189c07dbad754e51f1a067ecbb4c99ae42a386f 
  src/slave/flags.hpp 3067c128f1fded4f130f4d45f191584c2f30ad9c 
  src/slave/flags.cpp ce028825ad99f54a231b4b18dde277b63aa0525c 
  src/slave/slave.cpp 840534ff0687e82ed063c386e36bbabada230697 
  src/tests/mesos.cpp 90aef6bfe619dc0acdb4ccba6a7180482dd13ce5 

Diff: https://reviews.apache.org/r/44515/diff/


Testing
---

`sudo make check` was used to test on both OSX and Ubuntu 14.04.


Thanks,

Greg Mann

Re: Review Request 44515: Added agent flags for HTTP authentication.

[Adam B]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/44515/#review124996
---


Fix it, then Ship it!





src/slave/slave.cpp (line 358)


= NULL


- Adam B


On March 23, 2016, 1:51 a.m., Greg Mann wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44515/
> ---
> 
> (Updated March 23, 2016, 1:51 a.m.)
> 
> 
> Review request for mesos, Adam B, Alexander Rojas, Joerg Schad, and Till 
> Toenshoff.
> 
> 
> Bugs: MESOS-4849
> https://issues.apache.org/jira/browse/MESOS-4849
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added agent flags for HTTP authentication.
> 
> Three command-line flags have been added to the agent to enable HTTP 
> authentication: `--authenticate_http`, `--http_credentials`, and 
> `--http_authenticators`.
> 
> 
> Diffs
> -
> 
>   src/slave/constants.hpp 4189c07dbad754e51f1a067ecbb4c99ae42a386f 
>   src/slave/flags.hpp feb095da4521f678c96f4cc53bdfda262d350388 
>   src/slave/flags.cpp b77afa956834bb5b1f85301d7a5f386ab9da41e3 
>   src/slave/slave.cpp 840534ff0687e82ed063c386e36bbabada230697 
>   src/tests/mesos.cpp 90aef6bfe619dc0acdb4ccba6a7180482dd13ce5 
> 
> Diff: https://reviews.apache.org/r/44515/diff/
> 
> 
> Testing
> ---
> 
> `sudo make check` was used to test on both OSX and Ubuntu 14.04.
> 
> 
> Thanks,
> 
> Greg Mann
> 
>

Re: Review Request 44515: Added agent flags for HTTP authentication.

[Greg Mann]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/44515/
---

(Updated March 23, 2016, 8:51 a.m.)


Review request for mesos, Adam B, Alexander Rojas, Joerg Schad, and Till 
Toenshoff.


Changes
---

Addressed comment.


Bugs: MESOS-4849
https://issues.apache.org/jira/browse/MESOS-4849


Repository: mesos


Description
---

Added agent flags for HTTP authentication.

Three command-line flags have been added to the agent to enable HTTP 
authentication: `--authenticate_http`, `--http_credentials`, and 
`--http_authenticators`.


Diffs (updated)
-

  src/slave/constants.hpp 4189c07dbad754e51f1a067ecbb4c99ae42a386f 
  src/slave/flags.hpp feb095da4521f678c96f4cc53bdfda262d350388 
  src/slave/flags.cpp b77afa956834bb5b1f85301d7a5f386ab9da41e3 
  src/slave/slave.cpp 840534ff0687e82ed063c386e36bbabada230697 
  src/tests/mesos.cpp 90aef6bfe619dc0acdb4ccba6a7180482dd13ce5 

Diff: https://reviews.apache.org/r/44515/diff/


Testing
---

`sudo make check` was used to test on both OSX and Ubuntu 14.04.


Thanks,

Greg Mann

Re: Review Request 44515: Added agent flags for HTTP authentication.

[Adam B]

> On March 23, 2016, 12:30 a.m., Adam B wrote:
> > src/slave/slave.cpp, lines 413-415
> > 
> >
> > Who says custom authenticators can't use that flag? Make this a WARN 
> > instead

Oh, because we don't pass Flags to authenticator modules.. Nevermind, dropping.


- Adam


---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/44515/#review124982
---


On March 21, 2016, 8:42 a.m., Greg Mann wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44515/
> ---
> 
> (Updated March 21, 2016, 8:42 a.m.)
> 
> 
> Review request for mesos, Adam B, Alexander Rojas, Joerg Schad, and Till 
> Toenshoff.
> 
> 
> Bugs: MESOS-4849
> https://issues.apache.org/jira/browse/MESOS-4849
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added agent flags for HTTP authentication.
> 
> Three command-line flags have been added to the agent to enable HTTP 
> authentication: `--authenticate_http`, `--http_credentials`, and 
> `--http_authenticators`.
> 
> 
> Diffs
> -
> 
>   src/slave/constants.hpp 4189c07dbad754e51f1a067ecbb4c99ae42a386f 
>   src/slave/flags.hpp feb095da4521f678c96f4cc53bdfda262d350388 
>   src/slave/flags.cpp b77afa956834bb5b1f85301d7a5f386ab9da41e3 
>   src/slave/slave.cpp 840534ff0687e82ed063c386e36bbabada230697 
>   src/tests/mesos.cpp 90aef6bfe619dc0acdb4ccba6a7180482dd13ce5 
> 
> Diff: https://reviews.apache.org/r/44515/diff/
> 
> 
> Testing
> ---
> 
> `sudo make check` was used to test on both OSX and Ubuntu 14.04.
> 
> 
> Thanks,
> 
> Greg Mann
> 
>

Re: Review Request 44515: Added agent flags for HTTP authentication.

[Adam B]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/44515/#review124982
---


Fix it, then Ship it!




Looks good. Just a couple minor-but-functional changes to suggest. Let me know 
what you think.


src/slave/slave.cpp (lines 374 - 376)


} else {



src/slave/slave.cpp (lines 397 - 399)


Who says custom authenticators can't use that flag? Make this a WARN instead


- Adam B


On March 21, 2016, 8:42 a.m., Greg Mann wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44515/
> ---
> 
> (Updated March 21, 2016, 8:42 a.m.)
> 
> 
> Review request for mesos, Adam B, Alexander Rojas, Joerg Schad, and Till 
> Toenshoff.
> 
> 
> Bugs: MESOS-4849
> https://issues.apache.org/jira/browse/MESOS-4849
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added agent flags for HTTP authentication.
> 
> Three command-line flags have been added to the agent to enable HTTP 
> authentication: `--authenticate_http`, `--http_credentials`, and 
> `--http_authenticators`.
> 
> 
> Diffs
> -
> 
>   src/slave/constants.hpp 4189c07dbad754e51f1a067ecbb4c99ae42a386f 
>   src/slave/flags.hpp feb095da4521f678c96f4cc53bdfda262d350388 
>   src/slave/flags.cpp b77afa956834bb5b1f85301d7a5f386ab9da41e3 
>   src/slave/slave.cpp 840534ff0687e82ed063c386e36bbabada230697 
>   src/tests/mesos.cpp 90aef6bfe619dc0acdb4ccba6a7180482dd13ce5 
> 
> Diff: https://reviews.apache.org/r/44515/diff/
> 
> 
> Testing
> ---
> 
> `sudo make check` was used to test on both OSX and Ubuntu 14.04.
> 
> 
> Thanks,
> 
> Greg Mann
> 
>

Re: Review Request 44515: Added agent flags for HTTP authentication.

[Greg Mann]

> On March 21, 2016, 9:55 a.m., Adam B wrote:
> > Maybe all the http auth flag validation code should just go inside the 
> > `--authenticate_http` block, and you'll EXIT/WARN if they specify any 
> > related flags without enabling http authn.

Since there is a default value for `--http_authenticators`, it makes sense to 
leave that flag's validation code outside of the `if (flags.authenticate_http)` 
block, but I've moved everything else inside. This also means that we don't 
need to wrap `httpCredentials` and `httpAuthenticator` in `Option`s anymore. I 
added the extra EXITs, so we always fail hard if we observe an unexpected flag.


- Greg


---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/44515/#review124519
---


On March 21, 2016, 3:42 p.m., Greg Mann wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44515/
> ---
> 
> (Updated March 21, 2016, 3:42 p.m.)
> 
> 
> Review request for mesos, Adam B, Alexander Rojas, Joerg Schad, and Till 
> Toenshoff.
> 
> 
> Bugs: MESOS-4849
> https://issues.apache.org/jira/browse/MESOS-4849
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added agent flags for HTTP authentication.
> 
> Three command-line flags have been added to the agent to enable HTTP 
> authentication: `--authenticate_http`, `--http_credentials`, and 
> `--http_authenticators`.
> 
> 
> Diffs
> -
> 
>   src/slave/constants.hpp 4189c07dbad754e51f1a067ecbb4c99ae42a386f 
>   src/slave/flags.hpp feb095da4521f678c96f4cc53bdfda262d350388 
>   src/slave/flags.cpp b77afa956834bb5b1f85301d7a5f386ab9da41e3 
>   src/slave/slave.cpp 840534ff0687e82ed063c386e36bbabada230697 
>   src/tests/mesos.cpp 90aef6bfe619dc0acdb4ccba6a7180482dd13ce5 
> 
> Diff: https://reviews.apache.org/r/44515/diff/
> 
> 
> Testing
> ---
> 
> `sudo make check` was used to test on both OSX and Ubuntu 14.04.
> 
> 
> Thanks,
> 
> Greg Mann
> 
>

Re: Review Request 44515: Added agent flags for HTTP authentication.

[Greg Mann]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/44515/
---

(Updated March 21, 2016, 3:42 p.m.)


Review request for mesos, Adam B, Alexander Rojas, Joerg Schad, and Till 
Toenshoff.


Changes
---

Addressed comments.


Bugs: MESOS-4849
https://issues.apache.org/jira/browse/MESOS-4849


Repository: mesos


Description
---

Added agent flags for HTTP authentication.

Three command-line flags have been added to the agent to enable HTTP 
authentication: `--authenticate_http`, `--http_credentials`, and 
`--http_authenticators`.


Diffs (updated)
-

  src/slave/constants.hpp 4189c07dbad754e51f1a067ecbb4c99ae42a386f 
  src/slave/flags.hpp feb095da4521f678c96f4cc53bdfda262d350388 
  src/slave/flags.cpp b77afa956834bb5b1f85301d7a5f386ab9da41e3 
  src/slave/slave.cpp 840534ff0687e82ed063c386e36bbabada230697 
  src/tests/mesos.cpp 90aef6bfe619dc0acdb4ccba6a7180482dd13ce5 

Diff: https://reviews.apache.org/r/44515/diff/


Testing
---

`sudo make check` was used to test on both OSX and Ubuntu 14.04.


Thanks,

Greg Mann

Re: Review Request 44515: Added agent flags for HTTP authentication.

[Adam B]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/44515/#review124519
---



Maybe all the http auth flag validation code should just go inside the 
`--authenticate_http` block, and you'll EXIT/WARN if they specify any related 
flags without enabling http authn.


src/slave/slave.cpp (line 395)


Should we EXIT/WARN if a custom authenticator is used, but 
--http_credentials is specified (and ignored)?



src/slave/slave.cpp (lines 407 - 409)


Should we also EXIT/WARN if `--authenticate_http` is not set, and a custom 
authenticator is set: `--http_authenticators != DEFAULT_HTTP_AUTHENTICATOR`?


- Adam B


On March 20, 2016, 9:50 p.m., Greg Mann wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44515/
> ---
> 
> (Updated March 20, 2016, 9:50 p.m.)
> 
> 
> Review request for mesos, Adam B, Alexander Rojas, Joerg Schad, and Till 
> Toenshoff.
> 
> 
> Bugs: MESOS-4849
> https://issues.apache.org/jira/browse/MESOS-4849
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added agent flags for HTTP authentication.
> 
> Three command-line flags have been added to the agent to enable HTTP 
> authentication: `--authenticate_http`, `--http_credentials`, and 
> `--http_authenticators`.
> 
> 
> Diffs
> -
> 
>   src/slave/constants.hpp 4189c07dbad754e51f1a067ecbb4c99ae42a386f 
>   src/slave/flags.hpp feb095da4521f678c96f4cc53bdfda262d350388 
>   src/slave/flags.cpp b77afa956834bb5b1f85301d7a5f386ab9da41e3 
>   src/slave/slave.cpp 840534ff0687e82ed063c386e36bbabada230697 
>   src/tests/mesos.cpp 90aef6bfe619dc0acdb4ccba6a7180482dd13ce5 
> 
> Diff: https://reviews.apache.org/r/44515/diff/
> 
> 
> Testing
> ---
> 
> `sudo make check` was used to test on both OSX and Ubuntu 14.04.
> 
> 
> Thanks,
> 
> Greg Mann
> 
>

Re: Review Request 44515: Added agent flags for HTTP authentication.

[Adam B]

> On March 19, 2016, 3:03 a.m., Joerg Schad wrote:
> > src/slave/slave.cpp, line 373
> > 
> >
> > Do we actually have to get the authenticator above if this flag is not 
> > set?
> 
> Greg Mann wrote:
> The code directly above this line performs error checking on the value of 
> the `--http_authenticators` flag, which is necessary before this `if` block 
> uses that input. However, we also have the code that reads the 
> `--http_credentials` flag, which only needs to be executed if the user is 
> using the basic HTTP authenticator, so I've moved that code into the 
> appropriate scope.
> 
> Greg Mann wrote:
> However, moving this code means that we won't perform error checking on 
> the value of the `--http_credentials` flag when `--authenticate_http` is not 
> set. Arguably, we should always fail hard and tell a user if they've given us 
> invalid input for a flag. I'm going to move this code back to its original 
> location for now; let me know what you think.
> 
> We should also probably fail hard if a user attempts to set 
> `--http_credentials` when `--authenticate_http` is not set. I've added an 
> `else if` to handle this case just after the `if` block associated with this 
> comment.

So now we EXIT if the user specifies `--http_credentials` but not 
`--authenticate_http`, but we'll EXIT for invalid credentials (or authenticator 
name) first. I suppose since we're only showing 1 error at a time, it doesn't 
matter what order we tell them to fix it in (i.e. which error we exit with 
first).

But I do think that all the code referencing `httpAuthenticator` (including its 
definition) could go inside the `if (flags.authenticate_http)` block.


- Adam


---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/44515/#review124392
---


On March 20, 2016, 9:50 p.m., Greg Mann wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44515/
> ---
> 
> (Updated March 20, 2016, 9:50 p.m.)
> 
> 
> Review request for mesos, Adam B, Alexander Rojas, Joerg Schad, and Till 
> Toenshoff.
> 
> 
> Bugs: MESOS-4849
> https://issues.apache.org/jira/browse/MESOS-4849
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added agent flags for HTTP authentication.
> 
> Three command-line flags have been added to the agent to enable HTTP 
> authentication: `--authenticate_http`, `--http_credentials`, and 
> `--http_authenticators`.
> 
> 
> Diffs
> -
> 
>   src/slave/constants.hpp 4189c07dbad754e51f1a067ecbb4c99ae42a386f 
>   src/slave/flags.hpp feb095da4521f678c96f4cc53bdfda262d350388 
>   src/slave/flags.cpp b77afa956834bb5b1f85301d7a5f386ab9da41e3 
>   src/slave/slave.cpp 840534ff0687e82ed063c386e36bbabada230697 
>   src/tests/mesos.cpp 90aef6bfe619dc0acdb4ccba6a7180482dd13ce5 
> 
> Diff: https://reviews.apache.org/r/44515/diff/
> 
> 
> Testing
> ---
> 
> `sudo make check` was used to test on both OSX and Ubuntu 14.04.
> 
> 
> Thanks,
> 
> Greg Mann
> 
>

Re: Review Request 44515: Added agent flags for HTTP authentication.

[Greg Mann]

> On March 19, 2016, 10:03 a.m., Joerg Schad wrote:
> > src/slave/slave.cpp, line 373
> > 
> >
> > Do we actually have to get the authenticator above if this flag is not 
> > set?
> 
> Greg Mann wrote:
> The code directly above this line performs error checking on the value of 
> the `--http_authenticators` flag, which is necessary before this `if` block 
> uses that input. However, we also have the code that reads the 
> `--http_credentials` flag, which only needs to be executed if the user is 
> using the basic HTTP authenticator, so I've moved that code into the 
> appropriate scope.

However, moving this code means that we won't perform error checking on the 
value of the `--http_credentials` flag when `--authenticate_http` is not set. 
Arguably, we should always fail hard and tell a user if they've given us 
invalid input for a flag. I'm going to move this code back to its original 
location for now; let me know what you think.

We should also probably fail hard if a user attempts to set 
`--http_credentials` when `--authenticate_http` is not set. I've added an `else 
if` to handle this case just after the `if` block associated with this comment.


- Greg


---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/44515/#review124392
---


On March 21, 2016, 4:50 a.m., Greg Mann wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44515/
> ---
> 
> (Updated March 21, 2016, 4:50 a.m.)
> 
> 
> Review request for mesos, Adam B, Alexander Rojas, Joerg Schad, and Till 
> Toenshoff.
> 
> 
> Bugs: MESOS-4849
> https://issues.apache.org/jira/browse/MESOS-4849
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added agent flags for HTTP authentication.
> 
> Three command-line flags have been added to the agent to enable HTTP 
> authentication: `--authenticate_http`, `--http_credentials`, and 
> `--http_authenticators`.
> 
> 
> Diffs
> -
> 
>   src/slave/constants.hpp 4189c07dbad754e51f1a067ecbb4c99ae42a386f 
>   src/slave/flags.hpp feb095da4521f678c96f4cc53bdfda262d350388 
>   src/slave/flags.cpp b77afa956834bb5b1f85301d7a5f386ab9da41e3 
>   src/slave/slave.cpp 840534ff0687e82ed063c386e36bbabada230697 
>   src/tests/mesos.cpp 90aef6bfe619dc0acdb4ccba6a7180482dd13ce5 
> 
> Diff: https://reviews.apache.org/r/44515/diff/
> 
> 
> Testing
> ---
> 
> `sudo make check` was used to test on both OSX and Ubuntu 14.04.
> 
> 
> Thanks,
> 
> Greg Mann
> 
>

Re: Review Request 44515: Added agent flags for HTTP authentication.

[Greg Mann]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/44515/
---

(Updated March 21, 2016, 4:50 a.m.)


Review request for mesos, Adam B, Alexander Rojas, Joerg Schad, and Till 
Toenshoff.


Bugs: MESOS-4849
https://issues.apache.org/jira/browse/MESOS-4849


Repository: mesos


Description
---

Added agent flags for HTTP authentication.

Three command-line flags have been added to the agent to enable HTTP 
authentication: `--authenticate_http`, `--http_credentials`, and 
`--http_authenticators`.


Diffs (updated)
-

  src/slave/constants.hpp 4189c07dbad754e51f1a067ecbb4c99ae42a386f 
  src/slave/flags.hpp feb095da4521f678c96f4cc53bdfda262d350388 
  src/slave/flags.cpp b77afa956834bb5b1f85301d7a5f386ab9da41e3 
  src/slave/slave.cpp 840534ff0687e82ed063c386e36bbabada230697 
  src/tests/mesos.cpp 90aef6bfe619dc0acdb4ccba6a7180482dd13ce5 

Diff: https://reviews.apache.org/r/44515/diff/


Testing
---

`sudo make check` was used to test on both OSX and Ubuntu 14.04.


Thanks,

Greg Mann

Re: Review Request 44515: Added agent flags for HTTP authentication.

[Greg Mann]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/44515/
---

(Updated March 21, 2016, 4:24 a.m.)


Review request for mesos, Adam B, Alexander Rojas, Joerg Schad, and Till 
Toenshoff.


Changes
---

Addressed comment.


Bugs: MESOS-4849
https://issues.apache.org/jira/browse/MESOS-4849


Repository: mesos


Description
---

Added agent flags for HTTP authentication.

Three command-line flags have been added to the agent to enable HTTP 
authentication: `--authenticate_http`, `--http_credentials`, and 
`--http_authenticators`.


Diffs (updated)
-

  src/slave/constants.hpp 4189c07dbad754e51f1a067ecbb4c99ae42a386f 
  src/slave/flags.hpp feb095da4521f678c96f4cc53bdfda262d350388 
  src/slave/flags.cpp b77afa956834bb5b1f85301d7a5f386ab9da41e3 
  src/slave/slave.cpp 840534ff0687e82ed063c386e36bbabada230697 
  src/tests/mesos.cpp 90aef6bfe619dc0acdb4ccba6a7180482dd13ce5 

Diff: https://reviews.apache.org/r/44515/diff/


Testing
---

`sudo make check` was used to test on both OSX and Ubuntu 14.04.


Thanks,

Greg Mann

Re: Review Request 44515: Added agent flags for HTTP authentication.

[Greg Mann]

> On March 19, 2016, 10:03 a.m., Joerg Schad wrote:
> > src/slave/slave.cpp, line 373
> > 
> >
> > Do we actually have to get the authenticator above if this flag is not 
> > set?

The code directly above this line performs error checking on the value of the 
`--http_authenticators` flag, which is necessary before this `if` block uses 
that input. However, we also have the code that reads the `--http_credentials` 
flag, which only needs to be executed if the user is using the basic HTTP 
authenticator, so I've moved that code into the appropriate scope.


- Greg


---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/44515/#review124392
---


On March 21, 2016, 4:24 a.m., Greg Mann wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44515/
> ---
> 
> (Updated March 21, 2016, 4:24 a.m.)
> 
> 
> Review request for mesos, Adam B, Alexander Rojas, Joerg Schad, and Till 
> Toenshoff.
> 
> 
> Bugs: MESOS-4849
> https://issues.apache.org/jira/browse/MESOS-4849
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added agent flags for HTTP authentication.
> 
> Three command-line flags have been added to the agent to enable HTTP 
> authentication: `--authenticate_http`, `--http_credentials`, and 
> `--http_authenticators`.
> 
> 
> Diffs
> -
> 
>   src/slave/constants.hpp 4189c07dbad754e51f1a067ecbb4c99ae42a386f 
>   src/slave/flags.hpp feb095da4521f678c96f4cc53bdfda262d350388 
>   src/slave/flags.cpp b77afa956834bb5b1f85301d7a5f386ab9da41e3 
>   src/slave/slave.cpp 840534ff0687e82ed063c386e36bbabada230697 
>   src/tests/mesos.cpp 90aef6bfe619dc0acdb4ccba6a7180482dd13ce5 
> 
> Diff: https://reviews.apache.org/r/44515/diff/
> 
> 
> Testing
> ---
> 
> `sudo make check` was used to test on both OSX and Ubuntu 14.04.
> 
> 
> Thanks,
> 
> Greg Mann
> 
>

Re: Review Request 44515: Added agent flags for HTTP authentication.

[Greg Mann]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/44515/
---

(Updated March 20, 2016, 6:57 p.m.)


Review request for mesos, Adam B, Alexander Rojas, Joerg Schad, and Till 
Toenshoff.


Changes
---

Addressed comments.


Bugs: MESOS-4849
https://issues.apache.org/jira/browse/MESOS-4849


Repository: mesos


Description
---

Added agent flags for HTTP authentication.

Three command-line flags have been added to the agent to enable HTTP 
authentication: `--authenticate_http`, `--http_credentials`, and 
`--http_authenticators`.


Diffs (updated)
-

  src/slave/constants.hpp 4189c07dbad754e51f1a067ecbb4c99ae42a386f 
  src/slave/flags.hpp feb095da4521f678c96f4cc53bdfda262d350388 
  src/slave/flags.cpp b77afa956834bb5b1f85301d7a5f386ab9da41e3 
  src/slave/slave.cpp 840534ff0687e82ed063c386e36bbabada230697 
  src/tests/mesos.cpp 90aef6bfe619dc0acdb4ccba6a7180482dd13ce5 

Diff: https://reviews.apache.org/r/44515/diff/


Testing
---

`sudo make check` was used to test on both OSX and Ubuntu 14.04.


Thanks,

Greg Mann

Re: Review Request 44515: Added agent flags for HTTP authentication.

[Greg Mann]

> On March 19, 2016, 11:39 a.m., Joerg Schad wrote:
> > src/slave/flags.cpp, line 682
> > 
> >
> > One part I don't like is that the same flag on the master has a 
> > different name credentials.
> > I would suggest to deprecate (in another patch) to deprecate  masters 
> > credentials flag and introduce a new one for that. 
> > Already have a local patch for that, just interested in your opinion 
> > first.

Yes, the difference in naming is unfortunate. However, the master's 
`--credentials` flag is used for several purposes: HTTP authentication, 
framework authentication, and slave authentication, so I think that the more 
general name "credentials" makes sense for that reason.


- Greg


---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/44515/#review124401
---


On March 20, 2016, 5:27 p.m., Greg Mann wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44515/
> ---
> 
> (Updated March 20, 2016, 5:27 p.m.)
> 
> 
> Review request for mesos, Adam B, Alexander Rojas, Joerg Schad, and Till 
> Toenshoff.
> 
> 
> Bugs: MESOS-4849
> https://issues.apache.org/jira/browse/MESOS-4849
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added agent flags for HTTP authentication.
> 
> Three command-line flags have been added to the agent to enable HTTP 
> authentication: `--authenticate_http`, `--http_credentials`, and 
> `--http_authenticators`.
> 
> 
> Diffs
> -
> 
>   src/slave/constants.hpp 4189c07dbad754e51f1a067ecbb4c99ae42a386f 
>   src/slave/flags.hpp feb095da4521f678c96f4cc53bdfda262d350388 
>   src/slave/flags.cpp b77afa956834bb5b1f85301d7a5f386ab9da41e3 
>   src/slave/slave.cpp 840534ff0687e82ed063c386e36bbabada230697 
>   src/tests/mesos.cpp 90aef6bfe619dc0acdb4ccba6a7180482dd13ce5 
> 
> Diff: https://reviews.apache.org/r/44515/diff/
> 
> 
> Testing
> ---
> 
> `sudo make check` was used to test on both OSX and Ubuntu 14.04.
> 
> 
> Thanks,
> 
> Greg Mann
> 
>

Re: Review Request 44515: Added agent flags for HTTP authentication.

[Greg Mann]

> On March 19, 2016, 9:16 a.m., Adam B wrote:
> > src/tests/mesos.cpp, line 177
> > 
> >
> > Why use `os::getcwd()` instead of `directory.get()` like "credential" 
> > and "fetch" above? Seems like `CreateSlaveFlags` wanted to use a temporary 
> > directory removed later.
> > I'm not convinced these two are equivalent.

Whoops - thanks for catching this Adam!!! You are correct, these aren't 
equivalent and this was in fact causing the race that I had to patch up :-) 
Using `directory.get()` gives us a different temporary directory for each of 
the agents created in `ResourceOffersTest.ResourceOfferWithMultipleSlaves`, 
while using `os::getcwd()` uses a common directory for all of them, which 
allows them to step on each others' toes when they're being spun up.

Changing this to `directory.get()` solves the problem, and is clearly what I 
should be doing here. Patch has been updated.


- Greg


---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/44515/#review124387
---


On March 20, 2016, 5:27 p.m., Greg Mann wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44515/
> ---
> 
> (Updated March 20, 2016, 5:27 p.m.)
> 
> 
> Review request for mesos, Adam B, Alexander Rojas, Joerg Schad, and Till 
> Toenshoff.
> 
> 
> Bugs: MESOS-4849
> https://issues.apache.org/jira/browse/MESOS-4849
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added agent flags for HTTP authentication.
> 
> Three command-line flags have been added to the agent to enable HTTP 
> authentication: `--authenticate_http`, `--http_credentials`, and 
> `--http_authenticators`.
> 
> 
> Diffs
> -
> 
>   src/slave/constants.hpp 4189c07dbad754e51f1a067ecbb4c99ae42a386f 
>   src/slave/flags.hpp feb095da4521f678c96f4cc53bdfda262d350388 
>   src/slave/flags.cpp b77afa956834bb5b1f85301d7a5f386ab9da41e3 
>   src/slave/slave.cpp 840534ff0687e82ed063c386e36bbabada230697 
>   src/tests/mesos.cpp 90aef6bfe619dc0acdb4ccba6a7180482dd13ce5 
> 
> Diff: https://reviews.apache.org/r/44515/diff/
> 
> 
> Testing
> ---
> 
> `sudo make check` was used to test on both OSX and Ubuntu 14.04.
> 
> 
> Thanks,
> 
> Greg Mann
> 
>

Re: Review Request 44515: Added agent flags for HTTP authentication.

[Greg Mann]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/44515/
---

(Updated March 20, 2016, 5:27 p.m.)


Review request for mesos, Adam B, Alexander Rojas, Joerg Schad, and Till 
Toenshoff.


Changes
---

Addressed comment.


Bugs: MESOS-4849
https://issues.apache.org/jira/browse/MESOS-4849


Repository: mesos


Description
---

Added agent flags for HTTP authentication.

Three command-line flags have been added to the agent to enable HTTP 
authentication: `--authenticate_http`, `--http_credentials`, and 
`--http_authenticators`.


Diffs (updated)
-

  src/slave/constants.hpp 4189c07dbad754e51f1a067ecbb4c99ae42a386f 
  src/slave/flags.hpp feb095da4521f678c96f4cc53bdfda262d350388 
  src/slave/flags.cpp b77afa956834bb5b1f85301d7a5f386ab9da41e3 
  src/slave/slave.cpp 840534ff0687e82ed063c386e36bbabada230697 
  src/tests/mesos.cpp 90aef6bfe619dc0acdb4ccba6a7180482dd13ce5 

Diff: https://reviews.apache.org/r/44515/diff/


Testing
---

`sudo make check` was used to test on both OSX and Ubuntu 14.04.


Thanks,

Greg Mann

Re: Review Request 44515: Added agent flags for HTTP authentication.

[Adam B]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/44515/#review124157
---


Fix it, then Ship it!




Only minor nits


src/slave/flags.cpp (line 678)


... unauthenticated requests to HTTP endpoints are also allowed.
(otherwise it could be read as "requests to unauthenticated HTTP endpoints 
are also allowed")



src/slave/flags.cpp (lines 683 - 684)


"Path to a JSON-formatted file containing credentials used to authenticate 
HTTP endpoints on the slave."



src/slave/flags.cpp (line 686)


You can just say "Example:", since you don't have a "Text file Example" too.



src/slave/slave.cpp (line 338)


`s/_http_credentials/credentials/`
At the very least, you should use trailing underscore here instead of 
leading underscore. But you have the opportunity to not use a trailing 
underscore at all, so take it.


- Adam B


On March 17, 2016, 12:41 p.m., Greg Mann wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44515/
> ---
> 
> (Updated March 17, 2016, 12:41 p.m.)
> 
> 
> Review request for mesos, Adam B, Alexander Rojas, Joerg Schad, and Till 
> Toenshoff.
> 
> 
> Bugs: MESOS-4849
> https://issues.apache.org/jira/browse/MESOS-4849
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added agent flags for HTTP authentication.
> 
> Three command-line flags have been added to the agent to enable HTTP 
> authentication: `--authenticate_http`, `--http_credentials`, and 
> `--http_authenticators`.
> 
> 
> Diffs
> -
> 
>   src/slave/constants.hpp 4189c07dbad754e51f1a067ecbb4c99ae42a386f 
>   src/slave/flags.hpp feb095da4521f678c96f4cc53bdfda262d350388 
>   src/slave/flags.cpp b77afa956834bb5b1f85301d7a5f386ab9da41e3 
>   src/slave/slave.cpp 840534ff0687e82ed063c386e36bbabada230697 
>   src/tests/mesos.cpp 90aef6bfe619dc0acdb4ccba6a7180482dd13ce5 
> 
> Diff: https://reviews.apache.org/r/44515/diff/
> 
> 
> Testing
> ---
> 
> `sudo make check` was used to test on both OSX and Ubuntu 14.04.
> 
> 
> Thanks,
> 
> Greg Mann
> 
>

Re: Review Request 44515: Added agent flags for HTTP authentication.

[Joerg Schad]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/44515/#review124401
---




src/slave/flags.cpp (line 681)


One part I don't like is that the same flag on the master has a different 
name credentials.
I would suggest to deprecate (in another patch) to deprecate  masters 
credentials flag and introduce a new one for that. 
Already have a local patch for that, just interested in your opinion first.


- Joerg Schad


On March 18, 2016, 6:32 p.m., Greg Mann wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44515/
> ---
> 
> (Updated March 18, 2016, 6:32 p.m.)
> 
> 
> Review request for mesos, Adam B, Alexander Rojas, Joerg Schad, and Till 
> Toenshoff.
> 
> 
> Bugs: MESOS-4849
> https://issues.apache.org/jira/browse/MESOS-4849
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added agent flags for HTTP authentication.
> 
> Three command-line flags have been added to the agent to enable HTTP 
> authentication: `--authenticate_http`, `--http_credentials`, and 
> `--http_authenticators`.
> 
> 
> Diffs
> -
> 
>   src/slave/constants.hpp 4189c07dbad754e51f1a067ecbb4c99ae42a386f 
>   src/slave/flags.hpp feb095da4521f678c96f4cc53bdfda262d350388 
>   src/slave/flags.cpp b77afa956834bb5b1f85301d7a5f386ab9da41e3 
>   src/slave/slave.cpp 840534ff0687e82ed063c386e36bbabada230697 
>   src/tests/mesos.cpp 90aef6bfe619dc0acdb4ccba6a7180482dd13ce5 
> 
> Diff: https://reviews.apache.org/r/44515/diff/
> 
> 
> Testing
> ---
> 
> `sudo make check` was used to test on both OSX and Ubuntu 14.04.
> 
> 
> Thanks,
> 
> Greg Mann
> 
>

Re: Review Request 44515: Added agent flags for HTTP authentication.

[Joerg Schad]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/44515/#review124392
---




src/slave/flags.cpp (line 685)


s/could/can ?
Being more native than me feel free to drop.



src/slave/slave.cpp (line 320)


Can we add a comment here similar as below.
// Load credential for master/slave authentication.



src/slave/slave.cpp (line 373)


Do we actually have to get the authenticator above if this flag is not set?



src/tests/mesos.cpp (line 151)


/credential/ credential for master/agent authentication


- Joerg Schad


On March 18, 2016, 6:32 p.m., Greg Mann wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44515/
> ---
> 
> (Updated March 18, 2016, 6:32 p.m.)
> 
> 
> Review request for mesos, Adam B, Alexander Rojas, Joerg Schad, and Till 
> Toenshoff.
> 
> 
> Bugs: MESOS-4849
> https://issues.apache.org/jira/browse/MESOS-4849
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added agent flags for HTTP authentication.
> 
> Three command-line flags have been added to the agent to enable HTTP 
> authentication: `--authenticate_http`, `--http_credentials`, and 
> `--http_authenticators`.
> 
> 
> Diffs
> -
> 
>   src/slave/constants.hpp 4189c07dbad754e51f1a067ecbb4c99ae42a386f 
>   src/slave/flags.hpp feb095da4521f678c96f4cc53bdfda262d350388 
>   src/slave/flags.cpp b77afa956834bb5b1f85301d7a5f386ab9da41e3 
>   src/slave/slave.cpp 840534ff0687e82ed063c386e36bbabada230697 
>   src/tests/mesos.cpp 90aef6bfe619dc0acdb4ccba6a7180482dd13ce5 
> 
> Diff: https://reviews.apache.org/r/44515/diff/
> 
> 
> Testing
> ---
> 
> `sudo make check` was used to test on both OSX and Ubuntu 14.04.
> 
> 
> Thanks,
> 
> Greg Mann
> 
>

Re: Review Request 44515: Added agent flags for HTTP authentication.

[Adam B]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/44515/#review124387
---


Fix it, then Ship it!




One last question


src/tests/mesos.cpp (line 177)


Why use `os::getcwd()` instead of `directory.get()` like "credential" and 
"fetch" above? Seems like `CreateSlaveFlags` wanted to use a temporary 
directory removed later.
I'm not convinced these two are equivalent.


- Adam B


On March 18, 2016, 11:32 a.m., Greg Mann wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44515/
> ---
> 
> (Updated March 18, 2016, 11:32 a.m.)
> 
> 
> Review request for mesos, Adam B, Alexander Rojas, Joerg Schad, and Till 
> Toenshoff.
> 
> 
> Bugs: MESOS-4849
> https://issues.apache.org/jira/browse/MESOS-4849
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added agent flags for HTTP authentication.
> 
> Three command-line flags have been added to the agent to enable HTTP 
> authentication: `--authenticate_http`, `--http_credentials`, and 
> `--http_authenticators`.
> 
> 
> Diffs
> -
> 
>   src/slave/constants.hpp 4189c07dbad754e51f1a067ecbb4c99ae42a386f 
>   src/slave/flags.hpp feb095da4521f678c96f4cc53bdfda262d350388 
>   src/slave/flags.cpp b77afa956834bb5b1f85301d7a5f386ab9da41e3 
>   src/slave/slave.cpp 840534ff0687e82ed063c386e36bbabada230697 
>   src/tests/mesos.cpp 90aef6bfe619dc0acdb4ccba6a7180482dd13ce5 
> 
> Diff: https://reviews.apache.org/r/44515/diff/
> 
> 
> Testing
> ---
> 
> `sudo make check` was used to test on both OSX and Ubuntu 14.04.
> 
> 
> Thanks,
> 
> Greg Mann
> 
>

Re: Review Request 44515: Added agent flags for HTTP authentication.

[Greg Mann]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/44515/
---

(Updated March 18, 2016, 6:32 p.m.)


Review request for mesos, Adam B, Alexander Rojas, Joerg Schad, and Till 
Toenshoff.


Changes
---

Addressed comments.


Bugs: MESOS-4849
https://issues.apache.org/jira/browse/MESOS-4849


Repository: mesos


Description
---

Added agent flags for HTTP authentication.

Three command-line flags have been added to the agent to enable HTTP 
authentication: `--authenticate_http`, `--http_credentials`, and 
`--http_authenticators`.


Diffs (updated)
-

  src/slave/constants.hpp 4189c07dbad754e51f1a067ecbb4c99ae42a386f 
  src/slave/flags.hpp feb095da4521f678c96f4cc53bdfda262d350388 
  src/slave/flags.cpp b77afa956834bb5b1f85301d7a5f386ab9da41e3 
  src/slave/slave.cpp 840534ff0687e82ed063c386e36bbabada230697 
  src/tests/mesos.cpp 90aef6bfe619dc0acdb4ccba6a7180482dd13ce5 

Diff: https://reviews.apache.org/r/44515/diff/


Testing
---

`sudo make check` was used to test on both OSX and Ubuntu 14.04.


Thanks,

Greg Mann

Re: Review Request 44515: Added agent flags for HTTP authentication.

[Greg Mann]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/44515/
---

(Updated March 17, 2016, 7:41 p.m.)


Review request for mesos, Adam B, Alexander Rojas, Joerg Schad, and Till 
Toenshoff.


Changes
---

Rebase.


Bugs: MESOS-4849
https://issues.apache.org/jira/browse/MESOS-4849


Repository: mesos


Description
---

Added agent flags for HTTP authentication.

Three command-line flags have been added to the agent to enable HTTP 
authentication: `--authenticate_http`, `--http_credentials`, and 
`--http_authenticators`.


Diffs (updated)
-

  src/slave/constants.hpp 4189c07dbad754e51f1a067ecbb4c99ae42a386f 
  src/slave/flags.hpp feb095da4521f678c96f4cc53bdfda262d350388 
  src/slave/flags.cpp b77afa956834bb5b1f85301d7a5f386ab9da41e3 
  src/slave/slave.cpp 840534ff0687e82ed063c386e36bbabada230697 
  src/tests/mesos.cpp 90aef6bfe619dc0acdb4ccba6a7180482dd13ce5 

Diff: https://reviews.apache.org/r/44515/diff/


Testing
---

`sudo make check` was used to test on both OSX and Ubuntu 14.04.


Thanks,

Greg Mann

Re: Review Request 44515: Added agent flags for HTTP authentication.

[Greg Mann]

> On March 15, 2016, 10:33 a.m., Joerg Schad wrote:
> > src/slave/flags.cpp, line 682
> > 
> >
> > As we are actively deprecated the old text based format with Mesos-228, 
> > does it make sense to only document the new json format here?
> 
> Joerg Schad wrote:
> Mesos-2281
> 
> Greg Mann wrote:
> Perhaps we should see which patch lands first, and update the other one 
> accordingly? I can put a comment on Jan's review so that he's aware of this 
> one.

After looking at Jan's patch again I see that it's just beginning the 
deprecation cycle for that change; I think it makes sense to introduce the 
`--http_credentials` flag using only the JSON format, regardless of which patch 
lands first. I altered this patch chain accordingly. Thanks Joerg!


- Greg


---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/44515/#review123630
---


On March 15, 2016, 9:30 p.m., Greg Mann wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44515/
> ---
> 
> (Updated March 15, 2016, 9:30 p.m.)
> 
> 
> Review request for mesos, Adam B, Alexander Rojas, Joerg Schad, and Till 
> Toenshoff.
> 
> 
> Bugs: MESOS-4849
> https://issues.apache.org/jira/browse/MESOS-4849
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added agent flags for HTTP authentication.
> 
> Three command-line flags have been added to the agent to enable HTTP 
> authentication: `--authenticate_http`, `--http_credentials`, and 
> `--http_authenticators`.
> 
> 
> Diffs
> -
> 
>   src/slave/constants.hpp 0b4e41b4eee3280515cf8a1f049fe923503066f1 
>   src/slave/flags.hpp feb095da4521f678c96f4cc53bdfda262d350388 
>   src/slave/flags.cpp 4d10818105627738e258116647ccada374e3d7b9 
>   src/slave/slave.cpp 840534ff0687e82ed063c386e36bbabada230697 
>   src/tests/mesos.cpp 7cca4ed4753fa365b209d1d22f0df4650b19bc6a 
> 
> Diff: https://reviews.apache.org/r/44515/diff/
> 
> 
> Testing
> ---
> 
> `sudo make check` was used to test on both OSX and Ubuntu 14.04.
> 
> 
> Thanks,
> 
> Greg Mann
> 
>

Re: Review Request 44515: Added agent flags for HTTP authentication.

[Greg Mann]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/44515/
---

(Updated March 15, 2016, 9:30 p.m.)


Review request for mesos, Adam B, Alexander Rojas, Joerg Schad, and Till 
Toenshoff.


Changes
---

Removed documentation of deprecated credentials format.


Bugs: MESOS-4849
https://issues.apache.org/jira/browse/MESOS-4849


Repository: mesos


Description
---

Added agent flags for HTTP authentication.

Three command-line flags have been added to the agent to enable HTTP 
authentication: `--authenticate_http`, `--http_credentials`, and 
`--http_authenticators`.


Diffs (updated)
-

  src/slave/constants.hpp 0b4e41b4eee3280515cf8a1f049fe923503066f1 
  src/slave/flags.hpp feb095da4521f678c96f4cc53bdfda262d350388 
  src/slave/flags.cpp 4d10818105627738e258116647ccada374e3d7b9 
  src/slave/slave.cpp 840534ff0687e82ed063c386e36bbabada230697 
  src/tests/mesos.cpp 7cca4ed4753fa365b209d1d22f0df4650b19bc6a 

Diff: https://reviews.apache.org/r/44515/diff/


Testing
---

`sudo make check` was used to test on both OSX and Ubuntu 14.04.


Thanks,

Greg Mann

Re: Review Request 44515: Added agent flags for HTTP authentication.

[Greg Mann]

> On March 15, 2016, 10:33 a.m., Joerg Schad wrote:
> > src/slave/flags.cpp, line 682
> > 
> >
> > As we are actively deprecated the old text based format with Mesos-228, 
> > does it make sense to only document the new json format here?
> 
> Joerg Schad wrote:
> Mesos-2281

Perhaps we should see which patch lands first, and update the other one 
accordingly? I can put a comment on Jan's review so that he's aware of this one.


- Greg


---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/44515/#review123630
---


On March 14, 2016, 4:17 a.m., Greg Mann wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44515/
> ---
> 
> (Updated March 14, 2016, 4:17 a.m.)
> 
> 
> Review request for mesos, Adam B, Alexander Rojas, Joerg Schad, and Till 
> Toenshoff.
> 
> 
> Bugs: MESOS-4849
> https://issues.apache.org/jira/browse/MESOS-4849
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added agent flags for HTTP authentication.
> 
> Three command-line flags have been added to the agent to enable HTTP 
> authentication: `--authenticate_http`, `--http_credentials`, and 
> `--http_authenticators`.
> 
> 
> Diffs
> -
> 
>   src/slave/constants.hpp 4110061d576d5e20e512e84bb049b6ac910ceab0 
>   src/slave/flags.hpp feb095da4521f678c96f4cc53bdfda262d350388 
>   src/slave/flags.cpp 5bacf109bcd9c9eae1b7ec3c32095c72899ebdbd 
>   src/slave/slave.cpp 840534ff0687e82ed063c386e36bbabada230697 
>   src/tests/mesos.cpp 7cca4ed4753fa365b209d1d22f0df4650b19bc6a 
> 
> Diff: https://reviews.apache.org/r/44515/diff/
> 
> 
> Testing
> ---
> 
> `sudo make check` was used to test on both OSX and Ubuntu 14.04.
> 
> 
> Thanks,
> 
> Greg Mann
> 
>

Re: Review Request 44515: Added agent flags for HTTP authentication.

[Joerg Schad]

> On March 15, 2016, 10:33 a.m., Joerg Schad wrote:
> > src/slave/flags.cpp, line 682
> > 
> >
> > As we are actively deprecated the old text based format with Mesos-228, 
> > does it make sense to only document the new json format here?

Mesos-2281


- Joerg


---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/44515/#review123630
---


On March 14, 2016, 4:17 a.m., Greg Mann wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44515/
> ---
> 
> (Updated March 14, 2016, 4:17 a.m.)
> 
> 
> Review request for mesos, Adam B, Alexander Rojas, Joerg Schad, and Till 
> Toenshoff.
> 
> 
> Bugs: MESOS-4849
> https://issues.apache.org/jira/browse/MESOS-4849
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added agent flags for HTTP authentication.
> 
> Three command-line flags have been added to the agent to enable HTTP 
> authentication: `--authenticate_http`, `--http_credentials`, and 
> `--http_authenticators`.
> 
> 
> Diffs
> -
> 
>   src/slave/constants.hpp 4110061d576d5e20e512e84bb049b6ac910ceab0 
>   src/slave/flags.hpp feb095da4521f678c96f4cc53bdfda262d350388 
>   src/slave/flags.cpp 5bacf109bcd9c9eae1b7ec3c32095c72899ebdbd 
>   src/slave/slave.cpp 840534ff0687e82ed063c386e36bbabada230697 
>   src/tests/mesos.cpp 7cca4ed4753fa365b209d1d22f0df4650b19bc6a 
> 
> Diff: https://reviews.apache.org/r/44515/diff/
> 
> 
> Testing
> ---
> 
> `sudo make check` was used to test on both OSX and Ubuntu 14.04.
> 
> 
> Thanks,
> 
> Greg Mann
> 
>

Re: Review Request 44515: Added agent flags for HTTP authentication.

[Joerg Schad]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/44515/#review123630
---




src/slave/flags.cpp (line 681)


As we are actively deprecated the old text based format with Mesos-228, 
does it make sense to only document the new json format here?


- Joerg Schad


On March 14, 2016, 4:17 a.m., Greg Mann wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44515/
> ---
> 
> (Updated March 14, 2016, 4:17 a.m.)
> 
> 
> Review request for mesos, Adam B, Alexander Rojas, Joerg Schad, and Till 
> Toenshoff.
> 
> 
> Bugs: MESOS-4849
> https://issues.apache.org/jira/browse/MESOS-4849
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added agent flags for HTTP authentication.
> 
> Three command-line flags have been added to the agent to enable HTTP 
> authentication: `--authenticate_http`, `--http_credentials`, and 
> `--http_authenticators`.
> 
> 
> Diffs
> -
> 
>   src/slave/constants.hpp 4110061d576d5e20e512e84bb049b6ac910ceab0 
>   src/slave/flags.hpp feb095da4521f678c96f4cc53bdfda262d350388 
>   src/slave/flags.cpp 5bacf109bcd9c9eae1b7ec3c32095c72899ebdbd 
>   src/slave/slave.cpp 840534ff0687e82ed063c386e36bbabada230697 
>   src/tests/mesos.cpp 7cca4ed4753fa365b209d1d22f0df4650b19bc6a 
> 
> Diff: https://reviews.apache.org/r/44515/diff/
> 
> 
> Testing
> ---
> 
> `sudo make check` was used to test on both OSX and Ubuntu 14.04.
> 
> 
> Thanks,
> 
> Greg Mann
> 
>

Re: Review Request 44515: Added agent flags for HTTP authentication.

[Greg Mann]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/44515/
---

(Updated March 14, 2016, 4:17 a.m.)


Review request for mesos, Adam B, Alexander Rojas, Joerg Schad, and Till 
Toenshoff.


Changes
---

Rebase.


Bugs: MESOS-4849
https://issues.apache.org/jira/browse/MESOS-4849


Repository: mesos


Description
---

Added agent flags for HTTP authentication.

Three command-line flags have been added to the agent to enable HTTP 
authentication: `--authenticate_http`, `--http_credentials`, and 
`--http_authenticators`.


Diffs (updated)
-

  src/slave/constants.hpp 4110061d576d5e20e512e84bb049b6ac910ceab0 
  src/slave/flags.hpp feb095da4521f678c96f4cc53bdfda262d350388 
  src/slave/flags.cpp 5bacf109bcd9c9eae1b7ec3c32095c72899ebdbd 
  src/slave/slave.cpp 840534ff0687e82ed063c386e36bbabada230697 
  src/tests/mesos.cpp 7cca4ed4753fa365b209d1d22f0df4650b19bc6a 

Diff: https://reviews.apache.org/r/44515/diff/


Testing
---

`sudo make check` was used to test on both OSX and Ubuntu 14.04.


Thanks,

Greg Mann

Re: Review Request 44515: Added agent flags for HTTP authentication.

[Greg Mann]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/44515/
---

(Updated March 11, 2016, 9:48 a.m.)


Review request for mesos, Adam B, Alexander Rojas, Joerg Schad, and Till 
Toenshoff.


Bugs: MESOS-4849
https://issues.apache.org/jira/browse/MESOS-4849


Repository: mesos


Description
---

Added agent flags for HTTP authentication.

Three command-line flags have been added to the agent to enable HTTP 
authentication: `--authenticate_http`, `--http_credentials`, and 
`--http_authenticators`.


Diffs (updated)
-

  src/slave/constants.hpp bcbb1401aa8f9f04c4f9256bb4f560e18d8994e0 
  src/slave/constants.cpp 0f0d8e4b079d136d250f83cfc768de8c98b8bee2 
  src/slave/flags.hpp feb095da4521f678c96f4cc53bdfda262d350388 
  src/slave/flags.cpp eb470154f30634b3db439be1c122ff93d3147afe 
  src/slave/slave.cpp 840534ff0687e82ed063c386e36bbabada230697 
  src/tests/mesos.cpp 577072136415191334c74b69a1c0b7eed8e77e8b 

Diff: https://reviews.apache.org/r/44515/diff/


Testing
---

`sudo make check` was used to test on both OSX and Ubuntu 14.04.


Thanks,

Greg Mann

Re: Review Request 44515: Added agent flags for HTTP authentication.

[Alexander Rojas]

> On March 10, 2016, 9:45 a.m., Adam B wrote:
> > src/slave/flags.cpp, line 684
> > 
> >
> > I wonder if/when we'll ever deprecate one of these formats.
> > https://issues.apache.org/jira/browse/MESOS-2281
> 
> Greg Mann wrote:
> Ah yea; looks like one will be deprecated soon.

I wonder what the status of that issue is. It is marked as _In Progress_ for 
more than a month now. Since then Isabel moved to a new team and I'm not sure 
Till is actively looking to finish this issue.


- Alexander


---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/44515/#review122892
---


On March 10, 2016, 11:16 p.m., Greg Mann wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44515/
> ---
> 
> (Updated March 10, 2016, 11:16 p.m.)
> 
> 
> Review request for mesos, Adam B, Alexander Rojas, Joerg Schad, and Till 
> Toenshoff.
> 
> 
> Bugs: MESOS-4849
> https://issues.apache.org/jira/browse/MESOS-4849
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added agent flags for HTTP authentication.
> 
> Three command-line flags have been added to the agent to enable HTTP 
> authentication: `--authenticate_http`, `--http_credentials`, and 
> `--http_authenticators`.
> 
> 
> Diffs
> -
> 
>   src/slave/constants.hpp bcbb1401aa8f9f04c4f9256bb4f560e18d8994e0 
>   src/slave/constants.cpp 0f0d8e4b079d136d250f83cfc768de8c98b8bee2 
>   src/slave/flags.hpp feb095da4521f678c96f4cc53bdfda262d350388 
>   src/slave/flags.cpp eb470154f30634b3db439be1c122ff93d3147afe 
>   src/slave/slave.cpp 840534ff0687e82ed063c386e36bbabada230697 
>   src/tests/mesos.cpp 577072136415191334c74b69a1c0b7eed8e77e8b 
> 
> Diff: https://reviews.apache.org/r/44515/diff/
> 
> 
> Testing
> ---
> 
> `sudo make check` was used to test on both OSX and Ubuntu 14.04.
> 
> 
> Thanks,
> 
> Greg Mann
> 
>

Re: Review Request 44515: Added agent flags for HTTP authentication.

[Alexander Rojas]

> On March 10, 2016, 9:45 a.m., Adam B wrote:
> > src/tests/mesos.cpp, lines 182-184
> > 
> >
> > Any reason you can't reuse the previous `path` and `fd` variables?
> 
> Greg Mann wrote:
> We could do that; reusing the variables does improve efficiency, but also 
> reduces readability. Especially in tests, I would consider readability to be 
> the priority, so I decided to declare new variables for these. I've received 
> feedback in the past to declare new variables in similar situations, but I 
> would be interested to hear if you prefer reuse here.

I concur with you Gregg, however I think probably `path` and `fd` should change 
(not on this review though). Another option would be to separate logical blocks 
into their own scopes, or a third one, is to have each logical block into a 
different function. Though for this short method I would go for the scopes 
solution.


- Alexander


---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/44515/#review122892
---


On March 10, 2016, 11:16 p.m., Greg Mann wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44515/
> ---
> 
> (Updated March 10, 2016, 11:16 p.m.)
> 
> 
> Review request for mesos, Adam B, Alexander Rojas, Joerg Schad, and Till 
> Toenshoff.
> 
> 
> Bugs: MESOS-4849
> https://issues.apache.org/jira/browse/MESOS-4849
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added agent flags for HTTP authentication.
> 
> Three command-line flags have been added to the agent to enable HTTP 
> authentication: `--authenticate_http`, `--http_credentials`, and 
> `--http_authenticators`.
> 
> 
> Diffs
> -
> 
>   src/slave/constants.hpp bcbb1401aa8f9f04c4f9256bb4f560e18d8994e0 
>   src/slave/constants.cpp 0f0d8e4b079d136d250f83cfc768de8c98b8bee2 
>   src/slave/flags.hpp feb095da4521f678c96f4cc53bdfda262d350388 
>   src/slave/flags.cpp eb470154f30634b3db439be1c122ff93d3147afe 
>   src/slave/slave.cpp 840534ff0687e82ed063c386e36bbabada230697 
>   src/tests/mesos.cpp 577072136415191334c74b69a1c0b7eed8e77e8b 
> 
> Diff: https://reviews.apache.org/r/44515/diff/
> 
> 
> Testing
> ---
> 
> `sudo make check` was used to test on both OSX and Ubuntu 14.04.
> 
> 
> Thanks,
> 
> Greg Mann
> 
>

Re: Review Request 44515: Added agent flags for HTTP authentication.

[Greg Mann]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/44515/
---

(Updated March 10, 2016, 10:16 p.m.)


Review request for mesos, Adam B, Alexander Rojas, Joerg Schad, and Till 
Toenshoff.


Changes
---

Updated authenticator creation to include realm.


Bugs: MESOS-4849
https://issues.apache.org/jira/browse/MESOS-4849


Repository: mesos


Description
---

Added agent flags for HTTP authentication.

Three command-line flags have been added to the agent to enable HTTP 
authentication: `--authenticate_http`, `--http_credentials`, and 
`--http_authenticators`.


Diffs (updated)
-

  src/slave/constants.hpp bcbb1401aa8f9f04c4f9256bb4f560e18d8994e0 
  src/slave/constants.cpp 0f0d8e4b079d136d250f83cfc768de8c98b8bee2 
  src/slave/flags.hpp feb095da4521f678c96f4cc53bdfda262d350388 
  src/slave/flags.cpp eb470154f30634b3db439be1c122ff93d3147afe 
  src/slave/slave.cpp 840534ff0687e82ed063c386e36bbabada230697 
  src/tests/mesos.cpp 577072136415191334c74b69a1c0b7eed8e77e8b 

Diff: https://reviews.apache.org/r/44515/diff/


Testing
---

`sudo make check` was used to test on both OSX and Ubuntu 14.04.


Thanks,

Greg Mann

Re: Review Request 44515: Added agent flags for HTTP authentication.

[Greg Mann]

> On March 10, 2016, 3:55 p.m., Joerg Schad wrote:
> > src/slave/flags.cpp, line 665
> > 
> >
> > Did you add those flags to configuration.md or do you plan to do this 
> > via your script?

Documentation is added in a subsequent review: 
https://reviews.apache.org/r/44554/


- Greg


---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/44515/#review122929
---


On March 10, 2016, 3:52 p.m., Greg Mann wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44515/
> ---
> 
> (Updated March 10, 2016, 3:52 p.m.)
> 
> 
> Review request for mesos, Adam B, Alexander Rojas, Joerg Schad, and Till 
> Toenshoff.
> 
> 
> Bugs: MESOS-4849
> https://issues.apache.org/jira/browse/MESOS-4849
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added agent flags for HTTP authentication.
> 
> Three command-line flags have been added to the agent to enable HTTP 
> authentication: `--authenticate_http`, `--http_credentials`, and 
> `--http_authenticators`.
> 
> 
> Diffs
> -
> 
>   src/slave/constants.hpp bcbb1401aa8f9f04c4f9256bb4f560e18d8994e0 
>   src/slave/constants.cpp 0f0d8e4b079d136d250f83cfc768de8c98b8bee2 
>   src/slave/flags.hpp feb095da4521f678c96f4cc53bdfda262d350388 
>   src/slave/flags.cpp eb470154f30634b3db439be1c122ff93d3147afe 
>   src/slave/slave.cpp 840534ff0687e82ed063c386e36bbabada230697 
>   src/tests/mesos.cpp 577072136415191334c74b69a1c0b7eed8e77e8b 
> 
> Diff: https://reviews.apache.org/r/44515/diff/
> 
> 
> Testing
> ---
> 
> `sudo make check` was used to test on both OSX and Ubuntu 14.04.
> 
> 
> Thanks,
> 
> Greg Mann
> 
>

Re: Review Request 44515: Added agent flags for HTTP authentication.

[Joerg Schad]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/44515/#review122929
---




src/slave/flags.cpp (line 665)


Did you add those flags to configuration.md or do you plan to do this via 
your script?


- Joerg Schad


On March 10, 2016, 3:52 p.m., Greg Mann wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44515/
> ---
> 
> (Updated March 10, 2016, 3:52 p.m.)
> 
> 
> Review request for mesos, Adam B, Alexander Rojas, Joerg Schad, and Till 
> Toenshoff.
> 
> 
> Bugs: MESOS-4849
> https://issues.apache.org/jira/browse/MESOS-4849
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added agent flags for HTTP authentication.
> 
> Three command-line flags have been added to the agent to enable HTTP 
> authentication: `--authenticate_http`, `--http_credentials`, and 
> `--http_authenticators`.
> 
> 
> Diffs
> -
> 
>   src/slave/constants.hpp bcbb1401aa8f9f04c4f9256bb4f560e18d8994e0 
>   src/slave/constants.cpp 0f0d8e4b079d136d250f83cfc768de8c98b8bee2 
>   src/slave/flags.hpp feb095da4521f678c96f4cc53bdfda262d350388 
>   src/slave/flags.cpp eb470154f30634b3db439be1c122ff93d3147afe 
>   src/slave/slave.cpp 840534ff0687e82ed063c386e36bbabada230697 
>   src/tests/mesos.cpp 577072136415191334c74b69a1c0b7eed8e77e8b 
> 
> Diff: https://reviews.apache.org/r/44515/diff/
> 
> 
> Testing
> ---
> 
> `sudo make check` was used to test on both OSX and Ubuntu 14.04.
> 
> 
> Thanks,
> 
> Greg Mann
> 
>

Re: Review Request 44515: Added agent flags for HTTP authentication.

[Greg Mann]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/44515/
---

(Updated March 10, 2016, 3:52 p.m.)


Review request for mesos, Adam B, Alexander Rojas, Joerg Schad, and Till 
Toenshoff.


Changes
---

Addressed comments.


Bugs: MESOS-4849
https://issues.apache.org/jira/browse/MESOS-4849


Repository: mesos


Description
---

Added agent flags for HTTP authentication.

Three command-line flags have been added to the agent to enable HTTP 
authentication: `--authenticate_http`, `--http_credentials`, and 
`--http_authenticators`.


Diffs (updated)
-

  src/slave/constants.hpp bcbb1401aa8f9f04c4f9256bb4f560e18d8994e0 
  src/slave/constants.cpp 0f0d8e4b079d136d250f83cfc768de8c98b8bee2 
  src/slave/flags.hpp feb095da4521f678c96f4cc53bdfda262d350388 
  src/slave/flags.cpp eb470154f30634b3db439be1c122ff93d3147afe 
  src/slave/slave.cpp 840534ff0687e82ed063c386e36bbabada230697 
  src/tests/mesos.cpp 577072136415191334c74b69a1c0b7eed8e77e8b 

Diff: https://reviews.apache.org/r/44515/diff/


Testing
---

`sudo make check` was used to test on both OSX and Ubuntu 14.04.


Thanks,

Greg Mann

Re: Review Request 44515: Added agent flags for HTTP authentication.

[Greg Mann]

> On March 10, 2016, 8:45 a.m., Adam B wrote:
> > src/slave/slave.cpp, line 372
> > 
> >
> > So we only load the authenticators and even allow authentication if 
> > --authenticate_http is set? My understanding is that 
> > --authenticate_http=false means that both authenticated and unauthenticated 
> > requests will be allowed, but --authenticate_http=true means that we 
> > /require/ all requests to be authenticated. That's what the flag help seems 
> > to imply.
> > Maybe that behavior's only really true for authenticate_frameworks and 
> > authenticate_slaves

You're correct, if authenticate_http == false, we accept both unauthenticated 
requests as well as requests that attempt to authenticate. If no authenticator 
has been installed, the requests for authentication-enabled endpoints are 
simply forwarded to their handlers, regardless of the presence of 
authentication headers in the request.


> On March 10, 2016, 8:45 a.m., Adam B wrote:
> > src/tests/mesos.cpp, lines 182-184
> > 
> >
> > Any reason you can't reuse the previous `path` and `fd` variables?

We could do that; reusing the variables does improve efficiency, but also 
reduces readability. Especially in tests, I would consider readability to be 
the priority, so I decided to declare new variables for these. I've received 
feedback in the past to declare new variables in similar situations, but I 
would be interested to hear if you prefer reuse here.


> On March 10, 2016, 8:45 a.m., Adam B wrote:
> > src/slave/flags.cpp, line 684
> > 
> >
> > I wonder if/when we'll ever deprecate one of these formats.
> > https://issues.apache.org/jira/browse/MESOS-2281

Ah yea; looks like one will be deprecated soon.


- Greg


---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/44515/#review122892
---


On March 9, 2016, 8:47 p.m., Greg Mann wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44515/
> ---
> 
> (Updated March 9, 2016, 8:47 p.m.)
> 
> 
> Review request for mesos, Adam B, Alexander Rojas, and Till Toenshoff.
> 
> 
> Bugs: MESOS-4849
> https://issues.apache.org/jira/browse/MESOS-4849
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added agent flags for HTTP authentication.
> 
> Three command-line flags have been added to the agent to enable HTTP 
> authentication: `--authenticate_http`, `--http_credentials`, and 
> `--http_authenticators`.
> 
> 
> Diffs
> -
> 
>   src/slave/constants.hpp bcbb1401aa8f9f04c4f9256bb4f560e18d8994e0 
>   src/slave/constants.cpp 0f0d8e4b079d136d250f83cfc768de8c98b8bee2 
>   src/slave/flags.hpp feb095da4521f678c96f4cc53bdfda262d350388 
>   src/slave/flags.cpp eb470154f30634b3db439be1c122ff93d3147afe 
>   src/slave/slave.cpp 840534ff0687e82ed063c386e36bbabada230697 
>   src/tests/mesos.cpp 577072136415191334c74b69a1c0b7eed8e77e8b 
> 
> Diff: https://reviews.apache.org/r/44515/diff/
> 
> 
> Testing
> ---
> 
> `sudo make check` was used to test on both OSX and Ubuntu 14.04.
> 
> 
> Thanks,
> 
> Greg Mann
> 
>

Re: Review Request 44515: Added agent flags for HTTP authentication.

[Adam B]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/44515/#review122892
---



Looks great! Just some minor nits and a question about authenticating http 
requests even if --authenticate_http=false doesn't require authentication.


src/slave/constants.cpp (line 56)


"accommodate" (two 'm's)



src/slave/flags.cpp (line 679)


No need for the final '\n'



src/slave/flags.cpp (line 684)


I wonder if/when we'll ever deprecate one of these formats.
https://issues.apache.org/jira/browse/MESOS-2281



src/slave/flags.cpp (lines 685 - 686)


Can't these lines be joined?



src/slave/flags.cpp (lines 688 - 689)


Can't these lines be joined?



src/slave/slave.cpp (line 353)


s/At least...specified/If the http_authenticators flag is not specified, 
the default value will be filled in./



src/slave/slave.cpp (lines 364 - 365)


How about you just put the whole `"' not "` on the second line with 
`"found..."`



src/slave/slave.cpp (line 366)


s/''/'/?



src/slave/slave.cpp (line 372)


So we only load the authenticators and even allow authentication if 
--authenticate_http is set? My understanding is that --authenticate_http=false 
means that both authenticated and unauthenticated requests will be allowed, but 
--authenticate_http=true means that we /require/ all requests to be 
authenticated. That's what the flag help seems to imply.
Maybe that behavior's only really true for authenticate_frameworks and 
authenticate_slaves



src/tests/mesos.cpp (lines 182 - 184)


Any reason you can't reuse the previous `path` and `fd` variables?



src/tests/mesos.cpp (line 203)


s/credentials/http credentials/


- Adam B


On March 9, 2016, 12:47 p.m., Greg Mann wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44515/
> ---
> 
> (Updated March 9, 2016, 12:47 p.m.)
> 
> 
> Review request for mesos, Adam B, Alexander Rojas, and Till Toenshoff.
> 
> 
> Bugs: MESOS-4849
> https://issues.apache.org/jira/browse/MESOS-4849
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added agent flags for HTTP authentication.
> 
> Three command-line flags have been added to the agent to enable HTTP 
> authentication: `--authenticate_http`, `--http_credentials`, and 
> `--http_authenticators`.
> 
> 
> Diffs
> -
> 
>   src/slave/constants.hpp bcbb1401aa8f9f04c4f9256bb4f560e18d8994e0 
>   src/slave/constants.cpp 0f0d8e4b079d136d250f83cfc768de8c98b8bee2 
>   src/slave/flags.hpp feb095da4521f678c96f4cc53bdfda262d350388 
>   src/slave/flags.cpp eb470154f30634b3db439be1c122ff93d3147afe 
>   src/slave/slave.cpp 840534ff0687e82ed063c386e36bbabada230697 
>   src/tests/mesos.cpp 577072136415191334c74b69a1c0b7eed8e77e8b 
> 
> Diff: https://reviews.apache.org/r/44515/diff/
> 
> 
> Testing
> ---
> 
> `sudo make check` was used to test on both OSX and Ubuntu 14.04.
> 
> 
> Thanks,
> 
> Greg Mann
> 
>

Re: Review Request 44515: Added agent flags for HTTP authentication.

[Greg Mann]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/44515/
---

(Updated March 9, 2016, 8:47 p.m.)


Review request for mesos, Adam B, Alexander Rojas, and Till Toenshoff.


Bugs: MESOS-4849
https://issues.apache.org/jira/browse/MESOS-4849


Repository: mesos


Description
---

Added agent flags for HTTP authentication.

Three command-line flags have been added to the agent to enable HTTP 
authentication: `--authenticate_http`, `--http_credentials`, and 
`--http_authenticators`.


Diffs
-

  src/slave/constants.hpp bcbb1401aa8f9f04c4f9256bb4f560e18d8994e0 
  src/slave/constants.cpp 0f0d8e4b079d136d250f83cfc768de8c98b8bee2 
  src/slave/flags.hpp feb095da4521f678c96f4cc53bdfda262d350388 
  src/slave/flags.cpp eb470154f30634b3db439be1c122ff93d3147afe 
  src/slave/slave.cpp 840534ff0687e82ed063c386e36bbabada230697 
  src/tests/mesos.cpp 577072136415191334c74b69a1c0b7eed8e77e8b 

Diff: https://reviews.apache.org/r/44515/diff/


Testing (updated)
---

`sudo make check` was used to test on both OSX and Ubuntu 14.04.


Thanks,

Greg Mann

Re: Review Request 44515: Added agent flags for HTTP authentication.

[Greg Mann]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/44515/
---

(Updated March 9, 2016, 8:42 p.m.)


Review request for mesos, Adam B, Alexander Rojas, and Till Toenshoff.


Bugs: MESOS-4849
https://issues.apache.org/jira/browse/MESOS-4849


Repository: mesos


Description
---

Added agent flags for HTTP authentication.

Three command-line flags have been added to the agent to enable HTTP 
authentication: `--authenticate_http`, `--http_credentials`, and 
`--http_authenticators`.


Diffs (updated)
-

  src/slave/constants.hpp bcbb1401aa8f9f04c4f9256bb4f560e18d8994e0 
  src/slave/constants.cpp 0f0d8e4b079d136d250f83cfc768de8c98b8bee2 
  src/slave/flags.hpp feb095da4521f678c96f4cc53bdfda262d350388 
  src/slave/flags.cpp eb470154f30634b3db439be1c122ff93d3147afe 
  src/slave/slave.cpp 840534ff0687e82ed063c386e36bbabada230697 
  src/tests/mesos.cpp 577072136415191334c74b69a1c0b7eed8e77e8b 

Diff: https://reviews.apache.org/r/44515/diff/


Testing
---

`make check` was used to test. However, these flags aren't yet incorporated 
into any agent endpoints. We should probably wait to merge this patch along 
with the changes for https://issues.apache.org/jira/browse/MESOS-4850 when 
they're ready.


Thanks,

Greg Mann

Re: Review Request 44515: Added agent flags for HTTP authentication.

[Alexander Rojas]

> On March 9, 2016, 4:56 a.m., Alexander Rojas wrote:
> > src/slave/constants.cpp, line 58
> > 
> >
> > Can you create a JIRA issue so we make the naming of the realm symetric 
> > in the master to _mesos-master_. I think only the master's 
> > `DEFAULT_HTTP_AUTHENTICATION_REALM` should be changed.
> 
> Greg Mann wrote:
> I was planning on making this change as part of MESOS-4849, with the 
> follow-up review to this one: https://reviews.apache.org/r/44523/
> 
> Do you think we should create a separate JIRA for this?

If it is in the plans, then no. I will review that one tomorrow.


- Alexander


---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/44515/#review122668
---


On March 8, 2016, 7:59 p.m., Greg Mann wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44515/
> ---
> 
> (Updated March 8, 2016, 7:59 p.m.)
> 
> 
> Review request for mesos, Adam B, Alexander Rojas, and Till Toenshoff.
> 
> 
> Bugs: MESOS-4849
> https://issues.apache.org/jira/browse/MESOS-4849
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added agent flags for HTTP authentication.
> 
> Three command-line flags have been added to the agent to enable HTTP 
> authentication: `--authenticate_http`, `--http_credentials`, and 
> `--http_authenticators`.
> 
> 
> Diffs
> -
> 
>   src/slave/constants.hpp bcbb1401aa8f9f04c4f9256bb4f560e18d8994e0 
>   src/slave/constants.cpp 0f0d8e4b079d136d250f83cfc768de8c98b8bee2 
>   src/slave/flags.hpp feb095da4521f678c96f4cc53bdfda262d350388 
>   src/slave/flags.cpp 6e3fd69c06eefd40bc0e5c222ea72f34144c5534 
>   src/slave/slave.cpp 840534ff0687e82ed063c386e36bbabada230697 
>   src/tests/mesos.cpp e0f641c6828833de13a0a233e39ff6dc3f343d5c 
> 
> Diff: https://reviews.apache.org/r/44515/diff/
> 
> 
> Testing
> ---
> 
> `make check` was used to test. However, these flags aren't yet incorporated 
> into any agent endpoints. We should probably wait to merge this patch along 
> with the changes for https://issues.apache.org/jira/browse/MESOS-4850 when 
> they're ready.
> 
> 
> Thanks,
> 
> Greg Mann
> 
>

Re: Review Request 44515: Added agent flags for HTTP authentication.

[Greg Mann]

> On March 9, 2016, 3:56 a.m., Alexander Rojas wrote:
> > src/slave/constants.cpp, line 58
> > 
> >
> > Can you create a JIRA issue so we make the naming of the realm symetric 
> > in the master to _mesos-master_. I think only the master's 
> > `DEFAULT_HTTP_AUTHENTICATION_REALM` should be changed.

I was planning on making this change as part of MESOS-4849, with the follow-up 
review to this one: https://reviews.apache.org/r/44523/

Do you think we should create a separate JIRA for this?


- Greg


---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/44515/#review122668
---


On March 8, 2016, 6:59 p.m., Greg Mann wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44515/
> ---
> 
> (Updated March 8, 2016, 6:59 p.m.)
> 
> 
> Review request for mesos, Adam B, Alexander Rojas, and Till Toenshoff.
> 
> 
> Bugs: MESOS-4849
> https://issues.apache.org/jira/browse/MESOS-4849
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added agent flags for HTTP authentication.
> 
> Three command-line flags have been added to the agent to enable HTTP 
> authentication: `--authenticate_http`, `--http_credentials`, and 
> `--http_authenticators`.
> 
> 
> Diffs
> -
> 
>   src/slave/constants.hpp bcbb1401aa8f9f04c4f9256bb4f560e18d8994e0 
>   src/slave/constants.cpp 0f0d8e4b079d136d250f83cfc768de8c98b8bee2 
>   src/slave/flags.hpp feb095da4521f678c96f4cc53bdfda262d350388 
>   src/slave/flags.cpp 6e3fd69c06eefd40bc0e5c222ea72f34144c5534 
>   src/slave/slave.cpp 840534ff0687e82ed063c386e36bbabada230697 
>   src/tests/mesos.cpp e0f641c6828833de13a0a233e39ff6dc3f343d5c 
> 
> Diff: https://reviews.apache.org/r/44515/diff/
> 
> 
> Testing
> ---
> 
> `make check` was used to test. However, these flags aren't yet incorporated 
> into any agent endpoints. We should probably wait to merge this patch along 
> with the changes for https://issues.apache.org/jira/browse/MESOS-4850 when 
> they're ready.
> 
> 
> Thanks,
> 
> Greg Mann
> 
>

Re: Review Request 44515: Added agent flags for HTTP authentication.

[Alexander Rojas]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/44515/#review122668
---


Fix it, then Ship it!





src/slave/constants.cpp (line 58)


Can you create a JIRA issue so we make the naming of the realm symetric in 
the master to _mesos-master_. I think only the master's 
`DEFAULT_HTTP_AUTHENTICATION_REALM` should be changed.


- Alexander Rojas


On March 8, 2016, 7:59 p.m., Greg Mann wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44515/
> ---
> 
> (Updated March 8, 2016, 7:59 p.m.)
> 
> 
> Review request for mesos, Adam B, Alexander Rojas, and Till Toenshoff.
> 
> 
> Bugs: MESOS-4849
> https://issues.apache.org/jira/browse/MESOS-4849
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added agent flags for HTTP authentication.
> 
> Three command-line flags have been added to the agent to enable HTTP 
> authentication: `--authenticate_http`, `--http_credentials`, and 
> `--http_authenticators`.
> 
> 
> Diffs
> -
> 
>   src/slave/constants.hpp bcbb1401aa8f9f04c4f9256bb4f560e18d8994e0 
>   src/slave/constants.cpp 0f0d8e4b079d136d250f83cfc768de8c98b8bee2 
>   src/slave/flags.hpp feb095da4521f678c96f4cc53bdfda262d350388 
>   src/slave/flags.cpp 6e3fd69c06eefd40bc0e5c222ea72f34144c5534 
>   src/slave/slave.cpp 840534ff0687e82ed063c386e36bbabada230697 
>   src/tests/mesos.cpp e0f641c6828833de13a0a233e39ff6dc3f343d5c 
> 
> Diff: https://reviews.apache.org/r/44515/diff/
> 
> 
> Testing
> ---
> 
> `make check` was used to test. However, these flags aren't yet incorporated 
> into any agent endpoints. We should probably wait to merge this patch along 
> with the changes for https://issues.apache.org/jira/browse/MESOS-4850 when 
> they're ready.
> 
> 
> Thanks,
> 
> Greg Mann
> 
>

Review Request 44515: Added agent flags for HTTP authentication.

[Greg Mann]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/44515/
---

Review request for mesos, Adam B, Alexander Rojas, and Till Toenshoff.


Bugs: MESOS-4849
https://issues.apache.org/jira/browse/MESOS-4849


Repository: mesos


Description
---

Added agent flags for HTTP authentication.

Three command-line flags have been added to the agent to enable HTTP 
authentication: `--authenticate_http`, `--http_credentials`, and 
`--http_authenticators`.


Diffs
-

  src/slave/constants.hpp bcbb1401aa8f9f04c4f9256bb4f560e18d8994e0 
  src/slave/constants.cpp 0f0d8e4b079d136d250f83cfc768de8c98b8bee2 
  src/slave/flags.hpp feb095da4521f678c96f4cc53bdfda262d350388 
  src/slave/flags.cpp 6e3fd69c06eefd40bc0e5c222ea72f34144c5534 
  src/slave/slave.cpp 840534ff0687e82ed063c386e36bbabada230697 
  src/tests/mesos.cpp e0f641c6828833de13a0a233e39ff6dc3f343d5c 

Diff: https://reviews.apache.org/r/44515/diff/


Testing
---

`make check` was used to test. However, these flags aren't yet incorporated 
into any agent endpoints. We should probably wait to merge this patch along 
with the changes for https://issues.apache.org/jira/browse/MESOS-4850 when 
they're ready.


Thanks,

Greg Mann