diff options
10 files changed, 67 insertions, 91 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"); } diff --git a/config-model/src/main/java/com/yahoo/vespa/model/container/xml/ContainerModelBuilder.java b/config-model/src/main/java/com/yahoo/vespa/model/container/xml/ContainerModelBuilder.java index d749cdc465e..474561a0c0a 100644 --- a/config-model/src/main/java/com/yahoo/vespa/model/container/xml/ContainerModelBuilder.java +++ b/config-model/src/main/java/com/yahoo/vespa/model/container/xml/ContainerModelBuilder.java @@ -871,9 +871,7 @@ public class ContainerModelBuilder extends ConfigModelBuilder<ContainerModel> { String athenzDnsSuffix, Zone zone, DeploymentSpec spec) { - spec.instance(app.getApplicationId().instance()) - .flatMap(instanceSpec -> instanceSpec.athenzDomain()) - .or(() -> spec.athenzDomain()) + spec.athenzDomain() .ifPresent(domain -> { AthenzService service = spec.instance(app.getApplicationId().instance()) .flatMap(instanceSpec -> instanceSpec.athenzService(zone.environment(), zone.region())) diff --git a/config-model/src/main/resources/schema/deployment.rnc b/config-model/src/main/resources/schema/deployment.rnc index 6a8bc8f77b9..171112f6bd7 100644 --- a/config-model/src/main/resources/schema/deployment.rnc +++ b/config-model/src/main/resources/schema/deployment.rnc @@ -32,7 +32,6 @@ PrimitiveStep = Instance = element instance { attribute id { xsd:string } & - attribute athenz-domain { xsd:string }? & attribute athenz-service { xsd:string }? & StepExceptInstance } diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/ApplicationController.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/ApplicationController.java index e4197dab2cf..a5821812ce9 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/ApplicationController.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/ApplicationController.java @@ -917,7 +917,8 @@ public class ApplicationController { * @param deployer principal initiating the deployment, possibly empty */ public void verifyApplicationIdentityConfiguration(TenantName tenantName, Optional<InstanceName> instanceName, Optional<ZoneId> zoneId, ApplicationPackage applicationPackage, Optional<Principal> deployer) { - Optional<AthenzDomain> identityDomain = getIdentityDomain(applicationPackage); + Optional<AthenzDomain> identityDomain = applicationPackage.deploymentSpec().athenzDomain() + .map(domain -> new AthenzDomain(domain.value())); if(identityDomain.isEmpty()) { // If there is no domain configured in deployment.xml there is nothing to do. return; @@ -940,8 +941,8 @@ public class ApplicationController { var zone = zoneId.orElseThrow(() -> new IllegalArgumentException("Unable to evaluate access, no zone provided in deployment")); var serviceToLaunch = instanceName .flatMap(instance -> applicationPackage.deploymentSpec().instance(instance)) - .map(instanceSpec -> instanceSpec.athenzService(zone.environment(), zone.region())) - .orElse(applicationPackage.deploymentSpec().athenzService()) + .flatMap(instanceSpec -> instanceSpec.athenzService(zone.environment(), zone.region())) + .or(() -> applicationPackage.deploymentSpec().athenzService()) .map(service -> new AthenzService(identityDomain.get(), service.value())); if(serviceToLaunch.isPresent()) { @@ -980,35 +981,17 @@ public class ApplicationController { .map(AthenzUser.class::cast); } - private Optional<AthenzDomain> getIdentityDomain(ApplicationPackage applicationPackage) { - List<AthenzDomain> domains = Stream.concat(applicationPackage.deploymentSpec().athenzDomain().stream(), - applicationPackage.deploymentSpec().instances().stream() - .flatMap(spec -> spec.athenzDomain().stream())) - .distinct() - .map(domain -> new AthenzDomain(domain.value())) - .collect(toList()); - - - // We cannot have more than one domain, does not make sense that the domains are different ... - if(domains.size() > 1) { - throw new IllegalArgumentException(String.format("Multiple athenz-domain configured in deployment.xml (%s), can only have one.", domains.toString())); - } - return domains.stream().findAny(); - } - /* * Verifies that the configured athenz service (if any) can be launched. */ private void verifyAllowedLaunchAthenzService(DeploymentSpec deploymentSpec) { - controller.zoneRegistry().zones().reachable().ids().forEach(zone -> { - AthenzIdentity configServerAthenzIdentity = controller.zoneRegistry().getConfigServerHttpsIdentity(zone); - deploymentSpec.athenzDomain().ifPresent(domain -> { + deploymentSpec.athenzDomain().ifPresent(domain -> { + controller.zoneRegistry().zones().reachable().ids().forEach(zone -> { + AthenzIdentity configServerAthenzIdentity = controller.zoneRegistry().getConfigServerHttpsIdentity(zone); deploymentSpec.athenzService().ifPresent(service -> { verifyAthenzServiceCanBeLaunchedBy(configServerAthenzIdentity, new AthenzService(domain.value(), service.value())); }); - }); - deploymentSpec.instances().forEach(spec -> { - spec.athenzDomain().ifPresent(domain -> { + deploymentSpec.instances().forEach(spec -> { spec.athenzService(zone.environment(), zone.region()).ifPresent(service -> { verifyAthenzServiceCanBeLaunchedBy(configServerAthenzIdentity, new AthenzService(domain.value(), service.value())); }); diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/InternalStepRunner.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/InternalStepRunner.java index 20c64751335..10025f86540 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/InternalStepRunner.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/InternalStepRunner.java @@ -665,7 +665,7 @@ public class InternalStepRunner implements StepRunner { DEFAULT_TESTER_RESOURCES_AWS : DEFAULT_TESTER_RESOURCES)); byte[] testPackage = controller.applications().applicationStore().getTester(id.application().tenant(), id.application().application(), version); byte[] deploymentXml = deploymentXml(id.tester(), - spec.requireInstance(id.application().instance()).athenzDomain(), + spec.athenzDomain(), spec.requireInstance(id.application().instance()).athenzService(zone.environment(), zone.region())); try (ZipBuilder zipBuilder = new ZipBuilder(testPackage.length + servicesXml.length + 1000)) { |