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 42164: Allowed (un)reserve operations without a principal.
Date Thu, 14 Jan 2016 20:58:55 GMT


> On Jan. 13, 2016, 5:22 a.m., Michael Park wrote:
> > src/master/validation.cpp, line 703
> > <https://reviews.apache.org/r/42164/diff/5/?file=1194762#file1194762line703>
> >
> >     First thing here is that the `principal` field in `ReservationInfo` is still
marked `required`. What's our plan for changing it to `optional`?
> >     
> >     Second thing is that even if `principal.isNone()`, if is authorization is turned
off we don't care, right? That is, if authorization is turned off, even frameworks without
a principal can reserve any resources?
> 
> Greg Mann wrote:
>     Regarding your first question: per our discussion, we do indeed need a good plan
for migrating to an `optional` principal within `ReservationInfo`. It would seem that we have
to choose between two less-than-ideal situations: either we tolerate HTTP endpoints that don't
work when authentication is disabled, or we enforce that frameworks cannot register with `principal
== ""`, so that we can use the empty string to represent a null principal while we migrate
to an `optional` principal over a deprecation cycle.
>     
>     To your second point: again per our discussion, semantically the principal within
`ReservationInfo` represents the principal (or lack thereof) which reserved the resources,
so we should enforce that it matches the principal of the framework/operator, even if authorization
is not enabled.
> 
> Guangya Liu wrote:
>     If authorization is not enabled, then it is not a must to set principal for a framework,
so how can we enforce the match of principal?
> 
> Greg Mann wrote:
>     We should enforce that the principal (or lack of a principal) used for authorization
is the same as the principal in the `ReservationInfo`. So if no principal is provided (i.e.,
authorization disabled), then there should be no principal in `ReservationInfo`. Since the
principal in `ReservationInfo` is currently a required field, we would need to use something
like the empty string `""` to represent the case of no principal. Ideally, the principal field
in `ReservationInfo` should be an optional field so that it can be omitted when no principal
is used. The plan is to change this field to optional, but we're debating how and when it
will be best to do so. Any thoughts on how best to do this?
> 
> Guangya Liu wrote:
>     Compared with `HTTP endpoints that don't work when authentication is disabled`, I
think that `enforce that frameworks cannot register with principal == ""` might be better
as it will only impact some framework logic and more simple, but authorization require the
whole cluster and all frameworks must enable authorization.

So after some discussions it sounds like we're going to wait on implementing this patch until
we have migrated the `principal` field in `ReservationInfo` to `optional`. So for the time
being, HTTP authentication will be required in order to use the `/reserve` and `/unreserve`
endpoints. There are several options, none of which are ideal, and this seems to be the cleanest
solution overall.


- Greg


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


On Jan. 13, 2016, 2:45 a.m., Greg Mann wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42164/
> -----------------------------------------------------------
> 
> (Updated Jan. 13, 2016, 2:45 a.m.)
> 
> 
> Review request for mesos, Alexander Rojas, Michael Park, and Neil Conway.
> 
> 
> Bugs: MESOS-3940
>     https://issues.apache.org/jira/browse/MESOS-3940
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Allowed (un)reserve operations without a principal.
> 
> 
> Diffs
> -----
> 
>   src/master/validation.cpp 6a43bce5b7df6a9d939245c4726d060fa19eb305 
>   src/tests/master_validation_tests.cpp fbf8fadbc04a7cbc60ee6091e0224339389b400f 
>   src/tests/reservation_endpoints_tests.cpp b8edd6fafedd4c2221a8d19c1ebc71254071a8c7

> 
> Diff: https://reviews.apache.org/r/42164/diff/
> 
> 
> Testing
> -------
> 
> Two master validation tests were removed which tested to make sure that reserve and unreserve
operations would not succeed without a principal.
> 
> `make check` was used to test on both OSX and Ubuntu 14.04
> 
> 
> Thanks,
> 
> Greg Mann
> 
>


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