mesos-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Alexander Rojas <alexan...@mesosphere.io>
Subject Re: Review Request 65311: Added the ObjectApprovers to which unifies authorization logic.
Date Mon, 12 Mar 2018 14:08:26 GMT


> On March 9, 2018, 11:16 p.m., Greg Mann wrote:
> > src/common/http.hpp
> > Lines 198 (patched)
> > <https://reviews.apache.org/r/65311/diff/6/?file=1972585#file1972585line198>
> >
> >     See the comment I left on https://reviews.apache.org/r/65313/ - I wonder if
`action` really needs to be a template parameter here?

Strictly speaking it doesn't need to be a template, but then overrides like:

```c++
template <>
inline bool ObjectApprovers::approved<authorization::VIEW_ROLE>(
    const Resource& resource)
{
  // ...
}
```

Would have need to be dispatched to helper functions or handler in big ifs in `approved()`.

The end result would be, you just move the part hard to read from one side to the next.

My personal opinion is that the helper in the agent cleanup is more readable an a better price
to pay than a gigantic switch here.


- Alexander


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


On March 8, 2018, 1:14 p.m., Alexander Rojas wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/65311/
> -----------------------------------------------------------
> 
> (Updated March 8, 2018, 1:14 p.m.)
> 
> 
> Review request for mesos, Benjamin Hindman and Greg Mann.
> 
> 
> Bugs: MESOS-8434
>     https://issues.apache.org/jira/browse/MESOS-8434
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Introduces the class `ObjectApprovers` which unifies the different
> mechanisms used in Mesos in order to perform authorization.
> 
> This new class will supersede the `Acceptor` interfaces and their
> logic.
> 
> Follow up patches will make use of this interface and remove
> deprecated code.
> 
> 
> Diffs
> -----
> 
>   src/common/http.hpp 750d3bfba1647624983108bdd23295a3b3091fe4 
>   src/common/http.cpp 8d3a4ad63bd74e0313082bad3d89c21fbf54f256 
> 
> 
> Diff: https://reviews.apache.org/r/65311/diff/6/
> 
> 
> Testing
> -------
> 
> make check
> 
> 
> Thanks,
> 
> Alexander Rojas
> 
>


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