diff options
author | Martin Polden <mpolden@mpolden.no> | 2022-09-07 11:54:54 +0200 |
---|---|---|
committer | Martin Polden <mpolden@mpolden.no> | 2022-09-12 09:27:42 +0200 |
commit | 89dd1b69ed6d036715969bbe2d1c28d20c731d43 (patch) | |
tree | 1fdc5c966907496746c1394d825cc549d9f40858 | |
parent | 939eb860022dde6689c1dabae11ff640d529e9a1 (diff) |
Allow cloud-account attribute on non-production elements
9 files changed, 51 insertions, 24 deletions
diff --git a/config-model-api/abi-spec.json b/config-model-api/abi-spec.json index 672ad4071d6..7e17cd0f600 100644 --- a/config-model-api/abi-spec.json +++ b/config-model-api/abi-spec.json @@ -208,7 +208,7 @@ "public boolean canUpgradeAt(java.time.Instant)", "public boolean canChangeRevisionAt(java.time.Instant)", "public java.util.Optional athenzService(com.yahoo.config.provision.Environment, com.yahoo.config.provision.RegionName)", - "public java.util.Optional cloudAccount(com.yahoo.config.provision.Environment, com.yahoo.config.provision.RegionName)", + "public java.util.Optional cloudAccount(com.yahoo.config.provision.Environment, java.util.Optional)", "public com.yahoo.config.application.api.Notifications notifications()", "public java.util.List endpoints()", "public boolean deploysTo(com.yahoo.config.provision.Environment, com.yahoo.config.provision.RegionName)", 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 8cb70d50a59..cd20b5b8910 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 @@ -226,10 +226,9 @@ public class DeploymentInstanceSpec extends DeploymentSpec.Steps { } /** Returns the cloud account to use for given environment and region, if any */ - public Optional<CloudAccount> cloudAccount(Environment environment, RegionName region) { - if (!environment.isProduction()) return Optional.empty(); + public Optional<CloudAccount> cloudAccount(Environment environment, Optional<RegionName> region) { return zones().stream() - .filter(zone -> zone.concerns(environment, Optional.of(region))) + .filter(zone -> zone.concerns(environment, region)) .findFirst() .flatMap(DeploymentSpec.DeclaredZone::cloudAccount) .or(() -> cloudAccount); 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 c4fec6e668e..ffdceaf7567 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 @@ -376,8 +376,6 @@ public class DeploymentSpec { illegal("Non-prod environments cannot specify a region"); if (environment == Environment.prod && region.isEmpty()) illegal("Prod environments must be specified with a region"); - if (environment != Environment.prod && cloudAccount.isPresent()) - illegal("Non-prod environments cannot specify cloud account"); this.environment = Objects.requireNonNull(environment); this.region = Objects.requireNonNull(region); this.active = active; @@ -403,7 +401,7 @@ public class DeploymentSpec { } @Override - public List<DeclaredZone> zones() { return Collections.singletonList(this); } + public List<DeclaredZone> zones() { return List.of(this); } @Override public boolean concerns(Environment environment, Optional<RegionName> region) { @@ -492,7 +490,7 @@ public class DeploymentSpec { public List<DeclaredZone> zones() { return steps.stream() .flatMap(step -> step.zones().stream()) - .collect(Collectors.toUnmodifiableList()); + .toList(); } @Override 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 d08b6c46943..c98e429a52b 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 @@ -57,6 +57,8 @@ public class DeploymentSpecXmlReader { private static final String instanceTag = "instance"; private static final String testTag = "test"; private static final String stagingTag = "staging"; + private static final String devTag = "dev"; + private static final String perfTag = "perf"; private static final String upgradeTag = "upgrade"; private static final String blockChangeTag = "block-change"; private static final String prodTag = "prod"; @@ -234,11 +236,12 @@ public class DeploymentSpecXmlReader { case testTag: if (Stream.iterate(stepTag, Objects::nonNull, Node::getParentNode) .anyMatch(node -> prodTag.equals(node.getNodeName()))) { + // A production test return List.of(new DeclaredTest(RegionName.from(XML.getValue(stepTag).trim()))); } - return List.of(new DeclaredZone(Environment.from(stepTag.getTagName()), Optional.empty(), false, athenzService, testerFlavor, Optional.empty())); - case stagingTag: - return List.of(new DeclaredZone(Environment.from(stepTag.getTagName()), Optional.empty(), false, athenzService, testerFlavor, Optional.empty())); + return List.of(new DeclaredZone(Environment.from(stepTag.getTagName()), Optional.empty(), false, athenzService, testerFlavor, readCloudAccount(stepTag))); + case devTag, perfTag, stagingTag: + return List.of(new DeclaredZone(Environment.from(stepTag.getTagName()), Optional.empty(), false, athenzService, testerFlavor, readCloudAccount(stepTag))); case prodTag: // regions, delay and parallel may be nested within, but we can flatten them return XML.getChildren(stepTag).stream() .flatMap(child -> readNonInstanceSteps(child, prodAttributes, stepTag).stream()) @@ -433,7 +436,11 @@ public class DeploymentSpecXmlReader { Optional<String> testerFlavor, Element regionTag) { return new DeclaredZone(environment, Optional.of(RegionName.from(XML.getValue(regionTag).trim())), readActive(regionTag), athenzService, testerFlavor, - stringAttribute(cloudAccountAttribute, regionTag).map(CloudAccount::new)); + readCloudAccount(regionTag)); + } + + private Optional<CloudAccount> readCloudAccount(Element tag) { + return stringAttribute(cloudAccountAttribute, tag).map(CloudAccount::new); } private Optional<String> readGlobalServiceId(Element environmentTag) { 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 7f0e9b4cae8..6ffb4d2f1e0 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 @@ -1515,11 +1515,15 @@ public class DeploymentSpecTest { StringReader r = new StringReader( "<deployment version='1.0' cloud-account='100000000000'>" + " <instance id='beta' cloud-account='200000000000'>" + + " <staging cloud-account='600000000000'/>" + + " <perf cloud-account='700000000000'/>" + " <prod>" + " <region>us-west-1</region>" + " </prod>" + " </instance>" + " <instance id='main'>" + + " <test cloud-account='500000000000'/>" + + " <dev cloud-account='400000000000'/>" + " <prod>" + " <region cloud-account='300000000000'>us-east-1</region>" + " <region>eu-west-1</region>" + @@ -1528,9 +1532,19 @@ public class DeploymentSpecTest { "</deployment>" ); DeploymentSpec spec = DeploymentSpec.fromXml(r); - assertEquals(Optional.of(new CloudAccount("200000000000")), spec.requireInstance("beta").cloudAccount(Environment.prod, RegionName.from("us-west-1"))); - assertEquals(Optional.of(new CloudAccount("300000000000")), spec.requireInstance("main").cloudAccount(Environment.prod, RegionName.from("us-east-1"))); - assertEquals(Optional.of(new CloudAccount("100000000000")), spec.requireInstance("main").cloudAccount(Environment.prod, RegionName.from("eu-west-1"))); + assertCloudAccount("200000000000", spec.requireInstance("beta"), Environment.prod, "us-west-1"); + assertCloudAccount("600000000000", spec.requireInstance("beta"), Environment.staging, ""); + assertCloudAccount("700000000000", spec.requireInstance("beta"), Environment.perf, ""); + assertCloudAccount("200000000000", spec.requireInstance("beta"), Environment.dev, ""); + assertCloudAccount("300000000000", spec.requireInstance("main"), Environment.prod, "us-east-1"); + assertCloudAccount("100000000000", spec.requireInstance("main"), Environment.prod, "eu-west-1"); + assertCloudAccount("400000000000", spec.requireInstance("main"), Environment.dev, ""); + assertCloudAccount("500000000000", spec.requireInstance("main"), Environment.test, ""); + assertCloudAccount("100000000000", spec.requireInstance("main"), Environment.staging, ""); + } + + private void assertCloudAccount(String expected, DeploymentInstanceSpec instance, Environment environment, String region) { + assertEquals(Optional.of(expected).map(CloudAccount::new), instance.cloudAccount(environment, Optional.of(region).filter(s -> !s.isEmpty()).map(RegionName::from))); } private static void assertInvalid(String deploymentSpec, String errorMessagePart) { 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 1232f700fee..4acf0c692fb 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 @@ -715,9 +715,9 @@ public class DeploymentSpecWithoutInstanceTest { ); DeploymentSpec spec = DeploymentSpec.fromXml(r); DeploymentInstanceSpec instance = spec.requireInstance("default"); - assertEquals(Optional.of(new CloudAccount("219876543210")), instance.cloudAccount(Environment.prod, RegionName.from("us-east-1"))); - assertEquals(Optional.of(new CloudAccount("012345678912")), instance.cloudAccount(Environment.prod, RegionName.from("us-west-1"))); - assertEquals(Optional.empty(), instance.cloudAccount(Environment.staging, RegionName.defaultName())); + assertEquals(Optional.of(new CloudAccount("219876543210")), instance.cloudAccount(Environment.prod, Optional.of(RegionName.from("us-east-1")))); + assertEquals(Optional.of(new CloudAccount("012345678912")), instance.cloudAccount(Environment.prod, Optional.of(RegionName.from("us-west-1")))); + assertEquals(Optional.of(new CloudAccount("012345678912")), instance.cloudAccount(Environment.staging, Optional.empty())); r = new StringReader( "<deployment version='1.0'>" + @@ -728,8 +728,8 @@ public class DeploymentSpecWithoutInstanceTest { "</deployment>" ); spec = DeploymentSpec.fromXml(r); - assertEquals(Optional.of(new CloudAccount("219876543210")), spec.requireInstance("default").cloudAccount(Environment.prod, RegionName.from("us-east-1"))); - assertEquals(Optional.empty(), spec.requireInstance("default").cloudAccount(Environment.prod, RegionName.from("us-west-1"))); + assertEquals(Optional.of(new CloudAccount("219876543210")), spec.requireInstance("default").cloudAccount(Environment.prod, Optional.of(RegionName.from("us-east-1")))); + assertEquals(Optional.empty(), spec.requireInstance("default").cloudAccount(Environment.prod, Optional.of(RegionName.from("us-west-1")))); } private static Set<String> endpointRegions(String endpointId, DeploymentSpec spec) { diff --git a/config-model/src/main/resources/schema/deployment.rnc b/config-model/src/main/resources/schema/deployment.rnc index 251134c91e8..b95c9539750 100644 --- a/config-model/src/main/resources/schema/deployment.rnc +++ b/config-model/src/main/resources/schema/deployment.rnc @@ -24,6 +24,8 @@ StepExceptInstance = Endpoints? & Test? & Staging? & + Dev? & + Perf? & Prod* PrimitiveStep = @@ -101,6 +103,14 @@ Staging = element staging { text } +Dev = element dev { + attribute cloud-account { xsd:string }? +} + +Perf = element perf { + attribute cloud-account { xsd:string }? +} + Prod = element prod { attribute global-service-id { text }? & attribute athenz-service { xsd:string }? & 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 cb5bef81890..64e2c20e68c 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 @@ -59,7 +59,6 @@ import com.yahoo.vespa.hosted.controller.application.pkg.ApplicationPackageValid import com.yahoo.vespa.hosted.controller.athenz.impl.AthenzFacade; import com.yahoo.vespa.hosted.controller.certificate.EndpointCertificates; import com.yahoo.vespa.hosted.controller.concurrent.Once; -import com.yahoo.vespa.hosted.controller.deployment.DeploymentStatus; import com.yahoo.vespa.hosted.controller.deployment.DeploymentTrigger; import com.yahoo.vespa.hosted.controller.deployment.JobStatus; import com.yahoo.vespa.hosted.controller.deployment.Run; @@ -632,7 +631,7 @@ public class ApplicationController { .collect(toList()); Optional<CloudAccount> cloudAccount = applicationPackage.deploymentSpec() .instance(application.instance()) - .flatMap(spec -> spec.cloudAccount(zone.environment(), zone.region())); + .flatMap(spec -> spec.cloudAccount(zone.environment(), Optional.of(zone.region()))); ConfigServer.PreparedApplication preparedApplication = configServer.deploy(new DeploymentData(application, zone, applicationPackage.zippedContent(), platform, endpoints, endpointCertificateMetadata, dockerImageRepo, domain, diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/application/pkg/ApplicationPackageValidator.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/application/pkg/ApplicationPackageValidator.java index 5a131ba4a29..cb48b4aa621 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/application/pkg/ApplicationPackageValidator.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/application/pkg/ApplicationPackageValidator.java @@ -98,7 +98,7 @@ public class ApplicationPackageValidator { if (!controller.zoneRegistry().hasZone(ZoneId.from(environment, region))) { throw new IllegalArgumentException("Zone " + zone + " in deployment spec was not found in this system!"); } - Optional<CloudAccount> cloudAccount = spec.cloudAccount(environment, region); + Optional<CloudAccount> cloudAccount = spec.cloudAccount(environment, zone.region()); if (cloudAccount.isPresent() && !controller.zoneRegistry().hasZone(ZoneId.from(environment, region), cloudAccount.get())) { throw new IllegalArgumentException("Zone " + zone + " in deployment spec is not configured for " + "use in cloud account '" + cloudAccount.get().value() + @@ -195,7 +195,7 @@ public class ApplicationPackageValidator { for (var spec : deploymentSpec.instances()) { for (var zone : spec.zones()) { if (!zone.environment().isProduction()) continue; - Optional<CloudAccount> cloudAccount = spec.cloudAccount(zone.environment(), zone.region().get()); + Optional<CloudAccount> cloudAccount = spec.cloudAccount(zone.environment(), zone.region()); if (cloudAccount.isEmpty()) continue; if (validAccounts.contains(cloudAccount.get())) continue; throw new IllegalArgumentException("Cloud account '" + cloudAccount.get().value() + |