Merge pull request #32190 from vespa-engine/remove-ckms-access-for-ex…

…ternalId_pt1_try2 Remove ckms access for external id pt1 try2
vespa-engine · Aug 20, 2024 · 2458418 · 2458418
2 parents 5557453 + 0978a3f
commit 2458418
Show file tree

Hide file tree

Showing 5 changed files with 8 additions and 57 deletions.
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
@@ -80,10 +80,10 @@
 import com.yahoo.vespa.model.container.component.ConnectionLogComponent;
 import com.yahoo.vespa.model.container.component.FileStatusHandlerComponent;
 import com.yahoo.vespa.model.container.component.Handler;
+import com.yahoo.vespa.model.container.component.SignificanceModelRegistry;
 import com.yahoo.vespa.model.container.component.SimpleComponent;
 import com.yahoo.vespa.model.container.component.SystemBindingPattern;
 import com.yahoo.vespa.model.container.component.UserBindingPattern;
-import com.yahoo.vespa.model.container.component.SignificanceModelRegistry;
 import com.yahoo.vespa.model.container.docproc.ContainerDocproc;
 import com.yahoo.vespa.model.container.docproc.DocprocChains;
 import com.yahoo.vespa.model.container.http.AccessControl;
@@ -339,7 +339,7 @@ private void addCloudSecretStore(ApplicationContainerCluster cluster, Element se
                 throw new IllegalArgumentException("No configured secret store named " + account);
 
             if (secretStore.getExternalId().isEmpty())
-                throw new IllegalArgumentException("No external ID has been set");
+                throw new IllegalArgumentException("No external ID has been set for secret store " + secretStore.getName());
 
             cloudSecretStore.addConfig(account, region, secretStore.getAwsId(), secretStore.getRole(), secretStore.getExternalId().get());
         }

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
@@ -25,18 +25,15 @@
 import com.yahoo.config.provision.DataplaneToken;
 import com.yahoo.config.provision.DockerImage;
 import com.yahoo.config.provision.HostName;
-import com.yahoo.config.provision.NodeResources;
 import com.yahoo.config.provision.NodeResources.Architecture;
 import com.yahoo.config.provision.SharedHosts;
 import com.yahoo.config.provision.Zone;
 import com.yahoo.container.jdisc.secretstore.SecretStore;
-import com.yahoo.vespa.config.server.tenant.SecretStoreExternalIdRetriever;
 import com.yahoo.vespa.flags.Dimension;
 import com.yahoo.vespa.flags.FlagSource;
 import com.yahoo.vespa.flags.Flags;
 import com.yahoo.vespa.flags.PermanentFlags;
 import com.yahoo.vespa.flags.StringFlag;
-import com.yahoo.vespa.flags.UnboundFlag;
 
 import java.io.File;
 import java.net.URI;
@@ -46,7 +43,6 @@
 import java.util.Optional;
 import java.util.Set;
 import java.util.concurrent.ExecutorService;
-import java.util.function.Predicate;
 
 import static com.yahoo.vespa.config.server.ConfigServerSpec.fromConfig;
 import static com.yahoo.vespa.flags.Dimension.CLUSTER_TYPE;
@@ -454,7 +450,7 @@ public String athenzDnsSuffix() {
 
         @Override
         public List<TenantSecretStore> tenantSecretStores() {
-            return SecretStoreExternalIdRetriever.populateExternalId(secretStore, applicationId.tenant(), zone.system(), tenantSecretStores);
+            return tenantSecretStores;
         }
 
         @Override public String jvmGCOptions(Optional<ClusterSpec.Type> clusterType) {

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
@@ -49,7 +49,6 @@
 import com.yahoo.vespa.config.server.tenant.ContainerEndpointsCache;
 import com.yahoo.vespa.config.server.tenant.EndpointCertificateMetadataStore;
 import com.yahoo.vespa.config.server.tenant.EndpointCertificateRetriever;
-import com.yahoo.vespa.config.server.tenant.SecretStoreExternalIdRetriever;
 import com.yahoo.vespa.config.server.tenant.TenantRepository;
 import com.yahoo.vespa.curator.Curator;
 import com.yahoo.vespa.flags.BooleanFlag;
@@ -350,16 +349,10 @@ void makeResult(AllocatedHosts allocatedHosts) {
         void writeStateZK(FileReference filereference) {
             log.log(Level.FINE, "Writing application package state to zookeeper");
 
-            // TODO: this can be removed when all existing tenant secret stores have externalId in zk
+            // TODO: remove when tenant secret store integration test passes
             var tenantSecretStores = params.tenantSecretStores();
-
             if (! tenantSecretStores.isEmpty() && zone.system().isPublic() && zone.cloud().name().equals(CloudName.AWS)) {
-                try {
-                    tenantSecretStores = SecretStoreExternalIdRetriever
-                            .populateExternalId(secretStore, applicationId.tenant(), zone.system(), params.tenantSecretStores());
-                } catch (InvalidApplicationException e) {
-                    log.warning(e.getMessage() + " Secret store was probably deleted.");
-                }
+               tenantSecretStores.forEach(ss -> log.info("Tenant secret store:\n" + ss));
             }
             writeStateToZooKeeper(sessionZooKeeperClient,
                                   preprocessedApplicationPackage,
@@ -373,7 +366,7 @@ void writeStateZK(FileReference filereference) {
                                   prepareResult.allocatedHosts(),
                                   athenzDomain,
                                   params.quota(),
-                                  tenantSecretStores,
+                                  params.tenantSecretStores(),
                                   params.operatorCertificates(),
                                   params.cloudAccount(),
                                   params.dataplaneTokens(),

diff --git a/configserver/src/main/java/com/yahoo/vespa/config/server/session/SessionRepository.java b/configserver/src/main/java/com/yahoo/vespa/config/server/session/SessionRepository.java
@@ -38,7 +38,6 @@
 import com.yahoo.vespa.config.server.monitoring.MetricUpdater;
 import com.yahoo.vespa.config.server.monitoring.Metrics;
 import com.yahoo.vespa.config.server.provision.HostProvisionerProvider;
-import com.yahoo.vespa.config.server.tenant.SecretStoreExternalIdRetriever;
 import com.yahoo.vespa.config.server.tenant.TenantRepository;
 import com.yahoo.vespa.config.server.zookeeper.SessionCounter;
 import com.yahoo.vespa.config.server.zookeeper.ZKApplication;
@@ -607,15 +606,10 @@ private void childEvent(CuratorFramework ignored, PathChildrenCacheEvent event)
 
     private void write(Session existingSession, LocalSession session, ApplicationId applicationId, Instant created) {
 
-        // TODO: this can be removed when all existing tenant secret stores have externalId in zk
+        // TODO: remove when tenant secret store integration test passes
         var tenantSecretStores = existingSession.getTenantSecretStores();
         if (! tenantSecretStores.isEmpty() && zone.system().isPublic() && zone.cloud().name().equals(CloudName.AWS)) {
-            try {
-                tenantSecretStores = SecretStoreExternalIdRetriever
-                        .populateExternalId(secretStore, applicationId.tenant(), zone.system(), existingSession.getTenantSecretStores());
-            } catch (InvalidApplicationException e) {
-                log.warning(e.getMessage() + " Secret store was probably deleted.");
-            }
+            tenantSecretStores.forEach(ss -> log.info("Existing tenant secret store:\n" + ss));
         }
         SessionSerializer sessionSerializer = new SessionSerializer();
         sessionSerializer.write(session.getSessionZooKeeperClient(),

diff --git a/configserver/src/test/java/com/yahoo/vespa/config/server/deploy/HostedDeployTest.java b/configserver/src/test/java/com/yahoo/vespa/config/server/deploy/HostedDeployTest.java
@@ -9,26 +9,21 @@
 import com.yahoo.config.model.api.ModelCreateResult;
 import com.yahoo.config.model.api.ModelFactory;
 import com.yahoo.config.model.api.ServiceInfo;
-import com.yahoo.config.model.api.TenantSecretStore;
 import com.yahoo.config.model.api.ValidationParameters;
 import com.yahoo.config.model.provision.Host;
 import com.yahoo.config.model.provision.Hosts;
 import com.yahoo.config.model.provision.InMemoryProvisioner;
 import com.yahoo.config.model.test.HostedConfigModelRegistry;
 import com.yahoo.config.provision.ApplicationId;
-import com.yahoo.config.provision.Cloud;
 import com.yahoo.config.provision.CloudAccount;
-import com.yahoo.config.provision.CloudName;
 import com.yahoo.config.provision.ClusterSpec;
 import com.yahoo.config.provision.DockerImage;
 import com.yahoo.config.provision.Environment;
 import com.yahoo.config.provision.RegionName;
-import com.yahoo.config.provision.SystemName;
 import com.yahoo.config.provision.Zone;
 import com.yahoo.slime.SlimeUtils;
 import com.yahoo.test.ManualClock;
 import com.yahoo.vespa.config.server.MockConfigConvergenceChecker;
-import com.yahoo.vespa.config.server.MockSecretStore;
 import com.yahoo.vespa.config.server.application.ApplicationReindexing;
 import com.yahoo.vespa.config.server.application.ConfigConvergenceChecker;
 import com.yahoo.vespa.config.server.http.InternalServerException;
@@ -126,33 +121,6 @@ public void testReDeployWithWantedDockerImageRepositoryAndAthenzDomain() {
         assertEquals("myDomain", ((Deployment) deployment.get()).session().getAthenzDomain().get().value());
     }
 
-    @Test
-    public void testRedeployWithTenantSecretStores() {
-        var publicCdZone = new Zone(Cloud.builder().name(CloudName.AWS).build(), SystemName.PublicCd, Environment.prod, RegionName.defaultName());
-        var secretStore = new MockSecretStore();
-        secretStore.put("vespa.external.cd.tenant.secrets.external.id.deploytester.foo", "extId");
-
-        TenantSecretStore tenantSecretStore = new TenantSecretStore("foo", "extId", "role");
-        var tenantSecretStores = List.of(tenantSecretStore);
-        DeployTester tester = new DeployTester.Builder(temporaryFolder)
-                .hostedConfigserverConfig(publicCdZone)
-                .zone(publicCdZone)
-                .secretStore(secretStore)
-                .modelFactory(createHostedModelFactory(Version.fromString("4.5.6"), Clock.systemUTC()))
-                .build();
-
-        tester.deployApp("src/test/apps/hosted/", new PrepareParams.Builder()
-                .vespaVersion("4.5.6")
-                .tenantSecretStores(tenantSecretStores));
-
-        Optional<com.yahoo.config.provision.Deployment> deployment = tester.redeployFromLocalActive(tester.applicationId());
-        assertTrue(deployment.isPresent());
-        deployment.get().activate();
-
-        var expected = List.of(tenantSecretStore.withExternalId("extId"));
-        assertEquals(expected, ((Deployment) deployment.get()).session().getTenantSecretStores());
-    }
-
     @Test
     public void testDeployOnUnknownVersion() {
         List<ModelFactory> modelFactories = List.of(createHostedModelFactory(Version.fromString("1.0.0")));