Revert "Allow setting cloud account for non-production environments"

6 files changed, 47 insertions, 71 deletions

diff --git a/config-model-api/abi-spec.json b/config-model-api/abi-spec.json
index 7e17cd0f600..672ad4071d6 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, java.util.Optional)",
+      "public java.util.Optional cloudAccount(com.yahoo.config.provision.Environment, com.yahoo.config.provision.RegionName)",
       "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 cd20b5b8910..8cb70d50a59 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,9 +226,10 @@ 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, Optional<RegionName> region) {
+    public Optional<CloudAccount> cloudAccount(Environment environment, RegionName region) {
+        if (!environment.isProduction()) return Optional.empty();
         return zones().stream()
-                      .filter(zone -> zone.concerns(environment, region))
+                      .filter(zone -> zone.concerns(environment, Optional.of(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 ffdceaf7567..c4fec6e668e 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,6 +376,8 @@ 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;
@@ -401,7 +403,7 @@ public class DeploymentSpec {
         }
 
         @Override
-        public List<DeclaredZone> zones() { return List.of(this); }
+        public List<DeclaredZone> zones() { return Collections.singletonList(this); }
 
         @Override
         public boolean concerns(Environment environment, Optional<RegionName> region) {
@@ -490,7 +492,7 @@ public class DeploymentSpec {
         public List<DeclaredZone> zones() {
             return steps.stream()
                         .flatMap(step -> step.zones().stream())
-                        .toList();
+                        .collect(Collectors.toUnmodifiableList());
         }
 
         @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 83fba75325e..09d61835ae3 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,8 +57,6 @@ 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";
@@ -236,12 +234,11 @@ 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, readCloudAccount(stepTag)));
-            case devTag, perfTag, stagingTag:
-                return List.of(new DeclaredZone(Environment.from(stepTag.getTagName()), Optional.empty(), false, athenzService, testerFlavor, readCloudAccount(stepTag)));
+                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()));
             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())
@@ -436,11 +433,7 @@ public class DeploymentSpecXmlReader {
                                           Optional<String> testerFlavor, Element regionTag) {
         return new DeclaredZone(environment, Optional.of(RegionName.from(XML.getValue(regionTag).trim())),
                                 readActive(regionTag), athenzService, testerFlavor,
-                                readCloudAccount(regionTag));
-    }
-
-    private Optional<CloudAccount> readCloudAccount(Element tag) {
-        return mostSpecificAttribute(tag, cloudAccountAttribute).map(CloudAccount::new);
+                                stringAttribute(cloudAccountAttribute, regionTag).map(CloudAccount::new));
     }
 
     private Optional<String> readGlobalServiceId(Element environmentTag) {
@@ -490,42 +483,42 @@ public class DeploymentSpecXmlReader {
     }
 
     private DeploymentSpec.UpgradePolicy readUpgradePolicy(String policy) {
-        return switch (policy) {
-            case "canary" -> UpgradePolicy.canary;
-            case "default" -> UpgradePolicy.defaultPolicy;
-            case "conservative" -> UpgradePolicy.conservative;
-            default -> throw new IllegalArgumentException("Illegal upgrade policy '" + policy + "': " +
-                                                          "Must be one of 'canary', 'default', 'conservative'");
-        };
+        switch (policy) {
+            case "canary": return DeploymentSpec.UpgradePolicy.canary;
+            case "default": return DeploymentSpec.UpgradePolicy.defaultPolicy;
+            case "conservative": return DeploymentSpec.UpgradePolicy.conservative;
+            default: throw new IllegalArgumentException("Illegal upgrade policy '" + policy + "': " +
+                                                        "Must be one of 'canary', 'default', 'conservative'");
+        }
     }
 
     private DeploymentSpec.RevisionChange readRevisionChange(String revision) {
-        return switch (revision) {
-            case "when-clear" -> RevisionChange.whenClear;
-            case "when-failing" -> RevisionChange.whenFailing;
-            case "always" -> RevisionChange.always;
-            default -> throw new IllegalArgumentException("Illegal upgrade revision change policy '" + revision + "': " +
-                                                          "Must be one of 'always', 'when-failing', 'when-clear'");
-        };
+        switch (revision) {
+            case "when-clear": return DeploymentSpec.RevisionChange.whenClear;
+            case "when-failing": return DeploymentSpec.RevisionChange.whenFailing;
+            case "always": return DeploymentSpec.RevisionChange.always;
+            default: throw new IllegalArgumentException("Illegal upgrade revision change policy '" + revision + "': " +
+                                                        "Must be one of 'always', 'when-failing', 'when-clear'");
+        }
     }
 
     private DeploymentSpec.RevisionTarget readRevisionTarget(String revision) {
-        return switch (revision) {
-            case "next" -> RevisionTarget.next;
-            case "latest" -> RevisionTarget.latest;
-            default -> throw new IllegalArgumentException("Illegal upgrade revision target '" + revision + "': " +
-                                                          "Must be one of 'next', 'latest'");
-        };
+        switch (revision) {
+            case "next": return DeploymentSpec.RevisionTarget.next;
+            case "latest": return DeploymentSpec.RevisionTarget.latest;
+            default: throw new IllegalArgumentException("Illegal upgrade revision target '" + revision + "': " +
+                                                        "Must be one of 'next', 'latest'");
+        }
     }
 
     private DeploymentSpec.UpgradeRollout readUpgradeRollout(String rollout) {
-        return switch (rollout) {
-            case "separate" -> UpgradeRollout.separate;
-            case "leading" -> UpgradeRollout.leading;
-            case "simultaneous" -> UpgradeRollout.simultaneous;
-            default -> throw new IllegalArgumentException("Illegal upgrade rollout '" + rollout + "': " +
-                                                          "Must be one of 'separate', 'leading', 'simultaneous'");
-        };
+        switch (rollout) {
+            case "separate": return DeploymentSpec.UpgradeRollout.separate;
+            case "leading": return DeploymentSpec.UpgradeRollout.leading;
+            case "simultaneous": return DeploymentSpec.UpgradeRollout.simultaneous;
+            default: throw new IllegalArgumentException("Illegal upgrade rollout '" + rollout + "': " +
+                                                        "Must be one of 'separate', 'leading', 'simultaneous'");
+        }
     }
 
     private boolean readActive(Element regionTag) {
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 6373e0e0232..7f0e9b4cae8 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
@@ -1514,21 +1514,12 @@ public class DeploymentSpecTest {
     public void cloudAccount() {
         StringReader r = new StringReader(
                 "<deployment version='1.0' cloud-account='100000000000'>" +
-                "    <instance id='alpha'>" +
-                "      <prod cloud-account='800000000000'>" +
-                "          <region>us-east-1</region>" +
-                "      </prod>" +
-                "    </instance>" +
                 "    <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>" +
@@ -1537,20 +1528,9 @@ public class DeploymentSpecTest {
                 "</deployment>"
         );
         DeploymentSpec spec = DeploymentSpec.fromXml(r);
-        assertCloudAccount("800000000000", spec.requireInstance("alpha"), Environment.prod, "us-east-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)));
+        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")));
     }
 
     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 4acf0c692fb..1232f700fee 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, 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()));
+        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()));
 
         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, Optional.of(RegionName.from("us-east-1"))));
-        assertEquals(Optional.empty(), spec.requireInstance("default").cloudAccount(Environment.prod, Optional.of(RegionName.from("us-west-1"))));
+        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")));
     }
 
     private static Set<String> endpointRegions(String endpointId, DeploymentSpec spec) {