Handle deleted tenants

7 files changed, 131 insertions, 38 deletions

diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/LockedTenant.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/LockedTenant.java
index f25f1c64372..8bd31af890b 100644
--- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/LockedTenant.java
+++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/LockedTenant.java
@@ -14,6 +14,7 @@ import com.yahoo.vespa.hosted.controller.api.integration.organization.Contact;
 import com.yahoo.vespa.hosted.controller.api.integration.secrets.TenantSecretStore;
 import com.yahoo.vespa.hosted.controller.tenant.AthenzTenant;
 import com.yahoo.vespa.hosted.controller.tenant.CloudTenant;
+import com.yahoo.vespa.hosted.controller.tenant.DeletedTenant;
 import com.yahoo.vespa.hosted.controller.tenant.LastLoginInfo;
 import com.yahoo.vespa.hosted.controller.tenant.Tenant;
 import com.yahoo.vespa.hosted.controller.tenant.TenantInfo;
@@ -47,9 +48,10 @@ public abstract class LockedTenant {
 
     static LockedTenant of(Tenant tenant, Lock lock) {
         switch (tenant.type()) {
-            case athenz: return new Athenz((AthenzTenant) tenant);
-            case cloud:  return new Cloud((CloudTenant) tenant);
-            default:     throw new IllegalArgumentException("Unexpected tenant type '" + tenant.getClass().getName() + "'.");
+            case athenz:  return new Athenz((AthenzTenant) tenant);
+            case cloud:   return new Cloud((CloudTenant) tenant);
+            case deleted: return new Deleted((DeletedTenant) tenant);
+            default:      throw new IllegalArgumentException("Unexpected tenant type '" + tenant.getClass().getName() + "'.");
         }
     }
 
@@ -58,6 +60,10 @@ public abstract class LockedTenant {
 
     public abstract LockedTenant with(LastLoginInfo lastLoginInfo);
 
+    public Deleted deleted(Instant deletedAt) {
+        return new Deleted(new DeletedTenant(name, createdAt, deletedAt));
+    }
+
     @Override
     public String toString() {
         return "tenant '" + name + "'";
@@ -183,4 +189,26 @@ public abstract class LockedTenant {
         }
     }
 
+
+    /** A locked DeletedTenant. */
+    public static class Deleted extends LockedTenant {
+
+        private final Instant deletedAt;
+
+        private Deleted(DeletedTenant tenant) {
+            super(tenant.name(), tenant.createdAt(), tenant.lastLoginInfo());
+            this.deletedAt = tenant.deletedAt();
+        }
+
+        @Override
+        public DeletedTenant get() {
+            return new DeletedTenant(name, createdAt, deletedAt);
+        }
+
+        @Override
+        public LockedTenant with(LastLoginInfo lastLoginInfo) {
+            return this;
+        }
+    }
+
 }
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 8ff68c4cba5..b1f431ad47b 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
@@ -4,10 +4,7 @@ package com.yahoo.vespa.hosted.controller;
 import com.yahoo.config.provision.TenantName;
 import com.yahoo.text.Text;
 import com.yahoo.vespa.curator.Lock;
-import com.yahoo.vespa.flags.BooleanFlag;
-import com.yahoo.vespa.flags.FetchVector;
 import com.yahoo.vespa.flags.FlagSource;
-import com.yahoo.vespa.flags.Flags;
 import com.yahoo.vespa.hosted.controller.api.identifiers.TenantId;
 import com.yahoo.vespa.hosted.controller.application.SystemApplication;
 import com.yahoo.vespa.hosted.controller.concurrent.Once;
@@ -16,6 +13,7 @@ import com.yahoo.vespa.hosted.controller.persistence.CuratorDb;
 import com.yahoo.vespa.hosted.controller.security.AccessControl;
 import com.yahoo.vespa.hosted.controller.security.Credentials;
 import com.yahoo.vespa.hosted.controller.security.TenantSpec;
+import com.yahoo.vespa.hosted.controller.tenant.DeletedTenant;
 import com.yahoo.vespa.hosted.controller.tenant.LastLoginInfo;
 import com.yahoo.vespa.hosted.controller.tenant.Tenant;
 
@@ -26,6 +24,7 @@ import java.util.List;
 import java.util.Objects;
 import java.util.Optional;
 import java.util.function.Consumer;
+import java.util.function.Supplier;
 import java.util.logging.Level;
 import java.util.logging.Logger;
 import java.util.stream.Collectors;
@@ -63,11 +62,17 @@ public class TenantController {
         });
     }
 
-    /** Returns a list of all known tenants sorted by name */
+    /** Returns a list of all known, non-deleted tenants sorted by name */
     public List<Tenant> asList() {
+        return asList(false);
+    }
+
+    /** Returns a list of all known tenants sorted by name */
+    public List<Tenant> asList(boolean includeDeleted) {
         return curator.readTenants().stream()
-                      .sorted(Comparator.comparing(Tenant::name))
-                      .collect(Collectors.toList());
+                .filter(tenant -> tenant.type() != Tenant.Type.deleted || includeDeleted)
+                .sorted(Comparator.comparing(Tenant::name))
+                .collect(Collectors.toList());
     }
 
     /** Locks a tenant for modification and applies the given action. */
@@ -110,8 +115,8 @@ public class TenantController {
     /** Create a tenant, provided the given credentials are valid. */
     public void create(TenantSpec tenantSpec, Credentials credentials) {
         try (Lock lock = lock(tenantSpec.tenant())) {
-            requireNonExistent(tenantSpec.tenant());
             TenantId.validate(tenantSpec.tenant().value());
+            requireNonExistent(tenantSpec.tenant());
             curator.writeTenant(accessControl.createTenant(tenantSpec, controller.clock().instant(), credentials, asList()));
 
             // We should create tenant roles here but it takes too long - assuming the TenantRoleMaintainer will do it Soon™
@@ -120,7 +125,12 @@ public class TenantController {
 
     /** Find tenant by name */
     public Optional<Tenant> get(TenantName name) {
-        return curator.readTenant(name);
+        return get(name, false);
+    }
+
+    public Optional<Tenant> get(TenantName name, boolean includeDeleted) {
+        return curator.readTenant(name)
+                .filter(tenant -> tenant.type() != Tenant.Type.deleted || includeDeleted);
     }
 
     /** Find tenant by name */
@@ -153,22 +163,28 @@ public class TenantController {
     }
 
     /** Deletes the given tenant. */
-    public void delete(TenantName tenant, Credentials credentials) {
+    public void delete(TenantName tenant, Supplier<Credentials> credentials, boolean forget) {
         try (Lock lock = lock(tenant)) {
-            require(tenant);
-            if ( ! controller.applications().asList(tenant).isEmpty())
-                throw new IllegalArgumentException("Could not delete tenant '" + tenant.value()
-                                                   + "': This tenant has active applications");
-
-            curator.removeTenant(tenant);
-            accessControl.deleteTenant(tenant, credentials);
-            controller.notificationsDb().removeNotifications(NotificationSource.from(tenant));
+            Tenant oldTenant = get(tenant, true)
+                    .orElseThrow(() -> new NotExistsException("Could not delete tenant '" + tenant + "': Tenant not found"));
+
+            if (oldTenant.type() != Tenant.Type.deleted) {
+                if (!controller.applications().asList(tenant).isEmpty())
+                    throw new IllegalArgumentException("Could not delete tenant '" + tenant.value()
+                            + "': This tenant has active applications");
+
+                accessControl.deleteTenant(tenant, credentials.get());
+                controller.notificationsDb().removeNotifications(NotificationSource.from(tenant));
+            }
+
+            if (forget) curator.removeTenant(tenant);
+            else curator.writeTenant(new DeletedTenant(tenant, oldTenant.createdAt(), controller.clock().instant()));
         }
     }
 
     private void requireNonExistent(TenantName name) {
         if (SystemApplication.TENANT.equals(name)
-            || get(name).isPresent()
+            || get(name, true).isPresent()
             // Underscores are allowed in existing tenant names, but tenants with - and _ cannot co-exist. E.g.
             // my-tenant cannot be created if my_tenant exists.
             || get(name.value().replace('-', '_')).isPresent()) {
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 22bd3c9d062..3c7c5f6ac30 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
@@ -108,6 +108,7 @@ import com.yahoo.vespa.hosted.controller.security.Credentials;
 import com.yahoo.vespa.hosted.controller.support.access.SupportAccess;
 import com.yahoo.vespa.hosted.controller.tenant.AthenzTenant;
 import com.yahoo.vespa.hosted.controller.tenant.CloudTenant;
+import com.yahoo.vespa.hosted.controller.tenant.DeletedTenant;
 import com.yahoo.vespa.hosted.controller.tenant.LastLoginInfo;
 import com.yahoo.vespa.hosted.controller.tenant.Tenant;
 import com.yahoo.vespa.hosted.controller.tenant.TenantInfo;
@@ -371,7 +372,7 @@ public class ApplicationApiHandler extends AuditLoggingRequestHandler {
         Slime slime = new Slime();
         Cursor tenantArray = slime.setArray();
         List<Application> applications = controller.applications().asList();
-        for (Tenant tenant : controller.tenants().asList())
+        for (Tenant tenant : controller.tenants().asList(includeDeleted(request)))
             toSlime(tenantArray.addObject(),
                     tenant,
                     applications.stream().filter(app -> app.id().tenant().equals(tenant.name())).collect(toList()),
@@ -388,13 +389,13 @@ public class ApplicationApiHandler extends AuditLoggingRequestHandler {
     private HttpResponse tenants(HttpRequest request) {
         Slime slime = new Slime();
         Cursor response = slime.setArray();
-        for (Tenant tenant : controller.tenants().asList())
+        for (Tenant tenant : controller.tenants().asList(includeDeleted(request)))
             tenantInTenantsListToSlime(tenant, request.getUri(), response.addObject());
         return new SlimeJsonResponse(slime);
     }
 
     private HttpResponse tenant(String tenantName, HttpRequest request) {
-        return controller.tenants().get(TenantName.from(tenantName))
+        return controller.tenants().get(TenantName.from(tenantName), includeDeleted(request))
                          .map(tenant -> tenant(tenant, request))
                          .orElseGet(() -> ErrorResponse.notFoundError("Tenant '" + tenantName + "' does not exist"));
     }
@@ -556,8 +557,7 @@ public class ApplicationApiHandler extends AuditLoggingRequestHandler {
 
     private HttpResponse applications(String tenantName, Optional<String> applicationName, HttpRequest request) {
         TenantName tenant = TenantName.from(tenantName);
-        if (controller.tenants().get(tenantName).isEmpty())
-            return ErrorResponse.notFoundError("Tenant '" + tenantName + "' does not exist");
+        getTenantOrThrow(tenantName);
 
         List<Application> applications = applicationName.isEmpty() ?
                 controller.applications().asList(tenant) :
@@ -2015,17 +2015,17 @@ public class ApplicationApiHandler extends AuditLoggingRequestHandler {
     }
 
     private HttpResponse deleteTenant(String tenantName, HttpRequest request) {
-        Optional<Tenant> tenant = controller.tenants().get(tenantName);
-        if (tenant.isEmpty())
-            return ErrorResponse.notFoundError("Could not delete tenant '" + tenantName + "': Tenant not found");
+        boolean forget = request.getBooleanProperty("forget");
+        if (forget && !isOperator(request))
+            return ErrorResponse.forbidden("Only operators can forget a tenant");
 
-        controller.tenants().delete(tenant.get().name(),
-                                    accessControlRequests.credentials(tenant.get().name(),
+        controller.tenants().delete(TenantName.from(tenantName),
+                                    () -> accessControlRequests.credentials(TenantName.from(tenantName),
                                                                       toSlime(request.getData()).get(),
-                                                                      request.getJDiscRequest()));
+                                                                      request.getJDiscRequest()),
+                                    forget);
 
-        // TODO: Change to a message response saying the tenant was deleted
-        return tenant(tenant.get(), request);
+        return new MessageResponse("Deleted tenant " + tenantName);
     }
 
     private HttpResponse deleteApplication(String tenantName, String applicationName, HttpRequest request) {
@@ -2220,6 +2220,7 @@ public class ApplicationApiHandler extends AuditLoggingRequestHandler {
 
                 break;
             }
+            case deleted: break;
             default: throw new IllegalArgumentException("Unexpected tenant type '" + tenant.type() + "'.");
         }
         // TODO jonmv: This should list applications, not instances.
@@ -2309,6 +2310,7 @@ public class ApplicationApiHandler extends AuditLoggingRequestHandler {
                 metaData.setString("property", athenzTenant.property().id());
                 break;
             case cloud: break;
+            case deleted: break;
             default: throw new IllegalArgumentException("Unexpected tenant type '" + tenant.type() + "'.");
         }
         object.setString("url", withPath("/application/v4/tenant/" + tenant.name().value(), requestURI).toString());
@@ -2332,6 +2334,8 @@ public class ApplicationApiHandler extends AuditLoggingRequestHandler {
                                                        .flatMap(app -> app.latestVersion().flatMap(ApplicationVersion::buildTime).stream())
                                                        .max(Comparator.naturalOrder());
         object.setLong("createdAtMillis", tenant.createdAt().toEpochMilli());
+        if (tenant.type() == Tenant.Type.deleted)
+            object.setLong("deletedAtMillis", ((DeletedTenant) tenant).deletedAt().toEpochMilli());
         lastDev.ifPresent(instant -> object.setLong("lastDeploymentToDevMillis", instant.toEpochMilli()));
         lastSubmission.ifPresent(instant -> object.setLong("lastSubmissionToProdMillis", instant.toEpochMilli()));
 
@@ -2536,10 +2540,15 @@ public class ApplicationApiHandler extends AuditLoggingRequestHandler {
         return "true".equals(request.getProperty("activeInstances"));
     }
 
+    private static boolean includeDeleted(HttpRequest request) {
+        return "true".equals(request.getProperty("includeDeleted"));
+    }
+
     private static String tenantType(Tenant tenant) {
         switch (tenant.type()) {
             case athenz: return "ATHENS";
             case cloud: return "CLOUD";
+            case deleted: return "DELETED";
             default: throw new IllegalArgumentException("Unknown tenant type: " + tenant.getClass().getSimpleName());
         }
     }
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 0ecc8ac81df..044b7b76d1e 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
@@ -42,7 +42,6 @@ import com.yahoo.vespa.hosted.controller.tenant.Tenant;
 import com.yahoo.yolean.Exceptions;
 
 import java.security.PublicKey;
-import java.time.LocalDateTime;
 import java.time.format.DateTimeFormatter;
 import java.util.ArrayList;
 import java.util.Collection;
diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/security/CloudAccessControl.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/security/CloudAccessControl.java
index 48f8d3e43cb..b8a44a682e7 100644
--- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/security/CloudAccessControl.java
+++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/security/CloudAccessControl.java
@@ -72,7 +72,7 @@ public class CloudAccessControl implements AccessControl {
 
     private void requireTenantTrialLimitNotReached(List<Tenant> existing) {
         var trialPlanId = PlanId.from("trial");
-        var tenantNames = existing.stream().map(Tenant::name).collect(Collectors.toList());
+        var tenantNames = existing.stream().filter(tenant -> tenant.type() == Tenant.Type.cloud).map(Tenant::name).collect(Collectors.toList());
         var trialTenants = billingController.tenantsWithPlan(tenantNames, trialPlanId).size();
 
         if (maxTrialTenants.value() >= 0 && maxTrialTenants.value() <= trialTenants) {
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 bf2cd039afd..72f0fc283e5 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
@@ -863,7 +863,32 @@ public class ApplicationApiTest extends ControllerContainerTest {
         // DELETE an empty tenant
         tester.assertResponse(request("/application/v4/tenant/tenant1", DELETE).userIdentity(USER_ID)
                                       .oktaAccessToken(OKTA_AT).oktaIdentityToken(OKTA_IT),
-                              new File("tenant-without-applications.json"));
+                              "{\"message\":\"Deleted tenant tenant1\"}");
+
+        // The tenant is not found
+        tester.assertResponse(request("/application/v4/tenant/tenant1", GET).userIdentity(USER_ID)
+                        .oktaAccessToken(OKTA_AT).oktaIdentityToken(OKTA_IT),
+                "{\"error-code\":\"NOT_FOUND\",\"message\":\"Tenant 'tenant1' does not exist\"}", 404);
+
+        // ... unless we specify to show deleted tenants
+        tester.assertResponse(request("/application/v4/tenant/tenant1", GET).properties(Map.of("includeDeleted", "true"))
+                        .userIdentity(HOSTED_VESPA_OPERATOR),
+                new File("tenant1-deleted.json"));
+
+        // Tenant cannot be recreated
+        tester.assertResponse(request("/application/v4/tenant/tenant1", POST).userIdentity(USER_ID)
+                        .data("{\"athensDomain\":\"domain1\", \"property\":\"property1\"}")
+                        .oktaAccessToken(OKTA_AT).oktaIdentityToken(OKTA_IT),
+                "{\"error-code\":\"BAD_REQUEST\",\"message\":\"Tenant 'tenant1' already exists\"}", 400);
+
+
+        // Forget a deleted tenant
+        tester.assertResponse(request("/application/v4/tenant/tenant1", DELETE).properties(Map.of("forget", "true"))
+                        .userIdentity(HOSTED_VESPA_OPERATOR),
+                "{\"message\":\"Deleted tenant tenant1\"}");
+        tester.assertResponse(request("/application/v4/tenant/tenant1", GET).properties(Map.of("includeDeleted", "true"))
+                        .userIdentity(HOSTED_VESPA_OPERATOR),
+                "{\"error-code\":\"NOT_FOUND\",\"message\":\"Tenant 'tenant1' does not exist\"}", 404);
     }
 
     private void addIssues(DeploymentTester tester, TenantAndApplicationId id) {
@@ -1217,11 +1242,18 @@ public class ApplicationApiTest extends ControllerContainerTest {
                               "{\"error-code\":\"NOT_FOUND\",\"message\":\"Could not delete instance 'tenant1.application1.instance1': Instance not found\"}",
                               404);
 
+        // DELETE and forget an application as non-operator
+        tester.assertResponse(request("/application/v4/tenant/tenant1", DELETE).properties(Map.of("forget", "true"))
+                                      .userIdentity(USER_ID)
+                                      .oktaAccessToken(OKTA_AT).oktaIdentityToken(OKTA_IT),
+                              "{\"error-code\":\"FORBIDDEN\",\"message\":\"Only operators can forget a tenant\"}",
+                              403);
+
         // DELETE tenant
         tester.assertResponse(request("/application/v4/tenant/tenant1", DELETE)
                                       .userIdentity(USER_ID)
                                       .oktaAccessToken(OKTA_AT).oktaIdentityToken(OKTA_IT),
-                              new File("tenant-without-applications.json"));
+                              "{\"message\":\"Deleted tenant tenant1\"}");
         // DELETE tenant again returns 403 as tenant access cannot be determined when the tenant does not exist
         tester.assertResponse(request("/application/v4/tenant/tenant1", DELETE)
                                       .userIdentity(USER_ID),
diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/responses/tenant1-deleted.json b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/responses/tenant1-deleted.json
new file mode 100644
index 00000000000..1c4b76932ac
--- /dev/null
+++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/responses/tenant1-deleted.json
@@ -0,0 +1,9 @@
+{
+  "tenant": "tenant1",
+  "type": "DELETED",
+  "applications": [],
+  "metaData": {
+    "createdAtMillis": "(ignore)",
+    "deletedAtMillis": "(ignore)"
+  }
+}