You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@mesos.apache.org by Greg Mann <gr...@mesosphere.io> on 2017/02/22 01:20:36 UTC
Review Request 56901: Updated master validation code to use
'AuthenticationContext'.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/56901/
-----------------------------------------------------------
Review request for mesos, Adam B, Alexander Rojas, Jan Schlicht, Till Toenshoff, and Vinod Kone.
Bugs: MESOS-7003
https://issues.apache.org/jira/browse/MESOS-7003
Repository: mesos
Description
-------
This patch updates master validation code to make use
of the `AuthenticationContext` instead of an
`Option<string> principal`.
Diffs
-----
src/master/validation.hpp f03b3280704083be1ca074ca07c69edbb49dae19
src/master/validation.cpp b8c6b49ca5ca1f15e58a2967adc335436a35071f
Diff: https://reviews.apache.org/r/56901/diff/
Testing
-------
Testing details can be found at the end of this review chain.
Thanks,
Greg Mann
Re: Review Request 56901: Updated master validation code to use
'AuthenticationContext'.
Posted by Greg Mann <gr...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/56901/#review166737
-----------------------------------------------------------
src/master/validation.cpp (line 339)
<https://reviews.apache.org/r/56901/#comment238747>
We'll soon be adding an authentication module (the JWT authenticator) which will return authentication contexts without a principal, so it's possible. While our default implementations will not produce credentials intended for schedulers, I think we should write the validation code to accommodate the generic case, and an arbitrary custom authenticator might decide to make wider use of authContexts which do not contain principals.
src/master/validation.cpp (line 1644)
<https://reviews.apache.org/r/56901/#comment238748>
See above.
src/master/validation.cpp (line 1796)
<https://reviews.apache.org/r/56901/#comment238749>
See above.
- Greg Mann
On Feb. 22, 2017, 7:48 p.m., Greg Mann wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/56901/
> -----------------------------------------------------------
>
> (Updated Feb. 22, 2017, 7:48 p.m.)
>
>
> Review request for mesos, Adam B, Alexander Rojas, Jan Schlicht, Till Toenshoff, and Vinod Kone.
>
>
> Bugs: MESOS-7003
> https://issues.apache.org/jira/browse/MESOS-7003
>
>
> Repository: mesos
>
>
> Description
> -------
>
> This patch updates master validation code to make use
> of the `AuthenticationContext` instead of an
> `Option<string> principal`.
>
>
> Diffs
> -----
>
> src/master/validation.hpp f03b3280704083be1ca074ca07c69edbb49dae19
> src/master/validation.cpp b8c6b49ca5ca1f15e58a2967adc335436a35071f
>
> Diff: https://reviews.apache.org/r/56901/diff/
>
>
> Testing
> -------
>
> Testing details can be found at the end of this review chain.
>
>
> Thanks,
>
> Greg Mann
>
>
Re: Review Request 56901: Updated master validation code to use
'AuthenticationContext'.
Posted by Greg Mann <gr...@mesosphere.io>.
> On Feb. 24, 2017, 1:53 a.m., Vinod Kone wrote:
> > src/master/validation.cpp, line 339
> > <https://reviews.apache.org/r/56901/diff/2/?file=1642754#file1642754line339>
> >
> > Should this be a CHECK? When is this possible?
We'll soon be adding an authentication module (the JWT authenticator) which will return authentication contexts without a principal, so it's possible. While our default implementations will not produce credentials intended for schedulers, I think we should write the validation code to accommodate the generic case, and an arbitrary custom authenticator might decide to make wider use of authContexts which do not contain principals.
> On Feb. 24, 2017, 1:53 a.m., Vinod Kone wrote:
> > src/master/validation.cpp, line 1644
> > <https://reviews.apache.org/r/56901/diff/2/?file=1642754#file1642754line1644>
> >
> > can authContext be `Some` but not contain `principal`?
See above.
> On Feb. 24, 2017, 1:53 a.m., Vinod Kone wrote:
> > src/master/validation.cpp, line 1796
> > <https://reviews.apache.org/r/56901/diff/2/?file=1642754#file1642754line1796>
> >
> > ditto.
See above.
- Greg
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/56901/#review166633
-----------------------------------------------------------
On Feb. 22, 2017, 7:48 p.m., Greg Mann wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/56901/
> -----------------------------------------------------------
>
> (Updated Feb. 22, 2017, 7:48 p.m.)
>
>
> Review request for mesos, Adam B, Alexander Rojas, Jan Schlicht, Till Toenshoff, and Vinod Kone.
>
>
> Bugs: MESOS-7003
> https://issues.apache.org/jira/browse/MESOS-7003
>
>
> Repository: mesos
>
>
> Description
> -------
>
> This patch updates master validation code to make use
> of the `AuthenticationContext` instead of an
> `Option<string> principal`.
>
>
> Diffs
> -----
>
> src/master/validation.hpp f03b3280704083be1ca074ca07c69edbb49dae19
> src/master/validation.cpp b8c6b49ca5ca1f15e58a2967adc335436a35071f
>
> Diff: https://reviews.apache.org/r/56901/diff/
>
>
> Testing
> -------
>
> Testing details can be found at the end of this review chain.
>
>
> Thanks,
>
> Greg Mann
>
>
Re: Review Request 56901: Updated master validation code to use
'AuthenticationContext'.
Posted by Vinod Kone <vi...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/56901/#review166633
-----------------------------------------------------------
src/master/validation.cpp (line 339)
<https://reviews.apache.org/r/56901/#comment238632>
Should this be a CHECK? When is this possible?
src/master/validation.cpp (line 1644)
<https://reviews.apache.org/r/56901/#comment238633>
can authContext be `Some` but not contain `principal`?
src/master/validation.cpp (line 1796)
<https://reviews.apache.org/r/56901/#comment238634>
ditto.
- Vinod Kone
On Feb. 22, 2017, 7:48 p.m., Greg Mann wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/56901/
> -----------------------------------------------------------
>
> (Updated Feb. 22, 2017, 7:48 p.m.)
>
>
> Review request for mesos, Adam B, Alexander Rojas, Jan Schlicht, Till Toenshoff, and Vinod Kone.
>
>
> Bugs: MESOS-7003
> https://issues.apache.org/jira/browse/MESOS-7003
>
>
> Repository: mesos
>
>
> Description
> -------
>
> This patch updates master validation code to make use
> of the `AuthenticationContext` instead of an
> `Option<string> principal`.
>
>
> Diffs
> -----
>
> src/master/validation.hpp f03b3280704083be1ca074ca07c69edbb49dae19
> src/master/validation.cpp b8c6b49ca5ca1f15e58a2967adc335436a35071f
>
> Diff: https://reviews.apache.org/r/56901/diff/
>
>
> Testing
> -------
>
> Testing details can be found at the end of this review chain.
>
>
> Thanks,
>
> Greg Mann
>
>
Re: Review Request 56901: Updated master validation code to use the
'Principal' type.
Posted by Vinod Kone <vi...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/56901/#review167895
-----------------------------------------------------------
Ship it!
Ship It!
- Vinod Kone
On March 3, 2017, 6:41 p.m., Greg Mann wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/56901/
> -----------------------------------------------------------
>
> (Updated March 3, 2017, 6:41 p.m.)
>
>
> Review request for mesos, Adam B, Alexander Rojas, Jan Schlicht, Till Toenshoff, and Vinod Kone.
>
>
> Bugs: MESOS-7003
> https://issues.apache.org/jira/browse/MESOS-7003
>
>
> Repository: mesos
>
>
> Description
> -------
>
> This patch updates master validation code to
> make use of the `Principal` type instead of an
> `Option<string> principal`.
>
>
> Diffs
> -----
>
> src/master/validation.hpp 50a3e6ec6a6d44c9ad449edfbd5d2cf8540b3a73
> src/master/validation.cpp 41ef0d072ca53d9695c16b8dd98c319186f11f75
>
>
> Diff: https://reviews.apache.org/r/56901/diff/4/
>
>
> Testing
> -------
>
> Testing details can be found at the end of this review chain.
>
>
> Thanks,
>
> Greg Mann
>
>
Re: Review Request 56901: Updated master validation code to use the
'Principal' type.
Posted by Greg Mann <gr...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/56901/
-----------------------------------------------------------
(Updated March 3, 2017, 6:41 p.m.)
Review request for mesos, Adam B, Alexander Rojas, Jan Schlicht, Till Toenshoff, and Vinod Kone.
Changes
-------
Adjusted code for the `principal->value.isSome()` invariant.
Bugs: MESOS-7003
https://issues.apache.org/jira/browse/MESOS-7003
Repository: mesos
Description
-------
This patch updates master validation code to
make use of the `Principal` type instead of an
`Option<string> principal`.
Diffs (updated)
-----
src/master/validation.hpp 50a3e6ec6a6d44c9ad449edfbd5d2cf8540b3a73
src/master/validation.cpp 41ef0d072ca53d9695c16b8dd98c319186f11f75
Diff: https://reviews.apache.org/r/56901/diff/4/
Changes: https://reviews.apache.org/r/56901/diff/3-4/
Testing
-------
Testing details can be found at the end of this review chain.
Thanks,
Greg Mann
Re: Review Request 56901: Updated master validation code to use the
'Principal' type.
Posted by Greg Mann <gr...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/56901/#review167591
-----------------------------------------------------------
src/master/validation.cpp
Lines 344 (patched)
<https://reviews.apache.org/r/56901/#comment239498>
Remove?
- Greg Mann
On Feb. 28, 2017, 6:30 a.m., Greg Mann wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/56901/
> -----------------------------------------------------------
>
> (Updated Feb. 28, 2017, 6:30 a.m.)
>
>
> Review request for mesos, Adam B, Alexander Rojas, Jan Schlicht, Till Toenshoff, and Vinod Kone.
>
>
> Bugs: MESOS-7003
> https://issues.apache.org/jira/browse/MESOS-7003
>
>
> Repository: mesos
>
>
> Description
> -------
>
> This patch updates master validation code to
> make use of the `Principal` type instead of an
> `Option<string> principal`.
>
>
> Diffs
> -----
>
> src/master/validation.hpp f03b3280704083be1ca074ca07c69edbb49dae19
> src/master/validation.cpp b8c6b49ca5ca1f15e58a2967adc335436a35071f
>
>
> Diff: https://reviews.apache.org/r/56901/diff/3/
>
>
> Testing
> -------
>
> Testing details can be found at the end of this review chain.
>
>
> Thanks,
>
> Greg Mann
>
>
Re: Review Request 56901: Updated master validation code to use the
'Principal' type.
Posted by Greg Mann <gr...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/56901/
-----------------------------------------------------------
(Updated Feb. 28, 2017, 6:30 a.m.)
Review request for mesos, Adam B, Alexander Rojas, Jan Schlicht, Till Toenshoff, and Vinod Kone.
Changes
-------
Changed 'AuthenticationContext' to 'Principal'.
Summary (updated)
-----------------
Updated master validation code to use the 'Principal' type.
Bugs: MESOS-7003
https://issues.apache.org/jira/browse/MESOS-7003
Repository: mesos
Description (updated)
-------
This patch updates master validation code to
make use of the `Principal` type instead of an
`Option<string> principal`.
Diffs (updated)
-----
src/master/validation.hpp f03b3280704083be1ca074ca07c69edbb49dae19
src/master/validation.cpp b8c6b49ca5ca1f15e58a2967adc335436a35071f
Diff: https://reviews.apache.org/r/56901/diff/
Testing
-------
Testing details can be found at the end of this review chain.
Thanks,
Greg Mann
Re: Review Request 56901: Updated master validation code to use
'AuthenticationContext'.
Posted by Alexander Rojas <al...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/56901/#review166862
-----------------------------------------------------------
Ship it!
Ship It!
- Alexander Rojas
On Feb. 22, 2017, 8:48 p.m., Greg Mann wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/56901/
> -----------------------------------------------------------
>
> (Updated Feb. 22, 2017, 8:48 p.m.)
>
>
> Review request for mesos, Adam B, Alexander Rojas, Jan Schlicht, Till Toenshoff, and Vinod Kone.
>
>
> Bugs: MESOS-7003
> https://issues.apache.org/jira/browse/MESOS-7003
>
>
> Repository: mesos
>
>
> Description
> -------
>
> This patch updates master validation code to make use
> of the `AuthenticationContext` instead of an
> `Option<string> principal`.
>
>
> Diffs
> -----
>
> src/master/validation.hpp f03b3280704083be1ca074ca07c69edbb49dae19
> src/master/validation.cpp b8c6b49ca5ca1f15e58a2967adc335436a35071f
>
> Diff: https://reviews.apache.org/r/56901/diff/
>
>
> Testing
> -------
>
> Testing details can be found at the end of this review chain.
>
>
> Thanks,
>
> Greg Mann
>
>
Re: Review Request 56901: Updated master validation code to use
'AuthenticationContext'.
Posted by Greg Mann <gr...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/56901/
-----------------------------------------------------------
(Updated Feb. 22, 2017, 7:48 p.m.)
Review request for mesos, Adam B, Alexander Rojas, Jan Schlicht, Till Toenshoff, and Vinod Kone.
Bugs: MESOS-7003
https://issues.apache.org/jira/browse/MESOS-7003
Repository: mesos
Description
-------
This patch updates master validation code to make use
of the `AuthenticationContext` instead of an
`Option<string> principal`.
Diffs (updated)
-----
src/master/validation.hpp f03b3280704083be1ca074ca07c69edbb49dae19
src/master/validation.cpp b8c6b49ca5ca1f15e58a2967adc335436a35071f
Diff: https://reviews.apache.org/r/56901/diff/
Testing
-------
Testing details can be found at the end of this review chain.
Thanks,
Greg Mann