Merge pull request #18895 from vespa-engine/mortent/cleanup-application-roles-step1

Prepare for removing unused application roles

8 files changed, 3 insertions, 81 deletions

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> endpointCertificateSecrets;
         private final Optional<AthenzDomain> athenzDomain;
-        private final Optional<ApplicationRoles> applicationRoles;
         private final Quota quota;
         private final List<TenantSecretStore> tenantSecretStores;
         private final SecretStore secretStore;
@@ -312,7 +311,6 @@ public class ModelContextImpl implements ModelContext {
                           FlagSource flagSource,
                           Optional<EndpointCertificateSecrets> endpointCertificateSecrets,
                           Optional<AthenzDomain> athenzDomain,
-                          Optional<ApplicationRoles> applicationRoles,
                           Optional<Quota> maybeQuota,
                           List<TenantSecretStore> 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> athenzDomain() { return athenzDomain; }
 
-        @Override
-        public Optional<ApplicationRoles> 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<Application> {
                                                        .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> endpointCertificateMetadata;
     private final Optional<DockerImage> dockerImageRepository;
     private final Optional<AthenzDomain> athenzDomain;
-    private final Optional<ApplicationRoles> applicationRoles;
     private final Optional<Quota> quota;
     private final List<TenantSecretStore> tenantSecretStores;
     private final List<X509Certificate> operatorCertificates;
@@ -84,7 +81,7 @@ public final class PrepareParams {
                           List<ContainerEndpoint> containerEndpoints,
                           Optional<EndpointCertificateMetadata> endpointCertificateMetadata,
                           Optional<DockerImage> dockerImageRepository, Optional<AthenzDomain> athenzDomain,
-                          Optional<ApplicationRoles> applicationRoles, Optional<Quota> quota, List<TenantSecretStore> tenantSecretStores,
+                          Optional<Quota> quota, List<TenantSecretStore> tenantSecretStores,
                           boolean force, boolean waitForResourcesInPrepare, List<X509Certificate> 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> athenzDomain() { return athenzDomain; }
 
-    public Optional<ApplicationRoles> applicationRoles() {
-        return applicationRoles;
-    }
-
     public Optional<Quota> 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> endpointCertificateMetadata;
         private final Optional<AthenzDomain> athenzDomain;
-        private final ApplicationRolesStore applicationRolesStore;
-        private final Optional<ApplicationRoles> 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/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;
@@ -516,27 +515,6 @@ public class ApplicationRepositoryTest {
     }
 
     @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/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()),
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
@@ -117,28 +117,6 @@ public class PrepareParamsTest {
     }
 
     @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> 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}";
         var quotaEncoded = URLEncoder.encode(quotaParam, StandardCharsets.UTF_8);
@@ -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());
     }