> On Dec. 2, 2015, 4:05 p.m., Alexander Rukletsov wrote:
> > src/master/master.cpp, lines 3041-3043
> > <https://reviews.apache.org/r/39989/diff/12/?file=1150229#file1150229line3041>
> >
> >     We validate later on that in `principal` is `None`, reserve is aborted. 
> > IIUC, if `FrameworkInfo` does not specify principal, it cannot reserve 
> > resources. So the question is: do we need to check this first and only then 
> > proceed with authz?

Alex, from the impl. perspective, we thought about that in the earlier 
iteration, and found that it's quite difficult to do it cleanly given the 
current structure of the code. We want validation to be done at the same place 
(`Master::_accept`).

ALso, we had some discussion on whether 'principal' in ReservationInfo needs to 
be required or not (MESOS-3064). In the future, we might want to make it 
'optional' so that a framework without principal can also reserve resources 
(it's reserved resources can be unreserved by anyone).

So I would suggest we keep the current structure and add a comment here saying 
that: currently, if framework's principal is not specified, the operation 
validation will fail in `_accept` even thought authorization might succeed.


- Jie


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


On Dec. 2, 2015, 9:41 a.m., Greg Mann wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39989/
> -----------------------------------------------------------
> 
> (Updated Dec. 2, 2015, 9:41 a.m.)
> 
> 
> Review request for mesos, Adam B, Jie Yu, Michael Park, and Till Toenshoff.
> 
> 
> Bugs: MESOS-3062
>     https://issues.apache.org/jira/browse/MESOS-3062
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Added framework authorization for dynamic reservation.
> Note: this review is continued from https://reviews.apache.org/r/37127/
> 
> 
> Diffs
> -----
> 
>   src/master/master.cpp b918ae4a0e7dc3cd41165fc4b683ae7b6f031821 
>   src/tests/reservation_tests.cpp 15d180f92ec0aea99e6f3a7d0b505c62bd207b61 
> 
> Diff: https://reviews.apache.org/r/39989/diff/
> 
> 
> Testing
> -------
> 
> This is the fifth in a chain of 5 patches. New reservation tests were added 
> to `reservation_tests.cpp` to validate the authentication of framework 
> reserve and unreserve operations using ACLs. `make check` was run to test 
> after all patches were applied.
> 
> 
> Thanks,
> 
> Greg Mann
> 
>

Reply via email to