mesos-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Adam B <a...@mesosphere.io>
Subject Re: Review Request 47891: Added RUN_TASK authorization action.
Date Sat, 28 May 2016 06:19:50 GMT


> On May 27, 2016, 1:52 a.m., Adam B wrote:
> > include/mesos/authorizer/authorizer.proto, line 86
> > <https://reviews.apache.org/r/47891/diff/1/?file=1395390#file1395390line86>
> >
> >     I wonder if we should just alias RUN_TASK to the same enum value as RUN_TASK_WITH_USER..
There shouldn't be any backwards compatibility issues since these are only used in-memory,
and modules have to recompile anyway.
> 
> Benjamin Bannier wrote:
>     Good idea. I changed the proto message definitions to that effect. As a note, it
seems we usually don't use aliasing fields as this will be the only enum using proto's `allow_alias`
setting.

Saw that. Looks like it was added in protobuf 2.5.0, which we've supported for a while. Now
our minimum is 2.6.1, so it should compile fine. It's just dangerous, especially for out of
order enums.


> On May 27, 2016, 1:52 a.m., Adam B wrote:
> > src/tests/authorization_tests.cpp, line 203
> > <https://reviews.apache.org/r/47891/diff/1/?file=1395392#file1395392line203>
> >
> >     But principal "foo" could run as any other user, e.g. "bar", right? That'd be
worth testing.
> 
> Benjamin Bannier wrote:
>     Let's not mix this into this patch.

Fair enough.


> On May 27, 2016, 1:52 a.m., Adam B wrote:
> > src/tests/authorization_tests.cpp, line 529
> > <https://reviews.apache.org/r/47891/diff/1/?file=1395392#file1395392line529>
> >
> >     Would be better to test that "bar" cannot run a "user1", since we've shown previously
that somebody else ("foo") can.
> 
> Benjamin Bannier wrote:
>     Let's not mix this into this patch.

Fair enough.


- Adam


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


On May 27, 2016, 2:51 p.m., Benjamin Bannier wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/47891/
> -----------------------------------------------------------
> 
> (Updated May 27, 2016, 2:51 p.m.)
> 
> 
> Review request for mesos, Adam B, Alexander Rojas, Joerg Schad, and Michael Park.
> 
> 
> Bugs: MESOS-5459
>     https://issues.apache.org/jira/browse/MESOS-5459
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Authorization requests for RUN_TASK actions can pass `SOME`
> authorization object either in a `FrameworkInfo` holding a user, or a
> `TaskInfo` with optionally a `CommandInfo` which can optionally hold a
> user. If either of these fields is set it will be used as the object;
> otherwise an `ANY` type authorization object will be created.
> 
> `RUN_TASK` aliases `RUN_TASK_WITH_USER` which becomes deprecated with
> 0.29.
> 
> 
> Diffs
> -----
> 
>   include/mesos/authorizer/authorizer.proto 3ff67858a99915e0215f3ffb9966f9ac4a3fba8c

>   src/authorizer/local/authorizer.cpp 7ddb323df09a9b0ea46c6f9543c4af059d184308 
>   src/master/master.cpp 6442762c9fdfa368d5d9d7cd43b97f5addaf7f17 
>   src/tests/authorization_tests.cpp 54bfb46a807677f4a4a2bb88dcb78a358cf5121a 
> 
> Diff: https://reviews.apache.org/r/47891/diff/
> 
> 
> Testing
> -------
> 
> Tested on a range of Linux configurations on internal CI.
> 
> 
> Thanks,
> 
> Benjamin Bannier
> 
>


Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message