Merge pull request #20166 from vespa-engine/olaa/clean-up-delegated-roles

Moves role maintainer to controller-api. 

4 files changed, 27 insertions, 81 deletions

diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/ControllerMaintenance.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/ControllerMaintenance.java
index f11cd78c303..913d6dfeab8 100644
--- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/ControllerMaintenance.java
+++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/ControllerMaintenance.java
@@ -74,7 +74,7 @@ public class ControllerMaintenance extends AbstractComponent {
         maintainers.add(new VcmrMaintainer(controller, intervals.vcmrMaintainer));
         maintainers.add(new CloudTrialExpirer(controller, intervals.defaultInterval));
         maintainers.add(new RetriggerMaintainer(controller, intervals.retriggerMaintainer));
-        maintainers.add(new UserManagementMaintainer(controller, intervals.userManagementMaintainer, userManagement));
+        maintainers.add(new UserManagementMaintainer(controller, intervals.userManagementMaintainer, controller.serviceRegistry().roleMaintainer()));
     }
 
     public Upgrader upgrader() { return upgrader; }
diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/UserManagementMaintainer.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/UserManagementMaintainer.java
index 5f6f917bc75..52073ad13dc 100644
--- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/UserManagementMaintainer.java
+++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/UserManagementMaintainer.java
@@ -1,17 +1,13 @@
 // Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root.
 package com.yahoo.vespa.hosted.controller.maintenance;
 
+import com.yahoo.config.provision.ApplicationId;
+import com.yahoo.config.provision.InstanceName;
 import com.yahoo.config.provision.SystemName;
-import com.yahoo.vespa.hosted.controller.Application;
 import com.yahoo.vespa.hosted.controller.Controller;
-import com.yahoo.vespa.hosted.controller.api.integration.user.Roles;
-import com.yahoo.vespa.hosted.controller.api.integration.user.UserManagement;
-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.TenantRole;
+import com.yahoo.vespa.hosted.controller.api.integration.user.RoleMaintainer;
 
 import java.time.Duration;
-import java.util.List;
 import java.util.logging.Logger;
 import java.util.stream.Collectors;
 
@@ -23,43 +19,32 @@ import java.util.stream.Collectors;
  */
 public class UserManagementMaintainer extends ControllerMaintainer {
 
-    private final UserManagement userManagement;
-
+    private final RoleMaintainer roleMaintainer;
     private static final Logger logger = Logger.getLogger(UserManagementMaintainer.class.getName());
 
-    public UserManagementMaintainer(Controller controller, Duration interval, UserManagement userManagement) {
+    public UserManagementMaintainer(Controller controller, Duration interval, RoleMaintainer roleMaintainer) {
         super(controller, interval, UserManagementMaintainer.class.getSimpleName(), SystemName.allOf(SystemName::isPublic));
-        this.userManagement = userManagement;
-
+        this.roleMaintainer = roleMaintainer;
     }
 
     @Override
     protected double maintain() {
-        findLeftoverRoles().forEach(role -> {
-            logger.warning(String.format("Found unexpected %s - Deleting", role.toString()));
-            userManagement.deleteRole(role);
-        });
-        return 1.0;
-    }
-
-    // protected for testing
-    protected List<Role> findLeftoverRoles() {
-        var tenantRoles = controller().tenants().asList()
+        var tenants = controller().tenants().asList();
+        var applications = controller().applications().idList()
                 .stream()
-                .flatMap(tenant -> Roles.tenantRoles(tenant.name()).stream())
+                .map(appId -> ApplicationId.from(appId.tenant(), appId.application(), InstanceName.defaultName()))
                 .collect(Collectors.toList());
+        roleMaintainer.deleteLeftoverRoles(tenants, applications);
 
-        var applicationRoles = controller().applications().asList()
-                .stream()
-                .map(Application::id)
-                .flatMap(applicationId -> Roles.applicationRoles(applicationId.tenant(), applicationId.application()).stream())
-                .collect(Collectors.toList());
+        if (!controller().system().isPublic()) {
+            roleMaintainer.tenantsToDelete(tenants)
+                    .forEach(tenant -> {
+                        // TODO: controller().tenants().delete(tenant.name());
+                        logger.fine("Want to delete tenant " + tenant.name());
+                    });
+        }
 
-        return userManagement.listRoles().stream()
-                .peek(role -> logger.fine(role::toString))
-                .filter(role -> role instanceof TenantRole || role instanceof ApplicationRole)
-                .filter(role -> !tenantRoles.contains(role) && !applicationRoles.contains(role))
-                .collect(Collectors.toList());
+        return 1.0;
     }
 
 }
diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/integration/ServiceRegistryMock.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/integration/ServiceRegistryMock.java
index 43ef9daa178..b1311b8081c 100644
--- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/integration/ServiceRegistryMock.java
+++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/integration/ServiceRegistryMock.java
@@ -44,6 +44,8 @@ import com.yahoo.vespa.hosted.controller.api.integration.stubs.MockMailer;
 import com.yahoo.vespa.hosted.controller.api.integration.stubs.MockMeteringClient;
 import com.yahoo.vespa.hosted.controller.api.integration.stubs.MockRunDataStore;
 import com.yahoo.vespa.hosted.controller.api.integration.stubs.MockTesterCloud;
+import com.yahoo.vespa.hosted.controller.api.integration.user.RoleMaintainer;
+import com.yahoo.vespa.hosted.controller.api.integration.user.RoleMaintainerMock;
 import com.yahoo.vespa.hosted.controller.api.integration.vcmr.MockChangeRequestClient;
 
 /**
@@ -86,6 +88,7 @@ public class ServiceRegistryMock extends AbstractComponent implements ServiceReg
     private final PlanRegistry planRegistry = new PlanRegistryMock();
     private final ResourceDatabaseClient resourceDb = new ResourceDatabaseClientMock(planRegistry);
     private final BillingDatabaseClient billingDb = new BillingDatabaseClientMock(clock, planRegistry);
+    private final RoleMaintainer roleMaintainer = new RoleMaintainerMock();
 
     public ServiceRegistryMock(SystemName system) {
         this.zoneRegistryMock = new ZoneRegistryMock(system);
@@ -267,6 +270,11 @@ public class ServiceRegistryMock extends AbstractComponent implements ServiceReg
         return billingDb;
     }
 
+    @Override
+    public RoleMaintainer roleMaintainer() {
+        return roleMaintainer;
+    }
+
     public ConfigServerMock configServerMock() {
         return configServerMock;
     }
diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/UserManagementMaintainerTest.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/UserManagementMaintainerTest.java
deleted file mode 100644
index 52cb3ce121f..00000000000
--- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/UserManagementMaintainerTest.java
+++ /dev/null
@@ -1,47 +0,0 @@
-// Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root.
-package com.yahoo.vespa.hosted.controller.maintenance;
-
-import com.yahoo.config.provision.ApplicationName;
-import com.yahoo.config.provision.TenantName;
-import com.yahoo.vespa.hosted.controller.ControllerTester;
-import com.yahoo.vespa.hosted.controller.api.integration.stubs.MockUserManagement;
-import com.yahoo.vespa.hosted.controller.api.integration.user.Roles;
-import com.yahoo.vespa.hosted.controller.api.integration.user.UserManagement;
-import com.yahoo.vespa.hosted.controller.api.role.Role;
-import org.junit.Test;
-
-import java.time.Duration;
-
-import static org.junit.Assert.*;
-
-/**
- * @author olaa
- */
-public class UserManagementMaintainerTest {
-
-    private final ControllerTester tester = new ControllerTester();
-    private final UserManagement userManagement = new MockUserManagement();
-    private final UserManagementMaintainer userManagementMaintainer = new UserManagementMaintainer(tester.controller(), Duration.ofMinutes(1), userManagement);
-
-    private final TenantName tenant = TenantName.from("tenant1");
-    private final ApplicationName app = ApplicationName.from("app1");
-    private final TenantName deletedTenant = TenantName.from("deleted-tenant");
-
-    @Test
-    public void finds_superfluous_roles() {
-        tester.createTenant(tenant.value());
-        tester.createApplication(tenant.value(), app.value());
-
-        Roles.tenantRoles(tenant).forEach(userManagement::createRole);
-        Roles.applicationRoles(tenant, app).forEach(userManagement::createRole);
-        Roles.tenantRoles(deletedTenant).forEach(userManagement::createRole);
-        userManagement.createRole(Role.hostedSupporter());
-
-        var expectedRoles = Roles.tenantRoles(deletedTenant);
-        var actualRoles = userManagementMaintainer.findLeftoverRoles();
-
-        assertEquals(expectedRoles.size(), actualRoles.size());
-        assertTrue(expectedRoles.containsAll(actualRoles) && actualRoles.containsAll(expectedRoles));
-    }
-
-}