mesos-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Adam B <a...@mesosphere.io>
Subject Re: Review Request 54381: Adds authorization support when attaching containers input/output.
Date Tue, 06 Dec 2016 08:59:46 GMT


> On Dec. 5, 2016, 4:55 p.m., Vinod Kone wrote:
> > src/tests/api_tests.cpp, lines 3728-3793
> > <https://reviews.apache.org/r/54381/diff/1/?file=1576457#file1576457line3728>
> >
> >     why did you change these tests? the idea with the tests was to verify that `Containerizer::attach()`
was getting called from the API handler.
> 
> Alexander Rojas wrote:
>     Because one needs to find the containers to perform authorization, and with these
tests there was no container to be found, so no authz to be performed and therefor not mockContainerizer
to be called.

Alexander's code adds extra verification that the provided containerId maps to a known executor
before reaching the containerizer, which is why a `NotFound` is now returned and `attach`
is never called in these tests, since they use a completely random/invalid containerId. If
you want the tests to verify that `Containerizer::attach()` is getting called from the API
handler, then we should modify the tests to use a valid containerId, perhaps just the first
one listed in `/containers`, although that may require us to launch a task so that there is
a container running.


- Adam


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


On Dec. 5, 2016, 8:19 a.m., Alexander Rojas wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/54381/
> -----------------------------------------------------------
> 
> (Updated Dec. 5, 2016, 8:19 a.m.)
> 
> 
> Review request for mesos, Adam B, Kevin Klues, and Vinod Kone.
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Introduces authorization support for the newly introduced debug API
> calls: `AttachContainerInput` and `AttachContainerOutput`.
> 
> 
> Diffs
> -----
> 
>   src/slave/http.cpp 91eebe117d3dc86dd5b1fd47156c01e147165cef 
>   src/slave/slave.hpp 4b94dff9c1f9f212f84984674268ef38b43d93bd 
>   src/tests/api_tests.cpp 7c70f284a9225ca23abc7049f48f3efba314b641 
> 
> Diff: https://reviews.apache.org/r/54381/diff/
> 
> 
> Testing
> -------
> 
> make check
> 
> 
> Thanks,
> 
> Alexander Rojas
> 
>


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