Re: Review Request 58964: Added authorization support for operator endpoints.

[Alexander Rojas]

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

(Updated June 12, 2017, 3:24 p.m.)


Review request for mesos, Adam B, Greg Mann, and Till Toenshoff.


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


Repository: mesos


Description
---

Adds the actions 'UPDATE_MAINTENANCE_SCHEDULE',
'GET_MAINTENANCE_SCHEDULE', 'START_MAINTENANCE', 'STOP_MAINTENANCE'
and 'GET_MAINTENANCE_STATUS' to the authorizer API as well as the
necesary code to handle these new actions.

While the interface 'mesos::Authorizer' takes an object with type
'MachineID' to perform authorization; the default implementation of
the interface 'mesos::LocalAuthorizer' ignores the object choosing the
semantics of allow maintenance on all nodes or none. This was done to
extend the capacities of custom authorizers which may have special
rules for authorization.


Diffs (updated)
-

  docs/authorization.md d94f0f9d142e66118b89ecac28b9a4b21e88b6c8 
  include/mesos/authorizer/acls.proto 36b3ac231d8be9d40aad08940e870a69b0b72aa8 
  include/mesos/authorizer/authorizer.hpp 
4a7376fb6ca2be0a513ad54f56eea3cf8cdd024d 
  include/mesos/authorizer/authorizer.proto 
c9184d151befa4cea9bdebb36a315c760e6424b2 
  src/authorizer/local/authorizer.cpp 1f2a9902e88705cf412af834c127c3afe6d3bef4 
  src/tests/authorization_tests.cpp 6d85a5452d50390f96e0b11c2bac5c96722f6eac 


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

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


Testing
---

make check


Thanks,

Alexander Rojas

Re: Review Request 58964: Added authorization support for operator endpoints.

[Alexander Rojas]

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

(Updated June 12, 2017, 12:40 p.m.)


Review request for mesos, Adam B, Greg Mann, and Till Toenshoff.


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


Repository: mesos


Description (updated)
---

Adds the actions 'UPDATE_MAINTENANCE_SCHEDULE',
'GET_MAINTENANCE_SCHEDULE', 'START_MAINTENANCE', 'STOP_MAINTENANCE'
and 'GET_MAINTENANCE_STATUS' to the authorizer API as well as the
necesary code to handle these new actions.

While the interface 'mesos::Authorizer' takes an object with type
'MachineID' to perform authorization; the default implementation of
the interface 'mesos::LocalAuthorizer' ignores the object choosing the
semantics of allow maintenance on all nodes or none. This was done to
extend the capacities of custom authorizers which may have special
rules for authorization.


Diffs
-

  docs/authorization.md d94f0f9d142e66118b89ecac28b9a4b21e88b6c8 
  include/mesos/authorizer/acls.proto 36b3ac231d8be9d40aad08940e870a69b0b72aa8 
  include/mesos/authorizer/authorizer.hpp 
4a7376fb6ca2be0a513ad54f56eea3cf8cdd024d 
  include/mesos/authorizer/authorizer.proto 
c9184d151befa4cea9bdebb36a315c760e6424b2 
  src/authorizer/local/authorizer.cpp 1f2a9902e88705cf412af834c127c3afe6d3bef4 
  src/tests/authorization_tests.cpp 6d85a5452d50390f96e0b11c2bac5c96722f6eac 


Diff: https://reviews.apache.org/r/58964/diff/6/


Testing
---

make check


Thanks,

Alexander Rojas

Re: Review Request 58964: Added authorization support for operator endpoints.

[Till Toenshoff]

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


Fix it, then Ship it!




Looks great - one nit on the proto message member naming.


docs/authorization.md
Lines 858 (patched)


Not yours, but it seems we are missing a bunch here -- we should get them 
complete - mind doing this here or in a follow-up?

```
  optional quota.QuotaInfo quota_info = 6;
  optional WeightInfo weight_info = 7;
  optional Resource resource = 8;
  optional CommandInfo command_info = 9;
  optional ContainerID container_id = 10;
```



include/mesos/authorizer/acls.proto
Lines 472-476 (patched)


We are using repeated here, hence we are allowing for multiple elements and 
we do always use plural for the naming of such proto message arrays.

In this case, maybe we regard each of the members as an ACL; add "es" for a 
shortened way of saying ACLs.

```
  repeated ACL.UpdateMaintenanceSchedule update_maintenance_schedules = 35;
  repeated ACL.GetMaintenanceSchedule get_maintenance_schedules = 36;
  repeated ACL.StartMaintenance start_maintenances = 37;
  repeated ACL.StopMaintenance stop_maintenances = 38;
  repeated ACL.GetMaintenanceStatus get_maintenance_statuses = 39;
```


- Till Toenshoff


On June 2, 2017, 12:32 p.m., Alexander Rojas wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/58964/
> ---
> 
> (Updated June 2, 2017, 12:32 p.m.)
> 
> 
> Review request for mesos, Adam B, Greg Mann, and Till Toenshoff.
> 
> 
> Bugs: MESOS-7415
> https://issues.apache.org/jira/browse/MESOS-7415
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Adds the actions `UPDATE_MAINTENANCE_SCHEDULE`,
> `GET_MAINTENANCE_SCHEDULE`, `START_MAINTENANCE`, `STOP_MAINTENANCE`
> and `GET_MAINTENANCE_STATUS` to the authorizer API as well as the
> necesary code to handle these new actions.
> 
> While the interface `mesos::Authorizer` takes an object with type
> `MachineID` to perform authorization; the default implementation of
> the interface `mesos::LocalAuthorizer` ignores the object choosing the
> semantics of allow maintenance on all nodes or none. This was done to
> extend the capacities of custom authorizers which may have special
> rules for authorization.
> 
> 
> Diffs
> -
> 
>   docs/authorization.md d94f0f9d142e66118b89ecac28b9a4b21e88b6c8 
>   include/mesos/authorizer/acls.proto 
> 36b3ac231d8be9d40aad08940e870a69b0b72aa8 
>   include/mesos/authorizer/authorizer.hpp 
> 4a7376fb6ca2be0a513ad54f56eea3cf8cdd024d 
>   include/mesos/authorizer/authorizer.proto 
> c9184d151befa4cea9bdebb36a315c760e6424b2 
>   src/authorizer/local/authorizer.cpp 
> 1f2a9902e88705cf412af834c127c3afe6d3bef4 
>   src/tests/authorization_tests.cpp 6d85a5452d50390f96e0b11c2bac5c96722f6eac 
> 
> 
> Diff: https://reviews.apache.org/r/58964/diff/6/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Alexander Rojas
> 
>

Re: Review Request 58964: Added authorization support for operator endpoints.

[Greg Mann]

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


Fix it, then Ship it!





src/tests/authorization_tests.cpp
Lines 4955 (patched)


Indented too far. Here and below.



src/tests/authorization_tests.cpp
Lines 5011 (patched)


s/principal view/principal can view/


- Greg Mann


On June 2, 2017, 12:32 p.m., Alexander Rojas wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/58964/
> ---
> 
> (Updated June 2, 2017, 12:32 p.m.)
> 
> 
> Review request for mesos, Adam B, Greg Mann, and Till Toenshoff.
> 
> 
> Bugs: MESOS-7415
> https://issues.apache.org/jira/browse/MESOS-7415
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Adds the actions `UPDATE_MAINTENANCE_SCHEDULE`,
> `GET_MAINTENANCE_SCHEDULE`, `START_MAINTENANCE`, `STOP_MAINTENANCE`
> and `GET_MAINTENANCE_STATUS` to the authorizer API as well as the
> necesary code to handle these new actions.
> 
> While the interface `mesos::Authorizer` takes an object with type
> `MachineID` to perform authorization; the default implementation of
> the interface `mesos::LocalAuthorizer` ignores the object choosing the
> semantics of allow maintenance on all nodes or none. This was done to
> extend the capacities of custom authorizers which may have special
> rules for authorization.
> 
> 
> Diffs
> -
> 
>   docs/authorization.md d94f0f9d142e66118b89ecac28b9a4b21e88b6c8 
>   include/mesos/authorizer/acls.proto 
> 36b3ac231d8be9d40aad08940e870a69b0b72aa8 
>   include/mesos/authorizer/authorizer.hpp 
> 4a7376fb6ca2be0a513ad54f56eea3cf8cdd024d 
>   include/mesos/authorizer/authorizer.proto 
> c9184d151befa4cea9bdebb36a315c760e6424b2 
>   src/authorizer/local/authorizer.cpp 
> 1f2a9902e88705cf412af834c127c3afe6d3bef4 
>   src/tests/authorization_tests.cpp 6d85a5452d50390f96e0b11c2bac5c96722f6eac 
> 
> 
> Diff: https://reviews.apache.org/r/58964/diff/6/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Alexander Rojas
> 
>

Re: Review Request 58964: Added authorization support for operator endpoints.

[Alexander Rojas]

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

(Updated June 2, 2017, 2:32 p.m.)


Review request for mesos, Adam B, Greg Mann, and Till Toenshoff.


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


Repository: mesos


Description (updated)
---

Adds the actions `UPDATE_MAINTENANCE_SCHEDULE`,
`GET_MAINTENANCE_SCHEDULE`, `START_MAINTENANCE`, `STOP_MAINTENANCE`
and `GET_MAINTENANCE_STATUS` to the authorizer API as well as the
necesary code to handle these new actions.

While the interface `mesos::Authorizer` takes an object with type
`MachineID` to perform authorization; the default implementation of
the interface `mesos::LocalAuthorizer` ignores the object choosing the
semantics of allow maintenance on all nodes or none. This was done to
extend the capacities of custom authorizers which may have special
rules for authorization.


Diffs (updated)
-

  docs/authorization.md d94f0f9d142e66118b89ecac28b9a4b21e88b6c8 
  include/mesos/authorizer/acls.proto 36b3ac231d8be9d40aad08940e870a69b0b72aa8 
  include/mesos/authorizer/authorizer.hpp 
4a7376fb6ca2be0a513ad54f56eea3cf8cdd024d 
  include/mesos/authorizer/authorizer.proto 
c9184d151befa4cea9bdebb36a315c760e6424b2 
  src/authorizer/local/authorizer.cpp 1f2a9902e88705cf412af834c127c3afe6d3bef4 
  src/tests/authorization_tests.cpp 6d85a5452d50390f96e0b11c2bac5c96722f6eac 


Diff: https://reviews.apache.org/r/58964/diff/6/

Changes: https://reviews.apache.org/r/58964/diff/5-6/


Testing
---

make check


Thanks,

Alexander Rojas

Re: Review Request 58964: Added authorization support for operator endpoints.

[Greg Mann]

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



LGTM, modulo Till's comments.

- Greg Mann


On June 1, 2017, 2:56 p.m., Alexander Rojas wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/58964/
> ---
> 
> (Updated June 1, 2017, 2:56 p.m.)
> 
> 
> Review request for mesos, Adam B, Greg Mann, and Till Toenshoff.
> 
> 
> Bugs: MESOS-7415
> https://issues.apache.org/jira/browse/MESOS-7415
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Adds the actions `UPDATE_MAINTENANCE_SCHEDULE`,
> `GET_MAINTENANCE_SCHEDULE`, `START_MAINTENANCE`,
> `STOP_MAINTENANCE` and `GET_MAINTENANCE_STATUS` to the authorizer
> API as well as the necesary code to handle these new actions.
> 
> While the interface `mesos::Authorizer` takes an object with type
> `MachineID` to perform authorization; the default implementation of
> the interface `mesos::LocalAuthorizer` ignores the object choosing
> the semantics of allow maintenance on all nodes or none. This was done
> to extend the capacities of custom authorizers which may have special
> rules for authorization.
> 
> 
> Diffs
> -
> 
>   docs/authorization.md d94f0f9d142e66118b89ecac28b9a4b21e88b6c8 
>   include/mesos/authorizer/acls.proto 
> ae0b1ea2e6417d186b1606542d75f3a20e0811db 
>   include/mesos/authorizer/authorizer.hpp 
> 4a7376fb6ca2be0a513ad54f56eea3cf8cdd024d 
>   include/mesos/authorizer/authorizer.proto 
> c9184d151befa4cea9bdebb36a315c760e6424b2 
>   src/authorizer/local/authorizer.cpp 
> 89aaf4b712d337d519445c922606789c334e5101 
>   src/tests/authorization_tests.cpp 32aa6ac4db7854507127ea2fb88b3e92daa277c0 
> 
> 
> Diff: https://reviews.apache.org/r/58964/diff/5/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Alexander Rojas
> 
>

Re: Review Request 58964: Added authorization support for operator endpoints.

[Till Toenshoff]

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




docs/authorization.md
Lines 290 (patched)


s/my mesos/by Mesos/



docs/authorization.md
Lines 298 (patched)


s/my mesos/by Mesos/



docs/authorization.md
Lines 306 (patched)


s/in a/on a/



docs/authorization.md
Lines 313 (patched)


s/use types/use the types/



docs/authorization.md
Lines 316 (patched)


s/in a/on a/



docs/authorization.md
Lines 321 (patched)


s/use types/use the types/



src/authorizer/local/authorizer.cpp
Line 661 (original)


Thanks for fixing those.



src/tests/authorization_tests.cpp
Lines 4947 (patched)


Can we have a short leading comment on all the new tests please?
I do understand it wont add huge value but I like having those in general - 
without exceptions.



src/tests/authorization_tests.cpp
Lines 4972 (patched)


The term "whitelist" confuses me - especially given that we are now using 
quotes for both, principals as well as something like semantic matches. Maybe 
the following would be less irritating?

```
// "foo" should be allowed to update the schedule.
```

Can you update all others accordingly please?



src/tests/authorization_tests.cpp
Lines 4981 (patched)


```
// "bar" should not be allowed to update the schedule.
```


- Till Toenshoff


On June 1, 2017, 2:56 p.m., Alexander Rojas wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/58964/
> ---
> 
> (Updated June 1, 2017, 2:56 p.m.)
> 
> 
> Review request for mesos, Adam B, Greg Mann, and Till Toenshoff.
> 
> 
> Bugs: MESOS-7415
> https://issues.apache.org/jira/browse/MESOS-7415
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Adds the actions `UPDATE_MAINTENANCE_SCHEDULE`,
> `GET_MAINTENANCE_SCHEDULE`, `START_MAINTENANCE`,
> `STOP_MAINTENANCE` and `GET_MAINTENANCE_STATUS` to the authorizer
> API as well as the necesary code to handle these new actions.
> 
> While the interface `mesos::Authorizer` takes an object with type
> `MachineID` to perform authorization; the default implementation of
> the interface `mesos::LocalAuthorizer` ignores the object choosing
> the semantics of allow maintenance on all nodes or none. This was done
> to extend the capacities of custom authorizers which may have special
> rules for authorization.
> 
> 
> Diffs
> -
> 
>   docs/authorization.md d94f0f9d142e66118b89ecac28b9a4b21e88b6c8 
>   include/mesos/authorizer/acls.proto 
> ae0b1ea2e6417d186b1606542d75f3a20e0811db 
>   include/mesos/authorizer/authorizer.hpp 
> 4a7376fb6ca2be0a513ad54f56eea3cf8cdd024d 
>   include/mesos/authorizer/authorizer.proto 
> c9184d151befa4cea9bdebb36a315c760e6424b2 
>   src/authorizer/local/authorizer.cpp 
> 89aaf4b712d337d519445c922606789c334e5101 
>   src/tests/authorization_tests.cpp 32aa6ac4db7854507127ea2fb88b3e92daa277c0 
> 
> 
> Diff: https://reviews.apache.org/r/58964/diff/5/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Alexander Rojas
> 
>

Re: Review Request 58964: Added authorization support for operator endpoints.

[Alexander Rojas]

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

(Updated May 24, 2017, 3:17 p.m.)


Review request for mesos, Adam B and Greg Mann.


Changes
---

Updated documentation.


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


Repository: mesos


Description
---

Adds the actions `UPDATE_MAINTENANCE_SCHEDULE`,
`GET_MAINTENANCE_SCHEDULE`, `START_MAINTENANCE`, `STOP_MAINTENANCE`
and `GET_MAINTENANCE_STATUS` to the authorizer API as well as the
necesary code to handle these new actions.

While the interface `mesos::Authorizer` takes an object with type
`MachineID` to perform authorization; the default implementation of
the interface `mesos::LocalAuthorizer` ignores the object choosing the
semantics of allow maintenance on all nodes or none. This was done to
extend the capacities of custom authorizers which may have special
rules for authorization.


Diffs (updated)
-

  docs/authorization.md d94f0f9d142e66118b89ecac28b9a4b21e88b6c8 
  include/mesos/authorizer/acls.proto ae0b1ea2e6417d186b1606542d75f3a20e0811db 
  include/mesos/authorizer/authorizer.hpp 
4a7376fb6ca2be0a513ad54f56eea3cf8cdd024d 
  include/mesos/authorizer/authorizer.proto 
c9184d151befa4cea9bdebb36a315c760e6424b2 
  src/authorizer/local/authorizer.cpp 89aaf4b712d337d519445c922606789c334e5101 
  src/tests/authorization_tests.cpp 32aa6ac4db7854507127ea2fb88b3e92daa277c0 


Diff: https://reviews.apache.org/r/58964/diff/5/

Changes: https://reviews.apache.org/r/58964/diff/4-5/


Testing
---

make check


Thanks,

Alexander Rojas

Re: Review Request 58964: Added authorization support for operator endpoints.

[Alexander Rojas]

> On May 17, 2017, 11:57 a.m., Adam B wrote:
> > include/mesos/authorizer/acls.proto
> > Line 354 (original), 354 (patched)
> > 
> >
> > Should this be `agents` plural?
> 
> Alexander Rojas wrote:
> Probably, but if we want to change it it should be a blocker for 1.3.0. 
> Once it lands we will have to go through a six months deprecation cycle.

I created [r/59453/](https://reviews.apache.org/r/59453/), and I am talking 
with the release admins to see if we can change it before release.


> On May 17, 2017, 11:57 a.m., Adam B wrote:
> > include/mesos/authorizer/authorizer.proto
> > Lines 58 (patched)
> > 
> >
> > Unused?!?
> 
> Alexander Rojas wrote:
> sorry, original I was planning to have the request use the machine ID to 
> be authorized. I still think it makes sense to give the machine ID, which the 
> authorizer could ignore. Let's decide on that.

Well, I just modify it so authorization depends on the machine id, although the 
local authorizer doesn't use it.


- Alexander


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


On May 22, 2017, 6:05 p.m., Alexander Rojas wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/58964/
> ---
> 
> (Updated May 22, 2017, 6:05 p.m.)
> 
> 
> Review request for mesos, Adam B and Greg Mann.
> 
> 
> Bugs: MESOS-7415
> https://issues.apache.org/jira/browse/MESOS-7415
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Adds the actions `UPDATE_MAINTENANCE_SCHEDULE`,
> `GET_MAINTENANCE_SCHEDULE`, `START_MAINTENANCE`, `STOP_MAINTENANCE`
> and `GET_MAINTENANCE_STATUS` to the authorizer API as well as the
> necesary code to handle these new actions.
> 
> While the interface `mesos::Authorizer` takes an object with type
> `MachineID` to perform authorization; the default implementation of
> the interface `mesos::LocalAuthorizer` ignores the object choosing the
> semantics of allow maintenance on all nodes or none. This was done to
> extend the capacities of custom authorizers which may have special
> rules for authorization.
> 
> 
> Diffs
> -
> 
>   include/mesos/authorizer/acls.proto 
> ae0b1ea2e6417d186b1606542d75f3a20e0811db 
>   include/mesos/authorizer/authorizer.hpp 
> 4a7376fb6ca2be0a513ad54f56eea3cf8cdd024d 
>   include/mesos/authorizer/authorizer.proto 
> c9184d151befa4cea9bdebb36a315c760e6424b2 
>   src/authorizer/local/authorizer.cpp 
> 89aaf4b712d337d519445c922606789c334e5101 
>   src/tests/authorization_tests.cpp 32aa6ac4db7854507127ea2fb88b3e92daa277c0 
> 
> 
> Diff: https://reviews.apache.org/r/58964/diff/4/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Alexander Rojas
> 
>

Re: Review Request 58964: Added authorization support for operator endpoints.

[Alexander Rojas]

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

(Updated May 22, 2017, 6:05 p.m.)


Review request for mesos, Adam B and Greg Mann.


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


Repository: mesos


Description (updated)
---

Adds the actions `UPDATE_MAINTENANCE_SCHEDULE`,
`GET_MAINTENANCE_SCHEDULE`, `START_MAINTENANCE`, `STOP_MAINTENANCE`
and `GET_MAINTENANCE_STATUS` to the authorizer API as well as the
necesary code to handle these new actions.

While the interface `mesos::Authorizer` takes an object with type
`MachineID` to perform authorization; the default implementation of
the interface `mesos::LocalAuthorizer` ignores the object choosing the
semantics of allow maintenance on all nodes or none. This was done to
extend the capacities of custom authorizers which may have special
rules for authorization.


Diffs (updated)
-

  include/mesos/authorizer/acls.proto ae0b1ea2e6417d186b1606542d75f3a20e0811db 
  include/mesos/authorizer/authorizer.hpp 
4a7376fb6ca2be0a513ad54f56eea3cf8cdd024d 
  include/mesos/authorizer/authorizer.proto 
c9184d151befa4cea9bdebb36a315c760e6424b2 
  src/authorizer/local/authorizer.cpp 89aaf4b712d337d519445c922606789c334e5101 
  src/tests/authorization_tests.cpp 32aa6ac4db7854507127ea2fb88b3e92daa277c0 


Diff: https://reviews.apache.org/r/58964/diff/4/

Changes: https://reviews.apache.org/r/58964/diff/3-4/


Testing
---

make check


Thanks,

Alexander Rojas

Re: Review Request 58964: Added authorization support for operator endpoints.

[Alexander Rojas]

> On May 17, 2017, 11:57 a.m., Adam B wrote:
> > include/mesos/authorizer/acls.proto
> > Lines 364 (patched)
> > 
> >
> > Why do we think `machines` is the entity we want to authorize on? What 
> > if we decide we want to authorize on `schedules` in the future? This 
> > required field isn't very flexible.
> > Also, why not `agents` like in `RegisterAgent` above. Is there a 
> > distinction between agents and machines?

Schedules could be an interesting way to authorize, but also they would define 
a rather complex object which is not easy to specify by an entity. Moreover, a 
schedule is a beginning time, a duration and a set of machines. How you do 
define equality on them? does it make sense to say that someone is authorized 
to create a schedule in certain times and not in others. Likewise, machine 
could contain multiple agents. So what does it mean to be able to authorize one 
agent but not another in the same machine? that is why I decided machines made 
much more sense. 

Moreover, the request to set a maintenance schedule comes with a set of 
`machines_id`, which makes authorization much more easier and intuitive than 
using `agent_id` or any other.


> On May 17, 2017, 11:57 a.m., Adam B wrote:
> > include/mesos/authorizer/authorizer.proto
> > Lines 58 (patched)
> > 
> >
> > Unused?!?

sorry, original I was planning to have the request use the machine ID to be 
authorized. I still think it makes sense to give the machine ID, which the 
authorizer could ignore. Let's decide on that.


- Alexander


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


On May 12, 2017, 2:51 p.m., Alexander Rojas wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/58964/
> ---
> 
> (Updated May 12, 2017, 2:51 p.m.)
> 
> 
> Review request for mesos, Adam B and Greg Mann.
> 
> 
> Bugs: MESOS-7415
> https://issues.apache.org/jira/browse/MESOS-7415
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Adds the actions `UPDATE_MAINTENANCE_SCHEDULE`,
> `GET_MAINTENANCE_SCHEDULE`, `START_MAINTENANCE`, `STOP_MAINTENANCE`
> and `GET_MAINTENANCE_STATUS` to the authorizer API as well as the
> necesary code to handle these new actions.
> 
> 
> Diffs
> -
> 
>   include/mesos/authorizer/acls.proto 
> ae0b1ea2e6417d186b1606542d75f3a20e0811db 
>   include/mesos/authorizer/authorizer.proto 
> c9184d151befa4cea9bdebb36a315c760e6424b2 
>   src/authorizer/local/authorizer.cpp 
> 89aaf4b712d337d519445c922606789c334e5101 
>   src/tests/authorization_tests.cpp 32aa6ac4db7854507127ea2fb88b3e92daa277c0 
> 
> 
> Diff: https://reviews.apache.org/r/58964/diff/3/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Alexander Rojas
> 
>

Re: Review Request 58964: Added authorization support for operator endpoints.

[Alexander Rojas]

> On May 17, 2017, 11:57 a.m., Adam B wrote:
> > include/mesos/authorizer/acls.proto
> > Line 354 (original), 354 (patched)
> > 
> >
> > Should this be `agents` plural?

Probably, but if we want to change it it should be a blocker for 1.3.0. Once it 
lands we will have to go through a six months deprecation cycle.


- Alexander


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


On May 12, 2017, 2:51 p.m., Alexander Rojas wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/58964/
> ---
> 
> (Updated May 12, 2017, 2:51 p.m.)
> 
> 
> Review request for mesos, Adam B and Greg Mann.
> 
> 
> Bugs: MESOS-7415
> https://issues.apache.org/jira/browse/MESOS-7415
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Adds the actions `UPDATE_MAINTENANCE_SCHEDULE`,
> `GET_MAINTENANCE_SCHEDULE`, `START_MAINTENANCE`, `STOP_MAINTENANCE`
> and `GET_MAINTENANCE_STATUS` to the authorizer API as well as the
> necesary code to handle these new actions.
> 
> 
> Diffs
> -
> 
>   include/mesos/authorizer/acls.proto 
> ae0b1ea2e6417d186b1606542d75f3a20e0811db 
>   include/mesos/authorizer/authorizer.proto 
> c9184d151befa4cea9bdebb36a315c760e6424b2 
>   src/authorizer/local/authorizer.cpp 
> 89aaf4b712d337d519445c922606789c334e5101 
>   src/tests/authorization_tests.cpp 32aa6ac4db7854507127ea2fb88b3e92daa277c0 
> 
> 
> Diff: https://reviews.apache.org/r/58964/diff/3/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Alexander Rojas
> 
>

Re: Review Request 58964: Added authorization support for operator endpoints.

[Adam B]

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



Didn't get to the tests, but here's something to start with.


include/mesos/authorizer/acls.proto
Line 354 (original), 354 (patched)


Should this be `agents` plural?



include/mesos/authorizer/acls.proto
Lines 364 (patched)


Why do we think `machines` is the entity we want to authorize on? What if 
we decide we want to authorize on `schedules` in the future? This required 
field isn't very flexible.
Also, why not `agents` like in `RegisterAgent` above. Is there a 
distinction between agents and machines?



include/mesos/authorizer/acls.proto
Lines 377 (patched)


s/in/on/



include/mesos/authorizer/acls.proto
Lines 387 (patched)


s/in/on/



include/mesos/authorizer/acls.proto
Lines 391 (patched)


Missed the comment on this object entity



include/mesos/authorizer/acls.proto
Lines 394-395 (patched)


s/status of  maintenance in/maintenance status of/



include/mesos/authorizer/authorizer.proto
Lines 58 (patched)


Unused?!?



include/mesos/authorizer/authorizer.proto
Lines 204 (patched)


s/he's/is/g in all of these comments



include/mesos/authorizer/authorizer.proto
Lines 213 (patched)


"on all nodes or none" in each of these comments



include/mesos/authorizer/authorizer.proto
Lines 221 (patched)


"of all nodes or none"



src/authorizer/local/authorizer.cpp
Lines 391-398 (original), 391-406 (patched)


Why not merge these all into a single case body?



src/authorizer/local/authorizer.cpp
Lines 866-867 (original), 879-885 (patched)


These should probably be alpha-sorted as well



src/authorizer/local/authorizer.cpp
Lines 1024-1025 (original), 1042-1048 (patched)


These should probably be alpha-sorted into the list above.


- Adam B


On May 12, 2017, 5:51 a.m., Alexander Rojas wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/58964/
> ---
> 
> (Updated May 12, 2017, 5:51 a.m.)
> 
> 
> Review request for mesos, Adam B and Greg Mann.
> 
> 
> Bugs: MESOS-7415
> https://issues.apache.org/jira/browse/MESOS-7415
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Adds the actions `UPDATE_MAINTENANCE_SCHEDULE`,
> `GET_MAINTENANCE_SCHEDULE`, `START_MAINTENANCE`, `STOP_MAINTENANCE`
> and `GET_MAINTENANCE_STATUS` to the authorizer API as well as the
> necesary code to handle these new actions.
> 
> 
> Diffs
> -
> 
>   include/mesos/authorizer/acls.proto 
> ae0b1ea2e6417d186b1606542d75f3a20e0811db 
>   include/mesos/authorizer/authorizer.proto 
> c9184d151befa4cea9bdebb36a315c760e6424b2 
>   src/authorizer/local/authorizer.cpp 
> 89aaf4b712d337d519445c922606789c334e5101 
>   src/tests/authorization_tests.cpp 32aa6ac4db7854507127ea2fb88b3e92daa277c0 
> 
> 
> Diff: https://reviews.apache.org/r/58964/diff/3/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Alexander Rojas
> 
>

Re: Review Request 58964: Added authorization support for operator endpoints.

[Alexander Rojas]

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

(Updated May 12, 2017, 2:51 p.m.)


Review request for mesos, Adam B and Greg Mann.


Changes
---

Added tests, documentation and validation of ACLs


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


Repository: mesos


Description
---

Adds the actions `UPDATE_MAINTENANCE_SCHEDULE`,
`GET_MAINTENANCE_SCHEDULE`, `START_MAINTENANCE`, `STOP_MAINTENANCE`
and `GET_MAINTENANCE_STATUS` to the authorizer API as well as the
necesary code to handle these new actions.


Diffs (updated)
-

  include/mesos/authorizer/acls.proto ae0b1ea2e6417d186b1606542d75f3a20e0811db 
  include/mesos/authorizer/authorizer.proto 
c9184d151befa4cea9bdebb36a315c760e6424b2 
  src/authorizer/local/authorizer.cpp 89aaf4b712d337d519445c922606789c334e5101 
  src/tests/authorization_tests.cpp 32aa6ac4db7854507127ea2fb88b3e92daa277c0 


Diff: https://reviews.apache.org/r/58964/diff/3/

Changes: https://reviews.apache.org/r/58964/diff/2-3/


Testing (updated)
---

make check


Thanks,

Alexander Rojas

Re: Review Request 58964: Added authorization support for operator endpoints.

[Alexander Rojas]

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

(Updated May 9, 2017, 5:48 p.m.)


Review request for mesos, Adam B and Greg Mann.


Changes
---

Change to implicit object so that operator can work on all nodes or none.


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


Repository: mesos


Description
---

Adds the actions `UPDATE_MAINTENANCE_SCHEDULE`,
`GET_MAINTENANCE_SCHEDULE`, `START_MAINTENANCE`, `STOP_MAINTENANCE`
and `GET_MAINTENANCE_STATUS` to the authorizer API as well as the
necesary code to handle these new actions.


Diffs (updated)
-

  include/mesos/authorizer/acls.proto ae0b1ea2e6417d186b1606542d75f3a20e0811db 
  include/mesos/authorizer/authorizer.proto 
c9184d151befa4cea9bdebb36a315c760e6424b2 
  src/authorizer/local/authorizer.cpp 89aaf4b712d337d519445c922606789c334e5101 


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

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


Testing
---

make check

specific tests not yet written.


Thanks,

Alexander Rojas

Review Request 58964: Added authorization support for operator endpoints.

[Alexander Rojas]

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

Review request for mesos, Adam B and Greg Mann.


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


Repository: mesos


Description
---

Adds the actions `UPDATE_MAINTENANCE_SCHEDULE`,
`GET_MAINTENANCE_SCHEDULE`, `START_MAINTENANCE`, `STOP_MAINTENANCE`
and `GET_MAINTENANCE_STATUS` to the authorizer API as well as the
necesary code to handle these new actions.


Diffs
-

  include/mesos/authorizer/acls.proto ae0b1ea2e6417d186b1606542d75f3a20e0811db 
  include/mesos/authorizer/authorizer.hpp 
4a7376fb6ca2be0a513ad54f56eea3cf8cdd024d 
  include/mesos/authorizer/authorizer.proto 
c9184d151befa4cea9bdebb36a315c760e6424b2 
  src/authorizer/local/authorizer.cpp 89aaf4b712d337d519445c922606789c334e5101 


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


Testing
---

make check

specific tests not yet written.


Thanks,

Alexander Rojas