iting0321 commented on code in PR #3750:
URL: https://github.com/apache/polaris/pull/3750#discussion_r2839571916
##########
runtime/service/src/main/java/org/apache/polaris/service/catalog/iceberg/IcebergCatalogHandler.java:
##########
@@ -1327,6 +1346,39 @@ private void checkAllowExternalCatalogCredentialVending(
}
}
+ /**
+ * Resolves the access delegation mode by delegating to the configured {@link
+ * AccessDelegationModeResolver}.
+ *
+ * <p>If no modes are requested, returns {@link
AccessDelegationMode#UNKNOWN} immediately.
+ * Otherwise, delegates to the resolver to determine the optimal mode based
on catalog
+ * capabilities.
+ *
+ * @param requestedModes The set of delegation modes requested by the client
+ * @return The resolved access delegation mode
+ */
+ protected AccessDelegationMode resolveAccessDelegationModes(
+ EnumSet<AccessDelegationMode> requestedModes) {
+ if (requestedModes.isEmpty()) {
+ resolvedAccessDelegationMode = AccessDelegationMode.UNKNOWN;
+ return resolvedAccessDelegationMode;
+ }
+
+ CatalogEntity catalogEntity = getResolvedCatalogEntity();
+ resolvedAccessDelegationMode =
accessDelegationModeResolver().resolve(requestedModes, catalogEntity);
Review Comment:
Each API endpoint only calls `resolveAccessDelegationModes()` once
per request, so storing the result in an instance field provides no benefit.
I've removed the field and the unused getter method.
##########
runtime/service/src/main/java/org/apache/polaris/service/catalog/iceberg/IcebergCatalogHandler.java:
##########
@@ -1327,6 +1346,39 @@ private void checkAllowExternalCatalogCredentialVending(
}
}
+ /**
+ * Resolves the access delegation mode by delegating to the configured {@link
+ * AccessDelegationModeResolver}.
+ *
+ * <p>If no modes are requested, returns {@link
AccessDelegationMode#UNKNOWN} immediately.
+ * Otherwise, delegates to the resolver to determine the optimal mode based
on catalog
+ * capabilities.
+ *
+ * @param requestedModes The set of delegation modes requested by the client
+ * @return The resolved access delegation mode
+ */
+ protected AccessDelegationMode resolveAccessDelegationModes(
+ EnumSet<AccessDelegationMode> requestedModes) {
+ if (requestedModes.isEmpty()) {
+ resolvedAccessDelegationMode = AccessDelegationMode.UNKNOWN;
+ return resolvedAccessDelegationMode;
+ }
+
+ CatalogEntity catalogEntity = getResolvedCatalogEntity();
+ resolvedAccessDelegationMode =
accessDelegationModeResolver().resolve(requestedModes, catalogEntity);
+ return resolvedAccessDelegationMode;
+ }
+
+ /**
+ * Returns the resolved access delegation mode. Must be called after
+ * {@link #resolveAccessDelegationModes(EnumSet)} has been invoked.
+ *
+ * @return The resolved access delegation mode, or null if not yet resolved
+ */
+ protected AccessDelegationMode getResolvedAccessDelegationMode() {
Review Comment:
I just removed it.
##########
runtime/service/src/main/java/org/apache/polaris/service/catalog/DefaultAccessDelegationModeResolver.java:
##########
@@ -0,0 +1,220 @@
+/*
+ * 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.catalog;
+
+import static
org.apache.polaris.service.catalog.AccessDelegationMode.REMOTE_SIGNING;
+import static org.apache.polaris.service.catalog.AccessDelegationMode.UNKNOWN;
+import static
org.apache.polaris.service.catalog.AccessDelegationMode.VENDED_CREDENTIALS;
+
+import jakarta.annotation.Nonnull;
+import jakarta.annotation.Nullable;
+import jakarta.enterprise.context.RequestScoped;
+import jakarta.inject.Inject;
+import java.util.EnumSet;
+import org.apache.polaris.core.config.FeatureConfiguration;
+import org.apache.polaris.core.config.PolarisConfigurationStore;
+import org.apache.polaris.core.context.RealmContext;
+import org.apache.polaris.core.entity.CatalogEntity;
+import org.apache.polaris.core.storage.PolarisStorageConfigurationInfo;
+import org.apache.polaris.core.storage.aws.AwsStorageConfigurationInfo;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+/**
+ * Default implementation of {@link AccessDelegationModeResolver} that
resolves the optimal access
+ * delegation mode based on catalog capabilities and configuration.
+ *
+ * <p>The resolution logic:
+ *
+ * <ol>
+ * <li>If no delegation mode is requested, returns {@link
AccessDelegationMode#UNKNOWN}
+ * <li>If exactly one delegation mode is requested (excluding UNKNOWN),
returns that mode
+ * <li>If both {@link AccessDelegationMode#VENDED_CREDENTIALS} and {@link
+ * AccessDelegationMode#REMOTE_SIGNING} are requested:
+ * <ul>
+ * <li>If STS is unavailable for the catalog's AWS storage, returns
{@link
+ * AccessDelegationMode#REMOTE_SIGNING}
+ * <li>If credential subscoping is skipped, returns {@link
+ * AccessDelegationMode#REMOTE_SIGNING}
+ * <li>Otherwise, returns {@link
AccessDelegationMode#VENDED_CREDENTIALS}
+ * </ul>
+ * </ol>
+ */
+@RequestScoped
+public class DefaultAccessDelegationModeResolver implements
AccessDelegationModeResolver {
+
+ private static final Logger LOGGER =
+ LoggerFactory.getLogger(DefaultAccessDelegationModeResolver.class);
+
+ private final PolarisConfigurationStore configurationStore;
+ private final RealmContext realmContext;
+
+ @Inject
+ public DefaultAccessDelegationModeResolver(
+ PolarisConfigurationStore configurationStore, RealmContext realmContext)
{
+ this.configurationStore = configurationStore;
+ this.realmContext = realmContext;
+ }
+
+ @Override
+ @Nonnull
+ public AccessDelegationMode resolve(
+ @Nonnull EnumSet<AccessDelegationMode> requestedModes,
+ @Nullable CatalogEntity catalogEntity) {
+
+ // Case 1: No delegation mode requested
+ if (requestedModes.isEmpty()) {
+ LOGGER.debug("No delegation mode requested, returning UNKNOWN");
+ return UNKNOWN;
+ }
+
+ // Filter out UNKNOWN mode from consideration for selection logic
+ EnumSet<AccessDelegationMode> effectiveModes =
EnumSet.copyOf(requestedModes);
+ effectiveModes.remove(UNKNOWN);
+
+ if (effectiveModes.isEmpty()) {
+ LOGGER.debug("Only UNKNOWN mode requested, returning UNKNOWN");
+ return UNKNOWN;
+ }
+
+ // Case 2: Exactly one delegation mode requested
+ if (effectiveModes.size() == 1) {
+ AccessDelegationMode mode = effectiveModes.iterator().next();
+ LOGGER.debug("Single delegation mode requested: {}", mode);
+ return mode;
+ }
+
+ // Case 3: Both VENDED_CREDENTIALS and REMOTE_SIGNING requested
+ if (effectiveModes.contains(VENDED_CREDENTIALS) &&
effectiveModes.contains(REMOTE_SIGNING)) {
+ return resolveVendedCredentialsVsRemoteSigning(catalogEntity);
+ }
+
+ // Case 4: Unknown combination - default to VENDED_CREDENTIALS for
backward compatibility
+ LOGGER.warn(
+ "Unknown access delegation mode combination: {}, defaulting to
VENDED_CREDENTIALS",
+ requestedModes);
+ return VENDED_CREDENTIALS;
Review Comment:
I’ve removed the fallback to `VENDED_CREDENTIALS` when catalogEntity is null
and now I throw the error
```java
// Case 4: Unknown combination - reject to prevent unintended credential
exposure
throw new IllegalArgumentException(
"Unsupported access delegation mode combination: " + requestedModes);
}
```
--
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]
