diff options
Diffstat (limited to 'controller-server')
5 files changed, 8 insertions, 133 deletions
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 76fa52c0706..87531240752 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 @@ -1,6 +1,5 @@ package com.yahoo.vespa.hosted.controller.certificate; -import com.google.common.collect.Sets; import com.google.common.hash.Hashing; import com.google.common.io.BaseEncoding; import com.yahoo.config.application.api.DeploymentInstanceSpec; @@ -14,19 +13,15 @@ import com.yahoo.container.jdisc.secretstore.SecretNotFoundException; import com.yahoo.container.jdisc.secretstore.SecretStore; import com.yahoo.security.SubjectAlternativeName; import com.yahoo.security.X509CertificateUtils; -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.flags.StringFlag; import com.yahoo.vespa.hosted.controller.Instance; import com.yahoo.vespa.hosted.controller.api.integration.certificates.EndpointCertificateMetadata; import com.yahoo.vespa.hosted.controller.api.integration.certificates.EndpointCertificateProvider; 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; @@ -35,17 +30,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.HashSet; import java.util.LinkedHashSet; import java.util.List; import java.util.Optional; -import java.util.OptionalInt; import java.util.Set; -import java.util.concurrent.Executors; -import java.util.concurrent.TimeUnit; import java.util.logging.Level; import java.util.logging.Logger; import java.util.stream.Collectors; @@ -54,6 +44,8 @@ import java.util.stream.Collectors; * Looks up stored endpoint certificate metadata, provisions new certificates if none is found, * re-provisions if zone is not covered, and uses refreshed certificates if a newer version is available. * + * See also EndpointCertificateMaintainer, which handles refreshes, deletions and triggers deployments + * * @author andreer */ public class EndpointCertificateManager { @@ -66,9 +58,6 @@ public class EndpointCertificateManager { private final EndpointCertificateProvider endpointCertificateProvider; private final Clock clock; private final BooleanFlag validateEndpointCertificates; - private final StringFlag deleteUnusedEndpointCertificates; - private final BooleanFlag endpointCertInSharedRouting; - private final BooleanFlag useEndpointCertificateMaintainer; public EndpointCertificateManager(ZoneRegistry zoneRegistry, CuratorDb curator, @@ -81,16 +70,6 @@ public class EndpointCertificateManager { this.endpointCertificateProvider = endpointCertificateProvider; this.clock = clock; this.validateEndpointCertificates = Flags.VALIDATE_ENDPOINT_CERTIFICATES.bindTo(flagSource); - this.deleteUnusedEndpointCertificates = Flags.DELETE_UNUSED_ENDPOINT_CERTIFICATES.bindTo(flagSource); - this.endpointCertInSharedRouting = Flags.ENDPOINT_CERT_IN_SHARED_ROUTING.bindTo(flagSource); - this.useEndpointCertificateMaintainer = Flags.USE_ENDPOINT_CERTIFICATE_MAINTAINER.bindTo(flagSource); - Executors.newSingleThreadScheduledExecutor().scheduleAtFixedRate(() -> { - try { - this.deleteUnusedCertificates(); - } catch (Throwable t) { - log.log(Level.INFO, "Unexpected Throwable caught while deleting unused endpoint certificates", t); - } - }, 1, 10, TimeUnit.MINUTES); } public Optional<EndpointCertificateMetadata> getEndpointCertificateMetadata(Instance instance, ZoneId zone, Optional<DeploymentInstanceSpec> instanceSpec) { @@ -105,10 +84,6 @@ public class EndpointCertificateManager { @NotNull private Optional<EndpointCertificateMetadata> getOrProvision(Instance instance, ZoneId zone, Optional<DeploymentInstanceSpec> instanceSpec) { - boolean endpointCertInSharedRouting = this.endpointCertInSharedRouting.with(FetchVector.Dimension.APPLICATION_ID, instance.id().serializedForm()).value(); - if (!zoneRegistry.zones().directlyRouted().ids().contains(zone) && !endpointCertInSharedRouting) - return Optional.empty(); - final var currentCertificateMetadata = curator.readEndpointCertificateMetadata(instance.id()); if (currentCertificateMetadata.isEmpty()) { @@ -130,74 +105,10 @@ public class EndpointCertificateManager { return Optional.of(reprovisionedCertificateMetadata); } - if (!useEndpointCertificateMaintainer.value()) { - // 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()) - .withLastRefreshed(clock.instant().getEpochSecond()); - validateEndpointCertificate(refreshedCertificateMetadata, instance, zone); - curator.writeEndpointCertificateMetadata(instance.id(), refreshedCertificateMetadata); - return Optional.of(refreshedCertificateMetadata); - } - } - validateEndpointCertificate(currentCertificateMetadata.get(), instance, zone); return currentCertificateMetadata; } - enum CleanupMode { - DISABLE, - DRYRUN, - ENABLE - } - - private void deleteUnusedCertificates() { - CleanupMode mode = CleanupMode.valueOf(deleteUnusedEndpointCertificates.value().toUpperCase()); - if (mode == CleanupMode.DISABLE || useEndpointCertificateMaintainer.value()) return; - - var oneMonthAgo = clock.instant().minus(30, ChronoUnit.DAYS); - curator.readAllEndpointCertificateMetadata().forEach((applicationId, storedMetaData) -> { - var lastRequested = Instant.ofEpochSecond(storedMetaData.lastRequested()); - if (lastRequested.isBefore(oneMonthAgo) && hasNoDeployments(applicationId)) { - try (Lock lock = lock(applicationId)) { - if (Optional.of(storedMetaData).equals(curator.readEndpointCertificateMetadata(applicationId))) { - log.log(Level.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 Lock lock(ApplicationId applicationId) { - return curator.lock(TenantAndApplicationId.from(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())); - var keyVersions = new HashSet<>(secretStore.listSecretVersions(originalCertificateMetadata.keyName())); - return Sets.intersection(certVersions, keyVersions).stream().mapToInt(Integer::intValue).max(); - } catch (SecretNotFoundException s) { - return OptionalInt.empty(); // Likely because the certificate is very recently provisioned - keep current version - } - } - private EndpointCertificateMetadata provisionEndpointCertificate(Instance instance, Optional<EndpointCertificateMetadata> currentMetadata, ZoneId deploymentZone, Optional<DeploymentInstanceSpec> instanceSpec) { List<String> currentlyPresentNames = currentMetadata.isPresent() ? diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/EndpointCertificateMaintainer.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/EndpointCertificateMaintainer.java index a1d7c3d16b4..e59875e9588 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/EndpointCertificateMaintainer.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/EndpointCertificateMaintainer.java @@ -8,8 +8,6 @@ import com.yahoo.container.jdisc.secretstore.SecretNotFoundException; import com.yahoo.container.jdisc.secretstore.SecretStore; import com.yahoo.log.LogLevel; import com.yahoo.vespa.curator.Lock; -import com.yahoo.vespa.flags.BooleanFlag; -import com.yahoo.vespa.flags.Flags; import com.yahoo.vespa.hosted.controller.Controller; import com.yahoo.vespa.hosted.controller.Instance; import com.yahoo.vespa.hosted.controller.api.integration.certificates.EndpointCertificateMetadata; @@ -27,12 +25,13 @@ import java.time.temporal.ChronoUnit; import java.util.HashSet; import java.util.Optional; import java.util.OptionalInt; -import java.util.function.Predicate; import java.util.logging.Level; import java.util.logging.Logger; /** - * Updates refreshed endpoint certificates and triggers redeployment, and deletes unused certificates + * Updates refreshed endpoint certificates and triggers redeployment, and deletes unused certificates. + * + * See also EndpointCertificateManager, which provisions, reprovisions and validates certificates on deploy * * @author andreer */ @@ -45,7 +44,6 @@ public class EndpointCertificateMaintainer extends ControllerMaintainer { private final CuratorDb curator; private final SecretStore secretStore; private final EndpointCertificateProvider endpointCertificateProvider; - private final BooleanFlag useEndpointCertificateMaintainer; public EndpointCertificateMaintainer(Controller controller, Duration interval) { super(controller, interval, null, SystemName.all()); @@ -54,15 +52,10 @@ public class EndpointCertificateMaintainer extends ControllerMaintainer { this.secretStore = controller.secretStore(); this.curator = controller().curator(); this.endpointCertificateProvider = controller.serviceRegistry().endpointCertificateProvider(); - this.useEndpointCertificateMaintainer = Flags.USE_ENDPOINT_CERTIFICATE_MAINTAINER.bindTo(controller().flagSource()); } @Override protected boolean maintain() { - - if (!useEndpointCertificateMaintainer.value()) - return true; // handled by EndpointCertificateManager for now - try { // In order of importance deployRefreshedCertificates(); diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/ControllerTest.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/ControllerTest.java index 8fcbb365804..63452f40dbb 100644 --- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/ControllerTest.java +++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/ControllerTest.java @@ -736,8 +736,8 @@ public class ControllerTest { .map(prefix -> prefix + "app1.tenant1." + zone.region().value() + (zone.environment() == Environment.prod ? "" : "." + zone.environment().value()) + ".vespa.oath.cloud"))) - .collect(Collectors.toUnmodifiableList()), - tester.controllerTester().serviceRegistry().endpointCertificateMock().dnsNamesOf(context1.instanceId())); + .collect(Collectors.toUnmodifiableSet()), + Set.copyOf(tester.controllerTester().serviceRegistry().endpointCertificateMock().dnsNamesOf(context1.instanceId()))); // Next deployment reuses certificate context1.submit(applicationPackage).deploy(); @@ -751,7 +751,7 @@ public class ControllerTest { tester.controller().applications().deploy(context2.instanceId(), devZone, Optional.of(applicationPackage), DeployOptions.none()); assertTrue("Application deployed and activated", tester.configServer().application(context2.instanceId(), devZone).get().activated()); - assertFalse("Does not provision certificate in zones with routing layer", certificate.apply(context2.instance()).isPresent()); + assertTrue("Provisions certificate also in zone with routing layer", certificate.apply(context2.instance()).isPresent()); } @Test 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 4f2000b1902..0d70fe48a77 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 @@ -146,27 +146,6 @@ public class EndpointCertificateManagerTest { } @Test - public void uses_refreshed_certificate_when_available_and_valid() { - secretStore.setSecret(testKeyName, "secret-key", 7); - secretStore.setSecret(testCertName, "cert", 7); - secretStore.setSecret(testKeyName, KeyUtils.toPem(testKeyPair.getPrivate()), 8); - secretStore.setSecret(testKeyName, KeyUtils.toPem(testKeyPair.getPrivate()), 9); - secretStore.setSecret(testCertName, X509CertificateUtils.toPem(testCertificate) + X509CertificateUtils.toPem(testCertificate), 8); - mockCuratorDb.writeEndpointCertificateMetadata(testInstance.id(), new EndpointCertificateMetadata(testKeyName, testCertName, 7, 0, "request_id", - List.of("vt2ktgkqme5zlnp4tj4ttyor7fj3v7q5o.vespa.oath.cloud", - "default.default.global.vespa.oath.cloud", - "*.default.default.global.vespa.oath.cloud", - "default.default.aws-us-east-1a.vespa.oath.cloud", - "*.default.default.aws-us-east-1a.vespa.oath.cloud"), - "issuer", Optional.empty(), Optional.empty())); - Optional<EndpointCertificateMetadata> endpointCertificateMetadata = endpointCertificateManager.getEndpointCertificateMetadata(testInstance, testZone, Optional.empty()); - assertTrue(endpointCertificateMetadata.isPresent()); - assertEquals(testKeyName, endpointCertificateMetadata.get().keyName()); - assertEquals(testCertName, endpointCertificateMetadata.get().certName()); - assertEquals(8, endpointCertificateMetadata.get().version()); - } - - @Test public void reprovisions_certificate_when_necessary() { mockCuratorDb.writeEndpointCertificateMetadata(testInstance.id(), new EndpointCertificateMetadata(testKeyName, testCertName, -1, 0, "uuid", List.of(), "issuer", Optional.empty(), Optional.empty())); secretStore.setSecret("vespa.tls.default.default.default-key", KeyUtils.toPem(testKeyPair.getPrivate()), 0); diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/EndpointCertificateMaintainerTest.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/EndpointCertificateMaintainerTest.java index dbf102f23d7..66bda66bbf9 100644 --- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/EndpointCertificateMaintainerTest.java +++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/EndpointCertificateMaintainerTest.java @@ -2,15 +2,12 @@ package com.yahoo.vespa.hosted.controller.maintenance; import com.yahoo.config.provision.ApplicationId; -import com.yahoo.vespa.flags.Flags; -import com.yahoo.vespa.flags.InMemoryFlagSource; import com.yahoo.vespa.hosted.controller.ControllerTester; import com.yahoo.vespa.hosted.controller.api.integration.certificates.EndpointCertificateMetadata; import com.yahoo.vespa.hosted.controller.deployment.ApplicationPackageBuilder; import com.yahoo.vespa.hosted.controller.deployment.DeploymentContext; import com.yahoo.vespa.hosted.controller.deployment.DeploymentTester; import com.yahoo.vespa.hosted.controller.integration.SecretStoreMock; -import org.junit.Before; import org.junit.Test; import java.time.Duration; @@ -33,11 +30,6 @@ public class EndpointCertificateMaintainerTest { private final EndpointCertificateMaintainer maintainer = new EndpointCertificateMaintainer(tester.controller(), Duration.ofHours(1)); private final EndpointCertificateMetadata exampleMetadata = new EndpointCertificateMetadata("keyName", "certName", 0, 0, "uuid", List.of(), "issuer", Optional.empty(), Optional.empty()); - @Before - public void setUp() throws Exception { - ((InMemoryFlagSource) tester.controller().flagSource()).withBooleanFlag(Flags.USE_ENDPOINT_CERTIFICATE_MAINTAINER.id(), true); - } - @Test public void old_and_unused_cert_is_deleted() { tester.curator().writeEndpointCertificateMetadata(ApplicationId.defaultId(), exampleMetadata); |