From f76a2f86a3dc8417eef6bdfa0a24f6ba24364e13 Mon Sep 17 00:00:00 2001 From: jonmv Date: Tue, 17 Jan 2023 16:30:36 +0100 Subject: Validate disabled regions are not targets --- .../config/application/api/DeploymentSpec.java | 4 ++ .../api/xml/DeploymentSpecXmlReader.java | 44 ++++++++++++++++------ 2 files changed, 37 insertions(+), 11 deletions(-) (limited to 'config-model-api/src/main/java') 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 7d3014c15a3..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 @@ -150,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'"); } } } 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 81c081fd17d..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 @@ -46,6 +46,7 @@ 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; @@ -57,6 +58,8 @@ import java.util.Set; import java.util.function.Function; import java.util.stream.Stream; +import static java.util.Comparator.comparingInt; + /** * @author bratseth */ @@ -149,7 +152,7 @@ public class DeploymentSpecXmlReader { steps.addAll(readNonInstanceSteps(child, new HashMap<>(), root)); // (No global service id here) } } - readEndpoints(root, Optional.empty(), steps, applicationEndpoints, Map.of()); + readEndpoints(root, Optional.empty(), steps, applicationEndpoints, Map.of()); } return new DeploymentSpec(steps, @@ -325,7 +328,9 @@ public class DeploymentSpecXmlReader { Endpoint.Level level = instance.isEmpty() ? Endpoint.Level.application : Endpoint.Level.instance; Map endpointsById = new LinkedHashMap<>(); Map>> endpointsByZone = new LinkedHashMap<>(); - for (var endpointElement : XML.getChildren(endpointsElement, endpointTag)) { + 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); @@ -392,8 +397,16 @@ public class DeploymentSpecXmlReader { 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"); - if ( ! regions.add(RegionName.from(region))) illegal(msgPrefix + "duplicate 'region' element: '" + region + "'"); + 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()) { @@ -409,13 +422,22 @@ public class DeploymentSpecXmlReader { } else { if (regions.isEmpty()) { - // No explicit targets given for instance level endpoint. Include all declared regions by default - steps.stream() - .filter(step -> step.concerns(Environment.prod)) - .flatMap(step -> step.zones().stream()) - .flatMap(zone -> zone.region().stream()) - .forEach(regions::add); + // 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)); } @@ -428,7 +450,7 @@ public class DeploymentSpecXmlReader { } endpointsById.put(endpoint.endpointId(), endpoint); } - } + }); endpoints.addAll(endpointsById.values()); validateAndConsolidate(endpointsByZone, zoneEndpoints); } -- cgit v1.2.3