From 8c7a36a2d802e259062992f50465a7c321307c8c Mon Sep 17 00:00:00 2001 From: jonmv Date: Tue, 15 Nov 2022 11:03:33 +0100 Subject: Validate cloud accounts when receiving package --- .../hosted/controller/ApplicationController.java | 12 +++++++---- .../pkg/ApplicationPackageValidator.java | 23 ++++++++++++++++++++++ .../vespa/hosted/controller/ControllerTest.java | 16 ++++++++++++--- 3 files changed, 44 insertions(+), 7 deletions(-) (limited to 'controller-server/src') 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 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 getApplication(TenantAndApplicationId id) { return curator.readApplication(id); @@ -681,10 +688,7 @@ public class ApplicationController { return Optional.empty(); } TenantName tenant = deployment.applicationId().tenant(); - Set tenantAccounts = cloudAccountsFlag.with(FetchVector.Dimension.TENANT_ID, tenant.value()) - .value().stream() - .map(CloudAccount::from) - .collect(Collectors.toSet()); + Set 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 tenantAccounts = new TreeSet<>(controller.applications().accountsOf(application.id().tenant())); + Set 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 -- cgit v1.2.3