Review comments regarding ProtoRole

7 files changed, 55 insertions, 56 deletions

diff --git a/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/user/RoleId.java b/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/user/RoleId.java
index 3a5936e88aa..199f233835f 100644
--- a/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/user/RoleId.java
+++ b/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/user/RoleId.java
@@ -3,7 +3,7 @@ package com.yahoo.vespa.hosted.controller.api.integration.user;
 import com.yahoo.config.provision.ApplicationName;
 import com.yahoo.config.provision.TenantName;
 import com.yahoo.vespa.hosted.controller.api.role.ApplicationRole;
-import com.yahoo.vespa.hosted.controller.api.role.ProtoRole;
+import com.yahoo.vespa.hosted.controller.api.role.RoleDefinition;
 import com.yahoo.vespa.hosted.controller.api.role.Role;
 import com.yahoo.vespa.hosted.controller.api.role.Roles;
 import com.yahoo.vespa.hosted.controller.api.role.TenantRole;
@@ -78,11 +78,11 @@ public class RoleId {
     }
 
     private static String valueOf(TenantRole role) {
-        return valueOf(role.tenant()) + "." + valueOf(role.proto());
+        return valueOf(role.tenant()) + "." + valueOf(role.definition());
     }
 
     private static String valueOf(ApplicationRole role) {
-        return valueOf(role.tenant()) + "." + valueOf(role.application()) + "." + valueOf(role.proto());
+        return valueOf(role.tenant()) + "." + valueOf(role.application()) + "." + valueOf(role.definition());
     }
 
     private static String valueOf(TenantName tenant) {
@@ -99,7 +99,7 @@ public class RoleId {
         return application.value();
     }
 
-    private static String valueOf(ProtoRole role) {
+    private static String valueOf(RoleDefinition role) {
         switch (role) {
             case tenantOwner:          return "tenantOwner";
             case tenantAdmin:          return "tenantAdmin";
diff --git a/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/role/ApplicationRole.java b/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/role/ApplicationRole.java
index b1018fb08a6..cc1e8462580 100644
--- a/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/role/ApplicationRole.java
+++ b/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/role/ApplicationRole.java
@@ -11,8 +11,8 @@ import com.yahoo.config.provision.TenantName;
  */
 public class ApplicationRole extends Role {
 
-    ApplicationRole(ProtoRole protoRole, SystemName system, TenantName tenant, ApplicationName application) {
-        super(protoRole, Context.limitedTo(tenant, application, system));
+    ApplicationRole(RoleDefinition roleDefinition, SystemName system, TenantName tenant, ApplicationName application) {
+        super(roleDefinition, Context.limitedTo(tenant, application, system));
     }
 
     /** Returns the {@link TenantName} this is bound to. */
@@ -23,7 +23,7 @@ public class ApplicationRole extends Role {
 
     @Override
     public String toString() {
-        return "role '" + proto() + "' of '" + application() + "' owned by '" + tenant() + "'";
+        return "role '" + definition() + "' of '" + application() + "' owned by '" + tenant() + "'";
     }
 
 }
diff --git a/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/role/Role.java b/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/role/Role.java
index c98ddd5a37c..86d59b4bbb6 100644
--- a/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/role/Role.java
+++ b/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/role/Role.java
@@ -7,27 +7,27 @@ import java.util.Objects;
 import static java.util.Objects.requireNonNull;
 
 /**
- * A role is a combination of a {@link ProtoRole} and a {@link Context}, which allows evaluation
+ * A role is a combination of a {@link RoleDefinition} and a {@link Context}, which allows evaluation
  * of access control for a given action on a resource. Create using {@link Roles}.
  *
  * @author jonmv
  */
 public abstract class Role {
 
-    private final ProtoRole protoRole;
+    private final RoleDefinition roleDefinition;
     final Context context;
 
-    Role(ProtoRole protoRole, Context context) {
-        this.protoRole = requireNonNull(protoRole);
+    Role(RoleDefinition roleDefinition, Context context) {
+        this.roleDefinition = requireNonNull(roleDefinition);
         this.context = requireNonNull(context);
     }
 
-    /** Returns the proto role of this role. */
-    public ProtoRole proto() { return protoRole; }
+    /** Returns the role definition of this bound role. */
+    public RoleDefinition definition() { return roleDefinition; }
 
     /** Returns whether this role is allowed to perform the given action on the given resource. */
     public boolean allows(Action action, URI uri) {
-        return protoRole.policies().stream().anyMatch(policy -> policy.evaluate(action, uri, context));
+        return roleDefinition.policies().stream().anyMatch(policy -> policy.evaluate(action, uri, context));
     }
 
     @Override
@@ -35,13 +35,13 @@ public abstract class Role {
         if (this == o) return true;
         if (o == null || getClass() != o.getClass()) return false;
         Role role = (Role) o;
-        return protoRole == role.protoRole &&
-                Objects.equals(context, role.context);
+        return roleDefinition == role.roleDefinition &&
+               Objects.equals(context, role.context);
     }
 
     @Override
     public int hashCode() {
-        return Objects.hash(protoRole, context);
+        return Objects.hash(roleDefinition, context);
     }
 
 }
diff --git a/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/role/ProtoRole.java b/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/role/RoleDefinition.java
index 44d0d2356bd..e9c2f7bc643 100644
--- a/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/role/ProtoRole.java
+++ b/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/role/RoleDefinition.java
@@ -5,17 +5,16 @@ import java.util.Set;
 
 /**
  * This declares all tenant roles known to the controller. A role contains one or more {@link Policy}s which decide
- * what actions a member of a role can perform.
+ * what actions a member of a role can perform, given a {@link Context} for the action.
  *
- * Optionally, some role definition also inherit all policies from a "lower ranking" role. Read the list of roles
- * from {@code everyone} to {@code tenantAdmin}, in order, to see what policies these roles.
+ * Optionally, some role definitions also inherit all policies from a "lower ranking" role.
  *
  * See {@link Role} for roles bound to a context, where policies can be evaluated.
  *
  * @author mpolden
  * @author jonmv
  */
-public enum ProtoRole {
+public enum RoleDefinition {
 
     /** Deus ex machina. */
     hostedOperator(Policy.operator),
@@ -87,11 +86,11 @@ public enum ProtoRole {
 
     private final Set<Policy> policies;
 
-    ProtoRole(Policy... policies) {
+    RoleDefinition(Policy... policies) {
         this.policies = EnumSet.copyOf(Set.of(policies));
     }
 
-    ProtoRole(ProtoRole inherited, Policy... policies) {
+    RoleDefinition(RoleDefinition inherited, Policy... policies) {
         this.policies = EnumSet.copyOf(Set.of(policies));
         this.policies.addAll(inherited.policies);
     }
diff --git a/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/role/Roles.java b/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/role/Roles.java
index 101204748df..a6a4fdaf16c 100644
--- a/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/role/Roles.java
+++ b/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/role/Roles.java
@@ -9,7 +9,7 @@ import java.util.Objects;
 /**
  * Use if you need to create {@link Role}s for its system.
  *
- * This also defines the relationship between {@link ProtoRole}s and their required {@link Context}s.
+ * This also defines the relationship between {@link RoleDefinition}s and their required {@link Context}s.
  *
  * @author jonmv
  */
@@ -24,73 +24,73 @@ public class Roles {
 
 
     // General roles.
-    /** Returns a {@link ProtoRole#hostedOperator} for the current system. */
+    /** Returns a {@link RoleDefinition#hostedOperator} for the current system. */
     public UnboundRole hostedOperator() {
-        return new UnboundRole(ProtoRole.hostedOperator, system);
+        return new UnboundRole(RoleDefinition.hostedOperator, system);
     }
 
-    /** Returns a {@link ProtoRole#everyone} for the current system. */
+    /** Returns a {@link RoleDefinition#everyone} for the current system. */
     public UnboundRole everyone() {
-        return new UnboundRole(ProtoRole.everyone, system);
+        return new UnboundRole(RoleDefinition.everyone, system);
     }
 
 
     // Athenz based roles.
-    /** Returns a {@link ProtoRole#athenzTenantAdmin} for the current system and given tenant. */
+    /** Returns a {@link RoleDefinition#athenzTenantAdmin} for the current system and given tenant. */
     public TenantRole athenzTenantAdmin(TenantName tenant) {
-        return new TenantRole(ProtoRole.athenzTenantAdmin, system, tenant);
+        return new TenantRole(RoleDefinition.athenzTenantAdmin, system, tenant);
     }
 
-    /** Returns a {@link ProtoRole#tenantPipeline} for the current system and given tenant and application. */
+    /** Returns a {@link RoleDefinition#tenantPipeline} for the current system and given tenant and application. */
     public ApplicationRole tenantPipeline(TenantName tenant, ApplicationName application) {
-        return new ApplicationRole(ProtoRole.tenantPipeline, system, tenant, application);
+        return new ApplicationRole(RoleDefinition.tenantPipeline, system, tenant, application);
     }
 
 
     // Other identity provider based roles.
-    /** Returns a {@link ProtoRole#tenantOwner} for the current system and given tenant. */
+    /** Returns a {@link RoleDefinition#tenantOwner} for the current system and given tenant. */
     public TenantRole tenantOwner(TenantName tenant) {
-        return new TenantRole(ProtoRole.tenantOwner, system, tenant);
+        return new TenantRole(RoleDefinition.tenantOwner, system, tenant);
     }
 
-    /** Returns a {@link ProtoRole#tenantAdmin} for the current system and given tenant. */
+    /** Returns a {@link RoleDefinition#tenantAdmin} for the current system and given tenant. */
     public TenantRole tenantAdmin(TenantName tenant) {
-        return new TenantRole(ProtoRole.tenantAdmin, system, tenant);
+        return new TenantRole(RoleDefinition.tenantAdmin, system, tenant);
     }
 
-    /** Returns a {@link ProtoRole#tenantOperator} for the current system and given tenant. */
+    /** Returns a {@link RoleDefinition#tenantOperator} for the current system and given tenant. */
     public TenantRole tenantOperator(TenantName tenant) {
-        return new TenantRole(ProtoRole.tenantOperator, system, tenant);
+        return new TenantRole(RoleDefinition.tenantOperator, system, tenant);
     }
 
-    /** Returns a {@link ProtoRole#applicationOwner} for the current system and given tenant and application. */
+    /** Returns a {@link RoleDefinition#applicationOwner} for the current system and given tenant and application. */
     public ApplicationRole applicationOwner(TenantName tenant, ApplicationName application) {
-        return new ApplicationRole(ProtoRole.applicationOwner, system, tenant, application);
+        return new ApplicationRole(RoleDefinition.applicationOwner, system, tenant, application);
     }
 
-    /** Returns a {@link ProtoRole#applicationAdmin} for the current system and given tenant and application. */
+    /** Returns a {@link RoleDefinition#applicationAdmin} for the current system and given tenant and application. */
     public ApplicationRole applicationAdmin(TenantName tenant, ApplicationName application) {
-        return new ApplicationRole(ProtoRole.applicationAdmin, system, tenant, application);
+        return new ApplicationRole(RoleDefinition.applicationAdmin, system, tenant, application);
     }
 
-    /** Returns a {@link ProtoRole#applicationOperator} for the current system and given tenant and application. */
+    /** Returns a {@link RoleDefinition#applicationOperator} for the current system and given tenant and application. */
     public ApplicationRole applicationOperator(TenantName tenant, ApplicationName application) {
-        return new ApplicationRole(ProtoRole.applicationOperator, system, tenant, application);
+        return new ApplicationRole(RoleDefinition.applicationOperator, system, tenant, application);
     }
 
-    /** Returns a {@link ProtoRole#applicationDeveloper} for the current system and given tenant and application. */
+    /** Returns a {@link RoleDefinition#applicationDeveloper} for the current system and given tenant and application. */
     public ApplicationRole applicationDeveloper(TenantName tenant, ApplicationName application) {
-        return new ApplicationRole(ProtoRole.applicationDeveloper, system, tenant, application);
+        return new ApplicationRole(RoleDefinition.applicationDeveloper, system, tenant, application);
     }
 
-    /** Returns a {@link ProtoRole#applicationReader} for the current system and given tenant and application. */
+    /** Returns a {@link RoleDefinition#applicationReader} for the current system and given tenant and application. */
     public ApplicationRole applicationReader(TenantName tenant, ApplicationName application) {
-        return new ApplicationRole(ProtoRole.applicationReader, system, tenant, application);
+        return new ApplicationRole(RoleDefinition.applicationReader, system, tenant, application);
     }
 
-    /** Returns a {@link ProtoRole#buildService} for the current system and given tenant and application. */
+    /** Returns a {@link RoleDefinition#buildService} for the current system and given tenant and application. */
     public ApplicationRole buildService(TenantName tenant, ApplicationName application) {
-        return new ApplicationRole(ProtoRole.buildService, system, tenant, application);
+        return new ApplicationRole(RoleDefinition.buildService, system, tenant, application);
     }
 
 }
diff --git a/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/role/TenantRole.java b/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/role/TenantRole.java
index c3f8a5ef1b8..134628ec3a3 100644
--- a/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/role/TenantRole.java
+++ b/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/role/TenantRole.java
@@ -10,8 +10,8 @@ import com.yahoo.config.provision.TenantName;
  */
 public class TenantRole extends Role {
 
-    TenantRole(ProtoRole protoRole, SystemName system, TenantName tenant) {
-        super(protoRole, Context.limitedTo(tenant, system));
+    TenantRole(RoleDefinition roleDefinition, SystemName system, TenantName tenant) {
+        super(roleDefinition, Context.limitedTo(tenant, system));
     }
 
     /** Returns the {@link TenantName} this is bound to. */
@@ -19,7 +19,7 @@ public class TenantRole extends Role {
 
     @Override
     public String toString() {
-        return "role '" + proto() + "' of '" + tenant() + "'";
+        return "role '" + definition() + "' of '" + tenant() + "'";
     }
 
 }
diff --git a/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/role/UnboundRole.java b/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/role/UnboundRole.java
index ba01d949713..eb8319b2012 100644
--- a/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/role/UnboundRole.java
+++ b/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/role/UnboundRole.java
@@ -9,13 +9,13 @@ import com.yahoo.config.provision.SystemName;
  */
 public class UnboundRole extends Role {
 
-    UnboundRole(ProtoRole protoRole, SystemName system) {
-        super(protoRole, Context.unlimitedIn(system));
+    UnboundRole(RoleDefinition roleDefinition, SystemName system) {
+        super(roleDefinition, Context.unlimitedIn(system));
     }
 
     @Override
     public String toString() {
-        return "role '" + proto() + "'";
+        return "role '" + definition() + "'";
     }
 
 }