Mailing-List: contact reviews-help@mesos.apache.org; run by ezmlm
Precedence: bulk
Reply-To: reviews@mesos.apache.org
Content-Type: multipart/alternative;
 boundary="===============0694045657217955374=="
MIME-Version: 1.0
Subject: Re: Review Request 36048: Updated authorizer to allow for modularized
 implementations.
From: "Till Toenshoff" <toenshoff@me.com>
To: "Adam B" <adam@mesosphere.io>, "Kapil Arya" <kapil@mesosphere.io>,
 "Till Toenshoff" <toenshoff@me.com>, "Bernd Mathiske" <bernd@mesosphere.io>,
 "Alexander Rukletsov" <rukletsov@gmail.com>,
 "Jan Schlicht" <jan@mesosphere.io>
Cc: "mesos" <reviews@mesos.apache.org>,
 "Alexander Rojas" <alexander@mesosphere.io>
Date: Mon, 03 Aug 2015 10:52:23 -0000
Message-ID: <20150803105223.4205.20469@reviews.apache.org>
Auto-Submitted: auto-generated
Sender: "Till Toenshoff" <noreply@reviews.apache.org>
References: <20150803094711.4206.51571@reviews.apache.org>
In-Reply-To: <20150803094711.4206.51571@reviews.apache.org>
Reply-To: "Till Toenshoff" <toenshoff@me.com>

--===============0694045657217955374==
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/36048/#review93884
-----------------------------------------------------------

Ship it!


I think this looks ready for landing after fixing the remaining minor issues.


src/authorizer/authorizer.hpp (line 60)
<https://reviews.apache.org/r/36048/#comment148300>

    Add a comment please on what we do with this flag and why a Once or alike won't do.


src/authorizer/authorizer.cpp (line 236)
<https://reviews.apache.org/r/36048/#comment148301>

    s/LocalAuthorizer/local authorizer/


src/local/local.cpp (line 224)
<https://reviews.apache.org/r/36048/#comment148302>

    Seems a failure to instantiate the authorizer should not be connected to the settings on --acls - we should get rid of the "(see --acls flag)", no?


src/tests/cluster.hpp (line 60)
<https://reviews.apache.org/r/36048/#comment148305>

    I think this should move up, a couple of lines, above files/files.hpp


src/tests/cluster.hpp (line 359)
<https://reviews.apache.org/r/36048/#comment148306>

    "--acls" has no influence on the instantiation as commented above, please remove that part from the message.


- Till Toenshoff


On Aug. 3, 2015, 9:47 a.m., Alexander Rojas wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/36048/
> -----------------------------------------------------------
> 
> (Updated Aug. 3, 2015, 9:47 a.m.)
> 
> 
> Review request for mesos, Adam B, Alexander Rukletsov, Bernd Mathiske, Kapil Arya, Jan Schlicht, and Till Toenshoff.
> 
> 
> Bugs: MESOS-2946
>     https://issues.apache.org/jira/browse/MESOS-2946
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Splits and updates the original declaration of the `Authorizer` into its interface and a default implementation, the `LocalAuthorizer`.
> 
> Following the pattern of the modularized `Authenticator`, it generates a default constructor which is required when writing a `TYPED_TEST` in
> a follow up patch. Additionally, an initialize method has been added, needed for passing in the current ACL definitions as provided via
> flags.
> 
> Other changes are just updates to allow for compilation.
> 
> 
> Diffs
> -----
> 
>   include/mesos/authorizer/authorizer.hpp PRE-CREATION 
>   include/mesos/authorizer/authorizer.proto PRE-CREATION 
>   include/mesos/mesos.proto cb24125a3f05e0d38fb22e481a15ceb21f882d27 
>   include/mesos/type_utils.hpp 86b37ca1f63e4687af4e86731dceeb1c9c219c5e 
>   src/Makefile.am e2cbd1524f25b3e3a9419af6bdf8cc6e5022a784 
>   src/authorizer/authorizer.hpp c039d9412780aa199db169b31991bf9f45b07d0f 
>   src/authorizer/authorizer.cpp 21e97e315478a4ca9442af83732665f85eb2f8fc 
>   src/common/parse.hpp 8d7ddd6819dad98cd96d5aaae8fe57caf1ee7098 
>   src/examples/persistent_volume_framework.cpp c6d6ed337bfca91dc146cb31298cabebdbb13509 
>   src/local/local.cpp 1953d84c75a83f4ace944d6243456235d8a193ff 
>   src/master/flags.hpp f2cd19a6edfaa4e5bb31f024ef8d5beda32fbc2f 
>   src/master/http.cpp 6f5ca02c52462495b84e77525a6c88299746ece2 
>   src/master/main.cpp fd4de4d0d9c3e9617408022d10b5e161bdc911e1 
>   src/master/master.hpp 2343a684402972a8c336c0dcdde0bfaffabe7cec 
>   src/master/master.cpp 2f00f240ed2cd59ec0c2eae7fd2567f0edb8d9e0 
>   src/tests/authorization_tests.cpp 99bb06c1ee73a90abaeeabb742e45aa188c21a87 
>   src/tests/cluster.hpp ba17c0c74a9dc36c595c4ad77fe68be94c5c7c0b 
>   src/tests/mesos.hpp 23d9841c7cfb16abef89a53d29a256a2c5e94b52 
>   src/tests/mesos.cpp f09ef0f99573716de8905f49dcc0c9df20e31ea9 
> 
> Diff: https://reviews.apache.org/r/36048/diff/
> 
> 
> Testing
> -------
> 
> make check
> 
> 
> Thanks,
> 
> Alexander Rojas
> 
>


--===============0694045657217955374==--