diff options
57 files changed, 1080 insertions, 481 deletions
diff --git a/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/certificates/EndpointCertificateMetadata.java b/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/certificates/EndpointCertificateMetadata.java index b423fcb83f8..02afbb6ace6 100644 --- a/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/certificates/EndpointCertificateMetadata.java +++ b/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/certificates/EndpointCertificateMetadata.java @@ -2,7 +2,6 @@ package com.yahoo.vespa.hosted.controller.api.integration.certificates; import java.util.List; -import java.util.Objects; import java.util.Optional; /** @@ -12,76 +11,25 @@ import java.util.Optional; * * @author andreer */ -public class EndpointCertificateMetadata { +public record EndpointCertificateMetadata(String keyName, String certName, int version, long lastRequested, + String rootRequestId, // The id of the first request made for this certificate. Should not change. + Optional<String> leafRequestId, // The id of the last known request made for this certificate. Changes on refresh, may be outdated! + List<String> requestedDnsSans, String issuer, Optional<Long> expiry, + Optional<Long> lastRefreshed, Optional<String> randomizedId) { - private final String keyName; - private final String certName; - private final int version; - private final long lastRequested; - private final String rootRequestId; - private final Optional<String> leafRequestId; - private final List<String> requestedDnsSans; - private final String issuer; - private final Optional<Long> expiry; - private final Optional<Long> lastRefreshed; - - public EndpointCertificateMetadata(String keyName, String certName, int version, long lastRequested, String rootRequestId, Optional<String> leafRequestId, List<String> requestedDnsSans, String issuer, Optional<Long> expiry, Optional<Long> lastRefreshed) { - this.keyName = keyName; - this.certName = certName; - this.version = version; - this.lastRequested = lastRequested; - this.rootRequestId = rootRequestId; - this.leafRequestId = leafRequestId; - this.requestedDnsSans = requestedDnsSans; - this.issuer = issuer; - this.expiry = expiry; - this.lastRefreshed = lastRefreshed; - } - - public String keyName() { - return keyName; - } - - public String certName() { - return certName; - } - - public int version() { - return version; - } - - public long lastRequested() { - return lastRequested; - } - - /** - * @return The request id of the first request made for this certificate. Should not change. - */ - public String rootRequestId() { - return rootRequestId; - } - - /** - * @return The request id of the last known request made for this certificate. Changes on refresh, may be outdated! - */ - public Optional<String> leafRequestId() { - return leafRequestId; - } - - public List<String> requestedDnsSans() { - return requestedDnsSans; - } - - public String issuer() { - return issuer; - } - - public Optional<Long> expiry() { - return expiry; - } - - public Optional<Long> lastRefreshed() { - return lastRefreshed; + public EndpointCertificateMetadata withRandomizedId(String randomizedId) { + return new EndpointCertificateMetadata( + this.keyName, + this.certName, + this.version, + this.lastRequested, + this.rootRequestId, + this.leafRequestId, + this.requestedDnsSans, + this.issuer, + this.expiry, + this.lastRefreshed, + Optional.of(randomizedId)); } public EndpointCertificateMetadata withKeyName(String keyName) { @@ -95,7 +43,8 @@ public class EndpointCertificateMetadata { this.requestedDnsSans, this.issuer, this.expiry, - this.lastRefreshed); + this.lastRefreshed, + this.randomizedId); } public EndpointCertificateMetadata withCertName(String certName) { @@ -109,7 +58,8 @@ public class EndpointCertificateMetadata { this.requestedDnsSans, this.issuer, this.expiry, - this.lastRefreshed); + this.lastRefreshed, + this.randomizedId); } public EndpointCertificateMetadata withVersion(int version) { @@ -123,7 +73,8 @@ public class EndpointCertificateMetadata { this.requestedDnsSans, this.issuer, this.expiry, - this.lastRefreshed); + this.lastRefreshed, + this.randomizedId); } public EndpointCertificateMetadata withLastRequested(long lastRequested) { @@ -137,7 +88,8 @@ public class EndpointCertificateMetadata { this.requestedDnsSans, this.issuer, this.expiry, - this.lastRefreshed); + this.lastRefreshed, + this.randomizedId); } public EndpointCertificateMetadata withLastRefreshed(long lastRefreshed) { @@ -151,7 +103,8 @@ public class EndpointCertificateMetadata { this.requestedDnsSans, this.issuer, this.expiry, - Optional.of(lastRefreshed)); + Optional.of(lastRefreshed), + this.randomizedId); } public EndpointCertificateMetadata withRootRequestId(String rootRequestId) { @@ -165,7 +118,8 @@ public class EndpointCertificateMetadata { this.requestedDnsSans, this.issuer, this.expiry, - lastRefreshed); + this.lastRefreshed, + this.randomizedId); } public EndpointCertificateMetadata withLeafRequestId(Optional<String> leafRequestId) { @@ -179,45 +133,8 @@ public class EndpointCertificateMetadata { this.requestedDnsSans, this.issuer, this.expiry, - lastRefreshed); - } - - @Override - public String toString() { - return "EndpointCertificateMetadata{" + - "keyName='" + keyName + '\'' + - ", certName='" + certName + '\'' + - ", version=" + version + - ", lastRequested=" + lastRequested + - ", rootRequestId=" + rootRequestId + - ", leafRequestId=" + leafRequestId + - ", requestedDnsSans=" + requestedDnsSans + - ", issuer=" + issuer + - ", expiry=" + expiry + - ", lastRefreshed=" + lastRefreshed + - '}'; - } - - @Override - public boolean equals(Object o) { - if (this == o) return true; - if (o == null || getClass() != o.getClass()) return false; - EndpointCertificateMetadata that = (EndpointCertificateMetadata) o; - return version == that.version && - lastRequested == that.lastRequested && - keyName.equals(that.keyName) && - certName.equals(that.certName) && - rootRequestId.equals(that.rootRequestId) && - leafRequestId.equals(that.leafRequestId) && - requestedDnsSans.equals(that.requestedDnsSans) && - issuer.equals(that.issuer) && - expiry.equals(that.expiry) && - lastRefreshed.equals(that.lastRefreshed); - } - - @Override - public int hashCode() { - return Objects.hash(keyName, certName, version, lastRequested, rootRequestId, leafRequestId, requestedDnsSans, issuer, expiry, lastRefreshed); + this.lastRefreshed, + this.randomizedId); } } 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 7ae03d7ce6b..a0448e41b68 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 @@ -1,9 +1,6 @@ // Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. package com.yahoo.vespa.hosted.controller.api.integration.certificates; -import com.yahoo.config.provision.ApplicationId; - -import java.time.Clock; import java.time.Instant; import java.util.Collections; import java.util.HashMap; @@ -11,7 +8,6 @@ import java.util.List; import java.util.Map; import java.util.Optional; import java.util.UUID; -import java.util.stream.Collectors; /** * @author tokle @@ -19,29 +15,23 @@ import java.util.stream.Collectors; */ public class EndpointCertificateMock implements EndpointCertificateProvider { - private final Map<ApplicationId, List<String>> dnsNames = new HashMap<>(); + private final Map<String, List<String>> dnsNames = new HashMap<>(); private final Map<String, EndpointCertificateMetadata> providerMetadata = new HashMap<>(); - private final Clock clock; - - public EndpointCertificateMock(Clock clock) { - this.clock = clock; - } - public List<String> dnsNamesOf(ApplicationId application) { - return Collections.unmodifiableList(dnsNames.getOrDefault(application, List.of())); + public List<String> dnsNamesOf(String rootRequestId) { + return Collections.unmodifiableList(dnsNames.getOrDefault(rootRequestId, List.of())); } @Override - public EndpointCertificateMetadata requestCaSignedCertificate(ApplicationId applicationId, List<String> dnsNames, Optional<EndpointCertificateMetadata> currentMetadata) { - this.dnsNames.put(applicationId, dnsNames); - String endpointCertificatePrefix = String.format("vespa.tls.%s.%s.%s", applicationId.tenant(), - applicationId.application(), applicationId.instance()); + public EndpointCertificateMetadata requestCaSignedCertificate(String key, List<String> dnsNames, Optional<EndpointCertificateMetadata> currentMetadata, String algo, boolean useAlternativeProvider) { + String endpointCertificatePrefix = "vespa.tls.%s".formatted(key); long epochSecond = Instant.now().getEpochSecond(); long inAnHour = epochSecond + 3600; String requestId = UUID.randomUUID().toString(); + this.dnsNames.put(requestId, dnsNames); int version = currentMetadata.map(c -> currentMetadata.get().version()+1).orElse(0); EndpointCertificateMetadata metadata = new EndpointCertificateMetadata(endpointCertificatePrefix + "-key", endpointCertificatePrefix + "-cert", version, 0, - currentMetadata.map(EndpointCertificateMetadata::rootRequestId).orElse(requestId), Optional.of(requestId), dnsNames, "mockCa", Optional.of(inAnHour), Optional.of(epochSecond)); + currentMetadata.map(EndpointCertificateMetadata::rootRequestId).orElse(requestId), Optional.of(requestId), dnsNames, "mockCa", Optional.of(inAnHour), Optional.of(epochSecond), Optional.empty()); currentMetadata.ifPresent(c -> providerMetadata.remove(c.leafRequestId().orElseThrow())); providerMetadata.put(requestId, metadata); return metadata; @@ -70,8 +60,8 @@ public class EndpointCertificateMock implements EndpointCertificateProvider { } @Override - public void deleteCertificate(ApplicationId applicationId, String requestId) { - dnsNames.remove(applicationId); + public void deleteCertificate(String requestId) { + dnsNames.remove(requestId); providerMetadata.remove(requestId); } 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 26db25bd848..7c5268ea353 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 @@ -1,9 +1,6 @@ // Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. package com.yahoo.vespa.hosted.controller.api.integration.certificates; -import com.yahoo.config.provision.ApplicationId; - -import java.io.IOException; import java.util.List; import java.util.Optional; @@ -14,11 +11,11 @@ import java.util.Optional; */ public interface EndpointCertificateProvider { - EndpointCertificateMetadata requestCaSignedCertificate(ApplicationId applicationId, List<String> dnsNames, Optional<EndpointCertificateMetadata> currentMetadata); + EndpointCertificateMetadata requestCaSignedCertificate(String endpointCertificatePrefix, List<String> dnsNames, Optional<EndpointCertificateMetadata> currentMetadata, String algo, boolean useAlternativeProvider); List<EndpointCertificateRequestMetadata> listCertificates(); - void deleteCertificate(ApplicationId applicationId, String requestId); + void deleteCertificate(String requestId); EndpointCertificateDetails certificateDetails(String requestId); } diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/application/Endpoint.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/application/Endpoint.java index 7b2e8d0f4ed..1c19810c37e 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/application/Endpoint.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/application/Endpoint.java @@ -245,7 +245,7 @@ public class Endpoint { } /** Returns the DNS suffix used for endpoints in given system */ - private static String dnsSuffix(SystemName system) { + public static String dnsSuffix(SystemName system) { return switch (system) { case cd -> CD_OATH_DNS_SUFFIX; case main -> MAIN_OATH_DNS_SUFFIX; diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/certificate/AssignedCertificate.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/certificate/AssignedCertificate.java new file mode 100644 index 00000000000..0c8c64827fb --- /dev/null +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/certificate/AssignedCertificate.java @@ -0,0 +1,24 @@ +// Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. +package com.yahoo.vespa.hosted.controller.certificate; + +import com.yahoo.config.provision.InstanceName; +import com.yahoo.vespa.hosted.controller.api.integration.certificates.EndpointCertificateMetadata; +import com.yahoo.vespa.hosted.controller.application.TenantAndApplicationId; + +import java.util.Optional; + +/** + * Represents a certificate and its owner. A certificate is either assigned to all instances of an application, or a + * specific one. + * + * @author mpolden + */ +public record AssignedCertificate(TenantAndApplicationId application, + Optional<InstanceName> instance, + EndpointCertificateMetadata certificate) { + + public AssignedCertificate with(EndpointCertificateMetadata certificate) { + return new AssignedCertificate(application, instance, certificate); + } + +} diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/certificate/EndpointCertificates.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/certificate/EndpointCertificates.java index 4c869a5aabe..0f3f9479176 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/certificate/EndpointCertificates.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/certificate/EndpointCertificates.java @@ -4,8 +4,16 @@ package com.yahoo.vespa.hosted.controller.certificate; import com.yahoo.config.application.api.DeploymentInstanceSpec; import com.yahoo.config.application.api.DeploymentSpec; import com.yahoo.config.provision.CloudName; +import com.yahoo.config.provision.InstanceName; import com.yahoo.config.provision.zone.ZoneId; import com.yahoo.text.Text; +import com.yahoo.transaction.Mutex; +import com.yahoo.transaction.NestedTransaction; +import com.yahoo.vespa.flags.BooleanFlag; +import com.yahoo.vespa.flags.FetchVector; +import com.yahoo.vespa.flags.Flags; +import com.yahoo.vespa.flags.PermanentFlags; +import com.yahoo.vespa.flags.StringFlag; import com.yahoo.vespa.hosted.controller.Controller; import com.yahoo.vespa.hosted.controller.Instance; import com.yahoo.vespa.hosted.controller.api.identifiers.DeploymentId; @@ -13,11 +21,13 @@ import com.yahoo.vespa.hosted.controller.api.integration.certificates.EndpointCe import com.yahoo.vespa.hosted.controller.api.integration.certificates.EndpointCertificateProvider; import com.yahoo.vespa.hosted.controller.api.integration.certificates.EndpointCertificateValidator; import com.yahoo.vespa.hosted.controller.api.integration.secrets.GcpSecretStore; +import com.yahoo.vespa.hosted.controller.application.TenantAndApplicationId; import com.yahoo.vespa.hosted.controller.persistence.CuratorDb; import java.time.Clock; import java.time.Duration; import java.time.Instant; +import java.util.Comparator; import java.util.LinkedHashSet; import java.util.List; import java.util.Optional; @@ -26,6 +36,8 @@ import java.util.logging.Level; import java.util.logging.Logger; import java.util.stream.Collectors; +import static com.yahoo.vespa.hosted.controller.certificate.UnassignedCertificate.*; + /** * Looks up stored endpoint certificate metadata, provisions new certificates if none is found, * and re-provisions the certificate if the deploying-to zone is not covered. @@ -44,10 +56,16 @@ public class EndpointCertificates { private final Clock clock; private final EndpointCertificateProvider certificateProvider; private final EndpointCertificateValidator certificateValidator; + private final BooleanFlag useRandomizedCert; + private final BooleanFlag useAlternateCertProvider; + private final StringFlag endpointCertificateAlgo; public EndpointCertificates(Controller controller, EndpointCertificateProvider certificateProvider, EndpointCertificateValidator certificateValidator) { this.controller = controller; + this.useRandomizedCert = Flags.RANDOMIZED_ENDPOINT_NAMES.bindTo(controller.flagSource()); + this.useAlternateCertProvider = PermanentFlags.USE_ALTERNATIVE_ENDPOINT_CERTIFICATE_PROVIDER.bindTo(controller.flagSource()); + this.endpointCertificateAlgo = PermanentFlags.ENDPOINT_CERTIFICATE_ALGORITHM.bindTo(controller.flagSource()); this.curator = controller.curator(); this.clock = controller.clock(); this.certificateProvider = certificateProvider; @@ -58,13 +76,12 @@ public class EndpointCertificates { public Optional<EndpointCertificateMetadata> getMetadata(Instance instance, ZoneId zone, DeploymentSpec deploymentSpec) { Instant start = clock.instant(); Optional<EndpointCertificateMetadata> metadata = getOrProvision(instance, zone, deploymentSpec); - metadata.ifPresent(m -> curator.writeEndpointCertificateMetadata(instance.id(), m.withLastRequested(clock.instant().getEpochSecond()))); Duration duration = Duration.between(start, clock.instant()); if (duration.toSeconds() > 30) log.log(Level.INFO, Text.format("Getting endpoint certificate metadata for %s took %d seconds!", instance.id().serializedForm(), duration.toSeconds())); if (controller.zoneRegistry().zones().all().in(CloudName.GCP).ids().contains(zone)) { // Until CKMS is available from GCP - if(metadata.isPresent()) { + if (metadata.isPresent()) { // Validate metadata before copying cert to GCP. This will ensure we don't bug out on the first deployment, but will take more time certificateValidator.validate(metadata.get(), instance.id().serializedForm(), zone, controller.routing().certificateDnsNames(new DeploymentId(instance.id(), zone), deploymentSpec)); var m = metadata.get(); @@ -83,26 +100,65 @@ public class EndpointCertificates { return metadata; } + private EndpointCertificateMetadata assignFromPool(Instance instance, ZoneId zone) { + // Assign certificate per instance only in manually deployed environments. In other environments, we share the + // certificate because application endpoints can span instances + Optional<InstanceName> instanceName = zone.environment().isManuallyDeployed() ? Optional.of(instance.name()) : Optional.empty(); + TenantAndApplicationId application = TenantAndApplicationId.from(instance.id()); + Optional<AssignedCertificate> assignedCertificate = curator.readAssignedCertificate(application, instanceName); + if (assignedCertificate.isPresent()) { + AssignedCertificate updated = assignedCertificate.get().with(assignedCertificate.get().certificate().withLastRequested(clock.instant().getEpochSecond())); + curator.writeAssignedCertificate(updated); + return updated.certificate(); + } + try (Mutex lock = controller.curator().lockCertificatePool()) { + Optional<UnassignedCertificate> candidate = curator.readUnassignedCertificates().stream() + .filter(pc -> pc.state() == State.ready) + .min(Comparator.comparingLong(pc -> pc.certificate().lastRequested())); + if (candidate.isEmpty()) { + throw new IllegalArgumentException("No endpoint certificate available in pool, for deployment of " + instance.id() + " in " + zone); + } + try (NestedTransaction transaction = new NestedTransaction()) { + curator.removeUnassignedCertificate(candidate.get(), transaction); + curator.writeAssignedCertificate(new AssignedCertificate(application, instanceName, candidate.get().certificate()), + transaction); + transaction.commit(); + return candidate.get().certificate(); + } + } + } + private Optional<EndpointCertificateMetadata> getOrProvision(Instance instance, ZoneId zone, DeploymentSpec deploymentSpec) { - Optional<EndpointCertificateMetadata> currentCertificateMetadata = curator.readEndpointCertificateMetadata(instance.id()); + if (useRandomizedCert.with(FetchVector.Dimension.APPLICATION_ID, instance.id().toFullString()).value()) { + return Optional.of(assignFromPool(instance, zone)); + } + Optional<AssignedCertificate> assignedCertificate = curator.readAssignedCertificate(TenantAndApplicationId.from(instance.id()), Optional.of(instance.id().instance())); DeploymentId deployment = new DeploymentId(instance.id(), zone); - if (currentCertificateMetadata.isEmpty()) { + if (assignedCertificate.isEmpty()) { var provisionedCertificateMetadata = provisionEndpointCertificate(deployment, Optional.empty(), deploymentSpec); // We do not verify the certificate if one has never existed before - because we do not want to // wait for it to be available before we deploy. This allows the config server to start // provisioning nodes ASAP, and the risk is small for a new deployment. - curator.writeEndpointCertificateMetadata(instance.id(), provisionedCertificateMetadata); + curator.writeAssignedCertificate(new AssignedCertificate(TenantAndApplicationId.from(instance.id()), Optional.of(instance.id().instance()), provisionedCertificateMetadata)); return Optional.of(provisionedCertificateMetadata); + } else { + AssignedCertificate updated = assignedCertificate.get().with(assignedCertificate.get().certificate().withLastRequested(clock.instant().getEpochSecond())); + curator.writeAssignedCertificate(updated); } // Re-provision certificate if it is missing SANs for the zone we are deploying to - var requiredSansForZone = controller.routing().certificateDnsNames(deployment, deploymentSpec); + // Skip this validation for now if the cert has a randomized id + Optional<EndpointCertificateMetadata> currentCertificateMetadata = assignedCertificate.map(AssignedCertificate::certificate); + var requiredSansForZone = currentCertificateMetadata.get().randomizedId().isEmpty() ? + controller.routing().certificateDnsNames(deployment, deploymentSpec) : + List.<String>of(); + if (!currentCertificateMetadata.get().requestedDnsSans().containsAll(requiredSansForZone)) { var reprovisionedCertificateMetadata = provisionEndpointCertificate(deployment, currentCertificateMetadata, deploymentSpec) .withRootRequestId(currentCertificateMetadata.get().rootRequestId()); // We're required to keep the original request ID - curator.writeEndpointCertificateMetadata(instance.id(), reprovisionedCertificateMetadata); + curator.writeAssignedCertificate(assignedCertificate.get().with(reprovisionedCertificateMetadata)); // Verification is unlikely to succeed in this case, as certificate must be available first - controller will retry certificateValidator.validate(reprovisionedCertificateMetadata, instance.id().serializedForm(), zone, requiredSansForZone); return Optional.of(reprovisionedCertificateMetadata); @@ -141,7 +197,15 @@ public class EndpointCertificates { .filter(currentNames::containsAll) .forEach(requiredNames::addAll); - return certificateProvider.requestCaSignedCertificate(deployment.applicationId(), List.copyOf(requiredNames), currentMetadata); + log.log(Level.INFO, String.format("Requesting new endpoint certificate from Cameo for application %s", deployment.applicationId().serializedForm())); + String algo = this.endpointCertificateAlgo.with(FetchVector.Dimension.APPLICATION_ID, deployment.applicationId().serializedForm()).value(); + boolean useAlternativeProvider = useAlternateCertProvider.with(FetchVector.Dimension.APPLICATION_ID, deployment.applicationId().serializedForm()).value(); + String keyPrefix = deployment.applicationId().toFullString(); + var t0 = Instant.now(); + EndpointCertificateMetadata endpointCertificateMetadata = certificateProvider.requestCaSignedCertificate(keyPrefix, List.copyOf(requiredNames), currentMetadata, algo, useAlternativeProvider); + var t1 = Instant.now(); + log.log(Level.INFO, String.format("Endpoint certificate request for application %s returned after %s", deployment.applicationId().serializedForm(), Duration.between(t0, t1))); + return endpointCertificateMetadata; } } diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/certificate/UnassignedCertificate.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/certificate/UnassignedCertificate.java new file mode 100644 index 00000000000..2fbff02ffa9 --- /dev/null +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/certificate/UnassignedCertificate.java @@ -0,0 +1,38 @@ +package com.yahoo.vespa.hosted.controller.certificate; + +import com.yahoo.vespa.hosted.controller.api.integration.certificates.EndpointCertificateMetadata; + +/** + * An unassigned certificate, which exists in a pre-provisioned pool of certificates. Once assigned to an application, + * the certificate is removed from the pool. + * + * @param certificate Details of the certificate + * @param state Current state of this + * + * @author andreer + */ +public record UnassignedCertificate(EndpointCertificateMetadata certificate, UnassignedCertificate.State state) { + + public UnassignedCertificate { + if (certificate.randomizedId().isEmpty()) { + throw new IllegalArgumentException("randomizedId must be set for a pooled certificate"); + } + } + + public String id() { + return certificate.randomizedId().get(); + } + + public UnassignedCertificate withState(State state) { + return new UnassignedCertificate(certificate, state); + } + + public enum State { + /** The certificate is ready for assignment */ + ready, + + /** The certificate is requested and is being provisioned */ + requested + } + +} diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/CertificatePoolMaintainer.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/CertificatePoolMaintainer.java new file mode 100644 index 00000000000..82d4f068d6d --- /dev/null +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/CertificatePoolMaintainer.java @@ -0,0 +1,140 @@ +// 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.container.jdisc.secretstore.SecretNotFoundException; +import com.yahoo.container.jdisc.secretstore.SecretStore; +import com.yahoo.jdisc.Metric; +import com.yahoo.transaction.Mutex; +import com.yahoo.vespa.flags.BooleanFlag; +import com.yahoo.vespa.flags.Flags; +import com.yahoo.vespa.flags.IntFlag; +import com.yahoo.vespa.flags.PermanentFlags; +import com.yahoo.vespa.flags.StringFlag; +import com.yahoo.vespa.hosted.controller.Controller; +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.certificate.UnassignedCertificate; +import com.yahoo.vespa.hosted.controller.application.Endpoint; +import com.yahoo.vespa.hosted.controller.certificate.AssignedCertificate; +import com.yahoo.vespa.hosted.controller.persistence.CuratorDb; + +import java.time.Duration; +import java.util.List; +import java.util.Map; +import java.util.Optional; +import java.util.OptionalInt; +import java.util.Set; +import java.util.logging.Level; +import java.util.logging.Logger; +import java.util.random.RandomGenerator; +import java.util.stream.Collectors; + +/** + * Manages pool of ready-to-use randomized endpoint certificates + * + * @author andreer + */ +public class CertificatePoolMaintainer extends ControllerMaintainer { + + private static final Logger log = Logger.getLogger(CertificatePoolMaintainer.class.getName()); + + private final RandomGenerator random; + private final CuratorDb curator; + private final SecretStore secretStore; + private final EndpointCertificateProvider endpointCertificateProvider; + private final Metric metric; + private final Controller controller; + private final IntFlag certPoolSize; + private final String dnsSuffix; + private final StringFlag endpointCertificateAlgo; + private final BooleanFlag useAlternateCertProvider; + + public CertificatePoolMaintainer(Controller controller, Metric metric, Duration interval, RandomGenerator random) { + super(controller, interval, null, Set.of(SystemName.Public, SystemName.PublicCd)); + this.controller = controller; + this.secretStore = controller.secretStore(); + this.certPoolSize = Flags.CERT_POOL_SIZE.bindTo(controller.flagSource()); + this.useAlternateCertProvider = PermanentFlags.USE_ALTERNATIVE_ENDPOINT_CERTIFICATE_PROVIDER.bindTo(controller.flagSource()); + this.endpointCertificateAlgo = PermanentFlags.ENDPOINT_CERTIFICATE_ALGORITHM.bindTo(controller.flagSource()); + this.curator = controller.curator(); + this.endpointCertificateProvider = controller.serviceRegistry().endpointCertificateProvider(); + this.metric = metric; + this.dnsSuffix = Endpoint.dnsSuffix(controller.system()); + this.random = random; + } + + protected double maintain() { + try { + moveRequestedCertsToReady(); + List<UnassignedCertificate> certificatePool = curator.readUnassignedCertificates(); + // So we can alert if the pool goes too low + metric.set("preprovisioned.endpoint.certificates", certificatePool.stream().filter(c -> c.state() == UnassignedCertificate.State.ready).count(), metric.createContext(Map.of())); + if (certificatePool.size() < certPoolSize.value()) { + provisionRandomizedCertificate(); + } + } catch (Exception e) { + log.log(Level.SEVERE, "Exception caught while maintaining pool of unused randomized endpoint certs", e); + return 1.0; + } + return 0.0; + } + + private void moveRequestedCertsToReady() { + try (Mutex lock = controller.curator().lockCertificatePool()) { + for (UnassignedCertificate cert : curator.readUnassignedCertificates()) { + if (cert.state() == UnassignedCertificate.State.ready) continue; + try { + OptionalInt maxKeyVersion = secretStore.listSecretVersions(cert.certificate().keyName()).stream().mapToInt(i -> i).max(); + OptionalInt maxCertVersion = secretStore.listSecretVersions(cert.certificate().certName()).stream().mapToInt(i -> i).max(); + if (maxKeyVersion.isPresent() && maxCertVersion.equals(maxKeyVersion)) { + curator.writeUnassignedCertificate(cert.withState(UnassignedCertificate.State.ready)); + log.log(Level.INFO, "Randomized endpoint cert %s now ready for use".formatted(cert.id())); + } + } catch (SecretNotFoundException s) { + // Likely because the certificate is very recently provisioned - ignore till next time - should we log? + log.log(Level.INFO, "Could not yet read secrets for randomized endpoint cert %s - maybe next time ...".formatted(cert.id())); + } + } + } + } + + private void provisionRandomizedCertificate() { + try (Mutex lock = controller.curator().lockCertificatePool()) { + Set<String> existingNames = controller.curator().readUnassignedCertificates().stream().map(UnassignedCertificate::id).collect(Collectors.toSet()); + + curator.readAssignedCertificates().stream() + .map(AssignedCertificate::certificate) + .map(EndpointCertificateMetadata::randomizedId) + .forEach(id -> id.ifPresent(existingNames::add)); + + String id = generateRandomId(); + while (existingNames.contains(id)) id = generateRandomId(); + + EndpointCertificateMetadata f = endpointCertificateProvider.requestCaSignedCertificate( + "preprovisioned.%s".formatted(id), + List.of( + "*.%s.z%s".formatted(id, dnsSuffix), + "*.%s.g%s".formatted(id, dnsSuffix), + "*.%s.a%s".formatted(id, dnsSuffix) + ), + Optional.empty(), + endpointCertificateAlgo.value(), + useAlternateCertProvider.value()) + .withRandomizedId(id); + + UnassignedCertificate certificate = new UnassignedCertificate(f, UnassignedCertificate.State.requested); + curator.writeUnassignedCertificate(certificate); + } + } + + private String generateRandomId() { + String alphabet = "abcdef0123456789"; + StringBuilder sb = new StringBuilder(); + sb.append(alphabet.charAt(random.nextInt(6))); // start with letter + for (int i = 0; i < 7; i++) { + sb.append(alphabet.charAt(random.nextInt(alphabet.length()))); + } + return sb.toString(); + } +} 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 c83ddbd8045..65ca2028c5f 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 @@ -11,11 +11,13 @@ import com.yahoo.vespa.hosted.controller.Controller; import com.yahoo.vespa.hosted.controller.api.integration.athenz.AthenzClientFactory; import com.yahoo.vespa.hosted.controller.api.integration.user.UserManagement; +import java.security.SecureRandom; import java.time.Duration; import java.time.temporal.TemporalUnit; import java.util.Collections; import java.util.List; import java.util.Objects; +import java.util.Random; import java.util.concurrent.CopyOnWriteArrayList; import java.util.stream.Collectors; @@ -83,6 +85,7 @@ public class ControllerMaintenance extends AbstractComponent { maintainers.add(new BillingDatabaseMaintainer(controller, intervals.billingDatabaseMaintainer)); maintainers.add(new MeteringMonitorMaintainer(controller, intervals.meteringMonitorMaintainer, controller.serviceRegistry().resourceDatabase(), metric)); maintainers.add(new EnclaveAccessMaintainer(controller, intervals.defaultInterval)); + maintainers.add(new CertificatePoolMaintainer(controller, metric, intervals.certificatePoolMaintainer, new SecureRandom())); } public Upgrader upgrader() { return upgrader; } @@ -143,6 +146,7 @@ public class ControllerMaintenance extends AbstractComponent { private final Duration userManagementMaintainer; private final Duration billingDatabaseMaintainer; private final Duration meteringMonitorMaintainer; + private final Duration certificatePoolMaintainer; public Intervals(SystemName system) { this.system = Objects.requireNonNull(system); @@ -178,6 +182,7 @@ public class ControllerMaintenance extends AbstractComponent { this.userManagementMaintainer = duration(12, HOURS); this.billingDatabaseMaintainer = duration(5, MINUTES); this.meteringMonitorMaintainer = duration(30, MINUTES); + this.certificatePoolMaintainer = duration(15, MINUTES); } private Duration duration(long amount, TemporalUnit unit) { 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 713782eb7b9..935d562a8c2 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 @@ -4,19 +4,23 @@ package com.yahoo.vespa.hosted.controller.maintenance; import com.google.common.collect.Sets; import com.yahoo.component.annotation.Inject; import com.yahoo.config.provision.ApplicationId; +import com.yahoo.config.provision.InstanceName; import com.yahoo.container.jdisc.secretstore.SecretNotFoundException; import com.yahoo.container.jdisc.secretstore.SecretStore; import com.yahoo.transaction.Mutex; +import com.yahoo.transaction.NestedTransaction; +import com.yahoo.vespa.hosted.controller.Application; import com.yahoo.vespa.hosted.controller.Controller; -import com.yahoo.vespa.hosted.controller.Instance; import com.yahoo.vespa.hosted.controller.api.integration.certificates.EndpointCertificateDetails; 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.certificates.EndpointCertificateRequestMetadata; +import com.yahoo.vespa.hosted.controller.certificate.UnassignedCertificate; import com.yahoo.vespa.hosted.controller.api.integration.deployment.JobType; import com.yahoo.vespa.hosted.controller.api.integration.secrets.EndpointSecretManager; import com.yahoo.vespa.hosted.controller.application.Deployment; import com.yahoo.vespa.hosted.controller.application.TenantAndApplicationId; +import com.yahoo.vespa.hosted.controller.certificate.AssignedCertificate; import com.yahoo.vespa.hosted.controller.deployment.DeploymentTrigger; import com.yahoo.vespa.hosted.controller.persistence.CuratorDb; @@ -28,7 +32,6 @@ import java.util.ArrayList; import java.util.Comparator; import java.util.HashSet; import java.util.List; -import java.util.Map; import java.util.Optional; import java.util.OptionalInt; import java.util.logging.Level; @@ -82,23 +85,32 @@ public class EndpointCertificateMaintainer extends ControllerMaintainer { } private void updateRefreshedCertificates() { - curator.readAllEndpointCertificateMetadata().forEach(((applicationId, endpointCertificateMetadata) -> { + curator.readAssignedCertificates().forEach(assignedCertificate -> { // Look for and use refreshed certificate - var latestAvailableVersion = latestVersionInSecretStore(endpointCertificateMetadata); - if (latestAvailableVersion.isPresent() && latestAvailableVersion.getAsInt() > endpointCertificateMetadata.version()) { - var refreshedCertificateMetadata = endpointCertificateMetadata + var latestAvailableVersion = latestVersionInSecretStore(assignedCertificate.certificate()); + if (latestAvailableVersion.isPresent() && latestAvailableVersion.getAsInt() > assignedCertificate.certificate().version()) { + var refreshedCertificateMetadata = assignedCertificate.certificate() .withVersion(latestAvailableVersion.getAsInt()) .withLastRefreshed(clock.instant().getEpochSecond()); - try (Mutex lock = lock(applicationId)) { - if (Optional.of(endpointCertificateMetadata).equals(curator.readEndpointCertificateMetadata(applicationId))) { - curator.writeEndpointCertificateMetadata(applicationId, refreshedCertificateMetadata); // Certificate not validated here, but on deploy. + + try (Mutex lock = lock(assignedCertificate.application())) { + if (unchanged(assignedCertificate, lock)) { + try (NestedTransaction transaction = new NestedTransaction()) { + curator.writeAssignedCertificate(assignedCertificate.with(refreshedCertificateMetadata), transaction); // Certificate not validated here, but on deploy. + transaction.commit(); + } } } } - })); + }); + } + + private boolean unchanged(AssignedCertificate assignedCertificate, @SuppressWarnings("unused") Mutex lock) { + return Optional.of(assignedCertificate).equals(curator.readAssignedCertificate(assignedCertificate.application(), assignedCertificate.instance())); } record EligibleJob(Deployment deployment, ApplicationId applicationId, JobType job) {} + /** * If it's been four days since the cert has been refreshed, re-trigger prod deployment jobs (one at a time). */ @@ -106,17 +118,32 @@ public class EndpointCertificateMaintainer extends ControllerMaintainer { var now = clock.instant(); var eligibleJobs = new ArrayList<EligibleJob>(); - curator.readAllEndpointCertificateMetadata().forEach((applicationId, endpointCertificateMetadata) -> - endpointCertificateMetadata.lastRefreshed().ifPresent(lastRefreshTime -> { + curator.readAssignedCertificates().forEach(assignedCertificate -> + assignedCertificate.certificate().lastRefreshed().ifPresent(lastRefreshTime -> { Instant refreshTime = Instant.ofEpochSecond(lastRefreshTime); if (now.isAfter(refreshTime.plus(4, ChronoUnit.DAYS))) { - controller().applications().getInstance(applicationId) - .ifPresent(instance -> instance.productionDeployments().forEach((zone, deployment) -> { - if (deployment.at().isBefore(refreshTime)) { - JobType job = JobType.deploymentTo(zone); - eligibleJobs.add(new EligibleJob(deployment, applicationId, job)); - } - })); + if (assignedCertificate.instance().isPresent()) { + ApplicationId applicationId = assignedCertificate.application().instance(assignedCertificate.instance().get()); + controller().applications().getInstance(applicationId) + .ifPresent(instance -> instance.productionDeployments().forEach((zone, deployment) -> { + if (deployment.at().isBefore(refreshTime)) { + JobType job = JobType.deploymentTo(zone); + eligibleJobs.add(new EligibleJob(deployment, applicationId, job)); + } + })); + } else { + // This is an application-wide certificate. Trigger all instances + controller().applications().getApplication(assignedCertificate.application()).ifPresent(application -> { + application.instances().forEach((ignored, i) -> { + i.productionDeployments().forEach((zone, deployment) -> { + if (deployment.at().isBefore(refreshTime)) { + JobType job = JobType.deploymentTo(zone); + eligibleJobs.add(new EligibleJob(deployment, i.id(), job)); + } + }); + }); + }); + } } })); @@ -141,57 +168,71 @@ public class EndpointCertificateMaintainer extends ControllerMaintainer { private void deleteUnusedCertificates() { 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 (Mutex lock = lock(applicationId)) { - if (Optional.of(storedMetaData).equals(curator.readEndpointCertificateMetadata(applicationId))) { - log.log(Level.INFO, "Cert for app " + applicationId.serializedForm() + curator.readAssignedCertificates().forEach(assignedCertificate -> { + EndpointCertificateMetadata certificate = assignedCertificate.certificate(); + var lastRequested = Instant.ofEpochSecond(certificate.lastRequested()); + if (lastRequested.isBefore(oneMonthAgo) && hasNoDeployments(assignedCertificate.application())) { + try (Mutex lock = lock(assignedCertificate.application())) { + if (unchanged(assignedCertificate, lock)) { + log.log(Level.INFO, "Cert for app " + asString(assignedCertificate.application(), assignedCertificate.instance()) + " has not been requested in a month and app has no deployments, deleting from provider, ZK and secret store"); - endpointCertificateProvider.deleteCertificate(applicationId, storedMetaData.rootRequestId()); - curator.deleteEndpointCertificateMetadata(applicationId); - endpointSecretManager.deleteSecret(storedMetaData.certName()); - endpointSecretManager.deleteSecret(storedMetaData.keyName()); + endpointCertificateProvider.deleteCertificate(certificate.rootRequestId()); + curator.removeAssignedCertificate(assignedCertificate.application(), assignedCertificate.instance()); + endpointSecretManager.deleteSecret(certificate.certName()); + endpointSecretManager.deleteSecret(certificate.keyName()); } } } }); } - private Mutex lock(ApplicationId applicationId) { - return curator.lock(TenantAndApplicationId.from(applicationId)); + private Mutex lock(TenantAndApplicationId application) { + return curator.lock(application); } - private boolean hasNoDeployments(ApplicationId applicationId) { - return controller().applications().getInstance(applicationId) - .map(Instance::deployments) - .orElseGet(Map::of) - .isEmpty(); + private boolean hasNoDeployments(TenantAndApplicationId application) { + Optional<Application> app = controller().applications().getApplication(application); + if (app.isEmpty()) return true; + for (var instance : app.get().instances().values()) { + if (!instance.deployments().isEmpty()) return false; + } + return true; } private void deleteOrReportUnmanagedCertificates() { List<EndpointCertificateRequestMetadata> endpointCertificateMetadata = endpointCertificateProvider.listCertificates(); - Map<ApplicationId, EndpointCertificateMetadata> storedEndpointCertificateMetadata = curator.readAllEndpointCertificateMetadata(); + List<AssignedCertificate> assignedCertificates = curator.readAssignedCertificates(); + + List<String> leafRequestIds = assignedCertificates.stream().map(AssignedCertificate::certificate).flatMap(m -> m.leafRequestId().stream()).toList(); + List<String> rootRequestIds = assignedCertificates.stream().map(AssignedCertificate::certificate).map(EndpointCertificateMetadata::rootRequestId).toList(); + List<UnassignedCertificate> unassignedCertificates = curator.readUnassignedCertificates(); + List<String> certPoolRootIds = unassignedCertificates.stream().map(p -> p.certificate().leafRequestId()).flatMap(Optional::stream).toList(); + List<String> certPoolLeafIds = unassignedCertificates.stream().map(p -> p.certificate().rootRequestId()).toList(); - List<String> leafRequestIds = storedEndpointCertificateMetadata.values().stream().flatMap(m -> m.leafRequestId().stream()).toList(); - List<String> rootRequestIds = storedEndpointCertificateMetadata.values().stream().map(EndpointCertificateMetadata::rootRequestId).toList(); + var managedIds = new HashSet<String>(); + managedIds.addAll(leafRequestIds); + managedIds.addAll(rootRequestIds); + managedIds.addAll(certPoolRootIds); + managedIds.addAll(certPoolLeafIds); for (var providerCertificateMetadata : endpointCertificateMetadata) { - if (!rootRequestIds.contains(providerCertificateMetadata.requestId()) && !leafRequestIds.contains(providerCertificateMetadata.requestId())) { + if (!managedIds.contains(providerCertificateMetadata.requestId())) { // It could just be a refresh we're not aware of yet. See if it matches the cert/keyname of any known cert EndpointCertificateDetails unknownCertDetails = endpointCertificateProvider.certificateDetails(providerCertificateMetadata.requestId()); boolean matchFound = false; - for (Map.Entry<ApplicationId, EndpointCertificateMetadata> storedAppEntry : storedEndpointCertificateMetadata.entrySet()) { - ApplicationId storedApp = storedAppEntry.getKey(); - EndpointCertificateMetadata storedAppMetadata = storedAppEntry.getValue(); - if (storedAppMetadata.certName().equals(unknownCertDetails.cert_key_keyname())) { + for (AssignedCertificate assignedCertificate : assignedCertificates) { + if (assignedCertificate.certificate().certName().equals(unknownCertDetails.cert_key_keyname())) { matchFound = true; - try (Mutex lock = lock(storedApp)) { - if (Optional.of(storedAppMetadata).equals(curator.readEndpointCertificateMetadata(storedApp))) { - log.log(Level.INFO, "Cert for app " + storedApp.serializedForm() + try (Mutex lock = lock(assignedCertificate.application())) { + if (unchanged(assignedCertificate, lock)) { + log.log(Level.INFO, "Cert for app " + asString(assignedCertificate.application(), assignedCertificate.instance()) + " has a new leafRequestId " + unknownCertDetails.request_id() + ", updating in ZK"); - curator.writeEndpointCertificateMetadata(storedApp, storedAppMetadata.withLeafRequestId(Optional.of(unknownCertDetails.request_id()))); + try (NestedTransaction transaction = new NestedTransaction()) { + EndpointCertificateMetadata updated = assignedCertificate.certificate().withLeafRequestId(Optional.of(unknownCertDetails.request_id())); + curator.writeAssignedCertificate(assignedCertificate.with(updated), transaction); + transaction.commit(); + } } break; } @@ -204,10 +245,15 @@ public class EndpointCertificateMaintainer extends ControllerMaintainer { log.log(Level.INFO, String.format("Deleting unmaintained certificate with request_id %s and SANs %s", providerCertificateMetadata.requestId(), providerCertificateMetadata.dnsNames().stream().map(d -> d.dnsName).collect(Collectors.joining(", ")))); - endpointCertificateProvider.deleteCertificate(ApplicationId.fromSerializedForm("applicationid:is:unknown"), providerCertificateMetadata.requestId()); + endpointCertificateProvider.deleteCertificate(providerCertificateMetadata.requestId()); } } } } } + + private static String asString(TenantAndApplicationId application, Optional<InstanceName> instanceName) { + return application.toString() + instanceName.map(name -> "." + name.value()).orElse(""); + } + } 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 ce7129b7b8c..942d3167f7a 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 @@ -8,19 +8,23 @@ import com.yahoo.concurrent.UncheckedTimeoutException; import com.yahoo.config.provision.ApplicationId; import com.yahoo.config.provision.ClusterSpec; import com.yahoo.config.provision.HostName; +import com.yahoo.config.provision.InstanceName; import com.yahoo.config.provision.TenantName; import com.yahoo.config.provision.zone.ZoneId; import com.yahoo.path.Path; import com.yahoo.slime.Slime; import com.yahoo.slime.SlimeUtils; import com.yahoo.transaction.Mutex; +import com.yahoo.transaction.NestedTransaction; import com.yahoo.vespa.curator.Curator; +import com.yahoo.vespa.curator.transaction.CuratorOperation; +import com.yahoo.vespa.curator.transaction.CuratorOperations; +import com.yahoo.vespa.curator.transaction.CuratorTransaction; import com.yahoo.vespa.hosted.controller.Application; import com.yahoo.vespa.hosted.controller.api.identifiers.ClusterId; import com.yahoo.vespa.hosted.controller.api.identifiers.ControllerVersion; import com.yahoo.vespa.hosted.controller.api.identifiers.DeploymentId; import com.yahoo.vespa.hosted.controller.api.integration.archive.ArchiveBuckets; -import com.yahoo.vespa.hosted.controller.api.integration.certificates.EndpointCertificateMetadata; import com.yahoo.vespa.hosted.controller.api.integration.dataplanetoken.DataplaneTokenVersions; import com.yahoo.vespa.hosted.controller.api.integration.deployment.JobType; import com.yahoo.vespa.hosted.controller.api.integration.deployment.RunId; @@ -28,6 +32,8 @@ import com.yahoo.vespa.hosted.controller.api.integration.dns.VpcEndpointService. import com.yahoo.vespa.hosted.controller.api.integration.vcmr.VespaChangeRequest; import com.yahoo.vespa.hosted.controller.application.TenantAndApplicationId; import com.yahoo.vespa.hosted.controller.auditlog.AuditLog; +import com.yahoo.vespa.hosted.controller.certificate.AssignedCertificate; +import com.yahoo.vespa.hosted.controller.certificate.UnassignedCertificate; import com.yahoo.vespa.hosted.controller.deployment.RetriggerEntry; import com.yahoo.vespa.hosted.controller.deployment.RetriggerEntrySerializer; import com.yahoo.vespa.hosted.controller.deployment.Run; @@ -52,7 +58,6 @@ import java.time.Duration; import java.util.ArrayList; import java.util.Arrays; import java.util.Collections; -import java.util.HashMap; import java.util.List; import java.util.Map; import java.util.NavigableMap; @@ -103,6 +108,7 @@ public class CuratorDb { private static final Path supportAccessRoot = root.append("supportAccess"); private static final Path mailVerificationRoot = root.append("mailVerification"); private static final Path dataPlaneTokenRoot = root.append("dataplaneTokens"); + private static final Path certificatePoolRoot = root.append("certificatePool"); private final NodeVersionSerializer nodeVersionSerializer = new NodeVersionSerializer(); private final VersionStatusSerializer versionStatusSerializer = new VersionStatusSerializer(nodeVersionSerializer); @@ -121,6 +127,7 @@ public class CuratorDb { private final RetriggerEntrySerializer retriggerEntrySerializer = new RetriggerEntrySerializer(); private final NotificationsSerializer notificationsSerializer = new NotificationsSerializer(); private final DnsChallengeSerializer dnsChallengeSerializer = new DnsChallengeSerializer(); + private final UnassignedCertificateSerializer unassignedCertificateSerializer = new UnassignedCertificateSerializer(); private final Curator curator; private final Duration tryLockTimeout; @@ -248,6 +255,10 @@ public class CuratorDb { return curator.lock(lockRoot.append("pendingMailVerification").append(verificationCode), defaultLockTimeout); } + public Mutex lockCertificatePool() { + return curator.lock(lockRoot.append("certificatePool"), defaultLockTimeout); + } + // -------------- Helpers ------------------------------------------ /** Try locking with a low timeout, meaning it is OK to fail lock acquisition. @@ -597,27 +608,54 @@ public class CuratorDb { // -------------- Application endpoint certificates ---------------------------- - public void writeEndpointCertificateMetadata(ApplicationId applicationId, EndpointCertificateMetadata endpointCertificateMetadata) { - curator.set(endpointCertificatePath(applicationId), asJson(EndpointCertificateMetadataSerializer.toSlime(endpointCertificateMetadata))); + public void writeAssignedCertificate(AssignedCertificate certificate) { + try (NestedTransaction transaction = new NestedTransaction()) { + writeAssignedCertificate(certificate, transaction); + transaction.commit(); + } } - public void deleteEndpointCertificateMetadata(ApplicationId applicationId) { - curator.delete(endpointCertificatePath(applicationId)); + public void writeAssignedCertificate(AssignedCertificate certificate, NestedTransaction transaction) { + Path path = endpointCertificatePath(certificate.application(), certificate.instance()); + curator.create(path); + CuratorOperation operation = CuratorOperations.setData(path.getAbsolute(), + asJson(EndpointCertificateMetadataSerializer.toSlime(certificate.certificate()))); + transaction.add(CuratorTransaction.from(operation, curator)); } - public Optional<EndpointCertificateMetadata> readEndpointCertificateMetadata(ApplicationId applicationId) { - return curator.getData(endpointCertificatePath(applicationId)).map(String::new).map(EndpointCertificateMetadataSerializer::fromJsonString); + public void removeAssignedCertificate(TenantAndApplicationId application, Optional<InstanceName> instanceName) { + curator.delete(endpointCertificatePath(application, instanceName)); } - public Map<ApplicationId, EndpointCertificateMetadata> readAllEndpointCertificateMetadata() { - Map<ApplicationId, EndpointCertificateMetadata> allEndpointCertificateMetadata = new HashMap<>(); + // TODO(mpolden): Remove this. Caller should make an explicit decision to read certificate for a particular instance + public Optional<AssignedCertificate> readAssignedCertificate(ApplicationId applicationId) { + return readAssignedCertificate(TenantAndApplicationId.from(applicationId), Optional.of(applicationId.instance())); + } - for (String appIdString : curator.getChildren(endpointCertificateRoot)) { - ApplicationId applicationId = ApplicationId.fromSerializedForm(appIdString); - Optional<EndpointCertificateMetadata> endpointCertificateMetadata = readEndpointCertificateMetadata(applicationId); - allEndpointCertificateMetadata.put(applicationId, endpointCertificateMetadata.orElseThrow()); + public Optional<AssignedCertificate> readAssignedCertificate(TenantAndApplicationId application, Optional<InstanceName> instance) { + return readSlime(endpointCertificatePath(application, instance)).map(Slime::get) + .map(EndpointCertificateMetadataSerializer::fromSlime) + .map(cert -> new AssignedCertificate(application, instance, cert)); + } + + public List<AssignedCertificate> readAssignedCertificates() { + List<AssignedCertificate> certificates = new ArrayList<>(); + for (String value : curator.getChildren(endpointCertificateRoot)) { + final TenantAndApplicationId application; + final Optional<InstanceName> instanceName; + if (value.split(":").length == 3) { + ApplicationId instance = ApplicationId.fromSerializedForm(value); + application = TenantAndApplicationId.from(instance); + instanceName = Optional.of(instance.instance()); + } else { + application = TenantAndApplicationId.fromSerialized(value); + instanceName = Optional.empty(); + } + Optional<AssignedCertificate> assigned = readAssignedCertificate(application, instanceName); + if (assigned.isEmpty()) continue; // Deleted while reading + certificates.add(assigned.get()); } - return allEndpointCertificateMetadata; + return certificates; } // -------------- Metering view refresh times ---------------------------- @@ -739,6 +777,26 @@ public class CuratorDb { return readSlime(dataplaneTokenPath(tenantName)).map(DataplaneTokenSerializer::fromSlime).orElse(List.of()); } + // -------------- Endpoint certificate pool ------------------------------- + + public void writeUnassignedCertificate(UnassignedCertificate certificate) { + curator.set(certificatePoolPath(certificate.id()), asJson(unassignedCertificateSerializer.toSlime(certificate))); + } + + public Optional<UnassignedCertificate> readUnassignedCertificate(String id) { + return readSlime(certificatePoolPath(id)).map(unassignedCertificateSerializer::fromSlime); + } + + public void removeUnassignedCertificate(UnassignedCertificate certificate, NestedTransaction transaction) { + Path path = certificatePoolPath(certificate.id()); + CuratorTransaction curatorTransaction = CuratorTransaction.from(CuratorOperations.delete(path.getAbsolute()), curator); + transaction.add(curatorTransaction); + } + + public List<UnassignedCertificate> readUnassignedCertificates() { + return curator.getChildren(certificatePoolRoot).stream().flatMap(id -> readUnassignedCertificate(id).stream()).toList(); + } + // -------------- Paths --------------------------------------------------- private static Path upgradesPerMinutePath() { @@ -811,8 +869,10 @@ public class CuratorDb { return controllerRoot.append(hostname); } - private static Path endpointCertificatePath(ApplicationId id) { - return endpointCertificateRoot.append(id.serializedForm()); + private static Path endpointCertificatePath(TenantAndApplicationId application, Optional<InstanceName> instance) { + String id = instance.map(name -> application.instance(name).serializedForm()) + .orElseGet(application::serialized); + return endpointCertificateRoot.append(id); } private static Path meteringRefreshPath() { @@ -847,4 +907,8 @@ public class CuratorDb { return dataPlaneTokenRoot.append(tenantName.value()); } + private static Path certificatePoolPath(String id) { + return certificatePoolRoot.append(id); + } + } diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/persistence/EndpointCertificateMetadataSerializer.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/persistence/EndpointCertificateMetadataSerializer.java index 02b718829f3..822efcc7163 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/persistence/EndpointCertificateMetadataSerializer.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/persistence/EndpointCertificateMetadataSerializer.java @@ -9,7 +9,6 @@ import com.yahoo.slime.Type; import com.yahoo.vespa.hosted.controller.api.integration.certificates.EndpointCertificateMetadata; import java.util.Optional; -import java.util.stream.Collectors; import java.util.stream.IntStream; /** @@ -22,7 +21,7 @@ import java.util.stream.IntStream; */ public class EndpointCertificateMetadataSerializer { - // WARNING: Since there are multiple servers in a ZooKeeper cluster and they upgrade one by one + // WARNING: Since there are multiple servers in a ZooKeeper cluster, and they upgrade one by one // (and rewrite all nodes on startup), changes to the serialized format must be made // such that what is serialized on version N+1 can be read by version N: // - ADDING FIELDS: Always ok @@ -39,10 +38,16 @@ public class EndpointCertificateMetadataSerializer { private final static String issuerField = "issuer"; private final static String expiryField = "expiry"; private final static String lastRefreshedField = "lastRefreshed"; + private final static String randomizedIdField = "randomizedId"; public static Slime toSlime(EndpointCertificateMetadata metadata) { Slime slime = new Slime(); Cursor object = slime.setObject(); + toSlime(metadata, object); + return slime; + } + + public static void toSlime(EndpointCertificateMetadata metadata, Cursor object) { object.setString(keyNameField, metadata.keyName()); object.setString(certNameField, metadata.certName()); object.setLong(versionField, metadata.version()); @@ -54,8 +59,7 @@ public class EndpointCertificateMetadataSerializer { object.setString(issuerField, metadata.issuer()); metadata.expiry().ifPresent(expiry -> object.setLong(expiryField, expiry)); metadata.lastRefreshed().ifPresent(refreshTime -> object.setLong(lastRefreshedField, refreshTime)); - - return slime; + metadata.randomizedId().ifPresent(randomizedId -> object.setString(randomizedIdField, randomizedId)); } public static EndpointCertificateMetadata fromSlime(Inspector inspector) { @@ -77,10 +81,14 @@ public class EndpointCertificateMetadataSerializer { Optional.empty(), inspector.field(lastRefreshedField).valid() ? Optional.of(inspector.field(lastRefreshedField).asLong()) : + Optional.empty(), + inspector.field(randomizedIdField).valid() ? + Optional.of(inspector.field(randomizedIdField).asString()) : Optional.empty()); } public static EndpointCertificateMetadata fromJsonString(String zkData) { return fromSlime(SlimeUtils.jsonToSlime(zkData).get()); } + } diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/persistence/UnassignedCertificateSerializer.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/persistence/UnassignedCertificateSerializer.java new file mode 100644 index 00000000000..87778f1792f --- /dev/null +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/persistence/UnassignedCertificateSerializer.java @@ -0,0 +1,32 @@ +// Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. +package com.yahoo.vespa.hosted.controller.persistence; + +import com.yahoo.slime.Cursor; +import com.yahoo.slime.Slime; +import com.yahoo.vespa.hosted.controller.api.integration.certificates.EndpointCertificateMetadata; +import com.yahoo.vespa.hosted.controller.certificate.UnassignedCertificate; + +/** + * @author mpolden + */ +public class UnassignedCertificateSerializer { + + private static final String stateKey = "state"; + private static final String certificateKey = "certificate"; + + public Slime toSlime(UnassignedCertificate unassignedCertificate) { + Slime slime = new Slime(); + Cursor root = slime.setObject(); + root.setString(stateKey, unassignedCertificate.state().name()); + EndpointCertificateMetadataSerializer.toSlime(unassignedCertificate.certificate(), root.setObject(certificateKey)); + return slime; + } + + public UnassignedCertificate fromSlime(Slime slime) { + Cursor root = slime.get(); + UnassignedCertificate.State state = UnassignedCertificate.State.valueOf(root.field(stateKey).asString()); + EndpointCertificateMetadata certificate = EndpointCertificateMetadataSerializer.fromSlime(root.field(certificateKey)); + return new UnassignedCertificate(certificate, state); + } + +} 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 1e9fe91fff9..aa3f78f1395 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 @@ -917,11 +917,11 @@ public class ApplicationApiHandler extends AuditLoggingRequestHandler { for (DataplaneTokenVersions token : dataplaneTokenVersions) { Cursor tokenObject = tokensArray.addObject(); tokenObject.setString("id", token.tokenId().value()); - Cursor fingerprintsArray = tokenObject.setArray("fingerprints"); + Cursor fingerprintsArray = tokenObject.setArray("versions"); for (DataplaneTokenVersions.Version tokenVersion : token.tokenVersions()) { Cursor fingerprintObject = fingerprintsArray.addObject(); - fingerprintObject.setString("value", tokenVersion.fingerPrint().value()); - fingerprintObject.setString("created-at", tokenVersion.creationTime().toString()); + fingerprintObject.setString("fingerprint", tokenVersion.fingerPrint().value()); + fingerprintObject.setString("created", tokenVersion.creationTime().toString()); fingerprintObject.setString("author", tokenVersion.author()); } } @@ -1889,6 +1889,7 @@ public class ApplicationApiHandler extends AuditLoggingRequestHandler { object.setString("scope", endpointScopeString(endpoint.scope())); object.setString("routingMethod", routingMethodString(endpoint.routingMethod())); object.setBool("legacy", endpoint.legacy()); + object.setString("authMethod", endpoint.isTokenEndpoint() ? "token" : "mtls"); } private void toSlime(Cursor response, DeploymentId deploymentId, Deployment deployment, HttpRequest request) { diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/certificate/EndpointCertificatesHandler.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/certificate/EndpointCertificatesHandler.java index 9bee5623774..3980ef87613 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/certificate/EndpointCertificatesHandler.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/certificate/EndpointCertificatesHandler.java @@ -6,11 +6,18 @@ import com.yahoo.container.jdisc.HttpResponse; import com.yahoo.container.jdisc.ThreadedHttpRequestHandler; import com.yahoo.restapi.RestApiException; import com.yahoo.restapi.StringResponse; +import com.yahoo.vespa.flags.BooleanFlag; +import com.yahoo.vespa.flags.FetchVector; +import com.yahoo.vespa.flags.Flags; +import com.yahoo.vespa.flags.PermanentFlags; +import com.yahoo.vespa.flags.StringFlag; +import com.yahoo.vespa.hosted.controller.Controller; import com.yahoo.vespa.hosted.controller.api.integration.ServiceRegistry; 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.certificates.EndpointCertificateRequestMetadata; import com.yahoo.vespa.hosted.controller.application.TenantAndApplicationId; +import com.yahoo.vespa.hosted.controller.certificate.AssignedCertificate; import com.yahoo.vespa.hosted.controller.persistence.CuratorDb; import com.yahoo.vespa.hosted.controller.persistence.EndpointCertificateMetadataSerializer; @@ -33,11 +40,17 @@ public class EndpointCertificatesHandler extends ThreadedHttpRequestHandler { private final EndpointCertificateProvider endpointCertificateProvider; private final CuratorDb curator; + private final BooleanFlag useAlternateCertProvider; + private final StringFlag endpointCertificateAlgo; + private final BooleanFlag useRandomizedCert; - public EndpointCertificatesHandler(Executor executor, ServiceRegistry serviceRegistry, CuratorDb curator) { + public EndpointCertificatesHandler(Executor executor, ServiceRegistry serviceRegistry, CuratorDb curator, Controller controller) { super(executor); this.endpointCertificateProvider = serviceRegistry.endpointCertificateProvider(); this.curator = curator; + this.useAlternateCertProvider = PermanentFlags.USE_ALTERNATIVE_ENDPOINT_CERTIFICATE_PROVIDER.bindTo(controller.flagSource()); + this.endpointCertificateAlgo = PermanentFlags.ENDPOINT_CERTIFICATE_ALGORITHM.bindTo(controller.flagSource()); + this.useRandomizedCert = Flags.RANDOMIZED_ENDPOINT_NAMES.bindTo(controller.flagSource()); } public HttpResponse handle(HttpRequest request) { @@ -61,15 +74,25 @@ public class EndpointCertificatesHandler extends ThreadedHttpRequestHandler { public StringResponse reRequestEndpointCertificateFor(String instanceId, boolean ignoreExistingMetadata) { ApplicationId applicationId = ApplicationId.fromFullString(instanceId); - + if (useRandomizedCert.with(FetchVector.Dimension.APPLICATION_ID, instanceId).value()) { + throw new IllegalArgumentException("Cannot re-request certificate. " + instanceId + " is assigned certificate from a pool"); + } try (var lock = curator.lock(TenantAndApplicationId.from(applicationId))) { - EndpointCertificateMetadata endpointCertificateMetadata = curator.readEndpointCertificateMetadata(applicationId) - .orElseThrow(() -> new RestApiException.NotFound("No certificate found for application " + applicationId.serializedForm())); + AssignedCertificate assignedCertificate = curator.readAssignedCertificate(applicationId) + .orElseThrow(() -> new RestApiException.NotFound("No certificate found for application " + applicationId.serializedForm())); + + String algo = this.endpointCertificateAlgo.with(FetchVector.Dimension.APPLICATION_ID, applicationId.serializedForm()).value(); + boolean useAlternativeProvider = useAlternateCertProvider.with(FetchVector.Dimension.APPLICATION_ID, applicationId.serializedForm()).value(); + String keyPrefix = applicationId.toFullString(); EndpointCertificateMetadata reRequestedMetadata = endpointCertificateProvider.requestCaSignedCertificate( - applicationId, endpointCertificateMetadata.requestedDnsSans(), ignoreExistingMetadata ? Optional.empty() : Optional.of(endpointCertificateMetadata)); + keyPrefix, assignedCertificate.certificate().requestedDnsSans(), + ignoreExistingMetadata ? + Optional.empty() : + Optional.of(assignedCertificate.certificate()), + algo, useAlternativeProvider); - curator.writeEndpointCertificateMetadata(applicationId, reRequestedMetadata); + curator.writeAssignedCertificate(assignedCertificate.with(reRequestedMetadata)); return new StringResponse(EndpointCertificateMetadataSerializer.toSlime(reRequestedMetadata).toString()); } 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 475da3224cd..43c532548d6 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 @@ -38,6 +38,7 @@ import com.yahoo.vespa.hosted.controller.application.DeploymentMetrics; import com.yahoo.vespa.hosted.controller.application.Endpoint; import com.yahoo.vespa.hosted.controller.application.TenantAndApplicationId; import com.yahoo.vespa.hosted.controller.application.pkg.ApplicationPackage; +import com.yahoo.vespa.hosted.controller.certificate.AssignedCertificate; import com.yahoo.vespa.hosted.controller.deployment.ApplicationPackageBuilder; import com.yahoo.vespa.hosted.controller.deployment.DeploymentContext; import com.yahoo.vespa.hosted.controller.deployment.DeploymentTester; @@ -940,7 +941,7 @@ public class ControllerTest { @Test void testDeploySelectivelyProvisionsCertificate() { - Function<Instance, Optional<EndpointCertificateMetadata>> certificate = (application) -> tester.controller().curator().readEndpointCertificateMetadata(application.id()); + Function<Instance, Optional<EndpointCertificateMetadata>> certificate = (application) -> tester.controller().curator().readAssignedCertificate(application.id()).map(AssignedCertificate::certificate); // Create app1 var context1 = tester.newDeploymentContext("tenant1", "app1", "default"); @@ -964,7 +965,7 @@ public class ControllerTest { (zone.environment() == Environment.prod ? "" : "." + zone.environment().value()) + ".vespa.oath.cloud"))) .collect(Collectors.toUnmodifiableSet()), - Set.copyOf(tester.controllerTester().serviceRegistry().endpointCertificateMock().dnsNamesOf(context1.instanceId()))); + Set.copyOf(tester.controllerTester().serviceRegistry().endpointCertificateMock().dnsNamesOf(cert.get().rootRequestId()))); // Next deployment reuses certificate context1.submit(applicationPackage).deploy(); diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/certificate/EndpointCertificatesTest.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/certificate/EndpointCertificatesTest.java index 5f39507d748..8c94bac35b1 100644 --- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/certificate/EndpointCertificatesTest.java +++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/certificate/EndpointCertificatesTest.java @@ -17,11 +17,13 @@ import com.yahoo.security.SignatureAlgorithm; import com.yahoo.security.X509CertificateBuilder; import com.yahoo.security.X509CertificateUtils; import com.yahoo.test.ManualClock; +import com.yahoo.vespa.flags.Flags; import com.yahoo.vespa.hosted.controller.ControllerTester; 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.EndpointCertificateMock; import com.yahoo.vespa.hosted.controller.api.integration.certificates.EndpointCertificateValidatorImpl; +import com.yahoo.vespa.hosted.controller.application.TenantAndApplicationId; import com.yahoo.vespa.hosted.controller.application.pkg.ApplicationPackage; import com.yahoo.vespa.hosted.controller.deployment.ApplicationPackageBuilder; import com.yahoo.vespa.hosted.controller.integration.SecretStoreMock; @@ -45,6 +47,7 @@ import java.util.stream.Stream; import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertNotEquals; import static org.junit.jupiter.api.Assertions.assertTrue; +import static org.junit.jupiter.api.Assertions.fail; /** * @author andreer @@ -55,7 +58,7 @@ public class EndpointCertificatesTest { private final SecretStoreMock secretStore = new SecretStoreMock(); private final CuratorDb mockCuratorDb = tester.curator(); private final ManualClock clock = tester.clock(); - private final EndpointCertificateMock endpointCertificateMock = new EndpointCertificateMock(new ManualClock()); + private final EndpointCertificateMock endpointCertificateMock = new EndpointCertificateMock(); private final EndpointCertificateValidatorImpl endpointCertificateValidator = new EndpointCertificateValidatorImpl(secretStore, clock); private final EndpointCertificates endpointCertificates = new EndpointCertificates(tester.controller(), endpointCertificateMock, endpointCertificateValidator); private final KeyPair testKeyPair = KeyUtils.generateKeypair(KeyAlgorithm.EC, 192); @@ -103,15 +106,15 @@ public class EndpointCertificatesTest { return x509CertificateBuilder.build(); } - private final Instance testInstance = new Instance(ApplicationId.defaultId()); + private final Instance instance = new Instance(ApplicationId.defaultId()); private final String testKeyName = "testKeyName"; private final String testCertName = "testCertName"; - private ZoneId testZone; + private ZoneId prodZone; @BeforeEach public void setUp() { tester.zoneRegistry().exclusiveRoutingIn(tester.zoneRegistry().zones().all().zones()); - testZone = tester.zoneRegistry().zones().all().routingMethod(RoutingMethod.exclusive).in(Environment.prod).zones().stream().findFirst().orElseThrow().getId(); + prodZone = tester.zoneRegistry().zones().all().routingMethod(RoutingMethod.exclusive).in(Environment.prod).zones().stream().findFirst().orElseThrow().getId(); clock.setInstant(Instant.EPOCH); testCertificate = makeTestCert(expectedSans); testCertificate2 = makeTestCert(expectedCombinedSans); @@ -120,7 +123,7 @@ public class EndpointCertificatesTest { @Test void provisions_new_certificate_in_dev() { ZoneId testZone = tester.zoneRegistry().zones().all().routingMethod(RoutingMethod.exclusive).in(Environment.dev).zones().stream().findFirst().orElseThrow().getId(); - Optional<EndpointCertificateMetadata> endpointCertificateMetadata = endpointCertificates.getMetadata(testInstance, testZone, DeploymentSpec.empty); + Optional<EndpointCertificateMetadata> endpointCertificateMetadata = endpointCertificates.getMetadata(instance, testZone, DeploymentSpec.empty); assertTrue(endpointCertificateMetadata.isPresent()); assertTrue(endpointCertificateMetadata.get().keyName().matches("vespa.tls.default.default.*-key")); assertTrue(endpointCertificateMetadata.get().certName().matches("vespa.tls.default.default.*-cert")); @@ -130,7 +133,7 @@ public class EndpointCertificatesTest { @Test void provisions_new_certificate_in_prod() { - Optional<EndpointCertificateMetadata> endpointCertificateMetadata = endpointCertificates.getMetadata(testInstance, testZone, DeploymentSpec.empty); + Optional<EndpointCertificateMetadata> endpointCertificateMetadata = endpointCertificates.getMetadata(instance, prodZone, DeploymentSpec.empty); assertTrue(endpointCertificateMetadata.isPresent()); assertTrue(endpointCertificateMetadata.get().keyName().matches("vespa.tls.default.default.*-key")); assertTrue(endpointCertificateMetadata.get().certName().matches("vespa.tls.default.default.*-cert")); @@ -160,7 +163,7 @@ public class EndpointCertificatesTest { "default.default.us-east-3.staging.z.vespa-app.cloud", "*.default.default.us-east-3.staging.z.vespa-app.cloud" ); - Optional<EndpointCertificateMetadata> endpointCertificateMetadata = endpointCertificates.getMetadata(testInstance, testZone, DeploymentSpec.empty); + Optional<EndpointCertificateMetadata> endpointCertificateMetadata = endpointCertificates.getMetadata(instance, prodZone, DeploymentSpec.empty); assertTrue(endpointCertificateMetadata.isPresent()); assertTrue(endpointCertificateMetadata.get().keyName().matches("vespa.tls.default.default.*-key")); assertTrue(endpointCertificateMetadata.get().certName().matches("vespa.tls.default.default.*-cert")); @@ -170,16 +173,16 @@ public class EndpointCertificatesTest { @Test void reuses_stored_certificate_metadata() { - mockCuratorDb.writeEndpointCertificateMetadata(testInstance.id(), new EndpointCertificateMetadata(testKeyName, testCertName, 7, 0, "request_id", Optional.of("leaf-request-uuid"), - List.of("vt2ktgkqme5zlnp4tj4ttyor7fj3v7q5o.vespa.oath.cloud", + mockCuratorDb.writeAssignedCertificate(assignedCertificate(instance.id(), new EndpointCertificateMetadata(testKeyName, testCertName, 7, 0, "request_id", Optional.of("leaf-request-uuid"), + 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"), - "", Optional.empty(), Optional.empty())); + "", Optional.empty(), Optional.empty(), Optional.empty()))); secretStore.setSecret(testKeyName, KeyUtils.toPem(testKeyPair.getPrivate()), 7); secretStore.setSecret(testCertName, X509CertificateUtils.toPem(testCertificate) + X509CertificateUtils.toPem(testCertificate), 7); - Optional<EndpointCertificateMetadata> endpointCertificateMetadata = endpointCertificates.getMetadata(testInstance, testZone, DeploymentSpec.empty); + Optional<EndpointCertificateMetadata> endpointCertificateMetadata = endpointCertificates.getMetadata(instance, prodZone, DeploymentSpec.empty); assertTrue(endpointCertificateMetadata.isPresent()); assertEquals(testKeyName, endpointCertificateMetadata.get().keyName()); assertEquals(testCertName, endpointCertificateMetadata.get().certName()); @@ -188,30 +191,30 @@ public class EndpointCertificatesTest { @Test void reprovisions_certificate_when_necessary() { - mockCuratorDb.writeEndpointCertificateMetadata(testInstance.id(), new EndpointCertificateMetadata(testKeyName, testCertName, -1, 0, "root-request-uuid", Optional.of("leaf-request-uuid"), List.of(), "issuer", Optional.empty(), Optional.empty())); + mockCuratorDb.writeAssignedCertificate(assignedCertificate(instance.id(), new EndpointCertificateMetadata(testKeyName, testCertName, -1, 0, "root-request-uuid", Optional.of("leaf-request-uuid"), List.of(), "issuer", Optional.empty(), Optional.empty(), Optional.empty()))); secretStore.setSecret("vespa.tls.default.default.default-key", KeyUtils.toPem(testKeyPair.getPrivate()), 0); secretStore.setSecret("vespa.tls.default.default.default-cert", X509CertificateUtils.toPem(testCertificate) + X509CertificateUtils.toPem(testCertificate), 0); - Optional<EndpointCertificateMetadata> endpointCertificateMetadata = endpointCertificates.getMetadata(testInstance, testZone, DeploymentSpec.empty); + Optional<EndpointCertificateMetadata> endpointCertificateMetadata = endpointCertificates.getMetadata(instance, prodZone, DeploymentSpec.empty); assertTrue(endpointCertificateMetadata.isPresent()); assertEquals(0, endpointCertificateMetadata.get().version()); - assertEquals(endpointCertificateMetadata, mockCuratorDb.readEndpointCertificateMetadata(testInstance.id())); + assertEquals(endpointCertificateMetadata, mockCuratorDb.readAssignedCertificate(instance.id()).map(AssignedCertificate::certificate)); } @Test void reprovisions_certificate_with_added_sans_when_deploying_to_new_zone() { ZoneId testZone = ZoneId.from("prod.ap-northeast-1"); - mockCuratorDb.writeEndpointCertificateMetadata(testInstance.id(), new EndpointCertificateMetadata(testKeyName, testCertName, -1, 0, "original-request-uuid", Optional.of("leaf-request-uuid"), expectedSans, "mockCa", Optional.empty(), Optional.empty())); + mockCuratorDb.writeAssignedCertificate(assignedCertificate(instance.id(), new EndpointCertificateMetadata(testKeyName, testCertName, -1, 0, "original-request-uuid", Optional.of("leaf-request-uuid"), expectedSans, "mockCa", Optional.empty(), Optional.empty(), Optional.empty()))); secretStore.setSecret("vespa.tls.default.default.default-key", KeyUtils.toPem(testKeyPair.getPrivate()), -1); secretStore.setSecret("vespa.tls.default.default.default-cert", X509CertificateUtils.toPem(testCertificate) + X509CertificateUtils.toPem(testCertificate), -1); secretStore.setSecret("vespa.tls.default.default.default-key", KeyUtils.toPem(testKeyPair.getPrivate()), 0); secretStore.setSecret("vespa.tls.default.default.default-cert", X509CertificateUtils.toPem(testCertificate2) + X509CertificateUtils.toPem(testCertificate2), 0); - Optional<EndpointCertificateMetadata> endpointCertificateMetadata = endpointCertificates.getMetadata(testInstance, testZone, DeploymentSpec.empty); + Optional<EndpointCertificateMetadata> endpointCertificateMetadata = endpointCertificates.getMetadata(instance, testZone, DeploymentSpec.empty); assertTrue(endpointCertificateMetadata.isPresent()); assertEquals(0, endpointCertificateMetadata.get().version()); - assertEquals(endpointCertificateMetadata, mockCuratorDb.readEndpointCertificateMetadata(testInstance.id())); + assertEquals(endpointCertificateMetadata, mockCuratorDb.readAssignedCertificate(instance.id()).map(AssignedCertificate::certificate)); assertEquals("original-request-uuid", endpointCertificateMetadata.get().rootRequestId()); assertNotEquals(Optional.of("leaf-request-uuid"), endpointCertificateMetadata.get().leafRequestId()); assertEquals(Set.copyOf(expectedCombinedSans), Set.copyOf(endpointCertificateMetadata.get().requestedDnsSans())); @@ -233,7 +236,7 @@ public class EndpointCertificatesTest { ); ZoneId testZone = tester.zoneRegistry().zones().all().in(Environment.staging).zones().stream().findFirst().orElseThrow().getId(); - Optional<EndpointCertificateMetadata> endpointCertificateMetadata = endpointCertificates.getMetadata(testInstance, testZone, deploymentSpec); + Optional<EndpointCertificateMetadata> endpointCertificateMetadata = endpointCertificates.getMetadata(instance, testZone, deploymentSpec); assertTrue(endpointCertificateMetadata.isPresent()); assertTrue(endpointCertificateMetadata.get().keyName().matches("vespa.tls.default.default.*-key")); assertTrue(endpointCertificateMetadata.get().certName().matches("vespa.tls.default.default.*-cert")); @@ -285,4 +288,48 @@ public class EndpointCertificatesTest { assertEquals(expectedSans, endpointCertificateMetadata.get().requestedDnsSans().stream().sorted().toList()); } + @Test + public void assign_certificate_from_pool() { + tester.flagSource().withBooleanFlag(Flags.RANDOMIZED_ENDPOINT_NAMES.id(), true); + try { + addCertificateToPool("pool-cert-1", UnassignedCertificate.State.requested); + endpointCertificates.getMetadata(instance, prodZone, DeploymentSpec.empty); + fail("Expected exception as certificate is not ready"); + } catch (IllegalArgumentException ignored) {} + + { // prod + String certId = "pool-cert-1"; + addCertificateToPool(certId, UnassignedCertificate.State.ready); + Optional<EndpointCertificateMetadata> endpointCertificateMetadata = endpointCertificates.getMetadata(instance, prodZone, DeploymentSpec.empty); + assertEquals(certId, endpointCertificateMetadata.get().randomizedId().get()); + assertEquals(certId, tester.curator().readAssignedCertificate(TenantAndApplicationId.from(instance.id()), Optional.empty()).get().certificate().randomizedId().get(), "Certificate is assigned at application-level"); + assertTrue(tester.controller().curator().readUnassignedCertificate(certId).isEmpty(), "Certificate is removed from pool"); + } + + { // dev + String certId = "pool-cert-2"; + addCertificateToPool(certId, UnassignedCertificate.State.ready); + ZoneId devZone = tester.zoneRegistry().zones().all().routingMethod(RoutingMethod.exclusive).in(Environment.dev).zones().stream().findFirst().orElseThrow().getId(); + Optional<EndpointCertificateMetadata> endpointCertificateMetadata = endpointCertificates.getMetadata(instance, devZone, DeploymentSpec.empty); + assertEquals(certId, endpointCertificateMetadata.get().randomizedId().get()); + assertEquals(certId, tester.curator().readAssignedCertificate(instance.id()).get().certificate().randomizedId().get(), "Certificate is assigned at instance-level"); + assertTrue(tester.controller().curator().readUnassignedCertificate(certId).isEmpty(), "Certificate is removed from pool"); + } + } + + private void addCertificateToPool(String id, UnassignedCertificate.State state) { + EndpointCertificateMetadata cert = new EndpointCertificateMetadata(testKeyName, testCertName, 1, 0, + "request-id", + Optional.of("leaf-request-uuid"), + List.of("name1", "name2"), + "", Optional.empty(), + Optional.empty(), Optional.of(id)); + UnassignedCertificate pooledCert = new UnassignedCertificate(cert, state); + tester.controller().curator().writeUnassignedCertificate(pooledCert); + } + + private static AssignedCertificate assignedCertificate(ApplicationId instance, EndpointCertificateMetadata certificate) { + return new AssignedCertificate(TenantAndApplicationId.from(instance), Optional.of(instance.instance()), certificate); + } + } 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 d80c55fd00a..02b4d6de5ac 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 @@ -69,7 +69,7 @@ public class ServiceRegistryMock extends AbstractComponent implements ServiceReg private final MemoryNameService memoryNameService = new MemoryNameService(); private final MockVpcEndpointService vpcEndpointService = new MockVpcEndpointService(clock, memoryNameService); private final MockMailer mockMailer = new MockMailer(); - private final EndpointCertificateMock endpointCertificateMock = new EndpointCertificateMock(clock); + private final EndpointCertificateMock endpointCertificateMock = new EndpointCertificateMock(); private final EndpointCertificateValidatorMock endpointCertificateValidatorMock = new EndpointCertificateValidatorMock(); private final MockContactRetriever mockContactRetriever = new MockContactRetriever(); private final MockIssueHandler mockIssueHandler = new MockIssueHandler(); diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/CertificatePoolMaintainerTest.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/CertificatePoolMaintainerTest.java new file mode 100644 index 00000000000..f94120241e7 --- /dev/null +++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/CertificatePoolMaintainerTest.java @@ -0,0 +1,57 @@ +// 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.jdisc.test.MockMetric; +import com.yahoo.vespa.flags.Flags; +import com.yahoo.vespa.hosted.controller.ControllerTester; +import com.yahoo.vespa.hosted.controller.api.integration.certificates.EndpointCertificateMock; +import com.yahoo.vespa.hosted.controller.api.integration.certificates.EndpointCertificateRequestMetadata.DnsNameStatus; +import org.junit.jupiter.api.Test; + +import java.time.Duration; +import java.util.List; +import java.util.Random; + +import static org.junit.jupiter.api.Assertions.assertEquals; + +/** + * @author andreer + */ +public class CertificatePoolMaintainerTest { + + private final ControllerTester tester = new ControllerTester(); + private final CertificatePoolMaintainer maintainer = new CertificatePoolMaintainer(tester.controller(), new MockMetric(), Duration.ofHours(1), new Random(4)); + + @Test + void new_certs_are_requested_until_limit() { + tester.flagSource().withIntFlag(Flags.CERT_POOL_SIZE.id(), 3); + assertNumCerts(1); + assertNumCerts(2); + assertNumCerts(3); + assertNumCerts(3); + } + + @Test + void cert_contains_expected_names() { + tester.flagSource().withIntFlag(Flags.CERT_POOL_SIZE.id(), 1); + assertNumCerts(1); + EndpointCertificateMock endpointCertificateProvider = (EndpointCertificateMock) tester.controller().serviceRegistry().endpointCertificateProvider(); + + var metadata = endpointCertificateProvider.listCertificates().get(0); + + assertEquals( + List.of( + new DnsNameStatus("*.c8868d4e.z.vespa.oath.cloud", "done"), + new DnsNameStatus("*.c8868d4e.g.vespa.oath.cloud", "done"), + new DnsNameStatus("*.c8868d4e.a.vespa.oath.cloud", "done") + ), metadata.dnsNames()); + + assertEquals("vespa.tls.preprovisioned.c8868d4e-cert", endpointCertificateProvider.certificateDetails(metadata.requestId()).cert_key_keyname()); + assertEquals("vespa.tls.preprovisioned.c8868d4e-key", endpointCertificateProvider.certificateDetails(metadata.requestId()).private_key_keyname()); + } + + private void assertNumCerts(int n) { + assertEquals(0.0, maintainer.maintain(), 0.0000001); + assertEquals(n, tester.curator().readUnassignedCertificates().size()); + } +} 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 49cf8c634ba..24eb9f33d33 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 @@ -5,6 +5,8 @@ import com.yahoo.component.Version; import com.yahoo.config.provision.ApplicationId; import com.yahoo.config.provision.CloudAccount; import com.yahoo.config.provision.zone.ZoneId; +import com.yahoo.jdisc.test.MockMetric; +import com.yahoo.vespa.flags.Flags; import com.yahoo.vespa.hosted.controller.ControllerTester; import com.yahoo.vespa.hosted.controller.api.integration.certificates.EndpointCertificateMetadata; import com.yahoo.vespa.hosted.controller.api.integration.certificates.EndpointCertificateMock; @@ -14,6 +16,8 @@ import com.yahoo.vespa.hosted.controller.application.Deployment; import com.yahoo.vespa.hosted.controller.application.DeploymentActivity; import com.yahoo.vespa.hosted.controller.application.DeploymentMetrics; import com.yahoo.vespa.hosted.controller.application.QuotaUsage; +import com.yahoo.vespa.hosted.controller.application.TenantAndApplicationId; +import com.yahoo.vespa.hosted.controller.certificate.AssignedCertificate; import com.yahoo.vespa.hosted.controller.deployment.ApplicationPackageBuilder; import com.yahoo.vespa.hosted.controller.deployment.DeploymentContext; import com.yahoo.vespa.hosted.controller.deployment.DeploymentTester; @@ -27,6 +31,7 @@ import java.time.temporal.ChronoUnit; import java.util.List; import java.util.Optional; import java.util.OptionalDouble; +import java.util.Random; import java.util.stream.Stream; import static com.yahoo.vespa.hosted.controller.deployment.DeploymentContext.productionUsWest1; @@ -44,27 +49,28 @@ public class EndpointCertificateMaintainerTest { private final ControllerTester tester = new ControllerTester(); private final SecretStoreMock secretStore = (SecretStoreMock) tester.controller().secretStore(); private final EndpointCertificateMaintainer maintainer = new EndpointCertificateMaintainer(tester.controller(), Duration.ofHours(1)); - private final EndpointCertificateMetadata exampleMetadata = new EndpointCertificateMetadata("keyName", "certName", 0, 0, "root-request-uuid", Optional.of("leaf-request-uuid"), List.of(), "issuer", Optional.empty(), Optional.empty()); + private final CertificatePoolMaintainer certificatePoolMaintainer = new CertificatePoolMaintainer(tester.controller(), new MockMetric(), Duration.ofHours(1), new Random(4)); + private final EndpointCertificateMetadata exampleMetadata = new EndpointCertificateMetadata("keyName", "certName", 0, 0, "root-request-uuid", Optional.of("leaf-request-uuid"), List.of(), "issuer", Optional.empty(), Optional.empty(), Optional.empty()); @Test void old_and_unused_cert_is_deleted() { - tester.curator().writeEndpointCertificateMetadata(ApplicationId.defaultId(), exampleMetadata); + tester.curator().writeAssignedCertificate(assignedCertificate(ApplicationId.defaultId(), exampleMetadata)); assertEquals(0.0, maintainer.maintain(), 0.0000001); - assertTrue(tester.curator().readEndpointCertificateMetadata(ApplicationId.defaultId()).isEmpty()); + assertTrue(tester.curator().readAssignedCertificate(ApplicationId.defaultId()).isEmpty()); } @Test void unused_but_recently_used_cert_is_not_deleted() { EndpointCertificateMetadata recentlyRequestedCert = exampleMetadata.withLastRequested(tester.clock().instant().minusSeconds(3600).getEpochSecond()); - tester.curator().writeEndpointCertificateMetadata(ApplicationId.defaultId(), recentlyRequestedCert); + tester.curator().writeAssignedCertificate(assignedCertificate(ApplicationId.defaultId(), recentlyRequestedCert)); assertEquals(0.0, maintainer.maintain(), 0.0000001); - assertEquals(Optional.of(recentlyRequestedCert), tester.curator().readEndpointCertificateMetadata(ApplicationId.defaultId())); + assertEquals(Optional.of(recentlyRequestedCert), tester.curator().readAssignedCertificate(ApplicationId.defaultId()).map(AssignedCertificate::certificate)); } @Test void refreshed_certificate_is_updated() { EndpointCertificateMetadata recentlyRequestedCert = exampleMetadata.withLastRequested(tester.clock().instant().minusSeconds(3600).getEpochSecond()); - tester.curator().writeEndpointCertificateMetadata(ApplicationId.defaultId(), recentlyRequestedCert); + tester.curator().writeAssignedCertificate(assignedCertificate(ApplicationId.defaultId(), recentlyRequestedCert)); secretStore.setSecret(exampleMetadata.keyName(), "foo", 1); secretStore.setSecret(exampleMetadata.certName(), "bar", 1); @@ -73,7 +79,7 @@ public class EndpointCertificateMaintainerTest { var updatedCert = Optional.of(recentlyRequestedCert.withLastRefreshed(tester.clock().instant().getEpochSecond()).withVersion(1)); - assertEquals(updatedCert, tester.curator().readEndpointCertificateMetadata(ApplicationId.defaultId())); + assertEquals(updatedCert, tester.curator().readAssignedCertificate(ApplicationId.defaultId()).map(AssignedCertificate::certificate)); } @Test @@ -91,7 +97,7 @@ public class EndpointCertificateMaintainerTest { deploymentContext.submit(applicationPackage).runJob(systemTest).runJob(stagingTest).runJob(productionUsWest1); assertEquals(0.0, maintainer.maintain(), 0.0000001); - var metadata = tester.curator().readEndpointCertificateMetadata(appId).orElseThrow(); + var metadata = tester.curator().readAssignedCertificate(appId).orElseThrow().certificate(); tester.controller().serviceRegistry().endpointCertificateProvider().certificateDetails(metadata.rootRequestId()); // cert should not be deleted, the app is deployed! } @@ -107,25 +113,24 @@ public class EndpointCertificateMaintainerTest { DeploymentContext deploymentContext = deploymentTester.newDeploymentContext("tenant", "application", "default"); deploymentContext.submit(applicationPackage).runJob(systemTest).runJob(stagingTest).runJob(productionUsWest1); - var originalMetadata = tester.curator().readEndpointCertificateMetadata(appId).orElseThrow(); + var assignedCertificate = tester.curator().readAssignedCertificate(appId).orElseThrow(); // cert should not be deleted, the app is deployed! assertEquals(0.0, maintainer.maintain(), 0.0000001); - assertEquals(tester.curator().readEndpointCertificateMetadata(appId), Optional.of(originalMetadata)); - tester.controller().serviceRegistry().endpointCertificateProvider().certificateDetails(originalMetadata.rootRequestId()); + assertEquals(tester.curator().readAssignedCertificate(appId), Optional.of(assignedCertificate)); + tester.controller().serviceRegistry().endpointCertificateProvider().certificateDetails(assignedCertificate.certificate().rootRequestId()); // This simulates a cert refresh performed 3 days later tester.clock().advance(Duration.ofDays(3)); - secretStore.setSecret(originalMetadata.keyName(), "foo", 1); - secretStore.setSecret(originalMetadata.certName(), "bar", 1); - tester.controller().serviceRegistry().endpointCertificateProvider().requestCaSignedCertificate(appId, originalMetadata.requestedDnsSans(), Optional.of(originalMetadata)); - + secretStore.setSecret(assignedCertificate.certificate().keyName(), "foo", 1); + secretStore.setSecret(assignedCertificate.certificate().certName(), "bar", 1); + tester.controller().serviceRegistry().endpointCertificateProvider().requestCaSignedCertificate(appId.toFullString(), assignedCertificate.certificate().requestedDnsSans(), Optional.of(assignedCertificate.certificate()), "rsa_2048", false); // We should now pick up the new key and cert version + uuid, but not force trigger deployment yet assertEquals(0.0, maintainer.maintain(), 0.0000001); deploymentContext.assertNotRunning(productionUsWest1); - var updatedMetadata = tester.curator().readEndpointCertificateMetadata(appId).orElseThrow(); - assertNotEquals(originalMetadata.leafRequestId().orElseThrow(), updatedMetadata.leafRequestId().orElseThrow()); - assertEquals(updatedMetadata.version(), originalMetadata.version() + 1); + var updatedMetadata = tester.curator().readAssignedCertificate(appId).orElseThrow().certificate(); + assertNotEquals(assignedCertificate.certificate().leafRequestId().orElseThrow(), updatedMetadata.leafRequestId().orElseThrow()); + assertEquals(updatedMetadata.version(), assignedCertificate.certificate().version() + 1); // after another 4 days, we should force trigger deployment if it hasn't already happened tester.clock().advance(Duration.ofDays(4).plusSeconds(1)); @@ -154,11 +159,27 @@ public class EndpointCertificateMaintainerTest { EndpointCertificateMock endpointCertificateProvider = (EndpointCertificateMock) tester.controller().serviceRegistry().endpointCertificateProvider(); ApplicationId unknown = ApplicationId.fromSerializedForm("applicationid:is:unknown"); - endpointCertificateProvider.requestCaSignedCertificate(unknown, List.of("a", "b", "c"), Optional.empty()); // Unknown to controller! + var metadata = endpointCertificateProvider.requestCaSignedCertificate("something", List.of("a", "b", "c"), Optional.empty(), "rsa_2048", false);// Unknown to controller! assertEquals(0.0, maintainer.maintain(), 0.0000001); - assertTrue(endpointCertificateProvider.dnsNamesOf(unknown).isEmpty()); + assertTrue(endpointCertificateProvider.dnsNamesOf(metadata.rootRequestId()).isEmpty()); assertTrue(endpointCertificateProvider.listCertificates().isEmpty()); } + + @Test + void cert_pool_is_not_deleted() { + EndpointCertificateMock endpointCertificateProvider = (EndpointCertificateMock) tester.controller().serviceRegistry().endpointCertificateProvider(); + + tester.flagSource().withIntFlag(Flags.CERT_POOL_SIZE.id(), 3); + assertEquals(0.0, certificatePoolMaintainer.maintain(), 0.0000001); + assertEquals(0.0, maintainer.maintain(), 0.0000001); + + assertNotEquals(List.of(), endpointCertificateProvider.listCertificates()); + } + + private static AssignedCertificate assignedCertificate(ApplicationId instance, EndpointCertificateMetadata certificate) { + return new AssignedCertificate(TenantAndApplicationId.from(instance), Optional.of(instance.instance()), certificate); + } + } diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/persistence/EndpointCertificateMetadataSerializerTest.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/persistence/EndpointCertificateMetadataSerializerTest.java index 819d9282618..133b1a37cdb 100644 --- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/persistence/EndpointCertificateMetadataSerializerTest.java +++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/persistence/EndpointCertificateMetadataSerializerTest.java @@ -9,13 +9,16 @@ import java.util.Optional; import static org.junit.jupiter.api.Assertions.assertEquals; +/** + * @author andreer + */ public class EndpointCertificateMetadataSerializerTest { private final EndpointCertificateMetadata sampleWithOptionalFieldsSet = - new EndpointCertificateMetadata("keyName", "certName", 1, 0, "rootRequestId", Optional.of("leafRequestId"), List.of("SAN1", "SAN2"), "issuer", java.util.Optional.of(1628000000L), Optional.of(1612000000L)); + new EndpointCertificateMetadata("keyName", "certName", 1, 0, "rootRequestId", Optional.of("leafRequestId"), List.of("SAN1", "SAN2"), "issuer", java.util.Optional.of(1628000000L), Optional.of(1612000000L), Optional.empty()); private final EndpointCertificateMetadata sampleWithoutOptionalFieldsSet = - new EndpointCertificateMetadata("keyName", "certName", 1, 0, "rootRequestId", Optional.empty(), List.of("SAN1", "SAN2"), "issuer", Optional.empty(), Optional.empty()); + new EndpointCertificateMetadata("keyName", "certName", 1, 0, "rootRequestId", Optional.empty(), List.of("SAN1", "SAN2"), "issuer", Optional.empty(), Optional.empty(), Optional.empty()); @Test void serialize_with_optional_fields() { diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/persistence/UnassignedCertificateSerializerTest.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/persistence/UnassignedCertificateSerializerTest.java new file mode 100644 index 00000000000..3dcfbc4ae44 --- /dev/null +++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/persistence/UnassignedCertificateSerializerTest.java @@ -0,0 +1,29 @@ +package com.yahoo.vespa.hosted.controller.persistence; + +import com.yahoo.vespa.hosted.controller.api.integration.certificates.EndpointCertificateMetadata; +import com.yahoo.vespa.hosted.controller.certificate.UnassignedCertificate; +import org.junit.jupiter.api.Test; + +import java.util.List; +import java.util.Optional; + +import static org.junit.jupiter.api.Assertions.assertEquals; + +/** + * @author mpolden + */ +class UnassignedCertificateSerializerTest { + + @Test + public void serialization() { + EndpointCertificateMetadata certificate = new EndpointCertificateMetadata("keyName", "certName", 1, 0, + "rootRequestId", Optional.of("leafRequestId"), + List.of("SAN1", "SAN2"), "issuer", Optional.of(3L), + Optional.of(4L), Optional.of("my-id")); + UnassignedCertificate unassignedCertificate = new UnassignedCertificate(certificate, UnassignedCertificate.State.ready); + UnassignedCertificateSerializer serializer = new UnassignedCertificateSerializer(); + assertEquals(unassignedCertificate, serializer.fromSlime(serializer.toSlime(unassignedCertificate))); + } + + +} diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/ApplicationApiCloudTest.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/ApplicationApiCloudTest.java index 841e46ad881..b6ac65467ac 100644 --- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/ApplicationApiCloudTest.java +++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/ApplicationApiCloudTest.java @@ -476,7 +476,7 @@ public class ApplicationApiCloudTest extends ControllerContainerCloudTest { (response) -> Assertions.assertThat(new String(response.getBody(), UTF_8)).matches(Pattern.compile(regexGenerateToken)), 200); - String regexListTokens = "\\{\"tokens\":\\[\\{\"id\":\"myTokenId\",\"fingerprints\":\\[\\{\"value\":\".*\",\"created-at\":\".*\",\"author\":\"user@test\"}]}]}"; + String regexListTokens = "\\{\"tokens\":\\[\\{\"id\":\"myTokenId\",\"versions\":\\[\\{\"fingerprint\":\".*\",\"created\":\".*\",\"author\":\"user@test\"}]}]}"; tester.assertResponse(request("/application/v4/tenant/scoober/token", GET) .roles(Role.developer(tenantName)), (response) -> Assertions.assertThat(new String(response.getBody(), UTF_8)).matches(Pattern.compile(regexListTokens)), @@ -498,6 +498,25 @@ public class ApplicationApiCloudTest extends ControllerContainerCloudTest { 400); } + @Test + void dataplane_token_endpoint_test() { + ControllerTester wrapped = new ControllerTester(tester); + wrapped.upgradeSystem(Version.fromString("7.1")); + new DeploymentTester(wrapped).newDeploymentContext(ApplicationId.from(tenantName, applicationName, InstanceName.defaultName())) + .submit() + .deploy(); + + tester.assertResponse(request("/application/v4/tenant/scoober/application/albums/environment/prod/region/aws-us-east-1c/instance/default", GET) + .roles(Role.reader(tenantName)), + new File("deployment-cloud.json")); + + tester.assertResponse(request("/application/v4/tenant/scoober/archive-access/aws", DELETE).roles(Role.administrator(tenantName)), + "{\"message\":\"AWS archive access role removed for tenant scoober.\"}", 200); + tester.assertResponse(request("/application/v4/tenant/scoober", GET).roles(Role.reader(tenantName)), + (response) -> assertFalse(response.getBodyAsString().contains("archiveAccessRole")), + 200); + } + private ApplicationPackageBuilder prodBuilder() { return new ApplicationPackageBuilder() .withoutAthenzIdentity() diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/responses/deployment-cloud.json b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/responses/deployment-cloud.json index bb4136ed0ba..b576b32dd0c 100644 --- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/responses/deployment-cloud.json +++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/responses/deployment-cloud.json @@ -12,7 +12,8 @@ "url": "https://albums.scoober.aws-us-east-1c.z.vespa-app.cloud/", "scope": "zone", "routingMethod": "exclusive", - "legacy": false + "legacy": false, + "authMethod": "mtls" } ], "clusters": "http://localhost:8080/application/v4/tenant/scoober/application/albums/instance/default/environment/prod/region/aws-us-east-1c/clusters", diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/responses/deployment-with-routing-policy.json b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/responses/deployment-with-routing-policy.json index b0a8ceeeff0..9694df32e9f 100644 --- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/responses/deployment-with-routing-policy.json +++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/responses/deployment-with-routing-policy.json @@ -11,7 +11,8 @@ "url": "https://instance1.application1.tenant1.us-west-1.vespa.oath.cloud/", "scope": "zone", "routingMethod": "exclusive", - "legacy": false + "legacy": false, + "authMethod": "mtls" } ], "clusters": "http://localhost:8080/application/v4/tenant/tenant1/application/application1/instance/instance1/environment/prod/region/us-west-1/clusters", diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/responses/deployment-without-shared-endpoints.json b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/responses/deployment-without-shared-endpoints.json index b0a8ceeeff0..9694df32e9f 100644 --- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/responses/deployment-without-shared-endpoints.json +++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/responses/deployment-without-shared-endpoints.json @@ -11,7 +11,8 @@ "url": "https://instance1.application1.tenant1.us-west-1.vespa.oath.cloud/", "scope": "zone", "routingMethod": "exclusive", - "legacy": false + "legacy": false, + "authMethod": "mtls" } ], "clusters": "http://localhost:8080/application/v4/tenant/tenant1/application/application1/instance/instance1/environment/prod/region/us-west-1/clusters", diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/responses/deployment.json b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/responses/deployment.json index cc42b3e006c..e52085072c7 100644 --- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/responses/deployment.json +++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/responses/deployment.json @@ -11,7 +11,8 @@ "url": "https://instance1.application1.tenant1.us-central-1.vespa.oath.cloud/", "scope": "zone", "routingMethod": "sharedLayer4", - "legacy": false + "legacy": false, + "authMethod": "mtls" }, { "cluster": "foo", @@ -19,7 +20,8 @@ "url": "https://instance1.application1.tenant1.global.vespa.oath.cloud/", "scope": "global", "routingMethod": "sharedLayer4", - "legacy": false + "legacy": false, + "authMethod": "mtls" }, { "cluster": "foo", @@ -27,7 +29,8 @@ "url": "https://a0.application1.tenant1.a.vespa.oath.cloud/", "scope": "application", "routingMethod": "sharedLayer4", - "legacy": false + "legacy": false, + "authMethod": "mtls" } ], "clusters": "http://localhost:8080/application/v4/tenant/tenant1/application/application1/instance/instance1/environment/prod/region/us-central-1/clusters", diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/responses/instance1-recursive.json b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/responses/instance1-recursive.json index f37112ea887..32b091a92ca 100644 --- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/responses/instance1-recursive.json +++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/responses/instance1-recursive.json @@ -54,7 +54,8 @@ "url": "https://instance1.application1.tenant1.us-east-1.dev.vespa.oath.cloud/", "scope": "zone", "routingMethod": "sharedLayer4", - "legacy": false + "legacy": false, + "authMethod": "mtls" } ], "clusters": "http://localhost:8080/application/v4/tenant/tenant1/application/application1/instance/instance1/environment/dev/region/us-east-1/clusters", @@ -98,7 +99,8 @@ "url": "https://instance1.application1.tenant1.us-central-1.vespa.oath.cloud/", "scope": "zone", "routingMethod": "sharedLayer4", - "legacy": false + "legacy": false, + "authMethod": "mtls" }, { "cluster": "foo", @@ -106,7 +108,8 @@ "url": "https://instance1.application1.tenant1.global.vespa.oath.cloud/", "scope": "global", "routingMethod": "sharedLayer4", - "legacy": false + "legacy": false, + "authMethod": "mtls" }, { "cluster": "foo", @@ -114,7 +117,8 @@ "url": "https://a0.application1.tenant1.a.vespa.oath.cloud/", "scope": "application", "routingMethod": "sharedLayer4", - "legacy": false + "legacy": false, + "authMethod": "mtls" } ], "clusters": "http://localhost:8080/application/v4/tenant/tenant1/application/application1/instance/instance1/environment/prod/region/us-central-1/clusters", diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/responses/recursive-root.json b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/responses/recursive-root.json index 4458040858b..6dc58cc2800 100644 --- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/responses/recursive-root.json +++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/responses/recursive-root.json @@ -61,7 +61,8 @@ "url": "https://instance1.application1.tenant1.us-east-1.dev.vespa.oath.cloud/", "scope": "zone", "routingMethod": "sharedLayer4", - "legacy": false + "legacy": false, + "authMethod": "mtls" } ], "clusters": "http://localhost:8080/application/v4/tenant/tenant1/application/application1/instance/instance1/environment/dev/region/us-east-1/clusters", @@ -105,7 +106,8 @@ "url": "https://instance1.application1.tenant1.us-central-1.vespa.oath.cloud/", "scope": "zone", "routingMethod": "sharedLayer4", - "legacy": false + "legacy": false, + "authMethod": "mtls" }, { "cluster": "foo", @@ -113,7 +115,8 @@ "url": "https://instance1.application1.tenant1.global.vespa.oath.cloud/", "scope": "global", "routingMethod": "sharedLayer4", - "legacy": false + "legacy": false, + "authMethod": "mtls" }, { "cluster": "foo", @@ -121,7 +124,8 @@ "url": "https://a0.application1.tenant1.a.vespa.oath.cloud/", "scope": "application", "routingMethod": "sharedLayer4", - "legacy": false + "legacy": false, + "authMethod": "mtls" } ], "clusters": "http://localhost:8080/application/v4/tenant/tenant1/application/application1/instance/instance1/environment/prod/region/us-central-1/clusters", diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/responses/tenant1-recursive.json b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/responses/tenant1-recursive.json index ea025b60d1b..210a637ece8 100644 --- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/responses/tenant1-recursive.json +++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/responses/tenant1-recursive.json @@ -60,7 +60,8 @@ "url": "https://instance1.application1.tenant1.us-east-1.dev.vespa.oath.cloud/", "scope": "zone", "routingMethod": "sharedLayer4", - "legacy": false + "legacy": false, + "authMethod": "mtls" } ], "clusters": "http://localhost:8080/application/v4/tenant/tenant1/application/application1/instance/instance1/environment/dev/region/us-east-1/clusters", @@ -104,7 +105,8 @@ "url": "https://instance1.application1.tenant1.us-central-1.vespa.oath.cloud/", "scope": "zone", "routingMethod": "sharedLayer4", - "legacy": false + "legacy": false, + "authMethod": "mtls" }, { "cluster": "foo", @@ -112,7 +114,8 @@ "url": "https://instance1.application1.tenant1.global.vespa.oath.cloud/", "scope": "global", "routingMethod": "sharedLayer4", - "legacy": false + "legacy": false, + "authMethod": "mtls" }, { "cluster": "foo", @@ -120,7 +123,8 @@ "url": "https://a0.application1.tenant1.a.vespa.oath.cloud/", "scope": "application", "routingMethod": "sharedLayer4", - "legacy": false + "legacy": false, + "authMethod": "mtls" } ], "clusters": "http://localhost:8080/application/v4/tenant/tenant1/application/application1/instance/instance1/environment/prod/region/us-central-1/clusters", diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/controller/responses/maintenance.json b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/controller/responses/maintenance.json index 34a3520dd1d..7a0316dfec0 100644 --- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/controller/responses/maintenance.json +++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/controller/responses/maintenance.json @@ -25,6 +25,9 @@ "name": "BillingDatabaseMaintainer" }, { + "name": "CertificatePoolMaintainer" + }, + { "name": "ChangeRequestMaintainer" }, { 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 fb3ea5c1ab5..6ba0f394d38 100644 --- a/flags/src/main/java/com/yahoo/vespa/flags/Flags.java +++ b/flags/src/main/java/com/yahoo/vespa/flags/Flags.java @@ -379,7 +379,7 @@ public class Flags { ); public static final UnboundBooleanFlag ENABLE_CROWDSTRIKE = defineFeatureFlag( - "enable-crowdstrike", true, List.of("andreer"), "2023-04-13", "2023-07-13", + "enable-crowdstrike", true, List.of("andreer"), "2023-04-13", "2023-07-25", "Whether to enable CrowdStrike.", "Takes effect on next host admin tick", HOSTNAME); @@ -390,7 +390,7 @@ public class Flags { ZONE_ID, APPLICATION_ID); public static final UnboundBooleanFlag NEW_IDDOC_LAYOUT = defineFeatureFlag( - "new_iddoc_layout", false, List.of("tokle", "bjorncs", "olaa"), "2023-04-24", "2023-06-30", + "new_iddoc_layout", true, List.of("tokle", "bjorncs", "olaa"), "2023-04-24", "2023-06-30", "Whether to use new identity document layout", "Takes effect on node reboot", HOSTNAME, APPLICATION_ID, VESPA_VERSION); @@ -401,6 +401,12 @@ public class Flags { "Takes effect on application deployment", APPLICATION_ID); + public static final UnboundIntFlag CERT_POOL_SIZE = defineIntFlag( + "cert-pool-size", 0, List.of("andreer"), "2023-06-19", "2023-07-25", + "Target number of preprovisioned endpoints certificates to maintain", + "Takes effect on next run of CertPoolMaintainer" + ); + public static final UnboundBooleanFlag ENABLE_THE_ONE_THAT_SHOULD_NOT_BE_NAMED = defineFeatureFlag( "enable-the-one-that-should-not-be-named", false, List.of("hmusum"), "2023-05-08", "2023-08-15", "Whether to enable the one program that should not be named", diff --git a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/maintenance/identity/AthenzCredentialsMaintainer.java b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/maintenance/identity/AthenzCredentialsMaintainer.java index 2463d293854..1d3fcb5fbf8 100644 --- a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/maintenance/identity/AthenzCredentialsMaintainer.java +++ b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/maintenance/identity/AthenzCredentialsMaintainer.java @@ -83,7 +83,6 @@ public class AthenzCredentialsMaintainer implements CredentialsMaintainer { private final ServiceIdentityProvider hostIdentityProvider; private final IdentityDocumentClient identityDocumentClient; private final BooleanFlag tenantServiceIdentityFlag; - private final BooleanFlag useNewIdentityDocumentLayout; // Used as an optimization to ensure ZTS is not DDoS'ed on continuously failing refresh attempts private final Map<ContainerName, Instant> lastRefreshAttempt = new ConcurrentHashMap<>(); @@ -105,7 +104,6 @@ public class AthenzCredentialsMaintainer implements CredentialsMaintainer { new AthenzIdentityVerifier(Set.of(configServerInfo.getConfigServerIdentity()))); this.timer = timer; this.tenantServiceIdentityFlag = Flags.NODE_ADMIN_TENANT_SERVICE_REGISTRY.bindTo(flagSource); - this.useNewIdentityDocumentLayout = Flags.NEW_IDDOC_LAYOUT.bindTo(flagSource); } public boolean converge(NodeAgentContext context) { @@ -449,16 +447,7 @@ public class AthenzCredentialsMaintainer implements CredentialsMaintainer { Get the document version to ask for */ private int documentVersion(NodeAgentContext context) { - var version = context.node().currentVespaVersion() - .orElse(context.node().wantedVespaVersion().orElse(Version.emptyVersion)); - var appId = context.node().owner().orElse(ApplicationId.defaultId()); - return useNewIdentityDocumentLayout - .with(FetchVector.Dimension.HOSTNAME, context.hostname().value()) - .with(FetchVector.Dimension.VESPA_VERSION, version.toFullString()) - .with(FetchVector.Dimension.APPLICATION_ID, appId.serializedForm()) - .value() - ? SignedIdentityDocument.DEFAULT_DOCUMENT_VERSION - : SignedIdentityDocument.LEGACY_DEFAULT_DOCUMENT_VERSION; + return SignedIdentityDocument.DEFAULT_DOCUMENT_VERSION; } private List<String> getRoleList(NodeAgentContext context) { diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/HostCapacityMaintainer.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/HostCapacityMaintainer.java index e9e3fd5179a..e8ba75113c6 100644 --- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/HostCapacityMaintainer.java +++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/HostCapacityMaintainer.java @@ -85,16 +85,7 @@ public class HostCapacityMaintainer extends NodeRepositoryMaintainer { } private double markForRemoval(List<Node> provisionedSnapshot) { - // Group nodes by parent; no parent means it's a host. - Map<Optional<String>, List<Node>> nodesByParent = provisionedSnapshot.stream().collect(groupingBy(Node::parentHostname)); - - // Find all hosts that we once thought were empty (first clause), or whose children are now all removable (second clause). - List<Node> emptyHosts = nodesByParent.get(Optional.<String>empty()).stream() - .filter(host -> host.hostEmptyAt().isPresent() - || nodesByParent.getOrDefault(Optional.of(host.hostname()), List.of()) - .stream().allMatch(HostCapacityMaintainer::canDeprovision)) - .toList(); - + List<Node> emptyHosts = findEmptyOrRemovableHosts(provisionedSnapshot); if (emptyHosts.isEmpty()) return 1; int attempts = 0, success = 0; @@ -108,18 +99,16 @@ public class HostCapacityMaintainer extends NodeRepositoryMaintainer { // Re-read all nodes under lock and compute the candidates for removal. The actual nodes we want // to mark for removal is the intersection with typeEmptyHosts, which excludes the preprovisioned hosts. Map<Optional<String>, List<Node>> currentNodesByParent = nodeRepository().nodes().list().stream().collect(groupingBy(Node::parentHostname)); - List<Node> candidateHosts = new ArrayList<>(currentNodesByParent.get(Optional.<String>empty())); + List<Node> candidateHosts = new ArrayList<>(getHosts(currentNodesByParent)); candidateHosts.retainAll(typeEmptyHosts); for (Node host : candidateHosts) { attempts++; // Any hosts that are no longer empty should be marked as such, and excluded from removal. - if (currentNodesByParent.getOrDefault(Optional.of(host.hostname()), List.of()) - .stream().anyMatch(n -> ! canDeprovision(n))) { - if (host.hostEmptyAt().isPresent()) { - nodeRepository().nodes().write(host.withHostEmptyAt(null), lock); - } + if (currentNodesByParent.getOrDefault(Optional.of(host.hostname()), List.of()).stream().anyMatch(n -> ! canDeprovision(n)) + && host.hostEmptyAt().isPresent()) { + nodeRepository().nodes().write(host.withHostEmptyAt(null), lock); } // If the host is still empty, we can mark it as empty now, or mark it for removal if it has already expired. else { @@ -282,11 +271,38 @@ public class HostCapacityMaintainer extends NodeRepositoryMaintainer { nodeResources, nodeRepository().clock().instant())) .toList(); - } private static NodeResources toNodeResources(ClusterCapacity clusterCapacity) { - return new NodeResources(clusterCapacity.vcpu(), clusterCapacity.memoryGb(), clusterCapacity.diskGb(), - clusterCapacity.bandwidthGbps()); + return new NodeResources(clusterCapacity.vcpu(), + clusterCapacity.memoryGb(), + clusterCapacity.diskGb(), + clusterCapacity.bandwidthGbps(), + NodeResources.DiskSpeed.valueOf(clusterCapacity.diskSpeed()), + NodeResources.StorageType.valueOf(clusterCapacity.storageType()), + NodeResources.Architecture.valueOf(clusterCapacity.architecture())); } + + private static List<Node> findEmptyOrRemovableHosts(List<Node> provisionedSnapshot) { + // Group nodes by parent; no parent means it's a host. + var nodesByParent = provisionedSnapshot.stream().collect(groupingBy(Node::parentHostname)); + + // Find all hosts that we once thought were empty (first clause), or whose children are now all removable (second clause). + return getHosts(nodesByParent).stream() + .filter(host -> host.hostEmptyAt().isPresent() || allChildrenCanBeDeprovisioned(nodesByParent, host)) + .toList(); + } + + private static List<Node> getHosts(Map<Optional<String>, List<Node>> nodesByParent) { + return nodesByParent.get(Optional.<String>empty()); + } + + private static List<Node> getChildren(Map<Optional<String>, List<Node>> nodesByParent, Node host) { + return nodesByParent.getOrDefault(Optional.of(host.hostname()), List.of()); + } + + private static boolean allChildrenCanBeDeprovisioned(Map<Optional<String>, List<Node>> nodesByParent, Node host) { + return getChildren(nodesByParent, host).stream().allMatch(HostCapacityMaintainer::canDeprovision); + } + } diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/FlavorConfigBuilder.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/FlavorConfigBuilder.java index 2bc5a0719d9..2e9cca21052 100644 --- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/FlavorConfigBuilder.java +++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/FlavorConfigBuilder.java @@ -5,12 +5,18 @@ import com.yahoo.config.provision.Flavor; import com.yahoo.config.provision.NodeFlavors; import com.yahoo.config.provisioning.FlavorsConfig; +import static com.yahoo.config.provision.Flavor.Type.BARE_METAL; +import static com.yahoo.config.provision.Flavor.Type.DOCKER_CONTAINER; import static com.yahoo.config.provision.NodeResources.Architecture; +import static com.yahoo.config.provision.NodeResources.Architecture.arm64; +import static com.yahoo.config.provision.NodeResources.Architecture.x86_64; /** * Simplifies creation of a node-repository config containing flavors. * This is needed because the config builder API is inconvenient. * + * Note: Flavors added will have fast disk and remote storage unless explicitly specified. + * * @author bratseth */ public class FlavorConfigBuilder { @@ -27,7 +33,7 @@ public class FlavorConfigBuilder { double disk, double bandwidth, Flavor.Type type) { - return addFlavor(flavorName, cpu, mem, disk, bandwidth, true, true, type, Architecture.x86_64, 0, 0); + return addFlavor(flavorName, cpu, mem, disk, bandwidth, true, true, type, x86_64, 0, 0); } public FlavorsConfig.Flavor.Builder addFlavor(String flavorName, @@ -69,31 +75,20 @@ public class FlavorConfigBuilder { /** Convenience method which creates a node flavors instance from a list of flavor names */ public static NodeFlavors createDummies(String... flavors) { - - FlavorConfigBuilder flavorConfigBuilder = new FlavorConfigBuilder(); + FlavorConfigBuilder builder = new FlavorConfigBuilder(); for (String flavorName : flavors) { - if (flavorName.equals("docker")) - flavorConfigBuilder.addFlavor(flavorName, 1., 30., 20., 1.5, Flavor.Type.DOCKER_CONTAINER); - else if (flavorName.equals("docker2")) - flavorConfigBuilder.addFlavor(flavorName, 2., 40., 40., 0.5, Flavor.Type.DOCKER_CONTAINER); - else if (flavorName.equals("host")) - flavorConfigBuilder.addFlavor(flavorName, 7., 100., 120., 5, Flavor.Type.BARE_METAL); - else if (flavorName.equals("host2")) - flavorConfigBuilder.addFlavor(flavorName, 16, 24, 100, 1, Flavor.Type.BARE_METAL); - else if (flavorName.equals("host3")) - flavorConfigBuilder.addFlavor(flavorName, 24, 64, 100, 10, Flavor.Type.BARE_METAL); - else if (flavorName.equals("host4")) - flavorConfigBuilder.addFlavor(flavorName, 48, 128, 1000, 10, Flavor.Type.BARE_METAL); - else if (flavorName.equals("devhost")) - flavorConfigBuilder.addFlavor(flavorName, 4., 80., 100, 10, Flavor.Type.BARE_METAL); - else if (flavorName.equals("arm64")) - flavorConfigBuilder.addFlavor(flavorName,2., 30., 20., 3, Flavor.Type.BARE_METAL, Architecture.arm64); - else if (flavorName.equals("gpu")) - flavorConfigBuilder.addFlavor(flavorName,4, 16, 125, 10, true, false, Flavor.Type.BARE_METAL, Architecture.x86_64, 1, 16); - else - flavorConfigBuilder.addFlavor(flavorName, 1., 30., 20., 3, Flavor.Type.BARE_METAL); + switch (flavorName) { + case "docker" -> builder.addFlavor(flavorName, 1., 30., 20., 1.5, DOCKER_CONTAINER); + case "host" -> builder.addFlavor(flavorName, 7., 100., 120., 5, BARE_METAL); + case "host2" -> builder.addFlavor(flavorName, 16, 24, 100, 1, BARE_METAL); + case "host3" -> builder.addFlavor(flavorName, 24, 64, 100, 10, BARE_METAL); + case "host4" -> builder.addFlavor(flavorName, 48, 128, 1000, 10, BARE_METAL); + case "arm64" -> builder.addFlavor(flavorName, 2., 30., 20., 3, BARE_METAL, arm64); + case "gpu" -> builder.addFlavor(flavorName, 4, 16, 125, 10, true, false, BARE_METAL, x86_64, 1, 16); + default -> builder.addFlavor(flavorName, 1., 30., 20., 3, BARE_METAL); + } } - return new NodeFlavors(flavorConfigBuilder.build()); + return new NodeFlavors(builder.build()); } } diff --git a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/maintenance/HostCapacityMaintainerTest.java b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/maintenance/HostCapacityMaintainerTest.java index 66d4b67c7c2..1a97906697f 100644 --- a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/maintenance/HostCapacityMaintainerTest.java +++ b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/maintenance/HostCapacityMaintainerTest.java @@ -90,7 +90,7 @@ public class HostCapacityMaintainerTest { @Test public void does_not_deprovision_when_preprovisioning_enabled() { var tester = new DynamicProvisioningTester().addInitialNodes(); - tester.flagSource.withListFlag(PermanentFlags.PREPROVISION_CAPACITY.id(), List.of(new ClusterCapacity(1, 1.0, 3.0, 2.0, 1.0, "fast", "local", "x86_64")), ClusterCapacity.class); + tester.flagSource.withListFlag(PermanentFlags.PREPROVISION_CAPACITY.id(), List.of(new ClusterCapacity(1, 1.0, 3.0, 2.0, 1.0, "fast", "remote", "x86_64")), ClusterCapacity.class); Optional<Node> failedHost = tester.nodeRepository.nodes().node("host2"); assertTrue(failedHost.isPresent()); @@ -103,8 +103,8 @@ public class HostCapacityMaintainerTest { public void provision_deficit_and_deprovision_excess() { var tester = new DynamicProvisioningTester().addInitialNodes(); tester.flagSource.withListFlag(PermanentFlags.PREPROVISION_CAPACITY.id(), - List.of(new ClusterCapacity(2, 48.0, 128.0, 1000.0, 10.0, "fast", "local", "x86_64"), - new ClusterCapacity(1, 16.0, 24.0, 100.0, 1.0, "fast", "local", "x86_64")), + List.of(new ClusterCapacity(2, 48.0, 128.0, 1000.0, 10.0, "fast", "remote", "x86_64"), + new ClusterCapacity(1, 16.0, 24.0, 100.0, 1.0, "fast", "remote", "x86_64")), ClusterCapacity.class); assertEquals(0, tester.hostProvisioner.provisionedHosts().size()); @@ -141,7 +141,7 @@ public class HostCapacityMaintainerTest { var tester = new DynamicProvisioningTester().addInitialNodes(); // Makes provisioned hosts 48-128-1000-10 tester.hostProvisioner.setHostFlavor("host4"); - var clusterCapacity = new ClusterCapacity(2, 1.0, 30.0, 20.0, 3.0, "fast", "local", "x86_64"); + var clusterCapacity = new ClusterCapacity(2, 1.0, 30.0, 20.0, 3.0, "fast", "remote", "x86_64"); tester.flagSource.withListFlag(PermanentFlags.PREPROVISION_CAPACITY.id(), List.of(clusterCapacity), ClusterCapacity.class); @@ -179,7 +179,7 @@ public class HostCapacityMaintainerTest { tester.flagSource.withListFlag(PermanentFlags.PREPROVISION_CAPACITY.id(), List.of(clusterCapacity, - new ClusterCapacity(2, 24.0, 64.0, 100.0, 1.0, "fast", "local", "x86_64")), + new ClusterCapacity(2, 24.0, 64.0, 100.0, 1.0, "fast", "remote", "x86_64")), ClusterCapacity.class); tester.maintain(); @@ -194,7 +194,7 @@ public class HostCapacityMaintainerTest { // If the preprovision capacity is reduced, we should see shared hosts deprovisioned. tester.flagSource.withListFlag(PermanentFlags.PREPROVISION_CAPACITY.id(), - List.of(new ClusterCapacity(1, 1.0, 30.0, 20.0, 3.0, "fast", "local", "x86_64")), + List.of(new ClusterCapacity(1, 1.0, 30.0, 20.0, 3.0, "fast", "remote", "x86_64")), ClusterCapacity.class); tester.maintain(); diff --git a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/maintenance/MetricsReporterTest.java b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/maintenance/MetricsReporterTest.java index de2c060a0eb..7d90de1ccaf 100644 --- a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/maintenance/MetricsReporterTest.java +++ b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/maintenance/MetricsReporterTest.java @@ -182,7 +182,7 @@ public class MetricsReporterTest { @Test public void container_metrics() { - NodeFlavors nodeFlavors = FlavorConfigBuilder.createDummies("host", "docker", "docker2"); + NodeFlavors nodeFlavors = FlavorConfigBuilder.createDummies("host", "docker"); ProvisioningTester tester = new ProvisioningTester.Builder().flavors(nodeFlavors.getFlavors()).build(); NodeRepository nodeRepository = tester.nodeRepository(); diff --git a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/provisioning/HostCapacityTest.java b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/provisioning/HostCapacityTest.java index ec8a8f637c8..35d6cdbf5d0 100644 --- a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/provisioning/HostCapacityTest.java +++ b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/provisioning/HostCapacityTest.java @@ -45,7 +45,7 @@ public class HostCapacityTest { doAnswer(invocation -> ((Flavor)invocation.getArguments()[0]).resources()).when(hostResourcesCalculator).advertisedResourcesOf(any()); // Create flavors - NodeFlavors nodeFlavors = FlavorConfigBuilder.createDummies("host", "docker", "docker2"); + NodeFlavors nodeFlavors = FlavorConfigBuilder.createDummies("host", "docker"); // Create hosts host1 = Node.create("host1", IP.Config.of(Set.of("::1"), createIps(2, 4), List.of()), "host1", nodeFlavors.getFlavorOrThrow("host"), NodeType.host).build(); diff --git a/orchestrator/src/main/java/com/yahoo/vespa/orchestrator/resources/InstanceStatusResponse.java b/orchestrator/src/main/java/com/yahoo/vespa/orchestrator/resources/InstanceStatusResponse.java index 5766c344e32..93395822b9d 100644 --- a/orchestrator/src/main/java/com/yahoo/vespa/orchestrator/resources/InstanceStatusResponse.java +++ b/orchestrator/src/main/java/com/yahoo/vespa/orchestrator/resources/InstanceStatusResponse.java @@ -6,10 +6,8 @@ import com.yahoo.vespa.applicationmodel.ApplicationInstance; import com.yahoo.vespa.applicationmodel.HostName; import com.yahoo.vespa.orchestrator.restapi.wire.WireHostInfo; -import java.util.Map; import java.util.Objects; import java.util.TreeMap; -import java.util.stream.Collectors; /* * @author andreer @@ -35,16 +33,6 @@ public class InstanceStatusResponse { return applicationInstance; } - @JsonProperty("hostStates") - public Map<HostName, String> hostStates() { - // TODO: Remove this once all clients have been moved to hostStatus. - return hostInfos.entrySet().stream() - .collect(Collectors.toMap( - entry -> entry.getKey(), - entry -> entry.getValue().hostStatus() - )); - } - @JsonProperty("hostInfos") public TreeMap<HostName, WireHostInfo> hostInfos() { return hostInfos; diff --git a/searchlib/src/tests/docstore/logdatastore/logdatastore_test.cpp b/searchlib/src/tests/docstore/logdatastore/logdatastore_test.cpp index 2bf2a38b7e6..0d64683b4a6 100644 --- a/searchlib/src/tests/docstore/logdatastore/logdatastore_test.cpp +++ b/searchlib/src/tests/docstore/logdatastore/logdatastore_test.cpp @@ -565,7 +565,7 @@ TEST("Control static memory usage") { IDocumentStore &ds = vcs.getStore(); vespalib::MemoryUsage usage = ds.getMemoryUsage(); constexpr size_t mutex_size = sizeof(std::mutex) * 2 * (113 + 1); // sizeof(std::mutex) is platform dependent - EXPECT_EQUAL(74572 + mutex_size, usage.allocatedBytes()); + EXPECT_EQUAL(74668 + mutex_size, usage.allocatedBytes()); EXPECT_EQUAL(944u + mutex_size, usage.usedBytes()); } @@ -575,29 +575,29 @@ TEST("test the update cache strategy") { for (size_t i(1); i <= 10; i++) { vcs.write(i); } - TEST_DO(verifyCacheStats(ds.getCacheStats(), 0, 0, 0, 0)); + TEST_DO(verifyCacheStats(ds.getCacheStats(), 0, 0, 0, 28)); vcs.verifyRead(7); - TEST_DO(verifyCacheStats(ds.getCacheStats(), 0, 1, 1, 221)); + TEST_DO(verifyCacheStats(ds.getCacheStats(), 0, 1, 1, 241)); vcs.write(8); - TEST_DO(verifyCacheStats(ds.getCacheStats(), 0, 1, 1, 221)); + TEST_DO(verifyCacheStats(ds.getCacheStats(), 0, 1, 1, 241)); vcs.write(7, 17); - TEST_DO(verifyCacheStats(ds.getCacheStats(), 0, 1, 1, 282)); + TEST_DO(verifyCacheStats(ds.getCacheStats(), 0, 1, 1, 302)); vcs.verifyRead(7); - TEST_DO(verifyCacheStats(ds.getCacheStats(), 1, 1, 1, 282)); + TEST_DO(verifyCacheStats(ds.getCacheStats(), 1, 1, 1, 302)); vcs.remove(8); - TEST_DO(verifyCacheStats(ds.getCacheStats(), 1, 1, 1, 282)); + TEST_DO(verifyCacheStats(ds.getCacheStats(), 1, 1, 1, 302)); vcs.remove(7); - TEST_DO(verifyCacheStats(ds.getCacheStats(), 1, 1, 0, 0)); + TEST_DO(verifyCacheStats(ds.getCacheStats(), 1, 1, 0, 28)); vcs.write(7); - TEST_DO(verifyCacheStats(ds.getCacheStats(), 1, 1, 0, 0)); + TEST_DO(verifyCacheStats(ds.getCacheStats(), 1, 1, 0, 28)); vcs.verifyRead(7); - TEST_DO(verifyCacheStats(ds.getCacheStats(), 1, 2, 1, 221)); + TEST_DO(verifyCacheStats(ds.getCacheStats(), 1, 2, 1, 241)); vcs.write(7, 17); - TEST_DO(verifyCacheStats(ds.getCacheStats(), 1, 2, 1, 282)); + TEST_DO(verifyCacheStats(ds.getCacheStats(), 1, 2, 1, 302)); vcs.recreate(); IDocumentStore & ds2 = vcs.getStore(); vcs.verifyRead(7); - TEST_DO(verifyCacheStats(ds2.getCacheStats(), 0, 1, 1, 282)); + TEST_DO(verifyCacheStats(ds2.getCacheStats(), 0, 1, 1, 302)); } TEST("test the invalidate cache strategy") { @@ -606,23 +606,23 @@ TEST("test the invalidate cache strategy") { for (size_t i(1); i <= 10; i++) { vcs.write(i); } - TEST_DO(verifyCacheStats(ds.getCacheStats(), 0, 0, 0, 0)); + TEST_DO(verifyCacheStats(ds.getCacheStats(), 0, 0, 0, 28)); vcs.verifyRead(7); - TEST_DO(verifyCacheStats(ds.getCacheStats(), 0, 1, 1, 221)); + TEST_DO(verifyCacheStats(ds.getCacheStats(), 0, 1, 1, 241)); vcs.write(8); - TEST_DO(verifyCacheStats(ds.getCacheStats(), 0, 1, 1, 221)); + TEST_DO(verifyCacheStats(ds.getCacheStats(), 0, 1, 1, 241)); vcs.write(7); - TEST_DO(verifyCacheStats(ds.getCacheStats(), 0, 1, 0, 0)); + TEST_DO(verifyCacheStats(ds.getCacheStats(), 0, 1, 0, 28)); vcs.verifyRead(7); - TEST_DO(verifyCacheStats(ds.getCacheStats(), 0, 2, 1, 221)); + TEST_DO(verifyCacheStats(ds.getCacheStats(), 0, 2, 1, 241)); vcs.remove(8); - TEST_DO(verifyCacheStats(ds.getCacheStats(), 0, 2, 1, 221)); + TEST_DO(verifyCacheStats(ds.getCacheStats(), 0, 2, 1, 241)); vcs.remove(7); - TEST_DO(verifyCacheStats(ds.getCacheStats(), 0, 2, 0, 0)); + TEST_DO(verifyCacheStats(ds.getCacheStats(), 0, 2, 0, 28)); vcs.write(7); - TEST_DO(verifyCacheStats(ds.getCacheStats(), 0, 2, 0, 0)); + TEST_DO(verifyCacheStats(ds.getCacheStats(), 0, 2, 0, 28)); vcs.verifyRead(7); - TEST_DO(verifyCacheStats(ds.getCacheStats(), 0, 3, 1, 221)); + TEST_DO(verifyCacheStats(ds.getCacheStats(), 0, 3, 1, 241)); } TEST("test that the integrated visit cache works.") { @@ -631,12 +631,12 @@ TEST("test that the integrated visit cache works.") { for (size_t i(1); i <= 100; i++) { vcs.write(i); } - TEST_DO(verifyCacheStats(ds.getCacheStats(), 0, 0, 0, 0)); + TEST_DO(verifyCacheStats(ds.getCacheStats(), 0, 0, 0, 28)); for (size_t i(1); i <= 100; i++) { vcs.verifyRead(i); } - constexpr size_t BASE_SZ = 20594; + constexpr size_t BASE_SZ = 20602; TEST_DO(verifyCacheStats(ds.getCacheStats(), 0, 100, 100, BASE_SZ)); for (size_t i(1); i <= 100; i++) { vcs.verifyRead(i); @@ -646,32 +646,32 @@ TEST("test that the integrated visit cache works.") { vcs.verifyVisit({7,9,17,19,67,88}, false); TEST_DO(verifyCacheStats(ds.getCacheStats(), 100, 100, 100, BASE_SZ)); vcs.verifyVisit({7,9,17,19,67,88}, true); - TEST_DO(verifyCacheStats(ds.getCacheStats(), 100, 101, 101, BASE_SZ+557)); + TEST_DO(verifyCacheStats(ds.getCacheStats(), 100, 101, 101, BASE_SZ+16)); vcs.verifyVisit({7,9,17,19,67,88}, true); - TEST_DO(verifyCacheStats(ds.getCacheStats(), 101, 101, 101, BASE_SZ+557)); + TEST_DO(verifyCacheStats(ds.getCacheStats(), 101, 101, 101, BASE_SZ+16)); vcs.rewrite(8); - TEST_DO(verifyCacheStats(ds.getCacheStats(), 101, 101, 100, BASE_SZ+328)); // From the individual cache. + TEST_DO(verifyCacheStats(ds.getCacheStats(), 101, 101, 100, BASE_SZ-197)); // From the individual cache. vcs.rewrite(7); - TEST_DO(verifyCacheStats(ds.getCacheStats(), 101, 101, 98, BASE_SZ-442)); // From the both caches. + TEST_DO(verifyCacheStats(ds.getCacheStats(), 101, 101, 98, BASE_SZ-166)); // From the both caches. vcs.verifyVisit({7,9,17,19,67,88}, true); - TEST_DO(verifyCacheStats(ds.getCacheStats(), 101, 102, 99, BASE_SZ+130)); + TEST_DO(verifyCacheStats(ds.getCacheStats(), 101, 102, 99, BASE_SZ-410)); vcs.verifyVisit({7,9,17,19,67,88,89}, true); - TEST_DO(verifyCacheStats(ds.getCacheStats(), 101, 103, 99, BASE_SZ+180)); + TEST_DO(verifyCacheStats(ds.getCacheStats(), 101, 103, 99, BASE_SZ-406)); vcs.rewrite(17); - TEST_DO(verifyCacheStats(ds.getCacheStats(), 101, 103, 97, BASE_SZ-671)); + TEST_DO(verifyCacheStats(ds.getCacheStats(), 101, 103, 97, BASE_SZ-391)); vcs.verifyVisit({7,9,17,19,67,88,89}, true); - TEST_DO(verifyCacheStats(ds.getCacheStats(), 101, 104, 98, BASE_SZ-20)); + TEST_DO(verifyCacheStats(ds.getCacheStats(), 101, 104, 98, BASE_SZ-611)); vcs.remove(17); - TEST_DO(verifyCacheStats(ds.getCacheStats(), 101, 104, 97, BASE_SZ-671)); + TEST_DO(verifyCacheStats(ds.getCacheStats(), 101, 104, 97, BASE_SZ-391)); vcs.verifyVisit({7,9,17,19,67,88,89}, {7,9,19,67,88,89}, true); - TEST_DO(verifyCacheStats(ds.getCacheStats(), 101, 105, 98, BASE_SZ-70)); + TEST_DO(verifyCacheStats(ds.getCacheStats(), 101, 105, 98, BASE_SZ-611)); vcs.verifyVisit({41, 42}, true); - TEST_DO(verifyCacheStats(ds.getCacheStats(), 101, 106, 99, BASE_SZ+230)); + TEST_DO(verifyCacheStats(ds.getCacheStats(), 101, 106, 99, BASE_SZ-611)); vcs.verifyVisit({43, 44}, true); - TEST_DO(verifyCacheStats(ds.getCacheStats(), 101, 107, 100, BASE_SZ+540)); + TEST_DO(verifyCacheStats(ds.getCacheStats(), 101, 107, 100, BASE_SZ-611)); vcs.verifyVisit({41, 42, 43, 44}, true); - TEST_DO(verifyCacheStats(ds.getCacheStats(), 101, 108, 99, BASE_SZ+360)); + TEST_DO(verifyCacheStats(ds.getCacheStats(), 101, 108, 99, BASE_SZ-611)); } TEST("testWriteRead") { diff --git a/searchlib/src/vespa/searchlib/docstore/visitcache.cpp b/searchlib/src/vespa/searchlib/docstore/visitcache.cpp index b61cf49c438..322d0eb341b 100644 --- a/searchlib/src/vespa/searchlib/docstore/visitcache.cpp +++ b/searchlib/src/vespa/searchlib/docstore/visitcache.cpp @@ -165,6 +165,7 @@ public: CompressedBlobSet readSet(const KeySet & keys); void removeKey(uint32_t key); vespalib::MemoryUsage getStaticMemoryUsage() const override; + CacheStats get_stats() const override; private: void locateAndInvalidateOtherSubsets(const UniqueLock & cacheGuard, const KeySet & keys); using IdSet = vespalib::hash_set<uint64_t>; @@ -267,7 +268,8 @@ VisitCache::remove(uint32_t key) { CacheStats VisitCache::getCacheStats() const { - return _cache->get_stats(); + CacheStats stats = _cache->get_stats(); + return stats; } VisitCache::Cache::Cache(BackingStore & b, size_t maxBytes) : @@ -306,19 +308,22 @@ VisitCache::Cache::onRemove(const K & key) { vespalib::MemoryUsage VisitCache::Cache::getStaticMemoryUsage() const { vespalib::MemoryUsage usage = Parent::getStaticMemoryUsage(); - auto cacheGuard = getGuard(); size_t baseSelf = sizeof(_lid2Id) + sizeof(_id2KeySet); usage.incAllocatedBytes(baseSelf); - usage.incAllocatedBytes(_lid2Id.capacity() * sizeof(LidUniqueKeySetId::value_type)); - usage.incAllocatedBytes(_id2KeySet.capacity() * sizeof(IdKeySetMap::value_type)); usage.incUsedBytes(baseSelf); - usage.incUsedBytes(_lid2Id.size() * sizeof(LidUniqueKeySetId::value_type)); - usage.incUsedBytes(_id2KeySet.size() * sizeof(IdKeySetMap::value_type)); + return usage; +} + +CacheStats +VisitCache::Cache::get_stats() const { + CacheStats stats = Parent::get_stats(); + auto cacheGuard = getGuard(); + stats.memory_used += _lid2Id.capacity() * sizeof(LidUniqueKeySetId::value_type); + stats.memory_used += _id2KeySet.capacity() * sizeof(IdKeySetMap::value_type); for (const auto & entry: _id2KeySet) { - usage.incAllocatedBytes(entry.second.getKeys().capacity() * sizeof(uint32_t)); - usage.incUsedBytes(entry.second.getKeys().size() * sizeof(uint32_t)); + stats.memory_used = entry.second.getKeys().capacity() * sizeof(uint32_t); } - return usage; + return stats; } } diff --git a/vespa-application-maven-plugin/src/main/java/com/yahoo/container/plugin/mojo/ApplicationMojo.java b/vespa-application-maven-plugin/src/main/java/com/yahoo/container/plugin/mojo/ApplicationMojo.java index c6d056675a8..aff505f934f 100644 --- a/vespa-application-maven-plugin/src/main/java/com/yahoo/container/plugin/mojo/ApplicationMojo.java +++ b/vespa-application-maven-plugin/src/main/java/com/yahoo/container/plugin/mojo/ApplicationMojo.java @@ -70,6 +70,7 @@ public class ApplicationMojo extends AbstractMojo { vespaversion = project.getPlugin("com.yahoo.vespa:vespa-application-maven-plugin").getVersion(); Version compileVersion = Version.from(vespaversion); + if (compileVersion.isSnapshot()) return; MavenProject current = project; while (current.getParent() != null && current.getParent().getParentArtifact() != null) diff --git a/vespalib/src/tests/datastore/array_store/array_store_test.cpp b/vespalib/src/tests/datastore/array_store/array_store_test.cpp index a259fcaa4dc..2674acf1ce9 100644 --- a/vespalib/src/tests/datastore/array_store/array_store_test.cpp +++ b/vespalib/src/tests/datastore/array_store/array_store_test.cpp @@ -280,7 +280,7 @@ TYPED_TEST(NumberStoreTest, control_static_sizes) { EXPECT_EQ(202140u, usage.allocatedBytes()); EXPECT_EQ(197680u, usage.usedBytes()); } else { - EXPECT_EQ(202388u, usage.allocatedBytes()); + EXPECT_EQ(202328u, usage.allocatedBytes()); EXPECT_EQ(197568u, usage.usedBytes()); } } @@ -564,10 +564,10 @@ TYPED_TEST(NumberStoreTest, address_space_usage_is_ratio_between_used_arrays_and * allocated elements = 256 / sizeof(int) = 64. * limit = 64 / 3 = 21. * - * For dynamic buffer 3, we have 16 * 5 * sizeof(int) => 320 -> 512 - * limit = 512 / (5 * 4) = 25 + * For dynamic buffer 3, we have 16 * 5 * sizeof(int) => 320 -> 512 - 64 + * limit = (512 -64) / (5 * 4) = 22 */ - size_t type_id_3_entries = this->simple_buffers() ? 21 : 25; + size_t type_id_3_entries = this->simple_buffers() ? 21 : 22; size_t expLimit = fourgig - 4 * TestFixture::EntryRefType::offsetSize() + 3 * 16 + type_id_3_entries; EXPECT_EQ(static_cast<double>(2)/ expLimit, this->store.addressSpaceUsage().usage()); EXPECT_EQ(expLimit, this->store.addressSpaceUsage().limit()); diff --git a/vespalib/src/tests/datastore/dynamic_array_buffer_type/dynamic_array_buffer_type_test.cpp b/vespalib/src/tests/datastore/dynamic_array_buffer_type/dynamic_array_buffer_type_test.cpp index a703d9b18eb..b0af2f56492 100644 --- a/vespalib/src/tests/datastore/dynamic_array_buffer_type/dynamic_array_buffer_type_test.cpp +++ b/vespalib/src/tests/datastore/dynamic_array_buffer_type/dynamic_array_buffer_type_test.cpp @@ -120,20 +120,23 @@ protected: BufferType _buffer_type; size_t _entry_size; + size_t _buffer_underflow_size; size_t _buf_size; - std::unique_ptr<char[]> _buf; + std::unique_ptr<char[]> _buf_alloc; + char* _buf; }; DynamicArrayBufferTypeTest::DynamicArrayBufferTypeTest() : testing::Test(), _buffer_type(3, ArrayStoreConfig::AllocSpec(0, 10, 0, 0.2), {}), _entry_size(_buffer_type.entry_size()), + _buffer_underflow_size(_buffer_type.buffer_underflow_size()), _buf_size(2 * _entry_size), - _buf(std::make_unique<char[]>(_buf_size)) + _buf_alloc(std::make_unique<char[]>(_buf_size + _buffer_underflow_size)), + _buf(_buf_alloc.get() + _buffer_underflow_size) { // Call initialize_reserved_entries to force construction of empty element - _buffer_type.initialize_reserved_entries(_buf.get(), 1); - memset(_buf.get(), 55, _buf_size); + _buffer_type.initialize_reserved_entries(_buf, 1); // Reset counts after empty element has been constructed counts = Counts(); } @@ -178,7 +181,7 @@ DynamicArrayBufferTypeTest::get_max_vector(const void* buffer, uint32_t offset) void DynamicArrayBufferTypeTest::write_entry1() { - auto e1 = BufferType::get_entry(_buf.get(), 1, _entry_size); + auto e1 = BufferType::get_entry(_buf, 1, _entry_size); BufferType::set_dynamic_array_size(e1, 2); new (static_cast<void *>(e1)) WrapInt32(42); new (static_cast<void *>(e1 + 1)) WrapInt32(47); @@ -204,46 +207,47 @@ TEST_F(DynamicArrayBufferTypeTest, entry_size_is_calculated) TEST_F(DynamicArrayBufferTypeTest, initialize_reserved_entries) { - _buffer_type.initialize_reserved_entries(_buf.get(), 2); - EXPECT_EQ((std::vector<int>{}), get_vector(_buf.get(), 0)); - EXPECT_EQ((std::vector<int>{}), get_vector(_buf.get(), 1)); - EXPECT_EQ((std::vector<int>{0, 0, 0}), get_max_vector(_buf.get(), 0)); - EXPECT_EQ((std::vector<int>{0, 0, 0}), get_max_vector(_buf.get(), 1)); + _buffer_type.initialize_reserved_entries(_buf, 2); + EXPECT_EQ((std::vector<int>{}), get_vector(_buf, 0)); + EXPECT_EQ((std::vector<int>{}), get_vector(_buf, 1)); + EXPECT_EQ((std::vector<int>{0, 0, 0}), get_max_vector(_buf, 0)); + EXPECT_EQ((std::vector<int>{0, 0, 0}), get_max_vector(_buf, 1)); EXPECT_EQ(Counts(0, 0, 6, 0, 0), counts); } TEST_F(DynamicArrayBufferTypeTest, fallback_copy) { - _buffer_type.initialize_reserved_entries(_buf.get(), 1); + _buffer_type.initialize_reserved_entries(_buf, 1); write_entry1(); EXPECT_EQ(Counts(0, 3, 3, 0, 0), counts); - auto buf2 = std::make_unique<char[]>(_buf_size); - _buffer_type.fallback_copy(buf2.get(), _buf.get(), 2); - EXPECT_EQ((std::vector<int>{}), get_vector(buf2.get(), 0)); - EXPECT_EQ((std::vector<int>{42, 47}), get_vector(buf2.get(), 1)); - EXPECT_EQ((std::vector<int>{0, 0, 0}), get_max_vector(buf2.get(), 0)); - EXPECT_EQ((std::vector<int>{42, 47, 49}), get_max_vector(buf2.get(), 1)); + auto buf2_alloc = std::make_unique<char[]>(_buf_size + _buffer_underflow_size); + char* buf2 = buf2_alloc.get() + _buffer_underflow_size; + _buffer_type.fallback_copy(buf2, _buf, 2); + EXPECT_EQ((std::vector<int>{}), get_vector(buf2, 0)); + EXPECT_EQ((std::vector<int>{42, 47}), get_vector(buf2, 1)); + EXPECT_EQ((std::vector<int>{0, 0, 0}), get_max_vector(buf2, 0)); + EXPECT_EQ((std::vector<int>{42, 47, 49}), get_max_vector(buf2, 1)); EXPECT_EQ(Counts(0, 3, 9, 0, 0), counts); } TEST_F(DynamicArrayBufferTypeTest, destroy_entries) { - _buffer_type.initialize_reserved_entries(_buf.get(), 2); + _buffer_type.initialize_reserved_entries(_buf, 2); write_entry1(); - _buffer_type.destroy_entries(_buf.get(), 2); + _buffer_type.destroy_entries(_buf, 2); EXPECT_EQ(Counts(0, 3, 6, 6, 0), counts); } TEST_F(DynamicArrayBufferTypeTest, clean_hold) { - _buffer_type.initialize_reserved_entries(_buf.get(), 1); + _buffer_type.initialize_reserved_entries(_buf, 1); write_entry1(); MyCleanContext clean_context; - _buffer_type.clean_hold(_buf.get(), 1, 1, clean_context); - EXPECT_EQ((std::vector<int>{0, 0}), get_vector(_buf.get(), 1)); - EXPECT_EQ((std::vector<int>{0, 0, 49}), get_max_vector(_buf.get(), 1)); + _buffer_type.clean_hold(_buf, 1, 1, clean_context); + EXPECT_EQ((std::vector<int>{0, 0}), get_vector(_buf, 1)); + EXPECT_EQ((std::vector<int>{0, 0, 49}), get_max_vector(_buf, 1)); EXPECT_EQ(Counts(0, 3, 3, 0, 2), counts); - _buffer_type.clean_hold(_buf.get(), 0, 2, clean_context); + _buffer_type.clean_hold(_buf, 0, 2, clean_context); EXPECT_EQ(Counts(0, 3, 3, 0, 4), counts); } diff --git a/vespalib/src/vespa/vespalib/datastore/array_store.h b/vespalib/src/vespa/vespalib/datastore/array_store.h index f5c30c90c5b..0490687aeb8 100644 --- a/vespalib/src/vespa/vespalib/datastore/array_store.h +++ b/vespalib/src/vespa/vespalib/datastore/array_store.h @@ -126,7 +126,7 @@ public: return get_dynamic_array<typename TypeMapper::DynamicBufferType>(bufferAndMeta.get_buffer_acquire(), internalRef.offset(), bufferAndMeta.get_entry_size()); } } - return getSmallArray(internalRef, bufferAndMeta.getArraySize()); + return getSmallArray(internalRef, bufferAndMeta.get_array_size()); } else { return getLargeArray(internalRef); } diff --git a/vespalib/src/vespa/vespalib/datastore/buffer_type.cpp b/vespalib/src/vespa/vespalib/datastore/buffer_type.cpp index bb34c5d0f9d..1b087a01c58 100644 --- a/vespalib/src/vespa/vespalib/datastore/buffer_type.cpp +++ b/vespalib/src/vespa/vespalib/datastore/buffer_type.cpp @@ -27,6 +27,7 @@ BufferTypeBase::CleanContext::extraBytesCleaned(size_t value) } BufferTypeBase::BufferTypeBase(uint32_t entry_size_in, + uint32_t buffer_underflow_size_in, uint32_t arraySize, uint32_t min_entries, uint32_t max_entries, @@ -34,6 +35,7 @@ BufferTypeBase::BufferTypeBase(uint32_t entry_size_in, float allocGrowFactor) noexcept : _entry_size(entry_size_in), _arraySize(arraySize), + _buffer_underflow_size(buffer_underflow_size_in), _min_entries(std::min(min_entries, max_entries)), _max_entries(max_entries), _num_entries_for_new_buffer(std::min(num_entries_for_new_buffer, max_entries)), @@ -46,10 +48,11 @@ BufferTypeBase::BufferTypeBase(uint32_t entry_size_in, } BufferTypeBase::BufferTypeBase(uint32_t entry_size_in, + uint32_t buffer_underflow_size_in, uint32_t arraySize, uint32_t min_entries, uint32_t max_entries) noexcept - : BufferTypeBase(entry_size_in, arraySize, min_entries, max_entries, 0u, DEFAULT_ALLOC_GROW_FACTOR) + : BufferTypeBase(entry_size_in, buffer_underflow_size_in, arraySize, min_entries, max_entries, 0u, DEFAULT_ALLOC_GROW_FACTOR) { } @@ -117,6 +120,12 @@ BufferTypeBase::get_memory_allocator() const return nullptr; } +bool +BufferTypeBase::is_dynamic_array_buffer_type() const noexcept +{ + return false; +} + void BufferTypeBase::clamp_max_entries(uint32_t max_entries) { diff --git a/vespalib/src/vespa/vespalib/datastore/buffer_type.h b/vespalib/src/vespa/vespalib/datastore/buffer_type.h index 3370bd47fad..7b23a238ba2 100644 --- a/vespalib/src/vespa/vespalib/datastore/buffer_type.h +++ b/vespalib/src/vespa/vespalib/datastore/buffer_type.h @@ -39,8 +39,8 @@ public: BufferTypeBase & operator=(const BufferTypeBase &rhs) = delete; BufferTypeBase(BufferTypeBase &&rhs) noexcept = default; BufferTypeBase & operator=(BufferTypeBase &&rhs) noexcept = default; - BufferTypeBase(uint32_t entry_size_in, uint32_t arraySize, uint32_t min_entries, uint32_t max_entries) noexcept; - BufferTypeBase(uint32_t entry_size_in, uint32_t arraySize, uint32_t min_entries, uint32_t max_entries, + BufferTypeBase(uint32_t entry_size_in, uint32_t buffer_underflow_size_in, uint32_t arraySize, uint32_t min_entries, uint32_t max_entries) noexcept; + BufferTypeBase(uint32_t entry_size_in, uint32_t buffer_underflow_size_in, uint32_t arraySize, uint32_t min_entries, uint32_t max_entries, uint32_t num_entries_for_new_buffer, float allocGrowFactor) noexcept; virtual ~BufferTypeBase(); virtual void destroy_entries(void *buffer, EntryCount num_entries) = 0; @@ -57,6 +57,7 @@ public: */ virtual void initialize_reserved_entries(void *buffer, EntryCount reserved_entries) = 0; size_t entry_size() const noexcept { return _entry_size; } + uint32_t buffer_underflow_size() const noexcept { return _buffer_underflow_size; } virtual void clean_hold(void *buffer, size_t offset, EntryCount num_entries, CleanContext cleanCtx) = 0; size_t getArraySize() const noexcept { return _arraySize; } virtual void on_active(uint32_t bufferId, std::atomic<EntryCount>* used_entries, std::atomic<EntryCount>* dead_entries, void* buffer); @@ -64,6 +65,7 @@ public: virtual void on_free(EntryCount used_entries); void resume_primary_buffer(uint32_t buffer_id, std::atomic<EntryCount>* used_entries, std::atomic<EntryCount>* dead_entries); virtual const alloc::MemoryAllocator* get_memory_allocator() const; + virtual bool is_dynamic_array_buffer_type() const noexcept; /** * Calculate number of entries to allocate for new buffer given how many free entries are needed. @@ -114,6 +116,16 @@ protected: uint32_t _entry_size; // Number of bytes in an allocation unit uint32_t _arraySize; // Number of elements in an allocation unit + + /* + * Buffer underflow size is the size of an area before the start + * of the logical buffer that is safe to access (part of the same + * memory alloation as the buffer itself). This allows for data + * belonging to an entry to be placed at the end of what is normally + * the last part of the previos entry (e.g. dynamic array size + * for the dynamic array buffer type). + */ + uint32_t _buffer_underflow_size; uint32_t _min_entries; // Minimum number of entries to allocate in a buffer uint32_t _max_entries; // Maximum number of entries to allocate in a buffer // Number of entries needed before allocating a new buffer instead of just resizing the first one diff --git a/vespalib/src/vespa/vespalib/datastore/buffer_type.hpp b/vespalib/src/vespa/vespalib/datastore/buffer_type.hpp index 375c832d9fb..00d642be9bc 100644 --- a/vespalib/src/vespa/vespalib/datastore/buffer_type.hpp +++ b/vespalib/src/vespa/vespalib/datastore/buffer_type.hpp @@ -8,13 +8,13 @@ namespace vespalib::datastore { template <typename ElemT, typename EmptyT> BufferType<ElemT, EmptyT>::BufferType(uint32_t arraySize, uint32_t min_entries, uint32_t max_entries) noexcept - : BufferTypeBase(arraySize * sizeof(ElemT), arraySize, min_entries, max_entries) + : BufferTypeBase(arraySize * sizeof(ElemT), 0u, arraySize, min_entries, max_entries) { } template <typename ElemT, typename EmptyT> BufferType<ElemT, EmptyT>::BufferType(uint32_t arraySize, uint32_t min_entries, uint32_t max_entries, uint32_t num_entries_for_new_buffer, float allocGrowFactor) noexcept - : BufferTypeBase(arraySize * sizeof(ElemT), arraySize, min_entries, max_entries, num_entries_for_new_buffer, allocGrowFactor) + : BufferTypeBase(arraySize * sizeof(ElemT), 0u, arraySize, min_entries, max_entries, num_entries_for_new_buffer, allocGrowFactor) { } template <typename ElemT, typename EmptyT> diff --git a/vespalib/src/vespa/vespalib/datastore/bufferstate.cpp b/vespalib/src/vespa/vespalib/datastore/bufferstate.cpp index f312596d6f7..e7832a1c4e2 100644 --- a/vespalib/src/vespa/vespalib/datastore/bufferstate.cpp +++ b/vespalib/src/vespa/vespalib/datastore/bufferstate.cpp @@ -64,13 +64,14 @@ calc_allocation(uint32_t bufferId, { size_t alloc_entries = typeHandler.calc_entries_to_alloc(bufferId, free_entries_needed, resizing); size_t entry_size = typeHandler.entry_size(); - size_t allocBytes = roundUpToMatchAllocator(alloc_entries * entry_size); - size_t maxAllocBytes = typeHandler.get_max_entries() * entry_size; + auto buffer_underflow_size = typeHandler.buffer_underflow_size(); + size_t allocBytes = roundUpToMatchAllocator(alloc_entries * entry_size + buffer_underflow_size); + size_t maxAllocBytes = typeHandler.get_max_entries() * entry_size + buffer_underflow_size; if (allocBytes > maxAllocBytes) { // Ensure that allocated bytes does not exceed the maximum handled by this type. allocBytes = maxAllocBytes; } - size_t adjusted_alloc_entries = allocBytes / entry_size; + size_t adjusted_alloc_entries = (allocBytes - buffer_underflow_size) / entry_size; return AllocResult(adjusted_alloc_entries, allocBytes); } @@ -102,7 +103,8 @@ BufferState::on_active(uint32_t bufferId, uint32_t typeId, _buffer = (allocator != nullptr) ? Alloc::alloc_with_allocator(allocator) : Alloc::alloc(0, MemoryAllocator::HUGEPAGE_SIZE); _buffer.create(alloc.bytes).swap(_buffer); assert(_buffer.get() != nullptr || alloc.entries == 0u); - buffer.store(_buffer.get(), std::memory_order_release); + auto buffer_underflow_size = typeHandler->buffer_underflow_size(); + buffer.store(get_buffer(buffer_underflow_size), std::memory_order_release); _stats.set_alloc_entries(alloc.entries); _typeHandler.store(typeHandler, std::memory_order_release); assert(typeId <= std::numeric_limits<uint16_t>::max()); @@ -117,28 +119,30 @@ void BufferState::onHold(uint32_t buffer_id) { assert(getState() == State::ACTIVE); - assert(getTypeHandler() != nullptr); + auto type_handler = getTypeHandler(); + assert(type_handler != nullptr); _state.store(State::HOLD, std::memory_order_release); _compacting = false; assert(_stats.dead_entries() <= size()); assert(_stats.hold_entries() <= (size() - _stats.dead_entries())); _stats.set_dead_entries(0); _stats.set_hold_entries(size()); - getTypeHandler()->on_hold(buffer_id, &_stats.used_entries_ref(), &_stats.dead_entries_ref()); + type_handler->on_hold(buffer_id, &_stats.used_entries_ref(), &_stats.dead_entries_ref()); _free_list.disable(); } void BufferState::onFree(std::atomic<void*>& buffer) { - assert(buffer.load(std::memory_order_relaxed) == _buffer.get()); assert(getState() == State::HOLD); - assert(_typeHandler != nullptr); + auto type_handler = getTypeHandler(); + assert(type_handler != nullptr); + assert(buffer.load(std::memory_order_relaxed) == get_buffer(type_handler->buffer_underflow_size())); assert(_stats.dead_entries() <= size()); assert(_stats.hold_entries() == (size() - _stats.dead_entries())); - getTypeHandler()->destroy_entries(buffer, size()); + type_handler->destroy_entries(buffer, size()); Alloc::alloc().swap(_buffer); - getTypeHandler()->on_free(size()); + type_handler->on_free(size()); buffer.store(nullptr, std::memory_order_release); _stats.clear(); _state.store(State::FREE, std::memory_order_release); @@ -200,9 +204,11 @@ BufferState::free_entries(EntryRef ref, size_t num_entries, size_t ref_offset) } _stats.inc_dead_entries(num_entries); _stats.dec_hold_entries(num_entries); - getTypeHandler()->clean_hold(_buffer.get(), ref_offset, num_entries, - BufferTypeBase::CleanContext(_stats.extra_used_bytes_ref(), - _stats.extra_hold_bytes_ref())); + auto type_handler = getTypeHandler(); + auto buffer_underflow_size = type_handler->buffer_underflow_size(); + type_handler->clean_hold(get_buffer(buffer_underflow_size), ref_offset, num_entries, + BufferTypeBase::CleanContext(_stats.extra_used_bytes_ref(), + _stats.extra_hold_bytes_ref())); } void @@ -212,17 +218,19 @@ BufferState::fallback_resize(uint32_t bufferId, Alloc &holdBuffer) { assert(getState() == State::ACTIVE); - assert(_typeHandler != nullptr); + auto type_handler = getTypeHandler(); + assert(type_handler != nullptr); assert(holdBuffer.get() == nullptr); - AllocResult alloc = calc_allocation(bufferId, *_typeHandler, free_entries_needed, true); + auto buffer_underflow_size = type_handler->buffer_underflow_size(); + AllocResult alloc = calc_allocation(bufferId, *type_handler, free_entries_needed, true); assert(alloc.entries >= size() + free_entries_needed); assert(alloc.entries > capacity()); Alloc newBuffer = _buffer.create(alloc.bytes); - getTypeHandler()->fallback_copy(newBuffer.get(), buffer.load(std::memory_order_relaxed), size()); + type_handler->fallback_copy(get_buffer(newBuffer, buffer_underflow_size), buffer.load(std::memory_order_relaxed), size()); holdBuffer.swap(_buffer); std::atomic_thread_fence(std::memory_order_release); _buffer = std::move(newBuffer); - buffer.store(_buffer.get(), std::memory_order_release); + buffer.store(get_buffer(buffer_underflow_size), std::memory_order_release); _stats.set_alloc_entries(alloc.entries); } diff --git a/vespalib/src/vespa/vespalib/datastore/bufferstate.h b/vespalib/src/vespa/vespalib/datastore/bufferstate.h index 289be32e19b..01439586f5b 100644 --- a/vespalib/src/vespa/vespalib/datastore/bufferstate.h +++ b/vespalib/src/vespa/vespalib/datastore/bufferstate.h @@ -48,6 +48,8 @@ private: bool _disable_entry_hold_list : 1; bool _compacting : 1; + static void *get_buffer(Alloc& buffer, uint32_t buffer_underflow_size) noexcept { return static_cast<char *>(buffer.get()) + buffer_underflow_size; } + void *get_buffer(uint32_t buffer_underflow_size) noexcept { return get_buffer(_buffer, buffer_underflow_size); } public: /** * TODO: Check if per-buffer free lists are useful, or if @@ -137,24 +139,28 @@ public: void* get_buffer_relaxed() noexcept { return _buffer.load(std::memory_order_relaxed); } const void* get_buffer_acquire() const noexcept { return _buffer.load(std::memory_order_acquire); } uint32_t getTypeId() const { return _typeId; } - uint32_t getArraySize() const { return _arraySize; } + uint32_t get_array_size() const { return _array_size; } BufferState * get_state_relaxed() { return _state.load(std::memory_order_relaxed); } const BufferState * get_state_acquire() const { return _state.load(std::memory_order_acquire); } - uint32_t get_entry_size() const { return get_state_acquire()->getTypeHandler()->entry_size(); } + uint32_t get_entry_size() const noexcept { return _entry_size; } void setTypeId(uint32_t typeId) { _typeId = typeId; } - void setArraySize(uint32_t arraySize) { _arraySize = arraySize; } + void set_array_size(uint32_t arraySize) { _array_size = arraySize; } + void set_entry_size(uint32_t entry_size) noexcept { _entry_size = entry_size; } void set_state(BufferState * state) { _state.store(state, std::memory_order_release); } private: BufferAndMeta(void* buffer, BufferState * state, uint32_t typeId, uint32_t arraySize) : _buffer(buffer), _state(state), _typeId(typeId), - _arraySize(arraySize) + _array_size(arraySize) { } std::atomic<void*> _buffer; std::atomic<BufferState*> _state; uint32_t _typeId; - uint32_t _arraySize; + union { + uint32_t _array_size; // Valid unless buffer type is dynamic array buffer type + uint32_t _entry_size; // Valid if buffer type is dynamic array buffer type + }; }; } diff --git a/vespalib/src/vespa/vespalib/datastore/datastorebase.cpp b/vespalib/src/vespa/vespalib/datastore/datastorebase.cpp index 75ffe855a32..5c88900ae92 100644 --- a/vespalib/src/vespa/vespalib/datastore/datastorebase.cpp +++ b/vespalib/src/vespa/vespalib/datastore/datastorebase.cpp @@ -59,7 +59,8 @@ DataStoreBase::FallbackHold::FallbackHold(size_t bytesSize, BufferState::Alloc & DataStoreBase::FallbackHold::~FallbackHold() { - _typeHandler->destroy_entries(_buffer.get(), _used_entries); + auto buffer_underflow_size = _typeHandler->buffer_underflow_size(); + _typeHandler->destroy_entries(static_cast<char *>(_buffer.get()) + buffer_underflow_size, _used_entries); } class DataStoreBase::BufferHold : public GenerationHeldBase { @@ -415,7 +416,11 @@ DataStoreBase::on_active(uint32_t bufferId, uint32_t typeId, size_t entries_need assert(state->isFree()); state->on_active(bufferId, typeId, _typeHandlers[typeId], entries_needed, bufferMeta.get_atomic_buffer()); bufferMeta.setTypeId(typeId); - bufferMeta.setArraySize(state->getArraySize()); + if (_typeHandlers[typeId]->is_dynamic_array_buffer_type()) { + bufferMeta.set_entry_size(_typeHandlers[typeId]->entry_size()); + } else { + bufferMeta.set_array_size(state->getArraySize()); + } if (_freeListsEnabled && state->isActive() && !state->getCompacting()) { state->enable_free_list(_free_lists[state->getTypeId()]); } diff --git a/vespalib/src/vespa/vespalib/datastore/dynamic_array_buffer_type.h b/vespalib/src/vespa/vespalib/datastore/dynamic_array_buffer_type.h index fbd3c2361d1..daff28fcc85 100644 --- a/vespalib/src/vespa/vespalib/datastore/dynamic_array_buffer_type.h +++ b/vespalib/src/vespa/vespalib/datastore/dynamic_array_buffer_type.h @@ -26,12 +26,13 @@ class DynamicArrayBufferType : public BufferTypeBase { using AllocSpec = ArrayStoreConfig::AllocSpec; std::shared_ptr<alloc::MemoryAllocator> _memory_allocator; + public: using ElemType = ElemT; static constexpr size_t entry_min_align = std::max(alignof(uint32_t), alignof(ElemT)); using EntryMinAligner = Aligner<entry_min_align>; - static constexpr size_t entry_bias = EntryMinAligner::align(sizeof(uint32_t)); + static constexpr uint32_t dynamic_array_buffer_underflow_size = 64u; protected: static const ElemType& empty_entry() noexcept; ElemType* get_entry(void *buffer, size_t offset) noexcept { return get_entry(buffer, offset, entry_size()); } @@ -55,10 +56,11 @@ public: const vespalib::alloc::MemoryAllocator* get_memory_allocator() const override; static size_t calc_entry_size(size_t array_size) noexcept; static size_t calc_array_size(size_t entry_size) noexcept; - static ElemType* get_entry(void* buffer, size_t offset, uint32_t entry_size) noexcept { return reinterpret_cast<ElemType*>(static_cast<char*>(buffer) + offset * entry_size + entry_bias); } - static const ElemType* get_entry(const void* buffer, size_t offset, uint32_t entry_size) noexcept { return reinterpret_cast<const ElemType*>(static_cast<const char*>(buffer) + offset * entry_size + entry_bias); } + static ElemType* get_entry(void* buffer, size_t offset, uint32_t entry_size) noexcept { return reinterpret_cast<ElemType*>(static_cast<char*>(buffer) + offset * entry_size); } + static const ElemType* get_entry(const void* buffer, size_t offset, uint32_t entry_size) noexcept { return reinterpret_cast<const ElemType*>(static_cast<const char*>(buffer) + offset * entry_size); } static uint32_t get_dynamic_array_size(const ElemType* buffer) noexcept { return *(reinterpret_cast<const uint32_t*>(buffer) - 1); } static void set_dynamic_array_size(ElemType* buffer, uint32_t array_size) noexcept { *(reinterpret_cast<uint32_t*>(buffer) - 1) = array_size; } + bool is_dynamic_array_buffer_type() const noexcept override; }; extern template class DynamicArrayBufferType<char>; diff --git a/vespalib/src/vespa/vespalib/datastore/dynamic_array_buffer_type.hpp b/vespalib/src/vespa/vespalib/datastore/dynamic_array_buffer_type.hpp index bf3235a6b97..df27e241def 100644 --- a/vespalib/src/vespa/vespalib/datastore/dynamic_array_buffer_type.hpp +++ b/vespalib/src/vespa/vespalib/datastore/dynamic_array_buffer_type.hpp @@ -9,7 +9,7 @@ namespace vespalib::datastore { template <typename ElemT> DynamicArrayBufferType<ElemT>::DynamicArrayBufferType(uint32_t array_size, const AllocSpec& spec, std::shared_ptr<alloc::MemoryAllocator> memory_allocator) noexcept - : BufferTypeBase(calc_entry_size(array_size), array_size, spec.min_entries_in_buffer, spec.max_entries_in_buffer, spec.num_entries_for_new_buffer, spec.allocGrowFactor), + : BufferTypeBase(calc_entry_size(array_size), dynamic_array_buffer_underflow_size, array_size, spec.min_entries_in_buffer, spec.max_entries_in_buffer, spec.num_entries_for_new_buffer, spec.allocGrowFactor), _memory_allocator(std::move(memory_allocator)) { } @@ -24,14 +24,15 @@ template <typename ElemT> size_t DynamicArrayBufferType<ElemT>::calc_entry_size(size_t array_size) noexcept { - return EntryMinAligner::align(sizeof(ElemType) * array_size + entry_bias); + auto entry_size = EntryMinAligner::align(sizeof(ElemType) * array_size + sizeof(uint32_t)); + return entry_size; } template <typename ElemT> size_t DynamicArrayBufferType<ElemT>::calc_array_size(size_t entry_size) noexcept { - return (entry_size - entry_bias) / sizeof(ElemType); + return (entry_size - sizeof(uint32_t)) / sizeof(ElemType); } template <typename ElemT> @@ -116,4 +117,11 @@ DynamicArrayBufferType<ElemT>::get_memory_allocator() const return _memory_allocator.get(); } +template <typename ElemT> +bool +DynamicArrayBufferType<ElemT>::is_dynamic_array_buffer_type() const noexcept +{ + return true; +} + } diff --git a/vespalib/src/vespa/vespalib/datastore/unique_store_string_allocator.h b/vespalib/src/vespa/vespalib/datastore/unique_store_string_allocator.h index d3348950891..102808f7629 100644 --- a/vespalib/src/vespa/vespalib/datastore/unique_store_string_allocator.h +++ b/vespalib/src/vespa/vespalib/datastore/unique_store_string_allocator.h @@ -117,7 +117,7 @@ public: auto &state = _store.getBufferMeta(iRef.bufferId()); auto type_id = state.getTypeId(); if (type_id != 0) { - return *reinterpret_cast<const UniqueStoreEntryBase *>(_store.template getEntryArray<char>(iRef, state.getArraySize())); + return *reinterpret_cast<const UniqueStoreEntryBase *>(_store.template getEntryArray<char>(iRef, state.get_array_size())); } else { return *_store.template getEntry<WrappedExternalEntryType>(iRef); } @@ -127,7 +127,7 @@ public: auto &state = _store.getBufferMeta(iRef.bufferId()); auto type_id = state.getTypeId(); if (type_id != 0) { - return reinterpret_cast<const UniqueStoreSmallStringEntry *>(_store.template getEntryArray<char>(iRef, state.getArraySize()))->value(); + return reinterpret_cast<const UniqueStoreSmallStringEntry *>(_store.template getEntryArray<char>(iRef, state.get_array_size()))->value(); } else { return _store.template getEntry<WrappedExternalEntryType>(iRef)->value().c_str(); } diff --git a/vespalib/src/vespa/vespalib/datastore/unique_store_string_comparator.h b/vespalib/src/vespa/vespalib/datastore/unique_store_string_comparator.h index e507132a085..e71dcd3aafb 100644 --- a/vespalib/src/vespa/vespalib/datastore/unique_store_string_comparator.h +++ b/vespalib/src/vespa/vespalib/datastore/unique_store_string_comparator.h @@ -28,7 +28,7 @@ protected: const auto &meta = _store.getBufferMeta(iRef.bufferId()); auto type_id = meta.getTypeId(); if (type_id != 0) { - return reinterpret_cast<const UniqueStoreSmallStringEntry *>(_store.template getEntryArray<char>(iRef, meta.getArraySize()))->value(); + return reinterpret_cast<const UniqueStoreSmallStringEntry *>(_store.template getEntryArray<char>(iRef, meta.get_array_size()))->value(); } else { return _store.template getEntry<WrappedExternalEntryType>(iRef)->value().c_str(); } diff --git a/vespalib/src/vespa/vespalib/stllike/cache.h b/vespalib/src/vespa/vespalib/stllike/cache.h index f7456cda197..97ba23aba65 100644 --- a/vespalib/src/vespa/vespalib/stllike/cache.h +++ b/vespalib/src/vespa/vespalib/stllike/cache.h @@ -110,7 +110,7 @@ public: */ bool hasKey(const K & key) const; - CacheStats get_stats() const; + virtual CacheStats get_stats() const; size_t getHit() const { return _hit.load(std::memory_order_relaxed); } size_t getMiss() const { return _miss.load(std::memory_order_relaxed); } diff --git a/vespalib/src/vespa/vespalib/stllike/cache.hpp b/vespalib/src/vespa/vespalib/stllike/cache.hpp index 4e7736c9e5f..f767cf9812c 100644 --- a/vespalib/src/vespa/vespalib/stllike/cache.hpp +++ b/vespalib/src/vespa/vespalib/stllike/cache.hpp @@ -61,9 +61,7 @@ cache<P>::getStaticMemoryUsage() const { MemoryUsage usage; auto cacheGuard = getGuard(); usage.incAllocatedBytes(sizeof(*this)); - usage.incAllocatedBytes(Lru::capacity()*sizeof(typename Lru::value_type)); usage.incUsedBytes(sizeof(*this)); - usage.incUsedBytes(Lru::size()*sizeof(typename Lru::value_type)); return usage; } |