This is an automated email from the ASF dual-hosted git repository.
dimas pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/polaris.git
The following commit(s) were added to refs/heads/main by this push:
new ce015b61a Unify create/loadTable call paths (#2589)
ce015b61a is described below
commit ce015b61ae5a7e3a484c951a8ff7c0dc64abcd3b
Author: Dmitri Bourlatchkov <dmitri.bourlatch...@gmail.com>
AuthorDate: Fri Sep 19 11:14:53 2025 -0400
Unify create/loadTable call paths (#2589)
In preparation for implementing sending non-credential config
to REST Catalog clients for #2207 this PR unifies calls paths
for create/load table operations.
This change does not have any differences in authorization.
This change is not expecte to have any material behaviour
differences to the affected code paths.
The main idea is to consolidate decision-making for that
to include into REST responses and use method parameters
like `EnumSet<AccessDelegationMode> delegationModes` for
driving those decisions.
---
.../catalog/iceberg/IcebergCatalogAdapter.java | 37 ++---
.../catalog/iceberg/IcebergCatalogHandler.java | 175 +++++++++++++--------
2 files changed, 119 insertions(+), 93 deletions(-)
diff --git
a/runtime/service/src/main/java/org/apache/polaris/service/catalog/iceberg/IcebergCatalogAdapter.java
b/runtime/service/src/main/java/org/apache/polaris/service/catalog/iceberg/IcebergCatalogAdapter.java
index 0560c6497..a9552e78b 100644
---
a/runtime/service/src/main/java/org/apache/polaris/service/catalog/iceberg/IcebergCatalogAdapter.java
+++
b/runtime/service/src/main/java/org/apache/polaris/service/catalog/iceberg/IcebergCatalogAdapter.java
@@ -375,23 +375,14 @@ public class IcebergCatalogAdapter
prefix,
TableIdentifier.of(namespace, createTableRequest.name()));
if (createTableRequest.stageCreate()) {
- if (delegationModes.isEmpty()) {
- return Response.ok(catalog.createTableStaged(ns,
createTableRequest)).build();
- } else {
- return Response.ok(
- catalog.createTableStagedWithWriteDelegation(
- ns, createTableRequest, refreshCredentialsEndpoint))
- .build();
- }
- } else if (delegationModes.isEmpty()) {
- LoadTableResponse response = catalog.createTableDirect(ns,
createTableRequest);
- return tryInsertETagHeader(
- Response.ok(response), response, namespace,
createTableRequest.name())
+ return Response.ok(
+ catalog.createTableStaged(
+ ns, createTableRequest, delegationModes,
refreshCredentialsEndpoint))
.build();
} else {
LoadTableResponse response =
- catalog.createTableDirectWithWriteDelegation(
- ns, createTableRequest, refreshCredentialsEndpoint);
+ catalog.createTableDirect(
+ ns, createTableRequest, delegationModes,
refreshCredentialsEndpoint);
return tryInsertETagHeader(
Response.ok(response), response, namespace,
createTableRequest.name())
.build();
@@ -439,17 +430,13 @@ public class IcebergCatalogAdapter
securityContext,
prefix,
catalog -> {
- Optional<LoadTableResponse> response;
-
- if (delegationModes.isEmpty()) {
- response = catalog.loadTableIfStale(tableIdentifier, ifNoneMatch,
snapshots);
- } else {
- Optional<String> refreshCredentialsEndpoint =
- getRefreshCredentialsEndpoint(delegationModes, prefix,
tableIdentifier);
- response =
- catalog.loadTableWithAccessDelegationIfStale(
- tableIdentifier, ifNoneMatch, snapshots,
refreshCredentialsEndpoint);
- }
+ Optional<LoadTableResponse> response =
+ catalog.loadTable(
+ tableIdentifier,
+ snapshots,
+ ifNoneMatch,
+ delegationModes,
+ getRefreshCredentialsEndpoint(delegationModes, prefix,
tableIdentifier));
if (response.isEmpty()) {
return Response.notModified().build();
diff --git
a/runtime/service/src/main/java/org/apache/polaris/service/catalog/iceberg/IcebergCatalogHandler.java
b/runtime/service/src/main/java/org/apache/polaris/service/catalog/iceberg/IcebergCatalogHandler.java
index 265413b20..2b7c85384 100644
---
a/runtime/service/src/main/java/org/apache/polaris/service/catalog/iceberg/IcebergCatalogHandler.java
+++
b/runtime/service/src/main/java/org/apache/polaris/service/catalog/iceberg/IcebergCatalogHandler.java
@@ -19,6 +19,7 @@
package org.apache.polaris.service.catalog.iceberg;
import static
org.apache.polaris.core.config.FeatureConfiguration.LIST_PAGINATION_ENABLED;
+import static
org.apache.polaris.service.catalog.AccessDelegationMode.VENDED_CREDENTIALS;
import com.google.common.base.Preconditions;
import com.google.common.collect.Maps;
@@ -32,6 +33,7 @@ import java.time.OffsetDateTime;
import java.time.ZoneOffset;
import java.util.ArrayList;
import java.util.Arrays;
+import java.util.EnumSet;
import java.util.HashSet;
import java.util.List;
import java.util.Map;
@@ -98,6 +100,7 @@ import
org.apache.polaris.core.persistence.resolver.ResolutionManifestFactory;
import org.apache.polaris.core.secrets.UserSecretsManager;
import org.apache.polaris.core.storage.AccessConfig;
import org.apache.polaris.core.storage.PolarisStorageActions;
+import org.apache.polaris.service.catalog.AccessDelegationMode;
import org.apache.polaris.service.catalog.SupportsNotifications;
import org.apache.polaris.service.catalog.common.CatalogHandler;
import org.apache.polaris.service.config.ReservedProperties;
@@ -374,25 +377,8 @@ public class IcebergCatalogHandler extends CatalogHandler
implements AutoCloseab
* @return ETagged {@link LoadTableResponse} to uniquely identify the table
metadata
*/
public LoadTableResponse createTableDirect(Namespace namespace,
CreateTableRequest request) {
- PolarisAuthorizableOperation op =
PolarisAuthorizableOperation.CREATE_TABLE_DIRECT;
- TableIdentifier identifier = TableIdentifier.of(namespace, request.name());
- authorizeCreateTableLikeUnderNamespaceOperationOrThrow(op, identifier);
-
- CatalogEntity catalog = getResolvedCatalogEntity();
- if (catalog.isStaticFacade()) {
- throw new BadRequestException("Cannot create table on static-facade
external catalogs.");
- }
- CreateTableRequest requestWithoutReservedProperties =
- CreateTableRequest.builder()
- .withName(request.name())
- .withLocation(request.location())
- .withPartitionSpec(request.spec())
- .withSchema(request.schema())
- .withWriteOrder(request.writeOrder())
-
.setProperties(reservedProperties.removeReservedProperties(request.properties()))
- .build();
- return catalogHandlerUtils.createTable(
- baseCatalog, namespace, requestWithoutReservedProperties);
+ return createTableDirect(
+ namespace, request, EnumSet.noneOf(AccessDelegationMode.class),
Optional.empty());
}
/**
@@ -406,10 +392,32 @@ public class IcebergCatalogHandler extends CatalogHandler
implements AutoCloseab
Namespace namespace,
CreateTableRequest request,
Optional<String> refreshCredentialsEndpoint) {
- PolarisAuthorizableOperation op =
- PolarisAuthorizableOperation.CREATE_TABLE_DIRECT_WITH_WRITE_DELEGATION;
- authorizeCreateTableLikeUnderNamespaceOperationOrThrow(
- op, TableIdentifier.of(namespace, request.name()));
+ return createTableDirect(
+ namespace, request, EnumSet.of(VENDED_CREDENTIALS),
refreshCredentialsEndpoint);
+ }
+
+ public void authorizeCreateTableDirect(
+ Namespace namespace,
+ CreateTableRequest request,
+ EnumSet<AccessDelegationMode> delegationModes) {
+ if (delegationModes.isEmpty()) {
+ TableIdentifier identifier = TableIdentifier.of(namespace,
request.name());
+ authorizeCreateTableLikeUnderNamespaceOperationOrThrow(
+ PolarisAuthorizableOperation.CREATE_TABLE_DIRECT, identifier);
+ } else {
+ authorizeCreateTableLikeUnderNamespaceOperationOrThrow(
+
PolarisAuthorizableOperation.CREATE_TABLE_DIRECT_WITH_WRITE_DELEGATION,
+ TableIdentifier.of(namespace, request.name()));
+ }
+ }
+
+ public LoadTableResponse createTableDirect(
+ Namespace namespace,
+ CreateTableRequest request,
+ EnumSet<AccessDelegationMode> delegationModes,
+ Optional<String> refreshCredentialsEndpoint) {
+
+ authorizeCreateTableDirect(namespace, request, delegationModes);
CatalogEntity catalog = getResolvedCatalogEntity();
if (catalog.isStaticFacade()) {
@@ -440,11 +448,11 @@ public class IcebergCatalogHandler extends CatalogHandler
implements AutoCloseab
return buildLoadTableResponseWithDelegationCredentials(
tableIdentifier,
tableMetadata,
+ delegationModes,
Set.of(
PolarisStorageActions.READ,
PolarisStorageActions.WRITE,
PolarisStorageActions.LIST),
- SNAPSHOTS_ALL,
refreshCredentialsEndpoint)
.build();
} else if (table instanceof BaseMetadataTable) {
@@ -500,26 +508,40 @@ public class IcebergCatalogHandler extends CatalogHandler
implements AutoCloseab
}
public LoadTableResponse createTableStaged(Namespace namespace,
CreateTableRequest request) {
- PolarisAuthorizableOperation op =
PolarisAuthorizableOperation.CREATE_TABLE_STAGED;
- authorizeCreateTableLikeUnderNamespaceOperationOrThrow(
- op, TableIdentifier.of(namespace, request.name()));
+ return createTableStaged(
+ namespace, request, EnumSet.noneOf(AccessDelegationMode.class),
Optional.empty());
+ }
- CatalogEntity catalog = getResolvedCatalogEntity();
- if (catalog.isStaticFacade()) {
- throw new BadRequestException("Cannot create table on static-facade
external catalogs.");
+ public LoadTableResponse createTableStagedWithWriteDelegation(
+ Namespace namespace,
+ CreateTableRequest request,
+ Optional<String> refreshCredentialsEndpoint) {
+ return createTableStaged(
+ namespace, request, EnumSet.of(VENDED_CREDENTIALS),
refreshCredentialsEndpoint);
+ }
+
+ private void authorizeCreateTableStaged(
+ Namespace namespace,
+ CreateTableRequest request,
+ EnumSet<AccessDelegationMode> delegationModes) {
+ if (delegationModes.isEmpty()) {
+ authorizeCreateTableLikeUnderNamespaceOperationOrThrow(
+ PolarisAuthorizableOperation.CREATE_TABLE_STAGED,
+ TableIdentifier.of(namespace, request.name()));
+ } else {
+ authorizeCreateTableLikeUnderNamespaceOperationOrThrow(
+
PolarisAuthorizableOperation.CREATE_TABLE_STAGED_WITH_WRITE_DELEGATION,
+ TableIdentifier.of(namespace, request.name()));
}
- TableMetadata metadata = stageTableCreateHelper(namespace, request);
- return LoadTableResponse.builder().withTableMetadata(metadata).build();
}
- public LoadTableResponse createTableStagedWithWriteDelegation(
+ public LoadTableResponse createTableStaged(
Namespace namespace,
CreateTableRequest request,
+ EnumSet<AccessDelegationMode> delegationModes,
Optional<String> refreshCredentialsEndpoint) {
- PolarisAuthorizableOperation op =
- PolarisAuthorizableOperation.CREATE_TABLE_STAGED_WITH_WRITE_DELEGATION;
- authorizeCreateTableLikeUnderNamespaceOperationOrThrow(
- op, TableIdentifier.of(namespace, request.name()));
+
+ authorizeCreateTableStaged(namespace, request, delegationModes);
CatalogEntity catalog = getResolvedCatalogEntity();
if (catalog.isStaticFacade()) {
@@ -531,8 +553,8 @@ public class IcebergCatalogHandler extends CatalogHandler
implements AutoCloseab
return buildLoadTableResponseWithDelegationCredentials(
ident,
metadata,
+ EnumSet.of(VENDED_CREDENTIALS),
Set.of(PolarisStorageActions.ALL),
- SNAPSHOTS_ALL,
refreshCredentialsEndpoint)
.build();
}
@@ -616,32 +638,12 @@ public class IcebergCatalogHandler extends CatalogHandler
implements AutoCloseab
*/
public Optional<LoadTableResponse> loadTableIfStale(
TableIdentifier tableIdentifier, IfNoneMatch ifNoneMatch, String
snapshots) {
- PolarisAuthorizableOperation op = PolarisAuthorizableOperation.LOAD_TABLE;
- authorizeBasicTableLikeOperationOrThrow(
- op, PolarisEntitySubType.ICEBERG_TABLE, tableIdentifier);
-
- if (ifNoneMatch != null) {
- // Perform freshness-aware table loading if caller specified ifNoneMatch.
- IcebergTableLikeEntity tableEntity = getTableEntity(tableIdentifier);
- if (tableEntity == null || tableEntity.getMetadataLocation() == null) {
- LOGGER
- .atWarn()
- .addKeyValue("tableIdentifier", tableIdentifier)
- .addKeyValue("tableEntity", tableEntity)
- .log("Failed to getMetadataLocation to generate ETag when loading
table");
- } else {
- // TODO: Refactor null-checking into the helper method once we create
a more canonical
- // interface for associate etags with entities.
- String tableEntityTag =
-
IcebergHttpUtil.generateETagForMetadataFileLocation(tableEntity.getMetadataLocation());
- if (ifNoneMatch.anyMatch(tableEntityTag)) {
- return Optional.empty();
- }
- }
- }
-
- LoadTableResponse rawResponse = catalogHandlerUtils.loadTable(baseCatalog,
tableIdentifier);
- return Optional.of(filterResponseToSnapshots(rawResponse, snapshots));
+ return loadTable(
+ tableIdentifier,
+ snapshots,
+ ifNoneMatch,
+ EnumSet.noneOf(AccessDelegationMode.class),
+ Optional.empty());
}
public LoadTableResponse loadTableWithAccessDelegation(
@@ -668,6 +670,24 @@ public class IcebergCatalogHandler extends CatalogHandler
implements AutoCloseab
IfNoneMatch ifNoneMatch,
String snapshots,
Optional<String> refreshCredentialsEndpoint) {
+ return loadTable(
+ tableIdentifier,
+ snapshots,
+ ifNoneMatch,
+ EnumSet.of(VENDED_CREDENTIALS),
+ refreshCredentialsEndpoint);
+ }
+
+ private Set<PolarisStorageActions> authorizeLoadTable(
+ TableIdentifier tableIdentifier, EnumSet<AccessDelegationMode>
delegationModes) {
+ if (delegationModes.isEmpty()) {
+ authorizeBasicTableLikeOperationOrThrow(
+ PolarisAuthorizableOperation.LOAD_TABLE,
+ PolarisEntitySubType.ICEBERG_TABLE,
+ tableIdentifier);
+ return Set.of();
+ }
+
// Here we have a single method that falls through multiple candidate
// PolarisAuthorizableOperations because instead of identifying the
desired operation up-front
// and
@@ -709,6 +729,19 @@ public class IcebergCatalogHandler extends CatalogHandler
implements AutoCloseab
FeatureConfiguration.ALLOW_EXTERNAL_CATALOG_CREDENTIAL_VENDING.catalogConfig());
}
+ return actionsRequested;
+ }
+
+ public Optional<LoadTableResponse> loadTable(
+ TableIdentifier tableIdentifier,
+ String snapshots,
+ IfNoneMatch ifNoneMatch,
+ EnumSet<AccessDelegationMode> delegationModes,
+ Optional<String> refreshCredentialsEndpoint) {
+
+ Set<PolarisStorageActions> actionsRequested =
+ authorizeLoadTable(tableIdentifier, delegationModes);
+
if (ifNoneMatch != null) {
// Perform freshness-aware table loading if caller specified ifNoneMatch.
IcebergTableLikeEntity tableEntity = getTableEntity(tableIdentifier);
@@ -735,14 +768,15 @@ public class IcebergCatalogHandler extends CatalogHandler
implements AutoCloseab
if (table instanceof BaseTable baseTable) {
TableMetadata tableMetadata = baseTable.operations().current();
- return Optional.of(
+ LoadTableResponse response =
buildLoadTableResponseWithDelegationCredentials(
tableIdentifier,
tableMetadata,
+ delegationModes,
actionsRequested,
- snapshots,
refreshCredentialsEndpoint)
- .build());
+ .build();
+ return Optional.of(filterResponseToSnapshots(response, snapshots));
} else if (table instanceof BaseMetadataTable) {
// metadata tables are loaded on the client side, return
NoSuchTableException for now
throw new NoSuchTableException("Table does not exist: %s",
tableIdentifier.toString());
@@ -754,11 +788,16 @@ public class IcebergCatalogHandler extends CatalogHandler
implements AutoCloseab
private LoadTableResponse.Builder
buildLoadTableResponseWithDelegationCredentials(
TableIdentifier tableIdentifier,
TableMetadata tableMetadata,
+ EnumSet<AccessDelegationMode> delegationModes,
Set<PolarisStorageActions> actions,
- String snapshots,
Optional<String> refreshCredentialsEndpoint) {
LoadTableResponse.Builder responseBuilder =
LoadTableResponse.builder().withTableMetadata(tableMetadata);
+
+ if (!delegationModes.contains(VENDED_CREDENTIALS)) {
+ return responseBuilder;
+ }
+
if (baseCatalog instanceof SupportsCredentialDelegation
credentialDelegation) {
LOGGER
.atDebug()
