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/18 03:03:55 UTC

Review Request 56813: Updated master handlers to use 'AuthenticationContext'.

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

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 the HTTP endpoint handlers in the
agent process to accept an `AuthenticationContext`
instead of an `Option<string>& principal`.


Diffs
-----

  src/master/http.cpp e2fd71c5ae4178564b9a08756df5175aec5d6ca1 
  src/master/master.hpp 764adb18ddf09b62529c5c96f8e4dfaf7803483e 
  src/master/master.cpp 0f9fc39c1bd2af032f908d0263ed6a592ef1771c 
  src/master/quota_handler.cpp 3ad28e4a9363a877d0610b529a6c17fb30ece37a 
  src/master/registrar.cpp d7134eea34102ab7b24d2f0131363bdd9005cfd3 
  src/master/weights_handler.cpp da0b995e1866e7eb57b8ef885864c6c6d66405d6 

Diff: https://reviews.apache.org/r/56813/diff/


Testing
-------

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


Thanks,

Greg Mann


Re: Review Request 56813: Updated master handlers to use 'AuthenticationContext'.

Posted by Greg Mann <gr...@mesosphere.io>.

> On Feb. 24, 2017, 2:31 a.m., Vinod Kone wrote:
> > src/master/master.cpp, line 3446
> > <https://reviews.apache.org/r/56813/diff/3/?file=1642757#file1642757line3446>
> >
> >     This sounds like we should call it AuthenticatedEntity or authentication::Entity instead :) Also, I'm wondering if we can actually call this struct `Principal` based on the standard/wikipedia definition of principal https://en.wikipedia.org/wiki/Principal_(computer_security)?
> >     
> >     ```
> >     struct Principal 
> >     {
> >       Option<string> name;
> >       map<string, string> claims;
> >     }
> >     ```

Interesting... the `Principal` name would give us an object somewhat similar to the `ClaimsPrincipal` in the MS .NET framework: https://msdn.microsoft.com/en-us/library/system.security.claims.claimsprincipal(v=vs.110).aspx


- Greg


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


On Feb. 24, 2017, 5:59 p.m., Greg Mann wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/56813/
> -----------------------------------------------------------
> 
> (Updated Feb. 24, 2017, 5:59 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 the HTTP endpoint handlers in the
> master process to accept an `AuthenticationContext`
> instead of an `Option<string>& principal`.
> 
> 
> Diffs
> -----
> 
>   src/master/http.cpp e2fd71c5ae4178564b9a08756df5175aec5d6ca1 
>   src/master/master.hpp 764adb18ddf09b62529c5c96f8e4dfaf7803483e 
>   src/master/master.cpp 2ef836536784d5fda23c80dc2304239b176a8942 
>   src/master/quota_handler.cpp 3ad28e4a9363a877d0610b529a6c17fb30ece37a 
>   src/master/registrar.cpp d7134eea34102ab7b24d2f0131363bdd9005cfd3 
>   src/master/weights_handler.cpp 4f6a4cd29d27b64cbc9ee6d8909f79812313b681 
> 
> Diff: https://reviews.apache.org/r/56813/diff/
> 
> 
> Testing
> -------
> 
> Testing details can be found at the end of this review chain.
> 
> 
> Thanks,
> 
> Greg Mann
> 
>


Re: Review Request 56813: Updated master handlers to use 'AuthenticationContext'.

Posted by Greg Mann <gr...@mesosphere.io>.

> On Feb. 24, 2017, 2:31 a.m., Vinod Kone wrote:
> > src/master/http.cpp, line 842
> > <https://reviews.apache.org/r/56813/diff/3/?file=1642755#file1642755line842>
> >
> >     is it possible that this is none?

See comment in previous patch for agent handlers; new authenticator modules may return an `authContext` that is `SOME` but does not contain a principal.


- Greg


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


On Feb. 24, 2017, 11:07 p.m., Greg Mann wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/56813/
> -----------------------------------------------------------
> 
> (Updated Feb. 24, 2017, 11:07 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 the HTTP endpoint handlers in the
> master process to accept an `AuthenticationContext`
> instead of an `Option<string>& principal`.
> 
> 
> Diffs
> -----
> 
>   src/master/http.cpp 6e5178eeea6cc6b90ae253840da22be13444b088 
>   src/master/master.hpp 764adb18ddf09b62529c5c96f8e4dfaf7803483e 
>   src/master/master.cpp 2ef836536784d5fda23c80dc2304239b176a8942 
>   src/master/quota_handler.cpp 3ad28e4a9363a877d0610b529a6c17fb30ece37a 
>   src/master/registrar.cpp d7134eea34102ab7b24d2f0131363bdd9005cfd3 
>   src/master/weights_handler.cpp 4f6a4cd29d27b64cbc9ee6d8909f79812313b681 
> 
> Diff: https://reviews.apache.org/r/56813/diff/
> 
> 
> Testing
> -------
> 
> Testing details can be found at the end of this review chain.
> 
> 
> Thanks,
> 
> Greg Mann
> 
>


Re: Review Request 56813: Updated master handlers to use 'AuthenticationContext'.

Posted by Greg Mann <gr...@mesosphere.io>.

> On Feb. 24, 2017, 2:31 a.m., Vinod Kone wrote:
> > The description talks about the agent instead of master. Copy paste error?

Thanks; fixed.


- Greg


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


On Feb. 24, 2017, 11:07 p.m., Greg Mann wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/56813/
> -----------------------------------------------------------
> 
> (Updated Feb. 24, 2017, 11:07 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 the HTTP endpoint handlers in the
> master process to accept an `AuthenticationContext`
> instead of an `Option<string>& principal`.
> 
> 
> Diffs
> -----
> 
>   src/master/http.cpp 6e5178eeea6cc6b90ae253840da22be13444b088 
>   src/master/master.hpp 764adb18ddf09b62529c5c96f8e4dfaf7803483e 
>   src/master/master.cpp 2ef836536784d5fda23c80dc2304239b176a8942 
>   src/master/quota_handler.cpp 3ad28e4a9363a877d0610b529a6c17fb30ece37a 
>   src/master/registrar.cpp d7134eea34102ab7b24d2f0131363bdd9005cfd3 
>   src/master/weights_handler.cpp 4f6a4cd29d27b64cbc9ee6d8909f79812313b681 
> 
> Diff: https://reviews.apache.org/r/56813/diff/
> 
> 
> Testing
> -------
> 
> Testing details can be found at the end of this review chain.
> 
> 
> Thanks,
> 
> Greg Mann
> 
>


Re: Review Request 56813: Updated master handlers 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/56813/#review166638
-----------------------------------------------------------



The description talks about the agent instead of master. Copy paste error?


src/master/http.cpp (line 662)
<https://reviews.apache.org/r/56813/#comment238642>

    same question as previous review. 
    
    here and below.



src/master/http.cpp (line 839)
<https://reviews.apache.org/r/56813/#comment238646>

    is it possible that this is none?



src/master/master.cpp (line 3446)
<https://reviews.apache.org/r/56813/#comment238648>

    This sounds like we should call it AuthenticatedEntity or authentication::Entity instead :) Also, I'm wondering if we can actually call this struct `Principal` based on the standard/wikipedia definition of principal https://en.wikipedia.org/wiki/Principal_(computer_security)?
    
    ```
    struct Principal 
    {
      Option<string> name;
      map<string, string> claims;
    }
    ```


- Vinod Kone


On Feb. 22, 2017, 7:53 p.m., Greg Mann wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/56813/
> -----------------------------------------------------------
> 
> (Updated Feb. 22, 2017, 7:53 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 the HTTP endpoint handlers in the
> agent process to accept an `AuthenticationContext`
> instead of an `Option<string>& principal`.
> 
> 
> Diffs
> -----
> 
>   src/master/http.cpp e2fd71c5ae4178564b9a08756df5175aec5d6ca1 
>   src/master/master.hpp 764adb18ddf09b62529c5c96f8e4dfaf7803483e 
>   src/master/master.cpp 2ef836536784d5fda23c80dc2304239b176a8942 
>   src/master/quota_handler.cpp 3ad28e4a9363a877d0610b529a6c17fb30ece37a 
>   src/master/registrar.cpp d7134eea34102ab7b24d2f0131363bdd9005cfd3 
>   src/master/weights_handler.cpp 4f6a4cd29d27b64cbc9ee6d8909f79812313b681 
> 
> Diff: https://reviews.apache.org/r/56813/diff/
> 
> 
> Testing
> -------
> 
> Testing details can be found at the end of this review chain.
> 
> 
> Thanks,
> 
> Greg Mann
> 
>


Re: Review Request 56813: Updated master handlers 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/56813/#review167752
-----------------------------------------------------------




src/master/http.cpp
Lines 741-744 (original), 742-745 (patched)
<https://reviews.apache.org/r/56813/#comment239697>

    Add a check that `principal->value.isSome()` at top of this handler.


- Greg Mann


On Feb. 28, 2017, 6:34 p.m., Greg Mann wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/56813/
> -----------------------------------------------------------
> 
> (Updated Feb. 28, 2017, 6:34 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 the HTTP endpoint handlers in the
> master process to accept the `Principal` type instead
> of an `Option<string>& principal`.
> 
> 
> Diffs
> -----
> 
>   src/master/http.cpp 6e5178eeea6cc6b90ae253840da22be13444b088 
>   src/master/master.hpp 764adb18ddf09b62529c5c96f8e4dfaf7803483e 
>   src/master/master.cpp 2ef836536784d5fda23c80dc2304239b176a8942 
>   src/master/quota_handler.cpp 3ad28e4a9363a877d0610b529a6c17fb30ece37a 
>   src/master/registrar.cpp d7134eea34102ab7b24d2f0131363bdd9005cfd3 
>   src/master/weights_handler.cpp 4f6a4cd29d27b64cbc9ee6d8909f79812313b681 
> 
> 
> Diff: https://reviews.apache.org/r/56813/diff/6/
> 
> 
> Testing
> -------
> 
> Testing details can be found at the end of this review chain.
> 
> 
> Thanks,
> 
> Greg Mann
> 
>


Re: Review Request 56813: Updated master handlers to use the 'Principal' type.

Posted by Vinod Kone <vi...@gmail.com>.

> On March 2, 2017, 10:41 p.m., Greg Mann wrote:
> > src/master/master.cpp
> > Lines 1565-1573 (original), 1569-1577 (patched)
> > <https://reviews.apache.org/r/56813/diff/6/?file=1651813#file1651813line1569>
> >
> >     CHECK(principal->value.isSome())
> 
> Greg Mann wrote:
>     In this case, and in the case of `exceededCapacity`, I changed the `principal` parameter back to `Option<string>& principal`. It turns out these functions are only part of the `Master::visit` call path, so they only make use of the master's internal `frameworks.principals` map. Thus, they will always receive a string-type principal. I added a todo to change the signature when we resolve MESOS-7202.

sgtm. thanks.


- Vinod


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


On March 3, 2017, 6:45 p.m., Greg Mann wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/56813/
> -----------------------------------------------------------
> 
> (Updated March 3, 2017, 6:45 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 the HTTP endpoint handlers in the
> master process to accept the `Principal` type instead
> of an `Option<string>& principal`.
> 
> 
> Diffs
> -----
> 
>   src/master/http.cpp 37b96c07f17d3e5ba96f6eade69d86ba9e039b3d 
>   src/master/master.hpp 81320e0da3a2cd99d8dea1fbea9276296ef9e515 
>   src/master/master.cpp 2232cb7ad7c9ba7c067334d7327f592ab48e48fa 
>   src/master/quota_handler.cpp 3ad28e4a9363a877d0610b529a6c17fb30ece37a 
>   src/master/registrar.cpp d7134eea34102ab7b24d2f0131363bdd9005cfd3 
>   src/master/weights_handler.cpp d8047f2354322ec18b008afe44f507069b36a71f 
> 
> 
> Diff: https://reviews.apache.org/r/56813/diff/7/
> 
> 
> Testing
> -------
> 
> Testing details can be found at the end of this review chain.
> 
> 
> Thanks,
> 
> Greg Mann
> 
>


Re: Review Request 56813: Updated master handlers to use the 'Principal' type.

Posted by Greg Mann <gr...@mesosphere.io>.

> On March 2, 2017, 10:41 p.m., Greg Mann wrote:
> > src/master/master.hpp
> > Line 746 (original), 746 (patched)
> > <https://reviews.apache.org/r/56813/diff/6/?file=1651812#file1651812line746>
> >
> >     See if we can typedef this.

Sounds like we can't :(


> On March 2, 2017, 10:41 p.m., Greg Mann wrote:
> > src/master/master.cpp
> > Lines 1565-1573 (original), 1569-1577 (patched)
> > <https://reviews.apache.org/r/56813/diff/6/?file=1651813#file1651813line1569>
> >
> >     CHECK(principal->value.isSome())

In this case, and in the case of `exceededCapacity`, I changed the `principal` parameter back to `Option<string>& principal`. It turns out these functions are only part of the `Master::visit` call path, so they only make use of the master's internal `frameworks.principals` map. Thus, they will always receive a string-type principal. I added a todo to change the signature when we resolve MESOS-7202.


> On March 2, 2017, 10:41 p.m., Greg Mann wrote:
> > src/master/master.cpp
> > Lines 1608-1611 (original), 1612-1615 (patched)
> > <https://reviews.apache.org/r/56813/diff/6/?file=1651813#file1651813line1612>
> >
> >     Rework to make more readable

Since this now accepts a string principal again, I reverted to the previous logging.


> On March 2, 2017, 10:41 p.m., Greg Mann wrote:
> > src/master/master.cpp
> > Lines 3499-3501 (original), 3503-3504 (patched)
> > <https://reviews.apache.org/r/56813/diff/6/?file=1651813#file1651813line3503>
> >
> >     s/ANY/'ANY'/

It looks like we do quote JSON as well, so I'm leaving quotes in place. I also decided I liked the idea of making the streaming operator smarter, so that's been updated as well.


- Greg


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


On March 3, 2017, 6:45 p.m., Greg Mann wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/56813/
> -----------------------------------------------------------
> 
> (Updated March 3, 2017, 6:45 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 the HTTP endpoint handlers in the
> master process to accept the `Principal` type instead
> of an `Option<string>& principal`.
> 
> 
> Diffs
> -----
> 
>   src/master/http.cpp 37b96c07f17d3e5ba96f6eade69d86ba9e039b3d 
>   src/master/master.hpp 81320e0da3a2cd99d8dea1fbea9276296ef9e515 
>   src/master/master.cpp 2232cb7ad7c9ba7c067334d7327f592ab48e48fa 
>   src/master/quota_handler.cpp 3ad28e4a9363a877d0610b529a6c17fb30ece37a 
>   src/master/registrar.cpp d7134eea34102ab7b24d2f0131363bdd9005cfd3 
>   src/master/weights_handler.cpp d8047f2354322ec18b008afe44f507069b36a71f 
> 
> 
> Diff: https://reviews.apache.org/r/56813/diff/7/
> 
> 
> Testing
> -------
> 
> Testing details can be found at the end of this review chain.
> 
> 
> Thanks,
> 
> Greg Mann
> 
>


Re: Review Request 56813: Updated master handlers 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/56813/#review167754
-----------------------------------------------------------




src/master/http.cpp
Lines 841-845 (original), 844-851 (patched)
<https://reviews.apache.org/r/56813/#comment239698>

    CHECK(principal->value.isSome())?



src/master/http.cpp
Lines 1823-1825 (original), 1812-1814 (patched)
<https://reviews.apache.org/r/56813/#comment239700>

    Consider:
    
    Option<Subject> subject = createSubject(principal)
    if (subject.isSome()) {
        authRequest.mutable_subject()->CopyFrom(subject.get);
    }



src/master/master.hpp
Line 746 (original), 746 (patched)
<https://reviews.apache.org/r/56813/#comment239701>

    See if we can typedef this.



src/master/master.cpp
Lines 1565-1573 (original), 1569-1577 (patched)
<https://reviews.apache.org/r/56813/#comment239703>

    CHECK(principal->value.isSome())



src/master/master.cpp
Lines 1608-1611 (original), 1612-1615 (patched)
<https://reviews.apache.org/r/56813/#comment239704>

    Rework to make more readable



src/master/master.cpp
Lines 3499-3501 (original), 3503-3504 (patched)
<https://reviews.apache.org/r/56813/#comment239705>

    s/ANY/'ANY'/



src/master/master.cpp
Line 3537 (original), 3540 (patched)
<https://reviews.apache.org/r/56813/#comment239706>

    s/entity/principal/



src/master/master.cpp
Line 3553 (original), 3556 (patched)
<https://reviews.apache.org/r/56813/#comment239707>

    'ANY'



src/master/master.cpp
Line 3605 (original), 3607 (patched)
<https://reviews.apache.org/r/56813/#comment239708>

    'ANY'



src/master/quota_handler.cpp
Line 379 (original), 383 (patched)
<https://reviews.apache.org/r/56813/#comment239709>

    CHECK_SOME(principal->value)



src/master/quota_handler.cpp
Line 541 (original), 545 (patched)
<https://reviews.apache.org/r/56813/#comment239710>

    s/entity/principal



src/master/quota_handler.cpp
Line 542 (original), 546 (patched)
<https://reviews.apache.org/r/56813/#comment239711>

    'ANY'



src/master/quota_handler.cpp
Lines 571-572 (original), 575-576 (patched)
<https://reviews.apache.org/r/56813/#comment239712>

    principal
    
    'ANY'



src/master/quota_handler.cpp
Lines 599-600 (original), 603-604 (patched)
<https://reviews.apache.org/r/56813/#comment239713>

    principal
    
    'ANY'



src/master/weights_handler.cpp
Lines 323-324 (original), 327-328 (patched)
<https://reviews.apache.org/r/56813/#comment239714>

    principal
    
    'ANY'



src/master/weights_handler.cpp
Line 366 (original), 370 (patched)
<https://reviews.apache.org/r/56813/#comment239715>

    principal
    
    'ANY'


- Greg Mann


On Feb. 28, 2017, 6:34 p.m., Greg Mann wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/56813/
> -----------------------------------------------------------
> 
> (Updated Feb. 28, 2017, 6:34 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 the HTTP endpoint handlers in the
> master process to accept the `Principal` type instead
> of an `Option<string>& principal`.
> 
> 
> Diffs
> -----
> 
>   src/master/http.cpp 6e5178eeea6cc6b90ae253840da22be13444b088 
>   src/master/master.hpp 764adb18ddf09b62529c5c96f8e4dfaf7803483e 
>   src/master/master.cpp 2ef836536784d5fda23c80dc2304239b176a8942 
>   src/master/quota_handler.cpp 3ad28e4a9363a877d0610b529a6c17fb30ece37a 
>   src/master/registrar.cpp d7134eea34102ab7b24d2f0131363bdd9005cfd3 
>   src/master/weights_handler.cpp 4f6a4cd29d27b64cbc9ee6d8909f79812313b681 
> 
> 
> Diff: https://reviews.apache.org/r/56813/diff/6/
> 
> 
> Testing
> -------
> 
> Testing details can be found at the end of this review chain.
> 
> 
> Thanks,
> 
> Greg Mann
> 
>


Re: Review Request 56813: Updated master handlers 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/56813/
-----------------------------------------------------------

(Updated March 6, 2017, 7:11 p.m.)


Review request for mesos, Adam B, Alexander Rojas, Jan Schlicht, Till Toenshoff, and Vinod Kone.


Changes
-------

Removed a period


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


Repository: mesos


Description
-------

This patch updates the HTTP endpoint handlers in the
master process to accept the `Principal` type instead
of an `Option<string>& principal`.


Diffs (updated)
-----

  src/master/http.cpp 37b96c07f17d3e5ba96f6eade69d86ba9e039b3d 
  src/master/master.hpp 81320e0da3a2cd99d8dea1fbea9276296ef9e515 
  src/master/master.cpp a15c6d8fb1f99d117eed8e9453a643ec80319a52 
  src/master/quota_handler.cpp 3ad28e4a9363a877d0610b529a6c17fb30ece37a 
  src/master/registrar.cpp d7134eea34102ab7b24d2f0131363bdd9005cfd3 
  src/master/weights_handler.cpp d8047f2354322ec18b008afe44f507069b36a71f 


Diff: https://reviews.apache.org/r/56813/diff/8/

Changes: https://reviews.apache.org/r/56813/diff/7-8/


Testing
-------

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


Thanks,

Greg Mann


Re: Review Request 56813: Updated master handlers to use the 'Principal' type.

Posted by Greg Mann <gr...@mesosphere.io>.

> On March 4, 2017, 12:31 a.m., Vinod Kone wrote:
> > src/master/http.cpp
> > Lines 482 (patched)
> > <https://reviews.apache.org/r/56813/diff/3-7/?file=1642755#file1642755line483>
> >
> >     Maybe `BadRequest` is better here than `Forbidden`. For example we send `BadRequest` below in `scheduler()` handler when principals do not match etc. What do you think? 
> >     
> >     Here and below.

Let's consider the definitions of the two status codes:

```
10.4.1 400 Bad Request

The request could not be understood by the server due to malformed syntax. The client SHOULD NOT repeat the request without modifications.
```

```
10.4.4 403 Forbidden

The server understood the request, but is refusing to fulfill it. Authorization will not help and the request SHOULD NOT be repeated. If the request method was not HEAD and the server wishes to make public why the request has not been fulfilled, it SHOULD describe the reason for the refusal in the entity. If the server does not wish to make this information available to the client, the status code 404 (Not Found) can be used instead.
```

'400 Bad Request' is used to indicate malformed syntax in the request. In the case of mismatched principals elsewhere in `scheduler()`, for example, the request body is malformed because its `principal` field is required to match its authenticated principal. In this case, however, we are not indicating that the request is malformed, but rather that the authenticated principal is not allowed to access this resource. In a sense, this is a case of implicit authorization. I think that '403 Forbidden' makes more sense here.


- Greg


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


On March 3, 2017, 6:45 p.m., Greg Mann wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/56813/
> -----------------------------------------------------------
> 
> (Updated March 3, 2017, 6:45 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 the HTTP endpoint handlers in the
> master process to accept the `Principal` type instead
> of an `Option<string>& principal`.
> 
> 
> Diffs
> -----
> 
>   src/master/http.cpp 37b96c07f17d3e5ba96f6eade69d86ba9e039b3d 
>   src/master/master.hpp 81320e0da3a2cd99d8dea1fbea9276296ef9e515 
>   src/master/master.cpp 2232cb7ad7c9ba7c067334d7327f592ab48e48fa 
>   src/master/quota_handler.cpp 3ad28e4a9363a877d0610b529a6c17fb30ece37a 
>   src/master/registrar.cpp d7134eea34102ab7b24d2f0131363bdd9005cfd3 
>   src/master/weights_handler.cpp d8047f2354322ec18b008afe44f507069b36a71f 
> 
> 
> Diff: https://reviews.apache.org/r/56813/diff/7/
> 
> 
> Testing
> -------
> 
> Testing details can be found at the end of this review chain.
> 
> 
> Thanks,
> 
> Greg Mann
> 
>


Re: Review Request 56813: Updated master handlers to use the 'Principal' type.

Posted by Vinod Kone <vi...@gmail.com>.

> On March 4, 2017, 12:31 a.m., Vinod Kone wrote:
> > src/master/http.cpp
> > Lines 482 (patched)
> > <https://reviews.apache.org/r/56813/diff/3-7/?file=1642755#file1642755line483>
> >
> >     Maybe `BadRequest` is better here than `Forbidden`. For example we send `BadRequest` below in `scheduler()` handler when principals do not match etc. What do you think? 
> >     
> >     Here and below.
> 
> Greg Mann wrote:
>     Let's consider the definitions of the two status codes:
>     
>     ```
>     10.4.1 400 Bad Request
>     
>     The request could not be understood by the server due to malformed syntax. The client SHOULD NOT repeat the request without modifications.
>     ```
>     
>     ```
>     10.4.4 403 Forbidden
>     
>     The server understood the request, but is refusing to fulfill it. Authorization will not help and the request SHOULD NOT be repeated. If the request method was not HEAD and the server wishes to make public why the request has not been fulfilled, it SHOULD describe the reason for the refusal in the entity. If the server does not wish to make this information available to the client, the status code 404 (Not Found) can be used instead.
>     ```
>     
>     '400 Bad Request' is used to indicate malformed syntax in the request. In the case of mismatched principals elsewhere in `scheduler()`, for example, the request body is malformed because its `principal` field is required to match its authenticated principal. In this case, however, we are not indicating that the request is malformed, but rather that the authenticated principal is not allowed to access this resource. In a sense, this is a case of implicit authorization. I think that '403 Forbidden' makes more sense here.

makes sense.


- Vinod


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


On March 3, 2017, 6:45 p.m., Greg Mann wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/56813/
> -----------------------------------------------------------
> 
> (Updated March 3, 2017, 6:45 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 the HTTP endpoint handlers in the
> master process to accept the `Principal` type instead
> of an `Option<string>& principal`.
> 
> 
> Diffs
> -----
> 
>   src/master/http.cpp 37b96c07f17d3e5ba96f6eade69d86ba9e039b3d 
>   src/master/master.hpp 81320e0da3a2cd99d8dea1fbea9276296ef9e515 
>   src/master/master.cpp 2232cb7ad7c9ba7c067334d7327f592ab48e48fa 
>   src/master/quota_handler.cpp 3ad28e4a9363a877d0610b529a6c17fb30ece37a 
>   src/master/registrar.cpp d7134eea34102ab7b24d2f0131363bdd9005cfd3 
>   src/master/weights_handler.cpp d8047f2354322ec18b008afe44f507069b36a71f 
> 
> 
> Diff: https://reviews.apache.org/r/56813/diff/7/
> 
> 
> Testing
> -------
> 
> Testing details can be found at the end of this review chain.
> 
> 
> Thanks,
> 
> Greg Mann
> 
>


Re: Review Request 56813: Updated master handlers 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/56813/#review167908
-----------------------------------------------------------




src/master/http.cpp
Lines 482 (patched)
<https://reviews.apache.org/r/56813/#comment239878>

    Maybe `BadRequest` is better here than `Forbidden`. For example we send `BadRequest` below in `scheduler()` handler when principals do not match etc. What do you think? 
    
    Here and below.


- Vinod Kone


On March 3, 2017, 6:45 p.m., Greg Mann wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/56813/
> -----------------------------------------------------------
> 
> (Updated March 3, 2017, 6:45 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 the HTTP endpoint handlers in the
> master process to accept the `Principal` type instead
> of an `Option<string>& principal`.
> 
> 
> Diffs
> -----
> 
>   src/master/http.cpp 37b96c07f17d3e5ba96f6eade69d86ba9e039b3d 
>   src/master/master.hpp 81320e0da3a2cd99d8dea1fbea9276296ef9e515 
>   src/master/master.cpp 2232cb7ad7c9ba7c067334d7327f592ab48e48fa 
>   src/master/quota_handler.cpp 3ad28e4a9363a877d0610b529a6c17fb30ece37a 
>   src/master/registrar.cpp d7134eea34102ab7b24d2f0131363bdd9005cfd3 
>   src/master/weights_handler.cpp d8047f2354322ec18b008afe44f507069b36a71f 
> 
> 
> Diff: https://reviews.apache.org/r/56813/diff/7/
> 
> 
> Testing
> -------
> 
> Testing details can be found at the end of this review chain.
> 
> 
> Thanks,
> 
> Greg Mann
> 
>


Re: Review Request 56813: Updated master handlers 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/56813/
-----------------------------------------------------------

(Updated March 3, 2017, 6:45 p.m.)


Review request for mesos, Adam B, Alexander Rojas, Jan Schlicht, Till Toenshoff, and Vinod Kone.


Changes
-------

Added the constraint that `principal->value.isSome()` is true. Removed `createOptionalSubject()`.


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


Repository: mesos


Description
-------

This patch updates the HTTP endpoint handlers in the
master process to accept the `Principal` type instead
of an `Option<string>& principal`.


Diffs (updated)
-----

  src/master/http.cpp 37b96c07f17d3e5ba96f6eade69d86ba9e039b3d 
  src/master/master.hpp 81320e0da3a2cd99d8dea1fbea9276296ef9e515 
  src/master/master.cpp 2232cb7ad7c9ba7c067334d7327f592ab48e48fa 
  src/master/quota_handler.cpp 3ad28e4a9363a877d0610b529a6c17fb30ece37a 
  src/master/registrar.cpp d7134eea34102ab7b24d2f0131363bdd9005cfd3 
  src/master/weights_handler.cpp d8047f2354322ec18b008afe44f507069b36a71f 


Diff: https://reviews.apache.org/r/56813/diff/7/

Changes: https://reviews.apache.org/r/56813/diff/6-7/


Testing
-------

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


Thanks,

Greg Mann


Re: Review Request 56813: Updated master handlers 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/56813/
-----------------------------------------------------------

(Updated Feb. 28, 2017, 6:34 p.m.)


Review request for mesos, Adam B, Alexander Rojas, Jan Schlicht, Till Toenshoff, and Vinod Kone.


Changes
-------

Addressed comments.


Summary (updated)
-----------------

Updated master handlers to use the 'Principal' type.


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


Repository: mesos


Description
-------

This patch updates the HTTP endpoint handlers in the
master process to accept the `Principal` type instead
of an `Option<string>& principal`.


Diffs (updated)
-----

  src/master/http.cpp 6e5178eeea6cc6b90ae253840da22be13444b088 
  src/master/master.hpp 764adb18ddf09b62529c5c96f8e4dfaf7803483e 
  src/master/master.cpp 2ef836536784d5fda23c80dc2304239b176a8942 
  src/master/quota_handler.cpp 3ad28e4a9363a877d0610b529a6c17fb30ece37a 
  src/master/registrar.cpp d7134eea34102ab7b24d2f0131363bdd9005cfd3 
  src/master/weights_handler.cpp 4f6a4cd29d27b64cbc9ee6d8909f79812313b681 

Diff: https://reviews.apache.org/r/56813/diff/


Testing
-------

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


Thanks,

Greg Mann


Re: Review Request 56813: Updated master handlers to use the 'Principal' type.

Posted by Greg Mann <gr...@mesosphere.io>.

> On Feb. 28, 2017, 1:38 p.m., Alexander Rojas wrote:
> > src/master/http.cpp, lines 2224-2227
> > <https://reviews.apache.org/r/56813/diff/5/?file=1651003#file1651003line2224>
> >
> >     This line didn't need modification. However it does raises the issue of the signature of `validation::operation::validate()`. The common pattern in Mesos is to use a default parameter in the declaration, like:
> >     
> >     ```c++
> >     Option<Error> validate(
> >         const Offer::Operation::Reserve& reserve,
> >         const Option<string>& principal,
> >         const Option<FrameworkInfo>& frameworkInfo = None())
> >     ```
> >     
> >     so that the caller doesn't need to do it. Doing a grep on the srouce code in over a hundred instances of that pattern.
> >     
> >     Could you quickly fix that?
> 
> Greg Mann wrote:
>     Sure thing, I'll follow-up with another patch and leave a link here.

Follow-up review here: https://reviews.apache.org/r/57158/


- Greg


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


On Feb. 28, 2017, 6:34 p.m., Greg Mann wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/56813/
> -----------------------------------------------------------
> 
> (Updated Feb. 28, 2017, 6:34 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 the HTTP endpoint handlers in the
> master process to accept the `Principal` type instead
> of an `Option<string>& principal`.
> 
> 
> Diffs
> -----
> 
>   src/master/http.cpp 6e5178eeea6cc6b90ae253840da22be13444b088 
>   src/master/master.hpp 764adb18ddf09b62529c5c96f8e4dfaf7803483e 
>   src/master/master.cpp 2ef836536784d5fda23c80dc2304239b176a8942 
>   src/master/quota_handler.cpp 3ad28e4a9363a877d0610b529a6c17fb30ece37a 
>   src/master/registrar.cpp d7134eea34102ab7b24d2f0131363bdd9005cfd3 
>   src/master/weights_handler.cpp 4f6a4cd29d27b64cbc9ee6d8909f79812313b681 
> 
> Diff: https://reviews.apache.org/r/56813/diff/
> 
> 
> Testing
> -------
> 
> Testing details can be found at the end of this review chain.
> 
> 
> Thanks,
> 
> Greg Mann
> 
>


Re: Review Request 56813: Updated master handlers to use the 'Principal' type.

Posted by Greg Mann <gr...@mesosphere.io>.

> On Feb. 28, 2017, 1:38 p.m., Alexander Rojas wrote:
> > src/master/http.cpp, lines 889-891
> > <https://reviews.apache.org/r/56813/diff/5/?file=1651003#file1651003line889>
> >
> >     I'm not sure printing the whole `principal` with name and claims is such a good idea. The reason is that you only print `framework->info.principal()`, and to my knowledge, only compare the names. Perhaps is a better idea to only print `principal->name`?

I agree it would be better to just print the `value`; unfortunately, it's possible here that `principal->value` is NONE. We could have another conditional for the NONE case but I don't think it's worth it.


> On Feb. 28, 2017, 1:38 p.m., Alexander Rojas wrote:
> > src/master/http.cpp, lines 2224-2227
> > <https://reviews.apache.org/r/56813/diff/5/?file=1651003#file1651003line2224>
> >
> >     This line didn't need modification. However it does raises the issue of the signature of `validation::operation::validate()`. The common pattern in Mesos is to use a default parameter in the declaration, like:
> >     
> >     ```c++
> >     Option<Error> validate(
> >         const Offer::Operation::Reserve& reserve,
> >         const Option<string>& principal,
> >         const Option<FrameworkInfo>& frameworkInfo = None())
> >     ```
> >     
> >     so that the caller doesn't need to do it. Doing a grep on the srouce code in over a hundred instances of that pattern.
> >     
> >     Could you quickly fix that?

Sure thing, I'll follow-up with another patch and leave a link here.


- Greg


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


On Feb. 28, 2017, 6:34 p.m., Greg Mann wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/56813/
> -----------------------------------------------------------
> 
> (Updated Feb. 28, 2017, 6:34 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 the HTTP endpoint handlers in the
> master process to accept the `Principal` type instead
> of an `Option<string>& principal`.
> 
> 
> Diffs
> -----
> 
>   src/master/http.cpp 6e5178eeea6cc6b90ae253840da22be13444b088 
>   src/master/master.hpp 764adb18ddf09b62529c5c96f8e4dfaf7803483e 
>   src/master/master.cpp 2ef836536784d5fda23c80dc2304239b176a8942 
>   src/master/quota_handler.cpp 3ad28e4a9363a877d0610b529a6c17fb30ece37a 
>   src/master/registrar.cpp d7134eea34102ab7b24d2f0131363bdd9005cfd3 
>   src/master/weights_handler.cpp 4f6a4cd29d27b64cbc9ee6d8909f79812313b681 
> 
> Diff: https://reviews.apache.org/r/56813/diff/
> 
> 
> Testing
> -------
> 
> Testing details can be found at the end of this review chain.
> 
> 
> Thanks,
> 
> Greg Mann
> 
>


Re: Review Request 56813: Updated master handlers to use the 'Principal' type.

Posted by Greg Mann <gr...@mesosphere.io>.

> On Feb. 28, 2017, 1:38 p.m., Alexander Rojas wrote:
> > src/master/http.cpp, line 115
> > <https://reviews.apache.org/r/56813/diff/5/?file=1651003#file1651003line115>
> >
> >     wasn't metrics process used at all?

It wasn't - I split this change out into a separate review: https://reviews.apache.org/r/57153/


- Greg


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


On Feb. 28, 2017, 6:34 p.m., Greg Mann wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/56813/
> -----------------------------------------------------------
> 
> (Updated Feb. 28, 2017, 6:34 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 the HTTP endpoint handlers in the
> master process to accept the `Principal` type instead
> of an `Option<string>& principal`.
> 
> 
> Diffs
> -----
> 
>   src/master/http.cpp 6e5178eeea6cc6b90ae253840da22be13444b088 
>   src/master/master.hpp 764adb18ddf09b62529c5c96f8e4dfaf7803483e 
>   src/master/master.cpp 2ef836536784d5fda23c80dc2304239b176a8942 
>   src/master/quota_handler.cpp 3ad28e4a9363a877d0610b529a6c17fb30ece37a 
>   src/master/registrar.cpp d7134eea34102ab7b24d2f0131363bdd9005cfd3 
>   src/master/weights_handler.cpp 4f6a4cd29d27b64cbc9ee6d8909f79812313b681 
> 
> Diff: https://reviews.apache.org/r/56813/diff/
> 
> 
> Testing
> -------
> 
> Testing details can be found at the end of this review chain.
> 
> 
> Thanks,
> 
> Greg Mann
> 
>


Re: Review Request 56813: Updated master handlers 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/56813/#review167059
-----------------------------------------------------------




src/master/http.cpp (line 115)
<https://reviews.apache.org/r/56813/#comment239185>

    wasn't metrics process used at all?



src/master/http.cpp (lines 805 - 806)
<https://reviews.apache.org/r/56813/#comment239186>

    This can be merge into one line.



src/master/http.cpp (lines 884 - 886)
<https://reviews.apache.org/r/56813/#comment239187>

    I'm not sure printing the whole `principal` with name and claims is such a good idea. The reason is that you only print `framework->info.principal()`, and to my knowledge, only compare the names. Perhaps is a better idea to only print `principal->name`?



src/master/http.cpp (lines 1104 - 1106)
<https://reviews.apache.org/r/56813/#comment239188>

    This line doesn't need change.



src/master/http.cpp (lines 2203 - 2206)
<https://reviews.apache.org/r/56813/#comment239189>

    This line didn't need modification. However it does raises the issue of the signature of `validation::operation::validate()`. The common pattern in Mesos is to use a default parameter in the declaration, like:
    
    ```c++
    Option<Error> validate(
        const Offer::Operation::Reserve& reserve,
        const Option<string>& principal,
        const Option<FrameworkInfo>& frameworkInfo = None())
    ```
    
    so that the caller doesn't need to do it. Doing a grep on the srouce code in over a hundred instances of that pattern.
    
    Could you quickly fix that?



src/master/master.cpp (lines 1070 - 1073)
<https://reviews.apache.org/r/56813/#comment239191>

    No need for the break and indent.



src/master/master.cpp (line 3880)
<https://reviews.apache.org/r/56813/#comment239193>

    /Option<Principal>::none()/None()/



src/master/master.cpp (line 3893)
<https://reviews.apache.org/r/56813/#comment239194>

    /Option<Principal>::none()/None()/



src/master/master.cpp (line 3919)
<https://reviews.apache.org/r/56813/#comment239195>

    /Option<Principal>::none()/None()/


- Alexander Rojas


On Feb. 28, 2017, 7:56 a.m., Greg Mann wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/56813/
> -----------------------------------------------------------
> 
> (Updated Feb. 28, 2017, 7:56 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 the HTTP endpoint handlers in the
> master process to accept the `Principal` type instead
> of an `Option<string>& principal`.
> 
> 
> Diffs
> -----
> 
>   src/master/http.cpp 6e5178eeea6cc6b90ae253840da22be13444b088 
>   src/master/master.hpp 764adb18ddf09b62529c5c96f8e4dfaf7803483e 
>   src/master/master.cpp 2ef836536784d5fda23c80dc2304239b176a8942 
>   src/master/quota_handler.cpp 3ad28e4a9363a877d0610b529a6c17fb30ece37a 
>   src/master/registrar.cpp d7134eea34102ab7b24d2f0131363bdd9005cfd3 
>   src/master/weights_handler.cpp 4f6a4cd29d27b64cbc9ee6d8909f79812313b681 
> 
> Diff: https://reviews.apache.org/r/56813/diff/
> 
> 
> Testing
> -------
> 
> Testing details can be found at the end of this review chain.
> 
> 
> Thanks,
> 
> Greg Mann
> 
>


Re: Review Request 56813: Updated master handlers 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/56813/
-----------------------------------------------------------

(Updated Feb. 28, 2017, 6:56 a.m.)


Review request for mesos, Adam B, Alexander Rojas, Jan Schlicht, Till Toenshoff, and Vinod Kone.


Changes
-------

Changed 'AuthenticationContext' to 'Principal'.


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


Repository: mesos


Description (updated)
-------

This patch updates the HTTP endpoint handlers in the
master process to accept the `Principal` type instead
of an `Option<string>& principal`.


Diffs (updated)
-----

  src/master/http.cpp 6e5178eeea6cc6b90ae253840da22be13444b088 
  src/master/master.hpp 764adb18ddf09b62529c5c96f8e4dfaf7803483e 
  src/master/master.cpp 2ef836536784d5fda23c80dc2304239b176a8942 
  src/master/quota_handler.cpp 3ad28e4a9363a877d0610b529a6c17fb30ece37a 
  src/master/registrar.cpp d7134eea34102ab7b24d2f0131363bdd9005cfd3 
  src/master/weights_handler.cpp 4f6a4cd29d27b64cbc9ee6d8909f79812313b681 

Diff: https://reviews.apache.org/r/56813/diff/


Testing
-------

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


Thanks,

Greg Mann


Re: Review Request 56813: Updated master handlers 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/56813/
-----------------------------------------------------------

(Updated Feb. 24, 2017, 11:07 p.m.)


Review request for mesos, Adam B, Alexander Rojas, Jan Schlicht, Till Toenshoff, and Vinod Kone.


Changes
-------

Rebase.


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


Repository: mesos


Description
-------

This patch updates the HTTP endpoint handlers in the
master process to accept an `AuthenticationContext`
instead of an `Option<string>& principal`.


Diffs (updated)
-----

  src/master/http.cpp 6e5178eeea6cc6b90ae253840da22be13444b088 
  src/master/master.hpp 764adb18ddf09b62529c5c96f8e4dfaf7803483e 
  src/master/master.cpp 2ef836536784d5fda23c80dc2304239b176a8942 
  src/master/quota_handler.cpp 3ad28e4a9363a877d0610b529a6c17fb30ece37a 
  src/master/registrar.cpp d7134eea34102ab7b24d2f0131363bdd9005cfd3 
  src/master/weights_handler.cpp 4f6a4cd29d27b64cbc9ee6d8909f79812313b681 

Diff: https://reviews.apache.org/r/56813/diff/


Testing
-------

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


Thanks,

Greg Mann


Re: Review Request 56813: Updated master handlers 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/56813/
-----------------------------------------------------------

(Updated Feb. 24, 2017, 5:59 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 (updated)
-------

This patch updates the HTTP endpoint handlers in the
master process to accept an `AuthenticationContext`
instead of an `Option<string>& principal`.


Diffs
-----

  src/master/http.cpp e2fd71c5ae4178564b9a08756df5175aec5d6ca1 
  src/master/master.hpp 764adb18ddf09b62529c5c96f8e4dfaf7803483e 
  src/master/master.cpp 2ef836536784d5fda23c80dc2304239b176a8942 
  src/master/quota_handler.cpp 3ad28e4a9363a877d0610b529a6c17fb30ece37a 
  src/master/registrar.cpp d7134eea34102ab7b24d2f0131363bdd9005cfd3 
  src/master/weights_handler.cpp 4f6a4cd29d27b64cbc9ee6d8909f79812313b681 

Diff: https://reviews.apache.org/r/56813/diff/


Testing
-------

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


Thanks,

Greg Mann


Re: Review Request 56813: Updated master handlers 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/56813/
-----------------------------------------------------------

(Updated Feb. 22, 2017, 7:53 p.m.)


Review request for mesos, Adam B, Alexander Rojas, Jan Schlicht, Till Toenshoff, and Vinod Kone.


Changes
-------

Updated master's `scheduler` endpoint code.


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


Repository: mesos


Description
-------

This patch updates the HTTP endpoint handlers in the
agent process to accept an `AuthenticationContext`
instead of an `Option<string>& principal`.


Diffs (updated)
-----

  src/master/http.cpp e2fd71c5ae4178564b9a08756df5175aec5d6ca1 
  src/master/master.hpp 764adb18ddf09b62529c5c96f8e4dfaf7803483e 
  src/master/master.cpp 2ef836536784d5fda23c80dc2304239b176a8942 
  src/master/quota_handler.cpp 3ad28e4a9363a877d0610b529a6c17fb30ece37a 
  src/master/registrar.cpp d7134eea34102ab7b24d2f0131363bdd9005cfd3 
  src/master/weights_handler.cpp 4f6a4cd29d27b64cbc9ee6d8909f79812313b681 

Diff: https://reviews.apache.org/r/56813/diff/


Testing
-------

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


Thanks,

Greg Mann


Re: Review Request 56813: Updated master handlers 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/56813/
-----------------------------------------------------------

(Updated Feb. 22, 2017, 1:23 a.m.)


Review request for mesos, Adam B, Alexander Rojas, Jan Schlicht, Till Toenshoff, and Vinod Kone.


Changes
-------

Changed `context` to `authContext`.


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


Repository: mesos


Description
-------

This patch updates the HTTP endpoint handlers in the
agent process to accept an `AuthenticationContext`
instead of an `Option<string>& principal`.


Diffs (updated)
-----

  src/master/http.cpp e2fd71c5ae4178564b9a08756df5175aec5d6ca1 
  src/master/master.hpp 764adb18ddf09b62529c5c96f8e4dfaf7803483e 
  src/master/master.cpp 2ef836536784d5fda23c80dc2304239b176a8942 
  src/master/quota_handler.cpp 3ad28e4a9363a877d0610b529a6c17fb30ece37a 
  src/master/registrar.cpp d7134eea34102ab7b24d2f0131363bdd9005cfd3 
  src/master/weights_handler.cpp 4f6a4cd29d27b64cbc9ee6d8909f79812313b681 

Diff: https://reviews.apache.org/r/56813/diff/


Testing
-------

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


Thanks,

Greg Mann