Replace Roles with static factories in Role

5 files changed, 102 insertions, 147 deletions

diff --git a/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/user/UserRoles.java b/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/user/UserRoles.java
index 479fcbd2589..239d7216491 100644
--- a/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/user/UserRoles.java
+++ b/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/user/UserRoles.java
@@ -5,13 +5,10 @@ import com.yahoo.config.provision.TenantName;
 import com.yahoo.vespa.hosted.controller.api.role.ApplicationRole;
 import com.yahoo.vespa.hosted.controller.api.role.Role;
 import com.yahoo.vespa.hosted.controller.api.role.RoleDefinition;
-import com.yahoo.vespa.hosted.controller.api.role.Roles;
 import com.yahoo.vespa.hosted.controller.api.role.TenantRole;
 
 import java.util.List;
 
-import static java.util.Objects.requireNonNull;
-
 /**
  * Validation, utility and serialization methods for roles used in user management.
  *
@@ -19,26 +16,22 @@ import static java.util.Objects.requireNonNull;
  */
 public class UserRoles {
 
-    private final Roles roles;
-
     /** Creates a new UserRoles which can be used for serialisation and listing of bound user roles. */
-    public UserRoles(Roles roles) {
-        this.roles = requireNonNull(roles);
-    }
+    public UserRoles() { }
 
     /** Returns the list of {@link TenantRole}s a {@link UserId} may be a member of. */
     public List<TenantRole> tenantRoles(TenantName tenant) {
-        return List.of(roles.tenantOwner(tenant),
-                       roles.tenantAdmin(tenant),
-                       roles.tenantOperator(tenant));
+        return List.of(Role.tenantOwner(tenant),
+                       Role.tenantAdmin(tenant),
+                       Role.tenantOperator(tenant));
     }
 
     /** Returns the list of {@link ApplicationRole}s a {@link UserId} may be a member of. */
     public List<ApplicationRole> applicationRoles(TenantName tenant, ApplicationName application) {
-        return List.of(roles.applicationAdmin(tenant, application),
-                       roles.applicationOperator(tenant, application),
-                       roles.applicationDeveloper(tenant, application),
-                       roles.applicationReader(tenant, application));
+        return List.of(Role.applicationAdmin(tenant, application),
+                       Role.applicationOperator(tenant, application),
+                       Role.applicationDeveloper(tenant, application),
+                       Role.applicationReader(tenant, application));
     }
 
     /** Returns the {@link Role} the given value represents. */
@@ -52,7 +45,7 @@ public class UserRoles {
 
     public Role toOperatorRole(String roleName) {
         switch (roleName) {
-            case "hostedOperator": return roles.hostedOperator();
+            case "hostedOperator": return Role.hostedOperator();
             default: throw new IllegalArgumentException("Malformed or illegal role name '" + roleName + "'.");
         }
     }
@@ -60,9 +53,9 @@ public class UserRoles {
     /** Returns the {@link Role} the given tenant, application and role names correspond to. */
     public Role toRole(TenantName tenant, String roleName) {
         switch (roleName) {
-            case "tenantOwner": return roles.tenantOwner(tenant);
-            case "tenantAdmin": return roles.tenantAdmin(tenant);
-            case "tenantOperator": return roles.tenantOperator(tenant);
+            case "tenantOwner": return Role.tenantOwner(tenant);
+            case "tenantAdmin": return Role.tenantAdmin(tenant);
+            case "tenantOperator": return Role.tenantOperator(tenant);
             default: throw new IllegalArgumentException("Malformed or illegal role name '" + roleName + "'.");
         }
     }
@@ -70,10 +63,10 @@ public class UserRoles {
     /** Returns the {@link Role} the given tenant and role names correspond to. */
     public Role toRole(TenantName tenant, ApplicationName application, String roleName) {
         switch (roleName) {
-            case "applicationAdmin": return roles.applicationAdmin(tenant, application);
-            case "applicationOperator": return roles.applicationOperator(tenant, application);
-            case "applicationDeveloper": return roles.applicationDeveloper(tenant, application);
-            case "applicationReader": return roles.applicationReader(tenant, application);
+            case "applicationAdmin": return Role.applicationAdmin(tenant, application);
+            case "applicationOperator": return Role.applicationOperator(tenant, application);
+            case "applicationDeveloper": return Role.applicationDeveloper(tenant, application);
+            case "applicationReader": return Role.applicationReader(tenant, application);
             default: throw new IllegalArgumentException("Malformed or illegal role name '" + roleName + "'.");
         }
     }
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 c63f341c616..61f3f11db94 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
@@ -1,12 +1,15 @@
 // Copyright 2019 Oath Inc. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root.
 package com.yahoo.vespa.hosted.controller.api.role;
 
+import com.yahoo.config.provision.ApplicationName;
+import com.yahoo.config.provision.TenantName;
+
 import java.net.URI;
 import java.util.Objects;
 
 /**
  * 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}.
+ * of access control for a given action on a resource.
  *
  * @author jonmv
  */
@@ -20,6 +23,66 @@ public abstract class Role {
         this.context = Objects.requireNonNull(context);
     }
 
+    /** Returns a {@link RoleDefinition#hostedOperator} for the current system. */
+    public static UnboundRole hostedOperator() {
+        return new UnboundRole(RoleDefinition.hostedOperator);
+    }
+
+    /** Returns a {@link RoleDefinition#everyone} for the current system. */
+    public static UnboundRole everyone() {
+        return new UnboundRole(RoleDefinition.everyone);
+    }
+
+    /** Returns a {@link RoleDefinition#athenzTenantAdmin} for the current system and given tenant. */
+    public static TenantRole athenzTenantAdmin(TenantName tenant) {
+        return new TenantRole(RoleDefinition.athenzTenantAdmin, tenant);
+    }
+
+    /** Returns a {@link RoleDefinition#tenantPipeline} for the current system and given tenant and application. */
+    public static ApplicationRole tenantPipeline(TenantName tenant, ApplicationName application) {
+        return new ApplicationRole(RoleDefinition.tenantPipeline, tenant, application);
+    }
+
+    /** Returns a {@link RoleDefinition#tenantOwner} for the current system and given tenant. */
+    public static TenantRole tenantOwner(TenantName tenant) {
+        return new TenantRole(RoleDefinition.tenantOwner, tenant);
+    }
+
+    /** Returns a {@link RoleDefinition#tenantAdmin} for the current system and given tenant. */
+    public static TenantRole tenantAdmin(TenantName tenant) {
+        return new TenantRole(RoleDefinition.tenantAdmin, tenant);
+    }
+
+    /** Returns a {@link RoleDefinition#tenantOperator} for the current system and given tenant. */
+    public static TenantRole tenantOperator(TenantName tenant) {
+        return new TenantRole(RoleDefinition.tenantOperator, tenant);
+    }
+
+    /** Returns a {@link RoleDefinition#applicationAdmin} for the current system and given tenant and application. */
+    public static ApplicationRole applicationAdmin(TenantName tenant, ApplicationName application) {
+        return new ApplicationRole(RoleDefinition.applicationAdmin, tenant, application);
+    }
+
+    /** Returns a {@link RoleDefinition#applicationOperator} for the current system and given tenant and application. */
+    public static ApplicationRole applicationOperator(TenantName tenant, ApplicationName application) {
+        return new ApplicationRole(RoleDefinition.applicationOperator, tenant, application);
+    }
+
+    /** Returns a {@link RoleDefinition#applicationDeveloper} for the current system and given tenant and application. */
+    public static ApplicationRole applicationDeveloper(TenantName tenant, ApplicationName application) {
+        return new ApplicationRole(RoleDefinition.applicationDeveloper, tenant, application);
+    }
+
+    /** Returns a {@link RoleDefinition#applicationReader} for the current system and given tenant and application. */
+    public static ApplicationRole applicationReader(TenantName tenant, ApplicationName application) {
+        return new ApplicationRole(RoleDefinition.applicationReader, tenant, application);
+    }
+
+    /** Returns a {@link RoleDefinition#buildService} for the current system and given tenant and application. */
+    public static ApplicationRole buildService(TenantName tenant, ApplicationName application) {
+        return new ApplicationRole(RoleDefinition.buildService, tenant, application);
+    }
+
     /** Returns the role definition of this bound role. */
     public RoleDefinition definition() { return roleDefinition; }
 
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
deleted file mode 100644
index 24facdd59e9..00000000000
--- a/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/role/Roles.java
+++ /dev/null
@@ -1,98 +0,0 @@
-package com.yahoo.vespa.hosted.controller.api.role;
-
-import com.google.inject.Inject;
-import com.yahoo.config.provision.ApplicationName;
-import com.yahoo.config.provision.SystemName;
-import com.yahoo.config.provision.TenantName;
-import com.yahoo.vespa.hosted.controller.api.integration.zone.ZoneRegistry;
-
-import java.util.Objects;
-
-/**
- * Use if you need to create {@link Role}s for its system.
- *
- * This also defines the relationship between {@link RoleDefinition}s and their required {@link Context}s.
- *
- * @author jonmv
- */
-public class Roles {
-
-    private final SystemName system;
-
-    @Inject
-    public Roles(ZoneRegistry zones) {
-        this(zones.system());
-    }
-
-    /** Creates a Roles which can be used to create bound roles for the given system. */
-    public Roles(SystemName system) {
-        this.system = Objects.requireNonNull(system);
-    }
-
-
-    // General roles.
-    /** Returns a {@link RoleDefinition#hostedOperator} for the current system. */
-    public UnboundRole hostedOperator() {
-        return new UnboundRole(RoleDefinition.hostedOperator);
-    }
-
-    /** Returns a {@link RoleDefinition#everyone} for the current system. */
-    public UnboundRole everyone() {
-        return new UnboundRole(RoleDefinition.everyone);
-    }
-
-
-    // Athenz based roles.
-    /** Returns a {@link RoleDefinition#athenzTenantAdmin} for the current system and given tenant. */
-    public TenantRole athenzTenantAdmin(TenantName tenant) {
-        return new TenantRole(RoleDefinition.athenzTenantAdmin, tenant);
-    }
-
-    /** Returns a {@link RoleDefinition#tenantPipeline} for the current system and given tenant and application. */
-    public ApplicationRole tenantPipeline(TenantName tenant, ApplicationName application) {
-        return new ApplicationRole(RoleDefinition.tenantPipeline, tenant, application);
-    }
-
-
-    // Other identity provider based roles.
-    /** Returns a {@link RoleDefinition#tenantOwner} for the current system and given tenant. */
-    public TenantRole tenantOwner(TenantName tenant) {
-        return new TenantRole(RoleDefinition.tenantOwner, tenant);
-    }
-
-    /** Returns a {@link RoleDefinition#tenantAdmin} for the current system and given tenant. */
-    public TenantRole tenantAdmin(TenantName tenant) {
-        return new TenantRole(RoleDefinition.tenantAdmin, tenant);
-    }
-
-    /** Returns a {@link RoleDefinition#tenantOperator} for the current system and given tenant. */
-    public TenantRole tenantOperator(TenantName tenant) {
-        return new TenantRole(RoleDefinition.tenantOperator, tenant);
-    }
-
-    /** Returns a {@link RoleDefinition#applicationAdmin} for the current system and given tenant and application. */
-    public ApplicationRole applicationAdmin(TenantName tenant, ApplicationName application) {
-        return new ApplicationRole(RoleDefinition.applicationAdmin, tenant, 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(RoleDefinition.applicationOperator, tenant, 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(RoleDefinition.applicationDeveloper, tenant, 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(RoleDefinition.applicationReader, tenant, 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(RoleDefinition.buildService, tenant, application);
-    }
-
-}
diff --git a/controller-api/src/test/java/com/yahoo/vespa/hosted/controller/api/integration/user/UserRolesTest.java b/controller-api/src/test/java/com/yahoo/vespa/hosted/controller/api/integration/user/UserRolesTest.java
index 89df7a24559..c8e3d1987c9 100644
--- a/controller-api/src/test/java/com/yahoo/vespa/hosted/controller/api/integration/user/UserRolesTest.java
+++ b/controller-api/src/test/java/com/yahoo/vespa/hosted/controller/api/integration/user/UserRolesTest.java
@@ -1,10 +1,9 @@
 package com.yahoo.vespa.hosted.controller.api.integration.user;
 
 import com.yahoo.config.provision.ApplicationName;
-import com.yahoo.config.provision.SystemName;
 import com.yahoo.config.provision.TenantName;
 import com.yahoo.vespa.hosted.controller.api.role.ApplicationRole;
-import com.yahoo.vespa.hosted.controller.api.role.Roles;
+import com.yahoo.vespa.hosted.controller.api.role.Role;
 import com.yahoo.vespa.hosted.controller.api.role.TenantRole;
 import org.junit.Test;
 
@@ -15,8 +14,7 @@ import static org.junit.Assert.assertEquals;
  */
 public class UserRolesTest {
 
-    private static final Roles roles = new Roles(SystemName.main);
-    private static final UserRoles userRoles = new UserRoles(roles);
+    private static final UserRoles userRoles = new UserRoles();
 
     @Test
     public void testSerialization() {
@@ -28,25 +26,25 @@ public class UserRolesTest {
         for (ApplicationRole role : userRoles.applicationRoles(tenant, application))
             assertEquals(role, userRoles.toRole(UserRoles.valueOf(role)));
 
-        assertEquals(roles.tenantOperator(tenant),
+        assertEquals(Role.tenantOperator(tenant),
                      userRoles.toRole("my-tenant.tenantOperator"));
-        assertEquals(roles.applicationReader(tenant, application),
+        assertEquals(Role.applicationReader(tenant, application),
                      userRoles.toRole("my-tenant.my-application.applicationReader"));
     }
 
     @Test(expected = IllegalArgumentException.class)
     public void illegalTenantName() {
-        UserRoles.valueOf(roles.tenantAdmin(TenantName.from("my.tenant")));
+        UserRoles.valueOf(Role.tenantAdmin(TenantName.from("my.tenant")));
     }
 
     @Test(expected = IllegalArgumentException.class)
     public void illegalApplicationName() {
-        UserRoles.valueOf(roles.applicationOperator(TenantName.from("my-tenant"), ApplicationName.from("my.app")));
+        UserRoles.valueOf(Role.applicationOperator(TenantName.from("my-tenant"), ApplicationName.from("my.app")));
     }
 
     @Test(expected = IllegalArgumentException.class)
     public void illegalRole() {
-        UserRoles.valueOf(roles.tenantPipeline(TenantName.from("my-tenant"), ApplicationName.from("my-app")));
+        UserRoles.valueOf(Role.tenantPipeline(TenantName.from("my-tenant"), ApplicationName.from("my-app")));
     }
 
     @Test(expected = IllegalArgumentException.class)
@@ -66,7 +64,7 @@ public class UserRolesTest {
 
     @Test
     public void allowHostedOperator() {
-        assertEquals(roles.hostedOperator(), userRoles.toRole("hostedOperator"));
+        assertEquals(Role.hostedOperator(), userRoles.toRole("hostedOperator"));
     }
 
 }
diff --git a/controller-api/src/test/java/com/yahoo/vespa/hosted/controller/api/role/RoleTest.java b/controller-api/src/test/java/com/yahoo/vespa/hosted/controller/api/role/RoleTest.java
index 2bdd516aba2..2ce565de01a 100644
--- a/controller-api/src/test/java/com/yahoo/vespa/hosted/controller/api/role/RoleTest.java
+++ b/controller-api/src/test/java/com/yahoo/vespa/hosted/controller/api/role/RoleTest.java
@@ -21,7 +21,7 @@ public class RoleTest {
 
     @Test
     public void operator_membership() {
-        Role role = new Roles(SystemName.main).hostedOperator();
+        Role role = Role.hostedOperator();
 
         // Operator actions
         assertFalse(role.allows(Action.create, URI.create("/not/explicitly/defined"), mainEnforcer));
@@ -33,13 +33,13 @@ public class RoleTest {
 
     @Test
     public void tenant_membership() {
-        Role role = new Roles(SystemName.main).athenzTenantAdmin(TenantName.from("t1"));
+        Role role = Role.athenzTenantAdmin(TenantName.from("t1"));
         assertFalse(role.allows(Action.create, URI.create("/not/explicitly/defined"), mainEnforcer));
         assertFalse("Deny access to operator API", role.allows(Action.create, URI.create("/controller/v1/foo"), mainEnforcer));
         assertFalse("Deny access to other tenant and app", role.allows(Action.update, URI.create("/application/v4/tenant/t2/application/a2"), mainEnforcer));
         assertTrue(role.allows(Action.update, URI.create("/application/v4/tenant/t1/application/a1"), mainEnforcer));
 
-        Role publicSystem = new Roles(SystemName.vaas).athenzTenantAdmin(TenantName.from("t1"));
+        Role publicSystem = Role.athenzTenantAdmin(TenantName.from("t1"));
         assertFalse(publicSystem.allows(Action.read, URI.create("/controller/v1/foo"), vaasEnforcer));
         assertTrue(publicSystem.allows(Action.read, URI.create("/badge/v1/badge"), vaasEnforcer));
         assertTrue(publicSystem.allows(Action.update, URI.create("/application/v4/tenant/t1/application/a1"), vaasEnforcer));
@@ -47,7 +47,7 @@ public class RoleTest {
 
     @Test
     public void build_service_membership() {
-        Role role = new Roles(SystemName.vaas).tenantPipeline(TenantName.from("t1"), ApplicationName.from("a1"));
+        Role role = Role.tenantPipeline(TenantName.from("t1"), ApplicationName.from("a1"));
         assertFalse(role.allows(Action.create, URI.create("/not/explicitly/defined"), vaasEnforcer));
         assertFalse(role.allows(Action.update, URI.create("/application/v4/tenant/t1/application/a1"), vaasEnforcer));
         assertTrue(role.allows(Action.create, URI.create("/application/v4/tenant/t1/application/a1/jobreport"), vaasEnforcer));
@@ -56,22 +56,21 @@ public class RoleTest {
 
     @Test
     public void implications() {
-        Roles roles = new Roles(SystemName.main);
         TenantName tenant1 = TenantName.from("t1");
         ApplicationName application1 = ApplicationName.from("a1");
         TenantName tenant2 = TenantName.from("t2");
         ApplicationName application2 = ApplicationName.from("a2");
 
-        Role tenantOwner1 = roles.tenantOwner(tenant1);
-        Role tenantAdmin1 = roles.tenantAdmin(tenant1);
-        Role tenantAdmin2 = roles.tenantAdmin(tenant2);
-        Role tenantOperator1 = roles.tenantOperator(tenant1);
-        Role applicationAdmin11 = roles.applicationAdmin(tenant1, application1);
-        Role applicationOperator11 = roles.applicationOperator(tenant1, application1);
-        Role applicationDeveloper11 = roles.applicationDeveloper(tenant1, application1);
-        Role applicationReader11 = roles.applicationReader(tenant1, application1);
-        Role applicationReader12 = roles.applicationReader(tenant1, application2);
-        Role applicationReader22 = roles.applicationReader(tenant2, application2);
+        Role tenantOwner1 = Role.tenantOwner(tenant1);
+        Role tenantAdmin1 = Role.tenantAdmin(tenant1);
+        Role tenantAdmin2 = Role.tenantAdmin(tenant2);
+        Role tenantOperator1 = Role.tenantOperator(tenant1);
+        Role applicationAdmin11 = Role.applicationAdmin(tenant1, application1);
+        Role applicationOperator11 = Role.applicationOperator(tenant1, application1);
+        Role applicationDeveloper11 = Role.applicationDeveloper(tenant1, application1);
+        Role applicationReader11 = Role.applicationReader(tenant1, application1);
+        Role applicationReader12 = Role.applicationReader(tenant1, application2);
+        Role applicationReader22 = Role.applicationReader(tenant2, application2);
 
         assertFalse(tenantOwner1.implies(tenantOwner1));
         assertTrue(tenantOwner1.implies(tenantAdmin1));