diff options
5 files changed, 115 insertions, 21 deletions
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 5b5f89fd8d1..efe75d191b8 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 @@ -1,12 +1,10 @@ // Copyright 2017 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. package com.yahoo.config.application.api; -import com.google.common.collect.ImmutableList; import com.yahoo.config.application.api.xml.DeploymentSpecXmlReader; import com.yahoo.config.provision.AthenzDomain; import com.yahoo.config.provision.AthenzService; import com.yahoo.config.provision.Environment; -import com.yahoo.config.provision.Flavor; import com.yahoo.config.provision.RegionName; import java.io.BufferedReader; @@ -69,12 +67,12 @@ public class DeploymentSpec { this.upgradePolicy = upgradePolicy; this.majorVersion = majorVersion; this.changeBlockers = changeBlockers; - this.steps = ImmutableList.copyOf(completeSteps(new ArrayList<>(steps))); + this.steps = List.copyOf(completeSteps(new ArrayList<>(steps))); this.xmlForm = xmlForm; this.athenzDomain = athenzDomain; this.athenzService = athenzService; this.notifications = notifications; - this.endpoints = ImmutableList.copyOf(validateEndpoints(endpoints, this.steps)); + this.endpoints = List.copyOf(validateEndpoints(endpoints, this.steps)); validateZones(this.steps); validateAthenz(); validateEndpoints(this.steps, globalServiceId, this.endpoints); @@ -102,7 +100,7 @@ public class DeploymentSpec { } } - return ImmutableList.copyOf(rebuiltEndpointsList); + return List.copyOf(rebuiltEndpointsList); } /** Throw an IllegalArgumentException if the total delay exceeds 24 hours */ @@ -481,7 +479,7 @@ public class DeploymentSpec { private final List<DeclaredZone> zones; public ParallelZones(List<DeclaredZone> zones) { - this.zones = ImmutableList.copyOf(zones); + this.zones = List.copyOf(zones); } @Override 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 4ad35f20c39..972be87d86d 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 @@ -52,6 +52,7 @@ import com.yahoo.vespa.hosted.controller.application.ApplicationPackage; import com.yahoo.vespa.hosted.controller.application.AssignedRotation; import com.yahoo.vespa.hosted.controller.application.Deployment; import com.yahoo.vespa.hosted.controller.application.DeploymentMetrics; +import com.yahoo.vespa.hosted.controller.application.DeploymentSpecValidator; import com.yahoo.vespa.hosted.controller.application.Endpoint; import com.yahoo.vespa.hosted.controller.application.EndpointId; import com.yahoo.vespa.hosted.controller.application.EndpointList; @@ -61,7 +62,6 @@ import com.yahoo.vespa.hosted.controller.application.JobStatus.JobRun; import com.yahoo.vespa.hosted.controller.application.SystemApplication; import com.yahoo.vespa.hosted.controller.athenz.impl.AthenzFacade; import com.yahoo.vespa.hosted.controller.concurrent.Once; -import com.yahoo.vespa.hosted.controller.deployment.DeploymentSteps; import com.yahoo.vespa.hosted.controller.deployment.DeploymentTrigger; import com.yahoo.vespa.hosted.controller.dns.NameServiceQueue.Priority; import com.yahoo.vespa.hosted.controller.maintenance.RoutingPolicies; @@ -132,6 +132,7 @@ public class ApplicationController { private final DeploymentTrigger deploymentTrigger; private final BooleanFlag provisionApplicationCertificate; private final ApplicationCertificateProvider applicationCertificateProvider; + private final DeploymentSpecValidator deploymentSpecValidator; ApplicationController(Controller controller, CuratorDb curator, AccessControl accessControl, RotationsConfig rotationsConfig, @@ -154,6 +155,7 @@ public class ApplicationController { this.provisionApplicationCertificate = Flags.PROVISION_APPLICATION_CERTIFICATE.bindTo(controller.flagSource()); this.applicationCertificateProvider = controller.applicationCertificateProvider(); + this.deploymentSpecValidator = new DeploymentSpecValidator(controller); // Update serialization format of all applications Once.after(Duration.ofMinutes(1), () -> { @@ -413,7 +415,7 @@ public class ApplicationController { /** Stores the deployment spec and validation overrides from the application package, and runs cleanup. */ public LockedApplication storeWithUpdatedConfig(LockedApplication application, ApplicationPackage applicationPackage) { - validate(applicationPackage.deploymentSpec()); + deploymentSpecValidator.validate(applicationPackage.deploymentSpec()); application = application.with(applicationPackage.deploymentSpec()); application = application.with(applicationPackage.validationOverrides()); @@ -813,19 +815,6 @@ public class ApplicationController { return curator.lockForDeployment(application, zone); } - /** Verify that each of the production zones listed in the deployment spec exist in this system. */ - private void validate(DeploymentSpec deploymentSpec) { - new DeploymentSteps(deploymentSpec, controller::system).jobs(); - deploymentSpec.zones().stream() - .filter(zone -> zone.environment() == Environment.prod) - .forEach(zone -> { - if ( ! controller.zoneRegistry().hasZone(ZoneId.from(zone.environment(), - zone.region().orElse(null)))) { - throw new IllegalArgumentException("Zone " + zone + " in deployment spec was not found in this system!"); - } - }); - } - /** Verify that we don't downgrade an existing production deployment. */ private void validateRun(Application application, ZoneId zone, Version platformVersion, ApplicationVersion applicationVersion) { Deployment deployment = application.deployments().get(zone); diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/application/DeploymentSpecValidator.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/application/DeploymentSpecValidator.java new file mode 100644 index 00000000000..ce7904dc829 --- /dev/null +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/application/DeploymentSpecValidator.java @@ -0,0 +1,68 @@ +// Copyright 2019 Oath Inc. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. +package com.yahoo.vespa.hosted.controller.application; + +import com.yahoo.config.application.api.DeploymentSpec; +import com.yahoo.config.provision.CloudName; +import com.yahoo.config.provision.Environment; +import com.yahoo.config.provision.zone.ZoneApi; +import com.yahoo.config.provision.zone.ZoneId; +import com.yahoo.vespa.hosted.controller.Controller; +import com.yahoo.vespa.hosted.controller.deployment.DeploymentSteps; + +import java.util.HashSet; +import java.util.Objects; +import java.util.stream.Collectors; + +/** + * This contains validators for a {@link DeploymentSpec} that depend on a {@link Controller} to perform validation. + * + * @author mpolden + */ +public class DeploymentSpecValidator { + + private final Controller controller; + + public DeploymentSpecValidator(Controller controller) { + this.controller = Objects.requireNonNull(controller, "controller must be non-null"); + } + + /** + * Validate the given deploymentSpec + * + * @throws IllegalArgumentException if any validations fail + */ + public void validate(DeploymentSpec deploymentSpec) { + validateSteps(deploymentSpec); + validateEndpoints(deploymentSpec); + } + + /** Verify that each of the production zones listed in the deployment spec exist in this system */ + private void validateSteps(DeploymentSpec deploymentSpec) { + new DeploymentSteps(deploymentSpec, controller::system).jobs(); + deploymentSpec.zones().stream() + .filter(zone -> zone.environment() == Environment.prod) + .forEach(zone -> { + if ( ! controller.zoneRegistry().hasZone(ZoneId.from(zone.environment(), + zone.region().orElse(null)))) { + throw new IllegalArgumentException("Zone " + zone + " in deployment spec was not found in this system!"); + } + }); + } + + /** Verify that no single endpoint contains regions in different clouds */ + private void validateEndpoints(DeploymentSpec deploymentSpec) { + for (var endpoint : deploymentSpec.endpoints()) { + var clouds = new HashSet<CloudName>(); + for (var region : endpoint.regions()) { + for (ZoneApi zone : controller.zoneRegistry().zones().all().in(region).zones()) { + clouds.add(zone.getCloudName()); + } + } + if (clouds.size() != 1) { + throw new IllegalArgumentException("Endpoint '" + endpoint.endpointId() + "' cannot contain regions in different clouds: " + + endpoint.regions().stream().sorted().collect(Collectors.toList())); + } + } + } + +} 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 f226fe9e4e3..998589dc23c 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 @@ -8,6 +8,7 @@ import com.yahoo.config.application.api.ValidationId; import com.yahoo.config.application.api.ValidationOverrides; import com.yahoo.config.provision.ApplicationId; import com.yahoo.config.provision.ApplicationName; +import com.yahoo.config.provision.CloudName; import com.yahoo.config.provision.Environment; import com.yahoo.config.provision.InstanceName; import com.yahoo.config.provision.RegionName; @@ -734,6 +735,40 @@ public class ControllerTest { assertTrue("Provisions certificate in " + Environment.dev, certificate.apply(app2).isPresent()); } + @Test + public void testDeployWithCrossCloudEndpoints() { + tester.controllerTester().zoneRegistry().setZones( + ZoneApiMock.fromId("prod.us-west-1"), + ZoneApiMock.newBuilder().with(CloudName.from("aws")).withId("prod.aws-us-east-1").build() + ); + var application = tester.createApplication("app1", "tenant1", 1L, 1L); + var applicationPackage = new ApplicationPackageBuilder() + .region("aws-us-east-1") + .region("us-west-1") + .endpoint("default", "default") // Contains to all regions by default + .build(); + + try { + tester.deployCompletely(application, applicationPackage); + fail("Expected exception"); + } catch (IllegalArgumentException e) { + assertEquals("Endpoint 'default' cannot contain regions in different clouds: [aws-us-east-1, us-west-1]", e.getMessage()); + } + + var applicationPackage2 = new ApplicationPackageBuilder() + .region("aws-us-east-1") + .region("us-west-1") + .endpoint("aws", "default", "aws-us-east-1") + .endpoint("foo", "default", "aws-us-east-1", "us-west-1") + .build(); + try { + tester.deployCompletely(application, applicationPackage2); + fail("Expected exception"); + } catch (IllegalArgumentException e) { + assertEquals("Endpoint 'foo' cannot contain regions in different clouds: [aws-us-east-1, us-west-1]", e.getMessage()); + } + } + private void runUpgrade(DeploymentTester tester, ApplicationId application, ApplicationVersion version) { Version next = Version.fromString("6.2"); tester.upgradeSystem(next); diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/integration/ZoneRegistryMock.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/integration/ZoneRegistryMock.java index cff0f8da463..b555ae24305 100644 --- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/integration/ZoneRegistryMock.java +++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/integration/ZoneRegistryMock.java @@ -50,6 +50,10 @@ public class ZoneRegistryMock extends AbstractComponent implements ZoneRegistry this(SystemName.main); } + /** + * This sets the default list of zones contained in this. If your test need a particular set of zones, use + * {@link #setZones(List)} instead of changing the default set.} + */ public ZoneRegistryMock(SystemName system) { this.system = system; setZones(List.of( |