dimas-b commented on code in PR #2759: URL: https://github.com/apache/polaris/pull/2759#discussion_r2407449342
########## runtime/service/src/main/java/org/apache/polaris/service/credentials/connection/SigV4ConnectionCredentialVendor.java: ########## @@ -0,0 +1,134 @@ +/* + * 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.polaris.service.credentials.connection; + +import com.google.common.annotations.VisibleForTesting; +import jakarta.annotation.Nonnull; +import jakarta.annotation.Priority; +import jakarta.enterprise.context.ApplicationScoped; +import jakarta.inject.Inject; +import java.util.Optional; +import org.apache.polaris.core.connection.AuthenticationType; +import org.apache.polaris.core.connection.ConnectionConfigInfoDpo; +import org.apache.polaris.core.connection.SigV4AuthenticationParametersDpo; +import org.apache.polaris.core.credentials.connection.ConnectionCredentialProperty; +import org.apache.polaris.core.credentials.connection.ConnectionCredentialVendor; +import org.apache.polaris.core.credentials.connection.ConnectionCredentials; +import org.apache.polaris.core.identity.credential.AwsIamServiceIdentityCredential; +import org.apache.polaris.core.identity.credential.ServiceIdentityCredential; +import org.apache.polaris.core.identity.provider.ServiceIdentityProvider; +import org.apache.polaris.core.storage.aws.StsClientProvider; +import software.amazon.awssdk.services.sts.StsClient; +import software.amazon.awssdk.services.sts.model.AssumeRoleRequest; +import software.amazon.awssdk.services.sts.model.AssumeRoleResponse; + +/** + * Connection credential vendor for AWS SigV4 authentication. + * + * <p>This vendor uses Polaris's AWS IAM service identity to assume a customer-provided IAM role via + * AWS STS, generating temporary credentials that Polaris uses to access external AWS services + * (e.g., AWS Glue catalog) with SigV4 request signing. + * + * <p>Flow: + * + * <ol> + * <li>Receives Polaris's {@link AwsIamServiceIdentityCredential} (the IAM user/role Polaris owns) + * <li>Extracts customer's role ARN from {@link SigV4AuthenticationParametersDpo} + * <li>Calls AWS STS AssumeRole to get temporary credentials + * <li>Returns temporary access key, secret key, and session token + * </ol> + * + * <p>This is the default implementation with {@code @Priority(100)}. Custom implementations can + * override this by providing a higher priority value. + */ +@ApplicationScoped +@SupportsAuthType(AuthenticationType.SIGV4) Review Comment: How about just `@AuthType`?... The annotation being a `@Qualifier` makes it pretty clear that it is about "supporting" something, IMHO. ########## runtime/service/src/main/java/org/apache/polaris/service/credentials/DefaultPolarisCredentialManagerFactory.java: ########## @@ -0,0 +1,54 @@ +/* + * 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.polaris.service.credentials; + +import io.smallrye.common.annotation.Identifier; +import jakarta.enterprise.context.ApplicationScoped; +import jakarta.enterprise.inject.Any; +import jakarta.enterprise.inject.Instance; +import jakarta.inject.Inject; +import java.util.Map; +import java.util.concurrent.ConcurrentHashMap; +import org.apache.polaris.core.context.RealmContext; +import org.apache.polaris.core.credentials.PolarisCredentialManager; +import org.apache.polaris.core.credentials.PolarisCredentialManagerFactory; +import org.apache.polaris.core.credentials.connection.ConnectionCredentialVendor; + +@ApplicationScoped +@Identifier("default") +public class DefaultPolarisCredentialManagerFactory implements PolarisCredentialManagerFactory { + private final Map<String, PolarisCredentialManager> cachedCredentialManagers = + new ConcurrentHashMap<>(); + + private final Instance<ConnectionCredentialVendor> credentialVendors; + + @Inject + public DefaultPolarisCredentialManagerFactory( + @Any Instance<ConnectionCredentialVendor> credentialVendors) { + this.credentialVendors = credentialVendors; + } + + @Override + public PolarisCredentialManager getOrCreatePolarisCredentialManager(RealmContext realmContext) { + return cachedCredentialManagers.computeIfAbsent( + realmContext.getRealmIdentifier(), + key -> new DefaultPolarisCredentialManager(credentialVendors)); Review Comment: `DefaultPolarisCredentialManager` does not have any state... What's the value in caching its instances per realm? ########## runtime/service/src/main/java/org/apache/polaris/service/config/ServiceProducers.java: ########## @@ -242,6 +245,13 @@ public StsClientsPool stsClientsPool( return new StsClientsPool(config.effectiveClientsCacheMaxSize(), httpClient, meterRegistry); } + @Produces + public PolarisCredentialManagerFactory credentialManagerFactory( + PolarisCredentialManagerConfiguration config, + @Any Instance<PolarisCredentialManagerFactory> credentialManagerFactories) { + return credentialManagerFactories.select(Identifier.Literal.of(config.type())).get(); Review Comment: Do we really need a factory here?.. Why not produce `PolarisCredentialManager` directly? ########## runtime/defaults/src/main/resources/application.properties: ########## @@ -207,6 +210,7 @@ quarkus.arc.ignored-split-packages=\ org.apache.polaris.service.auth.external.mapping,\ org.apache.polaris.service.auth.external.tenant,\ org.apache.polaris.service.auth.internal,\ + org.apache.polaris.service.credentials,\ Review Comment: Can we avoid split packages in _new_ code? ########## runtime/service/src/main/java/org/apache/polaris/service/credentials/connection/SupportsAuthTypes.java: ########## @@ -0,0 +1,52 @@ +/* + * 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.polaris.service.credentials.connection; + +import jakarta.enterprise.util.Nonbinding; +import jakarta.inject.Qualifier; +import java.lang.annotation.ElementType; +import java.lang.annotation.Retention; +import java.lang.annotation.RetentionPolicy; +import java.lang.annotation.Target; +import org.apache.polaris.core.credentials.connection.ConnectionCredentialVendor; + +/** + * Container annotation for multiple {@link SupportsAuthType} annotations. + * + * <p>This annotation allows a single {@link ConnectionCredentialVendor} implementation to support + * multiple authentication types by applying multiple {@link SupportsAuthType} annotations. + * + * <p>Example usage: + * + * <pre>{@code + * @ApplicationScoped + * @SupportsAuthType(AuthenticationType.IMPLICIT) + * @SupportsAuthType(AuthenticationType.BEARER) + * public class NoOpConnectionCredentialVendor implements ConnectionCredentialVendor { + * // Handles both IMPLICIT and BEARER authentication (no credential transformation) + * } + * }</pre> + */ +@Qualifier +@Target(ElementType.TYPE) +@Retention(RetentionPolicy.RUNTIME) +public @interface SupportsAuthTypes { Review Comment: This annotation appears to be unused? 🤔 ########## polaris-core/src/main/java/org/apache/polaris/core/credentials/connection/ConnectionCredentialProperty.java: ########## @@ -0,0 +1,66 @@ +/* + * 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.polaris.core.credentials.connection; + +import org.apache.iceberg.aws.AwsProperties; + +/** + * A subset of Iceberg catalog properties recognized by Polaris. + * + * <p>Most of these properties are meant to initialize Catalog objects for accessing the remote + * Catalog service. + */ +public enum ConnectionCredentialProperty { + AWS_ACCESS_KEY_ID(String.class, AwsProperties.REST_ACCESS_KEY_ID, "the aws access key id", true), Review Comment: nit: WDYT about renaming to `CatalogAccessProperty` by analogy to `StorageAccessProperty`? ########## polaris-core/src/main/java/org/apache/polaris/core/credentials/PolarisCredentialManager.java: ########## @@ -0,0 +1,52 @@ +/* + * 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.polaris.core.credentials; + +import org.apache.polaris.core.connection.AuthenticationParametersDpo; +import org.apache.polaris.core.credentials.connection.ConnectionCredentialVendor; +import org.apache.polaris.core.identity.credential.ServiceIdentityCredential; +import org.apache.polaris.core.identity.provider.ServiceIdentityProvider; + +/** + * PolarisCredentialManager is responsible for retrieving the credentials Polaris needs to access + * remote services such as federated catalogs and cloud storage. + * + * <p>It delegates to {@link ConnectionCredentialVendor} implementations that combine + * service-managed identity information (e.g., an IAM user Polaris uses) with user-defined + * authentication parameters (e.g., roleArn) to generate the credentials required for authentication + * with external systems. + * + * <p>Typical flow for connection credentials: + * + * <ol> + * <li>The manager selects the appropriate {@link ConnectionCredentialVendor} based on the + * authentication type from the {@link AuthenticationParametersDpo}. + * <li>The vendor resolves the service identity using {@link ServiceIdentityProvider} to obtain a + * {@link ServiceIdentityCredential}. + * <li>The vendor uses the service identity credential together with user-provided authentication + * parameters to obtain temporary access credentials (e.g., via AWS STS AssumeRole). + * </ol> + * + * <p>This design supports both SaaS and self-managed deployments, ensuring a clear separation + * between user-provided configuration and Polaris-managed identity. In the future, this interface + * will be extended to also manage storage credentials, providing a unified interface for all + * credential management needs. + */ +public interface PolarisCredentialManager extends ConnectionCredentialVendor {} Review Comment: What's the value of having this empty interface? Why not merge with `ConnectionCredentialVendor`? ########## runtime/service/src/main/java/org/apache/polaris/service/credentials/DefaultPolarisCredentialManager.java: ########## @@ -0,0 +1,92 @@ +/* + * 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.polaris.service.credentials; + +import jakarta.annotation.Nonnull; +import jakarta.enterprise.inject.Any; +import jakarta.enterprise.inject.Instance; +import org.apache.polaris.core.connection.AuthenticationType; +import org.apache.polaris.core.connection.ConnectionConfigInfoDpo; +import org.apache.polaris.core.credentials.PolarisCredentialManager; +import org.apache.polaris.core.credentials.connection.ConnectionCredentialVendor; +import org.apache.polaris.core.credentials.connection.ConnectionCredentials; +import org.apache.polaris.service.credentials.connection.SupportsAuthType; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +/** + * Default implementation of {@link PolarisCredentialManager} responsible for retrieving credentials + * used by Polaris to access external systems such as remote catalogs or cloud storage. + * + * <p>This implementation delegates to {@link ConnectionCredentialVendor} implementations selected + * via CDI based on the authentication type. Each vendor handles the credential transformation logic + * for a specific authentication mechanism (e.g., SigV4, OAuth). + * + * <p>Flow: + * + * <ol> + * <li>Selects the appropriate {@link ConnectionCredentialVendor} based on the authentication type + * <li>Delegates to the vendor to generate the final connection credentials (the vendor will + * resolve the service identity internally) + * </ol> + */ +public class DefaultPolarisCredentialManager implements PolarisCredentialManager { + private static final Logger LOGGER = + LoggerFactory.getLogger(DefaultPolarisCredentialManager.class); + + private final Instance<ConnectionCredentialVendor> credentialVendors; + + public DefaultPolarisCredentialManager( + @Any Instance<ConnectionCredentialVendor> credentialVendors) { + this.credentialVendors = credentialVendors; + } + + @Override + public @Nonnull ConnectionCredentials getConnectionCredentials( + @Nonnull ConnectionConfigInfoDpo connectionConfig) { + + // Select the appropriate vendor based on authentication type + AuthenticationType authType = + connectionConfig.getAuthenticationParameters().getAuthenticationType(); + Instance<ConnectionCredentialVendor> selectedVendor = + credentialVendors.select(SupportsAuthType.Literal.of(authType)); Review Comment: This approach is fine, but it defers errors to runtime. I'd propose the `DefaultPolarisCredentialManager` to resolve all possible `ConnectionCredentialVendor` at build time. Note that defaults implementations are already present in the same build module (jar). With that approach we probably do not need `SupportsAuthType`. Downstream builds that have non-default `ConnectionCredentialVendor` implementations will have to override the `PolarisCredentialManager` too, but I think that would be a reasonable tradeoff for achieving built-time cross-reference validation (downstream builds will be able to do build time validation too). -- 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]
