Use cloud account from deployment spec

5 files changed, 69 insertions, 14 deletions

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 ab8ab659a0b..e48ad7596ea 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
@@ -22,7 +22,6 @@ import com.yahoo.vespa.athenz.api.AthenzService;
 import com.yahoo.vespa.athenz.api.AthenzUser;
 import com.yahoo.vespa.flags.FetchVector;
 import com.yahoo.vespa.flags.FlagSource;
-import com.yahoo.vespa.flags.Flags;
 import com.yahoo.vespa.flags.ListFlag;
 import com.yahoo.vespa.flags.PermanentFlags;
 import com.yahoo.vespa.flags.StringFlag;
@@ -134,8 +133,6 @@ public class ApplicationController {
     private final ListFlag<String> incompatibleVersions;
     private final BillingController billingController;
 
-    private final StringFlag cloudAccountFlag;
-
     ApplicationController(Controller controller, CuratorDb curator, AccessControl accessControl, Clock clock,
                           FlagSource flagSource, BillingController billingController) {
         this.controller = Objects.requireNonNull(controller);
@@ -154,7 +151,6 @@ public class ApplicationController {
         endpointCertificates = new EndpointCertificates(controller,
                                                         controller.serviceRegistry().endpointCertificateProvider(),
                                                         controller.serviceRegistry().endpointCertificateValidator());
-         cloudAccountFlag = Flags.PROVISION_IN_EXTERNAL_ACCOUNT.bindTo(controller.flagSource());
 
         // Update serialization format of all applications
         Once.after(Duration.ofMinutes(1), () -> {
@@ -619,12 +615,9 @@ public class ApplicationController {
             List<X509Certificate> operatorCertificates = controller.supportAccess().activeGrantsFor(deployment).stream()
                                                                    .map(SupportAccessGrant::certificate)
                                                                    .collect(toList());
-
-            // TODO(mpolden): Read this from DeploymentSpec and validate it against the valid accounts for the tenant,
-            //                as defined by PermanentFlags.EXTERNAL_ACCOUNTS
-            Optional<CloudAccount> cloudAccount = Optional.of(cloudAccountFlag.with(APPLICATION_ID, application.serializedForm()).value())
-                                                          .filter(account -> !account.isEmpty())
-                                                          .map(CloudAccount::new);
+            Optional<CloudAccount> cloudAccount = applicationPackage.deploymentSpec()
+                                                                    .instance(application.instance())
+                                                                    .flatMap(spec -> spec.cloudAccount(zone.environment(), 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 950eaea904a..ccad4fe92ad 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
@@ -6,11 +6,16 @@ import com.yahoo.config.application.api.DeploymentSpec;
 import com.yahoo.config.application.api.Endpoint;
 import com.yahoo.config.application.api.ValidationId;
 import com.yahoo.config.application.api.ValidationOverrides;
+import com.yahoo.config.provision.CloudAccount;
 import com.yahoo.config.provision.CloudName;
 import com.yahoo.config.provision.Environment;
 import com.yahoo.config.provision.InstanceName;
+import com.yahoo.config.provision.TenantName;
 import com.yahoo.config.provision.zone.ZoneApi;
 import com.yahoo.config.provision.zone.ZoneId;
+import com.yahoo.vespa.flags.FetchVector;
+import com.yahoo.vespa.flags.ListFlag;
+import com.yahoo.vespa.flags.PermanentFlags;
 import com.yahoo.vespa.hosted.controller.Application;
 import com.yahoo.vespa.hosted.controller.Controller;
 import com.yahoo.vespa.hosted.controller.application.EndpointId;
@@ -24,6 +29,7 @@ import java.util.List;
 import java.util.Map;
 import java.util.Objects;
 import java.util.Optional;
+import java.util.Set;
 import java.util.stream.Collectors;
 
 /**
@@ -34,9 +40,11 @@ import java.util.stream.Collectors;
 public class ApplicationPackageValidator {
 
     private final Controller controller;
+    private final ListFlag<String> cloudAccountsFlag;
 
     public ApplicationPackageValidator(Controller controller) {
         this.controller = Objects.requireNonNull(controller, "controller must be non-null");
+        this.cloudAccountsFlag = PermanentFlags.CLOUD_ACCOUNTS.bindTo(controller.flagSource());
     }
 
     /**
@@ -46,6 +54,7 @@ public class ApplicationPackageValidator {
      */
     public void validate(Application application, ApplicationPackage applicationPackage, Instant instant) {
         validateSteps(applicationPackage.deploymentSpec());
+        validateCloudAccounts(application, applicationPackage.deploymentSpec());
         validateEndpointRegions(applicationPackage.deploymentSpec());
         validateEndpointChange(application, applicationPackage, instant);
         validateCompactedEndpoint(applicationPackage);
@@ -145,6 +154,25 @@ public class ApplicationPackageValidator {
                                            ". " + ValidationOverrides.toAllowMessage(validationId));
     }
 
+    /** Verify that declared cloud accounts are allowed to be used by the tenant */
+    private void validateCloudAccounts(Application application, DeploymentSpec deploymentSpec) {
+        TenantName tenant = application.id().tenant();
+        Set<CloudAccount> validAccounts = cloudAccountsFlag.with(FetchVector.Dimension.TENANT_ID, tenant.value())
+                                                           .value().stream()
+                                                           .map(CloudAccount::new)
+                                                           .collect(Collectors.toSet());
+        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());
+                if (cloudAccount.isEmpty()) continue;
+                if (validAccounts.contains(cloudAccount.get())) continue;
+                throw new IllegalArgumentException("Cloud account '" + cloudAccount.get().value() +
+                                                   "' is not valid for tenant '" + tenant + "'");
+            }
+        }
+    }
+
     /** Returns whether newEndpoints contains all destinations in endpoints */
     private static boolean containsAllDestinationsOf(List<Endpoint> endpoints, List<Endpoint> newEndpoints) {
         var containsAllRegions = true;
diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/ControllerTest.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/ControllerTest.java
index 0ecac036913..f4f50de59d7 100644
--- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/ControllerTest.java
+++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/ControllerTest.java
@@ -4,7 +4,6 @@ package com.yahoo.vespa.hosted.controller;
 import com.google.common.collect.Sets;
 import com.yahoo.component.Version;
 import com.yahoo.config.application.api.DeploymentSpec;
-import com.yahoo.config.application.api.Notifications;
 import com.yahoo.config.application.api.ValidationId;
 import com.yahoo.config.application.api.ValidationOverrides;
 import com.yahoo.config.provision.ApplicationId;
@@ -1177,4 +1176,23 @@ public class ControllerTest {
         assertEquals(version2, tester.applications().compileVersion(application, OptionalInt.empty()));
     }
 
+    @Test
+    public void testCloudAccount() {
+        DeploymentContext context = tester.newDeploymentContext();
+        ZoneId zone = ZoneId.from("prod", "us-west-1");
+        String cloudAccount = "012345678912";
+        var applicationPackage = new ApplicationPackageBuilder()
+                .cloudAccount(cloudAccount)
+                .region(zone.region())
+                .build();
+        try {
+            context.submit(applicationPackage).deploy();
+            fail("Expected exception"); // Account invalid for tenant
+        } catch (IllegalArgumentException ignored) {}
+
+        tester.controllerTester().flagSource().withListFlag(PermanentFlags.CLOUD_ACCOUNTS.id(), List.of(cloudAccount), String.class);
+        context.submit(applicationPackage).deploy();
+        assertEquals(cloudAccount, tester.controllerTester().configServer().cloudAccount(context.deploymentIdIn(zone)).get().value());
+    }
+
 }
diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/deployment/ApplicationPackageBuilder.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/deployment/ApplicationPackageBuilder.java
index 27cf1554b4d..ffdd6da83a9 100644
--- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/deployment/ApplicationPackageBuilder.java
+++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/deployment/ApplicationPackageBuilder.java
@@ -66,6 +66,7 @@ public class ApplicationPackageBuilder {
     private boolean explicitSystemTest = false;
     private boolean explicitStagingTest = false;
     private Version compileVersion = Version.fromString("6.1");
+    private String cloudAccount = null;
 
     public ApplicationPackageBuilder majorVersion(int majorVersion) {
         this.majorVersion = OptionalInt.of(majorVersion);
@@ -256,6 +257,11 @@ public class ApplicationPackageBuilder {
         }
     }
 
+    public ApplicationPackageBuilder cloudAccount(String cloudAccount) {
+        this.cloudAccount = cloudAccount;
+        return this;
+    }
+
     private byte[] deploymentSpec() {
         StringBuilder xml = new StringBuilder();
         xml.append("<deployment version='1.0' ");
@@ -286,6 +292,11 @@ public class ApplicationPackageBuilder {
                 xml.append(globalServiceId);
                 xml.append("'");
             }
+            if (cloudAccount != null) {
+                xml.append(" cloud-account='");
+                xml.append(cloudAccount);
+                xml.append("'");
+            }
             xml.append(">\n");
             xml.append(prodBody);
             xml.append("    </prod>\n");
diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/integration/ConfigServerMock.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/integration/ConfigServerMock.java
index b3689dacea7..35a12f4b6d4 100644
--- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/integration/ConfigServerMock.java
+++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/integration/ConfigServerMock.java
@@ -4,10 +4,11 @@ package com.yahoo.vespa.hosted.controller.integration;
 import ai.vespa.http.DomainName;
 import ai.vespa.http.HttpURL.Path;
 import ai.vespa.http.HttpURL.Query;
-import com.yahoo.component.annotation.Inject;
 import com.yahoo.component.AbstractComponent;
 import com.yahoo.component.Version;
+import com.yahoo.component.annotation.Inject;
 import com.yahoo.config.provision.ApplicationId;
+import com.yahoo.config.provision.CloudAccount;
 import com.yahoo.config.provision.ClusterResources;
 import com.yahoo.config.provision.ClusterSpec;
 import com.yahoo.config.provision.DockerImage;
@@ -16,7 +17,6 @@ import com.yahoo.config.provision.HostName;
 import com.yahoo.config.provision.NodeResources;
 import com.yahoo.config.provision.NodeType;
 import com.yahoo.config.provision.zone.ZoneId;
-import com.yahoo.text.Text;
 import com.yahoo.vespa.flags.json.FlagData;
 import com.yahoo.vespa.hosted.controller.api.application.v4.model.ClusterMetrics;
 import com.yahoo.vespa.hosted.controller.api.application.v4.model.DeploymentData;
@@ -49,7 +49,6 @@ import com.yahoo.vespa.hosted.controller.application.pkg.ApplicationPackage;
 import java.io.ByteArrayInputStream;
 import java.io.InputStream;
 import java.net.URI;
-import java.nio.charset.StandardCharsets;
 import java.time.Duration;
 import java.time.Instant;
 import java.util.ArrayList;
@@ -94,6 +93,7 @@ public class ConfigServerMock extends AbstractComponent implements ConfigServer
     private final Map<DeploymentId, Set<ContainerEndpoint>> containerEndpoints = new HashMap<>();
     private final Map<DeploymentId, List<ClusterMetrics>> clusterMetrics = new HashMap<>();
     private final Map<DeploymentId, TestReport> testReport = new HashMap<>();
+    private final Map<DeploymentId, CloudAccount> cloudAccounts = new HashMap<>();
     private List<ProtonMetrics> protonMetrics;
 
     private Version lastPrepareVersion = null;
@@ -279,6 +279,10 @@ public class ConfigServerMock extends AbstractComponent implements ConfigServer
         return Collections.unmodifiableMap(containerEndpoints);
     }
 
+    public Optional<CloudAccount> cloudAccount(DeploymentId deployment) {
+        return Optional.ofNullable(cloudAccounts.get(deployment));
+    }
+
     public Set<String> containerEndpointNames(DeploymentId deployment) {
         return containerEndpoints.getOrDefault(deployment, Set.of()).stream()
                                  .map(ContainerEndpoint::names)
@@ -389,6 +393,7 @@ public class ConfigServerMock extends AbstractComponent implements ConfigServer
             provision(id.zoneId(), id.applicationId(), cluster);
 
         this.containerEndpoints.put(id, deployment.containerEndpoints());
+        deployment.cloudAccount().ifPresent(account -> this.cloudAccounts.put(id, account));
 
         if (!deferLoadBalancerProvisioning.contains(id.zoneId().environment())) {
             putLoadBalancers(id.zoneId(), List.of(new LoadBalancer(UUID.randomUUID().toString(),