Ignore given region when step specifies none

4 files changed, 54 insertions, 45 deletions

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 9c6013a127d..8813eaf9c8c 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
@@ -154,10 +154,6 @@ public class DeploymentInstanceSpec extends DeploymentSpec.Steps {
                              .noneMatch(block -> block.window().includes(instant));
     }
 
-    /** Returns the athenz domain if configured */
-    // TODO jonmv: Remove when 7.162 is older than the oldest deployed version.
-    public Optional<AthenzDomain> athenzDomain() { return Optional.empty(); }
-
     /** Returns the athenz service for environment/region if configured, defaulting to that of the instance */
     public Optional<AthenzService> athenzService(Environment environment, RegionName region) {
         return zones().stream()
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 a8fcf5e1315..e227e041177 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
@@ -260,7 +260,10 @@ public class DeploymentSpec {
             return concerns(environment, region);
         }
 
-        /** Returns whether this step specifies the given environment, and, optionally, region. */
+        /**
+         * Returns whether this step specifies the given environment, and, optionally,
+         * if this step specifies a region, whether this is also the given region.
+         */
         public abstract boolean concerns(Environment environment, Optional<RegionName> region);
 
         /** Returns the zones deployed to in this step. */
@@ -348,7 +351,7 @@ public class DeploymentSpec {
         @Override
         public boolean concerns(Environment environment, Optional<RegionName> region) {
             if (environment != this.environment) return false;
-            if (region.isPresent() && ! region.equals(this.region)) return false;
+            if (region.isPresent() && this.region.isPresent() && ! region.equals(this.region)) return false;
             return true;
         }
 
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 06db4fe44be..5561ebdef63 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
@@ -44,10 +44,10 @@ public class DeploymentSpecTest {
         assertEquals(1, spec.requireInstance("default").steps().size());
         assertFalse(spec.majorVersion().isPresent());
         assertTrue(spec.requireInstance("default").steps().get(0).concerns(Environment.test));
-        assertTrue(spec.requireInstance("default").deploysTo(Environment.test, Optional.empty()));
-        assertFalse(spec.requireInstance("default").deploysTo(Environment.test, Optional.of(RegionName.from("region1"))));
-        assertFalse(spec.requireInstance("default").deploysTo(Environment.staging, Optional.empty()));
-        assertFalse(spec.requireInstance("default").deploysTo(Environment.prod, Optional.empty()));
+        assertTrue(spec.requireInstance("default").concerns(Environment.test, Optional.empty()));
+        assertTrue(spec.requireInstance("default").concerns(Environment.test, Optional.of(RegionName.from("region1")))); // test steps specify no region
+        assertFalse(spec.requireInstance("default").concerns(Environment.staging, Optional.empty()));
+        assertFalse(spec.requireInstance("default").concerns(Environment.prod, Optional.empty()));
         assertFalse(spec.requireInstance("default").globalServiceId().isPresent());
     }
 
@@ -81,9 +81,9 @@ public class DeploymentSpecTest {
         assertEquals(1, spec.steps().size());
         assertEquals(1, spec.requireInstance("default").steps().size());
         assertTrue(spec.requireInstance("default").steps().get(0).concerns(Environment.staging));
-        assertFalse(spec.requireInstance("default").deploysTo(Environment.test, Optional.empty()));
-        assertTrue(spec.requireInstance("default").deploysTo(Environment.staging, Optional.empty()));
-        assertFalse(spec.requireInstance("default").deploysTo(Environment.prod, Optional.empty()));
+        assertFalse(spec.requireInstance("default").concerns(Environment.test, Optional.empty()));
+        assertTrue(spec.requireInstance("default").concerns(Environment.staging, Optional.empty()));
+        assertFalse(spec.requireInstance("default").concerns(Environment.prod, Optional.empty()));
         assertFalse(spec.requireInstance("default").globalServiceId().isPresent());
     }
 
@@ -110,11 +110,11 @@ public class DeploymentSpecTest {
         assertTrue(spec.requireInstance("default").steps().get(1).concerns(Environment.prod, Optional.of(RegionName.from("us-west1"))));
         assertTrue(((DeploymentSpec.DeclaredZone)spec.requireInstance("default").steps().get(1)).active());
 
-        assertFalse(spec.requireInstance("default").deploysTo(Environment.test, Optional.empty()));
-        assertFalse(spec.requireInstance("default").deploysTo(Environment.staging, Optional.empty()));
-        assertTrue(spec.requireInstance("default").deploysTo(Environment.prod, Optional.of(RegionName.from("us-east1"))));
-        assertTrue(spec.requireInstance("default").deploysTo(Environment.prod, Optional.of(RegionName.from("us-west1"))));
-        assertFalse(spec.requireInstance("default").deploysTo(Environment.prod, Optional.of(RegionName.from("no-such-region"))));
+        assertFalse(spec.requireInstance("default").concerns(Environment.test, Optional.empty()));
+        assertFalse(spec.requireInstance("default").concerns(Environment.staging, Optional.empty()));
+        assertTrue(spec.requireInstance("default").concerns(Environment.prod, Optional.of(RegionName.from("us-east1"))));
+        assertTrue(spec.requireInstance("default").concerns(Environment.prod, Optional.of(RegionName.from("us-west1"))));
+        assertFalse(spec.requireInstance("default").concerns(Environment.prod, Optional.of(RegionName.from("no-such-region"))));
         assertFalse(spec.requireInstance("default").globalServiceId().isPresent());
 
         assertEquals(DeploymentSpec.UpgradePolicy.defaultPolicy, spec.requireInstance("default").upgradePolicy());
@@ -289,12 +289,12 @@ public class DeploymentSpecTest {
         assertTrue(instance.steps().get(4).concerns(Environment.prod, Optional.of(RegionName.from("us-west1"))));
         assertTrue(((DeploymentSpec.DeclaredZone)instance.steps().get(4)).active());
 
-        assertTrue(instance.deploysTo(Environment.test, Optional.empty()));
-        assertFalse(instance.deploysTo(Environment.test, Optional.of(RegionName.from("region1"))));
-        assertTrue(instance.deploysTo(Environment.staging, Optional.empty()));
-        assertTrue(instance.deploysTo(Environment.prod, Optional.of(RegionName.from("us-east1"))));
-        assertTrue(instance.deploysTo(Environment.prod, Optional.of(RegionName.from("us-west1"))));
-        assertFalse(instance.deploysTo(Environment.prod, Optional.of(RegionName.from("no-such-region"))));
+        assertTrue(instance.concerns(Environment.test, Optional.empty()));
+        assertTrue(instance.concerns(Environment.test, Optional.of(RegionName.from("region1")))); // test steps specify no region
+        assertTrue(instance.concerns(Environment.staging, Optional.empty()));
+        assertTrue(instance.concerns(Environment.prod, Optional.of(RegionName.from("us-east1"))));
+        assertTrue(instance.concerns(Environment.prod, Optional.of(RegionName.from("us-west1"))));
+        assertFalse(instance.concerns(Environment.prod, Optional.of(RegionName.from("no-such-region"))));
         assertFalse(instance.globalServiceId().isPresent());
     }
 
@@ -909,9 +909,10 @@ public class DeploymentSpecTest {
     @Test
     public void athenz_service_is_overridden_from_environment() {
         StringReader r = new StringReader(
-                "<deployment athenz-domain='domain' athenz-service='service'>" +
+                "<deployment athenz-domain='domain' athenz-service='unused-service'>" +
                 "   <instance id='default' athenz-service='service'>" +
-                "      <test/>" +
+                "      <test />" +
+                "      <staging athenz-service='staging-service' />" +
                 "      <prod athenz-service='prod-service'>" +
                 "         <region active='true'>us-west-1</region>" +
                 "      </prod>" +
@@ -919,6 +920,12 @@ public class DeploymentSpecTest {
                 "</deployment>"
         );
         DeploymentSpec spec = DeploymentSpec.fromXml(r);
+        assertEquals("service",
+                     spec.requireInstance("default").athenzService(Environment.test,
+                                                                   RegionName.from("us-east-1")).get().value());
+        assertEquals("staging-service",
+                     spec.requireInstance("default").athenzService(Environment.staging,
+                                                                   RegionName.from("us-north-1")).get().value());
         assertEquals("prod-service",
                      spec.requireInstance("default").athenzService(Environment.prod,
                                                                    RegionName.from("us-west-1")).get().value());
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 450be3c9b06..77ce5c2175d 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
@@ -43,10 +43,10 @@ public class DeploymentSpecWithoutInstanceTest {
         assertEquals(1, spec.steps().size());
         assertFalse(spec.majorVersion().isPresent());
         assertTrue(spec.steps().get(0).concerns(Environment.test));
-        assertTrue(spec.requireInstance("default").deploysTo(Environment.test, Optional.empty()));
-        assertFalse(spec.requireInstance("default").deploysTo(Environment.test, Optional.of(RegionName.from("region1"))));
-        assertFalse(spec.requireInstance("default").deploysTo(Environment.staging, Optional.empty()));
-        assertFalse(spec.requireInstance("default").deploysTo(Environment.prod, Optional.empty()));
+        assertTrue(spec.requireInstance("default").concerns(Environment.test, Optional.empty()));
+        assertTrue(spec.requireInstance("default").concerns(Environment.test, Optional.of(RegionName.from("region1")))); // test steps specify no region
+        assertFalse(spec.requireInstance("default").concerns(Environment.staging, Optional.empty()));
+        assertFalse(spec.requireInstance("default").concerns(Environment.prod, Optional.empty()));
         assertFalse(spec.requireInstance("default").globalServiceId().isPresent());
     }
 
@@ -76,9 +76,9 @@ public class DeploymentSpecWithoutInstanceTest {
         assertEquals(1, spec.steps().size());
         assertEquals(1, spec.requireInstance("default").steps().size());
         assertTrue(spec.requireInstance("default").steps().get(0).concerns(Environment.staging));
-        assertFalse(spec.requireInstance("default").deploysTo(Environment.test, Optional.empty()));
-        assertTrue(spec.requireInstance("default").deploysTo(Environment.staging, Optional.empty()));
-        assertFalse(spec.requireInstance("default").deploysTo(Environment.prod, Optional.empty()));
+        assertFalse(spec.requireInstance("default").concerns(Environment.test, Optional.empty()));
+        assertTrue(spec.requireInstance("default").concerns(Environment.staging, Optional.empty()));
+        assertFalse(spec.requireInstance("default").concerns(Environment.prod, Optional.empty()));
         assertFalse(spec.requireInstance("default").globalServiceId().isPresent());
     }
 
@@ -103,11 +103,11 @@ public class DeploymentSpecWithoutInstanceTest {
         assertTrue(spec.requireInstance("default").steps().get(1).concerns(Environment.prod, Optional.of(RegionName.from("us-west1"))));
         assertTrue(((DeploymentSpec.DeclaredZone)spec.requireInstance("default").steps().get(1)).active());
 
-        assertFalse(spec.requireInstance("default").deploysTo(Environment.test, Optional.empty()));
-        assertFalse(spec.requireInstance("default").deploysTo(Environment.staging, Optional.empty()));
-        assertTrue(spec.requireInstance("default").deploysTo(Environment.prod, Optional.of(RegionName.from("us-east1"))));
-        assertTrue(spec.requireInstance("default").deploysTo(Environment.prod, Optional.of(RegionName.from("us-west1"))));
-        assertFalse(spec.requireInstance("default").deploysTo(Environment.prod, Optional.of(RegionName.from("no-such-region"))));
+        assertFalse(spec.requireInstance("default").concerns(Environment.test, Optional.empty()));
+        assertFalse(spec.requireInstance("default").concerns(Environment.staging, Optional.empty()));
+        assertTrue(spec.requireInstance("default").concerns(Environment.prod, Optional.of(RegionName.from("us-east1"))));
+        assertTrue(spec.requireInstance("default").concerns(Environment.prod, Optional.of(RegionName.from("us-west1"))));
+        assertFalse(spec.requireInstance("default").concerns(Environment.prod, Optional.of(RegionName.from("no-such-region"))));
         assertFalse(spec.requireInstance("default").globalServiceId().isPresent());
         
         assertEquals(DeploymentSpec.UpgradePolicy.defaultPolicy, spec.requireInstance("default").upgradePolicy());
@@ -144,12 +144,12 @@ public class DeploymentSpecWithoutInstanceTest {
         assertTrue(spec.requireInstance("default").steps().get(4).concerns(Environment.prod, Optional.of(RegionName.from("us-west1"))));
         assertTrue(((DeploymentSpec.DeclaredZone)spec.requireInstance("default").steps().get(4)).active());
 
-        assertTrue(spec.requireInstance("default").deploysTo(Environment.test, Optional.empty()));
-        assertFalse(spec.requireInstance("default").deploysTo(Environment.test, Optional.of(RegionName.from("region1"))));
-        assertTrue(spec.requireInstance("default").deploysTo(Environment.staging, Optional.empty()));
-        assertTrue(spec.requireInstance("default").deploysTo(Environment.prod, Optional.of(RegionName.from("us-east1"))));
-        assertTrue(spec.requireInstance("default").deploysTo(Environment.prod, Optional.of(RegionName.from("us-west1"))));
-        assertFalse(spec.requireInstance("default").deploysTo(Environment.prod, Optional.of(RegionName.from("no-such-region"))));
+        assertTrue(spec.requireInstance("default").concerns(Environment.test, Optional.empty()));
+        assertTrue(spec.requireInstance("default").concerns(Environment.test, Optional.of(RegionName.from("region1")))); // test steps specify no region
+        assertTrue(spec.requireInstance("default").concerns(Environment.staging, Optional.empty()));
+        assertTrue(spec.requireInstance("default").concerns(Environment.prod, Optional.of(RegionName.from("us-east1"))));
+        assertTrue(spec.requireInstance("default").concerns(Environment.prod, Optional.of(RegionName.from("us-west1"))));
+        assertFalse(spec.requireInstance("default").concerns(Environment.prod, Optional.of(RegionName.from("no-such-region"))));
         assertFalse(spec.requireInstance("default").globalServiceId().isPresent());
     }
 
@@ -552,7 +552,8 @@ public class DeploymentSpecWithoutInstanceTest {
     public void athenz_service_is_overridden_from_environment() {
         StringReader r = new StringReader(
                 "<deployment athenz-domain='domain' athenz-service='service'>\n" +
-                "  <test/>\n" +
+                "  <test />\n" +
+                "  <staging athenz-service='staging-service' />\n" +
                 "  <prod athenz-service='prod-service'>\n" +
                 "    <region active='true'>us-west-1</region>\n" +
                 "  </prod>\n" +
@@ -561,6 +562,8 @@ public class DeploymentSpecWithoutInstanceTest {
         DeploymentSpec spec = DeploymentSpec.fromXml(r);
         assertEquals("service", spec.athenzService().get().value());
         assertEquals(spec.athenzDomain().get().value(), "domain");
+        assertEquals(spec.requireInstance("default").athenzService(Environment.test, RegionName.from("us-east-1")).get().value(), "service");
+        assertEquals(spec.requireInstance("default").athenzService(Environment.staging, RegionName.from("us-north-1")).get().value(), "staging-service");
         assertEquals(spec.requireInstance("default").athenzService(Environment.prod, RegionName.from("us-west-1")).get().value(), "prod-service");
     }