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


Fix it, then Ship it!





src/tests/containerizer/docker_containerizer_tests.cpp (line 529)
<https://reviews.apache.org/r/44553/#comment185313>

    Add blank line.



src/tests/health_check_tests.cpp (line 273)
<https://reviews.apache.org/r/44553/#comment185307>

    http::get() should be sufficient here (namespace http = process::http;)



src/tests/health_check_tests.cpp (line 566)
<https://reviews.apache.org/r/44553/#comment185308>

    http::get() see above.



src/tests/health_check_tests.cpp (line 604)
<https://reviews.apache.org/r/44553/#comment185309>

    http::get() see above.



src/tests/slave_tests.cpp (line 1433)
<https://reviews.apache.org/r/44553/#comment185314>

    I would test querying the endpoint without credentials as well.



src/tests/slave_tests.cpp (line 1435)
<https://reviews.apache.org/r/44553/#comment185310>

    Should we add a comment that authn is required by default?



src/tests/slave_tests.cpp (line 1469)
<https://reviews.apache.org/r/44553/#comment185312>

    I would combine both test cases into a single one, saves code and setup 
time. Feel free to drop with good reason to keep them seperate.


- Joerg Schad


On March 11, 2016, 9:52 a.m., Greg Mann wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44553/
> -----------------------------------------------------------
> 
> (Updated March 11, 2016, 9:52 a.m.)
> 
> 
> Review request for mesos, Adam B, Alexander Rojas, Joerg Schad, and Till 
> Toenshoff.
> 
> 
> Bugs: MESOS-4850
>     https://issues.apache.org/jira/browse/MESOS-4850
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Added authentication to agent HTTP endpoints.
> 
> This patch adds HTTP authentication to the `/state`, `/state.json`, and 
> `/flags` endpoints. Tests are also updated to use authentication when hitting 
> these endpoints, and new tests were added to probe these endpoints' behavior 
> when bad credentials are supplied: `SlaveTest.StateEndpointBadCredential` and 
> `SlaveTest.FlagsEndpointBadCredential`.
> 
> 
> Diffs
> -----
> 
>   src/slave/http.cpp 4eb1fafdfa72094511b0b2684a3c2705bd8c7c5e 
>   src/slave/slave.hpp 7520cc356e2b1b7f5fff15f33071a46a7b05e762 
>   src/slave/slave.cpp 840534ff0687e82ed063c386e36bbabada230697 
>   src/tests/containerizer/docker_containerizer_tests.cpp 
> 6aecd912fc84b72d2b64f7a842891fddcbc469ac 
>   src/tests/fault_tolerance_tests.cpp 
> d193897e636efd0e3ef67bf67fcd6255a3de0341 
>   src/tests/health_check_tests.cpp d0fd27fd8a6b48511ef8cafab5dff59f65729d9f 
>   src/tests/slave_tests.cpp 124e9587180f2a55e659d966d1c9060234c19457 
> 
> Diff: https://reviews.apache.org/r/44553/diff/
> 
> 
> Testing
> -------
> 
> Tests were updated to use authentication when hitting the affected agent 
> endpoints, and new tests were added to probe the endpoint behavior when 
> invalid credentials are supplied.
> 
> `sudo make check` was used to test on both OSX and Ubuntu 14.04. The new 
> tests were run 1000 times to look for flakiness.
> 
> 
> Thanks,
> 
> Greg Mann
> 
>

Reply via email to