Validate cloud accounts when receiving package

3 files changed, 44 insertions, 7 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 ae97a2ffa71..e4feaccca89 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
@@ -191,6 +191,13 @@ public class ApplicationController {
         applicationPackageValidator.validate(application, applicationPackage, clock.instant());
     }
 
+    public Set<CloudAccount> accountsOf(TenantName tenant) {
+        return cloudAccountsFlag.with(FetchVector.Dimension.TENANT_ID, tenant.value())
+                                .value().stream()
+                                .map(CloudAccount::from)
+                                .collect(Collectors.toSet());
+    }
+
     /** Returns the application with the given id, or null if it is not present */
     public Optional<Application> getApplication(TenantAndApplicationId id) {
         return curator.readApplication(id);
@@ -681,10 +688,7 @@ public class ApplicationController {
             return Optional.empty();
         }
         TenantName tenant = deployment.applicationId().tenant();
-        Set<CloudAccount> tenantAccounts = cloudAccountsFlag.with(FetchVector.Dimension.TENANT_ID, tenant.value())
-                                                            .value().stream()
-                                                            .map(CloudAccount::from)
-                                                            .collect(Collectors.toSet());
+        Set<CloudAccount> tenantAccounts = accountsOf(tenant);
         if (!tenantAccounts.contains(requestedAccount.get())) {
             throw new IllegalArgumentException("Requested cloud account '" + requestedAccount.get().value() +
                                                "' is not valid for tenant '" + tenant + "'");
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 361ec8f1e51..977dd30c900 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
@@ -8,6 +8,7 @@ import com.yahoo.config.application.api.Endpoint;
 import com.yahoo.config.application.api.Endpoint.Level;
 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;
@@ -16,6 +17,7 @@ import com.yahoo.config.provision.zone.ZoneApi;
 import com.yahoo.config.provision.zone.ZoneId;
 import com.yahoo.vespa.hosted.controller.Application;
 import com.yahoo.vespa.hosted.controller.Controller;
+import com.yahoo.vespa.hosted.controller.api.identifiers.DeploymentId;
 import com.yahoo.vespa.hosted.controller.application.EndpointId;
 import com.yahoo.vespa.hosted.controller.versions.VespaVersion;
 
@@ -29,8 +31,11 @@ import java.util.Map;
 import java.util.Objects;
 import java.util.Optional;
 import java.util.Set;
+import java.util.TreeSet;
 import java.util.stream.Collectors;
 
+import static java.util.stream.Collectors.joining;
+
 /**
  * This contains validators for a {@link ApplicationPackage} that depend on a {@link Controller} to perform validation.
  *
@@ -56,6 +61,24 @@ public class ApplicationPackageValidator {
         validateCompactedEndpoint(applicationPackage);
         validateSecurityClientsPem(applicationPackage);
         validateDeprecatedElements(applicationPackage);
+        validateCloudAccounts(application, applicationPackage);
+    }
+
+    private void validateCloudAccounts(Application application, ApplicationPackage applicationPackage) {
+        Set<CloudAccount> tenantAccounts = new TreeSet<>(controller.applications().accountsOf(application.id().tenant()));
+        Set<CloudAccount> declaredAccounts = new TreeSet<>();
+        applicationPackage.deploymentSpec().cloudAccount().ifPresent(declaredAccounts::add);
+        for (DeploymentInstanceSpec instance : applicationPackage.deploymentSpec().instances())
+            for (ZoneId zone : controller.zoneRegistry().zones().controllerUpgraded().ids())
+                instance.cloudAccount(zone.environment(), Optional.of(zone.region())).ifPresent(declaredAccounts::add);
+
+        declaredAccounts.removeIf(tenantAccounts::contains);
+        declaredAccounts.removeIf(CloudAccount::isUnspecified);
+        if ( ! declaredAccounts.isEmpty())
+            throw new IllegalArgumentException("cloud accounts " +
+                                               declaredAccounts.stream().map(CloudAccount::value).collect(joining(", ", "[", "]")) +
+                                               " are not valid for tenant " +
+                                               application.id().tenant());
     }
 
     /** Verify that deployment spec does not use elements deprecated on a major version older than wanted major version */
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 3c0b2f36286..c87a4e490b9 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
@@ -38,6 +38,7 @@ import com.yahoo.vespa.hosted.controller.application.pkg.ApplicationPackage;
 import com.yahoo.vespa.hosted.controller.deployment.ApplicationPackageBuilder;
 import com.yahoo.vespa.hosted.controller.deployment.DeploymentContext;
 import com.yahoo.vespa.hosted.controller.deployment.DeploymentTester;
+import com.yahoo.vespa.hosted.controller.deployment.RunStatus;
 import com.yahoo.vespa.hosted.controller.deployment.Submission;
 import com.yahoo.vespa.hosted.controller.integration.ZoneApiMock;
 import com.yahoo.vespa.hosted.controller.notification.Notification;
@@ -1296,12 +1297,21 @@ public class ControllerTest {
                 .cloudAccount(cloudAccount)
                 .region(prodZone.region())
                 .build();
-        // Prod and dev deployments fail because cloud account is not declared for this tenant
-        context.submit(applicationPackage).runJobExpectingFailure(systemTest, "Requested cloud account '012345678912' is not valid for tenant 'tenant'");
+
+        // Submission fails because cloud account is not declared for this tenant
+        assertEquals("cloud accounts [012345678912] are not valid for tenant tenant",
+                     assertThrows(IllegalArgumentException.class,
+                                  () -> context.submit(applicationPackage))
+                             .getMessage());
+        assertEquals("cloud accounts [012345678912] are not valid for tenant tenant",
+                     assertThrows(IllegalArgumentException.class,
+                                  () -> context.runJob(devUsEast1, applicationPackage))
+                             .getMessage());
 
         // Deployment fails because zone is not configured in requested cloud account
         tester.controllerTester().flagSource().withListFlag(PermanentFlags.CLOUD_ACCOUNTS.id(), List.of(cloudAccount), String.class);
-        context.runJobExpectingFailure(systemTest, "Zone test.us-east-1 is not configured in requested cloud account '012345678912'")
+        context.submit(applicationPackage)
+               .runJobExpectingFailure(systemTest, "Zone test.us-east-1 is not configured in requested cloud account '012345678912'")
                .abortJob(stagingTest);
 
         // Deployment to prod succeeds once all zones are configured in requested account