Validate disabled regions are not targets

3 files changed, 104 insertions, 13 deletions

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<String, Endpoint> endpointsById = new LinkedHashMap<>();
         Map<String, Map<RegionName, List<ZoneEndpoint>>> 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<String> endpointId = stringAttribute("id", endpointElement);
             Optional<String> zoneEndpointType = getZoneEndpointType(endpointElement, level);
@@ -392,8 +397,16 @@ public class DeploymentSpecXmlReader {
                 Set<RegionName> 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<RegionName> 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);
     }
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 918239a646a..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
@@ -1348,7 +1348,7 @@ public class DeploymentSpecTest {
                                                    <endpoint id="foo" container-id="bar">
                                                       <region>us-east</region>
                                                    </endpoint>
-                                                   <endpoint container-id="bar" type='zone' enabled='false'>
+                                                   <endpoint container-id="bar" type='private'>
                                                       <region>us-east</region>
                                                    </endpoint>
                                                    <endpoint id="nalle" container-id="frosk" />
@@ -1361,7 +1361,7 @@ public class DeploymentSpecTest {
         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(false, false, List.of()),
+        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")));
@@ -1435,6 +1435,71 @@ public class DeploymentSpecTest {
     }
 
     @Test
+    public void cannotTargetDisabledEndpoints() {
+        assertEquals("Instance-level endpoint 'default': all eligible zone endpoints have 'enabled' set to 'false'",
+                     assertThrows(IllegalArgumentException.class,
+                                  () -> DeploymentSpec.fromXml("""
+                                                               <deployment>
+                                                                 <instance id="default">
+                                                                   <prod>
+                                                                     <region>us</region>
+                                                                     <region>eu</region>
+                                                                   </prod>
+                                                                   <endpoints>
+                                                                     <endpoint container-id='id' />
+                                                                     <endpoint type='zone' container-id='id' enabled='false' />
+                                                                   </endpoints>
+                                                                 </instance>
+                                                               </deployment>
+                                                               """))
+                             .getMessage());
+
+        assertEquals("Instance-level endpoint 'default': targets zone endpoint in 'us' with 'enabled' set to 'false'",
+                     assertThrows(IllegalArgumentException.class,
+                                  () -> DeploymentSpec.fromXml("""
+                                                               <deployment>
+                                                                 <instance id="default">
+                                                                   <prod>
+                                                                     <region>us</region>
+                                                                     <region>eu</region>
+                                                                   </prod>
+                                                                   <endpoints>
+                                                                     <endpoint container-id='id'>
+                                                                       <region>us</region>
+                                                                     </endpoint>
+                                                                     <endpoint type='zone' container-id='id' enabled='false' />
+                                                                   </endpoints>
+                                                                 </instance>
+                                                               </deployment>
+                                                               """))
+                             .getMessage());
+
+        assertEquals("Application-level endpoint 'default': targets 'us' in 'default', but its zone endpoint has 'enabled' set to 'false'",
+                     assertThrows(IllegalArgumentException.class,
+                                  () -> DeploymentSpec.fromXml("""
+                                                               <deployment>
+                                                                 <instance id="default">
+                                                                   <prod>
+                                                                     <region>us</region>
+                                                                     <region>eu</region>
+                                                                   </prod>
+                                                                   <endpoints>
+                                                                     <endpoint type='zone' container-id='id' enabled='false'>
+                                                                       <region>us</region>
+                                                                     </endpoint>
+                                                                   </endpoints>
+                                                                 </instance>
+                                                                 <endpoints>
+                                                                   <endpoint container-id='id' region='us'>
+                                                                     <instance weight='1'>default</instance>
+                                                                   </endpoint>
+                                                                 </endpoints>
+                                                               </deployment>
+                                                               """))
+                             .getMessage());
+    }
+
+    @Test
     public void applicationLevelEndpoint() {
         DeploymentSpec spec = DeploymentSpec.fromXml("""
                                                      <deployment>