mesos-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Jan Schlicht <...@mesosphere.io>
Subject Re: Review Request 47530: Added authorization to agent's '/containers' endpoint.
Date Wed, 18 May 2016 12:55:44 GMT

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




src/slave/http.cpp (line 756)
<https://reviews.apache.org/r/47530/#comment198316>

    Please make sure that `request.method` is "GET" when this function is called with enabled
authorization. See `Slave::Http::flags` for an example.
    
    Currently, for many existing endpoints, the request method isn't checked which can lead
to problems with authorization. We plan to change that later, see [MESOS-5346](https://issues.apache.org/jira/browse/MESOS-5346).



src/slave/http.cpp (line 786)
<https://reviews.apache.org/r/47530/#comment198322>

    Please call `authorizeEndpoint` as soon as possible, i.e. after the endpoint has been
extracted from the URL.
    
    While I like the idea of doing work in parallel, by requesting the containerizer statuses
prior to the authorization, this work should only be done after the authorization was successful.
Hence this part should be in the `_containers` continuation.



src/slave/slave.hpp (lines 96 - 97)
<https://reviews.apache.org/r/47530/#comment198315>

    Please don't do this in a header. The `using namespace process;` above is a bad example
and probably shouldn't even be there.


- Jan Schlicht


On May 18, 2016, 12:06 p.m., Abhishek Dasgupta wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/47530/
> -----------------------------------------------------------
> 
> (Updated May 18, 2016, 12:06 p.m.)
> 
> 
> Review request for mesos, Adam B, Alexander Rukletsov, Greg Mann, Jan Schlicht, and Till
Toenshoff.
> 
> 
> Bugs: MESOS-5317
>     https://issues.apache.org/jira/browse/MESOS-5317
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Used GET_ENDPOINT_WITH_PATH coarse-grained authz on agent's 
>  '/containers' endpoint to enable authorization on this endpoint.
>  Updated docs and testcases as well.
> 
> 
> Diffs
> -----
> 
>   docs/endpoints/slave/containers.md 959f40b9db4de4b6cea456ecf7bcb402f7a94f05 
>   src/slave/http.cpp fb48ec61e2fe0c83f80d3b8aa4c2ef5a96b748ae 
>   src/slave/slave.hpp 209f071448e3c52d16d3366d564003ee36b1d2e0 
>   src/tests/slave_authorization_tests.cpp 843cf1c631e0a25125ca1c0c0028ad1a920c2c2f 
> 
> Diff: https://reviews.apache.org/r/47530/diff/
> 
> 
> Testing
> -------
> 
> sudo GTEST_FILTER="*SlaveEndpointTest*.*" make -j2 check
> 
> On ubuntu 16.04.
> 
> Ran manual testing as well.
> 
> 
> Thanks,
> 
> Abhishek Dasgupta
> 
>


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