Revert "Revert "Athenz domain cannot be set on instance level, just on the root""

6 files changed, 57 insertions, 61 deletions

diff --git a/config-model-api/abi-spec.json b/config-model-api/abi-spec.json
index d8482499a93..25f35781a1f 100644
--- a/config-model-api/abi-spec.json
+++ b/config-model-api/abi-spec.json
@@ -194,7 +194,7 @@
       "public"
     ],
     "methods": [
-      "public void <init>(com.yahoo.config.provision.InstanceName, java.util.List, com.yahoo.config.application.api.DeploymentSpec$UpgradePolicy, java.util.List, java.util.Optional, java.util.Optional, java.util.Optional, com.yahoo.config.application.api.Notifications, java.util.List)",
+      "public void <init>(com.yahoo.config.provision.InstanceName, java.util.List, com.yahoo.config.application.api.DeploymentSpec$UpgradePolicy, java.util.List, java.util.Optional, java.util.Optional, com.yahoo.config.application.api.Notifications, java.util.List)",
       "public com.yahoo.config.provision.InstanceName name()",
       "public com.yahoo.config.application.api.DeploymentSpec$UpgradePolicy upgradePolicy()",
       "public java.util.List changeBlocker()",
@@ -605,4 +605,4 @@
       "public static final com.yahoo.config.application.api.ValidationOverrides all"
     ]
   }
-}
\ No newline at end of file
+}
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 8e7c255f27a..9c6013a127d 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
@@ -29,7 +29,6 @@ public class DeploymentInstanceSpec extends DeploymentSpec.Steps {
     private final DeploymentSpec.UpgradePolicy upgradePolicy;
     private final List<DeploymentSpec.ChangeBlocker> changeBlockers;
     private final Optional<String> globalServiceId;
-    private final Optional<AthenzDomain> athenzDomain;
     private final Optional<AthenzService> athenzService;
     private final Notifications notifications;
     private final List<Endpoint> endpoints;
@@ -39,7 +38,6 @@ public class DeploymentInstanceSpec extends DeploymentSpec.Steps {
                                   DeploymentSpec.UpgradePolicy upgradePolicy,
                                   List<DeploymentSpec.ChangeBlocker> changeBlockers,
                                   Optional<String> globalServiceId,
-                                  Optional<AthenzDomain> athenzDomain,
                                   Optional<AthenzService> athenzService,
                                   Notifications notifications,
                                   List<Endpoint> endpoints) {
@@ -48,13 +46,11 @@ public class DeploymentInstanceSpec extends DeploymentSpec.Steps {
         this.upgradePolicy = upgradePolicy;
         this.changeBlockers = changeBlockers;
         this.globalServiceId = globalServiceId;
-        this.athenzDomain = athenzDomain;
         this.athenzService = athenzService;
         this.notifications = notifications;
         this.endpoints = List.copyOf(validateEndpoints(endpoints, steps()));
         validateZones(new HashSet<>(), new HashSet<>(), this);
         validateEndpoints(steps(), globalServiceId, this.endpoints);
-        validateAthenz();
     }
 
     public InstanceName name() { return name; }
@@ -137,29 +133,6 @@ public class DeploymentInstanceSpec extends DeploymentSpec.Steps {
         }
     }
 
-    /**
-     * Throw an IllegalArgumentException if Athenz configuration violates:
-     * domain not configured -> no zone can configure service
-     * domain configured -> all zones must configure service
-     */
-    private void validateAthenz() {
-        // If athenz domain is not set, athenz service cannot be set on any level
-        if (athenzDomain.isEmpty()) {
-            for (DeploymentSpec.DeclaredZone zone : zones()) {
-                if (zone.athenzService().isPresent()) {
-                    throw new IllegalArgumentException("Athenz service configured for zone: " + zone + ", but Athenz domain is not configured");
-                }
-            }
-            // if athenz domain is not set, athenz service must be set implicitly or directly on all zones.
-        } else if (athenzService.isEmpty()) {
-            for (DeploymentSpec.DeclaredZone zone : zones()) {
-                if (zone.athenzService().isEmpty()) {
-                    throw new IllegalArgumentException("Athenz domain is configured, but Athenz service not configured for zone: " + zone);
-                }
-            }
-        }
-    }
-
     /** Returns the upgrade policy of this, which is defaultPolicy if none is specified */
     public DeploymentSpec.UpgradePolicy upgradePolicy() { return upgradePolicy; }
 
@@ -182,9 +155,10 @@ public class DeploymentInstanceSpec extends DeploymentSpec.Steps {
     }
 
     /** Returns the athenz domain if configured */
-    public Optional<AthenzDomain> athenzDomain() { return athenzDomain; }
+    // 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 */
+    /** 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()
                       .filter(zone -> zone.concerns(environment, Optional.of(region)))
@@ -213,7 +187,6 @@ public class DeploymentInstanceSpec extends DeploymentSpec.Steps {
                upgradePolicy == other.upgradePolicy &&
                changeBlockers.equals(other.changeBlockers) &&
                steps().equals(other.steps()) &&
-               athenzDomain.equals(other.athenzDomain) &&
                athenzService.equals(other.athenzService) &&
                notifications.equals(other.notifications) &&
                endpoints.equals(other.endpoints);
@@ -221,7 +194,7 @@ public class DeploymentInstanceSpec extends DeploymentSpec.Steps {
 
     @Override
     public int hashCode() {
-        return Objects.hash(globalServiceId, upgradePolicy, changeBlockers, steps(), athenzDomain, athenzService, notifications, endpoints);
+        return Objects.hash(globalServiceId, upgradePolicy, changeBlockers, steps(), athenzService, notifications, endpoints);
     }
 
     @Override
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 f778c2c2d0e..a19a83f1207 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
@@ -62,6 +62,7 @@ public class DeploymentSpec {
         this.xmlForm = xmlForm;
         validateTotalDelay(steps);
         validateUpgradePoliciesOfIncreasingConservativeness(steps);
+        validateAthenz();
     }
 
     /** Throw an IllegalArgumentException if the total delay exceeds 24 hours */
@@ -89,6 +90,35 @@ public class DeploymentSpec {
         }
     }
 
+    /**
+     * Throw an IllegalArgumentException if Athenz configuration violates:
+     * domain not configured -> no zone can configure service
+     * domain configured -> all zones must configure service
+     */
+    private void validateAthenz() {
+        // If athenz domain is not set, athenz service cannot be set on any level
+        if (athenzDomain.isEmpty()) {
+            for (DeploymentInstanceSpec instance : instances()) {
+                for (DeploymentSpec.DeclaredZone zone : instance.zones()) {
+                    if (zone.athenzService().isPresent()) {
+                        throw new IllegalArgumentException("Athenz service configured for zone: " + zone + ", but Athenz domain is not configured");
+                    }
+                }
+            }
+            // if athenz domain is not set, athenz service must be set implicitly or directly on all zones.
+        }
+        else if (athenzService.isEmpty()) {
+            for (DeploymentInstanceSpec instance : instances()) {
+                for (DeploymentSpec.DeclaredZone zone : instance.zones()) {
+                    if (zone.athenzService().isEmpty()) {
+                        throw new IllegalArgumentException("Athenz domain is configured, but Athenz service not configured for zone: " + zone);
+                    }
+                }
+            }
+        }
+    }
+
+
     /** Returns the major version this application is pinned to, or empty (default) to allow all major versions */
     public Optional<Integer> majorVersion() { return majorVersion; }
 
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 faa7c8cf932..68f27a21ce4 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
@@ -139,12 +139,7 @@ public class DeploymentSpecXmlReader {
         // Values where the parent may provide a default
         DeploymentSpec.UpgradePolicy upgradePolicy = readUpgradePolicy(instanceTag, parentTag);
         List<DeploymentSpec.ChangeBlocker> changeBlockers = readChangeBlockers(instanceTag, parentTag);
-        Optional<AthenzDomain> athenzDomain = stringAttribute(athenzDomainAttribute, instanceTag)
-                                                        .or(() -> stringAttribute(athenzDomainAttribute, parentTag))
-                                                        .map(AthenzDomain::from);
-        Optional<AthenzService> athenzService = stringAttribute(athenzServiceAttribute, instanceTag)
-                                                        .or(() -> stringAttribute(athenzServiceAttribute, parentTag))
-                                                        .map(AthenzService::from);
+        Optional<AthenzService> athenzService = mostSpecificAttribute(instanceTag, athenzServiceAttribute).map(AthenzService::from);
         Notifications notifications = readNotifications(instanceTag, parentTag);
 
         // Values where there is no default
@@ -161,7 +156,6 @@ public class DeploymentSpecXmlReader {
                                                              upgradePolicy,
                                                              changeBlockers,
                                                              globalServiceId.asOptional(),
-                                                             athenzDomain,
                                                              athenzService,
                                                              notifications,
                                                              endpoints))
@@ -179,11 +173,8 @@ public class DeploymentSpecXmlReader {
     // Consume the given tag as 0-N steps. 0 if it is not a step, >1 if it contains multiple nested steps that should be flattened
     @SuppressWarnings("fallthrough")
     private List<Step> readNonInstanceSteps(Element stepTag, MutableOptional<String> globalServiceId, Element parentTag) {
-        Optional<AthenzService> athenzService = stringAttribute(athenzServiceAttribute, stepTag)
-                                                        .or(() -> stringAttribute(athenzServiceAttribute, parentTag))
-                                                        .map(AthenzService::from);
-        Optional<String> testerFlavor = stringAttribute(testerFlavorAttribute, stepTag)
-                                                .or(() -> stringAttribute(testerFlavorAttribute, parentTag));
+        Optional<AthenzService> athenzService = mostSpecificAttribute(stepTag, athenzServiceAttribute).map(AthenzService::from);
+        Optional<String> testerFlavor = mostSpecificAttribute(stepTag, testerFlavorAttribute);
 
         if (prodTag.equals(stepTag.getTagName()))
             globalServiceId.set(readGlobalServiceId(stepTag));
@@ -349,7 +340,7 @@ public class DeploymentSpecXmlReader {
     /**
      * Returns the given attribute as a string, or Optional.empty if it is not present or empty
      */
-    private Optional<String> stringAttribute(String attributeName, Element tag) {
+    private static Optional<String> stringAttribute(String attributeName, Element tag) {
         String value = tag.getAttribute(attributeName);
         return Optional.ofNullable(value).filter(s -> !s.equals(""));
     }
@@ -425,6 +416,14 @@ public class DeploymentSpecXmlReader {
                || root.getAttributes().getLength() == 1 && root.hasAttribute("version");
     }
 
+    /** Returns the given attribute from the given tag or its closest ancestor with the attribute. */
+    private static Optional<String> mostSpecificAttribute(Element tag, String attributeName) {
+        return Stream.iterate(tag, Objects::nonNull, Node::getParentNode)
+                     .filter(Element.class::isInstance)
+                     .map(Element.class::cast)
+                     .flatMap(element -> stringAttribute(attributeName, element).stream())
+                     .findFirst();
+    }
 
     private static class MutableOptional<T> {
 
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 d0740b3e9b9..5bf103d1836 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
@@ -805,7 +805,6 @@ public class DeploymentSpecTest {
         );
         DeploymentSpec spec = DeploymentSpec.fromXml(r);
         assertEquals("domain", spec.athenzDomain().get().value());
-        assertEquals("domain", spec.requireInstance("instance1").athenzDomain().get().value());
         assertEquals("service", spec.athenzService().get().value());
         assertEquals("service", spec.requireInstance("instance1").athenzService(Environment.prod,
                                                                                 RegionName.from("us-west-1")).get().value());
@@ -830,7 +829,6 @@ public class DeploymentSpecTest {
         assertEquals("domain", spec.athenzDomain().get().value());
         assertEquals("service", spec.athenzService().get().value());
 
-        assertEquals("domain", spec.requireInstance("instance1").athenzDomain().get().value());
         assertEquals("prod-service", spec.requireInstance("instance1").athenzService(Environment.prod,
                                                                                      RegionName.from("us-central-1")).get().value());
         assertEquals("prod-service", spec.requireInstance("instance1").athenzService(Environment.prod,
@@ -865,7 +863,6 @@ public class DeploymentSpecTest {
         );
         DeploymentSpec spec = DeploymentSpec.fromXml(r);
         assertEquals("domain", spec.athenzDomain().get().value());
-        assertEquals("domain", spec.requireInstance("instance1").athenzDomain().get().value());
         assertEquals("service", spec.requireInstance("instance1").athenzService(Environment.prod,
                                                                                 RegionName.from("us-west-1")).get().value());
         assertEquals("service", spec.requireInstance("instance1").athenzService(Environment.prod,
@@ -877,8 +874,8 @@ public class DeploymentSpecTest {
     @Test
     public void athenz_config_is_read_from_instance() {
         StringReader r = new StringReader(
-                "<deployment>" +
-                "   <instance id='default' athenz-domain='domain' athenz-service='service'>" +
+                "<deployment athenz-domain='domain'>" +
+                "   <instance id='default' athenz-service='service'>" +
                 "      <prod>" +
                 "         <region active='true'>us-west-1</region>" +
                 "      </prod>" +
@@ -886,17 +883,16 @@ public class DeploymentSpecTest {
                 "</deployment>"
         );
         DeploymentSpec spec = DeploymentSpec.fromXml(r);
-        assertEquals(Optional.empty(), spec.athenzDomain());
+        assertEquals("domain", spec.athenzDomain().get().value());
         assertEquals(Optional.empty(), spec.athenzService());
-        assertEquals(spec.requireInstance("default").athenzDomain().get().value(), "domain");
-        assertEquals(spec.requireInstance("default").athenzService(Environment.prod, RegionName.from("us-west-1")).get().value(), "service");
+        assertEquals("service", spec.requireInstance("default").athenzService(Environment.prod, RegionName.from("us-west-1")).get().value());
     }
 
     @Test
     public void athenz_service_is_overridden_from_environment() {
         StringReader r = new StringReader(
                 "<deployment athenz-domain='domain' athenz-service='service'>" +
-                "   <instance id='default' athenz-domain='domain' athenz-service='service'>" +
+                "   <instance id='default' athenz-service='service'>" +
                 "      <test/>" +
                 "      <prod athenz-service='prod-service'>" +
                 "         <region active='true'>us-west-1</region>" +
@@ -905,7 +901,6 @@ public class DeploymentSpecTest {
                 "</deployment>"
         );
         DeploymentSpec spec = DeploymentSpec.fromXml(r);
-        assertEquals(spec.requireInstance("default").athenzDomain().get().value(), "domain");
         assertEquals("prod-service",
                      spec.requireInstance("default").athenzService(Environment.prod,
                                                                    RegionName.from("us-west-1")).get().value());
@@ -914,8 +909,8 @@ public class DeploymentSpecTest {
     @Test(expected = IllegalArgumentException.class)
     public void it_fails_when_athenz_service_is_not_defined() {
         StringReader r = new StringReader(
-                "<deployment>" +
-                "   <instance id='default' athenz-domain='domain'>" +
+                "<deployment athenz-domain='domain'>" +
+                "   <instance id='default'>" +
                 "      <prod>" +
                 "         <region active='true'>us-west-1</region>" +
                 "      </prod>" +
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 b5e5946262a..6a815a467f5 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
@@ -506,7 +506,7 @@ public class DeploymentSpecWithoutInstanceTest {
                 "</deployment>"
         );
         DeploymentSpec spec = DeploymentSpec.fromXml(r);
-        assertEquals(spec.requireInstance("default").athenzDomain().get().value(), "domain");
+        assertEquals(spec.athenzDomain().get().value(), "domain");
         assertEquals(spec.requireInstance("default").athenzService(Environment.prod, RegionName.from("us-west-1")).get().value(), "service");
     }
 
@@ -525,7 +525,6 @@ public class DeploymentSpecWithoutInstanceTest {
         );
         DeploymentSpec spec = DeploymentSpec.fromXml(r);
         assertEquals("domain", spec.athenzDomain().get().value());
-        assertEquals("domain", spec.requireInstance("default").athenzDomain().get().value());
         assertEquals("service", spec.athenzService().get().value());
         assertEquals("prod-service", spec.requireInstance("default").athenzService(Environment.prod, RegionName.from("us-central-1"))
                                          .get().value());
@@ -547,7 +546,7 @@ public class DeploymentSpecWithoutInstanceTest {
         );
         DeploymentSpec spec = DeploymentSpec.fromXml(r);
         assertEquals("service", spec.athenzService().get().value());
-        assertEquals(spec.requireInstance("default").athenzDomain().get().value(), "domain");
+        assertEquals(spec.athenzDomain().get().value(), "domain");
         assertEquals(spec.requireInstance("default").athenzService(Environment.prod, RegionName.from("us-west-1")).get().value(), "prod-service");
     }