mesos-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Vinod Kone <vinodk...@gmail.com>
Subject Re: Review Request 57671: Allowed the agent to require executor authentication.
Date Thu, 16 Mar 2017 13:37:59 GMT

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




src/common/http.hpp
Line 216 (original), 220 (patched)
<https://reviews.apache.org/r/57671/#comment241469>

    no default for this?



src/common/http.cpp
Lines 1058 (patched)
<https://reviews.apache.org/r/57671/#comment241471>

    s/authenticatorNames[0]/name/ ???



src/common/http.cpp
Lines 1060 (patched)
<https://reviews.apache.org/r/57671/#comment241472>

    ditto. should be `name`



src/slave/slave.cpp
Lines 271-276 (patched)
<https://reviews.apache.org/r/57671/#comment241476>

    I know why you did it this way, but it seems a bit unfortunate that we are adding the
JWT authenticator implicitly when only "basic" authenticator is specified in the flag (it's
probably ok if the value is "basic" here because user didn't set anything and it is the default
but we can't distinguish that).
    
    I'm wondering if it would be better to make the default flag value as "default" or "auto"
with the flag help specifiying the behavior on what authenticators are picked ("basic" if
executor auth is not specified, "basic,jwt" if executor auth is specified etc).
    
    WDYT?


- Vinod Kone


On March 15, 2017, 11:39 p.m., Greg Mann wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/57671/
> -----------------------------------------------------------
> 
> (Updated March 15, 2017, 11:39 p.m.)
> 
> 
> Review request for mesos, Jan Schlicht and Vinod Kone.
> 
> 
> Bugs: MESOS-6998
>     https://issues.apache.org/jira/browse/MESOS-6998
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> This patch updates the agent initialization code to make use
> of the new `--authenticate_http_executors` flag. When the
> flag is set, authentication is required on the executor realm
> and the JWT authenticator is loaded.
> 
> 
> Diffs
> -----
> 
>   src/common/http.hpp a3cfc5d8f0b2e453d5f6c3e485e92dbd643737a3 
>   src/common/http.cpp ce32ff36ee58b19f2cb11d80e69ab1ff007e75ef 
>   src/slave/slave.cpp a4f4a9ca80b726de8e07571fd6d93120947c278b 
> 
> 
> Diff: https://reviews.apache.org/r/57671/diff/1/
> 
> 
> Testing
> -------
> 
> make check
> 
> 
> Thanks,
> 
> Greg Mann
> 
>


Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message