Re: Review Request 39988: [4/5] Added authorization for dynamic reservation master endpoints.

[Greg Mann]

> On Dec. 2, 2015, 3:05 p.m., Alexander Rukletsov wrote:
> > src/master/http.cpp, lines 1035-1038
> > 
> >
> > Let's leave a comment here, that `principal` matches 
> > `reservation().principal()` for each resource in 
> > `operation.reserve().resources()`, hence it's OK to authorize for 
> > `principal` and use `reservation().principal()` in `unreserve()`. Maybe a 
> > symmetrical comment in `unreserve()` path would also make sense.
> > 
> > Maybe if you validate before authorizing it will be more easy to 
> > understand?
> 
> Jie Yu wrote:
> +1 on validate before authorizing. That could save us a big comment here.

I changed the order of validation with respect to authorization; hopefully that 
does enough to improve readability. Let me know if you think we would benefit 
from an additional comment when we validate, we can add one if necessary.


- Greg


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


On Dec. 2, 2015, 10:47 p.m., Greg Mann wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39988/
> ---
> 
> (Updated Dec. 2, 2015, 10:47 p.m.)
> 
> 
> Review request for mesos, Adam B, Jie Yu, Michael Park, and Till Toenshoff.
> 
> 
> Bugs: MESOS-3062
> https://issues.apache.org/jira/browse/MESOS-3062
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added authorization for dynamic reservation master endpoints.
> Note: this review is continued from https://reviews.apache.org/r/37126/
> 
> 
> Diffs
> -
> 
>   src/master/http.cpp 9d729ef7f7d7ad6185934648f833e4f8a4f0a123 
>   src/tests/reservation_endpoints_tests.cpp 
> f30ff8bc6a3e9773437fa7fd7c8f569b7d7e2d9d 
> 
> Diff: https://reviews.apache.org/r/39988/diff/
> 
> 
> Testing
> ---
> 
> This is the fourth in a chain of 5 patches. Added new reservation endpoints 
> tests to validate authorization of reserve and unreserve operations using 
> ACLs. `make check` was run to test after all patches were applied.
> 
> 
> Thanks,
> 
> Greg Mann
> 
>

Re: Review Request 39988: [4/5] Added authorization for dynamic reservation master endpoints.

[Greg Mann]

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

(Updated Dec. 2, 2015, 10:47 p.m.)


Review request for mesos, Adam B, Jie Yu, Michael Park, and Till Toenshoff.


Changes
---

Addressed comment, changed order of validation with respect to authorization.


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


Repository: mesos


Description
---

Added authorization for dynamic reservation master endpoints.
Note: this review is continued from https://reviews.apache.org/r/37126/


Diffs (updated)
-

  src/master/http.cpp 9d729ef7f7d7ad6185934648f833e4f8a4f0a123 
  src/tests/reservation_endpoints_tests.cpp 
f30ff8bc6a3e9773437fa7fd7c8f569b7d7e2d9d 

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


Testing
---

This is the fourth in a chain of 5 patches. Added new reservation endpoints 
tests to validate authorization of reserve and unreserve operations using ACLs. 
`make check` was run to test after all patches were applied.


Thanks,

Greg Mann

Re: Review Request 39988: [4/5] Added authorization for dynamic reservation master endpoints.

[Greg Mann]

> On Dec. 2, 2015, 2:26 p.m., Alexander Rukletsov wrote:
> > src/master/master.hpp, lines 1089-1095
> > 
> >
> > Do you want to do this cleanup as a separate patch? I believe we tend 
> > not to conflate semantic and stylistic changes in one patch.

Sounds good, I'll put these changes in another patch.


- Greg


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


On Dec. 2, 2015, 7:53 p.m., Greg Mann wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39988/
> ---
> 
> (Updated Dec. 2, 2015, 7:53 p.m.)
> 
> 
> Review request for mesos, Adam B, Jie Yu, Michael Park, and Till Toenshoff.
> 
> 
> Bugs: MESOS-3062
> https://issues.apache.org/jira/browse/MESOS-3062
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added authorization for dynamic reservation master endpoints.
> Note: this review is continued from https://reviews.apache.org/r/37126/
> 
> 
> Diffs
> -
> 
>   src/master/http.cpp 9d729ef7f7d7ad6185934648f833e4f8a4f0a123 
>   src/tests/reservation_endpoints_tests.cpp 
> f30ff8bc6a3e9773437fa7fd7c8f569b7d7e2d9d 
> 
> Diff: https://reviews.apache.org/r/39988/diff/
> 
> 
> Testing
> ---
> 
> This is the fourth in a chain of 5 patches. Added new reservation endpoints 
> tests to validate authorization of reserve and unreserve operations using 
> ACLs. `make check` was run to test after all patches were applied.
> 
> 
> Thanks,
> 
> Greg Mann
> 
>

Re: Review Request 39988: [4/5] Added authorization for dynamic reservation master endpoints.

[Greg Mann]

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

(Updated Dec. 2, 2015, 7:53 p.m.)


Review request for mesos, Adam B, Jie Yu, Michael Park, and Till Toenshoff.


Changes
---

Addressed comments.


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


Repository: mesos


Description
---

Added authorization for dynamic reservation master endpoints.
Note: this review is continued from https://reviews.apache.org/r/37126/


Diffs (updated)
-

  src/master/http.cpp 9d729ef7f7d7ad6185934648f833e4f8a4f0a123 
  src/tests/reservation_endpoints_tests.cpp 
f30ff8bc6a3e9773437fa7fd7c8f569b7d7e2d9d 

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


Testing
---

This is the fourth in a chain of 5 patches. Added new reservation endpoints 
tests to validate authorization of reserve and unreserve operations using ACLs. 
`make check` was run to test after all patches were applied.


Thanks,

Greg Mann

Re: Review Request 39988: [4/5] Added authorization for dynamic reservation master endpoints.

[Jie Yu]

> On Dec. 2, 2015, 3:05 p.m., Alexander Rukletsov wrote:
> > src/master/http.cpp, lines 1035-1038
> > 
> >
> > Let's leave a comment here, that `principal` matches 
> > `reservation().principal()` for each resource in 
> > `operation.reserve().resources()`, hence it's OK to authorize for 
> > `principal` and use `reservation().principal()` in `unreserve()`. Maybe a 
> > symmetrical comment in `unreserve()` path would also make sense.
> > 
> > Maybe if you validate before authorizing it will be more easy to 
> > understand?

+1 on validate before authorizing. That could save us a big comment here.


- Jie


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


On Dec. 2, 2015, 9:06 a.m., Greg Mann wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39988/
> ---
> 
> (Updated Dec. 2, 2015, 9:06 a.m.)
> 
> 
> Review request for mesos, Adam B, Jie Yu, Michael Park, and Till Toenshoff.
> 
> 
> Bugs: MESOS-3062
> https://issues.apache.org/jira/browse/MESOS-3062
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added authorization for dynamic reservation master endpoints.
> Note: this review is continued from https://reviews.apache.org/r/37126/
> 
> 
> Diffs
> -
> 
>   src/master/http.cpp 9d729ef7f7d7ad6185934648f833e4f8a4f0a123 
>   src/master/master.hpp 96951e766de32842197506504e5ac67a2caa3efe 
>   src/tests/reservation_endpoints_tests.cpp 
> f30ff8bc6a3e9773437fa7fd7c8f569b7d7e2d9d 
> 
> Diff: https://reviews.apache.org/r/39988/diff/
> 
> 
> Testing
> ---
> 
> This is the fourth in a chain of 5 patches. Added new reservation endpoints 
> tests to validate authorization of reserve and unreserve operations using 
> ACLs. `make check` was run to test after all patches were applied.
> 
> 
> Thanks,
> 
> Greg Mann
> 
>

Re: Review Request 39988: [4/5] Added authorization for dynamic reservation master endpoints.

[Alexander Rukletsov]

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



src/master/http.cpp (lines 1034 - 1037)


Let's leave a comment here, that `principal` matches 
`reservation().principal()` for each resource in 
`operation.reserve().resources()`, hence it's OK to authorize for `principal` 
and use `reservation().principal()` in `unreserve()`. Maybe a symmetrical 
comment in `unreserve()` path would also make sense.

Maybe if you validate before authorizing it will be more easy to understand?


- Alexander Rukletsov


On Dec. 2, 2015, 9:06 a.m., Greg Mann wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39988/
> ---
> 
> (Updated Dec. 2, 2015, 9:06 a.m.)
> 
> 
> Review request for mesos, Adam B, Jie Yu, Michael Park, and Till Toenshoff.
> 
> 
> Bugs: MESOS-3062
> https://issues.apache.org/jira/browse/MESOS-3062
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added authorization for dynamic reservation master endpoints.
> Note: this review is continued from https://reviews.apache.org/r/37126/
> 
> 
> Diffs
> -
> 
>   src/master/http.cpp 9d729ef7f7d7ad6185934648f833e4f8a4f0a123 
>   src/master/master.hpp 96951e766de32842197506504e5ac67a2caa3efe 
>   src/tests/reservation_endpoints_tests.cpp 
> f30ff8bc6a3e9773437fa7fd7c8f569b7d7e2d9d 
> 
> Diff: https://reviews.apache.org/r/39988/diff/
> 
> 
> Testing
> ---
> 
> This is the fourth in a chain of 5 patches. Added new reservation endpoints 
> tests to validate authorization of reserve and unreserve operations using 
> ACLs. `make check` was run to test after all patches were applied.
> 
> 
> Thanks,
> 
> Greg Mann
> 
>

Re: Review Request 39988: [4/5] Added authorization for dynamic reservation master endpoints.

[Alexander Rukletsov]

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



src/master/master.hpp (lines 1089 - 1094)


Do you want to do this cleanup as a separate patch? I believe we tend not 
to conflate semantic and stylistic changes in one patch.


- Alexander Rukletsov


On Dec. 2, 2015, 9:06 a.m., Greg Mann wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39988/
> ---
> 
> (Updated Dec. 2, 2015, 9:06 a.m.)
> 
> 
> Review request for mesos, Adam B, Jie Yu, Michael Park, and Till Toenshoff.
> 
> 
> Bugs: MESOS-3062
> https://issues.apache.org/jira/browse/MESOS-3062
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added authorization for dynamic reservation master endpoints.
> Note: this review is continued from https://reviews.apache.org/r/37126/
> 
> 
> Diffs
> -
> 
>   src/master/http.cpp 9d729ef7f7d7ad6185934648f833e4f8a4f0a123 
>   src/master/master.hpp 96951e766de32842197506504e5ac67a2caa3efe 
>   src/tests/reservation_endpoints_tests.cpp 
> f30ff8bc6a3e9773437fa7fd7c8f569b7d7e2d9d 
> 
> Diff: https://reviews.apache.org/r/39988/diff/
> 
> 
> Testing
> ---
> 
> This is the fourth in a chain of 5 patches. Added new reservation endpoints 
> tests to validate authorization of reserve and unreserve operations using 
> ACLs. `make check` was run to test after all patches were applied.
> 
> 
> Thanks,
> 
> Greg Mann
> 
>

Re: Review Request 39988: [4/5] Added authorization for dynamic reservation master endpoints.

[Greg Mann]

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

(Updated Dec. 2, 2015, 9:06 a.m.)


Review request for mesos, Adam B, Jie Yu, Michael Park, and Till Toenshoff.


Changes
---

Addressed comments.


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


Repository: mesos


Description
---

Added authorization for dynamic reservation master endpoints.
Note: this review is continued from https://reviews.apache.org/r/37126/


Diffs (updated)
-

  src/master/http.cpp 9d729ef7f7d7ad6185934648f833e4f8a4f0a123 
  src/master/master.hpp 96951e766de32842197506504e5ac67a2caa3efe 
  src/tests/reservation_endpoints_tests.cpp 
f30ff8bc6a3e9773437fa7fd7c8f569b7d7e2d9d 

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


Testing
---

This is the fourth in a chain of 5 patches. Added new reservation endpoints 
tests to validate authorization of reserve and unreserve operations using ACLs. 
`make check` was run to test after all patches were applied.


Thanks,

Greg Mann

Re: Review Request 39988: [4/5] Added authorization for dynamic reservation master endpoints.

[Michael Park]

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



src/master/http.cpp (lines 1037 - 1040)


Let's not lose the comment on the left.



src/master/http.cpp (lines 1037 - 1041)


It looks like the `repair` here is to catch the case where 
`authorizeReserveResources` __fails__ on an invalid reserve request.

In https://reviews.apache.org/r/39987/ I asked whether the validation step 
needs to be within the authorization step. I still think it should be 
separated. If we were to separate them out, with Jie's suggestion below in 
mind, this could look like:

```cpp
  return master->authorizeReserveResources(operation, principal)
.then(defer(master->self(), [=](bool authorized) -> Future {
  if (!authorized) {
return Unauthorized("Mesos master");
  }
  Option error =
validation::operation::validate(operation.reserve(), None(), 
principal);
  if (error.isSome()) {
return BadRequest("Invalid RESERVE operation: " + 
error.get().message);
  }
  return _operation(slaveId, resources.flatten(), operation);
}));
```



src/master/http.cpp (lines 2307 - 2317)


With Jie's suggestion, we don't need this anymore. However, I would like to 
point out some weirdness here for future reference.

Since the `_operation` is only being used with `.then` or in a direct call, 
the `authorized` can/should be `bool` rather than `Future`. Take a look 
at `_teardown`. The reason this works as-is is because `bool` is implicitly 
convertible to `Future`.

With that in mind, we can then observe that the `if 
(!authorized.isReady())` is actually dead code. Whatever error handling that 
was intended to be done in that if-clause actually gets caught by the `repair` 
above instead.


- Michael Park


On Nov. 30, 2015, 8:46 p.m., Greg Mann wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39988/
> ---
> 
> (Updated Nov. 30, 2015, 8:46 p.m.)
> 
> 
> Review request for mesos, Adam B, Jie Yu, Michael Park, and Till Toenshoff.
> 
> 
> Bugs: MESOS-3062
> https://issues.apache.org/jira/browse/MESOS-3062
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added authorization for dynamic reservation master endpoints.
> Note: this review is continued from https://reviews.apache.org/r/37126/
> 
> 
> Diffs
> -
> 
>   src/master/http.cpp 9d729ef7f7d7ad6185934648f833e4f8a4f0a123 
>   src/master/master.hpp 96951e766de32842197506504e5ac67a2caa3efe 
>   src/tests/reservation_endpoints_tests.cpp 
> f30ff8bc6a3e9773437fa7fd7c8f569b7d7e2d9d 
> 
> Diff: https://reviews.apache.org/r/39988/diff/
> 
> 
> Testing
> ---
> 
> This is the fourth in a chain of 5 patches. Added new reservation endpoints 
> tests to validate authorization of reserve and unreserve operations using 
> ACLs. `make check` was run to test after all patches were applied.
> 
> 
> Thanks,
> 
> Greg Mann
> 
>

Re: Review Request 39988: [4/5] Added authorization for dynamic reservation master endpoints.

[Michael Park]

> On Nov. 30, 2015, 9:44 p.m., Jie Yu wrote:
> > src/master/http.cpp, line 2313
> > 
> >
> > Instead of adding a new parameter to `_operation`. Can we put the 
> > logics of checking authorization result to `_reserve` and `_unreserve`? 
> > It's wiered to me that `_operation` needs to check authorization result.
> > 
> > ```
> > auto _unreserve = [=](bool authorized) {
> >   if (!authorized) {
> > return Unauthorized("Mesos master");
> >   }
> >   
> >   return _operation(slaveId, resources, operation);
> > };
> > 
> > return master->authorizeUnreserveResources(...)
> >   .then(defer(master->self(), _unreserve))
> >   .repair([](const Future& future) {
> > return BadRequest("Invalid UNRESERVE operation: " + 
> > future.failure());
> >   });
> > ```

I think `teardown` can benefit from this pattern as well. I've tried it out to 
see how it would fit: https://reviews.apache.org/r/40822/


- Michael


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


On Nov. 30, 2015, 8:46 p.m., Greg Mann wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39988/
> ---
> 
> (Updated Nov. 30, 2015, 8:46 p.m.)
> 
> 
> Review request for mesos, Adam B, Jie Yu, Michael Park, and Till Toenshoff.
> 
> 
> Bugs: MESOS-3062
> https://issues.apache.org/jira/browse/MESOS-3062
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added authorization for dynamic reservation master endpoints.
> Note: this review is continued from https://reviews.apache.org/r/37126/
> 
> 
> Diffs
> -
> 
>   src/master/http.cpp 9d729ef7f7d7ad6185934648f833e4f8a4f0a123 
>   src/master/master.hpp 96951e766de32842197506504e5ac67a2caa3efe 
>   src/tests/reservation_endpoints_tests.cpp 
> f30ff8bc6a3e9773437fa7fd7c8f569b7d7e2d9d 
> 
> Diff: https://reviews.apache.org/r/39988/diff/
> 
> 
> Testing
> ---
> 
> This is the fourth in a chain of 5 patches. Added new reservation endpoints 
> tests to validate authorization of reserve and unreserve operations using 
> ACLs. `make check` was run to test after all patches were applied.
> 
> 
> Thanks,
> 
> Greg Mann
> 
>

Re: Review Request 39988: [4/5] Added authorization for dynamic reservation master endpoints.

[Jie Yu]

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



src/master/http.cpp (line 2255)


s/authorized/future/

Since it could be the future returned from `_unreserve`



src/master/http.cpp (line 2307)


Instead of adding a new parameter to `_operation`. Can we put the logics of 
checking authorization result to `_reserve` and `_unreserve`? It's wiered to me 
that `_operation` needs to check authorization result.

```
auto _unreserve = [=](bool authorized) {
  if (!authorized) {
return Unauthorized("Mesos master");
  }
  
  return _operation(slaveId, resources, operation);
};

return master->authorizeUnreserveResources(...)
  .then(defer(master->self(), _unreserve))
  .repair([](const Future& future) {
return BadRequest("Invalid UNRESERVE operation: " + future.failure());
  });
```



src/master/http.cpp (lines 2309 - 2321)


As I suggested above, no need for this anymore.


- Jie Yu


On Nov. 30, 2015, 8:46 p.m., Greg Mann wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39988/
> ---
> 
> (Updated Nov. 30, 2015, 8:46 p.m.)
> 
> 
> Review request for mesos, Adam B, Jie Yu, Michael Park, and Till Toenshoff.
> 
> 
> Bugs: MESOS-3062
> https://issues.apache.org/jira/browse/MESOS-3062
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added authorization for dynamic reservation master endpoints.
> Note: this review is continued from https://reviews.apache.org/r/37126/
> 
> 
> Diffs
> -
> 
>   src/master/http.cpp 9d729ef7f7d7ad6185934648f833e4f8a4f0a123 
>   src/master/master.hpp 96951e766de32842197506504e5ac67a2caa3efe 
>   src/tests/reservation_endpoints_tests.cpp 
> f30ff8bc6a3e9773437fa7fd7c8f569b7d7e2d9d 
> 
> Diff: https://reviews.apache.org/r/39988/diff/
> 
> 
> Testing
> ---
> 
> This is the fourth in a chain of 5 patches. Added new reservation endpoints 
> tests to validate authorization of reserve and unreserve operations using 
> ACLs. `make check` was run to test after all patches were applied.
> 
> 
> Thanks,
> 
> Greg Mann
> 
>

Re: Review Request 39988: [4/5] Added authorization for dynamic reservation master endpoints.

[Greg Mann]

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

(Updated Nov. 30, 2015, 8:46 p.m.)


Review request for mesos, Adam B, Jie Yu, Michael Park, and Till Toenshoff.


Changes
---

Refactored validation into master's authorization helpers.


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


Repository: mesos


Description
---

Added authorization for dynamic reservation master endpoints.
Note: this review is continued from https://reviews.apache.org/r/37126/


Diffs (updated)
-

  src/master/http.cpp 9d729ef7f7d7ad6185934648f833e4f8a4f0a123 
  src/master/master.hpp 96951e766de32842197506504e5ac67a2caa3efe 
  src/tests/reservation_endpoints_tests.cpp 
f30ff8bc6a3e9773437fa7fd7c8f569b7d7e2d9d 

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


Testing
---

This is the fourth in a chain of 5 patches. Added new reservation endpoints 
tests to validate authorization of reserve and unreserve operations using ACLs. 
`make check` was run to test after all patches were applied.


Thanks,

Greg Mann

Re: Review Request 39988: [4/5] Added authorization for dynamic reservation master endpoints.

[Jie Yu]

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

Ship it!


Ship It!

- Jie Yu


On Nov. 19, 2015, 11:41 p.m., Greg Mann wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39988/
> ---
> 
> (Updated Nov. 19, 2015, 11:41 p.m.)
> 
> 
> Review request for mesos, Adam B, Jie Yu, Michael Park, and Till Toenshoff.
> 
> 
> Bugs: MESOS-3062
> https://issues.apache.org/jira/browse/MESOS-3062
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added authorization for dynamic reservation master endpoints.
> Note: this review is continued from https://reviews.apache.org/r/37126/
> 
> 
> Diffs
> -
> 
>   src/master/http.cpp 1c4f1406f5d917f5d655a7d61d311365f8999ce0 
>   src/master/master.hpp 5e5a575dc7dd49324f3c837028df8a7f75cd1f80 
>   src/tests/reservation_endpoints_tests.cpp 
> 1552e4537c4f4d79bfa4bc17ccab2df630bc32a4 
> 
> Diff: https://reviews.apache.org/r/39988/diff/
> 
> 
> Testing
> ---
> 
> This is the fourth in a chain of 5 patches. Added new reservation endpoints 
> tests to validate authorization of reserve and unreserve operations using 
> ACLs. `make check` was run to test after all patches were applied.
> 
> 
> Thanks,
> 
> Greg Mann
> 
>

Re: Review Request 39988: [4/5] Added authorization for dynamic reservation master endpoints.

[Greg Mann]

> On Nov. 13, 2015, 11:38 p.m., Jie Yu wrote:
> > src/master/http.cpp, line 853
> > 
> >
> > This looks problematic to me. 'this' will become invalid once this 
> > function returns. That means when `_reserve` is actually called after the 
> > authorization is done, it might be a dangling pointer.
> > 
> > Instead, you can try to copy the http object (something like below):
> > 
> > ```
> > lambda::function(bool)> _reserve =
> >   lambda::bind(
> >   &Master::Http::_operation,
> >   *this,
> >   slaveId,
> >   resources.flatten(),
> >   operation,
> >   lambda::_1);
> > 
> > return master->authorizeReserveResources(operation.reserve(), principal)
> >   .then(defer(master->self(), _reserve));
> > ```
> 
> Greg Mann wrote:
> Good catch, thanks!
> 
> Jie Yu wrote:
> Mpark reached out to me about this issue. Seems that this extra copying 
> is not necessary (my bad). The 'http' object will be bounded in the lambda 
> function and stored in Master's libprocess handlers map.

I checked out the IRC logs; that makes sense! I reverted back to passing `this`.


- Greg


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


On Nov. 19, 2015, 11:41 p.m., Greg Mann wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39988/
> ---
> 
> (Updated Nov. 19, 2015, 11:41 p.m.)
> 
> 
> Review request for mesos, Adam B, Jie Yu, Michael Park, and Till Toenshoff.
> 
> 
> Bugs: MESOS-3062
> https://issues.apache.org/jira/browse/MESOS-3062
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added authorization for dynamic reservation master endpoints.
> Note: this review is continued from https://reviews.apache.org/r/37126/
> 
> 
> Diffs
> -
> 
>   src/master/http.cpp 1c4f1406f5d917f5d655a7d61d311365f8999ce0 
>   src/master/master.hpp 5e5a575dc7dd49324f3c837028df8a7f75cd1f80 
>   src/tests/reservation_endpoints_tests.cpp 
> 1552e4537c4f4d79bfa4bc17ccab2df630bc32a4 
> 
> Diff: https://reviews.apache.org/r/39988/diff/
> 
> 
> Testing
> ---
> 
> This is the fourth in a chain of 5 patches. Added new reservation endpoints 
> tests to validate authorization of reserve and unreserve operations using 
> ACLs. `make check` was run to test after all patches were applied.
> 
> 
> Thanks,
> 
> Greg Mann
> 
>

Re: Review Request 39988: [4/5] Added authorization for dynamic reservation master endpoints.

[Greg Mann]

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

(Updated Nov. 19, 2015, 11:41 p.m.)


Review request for mesos, Adam B, Jie Yu, Michael Park, and Till Toenshoff.


Changes
---

Addressed comment.


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


Repository: mesos


Description
---

Added authorization for dynamic reservation master endpoints.
Note: this review is continued from https://reviews.apache.org/r/37126/


Diffs (updated)
-

  src/master/http.cpp 1c4f1406f5d917f5d655a7d61d311365f8999ce0 
  src/master/master.hpp 5e5a575dc7dd49324f3c837028df8a7f75cd1f80 
  src/tests/reservation_endpoints_tests.cpp 
1552e4537c4f4d79bfa4bc17ccab2df630bc32a4 

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


Testing
---

This is the fourth in a chain of 5 patches. Added new reservation endpoints 
tests to validate authorization of reserve and unreserve operations using ACLs. 
`make check` was run to test after all patches were applied.


Thanks,

Greg Mann

Re: Review Request 39988: [4/5] Added authorization for dynamic reservation master endpoints.

[Jie Yu]

> On Nov. 13, 2015, 11:38 p.m., Jie Yu wrote:
> > src/master/http.cpp, line 853
> > 
> >
> > This looks problematic to me. 'this' will become invalid once this 
> > function returns. That means when `_reserve` is actually called after the 
> > authorization is done, it might be a dangling pointer.
> > 
> > Instead, you can try to copy the http object (something like below):
> > 
> > ```
> > lambda::function(bool)> _reserve =
> >   lambda::bind(
> >   &Master::Http::_operation,
> >   *this,
> >   slaveId,
> >   resources.flatten(),
> >   operation,
> >   lambda::_1);
> > 
> > return master->authorizeReserveResources(operation.reserve(), principal)
> >   .then(defer(master->self(), _reserve));
> > ```
> 
> Greg Mann wrote:
> Good catch, thanks!

Mpark reached out to me about this issue. Seems that this extra copying is not 
necessary (my bad). The 'http' object will be bounded in the lambda function 
and stored in Master's libprocess handlers map.


- Jie


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


On Nov. 16, 2015, 11:53 p.m., Greg Mann wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39988/
> ---
> 
> (Updated Nov. 16, 2015, 11:53 p.m.)
> 
> 
> Review request for mesos, Adam B, Jie Yu, Michael Park, and Till Toenshoff.
> 
> 
> Bugs: MESOS-3062
> https://issues.apache.org/jira/browse/MESOS-3062
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added authorization for dynamic reservation master endpoints.
> Note: this review is continued from https://reviews.apache.org/r/37126/
> 
> 
> Diffs
> -
> 
>   src/master/http.cpp 1c4f1406f5d917f5d655a7d61d311365f8999ce0 
>   src/master/master.hpp 5e5a575dc7dd49324f3c837028df8a7f75cd1f80 
>   src/tests/reservation_endpoints_tests.cpp 
> 1552e4537c4f4d79bfa4bc17ccab2df630bc32a4 
> 
> Diff: https://reviews.apache.org/r/39988/diff/
> 
> 
> Testing
> ---
> 
> This is the fourth in a chain of 5 patches. Added new reservation endpoints 
> tests to validate authorization of reserve and unreserve operations using 
> ACLs. `make check` was run to test after all patches were applied.
> 
> 
> Thanks,
> 
> Greg Mann
> 
>

Re: Review Request 39988: [4/5] Added authorization for dynamic reservation master endpoints.

[Greg Mann]

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

(Updated Nov. 16, 2015, 11:53 p.m.)


Review request for mesos, Adam B, Jie Yu, Michael Park, and Till Toenshoff.


Changes
---

Rebase.


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


Repository: mesos


Description
---

Added authorization for dynamic reservation master endpoints.
Note: this review is continued from https://reviews.apache.org/r/37126/


Diffs (updated)
-

  src/master/http.cpp 1c4f1406f5d917f5d655a7d61d311365f8999ce0 
  src/master/master.hpp 5e5a575dc7dd49324f3c837028df8a7f75cd1f80 
  src/tests/reservation_endpoints_tests.cpp 
1552e4537c4f4d79bfa4bc17ccab2df630bc32a4 

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


Testing
---

This is the fourth in a chain of 5 patches. Added new reservation endpoints 
tests to validate authorization of reserve and unreserve operations using ACLs. 
`make check` was run to test after all patches were applied.


Thanks,

Greg Mann

Re: Review Request 39988: [4/5] Added authorization for dynamic reservation master endpoints.

[Greg Mann]

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

(Updated Nov. 14, 2015, 12:01 a.m.)


Review request for mesos, Adam B, Jie Yu, Michael Park, and Till Toenshoff.


Changes
---

Addressed comments: pass Http object by value, use helper functions in tests.


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


Repository: mesos


Description
---

Added authorization for dynamic reservation master endpoints.
Note: this review is continued from https://reviews.apache.org/r/37126/


Diffs (updated)
-

  src/master/http.cpp f4ec23d74e203b2a8f2af187f0e56fbde7d9b3e5 
  src/master/master.hpp ead8520b7108a0f2c3a0bb11ae7b543897d111a2 
  src/tests/reservation_endpoints_tests.cpp 
1552e4537c4f4d79bfa4bc17ccab2df630bc32a4 

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


Testing
---

This is the fourth in a chain of 5 patches. Added new reservation endpoints 
tests to validate authorization of reserve and unreserve operations using ACLs. 
`make check` was run to test after all patches were applied.


Thanks,

Greg Mann

Re: Review Request 39988: [4/5] Added authorization for dynamic reservation master endpoints.

[Greg Mann]

> On Nov. 13, 2015, 11:38 p.m., Jie Yu wrote:
> > src/master/http.cpp, line 853
> > 
> >
> > This looks problematic to me. 'this' will become invalid once this 
> > function returns. That means when `_reserve` is actually called after the 
> > authorization is done, it might be a dangling pointer.
> > 
> > Instead, you can try to copy the http object (something like below):
> > 
> > ```
> > lambda::function(bool)> _reserve =
> >   lambda::bind(
> >   &Master::Http::_operation,
> >   *this,
> >   slaveId,
> >   resources.flatten(),
> >   operation,
> >   lambda::_1);
> > 
> > return master->authorizeReserveResources(operation.reserve(), principal)
> >   .then(defer(master->self(), _reserve));
> > ```

Good catch, thanks!


- Greg


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


On Nov. 14, 2015, 12:01 a.m., Greg Mann wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39988/
> ---
> 
> (Updated Nov. 14, 2015, 12:01 a.m.)
> 
> 
> Review request for mesos, Adam B, Jie Yu, Michael Park, and Till Toenshoff.
> 
> 
> Bugs: MESOS-3062
> https://issues.apache.org/jira/browse/MESOS-3062
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added authorization for dynamic reservation master endpoints.
> Note: this review is continued from https://reviews.apache.org/r/37126/
> 
> 
> Diffs
> -
> 
>   src/master/http.cpp f4ec23d74e203b2a8f2af187f0e56fbde7d9b3e5 
>   src/master/master.hpp ead8520b7108a0f2c3a0bb11ae7b543897d111a2 
>   src/tests/reservation_endpoints_tests.cpp 
> 1552e4537c4f4d79bfa4bc17ccab2df630bc32a4 
> 
> Diff: https://reviews.apache.org/r/39988/diff/
> 
> 
> Testing
> ---
> 
> This is the fourth in a chain of 5 patches. Added new reservation endpoints 
> tests to validate authorization of reserve and unreserve operations using 
> ACLs. `make check` was run to test after all patches were applied.
> 
> 
> Thanks,
> 
> Greg Mann
> 
>

Re: Review Request 39988: [4/5] Added authorization for dynamic reservation master endpoints.

[Jie Yu]

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



src/master/http.cpp (line 853)


This looks problematic to me. 'this' will become invalid once this function 
returns. That means when `_reserve` is actually called after the authorization 
is done, it might be a dangling pointer.

Instead, you can try to copy the http object (something like below):

```
lambda::function(bool)> _reserve =
  lambda::bind(
  &Master::Http::_operation,
  *this,
  slaveId,
  resources.flatten(),
  operation,
  lambda::_1);

return master->authorizeReserveResources(operation.reserve(), principal)
  .then(defer(master->self(), _reserve));
```



src/master/http.cpp (lines 2037 - 2046)


Ditto here.



src/tests/reservation_endpoints_tests.cpp (lines 1038 - 1062)


Can you use createRequestBody and createBasicAuthHeaders like other tests?


- Jie Yu


On Nov. 13, 2015, 11:16 p.m., Greg Mann wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39988/
> ---
> 
> (Updated Nov. 13, 2015, 11:16 p.m.)
> 
> 
> Review request for mesos, Adam B, Jie Yu, Michael Park, and Till Toenshoff.
> 
> 
> Bugs: MESOS-3062
> https://issues.apache.org/jira/browse/MESOS-3062
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added authorization for dynamic reservation master endpoints.
> Note: this review is continued from https://reviews.apache.org/r/37126/
> 
> 
> Diffs
> -
> 
>   src/master/http.cpp f4ec23d74e203b2a8f2af187f0e56fbde7d9b3e5 
>   src/master/master.hpp ead8520b7108a0f2c3a0bb11ae7b543897d111a2 
>   src/tests/reservation_endpoints_tests.cpp 
> 1552e4537c4f4d79bfa4bc17ccab2df630bc32a4 
> 
> Diff: https://reviews.apache.org/r/39988/diff/
> 
> 
> Testing
> ---
> 
> This is the fourth in a chain of 5 patches. Added new reservation endpoints 
> tests to validate authorization of reserve and unreserve operations using 
> ACLs. `make check` was run to test after all patches were applied.
> 
> 
> Thanks,
> 
> Greg Mann
> 
>

Re: Review Request 39988: [4/5] Added authorization for dynamic reservation master endpoints.

[Greg Mann]

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

(Updated Nov. 13, 2015, 11:16 p.m.)


Review request for mesos, Adam B, Jie Yu, Michael Park, and Till Toenshoff.


Changes
---

Updated ACL names.


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


Repository: mesos


Description
---

Added authorization for dynamic reservation master endpoints.
Note: this review is continued from https://reviews.apache.org/r/37126/


Diffs (updated)
-

  src/master/http.cpp f4ec23d74e203b2a8f2af187f0e56fbde7d9b3e5 
  src/master/master.hpp ead8520b7108a0f2c3a0bb11ae7b543897d111a2 
  src/tests/reservation_endpoints_tests.cpp 
1552e4537c4f4d79bfa4bc17ccab2df630bc32a4 

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


Testing
---

This is the fourth in a chain of 5 patches. Added new reservation endpoints 
tests to validate authorization of reserve and unreserve operations using ACLs. 
`make check` was run to test after all patches were applied.


Thanks,

Greg Mann

Re: Review Request 39988: [4/5] Added authorization for dynamic reservation master endpoints.

[Greg Mann]

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

(Updated Nov. 13, 2015, 4:32 a.m.)


Review request for mesos, Adam B, Jie Yu, Michael Park, and Till Toenshoff.


Changes
---

Added comments to tests.


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


Repository: mesos


Description
---

Added authorization for dynamic reservation master endpoints.
Note: this review is continued from https://reviews.apache.org/r/37126/


Diffs (updated)
-

  src/master/http.cpp f4ec23d74e203b2a8f2af187f0e56fbde7d9b3e5 
  src/master/master.hpp ead8520b7108a0f2c3a0bb11ae7b543897d111a2 
  src/tests/reservation_endpoints_tests.cpp 
1552e4537c4f4d79bfa4bc17ccab2df630bc32a4 

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


Testing
---

This is the fourth in a chain of 5 patches. Added new reservation endpoints 
tests to validate authorization of reserve and unreserve operations using ACLs. 
`make check` was run to test after all patches were applied.


Thanks,

Greg Mann

Re: Review Request 39988: [4/5] Added authorization for dynamic reservation master endpoints.

[Greg Mann]

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

(Updated Nov. 9, 2015, 5:26 p.m.)


Review request for mesos, Adam B, Jie Yu, Michael Park, and Till Toenshoff.


Changes
---

Rebase.


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


Repository: mesos


Description
---

Added authorization for dynamic reservation master endpoints.
Note: this review is continued from https://reviews.apache.org/r/37126/


Diffs (updated)
-

  src/master/http.cpp b0bec97ee69413bb70c2673c4ae49e74988796bf 
  src/master/master.hpp ead8520b7108a0f2c3a0bb11ae7b543897d111a2 
  src/tests/reservation_endpoints_tests.cpp 
1552e4537c4f4d79bfa4bc17ccab2df630bc32a4 

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


Testing
---

This is the fourth in a chain of 5 patches. Added new reservation endpoints 
tests to validate authorization of reserve and unreserve operations using ACLs. 
`make check` was run to test after all patches were applied.


Thanks,

Greg Mann

Re: Review Request 39988: [4/5] Added authorization for dynamic reservation master endpoints.

[Guangya Liu]

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

Ship it!


Ship It!

- Guangya Liu


On 十一月 7, 2015, 1:48 a.m., Greg Mann wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39988/
> ---
> 
> (Updated 十一月 7, 2015, 1:48 a.m.)
> 
> 
> Review request for mesos, Adam B, Jie Yu, Michael Park, and Till Toenshoff.
> 
> 
> Bugs: MESOS-3062
> https://issues.apache.org/jira/browse/MESOS-3062
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added authorization for dynamic reservation master endpoints.
> Note: this review is continued from https://reviews.apache.org/r/37126/
> 
> 
> Diffs
> -
> 
>   src/master/http.cpp 3b2cded3004cde4ff134a43b9205061550b82f41 
>   src/master/master.hpp ead8520b7108a0f2c3a0bb11ae7b543897d111a2 
>   src/tests/reservation_endpoints_tests.cpp 
> f5f9c4834779a3a84d12d9be77e674e2d6088b5d 
> 
> Diff: https://reviews.apache.org/r/39988/diff/
> 
> 
> Testing
> ---
> 
> This is the fourth in a chain of 5 patches. Added new reservation endpoints 
> tests to validate authorization of reserve and unreserve operations using 
> ACLs. `make check` was run to test after all patches were applied.
> 
> 
> Thanks,
> 
> Greg Mann
> 
>

Re: Review Request 39988: [4/5] Added authorization for dynamic reservation master endpoints.

[Greg Mann]

> On Nov. 7, 2015, 1:16 a.m., Neil Conway wrote:
> > src/master/master.hpp, line 981
> > 
> >
> > Should update the comment for new function parameter.

Derp. Thanks Neil!


- Greg


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


On Nov. 7, 2015, 1:48 a.m., Greg Mann wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39988/
> ---
> 
> (Updated Nov. 7, 2015, 1:48 a.m.)
> 
> 
> Review request for mesos, Adam B, Jie Yu, Michael Park, and Till Toenshoff.
> 
> 
> Bugs: MESOS-3062
> https://issues.apache.org/jira/browse/MESOS-3062
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added authorization for dynamic reservation master endpoints.
> Note: this review is continued from https://reviews.apache.org/r/37126/
> 
> 
> Diffs
> -
> 
>   src/master/http.cpp 3b2cded3004cde4ff134a43b9205061550b82f41 
>   src/master/master.hpp ead8520b7108a0f2c3a0bb11ae7b543897d111a2 
>   src/tests/reservation_endpoints_tests.cpp 
> f5f9c4834779a3a84d12d9be77e674e2d6088b5d 
> 
> Diff: https://reviews.apache.org/r/39988/diff/
> 
> 
> Testing
> ---
> 
> This is the fourth in a chain of 5 patches. Added new reservation endpoints 
> tests to validate authorization of reserve and unreserve operations using 
> ACLs. `make check` was run to test after all patches were applied.
> 
> 
> Thanks,
> 
> Greg Mann
> 
>

Re: Review Request 39988: [4/5] Added authorization for dynamic reservation master endpoints.

[Greg Mann]

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

(Updated Nov. 7, 2015, 1:48 a.m.)


Review request for mesos, Adam B, Jie Yu, Michael Park, and Till Toenshoff.


Changes
---

Addressed comments.


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


Repository: mesos


Description
---

Added authorization for dynamic reservation master endpoints.
Note: this review is continued from https://reviews.apache.org/r/37126/


Diffs (updated)
-

  src/master/http.cpp 3b2cded3004cde4ff134a43b9205061550b82f41 
  src/master/master.hpp ead8520b7108a0f2c3a0bb11ae7b543897d111a2 
  src/tests/reservation_endpoints_tests.cpp 
f5f9c4834779a3a84d12d9be77e674e2d6088b5d 

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


Testing
---

This is the fourth in a chain of 5 patches. Added new reservation endpoints 
tests to validate authorization of reserve and unreserve operations using ACLs. 
`make check` was run to test after all patches were applied.


Thanks,

Greg Mann

Re: Review Request 39988: [4/5] Added authorization for dynamic reservation master endpoints.

[Neil Conway]

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



src/master/master.hpp (line 981)


Should update the comment for new function parameter.



src/tests/reservation_endpoints_tests.cpp (line 834)


Probably don't want this whitespace?



src/tests/reservation_endpoints_tests.cpp (line 923)


Probably don't want this whitespace?



src/tests/reservation_endpoints_tests.cpp (line 1001)


Whitespace.


- Neil Conway


On Nov. 6, 2015, 11:58 p.m., Greg Mann wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39988/
> ---
> 
> (Updated Nov. 6, 2015, 11:58 p.m.)
> 
> 
> Review request for mesos, Adam B, Jie Yu, Michael Park, and Till Toenshoff.
> 
> 
> Bugs: MESOS-3062
> https://issues.apache.org/jira/browse/MESOS-3062
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added authorization for dynamic reservation master endpoints.
> Note: this review is continued from https://reviews.apache.org/r/37126/
> 
> 
> Diffs
> -
> 
>   src/master/http.cpp 9d20346460b4f9659aacba3fcb950ab564ba9d0d 
>   src/master/master.hpp b19f23be135d191ce119cf87fd25c4d6746faed3 
>   src/tests/reservation_endpoints_tests.cpp 
> f5f9c4834779a3a84d12d9be77e674e2d6088b5d 
> 
> Diff: https://reviews.apache.org/r/39988/diff/
> 
> 
> Testing
> ---
> 
> This is the fourth in a chain of 5 patches. Added new reservation endpoints 
> tests to validate authorization of reserve and unreserve operations using 
> ACLs. `make check` was run to test after all patches were applied.
> 
> 
> Thanks,
> 
> Greg Mann
> 
>

Re: Review Request 39988: [4/5] Added authorization for dynamic reservation master endpoints.

[Greg Mann]

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

(Updated Nov. 6, 2015, 11:58 p.m.)


Review request for mesos, Adam B, Jie Yu, Michael Park, and Till Toenshoff.


Changes
---

Removed commented-out code.


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


Repository: mesos


Description
---

Added authorization for dynamic reservation master endpoints.
Note: this review is continued from https://reviews.apache.org/r/37126/


Diffs (updated)
-

  src/master/http.cpp 9d20346460b4f9659aacba3fcb950ab564ba9d0d 
  src/master/master.hpp b19f23be135d191ce119cf87fd25c4d6746faed3 
  src/tests/reservation_endpoints_tests.cpp 
f5f9c4834779a3a84d12d9be77e674e2d6088b5d 

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


Testing
---

This is the fourth in a chain of 5 patches. Added new reservation endpoints 
tests to validate authorization of reserve and unreserve operations using ACLs. 
`make check` was run to test after all patches were applied.


Thanks,

Greg Mann

Re: Review Request 39988: [4/5] Added authorization for dynamic reservation master endpoints.

[Greg Mann]

> On Nov. 6, 2015, 8:03 a.m., Guangya Liu wrote:
> > src/tests/reservation_endpoints_tests.cpp, lines 874-877
> > 
> >
> > remove this

Ahh, sorry!! Thanks Guangya :-)


- Greg


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


On Nov. 5, 2015, 9:53 p.m., Greg Mann wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39988/
> ---
> 
> (Updated Nov. 5, 2015, 9:53 p.m.)
> 
> 
> Review request for mesos, Adam B, Jie Yu, Michael Park, and Till Toenshoff.
> 
> 
> Bugs: MESOS-3062
> https://issues.apache.org/jira/browse/MESOS-3062
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added authorization for dynamic reservation master endpoints.
> Note: this review is continued from https://reviews.apache.org/r/37126/
> 
> 
> Diffs
> -
> 
>   src/master/http.cpp 9d20346460b4f9659aacba3fcb950ab564ba9d0d 
>   src/master/master.hpp b19f23be135d191ce119cf87fd25c4d6746faed3 
>   src/tests/reservation_endpoints_tests.cpp 
> f5f9c4834779a3a84d12d9be77e674e2d6088b5d 
> 
> Diff: https://reviews.apache.org/r/39988/diff/
> 
> 
> Testing
> ---
> 
> This is the fourth in a chain of 5 patches. Added new reservation endpoints 
> tests to validate authorization of reserve and unreserve operations using 
> ACLs. `make check` was run to test after all patches were applied.
> 
> 
> Thanks,
> 
> Greg Mann
> 
>

Re: Review Request 39988: [4/5] Added authorization for dynamic reservation master endpoints.

[Guangya Liu]

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



src/tests/reservation_endpoints_tests.cpp (lines 871 - 874)


remove this



src/tests/reservation_endpoints_tests.cpp (lines 871 - 874)


remove?



src/tests/reservation_endpoints_tests.cpp (lines 959 - 962)


remove this



src/tests/reservation_endpoints_tests.cpp (lines 959 - 962)


remove



src/tests/reservation_endpoints_tests.cpp (lines 1041 - 1044)


remove this



src/tests/reservation_endpoints_tests.cpp (lines 1041 - 1044)


remove


- Guangya Liu


On 十一月 5, 2015, 9:53 p.m., Greg Mann wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39988/
> ---
> 
> (Updated 十一月 5, 2015, 9:53 p.m.)
> 
> 
> Review request for mesos, Adam B, Jie Yu, Michael Park, and Till Toenshoff.
> 
> 
> Bugs: MESOS-3062
> https://issues.apache.org/jira/browse/MESOS-3062
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added authorization for dynamic reservation master endpoints.
> Note: this review is continued from https://reviews.apache.org/r/37126/
> 
> 
> Diffs
> -
> 
>   src/master/http.cpp 9d20346460b4f9659aacba3fcb950ab564ba9d0d 
>   src/master/master.hpp b19f23be135d191ce119cf87fd25c4d6746faed3 
>   src/tests/reservation_endpoints_tests.cpp 
> f5f9c4834779a3a84d12d9be77e674e2d6088b5d 
> 
> Diff: https://reviews.apache.org/r/39988/diff/
> 
> 
> Testing
> ---
> 
> This is the fourth in a chain of 5 patches. Added new reservation endpoints 
> tests to validate authorization of reserve and unreserve operations using 
> ACLs. `make check` was run to test after all patches were applied.
> 
> 
> Thanks,
> 
> Greg Mann
> 
>

Re: Review Request 39988: [4/5] Added authorization for dynamic reservation master endpoints.

[Greg Mann]

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

(Updated Nov. 5, 2015, 9:53 p.m.)


Review request for mesos, Adam B, Jie Yu, Michael Park, and Till Toenshoff.


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


Repository: mesos


Description (updated)
---

Added authorization for dynamic reservation master endpoints.
Note: this review is continued from https://reviews.apache.org/r/37126/


Diffs
-

  src/master/http.cpp 9d20346460b4f9659aacba3fcb950ab564ba9d0d 
  src/master/master.hpp b19f23be135d191ce119cf87fd25c4d6746faed3 
  src/tests/reservation_endpoints_tests.cpp 
f5f9c4834779a3a84d12d9be77e674e2d6088b5d 

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


Testing
---

This is the fourth in a chain of 5 patches. Added new reservation endpoints 
tests to validate authorization of reserve and unreserve operations using ACLs. 
`make check` was run to test after all patches were applied.


Thanks,

Greg Mann