mesos-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Chun-Hung Hsiao <chhs...@mesosphere.io>
Subject Re: Review Request 62762: Added `LocalResourceProvider::principal()` for authentication.
Date Thu, 23 Nov 2017 07:08:43 GMT


> On Nov. 23, 2017, 5:27 a.m., Jie Yu wrote:
> > src/resource_provider/local.cpp
> > Lines 45 (patched)
> > <https://reviews.apache.org/r/62762/diff/8/?file=1899402#file1899402line45>
> >
> >     We try to avoid non-POD (plain old data) global variables because it'll cause
issues during exit (see google style guide).
> >     
> >     So I'll make this a helper function.

I'm thinking about following this style: https://github.com/apache/mesos/blob/master/src/slave/containerizer/mesos/containerizer.cpp#L360
Let me update the patch and please see if that is better.


> On Nov. 23, 2017, 5:27 a.m., Jie Yu wrote:
> > src/resource_provider/local.cpp
> > Lines 49 (patched)
> > <https://reviews.apache.org/r/62762/diff/8/?file=1899402#file1899402line49>
> >
> >     Do we still need this? It looks to me that the current naming scheme makes it
so that you don't have to customize it for each LRP?

This is for future proof. What if a new LRP needs to use some other API?


- Chun-Hung


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


On Nov. 22, 2017, 3:10 a.m., Chun-Hung Hsiao wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/62762/
> -----------------------------------------------------------
> 
> (Updated Nov. 22, 2017, 3:10 a.m.)
> 
> 
> Review request for mesos, Alexander Rojas, Greg Mann, Jie Yu, and Joseph Wu.
> 
> 
> Bugs: MESOS-8100
>     https://issues.apache.org/jira/browse/MESOS-8100
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> The `LocalResourceProvider::principal()` function takes a
> `ResourceProviderInfo` and generates a principal with the following
> claim:
> 
>   {"cid_prefix", <type-with-dots-replaced-by-dashes>-<name>--}
> 
> For example, for resource provider with type
> 'org.apache.mesos.rp.local.storage' and name 'foo', the claim would be:
> 
>   {"cid_prefix", "org-apache-mesos-rp-local-storage-foo--"}
> 
> In the future, we could add more claims for authorizing other
> operations, such as authorization for Resource Provider API.
> 
> 
> Diffs
> -----
> 
>   src/resource_provider/local.hpp ebaa07d03ad77d516066ee2d4b60864be0611b5f 
>   src/resource_provider/local.cpp ad98f333c5668ca81de6e7ed3fc8f59323b151da 
>   src/resource_provider/storage/provider.hpp 6de88c2329b358fcf48bc39ddda0132170991c3c

>   src/resource_provider/storage/provider.cpp 46224997430ac0c568904d80014166a6f059907f

> 
> 
> Diff: https://reviews.apache.org/r/62762/diff/8/
> 
> 
> Testing
> -------
> 
> make
> 
> 
> Thanks,
> 
> Chun-Hung Hsiao
> 
>


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