diff options
author | Jon Marius Venstad <venstad@gmail.com> | 2019-10-25 09:43:37 +0200 |
---|---|---|
committer | Jon Marius Venstad <venstad@gmail.com> | 2019-11-05 09:17:58 +0100 |
commit | 9b680ebad5edcaf47293bb371507093cd37e6e11 (patch) | |
tree | 0a91b80b34cc09dfca011085bc86ba1ef37e2b69 | |
parent | b488286422f60248a50f8508ddd2f727bbb6d10e (diff) |
No more deprecated athenz domain or service usages
9 files changed, 91 insertions, 189 deletions
diff --git a/config-model-api/abi-spec.json b/config-model-api/abi-spec.json index 7f4273c827b..f1f668cba6d 100644 --- a/config-model-api/abi-spec.json +++ b/config-model-api/abi-spec.json @@ -340,8 +340,7 @@ "public java.util.List steps()", "public java.util.List zones()", "public java.util.Optional athenzDomain()", - "public java.util.Optional athenzService(com.yahoo.config.provision.Environment, com.yahoo.config.provision.RegionName)", - "public java.util.Optional athenzService(com.yahoo.config.provision.InstanceName, com.yahoo.config.provision.Environment, com.yahoo.config.provision.RegionName)", + "public java.util.Optional athenzService()", "public com.yahoo.config.application.api.Notifications notifications()", "public java.util.List endpoints()", "public java.lang.String xmlForm()", @@ -586,4 +585,4 @@ "public static final com.yahoo.config.application.api.ValidationOverrides all" ] } -}
\ No newline at end of file +} 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 0fc76e26d34..39453de5cdf 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 @@ -186,28 +186,17 @@ public class DeploymentSpec { /** Returns the Athenz domain set on the root tag, if any */ public Optional<AthenzDomain> athenzDomain() { return athenzDomain; } - /** Returns the Athenz service to use for the given environment and region, if any */ - // TODO: Remove after November 2019 - public Optional<AthenzService> athenzService(Environment environment, RegionName region) { - Optional<AthenzService> service = Optional.empty(); - if (hasSingleInstance(steps)) - service = singleInstance().athenzService(environment, region); - if (service.isPresent()) - return service; - return this.athenzService; - } - - /** - * Returns the Athenz service to use for the given instance, environment and region, if any. - * This returns the default set on the deploy tag (if any) if nothing is set for this particular - * combination of instance, environment and region, and also if that combination is not specified - * at all in services. - */ - public Optional<AthenzService> athenzService(InstanceName instanceName, Environment environment, RegionName region) { - Optional<DeploymentInstanceSpec> instance = instance(instanceName); - if (instance.isEmpty()) return this.athenzService; - return instance.get().athenzService(environment, region).or(() -> this.athenzService); - } + /** Returns the Athenz service set on the root tag, if any */ + // athenz-service can be overridden on almost all tags, and with legacy mode + standard + environment and region variants + // + tester application services it gets complicated, but: + // 1. any deployment outside dev/perf should happen only with declared instances, implicit or not, which means the spec for + // that instance should provide the correct service, based on environment and region, and we should not fall back to this; and + // 2. any deployment to dev/perf can only have the root or instance tags' value for service, which means we can ignore variants; and + // a. for single-instance specs the service is always set on the root tag, and deploying under an unknown instance leads here, and + // b. for multi-instance specs the root tag may or may not have a service, and unknown instances also lead here; and + // 3. any tester application deployment is always an unknown instance, and always gets here, but there should not be any reason + // to have environment, instance or region variants on those. + public Optional<AthenzService> athenzService() { return this.athenzService; } // TODO: Remove after November 2019 public Notifications notifications() { return singleInstance().notifications(); } diff --git a/config-model-api/src/test/java/com/yahoo/config/application/api/DeploymentSpecDeprecatedAPITest.java b/config-model-api/src/test/java/com/yahoo/config/application/api/DeploymentSpecDeprecatedAPITest.java index b0ebc98e12b..2724cecec6a 100644 --- a/config-model-api/src/test/java/com/yahoo/config/application/api/DeploymentSpecDeprecatedAPITest.java +++ b/config-model-api/src/test/java/com/yahoo/config/application/api/DeploymentSpecDeprecatedAPITest.java @@ -384,83 +384,6 @@ public class DeploymentSpecDeprecatedAPITest { } @Test - public void athenz_config_is_read_from_deployment() { - StringReader r = new StringReader( - "<deployment athenz-domain='domain' athenz-service='service'>\n" + - " <prod>\n" + - " <region active='true'>us-west-1</region>\n" + - " </prod>\n" + - "</deployment>" - ); - DeploymentSpec spec = DeploymentSpec.fromXml(r); - assertEquals(spec.athenzDomain().get().value(), "domain"); - assertEquals(spec.athenzService(Environment.prod, RegionName.from("us-west-1")).get().value(), "service"); - } - - @Test - public void athenz_config_is_propagated_through_parallel_zones() { - StringReader r = new StringReader( - "<deployment athenz-domain='domain' athenz-service='service'>" + - " <prod athenz-service='prod-service'>" + - " <region active='true'>us-central-1</region>" + - " <parallel>" + - " <region active='true'>us-west-1</region>" + - " <region active='true'>us-east-3</region>" + - " </parallel>" + - " </prod>" + - "</deployment>" - ); - DeploymentSpec spec = DeploymentSpec.fromXml(r); - assertEquals("domain", spec.athenzDomain().get().value()); - assertEquals("prod-service", spec.athenzService(Environment.prod, - RegionName.from("us-central-1")).get().value()); - assertEquals("prod-service", spec.athenzService(Environment.prod, - RegionName.from("us-west-1")).get().value()); - assertEquals("prod-service", spec.athenzService(Environment.prod, - RegionName.from("us-east-3")).get().value()); - assertEquals("domain", spec.athenzDomain().get().value()); - } - - @Test - public void athenz_service_is_overridden_from_environment() { - StringReader r = new StringReader( - "<deployment athenz-domain='domain' athenz-service='service'>\n" + - " <test/>\n" + - " <prod athenz-service='prod-service'>\n" + - " <region active='true'>us-west-1</region>\n" + - " </prod>\n" + - "</deployment>" - ); - DeploymentSpec spec = DeploymentSpec.fromXml(r); - assertEquals(spec.athenzDomain().get().value(), "domain"); - assertEquals(spec.athenzService(Environment.prod, RegionName.from("us-west-1")).get().value(), "prod-service"); - } - - @Test(expected = IllegalArgumentException.class) - public void it_fails_when_athenz_service_is_not_defined() { - StringReader r = new StringReader( - "<deployment athenz-domain='domain'>\n" + - " <prod>\n" + - " <region active='true'>us-west-1</region>\n" + - " </prod>\n" + - "</deployment>" - ); - DeploymentSpec spec = DeploymentSpec.fromXml(r); - } - - @Test(expected = IllegalArgumentException.class) - public void it_fails_when_athenz_service_is_configured_but_not_athenz_domain() { - StringReader r = new StringReader( - "<deployment>\n" + - " <prod athenz-service='service'>\n" + - " <region active='true'>us-west-1</region>\n" + - " </prod>\n" + - "</deployment>" - ); - DeploymentSpec spec = DeploymentSpec.fromXml(r); - } - - @Test public void noNotifications() { assertEquals(Notifications.none(), DeploymentSpec.fromXml("<deployment />").notifications()); diff --git a/config-model-api/src/test/java/com/yahoo/config/application/api/DeploymentSpecTest.java b/config-model-api/src/test/java/com/yahoo/config/application/api/DeploymentSpecTest.java index 7b571417ef8..ff51c336658 100644 --- a/config-model-api/src/test/java/com/yahoo/config/application/api/DeploymentSpecTest.java +++ b/config-model-api/src/test/java/com/yahoo/config/application/api/DeploymentSpecTest.java @@ -605,15 +605,10 @@ public class DeploymentSpecTest { ); DeploymentSpec spec = DeploymentSpec.fromXml(r); assertEquals("domain", spec.athenzDomain().get().value()); - assertEquals("service", spec.athenzService(InstanceName.from("instance1"), - Environment.prod, - RegionName.from("us-west-1")).get().value()); - assertEquals("service", spec.athenzService(InstanceName.from("non-existent"), - Environment.prod, - RegionName.from("us-west-1")).get().value()); assertEquals("domain", spec.requireInstance("instance1").athenzDomain().get().value()); + assertEquals("service", spec.athenzService().get().value()); assertEquals("service", spec.requireInstance("instance1").athenzService(Environment.prod, - RegionName.from("us-west-1")).get().value()); + RegionName.from("us-west-1")).get().value()); } @Test @@ -633,19 +628,15 @@ public class DeploymentSpecTest { ); DeploymentSpec spec = DeploymentSpec.fromXml(r); assertEquals("domain", spec.athenzDomain().get().value()); - assertEquals("prod-service", spec.athenzService(InstanceName.from("instance1"), - Environment.prod, - RegionName.from("us-west-1")).get().value()); - assertEquals("service", spec.athenzService(InstanceName.from("non-existent"), - Environment.prod, - RegionName.from("us-west-1")).get().value()); + assertEquals("service", spec.athenzService().get().value()); + assertEquals("domain", spec.requireInstance("instance1").athenzDomain().get().value()); assertEquals("prod-service", spec.requireInstance("instance1").athenzService(Environment.prod, - RegionName.from("us-central-1")).get().value()); + RegionName.from("us-central-1")).get().value()); assertEquals("prod-service", spec.requireInstance("instance1").athenzService(Environment.prod, - RegionName.from("us-west-1")).get().value()); + RegionName.from("us-west-1")).get().value()); assertEquals("prod-service", spec.requireInstance("instance1").athenzService(Environment.prod, - RegionName.from("us-east-3")).get().value()); + RegionName.from("us-east-3")).get().value()); } @Test @@ -674,12 +665,6 @@ public class DeploymentSpecTest { ); DeploymentSpec spec = DeploymentSpec.fromXml(r); assertEquals("domain", spec.athenzDomain().get().value()); - assertEquals("service", spec.athenzService(InstanceName.from("instance1"), - Environment.prod, - RegionName.from("us-west-1")).get().value()); - assertEquals("service", spec.athenzService(InstanceName.from("non-existent"), - Environment.prod, - RegionName.from("us-west-1")).get().value()); assertEquals("domain", spec.requireInstance("instance1").athenzDomain().get().value()); assertEquals("service", spec.requireInstance("instance1").athenzService(Environment.prod, RegionName.from("us-west-1")).get().value()); @@ -701,11 +686,10 @@ public class DeploymentSpecTest { "</deployment>" ); DeploymentSpec spec = DeploymentSpec.fromXml(r); + assertEquals(Optional.empty(), spec.athenzDomain()); + assertEquals(Optional.empty(), spec.athenzService()); assertEquals(spec.requireInstance("default").athenzDomain().get().value(), "domain"); assertEquals(spec.requireInstance("default").athenzService(Environment.prod, RegionName.from("us-west-1")).get().value(), "service"); - assertEquals(Optional.empty(), spec.athenzService(InstanceName.from("non-existent"), - Environment.prod, - RegionName.from("us-west-1"))); } @Test @@ -722,17 +706,9 @@ public class DeploymentSpecTest { ); DeploymentSpec spec = DeploymentSpec.fromXml(r); assertEquals(spec.requireInstance("default").athenzDomain().get().value(), "domain"); - assertEquals(spec.athenzService(InstanceName.from("default"), - Environment.prod, - RegionName.from("us-west-1")).get().value(), - "prod-service"); - assertEquals(spec.requireInstance("default").athenzService(Environment.prod, - RegionName.from("us-west-1")).get().value(), - "prod-service"); - assertEquals(spec.athenzService(InstanceName.from("non-existent"), - Environment.prod, - RegionName.from("us-west-1")).get().value(), - "service"); + assertEquals("prod-service", + spec.requireInstance("default").athenzService(Environment.prod, + RegionName.from("us-west-1")).get().value()); } @Test(expected = IllegalArgumentException.class) diff --git a/config-model-api/src/test/java/com/yahoo/config/application/api/DeploymentSpecWithoutInstanceTest.java b/config-model-api/src/test/java/com/yahoo/config/application/api/DeploymentSpecWithoutInstanceTest.java index 7805b73cc6a..b5af354aead 100644 --- a/config-model-api/src/test/java/com/yahoo/config/application/api/DeploymentSpecWithoutInstanceTest.java +++ b/config-model-api/src/test/java/com/yahoo/config/application/api/DeploymentSpecWithoutInstanceTest.java @@ -388,16 +388,14 @@ public class DeploymentSpecWithoutInstanceTest { ); DeploymentSpec spec = DeploymentSpec.fromXml(r); assertEquals("domain", spec.athenzDomain().get().value()); - assertEquals("prod-service", spec.athenzService(InstanceName.from("default"), - Environment.prod, - RegionName.from("us-central-1")).get().value()); - assertEquals("prod-service", spec.athenzService(InstanceName.from("default"), - Environment.prod, - RegionName.from("us-west-1")).get().value()); - assertEquals("prod-service", spec.athenzService(InstanceName.from("default"), - Environment.prod, - RegionName.from("us-east-3")).get().value()); - assertEquals("domain", spec.athenzDomain().get().value()); + assertEquals("domain", spec.requireInstance("default").athenzDomain().get().value()); + assertEquals("service", spec.athenzService().get().value()); + assertEquals("prod-service", spec.requireInstance("default").athenzService(Environment.prod, RegionName.from("us-central-1")) + .get().value()); + assertEquals("prod-service", spec.requireInstance("default").athenzService(Environment.prod, RegionName.from("us-west-1")) + .get().value()); + assertEquals("prod-service", spec.requireInstance("default").athenzService(Environment.prod, RegionName.from("us-east-3")) + .get().value()); } @Test @@ -411,6 +409,7 @@ public class DeploymentSpecWithoutInstanceTest { "</deployment>" ); DeploymentSpec spec = DeploymentSpec.fromXml(r); + assertEquals("service", spec.athenzService().get().value()); assertEquals(spec.requireInstance("default").athenzDomain().get().value(), "domain"); assertEquals(spec.requireInstance("default").athenzService(Environment.prod, RegionName.from("us-west-1")).get().value(), "prod-service"); } diff --git a/config-model/src/main/java/com/yahoo/vespa/model/container/xml/ContainerModelBuilder.java b/config-model/src/main/java/com/yahoo/vespa/model/container/xml/ContainerModelBuilder.java index 81e0b48090d..484021ad4d5 100644 --- a/config-model/src/main/java/com/yahoo/vespa/model/container/xml/ContainerModelBuilder.java +++ b/config-model/src/main/java/com/yahoo/vespa/model/container/xml/ContainerModelBuilder.java @@ -876,9 +876,14 @@ public class ContainerModelBuilder extends ConfigModelBuilder<ContainerModel> { String athenzDnsSuffix, Zone zone, DeploymentSpec spec) { - spec.athenzDomain().ifPresent(domain -> { - AthenzService service = spec.athenzService(app.getApplicationId().instance(), zone.environment(), zone.region()) - .orElseThrow(() -> new RuntimeException("Missing Athenz service configuration in instance '" + app.getApplicationId().instance() + "'")); + spec.instance(app.getApplicationId().instance()) + .flatMap(instanceSpec -> instanceSpec.athenzDomain()) + .or(() -> spec.athenzDomain()) + .ifPresent(domain -> { + AthenzService service = spec.instance(app.getApplicationId().instance()) + .flatMap(instanceSpec -> instanceSpec.athenzService(zone.environment(), zone.region())) + .or(() -> spec.athenzService()) + .orElseThrow(() -> new RuntimeException("Missing Athenz service configuration in instance '" + app.getApplicationId().instance() + "'")); String zoneDnsSuffix = zone.environment().value() + "-" + zone.region().value() + "." + athenzDnsSuffix; IdentityProvider identityProvider = new IdentityProvider(domain, service, getLoadBalancerName(loadBalancerName, configServerSpecs), ztsUrl, zoneDnsSuffix, zone); cluster.addComponent(identityProvider); 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 7c718518129..71cfc679ca7 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 @@ -945,50 +945,61 @@ public class ApplicationController { public void verifyApplicationIdentityConfiguration(TenantName tenantName, ApplicationPackage applicationPackage, Optional<Principal> deployer) { verifyAllowedLaunchAthenzService(applicationPackage.deploymentSpec()); - applicationPackage.deploymentSpec().athenzDomain().ifPresent(identityDomain -> { - Tenant tenant = controller.tenants().require(tenantName); - deployer.filter(AthenzPrincipal.class::isInstance) - .map(AthenzPrincipal.class::cast) - .map(AthenzPrincipal::getIdentity) - .filter(AthenzUser.class::isInstance) - .ifPresentOrElse(user -> { - if ( ! ((AthenzFacade) accessControl).hasTenantAdminAccess(user, new AthenzDomain(identityDomain.value()))) - throw new IllegalArgumentException("User " + user.getFullName() + " is not allowed to launch " + - "services in Athenz domain " + identityDomain.value() + ". " + - "Please reach out to the domain admin."); - }, - () -> { - if (tenant.type() != Tenant.Type.athenz) - throw new IllegalArgumentException("Athenz domain defined in deployment.xml, but no " + - "Athenz domain for tenant " + tenantName.value()); - - AthenzDomain tenantDomain = ((AthenzTenant) tenant).domain(); - if ( ! Objects.equals(tenantDomain.getName(), identityDomain.value())) - throw new IllegalArgumentException("Athenz domain in deployment.xml: [" + identityDomain.value() + "] " + - "must match tenant domain: [" + tenantDomain.getName() + "]"); - }); - }); + Tenant tenant = controller.tenants().require(tenantName); + Stream.concat(applicationPackage.deploymentSpec().athenzDomain().stream(), + applicationPackage.deploymentSpec().instances().stream() + .flatMap(spec -> spec.athenzDomain().stream())) + .distinct() + .forEach(identityDomain -> { + deployer.filter(AthenzPrincipal.class::isInstance) + .map(AthenzPrincipal.class::cast) + .map(AthenzPrincipal::getIdentity) + .filter(AthenzUser.class::isInstance) + .ifPresentOrElse(user -> { + if ( ! ((AthenzFacade) accessControl).hasTenantAdminAccess(user, new AthenzDomain(identityDomain.value()))) + throw new IllegalArgumentException("User " + user.getFullName() + " is not allowed to launch " + + "services in Athenz domain " + identityDomain.value() + ". " + + "Please reach out to the domain admin."); + }, + () -> { + if (tenant.type() != Tenant.Type.athenz) + throw new IllegalArgumentException("Athenz domain defined in deployment.xml, but no " + + "Athenz domain for tenant " + tenantName.value()); + + AthenzDomain tenantDomain = ((AthenzTenant) tenant).domain(); + if ( ! Objects.equals(tenantDomain.getName(), identityDomain.value())) + throw new IllegalArgumentException("Athenz domain in deployment.xml: [" + identityDomain.value() + "] " + + "must match tenant domain: [" + tenantDomain.getName() + "]"); + }); + }); } /* * Verifies that the configured athenz service (if any) can be launched. */ private void verifyAllowedLaunchAthenzService(DeploymentSpec deploymentSpec) { - deploymentSpec.athenzDomain().ifPresent(athenzDomain -> { - controller.zoneRegistry().zones().reachable().ids() - .forEach(zone -> { - AthenzIdentity configServerAthenzIdentity = controller.zoneRegistry().getConfigServerHttpsIdentity(zone); - deploymentSpec.athenzService(zone.environment(), zone.region()) - .map(service -> new AthenzService(athenzDomain.value(), service.value())) - .ifPresent(service -> { - boolean allowedToLaunch = ((AthenzFacade) accessControl).canLaunch(configServerAthenzIdentity, service); - if (!allowedToLaunch) - throw new IllegalArgumentException("Not allowed to launch Athenz service " + service.getFullName()); - }); - }); + controller.zoneRegistry().zones().reachable().ids().forEach(zone -> { + AthenzIdentity configServerAthenzIdentity = controller.zoneRegistry().getConfigServerHttpsIdentity(zone); + deploymentSpec.athenzDomain().ifPresent(domain -> { + deploymentSpec.athenzService().ifPresent(service -> { + verifyAthenzServiceCanBeLaunchedBy(configServerAthenzIdentity, new AthenzService(domain.value(), service.value())); + }); + }); + deploymentSpec.instances().forEach(spec -> { + spec.athenzDomain().ifPresent(domain -> { + spec.athenzService(zone.environment(), zone.region()).ifPresent(service -> { + verifyAthenzServiceCanBeLaunchedBy(configServerAthenzIdentity, new AthenzService(domain.value(), service.value())); + }); + }); + }); }); } + private void verifyAthenzServiceCanBeLaunchedBy(AthenzIdentity configServerAthenzIdentity, AthenzService athenzService) { + if ( ! ((AthenzFacade) accessControl).canLaunch(configServerAthenzIdentity, athenzService)) + throw new IllegalArgumentException("Not allowed to launch Athenz service " + athenzService.getFullName()); + } + /** Returns the latest known version within the given major. */ private Optional<Version> lastCompatibleVersion(int targetMajorVersion) { return controller.versionStatus().versions().stream() diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/InternalStepRunner.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/InternalStepRunner.java index 9df0dff3966..ce5a2a8dd21 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/InternalStepRunner.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/InternalStepRunner.java @@ -657,7 +657,8 @@ public class InternalStepRunner implements StepRunner { .orElse(zone.region().value().contains("aws-") ? DEFAULT_TESTER_RESOURCES_AWS : DEFAULT_TESTER_RESOURCES)); byte[] testPackage = controller.applications().applicationStore().getTester(id.application().tenant(), id.application().application(), version); - byte[] deploymentXml = deploymentXml(spec.athenzDomain(), spec.athenzService(zone.environment(), zone.region())); + byte[] deploymentXml = deploymentXml(spec.requireInstance(id.application().instance()).athenzDomain(), + spec.requireInstance(id.application().instance()).athenzService(zone.environment(), zone.region())); try (ZipBuilder zipBuilder = new ZipBuilder(testPackage.length + servicesXml.length + 1000)) { zipBuilder.add(testPackage); diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/deployment/InternalStepRunnerTest.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/deployment/InternalStepRunnerTest.java index d50399c6c78..2320ca41b49 100644 --- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/deployment/InternalStepRunnerTest.java +++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/deployment/InternalStepRunnerTest.java @@ -95,8 +95,7 @@ public class InternalStepRunnerTest { .application(app.testerId().id(), JobType.stagingTest.zone(system())).get() .applicationPackage().deploymentSpec(); assertEquals("domain", spec.athenzDomain().get().value()); - ZoneId zone = JobType.stagingTest.zone(system()); - assertEquals("service", spec.athenzService(zone.environment(), zone.region()).get().value()); + assertEquals("service", spec.athenzService().get().value()); } @Test |