You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@mesos.apache.org by Greg Mann <gr...@mesosphere.io> on 2016/01/20 19:59:16 UTC
Re: Review Request 42368: Added reservation endpoint test without
authentication.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/42368/
-----------------------------------------------------------
(Updated Jan. 20, 2016, 6:59 p.m.)
Review request for mesos, Jie Yu, Michael Park, and Vinod Kone.
Changes
-------
Updated comments.
Summary (updated)
-----------------
Added reservation endpoint test without authentication.
Bugs: MESOS-4195
https://issues.apache.org/jira/browse/MESOS-4195
Repository: mesos
Description (updated)
-------
Added reservation endpoint test without authentication.
Currently, dynamic reservation endpoints will not work when HTTP authentication is not set. Currently, the authentication code will always return `None()` as the principal when authentication is disabled. Furthermore, the `ReservationInfo.principal` field is being migrated to `optional`, so dynamic reservations without a principal are invalidated by the master. This test checks that these endpoints will fail as expected when HTTP authentication is disabled.
Diffs (updated)
-----
src/tests/reservation_endpoints_tests.cpp b8edd6fafedd4c2221a8d19c1ebc71254071a8c7
Diff: https://reviews.apache.org/r/42368/diff/
Testing
-------
A new test, `ReservationEndpointsTest.ReserveAndUnreserveNoAuthentication`, was added, and make check was used to test. The new test was also run with `--gtest_repeat=1000 --gtest_break_on_failure=1`.
Thanks,
Greg Mann
Re: Review Request 42368: Added reservation endpoint test without
authentication.
Posted by Mesos ReviewBot <re...@mesos.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/42368/#review115795
-----------------------------------------------------------
Patch looks great!
Reviews applied: [42530, 42361, 42368]
Passed command: export OS=ubuntu:14.04;export CONFIGURATION="--verbose";export COMPILER=gcc; ./support/docker_build.sh
- Mesos ReviewBot
On Jan. 22, 2016, 3:33 a.m., Greg Mann wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42368/
> -----------------------------------------------------------
>
> (Updated Jan. 22, 2016, 3:33 a.m.)
>
>
> Review request for mesos, Jie Yu, Michael Park, and Vinod Kone.
>
>
> Bugs: MESOS-4195
> https://issues.apache.org/jira/browse/MESOS-4195
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Added reservation endpoint test without authentication.
>
> Currently, dynamic reservation endpoints will not work when HTTP authentication is not set. Currently, the authentication code will always return `None()` as the principal when authentication is disabled. Furthermore, the `ReservationInfo.principal` field is being migrated to `optional`, so dynamic reservations without a principal are invalidated by the master. This test checks that these endpoints will fail as expected when HTTP authentication is disabled.
>
>
> Diffs
> -----
>
> src/tests/reservation_endpoints_tests.cpp a471f858061a694073afeebdf76be659da938d01
>
> Diff: https://reviews.apache.org/r/42368/diff/
>
>
> Testing
> -------
>
> A new test, `ReservationEndpointsTest.ReserveAndUnreserveNoAuthentication`, was added, and make check was used to test. The new test was also run with `--gtest_repeat=1000 --gtest_break_on_failure=1`.
>
>
> Thanks,
>
> Greg Mann
>
>
Re: Review Request 42368: Added reservation endpoint test without
authentication.
Posted by Greg Mann <gr...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/42368/
-----------------------------------------------------------
(Updated Jan. 22, 2016, 3:33 a.m.)
Review request for mesos, Jie Yu, Michael Park, and Vinod Kone.
Changes
-------
Addressed comments; removed extraneous test cases.
Bugs: MESOS-4195
https://issues.apache.org/jira/browse/MESOS-4195
Repository: mesos
Description
-------
Added reservation endpoint test without authentication.
Currently, dynamic reservation endpoints will not work when HTTP authentication is not set. Currently, the authentication code will always return `None()` as the principal when authentication is disabled. Furthermore, the `ReservationInfo.principal` field is being migrated to `optional`, so dynamic reservations without a principal are invalidated by the master. This test checks that these endpoints will fail as expected when HTTP authentication is disabled.
Diffs (updated)
-----
src/tests/reservation_endpoints_tests.cpp a471f858061a694073afeebdf76be659da938d01
Diff: https://reviews.apache.org/r/42368/diff/
Testing
-------
A new test, `ReservationEndpointsTest.ReserveAndUnreserveNoAuthentication`, was added, and make check was used to test. The new test was also run with `--gtest_repeat=1000 --gtest_break_on_failure=1`.
Thanks,
Greg Mann
Re: Review Request 42368: Added reservation endpoint test without
authentication.
Posted by Greg Mann <gr...@mesosphere.io>.
> On Jan. 21, 2016, 10:42 p.m., Michael Park wrote:
> >
Thanks MPark! Per our discussion, I've removed all of the requests from this test except the reserve and unreserve requests which contain no authentication headers as well as no principal in `ReservationInfo`, since the other requests were actually either testing for non-matching principals or testing that HTTP authentication behaves as we expect.
- Greg
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/42368/#review115715
-----------------------------------------------------------
On Jan. 22, 2016, 3:33 a.m., Greg Mann wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42368/
> -----------------------------------------------------------
>
> (Updated Jan. 22, 2016, 3:33 a.m.)
>
>
> Review request for mesos, Jie Yu, Michael Park, and Vinod Kone.
>
>
> Bugs: MESOS-4195
> https://issues.apache.org/jira/browse/MESOS-4195
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Added reservation endpoint test without authentication.
>
> Currently, dynamic reservation endpoints will not work when HTTP authentication is not set. Currently, the authentication code will always return `None()` as the principal when authentication is disabled. Furthermore, the `ReservationInfo.principal` field is being migrated to `optional`, so dynamic reservations without a principal are invalidated by the master. This test checks that these endpoints will fail as expected when HTTP authentication is disabled.
>
>
> Diffs
> -----
>
> src/tests/reservation_endpoints_tests.cpp a471f858061a694073afeebdf76be659da938d01
>
> Diff: https://reviews.apache.org/r/42368/diff/
>
>
> Testing
> -------
>
> A new test, `ReservationEndpointsTest.ReserveAndUnreserveNoAuthentication`, was added, and make check was used to test. The new test was also run with `--gtest_repeat=1000 --gtest_break_on_failure=1`.
>
>
> Thanks,
>
> Greg Mann
>
>
Re: Review Request 42368: Added reservation endpoint test without
authentication.
Posted by Michael Park <mp...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/42368/#review115715
-----------------------------------------------------------
src/tests/reservation_endpoints_tests.cpp (lines 1159 - 1160)
<https://reviews.apache.org/r/42368/#comment176752>
Can we leave the `TODO` here saying this should work and should be fixed in `0.28`? There are like 5 or 6 cases in this test, so the note at the top doesn't tell us which ones need to be updated.
src/tests/reservation_endpoints_tests.cpp (line 1161)
<https://reviews.apache.org/r/42368/#comment176749>
Can we name these as `dynamicallyReservedWithNoPrincipal`, and for the rest as well? e.g. `dynamicallyReservedWithDefaultPrincipal`.
src/tests/reservation_endpoints_tests.cpp (lines 1174 - 1176)
<https://reviews.apache.org/r/42368/#comment176760>
(1) `HTTP authorization`? -- Isn't it authentication?
(2) `and dynamic reservations are currently unallowed without a principal`? -- What does this mean? We passed a `ReservationInfo` with a `principal` set here, and generally, passing `None()` as the authentication header isn't disallowed.
src/tests/reservation_endpoints_tests.cpp (line 1192)
<https://reviews.apache.org/r/42368/#comment176750>
Can we call this: `dynamicallyReservedWithNonMatchingPrincipal`? "New principal" makes it sound as if we're in the context of trying to update an existing principal to a new principal or something.
- Michael Park
On Jan. 20, 2016, 6:59 p.m., Greg Mann wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42368/
> -----------------------------------------------------------
>
> (Updated Jan. 20, 2016, 6:59 p.m.)
>
>
> Review request for mesos, Jie Yu, Michael Park, and Vinod Kone.
>
>
> Bugs: MESOS-4195
> https://issues.apache.org/jira/browse/MESOS-4195
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Added reservation endpoint test without authentication.
>
> Currently, dynamic reservation endpoints will not work when HTTP authentication is not set. Currently, the authentication code will always return `None()` as the principal when authentication is disabled. Furthermore, the `ReservationInfo.principal` field is being migrated to `optional`, so dynamic reservations without a principal are invalidated by the master. This test checks that these endpoints will fail as expected when HTTP authentication is disabled.
>
>
> Diffs
> -----
>
> src/tests/reservation_endpoints_tests.cpp b8edd6fafedd4c2221a8d19c1ebc71254071a8c7
>
> Diff: https://reviews.apache.org/r/42368/diff/
>
>
> Testing
> -------
>
> A new test, `ReservationEndpointsTest.ReserveAndUnreserveNoAuthentication`, was added, and make check was used to test. The new test was also run with `--gtest_repeat=1000 --gtest_break_on_failure=1`.
>
>
> Thanks,
>
> Greg Mann
>
>
Re: Review Request 42368: Added reservation endpoint test without
authentication.
Posted by Mesos ReviewBot <re...@mesos.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/42368/#review115764
-----------------------------------------------------------
Patch looks great!
Reviews applied: [42530, 42361, 42368]
Passed command: export OS=ubuntu:14.04;export CONFIGURATION="--verbose";export COMPILER=gcc; ./support/docker_build.sh
- Mesos ReviewBot
On Jan. 20, 2016, 6:59 p.m., Greg Mann wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42368/
> -----------------------------------------------------------
>
> (Updated Jan. 20, 2016, 6:59 p.m.)
>
>
> Review request for mesos, Jie Yu, Michael Park, and Vinod Kone.
>
>
> Bugs: MESOS-4195
> https://issues.apache.org/jira/browse/MESOS-4195
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Added reservation endpoint test without authentication.
>
> Currently, dynamic reservation endpoints will not work when HTTP authentication is not set. Currently, the authentication code will always return `None()` as the principal when authentication is disabled. Furthermore, the `ReservationInfo.principal` field is being migrated to `optional`, so dynamic reservations without a principal are invalidated by the master. This test checks that these endpoints will fail as expected when HTTP authentication is disabled.
>
>
> Diffs
> -----
>
> src/tests/reservation_endpoints_tests.cpp b8edd6fafedd4c2221a8d19c1ebc71254071a8c7
>
> Diff: https://reviews.apache.org/r/42368/diff/
>
>
> Testing
> -------
>
> A new test, `ReservationEndpointsTest.ReserveAndUnreserveNoAuthentication`, was added, and make check was used to test. The new test was also run with `--gtest_repeat=1000 --gtest_break_on_failure=1`.
>
>
> Thanks,
>
> Greg Mann
>
>