Re: Review Request 42368: Added reservation endpoint test without authentication.

2016-01-21 Thread Michael Park 

---
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)


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)


Can we name these as `dynamicallyReservedWithNoPrincipal`, and for the rest 
as well? e.g. `dynamicallyReservedWithDefaultPrincipal`.



src/tests/reservation_endpoints_tests.cpp (lines 1174 - 1176)


(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)


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.

2016-01-21 Thread Greg Mann 


> 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.

2016-01-21 Thread Greg Mann 

---
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.

2016-01-21 Thread Mesos ReviewBot 

---
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
> 
>

Re: Review Request 42368: Added reservation endpoint test without authentication.

2016-01-20 Thread Greg Mann 

---
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