You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@brooklyn.apache.org by andreaturli <gi...@git.apache.org> on 2014/06/26 17:07:21 UTC

[GitHub] incubator-brooklyn pull request: entitlements scheme proposal

GitHub user andreaturli opened a pull request:

    https://github.com/apache/incubator-brooklyn/pull/23

    entitlements scheme proposal

    add EntitlementsTest
    record entitlement context for REST web requests, and log more info of web requests
    add AcmeEntitlementManagerTest
    add isEntitled check to ApplicationResource and EntityResource
    add unauthorized to WebResourceUtils
    add FROM_ENTITY function to EntityTransformer
    add EntitlementPredicates

You can merge this pull request into a Git repository by running:

    $ git pull https://github.com/andreaturli/incubator-brooklyn feature/entitlements

Alternatively you can review and apply these changes as the patch at:

    https://github.com/apache/incubator-brooklyn/pull/23.patch

To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:

    This closes #23
    
----
commit e774ccebe62708c756d5ffb3402586aae5b66596
Author: Andrea Turli <an...@gmail.com>
Date:   2014-06-09T21:42:19Z

    entitlements scheme proposal
    
    add EntitlementsTest
    record entitlement context for REST web requests, and log more info of web requests
    add AcmeEntitlementManagerTest
    add isEntitled check to ApplicationResource and EntityResource
    add unauthorized to WebResourceUtils
    add FROM_ENTITY function to EntityTransformer
    add EntitlementPredicates

----


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-brooklyn pull request: entitlements scheme proposal

Posted by aledsage <gi...@git.apache.org>.
Github user aledsage commented on a diff in the pull request:

    https://github.com/apache/incubator-brooklyn/pull/23#discussion_r14249377
  
    --- Diff: core/src/test/java/brooklyn/management/entitlement/AcmeEntitlementManagerTest.java ---
    @@ -0,0 +1,122 @@
    +/*
    + * Copyright 2009-2014 by Cloudsoft Corporation Limited
    --- End diff --
    
    I don't think we should have Copyright cloudsoft, should we?


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-brooklyn pull request: entitlements scheme proposal

Posted by ahgittin <gi...@git.apache.org>.
Github user ahgittin commented on a diff in the pull request:

    https://github.com/apache/incubator-brooklyn/pull/23#discussion_r14278119
  
    --- Diff: core/src/main/java/brooklyn/management/entitlement/EntitlementPredicates.java ---
    @@ -0,0 +1,18 @@
    +package brooklyn.management.entitlement;
    +
    +import javax.annotation.Nullable;
    +
    +import com.google.common.base.Predicate;
    +
    +public class EntitlementPredicates {
    +
    +    public static <T> Predicate<T> hasEntitlementClass(final EntitlementManager entitlementManager, final EntitlementClass entitlementClass) {
    --- End diff --
    
    +1


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-brooklyn pull request: entitlements scheme proposal

Posted by aledsage <gi...@git.apache.org>.
Github user aledsage commented on a diff in the pull request:

    https://github.com/apache/incubator-brooklyn/pull/23#discussion_r14249584
  
    --- Diff: core/src/main/java/brooklyn/management/entitlement/EntitlementPredicates.java ---
    @@ -0,0 +1,18 @@
    +package brooklyn.management.entitlement;
    +
    +import javax.annotation.Nullable;
    +
    +import com.google.common.base.Predicate;
    +
    +public class EntitlementPredicates {
    +
    +    public static <T> Predicate<T> hasEntitlementClass(final EntitlementManager entitlementManager, final EntitlementClass entitlementClass) {
    --- End diff --
    
    Use `EntitlementClass<T>` in method signature?


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-brooklyn pull request: entitlements scheme proposal

Posted by andreaturli <gi...@git.apache.org>.
Github user andreaturli commented on a diff in the pull request:

    https://github.com/apache/incubator-brooklyn/pull/23#discussion_r14253097
  
    --- Diff: core/src/main/java/brooklyn/management/entitlement/EntitlementPredicates.java ---
    @@ -0,0 +1,18 @@
    +package brooklyn.management.entitlement;
    +
    +import javax.annotation.Nullable;
    +
    +import com.google.common.base.Predicate;
    +
    +public class EntitlementPredicates {
    +
    +    public static <T> Predicate<T> hasEntitlementClass(final EntitlementManager entitlementManager, final EntitlementClass entitlementClass) {
    --- End diff --
    
    I'm ok. what do you think, @ahgittin?


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-brooklyn pull request: entitlements scheme proposal

Posted by richardcloudsoft <gi...@git.apache.org>.
Github user richardcloudsoft commented on a diff in the pull request:

    https://github.com/apache/incubator-brooklyn/pull/23#discussion_r14250466
  
    --- Diff: core/src/test/java/brooklyn/management/entitlement/AcmeEntitlementManagerTest.java ---
    @@ -0,0 +1,122 @@
    +/*
    + * Copyright 2009-2014 by Cloudsoft Corporation Limited
    --- End diff --
    
    @aledsage is correct, I have seen this issue appear on other Apache projects. This simplest route is to remove the copyright claims from this PR.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-brooklyn pull request: entitlements scheme proposal

Posted by ahgittin <gi...@git.apache.org>.
Github user ahgittin commented on a diff in the pull request:

    https://github.com/apache/incubator-brooklyn/pull/23#discussion_r14341591
  
    --- Diff: api/src/main/java/brooklyn/management/entitlement/EntitlementManager.java ---
    @@ -0,0 +1,16 @@
    +package brooklyn.management.entitlement;
    +
    +import javax.annotation.Nonnull;
    +import javax.annotation.Nullable;
    +
    +/** 
    + * Entitlement lookup relies on:
    + * <li>an "entitlement context", consisting of at minimum a string identifier of the user/actor for which entitlement is being requested
    + * <li>an "entitlement class", representing the category of activity for which entitlement is being requested
    + * <li>an "entitlement class argument", representing the specifics of the activity for which entitlement is being requested 
    + */
    +public interface EntitlementManager {
    +
    +    public <T> boolean isEntitled(@Nullable EntitlementContext context, @Nonnull EntitlementClass<T> entitlementClass, @Nullable T entitlementClassArgument);
    --- End diff --
    
    whatever initiates the outermost activity needs to set the `EntitlementContext` on a task or other `ThreadLocal`.  for REST calls there is a clear temporal wrap around this.
    
    but for other calls there is not.  there is not even any identifiable user logged in at that point.  e.g. if when launching we've said to launch `--app Foo` ... so i think it is another (big) chunk of work to apply entitlements to contexts other than REST.
    
    we could have some logic which says `if (threadLocalEntitlementContext==null) then use SystemEntitlementContext` but that feels messy, I'd rather say it's null for now, and then in time perhaps find a way to say something better


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-brooklyn pull request: entitlements scheme proposal

Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:

    https://github.com/apache/incubator-brooklyn/pull/23


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-brooklyn pull request: entitlements scheme proposal

Posted by aledsage <gi...@git.apache.org>.
Github user aledsage commented on a diff in the pull request:

    https://github.com/apache/incubator-brooklyn/pull/23#discussion_r14249494
  
    --- Diff: core/src/test/java/brooklyn/management/entitlement/AcmeEntitlementManagerTest.java ---
    @@ -0,0 +1,122 @@
    +/*
    + * Copyright 2009-2014 by Cloudsoft Corporation Limited
    + *
    + * Licensed under the Apache License, Version 2.0 (the "License");
    + * you may not use this file except in compliance with the License.
    + * You may obtain a copy of the License at
    + *
    + * http://www.apache.org/licenses/LICENSE-2.0
    + *
    + * Unless required by applicable law or agreed to in writing, software
    + * distributed under the License is distributed on an "AS IS" BASIS,
    + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
    + * See the License for the specific language governing permissions and
    + * limitations under the License.
    + */
    +package brooklyn.management.entitlement;
    +
    +import java.net.URI;
    +
    +import org.testng.Assert;
    +import org.testng.annotations.AfterMethod;
    +import org.testng.annotations.BeforeMethod;
    +import org.testng.annotations.Test;
    +
    +import com.google.common.collect.ImmutableList;
    +
    +import brooklyn.config.BrooklynProperties;
    +import brooklyn.entity.Application;
    +import brooklyn.entity.basic.ApplicationBuilder;
    +import brooklyn.entity.basic.BasicApplication;
    +import brooklyn.entity.basic.Entities;
    +import brooklyn.entity.proxying.EntitySpec;
    +import brooklyn.location.Location;
    +import brooklyn.management.ManagementContext;
    +import brooklyn.management.entitlement.Entitlements.EntityAndItem;
    +import brooklyn.test.entity.LocalManagementContextForTests;
    +import brooklyn.util.config.ConfigBag;
    +import brooklyn.util.exceptions.Exceptions;
    +
    +public class AcmeEntitlementManagerTest {
    +
    +    ManagementContext mgmt;
    +    Application app;
    +    ConfigBag configBag;
    +    
    +    public void setup(ConfigBag cfg) {
    +        mgmt = new LocalManagementContextForTests(BrooklynProperties.Factory.newEmpty().addFrom(cfg));
    +        app = ApplicationBuilder.newManagedApp(EntitySpec.create(BasicApplication.class), mgmt);
    +    }
    +
    +    @BeforeMethod
    --- End diff --
    
    always use `@BeforeMethod(alwaysRun=true)` - if someone later makes one of the tests in this class an Integration test (e.g. because it takes a few seconds) then it would fail in mvn because init wouldn't be called, but it would pass in Eclipse IDE!


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-brooklyn pull request: entitlements scheme proposal

Posted by aledsage <gi...@git.apache.org>.
Github user aledsage commented on a diff in the pull request:

    https://github.com/apache/incubator-brooklyn/pull/23#discussion_r14248979
  
    --- Diff: api/src/main/java/brooklyn/management/entitlement/EntitlementManager.java ---
    @@ -0,0 +1,16 @@
    +package brooklyn.management.entitlement;
    +
    +import javax.annotation.Nonnull;
    +import javax.annotation.Nullable;
    +
    +/** 
    + * Entitlement lookup relies on:
    + * <li>an "entitlement context", consisting of at minimum a string identifier of the user/actor for which entitlement is being requested
    + * <li>an "entitlement class", representing the category of activity for which entitlement is being requested
    + * <li>an "entitlement class argument", representing the specifics of the activity for which entitlement is being requested 
    + */
    +public interface EntitlementManager {
    +
    +    public <T> boolean isEntitled(@Nullable EntitlementContext context, @Nonnull EntitlementClass<T> entitlementClass, @Nullable T entitlementClassArgument);
    --- End diff --
    
    Why is `EntitlementContext` nullable?


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-brooklyn pull request: entitlements scheme proposal

Posted by aledsage <gi...@git.apache.org>.
Github user aledsage commented on a diff in the pull request:

    https://github.com/apache/incubator-brooklyn/pull/23#discussion_r14249093
  
    --- Diff: core/src/main/java/brooklyn/management/entitlement/BasicEntitlementContext.java ---
    @@ -0,0 +1,5 @@
    +package brooklyn.management.entitlement;
    +
    +public class BasicEntitlementContext {
    --- End diff --
    
    Should this `implements EntitlementContext`?


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-brooklyn pull request: entitlements scheme proposal

Posted by aledsage <gi...@git.apache.org>.
Github user aledsage commented on the pull request:

    https://github.com/apache/incubator-brooklyn/pull/23#issuecomment-47347602
  
    Below are entitlements review comments (from while I was offline, so not against the specific lines). Don't feel that you have to do all of these just because they are tick boxes (if I got my markdown right)! Please argue back if you disagree.
    
    [ ] add a liberal sprinkling of `@Beta`to classes such as Entitlements, EntitlementClass, etc
    [ ] EntityManagementSupport.listener.onEffectorStarting: pass in `entity` rather than `null`
    [ ] ApplicationResource.launch: effector is not `"createFromAppSpec"`.
        Will this have already gone through a `DEPLOY_APPLICATION` check? The effector would presumably be `start`?
    [ ] Very minor preference: `EntitlementManager... checkers` instead of ` EntitlementManager ...checkers`, because the `...` is part of the type rather than the parameter name.
    [ ] BrooklynRestResourceUtil.create: why check for INVOKE_EFFECTOR with `null` effector name? Feels like a code smell.
    [ ] ApplicationResource.delete: effector name of `"delete"` doesn't match what actually happens (calls "stop" and is then unmanaged).
        We need constants defined for things like this, if we're going to use special strings for things like delete or createFromAppSpec.
        Anywhere you have a hard-coded string in a call to `isEntitled`, then see about extracting that to a constant in `Entitlements` perhaps?
    [ ] Instead of `Entitlements.requireEntitled`, how about `Entitlements.checkEntitled` to match the guava naming conventions?
    [ ] `Entitlements.SEE_ALL_SERVER_INFO` not used except in tests - presumably it's work in progress?
    [ ] `Entitlements.SEE_SENSOR` not used except in tests - presumably it's work in progress?
    [ ] What is plan for adding/removing policies + enrichers? Will we have an explicit `EntitlementClass` for that?
    [ ] NotEntitledException: need to have getters for fields (and make fields `private final` for good measure?). Otherwise there's little point having the fields.
    [ ] NotEntitledException: constructor should call `super(message)` with a nice message to say who was trying to do what.
    [ ] In rest API, instead of doing `if (Entitlements.isEntitled...) ... else throw WebResourceUtils.unauthorized`, is it possible do something at the framework level with an annotation on `ApplicationApi` etc? This would be a similar idea to jclouds `@Fallback(MapHttp4xxCodesToExceptions.class)`, except we're doing it the other way round (instead of mapping http response to an exception, we're mapping an exception to an http response).
        e.g. could/should we do try-catch it in `BrooklynPropertiesSecurityFilter.doFilter`?
    [ ] I think you've missed `ApplicationResource.createPoly` for entitlements checks.
    [ ] EntityResource.getDescendantsSensor missing entitlements check; same for `getDescendants`
    [ ] What's the rule of thumb for putting entitlements check in `*Resource` (e.g. `EntityResoure`) versus in `BrooklynRestResourceUtils`?
        Currently looking at code it can be hard to tell if the check is being made, until you follow the call stack.
    [ ] Does `BasicEntitlementClassDefinition` or `EntityAndItem` need equals/hashCode implemented? I can't see anywhere it's used as a key, but just wanted to ask.
    [ ] Is the nested static class `Entitlements.FineGrainedEntitlements` pulling its weight, versus moving its methods into `Entitlements`?
    [ ] Class naming of `SinglePermissionEntitlementChecker implements EntitlementManager`: would be good to have consistent naming of "Manager" or "Checker". Suggest renaming class.
    [ ] (contentious?) In effort to keep `Entitlements` responsibility focused, could perhaps move `GLOBAL_ENTITLEMENT_MANAGER` and `newManager`/`newGlobalManager` somewhere else?
        Perhaps into `BrooklynServerConfig`, but not sure about that!
    [ ] Could also move `Entitlements.EntityAndItem` to a top-level class.
    [ ] Could move `Entitlements.SEE_ENTITY` etc into `EntitlementClasses`?
    [ ] The `EntitlementClass` of `Entitlements.ROOT` feels wrong - root isn't something you ask to do, it's a user and/or level of permission.
        It's also only used in tests.
    
    Long term:
    * we don't want to give away secrets on unauthorized; e.g. if we give different errors for entity does not exists versus the user is not authorized to access that entity, then that's not good for security?
      (appreciate other's thoughts on that @grkvlt)
    * we want to track the EntitlementContext for calls that span multiple threads. Need to think about...
      * If entity A calls entity B (thus going into a different thread), should we set the EntitlementContext for entity B's effector call?
        (currently we don't - see callers of `Entitlements.setEntitlementContext`, which uses thread-local storage)
      * If user Bob calls `app.deleteAll()` which calls `delete()` on each child, does Bob need read+write permissions on each child or just on the app? (sounds plausible)
      * Based on above, if app.deleteAll was iterating over its children but bob could only see some of them, would the effector only act on those Bob could see?! (sounds bad)



---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-brooklyn pull request: entitlements scheme proposal

Posted by ahgittin <gi...@git.apache.org>.
Github user ahgittin commented on a diff in the pull request:

    https://github.com/apache/incubator-brooklyn/pull/23#discussion_r14278105
  
    --- Diff: api/src/main/java/brooklyn/management/entitlement/EntitlementManager.java ---
    @@ -0,0 +1,16 @@
    +package brooklyn.management.entitlement;
    +
    +import javax.annotation.Nonnull;
    +import javax.annotation.Nullable;
    +
    +/** 
    + * Entitlement lookup relies on:
    + * <li>an "entitlement context", consisting of at minimum a string identifier of the user/actor for which entitlement is being requested
    + * <li>an "entitlement class", representing the category of activity for which entitlement is being requested
    + * <li>an "entitlement class argument", representing the specifics of the activity for which entitlement is being requested 
    + */
    +public interface EntitlementManager {
    +
    +    public <T> boolean isEntitled(@Nullable EntitlementContext context, @Nonnull EntitlementClass<T> entitlementClass, @Nullable T entitlementClassArgument);
    --- End diff --
    
    this may be invoked from places where no entitlement context is known, such as the CLI or tests.  all REST calls when properly configured with authentication will have an entitlement context set, however, so it is safe (and normal) for global entitlements managers to simply return true in this case.
    
    @andreaturli can you note that in the javadoc, and that implementers must be prepared for this case


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-brooklyn pull request: entitlements scheme proposal

Posted by aledsage <gi...@git.apache.org>.
Github user aledsage commented on a diff in the pull request:

    https://github.com/apache/incubator-brooklyn/pull/23#discussion_r14293644
  
    --- Diff: api/src/main/java/brooklyn/management/entitlement/EntitlementManager.java ---
    @@ -0,0 +1,16 @@
    +package brooklyn.management.entitlement;
    +
    +import javax.annotation.Nonnull;
    +import javax.annotation.Nullable;
    +
    +/** 
    + * Entitlement lookup relies on:
    + * <li>an "entitlement context", consisting of at minimum a string identifier of the user/actor for which entitlement is being requested
    + * <li>an "entitlement class", representing the category of activity for which entitlement is being requested
    + * <li>an "entitlement class argument", representing the specifics of the activity for which entitlement is being requested 
    + */
    +public interface EntitlementManager {
    +
    +    public <T> boolean isEntitled(@Nullable EntitlementContext context, @Nonnull EntitlementClass<T> entitlementClass, @Nullable T entitlementClassArgument);
    --- End diff --
    
    For CLI, why would context be null? We can find which user is logged in and we can indicate that it's called from CLI.
    For tests, that should never (if at all possible) drive an `@Nullable` on non-test code.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-brooklyn pull request: entitlements scheme proposal

Posted by aledsage <gi...@git.apache.org>.
Github user aledsage commented on a diff in the pull request:

    https://github.com/apache/incubator-brooklyn/pull/23#discussion_r14248880
  
    --- Diff: api/src/main/java/brooklyn/management/entitlement/EntitlementClass.java ---
    @@ -0,0 +1,9 @@
    +package brooklyn.management.entitlement;
    +
    +import com.google.common.reflect.TypeToken;
    +
    +/** @see EntitlementManager */
    +public interface EntitlementClass<T> {
    +    String entitlementClassIdentifier();
    +    TypeToken<T> aentitlementClassArgumentType();
    --- End diff --
    
    spelling.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-brooklyn pull request: entitlements scheme proposal

Posted by ahgittin <gi...@git.apache.org>.
Github user ahgittin commented on the pull request:

    https://github.com/apache/incubator-brooklyn/pull/23#issuecomment-47512511
  
    great comments @aledsage we will address these in #27


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-brooklyn pull request: entitlements scheme proposal

Posted by aledsage <gi...@git.apache.org>.
Github user aledsage commented on a diff in the pull request:

    https://github.com/apache/incubator-brooklyn/pull/23#discussion_r14249019
  
    --- Diff: core/src/main/java/brooklyn/management/entitlement/AcmeEntitlementManager.java ---
    @@ -0,0 +1,11 @@
    +package brooklyn.management.entitlement;
    +
    +class AcmeEntitlementManager extends PerUserEntitlementManagerWithDefault {
    --- End diff --
    
    Can we move this into src/test/java instead?


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-brooklyn pull request: entitlements scheme proposal

Posted by ahgittin <gi...@git.apache.org>.
Github user ahgittin commented on a diff in the pull request:

    https://github.com/apache/incubator-brooklyn/pull/23#discussion_r14278124
  
    --- Diff: core/src/main/java/brooklyn/management/entitlement/BasicEntitlementContext.java ---
    @@ -0,0 +1,5 @@
    +package brooklyn.management.entitlement;
    +
    +public class BasicEntitlementContext {
    --- End diff --
    
    Or delete this class?


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-brooklyn pull request: entitlements scheme proposal

Posted by ahgittin <gi...@git.apache.org>.
Github user ahgittin commented on the pull request:

    https://github.com/apache/incubator-brooklyn/pull/23#issuecomment-47303676
  
    agree w @aledsage comments; with them this is good to merge


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-brooklyn pull request: entitlements scheme proposal

Posted by aledsage <gi...@git.apache.org>.
Github user aledsage commented on a diff in the pull request:

    https://github.com/apache/incubator-brooklyn/pull/23#discussion_r14249832
  
    --- Diff: core/src/main/java/brooklyn/management/entitlement/EntitlementPredicates.java ---
    @@ -0,0 +1,18 @@
    +package brooklyn.management.entitlement;
    +
    +import javax.annotation.Nullable;
    +
    +import com.google.common.base.Predicate;
    +
    +public class EntitlementPredicates {
    +
    +    public static <T> Predicate<T> hasEntitlementClass(final EntitlementManager entitlementManager, final EntitlementClass entitlementClass) {
    --- End diff --
    
    I wonder about `hasEntitlement` as a better name?


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---