Remove unused tenant and application roles

8 files changed, 26 insertions, 161 deletions

diff --git a/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/stubs/MockUserManagement.java b/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/stubs/MockUserManagement.java
index 6df44ceaf9d..295f8e8fd98 100644
--- a/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/stubs/MockUserManagement.java
+++ b/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/stubs/MockUserManagement.java
@@ -22,6 +22,14 @@ public class MockUserManagement implements UserManagement {
 
     private final Map<Role, Set<User>> memberships = new HashMap<>();
 
+    private Set<User> get(Role role) {
+        var membership = memberships.get(role);
+        if (membership == null) {
+            throw new IllegalArgumentException(role + " not found");
+        }
+        return membership;
+    }
+
     @Override
     public void createRole(Role role) {
         if (memberships.containsKey(role))
@@ -40,7 +48,7 @@ public class MockUserManagement implements UserManagement {
         List<User> userObjs = users.stream()
                                    .map(id -> new User(id.value(), id.value(), null, null))
                                    .collect(Collectors.toList());
-        memberships.get(role).addAll(userObjs);
+        get(role).addAll(userObjs);
     }
 
     @Override
@@ -52,7 +60,7 @@ public class MockUserManagement implements UserManagement {
 
     @Override
     public void removeUsers(Role role, Collection<UserId> users) {
-        memberships.get(role).removeIf(user -> users.contains(new UserId(user.email())));
+        get(role).removeIf(user -> users.contains(new UserId(user.email())));
     }
 
     @Override
@@ -64,7 +72,7 @@ public class MockUserManagement implements UserManagement {
 
     @Override
     public List<User> listUsers(Role role) {
-        return List.copyOf(memberships.get(role));
+        return List.copyOf(get(role));
     }
 
 }
diff --git a/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/user/Roles.java b/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/user/Roles.java
index f5f3ebe8f35..578f516f01e 100644
--- a/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/user/Roles.java
+++ b/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/user/Roles.java
@@ -44,9 +44,6 @@ public class Roles {
     /** Returns the {@link Role} the given tenant, application and role names correspond to. */
     public static Role toRole(TenantName tenant, String roleName) {
         switch (roleName) {
-            case "tenantOwner": return Role.tenantOwner(tenant);
-            case "tenantAdmin": return Role.tenantAdmin(tenant);
-            case "tenantOperator": return Role.tenantOperator(tenant);
             case "administrator": return Role.administrator(tenant);
             case "developer": return Role.developer(tenant);
             case "reader": return Role.reader(tenant);
@@ -57,10 +54,6 @@ public class Roles {
     /** Returns the {@link Role} the given tenant and role names correspond to. */
     public static Role toRole(TenantName tenant, ApplicationName application, String roleName) {
         switch (roleName) {
-            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);
             case "headless": return Role.headless(tenant, application);
             default: throw new IllegalArgumentException("Malformed or illegal role name '" + roleName + "'.");
         }
@@ -97,13 +90,6 @@ public class Roles {
 
     private static String valueOf(RoleDefinition role) {
         switch (role) {
-            case tenantOwner:          return "tenantOwner";
-            case tenantAdmin:          return "tenantAdmin";
-            case tenantOperator:       return "tenantOperator";
-            case applicationAdmin:     return "applicationAdmin";
-            case applicationOperator:  return "applicationOperator";
-            case applicationDeveloper: return "applicationDeveloper";
-            case applicationReader:    return "applicationReader";
             case administrator:        return "administrator";
             case developer:            return "developer";
             case reader:               return "reader";
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 263e3284dbd..c2203e3da40 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
@@ -53,21 +53,6 @@ public abstract class Role {
         return new InstanceRole(RoleDefinition.athenzUser, tenant, application, instance);
     }
 
-    /** 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#reader} for the current system and given tenant. */
     public static TenantRole reader(TenantName tenant) {
         return new TenantRole(RoleDefinition.reader, tenant);
@@ -88,26 +73,6 @@ public abstract class Role {
         return new ApplicationRole(RoleDefinition.headless, tenant, application);
     }
 
-    /** 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);
diff --git a/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/role/RoleDefinition.java b/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/role/RoleDefinition.java
index 848866f7c33..0d7222781fa 100644
--- a/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/role/RoleDefinition.java
+++ b/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/role/RoleDefinition.java
@@ -31,50 +31,13 @@ public enum RoleDefinition {
              Policy.user,
              Policy.tenantCreate),
 
-    /** Application reader which can see all information about an application, its tenant and deployments. */
-    applicationReader(everyone,
-                      Policy.tenantRead,
-                      Policy.applicationRead,
-                      Policy.deploymentRead),
-
     /** Build service which may submit new applications for continuous deployment. */
-    buildService(applicationReader,
+    buildService(everyone,
+                 Policy.tenantRead,
+                 Policy.applicationRead,
+                 Policy.deploymentRead,
                  Policy.submission),
 
-    /** Application developer with access to deploy to development zones. */
-    applicationDeveloper(applicationReader,
-                         Policy.developmentDeployment),
-
-    /** Application operator with access to normal, operational tasks of an application. */
-    applicationOperator(applicationReader,
-                        Policy.applicationOperations),
-
-    /** Application administrator with full access to an already existing application, including emergency operations. */
-    applicationAdmin(applicationDeveloper,
-                     applicationOperator,
-                     Policy.applicationUpdate,
-                     Policy.applicationDelete,
-                     Policy.applicationManager,
-                     Policy.productionDeployment,
-                     Policy.submission),
-
-    /** Tenant operator with access to create application under a tenant, and to read the tenant's and public data. */
-    tenantOperator(everyone,
-                   Policy.tenantRead,
-                   Policy.applicationCreate,
-                   Policy.keyManagement),
-
-    /** Tenant admin with full access to all tenant resources, except deleting the tenant. */
-    tenantAdmin(tenantOperator,
-                applicationAdmin,
-                Policy.applicationDelete,
-                Policy.tenantManager,
-                Policy.tenantUpdate),
-
-    /** Tenant admin with full access to all tenant resources. */
-    tenantOwner(tenantAdmin,
-                Policy.tenantDelete),
-
     /** Reader — the base role for all tenant users */
     reader(Policy.tenantRead,
            Policy.applicationRead,
diff --git a/controller-api/src/test/java/com/yahoo/vespa/hosted/controller/api/integration/user/RolesTest.java b/controller-api/src/test/java/com/yahoo/vespa/hosted/controller/api/integration/user/RolesTest.java
index 22baedd16b4..2e7308e41e1 100644
--- a/controller-api/src/test/java/com/yahoo/vespa/hosted/controller/api/integration/user/RolesTest.java
+++ b/controller-api/src/test/java/com/yahoo/vespa/hosted/controller/api/integration/user/RolesTest.java
@@ -29,11 +29,6 @@ public class RolesTest {
                      Roles.toRole("hostedOperator"));
         assertEquals(Role.hostedSupporter(),
                      Roles.toRole("hostedSupporter"));
-        assertEquals(Role.tenantOperator(tenant),
-                     Roles.toRole("my-tenant.tenantOperator"));
-        assertEquals(Role.applicationReader(tenant, application),
-                     Roles.toRole("my-tenant.my-application.applicationReader"));
-
         assertEquals(Role.administrator(tenant), Roles.toRole("my-tenant.administrator"));
         assertEquals(Role.developer(tenant), Roles.toRole("my-tenant.developer"));
         assertEquals(Role.reader(tenant), Roles.toRole("my-tenant.reader"));
@@ -42,12 +37,12 @@ public class RolesTest {
 
     @Test(expected = IllegalArgumentException.class)
     public void illegalTenantName() {
-        Roles.valueOf(Role.tenantAdmin(TenantName.from("my.tenant")));
+        Roles.valueOf(Role.developer(TenantName.from("my.tenant")));
     }
 
     @Test(expected = IllegalArgumentException.class)
     public void illegalApplicationName() {
-        Roles.valueOf(Role.applicationOperator(TenantName.from("my-tenant"), ApplicationName.from("my.app")));
+        Roles.valueOf(Role.headless(TenantName.from("my-tenant"), ApplicationName.from("my.app")));
     }
 
     @Test(expected = IllegalArgumentException.class)
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 5348185c276..f0483e55b3e 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
@@ -90,54 +90,6 @@ public class RoleTest {
     }
 
     @Test
-    public void implications() {
-        TenantName tenant1 = TenantName.from("t1");
-        ApplicationName application1 = ApplicationName.from("a1");
-        TenantName tenant2 = TenantName.from("t2");
-        ApplicationName application2 = ApplicationName.from("a2");
-
-        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));
-        assertFalse(tenantOwner1.implies(tenantAdmin2));
-        assertTrue(tenantOwner1.implies(tenantOperator1));
-        assertTrue(tenantOwner1.implies(applicationAdmin11));
-        assertTrue(tenantOwner1.implies(applicationReader11));
-        assertTrue(tenantOwner1.implies(applicationReader12));
-        assertFalse(tenantOwner1.implies(applicationReader22));
-
-        assertFalse(tenantAdmin1.implies(tenantOwner1));
-        assertFalse(tenantAdmin1.implies(tenantAdmin2));
-        assertTrue(tenantAdmin1.implies(applicationDeveloper11));
-
-        assertFalse(tenantOperator1.implies(applicationReader11));
-
-        assertFalse(applicationAdmin11.implies(tenantAdmin1));
-        assertFalse(applicationAdmin11.implies(tenantOperator1));
-        assertTrue(applicationAdmin11.implies(applicationOperator11));
-        assertTrue(applicationAdmin11.implies(applicationDeveloper11));
-        assertTrue(applicationAdmin11.implies(applicationReader11));
-        assertFalse(applicationAdmin11.implies(applicationReader12));
-        assertFalse(applicationAdmin11.implies(applicationReader22));
-
-        assertFalse(applicationOperator11.implies(applicationDeveloper11));
-        assertTrue(applicationOperator11.implies(applicationReader11));
-
-        assertFalse(applicationDeveloper11.implies(applicationOperator11));
-        assertTrue(applicationDeveloper11.implies(applicationReader11));
-    }
-
-    @Test
     public void new_implications() {
         TenantName tenant1 = TenantName.from("t1");
         ApplicationName application1 = ApplicationName.from("a1");
@@ -188,8 +140,8 @@ public class RoleTest {
         // Write
         {
             var url = URI.create("/routing/v1/inactive/tenant/t1/application/a1/instance/i1/environment/prod/region/us-north-1");
-            var allowedRole = Role.applicationAdmin(TenantName.from("t1"), ApplicationName.from("a1"));
-            var disallowedRole = Role.applicationAdmin(TenantName.from("t2"), ApplicationName.from("a2"));
+            var allowedRole = Role.developer(TenantName.from("t1"));
+            var disallowedRole = Role.developer(TenantName.from("t2"));
             assertTrue(allowedRole + " can override status at " + url, mainEnforcer.allows(allowedRole, Action.create, url));
             assertTrue(allowedRole + " can clear status at " + url, mainEnforcer.allows(allowedRole, Action.delete, url));
             assertFalse(disallowedRole + " cannot override status at " + url, mainEnforcer.allows(disallowedRole, Action.create, url));
diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/user/UserApiHandler.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/user/UserApiHandler.java
index 66cbf4d17ef..0e3295b1143 100644
--- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/user/UserApiHandler.java
+++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/user/UserApiHandler.java
@@ -335,13 +335,6 @@ public class UserApiHandler extends LoggingRequestHandler {
 
     private static String valueOf(Role role) {
         switch (role.definition()) {
-            case tenantOwner:           return "tenantOwner";
-            case tenantAdmin:           return "tenantAdmin";
-            case tenantOperator:        return "tenantOperator";
-            case applicationAdmin:      return "applicationAdmin";
-            case applicationOperator:   return "applicationOperator";
-            case applicationDeveloper:  return "applicationDeveloper";
-            case applicationReader:     return "applicationReader";
             case administrator:         return "administrator";
             case developer:             return "developer";
             case reader:                return "reader";
diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/user/UserApiTest.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/user/UserApiTest.java
index 93d88ff8abd..6db5bc9f523 100644
--- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/user/UserApiTest.java
+++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/user/UserApiTest.java
@@ -102,7 +102,7 @@ public class UserApiTest extends ControllerContainerCloudTest {
         tester.assertResponse(request("/user/v1/tenant/my-tenant/application/my-app", POST)
                                       .roles(Set.of(Role.administrator(TenantName.from("my-tenant"))))
                                       .data("{\"user\":\"headless@app\",\"roleName\":\"headless\"}"),
-                              "{\"error-code\":\"INTERNAL_SERVER_ERROR\",\"message\":\"NullPointerException\"}", 500);
+                              "{\"error-code\":\"BAD_REQUEST\",\"message\":\"role 'headless' of 'my-app' owned by 'my-tenant' not found\"}", 400);
 
         // POST an application is allowed for a tenant developer.
         tester.assertResponse(request("/application/v4/tenant/my-tenant/application/my-app", POST)
@@ -193,10 +193,13 @@ public class UserApiTest extends ControllerContainerCloudTest {
                              .data("{\"user\":\"administrator@tenant\",\"roleName\":\"administrator\"}"),
                               "{\"error-code\":\"BAD_REQUEST\",\"message\":\"Can't remove the last administrator of a tenant.\"}", 400);
 
-        // DELETE the tenant is available to the tenant owner.
+        // DELETE the tenant is not allowed
         tester.assertResponse(request("/application/v4/tenant/my-tenant", DELETE)
-                                      .roles(Set.of(Role.tenantOwner(id.tenant()))),
-                              new File("tenant-without-applications.json"));
+                                      .roles(Set.of(Role.developer(id.tenant()))),
+                              "{\n" +
+                              "  \"code\" : 403,\n" +
+                              "  \"message\" : \"Access denied\"\n" +
+                              "}", 403);
     }
 
     @Test