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/03/23 16:18:50 UTC

Review Request 57879: Turned on executor authentication in the DefaultExecutorTests.

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

Review request for mesos, Anand Mazumdar and Vinod Kone.


Bugs: MESOS-6999
    https://issues.apache.org/jira/browse/MESOS-6999


Repository: mesos


Description
-------

Now that executor authentication has been added, this patch
enables authentication for the agent's operator endpoint in
the DefaultExecutorTests. This is only done when build with
SSL, since executor authentication currently has SSL as a
dependency.


Diffs
-----

  src/tests/default_executor_tests.cpp b7440d3c0b2b1b09f3237d2971ba99f904098c4b 


Diff: https://reviews.apache.org/r/57879/diff/1/


Testing
-------

Testing details can be found at the end of this patch chain.


Thanks,

Greg Mann


Re: Review Request 57879: Turned on executor authentication in the DefaultExecutorTests.

Posted by Anand Mazumdar <an...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/57879/#review169983
-----------------------------------------------------------




src/tests/default_executor_tests.cpp
Lines 86 (patched)
<https://reviews.apache.org/r/57879/#comment242710>

    Similar to my comment in the previous review create a virtual function `CreateSlaveFlags()`.



src/tests/default_executor_tests.cpp
Lines 91 (patched)
<https://reviews.apache.org/r/57879/#comment242711>

    Can you also add the reasoning as to why this is needed for posterity? You can copy/paste your comment in the previous review 
    
    ```cpp
      // Executor authentication currently has SSL as a dependency, so we cannot
      // require executors to authenticate with the agent's operator API if Mesos
      // was not built with SSL support.
    ```



src/tests/default_executor_tests.cpp
Lines 1395 (patched)
<https://reviews.apache.org/r/57879/#comment242712>

    Ditto as above.



src/tests/default_executor_tests.cpp
Lines 1400 (patched)
<https://reviews.apache.org/r/57879/#comment242713>

    Ditto as above.


- Anand Mazumdar


On March 23, 2017, 4:18 p.m., Greg Mann wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/57879/
> -----------------------------------------------------------
> 
> (Updated March 23, 2017, 4:18 p.m.)
> 
> 
> Review request for mesos, Anand Mazumdar and Vinod Kone.
> 
> 
> Bugs: MESOS-6999
>     https://issues.apache.org/jira/browse/MESOS-6999
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Now that executor authentication has been added, this patch
> enables authentication for the agent's operator endpoint in
> the DefaultExecutorTests. This is only done when build with
> SSL, since executor authentication currently has SSL as a
> dependency.
> 
> 
> Diffs
> -----
> 
>   src/tests/default_executor_tests.cpp b7440d3c0b2b1b09f3237d2971ba99f904098c4b 
> 
> 
> Diff: https://reviews.apache.org/r/57879/diff/1/
> 
> 
> Testing
> -------
> 
> Testing details can be found at the end of this patch chain.
> 
> 
> Thanks,
> 
> Greg Mann
> 
>


Re: Review Request 57879: Turned on executor authentication in the DefaultExecutorTests.

Posted by Anand Mazumdar <an...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/57879/#review170089
-----------------------------------------------------------


Ship it!




Ship It!

- Anand Mazumdar


On March 24, 2017, 2:05 p.m., Greg Mann wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/57879/
> -----------------------------------------------------------
> 
> (Updated March 24, 2017, 2:05 p.m.)
> 
> 
> Review request for mesos, Anand Mazumdar and Vinod Kone.
> 
> 
> Bugs: MESOS-6999
>     https://issues.apache.org/jira/browse/MESOS-6999
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Now that executor authentication has been added, this patch
> enables authentication for the agent's operator endpoint in
> the DefaultExecutorTests. This is only done when build with
> SSL, since executor authentication currently has SSL as a
> dependency.
> 
> 
> Diffs
> -----
> 
>   src/tests/default_executor_tests.cpp b7440d3c0b2b1b09f3237d2971ba99f904098c4b 
> 
> 
> Diff: https://reviews.apache.org/r/57879/diff/2/
> 
> 
> Testing
> -------
> 
> Testing details can be found at the end of this patch chain.
> 
> 
> Thanks,
> 
> Greg Mann
> 
>


Re: Review Request 57879: Turned on executor authentication in the DefaultExecutorTests.

Posted by Greg Mann <gr...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/57879/
-----------------------------------------------------------

(Updated March 24, 2017, 2:05 p.m.)


Review request for mesos, Anand Mazumdar and Vinod Kone.


Bugs: MESOS-6999
    https://issues.apache.org/jira/browse/MESOS-6999


Repository: mesos


Description
-------

Now that executor authentication has been added, this patch
enables authentication for the agent's operator endpoint in
the DefaultExecutorTests. This is only done when build with
SSL, since executor authentication currently has SSL as a
dependency.


Diffs (updated)
-----

  src/tests/default_executor_tests.cpp b7440d3c0b2b1b09f3237d2971ba99f904098c4b 


Diff: https://reviews.apache.org/r/57879/diff/2/

Changes: https://reviews.apache.org/r/57879/diff/1-2/


Testing
-------

Testing details can be found at the end of this patch chain.


Thanks,

Greg Mann