mesos-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Neil Conway <neil.con...@gmail.com>
Subject Re: Review Request 46501: Updated authorization.md to reflect current changes.
Date Fri, 29 Apr 2016 14:07:42 GMT

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




docs/authorization.md (line 8)
<https://reviews.apache.org/r/46501/#comment195038>

    First sentence: I would instead say something like, "In Mesos, the authorization subsystem
allows the operator to configure the actions that certain principals are allowed to perform.
For example, the operator can use authorization to ensure that principal `foo` can only register
frameworks in role `bar`, and no other principals can register frameworks in any role."
    
    I'd probably add a newline before "A reference"
    
    You might want to clarify that (a) ACLs are a property of the local authorizer only; other
authorizers might use different concepts for deciding whether a principal is allowed to perform
an action, (b) the remainder of the document (?) is specific to the local authorizer; other
authorizors might behave differently.



docs/authorization.md (line 12)
<https://reviews.apache.org/r/46501/#comment195036>

    "through a"
    
    "configured with"



docs/authorization.md (line 27)
<https://reviews.apache.org/r/46501/#comment195042>

    Did you remove the "Role vs. Principal" discussion deliberately? If so, why?



docs/authorization.md 
<https://reviews.apache.org/r/46501/#comment195045>

    The previous text had an example for every ACL action. I think giving an example for every
ACL action helps the reader understand how to use ACLs more concretely.



docs/authorization.md (line 64)
<https://reviews.apache.org/r/46501/#comment195039>

    "The order in which the rules are defined is important."
    
    "the acls" => "the ACLs"



docs/authorization.md (line 82)
<https://reviews.apache.org/r/46501/#comment195040>

    "ACLs"



docs/authorization.md (line 104)
<https://reviews.apache.org/r/46501/#comment195046>

    You might consider presenting this information using a Markdown table, rather than a list
where every element of the list has the same format.



docs/authorization.md (line 105)
<https://reviews.apache.org/r/46501/#comment195047>

    "ACLs entry" is imprecise; I would say it is the thing you're describing is the "action",
and a single ACL consists of the triple (subject, action, object).



docs/authorization.md (line 114)
<https://reviews.apache.org/r/46501/#comment195043>

    For consistency, this needs a newline (`<br/>`).



docs/authorization.md (line 160)
<https://reviews.apache.org/r/46501/#comment195049>

    The actual signature is `process::Future<bool> authorized(const authorization::Request&
request)`, right?
    
    Also, why are we using three back-ticks to denote inline code? AFAIK we should use a single
back-tick for that; three back-ticks are for code blocks.
    
    I would avoid saying "authorized() should return `false`..." -- authorized() returns a
future. I'd say something like "authorized() returns a future that indicates the result of
the (asynchronous) authorization operation. If the future is set to `true`, the request was
authorized successfully; if it was set to `false`, ..."


- Neil Conway


On April 28, 2016, 9:56 a.m., Alexander Rojas wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/46501/
> -----------------------------------------------------------
> 
> (Updated April 28, 2016, 9:56 a.m.)
> 
> 
> Review request for mesos, Adam B, Greg Mann, and Neil Conway.
> 
> 
> Bugs: MESOS-4785
>     https://issues.apache.org/jira/browse/MESOS-4785
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> The API of the authorization has been changing constantly over the
> last few versions. This patch attempts to update the documentation to
> the those changes into account.
> 
> 
> Diffs
> -----
> 
>   docs/authorization.md 0db5c345b3239814b3b9d2e8a87601ff69d0f869 
> 
> Diff: https://reviews.apache.org/r/46501/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Alexander Rojas
> 
>


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