Merge pull request #12452 from vespa-engine/mpolden/remove-unused-roles

Remove unused roles

5 files changed, 16 insertions, 20 deletions

diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/filter/AthenzRoleFilter.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/filter/AthenzRoleFilter.java
index 30f0d545ffe..4ae3c38bdf2 100644
--- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/filter/AthenzRoleFilter.java
+++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/filter/AthenzRoleFilter.java
@@ -110,7 +110,7 @@ public class AthenzRoleFilter extends JsonSecurityRequestFilterBase {
             futures.add(executor.submit(() -> {
                         if (   tenant.get().type() != Tenant.Type.athenz
                             || hasDeployerAccess(identity, ((AthenzTenant) tenant.get()).domain(), application.get()))
-                            roleMemberships.add(Role.tenantPipeline(tenant.get().name(), application.get()));
+                            roleMemberships.add(Role.buildService(tenant.get().name(), application.get()));
             }));
 
         futures.add(executor.submit(() -> {
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/application/ApplicationApiTest.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/ApplicationApiTest.java
index 852d4a98022..90254825f4c 100644
--- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/ApplicationApiTest.java
+++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/ApplicationApiTest.java
@@ -588,9 +588,11 @@ public class ApplicationApiTest extends ControllerContainerTest {
 
         // POST a 'restart application' command
         tester.assertResponse(request("/application/v4/tenant/tenant1/application/application1/environment/prod/region/us-central-1/instance/instance1/restart", POST)
-                                      .screwdriverIdentity(SCREWDRIVER_ID),
+                                      .userIdentity(HOSTED_VESPA_OPERATOR),
                               "{\"message\":\"Requested restart of tenant1.application1.instance1 in prod.us-central-1\"}");
 
+        addUserToHostedOperatorRole(HostedAthenzIdentities.from(SCREWDRIVER_ID));
+
         // POST a 'restart application' in staging environment command
         tester.assertResponse(request("/application/v4/tenant/tenant1/application/application1/environment/staging/region/us-central-1/instance/instance1/restart", POST)
                                       .screwdriverIdentity(SCREWDRIVER_ID),
@@ -941,10 +943,8 @@ public class ApplicationApiTest extends ControllerContainerTest {
                                       .oktaAccessToken(OKTA_AT).oktaIdentityToken(OKTA_IT),
                               new File("instance-reference.json"));
 
-        // Grant deploy access
-        addScrewdriverUserToDeployRole(SCREWDRIVER_ID,
-                                       ATHENZ_TENANT_DOMAIN,
-                                       ApplicationName.from("application1"));
+        // Add build service to operator role
+        addUserToHostedOperatorRole(HostedAthenzIdentities.from(SCREWDRIVER_ID));
 
         // POST (deploy) an application to a prod zone - allowed when project ID is not specified
         MultiPartStreamer entity = createApplicationDeployData(applicationPackageInstance1, true);
diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/filter/AthenzRoleFilterTest.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/filter/AthenzRoleFilterTest.java
index 4e06afea50d..c49f7a90194 100644
--- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/filter/AthenzRoleFilterTest.java
+++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/filter/AthenzRoleFilterTest.java
@@ -95,14 +95,14 @@ public class AthenzRoleFilterTest {
         assertEquals(Set.of(Role.athenzTenantAdmin(TENANT)),
                      filter.roles(TENANT_ADMIN, APPLICATION2_CONTEXT_PATH));
 
-        // Build services are members of the tenantPipeline role within their application subtree.
+        // Build services are members of the buildService role within their application subtree.
         assertEquals(Set.of(Role.everyone()),
                      filter.roles(TENANT_PIPELINE, NO_CONTEXT_PATH));
 
         assertEquals(Set.of(Role.everyone()),
                      filter.roles(TENANT_PIPELINE, TENANT_CONTEXT_PATH));
 
-        assertEquals(Set.of(Role.tenantPipeline(TENANT, APPLICATION)),
+        assertEquals(Set.of(Role.buildService(TENANT, APPLICATION)),
                      filter.roles(TENANT_PIPELINE, APPLICATION_CONTEXT_PATH));
 
         assertEquals(Set.of(Role.everyone()),
@@ -112,7 +112,7 @@ public class AthenzRoleFilterTest {
         assertEquals(Set.of(Role.athenzTenantAdmin(TENANT)),
                      filter.roles(TENANT_ADMIN_AND_PIPELINE, TENANT_CONTEXT_PATH));
 
-        assertEquals(Set.of(Role.athenzTenantAdmin(TENANT), Role.tenantPipeline(TENANT, APPLICATION)),
+        assertEquals(Set.of(Role.athenzTenantAdmin(TENANT), Role.buildService(TENANT, APPLICATION)),
                      filter.roles(TENANT_ADMIN_AND_PIPELINE, APPLICATION_CONTEXT_PATH));
 
         // Users have nothing special under their instance
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