You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@mesos.apache.org by Jan Schlicht <ja...@mesosphere.io> on 2016/04/08 10:52:18 UTC
Review Request 45922: Added agent authorization flags.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/45922/
-----------------------------------------------------------
Review request for mesos, Adam B and Alexander Rojas.
Bugs: MESOS-5142
https://issues.apache.org/jira/browse/MESOS-5142
Repository: mesos
Description
-------
See summary.
Diffs
-----
src/local/local.cpp df72ac52110b75d63df1076496b48e63d06d42ce
src/master/flags.cpp e522499586b731d522180f171731a9dd38b8344c
src/slave/constants.hpp 449c8cd9f43f71b4612023eb463969e9db0bc960
src/slave/flags.hpp 4236b7fe40b9aa26d57b651be06a0e36037f65c6
src/slave/flags.cpp 7164afef9c82a9bba0f137d7387c3569912b6599
src/slave/main.cpp 70df4f384b09a1fb078cd893efe52a5e3b116f48
src/slave/slave.hpp 76f3aff394e5cecc54dcb3065cb872e238bc228e
src/slave/slave.cpp f090c853b8affc4be5eecb4f616ec881fc2b60c3
src/tests/cluster.hpp 39ca15e3cc2f4f2ac9b8a73482c2bd945b1824e3
src/tests/cluster.cpp 7e488d28b7a0d3bef9cd76bf7df2de4822256ef6
src/tests/mesos.cpp 1b7a8fd703b90d77ffa3df079bdc2744752caac6
Diff: https://reviews.apache.org/r/45922/diff/
Testing
-------
make check
Thanks,
Jan Schlicht
Re: Review Request 45922: Added agent authorization flags.
Posted by Jan Schlicht <ja...@mesosphere.io>.
> On April 15, 2016, 1:33 p.m., Benjamin Bannier wrote:
> > src/tests/mesos.cpp, lines 373-377
> > <https://reviews.apache.org/r/45922/diff/6/?file=1345038#file1345038line373>
> >
> > Could you add a similar helper to inject e.g., a mock authorizer?
It looks like these helper functions are created on demand, so I'd rather not add one here now.
- Jan
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/45922/#review129097
-----------------------------------------------------------
On April 18, 2016, 11:49 a.m., Jan Schlicht wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45922/
> -----------------------------------------------------------
>
> (Updated April 18, 2016, 11:49 a.m.)
>
>
> Review request for mesos, Adam B and Alexander Rojas.
>
>
> Bugs: MESOS-5142
> https://issues.apache.org/jira/browse/MESOS-5142
>
>
> Repository: mesos
>
>
> Description
> -------
>
> See summary.
>
>
> Diffs
> -----
>
> docs/configuration.md cd9733002a0940f44edd4e56593a0ca3fe9f77f5
> src/local/local.cpp df72ac52110b75d63df1076496b48e63d06d42ce
> src/slave/constants.hpp 9978c11fec40055dd42f19c20cd3e9fef4e78cea
> src/slave/flags.hpp ee520acc459564fe68272950948fc80c5e24513a
> src/slave/flags.cpp 10d2974bd2b6e79255fc894979607f0d2d00c315
> src/slave/main.cpp 38bd00584dd9c6a872398678b2288edeed1cd2a4
> src/slave/slave.hpp f78c1b4e4d5378ef7223c6eb3ea45491c30fb4c1
> src/slave/slave.cpp d82dec2b10d496065013eb4ad6a35dc054b72553
> src/tests/cluster.hpp 887342acc72b33b4c904d610da47394f9a7d7188
> src/tests/cluster.cpp 31d2556a078429dd45315aa74cd21ad436372d31
> src/tests/mesos.hpp e4b63d41d883807ac39846799468b80e88c84e0b
> src/tests/mesos.cpp b5937af7713e1ee2af475518b3e968b2defa8beb
>
> Diff: https://reviews.apache.org/r/45922/diff/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Jan Schlicht
>
>
Re: Review Request 45922: Added agent authorization flags.
Posted by Benjamin Bannier <be...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/45922/#review129097
-----------------------------------------------------------
src/tests/mesos.cpp (lines 373 - 377)
<https://reviews.apache.org/r/45922/#comment192565>
Could you add a similar helper to inject e.g., a mock authorizer?
- Benjamin Bannier
On April 14, 2016, 6:51 p.m., Jan Schlicht wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45922/
> -----------------------------------------------------------
>
> (Updated April 14, 2016, 6:51 p.m.)
>
>
> Review request for mesos, Adam B and Alexander Rojas.
>
>
> Bugs: MESOS-5142
> https://issues.apache.org/jira/browse/MESOS-5142
>
>
> Repository: mesos
>
>
> Description
> -------
>
> See summary.
>
>
> Diffs
> -----
>
> docs/configuration.md ce51f26f0584eeecb5c96e39a5bcc21f5906a29c
> src/local/local.cpp df72ac52110b75d63df1076496b48e63d06d42ce
> src/slave/constants.hpp 9978c11fec40055dd42f19c20cd3e9fef4e78cea
> src/slave/flags.hpp ee520acc459564fe68272950948fc80c5e24513a
> src/slave/flags.cpp 316feec26e52a8cb1f0fd0739176b3f502e1ed86
> src/slave/main.cpp 70df4f384b09a1fb078cd893efe52a5e3b116f48
> src/slave/slave.hpp f78c1b4e4d5378ef7223c6eb3ea45491c30fb4c1
> src/slave/slave.cpp de99e9eb5cc812b2e07deb749b98b4f4db363728
> src/tests/cluster.hpp 887342acc72b33b4c904d610da47394f9a7d7188
> src/tests/cluster.cpp b4d69106388892b88f7de20b248cac8b950b861c
> src/tests/mesos.cpp 1b7a8fd703b90d77ffa3df079bdc2744752caac6
>
> Diff: https://reviews.apache.org/r/45922/diff/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Jan Schlicht
>
>
Re: Review Request 45922: Added agent authorization flags.
Posted by Adam B <ad...@mesosphere.io>.
> On April 20, 2016, 12:26 a.m., Adam B wrote:
> > src/tests/cluster.hpp, line 151
> > <https://reviews.apache.org/r/45922/diff/7/?file=1348010#file1348010line151>
> >
> > Why do you even need the overload for the authorizer here? Seems like most tests will either provide --acls and use the default, or set the modules/authorizer flags. We can leave this part out until a test needs it.
>
> Jan Schlicht wrote:
> We will need the overload to be able to use mocked authorizers. This will be important for later authz patches. Of course it is not needed right now, and I could remove it until a test needs it, on the other hand I think it should be part of this patch to provide a "ready-to-use" implementation of an agent authorizer.
Ok, I'm generally against adding things in case we need them later, but I agree that mock authorizers will be useful for testing later patches. Dropping the issue.
- Adam
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/45922/#review129694
-----------------------------------------------------------
On April 21, 2016, 7:48 a.m., Jan Schlicht wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45922/
> -----------------------------------------------------------
>
> (Updated April 21, 2016, 7:48 a.m.)
>
>
> Review request for mesos, Adam B and Alexander Rojas.
>
>
> Bugs: MESOS-5142
> https://issues.apache.org/jira/browse/MESOS-5142
>
>
> Repository: mesos
>
>
> Description
> -------
>
> See summary.
>
>
> Diffs
> -----
>
> docs/configuration.md 86ba66ac62295ca148524bcb2e57fee560ac4ac5
> src/local/local.cpp 7de8a2423185e49dfa849d069938d3243b4f4956
> src/slave/constants.hpp 9978c11fec40055dd42f19c20cd3e9fef4e78cea
> src/slave/flags.hpp ee520acc459564fe68272950948fc80c5e24513a
> src/slave/flags.cpp 10d2974bd2b6e79255fc894979607f0d2d00c315
> src/slave/main.cpp 38bd00584dd9c6a872398678b2288edeed1cd2a4
> src/slave/slave.hpp f78c1b4e4d5378ef7223c6eb3ea45491c30fb4c1
> src/slave/slave.cpp d82dec2b10d496065013eb4ad6a35dc054b72553
> src/tests/cluster.hpp 96ec52af16776e91200aa755c7847f56e33d71f4
> src/tests/cluster.cpp 3e5fdf6b32a0d99d3ca83743386232d38471e34f
> src/tests/mesos.hpp e4b63d41d883807ac39846799468b80e88c84e0b
> src/tests/mesos.cpp b5937af7713e1ee2af475518b3e968b2defa8beb
>
> Diff: https://reviews.apache.org/r/45922/diff/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Jan Schlicht
>
>
Re: Review Request 45922: Added agent authorization flags.
Posted by Jan Schlicht <ja...@mesosphere.io>.
> On April 20, 2016, 9:26 a.m., Adam B wrote:
> > src/tests/cluster.cpp, lines 406-412
> > <https://reviews.apache.org/r/45922/diff/7/?file=1348011#file1348011line406>
> >
> > Why is it an error to start an agent with no authorizer and no ACLs? What if I don't want to do any authorization?
This was due to a bug that will set `slave::Slave::authorizer` to `nullptr` instead of `None()` if no ACLs were provided -- which is how to disable authorization. While the above code fixed that, it also removed the possibility to disable agent authorization in tests. I've changed the approach, to support tests with disabled agent authorization.
- Jan
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/45922/#review129694
-----------------------------------------------------------
On April 21, 2016, 3:45 p.m., Jan Schlicht wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45922/
> -----------------------------------------------------------
>
> (Updated April 21, 2016, 3:45 p.m.)
>
>
> Review request for mesos, Adam B and Alexander Rojas.
>
>
> Bugs: MESOS-5142
> https://issues.apache.org/jira/browse/MESOS-5142
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Added agent authorization flags.
>
>
> Diffs
> -----
>
> docs/configuration.md 86ba66ac62295ca148524bcb2e57fee560ac4ac5
> src/local/local.cpp 7de8a2423185e49dfa849d069938d3243b4f4956
> src/slave/constants.hpp 9978c11fec40055dd42f19c20cd3e9fef4e78cea
> src/slave/flags.hpp ee520acc459564fe68272950948fc80c5e24513a
> src/slave/flags.cpp 10d2974bd2b6e79255fc894979607f0d2d00c315
> src/slave/main.cpp 38bd00584dd9c6a872398678b2288edeed1cd2a4
> src/slave/slave.hpp f78c1b4e4d5378ef7223c6eb3ea45491c30fb4c1
> src/slave/slave.cpp d82dec2b10d496065013eb4ad6a35dc054b72553
> src/tests/cluster.hpp 96ec52af16776e91200aa755c7847f56e33d71f4
> src/tests/cluster.cpp 3e5fdf6b32a0d99d3ca83743386232d38471e34f
> src/tests/mesos.hpp e4b63d41d883807ac39846799468b80e88c84e0b
> src/tests/mesos.cpp b5937af7713e1ee2af475518b3e968b2defa8beb
>
> Diff: https://reviews.apache.org/r/45922/diff/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Jan Schlicht
>
>
Re: Review Request 45922: Added agent authorization flags.
Posted by Jan Schlicht <ja...@mesosphere.io>.
> On April 20, 2016, 9:26 a.m., Adam B wrote:
> > src/tests/cluster.hpp, line 151
> > <https://reviews.apache.org/r/45922/diff/7/?file=1348010#file1348010line151>
> >
> > Why do you even need the overload for the authorizer here? Seems like most tests will either provide --acls and use the default, or set the modules/authorizer flags. We can leave this part out until a test needs it.
We will need the overload to be able to use mocked authorizers. This will be important for later authz patches. Of course it is not needed right now, and I could remove it until a test needs it, on the other hand I think it should be part of this patch to provide a "ready-to-use" implementation of an agent authorizer.
- Jan
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/45922/#review129694
-----------------------------------------------------------
On April 18, 2016, 11:49 a.m., Jan Schlicht wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45922/
> -----------------------------------------------------------
>
> (Updated April 18, 2016, 11:49 a.m.)
>
>
> Review request for mesos, Adam B and Alexander Rojas.
>
>
> Bugs: MESOS-5142
> https://issues.apache.org/jira/browse/MESOS-5142
>
>
> Repository: mesos
>
>
> Description
> -------
>
> See summary.
>
>
> Diffs
> -----
>
> docs/configuration.md cd9733002a0940f44edd4e56593a0ca3fe9f77f5
> src/local/local.cpp df72ac52110b75d63df1076496b48e63d06d42ce
> src/slave/constants.hpp 9978c11fec40055dd42f19c20cd3e9fef4e78cea
> src/slave/flags.hpp ee520acc459564fe68272950948fc80c5e24513a
> src/slave/flags.cpp 10d2974bd2b6e79255fc894979607f0d2d00c315
> src/slave/main.cpp 38bd00584dd9c6a872398678b2288edeed1cd2a4
> src/slave/slave.hpp f78c1b4e4d5378ef7223c6eb3ea45491c30fb4c1
> src/slave/slave.cpp d82dec2b10d496065013eb4ad6a35dc054b72553
> src/tests/cluster.hpp 887342acc72b33b4c904d610da47394f9a7d7188
> src/tests/cluster.cpp 31d2556a078429dd45315aa74cd21ad436372d31
> src/tests/mesos.hpp e4b63d41d883807ac39846799468b80e88c84e0b
> src/tests/mesos.cpp b5937af7713e1ee2af475518b3e968b2defa8beb
>
> Diff: https://reviews.apache.org/r/45922/diff/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Jan Schlicht
>
>
Re: Review Request 45922: Added agent authorization flags.
Posted by Adam B <ad...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/45922/#review129694
-----------------------------------------------------------
Looks great. So very close! Just an unnecessary slave::start() parameter in cluster.hpp, and dubious error when authorization is completely disabled in a test.
I'll review the /flags authz now, so I can see how these flags/APIs will be used.
docs/configuration.md (line 889)
<https://reviews.apache.org/r/45922/#comment193214>
`s/flag --authorizer/--authorizer flag/`
docs/configuration.md (line 890)
<https://reviews.apache.org/r/45922/#comment193213>
s/different/other/
docs/configuration.md (line 932)
<https://reviews.apache.org/r/45922/#comment193215>
`s/flag --authorizer/--authorizer flag/`
docs/configuration.md (line 933)
<https://reviews.apache.org/r/45922/#comment193222>
s/different/other/
docs/configuration.md (line 935)
<https://reviews.apache.org/r/45922/#comment193219>
"See the ACLs protobuf in authorizer.proto for the expected format." (like in the flags)
Or is this really "acls.proto" now?
src/slave/flags.cpp (line 453)
<https://reviews.apache.org/r/45922/#comment193217>
`s/flag --authorizer/--authorizer flag/`
src/slave/flags.cpp (line 454)
<https://reviews.apache.org/r/45922/#comment193218>
s/different/other/
src/slave/flags.cpp (line 457)
<https://reviews.apache.org/r/45922/#comment193229>
Isn't this acls.proto now?
src/slave/flags.cpp (line 733)
<https://reviews.apache.org/r/45922/#comment193220>
`--authorizer flag`
src/slave/flags.cpp (line 734)
<https://reviews.apache.org/r/45922/#comment193221>
s/different/other/
src/slave/main.cpp (line 291)
<https://reviews.apache.org/r/45922/#comment193223>
`s/flag --foo/--foo flag/g`
src/slave/main.cpp (line 292)
<https://reviews.apache.org/r/45922/#comment193224>
"non-default"
s/it will be used and //
src/tests/cluster.hpp (line 151)
<https://reviews.apache.org/r/45922/#comment193225>
Why do you even need the overload for the authorizer here? Seems like most tests will either provide --acls and use the default, or set the modules/authorizer flags. We can leave this part out until a test needs it.
src/tests/cluster.cpp (lines 406 - 412)
<https://reviews.apache.org/r/45922/#comment193228>
Why is it an error to start an agent with no authorizer and no ACLs? What if I don't want to do any authorization?
src/tests/mesos.cpp (line 179)
<https://reviews.apache.org/r/45922/#comment193226>
"Set default (permissive) ACLs."
src/tests/mesos.cpp (line 180)
<https://reviews.apache.org/r/45922/#comment193227>
This assumes we separate master and agent ACLs in the local authorizer, which your next patch seems to ignore
- Adam B
On April 18, 2016, 2:49 a.m., Jan Schlicht wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45922/
> -----------------------------------------------------------
>
> (Updated April 18, 2016, 2:49 a.m.)
>
>
> Review request for mesos, Adam B and Alexander Rojas.
>
>
> Bugs: MESOS-5142
> https://issues.apache.org/jira/browse/MESOS-5142
>
>
> Repository: mesos
>
>
> Description
> -------
>
> See summary.
>
>
> Diffs
> -----
>
> docs/configuration.md cd9733002a0940f44edd4e56593a0ca3fe9f77f5
> src/local/local.cpp df72ac52110b75d63df1076496b48e63d06d42ce
> src/slave/constants.hpp 9978c11fec40055dd42f19c20cd3e9fef4e78cea
> src/slave/flags.hpp ee520acc459564fe68272950948fc80c5e24513a
> src/slave/flags.cpp 10d2974bd2b6e79255fc894979607f0d2d00c315
> src/slave/main.cpp 38bd00584dd9c6a872398678b2288edeed1cd2a4
> src/slave/slave.hpp f78c1b4e4d5378ef7223c6eb3ea45491c30fb4c1
> src/slave/slave.cpp d82dec2b10d496065013eb4ad6a35dc054b72553
> src/tests/cluster.hpp 887342acc72b33b4c904d610da47394f9a7d7188
> src/tests/cluster.cpp 31d2556a078429dd45315aa74cd21ad436372d31
> src/tests/mesos.hpp e4b63d41d883807ac39846799468b80e88c84e0b
> src/tests/mesos.cpp b5937af7713e1ee2af475518b3e968b2defa8beb
>
> Diff: https://reviews.apache.org/r/45922/diff/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Jan Schlicht
>
>
Re: Review Request 45922: Added agent authorization flags.
Posted by Jan Schlicht <ja...@mesosphere.io>.
> On April 22, 2016, 9:35 a.m., Adam B wrote:
> > src/tests/cluster.cpp, line 392
> > <https://reviews.apache.org/r/45922/diff/7-8/?file=1348011#file1348011line392>
> >
> > Double underscores for variable names are discouraged (usually only acceptable for continuation function names).
> > Maybe the original parameter is `providedAuthorizer`, and `authorizer__` becomes `newAuthorizer`, then you could have:
> > `authorizer = providedAuthorizer;`
> > `if(...) newAuthorizer = Authorizer::create();`
> > `if(newAuthorizer.isSome()) authorizer = newAuthorizer;`
> > Or maybe you've got better names, but you get the idea.
Thanks for the explanation! I've changed them to `authorizer`, `providedAuthorizer` and `createdAuthorizer`.
- Jan
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/45922/#review130041
-----------------------------------------------------------
On April 22, 2016, 12:05 p.m., Jan Schlicht wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45922/
> -----------------------------------------------------------
>
> (Updated April 22, 2016, 12:05 p.m.)
>
>
> Review request for mesos, Adam B and Alexander Rojas.
>
>
> Bugs: MESOS-5142
> https://issues.apache.org/jira/browse/MESOS-5142
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Added agent authorization flags.
>
>
> Diffs
> -----
>
> docs/configuration.md 86ba66ac62295ca148524bcb2e57fee560ac4ac5
> src/local/local.cpp 7de8a2423185e49dfa849d069938d3243b4f4956
> src/slave/constants.hpp 9978c11fec40055dd42f19c20cd3e9fef4e78cea
> src/slave/flags.hpp ee520acc459564fe68272950948fc80c5e24513a
> src/slave/flags.cpp 10d2974bd2b6e79255fc894979607f0d2d00c315
> src/slave/main.cpp 38bd00584dd9c6a872398678b2288edeed1cd2a4
> src/slave/slave.hpp 20a4bcd0bb9dad06ea81fc4ad9b2fa462c69d2c5
> src/slave/slave.cpp a365e8f5f8d63bf80614fccf6adf35a4fee07526
> src/tests/cluster.hpp 96ec52af16776e91200aa755c7847f56e33d71f4
> src/tests/cluster.cpp 3e5fdf6b32a0d99d3ca83743386232d38471e34f
> src/tests/mesos.hpp e4b63d41d883807ac39846799468b80e88c84e0b
> src/tests/mesos.cpp b5937af7713e1ee2af475518b3e968b2defa8beb
>
> Diff: https://reviews.apache.org/r/45922/diff/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Jan Schlicht
>
>
Re: Review Request 45922: Added agent authorization flags.
Posted by Adam B <ad...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/45922/#review130041
-----------------------------------------------------------
Fix it, then Ship it!
Looks great! Just one issue with variable naming (one underscore doesn't bother me much, but two does)
docs/configuration.md (lines 937 - 939)
<https://reviews.apache.org/r/45922/#comment193654>
Unnecessary here; only necessary for the --acls flag.
src/slave/main.cpp (line 292)
<https://reviews.apache.org/r/45922/#comment193655>
You can leave out the quotes. I just wanted the hyphenation
src/tests/cluster.cpp (line 392)
<https://reviews.apache.org/r/45922/#comment193656>
Double underscores for variable names are discouraged (usually only acceptable for continuation function names).
Maybe the original parameter is `providedAuthorizer`, and `authorizer__` becomes `newAuthorizer`, then you could have:
`authorizer = providedAuthorizer;`
`if(...) newAuthorizer = Authorizer::create();`
`if(newAuthorizer.isSome()) authorizer = newAuthorizer;`
Or maybe you've got better names, but you get the idea.
src/slave/main.cpp (lines 291 - 293)
<https://reviews.apache.org/r/45922/#comment193668>
Let's move this comment into the `if (authorizerName != slave::DEFAULT_AUTHORIZER)` block, since that's the condition it applies to. Then you can even simplify it down to "NOTE: The contents of --acls are ignored for custom authorizers."
src/tests/cluster.cpp (lines 389 - 391)
<https://reviews.apache.org/r/45922/#comment193669>
Simplify and move to before `Authorizer::create(authorizerName);`
- Adam B
On April 21, 2016, 7:48 a.m., Jan Schlicht wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45922/
> -----------------------------------------------------------
>
> (Updated April 21, 2016, 7:48 a.m.)
>
>
> Review request for mesos, Adam B and Alexander Rojas.
>
>
> Bugs: MESOS-5142
> https://issues.apache.org/jira/browse/MESOS-5142
>
>
> Repository: mesos
>
>
> Description
> -------
>
> See summary.
>
>
> Diffs
> -----
>
> docs/configuration.md 86ba66ac62295ca148524bcb2e57fee560ac4ac5
> src/local/local.cpp 7de8a2423185e49dfa849d069938d3243b4f4956
> src/slave/constants.hpp 9978c11fec40055dd42f19c20cd3e9fef4e78cea
> src/slave/flags.hpp ee520acc459564fe68272950948fc80c5e24513a
> src/slave/flags.cpp 10d2974bd2b6e79255fc894979607f0d2d00c315
> src/slave/main.cpp 38bd00584dd9c6a872398678b2288edeed1cd2a4
> src/slave/slave.hpp f78c1b4e4d5378ef7223c6eb3ea45491c30fb4c1
> src/slave/slave.cpp d82dec2b10d496065013eb4ad6a35dc054b72553
> src/tests/cluster.hpp 96ec52af16776e91200aa755c7847f56e33d71f4
> src/tests/cluster.cpp 3e5fdf6b32a0d99d3ca83743386232d38471e34f
> src/tests/mesos.hpp e4b63d41d883807ac39846799468b80e88c84e0b
> src/tests/mesos.cpp b5937af7713e1ee2af475518b3e968b2defa8beb
>
> Diff: https://reviews.apache.org/r/45922/diff/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Jan Schlicht
>
>
Re: Review Request 45922: Added agent authorization flags.
Posted by Mesos ReviewBot <re...@mesos.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/45922/#review130153
-----------------------------------------------------------
Patch looks great!
Reviews applied: [45922]
Passed command: export OS='ubuntu:14.04' CONFIGURATION='--verbose' COMPILER='gcc' ENVIRONMENT='GLOG_v=1 MESOS_VERBOSE=1'; ./support/docker_build.sh
- Mesos ReviewBot
On April 22, 2016, 10:11 a.m., Jan Schlicht wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45922/
> -----------------------------------------------------------
>
> (Updated April 22, 2016, 10:11 a.m.)
>
>
> Review request for mesos, Adam B and Alexander Rojas.
>
>
> Bugs: MESOS-5142
> https://issues.apache.org/jira/browse/MESOS-5142
>
>
> Repository: mesos
>
>
> Description
> -------
>
> See summary.
>
>
> Diffs
> -----
>
> docs/configuration.md 86ba66ac62295ca148524bcb2e57fee560ac4ac5
> src/local/local.cpp 7de8a2423185e49dfa849d069938d3243b4f4956
> src/slave/constants.hpp 9978c11fec40055dd42f19c20cd3e9fef4e78cea
> src/slave/flags.hpp ee520acc459564fe68272950948fc80c5e24513a
> src/slave/flags.cpp 10d2974bd2b6e79255fc894979607f0d2d00c315
> src/slave/main.cpp 38bd00584dd9c6a872398678b2288edeed1cd2a4
> src/slave/slave.hpp 20a4bcd0bb9dad06ea81fc4ad9b2fa462c69d2c5
> src/slave/slave.cpp a365e8f5f8d63bf80614fccf6adf35a4fee07526
> src/tests/cluster.hpp 96ec52af16776e91200aa755c7847f56e33d71f4
> src/tests/cluster.cpp 3e5fdf6b32a0d99d3ca83743386232d38471e34f
> src/tests/mesos.hpp e4b63d41d883807ac39846799468b80e88c84e0b
> src/tests/mesos.cpp b5937af7713e1ee2af475518b3e968b2defa8beb
>
> Diff: https://reviews.apache.org/r/45922/diff/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Jan Schlicht
>
>
Re: Review Request 45922: Added agent authorization flags.
Posted by Alexander Rojas <al...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/45922/#review130419
-----------------------------------------------------------
Ship it!
Ship It!
- Alexander Rojas
On April 22, 2016, 12:11 p.m., Jan Schlicht wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45922/
> -----------------------------------------------------------
>
> (Updated April 22, 2016, 12:11 p.m.)
>
>
> Review request for mesos, Adam B and Alexander Rojas.
>
>
> Bugs: MESOS-5142
> https://issues.apache.org/jira/browse/MESOS-5142
>
>
> Repository: mesos
>
>
> Description
> -------
>
> See summary.
>
>
> Diffs
> -----
>
> docs/configuration.md 86ba66ac62295ca148524bcb2e57fee560ac4ac5
> src/local/local.cpp 7de8a2423185e49dfa849d069938d3243b4f4956
> src/slave/constants.hpp 9978c11fec40055dd42f19c20cd3e9fef4e78cea
> src/slave/flags.hpp ee520acc459564fe68272950948fc80c5e24513a
> src/slave/flags.cpp 10d2974bd2b6e79255fc894979607f0d2d00c315
> src/slave/main.cpp 38bd00584dd9c6a872398678b2288edeed1cd2a4
> src/slave/slave.hpp 20a4bcd0bb9dad06ea81fc4ad9b2fa462c69d2c5
> src/slave/slave.cpp a365e8f5f8d63bf80614fccf6adf35a4fee07526
> src/tests/cluster.hpp 96ec52af16776e91200aa755c7847f56e33d71f4
> src/tests/cluster.cpp 3e5fdf6b32a0d99d3ca83743386232d38471e34f
> src/tests/mesos.hpp e4b63d41d883807ac39846799468b80e88c84e0b
> src/tests/mesos.cpp b5937af7713e1ee2af475518b3e968b2defa8beb
>
> Diff: https://reviews.apache.org/r/45922/diff/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Jan Schlicht
>
>
Re: Review Request 45922: Added agent authorization flags.
Posted by Adam B <ad...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/45922/#review130384
-----------------------------------------------------------
Ship it!
Ship It!
- Adam B
On April 22, 2016, 3:11 a.m., Jan Schlicht wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45922/
> -----------------------------------------------------------
>
> (Updated April 22, 2016, 3:11 a.m.)
>
>
> Review request for mesos, Adam B and Alexander Rojas.
>
>
> Bugs: MESOS-5142
> https://issues.apache.org/jira/browse/MESOS-5142
>
>
> Repository: mesos
>
>
> Description
> -------
>
> See summary.
>
>
> Diffs
> -----
>
> docs/configuration.md 86ba66ac62295ca148524bcb2e57fee560ac4ac5
> src/local/local.cpp 7de8a2423185e49dfa849d069938d3243b4f4956
> src/slave/constants.hpp 9978c11fec40055dd42f19c20cd3e9fef4e78cea
> src/slave/flags.hpp ee520acc459564fe68272950948fc80c5e24513a
> src/slave/flags.cpp 10d2974bd2b6e79255fc894979607f0d2d00c315
> src/slave/main.cpp 38bd00584dd9c6a872398678b2288edeed1cd2a4
> src/slave/slave.hpp 20a4bcd0bb9dad06ea81fc4ad9b2fa462c69d2c5
> src/slave/slave.cpp a365e8f5f8d63bf80614fccf6adf35a4fee07526
> src/tests/cluster.hpp 96ec52af16776e91200aa755c7847f56e33d71f4
> src/tests/cluster.cpp 3e5fdf6b32a0d99d3ca83743386232d38471e34f
> src/tests/mesos.hpp e4b63d41d883807ac39846799468b80e88c84e0b
> src/tests/mesos.cpp b5937af7713e1ee2af475518b3e968b2defa8beb
>
> Diff: https://reviews.apache.org/r/45922/diff/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Jan Schlicht
>
>
Re: Review Request 45922: Added agent authorization flags.
Posted by Jan Schlicht <ja...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/45922/
-----------------------------------------------------------
(Updated April 22, 2016, 12:11 p.m.)
Review request for mesos, Adam B and Alexander Rojas.
Bugs: MESOS-5142
https://issues.apache.org/jira/browse/MESOS-5142
Repository: mesos
Description (updated)
-------
See summary.
Diffs
-----
docs/configuration.md 86ba66ac62295ca148524bcb2e57fee560ac4ac5
src/local/local.cpp 7de8a2423185e49dfa849d069938d3243b4f4956
src/slave/constants.hpp 9978c11fec40055dd42f19c20cd3e9fef4e78cea
src/slave/flags.hpp ee520acc459564fe68272950948fc80c5e24513a
src/slave/flags.cpp 10d2974bd2b6e79255fc894979607f0d2d00c315
src/slave/main.cpp 38bd00584dd9c6a872398678b2288edeed1cd2a4
src/slave/slave.hpp 20a4bcd0bb9dad06ea81fc4ad9b2fa462c69d2c5
src/slave/slave.cpp a365e8f5f8d63bf80614fccf6adf35a4fee07526
src/tests/cluster.hpp 96ec52af16776e91200aa755c7847f56e33d71f4
src/tests/cluster.cpp 3e5fdf6b32a0d99d3ca83743386232d38471e34f
src/tests/mesos.hpp e4b63d41d883807ac39846799468b80e88c84e0b
src/tests/mesos.cpp b5937af7713e1ee2af475518b3e968b2defa8beb
Diff: https://reviews.apache.org/r/45922/diff/
Testing
-------
make check
Thanks,
Jan Schlicht
Re: Review Request 45922: Added agent authorization flags.
Posted by Jan Schlicht <ja...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/45922/
-----------------------------------------------------------
(Updated April 22, 2016, 12:05 p.m.)
Review request for mesos, Adam B and Alexander Rojas.
Changes
-------
Addressed issues and rebased.
Bugs: MESOS-5142
https://issues.apache.org/jira/browse/MESOS-5142
Repository: mesos
Description (updated)
-------
Added agent authorization flags.
Diffs (updated)
-----
docs/configuration.md 86ba66ac62295ca148524bcb2e57fee560ac4ac5
src/local/local.cpp 7de8a2423185e49dfa849d069938d3243b4f4956
src/slave/constants.hpp 9978c11fec40055dd42f19c20cd3e9fef4e78cea
src/slave/flags.hpp ee520acc459564fe68272950948fc80c5e24513a
src/slave/flags.cpp 10d2974bd2b6e79255fc894979607f0d2d00c315
src/slave/main.cpp 38bd00584dd9c6a872398678b2288edeed1cd2a4
src/slave/slave.hpp 20a4bcd0bb9dad06ea81fc4ad9b2fa462c69d2c5
src/slave/slave.cpp a365e8f5f8d63bf80614fccf6adf35a4fee07526
src/tests/cluster.hpp 96ec52af16776e91200aa755c7847f56e33d71f4
src/tests/cluster.cpp 3e5fdf6b32a0d99d3ca83743386232d38471e34f
src/tests/mesos.hpp e4b63d41d883807ac39846799468b80e88c84e0b
src/tests/mesos.cpp b5937af7713e1ee2af475518b3e968b2defa8beb
Diff: https://reviews.apache.org/r/45922/diff/
Testing
-------
make check
Thanks,
Jan Schlicht
Re: Review Request 45922: Added agent authorization flags.
Posted by Jan Schlicht <ja...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/45922/
-----------------------------------------------------------
(Updated April 21, 2016, 4:48 p.m.)
Review request for mesos, Adam B and Alexander Rojas.
Bugs: MESOS-5142
https://issues.apache.org/jira/browse/MESOS-5142
Repository: mesos
Description (updated)
-------
See summary.
Diffs
-----
docs/configuration.md 86ba66ac62295ca148524bcb2e57fee560ac4ac5
src/local/local.cpp 7de8a2423185e49dfa849d069938d3243b4f4956
src/slave/constants.hpp 9978c11fec40055dd42f19c20cd3e9fef4e78cea
src/slave/flags.hpp ee520acc459564fe68272950948fc80c5e24513a
src/slave/flags.cpp 10d2974bd2b6e79255fc894979607f0d2d00c315
src/slave/main.cpp 38bd00584dd9c6a872398678b2288edeed1cd2a4
src/slave/slave.hpp f78c1b4e4d5378ef7223c6eb3ea45491c30fb4c1
src/slave/slave.cpp d82dec2b10d496065013eb4ad6a35dc054b72553
src/tests/cluster.hpp 96ec52af16776e91200aa755c7847f56e33d71f4
src/tests/cluster.cpp 3e5fdf6b32a0d99d3ca83743386232d38471e34f
src/tests/mesos.hpp e4b63d41d883807ac39846799468b80e88c84e0b
src/tests/mesos.cpp b5937af7713e1ee2af475518b3e968b2defa8beb
Diff: https://reviews.apache.org/r/45922/diff/
Testing
-------
make check
Thanks,
Jan Schlicht
Re: Review Request 45922: Added agent authorization flags.
Posted by Jan Schlicht <ja...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/45922/
-----------------------------------------------------------
(Updated April 21, 2016, 3:45 p.m.)
Review request for mesos, Adam B and Alexander Rojas.
Changes
-------
Addressed Adam's issues.
Bugs: MESOS-5142
https://issues.apache.org/jira/browse/MESOS-5142
Repository: mesos
Description (updated)
-------
Added agent authorization flags.
Diffs (updated)
-----
docs/configuration.md 86ba66ac62295ca148524bcb2e57fee560ac4ac5
src/local/local.cpp 7de8a2423185e49dfa849d069938d3243b4f4956
src/slave/constants.hpp 9978c11fec40055dd42f19c20cd3e9fef4e78cea
src/slave/flags.hpp ee520acc459564fe68272950948fc80c5e24513a
src/slave/flags.cpp 10d2974bd2b6e79255fc894979607f0d2d00c315
src/slave/main.cpp 38bd00584dd9c6a872398678b2288edeed1cd2a4
src/slave/slave.hpp f78c1b4e4d5378ef7223c6eb3ea45491c30fb4c1
src/slave/slave.cpp d82dec2b10d496065013eb4ad6a35dc054b72553
src/tests/cluster.hpp 96ec52af16776e91200aa755c7847f56e33d71f4
src/tests/cluster.cpp 3e5fdf6b32a0d99d3ca83743386232d38471e34f
src/tests/mesos.hpp e4b63d41d883807ac39846799468b80e88c84e0b
src/tests/mesos.cpp b5937af7713e1ee2af475518b3e968b2defa8beb
Diff: https://reviews.apache.org/r/45922/diff/
Testing
-------
make check
Thanks,
Jan Schlicht
Re: Review Request 45922: Added agent authorization flags.
Posted by Jan Schlicht <ja...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/45922/
-----------------------------------------------------------
(Updated April 18, 2016, 11:49 a.m.)
Review request for mesos, Adam B and Alexander Rojas.
Changes
-------
Addressed issues and rebased.
Bugs: MESOS-5142
https://issues.apache.org/jira/browse/MESOS-5142
Repository: mesos
Description
-------
See summary.
Diffs (updated)
-----
docs/configuration.md cd9733002a0940f44edd4e56593a0ca3fe9f77f5
src/local/local.cpp df72ac52110b75d63df1076496b48e63d06d42ce
src/slave/constants.hpp 9978c11fec40055dd42f19c20cd3e9fef4e78cea
src/slave/flags.hpp ee520acc459564fe68272950948fc80c5e24513a
src/slave/flags.cpp 10d2974bd2b6e79255fc894979607f0d2d00c315
src/slave/main.cpp 38bd00584dd9c6a872398678b2288edeed1cd2a4
src/slave/slave.hpp f78c1b4e4d5378ef7223c6eb3ea45491c30fb4c1
src/slave/slave.cpp d82dec2b10d496065013eb4ad6a35dc054b72553
src/tests/cluster.hpp 887342acc72b33b4c904d610da47394f9a7d7188
src/tests/cluster.cpp 31d2556a078429dd45315aa74cd21ad436372d31
src/tests/mesos.hpp e4b63d41d883807ac39846799468b80e88c84e0b
src/tests/mesos.cpp b5937af7713e1ee2af475518b3e968b2defa8beb
Diff: https://reviews.apache.org/r/45922/diff/
Testing
-------
make check
Thanks,
Jan Schlicht
Re: Review Request 45922: Added agent authorization flags.
Posted by Jan Schlicht <ja...@mesosphere.io>.
> On April 15, 2016, 12:16 p.m., Alexander Rojas wrote:
> > src/slave/slave.hpp, line 108
> > <https://reviews.apache.org/r/45922/diff/6/?file=1345034#file1345034line108>
> >
> > I think is a better practice to move optional parameters at the end and default initialize them to `None()`.
> >
> > In fact if you check `master.hpp` it is how it is done there.
It's moved to the end now. Nevertheless, they default initialization to `None()` is done in `cluster.hpp`, because how it's done for all of the other parameters.
- Jan
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/45922/#review129089
-----------------------------------------------------------
On April 18, 2016, 11:49 a.m., Jan Schlicht wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45922/
> -----------------------------------------------------------
>
> (Updated April 18, 2016, 11:49 a.m.)
>
>
> Review request for mesos, Adam B and Alexander Rojas.
>
>
> Bugs: MESOS-5142
> https://issues.apache.org/jira/browse/MESOS-5142
>
>
> Repository: mesos
>
>
> Description
> -------
>
> See summary.
>
>
> Diffs
> -----
>
> docs/configuration.md cd9733002a0940f44edd4e56593a0ca3fe9f77f5
> src/local/local.cpp df72ac52110b75d63df1076496b48e63d06d42ce
> src/slave/constants.hpp 9978c11fec40055dd42f19c20cd3e9fef4e78cea
> src/slave/flags.hpp ee520acc459564fe68272950948fc80c5e24513a
> src/slave/flags.cpp 10d2974bd2b6e79255fc894979607f0d2d00c315
> src/slave/main.cpp 38bd00584dd9c6a872398678b2288edeed1cd2a4
> src/slave/slave.hpp f78c1b4e4d5378ef7223c6eb3ea45491c30fb4c1
> src/slave/slave.cpp d82dec2b10d496065013eb4ad6a35dc054b72553
> src/tests/cluster.hpp 887342acc72b33b4c904d610da47394f9a7d7188
> src/tests/cluster.cpp 31d2556a078429dd45315aa74cd21ad436372d31
> src/tests/mesos.hpp e4b63d41d883807ac39846799468b80e88c84e0b
> src/tests/mesos.cpp b5937af7713e1ee2af475518b3e968b2defa8beb
>
> Diff: https://reviews.apache.org/r/45922/diff/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Jan Schlicht
>
>
Re: Review Request 45922: Added agent authorization flags.
Posted by Alexander Rojas <al...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/45922/#review129089
-----------------------------------------------------------
src/local/local.cpp (line 393)
<https://reviews.apache.org/r/45922/#comment192554>
Move this comment to line 398
src/local/local.cpp (line 398)
<https://reviews.apache.org/r/45922/#comment192555>
```
authorizer_, // Same authorizer as master.
```
src/slave/slave.hpp (line 108)
<https://reviews.apache.org/r/45922/#comment192559>
I think is a better practice to move optional parameters at the end and default initialize them to `None()`.
In fact if you check `master.hpp` it is how it is done there.
src/tests/mesos.cpp (line 468)
<https://reviews.apache.org/r/45922/#comment192560>
This is what I meant. If this were at the end and default initialize, you wouldn't have to define it.
- Alexander Rojas
On April 14, 2016, 6:51 p.m., Jan Schlicht wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45922/
> -----------------------------------------------------------
>
> (Updated April 14, 2016, 6:51 p.m.)
>
>
> Review request for mesos, Adam B and Alexander Rojas.
>
>
> Bugs: MESOS-5142
> https://issues.apache.org/jira/browse/MESOS-5142
>
>
> Repository: mesos
>
>
> Description
> -------
>
> See summary.
>
>
> Diffs
> -----
>
> docs/configuration.md ce51f26f0584eeecb5c96e39a5bcc21f5906a29c
> src/local/local.cpp df72ac52110b75d63df1076496b48e63d06d42ce
> src/slave/constants.hpp 9978c11fec40055dd42f19c20cd3e9fef4e78cea
> src/slave/flags.hpp ee520acc459564fe68272950948fc80c5e24513a
> src/slave/flags.cpp 316feec26e52a8cb1f0fd0739176b3f502e1ed86
> src/slave/main.cpp 70df4f384b09a1fb078cd893efe52a5e3b116f48
> src/slave/slave.hpp f78c1b4e4d5378ef7223c6eb3ea45491c30fb4c1
> src/slave/slave.cpp de99e9eb5cc812b2e07deb749b98b4f4db363728
> src/tests/cluster.hpp 887342acc72b33b4c904d610da47394f9a7d7188
> src/tests/cluster.cpp b4d69106388892b88f7de20b248cac8b950b861c
> src/tests/mesos.cpp 1b7a8fd703b90d77ffa3df079bdc2744752caac6
>
> Diff: https://reviews.apache.org/r/45922/diff/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Jan Schlicht
>
>
Re: Review Request 45922: Added agent authorization flags.
Posted by Jan Schlicht <ja...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/45922/
-----------------------------------------------------------
(Updated April 14, 2016, 6:51 p.m.)
Review request for mesos, Adam B and Alexander Rojas.
Changes
-------
Rebased.
Bugs: MESOS-5142
https://issues.apache.org/jira/browse/MESOS-5142
Repository: mesos
Description (updated)
-------
See summary.
Diffs (updated)
-----
docs/configuration.md ce51f26f0584eeecb5c96e39a5bcc21f5906a29c
src/local/local.cpp df72ac52110b75d63df1076496b48e63d06d42ce
src/slave/constants.hpp 9978c11fec40055dd42f19c20cd3e9fef4e78cea
src/slave/flags.hpp ee520acc459564fe68272950948fc80c5e24513a
src/slave/flags.cpp 316feec26e52a8cb1f0fd0739176b3f502e1ed86
src/slave/main.cpp 70df4f384b09a1fb078cd893efe52a5e3b116f48
src/slave/slave.hpp f78c1b4e4d5378ef7223c6eb3ea45491c30fb4c1
src/slave/slave.cpp de99e9eb5cc812b2e07deb749b98b4f4db363728
src/tests/cluster.hpp 887342acc72b33b4c904d610da47394f9a7d7188
src/tests/cluster.cpp b4d69106388892b88f7de20b248cac8b950b861c
src/tests/mesos.cpp 1b7a8fd703b90d77ffa3df079bdc2744752caac6
Diff: https://reviews.apache.org/r/45922/diff/
Testing
-------
make check
Thanks,
Jan Schlicht
Re: Review Request 45922: Added agent authorization flags.
Posted by Jan Schlicht <ja...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/45922/
-----------------------------------------------------------
(Updated April 14, 2016, 6:16 p.m.)
Review request for mesos, Adam B and Alexander Rojas.
Bugs: MESOS-5142
https://issues.apache.org/jira/browse/MESOS-5142
Repository: mesos
Description (updated)
-------
Added agent authorization flags.
Diffs (updated)
-----
docs/configuration.md ce51f26f0584eeecb5c96e39a5bcc21f5906a29c
src/local/local.cpp df72ac52110b75d63df1076496b48e63d06d42ce
src/slave/constants.hpp 9978c11fec40055dd42f19c20cd3e9fef4e78cea
src/slave/flags.hpp ee520acc459564fe68272950948fc80c5e24513a
src/slave/flags.cpp 316feec26e52a8cb1f0fd0739176b3f502e1ed86
src/slave/main.cpp 70df4f384b09a1fb078cd893efe52a5e3b116f48
src/slave/slave.hpp 76f3aff394e5cecc54dcb3065cb872e238bc228e
src/slave/slave.cpp 49fa4a06e26d0d4475ed50db254a320a7030f896
src/tests/cluster.hpp 887342acc72b33b4c904d610da47394f9a7d7188
src/tests/cluster.cpp b4d69106388892b88f7de20b248cac8b950b861c
src/tests/mesos.cpp 1b7a8fd703b90d77ffa3df079bdc2744752caac6
Diff: https://reviews.apache.org/r/45922/diff/
Testing
-------
make check
Thanks,
Jan Schlicht
Re: Review Request 45922: Added agent authorization flags.
Posted by Mesos ReviewBot <re...@mesos.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/45922/#review128875
-----------------------------------------------------------
Patch looks great!
Reviews applied: [45922]
Passed command: export OS='ubuntu:14.04' CONFIGURATION='--verbose' COMPILER='gcc' ENVIRONMENT='GLOG_v=1 MESOS_VERBOSE=1'; ./support/docker_build.sh
- Mesos ReviewBot
On April 14, 2016, 11 a.m., Jan Schlicht wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45922/
> -----------------------------------------------------------
>
> (Updated April 14, 2016, 11 a.m.)
>
>
> Review request for mesos, Adam B and Alexander Rojas.
>
>
> Bugs: MESOS-5142
> https://issues.apache.org/jira/browse/MESOS-5142
>
>
> Repository: mesos
>
>
> Description
> -------
>
> See summary.
>
>
> Diffs
> -----
>
> docs/configuration.md ce51f26f0584eeecb5c96e39a5bcc21f5906a29c
> src/local/local.cpp df72ac52110b75d63df1076496b48e63d06d42ce
> src/slave/constants.hpp 9978c11fec40055dd42f19c20cd3e9fef4e78cea
> src/slave/flags.hpp ee520acc459564fe68272950948fc80c5e24513a
> src/slave/flags.cpp 316feec26e52a8cb1f0fd0739176b3f502e1ed86
> src/slave/main.cpp 70df4f384b09a1fb078cd893efe52a5e3b116f48
> src/slave/slave.hpp 76f3aff394e5cecc54dcb3065cb872e238bc228e
> src/slave/slave.cpp 49fa4a06e26d0d4475ed50db254a320a7030f896
> src/tests/cluster.hpp 887342acc72b33b4c904d610da47394f9a7d7188
> src/tests/cluster.cpp b4d69106388892b88f7de20b248cac8b950b861c
> src/tests/mesos.cpp 1b7a8fd703b90d77ffa3df079bdc2744752caac6
>
> Diff: https://reviews.apache.org/r/45922/diff/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Jan Schlicht
>
>
Re: Review Request 45922: Added agent authorization flags.
Posted by Jan Schlicht <ja...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/45922/
-----------------------------------------------------------
(Updated April 14, 2016, 1 p.m.)
Review request for mesos, Adam B and Alexander Rojas.
Changes
-------
Interim patch. It's fixing the open issues but doesn't add a test case. This will be done in a later rev.
Bugs: MESOS-5142
https://issues.apache.org/jira/browse/MESOS-5142
Repository: mesos
Description (updated)
-------
See summary.
Diffs (updated)
-----
docs/configuration.md ce51f26f0584eeecb5c96e39a5bcc21f5906a29c
src/local/local.cpp df72ac52110b75d63df1076496b48e63d06d42ce
src/slave/constants.hpp 9978c11fec40055dd42f19c20cd3e9fef4e78cea
src/slave/flags.hpp ee520acc459564fe68272950948fc80c5e24513a
src/slave/flags.cpp 316feec26e52a8cb1f0fd0739176b3f502e1ed86
src/slave/main.cpp 70df4f384b09a1fb078cd893efe52a5e3b116f48
src/slave/slave.hpp 76f3aff394e5cecc54dcb3065cb872e238bc228e
src/slave/slave.cpp 49fa4a06e26d0d4475ed50db254a320a7030f896
src/tests/cluster.hpp 887342acc72b33b4c904d610da47394f9a7d7188
src/tests/cluster.cpp b4d69106388892b88f7de20b248cac8b950b861c
src/tests/mesos.cpp 1b7a8fd703b90d77ffa3df079bdc2744752caac6
Diff: https://reviews.apache.org/r/45922/diff/
Testing
-------
make check
Thanks,
Jan Schlicht
Re: Review Request 45922: Added agent authorization flags.
Posted by Jan Schlicht <ja...@mesosphere.io>.
> On April 13, 2016, 3:58 p.m., Benjamin Bannier wrote:
> > src/tests/cluster.cpp, lines 398-410
> > <https://reviews.apache.org/r/45922/diff/3/?file=1337119#file1337119line398>
> >
> > You are shadowing the outer `Option<Authorizer*>` here with a `Result<Authorizer*>`. All of the following assignments are to that and not the outer one. I believe this leads us to assigning not what was intended in l.462 below. Having at least a different name would help readability some here.
> >
> > As an aside, for my taste the control flow in this block has too much nesting making this less transparent than it should be.
The shadowing has been resolved. If no authorizer and no ACLs were provided, a segfault could occur. This has been fixed as well by making it an error.
- Jan
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/45922/#review128666
-----------------------------------------------------------
On April 14, 2016, 1 p.m., Jan Schlicht wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45922/
> -----------------------------------------------------------
>
> (Updated April 14, 2016, 1 p.m.)
>
>
> Review request for mesos, Adam B and Alexander Rojas.
>
>
> Bugs: MESOS-5142
> https://issues.apache.org/jira/browse/MESOS-5142
>
>
> Repository: mesos
>
>
> Description
> -------
>
> See summary.
>
>
> Diffs
> -----
>
> docs/configuration.md ce51f26f0584eeecb5c96e39a5bcc21f5906a29c
> src/local/local.cpp df72ac52110b75d63df1076496b48e63d06d42ce
> src/slave/constants.hpp 9978c11fec40055dd42f19c20cd3e9fef4e78cea
> src/slave/flags.hpp ee520acc459564fe68272950948fc80c5e24513a
> src/slave/flags.cpp 316feec26e52a8cb1f0fd0739176b3f502e1ed86
> src/slave/main.cpp 70df4f384b09a1fb078cd893efe52a5e3b116f48
> src/slave/slave.hpp 76f3aff394e5cecc54dcb3065cb872e238bc228e
> src/slave/slave.cpp 49fa4a06e26d0d4475ed50db254a320a7030f896
> src/tests/cluster.hpp 887342acc72b33b4c904d610da47394f9a7d7188
> src/tests/cluster.cpp b4d69106388892b88f7de20b248cac8b950b861c
> src/tests/mesos.cpp 1b7a8fd703b90d77ffa3df079bdc2744752caac6
>
> Diff: https://reviews.apache.org/r/45922/diff/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Jan Schlicht
>
>
Re: Review Request 45922: Added agent authorization flags.
Posted by Benjamin Bannier <be...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/45922/#review128666
-----------------------------------------------------------
src/tests/cluster.cpp (lines 398 - 410)
<https://reviews.apache.org/r/45922/#comment192137>
You are shadowing the outer `Option<Authorizer*>` here with a `Result<Authorizer*>`. All of the following assignments are to that and not the outer one. I believe this leads us to assigning not what was intended in l.462 below. Having at least a different name would help readability some here.
As an aside, for my taste the control flow in this block has too much nesting making this less transparent than it should be.
- Benjamin Bannier
On April 8, 2016, 4:25 p.m., Jan Schlicht wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45922/
> -----------------------------------------------------------
>
> (Updated April 8, 2016, 4:25 p.m.)
>
>
> Review request for mesos, Adam B and Alexander Rojas.
>
>
> Bugs: MESOS-5142
> https://issues.apache.org/jira/browse/MESOS-5142
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Added agent authorization flags.
>
>
> Diffs
> -----
>
> docs/configuration.md 309a5a05eab386c8943ba6bdee8d5efeb448aa0c
> src/local/local.cpp df72ac52110b75d63df1076496b48e63d06d42ce
> src/master/flags.cpp e522499586b731d522180f171731a9dd38b8344c
> src/slave/constants.hpp 449c8cd9f43f71b4612023eb463969e9db0bc960
> src/slave/flags.hpp 4236b7fe40b9aa26d57b651be06a0e36037f65c6
> src/slave/flags.cpp 7164afef9c82a9bba0f137d7387c3569912b6599
> src/slave/main.cpp 70df4f384b09a1fb078cd893efe52a5e3b116f48
> src/slave/slave.hpp 76f3aff394e5cecc54dcb3065cb872e238bc228e
> src/slave/slave.cpp f090c853b8affc4be5eecb4f616ec881fc2b60c3
> src/tests/cluster.hpp 39ca15e3cc2f4f2ac9b8a73482c2bd945b1824e3
> src/tests/cluster.cpp 7e488d28b7a0d3bef9cd76bf7df2de4822256ef6
> src/tests/mesos.cpp 1b7a8fd703b90d77ffa3df079bdc2744752caac6
>
> Diff: https://reviews.apache.org/r/45922/diff/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Jan Schlicht
>
>
Re: Review Request 45922: Added agent authorization flags.
Posted by Mesos ReviewBot <re...@mesos.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/45922/#review127833
-----------------------------------------------------------
Patch looks great!
Reviews applied: [45922]
Passed command: export OS='ubuntu:14.04' CONFIGURATION='--verbose' COMPILER='gcc' ENVIRONMENT='GLOG_v=1 MESOS_VERBOSE=1'; ./support/docker_build.sh
- Mesos ReviewBot
On April 8, 2016, 2:25 p.m., Jan Schlicht wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45922/
> -----------------------------------------------------------
>
> (Updated April 8, 2016, 2:25 p.m.)
>
>
> Review request for mesos, Adam B and Alexander Rojas.
>
>
> Bugs: MESOS-5142
> https://issues.apache.org/jira/browse/MESOS-5142
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Added agent authorization flags.
>
>
> Diffs
> -----
>
> docs/configuration.md 309a5a05eab386c8943ba6bdee8d5efeb448aa0c
> src/local/local.cpp df72ac52110b75d63df1076496b48e63d06d42ce
> src/master/flags.cpp e522499586b731d522180f171731a9dd38b8344c
> src/slave/constants.hpp 449c8cd9f43f71b4612023eb463969e9db0bc960
> src/slave/flags.hpp 4236b7fe40b9aa26d57b651be06a0e36037f65c6
> src/slave/flags.cpp 7164afef9c82a9bba0f137d7387c3569912b6599
> src/slave/main.cpp 70df4f384b09a1fb078cd893efe52a5e3b116f48
> src/slave/slave.hpp 76f3aff394e5cecc54dcb3065cb872e238bc228e
> src/slave/slave.cpp f090c853b8affc4be5eecb4f616ec881fc2b60c3
> src/tests/cluster.hpp 39ca15e3cc2f4f2ac9b8a73482c2bd945b1824e3
> src/tests/cluster.cpp 7e488d28b7a0d3bef9cd76bf7df2de4822256ef6
> src/tests/mesos.cpp 1b7a8fd703b90d77ffa3df079bdc2744752caac6
>
> Diff: https://reviews.apache.org/r/45922/diff/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Jan Schlicht
>
>
Re: Review Request 45922: Added agent authorization flags.
Posted by Adam B <ad...@mesosphere.io>.
> On April 12, 2016, 2:31 a.m., Adam B wrote:
> > docs/configuration.md, line 94
> > <https://reviews.apache.org/r/45922/diff/3/?file=1337109#file1337109line94>
> >
> > Why would we need to support multiple authorizers? Would a particular request check two authorizers then and/or the results?
> > Or are we somehow using one authorizer for some things and another for others? This could be explainable for the master's framework authz vs. http authz, but makes no sense for the agent.
>
> Alexander Rojas wrote:
> That was an original requirement on the master, since the original crammd5 authorizer was supposed to come also in the multiple version. I think we can, not only remove it here, but also in the master. I don't think we will move onto the multiple authorizers anytime soon.
+1, although I dread deprecating the plural flag in master. I could live with it being singular on agent and plural on master, at least for this release; but I guess we should start deprecations ASAP if we have to wait 6mo before we can remove them.
- Adam
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/45922/#review128350
-----------------------------------------------------------
On April 8, 2016, 7:25 a.m., Jan Schlicht wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45922/
> -----------------------------------------------------------
>
> (Updated April 8, 2016, 7:25 a.m.)
>
>
> Review request for mesos, Adam B and Alexander Rojas.
>
>
> Bugs: MESOS-5142
> https://issues.apache.org/jira/browse/MESOS-5142
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Added agent authorization flags.
>
>
> Diffs
> -----
>
> docs/configuration.md 309a5a05eab386c8943ba6bdee8d5efeb448aa0c
> src/local/local.cpp df72ac52110b75d63df1076496b48e63d06d42ce
> src/master/flags.cpp e522499586b731d522180f171731a9dd38b8344c
> src/slave/constants.hpp 449c8cd9f43f71b4612023eb463969e9db0bc960
> src/slave/flags.hpp 4236b7fe40b9aa26d57b651be06a0e36037f65c6
> src/slave/flags.cpp 7164afef9c82a9bba0f137d7387c3569912b6599
> src/slave/main.cpp 70df4f384b09a1fb078cd893efe52a5e3b116f48
> src/slave/slave.hpp 76f3aff394e5cecc54dcb3065cb872e238bc228e
> src/slave/slave.cpp f090c853b8affc4be5eecb4f616ec881fc2b60c3
> src/tests/cluster.hpp 39ca15e3cc2f4f2ac9b8a73482c2bd945b1824e3
> src/tests/cluster.cpp 7e488d28b7a0d3bef9cd76bf7df2de4822256ef6
> src/tests/mesos.cpp 1b7a8fd703b90d77ffa3df079bdc2744752caac6
>
> Diff: https://reviews.apache.org/r/45922/diff/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Jan Schlicht
>
>
Re: Review Request 45922: Added agent authorization flags.
Posted by Joerg Schad <jo...@mesosphere.io>.
> On April 12, 2016, 9:31 a.m., Adam B wrote:
> > src/slave/flags.cpp, lines 446-449
> > <https://reviews.apache.org/r/45922/diff/3/?file=1337114#file1337114line446>
> >
> > I'd rather have no example than a confusing/invalid example. We can add an example after we have at least one ACL
+1
> On April 12, 2016, 9:31 a.m., Adam B wrote:
> > src/slave/main.cpp, line 301
> > <https://reviews.apache.org/r/45922/diff/3/?file=1337115#file1337115line301>
> >
> > It seems that by naming the flag `authorizers`, you've already decided that we want to add support for multiple authorizers
Isn't that also implied by the parsing logic above/in the master/main.cpp?
> On April 12, 2016, 9:31 a.m., Adam B wrote:
> > src/tests/cluster.hpp, line 146
> > <https://reviews.apache.org/r/45922/diff/3/?file=1337118#file1337118line146>
> >
> > Why add this here instead of at the end? Do you expect 'authorizer' to be a more populer parameter than the ones after it?
This also causes that you have to touch a lot of tests....
- Joerg
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/45922/#review128350
-----------------------------------------------------------
On April 8, 2016, 2:25 p.m., Jan Schlicht wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45922/
> -----------------------------------------------------------
>
> (Updated April 8, 2016, 2:25 p.m.)
>
>
> Review request for mesos, Adam B and Alexander Rojas.
>
>
> Bugs: MESOS-5142
> https://issues.apache.org/jira/browse/MESOS-5142
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Added agent authorization flags.
>
>
> Diffs
> -----
>
> docs/configuration.md 309a5a05eab386c8943ba6bdee8d5efeb448aa0c
> src/local/local.cpp df72ac52110b75d63df1076496b48e63d06d42ce
> src/master/flags.cpp e522499586b731d522180f171731a9dd38b8344c
> src/slave/constants.hpp 449c8cd9f43f71b4612023eb463969e9db0bc960
> src/slave/flags.hpp 4236b7fe40b9aa26d57b651be06a0e36037f65c6
> src/slave/flags.cpp 7164afef9c82a9bba0f137d7387c3569912b6599
> src/slave/main.cpp 70df4f384b09a1fb078cd893efe52a5e3b116f48
> src/slave/slave.hpp 76f3aff394e5cecc54dcb3065cb872e238bc228e
> src/slave/slave.cpp f090c853b8affc4be5eecb4f616ec881fc2b60c3
> src/tests/cluster.hpp 39ca15e3cc2f4f2ac9b8a73482c2bd945b1824e3
> src/tests/cluster.cpp 7e488d28b7a0d3bef9cd76bf7df2de4822256ef6
> src/tests/mesos.cpp 1b7a8fd703b90d77ffa3df079bdc2744752caac6
>
> Diff: https://reviews.apache.org/r/45922/diff/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Jan Schlicht
>
>
Re: Review Request 45922: Added agent authorization flags.
Posted by Jan Schlicht <ja...@mesosphere.io>.
> On April 12, 2016, 11:31 a.m., Adam B wrote:
> > docs/configuration.md, line 94
> > <https://reviews.apache.org/r/45922/diff/3/?file=1337109#file1337109line94>
> >
> > Why would we need to support multiple authorizers? Would a particular request check two authorizers then and/or the results?
> > Or are we somehow using one authorizer for some things and another for others? This could be explainable for the master's framework authz vs. http authz, but makes no sense for the agent.
>
> Alexander Rojas wrote:
> That was an original requirement on the master, since the original crammd5 authorizer was supposed to come also in the multiple version. I think we can, not only remove it here, but also in the master. I don't think we will move onto the multiple authorizers anytime soon.
>
> Adam B wrote:
> +1, although I dread deprecating the plural flag in master. I could live with it being singular on agent and plural on master, at least for this release; but I guess we should start deprecations ASAP if we have to wait 6mo before we can remove them.
Got it! I'll remove support for multiple authorizers on the agent.
> On April 12, 2016, 11:31 a.m., Adam B wrote:
> > src/slave/flags.cpp, lines 446-449
> > <https://reviews.apache.org/r/45922/diff/3/?file=1337114#file1337114line446>
> >
> > I'd rather have no example than a confusing/invalid example. We can add an example after we have at least one ACL
>
> Joerg Schad wrote:
> +1
Will remove the example.
- Jan
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/45922/#review128350
-----------------------------------------------------------
On April 8, 2016, 4:25 p.m., Jan Schlicht wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45922/
> -----------------------------------------------------------
>
> (Updated April 8, 2016, 4:25 p.m.)
>
>
> Review request for mesos, Adam B and Alexander Rojas.
>
>
> Bugs: MESOS-5142
> https://issues.apache.org/jira/browse/MESOS-5142
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Added agent authorization flags.
>
>
> Diffs
> -----
>
> docs/configuration.md 309a5a05eab386c8943ba6bdee8d5efeb448aa0c
> src/local/local.cpp df72ac52110b75d63df1076496b48e63d06d42ce
> src/master/flags.cpp e522499586b731d522180f171731a9dd38b8344c
> src/slave/constants.hpp 449c8cd9f43f71b4612023eb463969e9db0bc960
> src/slave/flags.hpp 4236b7fe40b9aa26d57b651be06a0e36037f65c6
> src/slave/flags.cpp 7164afef9c82a9bba0f137d7387c3569912b6599
> src/slave/main.cpp 70df4f384b09a1fb078cd893efe52a5e3b116f48
> src/slave/slave.hpp 76f3aff394e5cecc54dcb3065cb872e238bc228e
> src/slave/slave.cpp f090c853b8affc4be5eecb4f616ec881fc2b60c3
> src/tests/cluster.hpp 39ca15e3cc2f4f2ac9b8a73482c2bd945b1824e3
> src/tests/cluster.cpp 7e488d28b7a0d3bef9cd76bf7df2de4822256ef6
> src/tests/mesos.cpp 1b7a8fd703b90d77ffa3df079bdc2744752caac6
>
> Diff: https://reviews.apache.org/r/45922/diff/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Jan Schlicht
>
>
Re: Review Request 45922: Added agent authorization flags.
Posted by Alexander Rojas <al...@mesosphere.io>.
> On April 12, 2016, 11:31 a.m., Adam B wrote:
> > docs/configuration.md, line 94
> > <https://reviews.apache.org/r/45922/diff/3/?file=1337109#file1337109line94>
> >
> > Why would we need to support multiple authorizers? Would a particular request check two authorizers then and/or the results?
> > Or are we somehow using one authorizer for some things and another for others? This could be explainable for the master's framework authz vs. http authz, but makes no sense for the agent.
That was an original requirement on the master, since the original crammd5 authorizer was supposed to come also in the multiple version. I think we can, not only remove it here, but also in the master. I don't think we will move onto the multiple authorizers anytime soon.
- Alexander
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/45922/#review128350
-----------------------------------------------------------
On April 8, 2016, 4:25 p.m., Jan Schlicht wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45922/
> -----------------------------------------------------------
>
> (Updated April 8, 2016, 4:25 p.m.)
>
>
> Review request for mesos, Adam B and Alexander Rojas.
>
>
> Bugs: MESOS-5142
> https://issues.apache.org/jira/browse/MESOS-5142
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Added agent authorization flags.
>
>
> Diffs
> -----
>
> docs/configuration.md 309a5a05eab386c8943ba6bdee8d5efeb448aa0c
> src/local/local.cpp df72ac52110b75d63df1076496b48e63d06d42ce
> src/master/flags.cpp e522499586b731d522180f171731a9dd38b8344c
> src/slave/constants.hpp 449c8cd9f43f71b4612023eb463969e9db0bc960
> src/slave/flags.hpp 4236b7fe40b9aa26d57b651be06a0e36037f65c6
> src/slave/flags.cpp 7164afef9c82a9bba0f137d7387c3569912b6599
> src/slave/main.cpp 70df4f384b09a1fb078cd893efe52a5e3b116f48
> src/slave/slave.hpp 76f3aff394e5cecc54dcb3065cb872e238bc228e
> src/slave/slave.cpp f090c853b8affc4be5eecb4f616ec881fc2b60c3
> src/tests/cluster.hpp 39ca15e3cc2f4f2ac9b8a73482c2bd945b1824e3
> src/tests/cluster.cpp 7e488d28b7a0d3bef9cd76bf7df2de4822256ef6
> src/tests/mesos.cpp 1b7a8fd703b90d77ffa3df079bdc2744752caac6
>
> Diff: https://reviews.apache.org/r/45922/diff/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Jan Schlicht
>
>
Re: Review Request 45922: Added agent authorization flags.
Posted by Jan Schlicht <ja...@mesosphere.io>.
> On April 12, 2016, 11:31 a.m., Adam B wrote:
> > docs/configuration.md, line 886
> > <https://reviews.apache.org/r/45922/diff/3/?file=1337109#file1337109line886>
> >
> > None of these examples apply to an agent. Maybe we need to implement an ACL (e.g. GET_ENDPOINT_WITH_PATH "/flags") before we can provide any example ACLs for the agent.
Will be implemented in https://reviews.apache.org/r/46203
- Jan
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/45922/#review128350
-----------------------------------------------------------
On April 14, 2016, 6:16 p.m., Jan Schlicht wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45922/
> -----------------------------------------------------------
>
> (Updated April 14, 2016, 6:16 p.m.)
>
>
> Review request for mesos, Adam B and Alexander Rojas.
>
>
> Bugs: MESOS-5142
> https://issues.apache.org/jira/browse/MESOS-5142
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Added agent authorization flags.
>
>
> Diffs
> -----
>
> docs/configuration.md ce51f26f0584eeecb5c96e39a5bcc21f5906a29c
> src/local/local.cpp df72ac52110b75d63df1076496b48e63d06d42ce
> src/slave/constants.hpp 9978c11fec40055dd42f19c20cd3e9fef4e78cea
> src/slave/flags.hpp ee520acc459564fe68272950948fc80c5e24513a
> src/slave/flags.cpp 316feec26e52a8cb1f0fd0739176b3f502e1ed86
> src/slave/main.cpp 70df4f384b09a1fb078cd893efe52a5e3b116f48
> src/slave/slave.hpp 76f3aff394e5cecc54dcb3065cb872e238bc228e
> src/slave/slave.cpp 49fa4a06e26d0d4475ed50db254a320a7030f896
> src/tests/cluster.hpp 887342acc72b33b4c904d610da47394f9a7d7188
> src/tests/cluster.cpp b4d69106388892b88f7de20b248cac8b950b861c
> src/tests/mesos.cpp 1b7a8fd703b90d77ffa3df079bdc2744752caac6
>
> Diff: https://reviews.apache.org/r/45922/diff/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Jan Schlicht
>
>
Re: Review Request 45922: Added agent authorization flags.
Posted by Adam B <ad...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/45922/#review128350
-----------------------------------------------------------
Looks nice and clean. Three main concerns:
1. Why support multiple authorizers?
2. Don't show confusing/invalid examples
3. How will we test this? Do we need to add authz to at least one endpoint?
docs/configuration.md (line 94)
<https://reviews.apache.org/r/45922/#comment191774>
Why would we need to support multiple authorizers? Would a particular request check two authorizers then and/or the results?
Or are we somehow using one authorizer for some things and another for others? This could be explainable for the master's framework authz vs. http authz, but makes no sense for the agent.
docs/configuration.md (line 869)
<https://reviews.apache.org/r/45922/#comment191775>
None of these examples apply to an agent. Maybe we need to implement an ACL (e.g. GET_ENDPOINT_WITH_PATH "/flags") before we can provide any example ACLs for the agent.
src/slave/flags.cpp (lines 446 - 449)
<https://reviews.apache.org/r/45922/#comment191777>
I'd rather have no example than a confusing/invalid example. We can add an example after we have at least one ACL
src/slave/flags.cpp (line 777)
<https://reviews.apache.org/r/45922/#comment191778>
I still can't imagine an agent with multiple authorizers.
src/slave/main.cpp (lines 22 - 26)
<https://reviews.apache.org/r/45922/#comment191780>
Pretty sure `mesos/authorizer/*` goes before `mesos/master/*` alphabetically
src/slave/main.cpp (line 301)
<https://reviews.apache.org/r/45922/#comment191783>
It seems that by naming the flag `authorizers`, you've already decided that we want to add support for multiple authorizers
src/tests/cluster.hpp (line 146)
<https://reviews.apache.org/r/45922/#comment191785>
Why add this here instead of at the end? Do you expect 'authorizer' to be a more populer parameter than the ones after it?
- Adam B
On April 8, 2016, 7:25 a.m., Jan Schlicht wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45922/
> -----------------------------------------------------------
>
> (Updated April 8, 2016, 7:25 a.m.)
>
>
> Review request for mesos, Adam B and Alexander Rojas.
>
>
> Bugs: MESOS-5142
> https://issues.apache.org/jira/browse/MESOS-5142
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Added agent authorization flags.
>
>
> Diffs
> -----
>
> docs/configuration.md 309a5a05eab386c8943ba6bdee8d5efeb448aa0c
> src/local/local.cpp df72ac52110b75d63df1076496b48e63d06d42ce
> src/master/flags.cpp e522499586b731d522180f171731a9dd38b8344c
> src/slave/constants.hpp 449c8cd9f43f71b4612023eb463969e9db0bc960
> src/slave/flags.hpp 4236b7fe40b9aa26d57b651be06a0e36037f65c6
> src/slave/flags.cpp 7164afef9c82a9bba0f137d7387c3569912b6599
> src/slave/main.cpp 70df4f384b09a1fb078cd893efe52a5e3b116f48
> src/slave/slave.hpp 76f3aff394e5cecc54dcb3065cb872e238bc228e
> src/slave/slave.cpp f090c853b8affc4be5eecb4f616ec881fc2b60c3
> src/tests/cluster.hpp 39ca15e3cc2f4f2ac9b8a73482c2bd945b1824e3
> src/tests/cluster.cpp 7e488d28b7a0d3bef9cd76bf7df2de4822256ef6
> src/tests/mesos.cpp 1b7a8fd703b90d77ffa3df079bdc2744752caac6
>
> Diff: https://reviews.apache.org/r/45922/diff/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Jan Schlicht
>
>
Re: Review Request 45922: Added agent authorization flags.
Posted by Jan Schlicht <ja...@mesosphere.io>.
> On April 12, 2016, 12:30 p.m., Joerg Schad wrote:
> > src/slave/flags.hpp, line 102
> > <https://reviews.apache.org/r/45922/diff/3/?file=1337113#file1337113line102>
> >
> > Do we have a particular order here?
I don't know exactly. I kept it consistent to the one in `master/flags.hpp`. In that context it does make sense to place `acls` next to `credentials`. Of course other orders could be possible as well.
- Jan
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/45922/#review128356
-----------------------------------------------------------
On April 18, 2016, 11:49 a.m., Jan Schlicht wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45922/
> -----------------------------------------------------------
>
> (Updated April 18, 2016, 11:49 a.m.)
>
>
> Review request for mesos, Adam B and Alexander Rojas.
>
>
> Bugs: MESOS-5142
> https://issues.apache.org/jira/browse/MESOS-5142
>
>
> Repository: mesos
>
>
> Description
> -------
>
> See summary.
>
>
> Diffs
> -----
>
> docs/configuration.md cd9733002a0940f44edd4e56593a0ca3fe9f77f5
> src/local/local.cpp df72ac52110b75d63df1076496b48e63d06d42ce
> src/slave/constants.hpp 9978c11fec40055dd42f19c20cd3e9fef4e78cea
> src/slave/flags.hpp ee520acc459564fe68272950948fc80c5e24513a
> src/slave/flags.cpp 10d2974bd2b6e79255fc894979607f0d2d00c315
> src/slave/main.cpp 38bd00584dd9c6a872398678b2288edeed1cd2a4
> src/slave/slave.hpp f78c1b4e4d5378ef7223c6eb3ea45491c30fb4c1
> src/slave/slave.cpp d82dec2b10d496065013eb4ad6a35dc054b72553
> src/tests/cluster.hpp 887342acc72b33b4c904d610da47394f9a7d7188
> src/tests/cluster.cpp 31d2556a078429dd45315aa74cd21ad436372d31
> src/tests/mesos.hpp e4b63d41d883807ac39846799468b80e88c84e0b
> src/tests/mesos.cpp b5937af7713e1ee2af475518b3e968b2defa8beb
>
> Diff: https://reviews.apache.org/r/45922/diff/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Jan Schlicht
>
>
Re: Review Request 45922: Added agent authorization flags.
Posted by Joerg Schad <jo...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/45922/#review128356
-----------------------------------------------------------
docs/configuration.md (line 867)
<https://reviews.apache.org/r/45922/#comment191787>
This seems not relevant at the agent (register framework). If it is just used a placeholder please remove.
src/local/local.cpp (line 241)
<https://reviews.apache.org/r/45922/#comment191792>
Add a comment that this (master flag) is currently also used for the agent authorizer.
src/local/local.cpp (line 397)
<https://reviews.apache.org/r/45922/#comment191791>
Could we add a comment here that this is currently the same authorizer as the master i.e. is already checked byt the Master code?
src/slave/flags.hpp (line 102)
<https://reviews.apache.org/r/45922/#comment191793>
Do we have a particular order here?
src/slave/main.cpp (line 26)
<https://reviews.apache.org/r/45922/#comment191790>
This should go above mesos/master/detector or?
src/slave/main.cpp (line 293)
<https://reviews.apache.org/r/45922/#comment191800>
The blank lines between calls are inconsistent with
src/tests/cluster.cpp below.
src/tests/cluster.cpp (line 395)
<https://reviews.apache.org/r/45922/#comment191801>
blank lines inconsistent with main.cpp above.
src/tests/cluster.cpp (line 406)
<https://reviews.apache.org/r/45922/#comment191797>
We only create an authorizer if the acls flags are set, correct?
In the case of no acl flags it means the local authorizer permits everything and hence we don't have to specify it (i.e. authorizer((None())). Ist that correct?
In that case maybe "Creating default '" << authorizerName << "' authorizer as acls flags are are set"?
- Joerg Schad
On April 8, 2016, 2:25 p.m., Jan Schlicht wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45922/
> -----------------------------------------------------------
>
> (Updated April 8, 2016, 2:25 p.m.)
>
>
> Review request for mesos, Adam B and Alexander Rojas.
>
>
> Bugs: MESOS-5142
> https://issues.apache.org/jira/browse/MESOS-5142
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Added agent authorization flags.
>
>
> Diffs
> -----
>
> docs/configuration.md 309a5a05eab386c8943ba6bdee8d5efeb448aa0c
> src/local/local.cpp df72ac52110b75d63df1076496b48e63d06d42ce
> src/master/flags.cpp e522499586b731d522180f171731a9dd38b8344c
> src/slave/constants.hpp 449c8cd9f43f71b4612023eb463969e9db0bc960
> src/slave/flags.hpp 4236b7fe40b9aa26d57b651be06a0e36037f65c6
> src/slave/flags.cpp 7164afef9c82a9bba0f137d7387c3569912b6599
> src/slave/main.cpp 70df4f384b09a1fb078cd893efe52a5e3b116f48
> src/slave/slave.hpp 76f3aff394e5cecc54dcb3065cb872e238bc228e
> src/slave/slave.cpp f090c853b8affc4be5eecb4f616ec881fc2b60c3
> src/tests/cluster.hpp 39ca15e3cc2f4f2ac9b8a73482c2bd945b1824e3
> src/tests/cluster.cpp 7e488d28b7a0d3bef9cd76bf7df2de4822256ef6
> src/tests/mesos.cpp 1b7a8fd703b90d77ffa3df079bdc2744752caac6
>
> Diff: https://reviews.apache.org/r/45922/diff/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Jan Schlicht
>
>
Re: Review Request 45922: Added agent authorization flags.
Posted by Jan Schlicht <ja...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/45922/
-----------------------------------------------------------
(Updated April 8, 2016, 4:25 p.m.)
Review request for mesos, Adam B and Alexander Rojas.
Changes
-------
Add a comment explaining the current ACLs example.
Bugs: MESOS-5142
https://issues.apache.org/jira/browse/MESOS-5142
Repository: mesos
Description (updated)
-------
Added agent authorization flags.
Diffs (updated)
-----
docs/configuration.md 309a5a05eab386c8943ba6bdee8d5efeb448aa0c
src/local/local.cpp df72ac52110b75d63df1076496b48e63d06d42ce
src/master/flags.cpp e522499586b731d522180f171731a9dd38b8344c
src/slave/constants.hpp 449c8cd9f43f71b4612023eb463969e9db0bc960
src/slave/flags.hpp 4236b7fe40b9aa26d57b651be06a0e36037f65c6
src/slave/flags.cpp 7164afef9c82a9bba0f137d7387c3569912b6599
src/slave/main.cpp 70df4f384b09a1fb078cd893efe52a5e3b116f48
src/slave/slave.hpp 76f3aff394e5cecc54dcb3065cb872e238bc228e
src/slave/slave.cpp f090c853b8affc4be5eecb4f616ec881fc2b60c3
src/tests/cluster.hpp 39ca15e3cc2f4f2ac9b8a73482c2bd945b1824e3
src/tests/cluster.cpp 7e488d28b7a0d3bef9cd76bf7df2de4822256ef6
src/tests/mesos.cpp 1b7a8fd703b90d77ffa3df079bdc2744752caac6
Diff: https://reviews.apache.org/r/45922/diff/
Testing
-------
make check
Thanks,
Jan Schlicht
Re: Review Request 45922: Added agent authorization flags.
Posted by Alexander Rojas <al...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/45922/#review127768
-----------------------------------------------------------
src/slave/flags.cpp (lines 459 - 491)
<https://reviews.apache.org/r/45922/#comment191150>
Can you at least add a note mentioned that the example is not relevant to the agent but show how the format may look like.
- Alexander Rojas
On April 8, 2016, 11:26 a.m., Jan Schlicht wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45922/
> -----------------------------------------------------------
>
> (Updated April 8, 2016, 11:26 a.m.)
>
>
> Review request for mesos, Adam B and Alexander Rojas.
>
>
> Bugs: MESOS-5142
> https://issues.apache.org/jira/browse/MESOS-5142
>
>
> Repository: mesos
>
>
> Description
> -------
>
> See summary.
>
>
> Diffs
> -----
>
> docs/configuration.md 309a5a05eab386c8943ba6bdee8d5efeb448aa0c
> src/local/local.cpp df72ac52110b75d63df1076496b48e63d06d42ce
> src/master/flags.cpp e522499586b731d522180f171731a9dd38b8344c
> src/slave/constants.hpp 449c8cd9f43f71b4612023eb463969e9db0bc960
> src/slave/flags.hpp 4236b7fe40b9aa26d57b651be06a0e36037f65c6
> src/slave/flags.cpp 7164afef9c82a9bba0f137d7387c3569912b6599
> src/slave/main.cpp 70df4f384b09a1fb078cd893efe52a5e3b116f48
> src/slave/slave.hpp 76f3aff394e5cecc54dcb3065cb872e238bc228e
> src/slave/slave.cpp f090c853b8affc4be5eecb4f616ec881fc2b60c3
> src/tests/cluster.hpp 39ca15e3cc2f4f2ac9b8a73482c2bd945b1824e3
> src/tests/cluster.cpp 7e488d28b7a0d3bef9cd76bf7df2de4822256ef6
> src/tests/mesos.cpp 1b7a8fd703b90d77ffa3df079bdc2744752caac6
>
> Diff: https://reviews.apache.org/r/45922/diff/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Jan Schlicht
>
>
Re: Review Request 45922: Added agent authorization flags.
Posted by Mesos ReviewBot <re...@mesos.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/45922/#review127781
-----------------------------------------------------------
Patch looks great!
Reviews applied: [45922]
Passed command: export OS='ubuntu:14.04' CONFIGURATION='--verbose' COMPILER='gcc' ENVIRONMENT='GLOG_v=1 MESOS_VERBOSE=1'; ./support/docker_build.sh
- Mesos ReviewBot
On April 8, 2016, 9:26 a.m., Jan Schlicht wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45922/
> -----------------------------------------------------------
>
> (Updated April 8, 2016, 9:26 a.m.)
>
>
> Review request for mesos, Adam B and Alexander Rojas.
>
>
> Bugs: MESOS-5142
> https://issues.apache.org/jira/browse/MESOS-5142
>
>
> Repository: mesos
>
>
> Description
> -------
>
> See summary.
>
>
> Diffs
> -----
>
> docs/configuration.md 309a5a05eab386c8943ba6bdee8d5efeb448aa0c
> src/local/local.cpp df72ac52110b75d63df1076496b48e63d06d42ce
> src/master/flags.cpp e522499586b731d522180f171731a9dd38b8344c
> src/slave/constants.hpp 449c8cd9f43f71b4612023eb463969e9db0bc960
> src/slave/flags.hpp 4236b7fe40b9aa26d57b651be06a0e36037f65c6
> src/slave/flags.cpp 7164afef9c82a9bba0f137d7387c3569912b6599
> src/slave/main.cpp 70df4f384b09a1fb078cd893efe52a5e3b116f48
> src/slave/slave.hpp 76f3aff394e5cecc54dcb3065cb872e238bc228e
> src/slave/slave.cpp f090c853b8affc4be5eecb4f616ec881fc2b60c3
> src/tests/cluster.hpp 39ca15e3cc2f4f2ac9b8a73482c2bd945b1824e3
> src/tests/cluster.cpp 7e488d28b7a0d3bef9cd76bf7df2de4822256ef6
> src/tests/mesos.cpp 1b7a8fd703b90d77ffa3df079bdc2744752caac6
>
> Diff: https://reviews.apache.org/r/45922/diff/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Jan Schlicht
>
>
Re: Review Request 45922: Added agent authorization flags.
Posted by Jan Schlicht <ja...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/45922/
-----------------------------------------------------------
(Updated April 8, 2016, 11:26 a.m.)
Review request for mesos, Adam B and Alexander Rojas.
Changes
-------
Added flags documentation in `docs/configuration.md`.
Bugs: MESOS-5142
https://issues.apache.org/jira/browse/MESOS-5142
Repository: mesos
Description
-------
See summary.
Diffs (updated)
-----
docs/configuration.md 309a5a05eab386c8943ba6bdee8d5efeb448aa0c
src/local/local.cpp df72ac52110b75d63df1076496b48e63d06d42ce
src/master/flags.cpp e522499586b731d522180f171731a9dd38b8344c
src/slave/constants.hpp 449c8cd9f43f71b4612023eb463969e9db0bc960
src/slave/flags.hpp 4236b7fe40b9aa26d57b651be06a0e36037f65c6
src/slave/flags.cpp 7164afef9c82a9bba0f137d7387c3569912b6599
src/slave/main.cpp 70df4f384b09a1fb078cd893efe52a5e3b116f48
src/slave/slave.hpp 76f3aff394e5cecc54dcb3065cb872e238bc228e
src/slave/slave.cpp f090c853b8affc4be5eecb4f616ec881fc2b60c3
src/tests/cluster.hpp 39ca15e3cc2f4f2ac9b8a73482c2bd945b1824e3
src/tests/cluster.cpp 7e488d28b7a0d3bef9cd76bf7df2de4822256ef6
src/tests/mesos.cpp 1b7a8fd703b90d77ffa3df079bdc2744752caac6
Diff: https://reviews.apache.org/r/45922/diff/
Testing
-------
make check
Thanks,
Jan Schlicht
Re: Review Request 45922: Added agent authorization flags.
Posted by Jan Schlicht <ja...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/45922/#review127751
-----------------------------------------------------------
src/slave/flags.cpp (lines 459 - 491)
<https://reviews.apache.org/r/45922/#comment191138>
The agent ACLs will probably change, but because they aren't defined/used yet, I've kept the example the same as in `master/flags.cpp`, but could also remove it completely.
- Jan Schlicht
On April 8, 2016, 10:52 a.m., Jan Schlicht wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45922/
> -----------------------------------------------------------
>
> (Updated April 8, 2016, 10:52 a.m.)
>
>
> Review request for mesos, Adam B and Alexander Rojas.
>
>
> Bugs: MESOS-5142
> https://issues.apache.org/jira/browse/MESOS-5142
>
>
> Repository: mesos
>
>
> Description
> -------
>
> See summary.
>
>
> Diffs
> -----
>
> src/local/local.cpp df72ac52110b75d63df1076496b48e63d06d42ce
> src/master/flags.cpp e522499586b731d522180f171731a9dd38b8344c
> src/slave/constants.hpp 449c8cd9f43f71b4612023eb463969e9db0bc960
> src/slave/flags.hpp 4236b7fe40b9aa26d57b651be06a0e36037f65c6
> src/slave/flags.cpp 7164afef9c82a9bba0f137d7387c3569912b6599
> src/slave/main.cpp 70df4f384b09a1fb078cd893efe52a5e3b116f48
> src/slave/slave.hpp 76f3aff394e5cecc54dcb3065cb872e238bc228e
> src/slave/slave.cpp f090c853b8affc4be5eecb4f616ec881fc2b60c3
> src/tests/cluster.hpp 39ca15e3cc2f4f2ac9b8a73482c2bd945b1824e3
> src/tests/cluster.cpp 7e488d28b7a0d3bef9cd76bf7df2de4822256ef6
> src/tests/mesos.cpp 1b7a8fd703b90d77ffa3df079bdc2744752caac6
>
> Diff: https://reviews.apache.org/r/45922/diff/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Jan Schlicht
>
>
Re: Review Request 45922: Added agent authorization flags.
Posted by Joerg Schad <jo...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/45922/#review127753
-----------------------------------------------------------
src/slave/flags.cpp (line 446)
<https://reviews.apache.org/r/45922/#comment191140>
Please also add this to https://github.com/apache/mesos/blob/master/docs/configuration.md
- Joerg Schad
On April 8, 2016, 8:52 a.m., Jan Schlicht wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45922/
> -----------------------------------------------------------
>
> (Updated April 8, 2016, 8:52 a.m.)
>
>
> Review request for mesos, Adam B and Alexander Rojas.
>
>
> Bugs: MESOS-5142
> https://issues.apache.org/jira/browse/MESOS-5142
>
>
> Repository: mesos
>
>
> Description
> -------
>
> See summary.
>
>
> Diffs
> -----
>
> src/local/local.cpp df72ac52110b75d63df1076496b48e63d06d42ce
> src/master/flags.cpp e522499586b731d522180f171731a9dd38b8344c
> src/slave/constants.hpp 449c8cd9f43f71b4612023eb463969e9db0bc960
> src/slave/flags.hpp 4236b7fe40b9aa26d57b651be06a0e36037f65c6
> src/slave/flags.cpp 7164afef9c82a9bba0f137d7387c3569912b6599
> src/slave/main.cpp 70df4f384b09a1fb078cd893efe52a5e3b116f48
> src/slave/slave.hpp 76f3aff394e5cecc54dcb3065cb872e238bc228e
> src/slave/slave.cpp f090c853b8affc4be5eecb4f616ec881fc2b60c3
> src/tests/cluster.hpp 39ca15e3cc2f4f2ac9b8a73482c2bd945b1824e3
> src/tests/cluster.cpp 7e488d28b7a0d3bef9cd76bf7df2de4822256ef6
> src/tests/mesos.cpp 1b7a8fd703b90d77ffa3df079bdc2744752caac6
>
> Diff: https://reviews.apache.org/r/45922/diff/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Jan Schlicht
>
>