mesos-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Greg Mann <g...@mesosphere.io>
Subject Re: Review Request 43639: Allowed dynamic reservation without a principal.
Date Mon, 22 Feb 2016 18:42:02 GMT


> On Feb. 18, 2016, 6:57 a.m., Guangya Liu wrote:
> > src/master/validation.cpp, lines 692-711
> > <https://reviews.apache.org/r/43639/diff/1/?file=1253091#file1253091line692>
> >
> >     Can you please add some test cases to cover those scenarios?
> >     
> >     1) authentication is enabled and reservation does not have principal.
> >     2) authentication is enabled and reservation pricipal does not same as authentication
principal.
> >     3) authentication is disabled but reservation has principal.
> 
> Guangya Liu wrote:
>     Seems 2) was covered by `ReservationEndpointsTest, NonMatchingPrincipal`, but 1)
and 3) was not covered for now.

Good call, I added a test case to the `ReservationTest` tests for #3.

Due to the way authentication works, I'm not sure that test case #1 is necessary. When authentication
is enabled, HTTP endpoints require a principal and frameworks must supply a principal to register.
The best we could do to test #1 is to have a request with an authenticated principal but no
principal in `ReservationInfo`, which is really just testing that a request with non-matching
principals will fail. The non-matching principal case is covered by `ReservationEndpointsTest.NonMatchingPrincipal`.
What do you think?


- Greg


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


On Feb. 22, 2016, 6:39 p.m., Greg Mann wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43639/
> -----------------------------------------------------------
> 
> (Updated Feb. 22, 2016, 6:39 p.m.)
> 
> 
> Review request for mesos, Michael Park and Neil Conway.
> 
> 
> Bugs: MESOS-3940
>     https://issues.apache.org/jira/browse/MESOS-3940
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Allowed dynamic reservation without a principal.
> 
> The `ReservationInfo.principal` field has been migrated to `optional`, which means we
can now allow dynamic reservation and unreservation without a principal. This allows the use
of the `/reserve` and `/unreserve` HTTP endpoints when HTTP authentication is disabled.
> 
> Note that we still require that frameworks/operators set the `ReservationInfo.principal`
field to match their own principal, if present. It may be desirable to remove this requirement;
this improvement is tracked in MESOS-4696.
> 
> 
> Diffs
> -----
> 
>   src/master/validation.cpp 66898e914c7b4ab83c4580be67530f355cfb05ca 
>   src/tests/master_validation_tests.cpp 6fae01fa1833ae05ec82618a4ae28ac5bd275bd5 
>   src/tests/reservation_endpoints_tests.cpp afe81b1d38a1b3a82583720f26482ddcde8f5e85

>   src/tests/reservation_tests.cpp d2ef15934556cb879f31850d52712aec77231fc7 
> 
> Diff: https://reviews.apache.org/r/43639/diff/
> 
> 
> Testing
> -------
> 
> A new test case was added to `ReservationTest.NoAuthentication`.
> 
> `make check` was used to test on OSX, both with and without SSL enabled.
> 
> Also manually reserved/unreserved resources using curl, with a command like this: `curl
-i -d slaveId="8288b2f0-e33d-4547-a2b4-5230ba6e5279-S0" -d resources='[ { "name": "cpus",
 "type": "SCALAR", "scalar": { "value": 3 }, "role": "ads", "reservation": { } } ]'  -X POST
http://127.0.0.1:5050/master/reserve`
> 
> Inspecting `/master/state` before & after these operations confirmed that the reserve/unreserve
operations were successful.
> 
> 
> Thanks,
> 
> Greg Mann
> 
>


Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message