Re: Review Request 42164: Allowed (un)reserve operations without a principal.

[Greg Mann]

> On Jan. 13, 2016, 5:22 a.m., Michael Park wrote:
> > src/master/validation.cpp, line 703
> > 
> >
> > 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.

[Guangya Liu]

> On 一月 13, 2016, 5:22 a.m., Michael Park wrote:
> > src/master/validation.cpp, line 703
> > 
> >
> > 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.

[Greg Mann]

> On Jan. 13, 2016, 5:22 a.m., Michael Park wrote:
> > src/master/validation.cpp, line 703
> > 
> >
> > 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.

[Guangya Liu]

> On 一月 13, 2016, 5:22 a.m., Michael Park wrote:
> > src/master/validation.cpp, line 703
> > 
> >
> > 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.

[Greg Mann]

> On Jan. 13, 2016, 5:22 a.m., Michael Park wrote:
> > src/master/validation.cpp, line 703
> > 
> >
> > 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.

[Michael Park]

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



src/master/validation.cpp (line 699)


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.

[Guangya Liu]

> On 一月 13, 2016, 2:56 a.m., Guangya Liu wrote:
> > src/tests/master_validation_tests.cpp, lines 368-381
> > 
> >
> > 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.

[Greg Mann]

> On Jan. 13, 2016, 2:56 a.m., Guangya Liu wrote:
> > src/tests/master_validation_tests.cpp, lines 368-381
> > 
> >
> > 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.

[Guangya Liu]

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



src/tests/master_validation_tests.cpp 


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.

[Greg Mann]

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

[Greg Mann]

> On Jan. 13, 2016, 2:17 a.m., Guangya Liu wrote:
> > src/tests/master_validation_tests.cpp, line 294
> > 
> >
> > 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.

[Guangya Liu]

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



src/tests/master_validation_tests.cpp 


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.

[Greg Mann]

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

[Greg Mann]

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

[Greg Mann]

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

[Greg Mann]

> On Jan. 12, 2016, 9:53 a.m., Alexander Rojas wrote:
> > src/master/validation.cpp, line 696
> > 
> >
> > 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.

[Alexander Rojas]

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



src/master/validation.cpp (line 692)


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.

[Greg Mann]

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

Review Request 42164: Allowed (un)reserve operations without a principal.

[Greg Mann]

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