Skip to content

Commit 1a0d7ea

Browse files
dimas-bdimas-b
committed
Unify create/loadTable call paths
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.
1 parent d8602f6 commit 1a0d7ea

File tree

2 files changed

+119
-93
lines changed

2 files changed

+119
-93
lines changed

‎runtime/service/src/main/java/org/apache/polaris/service/catalog/iceberg/IcebergCatalogAdapter.java‎

Lines changed: 12 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -375,23 +375,14 @@ public Response createTable(
375375
prefix,
376376
TableIdentifier.of(namespace, createTableRequest.name()));
377377
if (createTableRequest.stageCreate()) {
378-
if (delegationModes.isEmpty()) {
379-
return Response.ok(catalog.createTableStaged(ns, createTableRequest)).build();
380-
} else {
381-
return Response.ok(
382-
catalog.createTableStagedWithWriteDelegation(
383-
ns, createTableRequest, refreshCredentialsEndpoint))
384-
.build();
385-
}
386-
} else if (delegationModes.isEmpty()) {
387-
LoadTableResponse response = catalog.createTableDirect(ns, createTableRequest);
388-
return tryInsertETagHeader(
389-
Response.ok(response), response, namespace, createTableRequest.name())
378+
return Response.ok(
379+
catalog.createTableStaged(
380+
ns, createTableRequest, delegationModes, refreshCredentialsEndpoint))
390381
.build();
391382
} else {
392383
LoadTableResponse response =
393-
catalog.createTableDirectWithWriteDelegation(
394-
ns, createTableRequest, refreshCredentialsEndpoint);
384+
catalog.createTableDirect(
385+
ns, createTableRequest, delegationModes, refreshCredentialsEndpoint);
395386
return tryInsertETagHeader(
396387
Response.ok(response), response, namespace, createTableRequest.name())
397388
.build();
@@ -439,17 +430,13 @@ public Response loadTable(
439430
securityContext,
440431
prefix,
441432
catalog -> {
442-
Optional<LoadTableResponse> response;
443-
444-
if (delegationModes.isEmpty()) {
445-
response = catalog.loadTableIfStale(tableIdentifier, ifNoneMatch, snapshots);
446-
} else {
447-
Optional<String> refreshCredentialsEndpoint =
448-
getRefreshCredentialsEndpoint(delegationModes, prefix, tableIdentifier);
449-
response =
450-
catalog.loadTableWithAccessDelegationIfStale(
451-
tableIdentifier, ifNoneMatch, snapshots, refreshCredentialsEndpoint);
452-
}
433+
Optional<LoadTableResponse> response =
434+
catalog.loadTable(
435+
tableIdentifier,
436+
snapshots,
437+
ifNoneMatch,
438+
delegationModes,
439+
getRefreshCredentialsEndpoint(delegationModes, prefix, tableIdentifier));
453440

454441
if (response.isEmpty()) {
455442
return Response.notModified().build();

‎runtime/service/src/main/java/org/apache/polaris/service/catalog/iceberg/IcebergCatalogHandler.java‎

Lines changed: 107 additions & 68 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@
1919
package org.apache.polaris.service.catalog.iceberg;
2020

2121
import static org.apache.polaris.core.config.FeatureConfiguration.LIST_PAGINATION_ENABLED;
22+
import static org.apache.polaris.service.catalog.AccessDelegationMode.VENDED_CREDENTIALS;
2223

2324
import com.google.common.base.Preconditions;
2425
import com.google.common.collect.Maps;
@@ -32,6 +33,7 @@
3233
import java.time.ZoneOffset;
3334
import java.util.ArrayList;
3435
import java.util.Arrays;
36+
import java.util.EnumSet;
3537
import java.util.HashSet;
3638
import java.util.List;
3739
import java.util.Map;
@@ -98,6 +100,7 @@
98100
import org.apache.polaris.core.secrets.UserSecretsManager;
99101
import org.apache.polaris.core.storage.AccessConfig;
100102
import org.apache.polaris.core.storage.PolarisStorageActions;
103+
import org.apache.polaris.service.catalog.AccessDelegationMode;
101104
import org.apache.polaris.service.catalog.SupportsNotifications;
102105
import org.apache.polaris.service.catalog.common.CatalogHandler;
103106
import org.apache.polaris.service.config.ReservedProperties;
@@ -374,25 +377,8 @@ public ListTablesResponse listTables(Namespace namespace) {
374377
* @return ETagged {@link LoadTableResponse} to uniquely identify the table metadata
375378
*/
376379
public LoadTableResponse createTableDirect(Namespace namespace, CreateTableRequest request) {
377-
PolarisAuthorizableOperation op = PolarisAuthorizableOperation.CREATE_TABLE_DIRECT;
378-
TableIdentifier identifier = TableIdentifier.of(namespace, request.name());
379-
authorizeCreateTableLikeUnderNamespaceOperationOrThrow(op, identifier);
380-
381-
CatalogEntity catalog = getResolvedCatalogEntity();
382-
if (catalog.isStaticFacade()) {
383-
throw new BadRequestException("Cannot create table on static-facade external catalogs.");
384-
}
385-
CreateTableRequest requestWithoutReservedProperties =
386-
CreateTableRequest.builder()
387-
.withName(request.name())
388-
.withLocation(request.location())
389-
.withPartitionSpec(request.spec())
390-
.withSchema(request.schema())
391-
.withWriteOrder(request.writeOrder())
392-
.setProperties(reservedProperties.removeReservedProperties(request.properties()))
393-
.build();
394-
return catalogHandlerUtils.createTable(
395-
baseCatalog, namespace, requestWithoutReservedProperties);
380+
return createTableDirect(
381+
namespace, request, EnumSet.noneOf(AccessDelegationMode.class), Optional.empty());
396382
}
397383

398384
/**
@@ -406,10 +392,32 @@ public LoadTableResponse createTableDirectWithWriteDelegation(
406392
Namespace namespace,
407393
CreateTableRequest request,
408394
Optional<String> refreshCredentialsEndpoint) {
409-
PolarisAuthorizableOperation op =
410-
PolarisAuthorizableOperation.CREATE_TABLE_DIRECT_WITH_WRITE_DELEGATION;
411-
authorizeCreateTableLikeUnderNamespaceOperationOrThrow(
412-
op, TableIdentifier.of(namespace, request.name()));
395+
return createTableDirect(
396+
namespace, request, EnumSet.of(VENDED_CREDENTIALS), refreshCredentialsEndpoint);
397+
}
398+
399+
public void authorizeCreateTableDirect(
400+
Namespace namespace,
401+
CreateTableRequest request,
402+
EnumSet<AccessDelegationMode> delegationModes) {
403+
if (delegationModes.isEmpty()) {
404+
TableIdentifier identifier = TableIdentifier.of(namespace, request.name());
405+
authorizeCreateTableLikeUnderNamespaceOperationOrThrow(
406+
PolarisAuthorizableOperation.CREATE_TABLE_DIRECT, identifier);
407+
} else {
408+
authorizeCreateTableLikeUnderNamespaceOperationOrThrow(
409+
PolarisAuthorizableOperation.CREATE_TABLE_DIRECT_WITH_WRITE_DELEGATION,
410+
TableIdentifier.of(namespace, request.name()));
411+
}
412+
}
413+
414+
public LoadTableResponse createTableDirect(
415+
Namespace namespace,
416+
CreateTableRequest request,
417+
EnumSet<AccessDelegationMode> delegationModes,
418+
Optional<String> refreshCredentialsEndpoint) {
419+
420+
authorizeCreateTableDirect(namespace, request, delegationModes);
413421

414422
CatalogEntity catalog = getResolvedCatalogEntity();
415423
if (catalog.isStaticFacade()) {
@@ -440,11 +448,11 @@ public LoadTableResponse createTableDirectWithWriteDelegation(
440448
return buildLoadTableResponseWithDelegationCredentials(
441449
tableIdentifier,
442450
tableMetadata,
451+
delegationModes,
443452
Set.of(
444453
PolarisStorageActions.READ,
445454
PolarisStorageActions.WRITE,
446455
PolarisStorageActions.LIST),
447-
SNAPSHOTS_ALL,
448456
refreshCredentialsEndpoint)
449457
.build();
450458
} else if (table instanceof BaseMetadataTable) {
@@ -500,26 +508,40 @@ private TableMetadata stageTableCreateHelper(Namespace namespace, CreateTableReq
500508
}
501509

502510
public LoadTableResponse createTableStaged(Namespace namespace, CreateTableRequest request) {
503-
PolarisAuthorizableOperation op = PolarisAuthorizableOperation.CREATE_TABLE_STAGED;
504-
authorizeCreateTableLikeUnderNamespaceOperationOrThrow(
505-
op, TableIdentifier.of(namespace, request.name()));
511+
return createTableStaged(
512+
namespace, request, EnumSet.noneOf(AccessDelegationMode.class), Optional.empty());
513+
}
506514

507-
CatalogEntity catalog = getResolvedCatalogEntity();
508-
if (catalog.isStaticFacade()) {
509-
throw new BadRequestException("Cannot create table on static-facade external catalogs.");
515+
public LoadTableResponse createTableStagedWithWriteDelegation(
516+
Namespace namespace,
517+
CreateTableRequest request,
518+
Optional<String> refreshCredentialsEndpoint) {
519+
return createTableStaged(
520+
namespace, request, EnumSet.of(VENDED_CREDENTIALS), refreshCredentialsEndpoint);
521+
}
522+
523+
private void authorizeCreateTableStaged(
524+
Namespace namespace,
525+
CreateTableRequest request,
526+
EnumSet<AccessDelegationMode> delegationModes) {
527+
if (delegationModes.isEmpty()) {
528+
authorizeCreateTableLikeUnderNamespaceOperationOrThrow(
529+
PolarisAuthorizableOperation.CREATE_TABLE_STAGED,
530+
TableIdentifier.of(namespace, request.name()));
531+
} else {
532+
authorizeCreateTableLikeUnderNamespaceOperationOrThrow(
533+
PolarisAuthorizableOperation.CREATE_TABLE_STAGED_WITH_WRITE_DELEGATION,
534+
TableIdentifier.of(namespace, request.name()));
510535
}
511-
TableMetadata metadata = stageTableCreateHelper(namespace, request);
512-
return LoadTableResponse.builder().withTableMetadata(metadata).build();
513536
}
514537

515-
public LoadTableResponse createTableStagedWithWriteDelegation(
538+
public LoadTableResponse createTableStaged(
516539
Namespace namespace,
517540
CreateTableRequest request,
541+
EnumSet<AccessDelegationMode> delegationModes,
518542
Optional<String> refreshCredentialsEndpoint) {
519-
PolarisAuthorizableOperation op =
520-
PolarisAuthorizableOperation.CREATE_TABLE_STAGED_WITH_WRITE_DELEGATION;
521-
authorizeCreateTableLikeUnderNamespaceOperationOrThrow(
522-
op, TableIdentifier.of(namespace, request.name()));
543+
544+
authorizeCreateTableStaged(namespace, request, delegationModes);
523545

524546
CatalogEntity catalog = getResolvedCatalogEntity();
525547
if (catalog.isStaticFacade()) {
@@ -531,8 +553,8 @@ public LoadTableResponse createTableStagedWithWriteDelegation(
531553
return buildLoadTableResponseWithDelegationCredentials(
532554
ident,
533555
metadata,
556+
EnumSet.of(VENDED_CREDENTIALS),
534557
Set.of(PolarisStorageActions.ALL),
535-
SNAPSHOTS_ALL,
536558
refreshCredentialsEndpoint)
537559
.build();
538560
}
@@ -616,32 +638,12 @@ public LoadTableResponse loadTable(TableIdentifier tableIdentifier, String snaps
616638
*/
617639
public Optional<LoadTableResponse> loadTableIfStale(
618640
TableIdentifier tableIdentifier, IfNoneMatch ifNoneMatch, String snapshots) {
619-
PolarisAuthorizableOperation op = PolarisAuthorizableOperation.LOAD_TABLE;
620-
authorizeBasicTableLikeOperationOrThrow(
621-
op, PolarisEntitySubType.ICEBERG_TABLE, tableIdentifier);
622-
623-
if (ifNoneMatch != null) {
624-
// Perform freshness-aware table loading if caller specified ifNoneMatch.
625-
IcebergTableLikeEntity tableEntity = getTableEntity(tableIdentifier);
626-
if (tableEntity == null || tableEntity.getMetadataLocation() == null) {
627-
LOGGER
628-
.atWarn()
629-
.addKeyValue("tableIdentifier", tableIdentifier)
630-
.addKeyValue("tableEntity", tableEntity)
631-
.log("Failed to getMetadataLocation to generate ETag when loading table");
632-
} else {
633-
// TODO: Refactor null-checking into the helper method once we create a more canonical
634-
// interface for associate etags with entities.
635-
String tableEntityTag =
636-
IcebergHttpUtil.generateETagForMetadataFileLocation(tableEntity.getMetadataLocation());
637-
if (ifNoneMatch.anyMatch(tableEntityTag)) {
638-
return Optional.empty();
639-
}
640-
}
641-
}
642-
643-
LoadTableResponse rawResponse = catalogHandlerUtils.loadTable(baseCatalog, tableIdentifier);
644-
return Optional.of(filterResponseToSnapshots(rawResponse, snapshots));
641+
return loadTable(
642+
tableIdentifier,
643+
snapshots,
644+
ifNoneMatch,
645+
EnumSet.noneOf(AccessDelegationMode.class),
646+
Optional.empty());
645647
}
646648

647649
public LoadTableResponse loadTableWithAccessDelegation(
@@ -668,6 +670,24 @@ public Optional<LoadTableResponse> loadTableWithAccessDelegationIfStale(
668670
IfNoneMatch ifNoneMatch,
669671
String snapshots,
670672
Optional<String> refreshCredentialsEndpoint) {
673+
return loadTable(
674+
tableIdentifier,
675+
snapshots,
676+
ifNoneMatch,
677+
EnumSet.of(VENDED_CREDENTIALS),
678+
refreshCredentialsEndpoint);
679+
}
680+
681+
private Set<PolarisStorageActions> authorizeLoadTable(
682+
TableIdentifier tableIdentifier, EnumSet<AccessDelegationMode> delegationModes) {
683+
if (delegationModes.isEmpty()) {
684+
authorizeBasicTableLikeOperationOrThrow(
685+
PolarisAuthorizableOperation.LOAD_TABLE,
686+
PolarisEntitySubType.ICEBERG_TABLE,
687+
tableIdentifier);
688+
return Set.of();
689+
}
690+
671691
// Here we have a single method that falls through multiple candidate
672692
// PolarisAuthorizableOperations because instead of identifying the desired operation up-front
673693
// and
@@ -709,6 +729,19 @@ public Optional<LoadTableResponse> loadTableWithAccessDelegationIfStale(
709729
FeatureConfiguration.ALLOW_EXTERNAL_CATALOG_CREDENTIAL_VENDING.catalogConfig());
710730
}
711731

732+
return actionsRequested;
733+
}
734+
735+
public Optional<LoadTableResponse> loadTable(
736+
TableIdentifier tableIdentifier,
737+
String snapshots,
738+
IfNoneMatch ifNoneMatch,
739+
EnumSet<AccessDelegationMode> delegationModes,
740+
Optional<String> refreshCredentialsEndpoint) {
741+
742+
Set<PolarisStorageActions> actionsRequested =
743+
authorizeLoadTable(tableIdentifier, delegationModes);
744+
712745
if (ifNoneMatch != null) {
713746
// Perform freshness-aware table loading if caller specified ifNoneMatch.
714747
IcebergTableLikeEntity tableEntity = getTableEntity(tableIdentifier);
@@ -735,14 +768,15 @@ public Optional<LoadTableResponse> loadTableWithAccessDelegationIfStale(
735768

736769
if (table instanceof BaseTable baseTable) {
737770
TableMetadata tableMetadata = baseTable.operations().current();
738-
return Optional.of(
771+
LoadTableResponse response =
739772
buildLoadTableResponseWithDelegationCredentials(
740773
tableIdentifier,
741774
tableMetadata,
775+
delegationModes,
742776
actionsRequested,
743-
snapshots,
744777
refreshCredentialsEndpoint)
745-
.build());
778+
.build();
779+
return Optional.of(filterResponseToSnapshots(response, snapshots));
746780
} else if (table instanceof BaseMetadataTable) {
747781
// metadata tables are loaded on the client side, return NoSuchTableException for now
748782
throw new NoSuchTableException("Table does not exist: %s", tableIdentifier.toString());
@@ -754,11 +788,16 @@ public Optional<LoadTableResponse> loadTableWithAccessDelegationIfStale(
754788
private LoadTableResponse.Builder buildLoadTableResponseWithDelegationCredentials(
755789
TableIdentifier tableIdentifier,
756790
TableMetadata tableMetadata,
791+
EnumSet<AccessDelegationMode> delegationModes,
757792
Set<PolarisStorageActions> actions,
758-
String snapshots,
759793
Optional<String> refreshCredentialsEndpoint) {
760794
LoadTableResponse.Builder responseBuilder =
761795
LoadTableResponse.builder().withTableMetadata(tableMetadata);
796+
797+
if (!delegationModes.contains(VENDED_CREDENTIALS)) {
798+
return responseBuilder;
799+
}
800+
762801
if (baseCatalog instanceof SupportsCredentialDelegation credentialDelegation) {
763802
LOGGER
764803
.atDebug()

0 commit comments

Comments
 (0)