From 0cb625f5fd5f94b4b8025a9d4f9b546c7ec94d41 Mon Sep 17 00:00:00 2001 From: jonmv Date: Wed, 18 Jan 2023 13:29:34 +0100 Subject: Revert "Merge pull request #25614 from vespa-engine/revert-25587-jonmv/private-endpoints" This reverts commit 7b736f0a09444664cff118eac5b28e608632de72, reversing changes made to 6c457e6dd5993ec2ef15177dab4a16e3d3702b85. --- .../application/api/DeploymentInstanceSpec.java | 31 ++- .../config/application/api/DeploymentSpec.java | 26 +- .../yahoo/config/application/api/ValidationId.java | 1 + .../api/xml/DeploymentSpecXmlReader.java | 214 +++++++++++++--- .../model/api/ApplicationClusterEndpoint.java | 2 +- .../config/application/api/DeploymentSpecTest.java | 282 ++++++++++++++++----- .../api/DeploymentSpecWithoutInstanceTest.java | 52 +++- .../com/yahoo/config/model/ConfigModelRepo.java | 2 +- .../yahoo/config/model/deploy/TestProperties.java | 2 +- .../vespa/model/builder/VespaModelBuilder.java | 2 +- .../model/builder/xml/dom/NodesSpecification.java | 8 +- .../model/builder/xml/dom/VespaDomBuilder.java | 30 ++- .../model/container/xml/ContainerModelBuilder.java | 25 +- .../src/main/resources/schema/containercluster.rnc | 13 +- .../src/main/resources/schema/deployment.rnc | 11 +- .../container/xml/ContainerModelBuilderTest.java | 162 +++++++++--- .../yahoo/config/provision/ClusterMembership.java | 10 +- .../com/yahoo/config/provision/ClusterSpec.java | 28 +- .../config/provision/LoadBalancerSettings.java | 20 -- .../com/yahoo/config/provision/ZoneEndpoint.java | 132 ++++++++++ .../serialization/AllocatedHostsSerializer.java | 55 +++- .../AllocatedHostsSerializerTest.java | 5 +- configserver/src/test/apps/hosted/services.xml | 5 - .../api/integration/configserver/LoadBalancer.java | 3 +- .../hosted/controller/ApplicationController.java | 4 +- .../pkg/ApplicationPackageValidator.java | 53 +++- .../controller/deployment/JobController.java | 2 +- .../restapi/application/ApplicationApiHandler.java | 11 +- .../vespa/hosted/controller/ControllerTest.java | 88 +++++++ .../controller/integration/ConfigServerMock.java | 4 +- .../restapi/application/ApplicationApiTest.java | 2 +- .../hosted/provision/lb/LoadBalancerInstance.java | 8 +- .../provision/lb/LoadBalancerServiceMock.java | 4 +- .../hosted/provision/lb/LoadBalancerSpec.java | 8 +- .../provision/lb/SharedLoadBalancerService.java | 8 +- .../persistence/LoadBalancerSerializer.java | 50 +++- .../provisioning/LoadBalancerProvisioner.java | 28 +- .../provision/restapi/LoadBalancersResponse.java | 14 +- .../provision/testutils/MockNodeRepository.java | 4 +- .../lb/SharedLoadBalancerServiceTest.java | 4 +- .../persistence/LoadBalancerSerializerTest.java | 4 +- .../provisioning/LoadBalancerProvisionerTest.java | 12 +- .../provisioning/VirtualNodeProvisioningTest.java | 30 +-- .../restapi/responses/load-balancers.json | 7 +- .../vespa/feed/client/impl/ApacheClusterTest.java | 4 +- 45 files changed, 1153 insertions(+), 317 deletions(-) delete mode 100644 config-provisioning/src/main/java/com/yahoo/config/provision/LoadBalancerSettings.java create mode 100644 config-provisioning/src/main/java/com/yahoo/config/provision/ZoneEndpoint.java diff --git a/config-model-api/src/main/java/com/yahoo/config/application/api/DeploymentInstanceSpec.java b/config-model-api/src/main/java/com/yahoo/config/application/api/DeploymentInstanceSpec.java index fdde4c38fb8..b36c1409459 100644 --- a/config-model-api/src/main/java/com/yahoo/config/application/api/DeploymentInstanceSpec.java +++ b/config-model-api/src/main/java/com/yahoo/config/application/api/DeploymentInstanceSpec.java @@ -3,17 +3,23 @@ package com.yahoo.config.application.api; import com.yahoo.config.provision.AthenzService; import com.yahoo.config.provision.CloudAccount; +import com.yahoo.config.provision.ClusterSpec; import com.yahoo.config.provision.Environment; import com.yahoo.config.provision.InstanceName; import com.yahoo.config.provision.RegionName; import com.yahoo.config.provision.Tags; +import com.yahoo.config.provision.ZoneEndpoint; +import com.yahoo.config.provision.zone.ZoneId; import java.time.Duration; import java.time.Instant; import java.time.temporal.ChronoUnit; import java.util.Arrays; +import java.util.Collections; +import java.util.HashMap; import java.util.HashSet; import java.util.List; +import java.util.Map; import java.util.Objects; import java.util.Optional; import java.util.Set; @@ -26,7 +32,6 @@ import static ai.vespa.validation.Validation.requireInRange; import static com.yahoo.config.application.api.DeploymentSpec.RevisionChange.whenClear; import static com.yahoo.config.application.api.DeploymentSpec.RevisionTarget.next; import static com.yahoo.config.provision.Environment.prod; -import static java.util.stream.Collectors.toList; /** * The deployment spec for an application instance @@ -55,6 +60,7 @@ public class DeploymentInstanceSpec extends DeploymentSpec.Steps { private final Optional cloudAccount; private final Notifications notifications; private final List endpoints; + private final Map> zoneEndpoints; public DeploymentInstanceSpec(InstanceName name, Tags tags, @@ -70,6 +76,7 @@ public class DeploymentInstanceSpec extends DeploymentSpec.Steps { Optional cloudAccount, Notifications notifications, List endpoints, + Map> zoneEndpoints, Instant now) { super(steps); this.name = Objects.requireNonNull(name); @@ -91,6 +98,9 @@ public class DeploymentInstanceSpec extends DeploymentSpec.Steps { this.cloudAccount = Objects.requireNonNull(cloudAccount); this.notifications = Objects.requireNonNull(notifications); this.endpoints = List.copyOf(Objects.requireNonNull(endpoints)); + Map> zoneEndpointsCopy = new HashMap<>(); + for (var entry : zoneEndpoints.entrySet()) zoneEndpointsCopy.put(entry.getKey(), Collections.unmodifiableMap(new HashMap<>(entry.getValue()))); + this.zoneEndpoints = Collections.unmodifiableMap(zoneEndpointsCopy); validateZones(new HashSet<>(), new HashSet<>(), this); validateEndpoints(steps(), globalServiceId, this.endpoints); validateChangeBlockers(changeBlockers, now); @@ -251,6 +261,25 @@ public class DeploymentInstanceSpec extends DeploymentSpec.Steps { return zones().stream().anyMatch(zone -> zone.concerns(environment, Optional.of(region))); } + /** Returns the zone endpoint specified for the given region, or empty. */ + Optional zoneEndpoint(ZoneId zone, ClusterSpec.Id cluster) { + return Optional.ofNullable(zoneEndpoints.get(cluster)) + .filter(__ -> deploysTo(zone.environment(), zone.region())) + .map(zoneEndpoints -> zoneEndpoints.get(zoneEndpoints.containsKey(zone) ? zone : null)); + } + + /** Returns the zone endpoint data for this instance. */ + Map> zoneEndpoints() { + return zoneEndpoints; + } + + /** The zone endpoints in the given zone, possibly default values. */ + public Map zoneEndpoints(ZoneId zone) { + return zoneEndpoints.keySet().stream() + .collect(Collectors.toMap(cluster -> cluster, + cluster -> zoneEndpoint(zone, cluster).orElse(ZoneEndpoint.defaultEndpoint))); + } + @Override public boolean equals(Object o) { if (this == o) return true; diff --git a/config-model-api/src/main/java/com/yahoo/config/application/api/DeploymentSpec.java b/config-model-api/src/main/java/com/yahoo/config/application/api/DeploymentSpec.java index 6c519a4656e..d731e09d4e4 100644 --- a/config-model-api/src/main/java/com/yahoo/config/application/api/DeploymentSpec.java +++ b/config-model-api/src/main/java/com/yahoo/config/application/api/DeploymentSpec.java @@ -6,16 +6,21 @@ import com.yahoo.config.application.api.xml.DeploymentSpecXmlReader; import com.yahoo.config.provision.AthenzDomain; import com.yahoo.config.provision.AthenzService; import com.yahoo.config.provision.CloudAccount; +import com.yahoo.config.provision.ClusterSpec; import com.yahoo.config.provision.Environment; import com.yahoo.config.provision.InstanceName; import com.yahoo.config.provision.RegionName; +import com.yahoo.config.provision.ZoneEndpoint; +import com.yahoo.config.provision.zone.ZoneId; import java.io.Reader; import java.time.Duration; import java.util.Arrays; import java.util.Collections; import java.util.Comparator; +import java.util.HashMap; import java.util.List; +import java.util.Map; import java.util.Objects; import java.util.Optional; import java.util.stream.Collectors; @@ -47,7 +52,7 @@ public class DeploymentSpec { private final List steps; - // Attributes which can be set on the root tag and which must be available outside of any particular instance + // Attributes which can be set on the root tag and which must be available outside any particular instance private final Optional majorVersion; private final Optional athenzDomain; private final Optional athenzService; @@ -145,6 +150,10 @@ public class DeploymentSpec { illegal(prefix + "targets undeclared region '" + target.region() + "' in instance '" + target.instance() + "'"); } + if (instance.get().zoneEndpoint(ZoneId.from(Environment.prod, target.region()), ClusterSpec.Id.from(endpoint.containerId())) + .map(zoneEndpoint -> ! zoneEndpoint.isPublicEndpoint()).orElse(false)) + illegal(prefix + "targets '" + target.region().value() + "' in '" + target.instance().value() + + "', but its zone endpoint has 'enabled' set to 'false'"); } } } @@ -175,6 +184,21 @@ public class DeploymentSpec { /** Cloud account set on the deployment root; see discussion for {@link #athenzService}. */ public Optional cloudAccount() { return cloudAccount; } + /** + * Returns the most specific zone endpoint, where specificity is given, in decreasing order: + * 1. The given instance has declared a zone endpoint for the cluster, for the given region. + * 2. The given instance has declared a universal zone endpoint for the cluster. + * 3. The application has declared a zone endpoint for the cluster, for the given region. + * 4. The application has declared a universal zone endpoint for the cluster. + * 5. None of the above apply, and the default of a publicly visible endpoint is used. + */ + public ZoneEndpoint zoneEndpoint(InstanceName instance, ZoneId zone, ClusterSpec.Id cluster) { + // TODO: look up endpoints from tag, or so, if we're to support non-prod settings. + if (zone.environment() != Environment.prod) return ZoneEndpoint.defaultEndpoint; + return instance(instance).flatMap(spec -> spec.zoneEndpoint(zone, cluster)) + .orElse(ZoneEndpoint.defaultEndpoint); + } + /** Returns the XML form of this spec, or null if it was not created by fromXml, nor is empty */ public String xmlForm() { return xmlForm; } diff --git a/config-model-api/src/main/java/com/yahoo/config/application/api/ValidationId.java b/config-model-api/src/main/java/com/yahoo/config/application/api/ValidationId.java index 571cc3c7d5c..b4be99ad20b 100644 --- a/config-model-api/src/main/java/com/yahoo/config/application/api/ValidationId.java +++ b/config-model-api/src/main/java/com/yahoo/config/application/api/ValidationId.java @@ -24,6 +24,7 @@ public enum ValidationId { skipOldConfigModels("skip-old-config-models"), // Internal use accessControl("access-control"), // Internal use, used in zones where there should be no access-control globalEndpointChange("global-endpoint-change"), // Changing global endpoints + zoneEndpointChange("zone-endpoint-change"), // Changing zone (possibly private) endpoint settings redundancyIncrease("redundancy-increase"), // Increasing redundancy - may easily cause feed blocked redundancyOne("redundancy-one"), // redundancy=1 requires a validation override on first deployment pagedSettingRemoval("paged-setting-removal"), // May cause content nodes to run out of memory diff --git a/config-model-api/src/main/java/com/yahoo/config/application/api/xml/DeploymentSpecXmlReader.java b/config-model-api/src/main/java/com/yahoo/config/application/api/xml/DeploymentSpecXmlReader.java index 8182e697e7e..fb6d834f783 100644 --- a/config-model-api/src/main/java/com/yahoo/config/application/api/xml/DeploymentSpecXmlReader.java +++ b/config-model-api/src/main/java/com/yahoo/config/application/api/xml/DeploymentSpecXmlReader.java @@ -15,6 +15,8 @@ import com.yahoo.config.application.api.DeploymentSpec.Steps; import com.yahoo.config.application.api.DeploymentSpec.UpgradePolicy; import com.yahoo.config.application.api.DeploymentSpec.UpgradeRollout; import com.yahoo.config.application.api.Endpoint; +import com.yahoo.config.application.api.Endpoint.Level; +import com.yahoo.config.application.api.Endpoint.Target; import com.yahoo.config.application.api.Notifications; import com.yahoo.config.application.api.Notifications.Role; import com.yahoo.config.application.api.Notifications.When; @@ -22,10 +24,15 @@ import com.yahoo.config.application.api.TimeWindow; import com.yahoo.config.provision.AthenzDomain; import com.yahoo.config.provision.AthenzService; import com.yahoo.config.provision.CloudAccount; +import com.yahoo.config.provision.ClusterSpec; import com.yahoo.config.provision.Environment; import com.yahoo.config.provision.InstanceName; import com.yahoo.config.provision.RegionName; import com.yahoo.config.provision.Tags; +import com.yahoo.config.provision.ZoneEndpoint; +import com.yahoo.config.provision.ZoneEndpoint.AllowedUrn; +import com.yahoo.config.provision.ZoneEndpoint.AccessType; +import com.yahoo.config.provision.zone.ZoneId; import com.yahoo.io.IOUtils; import com.yahoo.text.XML; import org.w3c.dom.Element; @@ -39,16 +46,20 @@ import java.time.Instant; import java.util.ArrayList; import java.util.Arrays; import java.util.Collections; +import java.util.Comparator; import java.util.HashMap; import java.util.LinkedHashMap; +import java.util.LinkedHashSet; import java.util.List; import java.util.Map; import java.util.Objects; import java.util.Optional; +import java.util.Set; import java.util.function.Function; -import java.util.stream.Collectors; import java.util.stream.Stream; +import static java.util.Comparator.comparingInt; + /** * @author bratseth */ @@ -123,7 +134,7 @@ public class DeploymentSpecXmlReader { return DeploymentSpec.empty; List steps = new ArrayList<>(); - List applicationEndpoints = List.of(); + List applicationEndpoints = new ArrayList<>(); if ( ! containsTag(instanceTag, root)) { // deployment spec skipping explicit instance -> "default" instance steps.addAll(readInstanceContent("default", root, new HashMap<>(), root)); } @@ -141,7 +152,7 @@ public class DeploymentSpecXmlReader { steps.addAll(readNonInstanceSteps(child, new HashMap<>(), root)); // (No global service id here) } } - applicationEndpoints = readEndpoints(root, Optional.empty(), steps); + readEndpoints(root, Optional.empty(), steps, applicationEndpoints, Map.of()); } return new DeploymentSpec(steps, @@ -190,11 +201,13 @@ public class DeploymentSpecXmlReader { Notifications notifications = readNotifications(instanceElement, parentTag); // Values where there is no default - Tags tags = XML.attribute(tagsTag, instanceElement).map(value -> Tags.fromString(value)).orElse(Tags.empty()); + Tags tags = XML.attribute(tagsTag, instanceElement).map(Tags::fromString).orElse(Tags.empty()); List steps = new ArrayList<>(); for (Element instanceChild : XML.getChildren(instanceElement)) steps.addAll(readNonInstanceSteps(instanceChild, prodAttributes, instanceChild)); - List endpoints = readEndpoints(instanceElement, Optional.of(instanceNameString), steps); + List endpoints = new ArrayList<>(); + Map> zoneEndpoints = new LinkedHashMap<>(); + readEndpoints(instanceElement, Optional.of(instanceNameString), steps, endpoints, zoneEndpoints); // Build and return instances with these values Instant now = clock.instant(); @@ -214,6 +227,7 @@ public class DeploymentSpecXmlReader { cloudAccount, notifications, endpoints, + zoneEndpoints, now)) .toList(); } @@ -306,19 +320,54 @@ public class DeploymentSpecXmlReader { return Notifications.of(emailAddresses, emailRoles); } - private List readEndpoints(Element parent, Optional instance, List steps) { + private void readEndpoints(Element parent, Optional instance, List steps, List endpoints, + Map> zoneEndpoints) { var endpointsElement = XML.getChild(parent, endpointsTag); - if (endpointsElement == null) return List.of(); + if (endpointsElement == null) return; Endpoint.Level level = instance.isEmpty() ? Endpoint.Level.application : Endpoint.Level.instance; - Map endpoints = new LinkedHashMap<>(); - for (var endpointElement : XML.getChildren(endpointsElement, endpointTag)) { - String endpointId = stringAttribute("id", endpointElement).orElse(Endpoint.DEFAULT_ID); + Map endpointsById = new LinkedHashMap<>(); + Map>> endpointsByZone = new LinkedHashMap<>(); + XML.getChildren(endpointsElement, endpointTag).stream() // Read zone settings first. + .sorted(comparingInt(endpoint -> getZoneEndpointType(endpoint, level).isPresent() ? 0 : 1)) + .forEach(endpointElement -> { String containerId = requireStringAttribute("container-id", endpointElement); + Optional endpointId = stringAttribute("id", endpointElement); + Optional zoneEndpointType = getZoneEndpointType(endpointElement, level); String msgPrefix = (level == Endpoint.Level.application ? "Application-level" : "Instance-level") + - " endpoint '" + endpointId + "': "; + " endpoint '" + endpointId.orElse(Endpoint.DEFAULT_ID) + "': "; + + if (zoneEndpointType.isPresent() && endpointId.isPresent()) + illegal(msgPrefix + "cannot declare 'id' with type 'zone' or 'private'"); + String invalidChild = level == Endpoint.Level.application ? "region" : "instance"; - if (!XML.getChildren(endpointElement, invalidChild).isEmpty()) illegal(msgPrefix + "invalid element '" + invalidChild + "'"); + if ( ! XML.getChildren(endpointElement, invalidChild).isEmpty()) + illegal(msgPrefix + "invalid element '" + invalidChild + "'"); + + boolean enabled = XML.attribute("enabled", endpointElement) + .map(value -> { + if (zoneEndpointType.isEmpty() || ! zoneEndpointType.get().equals("zone")) + illegal(msgPrefix + "only endpoints of type 'zone' can specify 'enabled'"); + + return switch (value) { + case "true" -> true; + case "false" -> false; + default -> throw new IllegalArgumentException(msgPrefix + "invalid 'enabled' value; must be 'true' or 'false'"); + }; + }).orElse(true); + + List allowedUrns = new ArrayList<>(); + for (var allow : XML.getChildren(endpointElement, "allow")) { + if (zoneEndpointType.isEmpty() || ! zoneEndpointType.get().equals("private")) + illegal(msgPrefix + "only endpoints of type 'private' can specify 'allow' children"); + + switch (requireStringAttribute("with", allow)) { + case "aws-private-link" -> allowedUrns.add(new AllowedUrn(AccessType.awsPrivateLink, requireStringAttribute("arn", allow))); + case "gcp-service-connect" -> allowedUrns.add(new AllowedUrn(AccessType.gcpServiceConnect, requireStringAttribute("project", allow))); + default -> illegal("Private endpoint for container-id '" + containerId + "': " + + "invalid attribute 'with': '" + requireStringAttribute("with", allow) + "'"); + } + } List targets = new ArrayList<>(); if (level == Endpoint.Level.application) { @@ -345,33 +394,132 @@ public class DeploymentSpecXmlReader { if (weightSum == 0) illegal(msgPrefix + "sum of all weights must be positive, got " + weightSum); } else { if (stringAttribute("region", endpointElement).isPresent()) illegal(msgPrefix + "invalid 'region' attribute"); + Set regions = new LinkedHashSet<>(); for (var regionElement : XML.getChildren(endpointElement, "region")) { String region = regionElement.getTextContent(); - if (region == null || region.isBlank()) illegal(msgPrefix + "empty 'region' element"); - targets.add(new Endpoint.Target(RegionName.from(region), - InstanceName.from(instance.get()), - 1)); + if (region == null || region.isBlank()) + illegal(msgPrefix + "empty 'region' element"); + if ( zoneEndpointType.isEmpty() + && Stream.of(RegionName.from(region), null) + .map(endpointsByZone.getOrDefault(containerId, new HashMap<>())::get) + .flatMap(maybeEndpoints -> maybeEndpoints == null ? Stream.empty() : maybeEndpoints.stream()) + .anyMatch(endpoint -> ! endpoint.isPublicEndpoint())) + illegal(msgPrefix + "targets zone endpoint in '" + region + "' with 'enabled' set to 'false'"); + if ( ! regions.add(RegionName.from(region))) + illegal(msgPrefix + "duplicate 'region' element: '" + region + "'"); + } + + if (zoneEndpointType.isPresent()) { + if (regions.isEmpty()) regions.add(null); + ZoneEndpoint endpoint = switch (zoneEndpointType.get()) { + case "zone" -> new ZoneEndpoint(enabled, false, List.of()); + case "private" -> new ZoneEndpoint(true, true, allowedUrns); // Doesn't turn off public visibility. + default -> throw new IllegalArgumentException("unsupported zone endpoint type '" + zoneEndpointType.get() + "'"); + }; + for (RegionName region : regions) endpointsByZone.computeIfAbsent(containerId, __ -> new LinkedHashMap<>()) + .computeIfAbsent(region, __ -> new ArrayList<>()) + .add(endpoint); + } + else { + if (regions.isEmpty()) { + // No explicit targets given for instance level endpoint. Include all declared, enabled regions by default + List declared = + steps.stream() + .filter(step -> step.concerns(Environment.prod)) + .flatMap(step -> step.zones().stream()) + .flatMap(zone -> zone.region().stream()) + .toList(); + if (declared.isEmpty()) illegal(msgPrefix + "no declared regions to target"); + + declared.stream().filter(region -> Stream.of(region, null) + .map(endpointsByZone.getOrDefault(containerId, new HashMap<>())::get) + .flatMap(maybeEndpoints -> maybeEndpoints == null ? Stream.empty() : maybeEndpoints.stream()) + .allMatch(ZoneEndpoint::isPublicEndpoint)) + .forEach(regions::add); + } + if (regions.isEmpty()) illegal(msgPrefix + "all eligible zone endpoints have 'enabled' set to 'false'"); + InstanceName instanceName = instance.map(InstanceName::from).get(); + for (RegionName region : regions) targets.add(new Target(region, instanceName, 1)); } - } - if (targets.isEmpty() && level == Endpoint.Level.instance) { - // No explicit targets given for instance level endpoint. Include all declared regions by default - InstanceName instanceName = instance.map(InstanceName::from).get(); - steps.stream() - .filter(step -> step.concerns(Environment.prod)) - .flatMap(step -> step.zones().stream()) - .flatMap(zone -> zone.region().stream()) - .distinct() - .map(region -> new Endpoint.Target(region, instanceName, 1)) - .forEach(targets::add); } - Endpoint endpoint = new Endpoint(endpointId, containerId, level, targets); - if (endpoints.containsKey(endpoint.endpointId())) { - illegal("Endpoint ID '" + endpoint.endpointId() + "' is specified multiple times"); + if (zoneEndpointType.isEmpty()) { + Endpoint endpoint = new Endpoint(endpointId.orElse(Endpoint.DEFAULT_ID), containerId, level, targets); + if (endpointsById.containsKey(endpoint.endpointId())) { + illegal("Endpoint ID '" + endpoint.endpointId() + "' is specified multiple times"); + } + endpointsById.put(endpoint.endpointId(), endpoint); } - endpoints.put(endpoint.endpointId(), endpoint); - } - return List.copyOf(endpoints.values()); + }); + endpoints.addAll(endpointsById.values()); + validateAndConsolidate(endpointsByZone, zoneEndpoints); + } + + static void validateAndConsolidate(Map>> in, Map> out) { + in.forEach((cluster, regions) -> { + List wildcards = regions.remove(null); + ZoneEndpoint wildcardZoneEndpoint = null; + ZoneEndpoint wildcardPrivateEndpoint = null; + if (wildcards != null) { + for (ZoneEndpoint endpoint : wildcards) { + if (endpoint.isPrivateEndpoint()) { + if (wildcardPrivateEndpoint != null) illegal("Multiple private endpoints (for all regions) declared for " + + "container id '" + cluster + "'"); + wildcardPrivateEndpoint = endpoint; + } + else { + if (wildcardZoneEndpoint != null) illegal("Multiple zone endpoints (for all regions) declared " + + "for container id '" + cluster + "'"); + wildcardZoneEndpoint = endpoint; + } + } + } + for (RegionName region : regions.keySet()) { + ZoneEndpoint zoneEndpoint = null; + ZoneEndpoint privateEndpoint = null; + for (ZoneEndpoint endpoint : regions.getOrDefault(region, List.of())) { + if (endpoint.isPrivateEndpoint()) { + if (privateEndpoint != null) illegal("Multiple private endpoints declared for " + + "container id '" + cluster + "' in region '" + region + "'"); + privateEndpoint = endpoint; + } + else { + if (zoneEndpoint != null) illegal("Multiple zone endpoints (without regions) declared " + + "for container id '" + cluster + "' in region '" + region + "'"); + zoneEndpoint = endpoint; + } + } + if (wildcardZoneEndpoint != null && zoneEndpoint != null) illegal("Zone endpoint for container id '" + cluster + "' declared " + + "both with region '" + region + "', and for all regions."); + if (wildcardPrivateEndpoint != null && privateEndpoint != null) illegal("Private endpoint for container id '" + cluster + "' declared " + + "both with region '" + region + "', and for all regions."); + + if (zoneEndpoint == null) zoneEndpoint = wildcardZoneEndpoint; + if (privateEndpoint == null) privateEndpoint = wildcardPrivateEndpoint; + + // Gosh, we made it here! Now we'll combine the settings for zone and private types into one ZoneEndpoint to rule them all. + out.computeIfAbsent(ClusterSpec.Id.from(cluster), __ -> new LinkedHashMap<>()) + .put(ZoneId.from(Environment.prod, region), new ZoneEndpoint(zoneEndpoint == null || zoneEndpoint.isPublicEndpoint(), + privateEndpoint != null, + privateEndpoint != null ? privateEndpoint.allowedUrns() : List.of())); + } + out.computeIfAbsent(ClusterSpec.Id.from(cluster), __ -> new LinkedHashMap<>()) + .put(null, new ZoneEndpoint(wildcardZoneEndpoint == null || wildcardZoneEndpoint.isPublicEndpoint(), + wildcardPrivateEndpoint != null, + wildcardPrivateEndpoint != null ? wildcardPrivateEndpoint.allowedUrns() : List.of())); + }); + } + + /** Returns endpoint type if a private or zone type endpoint, throws if invalid, or otherwise returns empty (global, application). */ + static Optional getZoneEndpointType(Element endpoint, Level level) { + Optional type = XML.attribute("type", endpoint); + if (type.isPresent() && ! List.of("zone", "private", "global", "application").contains(type.get())) + illegal("Illegal endpoint type '" + type.get() + "'"); + + String implied = switch (level) { case instance -> "global"; case application -> "application"; }; + if (type.isEmpty() || type.get().equals(implied)) return Optional.empty(); + if (level == Level.instance && (type.get().equals("zone") || type.get().equals("private"))) return type; + throw new IllegalArgumentException("Endpoints at " + level + " level cannot be of type '" + type.get() + "'"); } /** diff --git a/config-model-api/src/main/java/com/yahoo/config/model/api/ApplicationClusterEndpoint.java b/config-model-api/src/main/java/com/yahoo/config/model/api/ApplicationClusterEndpoint.java index 99bf768f67e..b7969267328 100644 --- a/config-model-api/src/main/java/com/yahoo/config/model/api/ApplicationClusterEndpoint.java +++ b/config-model-api/src/main/java/com/yahoo/config/model/api/ApplicationClusterEndpoint.java @@ -26,7 +26,7 @@ public class ApplicationClusterEndpoint { ", routingMethod=" + routingMethod + ", weight=" + weight + ", hostNames=" + hostNames + - ", clusterId='" + clusterId + '\'' + + ", clusterId='" + clusterId + "'" + '}'; } diff --git a/config-model-api/src/test/java/com/yahoo/config/application/api/DeploymentSpecTest.java b/config-model-api/src/test/java/com/yahoo/config/application/api/DeploymentSpecTest.java index 96cd4810ec4..355ce651c34 100644 --- a/config-model-api/src/test/java/com/yahoo/config/application/api/DeploymentSpecTest.java +++ b/config-model-api/src/test/java/com/yahoo/config/application/api/DeploymentSpecTest.java @@ -6,10 +6,14 @@ import com.yahoo.config.application.api.Endpoint.Level; import com.yahoo.config.application.api.Endpoint.Target; import com.yahoo.config.application.api.xml.DeploymentSpecXmlReader; import com.yahoo.config.provision.CloudAccount; +import com.yahoo.config.provision.ClusterSpec; import com.yahoo.config.provision.Environment; import com.yahoo.config.provision.InstanceName; import com.yahoo.config.provision.RegionName; import com.yahoo.config.provision.Tags; +import com.yahoo.config.provision.ZoneEndpoint; +import com.yahoo.config.provision.ZoneEndpoint.AllowedUrn; +import com.yahoo.config.provision.ZoneEndpoint.AccessType; import com.yahoo.test.ManualClock; import org.junit.Test; @@ -19,6 +23,7 @@ import java.time.Duration; import java.time.Instant; import java.time.ZoneId; import java.util.Collections; +import java.util.HashSet; import java.util.List; import java.util.Optional; import java.util.Set; @@ -27,9 +32,12 @@ import java.util.stream.Collectors; import static com.yahoo.config.application.api.Notifications.Role.author; import static com.yahoo.config.application.api.Notifications.When.failing; import static com.yahoo.config.application.api.Notifications.When.failingCommit; +import static com.yahoo.config.provision.zone.ZoneId.defaultId; +import static com.yahoo.config.provision.zone.ZoneId.from; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertNotEquals; +import static org.junit.Assert.assertNull; import static org.junit.Assert.assertThrows; import static org.junit.Assert.assertTrue; import static org.junit.Assert.fail; @@ -1183,15 +1191,16 @@ public class DeploymentSpecTest { @Test public void customTesterFlavor() { - DeploymentSpec spec = DeploymentSpec.fromXml("" + - " " + - " " + - " " + - " " + - " us-north-7" + - " " + - " " + - ""); + DeploymentSpec spec = DeploymentSpec.fromXml(""" + + + + + + us-north-7 + + + """); assertEquals(Optional.of("d-1-4-20"), spec.requireInstance("default").steps().get(0).zones().get(0).testerFlavor()); assertEquals(Optional.empty(), spec.requireInstance("default").steps().get(1).zones().get(0).testerFlavor()); assertEquals(Optional.of("d-2-8-50"), spec.requireInstance("default").steps().get(2).zones().get(0).testerFlavor()); @@ -1199,39 +1208,55 @@ public class DeploymentSpecTest { @Test public void noEndpoints() { - assertEquals(Collections.emptyList(), - DeploymentSpec.fromXml("" + - " " + - "").requireInstance("default").endpoints()); + DeploymentSpec spec = DeploymentSpec.fromXml(""" + + + + """); + assertEquals(Collections.emptyList(), spec.requireInstance("default").endpoints()); + assertEquals(ZoneEndpoint.defaultEndpoint, spec.zoneEndpoint(InstanceName.defaultName(), + defaultId(), + ClusterSpec.Id.from("cluster"))); } @Test public void emptyEndpoints() { - var spec = DeploymentSpec.fromXml("" + - " " + - " " + - " " + - ""); + var spec = DeploymentSpec.fromXml(""" + + + + + """); assertEquals(List.of(), spec.requireInstance("default").endpoints()); + assertEquals(ZoneEndpoint.defaultEndpoint, spec.zoneEndpoint(InstanceName.defaultName(), + defaultId(), + ClusterSpec.Id.from("cluster"))); } @Test public void someEndpoints() { - var spec = DeploymentSpec.fromXml("" + - "" + - " " + - " " + - " us-east" + - " " + - " " + - " " + - " us-east" + - " " + - " " + - " " + - " " + - " " + - ""); + var spec = DeploymentSpec.fromXml(""" + + + + us-east + + + + us-east + + + + + + + us-east + + + + + + """); assertEquals( List.of("foo", "nalle", "default"), @@ -1244,18 +1269,59 @@ public class DeploymentSpecTest { ); assertEquals(List.of(RegionName.from("us-east")), spec.requireInstance("default").endpoints().get(0).regions()); + + var zone = from(Environment.prod, RegionName.from("us-east")); + assertEquals(ZoneEndpoint.defaultEndpoint, + spec.zoneEndpoint(InstanceName.from("custom"), zone, ClusterSpec.Id.from("bax"))); + assertEquals(ZoneEndpoint.defaultEndpoint, + spec.zoneEndpoint(InstanceName.from("default"), defaultId(), ClusterSpec.Id.from("bax"))); + assertEquals(ZoneEndpoint.defaultEndpoint, + spec.zoneEndpoint(InstanceName.from("default"), zone, ClusterSpec.Id.from("bax"))); + + assertEquals(new ZoneEndpoint(false, true, List.of(new AllowedUrn(AccessType.awsPrivateLink, "barn"), + new AllowedUrn(AccessType.gcpServiceConnect, "nine"))), + spec.zoneEndpoint(InstanceName.from("default"), zone, ClusterSpec.Id.from("froz"))); } @Test public void invalidEndpoints() { - assertInvalidEndpoints(""); // Uppercase - assertInvalidEndpoints(""); // Starting with non-character - assertInvalidEndpoints(""); // Non-alphanumeric - assertInvalidEndpoints(""); - assertInvalidEndpoints(""); // Multiple consecutive dashes - assertInvalidEndpoints(""); // Trailing dash - assertInvalidEndpoints(""); // Too long - assertInvalidEndpoints(""); // Duplicate + assertInvalidEndpoints("", + "Endpoint ID must be all lowercase, alphanumeric, with no consecutive dashes, of length 1 to 12, and begin with a character; but got 'FOO'"); + assertInvalidEndpoints("", + "Endpoint ID must be all lowercase, alphanumeric, with no consecutive dashes, of length 1 to 12, and begin with a character; but got '123'"); + assertInvalidEndpoints("", + "Endpoint ID must be all lowercase, alphanumeric, with no consecutive dashes, of length 1 to 12, and begin with a character; but got 'foo!'"); + assertInvalidEndpoints("", + "Endpoint ID must be all lowercase, alphanumeric, with no consecutive dashes, of length 1 to 12, and begin with a character; but got 'foo.bar'"); + assertInvalidEndpoints("", + "Endpoint ID must be all lowercase, alphanumeric, with no consecutive dashes, of length 1 to 12, and begin with a character; but got 'foo--bar'"); + assertInvalidEndpoints("", + "Endpoint ID must be all lowercase, alphanumeric, with no consecutive dashes, of length 1 to 12, and begin with a character; but got 'foo-'"); + assertInvalidEndpoints("", + "Endpoint ID must be all lowercase, alphanumeric, with no consecutive dashes, of length 1 to 12, and begin with a character; but got 'foooooooooooo'"); + + assertInvalidEndpoints("", + "Endpoint ID 'foo' is specified multiple times"); + assertInvalidEndpoints("", + "Instance-level endpoint 'default': cannot declare 'id' with type 'zone' or 'private'"); + assertInvalidEndpoints("", + "Instance-level endpoint 'default': cannot declare 'id' with type 'zone' or 'private'"); + assertInvalidEndpoints("", + "Missing required attribute 'container-id' in 'endpoint'"); + assertInvalidEndpoints("", + "Missing required attribute 'container-id' in 'endpoint'"); + assertInvalidEndpoints("", + "Instance-level endpoint 'default': only endpoints of type 'private' can specify 'allow' children"); + assertInvalidEndpoints("", + "Instance-level endpoint 'default': only endpoints of type 'zone' can specify 'enabled'"); + assertInvalidEndpoints("", + "Multiple zone endpoints (for all regions) declared for container id 'qrs'"); + assertInvalidEndpoints("us" + + "us", + "Multiple private endpoints declared for container id 'qrs' in region 'us'"); + assertInvalidEndpoints("" + + "us", + "Zone endpoint for container id 'qrs' declared both with region 'us', and for all regions."); } @Test @@ -1271,25 +1337,44 @@ public class DeploymentSpecTest { @Test public void endpointDefaultRegions() { - var spec = DeploymentSpec.fromXml("" + - " " + - " " + - " us-east" + - " us-west" + - " " + - " " + - " " + - " us-east" + - " " + - " " + - " " + - " " + - " " + - ""); + var spec = DeploymentSpec.fromXml(""" + + + + us-east + us-west + + + + us-east + + + us-east + + + + + + + """); assertEquals(Set.of("us-east"), endpointRegions("foo", spec)); assertEquals(Set.of("us-east", "us-west"), endpointRegions("nalle", spec)); assertEquals(Set.of("us-east", "us-west"), endpointRegions("default", spec)); + assertEquals(new ZoneEndpoint(true, true, List.of()), + spec.zoneEndpoint(InstanceName.from("default"), from("prod", "us-east"), ClusterSpec.Id.from("bar"))); + assertEquals(new ZoneEndpoint(true, false, List.of()), + spec.zoneEndpoint(InstanceName.from("default"), from("prod", "us-west"), ClusterSpec.Id.from("bar"))); + assertEquals(new ZoneEndpoint(true, true, List.of()), + spec.zoneEndpoint(InstanceName.from("default"), from("prod", "us-east"), ClusterSpec.Id.from("quux"))); + assertEquals(new ZoneEndpoint(true, true, List.of()), + spec.zoneEndpoint(InstanceName.from("default"), from("prod", "us-west"), ClusterSpec.Id.from("quux"))); + assertEquals(new HashSet<>() {{ add(null); add(from("prod", "us-east")); }}, + spec.requireInstance("default").zoneEndpoints().get(ClusterSpec.Id.from("bar")).keySet()); + assertEquals(new HashSet<>() {{ add(null); }}, + spec.requireInstance("default").zoneEndpoints().get(ClusterSpec.Id.from("quux")).keySet()); + assertEquals(Set.of(ClusterSpec.Id.from("bar"), ClusterSpec.Id.from("quux")), + spec.requireInstance("default").zoneEndpoints().keySet()); } @Test @@ -1302,14 +1387,16 @@ public class DeploymentSpecTest { us-west - + %s """; - assertInvalid(String.format(xmlForm, "region='us-east'", "us-east"), "Instance-level endpoint 'foo': invalid 'region' attribute"); - assertInvalid(String.format(xmlForm, "", "us-east"), "Instance-level endpoint 'foo': invalid element 'instance'"); + assertInvalid(String.format(xmlForm, "id='foo' region='us-east'", "us-east"), "Instance-level endpoint 'foo': invalid 'region' attribute"); + assertInvalid(String.format(xmlForm, "id='foo'", "us-east"), "Instance-level endpoint 'foo': invalid element 'instance'"); + assertInvalid(String.format(xmlForm, "type='zone'", "us-east"), "Instance-level endpoint 'default': invalid element 'instance'"); + assertInvalid(String.format(xmlForm, "type='private'", "us-east"), "Instance-level endpoint 'default': invalid element 'instance'"); } @Test @@ -1343,6 +1430,73 @@ public class DeploymentSpecTest { assertInvalid(String.format(xmlForm, "region='us-west-1'", "weight='foo'", "", "main", ""), "Application-level endpoint 'foo': invalid weight value 'foo'"); assertInvalid(String.format(xmlForm, "region='us-west-1'", "weight='1'", "", "main", "us-east-3"), "Application-level endpoint 'foo': invalid element 'region'"); assertInvalid(String.format(xmlForm, "region='us-west-1'", "weight='0'", "", "main", ""), "Application-level endpoint 'foo': sum of all weights must be positive, got 0"); + assertInvalid(String.format(xmlForm, "type='zone'", "weight='1'", "", "main", ""), "Endpoints at application level cannot be of type 'zone'"); + assertInvalid(String.format(xmlForm, "type='private'", "weight='1'", "", "main", ""), "Endpoints at application level cannot be of type 'private'"); + } + + @Test + public void cannotTargetDisabledEndpoints() { + assertEquals("Instance-level endpoint 'default': all eligible zone endpoints have 'enabled' set to 'false'", + assertThrows(IllegalArgumentException.class, + () -> DeploymentSpec.fromXml(""" + + + + us + eu + + + + + + + + """)) + .getMessage()); + + assertEquals("Instance-level endpoint 'default': targets zone endpoint in 'us' with 'enabled' set to 'false'", + assertThrows(IllegalArgumentException.class, + () -> DeploymentSpec.fromXml(""" + + + + us + eu + + + + us + + + + + + """)) + .getMessage()); + + assertEquals("Application-level endpoint 'default': targets 'us' in 'default', but its zone endpoint has 'enabled' set to 'false'", + assertThrows(IllegalArgumentException.class, + () -> DeploymentSpec.fromXml(""" + + + + us + eu + + + + us + + + + + + default + + + + """)) + .getMessage()); } @Test @@ -1648,11 +1802,11 @@ public class DeploymentSpecTest { } } - private static void assertInvalidEndpoints(String endpointsBody) { - try { - endpointIds(endpointsBody); - fail("Expected exception for input '" + endpointsBody + "'"); - } catch (IllegalArgumentException ignored) {} + private static void assertInvalidEndpoints(String endpointsBody, String error) { + assertEquals(error, + assertThrows(IllegalArgumentException.class, + () -> endpointIds(endpointsBody)) + .getMessage()); } private static Set endpointRegions(String endpointId, DeploymentSpec spec) { diff --git a/config-model-api/src/test/java/com/yahoo/config/application/api/DeploymentSpecWithoutInstanceTest.java b/config-model-api/src/test/java/com/yahoo/config/application/api/DeploymentSpecWithoutInstanceTest.java index 6ff5616a80f..38410cc5b37 100644 --- a/config-model-api/src/test/java/com/yahoo/config/application/api/DeploymentSpecWithoutInstanceTest.java +++ b/config-model-api/src/test/java/com/yahoo/config/application/api/DeploymentSpecWithoutInstanceTest.java @@ -3,8 +3,13 @@ package com.yahoo.config.application.api; import com.google.common.collect.ImmutableSet; import com.yahoo.config.provision.CloudAccount; +import com.yahoo.config.provision.ClusterSpec; import com.yahoo.config.provision.Environment; +import com.yahoo.config.provision.InstanceName; import com.yahoo.config.provision.RegionName; +import com.yahoo.config.provision.ZoneEndpoint; +import com.yahoo.config.provision.ZoneEndpoint.AllowedUrn; +import com.yahoo.config.provision.ZoneEndpoint.AccessType; import org.junit.Test; import java.io.StringReader; @@ -20,6 +25,8 @@ import java.util.stream.Collectors; import static com.yahoo.config.application.api.Notifications.Role.author; import static com.yahoo.config.application.api.Notifications.When.failing; import static com.yahoo.config.application.api.Notifications.When.failingCommit; +import static com.yahoo.config.provision.zone.ZoneId.defaultId; +import static com.yahoo.config.provision.zone.ZoneId.from; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertNotEquals; @@ -654,19 +661,26 @@ public class DeploymentSpecWithoutInstanceTest { @Test public void someEndpoints() { - var spec = DeploymentSpec.fromXml("" + - "" + - " " + - " us-east" + - " " + - " " + - " " + - " us-east" + - " " + - " " + - " " + - " " + - ""); + var spec = DeploymentSpec.fromXml(""" + + + us-east + + + + us-east + + + + + + + us-east + + + + + """); assertEquals( List.of("foo", "nalle", "default"), @@ -679,6 +693,18 @@ public class DeploymentSpecWithoutInstanceTest { ); assertEquals(List.of(RegionName.from("us-east")), spec.requireInstance("default").endpoints().get(0).regions()); + + var zone = from(Environment.prod, RegionName.from("us-east")); + assertEquals(ZoneEndpoint.defaultEndpoint, + spec.zoneEndpoint(InstanceName.from("custom"), zone, ClusterSpec.Id.from("bax"))); + assertEquals(ZoneEndpoint.defaultEndpoint, + spec.zoneEndpoint(InstanceName.from("default"), defaultId(), ClusterSpec.Id.from("bax"))); + assertEquals(ZoneEndpoint.defaultEndpoint, + spec.zoneEndpoint(InstanceName.from("default"), zone, ClusterSpec.Id.from("bax"))); + + assertEquals(new ZoneEndpoint(false, true, List.of(new AllowedUrn(AccessType.awsPrivateLink, "barn"), + new AllowedUrn(AccessType.gcpServiceConnect, "nine"))), + spec.zoneEndpoint(InstanceName.from("default"), zone, ClusterSpec.Id.from("froz"))); } @Test diff --git a/config-model/src/main/java/com/yahoo/config/model/ConfigModelRepo.java b/config-model/src/main/java/com/yahoo/config/model/ConfigModelRepo.java index 95d97bc9e87..756646beddb 100644 --- a/config-model/src/main/java/com/yahoo/config/model/ConfigModelRepo.java +++ b/config-model/src/main/java/com/yahoo/config/model/ConfigModelRepo.java @@ -80,7 +80,7 @@ public class ConfigModelRepo implements ConfigModelRepoAdder, Serializable, Iter ConfigModelRegistry configModelRegistry) throws IOException { Element userServicesElement = getServicesFromApp(deployState.getApplicationPackage()); readConfigModels(root, userServicesElement, deployState, vespaModel, configModelRegistry); - builder.postProc(deployState.getDeployLogger(), root, this); + builder.postProc(deployState, root, this); } private Element getServicesFromApp(ApplicationPackage applicationPackage) throws IOException { diff --git a/config-model/src/main/java/com/yahoo/config/model/deploy/TestProperties.java b/config-model/src/main/java/com/yahoo/config/model/deploy/TestProperties.java index 7cb0672699f..49194a5d1bb 100644 --- a/config-model/src/main/java/com/yahoo/config/model/deploy/TestProperties.java +++ b/config-model/src/main/java/com/yahoo/config/model/deploy/TestProperties.java @@ -37,7 +37,7 @@ public class TestProperties implements ModelContext.Properties, ModelContext.Fea private ApplicationId applicationId = ApplicationId.defaultId(); private List configServerSpecs = Collections.emptyList(); private boolean hostedVespa = false; - private Zone zone; + private Zone zone = Zone.defaultZone(); private final Set endpoints = Collections.emptySet(); private boolean useDedicatedNodeForLogserver = false; private double defaultTermwiseLimit = 1.0; diff --git a/config-model/src/main/java/com/yahoo/vespa/model/builder/VespaModelBuilder.java b/config-model/src/main/java/com/yahoo/vespa/model/builder/VespaModelBuilder.java index 2cf32f1e8ff..421e3a2902c 100644 --- a/config-model/src/main/java/com/yahoo/vespa/model/builder/VespaModelBuilder.java +++ b/config-model/src/main/java/com/yahoo/vespa/model/builder/VespaModelBuilder.java @@ -22,6 +22,6 @@ public abstract class VespaModelBuilder { * @param producerRoot the root producer. * @param configModelRepo a {@link com.yahoo.config.model.ConfigModelRepo instance} */ - public abstract void postProc(DeployLogger deployLogger, AbstractConfigProducer producerRoot, ConfigModelRepo configModelRepo); + public abstract void postProc(DeployState deployState, AbstractConfigProducer producerRoot, ConfigModelRepo configModelRepo); } diff --git a/config-model/src/main/java/com/yahoo/vespa/model/builder/xml/dom/NodesSpecification.java b/config-model/src/main/java/com/yahoo/vespa/model/builder/xml/dom/NodesSpecification.java index a31e3fcce71..aac968f9038 100644 --- a/config-model/src/main/java/com/yahoo/vespa/model/builder/xml/dom/NodesSpecification.java +++ b/config-model/src/main/java/com/yahoo/vespa/model/builder/xml/dom/NodesSpecification.java @@ -4,6 +4,7 @@ package com.yahoo.vespa.model.builder.xml.dom; import com.yahoo.collections.Pair; import com.yahoo.component.Version; import com.yahoo.config.application.api.DeployLogger; +import com.yahoo.config.provision.ZoneEndpoint; import com.yahoo.config.model.ConfigModelContext; import com.yahoo.config.provision.Capacity; import com.yahoo.config.provision.CloudAccount; @@ -11,7 +12,6 @@ import com.yahoo.config.provision.ClusterMembership; import com.yahoo.config.provision.ClusterResources; import com.yahoo.config.provision.ClusterSpec; import com.yahoo.config.provision.DockerImage; -import com.yahoo.config.provision.LoadBalancerSettings; import com.yahoo.config.provision.NodeResources; import com.yahoo.text.XML; import com.yahoo.vespa.model.HostResource; @@ -256,13 +256,13 @@ public class NodesSpecification { ClusterSpec.Id clusterId, DeployLogger logger, boolean stateful) { - return provision(hostSystem, clusterType, clusterId, LoadBalancerSettings.empty, logger, stateful); + return provision(hostSystem, clusterType, clusterId, ZoneEndpoint.defaultEndpoint, logger, stateful); } public Map provision(HostSystem hostSystem, ClusterSpec.Type clusterType, ClusterSpec.Id clusterId, - LoadBalancerSettings loadBalancerSettings, + ZoneEndpoint zoneEndpoint, DeployLogger logger, boolean stateful) { if (combinedId.isPresent()) @@ -272,7 +272,7 @@ public class NodesSpecification { .exclusive(exclusive) .combinedId(combinedId.map(ClusterSpec.Id::from)) .dockerImageRepository(dockerImageRepo) - .loadBalancerSettings(loadBalancerSettings) + .loadBalancerSettings(zoneEndpoint) .stateful(stateful) .build(); return hostSystem.allocateHosts(cluster, Capacity.from(min, max, required, canFail, cloudAccount), logger); diff --git a/config-model/src/main/java/com/yahoo/vespa/model/builder/xml/dom/VespaDomBuilder.java b/config-model/src/main/java/com/yahoo/vespa/model/builder/xml/dom/VespaDomBuilder.java index cb3c43074fc..e4e56dcaaca 100644 --- a/config-model/src/main/java/com/yahoo/vespa/model/builder/xml/dom/VespaDomBuilder.java +++ b/config-model/src/main/java/com/yahoo/vespa/model/builder/xml/dom/VespaDomBuilder.java @@ -1,6 +1,7 @@ // Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. package com.yahoo.vespa.model.builder.xml.dom; +import ai.vespa.validation.Validation; import com.yahoo.config.application.api.DeployLogger; import com.yahoo.config.model.ApplicationConfigProducerRoot; import com.yahoo.config.model.ConfigModelRepo; @@ -22,8 +23,15 @@ import com.yahoo.vespa.model.container.docproc.ContainerDocproc; import com.yahoo.vespa.model.content.Content; import com.yahoo.vespa.model.search.SearchCluster; import org.w3c.dom.Element; + +import java.util.HashSet; +import java.util.LinkedHashMap; +import java.util.LinkedHashSet; +import java.util.Map; +import java.util.Set; import java.util.logging.Level; import java.util.logging.Logger; +import java.util.regex.Pattern; /** * Builds Vespa model components using the w3c dom api @@ -43,10 +51,12 @@ public class VespaDomBuilder extends VespaModelBuilder { public static final String VESPAMALLOC = "vespamalloc"; // Intended for vespa engineers public static final String VESPAMALLOC_DEBUG = "vespamalloc-debug"; // Intended for vespa engineers public static final String VESPAMALLOC_DEBUG_STACKTRACE = "vespamalloc-debug-stacktrace"; // Intended for vespa engineers - private static final String CPU_SOCKET_ATTRIB_NAME = "cpu-socket"; public static final String CPU_SOCKET_AFFINITY_ATTRIB_NAME = "cpu-socket-affinity"; public static final String Allocated_MEMORY_ATTRIB_NAME = "allocated-memory"; + private static final String CPU_SOCKET_ATTRIB_NAME = "cpu-socket"; + private static final Pattern clusterPattern = Pattern.compile("([a-z0-9]|[a-z0-9][a-z0-9_-]{0,61}[a-z0-9])"); + public static final Logger log = Logger.getLogger(VespaDomBuilder.class.getPackage().toString()); @@ -232,13 +242,14 @@ public class VespaDomBuilder extends VespaModelBuilder { * @param root root config producer * @param configModelRepo a {@link ConfigModelRepo} */ - public void postProc(DeployLogger deployLogger, AbstractConfigProducer root, ConfigModelRepo configModelRepo) { + public void postProc(DeployState deployState, AbstractConfigProducer root, ConfigModelRepo configModelRepo) { setContentSearchClusterIndexes(configModelRepo); createDocprocMBusServersAndClients(configModelRepo); + if (deployState.isHosted()) validateContainerClusterIds(configModelRepo); } private void createDocprocMBusServersAndClients(ConfigModelRepo pc) { - for (ContainerCluster cluster: ContainerModel.containerClusters(pc)) { + for (ContainerCluster cluster: ContainerModel.containerClusters(pc)) { addServerAndClientsForChains(cluster.getDocproc()); } } @@ -248,6 +259,19 @@ public class VespaDomBuilder extends VespaModelBuilder { docproc.getChains().addServersAndClientsForChains(); } + private void validateContainerClusterIds(ConfigModelRepo pc) { + Map normalizedClusterIds = new LinkedHashMap<>(); + for (ContainerCluster cluster: ContainerModel.containerClusters(pc)) { + if (cluster.getHttp() == null) continue; + String name = cluster.getName(); + Validation.requireMatch(name, "container cluster name", clusterPattern); + String clashing = normalizedClusterIds.put(name.replaceAll("_", "-"), name); + if (clashing != null) throw new IllegalArgumentException("container clusters '" + clashing + "' and '" + name + + "' have clashing endpoint names, when '_' is replaced " + + "with '-' to form valid domain names"); + } + } + /** * For some reason, search clusters need to be enumerated. * @param configModelRepo a {@link ConfigModelRepo} diff --git a/config-model/src/main/java/com/yahoo/vespa/model/container/xml/ContainerModelBuilder.java b/config-model/src/main/java/com/yahoo/vespa/model/container/xml/ContainerModelBuilder.java index d0a03be2869..4c7bad575d2 100644 --- a/config-model/src/main/java/com/yahoo/vespa/model/container/xml/ContainerModelBuilder.java +++ b/config-model/src/main/java/com/yahoo/vespa/model/container/xml/ContainerModelBuilder.java @@ -11,6 +11,8 @@ import com.yahoo.config.application.api.ApplicationPackage; import com.yahoo.config.application.api.DeployLogger; import com.yahoo.config.application.api.DeploymentInstanceSpec; import com.yahoo.config.application.api.DeploymentSpec; +import com.yahoo.config.provision.ZoneEndpoint; +import com.yahoo.config.application.api.xml.DeploymentSpecXmlReader; import com.yahoo.config.model.ConfigModelContext; import com.yahoo.config.model.api.ApplicationClusterEndpoint; import com.yahoo.config.model.api.ConfigServerSpec; @@ -29,10 +31,11 @@ import com.yahoo.config.provision.ClusterMembership; import com.yahoo.config.provision.ClusterResources; import com.yahoo.config.provision.ClusterSpec; import com.yahoo.config.provision.HostName; -import com.yahoo.config.provision.LoadBalancerSettings; +import com.yahoo.config.provision.InstanceName; import com.yahoo.config.provision.NodeResources; import com.yahoo.config.provision.NodeType; import com.yahoo.config.provision.Zone; +import com.yahoo.config.provision.zone.ZoneId; import com.yahoo.container.bundle.BundleInstantiationSpecification; import com.yahoo.container.logging.FileConnectionLog; import com.yahoo.io.IOUtils; @@ -847,11 +850,14 @@ public class ContainerModelBuilder extends ConfigModelBuilder { } } - private LoadBalancerSettings loadBalancerSettings(Element loadBalancerElement) { - List allowedUrnElements = XML.getChildren(XML.getChild(loadBalancerElement, "private-access"), - "allow-urn") - .stream().map(XML::getValue).toList(); - return new LoadBalancerSettings(allowedUrnElements); + private ZoneEndpoint zoneEndpoint(ConfigModelContext context, ClusterSpec.Id cluster) { + InstanceName instance = context.properties().applicationId().instance(); + ZoneId zone = ZoneId.from(context.properties().zone().environment(), + context.properties().zone().region()); + DeploymentSpec spec = context.getApplicationPackage().getDeployment() + .map(new DeploymentSpecXmlReader(false)::read) + .orElse(DeploymentSpec.empty); + return spec.zoneEndpoint(instance, zone, cluster); } private static Map getEnvironmentVariables(Element environmentVariables) { @@ -940,11 +946,12 @@ public class ContainerModelBuilder extends ConfigModelBuilder { private List createNodesFromNodeCount(ApplicationContainerCluster cluster, Element containerElement, Element nodesElement, ConfigModelContext context) { NodesSpecification nodesSpecification = NodesSpecification.from(new ModelElement(nodesElement), context); - LoadBalancerSettings loadBalancerSettings = loadBalancerSettings(XML.getChild(containerElement, "load-balancer")); + ClusterSpec.Id clusterId = ClusterSpec.Id.from(cluster.name()); + ZoneEndpoint zoneEndpoint = zoneEndpoint(context, clusterId); Map hosts = nodesSpecification.provision(cluster.getRoot().hostSystem(), ClusterSpec.Type.container, - ClusterSpec.Id.from(cluster.getName()), - loadBalancerSettings, + clusterId, + zoneEndpoint, log, getZooKeeper(containerElement) != null); return createNodesFromHosts(hosts, cluster, context.getDeployState()); diff --git a/config-model/src/main/resources/schema/containercluster.rnc b/config-model/src/main/resources/schema/containercluster.rnc index 933ec528c42..81455084ad2 100644 --- a/config-model/src/main/resources/schema/containercluster.rnc +++ b/config-model/src/main/resources/schema/containercluster.rnc @@ -6,8 +6,7 @@ ContainerCluster = element container { ContainerServices & DocumentBinding* & NodesOfContainerCluster? & - ClientAuthorize? & - LoadBalancer? + ClientAuthorize? } ContainerServices = @@ -312,16 +311,6 @@ NodesOfContainerCluster = element nodes { ) } -LoadBalancer = element load-balancer { - element private-access { - element allow-urn { - xsd:string - }* - }? -} - - - #DOCUMENT BINDINGS: DocumentBinding = element document { diff --git a/config-model/src/main/resources/schema/deployment.rnc b/config-model/src/main/resources/schema/deployment.rnc index 444f66a92ab..d63b8885a57 100644 --- a/config-model/src/main/resources/schema/deployment.rnc +++ b/config-model/src/main/resources/schema/deployment.rnc @@ -150,12 +150,21 @@ EndpointInstance = element instance { text } +AllowedUrn = element allow { + attribute with { xsd:string } & + attribute arn { xsd:string }? & + attribute project { xsd:string }? +} + Endpoint = element endpoint { attribute id { xsd:string }? & attribute container-id { xsd:string } & attribute region { xsd:string }? & + attribute type { xsd:string }? & + attribute enabled { xsd:boolean }? & EndpointRegion* & - EndpointInstance* + EndpointInstance* & + AllowedUrn* } Endpoints = element endpoints { diff --git a/config-model/src/test/java/com/yahoo/vespa/model/container/xml/ContainerModelBuilderTest.java b/config-model/src/test/java/com/yahoo/vespa/model/container/xml/ContainerModelBuilderTest.java index addf4dffde2..f0c39ecc920 100644 --- a/config-model/src/test/java/com/yahoo/vespa/model/container/xml/ContainerModelBuilderTest.java +++ b/config-model/src/test/java/com/yahoo/vespa/model/container/xml/ContainerModelBuilderTest.java @@ -22,6 +22,9 @@ import com.yahoo.config.provision.Environment; import com.yahoo.config.provision.Flavor; import com.yahoo.config.provision.RegionName; import com.yahoo.config.provision.Zone; +import com.yahoo.config.provision.ZoneEndpoint; +import com.yahoo.config.provision.ZoneEndpoint.AccessType; +import com.yahoo.config.provision.ZoneEndpoint.AllowedUrn; import com.yahoo.config.provisioning.FlavorsConfig; import com.yahoo.container.ComponentsConfig; import com.yahoo.container.QrConfig; @@ -57,7 +60,6 @@ import java.util.Map; import java.util.Set; import java.util.function.Function; import java.util.logging.Level; -import java.util.stream.Collectors; import static com.yahoo.config.model.test.TestUtil.joinLines; import static com.yahoo.test.LinePatternMatcher.containsLineWithPattern; @@ -178,6 +180,63 @@ public class ContainerModelBuilderTest extends ContainerModelBuilderTestBase { createModel(root, cluster1Elem, cluster2Elem); } + @Test + void container_cluster_with_invalid_name_throws_exception_when_hosted() throws IOException, SAXException { + String servicesXml = """ + + + + + + """; + + assertEquals("container cluster name must match '([a-z0-9]|[a-z0-9][a-z0-9_-]{0,61}[a-z0-9])', but got: 'C-1'", + assertThrows(IllegalArgumentException.class, + () -> + new VespaModel(new NullConfigModelRegistry(), new DeployState.Builder() + .modelHostProvisioner(new InMemoryProvisioner(4, false)) + .applicationPackage(new MockApplicationPackage.Builder().withServices(servicesXml).build()) + .properties(new TestProperties().setHostedVespa(true)) + .build())) + .getMessage()); + + new VespaModel(new NullConfigModelRegistry(), new DeployState.Builder() + .modelHostProvisioner(new InMemoryProvisioner(4, false)) + .applicationPackage(new MockApplicationPackage.Builder().withServices(servicesXml).build()) + .properties(new TestProperties().setHostedVespa(false)) + .build()); + } + + @Test + void two_clusters_with_clashing_cluster_names_throws_exception_when_hosted() throws IOException, SAXException { + String servicesXml = """ + + + + + + + + + """; + + assertEquals("container clusters 'c-1' and 'c_1' have clashing endpoint names, when '_' is replaced with '-' to form valid domain names", + assertThrows(IllegalArgumentException.class, + () -> + new VespaModel(new NullConfigModelRegistry(), new DeployState.Builder() + .modelHostProvisioner(new InMemoryProvisioner(4, false)) + .applicationPackage(new MockApplicationPackage.Builder().withServices(servicesXml).build()) + .properties(new TestProperties().setHostedVespa(true)) + .build())) + .getMessage()); + + new VespaModel(new NullConfigModelRegistry(), new DeployState.Builder() + .modelHostProvisioner(new InMemoryProvisioner(4, false)) + .applicationPackage(new MockApplicationPackage.Builder().withServices(servicesXml).build()) + .properties(new TestProperties().setHostedVespa(false)) + .build()); + } + @Test void two_clusters_without_explicit_port_throws_exception() { Element cluster1Elem = DomBuilderTest.parse( @@ -198,53 +257,96 @@ public class ContainerModelBuilderTest extends ContainerModelBuilderTestBase { @Test void load_balancers_can_be_set() throws IOException, SAXException { - // No load-balancer or nodes elements - verifyAllowedUrns(""); + // No endpoints + verifyAllowedUrns("", Environment.prod, "eu", ZoneEndpoint.defaultEndpoint); - // No load-balancer element - verifyAllowedUrns(""); + // No non-default settings + verifyAllowedUrns(""" + + """, + Environment.prod, + "eu", + ZoneEndpoint.defaultEndpoint); - // No nodes element + // No allowed urns verifyAllowedUrns(""" - - - foo - bar - - - """); - - // Both load-balancer and nodes + + """, + Environment.prod, + "eu", + new ZoneEndpoint(true, true, List.of())); + + // Various settings + verifyAllowedUrns(""" + + + eu + + + + """, + Environment.prod, + "eu", + new ZoneEndpoint(false, true, List.of(new AllowedUrn(AccessType.awsPrivateLink, "barn"), + new AllowedUrn(AccessType.gcpServiceConnect, "nine")))); + + // Various settings, but wrong region verifyAllowedUrns(""" - - - foo - bar - - - + + + eu + + + """, - "foo", "bar"); + Environment.prod, + "us", + ZoneEndpoint.defaultEndpoint); + + // Various settings, but wrong environment + verifyAllowedUrns(""" + + + eu + + + + """, + Environment.dev, + "eu", + ZoneEndpoint.defaultEndpoint); } - private void verifyAllowedUrns(String containerXml, String... expectedAllowedUrns) throws IOException, SAXException { + private void verifyAllowedUrns(String endpointsTag, Environment environment, String region, ZoneEndpoint expected) throws IOException, SAXException { String servicesXml = """ - %s + - """.formatted(containerXml); - ApplicationPackage applicationPackage = new MockApplicationPackage.Builder().withServices(servicesXml).build(); + """; + String deploymentXml = """ + + + eu + + + %s + + + """.formatted(endpointsTag); + ApplicationPackage applicationPackage = new MockApplicationPackage.Builder().withServices(servicesXml).withDeploymentSpec(deploymentXml).build(); InMemoryProvisioner provisioner = new InMemoryProvisioner(true, false, "host1.yahoo.com", "host2.yahoo.com"); VespaModel model = new VespaModel(new NullConfigModelRegistry(), new DeployState.Builder() .modelHostProvisioner(provisioner) .provisioned(provisioner.startProvisionedRecording()) .applicationPackage(applicationPackage) - .properties(new TestProperties().setMultitenant(true).setHostedVespa(true)) + .properties(new TestProperties().setMultitenant(true) + .setHostedVespa(true) + .setZone(new Zone(environment, RegionName.from(region)))) .build()); assertEquals(2, model.hostSystem().getHosts().size()); assertEquals(1, provisioner.provisionedClusters().size()); - assertEquals(List.of(expectedAllowedUrns), - provisioner.provisionedClusters().iterator().next().loadBalancerSettings().allowedUrns()); + assertEquals(expected, + provisioner.provisionedClusters().iterator().next().zoneEndpoint()); } @Test diff --git a/config-provisioning/src/main/java/com/yahoo/config/provision/ClusterMembership.java b/config-provisioning/src/main/java/com/yahoo/config/provision/ClusterMembership.java index 213166447ca..9e8388b6442 100644 --- a/config-provisioning/src/main/java/com/yahoo/config/provision/ClusterMembership.java +++ b/config-provisioning/src/main/java/com/yahoo/config/provision/ClusterMembership.java @@ -20,7 +20,7 @@ public class ClusterMembership { private final String stringValue; private ClusterMembership(String stringValue, Version vespaVersion, Optional dockerImageRepo, - LoadBalancerSettings loadBalancerSettings) { + ZoneEndpoint zoneEndpoint) { String[] components = stringValue.split("/"); if (components.length < 4) throw new RuntimeException("Could not parse '" + stringValue + "' to a cluster membership. " + @@ -49,7 +49,7 @@ public class ClusterMembership { .exclusive(exclusive) .combinedId(combinedId.map(ClusterSpec.Id::from)) .dockerImageRepository(dockerImageRepo) - .loadBalancerSettings(loadBalancerSettings) + .loadBalancerSettings(zoneEndpoint) .stateful(stateful) .build(); this.index = Integer.parseInt(components[3]); @@ -125,12 +125,12 @@ public class ClusterMembership { public String toString() { return stringValue(); } public static ClusterMembership from(String stringValue, Version vespaVersion, Optional dockerImageRepo) { - return from(stringValue, vespaVersion, dockerImageRepo, LoadBalancerSettings.empty); + return from(stringValue, vespaVersion, dockerImageRepo, ZoneEndpoint.defaultEndpoint); } public static ClusterMembership from(String stringValue, Version vespaVersion, Optional dockerImageRepo, - LoadBalancerSettings loadBalancerSettings) { - return new ClusterMembership(stringValue, vespaVersion, dockerImageRepo, loadBalancerSettings); + ZoneEndpoint zoneEndpoint) { + return new ClusterMembership(stringValue, vespaVersion, dockerImageRepo, zoneEndpoint); } public static ClusterMembership from(ClusterSpec cluster, int index) { diff --git a/config-provisioning/src/main/java/com/yahoo/config/provision/ClusterSpec.java b/config-provisioning/src/main/java/com/yahoo/config/provision/ClusterSpec.java index 153b305dc01..196255a8342 100644 --- a/config-provisioning/src/main/java/com/yahoo/config/provision/ClusterSpec.java +++ b/config-provisioning/src/main/java/com/yahoo/config/provision/ClusterSpec.java @@ -24,12 +24,12 @@ public final class ClusterSpec { private final boolean exclusive; private final Optional combinedId; private final Optional dockerImageRepo; - private final LoadBalancerSettings loadBalancerSettings; + private final ZoneEndpoint zoneEndpoint; private final boolean stateful; private ClusterSpec(Type type, Id id, Optional groupId, Version vespaVersion, boolean exclusive, Optional combinedId, Optional dockerImageRepo, - LoadBalancerSettings loadBalancerSettings, boolean stateful) { + ZoneEndpoint zoneEndpoint, boolean stateful) { this.type = type; this.id = id; this.groupId = groupId; @@ -47,7 +47,7 @@ public final class ClusterSpec { if (type.isContent() && !stateful) { throw new IllegalArgumentException("Cluster of type " + type + " must be stateful"); } - this.loadBalancerSettings = Objects.requireNonNull(loadBalancerSettings); + this.zoneEndpoint = Objects.requireNonNull(zoneEndpoint); this.stateful = stateful; } @@ -63,8 +63,8 @@ public final class ClusterSpec { /** Returns the docker image (repository + vespa version) we want this cluster to run */ public Optional dockerImage() { return dockerImageRepo.map(repo -> repo.withTag(vespaVersion).asString()); } - /** Returns any additional load balancer settings for application container clusters. */ - public LoadBalancerSettings loadBalancerSettings() { return loadBalancerSettings; } + /** Returns any additional zone endpoint settings for application container clusters. */ + public ZoneEndpoint zoneEndpoint() { return zoneEndpoint; } /** Returns the version of Vespa that we want this cluster to run */ public Version vespaVersion() { return vespaVersion; } @@ -87,15 +87,15 @@ public final class ClusterSpec { public boolean isStateful() { return stateful; } public ClusterSpec with(Optional newGroup) { - return new ClusterSpec(type, id, newGroup, vespaVersion, exclusive, combinedId, dockerImageRepo, loadBalancerSettings, stateful); + return new ClusterSpec(type, id, newGroup, vespaVersion, exclusive, combinedId, dockerImageRepo, zoneEndpoint, stateful); } public ClusterSpec withExclusivity(boolean exclusive) { - return new ClusterSpec(type, id, groupId, vespaVersion, exclusive, combinedId, dockerImageRepo, loadBalancerSettings, stateful); + return new ClusterSpec(type, id, groupId, vespaVersion, exclusive, combinedId, dockerImageRepo, zoneEndpoint, stateful); } public ClusterSpec exclusive(boolean exclusive) { - return new ClusterSpec(type, id, groupId, vespaVersion, exclusive, combinedId, dockerImageRepo, loadBalancerSettings, stateful); + return new ClusterSpec(type, id, groupId, vespaVersion, exclusive, combinedId, dockerImageRepo, zoneEndpoint, stateful); } /** Creates a ClusterSpec when requesting a cluster */ @@ -119,7 +119,7 @@ public final class ClusterSpec { private Version vespaVersion; private boolean exclusive = false; private Optional combinedId = Optional.empty(); - private LoadBalancerSettings loadBalancerSettings = LoadBalancerSettings.empty; + private ZoneEndpoint zoneEndpoint = ZoneEndpoint.defaultEndpoint; private boolean stateful; private Builder(Type type, Id id, boolean specification) { @@ -135,7 +135,7 @@ public final class ClusterSpec { if (vespaVersion == null) throw new IllegalArgumentException("vespaVersion is required to be set when creating a ClusterSpec with specification()"); } else if (groupId.isPresent()) throw new IllegalArgumentException("groupId is not allowed to be set when creating a ClusterSpec with request()"); - return new ClusterSpec(type, id, groupId, vespaVersion, exclusive, combinedId, dockerImageRepo, loadBalancerSettings, stateful); + return new ClusterSpec(type, id, groupId, vespaVersion, exclusive, combinedId, dockerImageRepo, zoneEndpoint, stateful); } public Builder group(Group groupId) { @@ -168,8 +168,8 @@ public final class ClusterSpec { return this; } - public Builder loadBalancerSettings(LoadBalancerSettings loadBalancerSettings) { - this.loadBalancerSettings = loadBalancerSettings; + public Builder loadBalancerSettings(ZoneEndpoint zoneEndpoint) { + this.zoneEndpoint = zoneEndpoint; return this; } @@ -198,12 +198,12 @@ public final class ClusterSpec { vespaVersion.equals(that.vespaVersion) && combinedId.equals(that.combinedId) && dockerImageRepo.equals(that.dockerImageRepo) && - loadBalancerSettings.equals(that.loadBalancerSettings); + zoneEndpoint.equals(that.zoneEndpoint); } @Override public int hashCode() { - return Objects.hash(type, id, groupId, vespaVersion, exclusive, combinedId, dockerImageRepo, loadBalancerSettings, stateful); + return Objects.hash(type, id, groupId, vespaVersion, exclusive, combinedId, dockerImageRepo, zoneEndpoint, stateful); } /** diff --git a/config-provisioning/src/main/java/com/yahoo/config/provision/LoadBalancerSettings.java b/config-provisioning/src/main/java/com/yahoo/config/provision/LoadBalancerSettings.java deleted file mode 100644 index 723de25fa87..00000000000 --- a/config-provisioning/src/main/java/com/yahoo/config/provision/LoadBalancerSettings.java +++ /dev/null @@ -1,20 +0,0 @@ -package com.yahoo.config.provision; - -import java.util.List; - -/** - * Settings for a load balancer provisioned for an application container cluster. - * - * @author jonmv - */ -public record LoadBalancerSettings(List allowedUrns) { - - public static final LoadBalancerSettings empty = new LoadBalancerSettings(List.of()); - - public LoadBalancerSettings(List allowedUrns) { - this.allowedUrns = List.copyOf(allowedUrns); - } - - public boolean isEmpty() { return allowedUrns.isEmpty(); } - -} diff --git a/config-provisioning/src/main/java/com/yahoo/config/provision/ZoneEndpoint.java b/config-provisioning/src/main/java/com/yahoo/config/provision/ZoneEndpoint.java new file mode 100644 index 00000000000..10e22f8df06 --- /dev/null +++ b/config-provisioning/src/main/java/com/yahoo/config/provision/ZoneEndpoint.java @@ -0,0 +1,132 @@ +package com.yahoo.config.provision; + +import ai.vespa.validation.Validation; + +import java.util.List; +import java.util.Objects; + +/** + * Settings for a zone endpoint of a deployment. + * + * TODO: Fix isEmpty + * Inline empty and constructor + * + * @author jonmv + */ +public class ZoneEndpoint { + + public static final ZoneEndpoint defaultEndpoint = new ZoneEndpoint(true, false, List.of()); + + private final boolean isPublicEndpoint; + private final boolean isPrivateEndpoint; + private final List allowedUrns; + + public ZoneEndpoint(List allowedUrns) { + this(true, true, allowedUrns.stream().map(arn -> new AllowedUrn(AccessType.awsPrivateLink, arn)).toList()); + } + + public ZoneEndpoint(boolean isPublicEndpoint, boolean isPrivateEndpoint, List allowedUrns) { + if ( ! allowedUrns.isEmpty() && ! isPrivateEndpoint) + throw new IllegalArgumentException("cannot list allowed urns, without also enabling private visibility"); + this.isPublicEndpoint = isPublicEndpoint; + this.isPrivateEndpoint = isPrivateEndpoint; + this.allowedUrns = List.copyOf(allowedUrns); + } + + /** Whether this has an endpoint which is visible from the public internet. */ + public boolean isPublicEndpoint() { + return isPublicEndpoint; + } + + /** Whether this has an endpoint which is visible through private DNS of the cloud. */ + public boolean isPrivateEndpoint() { + return isPrivateEndpoint; + } + + /** List of allowed URNs, for specified private access types. */ + public List allowedUrns() { + return allowedUrns; + } + + /** List of URNs for the given access type. */ + public List allowedUrnsWith(AccessType type) { + return allowedUrns.stream().filter(urn -> urn.type == type).map(AllowedUrn::urn).toList(); + } + + public boolean isDefault() { + return equals(defaultEndpoint); + } + + @Override + public boolean equals(Object o) { + if (this == o) return true; + if (o == null || getClass() != o.getClass()) return false; + ZoneEndpoint that = (ZoneEndpoint) o; + return isPublicEndpoint == that.isPublicEndpoint && isPrivateEndpoint == that.isPrivateEndpoint && allowedUrns.equals(that.allowedUrns); + } + + @Override + public int hashCode() { + return Objects.hash(isPublicEndpoint, isPrivateEndpoint, allowedUrns); + } + + @Override + public String toString() { + return "ZoneEndpoint{" + + "isPublicEndpoint=" + isPublicEndpoint + + ", isPrivateEndpoint=" + isPrivateEndpoint + + ", allowedUrns=" + allowedUrns + + '}'; + } + + public enum AccessType { + awsPrivateLink, + gcpServiceConnect, + } + + /** A URN allowed to access this (private) endpoint, through a {@link AccessType} method. */ + public static class AllowedUrn { + + private final AccessType type; + private final String urn; + + public AllowedUrn(AccessType type, String urn) { + this.type = Objects.requireNonNull(type); + this.urn = Validation.requireNonBlank(urn, "URN"); + } + + /** Type of private connection. */ + public AccessType type() { + return type; + } + + /** URN allowed to access this private endpoint. */ + public String urn() { + return urn; + } + + @Override + public boolean equals(Object o) { + if (this == o) return true; + if (o == null || getClass() != o.getClass()) return false; + AllowedUrn that = (AllowedUrn) o; + return type == that.type && urn.equals(that.urn); + } + + @Override + public int hashCode() { + return Objects.hash(type, urn); + } + + @Override + public String toString() { + return "'" + urn + "' through '" + + switch (type) { + case awsPrivateLink -> "aws-private-link"; + case gcpServiceConnect -> "gcp-service-connect"; + } + "'"; + } + + } + +} diff --git a/config-provisioning/src/main/java/com/yahoo/config/provision/serialization/AllocatedHostsSerializer.java b/config-provisioning/src/main/java/com/yahoo/config/provision/serialization/AllocatedHostsSerializer.java index 01bb0ca45ff..64e8a7feb94 100644 --- a/config-provisioning/src/main/java/com/yahoo/config/provision/serialization/AllocatedHostsSerializer.java +++ b/config-provisioning/src/main/java/com/yahoo/config/provision/serialization/AllocatedHostsSerializer.java @@ -6,8 +6,10 @@ import com.yahoo.config.provision.AllocatedHosts; import com.yahoo.config.provision.ClusterMembership; import com.yahoo.config.provision.DockerImage; import com.yahoo.config.provision.HostSpec; -import com.yahoo.config.provision.LoadBalancerSettings; import com.yahoo.config.provision.NodeResources; +import com.yahoo.config.provision.ZoneEndpoint; +import com.yahoo.config.provision.ZoneEndpoint.AllowedUrn; +import com.yahoo.config.provision.ZoneEndpoint.AccessType; import com.yahoo.slime.ArrayTraverser; import com.yahoo.slime.Cursor; import com.yahoo.slime.Inspector; @@ -40,8 +42,12 @@ public class AllocatedHostsSerializer { private static final String hostSpecKey = "hostSpec"; private static final String hostSpecHostNameKey = "hostName"; private static final String hostSpecMembershipKey = "membership"; - private static final String loadBalancerSettingsKey = "loadBalancerSettings"; - private static final String allowedUrnsKey = "allowedUrns"; + private static final String loadBalancerSettingsKey = "zoneEndpoint"; + private static final String publicField = "public"; + private static final String privateField = "private"; + private static final String allowedUrnsField = "allowedUrns"; + private static final String accessTypeField = "type"; + private static final String urnField = "urn"; private static final String realResourcesKey = "realResources"; private static final String advertisedResourcesKey = "advertisedResources"; @@ -85,9 +91,8 @@ public class AllocatedHostsSerializer { host.membership().ifPresent(membership -> { object.setString(hostSpecMembershipKey, membership.stringValue()); object.setString(hostSpecVespaVersionKey, membership.cluster().vespaVersion().toFullString()); - if ( ! membership.cluster().loadBalancerSettings().isEmpty()) - membership.cluster().loadBalancerSettings().allowedUrns() - .forEach(object.setObject(loadBalancerSettingsKey).setArray(allowedUrnsKey)::addString); + if ( ! membership.cluster().zoneEndpoint().isDefault()) + toSlime(object.setObject(loadBalancerSettingsKey), membership.cluster().zoneEndpoint()); membership.cluster().dockerImageRepo().ifPresent(repo -> object.setString(hostSpecDockerImageRepoKey, repo.untagged())); }); toSlime(host.realResources(), object.setObject(realResourcesKey)); @@ -222,13 +227,41 @@ public class AllocatedHostsSerializer { object.field(hostSpecDockerImageRepoKey).valid() ? Optional.of(DockerImage.fromString(object.field(hostSpecDockerImageRepoKey).asString())) : Optional.empty(), - object.field(loadBalancerSettingsKey).valid() - ? new LoadBalancerSettings(SlimeUtils.entriesStream(object.field(loadBalancerSettingsKey).field(allowedUrnsKey)) - .map(Inspector::asString) - .toList()) - : LoadBalancerSettings.empty); + zoneEndpoint(object.field(loadBalancerSettingsKey))); } + private static void toSlime(Cursor settingsObject, ZoneEndpoint settings) { + settingsObject.setBool(publicField, settings.isPublicEndpoint()); + settingsObject.setBool(privateField, settings.isPrivateEndpoint()); + if (settings.isPrivateEndpoint()) { + Cursor allowedUrnsArray = settingsObject.setArray(allowedUrnsField); + for (AllowedUrn urn : settings.allowedUrns()) { + Cursor urnObject = allowedUrnsArray.addObject(); + urnObject.setString(urnField, urn.urn()); + urnObject.setString(accessTypeField, + switch (urn.type()) { + case awsPrivateLink -> "awsPrivateLink"; + case gcpServiceConnect -> "gcpServiceConnect"; + }); + } + } + } + + private static ZoneEndpoint zoneEndpoint(Inspector settingsObject) { + if ( ! settingsObject.valid()) return ZoneEndpoint.defaultEndpoint; + return new ZoneEndpoint(settingsObject.field(publicField).asBool(), + settingsObject.field(privateField).asBool(), + SlimeUtils.entriesStream(settingsObject.field(allowedUrnsField)) + .map(urnObject -> new AllowedUrn(switch (urnObject.field(accessTypeField).asString()) { + case "awsPrivateLink" -> AccessType.awsPrivateLink; + case "gcpServiceConnect" -> AccessType.gcpServiceConnect; + default -> throw new IllegalArgumentException("unknown service access type in '" + urnObject + "'"); + }, + urnObject.field(urnField).asString())) + .toList()); + } + + private static Optional optionalString(Inspector inspector) { if ( ! inspector.valid()) return Optional.empty(); return Optional.of(inspector.asString()); diff --git a/config-provisioning/src/test/java/com/yahoo/config/provision/serialization/AllocatedHostsSerializerTest.java b/config-provisioning/src/test/java/com/yahoo/config/provision/serialization/AllocatedHostsSerializerTest.java index bcb3b8cd4aa..3404d7ed55e 100644 --- a/config-provisioning/src/test/java/com/yahoo/config/provision/serialization/AllocatedHostsSerializerTest.java +++ b/config-provisioning/src/test/java/com/yahoo/config/provision/serialization/AllocatedHostsSerializerTest.java @@ -6,9 +6,9 @@ import com.yahoo.config.provision.AllocatedHosts; import com.yahoo.config.provision.ClusterMembership; import com.yahoo.config.provision.DockerImage; import com.yahoo.config.provision.HostSpec; -import com.yahoo.config.provision.LoadBalancerSettings; import com.yahoo.config.provision.NetworkPorts; import com.yahoo.config.provision.NodeResources; +import com.yahoo.config.provision.ZoneEndpoint; import org.junit.jupiter.api.Test; import java.io.IOException; @@ -20,7 +20,6 @@ import java.util.Set; import static com.yahoo.config.provision.serialization.AllocatedHostsSerializer.fromJson; import static com.yahoo.config.provision.serialization.AllocatedHostsSerializer.toJson; import static org.junit.jupiter.api.Assertions.assertEquals; -import static org.junit.jupiter.api.Assertions.fail; /** * @author bratseth @@ -69,7 +68,7 @@ public class AllocatedHostsSerializerTest { bigSlowDiskSpeedNode, anyDiskSpeedNode, ClusterMembership.from("container/test/0/0", Version.fromString("6.73.1"), - Optional.empty(), new LoadBalancerSettings(List.of("burn"))), + Optional.empty(), new ZoneEndpoint(List.of("burn"))), Optional.empty(), Optional.empty(), Optional.empty())); diff --git a/configserver/src/test/apps/hosted/services.xml b/configserver/src/test/apps/hosted/services.xml index 22bccee9f5a..f1435d8cc4f 100644 --- a/configserver/src/test/apps/hosted/services.xml +++ b/configserver/src/test/apps/hosted/services.xml @@ -15,11 +15,6 @@ - - - burn - - diff --git a/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/configserver/LoadBalancer.java b/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/configserver/LoadBalancer.java index a4e26fbe7b3..26330f11d65 100644 --- a/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/configserver/LoadBalancer.java +++ b/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/configserver/LoadBalancer.java @@ -5,6 +5,7 @@ import ai.vespa.http.DomainName; import com.yahoo.config.provision.ApplicationId; import com.yahoo.config.provision.CloudAccount; import com.yahoo.config.provision.ClusterSpec; +import com.yahoo.config.provision.ZoneEndpoint.AllowedUrn; import java.util.List; import java.util.Objects; @@ -84,6 +85,6 @@ public class LoadBalancer { unknown } - public record PrivateServiceInfo(String id, List allowedUrns) { } + public record PrivateServiceInfo(String id, List allowedUrns) { } } diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/ApplicationController.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/ApplicationController.java index 4159b099387..2693fdcbd7c 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/ApplicationController.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/ApplicationController.java @@ -946,7 +946,7 @@ public class ApplicationController { // Either the user is member of the domain admin role, or is given the "launch" privilege on the service. Optional athenzUser = getUser(deployer); if (athenzUser.isPresent()) { - // We only need to validate the root and instance in deployment.xml. Not possible to add dev or perf tags to deployment.xml + // We only need to validate the root and instance in deployment.xml. Dev/perf entries are found at the instance level as well. var zone = zoneId.orElseThrow(() -> new IllegalArgumentException("Unable to evaluate access, no zone provided in deployment")); var serviceToLaunch = instanceName .flatMap(instance -> applicationPackage.deploymentSpec().instance(instance)) @@ -954,7 +954,7 @@ public class ApplicationController { .or(() -> applicationPackage.deploymentSpec().athenzService()) .map(service -> new AthenzService(identityDomain.get(), service.value())); - if(serviceToLaunch.isPresent()) { + if (serviceToLaunch.isPresent()) { if ( ! ((AthenzFacade) accessControl).canLaunch(athenzUser.get(), serviceToLaunch.get()) && // launch privilege ! ((AthenzFacade) accessControl).hasTenantAdminAccess(athenzUser.get(), identityDomain.get()) // tenant admin diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/application/pkg/ApplicationPackageValidator.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/application/pkg/ApplicationPackageValidator.java index bce9d44f8c6..842d99abe71 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/application/pkg/ApplicationPackageValidator.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/application/pkg/ApplicationPackageValidator.java @@ -4,26 +4,28 @@ package com.yahoo.vespa.hosted.controller.application.pkg; import com.yahoo.component.Version; import com.yahoo.config.application.api.DeploymentInstanceSpec; import com.yahoo.config.application.api.DeploymentSpec; +import com.yahoo.config.application.api.DeploymentSpec.DeclaredZone; import com.yahoo.config.application.api.Endpoint; import com.yahoo.config.application.api.Endpoint.Level; import com.yahoo.config.application.api.ValidationId; import com.yahoo.config.application.api.ValidationOverrides; import com.yahoo.config.provision.CloudAccount; import com.yahoo.config.provision.CloudName; +import com.yahoo.config.provision.ClusterSpec; import com.yahoo.config.provision.Environment; import com.yahoo.config.provision.InstanceName; import com.yahoo.config.provision.RegionName; +import com.yahoo.config.provision.ZoneEndpoint; +import com.yahoo.config.provision.ZoneEndpoint.AllowedUrn; import com.yahoo.config.provision.zone.ZoneApi; import com.yahoo.config.provision.zone.ZoneId; import com.yahoo.vespa.hosted.controller.Application; import com.yahoo.vespa.hosted.controller.Controller; -import com.yahoo.vespa.hosted.controller.api.identifiers.DeploymentId; import com.yahoo.vespa.hosted.controller.application.EndpointId; import com.yahoo.vespa.hosted.controller.versions.VespaVersion; import java.time.Instant; import java.util.ArrayList; -import java.util.EnumSet; import java.util.HashMap; import java.util.HashSet; import java.util.List; @@ -150,10 +152,10 @@ public class ApplicationPackageValidator { /** Verify endpoint configuration of given application package */ private void validateEndpointChange(Application application, ApplicationPackage applicationPackage, Instant instant) { - applicationPackage.deploymentSpec().instances().forEach(instance -> validateEndpointChange(application, - instance.name(), - applicationPackage, - instant)); + for (DeploymentInstanceSpec instance : applicationPackage.deploymentSpec().instances()) { + validateGlobalEndpointChanges(application, instance.name(), applicationPackage, instant); + validateZoneEndpointChanges(application, instance.name(), applicationPackage, instant); + } } /** Verify that compactable endpoint parts (instance name and endpoint ID) do not clash */ @@ -176,7 +178,7 @@ public class ApplicationPackageValidator { } /** Verify changes to endpoint configuration by comparing given application package to the existing one, if any */ - private void validateEndpointChange(Application application, InstanceName instanceName, ApplicationPackage applicationPackage, Instant instant) { + private void validateGlobalEndpointChanges(Application application, InstanceName instanceName, ApplicationPackage applicationPackage, Instant instant) { var validationId = ValidationId.globalEndpointChange; if (applicationPackage.validationOverrides().allows(validationId, instant)) return; @@ -200,6 +202,43 @@ public class ApplicationPackageValidator { ". " + ValidationOverrides.toAllowMessage(validationId)); } + /** Verify changes to endpoint configuration by comparing given application package to the existing one, if any */ + private void validateZoneEndpointChanges(Application application, InstanceName instance, ApplicationPackage applicationPackage, Instant now) { + ValidationId validationId = ValidationId.zoneEndpointChange; + if (applicationPackage.validationOverrides().allows(validationId, now)) return;; + + String prefix = validationId + ": application '" + application.id() + + (instance.isDefault() ? "" : "." + instance.value()) + "' "; + DeploymentInstanceSpec spec = applicationPackage.deploymentSpec().requireInstance(instance); + for (DeclaredZone zone : spec.zones()) { + if (zone.environment() == Environment.prod) { + Map newEndpoints = spec.zoneEndpoints(ZoneId.from(zone.environment(), zone.region().get())); + application.deploymentSpec().instance(instance) // If old spec has this instance ... + .filter(oldSpec -> oldSpec.concerns(zone.environment(), zone.region())) // ... and deploys to this zone ... + .map(oldSpec -> oldSpec.zoneEndpoints(ZoneId.from(zone.environment(), zone.region().get()))) + .ifPresent(oldEndpoints -> { // ... then we compare the endpoints present in both. + oldEndpoints.forEach((cluster, oldEndpoint) -> { + ZoneEndpoint newEndpoint = newEndpoints.getOrDefault(cluster, ZoneEndpoint.defaultEndpoint); + if ( ! newEndpoint.allowedUrns().containsAll(oldEndpoint.allowedUrns())) + throw new IllegalArgumentException(prefix + "allows access to cluster '" + cluster.value() + + "' in '" + zone.region().get().value() + "' to " + + oldEndpoint.allowedUrns().stream().map(AllowedUrn::toString).collect(joining(", ", "[", "]")) + + ", but does not include all these in the new deployment spec. " + + "Deploying with the new settings will allow access to " + + (newEndpoint.allowedUrns().isEmpty() ? "no one" : newEndpoint.allowedUrns().stream().map(AllowedUrn::toString).collect(joining(", ", "[", "]")))); + }); + newEndpoints.forEach((cluster, newEndpoint) -> { + ZoneEndpoint oldEndpoint = oldEndpoints.getOrDefault(cluster, ZoneEndpoint.defaultEndpoint); + if (oldEndpoint.isPublicEndpoint() && ! newEndpoint.isPublicEndpoint()) + throw new IllegalArgumentException(prefix + "has a public endpoint for cluster '" + cluster.value() + + "' in '" + zone.region().get().value() + "', but the new deployment spec " + + "disables this"); + }); + }); + } + } + } + /** Returns whether newEndpoints contains all destinations in endpoints */ private static boolean containsAllDestinationsOf(List endpoints, List newEndpoints) { var containsAllRegions = true; diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/JobController.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/JobController.java index 674d71001e3..73c64be3e47 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/JobController.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/JobController.java @@ -619,7 +619,7 @@ public class JobController { submission.applicationPackage().deploymentSpec().majorVersion().ifPresent(explicitMajor -> { if ( ! controller.readVersionStatus().isOnCurrentMajor(new Version(explicitMajor))) controller.notificationsDb().setNotification(NotificationSource.from(id), Type.submission, Notification.Level.warning, - "Vespa " + explicitMajor + " will soon be end of life, upgrade to Vespa " + (explicitMajor + 1) + " now: " + + "Vespa " + explicitMajor + " will soon reach end of life, upgrade to Vespa " + (explicitMajor + 1) + " now: " + "https://cloud.vespa.ai/en/vespa" + (explicitMajor + 1) + "-release-notes.html"); // ∠( ᐛ 」∠)＿ }); } 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 c9ad34b5082..3f68be611eb 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 @@ -23,6 +23,7 @@ import com.yahoo.config.provision.HostName; import com.yahoo.config.provision.InstanceName; import com.yahoo.config.provision.NodeResources; import com.yahoo.config.provision.TenantName; +import com.yahoo.config.provision.ZoneEndpoint.AllowedUrn; import com.yahoo.config.provision.zone.RoutingMethod; import com.yahoo.config.provision.zone.ZoneId; import com.yahoo.container.handler.metrics.JsonResponse; @@ -1978,7 +1979,15 @@ public class ApplicationApiHandler extends AuditLoggingRequestHandler { lbObject.setString("cluster", lb.cluster().value()); lb.service().ifPresent(service -> { lbObject.setString("serviceId", service.id()); // Really the "serviceName", but this is what the user needs >_< - service.allowedUrns().forEach(lbObject.setArray("allowedUrns")::addString); + Cursor urnsArray = lbObject.setArray("allowedUrns"); + for (AllowedUrn urn : service.allowedUrns()) { + Cursor urnObject = urnsArray.addObject(); + urnObject.setString("type", switch (urn.type()) { + case awsPrivateLink -> "aws-private-link"; + case gcpServiceConnect -> "gcp-service-connect"; + }); + urnObject.setString("urn", urn.urn()); + } Cursor endpointsArray = lbObject.setArray("endpoints"); controller.serviceRegistry().vpcEndpointService() .getConnections(new ClusterId(id, lb.cluster()), 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 95b81dffaed..db96c265363 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 @@ -1118,6 +1118,94 @@ public class ControllerTest { } @Test + void testZoneEndpointChanges() { + DeploymentContext app = tester.newDeploymentContext(); + // Set up app with default settings. + app.submit(ApplicationPackageBuilder.fromDeploymentXml(""" + + + us-east-3 + + """)); + + assertEquals("zone-endpoint-change: application 'tenant.application' has a public endpoint for cluster 'foo' in 'us-east-3', but the new deployment spec disables this", + assertThrows(IllegalArgumentException.class, + () -> app.submit(ApplicationPackageBuilder.fromDeploymentXml(""" + + + us-east-3 + + + + + """))) + .getMessage()); + + // Disabling endpoints is OK with override. + app.submit(ApplicationPackageBuilder.fromDeploymentXml(""" + + + us-east-3 + + + + + """, + ValidationId.zoneEndpointChange)); + + // Enabling endpoints again is OK, as is adding a private endpoint with some URN. + app.submit(ApplicationPackageBuilder.fromDeploymentXml(""" + + + us-east-3 + + + + + + + """, + ValidationId.zoneEndpointChange)); + + // Changing URNs is guarded. + assertEquals("zone-endpoint-change: application 'tenant.application' allows access to cluster 'foo' in 'us-east-3' to " + + "['yarn' through 'aws-private-link'], but does not include all these in the new deployment spec. " + + "Deploying with the new settings will allow access to ['yarn' through 'gcp-service-connect']", + assertThrows(IllegalArgumentException.class, + () -> app.submit(ApplicationPackageBuilder.fromDeploymentXml(""" + + + us-east-3 + + + + + + + """))) + .getMessage()); + + // Changing cluster, effectively removing old URNs, is also guarded. + assertEquals("zone-endpoint-change: application 'tenant.application' allows access to cluster 'foo' in 'us-east-3' to " + + "['yarn' through 'aws-private-link'], but does not include all these in the new deployment spec. " + + "Deploying with the new settings will allow access to no one", + assertThrows(IllegalArgumentException.class, + () -> app.submit(ApplicationPackageBuilder.fromDeploymentXml(""" + + + us-east-3 + + + + + + + """))) + .getMessage()); + } + + + @Test void testReadableApplications() { var db = new MockCuratorDb(tester.controller().system()); var tester = new DeploymentTester(new ControllerTester(db)); diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/integration/ConfigServerMock.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/integration/ConfigServerMock.java index 93b804805c3..6f859ff3d15 100644 --- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/integration/ConfigServerMock.java +++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/integration/ConfigServerMock.java @@ -16,6 +16,8 @@ import com.yahoo.config.provision.Environment; import com.yahoo.config.provision.HostName; import com.yahoo.config.provision.NodeResources; import com.yahoo.config.provision.NodeType; +import com.yahoo.config.provision.ZoneEndpoint.AllowedUrn; +import com.yahoo.config.provision.ZoneEndpoint.AccessType; import com.yahoo.config.provision.zone.ZoneId; import com.yahoo.vespa.flags.json.FlagData; import com.yahoo.vespa.hosted.controller.api.application.v4.model.ClusterMetrics; @@ -417,7 +419,7 @@ public class ConfigServerMock extends AbstractComponent implements ConfigServer LoadBalancer.State.active, Optional.of("dns-zone-1"), Optional.empty(), - Optional.of(new PrivateServiceInfo("service", List.of("arne")))))); + Optional.of(new PrivateServiceInfo("service", List.of(new AllowedUrn(AccessType.awsPrivateLink, "arne"))))))); } Application application = applications.get(id); diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/ApplicationApiTest.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/ApplicationApiTest.java index d40485ff5c0..be405c7b876 100644 --- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/ApplicationApiTest.java +++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/ApplicationApiTest.java @@ -701,7 +701,7 @@ public class ApplicationApiTest extends ControllerContainerTest { tester.assertResponse(request("/application/v4/tenant/tenant1/application/application1/instance/instance1/environment/prod/region/us-central-1/private-service", GET) .userIdentity(USER_ID), """ - {"loadBalancers":[{"cluster":"default","serviceId":"service","allowedUrns":["arne"],"endpoints":[{"endpointId":"endpoint-1","state":"available"}]}]}"""); + {"loadBalancers":[{"cluster":"default","serviceId":"service","allowedUrns":[{"type":"aws-private-link","urn":"arne"}],"endpoints":[{"endpointId":"endpoint-1","state":"available"}]}]}"""); // GET service/state/v1 tester.assertResponse(request("/application/v4/tenant/tenant1/application/application1/instance/instance1/environment/prod/region/us-central-1/service/storagenode/host.com/state/v1/?foo=bar", GET) diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/lb/LoadBalancerInstance.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/lb/LoadBalancerInstance.java index 33c9edf694d..2856a38075b 100644 --- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/lb/LoadBalancerInstance.java +++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/lb/LoadBalancerInstance.java @@ -4,7 +4,7 @@ package com.yahoo.vespa.hosted.provision.lb; import ai.vespa.http.DomainName; import com.google.common.collect.ImmutableSortedSet; import com.yahoo.config.provision.CloudAccount; -import com.yahoo.config.provision.LoadBalancerSettings; +import com.yahoo.config.provision.ZoneEndpoint; import java.util.Objects; import java.util.Optional; @@ -24,12 +24,12 @@ public class LoadBalancerInstance { private final Set ports; private final Set networks; private final Set reals; - private final LoadBalancerSettings settings; + private final ZoneEndpoint settings; private final Optional serviceId; private final CloudAccount cloudAccount; public LoadBalancerInstance(Optional hostname, Optional ipAddress, Optional dnsZone, - Set ports, Set networks, Set reals, LoadBalancerSettings settings, + Set ports, Set networks, Set reals, ZoneEndpoint settings, Optional serviceId, CloudAccount cloudAccount) { this.hostname = Objects.requireNonNull(hostname, "hostname must be non-null"); this.ipAddress = Objects.requireNonNull(ipAddress, "ip must be non-null"); @@ -78,7 +78,7 @@ public class LoadBalancerInstance { } /** Static user-configured settings of this load balancer */ - public LoadBalancerSettings settings() { + public ZoneEndpoint settings() { return settings; } diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/lb/LoadBalancerServiceMock.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/lb/LoadBalancerServiceMock.java index e0dd41f9008..c19aebcda6e 100644 --- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/lb/LoadBalancerServiceMock.java +++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/lb/LoadBalancerServiceMock.java @@ -4,8 +4,8 @@ package com.yahoo.vespa.hosted.provision.lb; import ai.vespa.http.DomainName; import com.google.common.collect.ImmutableSet; import com.yahoo.config.provision.ClusterSpec; -import com.yahoo.config.provision.LoadBalancerSettings; import com.yahoo.config.provision.NodeType; +import com.yahoo.config.provision.ZoneEndpoint; import java.util.Collections; import java.util.HashMap; @@ -62,7 +62,7 @@ public class LoadBalancerServiceMock implements LoadBalancerService { Collections.singleton(4443), ImmutableSet.of("10.2.3.0/24", "10.4.5.0/24"), spec.reals(), - spec.settings().orElse(LoadBalancerSettings.empty), + spec.settings().orElse(ZoneEndpoint.defaultEndpoint), spec.settings().map(__ -> PrivateServiceId.of("service")), spec.cloudAccount()); instances.put(id, instance); diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/lb/LoadBalancerSpec.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/lb/LoadBalancerSpec.java index dca6d434330..e0ef6739542 100644 --- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/lb/LoadBalancerSpec.java +++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/lb/LoadBalancerSpec.java @@ -5,7 +5,7 @@ import com.google.common.collect.ImmutableSortedSet; import com.yahoo.config.provision.ApplicationId; import com.yahoo.config.provision.CloudAccount; import com.yahoo.config.provision.ClusterSpec; -import com.yahoo.config.provision.LoadBalancerSettings; +import com.yahoo.config.provision.ZoneEndpoint; import java.util.Objects; import java.util.Optional; @@ -21,11 +21,11 @@ public class LoadBalancerSpec { private final ApplicationId application; private final ClusterSpec.Id cluster; private final Set reals; - private final Optional settings; + private final Optional settings; private final CloudAccount cloudAccount; public LoadBalancerSpec(ApplicationId application, ClusterSpec.Id cluster, Set reals, - LoadBalancerSettings settings, CloudAccount cloudAccount) { + ZoneEndpoint settings, CloudAccount cloudAccount) { this.application = Objects.requireNonNull(application); this.cluster = Objects.requireNonNull(cluster); this.reals = ImmutableSortedSet.copyOf(Objects.requireNonNull(reals)); @@ -49,7 +49,7 @@ public class LoadBalancerSpec { } /** Static user-configured settings for this load balancer. */ - public Optional settings() { + public Optional settings() { return settings; } diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/lb/SharedLoadBalancerService.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/lb/SharedLoadBalancerService.java index c8fb1226b81..5dc099460a4 100644 --- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/lb/SharedLoadBalancerService.java +++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/lb/SharedLoadBalancerService.java @@ -3,8 +3,8 @@ package com.yahoo.vespa.hosted.provision.lb; import ai.vespa.http.DomainName; import com.yahoo.config.provision.ClusterSpec; -import com.yahoo.config.provision.LoadBalancerSettings; import com.yahoo.config.provision.NodeType; +import com.yahoo.config.provision.ZoneEndpoint; import java.util.Objects; import java.util.Optional; @@ -29,15 +29,15 @@ public class SharedLoadBalancerService implements LoadBalancerService { @Override public LoadBalancerInstance create(LoadBalancerSpec spec, boolean force) { - if (spec.settings().isPresent() && ! spec.settings().get().isEmpty()) - throw new IllegalArgumentException("custom load balancer settings are not supported with " + getClass()); + if (spec.settings().isPresent() && ! spec.settings().get().isDefault()) + throw new IllegalArgumentException("custom zone endpoint settings are not supported with " + getClass()); return new LoadBalancerInstance(Optional.of(DomainName.of(vipHostname)), Optional.empty(), Optional.empty(), Set.of(4443), Set.of(), spec.reals(), - spec.settings().orElse(LoadBalancerSettings.empty), + spec.settings().orElse(ZoneEndpoint.defaultEndpoint), Optional.empty(), spec.cloudAccount()); } diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/persistence/LoadBalancerSerializer.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/persistence/LoadBalancerSerializer.java index 3d352f5596b..6bac1dab3dd 100644 --- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/persistence/LoadBalancerSerializer.java +++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/persistence/LoadBalancerSerializer.java @@ -3,7 +3,9 @@ package com.yahoo.vespa.hosted.provision.persistence; import ai.vespa.http.DomainName; import com.yahoo.config.provision.CloudAccount; -import com.yahoo.config.provision.LoadBalancerSettings; +import com.yahoo.config.provision.ZoneEndpoint; +import com.yahoo.config.provision.ZoneEndpoint.AllowedUrn; +import com.yahoo.config.provision.ZoneEndpoint.AccessType; import com.yahoo.slime.ArrayTraverser; import com.yahoo.slime.Cursor; import com.yahoo.slime.Inspector; @@ -52,7 +54,11 @@ public class LoadBalancerSerializer { private static final String serviceIdField = "serviceId"; private static final String cloudAccountField = "cloudAccount"; private static final String settingsField = "settings"; + private static final String publicField = "public"; + private static final String privateField = "private"; private static final String allowedUrnsField = "allowedUrns"; + private static final String accessTypeField = "type"; + private static final String urnField = "urn"; public static byte[] toJson(LoadBalancer loadBalancer) { Slime slime = new Slime(); @@ -77,15 +83,13 @@ public class LoadBalancerSerializer { })); loadBalancer.instance() .map(LoadBalancerInstance::settings) - .filter(settings -> ! settings.isEmpty()) - .ifPresent(settings -> settings.allowedUrns().forEach(root.setObject(settingsField) - .setArray(allowedUrnsField)::addString)); + .ifPresent(settings -> toSlime(root.setObject(settingsField), settings)); loadBalancer.instance() .flatMap(LoadBalancerInstance::serviceId) .ifPresent(serviceId -> root.setString(serviceIdField, serviceId.value())); loadBalancer.instance() .map(LoadBalancerInstance::cloudAccount) - .filter(cloudAccount -> !cloudAccount.isUnspecified()) + .filter(cloudAccount -> ! cloudAccount.isUnspecified()) .ifPresent(cloudAccount -> root.setString(cloudAccountField, cloudAccount.value())); try { return SlimeUtils.toJsonBytes(slime); @@ -114,7 +118,7 @@ public class LoadBalancerSerializer { Optional hostname = optionalString(object.field(hostnameField), Function.identity()).filter(s -> !s.isEmpty()).map(DomainName::of); Optional ipAddress = optionalString(object.field(lbIpAddressField), Function.identity()).filter(s -> !s.isEmpty()); Optional dnsZone = optionalString(object.field(dnsZoneField), DnsZone::new); - LoadBalancerSettings settings = loadBalancerSettings(object.field(settingsField)); + ZoneEndpoint settings = zoneEndpoint(object.field(settingsField)); Optional serviceId = optionalString(object.field(serviceIdField), PrivateServiceId::of); CloudAccount cloudAccount = optionalString(object.field(cloudAccountField), CloudAccount::from).orElse(CloudAccount.empty); Optional instance = hostname.isEmpty() && ipAddress.isEmpty() ? Optional.empty() : @@ -126,11 +130,35 @@ public class LoadBalancerSerializer { Instant.ofEpochMilli(object.field(changedAtField).asLong())); } - private static LoadBalancerSettings loadBalancerSettings(Inspector settingsObject) { - if ( ! settingsObject.valid()) return LoadBalancerSettings.empty; - return new LoadBalancerSettings(SlimeUtils.entriesStream(settingsObject.field(allowedUrnsField)) - .map(Inspector::asString) - .toList()); + private static void toSlime(Cursor settingsObject, ZoneEndpoint settings) { + settingsObject.setBool(publicField, settings.isPublicEndpoint()); + settingsObject.setBool(privateField, settings.isPrivateEndpoint()); + if (settings.isPrivateEndpoint()) { + Cursor allowedUrnsArray = settingsObject.setArray(allowedUrnsField); + for (AllowedUrn urn : settings.allowedUrns()) { + Cursor urnObject = allowedUrnsArray.addObject(); + urnObject.setString(urnField, urn.urn()); + urnObject.setString(accessTypeField, + switch (urn.type()) { + case awsPrivateLink -> "awsPrivateLink"; + case gcpServiceConnect -> "gcpServiceConnect"; + }); + } + } + } + + private static ZoneEndpoint zoneEndpoint(Inspector settingsObject) { + if ( ! settingsObject.valid()) return ZoneEndpoint.defaultEndpoint; + return new ZoneEndpoint(settingsObject.field(publicField).asBool(), + settingsObject.field(privateField).asBool(), + SlimeUtils.entriesStream(settingsObject.field(allowedUrnsField)) + .map(urnObject -> new AllowedUrn(switch (urnObject.field(accessTypeField).asString()) { + case "awsPrivateLink" -> AccessType.awsPrivateLink; + case "gcpServiceConnect" -> AccessType.gcpServiceConnect; + default -> throw new IllegalArgumentException("unknown service access type in '" + urnObject + "'"); + }, + urnObject.field(urnField).asString())) + .toList()); } private static Optional optionalValue(Inspector field, Function fieldMapper) { diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/LoadBalancerProvisioner.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/LoadBalancerProvisioner.java index 3e8124d5309..92fdb1d2e52 100644 --- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/LoadBalancerProvisioner.java +++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/LoadBalancerProvisioner.java @@ -7,9 +7,9 @@ import com.yahoo.config.provision.ApplicationTransaction; import com.yahoo.config.provision.CloudAccount; import com.yahoo.config.provision.ClusterSpec; import com.yahoo.config.provision.HostName; -import com.yahoo.config.provision.LoadBalancerSettings; import com.yahoo.config.provision.NodeType; import com.yahoo.config.provision.TenantName; +import com.yahoo.config.provision.ZoneEndpoint; import com.yahoo.config.provision.exception.LoadBalancerServiceException; import com.yahoo.transaction.NestedTransaction; import com.yahoo.vespa.flags.BooleanFlag; @@ -108,11 +108,13 @@ public class LoadBalancerProvisioner { * Calling this when no load balancer has been prepared for given cluster is a no-op. */ public void activate(Set clusters, NodeList newActive, ApplicationTransaction transaction) { - Map activatingClusters = clusters.stream() - .collect(groupingBy(LoadBalancerProvisioner::effectiveId, - reducing(LoadBalancerSettings.empty, - ClusterSpec::loadBalancerSettings, - (o, n) -> o.isEmpty() ? n : o))); + Map activatingClusters = clusters.stream() + // .collect(Collectors.toMap(ClusterSpec::id, ClusterSpec::zoneEndpoint)); + // TODO: this dies with combined clusters Ü + .collect(groupingBy(LoadBalancerProvisioner::effectiveId, + reducing(ZoneEndpoint.defaultEndpoint, + ClusterSpec::zoneEndpoint, + (o, n) -> o.isDefault() ? n : o))); for (var cluster : loadBalancedClustersOf(newActive).entrySet()) { if ( ! activatingClusters.containsKey(cluster.getKey())) continue; @@ -209,7 +211,7 @@ public class LoadBalancerProvisioner { requireInstance(id, instance, cloudAccount); } - private void activate(ApplicationTransaction transaction, ClusterSpec.Id cluster, LoadBalancerSettings settings, NodeList nodes) { + private void activate(ApplicationTransaction transaction, ClusterSpec.Id cluster, ZoneEndpoint settings, NodeList nodes) { Instant now = nodeRepository.clock().instant(); LoadBalancerId id = new LoadBalancerId(transaction.application(), cluster); Optional loadBalancer = db.readLoadBalancer(id); @@ -226,7 +228,7 @@ public class LoadBalancerProvisioner { /** Provision or reconfigure a load balancer instance, if necessary */ private Optional provisionInstance(LoadBalancerId id, NodeList nodes, Optional currentLoadBalancer, - LoadBalancerSettings loadBalancerSettings, + ZoneEndpoint zoneEndpoint, CloudAccount cloudAccount) { boolean shouldDeactivateRouting = deactivateRouting.with(FetchVector.Dimension.APPLICATION_ID, id.application().serializedForm()) @@ -237,13 +239,13 @@ public class LoadBalancerProvisioner { } else { reals = realsOf(nodes); } - if (isUpToDate(currentLoadBalancer, reals, loadBalancerSettings)) + if (isUpToDate(currentLoadBalancer, reals, zoneEndpoint)) return currentLoadBalancer.get().instance(); log.log(Level.INFO, () -> "Provisioning instance for " + id + ", targeting: " + reals); try { // Override settings at activation, otherwise keep existing ones. - LoadBalancerSettings settings = loadBalancerSettings != null ? loadBalancerSettings - : currentLoadBalancer.flatMap(LoadBalancer::instance) + ZoneEndpoint settings = zoneEndpoint != null ? zoneEndpoint + : currentLoadBalancer.flatMap(LoadBalancer::instance) .map(LoadBalancerInstance::settings) .orElse(null); LoadBalancerInstance created = service.create(new LoadBalancerSpec(id.application(), id.cluster(), reals, settings, cloudAccount), @@ -306,11 +308,11 @@ public class LoadBalancerProvisioner { } /** Returns whether load balancer has given reals, and settings if specified*/ - private static boolean isUpToDate(Optional loadBalancer, Set reals, LoadBalancerSettings loadBalancerSettings) { + private static boolean isUpToDate(Optional loadBalancer, Set reals, ZoneEndpoint zoneEndpoint) { if (loadBalancer.isEmpty()) return false; if (loadBalancer.get().instance().isEmpty()) return false; return loadBalancer.get().instance().get().reals().equals(reals) - && (loadBalancerSettings == null || loadBalancer.get().instance().get().settings().equals(loadBalancerSettings)); + && (zoneEndpoint == null || loadBalancer.get().instance().get().settings().equals(zoneEndpoint)); } /** Returns whether to allow given load balancer to have no reals */ diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/restapi/LoadBalancersResponse.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/restapi/LoadBalancersResponse.java index fdf69b60690..15a799c06d8 100644 --- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/restapi/LoadBalancersResponse.java +++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/restapi/LoadBalancersResponse.java @@ -2,6 +2,7 @@ package com.yahoo.vespa.hosted.provision.restapi; import com.yahoo.config.provision.ApplicationId; +import com.yahoo.config.provision.ZoneEndpoint.AllowedUrn; import com.yahoo.container.jdisc.HttpRequest; import com.yahoo.restapi.SlimeJsonResponse; import com.yahoo.slime.Cursor; @@ -76,8 +77,17 @@ public class LoadBalancersResponse extends SlimeJsonResponse { }); }); lb.instance().ifPresent(instance -> { - if ( ! instance.settings().isEmpty()) - instance.settings().allowedUrns().forEach(lbObject.setObject("settings").setArray("allowedUrns")::addString); + if ( ! instance.settings().isDefault()) { + Cursor urnsArray = lbObject.setObject("settings").setArray("allowedUrns"); + for (AllowedUrn urn : instance.settings().allowedUrns()) { + Cursor urnObject = urnsArray.addObject(); + urnObject.setString("type", switch (urn.type()) { + case awsPrivateLink -> "aws-private-link"; + case gcpServiceConnect -> "gcp-service-connect"; + }); + urnObject.setString("urn", urn.urn()); + } + } instance.serviceId().ifPresent(serviceId -> lbObject.setString("serviceId", serviceId.value())); }); lb.instance() diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/testutils/MockNodeRepository.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/testutils/MockNodeRepository.java index 91c8f803429..92ffe9828c3 100644 --- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/testutils/MockNodeRepository.java +++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/testutils/MockNodeRepository.java @@ -14,12 +14,12 @@ import com.yahoo.config.provision.DockerImage; import com.yahoo.config.provision.Flavor; import com.yahoo.config.provision.HostSpec; import com.yahoo.config.provision.InstanceName; -import com.yahoo.config.provision.LoadBalancerSettings; import com.yahoo.config.provision.NodeFlavors; import com.yahoo.config.provision.NodeResources; import com.yahoo.config.provision.NodeType; import com.yahoo.config.provision.TenantName; import com.yahoo.config.provision.Zone; +import com.yahoo.config.provision.ZoneEndpoint; import com.yahoo.transaction.Mutex; import com.yahoo.transaction.NestedTransaction; import com.yahoo.vespa.curator.mock.MockCurator; @@ -189,7 +189,7 @@ public class MockNodeRepository extends NodeRepository { activate(provisioner.prepare(zoneApp, zoneCluster, Capacity.fromRequiredNodeType(NodeType.host), null), zoneApp, provisioner); ApplicationId app1Id = ApplicationId.from(TenantName.from("tenant1"), ApplicationName.from("application1"), InstanceName.from("instance1")); - ClusterSpec cluster1Id = ClusterSpec.request(ClusterSpec.Type.container, ClusterSpec.Id.from("id1")).vespaVersion("6.42").loadBalancerSettings(new LoadBalancerSettings(List.of("arne"))).build(); + ClusterSpec cluster1Id = ClusterSpec.request(ClusterSpec.Type.container, ClusterSpec.Id.from("id1")).vespaVersion("6.42").loadBalancerSettings(new ZoneEndpoint(List.of("arne"))).build(); activate(provisioner.prepare(app1Id, cluster1Id, Capacity.from(new ClusterResources(2, 1, new NodeResources(2, 8, 50, 1)), diff --git a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/lb/SharedLoadBalancerServiceTest.java b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/lb/SharedLoadBalancerServiceTest.java index 92c7ba7fe27..a646d26ea29 100644 --- a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/lb/SharedLoadBalancerServiceTest.java +++ b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/lb/SharedLoadBalancerServiceTest.java @@ -5,7 +5,7 @@ import com.yahoo.config.provision.ApplicationId; import com.yahoo.config.provision.CloudAccount; import com.yahoo.config.provision.ClusterSpec; import com.yahoo.config.provision.HostName; -import com.yahoo.config.provision.LoadBalancerSettings; +import com.yahoo.config.provision.ZoneEndpoint; import org.junit.Test; import java.util.Optional; @@ -29,7 +29,7 @@ public class SharedLoadBalancerServiceTest { @Test public void test_create_lb() { var lb = loadBalancerService.create(new LoadBalancerSpec(applicationId, clusterId, reals, - LoadBalancerSettings.empty, CloudAccount.empty), + ZoneEndpoint.defaultEndpoint, CloudAccount.empty), false); assertEquals(Optional.of(HostName.of("vip.example.com")), lb.hostname()); diff --git a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/persistence/LoadBalancerSerializerTest.java b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/persistence/LoadBalancerSerializerTest.java index d5722a59f3e..dee895b02d2 100644 --- a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/persistence/LoadBalancerSerializerTest.java +++ b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/persistence/LoadBalancerSerializerTest.java @@ -6,7 +6,7 @@ import com.google.common.collect.ImmutableSet; import com.yahoo.config.provision.ApplicationId; import com.yahoo.config.provision.CloudAccount; import com.yahoo.config.provision.ClusterSpec; -import com.yahoo.config.provision.LoadBalancerSettings; +import com.yahoo.config.provision.ZoneEndpoint; import com.yahoo.vespa.hosted.provision.lb.DnsZone; import com.yahoo.vespa.hosted.provision.lb.LoadBalancer; import com.yahoo.vespa.hosted.provision.lb.LoadBalancerId; @@ -46,7 +46,7 @@ public class LoadBalancerSerializerTest { new Real(DomainName.of("real-2"), "127.0.0.2", 4080)), - new LoadBalancerSettings(List.of("123")), + new ZoneEndpoint(List.of("123")), Optional.of(PrivateServiceId.of("foo")), CloudAccount.from("012345678912"))), LoadBalancer.State.active, diff --git a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/provisioning/LoadBalancerProvisionerTest.java b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/provisioning/LoadBalancerProvisionerTest.java index e32643860f5..3653e20d848 100644 --- a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/provisioning/LoadBalancerProvisionerTest.java +++ b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/provisioning/LoadBalancerProvisionerTest.java @@ -9,9 +9,9 @@ import com.yahoo.config.provision.CloudAccount; import com.yahoo.config.provision.ClusterResources; import com.yahoo.config.provision.ClusterSpec; import com.yahoo.config.provision.HostSpec; -import com.yahoo.config.provision.LoadBalancerSettings; import com.yahoo.config.provision.NodeResources; import com.yahoo.config.provision.NodeType; +import com.yahoo.config.provision.ZoneEndpoint; import com.yahoo.config.provision.exception.LoadBalancerServiceException; import com.yahoo.transaction.NestedTransaction; import com.yahoo.vespa.flags.InMemoryFlagSource; @@ -215,7 +215,7 @@ public class LoadBalancerProvisionerTest { public void provision_load_balancer_combined_cluster() { Supplier> lbs = () -> tester.nodeRepository().loadBalancers().list(app1).asList(); var combinedId = ClusterSpec.Id.from("container1"); - var nodes = prepare(app1, clusterRequest(ClusterSpec.Type.combined, ClusterSpec.Id.from("content1"), Optional.of(combinedId), LoadBalancerSettings.empty)); + var nodes = prepare(app1, clusterRequest(ClusterSpec.Type.combined, ClusterSpec.Id.from("content1"), Optional.of(combinedId), ZoneEndpoint.defaultEndpoint)); assertEquals(1, lbs.get().size()); assertEquals("Prepare provisions load balancer with reserved nodes", 2, lbs.get().get(0).instance().get().reals().size()); tester.activate(app1, nodes); @@ -320,10 +320,10 @@ public class LoadBalancerProvisionerTest { tester.activate(app1, prepare(app1, capacity, clusterRequest(ClusterSpec.Type.container, ClusterSpec.Id.from("c1")))); LoadBalancerList loadBalancers = tester.nodeRepository().loadBalancers().list(); assertEquals(1, loadBalancers.size()); - assertEquals(LoadBalancerSettings.empty, loadBalancers.first().get().instance().get().settings()); + assertEquals(ZoneEndpoint.defaultEndpoint, loadBalancers.first().get().instance().get().settings()); // Next deployment contains new settings - LoadBalancerSettings settings = new LoadBalancerSettings(List.of("alice", "bob")); + ZoneEndpoint settings = new ZoneEndpoint(List.of("alice", "bob")); tester.activate(app1, prepare(app1, capacity, clusterRequest(ClusterSpec.Type.container, ClusterSpec.Id.from("c1"), Optional.empty(), settings))); loadBalancers = tester.nodeRepository().loadBalancers().list(); assertEquals(1, loadBalancers.size()); @@ -430,10 +430,10 @@ public class LoadBalancerProvisionerTest { } private static ClusterSpec clusterRequest(ClusterSpec.Type type, ClusterSpec.Id id) { - return clusterRequest(type, id, Optional.empty(), LoadBalancerSettings.empty); + return clusterRequest(type, id, Optional.empty(), ZoneEndpoint.defaultEndpoint); } - private static ClusterSpec clusterRequest(ClusterSpec.Type type, ClusterSpec.Id id, Optional combinedId, LoadBalancerSettings settings) { + private static ClusterSpec clusterRequest(ClusterSpec.Type type, ClusterSpec.Id id, Optional combinedId, ZoneEndpoint settings) { return ClusterSpec.request(type, id).vespaVersion("6.42").combinedId(combinedId).loadBalancerSettings(settings).build(); } diff --git a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/provisioning/VirtualNodeProvisioningTest.java b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/provisioning/VirtualNodeProvisioningTest.java index 5143aa91f56..086df4d0c33 100644 --- a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/provisioning/VirtualNodeProvisioningTest.java +++ b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/provisioning/VirtualNodeProvisioningTest.java @@ -49,8 +49,8 @@ public class VirtualNodeProvisioningTest { private static final NodeResources resources1 = new NodeResources(4, 8, 100, 1); private static final NodeResources resources2 = new NodeResources(1, 4, 100, 1, NodeResources.DiskSpeed.fast, NodeResources.StorageType.local); - private static final ClusterSpec contentClusterSpec = ClusterSpec.request(ClusterSpec.Type.content, ClusterSpec.Id.from("myContent")).vespaVersion("6.42").build(); - private static final ClusterSpec containerClusterSpec = ClusterSpec.request(ClusterSpec.Type.container, ClusterSpec.Id.from("myContainer")).vespaVersion("6.42").build(); + private static final ClusterSpec contentClusterSpec = ClusterSpec.request(ClusterSpec.Type.content, ClusterSpec.Id.from("my-content")).vespaVersion("6.42").build(); + private static final ClusterSpec containerClusterSpec = ClusterSpec.request(ClusterSpec.Type.container, ClusterSpec.Id.from("my-container")).vespaVersion("6.42").build(); private final ApplicationId applicationId = ProvisioningTester.applicationId("test"); @@ -242,7 +242,7 @@ public class VirtualNodeProvisioningTest { Version wantedVespaVersion = Version.fromString("6.39"); int nodeCount = 7; List hosts = tester.prepare(application1, - ClusterSpec.request(ClusterSpec.Type.content, ClusterSpec.Id.from("myContent")).vespaVersion(wantedVespaVersion).build(), + ClusterSpec.request(ClusterSpec.Type.content, ClusterSpec.Id.from("my-content")).vespaVersion(wantedVespaVersion).build(), nodeCount, 1, resources2); tester.activate(application1, new HashSet<>(hosts)); @@ -253,7 +253,7 @@ public class VirtualNodeProvisioningTest { // Upgrade Vespa version on nodes Version upgradedWantedVespaVersion = Version.fromString("6.40"); List upgradedHosts = tester.prepare(application1, - ClusterSpec.request(ClusterSpec.Type.content, ClusterSpec.Id.from("myContent")).vespaVersion(upgradedWantedVespaVersion).build(), + ClusterSpec.request(ClusterSpec.Type.content, ClusterSpec.Id.from("my-content")).vespaVersion(upgradedWantedVespaVersion).build(), nodeCount, 1, resources2); tester.activate(application1, new HashSet<>(upgradedHosts)); NodeList upgradedNodes = tester.getNodes(application1, Node.State.active); @@ -275,7 +275,7 @@ public class VirtualNodeProvisioningTest { int nodeCount = 7; try { List nodes = tester.prepare(application1, - ClusterSpec.request(ClusterSpec.Type.content, ClusterSpec.Id.from("myContent")).vespaVersion(wantedVespaVersion).build(), + ClusterSpec.request(ClusterSpec.Type.content, ClusterSpec.Id.from("my-content")).vespaVersion(wantedVespaVersion).build(), nodeCount, 1, resources2); fail("Expected the allocation to fail due to parent hosts not being active yet"); } catch (NodeAllocationException expected) { } @@ -285,7 +285,7 @@ public class VirtualNodeProvisioningTest { // Try allocating tenants again List nodes = tester.prepare(application1, - ClusterSpec.request(ClusterSpec.Type.content, ClusterSpec.Id.from("myContent")).vespaVersion(wantedVespaVersion).build(), + ClusterSpec.request(ClusterSpec.Type.content, ClusterSpec.Id.from("my-content")).vespaVersion(wantedVespaVersion).build(), nodeCount, 1, resources2); tester.activate(application1, new HashSet<>(nodes)); @@ -309,14 +309,14 @@ public class VirtualNodeProvisioningTest { Version wantedVespaVersion = Version.fromString("6.39"); List nodes = tester.prepare(application2_1, - ClusterSpec.request(ClusterSpec.Type.container, ClusterSpec.Id.from("myContent")).vespaVersion(wantedVespaVersion).build(), + ClusterSpec.request(ClusterSpec.Type.container, ClusterSpec.Id.from("my-content")).vespaVersion(wantedVespaVersion).build(), 6, 1, resources); assertHostSpecParentReservation(nodes, Optional.empty(), tester); // We do not get nodes on hosts reserved to tenant1 tester.activate(application2_1, nodes); try { tester.prepare(application2_2, - ClusterSpec.request(ClusterSpec.Type.container, ClusterSpec.Id.from("myContent")).vespaVersion(wantedVespaVersion).build(), + ClusterSpec.request(ClusterSpec.Type.container, ClusterSpec.Id.from("my-content")).vespaVersion(wantedVespaVersion).build(), 5, 1, resources); fail("Expected exception"); } @@ -325,7 +325,7 @@ public class VirtualNodeProvisioningTest { } nodes = tester.prepare(application1_1, - ClusterSpec.request(ClusterSpec.Type.container, ClusterSpec.Id.from("myContent")).vespaVersion(wantedVespaVersion).build(), + ClusterSpec.request(ClusterSpec.Type.container, ClusterSpec.Id.from("my-content")).vespaVersion(wantedVespaVersion).build(), 10, 1, resources); assertHostSpecParentReservation(nodes, Optional.of(tenant1), tester); tester.activate(application1_1, nodes); @@ -346,14 +346,14 @@ public class VirtualNodeProvisioningTest { try { // No capacity for 'container' nodes tester.prepare(applicationId, - ClusterSpec.request(ClusterSpec.Type.container, ClusterSpec.Id.from("myContent")).vespaVersion(wantedVespaVersion).build(), + ClusterSpec.request(ClusterSpec.Type.container, ClusterSpec.Id.from("my-content")).vespaVersion(wantedVespaVersion).build(), 6, 1, resources); fail("Expected to fail node allocation"); } catch (NodeAllocationException ignored) { } // Same cluster, but content type is now 'content' List nodes = tester.prepare(applicationId, - ClusterSpec.request(ClusterSpec.Type.content, ClusterSpec.Id.from("myContent")).vespaVersion(wantedVespaVersion).build(), + ClusterSpec.request(ClusterSpec.Type.content, ClusterSpec.Id.from("my-content")).vespaVersion(wantedVespaVersion).build(), 6, 1, resources); tester.activate(applicationId, nodes); @@ -445,7 +445,7 @@ public class VirtualNodeProvisioningTest { assertEquals("No room for 3 nodes as 2 of 4 hosts are exclusive", "Could not satisfy request for 3 nodes with " + "[vcpu: 1.0, memory: 4.0 Gb, disk 100.0 Gb, bandwidth: 1.0 Gbps, architecture: x86_64] " + - "in tenant2.app2 container cluster 'myContainer' 6.39: " + + "in tenant2.app2 container cluster 'my-container' 6.39: " + "Node allocation failure on group 0: " + "Not enough suitable nodes available due to host exclusivity constraints", e.getMessage()); @@ -465,14 +465,14 @@ public class VirtualNodeProvisioningTest { tester.makeReadyChildren(1, resources2, "host2"); tester.prepare(application1, - ClusterSpec.request(ClusterSpec.Type.content, ClusterSpec.Id.from("myContent")).vespaVersion("6.42").build(), + ClusterSpec.request(ClusterSpec.Type.content, ClusterSpec.Id.from("my-content")).vespaVersion("6.42").build(), 2, 1, resources2.with(NodeResources.StorageType.remote)); } catch (NodeAllocationException e) { assertEquals("Could not satisfy request for 2 nodes with " + "[vcpu: 1.0, memory: 4.0 Gb, disk 100.0 Gb, bandwidth: 1.0 Gbps, storage type: remote, architecture: x86_64] " + - "in tenant.app1 content cluster 'myContent'" + + "in tenant.app1 content cluster 'my-content'" + " 6.42: Node allocation failure on group 0", e.getMessage()); } @@ -672,7 +672,7 @@ public class VirtualNodeProvisioningTest { private void prepareAndActivate(ApplicationId application, int nodeCount, boolean exclusive, NodeResources resources, ProvisioningTester tester) { Set hosts = new HashSet<>(tester.prepare(application, - ClusterSpec.request(ClusterSpec.Type.container, ClusterSpec.Id.from("myContainer")).vespaVersion("6.39").exclusive(exclusive).build(), + ClusterSpec.request(ClusterSpec.Type.container, ClusterSpec.Id.from("my-container")).vespaVersion("6.39").exclusive(exclusive).build(), Capacity.from(new ClusterResources(nodeCount, 1, resources), false, true))); tester.activate(application, hosts); } diff --git a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/restapi/responses/load-balancers.json b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/restapi/responses/load-balancers.json index bbccc72c7f9..becca98a913 100644 --- a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/restapi/responses/load-balancers.json +++ b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/restapi/responses/load-balancers.json @@ -30,7 +30,12 @@ } ], "settings": { - "allowedUrns": [ "arne" ] + "allowedUrns": [ + { + "type": "aws-private-link", + "urn": "arne" + } + ] }, "serviceId": "service" }, diff --git a/vespa-feed-client/src/test/java/ai/vespa/feed/client/impl/ApacheClusterTest.java b/vespa-feed-client/src/test/java/ai/vespa/feed/client/impl/ApacheClusterTest.java index f45f7f9d246..30ed8dcfdd4 100644 --- a/vespa-feed-client/src/test/java/ai/vespa/feed/client/impl/ApacheClusterTest.java +++ b/vespa-feed-client/src/test/java/ai/vespa/feed/client/impl/ApacheClusterTest.java @@ -48,9 +48,9 @@ class ApacheClusterTest { Map.of("name1", () -> "value1", "name2", () -> "value2"), "content".getBytes(UTF_8), - Duration.ofSeconds(5)), + Duration.ofSeconds(10)), vessel); - HttpResponse response = vessel.get(5, TimeUnit.SECONDS); + HttpResponse response = vessel.get(15, TimeUnit.SECONDS); assertEquals("{}", new String(response.body(), UTF_8)); assertEquals(200, response.code()); -- cgit v1.2.3