Merge pull request #20317 from vespa-engine/olaa/delete-tenant-without-domain

Delete tenants without Athenz domain

7 files changed, 79 insertions, 9 deletions

diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/ApplicationController.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/ApplicationController.java
index 9e7c614d4e8..49939f4bfd2 100644
--- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/ApplicationController.java
+++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/ApplicationController.java
@@ -561,6 +561,10 @@ public class ApplicationController {
      * @throws IllegalArgumentException if the application has deployments or the caller is not authorized
      */
     public void deleteApplication(TenantAndApplicationId id, Credentials credentials) {
+        deleteApplication(id, Optional.of(credentials));
+    }
+
+    public void deleteApplication(TenantAndApplicationId id, Optional<Credentials> credentials) {
         lockApplicationOrThrow(id, application -> {
             var deployments = application.get().instances().values().stream()
                                          .filter(instance -> ! instance.deployments().isEmpty())
@@ -580,7 +584,7 @@ public class ApplicationController {
             applicationStore.removeAllTesters(id.tenant(), id.application());
             applicationStore.putMetaTombstone(id.tenant(), id.application(), clock.instant());
 
-            accessControl.deleteApplication(id, credentials);
+            credentials.ifPresent(creds -> accessControl.deleteApplication(id, creds));
             curator.removeApplication(id);
 
             controller.jobController().collectGarbage();
diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/TenantController.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/TenantController.java
index 537603427f5..59877fce634 100644
--- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/TenantController.java
+++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/TenantController.java
@@ -161,7 +161,7 @@ public class TenantController {
     }
 
     /** Deletes the given tenant. */
-    public void delete(TenantName tenant, Supplier<Credentials> credentials, boolean forget) {
+    public void delete(TenantName tenant, Optional<Credentials> credentials, boolean forget) {
         try (Lock lock = lock(tenant)) {
             Tenant oldTenant = get(tenant, true)
                     .orElseThrow(() -> new NotExistsException("Could not delete tenant '" + tenant + "': Tenant not found"));
@@ -171,7 +171,7 @@ public class TenantController {
                     throw new IllegalArgumentException("Could not delete tenant '" + tenant.value()
                             + "': This tenant has active applications");
 
-                accessControl.deleteTenant(tenant, credentials.get());
+                credentials.ifPresent(creds -> accessControl.deleteTenant(tenant, creds));
                 controller.notificationsDb().removeNotifications(NotificationSource.from(tenant));
             }
 
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 33012763f97..05a7e2368d1 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
@@ -8,12 +8,13 @@ import com.yahoo.vespa.hosted.controller.Controller;
 import com.yahoo.vespa.hosted.controller.api.integration.user.RoleMaintainer;
 
 import java.time.Duration;
+import java.util.Optional;
 import java.util.logging.Logger;
 import java.util.stream.Collectors;
 
 /**
  * Maintains user management resources.
- * For now, ensures there's no discrepnacy between expected tenant/application roles and Auth0 roles
+ * For now, ensures there's no discrepnacy between expected tenant/application roles and auth0/athenz roles
  *
  * @author olaa
  */
@@ -39,8 +40,10 @@ public class UserManagementMaintainer extends ControllerMaintainer {
         if (!controller().system().isPublic()) {
             roleMaintainer.tenantsToDelete(tenants)
                     .forEach(tenant -> {
-                        // TODO: controller().tenants().delete(tenant.name());
-                        logger.fine("Want to delete tenant " + tenant.name());
+                        logger.warning(tenant.name() + " has a non-existing Athenz domain. Deleting");
+                        controller().applications().asList(tenant.name())
+                                .forEach(application -> controller().applications().deleteApplication(application.id(), Optional.empty()));
+                        controller().tenants().delete(tenant.name(), Optional.empty(), false);
                     });
         }
 
diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/application/ApplicationApiHandler.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/application/ApplicationApiHandler.java
index cf85d862041..8af26f564a6 100644
--- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/application/ApplicationApiHandler.java
+++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/application/ApplicationApiHandler.java
@@ -2018,9 +2018,9 @@ public class ApplicationApiHandler extends AuditLoggingRequestHandler {
             return ErrorResponse.forbidden("Only operators can forget a tenant");
 
         controller.tenants().delete(TenantName.from(tenantName),
-                                    () -> accessControlRequests.credentials(TenantName.from(tenantName),
+                                    Optional.of(accessControlRequests.credentials(TenantName.from(tenantName),
                                                                       toSlime(request.getData()).get(),
-                                                                      request.getJDiscRequest()),
+                                                                      request.getJDiscRequest())),
                                     forget);
 
         return new MessageResponse("Deleted tenant " + tenantName);
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 b1311b8081c..b81b3ae5d66 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
@@ -88,7 +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();
+    private final RoleMaintainerMock roleMaintainer = new RoleMaintainerMock();
 
     public ServiceRegistryMock(SystemName system) {
         this.zoneRegistryMock = new ZoneRegistryMock(system);
@@ -291,4 +291,7 @@ public class ServiceRegistryMock extends AbstractComponent implements ServiceReg
         return endpointCertificateMock;
     }
 
+    public RoleMaintainerMock roleMaintainerMock() {
+        return roleMaintainer;
+    }
 }
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
new file mode 100644
index 00000000000..e35c2058eb4
--- /dev/null
+++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/UserManagementMaintainerTest.java
@@ -0,0 +1,58 @@
+// 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.SystemName;
+import com.yahoo.vespa.hosted.controller.ControllerTester;
+import org.junit.Test;
+
+import java.time.Duration;
+
+import static org.junit.Assert.*;
+
+/**
+ * @author olaa
+ */
+public class UserManagementMaintainerTest {
+
+    private final String TENANT_1 = "tenant1";
+    private final String TENANT_2 = "tenant2";
+    private final String APP_NAME = "some-app";
+
+    @Test
+    public void deletes_tenant_when_not_public() {
+        var tester = createTester(SystemName.main);
+        var maintainer = new UserManagementMaintainer(tester.controller(), Duration.ofMinutes(5), tester.serviceRegistry().roleMaintainer());
+        maintainer.maintain();
+
+        var tenants = tester.controller().tenants().asList();
+        var apps = tester.controller().applications().asList();
+        assertEquals(1, tenants.size());
+        assertEquals(1, apps.size());
+        assertEquals(TENANT_2, tenants.get(0).name().value());
+    }
+
+    @Test
+    public void no_tenant_deletion_in_public() {
+        var tester = createTester(SystemName.Public);
+        var maintainer = new UserManagementMaintainer(tester.controller(), Duration.ofMinutes(5), tester.serviceRegistry().roleMaintainer());
+        maintainer.maintain();
+
+        var tenants = tester.controller().tenants().asList();
+        var apps = tester.controller().applications().asList();
+        assertEquals(2, tenants.size());
+        assertEquals(2, apps.size());
+    }
+
+    private ControllerTester createTester(SystemName systemName) {
+        var tester = new ControllerTester(systemName);
+        tester.createTenant(TENANT_1);
+        tester.createTenant(TENANT_2);
+        tester.createApplication(TENANT_1, APP_NAME);
+        tester.createApplication(TENANT_2, APP_NAME);
+
+        var tenantToDelete = tester.controller().tenants().get(TENANT_1).get();
+        tester.serviceRegistry().roleMaintainerMock().mockTenantToDelete(tenantToDelete);
+        return tester;
+    }
+
+}
\ No newline at end of file
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 6cf3e89bdfe..b6aa2313ab3 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
@@ -894,6 +894,8 @@ public class ApplicationApiTest extends ControllerContainerTest {
 
         // Forget a deleted tenant
         tester.assertResponse(request("/application/v4/tenant/tenant1", DELETE).properties(Map.of("forget", "true"))
+                        .data("{\"athensDomain\":\"domain1\"}")
+                        .oktaAccessToken(OKTA_AT).oktaIdentityToken(OKTA_IT)
                         .userIdentity(HOSTED_VESPA_OPERATOR),
                 "{\"message\":\"Deleted tenant tenant1\"}");
         tester.assertResponse(request("/application/v4/tenant/tenant1", GET).properties(Map.of("includeDeleted", "true"))