From be15742773154a22269e87ac80a8b7ecd072032b Mon Sep 17 00:00:00 2001 From: Morten Tokle Date: Mon, 30 Aug 2021 11:41:13 +0200 Subject: Stop accepting application roles in deploy --- .../main/java/com/yahoo/config/model/api/ModelContext.java | 3 ++- .../java/com/yahoo/config/model/deploy/TestProperties.java | 1 - .../yahoo/vespa/config/server/deploy/ModelContextImpl.java | 8 -------- .../config/server/modelfactory/ActivatedModelsBuilder.java | 2 -- .../yahoo/vespa/config/server/session/PrepareParams.java | 14 ++------------ .../yahoo/vespa/config/server/session/SessionPreparer.java | 12 ------------ .../vespa/config/server/tenant/ApplicationRolesStore.java | 2 +- .../yahoo/vespa/config/server/ModelContextImplTest.java | 1 - 8 files changed, 5 insertions(+), 38 deletions(-) diff --git a/config-model-api/src/main/java/com/yahoo/config/model/api/ModelContext.java b/config-model-api/src/main/java/com/yahoo/config/model/api/ModelContext.java index f6861c4f5cf..224fbf194c9 100644 --- a/config-model-api/src/main/java/com/yahoo/config/model/api/ModelContext.java +++ b/config-model-api/src/main/java/com/yahoo/config/model/api/ModelContext.java @@ -118,7 +118,8 @@ public interface ModelContext { default Optional athenzDomain() { return Optional.empty(); } - Optional applicationRoles(); + // applicationRoles is no longer in use and should be removed. Replaced by AwsEnvironment. Remove after 7.458 + default Optional applicationRoles() { return Optional.empty(); } default Quota quota() { return Quota.unlimited(); } diff --git a/config-model/src/main/java/com/yahoo/config/model/deploy/TestProperties.java b/config-model/src/main/java/com/yahoo/config/model/deploy/TestProperties.java index 7d8932b9d00..38d1e603570 100644 --- a/config-model/src/main/java/com/yahoo/config/model/deploy/TestProperties.java +++ b/config-model/src/main/java/com/yahoo/config/model/deploy/TestProperties.java @@ -86,7 +86,6 @@ public class TestProperties implements ModelContext.Properties, ModelContext.Fea @Override public double defaultTermwiseLimit() { return defaultTermwiseLimit; } @Override public boolean useThreePhaseUpdates() { return useThreePhaseUpdates; } @Override public Optional athenzDomain() { return Optional.ofNullable(athenzDomain); } - @Override public Optional applicationRoles() { return Optional.ofNullable(applicationRoles); } @Override public String responseSequencerType() { return responseSequencerType; } @Override public int defaultNumResponseThreads() { return responseNumThreads; } @Override public boolean skipCommunicationManagerThread() { return false; } diff --git a/configserver/src/main/java/com/yahoo/vespa/config/server/deploy/ModelContextImpl.java b/configserver/src/main/java/com/yahoo/vespa/config/server/deploy/ModelContextImpl.java index 529948179b1..a63700e0bb5 100644 --- a/configserver/src/main/java/com/yahoo/vespa/config/server/deploy/ModelContextImpl.java +++ b/configserver/src/main/java/com/yahoo/vespa/config/server/deploy/ModelContextImpl.java @@ -294,7 +294,6 @@ public class ModelContextImpl implements ModelContext { private final boolean isFirstTimeDeployment; private final Optional endpointCertificateSecrets; private final Optional athenzDomain; - private final Optional applicationRoles; private final Quota quota; private final List tenantSecretStores; private final SecretStore secretStore; @@ -312,7 +311,6 @@ public class ModelContextImpl implements ModelContext { FlagSource flagSource, Optional endpointCertificateSecrets, Optional athenzDomain, - Optional applicationRoles, Optional maybeQuota, List tenantSecretStores, SecretStore secretStore, @@ -331,7 +329,6 @@ public class ModelContextImpl implements ModelContext { this.isFirstTimeDeployment = isFirstTimeDeployment; this.endpointCertificateSecrets = endpointCertificateSecrets; this.athenzDomain = athenzDomain; - this.applicationRoles = applicationRoles; this.quota = maybeQuota.orElseGet(Quota::unlimited); this.tenantSecretStores = tenantSecretStores; this.secretStore = secretStore; @@ -389,11 +386,6 @@ public class ModelContextImpl implements ModelContext { @Override public Optional athenzDomain() { return athenzDomain; } - @Override - public Optional applicationRoles() { - return applicationRoles; - } - @Override public Quota quota() { return quota; } @Override diff --git a/configserver/src/main/java/com/yahoo/vespa/config/server/modelfactory/ActivatedModelsBuilder.java b/configserver/src/main/java/com/yahoo/vespa/config/server/modelfactory/ActivatedModelsBuilder.java index 2b7ce234777..ffb4550caf0 100644 --- a/configserver/src/main/java/com/yahoo/vespa/config/server/modelfactory/ActivatedModelsBuilder.java +++ b/configserver/src/main/java/com/yahoo/vespa/config/server/modelfactory/ActivatedModelsBuilder.java @@ -162,8 +162,6 @@ public class ActivatedModelsBuilder extends ModelsBuilder { .readEndpointCertificateMetadata(applicationId) .flatMap(new EndpointCertificateRetriever(secretStore)::readEndpointCertificateSecrets), zkClient.readAthenzDomain(), - new ApplicationRolesStore(curator, TenantRepository.getTenantPath(tenant)) - .readApplicationRoles(applicationId), zkClient.readQuota(), zkClient.readTenantSecretStores(), secretStore, diff --git a/configserver/src/main/java/com/yahoo/vespa/config/server/session/PrepareParams.java b/configserver/src/main/java/com/yahoo/vespa/config/server/session/PrepareParams.java index 071a0dd8f0c..9ed62a99708 100644 --- a/configserver/src/main/java/com/yahoo/vespa/config/server/session/PrepareParams.java +++ b/configserver/src/main/java/com/yahoo/vespa/config/server/session/PrepareParams.java @@ -53,8 +53,6 @@ public final class PrepareParams { static final String ENDPOINT_CERTIFICATE_METADATA_PARAM_NAME = "endpointCertificateMetadata"; static final String DOCKER_IMAGE_REPOSITORY = "dockerImageRepository"; static final String ATHENZ_DOMAIN = "athenzDomain"; - static final String APPLICATION_HOST_ROLE = "applicationHostRole"; - static final String APPLICATION_CONTAINER_ROLE = "applicationContainerRole"; static final String QUOTA_PARAM_NAME = "quota"; static final String TENANT_SECRET_STORES_PARAM_NAME = "tenantSecretStores"; static final String FORCE_PARAM_NAME = "force"; @@ -74,7 +72,6 @@ public final class PrepareParams { private final Optional endpointCertificateMetadata; private final Optional dockerImageRepository; private final Optional athenzDomain; - private final Optional applicationRoles; private final Optional quota; private final List tenantSecretStores; private final List operatorCertificates; @@ -84,7 +81,7 @@ public final class PrepareParams { List containerEndpoints, Optional endpointCertificateMetadata, Optional dockerImageRepository, Optional athenzDomain, - Optional applicationRoles, Optional quota, List tenantSecretStores, + Optional quota, List tenantSecretStores, boolean force, boolean waitForResourcesInPrepare, List operatorCertificates) { this.timeoutBudget = timeoutBudget; this.applicationId = Objects.requireNonNull(applicationId); @@ -97,7 +94,6 @@ public final class PrepareParams { this.endpointCertificateMetadata = endpointCertificateMetadata; this.dockerImageRepository = dockerImageRepository; this.athenzDomain = athenzDomain; - this.applicationRoles = applicationRoles; this.quota = quota; this.tenantSecretStores = tenantSecretStores; this.force = force; @@ -265,7 +261,7 @@ public final class PrepareParams { return new PrepareParams(applicationId, timeoutBudget, ignoreValidationErrors, dryRun, verbose, isBootstrap, vespaVersion, containerEndpoints, endpointCertificateMetadata, dockerImageRepository, athenzDomain, - applicationRoles, quota, tenantSecretStores, force, waitForResourcesInPrepare, + quota, tenantSecretStores, force, waitForResourcesInPrepare, operatorCertificates); } } @@ -281,7 +277,6 @@ public final class PrepareParams { .endpointCertificateMetadata(request.getProperty(ENDPOINT_CERTIFICATE_METADATA_PARAM_NAME)) .dockerImageRepository(request.getProperty(DOCKER_IMAGE_REPOSITORY)) .athenzDomain(request.getProperty(ATHENZ_DOMAIN)) - .applicationRoles(ApplicationRoles.fromString(request.getProperty(APPLICATION_HOST_ROLE), request.getProperty(APPLICATION_CONTAINER_ROLE))) .quota(request.getProperty(QUOTA_PARAM_NAME)) .tenantSecretStores(request.getProperty(TENANT_SECRET_STORES_PARAM_NAME)) .force(request.getBooleanProperty(FORCE_PARAM_NAME)) @@ -304,7 +299,6 @@ public final class PrepareParams { .endpointCertificateMetadata(deserialize(params.field(ENDPOINT_CERTIFICATE_METADATA_PARAM_NAME), EndpointCertificateMetadataSerializer::fromSlime)) .dockerImageRepository(SlimeUtils.optionalString(params.field(DOCKER_IMAGE_REPOSITORY)).orElse(null)) .athenzDomain(SlimeUtils.optionalString(params.field(ATHENZ_DOMAIN)).orElse(null)) - .applicationRoles(ApplicationRoles.fromString(SlimeUtils.optionalString(params.field(APPLICATION_HOST_ROLE)).orElse(null), SlimeUtils.optionalString(params.field(APPLICATION_CONTAINER_ROLE)).orElse(null))) .quota(deserialize(params.field(QUOTA_PARAM_NAME), Quota::fromSlime)) .tenantSecretStores(deserialize(params.field(TENANT_SECRET_STORES_PARAM_NAME), TenantSecretStoreSerializer::listFromSlime, List.of())) .force(booleanValue(params, FORCE_PARAM_NAME)) @@ -414,10 +408,6 @@ public final class PrepareParams { public Optional athenzDomain() { return athenzDomain; } - public Optional applicationRoles() { - return applicationRoles; - } - public Optional quota() { return quota; } diff --git a/configserver/src/main/java/com/yahoo/vespa/config/server/session/SessionPreparer.java b/configserver/src/main/java/com/yahoo/vespa/config/server/session/SessionPreparer.java index b9f79763a41..14dddf458cd 100644 --- a/configserver/src/main/java/com/yahoo/vespa/config/server/session/SessionPreparer.java +++ b/configserver/src/main/java/com/yahoo/vespa/config/server/session/SessionPreparer.java @@ -132,7 +132,6 @@ public class SessionPreparer { preparation.writeStateZK(); preparation.writeEndpointCertificateMetadataZK(); preparation.writeContainerEndpointsZK(); - preparation.writeApplicationRoles(); } log.log(Level.FINE, () -> "time used " + params.getTimeoutBudget().timesUsed() + " : " + applicationId); return preparation.result(); @@ -161,8 +160,6 @@ public class SessionPreparer { private final EndpointCertificateMetadataStore endpointCertificateMetadataStore; private final Optional endpointCertificateMetadata; private final Optional athenzDomain; - private final ApplicationRolesStore applicationRolesStore; - private final Optional applicationRoles; private final ApplicationPackage applicationPackage; private final SessionZooKeeperClient sessionZooKeeperClient; @@ -193,9 +190,6 @@ public class SessionPreparer { .flatMap(endpointCertificateRetriever::readEndpointCertificateSecrets); this.containerEndpoints = readEndpointsIfNull(params.containerEndpoints()); this.athenzDomain = params.athenzDomain(); - this.applicationRolesStore = new ApplicationRolesStore(curator, tenantPath); - this.applicationRoles = params.applicationRoles() - .or(() -> applicationRolesStore.readApplicationRoles(applicationId)); this.properties = new ModelContextImpl.Properties(params.getApplicationId(), configserverConfig, zone, @@ -205,7 +199,6 @@ public class SessionPreparer { flagSource, endpointCertificateSecrets, athenzDomain, - applicationRoles, params.quota(), params.tenantSecretStores(), secretStore, @@ -298,11 +291,6 @@ public class SessionPreparer { checkTimeout("write container endpoints to zookeeper"); } - void writeApplicationRoles() { - applicationRoles.ifPresent(roles -> applicationRolesStore.writeApplicationRoles(applicationId, roles)); - checkTimeout("write application roles to zookeeper"); - } - PrepareResult result() { return prepareResult; } diff --git a/configserver/src/main/java/com/yahoo/vespa/config/server/tenant/ApplicationRolesStore.java b/configserver/src/main/java/com/yahoo/vespa/config/server/tenant/ApplicationRolesStore.java index a41e5465509..2dedce41ed8 100644 --- a/configserver/src/main/java/com/yahoo/vespa/config/server/tenant/ApplicationRolesStore.java +++ b/configserver/src/main/java/com/yahoo/vespa/config/server/tenant/ApplicationRolesStore.java @@ -14,9 +14,9 @@ import java.util.Optional; /** * Stores application roles for an application. - * * @author mortent */ + // TODO: Remove and clean up zk after 7.458 public class ApplicationRolesStore { private final Path path; diff --git a/configserver/src/test/java/com/yahoo/vespa/config/server/ModelContextImplTest.java b/configserver/src/test/java/com/yahoo/vespa/config/server/ModelContextImplTest.java index 0acf4404326..d97f809da6e 100644 --- a/configserver/src/test/java/com/yahoo/vespa/config/server/ModelContextImplTest.java +++ b/configserver/src/test/java/com/yahoo/vespa/config/server/ModelContextImplTest.java @@ -72,7 +72,6 @@ public class ModelContextImplTest { null, Optional.empty(), Optional.empty(), - Optional.empty(), List.of(), new SecretStoreProvider().get(), List.of()), -- cgit v1.2.3 From b5d0aa03a886725d9106f0943af539686f49be0f Mon Sep 17 00:00:00 2001 From: Morten Tokle Date: Mon, 30 Aug 2021 11:50:49 +0200 Subject: Skip application roles in deploy --- .../controller/api/application/v4/model/DeploymentData.java | 7 ------- .../yahoo/vespa/hosted/controller/ApplicationController.java | 12 +++++------- 2 files changed, 5 insertions(+), 14 deletions(-) diff --git a/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/application/v4/model/DeploymentData.java b/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/application/v4/model/DeploymentData.java index 55e1e879ef7..1608e72b0b6 100644 --- a/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/application/v4/model/DeploymentData.java +++ b/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/application/v4/model/DeploymentData.java @@ -34,7 +34,6 @@ public class DeploymentData { private final Optional endpointCertificateMetadata; private final Optional dockerImageRepo; private final Optional athenzDomain; - private final Optional tenantRoles; private final Quota quota; private final List tenantSecretStores; private final List operatorCertificates; @@ -44,7 +43,6 @@ public class DeploymentData { Optional endpointCertificateMetadata, Optional dockerImageRepo, Optional athenzDomain, - Optional tenantRoles, Quota quota, List tenantSecretStores, List operatorCertificates) { @@ -56,7 +54,6 @@ public class DeploymentData { this.endpointCertificateMetadata = requireNonNull(endpointCertificateMetadata); this.dockerImageRepo = requireNonNull(dockerImageRepo); this.athenzDomain = athenzDomain; - this.tenantRoles = tenantRoles; this.quota = quota; this.tenantSecretStores = tenantSecretStores; this.operatorCertificates = operatorCertificates; @@ -94,10 +91,6 @@ public class DeploymentData { return athenzDomain; } - public Optional tenantRoles() { - return tenantRoles; - } - public Quota quota() { return quota; } 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 804604cfc52..6557247e21a 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 @@ -363,7 +363,6 @@ public class ApplicationController { try (Lock deploymentLock = lockForDeployment(job.application(), zone)) { Set containerEndpoints; Optional endpointCertificateMetadata; - Optional tenantRoles = Optional.empty(); Run run = controller.jobController().last(job) .orElseThrow(() -> new IllegalStateException("No known run of '" + job + "'")); @@ -391,7 +390,7 @@ public class ApplicationController { } // Release application lock while doing the deployment, which is a lengthy task. // Carry out deployment without holding the application lock. - ActivateResult result = deploy(job.application(), applicationPackage, zone, platform, containerEndpoints, endpointCertificateMetadata, tenantRoles); + ActivateResult result = deploy(job.application(), applicationPackage, zone, platform, containerEndpoints, endpointCertificateMetadata); // Record the quota usage for this application var quotaUsage = deploymentQuotaUsage(zone, job.application()); @@ -473,7 +472,7 @@ public class ApplicationController { ApplicationPackage applicationPackage = new ApplicationPackage( artifactRepository.getSystemApplicationPackage(application.id(), zone, version) ); - return deploy(application.id(), applicationPackage, zone, version, Set.of(), /* No application cert */ Optional.empty(), Optional.empty()); + return deploy(application.id(), applicationPackage, zone, version, Set.of(), /* No application cert */ Optional.empty()); } else { throw new RuntimeException("This system application does not have an application package: " + application.id().toShortString()); } @@ -481,13 +480,12 @@ public class ApplicationController { /** Deploys the given tester application to the given zone. */ public ActivateResult deployTester(TesterId tester, ApplicationPackage applicationPackage, ZoneId zone, Version platform) { - return deploy(tester.id(), applicationPackage, zone, platform, Set.of(), /* No application cert for tester*/ Optional.empty(), Optional.empty()); + return deploy(tester.id(), applicationPackage, zone, platform, Set.of(), /* No application cert for tester*/ Optional.empty()); } private ActivateResult deploy(ApplicationId application, ApplicationPackage applicationPackage, ZoneId zone, Version platform, Set endpoints, - Optional endpointCertificateMetadata, - Optional tenantRoles) { + Optional endpointCertificateMetadata) { try { Optional dockerImageRepo = Optional.ofNullable( dockerImageRepoFlag @@ -521,7 +519,7 @@ public class ApplicationController { ConfigServer.PreparedApplication preparedApplication = configServer.deploy(new DeploymentData(application, zone, applicationPackage.zippedContent(), platform, endpoints, endpointCertificateMetadata, dockerImageRepo, domain, - tenantRoles, deploymentQuota, tenantSecretStores, operatorCertificates)); + deploymentQuota, tenantSecretStores, operatorCertificates)); return new ActivateResult(new RevisionId(applicationPackage.hash()), preparedApplication.prepareResponse(), applicationPackage.zippedContent().length); -- cgit v1.2.3 From da154c8926facc022abee11cd97f380f3328d6fd Mon Sep 17 00:00:00 2001 From: Morten Tokle Date: Mon, 30 Aug 2021 12:55:36 +0200 Subject: Fix tests --- .../config/server/ApplicationRepositoryTest.java | 22 --------------------- .../config/server/session/PrepareParamsTest.java | 23 ---------------------- 2 files changed, 45 deletions(-) diff --git a/configserver/src/test/java/com/yahoo/vespa/config/server/ApplicationRepositoryTest.java b/configserver/src/test/java/com/yahoo/vespa/config/server/ApplicationRepositoryTest.java index 665b603acfc..07a514f60b5 100644 --- a/configserver/src/test/java/com/yahoo/vespa/config/server/ApplicationRepositoryTest.java +++ b/configserver/src/test/java/com/yahoo/vespa/config/server/ApplicationRepositoryTest.java @@ -7,7 +7,6 @@ import com.yahoo.config.ConfigInstance; import com.yahoo.config.SimpletypesConfig; import com.yahoo.config.application.api.ApplicationMetaData; import com.yahoo.config.model.NullConfigModelRegistry; -import com.yahoo.config.model.api.ApplicationRoles; import com.yahoo.config.model.application.provider.FilesApplicationPackage; import com.yahoo.config.provision.AllocatedHosts; import com.yahoo.config.provision.ApplicationId; @@ -515,27 +514,6 @@ public class ApplicationRepositoryTest { assertEquals(expected.values, actual.values); } - @Test - public void deletesApplicationRoles() { - var tenant = applicationRepository.getTenant(applicationId()); - var applicationId = applicationId(tenant1); - var prepareParams = new PrepareParams.Builder().applicationId(applicationId) - .applicationRoles(ApplicationRoles.fromString("hostRole","containerRole")).build(); - deployApp(testApp, prepareParams); - var approlesStore = new ApplicationRolesStore(tenantRepository.getCurator(), tenant.getPath()); - var appRoles = approlesStore.readApplicationRoles(applicationId); - - // App roles present after deploy - assertTrue(appRoles.isPresent()); - assertEquals("hostRole", appRoles.get().applicationHostRole()); - assertEquals("containerRole", appRoles.get().applicationContainerRole()); - - assertTrue(applicationRepository.delete(applicationId)); - - // App roles deleted on application delete - assertTrue(approlesStore.readApplicationRoles(applicationId).isEmpty()); - } - @Test public void require_that_provision_info_can_be_read() { prepareAndActivate(testAppJdiscOnly); diff --git a/configserver/src/test/java/com/yahoo/vespa/config/server/session/PrepareParamsTest.java b/configserver/src/test/java/com/yahoo/vespa/config/server/session/PrepareParamsTest.java index f68e79ae266..19f665689a2 100644 --- a/configserver/src/test/java/com/yahoo/vespa/config/server/session/PrepareParamsTest.java +++ b/configserver/src/test/java/com/yahoo/vespa/config/server/session/PrepareParamsTest.java @@ -116,28 +116,6 @@ public class PrepareParamsTest { assertPrepareParamsEqual(prepareParams, prepareParamsJson); } - @Test - public void testCorrectParsingWithApplicationRoles() throws IOException { - String req = request + "&" + - PrepareParams.APPLICATION_HOST_ROLE + "=hostRole&" + - PrepareParams.APPLICATION_CONTAINER_ROLE + "=containerRole"; - var prepareParams = createParams(req, TenantName.from("foo")); - - Optional applicationRoles = prepareParams.applicationRoles(); - assertTrue(applicationRoles.isPresent()); - assertEquals("hostRole", applicationRoles.get().applicationHostRole()); - assertEquals("containerRole", applicationRoles.get().applicationContainerRole()); - - // Verify using json object - var slime = SlimeUtils.jsonToSlime(json); - var cursor = slime.get(); - cursor.setString(PrepareParams.APPLICATION_HOST_ROLE, "hostRole"); - cursor.setString(PrepareParams.APPLICATION_CONTAINER_ROLE, "containerRole"); - - PrepareParams prepareParamsJson = PrepareParams.fromJson(SlimeUtils.toJsonBytes(slime), TenantName.from("foo"), Duration.ofSeconds(60)); - assertPrepareParamsEqual(prepareParams, prepareParamsJson); - } - @Test public void testQuotaParsing() throws IOException { var quotaParam = "{\"clusterSize\": 23, \"budget\": 23232323}"; @@ -239,7 +217,6 @@ public class PrepareParamsTest { assertEquals(urlParams.endpointCertificateMetadata(), jsonParams.endpointCertificateMetadata()); assertEquals(urlParams.dockerImageRepository(), jsonParams.dockerImageRepository()); assertEquals(urlParams.athenzDomain(), jsonParams.athenzDomain()); - assertEquals(urlParams.applicationRoles(), jsonParams.applicationRoles()); assertEquals(urlParams.quota(), jsonParams.quota()); assertEquals(urlParams.tenantSecretStores(), jsonParams.tenantSecretStores()); } -- cgit v1.2.3