Allow multiple regions and instances in application endpoints, part XML

3 files changed, 40 insertions, 27 deletions

diff --git a/config-model-api/src/main/java/com/yahoo/config/application/api/Endpoint.java b/config-model-api/src/main/java/com/yahoo/config/application/api/Endpoint.java
index 78bf9fc68c4..83106e75627 100644
--- a/config-model-api/src/main/java/com/yahoo/config/application/api/Endpoint.java
+++ b/config-model-api/src/main/java/com/yahoo/config/application/api/Endpoint.java
@@ -52,12 +52,10 @@ public class Endpoint {
                 Target a = targets.get(i);
                 Target b = targets.get(j);
                 if (level == Level.application) {
-                    // - All instance names must be distinct
-                    // - All region names must be equal
-                    if (a.instance().equals(b.instance())) throw new IllegalArgumentException("Instance '" + a.instance +
-                                                                                              "' declared multiple times, but allowed at most once");
-                    if (!a.region().equals(b.region())) throw new IllegalArgumentException("Instance '" + a.instance + "' declares a region different from instance '" +
-                                                                                           b.instance() + "': '" + a.region() + "'");
+                    // - All instance name and region combinations must be distinct
+                    if (a.instance().equals(b.instance()) && a.region.equals(b.region))
+                        throw new IllegalArgumentException("Instance '" + a.instance + "' declared multiple times " +
+                                                           "with region '" + a.region + "', but allowed at most once");
                 }
                 if (level == Level.instance && a.region.equals(b.region)) {
                     // - Instance name is implicit
@@ -78,9 +76,9 @@ public class Endpoint {
         return containerId;
     }
 
-    /** The regions of this points to */
+    /** The regions this points to */
     public List<RegionName> regions() {
-        return targets.stream().map(Target::region).collect(Collectors.toUnmodifiableList());
+        return targets.stream().map(Target::region).toList();
     }
 
     /** The level of targets in this */
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 77894a8cf1f..75949d2b6cd 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
@@ -321,12 +321,15 @@ public class DeploymentSpecXmlReader {
 
             List<Endpoint.Target> targets = new ArrayList<>();
             if (level == Endpoint.Level.application) {
-                String region = requireStringAttribute("region", endpointElement);
+                Optional<String> endpointRegion = stringAttribute("region", endpointElement);
                 int weightSum = 0;
                 for (var instanceElement : XML.getChildren(endpointElement, "instance")) {
                     String instanceName = instanceElement.getTextContent();
-                    String weightFromAttribute = requireStringAttribute("weight", instanceElement);
                     if (instanceName == null || instanceName.isBlank()) illegal(msgPrefix + "empty 'instance' element");
+                    Optional<String> instanceRegion = stringAttribute("region", instanceElement);
+                    if (endpointRegion.isPresent() == instanceRegion.isPresent())
+                        illegal(msgPrefix + "'region' attribute must be declared on either <endpoint> or <instance> tag");
+                    String weightFromAttribute = requireStringAttribute("weight", instanceElement);
                     int weight;
                     try {
                         weight = Integer.parseInt(weightFromAttribute);
@@ -334,7 +337,7 @@ public class DeploymentSpecXmlReader {
                         throw new IllegalArgumentException(msgPrefix + "invalid weight value '" + weightFromAttribute + "'");
                     }
                     weightSum += weight;
-                    targets.add(new Endpoint.Target(RegionName.from(region),
+                    targets.add(new Endpoint.Target(RegionName.from(endpointRegion.orElseGet(instanceRegion::get)),
                                                     InstanceName.from(instanceName),
                                                     weight));
                 }
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 3870768ceb4..2fda56e5290 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
@@ -2,6 +2,8 @@
 package com.yahoo.config.application.api;
 
 import com.google.common.collect.ImmutableSet;
+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.Environment;
@@ -1328,18 +1330,19 @@ public class DeploymentSpecTest {
                            </instance>
                            <endpoints>
                              <endpoint id="foo" container-id="qrs" %s>
-                               <instance %s>%s</instance>
+                               <instance %s %s>%s</instance>
                          %s    </endpoint>
                            </endpoints>
                          </deployment>
                          """;
-        assertInvalid(String.format(xmlForm, "", "", "", ""), "Missing required attribute 'region' in 'endpoint'");
-        assertInvalid(String.format(xmlForm, "region='us-west-1'", "", "main", ""), "Missing required attribute 'weight' in 'instance");
-        assertInvalid(String.format(xmlForm, "region='us-west-1'", "weight='1'", "", ""), "Application-level endpoint 'foo': empty 'instance' element");
-        assertInvalid(String.format(xmlForm, "region='invalid'", "weight='1'", "main", ""), "Application-level endpoint 'foo': targets undeclared region 'invalid' in instance 'main'");
-        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", "<region>us-east-3</region>"), "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, "", "weight='1'", "", "main", ""), "'region' attribute must be declared on either <endpoint> or <instance> tag");
+        assertInvalid(String.format(xmlForm, "region='us-west-1'", "weight='1'", "region='us-west-1'", "main", ""), "'region' attribute must be declared on either <endpoint> or <instance> tag");
+        assertInvalid(String.format(xmlForm, "region='us-west-1'", "", "", "main", ""), "Missing required attribute 'weight' in 'instance");
+        assertInvalid(String.format(xmlForm, "region='us-west-1'", "weight='1'", "", "", ""), "Application-level endpoint 'foo': empty 'instance' element");
+        assertInvalid(String.format(xmlForm, "region='invalid'", "weight='1'", "", "main", ""), "Application-level endpoint 'foo': targets undeclared region 'invalid' in instance 'main'");
+        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", "<region>us-east-3</region>"), "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");
     }
 
     @Test
@@ -1369,18 +1372,27 @@ public class DeploymentSpecTest {
                                                          <endpoint id="bar" container-id="music" region='us-east-3'>
                                                            <instance weight="10">main</instance>
                                                          </endpoint>
+                                                         <endpoint id="baz" container-id="moose">
+                                                           <instance weight="1" region='us-west-1'>main</instance>
+                                                           <instance weight="2" region='us-east-3'>main</instance>
+                                                           <instance weight="3" region='us-west-1'>beta</instance>
+                                                         </endpoint>
                                                        </endpoints>
                                                      </deployment>
                                                      """);
-        assertEquals(List.of(new Endpoint("foo", "movies", Endpoint.Level.application,
-                                          List.of(new Endpoint.Target(RegionName.from("us-west-1"), InstanceName.from("beta"), 2),
-                                                  new Endpoint.Target(RegionName.from("us-west-1"), InstanceName.from("main"), 8))),
-                             new Endpoint("bar", "music", Endpoint.Level.application,
-                                          List.of(new Endpoint.Target(RegionName.from("us-east-3"), InstanceName.from("main"), 10)))),
+        assertEquals(List.of(new Endpoint("foo", "movies", Level.application,
+                                          List.of(new Target(RegionName.from("us-west-1"), InstanceName.from("beta"), 2),
+                                                  new Target(RegionName.from("us-west-1"), InstanceName.from("main"), 8))),
+                             new Endpoint("bar", "music", Level.application,
+                                          List.of(new Target(RegionName.from("us-east-3"), InstanceName.from("main"), 10))),
+                             new Endpoint("baz", "moose", Level.application,
+                                          List.of(new Target(RegionName.from("us-west-1"), InstanceName.from("main"), 1),
+                                                  new Target(RegionName.from("us-east-3"), InstanceName.from("main"), 2),
+                                                  new Target(RegionName.from("us-west-1"), InstanceName.from("beta"), 3)))),
                              spec.endpoints());
-        assertEquals(List.of(new Endpoint("glob", "music", Endpoint.Level.instance,
-                                          List.of(new Endpoint.Target(RegionName.from("us-west-1"), InstanceName.from("main"), 1),
-                                                  new Endpoint.Target(RegionName.from("us-east-3"), InstanceName.from("main"), 1)))),
+        assertEquals(List.of(new Endpoint("glob", "music", Level.instance,
+                                          List.of(new Target(RegionName.from("us-west-1"), InstanceName.from("main"), 1),
+                                                  new Target(RegionName.from("us-east-3"), InstanceName.from("main"), 1)))),
                      spec.requireInstance("main").endpoints());
     }