mesos-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Jie Yu" <yujie....@gmail.com>
Subject Re: Review Request 39988: [4/5] Added authorization for dynamic reservation master endpoints.
Date Thu, 19 Nov 2015 23:20:00 GMT


> On Nov. 13, 2015, 11:38 p.m., Jie Yu wrote:
> > src/master/http.cpp, line 853
> > <https://reviews.apache.org/r/39988/diff/6/?file=1125444#file1125444line853>
> >
> >     This looks problematic to me. 'this' will become invalid once this function
returns. That means when `_reserve` is actually called after the authorization is done, it
might be a dangling pointer.
> >     
> >     Instead, you can try to copy the http object (something like below):
> >     
> >     ```
> >     lambda::function<Future<Response>(bool)> _reserve =
> >       lambda::bind(
> >           &Master::Http::_operation,
> >           *this,
> >           slaveId,
> >           resources.flatten(),
> >           operation,
> >           lambda::_1);
> >     
> >     return master->authorizeReserveResources(operation.reserve(), principal)
> >       .then(defer(master->self(), _reserve));
> >     ```
> 
> Greg Mann wrote:
>     Good catch, thanks!

Mpark reached out to me about this issue. Seems that this extra copying is not necessary (my
bad). The 'http' object will be bounded in the lambda function and stored in Master's libprocess
handlers map.


- Jie


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


On Nov. 16, 2015, 11:53 p.m., Greg Mann wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39988/
> -----------------------------------------------------------
> 
> (Updated Nov. 16, 2015, 11:53 p.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 authorization for dynamic reservation master endpoints.
> Note: this review is continued from https://reviews.apache.org/r/37126/
> 
> 
> Diffs
> -----
> 
>   src/master/http.cpp 1c4f1406f5d917f5d655a7d61d311365f8999ce0 
>   src/master/master.hpp 5e5a575dc7dd49324f3c837028df8a7f75cd1f80 
>   src/tests/reservation_endpoints_tests.cpp 1552e4537c4f4d79bfa4bc17ccab2df630bc32a4

> 
> Diff: https://reviews.apache.org/r/39988/diff/
> 
> 
> Testing
> -------
> 
> This is the fourth in a chain of 5 patches. Added new reservation endpoints tests to
validate authorization of reserve and unreserve operations using ACLs. `make check` was run
to test after all patches were applied.
> 
> 
> Thanks,
> 
> Greg Mann
> 
>


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