From f89f2a9b4846d6a3a479ce51a79021fd85dd5c87 Mon Sep 17 00:00:00 2001 From: itsankit-google Date: Wed, 8 Nov 2023 14:31:25 +0000 Subject: [PATCH 1/2] do not set WI env vars when NSA is enabled --- .../preview/DistributedPreviewManager.java | 2 ++ .../app/worker/TaskWorkerServiceLauncher.java | 2 ++ .../cdap/k8s/runtime/KubeTwillPreparer.java | 20 ++++++++++++++++--- .../master/spi/twill/SecureTwillPreparer.java | 9 +++++++++ 4 files changed, 30 insertions(+), 3 deletions(-) diff --git a/cdap-app-fabric/src/main/java/io/cdap/cdap/internal/app/preview/DistributedPreviewManager.java b/cdap-app-fabric/src/main/java/io/cdap/cdap/internal/app/preview/DistributedPreviewManager.java index 12a6701a7729..08be41066108 100644 --- a/cdap-app-fabric/src/main/java/io/cdap/cdap/internal/app/preview/DistributedPreviewManager.java +++ b/cdap-app-fabric/src/main/java/io/cdap/cdap/internal/app/preview/DistributedPreviewManager.java @@ -235,6 +235,8 @@ public void run() { String.format("%s:%s", localhost, cConf.getInt(Constants.ArtifactLocalizer.PORT)) )); + twillPreparer = ((SecureTwillPreparer) twillPreparer) + .withNamespacedWorkloadIdentity(PreviewRunnerTwillRunnable.class.getSimpleName()); } String priorityClass = cConf.get(Constants.Preview.CONTAINER_PRIORITY_CLASS_NAME); diff --git a/cdap-app-fabric/src/main/java/io/cdap/cdap/internal/app/worker/TaskWorkerServiceLauncher.java b/cdap-app-fabric/src/main/java/io/cdap/cdap/internal/app/worker/TaskWorkerServiceLauncher.java index 7156bac11787..a776ee09f688 100644 --- a/cdap-app-fabric/src/main/java/io/cdap/cdap/internal/app/worker/TaskWorkerServiceLauncher.java +++ b/cdap-app-fabric/src/main/java/io/cdap/cdap/internal/app/worker/TaskWorkerServiceLauncher.java @@ -213,6 +213,8 @@ public void run() { String.format("%s:%s", localhost, cConf.getInt(Constants.ArtifactLocalizer.PORT)) )); + twillPreparer = ((SecureTwillPreparer) twillPreparer) + .withNamespacedWorkloadIdentity(TaskWorkerTwillRunnable.class.getSimpleName()); } String priorityClass = cConf.get(Constants.TaskWorker.CONTAINER_PRIORITY_CLASS_NAME); diff --git a/cdap-kubernetes/src/main/java/io/cdap/cdap/k8s/runtime/KubeTwillPreparer.java b/cdap-kubernetes/src/main/java/io/cdap/cdap/k8s/runtime/KubeTwillPreparer.java index 5e85f18baee1..46797f0c77b4 100644 --- a/cdap-kubernetes/src/main/java/io/cdap/cdap/k8s/runtime/KubeTwillPreparer.java +++ b/cdap-kubernetes/src/main/java/io/cdap/cdap/k8s/runtime/KubeTwillPreparer.java @@ -189,6 +189,7 @@ class KubeTwillPreparer implements DependentTwillPreparer, StatefulTwillPreparer private final String resourcePrefix; private final Map extraLabels; private final Map secretDiskRunnables; + private final Set withNamespaceWorkloadIdentityRunnables; private final Map containerSecurityContexts; private final Map> readonlyDisks; private final Map> runnableConfigs; @@ -240,6 +241,7 @@ class KubeTwillPreparer implements DependentTwillPreparer, StatefulTwillPreparer this.dependentRunnableNames = new HashSet<>(); this.serviceAccountName = null; this.secretDiskRunnables = new HashMap<>(); + this.withNamespaceWorkloadIdentityRunnables = new HashSet<>(); this.containerSecurityContexts = new HashMap<>(); this.readonlyDisks = new HashMap<>(); this.runnableConfigs = runnables.stream() @@ -368,6 +370,12 @@ public SecureTwillPreparer withSecretDisk(String runnableName, SecretDisk... sec return this; } + @Override + public SecureTwillPreparer withNamespacedWorkloadIdentity(String runnableName) { + withNamespaceWorkloadIdentityRunnables.add(runnableName); + return this; + } + @Override public SecureTwillPreparer withSecurityContext(String runnableName, SecurityContext securityContext) { @@ -1285,9 +1293,8 @@ private List createContainers(Map run environs.put(JAVA_OPTS_KEY, jvmOpts); // Add workload identity environment variable if applicable. if (workloadIdentityEnabled && WorkloadIdentityUtil.shouldMountWorkloadIdentity( - cdapInstallNamespace, - programRuntimeNamespace, - workloadIdentityServiceAccount)) { + cdapInstallNamespace, programRuntimeNamespace, workloadIdentityServiceAccount) + && !withNamespaceWorkloadIdentityRunnables.contains(runnableName)) { V1EnvVar workloadIdentityEnvVar = WorkloadIdentityUtil.generateWorkloadIdentityEnvVar(); environs.put(workloadIdentityEnvVar.getName(), workloadIdentityEnvVar.getValue()); } @@ -1314,6 +1321,13 @@ private List createContainers(Map run .filter(entry -> !entry.getKey().equals(GCE_METADATA_HOST_ENV_VAR)) .collect(Collectors.toMap(Map.Entry::getKey, Map.Entry::getValue)); + // Add workload identity environment variable in the dependent runnable if applicable. + if (workloadIdentityEnabled && WorkloadIdentityUtil.shouldMountWorkloadIdentity( + cdapInstallNamespace, programRuntimeNamespace, workloadIdentityServiceAccount) + && !withNamespaceWorkloadIdentityRunnables.contains(name)) { + V1EnvVar workloadIdentityEnvVar = WorkloadIdentityUtil.generateWorkloadIdentityEnvVar(); + envs.put(workloadIdentityEnvVar.getName(), workloadIdentityEnvVar.getValue()); + } mounts = addSecreteVolMountIfNeeded(spec, volumeMounts); containers.add( createContainer(name, podInfo.getContainerImage(), podInfo.getImagePullPolicy(), workDir, diff --git a/cdap-master-spi/src/main/java/io/cdap/cdap/master/spi/twill/SecureTwillPreparer.java b/cdap-master-spi/src/main/java/io/cdap/cdap/master/spi/twill/SecureTwillPreparer.java index 8355750eb74c..d49f5d455108 100644 --- a/cdap-master-spi/src/main/java/io/cdap/cdap/master/spi/twill/SecureTwillPreparer.java +++ b/cdap-master-spi/src/main/java/io/cdap/cdap/master/spi/twill/SecureTwillPreparer.java @@ -44,4 +44,13 @@ public interface SecureTwillPreparer extends TwillPreparer { SecureTwillPreparer withSecurityContext(String runnableName, SecurityContext securityContext); + /** + * Runs the given runnable with namespace workload identity, + * this feature removes the GOOGLE_APPLICATION_CREDENTIALS environment variable + * to enable namespaced service accounts. + * + * @param runnableName name of the {@link TwillRunnable} + * @return this {@link TwillPreparer} + */ + SecureTwillPreparer withNamespacedWorkloadIdentity(String runnableName); } From 1576288dfba9f4f5a652a076269f6bad9def7548 Mon Sep 17 00:00:00 2001 From: itsankit-google Date: Wed, 8 Nov 2023 21:52:54 +0000 Subject: [PATCH 2/2] use gcp remote authenticator to fetch token --- .../sidecar/ArtifactLocalizerService.java | 5 ++- .../ArtifactLocalizerTwillRunnable.java | 11 ++++- .../GcpMetadataHttpHandlerInternal.java | 45 +++++++------------ .../app/deploy/RemoteConfiguratorTest.java | 4 +- .../sidecar/ArtifactLocalizerServiceTest.java | 3 +- .../GcpMetadataHttpHandlerInternalTest.java | 4 +- .../gcp/GCPRemoteAuthenticator.java | 3 +- .../io/cdap/cdap/common/conf/Constants.java | 2 + .../cdap/cdap/proto/security/Credential.java | 26 +++++++++++ 9 files changed, 67 insertions(+), 36 deletions(-) diff --git a/cdap-app-fabric/src/main/java/io/cdap/cdap/internal/app/worker/sidecar/ArtifactLocalizerService.java b/cdap-app-fabric/src/main/java/io/cdap/cdap/internal/app/worker/sidecar/ArtifactLocalizerService.java index 2de2ac698e78..4205700ee570 100644 --- a/cdap-app-fabric/src/main/java/io/cdap/cdap/internal/app/worker/sidecar/ArtifactLocalizerService.java +++ b/cdap-app-fabric/src/main/java/io/cdap/cdap/internal/app/worker/sidecar/ArtifactLocalizerService.java @@ -23,6 +23,7 @@ import io.cdap.cdap.common.conf.Constants; import io.cdap.cdap.common.http.CommonNettyHttpServiceFactory; import io.cdap.cdap.common.internal.remote.RemoteClientFactory; +import io.cdap.cdap.security.spi.authenticator.RemoteAuthenticator; import io.cdap.http.NettyHttpService; import java.net.InetAddress; import java.nio.file.Paths; @@ -52,7 +53,7 @@ public class ArtifactLocalizerService extends AbstractIdleService { ArtifactLocalizerService(CConfiguration cConf, ArtifactLocalizer artifactLocalizer, CommonNettyHttpServiceFactory commonNettyHttpServiceFactory, - RemoteClientFactory remoteClientFactory) { + RemoteClientFactory remoteClientFactory, RemoteAuthenticator remoteAuthenticator) { this.cConf = cConf; this.artifactLocalizer = artifactLocalizer; this.httpService = commonNettyHttpServiceFactory.builder(Constants.Service.TASK_WORKER) @@ -61,7 +62,7 @@ public class ArtifactLocalizerService extends AbstractIdleService { .setBossThreadPoolSize(cConf.getInt(Constants.ArtifactLocalizer.BOSS_THREADS)) .setWorkerThreadPoolSize(cConf.getInt(Constants.ArtifactLocalizer.WORKER_THREADS)) .setHttpHandlers(new ArtifactLocalizerHttpHandlerInternal(artifactLocalizer), - new GcpMetadataHttpHandlerInternal(cConf, remoteClientFactory)) + new GcpMetadataHttpHandlerInternal(cConf, remoteClientFactory, remoteAuthenticator)) .build(); this.cacheCleanupInterval = cConf.getInt( diff --git a/cdap-app-fabric/src/main/java/io/cdap/cdap/internal/app/worker/sidecar/ArtifactLocalizerTwillRunnable.java b/cdap-app-fabric/src/main/java/io/cdap/cdap/internal/app/worker/sidecar/ArtifactLocalizerTwillRunnable.java index b9e8d676dbef..584b0c9943c3 100644 --- a/cdap-app-fabric/src/main/java/io/cdap/cdap/internal/app/worker/sidecar/ArtifactLocalizerTwillRunnable.java +++ b/cdap-app-fabric/src/main/java/io/cdap/cdap/internal/app/worker/sidecar/ArtifactLocalizerTwillRunnable.java @@ -25,9 +25,11 @@ import com.google.inject.Guice; import com.google.inject.Injector; import com.google.inject.Module; +import io.cdap.cdap.api.feature.FeatureFlagsProvider; import io.cdap.cdap.app.guice.DistributedArtifactManagerModule; import io.cdap.cdap.common.conf.CConfiguration; import io.cdap.cdap.common.conf.Constants; +import io.cdap.cdap.common.feature.DefaultFeatureFlagsProvider; import io.cdap.cdap.common.guice.ConfigModule; import io.cdap.cdap.common.guice.DFSLocationModule; import io.cdap.cdap.common.guice.IOModule; @@ -40,6 +42,7 @@ import io.cdap.cdap.common.logging.LoggingContext; import io.cdap.cdap.common.logging.LoggingContextAccessor; import io.cdap.cdap.common.logging.ServiceLoggingContext; +import io.cdap.cdap.features.Feature; import io.cdap.cdap.logging.appender.LogAppenderInitializer; import io.cdap.cdap.logging.guice.KafkaLogAppenderModule; import io.cdap.cdap.logging.guice.RemoteLogAppenderModule; @@ -100,7 +103,13 @@ public static Injector createInjector(CConfiguration cConf, Configuration hConf) modules.add(new ConfigModule(cConf, hConf)); modules.add(new IOModule()); - modules.add(RemoteAuthenticatorModules.getDefaultModule()); + FeatureFlagsProvider featureFlagsProvider = new DefaultFeatureFlagsProvider(cConf); + if (Feature.NAMESPACED_SERVICE_ACCOUNTS.isEnabled(featureFlagsProvider)) { + modules.add(RemoteAuthenticatorModules.getDefaultModule( + Constants.ArtifactLocalizer.REMOTE_AUTHENTICATOR_NAME)); + } else { + modules.add(RemoteAuthenticatorModules.getDefaultModule()); + } modules.add(new AuthenticationContextModules().getMasterModule()); modules.add(coreSecurityModule); modules.add(new MessagingServiceModule(cConf)); diff --git a/cdap-app-fabric/src/main/java/io/cdap/cdap/internal/app/worker/sidecar/GcpMetadataHttpHandlerInternal.java b/cdap-app-fabric/src/main/java/io/cdap/cdap/internal/app/worker/sidecar/GcpMetadataHttpHandlerInternal.java index cb860ffeb750..6a6cd2028c25 100644 --- a/cdap-app-fabric/src/main/java/io/cdap/cdap/internal/app/worker/sidecar/GcpMetadataHttpHandlerInternal.java +++ b/cdap-app-fabric/src/main/java/io/cdap/cdap/internal/app/worker/sidecar/GcpMetadataHttpHandlerInternal.java @@ -16,6 +16,7 @@ package io.cdap.cdap.internal.app.worker.sidecar; +import com.google.common.base.Strings; import com.google.common.cache.CacheBuilder; import com.google.common.cache.CacheLoader; import com.google.common.cache.LoadingCache; @@ -37,17 +38,15 @@ import io.cdap.cdap.proto.credential.NamespaceCredentialProvider; import io.cdap.cdap.proto.credential.NotFoundException; import io.cdap.cdap.proto.credential.ProvisionedCredential; +import io.cdap.cdap.proto.security.Credential; import io.cdap.cdap.proto.security.GcpMetadataTaskContext; -import io.cdap.common.http.HttpRequests; -import io.cdap.common.http.HttpResponse; +import io.cdap.cdap.security.spi.authenticator.RemoteAuthenticator; import io.cdap.http.HttpHandler; import io.cdap.http.HttpResponder; import io.netty.handler.codec.http.DefaultHttpHeaders; import io.netty.handler.codec.http.FullHttpRequest; import io.netty.handler.codec.http.HttpRequest; import io.netty.handler.codec.http.HttpResponseStatus; -import java.io.IOException; -import java.net.URL; import java.time.Duration; import java.time.Instant; import java.util.concurrent.ExecutionException; @@ -57,7 +56,6 @@ import javax.ws.rs.PUT; import javax.ws.rs.Path; import javax.ws.rs.QueryParam; -import joptsimple.internal.Strings; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -73,8 +71,8 @@ public class GcpMetadataHttpHandlerInternal extends AbstractAppFabricHttpHandler private static final Gson GSON = new GsonBuilder().registerTypeAdapter(BasicThrowable.class, new BasicThrowableCodec()).create(); private final CConfiguration cConf; - private final String metadataServiceTokenEndpoint; private final NamespaceCredentialProvider credentialProvider; + private final RemoteAuthenticator remoteAuthenticator; private final GcpWorkloadIdentityInternalAuthenticator gcpWorkloadIdentityInternalAuthenticator; private GcpMetadataTaskContext gcpMetadataTaskContext; private final LoadingCache new NoOpArtifactManager())) ), - new GcpMetadataHttpHandlerInternal(cConf, remoteClientFactory) + new GcpMetadataHttpHandlerInternal(cConf, remoteClientFactory, + new NoOpRemoteAuthenticator()) ) .setChannelPipelineModifier(new ChannelPipelineModifier() { @Override diff --git a/cdap-app-fabric/src/test/java/io/cdap/cdap/internal/app/worker/sidecar/ArtifactLocalizerServiceTest.java b/cdap-app-fabric/src/test/java/io/cdap/cdap/internal/app/worker/sidecar/ArtifactLocalizerServiceTest.java index 61ffaf5c998b..ea9f953f718e 100644 --- a/cdap-app-fabric/src/test/java/io/cdap/cdap/internal/app/worker/sidecar/ArtifactLocalizerServiceTest.java +++ b/cdap-app-fabric/src/test/java/io/cdap/cdap/internal/app/worker/sidecar/ArtifactLocalizerServiceTest.java @@ -27,6 +27,7 @@ import io.cdap.cdap.common.id.Id; import io.cdap.cdap.common.internal.remote.DefaultInternalAuthenticator; import io.cdap.cdap.common.internal.remote.NoOpInternalAuthenticator; +import io.cdap.cdap.common.internal.remote.NoOpRemoteAuthenticator; import io.cdap.cdap.common.internal.remote.RemoteClientFactory; import io.cdap.cdap.common.io.Locations; import io.cdap.cdap.common.metrics.NoOpMetricsCollectionService; @@ -85,7 +86,7 @@ private ArtifactLocalizerService setupArtifactLocalizerService(CConfiguration cC cConf, new ArtifactLocalizer(cConf, remoteClientFactory, (namespaceId, retryStrategy) -> { return new NoOpArtifactManager(); }), new CommonNettyHttpServiceFactory(cConf, new NoOpMetricsCollectionService()), - remoteClientFactory); + remoteClientFactory, new NoOpRemoteAuthenticator()); // start the service artifactLocalizerService.startAndWait(); diff --git a/cdap-app-fabric/src/test/java/io/cdap/cdap/internal/app/worker/sidecar/GcpMetadataHttpHandlerInternalTest.java b/cdap-app-fabric/src/test/java/io/cdap/cdap/internal/app/worker/sidecar/GcpMetadataHttpHandlerInternalTest.java index b33c23294972..0e7eac4d507c 100644 --- a/cdap-app-fabric/src/test/java/io/cdap/cdap/internal/app/worker/sidecar/GcpMetadataHttpHandlerInternalTest.java +++ b/cdap-app-fabric/src/test/java/io/cdap/cdap/internal/app/worker/sidecar/GcpMetadataHttpHandlerInternalTest.java @@ -21,6 +21,7 @@ import io.cdap.cdap.common.conf.CConfiguration; import io.cdap.cdap.common.conf.Constants; import io.cdap.cdap.common.http.CommonNettyHttpServiceBuilder; +import io.cdap.cdap.common.internal.remote.NoOpRemoteAuthenticator; import io.cdap.cdap.common.internal.remote.RemoteClientFactory; import io.cdap.cdap.common.metrics.NoOpMetricsCollectionService; import io.cdap.cdap.common.namespace.InMemoryNamespaceAdmin; @@ -69,7 +70,8 @@ public static void init() throws Exception { httpService = new CommonNettyHttpServiceBuilder(cConf, "test", new NoOpMetricsCollectionService()) .setHttpHandlers( - new GcpMetadataHttpHandlerInternal(cConf, remoteClientFactory) + new GcpMetadataHttpHandlerInternal(cConf, remoteClientFactory, + new NoOpRemoteAuthenticator()) ) .setChannelPipelineModifier(new ChannelPipelineModifier() { @Override diff --git a/cdap-authenticator-ext-gcp/src/main/java/io/cdap/cdap/authenticator/gcp/GCPRemoteAuthenticator.java b/cdap-authenticator-ext-gcp/src/main/java/io/cdap/cdap/authenticator/gcp/GCPRemoteAuthenticator.java index ed04d4bc0aaa..d9c45aab14c7 100644 --- a/cdap-authenticator-ext-gcp/src/main/java/io/cdap/cdap/authenticator/gcp/GCPRemoteAuthenticator.java +++ b/cdap-authenticator-ext-gcp/src/main/java/io/cdap/cdap/authenticator/gcp/GCPRemoteAuthenticator.java @@ -64,6 +64,7 @@ public Credential getCredentials() throws IOException { if (accessToken == null || accessToken.getExpirationTime().before(Date.from(clock.instant()))) { accessToken = googleCredentials.refreshAccessToken(); } - return new Credential(accessToken.getTokenValue(), Credential.CredentialType.EXTERNAL_BEARER); + return new Credential(accessToken.getTokenValue(), Credential.CredentialType.EXTERNAL_BEARER, + accessToken.getExpirationTime().getTime() / 1000L); } } diff --git a/cdap-common/src/main/java/io/cdap/cdap/common/conf/Constants.java b/cdap-common/src/main/java/io/cdap/cdap/common/conf/Constants.java index f3a5dc32a179..d6b427e6ca8f 100644 --- a/cdap-common/src/main/java/io/cdap/cdap/common/conf/Constants.java +++ b/cdap-common/src/main/java/io/cdap/cdap/common/conf/Constants.java @@ -594,6 +594,8 @@ public static final class ArtifactLocalizer { public static final String WORKER_THREADS = "artifact.localizer.worker.threads"; public static final String PRELOAD_LIST = "artifact.localizer.preload.list"; public static final String PRELOAD_VERSION_LIMIT = "artifact.localizer.preload.version.limit"; + public static final String REMOTE_AUTHENTICATOR_NAME = + "artifact.localizer.remote.authenticator.name"; } /** diff --git a/cdap-proto/src/main/java/io/cdap/cdap/proto/security/Credential.java b/cdap-proto/src/main/java/io/cdap/cdap/proto/security/Credential.java index 0f8887611d98..b22eb9be2cd6 100644 --- a/cdap-proto/src/main/java/io/cdap/cdap/proto/security/Credential.java +++ b/cdap-proto/src/main/java/io/cdap/cdap/proto/security/Credential.java @@ -84,10 +84,31 @@ public static CredentialType fromQualifiedName(String qualifiedName) { private final String value; private final CredentialType type; + private final Long expirationTimeSecs; + /** + * Constructs the Credential. + * + * @param value credential value + * @param type credential type + */ public Credential(String value, CredentialType type) { this.value = value; this.type = type; + this.expirationTimeSecs = null; + } + + /** + * Constructs the Credential. + * + * @param value credential value + * @param type credential type + * @param expirationTimeSecs the time in seconds after which credential will expire + */ + public Credential(String value, CredentialType type, Long expirationTimeSecs) { + this.value = value; + this.type = type; + this.expirationTimeSecs = expirationTimeSecs; } public String getValue() { @@ -98,10 +119,15 @@ public CredentialType getType() { return type; } + public Long getExpirationTimeSecs() { + return expirationTimeSecs; + } + @Override public String toString() { return "Credential{" + "type=" + type + + ", expires_in=" + expirationTimeSecs + ", length=" + value.length() + "}"; }