diff options
author | Jon Marius Venstad <venstad@gmail.com> | 2020-01-15 14:51:41 +0100 |
---|---|---|
committer | Jon Marius Venstad <venstad@gmail.com> | 2020-01-15 14:51:41 +0100 |
commit | bdeffbd5779255b9924e27c4350400936e6cca68 (patch) | |
tree | 8ed477c258baba9150ef7622d2385114846b507b /config-model-api | |
parent | a6df129da95d0271d83689467fa3da77dad360d8 (diff) |
Athenz domain cannot be set on instance level, just on the root
Diffstat (limited to 'config-model-api')
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..0678629dcc2 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 the commit with this comment is deployed on all config servers. + 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"); } |