diff options
author | Martin Polden <mpolden@mpolden.no> | 2022-05-12 14:59:02 +0200 |
---|---|---|
committer | Martin Polden <mpolden@mpolden.no> | 2022-05-12 15:09:16 +0200 |
commit | 378a600e305d4f5720b0f057436f327efa1c3c42 (patch) | |
tree | 287a93270a8718cea7468f8a24a6922def520bbc /controller-server | |
parent | cb40193d64d26eaba2be248f8bfea078891c963f (diff) |
Use cloud account from deployment spec
Diffstat (limited to 'controller-server')
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(), |