andreer/delete unused certs 1 (#14901)

* add delete cert method to endpoint certificate providers

* delete unused certificates (guarded by feature flag)

7 files changed, 61 insertions, 75 deletions

diff --git a/container-disc/src/main/java/com/yahoo/container/jdisc/SecretStoreProvider.java b/container-disc/src/main/java/com/yahoo/container/jdisc/SecretStoreProvider.java
index 6012fbe394c..9c1dd00fdd4 100644
--- a/container-disc/src/main/java/com/yahoo/container/jdisc/SecretStoreProvider.java
+++ b/container-disc/src/main/java/com/yahoo/container/jdisc/SecretStoreProvider.java
@@ -1,9 +1,12 @@
 // Copyright 2019 Oath Inc. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root.
 package com.yahoo.container.jdisc;
 
+import com.yahoo.container.jdisc.secretstore.SecretNotFoundException;
 import com.yahoo.container.jdisc.secretstore.SecretStore;
 import com.yahoo.container.di.componentgraph.Provider;
 
+import java.util.List;
+
 public class SecretStoreProvider implements Provider<SecretStore> {
 
     private static final ThrowingSecretStore instance = new ThrowingSecretStore();
@@ -25,5 +28,10 @@ public class SecretStoreProvider implements Provider<SecretStore> {
         public String getSecret(String key, int version) {
             throw new UnsupportedOperationException("A secret store is not available");
         }
+
+        @Override
+        public List<Integer> listSecretVersions(String key) {
+            throw new SecretNotFoundException("A secret store is not available");
+        }
     }
 }
diff --git a/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/certificates/EndpointCertificateMock.java b/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/certificates/EndpointCertificateMock.java
index a484bb329a3..8c63613ec91 100644
--- a/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/certificates/EndpointCertificateMock.java
+++ b/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/certificates/EndpointCertificateMock.java
@@ -33,4 +33,8 @@ public class EndpointCertificateMock implements EndpointCertificateProvider {
         return Collections.emptyList();
     }
 
+    @Override
+    public void deleteCertificate(ApplicationId applicationId, EndpointCertificateMetadata endpointCertificateMetadata) {
+        dnsNames.remove(applicationId);
+    }
 }
diff --git a/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/certificates/EndpointCertificateProvider.java b/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/certificates/EndpointCertificateProvider.java
index 9c5c25c1c71..a4c9d4d8b3a 100644
--- a/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/certificates/EndpointCertificateProvider.java
+++ b/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/certificates/EndpointCertificateProvider.java
@@ -16,4 +16,6 @@ public interface EndpointCertificateProvider  {
     EndpointCertificateMetadata requestCaSignedCertificate(ApplicationId applicationId, List<String> dnsNames, Optional<EndpointCertificateMetadata> currentMetadata);
 
     List<EndpointCertificateMetadata> listCertificates();
+
+    void deleteCertificate(ApplicationId applicationId, EndpointCertificateMetadata endpointCertificateMetadata);
 }
diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/certificate/EndpointCertificateManager.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/certificate/EndpointCertificateManager.java
index e4ec0b04978..dd8515f1fe8 100644
--- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/certificate/EndpointCertificateManager.java
+++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/certificate/EndpointCertificateManager.java
@@ -12,6 +12,7 @@ import com.yahoo.config.provision.zone.ZoneApi;
 import com.yahoo.config.provision.zone.ZoneId;
 import com.yahoo.container.jdisc.secretstore.SecretNotFoundException;
 import com.yahoo.container.jdisc.secretstore.SecretStore;
+import com.yahoo.log.LogLevel;
 import com.yahoo.security.SubjectAlternativeName;
 import com.yahoo.security.X509CertificateUtils;
 import com.yahoo.vespa.flags.BooleanFlag;
@@ -25,6 +26,7 @@ import com.yahoo.vespa.hosted.controller.api.integration.certificates.EndpointCe
 import com.yahoo.vespa.hosted.controller.api.integration.zone.ZoneRegistry;
 import com.yahoo.vespa.hosted.controller.application.Endpoint;
 import com.yahoo.vespa.hosted.controller.application.EndpointId;
+import com.yahoo.vespa.hosted.controller.application.TenantAndApplicationId;
 import com.yahoo.vespa.hosted.controller.persistence.CuratorDb;
 import org.jetbrains.annotations.NotNull;
 
@@ -33,13 +35,12 @@ import java.security.cert.X509Certificate;
 import java.time.Clock;
 import java.time.Duration;
 import java.time.Instant;
+import java.time.temporal.ChronoUnit;
 import java.util.ArrayList;
 import java.util.Collections;
-import java.util.HashMap;
 import java.util.HashSet;
 import java.util.LinkedHashSet;
 import java.util.List;
-import java.util.Map;
 import java.util.Optional;
 import java.util.OptionalInt;
 import java.util.Set;
@@ -64,9 +65,8 @@ public class EndpointCertificateManager {
     private final SecretStore secretStore;
     private final EndpointCertificateProvider endpointCertificateProvider;
     private final Clock clock;
-    private final BooleanFlag useRefreshedEndpointCertificate;
     private final BooleanFlag validateEndpointCertificates;
-    private final StringFlag endpointCertificateBackfill;
+    private final StringFlag deleteUnusedEndpointCertificates;
     private final BooleanFlag endpointCertInSharedRouting;
 
     public EndpointCertificateManager(ZoneRegistry zoneRegistry,
@@ -79,15 +79,14 @@ public class EndpointCertificateManager {
         this.secretStore = secretStore;
         this.endpointCertificateProvider = endpointCertificateProvider;
         this.clock = clock;
-        this.useRefreshedEndpointCertificate = Flags.USE_REFRESHED_ENDPOINT_CERTIFICATE.bindTo(flagSource);
         this.validateEndpointCertificates = Flags.VALIDATE_ENDPOINT_CERTIFICATES.bindTo(flagSource);
-        this.endpointCertificateBackfill = Flags.ENDPOINT_CERTIFICATE_BACKFILL.bindTo(flagSource);
+        this.deleteUnusedEndpointCertificates = Flags.DELETE_UNUSED_ENDPOINT_CERTIFICATES.bindTo(flagSource);
         this.endpointCertInSharedRouting = Flags.ENDPOINT_CERT_IN_SHARED_ROUTING.bindTo(flagSource);
         Executors.newSingleThreadScheduledExecutor().scheduleAtFixedRate(() -> {
             try {
-                this.backfillCertificateMetadata();
+                this.deleteUnusedCertificates();
             } catch (Throwable t) {
-                log.log(Level.INFO, "Unexpected Throwable caught while backfilling certificate metadata", t);
+                log.log(Level.INFO, "Unexpected Throwable caught while deleting unused endpoint certificates", t);
             }
         }, 1, 10, TimeUnit.MINUTES);
     }
@@ -97,7 +96,8 @@ public class EndpointCertificateManager {
         Optional<EndpointCertificateMetadata> metadata = getOrProvision(instance, zone, instanceSpec);
         metadata.ifPresent(m -> curator.writeEndpointCertificateMetadata(instance.id(), m.withLastRequested(clock.instant().getEpochSecond())));
         Duration duration = Duration.between(t0, Instant.now());
-        if (duration.toSeconds() > 30) log.log(Level.INFO, String.format("Getting endpoint certificate metadata for %s took %d seconds!", instance.id().serializedForm(), duration.toSeconds()));
+        if (duration.toSeconds() > 30)
+            log.log(Level.INFO, String.format("Getting endpoint certificate metadata for %s took %d seconds!", instance.id().serializedForm(), duration.toSeconds()));
         return metadata;
     }
 
@@ -129,76 +129,52 @@ public class EndpointCertificateManager {
             return Optional.of(reprovisionedCertificateMetadata);
         }
 
-        // If feature flag set for application, look for and use refreshed certificate
-        if (useRefreshedEndpointCertificate.with(FetchVector.Dimension.APPLICATION_ID, instance.id().serializedForm()).value()) {
-            var latestAvailableVersion = latestVersionInSecretStore(currentCertificateMetadata.get());
-
-            if (latestAvailableVersion.isPresent() && latestAvailableVersion.getAsInt() > currentCertificateMetadata.get().version()) {
-                var refreshedCertificateMetadata = currentCertificateMetadata.get().withVersion(latestAvailableVersion.getAsInt());
-                validateEndpointCertificate(refreshedCertificateMetadata, instance, zone);
-                curator.writeEndpointCertificateMetadata(instance.id(), refreshedCertificateMetadata);
-                return Optional.of(refreshedCertificateMetadata);
-            }
+        // Look for and use refreshed certificate
+        var latestAvailableVersion = latestVersionInSecretStore(currentCertificateMetadata.get());
+        if (latestAvailableVersion.isPresent() && latestAvailableVersion.getAsInt() > currentCertificateMetadata.get().version()) {
+            var refreshedCertificateMetadata = currentCertificateMetadata.get().withVersion(latestAvailableVersion.getAsInt());
+            validateEndpointCertificate(refreshedCertificateMetadata, instance, zone);
+            curator.writeEndpointCertificateMetadata(instance.id(), refreshedCertificateMetadata);
+            return Optional.of(refreshedCertificateMetadata);
         }
 
         validateEndpointCertificate(currentCertificateMetadata.get(), instance, zone);
         return currentCertificateMetadata;
     }
 
-    enum BackfillMode {
+    enum CleanupMode {
         DISABLE,
         DRYRUN,
         ENABLE
     }
 
-    private void backfillCertificateMetadata() {
-        BackfillMode mode = BackfillMode.valueOf(endpointCertificateBackfill.value());
-        if (mode == BackfillMode.DISABLE) return;
-
-        List<EndpointCertificateMetadata> allProviderCertificateMetadata = endpointCertificateProvider.listCertificates();
-        Map<String, EndpointCertificateMetadata> sanToEndpointCertificate = new HashMap<>();
-
-        allProviderCertificateMetadata.forEach((providerMetadata -> {
-            if (providerMetadata.request_id().isEmpty())
-                throw new RuntimeException("Backfill failed - provider metadata missing request_id");
-            if (providerMetadata.requestedDnsSans().isEmpty())
-                throw new RuntimeException("Backfill failed - provider metadata missing DNS SANs for " + providerMetadata.request_id().get());
-            providerMetadata.requestedDnsSans().get().forEach(san -> sanToEndpointCertificate.put(san, providerMetadata)
-            );
-        }));
-
-        Map<ApplicationId, EndpointCertificateMetadata> allEndpointCertificateMetadata = curator.readAllEndpointCertificateMetadata();
-
-        allEndpointCertificateMetadata.forEach((applicationId, storedMetaData) -> {
-            if (storedMetaData.requestedDnsSans().isPresent() && storedMetaData.request_id().isPresent() && storedMetaData.issuer().isPresent())
-                return;
-
-            var hashedCn = commonNameHashOf(applicationId, zoneRegistry.system()); // use as join key
-            EndpointCertificateMetadata providerMetadata = sanToEndpointCertificate.get(hashedCn);
-
-            if (providerMetadata == null) {
-                log.log(Level.INFO, "No matching certificate provider metadata found for application " + applicationId.serializedForm());
-                return;
-            }
-
-            EndpointCertificateMetadata backfilledMetadata =
-                    new EndpointCertificateMetadata(
-                            storedMetaData.keyName(),
-                            storedMetaData.certName(),
-                            storedMetaData.version(),
-                            Instant.now().getEpochSecond(),
-                            providerMetadata.request_id(),
-                            providerMetadata.requestedDnsSans(),
-                            providerMetadata.issuer());
-
-            if (mode == BackfillMode.DRYRUN) {
-                log.log(Level.INFO, "Would update stored metadata " + storedMetaData + " with data from provider: " + backfilledMetadata);
-            } else if (mode == BackfillMode.ENABLE) {
-                curator.writeEndpointCertificateMetadata(applicationId, backfilledMetadata);
+    private void deleteUnusedCertificates() {
+        CleanupMode mode = CleanupMode.valueOf(deleteUnusedEndpointCertificates.value());
+        if (mode == CleanupMode.DISABLE) return;
+
+        var oneMonthAgo = clock.instant().minus(1, ChronoUnit.MONTHS);
+        curator.readAllEndpointCertificateMetadata().forEach((applicationId, storedMetaData) -> {
+            var lastRequested = Instant.ofEpochSecond(storedMetaData.lastRequested());
+            if (lastRequested.isBefore(oneMonthAgo) && hasNoDeployments(applicationId)) {
+                log.log(LogLevel.INFO, "Cert for app " + applicationId.serializedForm()
+                        + " has not been requested in a month and app has no deployments"
+                        + (mode == CleanupMode.ENABLE ? ", deleting from provider and ZK" : ""));
+                if (mode == CleanupMode.ENABLE) {
+                    endpointCertificateProvider.deleteCertificate(applicationId, storedMetaData);
+                    curator.deleteEndpointCertificateMetadata(applicationId);
+                }
             }
         });
     }
 
+    private boolean hasNoDeployments(ApplicationId applicationId) {
+        var deployments = curator.readApplication(TenantAndApplicationId.from(applicationId))
+                .flatMap(app -> app.get(applicationId.instance()))
+                .map(Instance::deployments);
+
+        return deployments.isEmpty() || deployments.get().size() == 0;
+    }
+
     private OptionalInt latestVersionInSecretStore(EndpointCertificateMetadata originalCertificateMetadata) {
         try {
             var certVersions = new HashSet<>(secretStore.listSecretVersions(originalCertificateMetadata.certName()));
diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/persistence/CuratorDb.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/persistence/CuratorDb.java
index 0a9dd2e55a6..eda4f713732 100644
--- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/persistence/CuratorDb.java
+++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/persistence/CuratorDb.java
@@ -500,6 +500,10 @@ public class CuratorDb {
         curator.set(endpointCertificatePath(applicationId), asJson(EndpointCertificateMetadataSerializer.toSlime(endpointCertificateMetadata)));
     }
 
+    public void deleteEndpointCertificateMetadata(ApplicationId applicationId) {
+        curator.delete(endpointCertificatePath(applicationId));
+    }
+
     public Optional<EndpointCertificateMetadata> readEndpointCertificateMetadata(ApplicationId applicationId) {
         return curator.getData(endpointCertificatePath(applicationId)).map(String::new).map(EndpointCertificateMetadataSerializer::fromJsonString);
     }
diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/certificate/EndpointCertificateManagerTest.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/certificate/EndpointCertificateManagerTest.java
index fe237797641..7213407a236 100644
--- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/certificate/EndpointCertificateManagerTest.java
+++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/certificate/EndpointCertificateManagerTest.java
@@ -141,8 +141,6 @@ public class EndpointCertificateManagerTest {
 
     @Test
     public void uses_refreshed_certificate_when_available_and_valid() {
-        inMemoryFlagSource.withBooleanFlag(Flags.USE_REFRESHED_ENDPOINT_CERTIFICATE.id(), true);
-
         secretStore.setSecret(testKeyName, "secret-key", 7);
         secretStore.setSecret(testCertName, "cert", 7);
         secretStore.setSecret(testKeyName, KeyUtils.toPem(testKeyPair.getPrivate()), 8);
diff --git a/flags/src/main/java/com/yahoo/vespa/flags/Flags.java b/flags/src/main/java/com/yahoo/vespa/flags/Flags.java
index 3fd31f0fdd8..a87246df6e4 100644
--- a/flags/src/main/java/com/yahoo/vespa/flags/Flags.java
+++ b/flags/src/main/java/com/yahoo/vespa/flags/Flags.java
@@ -238,20 +238,14 @@ public class Flags {
             "Takes effect on next node agent tick (but does not clear existing failure reports)",
             HOSTNAME);
 
-    public static final UnboundBooleanFlag USE_REFRESHED_ENDPOINT_CERTIFICATE = defineFeatureFlag(
-            "use-refreshed-endpoint-certificate", false,
-            "Whether an application should start using a newer certificate/key pair if available",
-            "Takes effect on the next deployment of the application",
-            APPLICATION_ID);
-
     public static final UnboundBooleanFlag VALIDATE_ENDPOINT_CERTIFICATES = defineFeatureFlag(
             "validate-endpoint-certificates", false,
             "Whether endpoint certificates should be validated before use",
             "Takes effect on the next deployment of the application");
 
-    public static final UnboundStringFlag ENDPOINT_CERTIFICATE_BACKFILL = defineStringFlag(
-            "endpoint-certificate-backfill", "disable",
-            "Whether the endpoint certificate maintainer should backfill missing certificate data from cameo",
+    public static final UnboundStringFlag DELETE_UNUSED_ENDPOINT_CERTIFICATES = defineStringFlag(
+            "delete-unused-endpoint-certificates", "disable",
+            "Whether the endpoint certificate maintainer should delete unused certificates in cameo/zk",
             "Takes effect on next scheduled run of maintainer - set to \"disable\", \"dryrun\" or \"enable\"");
 
     public static final UnboundBooleanFlag USE_ALTERNATIVE_ENDPOINT_CERTIFICATE_PROVIDER = defineFeatureFlag(