Mailing-List: contact reviews-help@mesos.apache.org; run by ezmlm
Precedence: bulk
Reply-To: reviews@mesos.apache.org
Content-Type: multipart/alternative;
 boundary="===============6503091203517685117=="
MIME-Version: 1.0
Subject: Re: Review Request 61189: Added authorization for V1 events.
From: Greg Mann <greg@mesosphere.io>
To: Anand Mazumdar <anand@apache.org>, Greg Mann <greg@mesosphere.io>,
 Alexander Rojas <alexander@mesosphere.io>
Cc: Quinn Leng <quinn.leng.666@gmail.com>, mesos <reviews@mesos.apache.org>
Date: Wed, 30 Aug 2017 19:37:05 -0000
Message-ID: <20170830193705.20155.4447@reviews-vm2.apache.org>
Auto-Submitted: auto-generated
Sender: Greg Mann <noreply@reviews.apache.org>
References: <20170819033334.44503.57412@reviews-vm2.apache.org>
In-Reply-To: <20170819033334.44503.57412@reviews-vm2.apache.org>
Reply-To: Greg Mann <greg@mesosphere.io>
archived-at: Wed, 30 Aug 2017 19:37:10 -0000

--===============6503091203517685117==
MIME-Version: 1.0
Content-Type: text/plain; charset="utf-8"
Content-Transfer-Encoding: 7bit


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


Ship it!


LGTM, thanks Quinn! A couple small comments below, but I can fix them while committing.


src/tests/api_tests.cpp
Lines 2015 (patched)
<https://reviews.apache.org/r/61189/#comment260142>

    I think you moved the wrong `scheduler` declaration - I was referring to the one in the test you added :)
    
    I can fix this while committing.


src/tests/api_tests.cpp
Lines 2420-2422 (patched)
<https://reviews.apache.org/r/61189/#comment260143>

    Ah sorry, let's place this block directly after the `AWAIT_READY(event)` call. I can do this while committing.


src/tests/api_tests.cpp
Lines 2421-2422 (patched)
<https://reviews.apache.org/r/61189/#comment260144>

    Fits on one line. I can fix this while committing.


- Greg Mann


On Aug. 19, 2017, 3:33 a.m., Quinn Leng wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/61189/
> -----------------------------------------------------------
> 
> (Updated Aug. 19, 2017, 3:33 a.m.)
> 
> 
> Review request for mesos, Anand Mazumdar, Alexander Rojas, and Greg Mann.
> 
> 
> Bugs: MESOS-7785
>     https://issues.apache.org/jira/browse/MESOS-7785
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Added authorization filtering for the V1 operator events on the
> master, the subscriber should only receive events that are
> authorized based on their principal and ACLs. Since the authorizer
> is called for every event emitted on the stream, the change of
> authorization rule will affect events that the subscribers can
> receive.
> 
> 
> Diffs
> -----
> 
>   src/master/http.cpp b09a9d0ab5fab179d0e400932dce7d3ca958d7a8 
>   src/master/master.hpp d9cfc42646c874661e4ec79322126d93a2caf604 
>   src/master/master.cpp 7f38a5e21884546d4b4c866ca5918db779af8f99 
>   src/tests/api_tests.cpp 34480ea6818c904c64cb678562866f0b43dae0d6 
> 
> 
> Diff: https://reviews.apache.org/r/61189/diff/6/
> 
> 
> Testing
> -------
> 
> make check
> GLOG_v=2 ./bin/mesos-tests.sh --gtest_filter="ContentType/MasterAPITest.EventsAuthorizationFiltering*" --verbose --gtest_repeat=100 --gtest_break_on_failure
> 
> 
> Thanks,
> 
> Quinn Leng
> 
>


--===============6503091203517685117==--