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 2016/01/11 23:40:08 UTC
Review Request 42164: Allowed (un)reserve operations without a
principal.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/42164/
-----------------------------------------------------------
Review request for mesos, Alexander Rojas, Michael Park, and Neil Conway.
Bugs: MESOS-3940
https://issues.apache.org/jira/browse/MESOS-3940
Repository: mesos
Description
-------
Allowed (un)reserve operations without a principal.
Diffs
-----
3rdparty/libprocess/3rdparty/stout/include/stout/duration.hpp 40b22604ec4eb01f44a36624bababb9b59d16bdb
Makefile.am fbd4e5a6356e90c867ba47c48c86fc9161ddd98e
docs/getting-started.md 0982d9038d87cec6eaa72b5584e0877fc2b9f04c
include/mesos/mesos.proto 74e9d00d6826adfb7fd2433c3deced6d2ca51e98
src/CMakeLists.txt 39a23df3227a4f524ea0d408dc894fa5bbab7d10
src/Makefile.am 8cbfb1ba5fa49f2d3cc26ea325838a1c68a79660
src/master/validation.cpp 6a43bce5b7df6a9d939245c4726d060fa19eb305
src/tests/containerizer/port_mapping_tests.cpp e3aea53468fa00374320a8b89bdbb64f38e44b01
src/tests/environment.cpp 20218a086baefcefb310eb45ed9024e5425ce787
src/tests/master_validation_tests.cpp fbf8fadbc04a7cbc60ee6091e0224339389b400f
src/tests/reservation_tests.cpp 61d275c47fbb02baf22e4ad8f9b1580886da51d9
src/tests/role_tests.cpp 373ae267b85588fe491ab0a0ce8aa195f971aac3
src/uri/fetcher.hpp 5bc9703b76457413b5841710e0313f8bfa9047a1
src/uri/fetcher.cpp dfda732348fec3b686cf82b55ad94fda4829469b
src/uri/fetchers/curl.cpp 269df874f3a3a65d045a0822af57ba65e23a9fe0
src/uri/fetchers/docker.hpp c855a2b55a07bb398f7547b44a85b8ba2d2b2ec3
src/uri/fetchers/docker.cpp 3595f915cb4b305659fa803e741333be5115aecf
support/docker_build.sh da94be87fae98825e60331634259eab0e7a4ebd1
Diff: https://reviews.apache.org/r/42164/diff/
Testing
-------
Two master validation tests were removed which tested to make sure that reserve and unreserve operations would not succeed without a principal.
`make check` was used to test on both OSX and Ubuntu 14.04
Thanks,
Greg Mann
Re: Review Request 42164: Allowed (un)reserve operations without a
principal.
Posted by Greg Mann <gr...@mesosphere.io>.
> On Jan. 12, 2016, 9:53 a.m., Alexander Rojas wrote:
> > src/master/validation.cpp, line 696
> > <https://reviews.apache.org/r/42164/diff/2/?file=1192809#file1192809line696>
> >
> > This line should be executed only if the principal is some. The error reported by _gyliu_ in the follow up patch happens in this line.
:facepalm:
Thanks Alex; I fixed this error and added a test case that performs these operations without a principal. It caused a segfault before the fix, and completes successfully after the fix.
- Greg
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/42164/#review113965
-----------------------------------------------------------
On Jan. 12, 2016, 4:43 p.m., Greg Mann wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42164/
> -----------------------------------------------------------
>
> (Updated Jan. 12, 2016, 4:43 p.m.)
>
>
> Review request for mesos, Alexander Rojas, Michael Park, and Neil Conway.
>
>
> Bugs: MESOS-3940
> https://issues.apache.org/jira/browse/MESOS-3940
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Allowed (un)reserve operations without a principal.
>
>
> Diffs
> -----
>
> src/master/validation.cpp 6a43bce5b7df6a9d939245c4726d060fa19eb305
> src/tests/master_validation_tests.cpp fbf8fadbc04a7cbc60ee6091e0224339389b400f
> src/tests/reservation_endpoints_tests.cpp b8edd6fafedd4c2221a8d19c1ebc71254071a8c7
>
> Diff: https://reviews.apache.org/r/42164/diff/
>
>
> Testing
> -------
>
> Two master validation tests were removed which tested to make sure that reserve and unreserve operations would not succeed without a principal.
>
> `make check` was used to test on both OSX and Ubuntu 14.04
>
>
> Thanks,
>
> Greg Mann
>
>
Re: Review Request 42164: Allowed (un)reserve operations without a
principal.
Posted by Alexander Rojas <al...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/42164/#review113965
-----------------------------------------------------------
src/master/validation.cpp (line 692)
<https://reviews.apache.org/r/42164/#comment174749>
This line should be executed only if the principal is some. The error reported by _gyliu_ in the follow up patch happens in this line.
- Alexander Rojas
On Jan. 12, 2016, 1:52 a.m., Greg Mann wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42164/
> -----------------------------------------------------------
>
> (Updated Jan. 12, 2016, 1:52 a.m.)
>
>
> Review request for mesos, Alexander Rojas, Michael Park, and Neil Conway.
>
>
> Bugs: MESOS-3940
> https://issues.apache.org/jira/browse/MESOS-3940
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Allowed (un)reserve operations without a principal.
>
>
> Diffs
> -----
>
> src/master/validation.cpp 6a43bce5b7df6a9d939245c4726d060fa19eb305
> src/tests/master_validation_tests.cpp fbf8fadbc04a7cbc60ee6091e0224339389b400f
>
> Diff: https://reviews.apache.org/r/42164/diff/
>
>
> Testing
> -------
>
> Two master validation tests were removed which tested to make sure that reserve and unreserve operations would not succeed without a principal.
>
> `make check` was used to test on both OSX and Ubuntu 14.04
>
>
> Thanks,
>
> Greg Mann
>
>
Re: Review Request 42164: Allowed (un)reserve operations without a
principal.
Posted by Greg Mann <gr...@mesosphere.io>.
> On Jan. 13, 2016, 2:56 a.m., Guangya Liu wrote:
> > src/tests/master_validation_tests.cpp, lines 368-381
> > <https://reviews.apache.org/r/42164/diff/6/?file=1195184#file1195184line368>
> >
> > Sorry, I should have propsed my comments in one review. Seems we need validate both reserve and unreserve without a principal.
That's OK, I thought about including such a test for the unreserve operation and I decided it wasn't worth it since we're not actually testing anything about the principal in the unreserve validation function. In the subsequent patch, I remove the `hasPrincipal` parameter, and we don't check anything about the principal in the resource's `ReservationInfo`, so it wasn't clear to me what exactly we would be verifying with such a test.
What do you think?
- Greg
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/42164/#review114140
-----------------------------------------------------------
On Jan. 13, 2016, 2:45 a.m., Greg Mann wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42164/
> -----------------------------------------------------------
>
> (Updated Jan. 13, 2016, 2:45 a.m.)
>
>
> Review request for mesos, Alexander Rojas, Michael Park, and Neil Conway.
>
>
> Bugs: MESOS-3940
> https://issues.apache.org/jira/browse/MESOS-3940
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Allowed (un)reserve operations without a principal.
>
>
> Diffs
> -----
>
> src/master/validation.cpp 6a43bce5b7df6a9d939245c4726d060fa19eb305
> src/tests/master_validation_tests.cpp fbf8fadbc04a7cbc60ee6091e0224339389b400f
> src/tests/reservation_endpoints_tests.cpp b8edd6fafedd4c2221a8d19c1ebc71254071a8c7
>
> Diff: https://reviews.apache.org/r/42164/diff/
>
>
> Testing
> -------
>
> Two master validation tests were removed which tested to make sure that reserve and unreserve operations would not succeed without a principal.
>
> `make check` was used to test on both OSX and Ubuntu 14.04
>
>
> Thanks,
>
> Greg Mann
>
>
Re: Review Request 42164: Allowed (un)reserve operations without a
principal.
Posted by Guangya Liu <gy...@gmail.com>.
> On 一月 13, 2016, 2:56 a.m., Guangya Liu wrote:
> > src/tests/master_validation_tests.cpp, lines 368-381
> > <https://reviews.apache.org/r/42164/diff/6/?file=1195184#file1195184line368>
> >
> > Sorry, I should have propsed my comments in one review. Seems we need validate both reserve and unreserve without a principal.
>
> Greg Mann wrote:
> That's OK, I thought about including such a test for the unreserve operation and I decided it wasn't worth it since we're not actually testing anything about the principal in the unreserve validation function. In the subsequent patch, I remove the `hasPrincipal` parameter, and we don't check anything about the principal in the resource's `ReservationInfo`, so it wasn't clear to me what exactly we would be verifying with such a test.
>
> What do you think?
Got it, agree that we can ignore this.
- Guangya
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/42164/#review114140
-----------------------------------------------------------
On 一月 13, 2016, 2:45 a.m., Greg Mann wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42164/
> -----------------------------------------------------------
>
> (Updated 一月 13, 2016, 2:45 a.m.)
>
>
> Review request for mesos, Alexander Rojas, Michael Park, and Neil Conway.
>
>
> Bugs: MESOS-3940
> https://issues.apache.org/jira/browse/MESOS-3940
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Allowed (un)reserve operations without a principal.
>
>
> Diffs
> -----
>
> src/master/validation.cpp 6a43bce5b7df6a9d939245c4726d060fa19eb305
> src/tests/master_validation_tests.cpp fbf8fadbc04a7cbc60ee6091e0224339389b400f
> src/tests/reservation_endpoints_tests.cpp b8edd6fafedd4c2221a8d19c1ebc71254071a8c7
>
> Diff: https://reviews.apache.org/r/42164/diff/
>
>
> Testing
> -------
>
> Two master validation tests were removed which tested to make sure that reserve and unreserve operations would not succeed without a principal.
>
> `make check` was used to test on both OSX and Ubuntu 14.04
>
>
> Thanks,
>
> Greg Mann
>
>
Re: Review Request 42164: Allowed (un)reserve operations without a
principal.
Posted by Guangya Liu <gy...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/42164/#review114140
-----------------------------------------------------------
src/tests/master_validation_tests.cpp
<https://reviews.apache.org/r/42164/#comment174959>
Sorry, I should have propsed my comments in one review. Seems we need validate both reserve and unreserve without a principal.
- Guangya Liu
On 一月 13, 2016, 2:45 a.m., Greg Mann wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42164/
> -----------------------------------------------------------
>
> (Updated 一月 13, 2016, 2:45 a.m.)
>
>
> Review request for mesos, Alexander Rojas, Michael Park, and Neil Conway.
>
>
> Bugs: MESOS-3940
> https://issues.apache.org/jira/browse/MESOS-3940
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Allowed (un)reserve operations without a principal.
>
>
> Diffs
> -----
>
> src/master/validation.cpp 6a43bce5b7df6a9d939245c4726d060fa19eb305
> src/tests/master_validation_tests.cpp fbf8fadbc04a7cbc60ee6091e0224339389b400f
> src/tests/reservation_endpoints_tests.cpp b8edd6fafedd4c2221a8d19c1ebc71254071a8c7
>
> Diff: https://reviews.apache.org/r/42164/diff/
>
>
> Testing
> -------
>
> Two master validation tests were removed which tested to make sure that reserve and unreserve operations would not succeed without a principal.
>
> `make check` was used to test on both OSX and Ubuntu 14.04
>
>
> Thanks,
>
> Greg Mann
>
>
Re: Review Request 42164: Allowed (un)reserve operations without a
principal.
Posted by Guangya Liu <gy...@gmail.com>.
> On 一月 13, 2016, 5:22 a.m., Michael Park wrote:
> > src/master/validation.cpp, line 703
> > <https://reviews.apache.org/r/42164/diff/5/?file=1194762#file1194762line703>
> >
> > First thing here is that the `principal` field in `ReservationInfo` is still marked `required`. What's our plan for changing it to `optional`?
> >
> > Second thing is that even if `principal.isNone()`, if is authorization is turned off we don't care, right? That is, if authorization is turned off, even frameworks without a principal can reserve any resources?
>
> Greg Mann wrote:
> Regarding your first question: per our discussion, we do indeed need a good plan for migrating to an `optional` principal within `ReservationInfo`. It would seem that we have to choose between two less-than-ideal situations: either we tolerate HTTP endpoints that don't work when authentication is disabled, or we enforce that frameworks cannot register with `principal == ""`, so that we can use the empty string to represent a null principal while we migrate to an `optional` principal over a deprecation cycle.
>
> To your second point: again per our discussion, semantically the principal within `ReservationInfo` represents the principal (or lack thereof) which reserved the resources, so we should enforce that it matches the principal of the framework/operator, even if authorization is not enabled.
If authorization is not enabled, then it is not a must to set principal for a framework, so how can we enforce the match of principal?
- Guangya
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/42164/#review114149
-----------------------------------------------------------
On 一月 13, 2016, 2:45 a.m., Greg Mann wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42164/
> -----------------------------------------------------------
>
> (Updated 一月 13, 2016, 2:45 a.m.)
>
>
> Review request for mesos, Alexander Rojas, Michael Park, and Neil Conway.
>
>
> Bugs: MESOS-3940
> https://issues.apache.org/jira/browse/MESOS-3940
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Allowed (un)reserve operations without a principal.
>
>
> Diffs
> -----
>
> src/master/validation.cpp 6a43bce5b7df6a9d939245c4726d060fa19eb305
> src/tests/master_validation_tests.cpp fbf8fadbc04a7cbc60ee6091e0224339389b400f
> src/tests/reservation_endpoints_tests.cpp b8edd6fafedd4c2221a8d19c1ebc71254071a8c7
>
> Diff: https://reviews.apache.org/r/42164/diff/
>
>
> Testing
> -------
>
> Two master validation tests were removed which tested to make sure that reserve and unreserve operations would not succeed without a principal.
>
> `make check` was used to test on both OSX and Ubuntu 14.04
>
>
> Thanks,
>
> Greg Mann
>
>
Re: Review Request 42164: Allowed (un)reserve operations without a
principal.
Posted by Greg Mann <gr...@mesosphere.io>.
> On Jan. 13, 2016, 5:22 a.m., Michael Park wrote:
> > src/master/validation.cpp, line 703
> > <https://reviews.apache.org/r/42164/diff/5/?file=1194762#file1194762line703>
> >
> > First thing here is that the `principal` field in `ReservationInfo` is still marked `required`. What's our plan for changing it to `optional`?
> >
> > Second thing is that even if `principal.isNone()`, if is authorization is turned off we don't care, right? That is, if authorization is turned off, even frameworks without a principal can reserve any resources?
>
> Greg Mann wrote:
> Regarding your first question: per our discussion, we do indeed need a good plan for migrating to an `optional` principal within `ReservationInfo`. It would seem that we have to choose between two less-than-ideal situations: either we tolerate HTTP endpoints that don't work when authentication is disabled, or we enforce that frameworks cannot register with `principal == ""`, so that we can use the empty string to represent a null principal while we migrate to an `optional` principal over a deprecation cycle.
>
> To your second point: again per our discussion, semantically the principal within `ReservationInfo` represents the principal (or lack thereof) which reserved the resources, so we should enforce that it matches the principal of the framework/operator, even if authorization is not enabled.
>
> Guangya Liu wrote:
> If authorization is not enabled, then it is not a must to set principal for a framework, so how can we enforce the match of principal?
We should enforce that the principal (or lack of a principal) used for authorization is the same as the principal in the `ReservationInfo`. So if no principal is provided (i.e., authorization disabled), then there should be no principal in `ReservationInfo`. Since the principal in `ReservationInfo` is currently a required field, we would need to use something like the empty string `""` to represent the case of no principal. Ideally, the principal field in `ReservationInfo` should be an optional field so that it can be omitted when no principal is used. The plan is to change this field to optional, but we're debating how and when it will be best to do so. Any thoughts on how best to do this?
- Greg
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/42164/#review114149
-----------------------------------------------------------
On Jan. 13, 2016, 2:45 a.m., Greg Mann wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42164/
> -----------------------------------------------------------
>
> (Updated Jan. 13, 2016, 2:45 a.m.)
>
>
> Review request for mesos, Alexander Rojas, Michael Park, and Neil Conway.
>
>
> Bugs: MESOS-3940
> https://issues.apache.org/jira/browse/MESOS-3940
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Allowed (un)reserve operations without a principal.
>
>
> Diffs
> -----
>
> src/master/validation.cpp 6a43bce5b7df6a9d939245c4726d060fa19eb305
> src/tests/master_validation_tests.cpp fbf8fadbc04a7cbc60ee6091e0224339389b400f
> src/tests/reservation_endpoints_tests.cpp b8edd6fafedd4c2221a8d19c1ebc71254071a8c7
>
> Diff: https://reviews.apache.org/r/42164/diff/
>
>
> Testing
> -------
>
> Two master validation tests were removed which tested to make sure that reserve and unreserve operations would not succeed without a principal.
>
> `make check` was used to test on both OSX and Ubuntu 14.04
>
>
> Thanks,
>
> Greg Mann
>
>
Re: Review Request 42164: Allowed (un)reserve operations without a
principal.
Posted by Greg Mann <gr...@mesosphere.io>.
> On Jan. 13, 2016, 5:22 a.m., Michael Park wrote:
> > src/master/validation.cpp, line 703
> > <https://reviews.apache.org/r/42164/diff/5/?file=1194762#file1194762line703>
> >
> > First thing here is that the `principal` field in `ReservationInfo` is still marked `required`. What's our plan for changing it to `optional`?
> >
> > Second thing is that even if `principal.isNone()`, if is authorization is turned off we don't care, right? That is, if authorization is turned off, even frameworks without a principal can reserve any resources?
Regarding your first question: per our discussion, we do indeed need a good plan for migrating to an `optional` principal within `ReservationInfo`. It would seem that we have to choose between two less-than-ideal situations: either we tolerate HTTP endpoints that don't work when authentication is disabled, or we enforce that frameworks cannot register with `principal == ""`, so that we can use the empty string to represent a null principal while we migrate to an `optional` principal over a deprecation cycle.
To your second point: again per our discussion, semantically the principal within `ReservationInfo` represents the principal (or lack thereof) which reserved the resources, so we should enforce that it matches the principal of the framework/operator, even if authorization is not enabled.
- Greg
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/42164/#review114149
-----------------------------------------------------------
On Jan. 13, 2016, 2:45 a.m., Greg Mann wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42164/
> -----------------------------------------------------------
>
> (Updated Jan. 13, 2016, 2:45 a.m.)
>
>
> Review request for mesos, Alexander Rojas, Michael Park, and Neil Conway.
>
>
> Bugs: MESOS-3940
> https://issues.apache.org/jira/browse/MESOS-3940
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Allowed (un)reserve operations without a principal.
>
>
> Diffs
> -----
>
> src/master/validation.cpp 6a43bce5b7df6a9d939245c4726d060fa19eb305
> src/tests/master_validation_tests.cpp fbf8fadbc04a7cbc60ee6091e0224339389b400f
> src/tests/reservation_endpoints_tests.cpp b8edd6fafedd4c2221a8d19c1ebc71254071a8c7
>
> Diff: https://reviews.apache.org/r/42164/diff/
>
>
> Testing
> -------
>
> Two master validation tests were removed which tested to make sure that reserve and unreserve operations would not succeed without a principal.
>
> `make check` was used to test on both OSX and Ubuntu 14.04
>
>
> Thanks,
>
> Greg Mann
>
>
Re: Review Request 42164: Allowed (un)reserve operations without a
principal.
Posted by Guangya Liu <gy...@gmail.com>.
> On 一月 13, 2016, 5:22 a.m., Michael Park wrote:
> > src/master/validation.cpp, line 703
> > <https://reviews.apache.org/r/42164/diff/5/?file=1194762#file1194762line703>
> >
> > First thing here is that the `principal` field in `ReservationInfo` is still marked `required`. What's our plan for changing it to `optional`?
> >
> > Second thing is that even if `principal.isNone()`, if is authorization is turned off we don't care, right? That is, if authorization is turned off, even frameworks without a principal can reserve any resources?
>
> Greg Mann wrote:
> Regarding your first question: per our discussion, we do indeed need a good plan for migrating to an `optional` principal within `ReservationInfo`. It would seem that we have to choose between two less-than-ideal situations: either we tolerate HTTP endpoints that don't work when authentication is disabled, or we enforce that frameworks cannot register with `principal == ""`, so that we can use the empty string to represent a null principal while we migrate to an `optional` principal over a deprecation cycle.
>
> To your second point: again per our discussion, semantically the principal within `ReservationInfo` represents the principal (or lack thereof) which reserved the resources, so we should enforce that it matches the principal of the framework/operator, even if authorization is not enabled.
>
> Guangya Liu wrote:
> If authorization is not enabled, then it is not a must to set principal for a framework, so how can we enforce the match of principal?
>
> Greg Mann wrote:
> We should enforce that the principal (or lack of a principal) used for authorization is the same as the principal in the `ReservationInfo`. So if no principal is provided (i.e., authorization disabled), then there should be no principal in `ReservationInfo`. Since the principal in `ReservationInfo` is currently a required field, we would need to use something like the empty string `""` to represent the case of no principal. Ideally, the principal field in `ReservationInfo` should be an optional field so that it can be omitted when no principal is used. The plan is to change this field to optional, but we're debating how and when it will be best to do so. Any thoughts on how best to do this?
Compared with `HTTP endpoints that don't work when authentication is disabled`, I think that `enforce that frameworks cannot register with principal == ""` might be better as it will only impact some framework logic and more simple, but authorization require the whole cluster and all frameworks must enable authorization.
- Guangya
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/42164/#review114149
-----------------------------------------------------------
On 一月 13, 2016, 2:45 a.m., Greg Mann wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42164/
> -----------------------------------------------------------
>
> (Updated 一月 13, 2016, 2:45 a.m.)
>
>
> Review request for mesos, Alexander Rojas, Michael Park, and Neil Conway.
>
>
> Bugs: MESOS-3940
> https://issues.apache.org/jira/browse/MESOS-3940
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Allowed (un)reserve operations without a principal.
>
>
> Diffs
> -----
>
> src/master/validation.cpp 6a43bce5b7df6a9d939245c4726d060fa19eb305
> src/tests/master_validation_tests.cpp fbf8fadbc04a7cbc60ee6091e0224339389b400f
> src/tests/reservation_endpoints_tests.cpp b8edd6fafedd4c2221a8d19c1ebc71254071a8c7
>
> Diff: https://reviews.apache.org/r/42164/diff/
>
>
> Testing
> -------
>
> Two master validation tests were removed which tested to make sure that reserve and unreserve operations would not succeed without a principal.
>
> `make check` was used to test on both OSX and Ubuntu 14.04
>
>
> Thanks,
>
> Greg Mann
>
>
Re: Review Request 42164: Allowed (un)reserve operations without a
principal.
Posted by Greg Mann <gr...@mesosphere.io>.
> On Jan. 13, 2016, 5:22 a.m., Michael Park wrote:
> > src/master/validation.cpp, line 703
> > <https://reviews.apache.org/r/42164/diff/5/?file=1194762#file1194762line703>
> >
> > First thing here is that the `principal` field in `ReservationInfo` is still marked `required`. What's our plan for changing it to `optional`?
> >
> > Second thing is that even if `principal.isNone()`, if is authorization is turned off we don't care, right? That is, if authorization is turned off, even frameworks without a principal can reserve any resources?
>
> Greg Mann wrote:
> Regarding your first question: per our discussion, we do indeed need a good plan for migrating to an `optional` principal within `ReservationInfo`. It would seem that we have to choose between two less-than-ideal situations: either we tolerate HTTP endpoints that don't work when authentication is disabled, or we enforce that frameworks cannot register with `principal == ""`, so that we can use the empty string to represent a null principal while we migrate to an `optional` principal over a deprecation cycle.
>
> To your second point: again per our discussion, semantically the principal within `ReservationInfo` represents the principal (or lack thereof) which reserved the resources, so we should enforce that it matches the principal of the framework/operator, even if authorization is not enabled.
>
> Guangya Liu wrote:
> If authorization is not enabled, then it is not a must to set principal for a framework, so how can we enforce the match of principal?
>
> Greg Mann wrote:
> We should enforce that the principal (or lack of a principal) used for authorization is the same as the principal in the `ReservationInfo`. So if no principal is provided (i.e., authorization disabled), then there should be no principal in `ReservationInfo`. Since the principal in `ReservationInfo` is currently a required field, we would need to use something like the empty string `""` to represent the case of no principal. Ideally, the principal field in `ReservationInfo` should be an optional field so that it can be omitted when no principal is used. The plan is to change this field to optional, but we're debating how and when it will be best to do so. Any thoughts on how best to do this?
>
> Guangya Liu wrote:
> Compared with `HTTP endpoints that don't work when authentication is disabled`, I think that `enforce that frameworks cannot register with principal == ""` might be better as it will only impact some framework logic and more simple, but authorization require the whole cluster and all frameworks must enable authorization.
So after some discussions it sounds like we're going to wait on implementing this patch until we have migrated the `principal` field in `ReservationInfo` to `optional`. So for the time being, HTTP authentication will be required in order to use the `/reserve` and `/unreserve` endpoints. There are several options, none of which are ideal, and this seems to be the cleanest solution overall.
- Greg
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/42164/#review114149
-----------------------------------------------------------
On Jan. 13, 2016, 2:45 a.m., Greg Mann wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42164/
> -----------------------------------------------------------
>
> (Updated Jan. 13, 2016, 2:45 a.m.)
>
>
> Review request for mesos, Alexander Rojas, Michael Park, and Neil Conway.
>
>
> Bugs: MESOS-3940
> https://issues.apache.org/jira/browse/MESOS-3940
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Allowed (un)reserve operations without a principal.
>
>
> Diffs
> -----
>
> src/master/validation.cpp 6a43bce5b7df6a9d939245c4726d060fa19eb305
> src/tests/master_validation_tests.cpp fbf8fadbc04a7cbc60ee6091e0224339389b400f
> src/tests/reservation_endpoints_tests.cpp b8edd6fafedd4c2221a8d19c1ebc71254071a8c7
>
> Diff: https://reviews.apache.org/r/42164/diff/
>
>
> Testing
> -------
>
> Two master validation tests were removed which tested to make sure that reserve and unreserve operations would not succeed without a principal.
>
> `make check` was used to test on both OSX and Ubuntu 14.04
>
>
> Thanks,
>
> Greg Mann
>
>
Re: Review Request 42164: Allowed (un)reserve operations without a
principal.
Posted by Michael Park <mp...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/42164/#review114149
-----------------------------------------------------------
src/master/validation.cpp (line 699)
<https://reviews.apache.org/r/42164/#comment174965>
First thing here is that the `principal` field in `ReservationInfo` is still marked `required`. What's our plan for changing it to `optional`?
Second thing is that even if `principal.isNone()`, if is authorization is turned off we don't care, right? That is, if authorization is turned off, even frameworks without a principal can reserve any resources?
- Michael Park
On Jan. 13, 2016, 2:45 a.m., Greg Mann wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42164/
> -----------------------------------------------------------
>
> (Updated Jan. 13, 2016, 2:45 a.m.)
>
>
> Review request for mesos, Alexander Rojas, Michael Park, and Neil Conway.
>
>
> Bugs: MESOS-3940
> https://issues.apache.org/jira/browse/MESOS-3940
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Allowed (un)reserve operations without a principal.
>
>
> Diffs
> -----
>
> src/master/validation.cpp 6a43bce5b7df6a9d939245c4726d060fa19eb305
> src/tests/master_validation_tests.cpp fbf8fadbc04a7cbc60ee6091e0224339389b400f
> src/tests/reservation_endpoints_tests.cpp b8edd6fafedd4c2221a8d19c1ebc71254071a8c7
>
> Diff: https://reviews.apache.org/r/42164/diff/
>
>
> Testing
> -------
>
> Two master validation tests were removed which tested to make sure that reserve and unreserve operations would not succeed without a principal.
>
> `make check` was used to test on both OSX and Ubuntu 14.04
>
>
> Thanks,
>
> Greg Mann
>
>
Re: Review Request 42164: Allowed (un)reserve operations without a
principal.
Posted by Greg Mann <gr...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/42164/
-----------------------------------------------------------
(Updated Jan. 13, 2016, 2:45 a.m.)
Review request for mesos, Alexander Rojas, Michael Park, and Neil Conway.
Changes
-------
Addressed comment.
Bugs: MESOS-3940
https://issues.apache.org/jira/browse/MESOS-3940
Repository: mesos
Description
-------
Allowed (un)reserve operations without a principal.
Diffs (updated)
-----
src/master/validation.cpp 6a43bce5b7df6a9d939245c4726d060fa19eb305
src/tests/master_validation_tests.cpp fbf8fadbc04a7cbc60ee6091e0224339389b400f
src/tests/reservation_endpoints_tests.cpp b8edd6fafedd4c2221a8d19c1ebc71254071a8c7
Diff: https://reviews.apache.org/r/42164/diff/
Testing
-------
Two master validation tests were removed which tested to make sure that reserve and unreserve operations would not succeed without a principal.
`make check` was used to test on both OSX and Ubuntu 14.04
Thanks,
Greg Mann
Re: Review Request 42164: Allowed (un)reserve operations without a
principal.
Posted by Greg Mann <gr...@mesosphere.io>.
> On Jan. 13, 2016, 2:17 a.m., Guangya Liu wrote:
> > src/tests/master_validation_tests.cpp, line 294
> > <https://reviews.apache.org/r/42164/diff/5/?file=1194763#file1194763line294>
> >
> > Can you update the test case to verify that those validation still passed even without a principal?
Great idea, thanks!
- Greg
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/42164/#review114136
-----------------------------------------------------------
On Jan. 13, 2016, 2:45 a.m., Greg Mann wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42164/
> -----------------------------------------------------------
>
> (Updated Jan. 13, 2016, 2:45 a.m.)
>
>
> Review request for mesos, Alexander Rojas, Michael Park, and Neil Conway.
>
>
> Bugs: MESOS-3940
> https://issues.apache.org/jira/browse/MESOS-3940
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Allowed (un)reserve operations without a principal.
>
>
> Diffs
> -----
>
> src/master/validation.cpp 6a43bce5b7df6a9d939245c4726d060fa19eb305
> src/tests/master_validation_tests.cpp fbf8fadbc04a7cbc60ee6091e0224339389b400f
> src/tests/reservation_endpoints_tests.cpp b8edd6fafedd4c2221a8d19c1ebc71254071a8c7
>
> Diff: https://reviews.apache.org/r/42164/diff/
>
>
> Testing
> -------
>
> Two master validation tests were removed which tested to make sure that reserve and unreserve operations would not succeed without a principal.
>
> `make check` was used to test on both OSX and Ubuntu 14.04
>
>
> Thanks,
>
> Greg Mann
>
>
Re: Review Request 42164: Allowed (un)reserve operations without a
principal.
Posted by Guangya Liu <gy...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/42164/#review114136
-----------------------------------------------------------
src/tests/master_validation_tests.cpp
<https://reviews.apache.org/r/42164/#comment174947>
Can you update the test case to verify that those validation still passed even without a principal?
- Guangya Liu
On 一月 12, 2016, 6:53 p.m., Greg Mann wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42164/
> -----------------------------------------------------------
>
> (Updated 一月 12, 2016, 6:53 p.m.)
>
>
> Review request for mesos, Alexander Rojas, Michael Park, and Neil Conway.
>
>
> Bugs: MESOS-3940
> https://issues.apache.org/jira/browse/MESOS-3940
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Allowed (un)reserve operations without a principal.
>
>
> Diffs
> -----
>
> src/master/validation.cpp 6a43bce5b7df6a9d939245c4726d060fa19eb305
> src/tests/master_validation_tests.cpp fbf8fadbc04a7cbc60ee6091e0224339389b400f
> src/tests/reservation_endpoints_tests.cpp b8edd6fafedd4c2221a8d19c1ebc71254071a8c7
>
> Diff: https://reviews.apache.org/r/42164/diff/
>
>
> Testing
> -------
>
> Two master validation tests were removed which tested to make sure that reserve and unreserve operations would not succeed without a principal.
>
> `make check` was used to test on both OSX and Ubuntu 14.04
>
>
> Thanks,
>
> Greg Mann
>
>
Re: Review Request 42164: Allowed (un)reserve operations without a
principal.
Posted by Greg Mann <gr...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/42164/
-----------------------------------------------------------
(Updated Jan. 12, 2016, 6:53 p.m.)
Review request for mesos, Alexander Rojas, Michael Park, and Neil Conway.
Changes
-------
Improved conditional.
Bugs: MESOS-3940
https://issues.apache.org/jira/browse/MESOS-3940
Repository: mesos
Description
-------
Allowed (un)reserve operations without a principal.
Diffs (updated)
-----
src/master/validation.cpp 6a43bce5b7df6a9d939245c4726d060fa19eb305
src/tests/master_validation_tests.cpp fbf8fadbc04a7cbc60ee6091e0224339389b400f
src/tests/reservation_endpoints_tests.cpp b8edd6fafedd4c2221a8d19c1ebc71254071a8c7
Diff: https://reviews.apache.org/r/42164/diff/
Testing
-------
Two master validation tests were removed which tested to make sure that reserve and unreserve operations would not succeed without a principal.
`make check` was used to test on both OSX and Ubuntu 14.04
Thanks,
Greg Mann
Re: Review Request 42164: Allowed (un)reserve operations without a
principal.
Posted by Greg Mann <gr...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/42164/
-----------------------------------------------------------
(Updated Jan. 12, 2016, 6 p.m.)
Review request for mesos, Alexander Rojas, Michael Park, and Neil Conway.
Changes
-------
Enforce an empty principal in `ReservationInfo` when no authentication/authorization is performed.
Bugs: MESOS-3940
https://issues.apache.org/jira/browse/MESOS-3940
Repository: mesos
Description
-------
Allowed (un)reserve operations without a principal.
Diffs (updated)
-----
src/master/validation.cpp 6a43bce5b7df6a9d939245c4726d060fa19eb305
src/tests/master_validation_tests.cpp fbf8fadbc04a7cbc60ee6091e0224339389b400f
src/tests/reservation_endpoints_tests.cpp b8edd6fafedd4c2221a8d19c1ebc71254071a8c7
Diff: https://reviews.apache.org/r/42164/diff/
Testing
-------
Two master validation tests were removed which tested to make sure that reserve and unreserve operations would not succeed without a principal.
`make check` was used to test on both OSX and Ubuntu 14.04
Thanks,
Greg Mann
Re: Review Request 42164: Allowed (un)reserve operations without a
principal.
Posted by Greg Mann <gr...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/42164/
-----------------------------------------------------------
(Updated Jan. 12, 2016, 4:43 p.m.)
Review request for mesos, Alexander Rojas, Michael Park, and Neil Conway.
Changes
-------
Addressed comment, fixed segfault in the absence of a principal.
Bugs: MESOS-3940
https://issues.apache.org/jira/browse/MESOS-3940
Repository: mesos
Description
-------
Allowed (un)reserve operations without a principal.
Diffs (updated)
-----
src/master/validation.cpp 6a43bce5b7df6a9d939245c4726d060fa19eb305
src/tests/master_validation_tests.cpp fbf8fadbc04a7cbc60ee6091e0224339389b400f
src/tests/reservation_endpoints_tests.cpp b8edd6fafedd4c2221a8d19c1ebc71254071a8c7
Diff: https://reviews.apache.org/r/42164/diff/
Testing
-------
Two master validation tests were removed which tested to make sure that reserve and unreserve operations would not succeed without a principal.
`make check` was used to test on both OSX and Ubuntu 14.04
Thanks,
Greg Mann
Re: Review Request 42164: Allowed (un)reserve operations without a
principal.
Posted by Greg Mann <gr...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/42164/
-----------------------------------------------------------
(Updated Jan. 12, 2016, 12:52 a.m.)
Review request for mesos, Alexander Rojas, Michael Park, and Neil Conway.
Changes
-------
Rebase, remove spurious changes from diff.
Bugs: MESOS-3940
https://issues.apache.org/jira/browse/MESOS-3940
Repository: mesos
Description
-------
Allowed (un)reserve operations without a principal.
Diffs (updated)
-----
src/master/validation.cpp 6a43bce5b7df6a9d939245c4726d060fa19eb305
src/tests/master_validation_tests.cpp fbf8fadbc04a7cbc60ee6091e0224339389b400f
Diff: https://reviews.apache.org/r/42164/diff/
Testing
-------
Two master validation tests were removed which tested to make sure that reserve and unreserve operations would not succeed without a principal.
`make check` was used to test on both OSX and Ubuntu 14.04
Thanks,
Greg Mann