mesos-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Till Toenshoff <toensh...@me.com>
Subject Re: Review Request 59100: Enabled authz on API calls which retrieve maintenance schedules.
Date Mon, 12 Jun 2017 21:34:28 GMT

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


Ship it!




Ship It!


src/master/http.cpp
Lines 4122 (patched)
<https://reviews.apache.org/r/59100/#comment251336>

    This confused me initially. 
    
    So do we really want to expose this to the caller given that this is a filtering endpoint
or would it be better to simply log warnings to the master log while returning a possibly
empty schedule?
    
    I can see that `src/common/http.cpp` does it similarly in e.g. `approveViewExecutorInfo`.
    
    We should agree on a solution here it seems, we keep copying the TODO :)
    
    So the question is, should we show this kind of error to the user requesting data on an
endpoint? Should we also show it to the administrator? To me it seems both questions should
be answered by yes -- objections?


- Till Toenshoff


On June 12, 2017, 2:08 p.m., Alexander Rojas wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/59100/
> -----------------------------------------------------------
> 
> (Updated June 12, 2017, 2:08 p.m.)
> 
> 
> Review request for mesos, Adam B, Greg Mann, and Till Toenshoff.
> 
> 
> Bugs: MESOS-7415
>     https://issues.apache.org/jira/browse/MESOS-7415
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Enables the use of authorization for the endpoint
> '/maintenance/schedule' as well as the 'GET_MAINTENANCE_SCHEDULE' v1
> API call, using the ACL 'GetMaintenanceSchedule' and the action of the
> same name as the API call.
> 
> It also updates the ApiTests to take into account the authorization
> case.
> 
> 
> Diffs
> -----
> 
>   src/master/http.cpp 1dcfe6ef00b0e3984deb79a511e665f638661323 
>   src/master/master.hpp e8ddddf273256b14cde1cac390163f948241757f 
>   src/tests/api_tests.cpp 91b3473452b8e65cab9f2e873837d64a0edf4b54 
> 
> 
> Diff: https://reviews.apache.org/r/59100/diff/7/
> 
> 
> Testing
> -------
> 
> make check
> 
> 
> Thanks,
> 
> Alexander Rojas
> 
>


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