Re: Review Request 56901: Updated master validation code to use 'AuthenticationContext'.

[Alexander Rojas]

---
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 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'.

[Greg Mann]

> On Feb. 24, 2017, 1:53 a.m., Vinod Kone wrote:
> > src/master/validation.cpp, line 339
> > 
> >
> > 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
> > 
> >
> > 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
> > 
> >
> > 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 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'.

[Greg Mann]

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




src/master/validation.cpp (line 339)


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)


See above.



src/master/validation.cpp (line 1796)


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 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'.

[Vinod Kone]

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




src/master/validation.cpp (line 339)


Should this be a CHECK? When is this possible?



src/master/validation.cpp (line 1644)


can authContext be `Some` but not contain `principal`?



src/master/validation.cpp (line 1796)


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 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'.

[Greg Mann]

---
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 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