sarutak commented on code in PR #57655: URL: https://github.com/apache/spark/pull/57655#discussion_r3688004287
########## connector/credential-aws/src/main/java/org/apache/spark/security/aws/AwsStsCredentialProvider.java: ########## @@ -0,0 +1,316 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.apache.spark.security.aws; + +import java.net.URI; +import java.time.Duration; +import java.time.Instant; +import java.util.Map; +import java.util.Set; + +import software.amazon.awssdk.auth.credentials.AnonymousCredentialsProvider; +import software.amazon.awssdk.core.exception.SdkException; +import software.amazon.awssdk.regions.Region; +import software.amazon.awssdk.services.sts.StsClient; +import software.amazon.awssdk.services.sts.StsClientBuilder; +import software.amazon.awssdk.services.sts.model.AssumeRoleWithWebIdentityRequest; +import software.amazon.awssdk.services.sts.model.AssumeRoleWithWebIdentityResponse; +import software.amazon.awssdk.services.sts.model.Credentials; + +import org.apache.spark.annotation.DeveloperApi; +import org.apache.spark.security.CredentialProvider; +import org.apache.spark.security.CredentialResolutionException; +import org.apache.spark.security.ServiceCredential; +import org.apache.spark.security.UserContext; + +/** + * :: DeveloperApi :: + * A {@link CredentialProvider} that exchanges an OIDC identity token for temporary + * AWS credentials via the STS {@code AssumeRoleWithWebIdentity} API. + * <p> + * The returned {@link ServiceCredential} contains S3A-compatible Hadoop configuration + * properties ({@code fs.s3a.access.key}, {@code fs.s3a.secret.key}, + * {@code fs.s3a.session.token}) that can be propagated to executors for accessing + * S3-compatible storage. + * <p> + * <b>Configuration keys</b> (passed via {@code spark.security.oidc.*}): + * <ul> + * <li>{@code spark.security.oidc.roleArn} (required) — the ARN of the IAM role to assume</li> + * <li>{@code spark.security.oidc.sessionName} (optional) — the role session name; + * defaults to a value derived from the user's principal or "spark-oidc"</li> + * <li>{@code spark.security.oidc.durationSeconds} (optional) — credential duration in seconds; + * if unset, STS uses the role's default maximum</li> + * <li>{@code spark.security.oidc.region} (optional) — the AWS region for the STS endpoint; + * defaults to us-east-1 when only {@code stsEndpoint} is set</li> + * <li>{@code spark.security.oidc.stsEndpoint} (optional) — a custom STS endpoint URL for + * non-AWS environments (MinIO, Ceph, LocalStack, etc.)</li> + * </ul> + * <p> + * <b>Security note:</b> The OIDC raw token is never included in log messages or + * exception messages. It is passed directly to the STS API call and discarded. + * + * @since 4.3.0 + */ +@DeveloperApi +public class AwsStsCredentialProvider implements CredentialProvider { Review Comment: `StsClient` implements `SdkAutoCloseable` and holds HTTP connection pools internally. The current implementation creates the client in `init()` but never closes it. `CredentialProvider` uses an `init()` pattern where implementations construct long-lived resources. This means the SPI should provide a lifecycle hook for cleanup. I'll open a PR to add `AutoCloseable` to `CredentialProvider` (with a default no-op `close()`) and wire `UserCredentialManager.stop()` to call `provider.close()`. Once that's merged, could you implement `close()` here to shut down the `StsClient`? In the meantime, could you also add a re-initialization guard at the top of `init()`? ```java if (this.config != null) { throw new IllegalStateException("AwsStsCredentialProvider is already initialized"); } ``` -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
