From 63c7dc7422ce5e9874de7f0ed58fd21c74c906d3 Mon Sep 17 00:00:00 2001 From: David Lu Date: Wed, 12 Mar 2025 17:05:24 -0400 Subject: [PATCH 01/20] add refresh credentials property to loadTableResult --- ...PolarisCatalogHandlerWrapperAuthzTest.java | 10 +++++---- .../catalog/IcebergCatalogAdapter.java | 5 +++-- .../catalog/PolarisCatalogHandlerWrapper.java | 21 +++++++++++++++---- 3 files changed, 26 insertions(+), 10 deletions(-) diff --git a/quarkus/service/src/test/java/org/apache/polaris/service/quarkus/catalog/PolarisCatalogHandlerWrapperAuthzTest.java b/quarkus/service/src/test/java/org/apache/polaris/service/quarkus/catalog/PolarisCatalogHandlerWrapperAuthzTest.java index a17091cba..5ea43e71b 100644 --- a/quarkus/service/src/test/java/org/apache/polaris/service/quarkus/catalog/PolarisCatalogHandlerWrapperAuthzTest.java +++ b/quarkus/service/src/test/java/org/apache/polaris/service/quarkus/catalog/PolarisCatalogHandlerWrapperAuthzTest.java @@ -23,6 +23,7 @@ import io.quarkus.test.junit.TestProfile; import jakarta.ws.rs.core.SecurityContext; import java.time.Instant; +import java.util.EnumSet; import java.util.List; import java.util.Map; import java.util.Set; @@ -64,6 +65,7 @@ import org.apache.polaris.core.persistence.PolarisEntityManager; import org.apache.polaris.core.persistence.dao.entity.CreatePrincipalResult; import org.apache.polaris.core.persistence.resolver.PolarisResolutionManifest; +import org.apache.polaris.service.catalog.AccessDelegationMode; import org.apache.polaris.service.catalog.PolarisCatalogHandlerWrapper; import org.apache.polaris.service.catalog.io.DefaultFileIOFactory; import org.apache.polaris.service.config.RealmEntityManagerFactory; @@ -872,7 +874,7 @@ public void testLoadTableWithReadAccessDelegationSufficientPrivileges() { PolarisPrivilege.TABLE_READ_DATA, PolarisPrivilege.TABLE_WRITE_DATA, PolarisPrivilege.CATALOG_MANAGE_CONTENT), - () -> newWrapper().loadTableWithAccessDelegation(TABLE_NS1A_2, "all"), + () -> newWrapper().loadTableWithAccessDelegation(TABLE_NS1A_2, "all", EnumSet.noneOf(AccessDelegationMode.class)), null /* cleanupAction */); } @@ -888,7 +890,7 @@ public void testLoadTableWithReadAccessDelegationInsufficientPermissions() { PolarisPrivilege.TABLE_CREATE, PolarisPrivilege.TABLE_LIST, PolarisPrivilege.TABLE_DROP), - () -> newWrapper().loadTableWithAccessDelegation(TABLE_NS1A_2, "all")); + () -> newWrapper().loadTableWithAccessDelegation(TABLE_NS1A_2, "all", EnumSet.noneOf(AccessDelegationMode.class))); } @Test @@ -901,7 +903,7 @@ public void testLoadTableWithWriteAccessDelegationSufficientPrivileges() { PolarisPrivilege.TABLE_READ_DATA, PolarisPrivilege.TABLE_WRITE_DATA, PolarisPrivilege.CATALOG_MANAGE_CONTENT), - () -> newWrapper().loadTableWithAccessDelegation(TABLE_NS1A_2, "all"), + () -> newWrapper().loadTableWithAccessDelegation(TABLE_NS1A_2, "all", EnumSet.noneOf(AccessDelegationMode.class)), null /* cleanupAction */); } @@ -917,7 +919,7 @@ public void testLoadTableWithWriteAccessDelegationInsufficientPermissions() { PolarisPrivilege.TABLE_CREATE, PolarisPrivilege.TABLE_LIST, PolarisPrivilege.TABLE_DROP), - () -> newWrapper().loadTableWithAccessDelegation(TABLE_NS1A_2, "all")); + () -> newWrapper().loadTableWithAccessDelegation(TABLE_NS1A_2, "all", EnumSet.noneOf(AccessDelegationMode.class))); } @Test diff --git a/service/common/src/main/java/org/apache/polaris/service/catalog/IcebergCatalogAdapter.java b/service/common/src/main/java/org/apache/polaris/service/catalog/IcebergCatalogAdapter.java index eb199f0e9..928afb531 100644 --- a/service/common/src/main/java/org/apache/polaris/service/catalog/IcebergCatalogAdapter.java +++ b/service/common/src/main/java/org/apache/polaris/service/catalog/IcebergCatalogAdapter.java @@ -30,6 +30,7 @@ import jakarta.ws.rs.core.SecurityContext; import java.net.URLEncoder; import java.nio.charset.Charset; +import java.util.Collections; import java.util.EnumSet; import java.util.Map; import java.util.Optional; @@ -348,7 +349,7 @@ public Response loadTable( } else { return Response.ok( newHandlerWrapper(realmContext, securityContext, prefix) - .loadTableWithAccessDelegation(tableIdentifier, snapshots)) + .loadTableWithAccessDelegation(tableIdentifier, snapshots, delegationModes)) .build(); } } @@ -472,7 +473,7 @@ public Response loadCredentials( TableIdentifier tableIdentifier = TableIdentifier.of(ns, RESTUtil.decodeString(table)); LoadTableResponse loadTableResponse = newHandlerWrapper(realmContext, securityContext, prefix) - .loadTableWithAccessDelegation(tableIdentifier, "all"); + .loadTableWithAccessDelegation(tableIdentifier, "all", EnumSet.noneOf(AccessDelegationMode.class)); return Response.ok( ImmutableLoadCredentialsResponse.builder() .credentials(loadTableResponse.credentials()) diff --git a/service/common/src/main/java/org/apache/polaris/service/catalog/PolarisCatalogHandlerWrapper.java b/service/common/src/main/java/org/apache/polaris/service/catalog/PolarisCatalogHandlerWrapper.java index 837fb509f..d09cfbdaa 100644 --- a/service/common/src/main/java/org/apache/polaris/service/catalog/PolarisCatalogHandlerWrapper.java +++ b/service/common/src/main/java/org/apache/polaris/service/catalog/PolarisCatalogHandlerWrapper.java @@ -23,10 +23,15 @@ import jakarta.ws.rs.core.SecurityContext; import java.io.Closeable; import java.io.IOException; +import java.net.InetAddress; +import java.net.URI; +import java.net.URISyntaxException; +import java.net.UnknownHostException; 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; @@ -43,6 +48,7 @@ import org.apache.iceberg.TableMetadata; import org.apache.iceberg.TableOperations; import org.apache.iceberg.UpdateRequirement; +import org.apache.iceberg.aws.AwsClientProperties; import org.apache.iceberg.catalog.Catalog; import org.apache.iceberg.catalog.Namespace; import org.apache.iceberg.catalog.SupportsNamespaces; @@ -814,7 +820,7 @@ public LoadTableResponse loadTable(TableIdentifier tableIdentifier, String snaps } public LoadTableResponse loadTableWithAccessDelegation( - TableIdentifier tableIdentifier, String snapshots) { + TableIdentifier tableIdentifier, String snapshots, EnumSet accessDelegationModes) { // Here we have a single method that falls through multiple candidate // PolarisAuthorizableOperations because instead of identifying the desired operation up-front // and @@ -877,15 +883,22 @@ public LoadTableResponse loadTableWithAccessDelegation( credentialDelegation.getCredentialConfig( tableIdentifier, tableMetadata, actionsRequested)); } - return responseBuilder.build(); + if(accessDelegationModes.contains(AccessDelegationMode.VENDED_CREDENTIALS)){ + try { + String hostName = InetAddress.getLocalHost().getCanonicalHostName(); + responseBuilder.addConfig(AwsClientProperties.REFRESH_CREDENTIALS_ENABLED, "true"); + responseBuilder.addConfig(AwsClientProperties.REFRESH_CREDENTIALS_ENDPOINT, new URI("https", hostName, String.format("/v1/%s/namespaces/%s/tables/%s/credentials", catalogName, tableIdentifier.namespace().toString(), tableIdentifier.name()), null).toString()); + } catch (UnknownHostException | URISyntaxException e) { + LOGGER.error("Error while fetching hostname. Ignoring refresh credentials properties", e); + } + return responseBuilder.build();} } 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()); } throw new IllegalStateException("Cannot wrap catalog that does not produce BaseTable"); - }); - } + });} private UpdateTableRequest applyUpdateFilters(UpdateTableRequest request) { // Certain MetadataUpdates need to be explicitly transformed to achieve the same behavior From a885ffb9d25952984206d65944ab7ec9f7516efd Mon Sep 17 00:00:00 2001 From: David Lu Date: Wed, 12 Mar 2025 17:11:50 -0400 Subject: [PATCH 02/20] remove bad method and replace with localhost --- .../service/catalog/PolarisCatalogHandlerWrapper.java | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/service/common/src/main/java/org/apache/polaris/service/catalog/PolarisCatalogHandlerWrapper.java b/service/common/src/main/java/org/apache/polaris/service/catalog/PolarisCatalogHandlerWrapper.java index d09cfbdaa..1bb6972fb 100644 --- a/service/common/src/main/java/org/apache/polaris/service/catalog/PolarisCatalogHandlerWrapper.java +++ b/service/common/src/main/java/org/apache/polaris/service/catalog/PolarisCatalogHandlerWrapper.java @@ -885,10 +885,11 @@ public LoadTableResponse loadTableWithAccessDelegation( } if(accessDelegationModes.contains(AccessDelegationMode.VENDED_CREDENTIALS)){ try { - String hostName = InetAddress.getLocalHost().getCanonicalHostName(); + // String hostName = InetAddress.getLocalHost().getCanonicalHostName(); + String hostName = "localhost"; responseBuilder.addConfig(AwsClientProperties.REFRESH_CREDENTIALS_ENABLED, "true"); responseBuilder.addConfig(AwsClientProperties.REFRESH_CREDENTIALS_ENDPOINT, new URI("https", hostName, String.format("/v1/%s/namespaces/%s/tables/%s/credentials", catalogName, tableIdentifier.namespace().toString(), tableIdentifier.name()), null).toString()); - } catch (UnknownHostException | URISyntaxException e) { + } catch (URISyntaxException e) { LOGGER.error("Error while fetching hostname. Ignoring refresh credentials properties", e); } return responseBuilder.build();} From 7912eca7df1c53548625aa0a58b52f826f5b0ec2 Mon Sep 17 00:00:00 2001 From: David Lu Date: Wed, 12 Mar 2025 17:13:03 -0400 Subject: [PATCH 03/20] spotless --- ...PolarisCatalogHandlerWrapperAuthzTest.java | 20 ++++++++++--- .../catalog/IcebergCatalogAdapter.java | 4 +-- .../catalog/PolarisCatalogHandlerWrapper.java | 30 ++++++++++++++----- 3 files changed, 40 insertions(+), 14 deletions(-) diff --git a/quarkus/service/src/test/java/org/apache/polaris/service/quarkus/catalog/PolarisCatalogHandlerWrapperAuthzTest.java b/quarkus/service/src/test/java/org/apache/polaris/service/quarkus/catalog/PolarisCatalogHandlerWrapperAuthzTest.java index 5ea43e71b..a5829190d 100644 --- a/quarkus/service/src/test/java/org/apache/polaris/service/quarkus/catalog/PolarisCatalogHandlerWrapperAuthzTest.java +++ b/quarkus/service/src/test/java/org/apache/polaris/service/quarkus/catalog/PolarisCatalogHandlerWrapperAuthzTest.java @@ -874,7 +874,10 @@ public void testLoadTableWithReadAccessDelegationSufficientPrivileges() { PolarisPrivilege.TABLE_READ_DATA, PolarisPrivilege.TABLE_WRITE_DATA, PolarisPrivilege.CATALOG_MANAGE_CONTENT), - () -> newWrapper().loadTableWithAccessDelegation(TABLE_NS1A_2, "all", EnumSet.noneOf(AccessDelegationMode.class)), + () -> + newWrapper() + .loadTableWithAccessDelegation( + TABLE_NS1A_2, "all", EnumSet.noneOf(AccessDelegationMode.class)), null /* cleanupAction */); } @@ -890,7 +893,10 @@ public void testLoadTableWithReadAccessDelegationInsufficientPermissions() { PolarisPrivilege.TABLE_CREATE, PolarisPrivilege.TABLE_LIST, PolarisPrivilege.TABLE_DROP), - () -> newWrapper().loadTableWithAccessDelegation(TABLE_NS1A_2, "all", EnumSet.noneOf(AccessDelegationMode.class))); + () -> + newWrapper() + .loadTableWithAccessDelegation( + TABLE_NS1A_2, "all", EnumSet.noneOf(AccessDelegationMode.class))); } @Test @@ -903,7 +909,10 @@ public void testLoadTableWithWriteAccessDelegationSufficientPrivileges() { PolarisPrivilege.TABLE_READ_DATA, PolarisPrivilege.TABLE_WRITE_DATA, PolarisPrivilege.CATALOG_MANAGE_CONTENT), - () -> newWrapper().loadTableWithAccessDelegation(TABLE_NS1A_2, "all", EnumSet.noneOf(AccessDelegationMode.class)), + () -> + newWrapper() + .loadTableWithAccessDelegation( + TABLE_NS1A_2, "all", EnumSet.noneOf(AccessDelegationMode.class)), null /* cleanupAction */); } @@ -919,7 +928,10 @@ public void testLoadTableWithWriteAccessDelegationInsufficientPermissions() { PolarisPrivilege.TABLE_CREATE, PolarisPrivilege.TABLE_LIST, PolarisPrivilege.TABLE_DROP), - () -> newWrapper().loadTableWithAccessDelegation(TABLE_NS1A_2, "all", EnumSet.noneOf(AccessDelegationMode.class))); + () -> + newWrapper() + .loadTableWithAccessDelegation( + TABLE_NS1A_2, "all", EnumSet.noneOf(AccessDelegationMode.class))); } @Test diff --git a/service/common/src/main/java/org/apache/polaris/service/catalog/IcebergCatalogAdapter.java b/service/common/src/main/java/org/apache/polaris/service/catalog/IcebergCatalogAdapter.java index 928afb531..c4dae6599 100644 --- a/service/common/src/main/java/org/apache/polaris/service/catalog/IcebergCatalogAdapter.java +++ b/service/common/src/main/java/org/apache/polaris/service/catalog/IcebergCatalogAdapter.java @@ -30,7 +30,6 @@ import jakarta.ws.rs.core.SecurityContext; import java.net.URLEncoder; import java.nio.charset.Charset; -import java.util.Collections; import java.util.EnumSet; import java.util.Map; import java.util.Optional; @@ -473,7 +472,8 @@ public Response loadCredentials( TableIdentifier tableIdentifier = TableIdentifier.of(ns, RESTUtil.decodeString(table)); LoadTableResponse loadTableResponse = newHandlerWrapper(realmContext, securityContext, prefix) - .loadTableWithAccessDelegation(tableIdentifier, "all", EnumSet.noneOf(AccessDelegationMode.class)); + .loadTableWithAccessDelegation( + tableIdentifier, "all", EnumSet.noneOf(AccessDelegationMode.class)); return Response.ok( ImmutableLoadCredentialsResponse.builder() .credentials(loadTableResponse.credentials()) diff --git a/service/common/src/main/java/org/apache/polaris/service/catalog/PolarisCatalogHandlerWrapper.java b/service/common/src/main/java/org/apache/polaris/service/catalog/PolarisCatalogHandlerWrapper.java index 1bb6972fb..72bd1863b 100644 --- a/service/common/src/main/java/org/apache/polaris/service/catalog/PolarisCatalogHandlerWrapper.java +++ b/service/common/src/main/java/org/apache/polaris/service/catalog/PolarisCatalogHandlerWrapper.java @@ -23,10 +23,8 @@ import jakarta.ws.rs.core.SecurityContext; import java.io.Closeable; import java.io.IOException; -import java.net.InetAddress; import java.net.URI; import java.net.URISyntaxException; -import java.net.UnknownHostException; import java.time.OffsetDateTime; import java.time.ZoneOffset; import java.util.ArrayList; @@ -820,7 +818,9 @@ public LoadTableResponse loadTable(TableIdentifier tableIdentifier, String snaps } public LoadTableResponse loadTableWithAccessDelegation( - TableIdentifier tableIdentifier, String snapshots, EnumSet accessDelegationModes) { + TableIdentifier tableIdentifier, + String snapshots, + EnumSet accessDelegationModes) { // Here we have a single method that falls through multiple candidate // PolarisAuthorizableOperations because instead of identifying the desired operation up-front // and @@ -883,23 +883,37 @@ public LoadTableResponse loadTableWithAccessDelegation( credentialDelegation.getCredentialConfig( tableIdentifier, tableMetadata, actionsRequested)); } - if(accessDelegationModes.contains(AccessDelegationMode.VENDED_CREDENTIALS)){ + if (accessDelegationModes.contains(AccessDelegationMode.VENDED_CREDENTIALS)) { try { // String hostName = InetAddress.getLocalHost().getCanonicalHostName(); String hostName = "localhost"; responseBuilder.addConfig(AwsClientProperties.REFRESH_CREDENTIALS_ENABLED, "true"); - responseBuilder.addConfig(AwsClientProperties.REFRESH_CREDENTIALS_ENDPOINT, new URI("https", hostName, String.format("/v1/%s/namespaces/%s/tables/%s/credentials", catalogName, tableIdentifier.namespace().toString(), tableIdentifier.name()), null).toString()); + responseBuilder.addConfig( + AwsClientProperties.REFRESH_CREDENTIALS_ENDPOINT, + new URI( + "https", + hostName, + String.format( + "/v1/%s/namespaces/%s/tables/%s/credentials", + catalogName, + tableIdentifier.namespace().toString(), + tableIdentifier.name()), + null) + .toString()); } catch (URISyntaxException e) { - LOGGER.error("Error while fetching hostname. Ignoring refresh credentials properties", e); + LOGGER.error( + "Error while fetching hostname. Ignoring refresh credentials properties", e); } - return responseBuilder.build();} + return responseBuilder.build(); + } } 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()); } throw new IllegalStateException("Cannot wrap catalog that does not produce BaseTable"); - });} + }); + } private UpdateTableRequest applyUpdateFilters(UpdateTableRequest request) { // Certain MetadataUpdates need to be explicitly transformed to achieve the same behavior From aa6f1b1628ee932a430cfa52750319bc5d585bc7 Mon Sep 17 00:00:00 2001 From: David Lu Date: Fri, 14 Mar 2025 13:31:53 -0400 Subject: [PATCH 04/20] add request base uri to call context --- .../polaris/core/context/CallContext.java | 41 +++++++++++++++++++ .../quarkus/config/QuarkusProducers.java | 17 +++++++- .../catalog/PolarisCatalogHandlerWrapper.java | 34 ++++++--------- 3 files changed, 68 insertions(+), 24 deletions(-) diff --git a/polaris-core/src/main/java/org/apache/polaris/core/context/CallContext.java b/polaris-core/src/main/java/org/apache/polaris/core/context/CallContext.java index 71c457720..0999880b6 100644 --- a/polaris-core/src/main/java/org/apache/polaris/core/context/CallContext.java +++ b/polaris-core/src/main/java/org/apache/polaris/core/context/CallContext.java @@ -20,6 +20,7 @@ import jakarta.annotation.Nonnull; import java.io.IOException; +import java.net.URI; import java.util.HashMap; import java.util.Map; import java.util.stream.Collectors; @@ -72,6 +73,34 @@ static void unsetCurrentContext() { CURRENT_CONTEXT.remove(); } + static CallContext of( + final RealmContext realmContext, + final PolarisCallContext polarisCallContext, + final URI baseUri) { + Map map = new HashMap<>(); + return new CallContext() { + @Override + public RealmContext getRealmContext() { + return realmContext; + } + + @Override + public PolarisCallContext getPolarisCallContext() { + return polarisCallContext; + } + + @Override + public Map contextVariables() { + return map; + } + + @Override + public URI getBaseUri() { + return baseUri; + } + }; + } + static CallContext of( final RealmContext realmContext, final PolarisCallContext polarisCallContext) { Map map = new HashMap<>(); @@ -90,6 +119,11 @@ public PolarisCallContext getPolarisCallContext() { public Map contextVariables() { return map; } + + @Override + public URI getBaseUri() { + return null; + } }; } @@ -122,6 +156,11 @@ public PolarisCallContext getPolarisCallContext() { public Map contextVariables() { return contextVariables; } + + @Override + public URI getBaseUri() { + return base.getBaseUri(); + } }; } @@ -134,6 +173,8 @@ public Map contextVariables() { Map contextVariables(); + URI getBaseUri(); + default @Nonnull CloseableGroup closeables() { return (CloseableGroup) contextVariables().computeIfAbsent(CLOSEABLES, key -> new CloseableGroup()); diff --git a/quarkus/service/src/main/java/org/apache/polaris/service/quarkus/config/QuarkusProducers.java b/quarkus/service/src/main/java/org/apache/polaris/service/quarkus/config/QuarkusProducers.java index c017c6135..3122f8ea8 100644 --- a/quarkus/service/src/main/java/org/apache/polaris/service/quarkus/config/QuarkusProducers.java +++ b/quarkus/service/src/main/java/org/apache/polaris/service/quarkus/config/QuarkusProducers.java @@ -31,6 +31,8 @@ import jakarta.inject.Singleton; import jakarta.ws.rs.container.ContainerRequestContext; import jakarta.ws.rs.core.Context; +import java.net.URI; +import java.net.URISyntaxException; import java.time.Clock; import org.apache.polaris.core.PolarisCallContext; import org.apache.polaris.core.PolarisDefaultDiagServiceImpl; @@ -71,6 +73,9 @@ public class QuarkusProducers { + @ConfigProperty(name = "quarkus.http.proxy.enable-forwarded-prefix") + boolean enableForwardedPrefix; + @Produces @ApplicationScoped // cannot be singleton because it is mocked in tests public Clock clock() { @@ -120,8 +125,16 @@ public PolarisCallContext polarisCallContext( @Produces @RequestScoped - public CallContext callContext(RealmContext realmContext, PolarisCallContext polarisCallContext) { - return CallContext.of(realmContext, polarisCallContext); + public CallContext callContext( + RealmContext realmContext, + PolarisCallContext polarisCallContext, + @Context ContainerRequestContext request) + throws URISyntaxException { + URI baseUri = + enableForwardedPrefix + ? new URI(request.getHeaderString("X-Forwarded-Prefix")) + : request.getUriInfo().getBaseUri(); + return CallContext.of(realmContext, polarisCallContext, baseUri); } public void closeCallContext(@Disposes CallContext callContext) { diff --git a/service/common/src/main/java/org/apache/polaris/service/catalog/PolarisCatalogHandlerWrapper.java b/service/common/src/main/java/org/apache/polaris/service/catalog/PolarisCatalogHandlerWrapper.java index 72bd1863b..04a0347d5 100644 --- a/service/common/src/main/java/org/apache/polaris/service/catalog/PolarisCatalogHandlerWrapper.java +++ b/service/common/src/main/java/org/apache/polaris/service/catalog/PolarisCatalogHandlerWrapper.java @@ -23,8 +23,6 @@ import jakarta.ws.rs.core.SecurityContext; import java.io.Closeable; import java.io.IOException; -import java.net.URI; -import java.net.URISyntaxException; import java.time.OffsetDateTime; import java.time.ZoneOffset; import java.util.ArrayList; @@ -884,26 +882,18 @@ public LoadTableResponse loadTableWithAccessDelegation( tableIdentifier, tableMetadata, actionsRequested)); } if (accessDelegationModes.contains(AccessDelegationMode.VENDED_CREDENTIALS)) { - try { - // String hostName = InetAddress.getLocalHost().getCanonicalHostName(); - String hostName = "localhost"; - responseBuilder.addConfig(AwsClientProperties.REFRESH_CREDENTIALS_ENABLED, "true"); - responseBuilder.addConfig( - AwsClientProperties.REFRESH_CREDENTIALS_ENDPOINT, - new URI( - "https", - hostName, - String.format( - "/v1/%s/namespaces/%s/tables/%s/credentials", - catalogName, - tableIdentifier.namespace().toString(), - tableIdentifier.name()), - null) - .toString()); - } catch (URISyntaxException e) { - LOGGER.error( - "Error while fetching hostname. Ignoring refresh credentials properties", e); - } + responseBuilder.addConfig(AwsClientProperties.REFRESH_CREDENTIALS_ENABLED, "true"); + responseBuilder.addConfig( + AwsClientProperties.REFRESH_CREDENTIALS_ENDPOINT, + callContext + .getBaseUri() + .resolve( + String.format( + "/v1/%s/namespaces/%s/tables/%s/credentials", + catalogName, + tableIdentifier.namespace().toString(), + tableIdentifier.name())) + .toString()); return responseBuilder.build(); } } else if (table instanceof BaseMetadataTable) { From 9c1c283fa05217e4578bdb1bd76efc6f07c1f993 Mon Sep 17 00:00:00 2001 From: David Lu Date: Mon, 17 Mar 2025 14:34:44 -0400 Subject: [PATCH 05/20] fix tests --- .../polaris/service/catalog/io/FileIOFactoryTest.java | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/service/common/src/test/java/org/apache/polaris/service/catalog/io/FileIOFactoryTest.java b/service/common/src/test/java/org/apache/polaris/service/catalog/io/FileIOFactoryTest.java index 760185a1d..a59cc3de6 100644 --- a/service/common/src/test/java/org/apache/polaris/service/catalog/io/FileIOFactoryTest.java +++ b/service/common/src/test/java/org/apache/polaris/service/catalog/io/FileIOFactoryTest.java @@ -25,6 +25,7 @@ import com.google.common.collect.ImmutableMap; import jakarta.annotation.Nonnull; import java.lang.reflect.Method; +import java.net.URI; import java.time.Clock; import java.util.HashMap; import java.util.List; @@ -149,6 +150,11 @@ public PolarisCallContext getPolarisCallContext() { public Map contextVariables() { return new HashMap<>(); } + + @Override + public URI getBaseUri() { + return null; + } }; } From 0241b808108ea25d7ef509ebd406e58fb32791d5 Mon Sep 17 00:00:00 2001 From: David Lu Date: Wed, 19 Mar 2025 12:18:21 -0400 Subject: [PATCH 06/20] add uri builder --- .../quarkus/config/QuarkusProducers.java | 20 +++++++++++++------ .../auth/JWTSymmetricKeyGeneratorTest.java | 6 ++++++ .../catalog/PolarisCatalogHandlerWrapper.java | 16 +++++++-------- .../apache/polaris/service/TestServices.java | 6 ++++++ 4 files changed, 34 insertions(+), 14 deletions(-) diff --git a/quarkus/service/src/main/java/org/apache/polaris/service/quarkus/config/QuarkusProducers.java b/quarkus/service/src/main/java/org/apache/polaris/service/quarkus/config/QuarkusProducers.java index 3122f8ea8..97f393d1d 100644 --- a/quarkus/service/src/main/java/org/apache/polaris/service/quarkus/config/QuarkusProducers.java +++ b/quarkus/service/src/main/java/org/apache/polaris/service/quarkus/config/QuarkusProducers.java @@ -31,7 +31,7 @@ import jakarta.inject.Singleton; import jakarta.ws.rs.container.ContainerRequestContext; import jakarta.ws.rs.core.Context; -import java.net.URI; +import jakarta.ws.rs.core.UriBuilder; import java.net.URISyntaxException; import java.time.Clock; import org.apache.polaris.core.PolarisCallContext; @@ -130,11 +130,19 @@ public CallContext callContext( PolarisCallContext polarisCallContext, @Context ContainerRequestContext request) throws URISyntaxException { - URI baseUri = - enableForwardedPrefix - ? new URI(request.getHeaderString("X-Forwarded-Prefix")) - : request.getUriInfo().getBaseUri(); - return CallContext.of(realmContext, polarisCallContext, baseUri); + String forwardedProto = request.getHeaderString("X-Forwarded-Proto"); + UriBuilder baseUriBuilder = request.getUriInfo().getBaseUriBuilder(); + if (forwardedProto != null) { + baseUriBuilder.scheme(forwardedProto); // default value for this is http + } + if (enableForwardedPrefix) { + String forwardedPrefix = request.getHeaderString("X-Forwarded-Prefix"); + if (!forwardedPrefix.endsWith("/")) { + forwardedPrefix += "/"; + } + baseUriBuilder.path(forwardedPrefix); + } + return CallContext.of(realmContext, polarisCallContext, baseUriBuilder.build()); } public void closeCallContext(@Disposes CallContext callContext) { diff --git a/quarkus/service/src/test/java/org/apache/polaris/service/quarkus/auth/JWTSymmetricKeyGeneratorTest.java b/quarkus/service/src/test/java/org/apache/polaris/service/quarkus/auth/JWTSymmetricKeyGeneratorTest.java index fed5d20db..7f459a71d 100644 --- a/quarkus/service/src/test/java/org/apache/polaris/service/quarkus/auth/JWTSymmetricKeyGeneratorTest.java +++ b/quarkus/service/src/test/java/org/apache/polaris/service/quarkus/auth/JWTSymmetricKeyGeneratorTest.java @@ -24,6 +24,7 @@ import com.auth0.jwt.JWTVerifier; import com.auth0.jwt.algorithms.Algorithm; import com.auth0.jwt.interfaces.DecodedJWT; +import java.net.URI; import java.util.Map; import org.apache.polaris.core.PolarisCallContext; import org.apache.polaris.core.context.CallContext; @@ -65,6 +66,11 @@ public PolarisCallContext getPolarisCallContext() { public Map contextVariables() { return Map.of(); } + + @Override + public URI getBaseUri() { + return null; + } }); PolarisMetaStoreManager metastoreManager = Mockito.mock(PolarisMetaStoreManager.class); String mainSecret = "test_secret"; diff --git a/service/common/src/main/java/org/apache/polaris/service/catalog/PolarisCatalogHandlerWrapper.java b/service/common/src/main/java/org/apache/polaris/service/catalog/PolarisCatalogHandlerWrapper.java index 04a0347d5..c828ad8c2 100644 --- a/service/common/src/main/java/org/apache/polaris/service/catalog/PolarisCatalogHandlerWrapper.java +++ b/service/common/src/main/java/org/apache/polaris/service/catalog/PolarisCatalogHandlerWrapper.java @@ -21,6 +21,7 @@ import com.google.common.base.Preconditions; import com.google.common.collect.Maps; import jakarta.ws.rs.core.SecurityContext; +import jakarta.ws.rs.core.UriBuilder; import java.io.Closeable; import java.io.IOException; import java.time.OffsetDateTime; @@ -882,18 +883,17 @@ public LoadTableResponse loadTableWithAccessDelegation( tableIdentifier, tableMetadata, actionsRequested)); } if (accessDelegationModes.contains(AccessDelegationMode.VENDED_CREDENTIALS)) { - responseBuilder.addConfig(AwsClientProperties.REFRESH_CREDENTIALS_ENABLED, "true"); - responseBuilder.addConfig( - AwsClientProperties.REFRESH_CREDENTIALS_ENDPOINT, - callContext - .getBaseUri() - .resolve( + UriBuilder uriBuilder = + UriBuilder.fromUri(callContext.getBaseUri()) + .path( String.format( "/v1/%s/namespaces/%s/tables/%s/credentials", catalogName, tableIdentifier.namespace().toString(), - tableIdentifier.name())) - .toString()); + tableIdentifier.name())); + responseBuilder.addConfig(AwsClientProperties.REFRESH_CREDENTIALS_ENABLED, "true"); + responseBuilder.addConfig( + AwsClientProperties.REFRESH_CREDENTIALS_ENDPOINT, uriBuilder.build().toString()); return responseBuilder.build(); } } else if (table instanceof BaseMetadataTable) { diff --git a/service/common/src/testFixtures/java/org/apache/polaris/service/TestServices.java b/service/common/src/testFixtures/java/org/apache/polaris/service/TestServices.java index 0f63106ac..082a59fa1 100644 --- a/service/common/src/testFixtures/java/org/apache/polaris/service/TestServices.java +++ b/service/common/src/testFixtures/java/org/apache/polaris/service/TestServices.java @@ -21,6 +21,7 @@ import com.google.auth.oauth2.AccessToken; import com.google.auth.oauth2.GoogleCredentials; import jakarta.ws.rs.core.SecurityContext; +import java.net.URI; import java.security.Principal; import java.time.Clock; import java.time.Instant; @@ -158,6 +159,11 @@ public PolarisCallContext getPolarisCallContext() { public Map contextVariables() { return new HashMap<>(); } + + @Override + public URI getBaseUri() { + return null; + } }; FileIOFactory fileIOFactory = From c1f2cbe3a03797c15e0c31ed900cd607a49d2068 Mon Sep 17 00:00:00 2001 From: David Lu Date: Wed, 19 Mar 2025 12:40:15 -0400 Subject: [PATCH 07/20] clean up code --- .../service/quarkus/config/QuarkusProducers.java | 11 +++-------- 1 file changed, 3 insertions(+), 8 deletions(-) diff --git a/quarkus/service/src/main/java/org/apache/polaris/service/quarkus/config/QuarkusProducers.java b/quarkus/service/src/main/java/org/apache/polaris/service/quarkus/config/QuarkusProducers.java index 97f393d1d..86bde823d 100644 --- a/quarkus/service/src/main/java/org/apache/polaris/service/quarkus/config/QuarkusProducers.java +++ b/quarkus/service/src/main/java/org/apache/polaris/service/quarkus/config/QuarkusProducers.java @@ -32,7 +32,6 @@ import jakarta.ws.rs.container.ContainerRequestContext; import jakarta.ws.rs.core.Context; import jakarta.ws.rs.core.UriBuilder; -import java.net.URISyntaxException; import java.time.Clock; import org.apache.polaris.core.PolarisCallContext; import org.apache.polaris.core.PolarisDefaultDiagServiceImpl; @@ -73,9 +72,6 @@ public class QuarkusProducers { - @ConfigProperty(name = "quarkus.http.proxy.enable-forwarded-prefix") - boolean enableForwardedPrefix; - @Produces @ApplicationScoped // cannot be singleton because it is mocked in tests public Clock clock() { @@ -128,15 +124,14 @@ public PolarisCallContext polarisCallContext( public CallContext callContext( RealmContext realmContext, PolarisCallContext polarisCallContext, - @Context ContainerRequestContext request) - throws URISyntaxException { + @Context ContainerRequestContext request) { String forwardedProto = request.getHeaderString("X-Forwarded-Proto"); + String forwardedPrefix = request.getHeaderString("X-Forwarded-Prefix"); UriBuilder baseUriBuilder = request.getUriInfo().getBaseUriBuilder(); if (forwardedProto != null) { baseUriBuilder.scheme(forwardedProto); // default value for this is http } - if (enableForwardedPrefix) { - String forwardedPrefix = request.getHeaderString("X-Forwarded-Prefix"); + if (forwardedPrefix!=null) { if (!forwardedPrefix.endsWith("/")) { forwardedPrefix += "/"; } From b0057d445b033dc0d7d68f5fc7e2e3adc5c46856 Mon Sep 17 00:00:00 2001 From: David Lu Date: Wed, 19 Mar 2025 14:47:27 -0400 Subject: [PATCH 08/20] fix wrong url --- .../polaris/service/catalog/PolarisCatalogHandlerWrapper.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/service/common/src/main/java/org/apache/polaris/service/catalog/PolarisCatalogHandlerWrapper.java b/service/common/src/main/java/org/apache/polaris/service/catalog/PolarisCatalogHandlerWrapper.java index c828ad8c2..534e08190 100644 --- a/service/common/src/main/java/org/apache/polaris/service/catalog/PolarisCatalogHandlerWrapper.java +++ b/service/common/src/main/java/org/apache/polaris/service/catalog/PolarisCatalogHandlerWrapper.java @@ -887,7 +887,7 @@ public LoadTableResponse loadTableWithAccessDelegation( UriBuilder.fromUri(callContext.getBaseUri()) .path( String.format( - "/v1/%s/namespaces/%s/tables/%s/credentials", + "/api/catalog/v1/%s/namespaces/%s/tables/%s/credentials", catalogName, tableIdentifier.namespace().toString(), tableIdentifier.name())); From d8e55c02b0df6507c1270b79b65778c47a2aeff1 Mon Sep 17 00:00:00 2001 From: David Lu Date: Wed, 19 Mar 2025 15:10:39 -0400 Subject: [PATCH 09/20] remove delegation modes --- .../catalog/IcebergCatalogHandlerWrapperAuthzTest.java | 8 ++++---- .../service/catalog/iceberg/IcebergCatalogAdapter.java | 4 ++-- .../catalog/iceberg/IcebergCatalogHandlerWrapper.java | 6 ++---- 3 files changed, 8 insertions(+), 10 deletions(-) diff --git a/quarkus/service/src/test/java/org/apache/polaris/service/quarkus/catalog/IcebergCatalogHandlerWrapperAuthzTest.java b/quarkus/service/src/test/java/org/apache/polaris/service/quarkus/catalog/IcebergCatalogHandlerWrapperAuthzTest.java index 475de50e8..205c8a3bb 100644 --- a/quarkus/service/src/test/java/org/apache/polaris/service/quarkus/catalog/IcebergCatalogHandlerWrapperAuthzTest.java +++ b/quarkus/service/src/test/java/org/apache/polaris/service/quarkus/catalog/IcebergCatalogHandlerWrapperAuthzTest.java @@ -877,7 +877,7 @@ public void testLoadTableWithReadAccessDelegationSufficientPrivileges() { () -> newWrapper() .loadTableWithAccessDelegation( - TABLE_NS1A_2, "all", EnumSet.noneOf(AccessDelegationMode.class)), + TABLE_NS1A_2, "all"), null /* cleanupAction */); } @@ -896,7 +896,7 @@ public void testLoadTableWithReadAccessDelegationInsufficientPermissions() { () -> newWrapper() .loadTableWithAccessDelegation( - TABLE_NS1A_2, "all", EnumSet.noneOf(AccessDelegationMode.class))); + TABLE_NS1A_2, "all")); } @Test @@ -912,7 +912,7 @@ public void testLoadTableWithWriteAccessDelegationSufficientPrivileges() { () -> newWrapper() .loadTableWithAccessDelegation( - TABLE_NS1A_2, "all", EnumSet.noneOf(AccessDelegationMode.class)), + TABLE_NS1A_2, "all"), null /* cleanupAction */); } @@ -931,7 +931,7 @@ public void testLoadTableWithWriteAccessDelegationInsufficientPermissions() { () -> newWrapper() .loadTableWithAccessDelegation( - TABLE_NS1A_2, "all", EnumSet.noneOf(AccessDelegationMode.class))); + TABLE_NS1A_2, "all")); } @Test diff --git a/service/common/src/main/java/org/apache/polaris/service/catalog/iceberg/IcebergCatalogAdapter.java b/service/common/src/main/java/org/apache/polaris/service/catalog/iceberg/IcebergCatalogAdapter.java index c31ecc0d9..a823bf575 100644 --- a/service/common/src/main/java/org/apache/polaris/service/catalog/iceberg/IcebergCatalogAdapter.java +++ b/service/common/src/main/java/org/apache/polaris/service/catalog/iceberg/IcebergCatalogAdapter.java @@ -344,7 +344,7 @@ public Response loadTable( } else { return Response.ok( newHandlerWrapper(realmContext, securityContext, prefix) - .loadTableWithAccessDelegation(tableIdentifier, snapshots, delegationModes)) + .loadTableWithAccessDelegation(tableIdentifier, snapshots)) .build(); } } @@ -469,7 +469,7 @@ public Response loadCredentials( LoadTableResponse loadTableResponse = newHandlerWrapper(realmContext, securityContext, prefix) .loadTableWithAccessDelegation( - tableIdentifier, "all", EnumSet.noneOf(AccessDelegationMode.class)); + tableIdentifier, "all"); return Response.ok( ImmutableLoadCredentialsResponse.builder() .credentials(loadTableResponse.credentials()) diff --git a/service/common/src/main/java/org/apache/polaris/service/catalog/iceberg/IcebergCatalogHandlerWrapper.java b/service/common/src/main/java/org/apache/polaris/service/catalog/iceberg/IcebergCatalogHandlerWrapper.java index f95bdbeab..55494e420 100644 --- a/service/common/src/main/java/org/apache/polaris/service/catalog/iceberg/IcebergCatalogHandlerWrapper.java +++ b/service/common/src/main/java/org/apache/polaris/service/catalog/iceberg/IcebergCatalogHandlerWrapper.java @@ -95,6 +95,7 @@ import org.apache.polaris.core.persistence.resolver.ResolverPath; import org.apache.polaris.core.persistence.resolver.ResolverStatus; 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.context.CallContextCatalogFactory; import org.apache.polaris.service.types.NotificationRequest; @@ -824,8 +825,7 @@ public LoadTableResponse loadTable(TableIdentifier tableIdentifier, String snaps public LoadTableResponse loadTableWithAccessDelegation( TableIdentifier tableIdentifier, - String snapshots, - EnumSet accessDelegationModes) { + String snapshots) { // Here we have a single method that falls through multiple candidate // PolarisAuthorizableOperations because instead of identifying the desired operation up-front // and @@ -888,7 +888,6 @@ public LoadTableResponse loadTableWithAccessDelegation( credentialDelegation.getCredentialConfig( tableIdentifier, tableMetadata, actionsRequested)); } - if (accessDelegationModes.contains(AccessDelegationMode.VENDED_CREDENTIALS)) { UriBuilder uriBuilder = UriBuilder.fromUri(callContext.getBaseUri()) .path( @@ -901,7 +900,6 @@ public LoadTableResponse loadTableWithAccessDelegation( responseBuilder.addConfig( AwsClientProperties.REFRESH_CREDENTIALS_ENDPOINT, uriBuilder.build().toString()); return responseBuilder.build(); - } } 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()); From 13d33a5dc8a1b75d25105714166afc9b994a675e Mon Sep 17 00:00:00 2001 From: David Lu Date: Wed, 19 Mar 2025 16:15:01 -0400 Subject: [PATCH 10/20] cleanup code --- ...IcebergCatalogHandlerWrapperAuthzTest.java | 8 ++++---- .../catalog/iceberg/IcebergCatalog.java | 19 ++++++++++++++++++ .../iceberg/IcebergCatalogAdapter.java | 4 ++-- .../iceberg/IcebergCatalogHandlerWrapper.java | 20 +++++++------------ .../iceberg/SupportsCredentialDelegation.java | 4 ++++ 5 files changed, 36 insertions(+), 19 deletions(-) diff --git a/quarkus/service/src/test/java/org/apache/polaris/service/quarkus/catalog/IcebergCatalogHandlerWrapperAuthzTest.java b/quarkus/service/src/test/java/org/apache/polaris/service/quarkus/catalog/IcebergCatalogHandlerWrapperAuthzTest.java index 205c8a3bb..475de50e8 100644 --- a/quarkus/service/src/test/java/org/apache/polaris/service/quarkus/catalog/IcebergCatalogHandlerWrapperAuthzTest.java +++ b/quarkus/service/src/test/java/org/apache/polaris/service/quarkus/catalog/IcebergCatalogHandlerWrapperAuthzTest.java @@ -877,7 +877,7 @@ public void testLoadTableWithReadAccessDelegationSufficientPrivileges() { () -> newWrapper() .loadTableWithAccessDelegation( - TABLE_NS1A_2, "all"), + TABLE_NS1A_2, "all", EnumSet.noneOf(AccessDelegationMode.class)), null /* cleanupAction */); } @@ -896,7 +896,7 @@ public void testLoadTableWithReadAccessDelegationInsufficientPermissions() { () -> newWrapper() .loadTableWithAccessDelegation( - TABLE_NS1A_2, "all")); + TABLE_NS1A_2, "all", EnumSet.noneOf(AccessDelegationMode.class))); } @Test @@ -912,7 +912,7 @@ public void testLoadTableWithWriteAccessDelegationSufficientPrivileges() { () -> newWrapper() .loadTableWithAccessDelegation( - TABLE_NS1A_2, "all"), + TABLE_NS1A_2, "all", EnumSet.noneOf(AccessDelegationMode.class)), null /* cleanupAction */); } @@ -931,7 +931,7 @@ public void testLoadTableWithWriteAccessDelegationInsufficientPermissions() { () -> newWrapper() .loadTableWithAccessDelegation( - TABLE_NS1A_2, "all")); + TABLE_NS1A_2, "all", EnumSet.noneOf(AccessDelegationMode.class))); } @Test diff --git a/service/common/src/main/java/org/apache/polaris/service/catalog/iceberg/IcebergCatalog.java b/service/common/src/main/java/org/apache/polaris/service/catalog/iceberg/IcebergCatalog.java index 00469cc49..593f52d8a 100644 --- a/service/common/src/main/java/org/apache/polaris/service/catalog/iceberg/IcebergCatalog.java +++ b/service/common/src/main/java/org/apache/polaris/service/catalog/iceberg/IcebergCatalog.java @@ -28,6 +28,8 @@ import jakarta.annotation.Nonnull; import jakarta.annotation.Nullable; import jakarta.ws.rs.core.SecurityContext; +import jakarta.ws.rs.core.UriBuilder; + import java.io.Closeable; import java.io.IOException; import java.util.Arrays; @@ -51,6 +53,7 @@ import org.apache.iceberg.TableMetadata; import org.apache.iceberg.TableMetadataParser; import org.apache.iceberg.TableOperations; +import org.apache.iceberg.aws.AwsClientProperties; import org.apache.iceberg.catalog.Namespace; import org.apache.iceberg.catalog.SupportsNamespaces; import org.apache.iceberg.catalog.TableIdentifier; @@ -852,6 +855,22 @@ public Map getCredentialConfig( storageInfo.get()); } + @Override + public Map getVendedCredentialConfig(TableIdentifier tableIdentifier){ + Map vendedCredentialConfig = new HashMap<>(); + UriBuilder uriBuilder = + UriBuilder.fromUri(callContext.getBaseUri()) + .path( + String.format( + "/api/catalog/v1/%s/namespaces/%s/tables/%s/credentials", + catalogName, + tableIdentifier.namespace().toString(), + tableIdentifier.name())); + vendedCredentialConfig.put(AwsClientProperties.REFRESH_CREDENTIALS_ENABLED, "true"); + vendedCredentialConfig.put(AwsClientProperties.REFRESH_CREDENTIALS_ENDPOINT, uriBuilder.build().toString()); + return vendedCredentialConfig; + } + /** * Based on configuration settings, for callsites that need to handle potentially setting a new * base location for a TableLike entity, produces the transformed location if applicable, or else diff --git a/service/common/src/main/java/org/apache/polaris/service/catalog/iceberg/IcebergCatalogAdapter.java b/service/common/src/main/java/org/apache/polaris/service/catalog/iceberg/IcebergCatalogAdapter.java index a823bf575..c31ecc0d9 100644 --- a/service/common/src/main/java/org/apache/polaris/service/catalog/iceberg/IcebergCatalogAdapter.java +++ b/service/common/src/main/java/org/apache/polaris/service/catalog/iceberg/IcebergCatalogAdapter.java @@ -344,7 +344,7 @@ public Response loadTable( } else { return Response.ok( newHandlerWrapper(realmContext, securityContext, prefix) - .loadTableWithAccessDelegation(tableIdentifier, snapshots)) + .loadTableWithAccessDelegation(tableIdentifier, snapshots, delegationModes)) .build(); } } @@ -469,7 +469,7 @@ public Response loadCredentials( LoadTableResponse loadTableResponse = newHandlerWrapper(realmContext, securityContext, prefix) .loadTableWithAccessDelegation( - tableIdentifier, "all"); + tableIdentifier, "all", EnumSet.noneOf(AccessDelegationMode.class)); return Response.ok( ImmutableLoadCredentialsResponse.builder() .credentials(loadTableResponse.credentials()) diff --git a/service/common/src/main/java/org/apache/polaris/service/catalog/iceberg/IcebergCatalogHandlerWrapper.java b/service/common/src/main/java/org/apache/polaris/service/catalog/iceberg/IcebergCatalogHandlerWrapper.java index 55494e420..42aaf43b4 100644 --- a/service/common/src/main/java/org/apache/polaris/service/catalog/iceberg/IcebergCatalogHandlerWrapper.java +++ b/service/common/src/main/java/org/apache/polaris/service/catalog/iceberg/IcebergCatalogHandlerWrapper.java @@ -825,7 +825,8 @@ public LoadTableResponse loadTable(TableIdentifier tableIdentifier, String snaps public LoadTableResponse loadTableWithAccessDelegation( TableIdentifier tableIdentifier, - String snapshots) { + String snapshots, + EnumSet accessDelegationModes) { // Here we have a single method that falls through multiple candidate // PolarisAuthorizableOperations because instead of identifying the desired operation up-front // and @@ -887,18 +888,11 @@ public LoadTableResponse loadTableWithAccessDelegation( responseBuilder.addAllConfig( credentialDelegation.getCredentialConfig( tableIdentifier, tableMetadata, actionsRequested)); - } - UriBuilder uriBuilder = - UriBuilder.fromUri(callContext.getBaseUri()) - .path( - String.format( - "/api/catalog/v1/%s/namespaces/%s/tables/%s/credentials", - catalogName, - tableIdentifier.namespace().toString(), - tableIdentifier.name())); - responseBuilder.addConfig(AwsClientProperties.REFRESH_CREDENTIALS_ENABLED, "true"); - responseBuilder.addConfig( - AwsClientProperties.REFRESH_CREDENTIALS_ENDPOINT, uriBuilder.build().toString()); + if (accessDelegationModes.contains(AccessDelegationMode.VENDED_CREDENTIALS)){ + responseBuilder.addAllConfig( + credentialDelegation.getVendedCredentialConfig( + tableIdentifier)); + }} return responseBuilder.build(); } else if (table instanceof BaseMetadataTable) { // metadata tables are loaded on the client side, return NoSuchTableException for now diff --git a/service/common/src/main/java/org/apache/polaris/service/catalog/iceberg/SupportsCredentialDelegation.java b/service/common/src/main/java/org/apache/polaris/service/catalog/iceberg/SupportsCredentialDelegation.java index 06ca7fbde..edb9e1884 100644 --- a/service/common/src/main/java/org/apache/polaris/service/catalog/iceberg/SupportsCredentialDelegation.java +++ b/service/common/src/main/java/org/apache/polaris/service/catalog/iceberg/SupportsCredentialDelegation.java @@ -36,4 +36,8 @@ Map getCredentialConfig( TableIdentifier tableIdentifier, TableMetadata tableMetadata, Set storageActions); + + Map getVendedCredentialConfig( + TableIdentifier tableIdentifier + ); } From 7961dc2f8964b5de6ed0c2786d7f3302eb8662a9 Mon Sep 17 00:00:00 2001 From: David Lu Date: Fri, 21 Mar 2025 12:54:21 -0400 Subject: [PATCH 11/20] change based on iceberg latest updates --- .../polaris/core/context/CallContext.java | 40 ------------------- .../quarkus/config/QuarkusProducers.java | 17 +------- .../catalog/iceberg/IcebergCatalog.java | 9 ++--- 3 files changed, 5 insertions(+), 61 deletions(-) diff --git a/polaris-core/src/main/java/org/apache/polaris/core/context/CallContext.java b/polaris-core/src/main/java/org/apache/polaris/core/context/CallContext.java index 0999880b6..93f71e1c6 100644 --- a/polaris-core/src/main/java/org/apache/polaris/core/context/CallContext.java +++ b/polaris-core/src/main/java/org/apache/polaris/core/context/CallContext.java @@ -73,34 +73,6 @@ static void unsetCurrentContext() { CURRENT_CONTEXT.remove(); } - static CallContext of( - final RealmContext realmContext, - final PolarisCallContext polarisCallContext, - final URI baseUri) { - Map map = new HashMap<>(); - return new CallContext() { - @Override - public RealmContext getRealmContext() { - return realmContext; - } - - @Override - public PolarisCallContext getPolarisCallContext() { - return polarisCallContext; - } - - @Override - public Map contextVariables() { - return map; - } - - @Override - public URI getBaseUri() { - return baseUri; - } - }; - } - static CallContext of( final RealmContext realmContext, final PolarisCallContext polarisCallContext) { Map map = new HashMap<>(); @@ -119,11 +91,6 @@ public PolarisCallContext getPolarisCallContext() { public Map contextVariables() { return map; } - - @Override - public URI getBaseUri() { - return null; - } }; } @@ -156,11 +123,6 @@ public PolarisCallContext getPolarisCallContext() { public Map contextVariables() { return contextVariables; } - - @Override - public URI getBaseUri() { - return base.getBaseUri(); - } }; } @@ -173,8 +135,6 @@ public URI getBaseUri() { Map contextVariables(); - URI getBaseUri(); - default @Nonnull CloseableGroup closeables() { return (CloseableGroup) contextVariables().computeIfAbsent(CLOSEABLES, key -> new CloseableGroup()); diff --git a/quarkus/service/src/main/java/org/apache/polaris/service/quarkus/config/QuarkusProducers.java b/quarkus/service/src/main/java/org/apache/polaris/service/quarkus/config/QuarkusProducers.java index 8476d59ee..3b9de1efe 100644 --- a/quarkus/service/src/main/java/org/apache/polaris/service/quarkus/config/QuarkusProducers.java +++ b/quarkus/service/src/main/java/org/apache/polaris/service/quarkus/config/QuarkusProducers.java @@ -123,21 +123,8 @@ public PolarisCallContext polarisCallContext( @RequestScoped public CallContext callContext( RealmContext realmContext, - PolarisCallContext polarisCallContext, - @Context ContainerRequestContext request) { - String forwardedProto = request.getHeaderString("X-Forwarded-Proto"); - String forwardedPrefix = request.getHeaderString("X-Forwarded-Prefix"); - UriBuilder baseUriBuilder = request.getUriInfo().getBaseUriBuilder(); - if (forwardedProto != null) { - baseUriBuilder.scheme(forwardedProto); // default value for this is http - } - if (forwardedPrefix!=null) { - if (!forwardedPrefix.endsWith("/")) { - forwardedPrefix += "/"; - } - baseUriBuilder.path(forwardedPrefix); - } - return CallContext.of(realmContext, polarisCallContext, baseUriBuilder.build()); + PolarisCallContext polarisCallContext) { + return CallContext.of(realmContext, polarisCallContext); } public void closeCallContext(@Disposes CallContext callContext) { diff --git a/service/common/src/main/java/org/apache/polaris/service/catalog/iceberg/IcebergCatalog.java b/service/common/src/main/java/org/apache/polaris/service/catalog/iceberg/IcebergCatalog.java index 593f52d8a..588edf376 100644 --- a/service/common/src/main/java/org/apache/polaris/service/catalog/iceberg/IcebergCatalog.java +++ b/service/common/src/main/java/org/apache/polaris/service/catalog/iceberg/IcebergCatalog.java @@ -858,16 +858,13 @@ public Map getCredentialConfig( @Override public Map getVendedCredentialConfig(TableIdentifier tableIdentifier){ Map vendedCredentialConfig = new HashMap<>(); - UriBuilder uriBuilder = - UriBuilder.fromUri(callContext.getBaseUri()) - .path( - String.format( + String credentialsEndpoint = String.format( "/api/catalog/v1/%s/namespaces/%s/tables/%s/credentials", catalogName, tableIdentifier.namespace().toString(), - tableIdentifier.name())); + tableIdentifier.name()); vendedCredentialConfig.put(AwsClientProperties.REFRESH_CREDENTIALS_ENABLED, "true"); - vendedCredentialConfig.put(AwsClientProperties.REFRESH_CREDENTIALS_ENDPOINT, uriBuilder.build().toString()); + vendedCredentialConfig.put(AwsClientProperties.REFRESH_CREDENTIALS_ENDPOINT, credentialsEndpoint); return vendedCredentialConfig; } From 348f1eec5f50d437a9ff6275f242f9f41f8db099 Mon Sep 17 00:00:00 2001 From: David Lu Date: Fri, 21 Mar 2025 13:04:50 -0400 Subject: [PATCH 12/20] spotlessApply --- .../polaris/core/context/CallContext.java | 1 - .../quarkus/config/QuarkusProducers.java | 5 +- .../catalog/iceberg/IcebergCatalog.java | 16 +++---- .../iceberg/IcebergCatalogAdapter.java | 7 ++- .../iceberg/IcebergCatalogHandlerWrapper.java | 47 +++++++++---------- .../iceberg/SupportsCredentialDelegation.java | 4 +- 6 files changed, 37 insertions(+), 43 deletions(-) diff --git a/polaris-core/src/main/java/org/apache/polaris/core/context/CallContext.java b/polaris-core/src/main/java/org/apache/polaris/core/context/CallContext.java index 93f71e1c6..71c457720 100644 --- a/polaris-core/src/main/java/org/apache/polaris/core/context/CallContext.java +++ b/polaris-core/src/main/java/org/apache/polaris/core/context/CallContext.java @@ -20,7 +20,6 @@ import jakarta.annotation.Nonnull; import java.io.IOException; -import java.net.URI; import java.util.HashMap; import java.util.Map; import java.util.stream.Collectors; diff --git a/quarkus/service/src/main/java/org/apache/polaris/service/quarkus/config/QuarkusProducers.java b/quarkus/service/src/main/java/org/apache/polaris/service/quarkus/config/QuarkusProducers.java index 3b9de1efe..16b743979 100644 --- a/quarkus/service/src/main/java/org/apache/polaris/service/quarkus/config/QuarkusProducers.java +++ b/quarkus/service/src/main/java/org/apache/polaris/service/quarkus/config/QuarkusProducers.java @@ -31,7 +31,6 @@ import jakarta.inject.Singleton; import jakarta.ws.rs.container.ContainerRequestContext; import jakarta.ws.rs.core.Context; -import jakarta.ws.rs.core.UriBuilder; import java.time.Clock; import org.apache.polaris.core.PolarisCallContext; import org.apache.polaris.core.PolarisDefaultDiagServiceImpl; @@ -121,9 +120,7 @@ public PolarisCallContext polarisCallContext( @Produces @RequestScoped - public CallContext callContext( - RealmContext realmContext, - PolarisCallContext polarisCallContext) { + public CallContext callContext(RealmContext realmContext, PolarisCallContext polarisCallContext) { return CallContext.of(realmContext, polarisCallContext); } diff --git a/service/common/src/main/java/org/apache/polaris/service/catalog/iceberg/IcebergCatalog.java b/service/common/src/main/java/org/apache/polaris/service/catalog/iceberg/IcebergCatalog.java index 588edf376..8f731ee40 100644 --- a/service/common/src/main/java/org/apache/polaris/service/catalog/iceberg/IcebergCatalog.java +++ b/service/common/src/main/java/org/apache/polaris/service/catalog/iceberg/IcebergCatalog.java @@ -28,8 +28,6 @@ import jakarta.annotation.Nonnull; import jakarta.annotation.Nullable; import jakarta.ws.rs.core.SecurityContext; -import jakarta.ws.rs.core.UriBuilder; - import java.io.Closeable; import java.io.IOException; import java.util.Arrays; @@ -856,15 +854,15 @@ public Map getCredentialConfig( } @Override - public Map getVendedCredentialConfig(TableIdentifier tableIdentifier){ + public Map getVendedCredentialConfig(TableIdentifier tableIdentifier) { Map vendedCredentialConfig = new HashMap<>(); - String credentialsEndpoint = String.format( - "/api/catalog/v1/%s/namespaces/%s/tables/%s/credentials", - catalogName, - tableIdentifier.namespace().toString(), - tableIdentifier.name()); + String credentialsEndpoint = + String.format( + "/api/catalog/v1/%s/namespaces/%s/tables/%s/credentials", + catalogName, tableIdentifier.namespace().toString(), tableIdentifier.name()); vendedCredentialConfig.put(AwsClientProperties.REFRESH_CREDENTIALS_ENABLED, "true"); - vendedCredentialConfig.put(AwsClientProperties.REFRESH_CREDENTIALS_ENDPOINT, credentialsEndpoint); + vendedCredentialConfig.put( + AwsClientProperties.REFRESH_CREDENTIALS_ENDPOINT, credentialsEndpoint); return vendedCredentialConfig; } diff --git a/service/common/src/main/java/org/apache/polaris/service/catalog/iceberg/IcebergCatalogAdapter.java b/service/common/src/main/java/org/apache/polaris/service/catalog/iceberg/IcebergCatalogAdapter.java index d7031e681..3cbabc9f2 100644 --- a/service/common/src/main/java/org/apache/polaris/service/catalog/iceberg/IcebergCatalogAdapter.java +++ b/service/common/src/main/java/org/apache/polaris/service/catalog/iceberg/IcebergCatalogAdapter.java @@ -344,7 +344,9 @@ public Response loadTable( if (delegationModes.isEmpty()) { return Response.ok(catalog.loadTable(tableIdentifier, snapshots)).build(); } else { - return Response.ok(catalog.loadTableWithAccessDelegation(tableIdentifier, snapshots, delegationModes)) + return Response.ok( + catalog.loadTableWithAccessDelegation( + tableIdentifier, snapshots, delegationModes)) .build(); } }); @@ -485,7 +487,8 @@ public Response loadCredentials( prefix, catalog -> { LoadTableResponse loadTableResponse = - catalog.loadTableWithAccessDelegation(tableIdentifier, "all", EnumSet.noneOf(AccessDelegationMode.class)); + catalog.loadTableWithAccessDelegation( + tableIdentifier, "all", EnumSet.noneOf(AccessDelegationMode.class)); return Response.ok( ImmutableLoadCredentialsResponse.builder() .credentials(loadTableResponse.credentials()) diff --git a/service/common/src/main/java/org/apache/polaris/service/catalog/iceberg/IcebergCatalogHandlerWrapper.java b/service/common/src/main/java/org/apache/polaris/service/catalog/iceberg/IcebergCatalogHandlerWrapper.java index db6d41ace..0f90e91e0 100644 --- a/service/common/src/main/java/org/apache/polaris/service/catalog/iceberg/IcebergCatalogHandlerWrapper.java +++ b/service/common/src/main/java/org/apache/polaris/service/catalog/iceberg/IcebergCatalogHandlerWrapper.java @@ -42,7 +42,6 @@ import org.apache.iceberg.TableMetadata; import org.apache.iceberg.TableOperations; import org.apache.iceberg.UpdateRequirement; -import org.apache.iceberg.aws.AwsClientProperties; import org.apache.iceberg.catalog.Catalog; import org.apache.iceberg.catalog.Namespace; import org.apache.iceberg.catalog.SupportsNamespaces; @@ -831,29 +830,29 @@ public LoadTableResponse loadTableWithAccessDelegation( // when data-access is specified but access delegation grants are not found. Table table = baseCatalog.loadTable(tableIdentifier); - if (table instanceof BaseTable baseTable) { - TableMetadata tableMetadata = baseTable.operations().current(); - LoadTableResponse.Builder responseBuilder = - LoadTableResponse.builder().withTableMetadata(tableMetadata); - if (baseCatalog instanceof SupportsCredentialDelegation credentialDelegation) { - LOGGER - .atDebug() - .addKeyValue("tableIdentifier", tableIdentifier) - .addKeyValue("tableLocation", tableMetadata.location()) - .log("Fetching client credentials for table"); - responseBuilder.addAllConfig( - credentialDelegation.getCredentialConfig( - tableIdentifier, tableMetadata, actionsRequested)); - if (accessDelegationModes.contains(AccessDelegationMode.VENDED_CREDENTIALS)){ - responseBuilder.addAllConfig( - credentialDelegation.getVendedCredentialConfig( - tableIdentifier)); - }} - return responseBuilder.build(); - } 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()); - } + if (table instanceof BaseTable baseTable) { + TableMetadata tableMetadata = baseTable.operations().current(); + LoadTableResponse.Builder responseBuilder = + LoadTableResponse.builder().withTableMetadata(tableMetadata); + if (baseCatalog instanceof SupportsCredentialDelegation credentialDelegation) { + LOGGER + .atDebug() + .addKeyValue("tableIdentifier", tableIdentifier) + .addKeyValue("tableLocation", tableMetadata.location()) + .log("Fetching client credentials for table"); + responseBuilder.addAllConfig( + credentialDelegation.getCredentialConfig( + tableIdentifier, tableMetadata, actionsRequested)); + if (accessDelegationModes.contains(AccessDelegationMode.VENDED_CREDENTIALS)) { + responseBuilder.addAllConfig( + credentialDelegation.getVendedCredentialConfig(tableIdentifier)); + } + } + return responseBuilder.build(); + } 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()); + } throw new IllegalStateException("Cannot wrap catalog that does not produce BaseTable"); } diff --git a/service/common/src/main/java/org/apache/polaris/service/catalog/iceberg/SupportsCredentialDelegation.java b/service/common/src/main/java/org/apache/polaris/service/catalog/iceberg/SupportsCredentialDelegation.java index edb9e1884..484bbdda7 100644 --- a/service/common/src/main/java/org/apache/polaris/service/catalog/iceberg/SupportsCredentialDelegation.java +++ b/service/common/src/main/java/org/apache/polaris/service/catalog/iceberg/SupportsCredentialDelegation.java @@ -37,7 +37,5 @@ Map getCredentialConfig( TableMetadata tableMetadata, Set storageActions); - Map getVendedCredentialConfig( - TableIdentifier tableIdentifier - ); + Map getVendedCredentialConfig(TableIdentifier tableIdentifier); } From a2a46403ca92e20bb645b917bbcec50aa5b69598 Mon Sep 17 00:00:00 2001 From: David Lu Date: Fri, 21 Mar 2025 13:15:37 -0400 Subject: [PATCH 13/20] fix path --- .../apache/polaris/service/catalog/iceberg/IcebergCatalog.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/service/common/src/main/java/org/apache/polaris/service/catalog/iceberg/IcebergCatalog.java b/service/common/src/main/java/org/apache/polaris/service/catalog/iceberg/IcebergCatalog.java index 8f731ee40..e4acd0909 100644 --- a/service/common/src/main/java/org/apache/polaris/service/catalog/iceberg/IcebergCatalog.java +++ b/service/common/src/main/java/org/apache/polaris/service/catalog/iceberg/IcebergCatalog.java @@ -858,7 +858,7 @@ public Map getVendedCredentialConfig(TableIdentifier tableIdenti Map vendedCredentialConfig = new HashMap<>(); String credentialsEndpoint = String.format( - "/api/catalog/v1/%s/namespaces/%s/tables/%s/credentials", + "/v1/%s/namespaces/%s/tables/%s/credentials", catalogName, tableIdentifier.namespace().toString(), tableIdentifier.name()); vendedCredentialConfig.put(AwsClientProperties.REFRESH_CREDENTIALS_ENABLED, "true"); vendedCredentialConfig.put( From 87d8358dde91988b40468d3ed5e1f4788e43ccc5 Mon Sep 17 00:00:00 2001 From: David Lu Date: Thu, 10 Apr 2025 16:37:31 -0400 Subject: [PATCH 14/20] remove getBaseUri --- .../service/quarkus/auth/JWTSymmetricKeyGeneratorTest.java | 5 ----- .../apache/polaris/service/catalog/io/FileIOFactoryTest.java | 5 ----- .../java/org/apache/polaris/service/TestServices.java | 5 ----- 3 files changed, 15 deletions(-) diff --git a/quarkus/service/src/test/java/org/apache/polaris/service/quarkus/auth/JWTSymmetricKeyGeneratorTest.java b/quarkus/service/src/test/java/org/apache/polaris/service/quarkus/auth/JWTSymmetricKeyGeneratorTest.java index 7f459a71d..17ae0a0d1 100644 --- a/quarkus/service/src/test/java/org/apache/polaris/service/quarkus/auth/JWTSymmetricKeyGeneratorTest.java +++ b/quarkus/service/src/test/java/org/apache/polaris/service/quarkus/auth/JWTSymmetricKeyGeneratorTest.java @@ -66,11 +66,6 @@ public PolarisCallContext getPolarisCallContext() { public Map contextVariables() { return Map.of(); } - - @Override - public URI getBaseUri() { - return null; - } }); PolarisMetaStoreManager metastoreManager = Mockito.mock(PolarisMetaStoreManager.class); String mainSecret = "test_secret"; diff --git a/service/common/src/test/java/org/apache/polaris/service/catalog/io/FileIOFactoryTest.java b/service/common/src/test/java/org/apache/polaris/service/catalog/io/FileIOFactoryTest.java index 2d42ce36f..15d8935b3 100644 --- a/service/common/src/test/java/org/apache/polaris/service/catalog/io/FileIOFactoryTest.java +++ b/service/common/src/test/java/org/apache/polaris/service/catalog/io/FileIOFactoryTest.java @@ -150,11 +150,6 @@ public PolarisCallContext getPolarisCallContext() { public Map contextVariables() { return new HashMap<>(); } - - @Override - public URI getBaseUri() { - return null; - } }; } diff --git a/service/common/src/testFixtures/java/org/apache/polaris/service/TestServices.java b/service/common/src/testFixtures/java/org/apache/polaris/service/TestServices.java index 11586ae37..0539a5d3d 100644 --- a/service/common/src/testFixtures/java/org/apache/polaris/service/TestServices.java +++ b/service/common/src/testFixtures/java/org/apache/polaris/service/TestServices.java @@ -159,11 +159,6 @@ public PolarisCallContext getPolarisCallContext() { public Map contextVariables() { return new HashMap<>(); } - - @Override - public URI getBaseUri() { - return null; - } }; FileIOFactory fileIOFactory = From 7002409fc8c8f8ab769eef82b1d13dfee75c026b Mon Sep 17 00:00:00 2001 From: David Lu Date: Thu, 10 Apr 2025 17:10:40 -0400 Subject: [PATCH 15/20] refactor based on merge --- .../quarkus/catalog/IcebergCatalogHandlerAuthzTest.java | 8 ++++---- .../service/catalog/iceberg/IcebergCatalogAdapter.java | 2 +- .../service/catalog/iceberg/IcebergCatalogHandler.java | 6 +++--- 3 files changed, 8 insertions(+), 8 deletions(-) diff --git a/quarkus/service/src/test/java/org/apache/polaris/service/quarkus/catalog/IcebergCatalogHandlerAuthzTest.java b/quarkus/service/src/test/java/org/apache/polaris/service/quarkus/catalog/IcebergCatalogHandlerAuthzTest.java index 12f0dd3bf..0ae7bba23 100644 --- a/quarkus/service/src/test/java/org/apache/polaris/service/quarkus/catalog/IcebergCatalogHandlerAuthzTest.java +++ b/quarkus/service/src/test/java/org/apache/polaris/service/quarkus/catalog/IcebergCatalogHandlerAuthzTest.java @@ -974,7 +974,7 @@ public void testLoadTableWithReadAccessDelegationIfStaleSufficientPrivileges() { () -> newWrapper() .loadTableWithAccessDelegationIfStale( - TABLE_NS1A_2, IfNoneMatch.fromHeader("W/\"0:0\""), "all"), + TABLE_NS1A_2, IfNoneMatch.fromHeader("W/\"0:0\""), "all", EnumSet.noneOf(AccessDelegationMode.class)), null /* cleanupAction */); } @@ -993,7 +993,7 @@ public void testLoadTableWithReadAccessDelegationIfStaleInsufficientPermissions( () -> newWrapper() .loadTableWithAccessDelegationIfStale( - TABLE_NS1A_2, IfNoneMatch.fromHeader("W/\"0:0\""), "all")); + TABLE_NS1A_2, IfNoneMatch.fromHeader("W/\"0:0\""), "all", EnumSet.noneOf(AccessDelegationMode.class))); } @Test @@ -1009,7 +1009,7 @@ public void testLoadTableWithWriteAccessDelegationIfStaleSufficientPrivileges() () -> newWrapper() .loadTableWithAccessDelegationIfStale( - TABLE_NS1A_2, IfNoneMatch.fromHeader("W/\"0:0\""), "all"), + TABLE_NS1A_2, IfNoneMatch.fromHeader("W/\"0:0\""), "all", EnumSet.noneOf(AccessDelegationMode.class)), null /* cleanupAction */); } @@ -1028,7 +1028,7 @@ public void testLoadTableWithWriteAccessDelegationIfStaleInsufficientPermissions () -> newWrapper() .loadTableWithAccessDelegationIfStale( - TABLE_NS1A_2, IfNoneMatch.fromHeader("W/\"0:0\""), "all")); + TABLE_NS1A_2, IfNoneMatch.fromHeader("W/\"0:0\""), "all", EnumSet.noneOf(AccessDelegationMode.class))); } @Test diff --git a/service/common/src/main/java/org/apache/polaris/service/catalog/iceberg/IcebergCatalogAdapter.java b/service/common/src/main/java/org/apache/polaris/service/catalog/iceberg/IcebergCatalogAdapter.java index 47f6fe5e2..c0b011211 100644 --- a/service/common/src/main/java/org/apache/polaris/service/catalog/iceberg/IcebergCatalogAdapter.java +++ b/service/common/src/main/java/org/apache/polaris/service/catalog/iceberg/IcebergCatalogAdapter.java @@ -400,7 +400,7 @@ public Response loadTable( } else { response = catalog - .loadTableWithAccessDelegationIfStale(tableIdentifier, ifNoneMatch, snapshots) + .loadTableWithAccessDelegationIfStale(tableIdentifier, ifNoneMatch, snapshots, delegationModes) .orElseThrow(() -> new WebApplicationException(Response.Status.NOT_MODIFIED)); } diff --git a/service/common/src/main/java/org/apache/polaris/service/catalog/iceberg/IcebergCatalogHandler.java b/service/common/src/main/java/org/apache/polaris/service/catalog/iceberg/IcebergCatalogHandler.java index f013eebf9..9739ec4ae 100644 --- a/service/common/src/main/java/org/apache/polaris/service/catalog/iceberg/IcebergCatalogHandler.java +++ b/service/common/src/main/java/org/apache/polaris/service/catalog/iceberg/IcebergCatalogHandler.java @@ -554,8 +554,8 @@ public Optional loadTableIfStale( } public LoadTableResponse loadTableWithAccessDelegation( - TableIdentifier tableIdentifier, String snapshots) { - return loadTableWithAccessDelegationIfStale(tableIdentifier, null, snapshots).get(); + TableIdentifier tableIdentifier, String snapshots, EnumSet accessDelegationModes) { + return loadTableWithAccessDelegationIfStale(tableIdentifier, null, snapshots, accessDelegationModes).get(); } /** @@ -569,7 +569,7 @@ public LoadTableResponse loadTableWithAccessDelegation( * load table response, otherwise */ public Optional loadTableWithAccessDelegationIfStale( - TableIdentifier tableIdentifier, IfNoneMatch ifNoneMatch, String snapshots) { + TableIdentifier tableIdentifier, IfNoneMatch ifNoneMatch, String snapshots, EnumSet accessDelegationModes) { // Here we have a single method that falls through multiple candidate // PolarisAuthorizableOperations because instead of identifying the desired operation up-front // and From aec31d7acf293bdb428dc43a7661618c2885a99b Mon Sep 17 00:00:00 2001 From: David Lu Date: Thu, 10 Apr 2025 17:31:38 -0400 Subject: [PATCH 16/20] move endpoint construction to IcebergCatalogAdapter --- .../polaris/service/catalog/iceberg/IcebergCatalog.java | 8 ++------ .../service/catalog/iceberg/IcebergCatalogAdapter.java | 8 +++++++- .../service/catalog/iceberg/IcebergCatalogHandler.java | 2 +- .../catalog/iceberg/SupportsCredentialDelegation.java | 2 +- 4 files changed, 11 insertions(+), 9 deletions(-) diff --git a/service/common/src/main/java/org/apache/polaris/service/catalog/iceberg/IcebergCatalog.java b/service/common/src/main/java/org/apache/polaris/service/catalog/iceberg/IcebergCatalog.java index 5c4fc16c6..90ed958d6 100644 --- a/service/common/src/main/java/org/apache/polaris/service/catalog/iceberg/IcebergCatalog.java +++ b/service/common/src/main/java/org/apache/polaris/service/catalog/iceberg/IcebergCatalog.java @@ -858,15 +858,11 @@ public Map getCredentialConfig( } @Override - public Map getVendedCredentialConfig(TableIdentifier tableIdentifier) { + public Map getVendedCredentialConfig(TableIdentifier tableIdentifier, String decodedCredentialsPath) { Map vendedCredentialConfig = new HashMap<>(); - String credentialsEndpoint = - String.format( - "/v1/%s/namespaces/%s/tables/%s/credentials", - catalogName, tableIdentifier.namespace().toString(), tableIdentifier.name()); vendedCredentialConfig.put(AwsClientProperties.REFRESH_CREDENTIALS_ENABLED, "true"); vendedCredentialConfig.put( - AwsClientProperties.REFRESH_CREDENTIALS_ENDPOINT, credentialsEndpoint); + AwsClientProperties.REFRESH_CREDENTIALS_ENDPOINT, decodedCredentialsPath); return vendedCredentialConfig; } diff --git a/service/common/src/main/java/org/apache/polaris/service/catalog/iceberg/IcebergCatalogAdapter.java b/service/common/src/main/java/org/apache/polaris/service/catalog/iceberg/IcebergCatalogAdapter.java index c0b011211..c39df9358 100644 --- a/service/common/src/main/java/org/apache/polaris/service/catalog/iceberg/IcebergCatalogAdapter.java +++ b/service/common/src/main/java/org/apache/polaris/service/catalog/iceberg/IcebergCatalogAdapter.java @@ -378,6 +378,7 @@ public Response loadTable( Namespace ns = decodeNamespace(namespace); TableIdentifier tableIdentifier = TableIdentifier.of(ns, RESTUtil.decodeString(table)); + // TODO: Populate with header value from parameter once the generated interface // contains the if-none-match header IfNoneMatch ifNoneMatch = IfNoneMatch.fromHeader(null); @@ -392,15 +393,20 @@ public Response loadTable( catalog -> { LoadTableResponse response; + if (delegationModes.isEmpty()) { response = catalog .loadTableIfStale(tableIdentifier, ifNoneMatch, snapshots) .orElseThrow(() -> new WebApplicationException(Response.Status.NOT_MODIFIED)); } else { + String credentialsEndpoint = + String.format( + "/v1/%s/namespaces/%s/tables/%s/credentials", + prefix, tableIdentifier.namespace().toString(), tableIdentifier.name()); response = catalog - .loadTableWithAccessDelegationIfStale(tableIdentifier, ifNoneMatch, snapshots, delegationModes) + .loadTableWithAccessDelegationIfStale(tableIdentifier, ifNoneMatch, snapshots, delegationModes, RESTUtil.decodeString(credentialsEndpoint)) .orElseThrow(() -> new WebApplicationException(Response.Status.NOT_MODIFIED)); } diff --git a/service/common/src/main/java/org/apache/polaris/service/catalog/iceberg/IcebergCatalogHandler.java b/service/common/src/main/java/org/apache/polaris/service/catalog/iceberg/IcebergCatalogHandler.java index 9739ec4ae..73a6dea3c 100644 --- a/service/common/src/main/java/org/apache/polaris/service/catalog/iceberg/IcebergCatalogHandler.java +++ b/service/common/src/main/java/org/apache/polaris/service/catalog/iceberg/IcebergCatalogHandler.java @@ -569,7 +569,7 @@ public LoadTableResponse loadTableWithAccessDelegation( * load table response, otherwise */ public Optional loadTableWithAccessDelegationIfStale( - TableIdentifier tableIdentifier, IfNoneMatch ifNoneMatch, String snapshots, EnumSet accessDelegationModes) { + TableIdentifier tableIdentifier, IfNoneMatch ifNoneMatch, String snapshots, EnumSet accessDelegationModes, String decodedCredentialPath) { // Here we have a single method that falls through multiple candidate // PolarisAuthorizableOperations because instead of identifying the desired operation up-front // and diff --git a/service/common/src/main/java/org/apache/polaris/service/catalog/iceberg/SupportsCredentialDelegation.java b/service/common/src/main/java/org/apache/polaris/service/catalog/iceberg/SupportsCredentialDelegation.java index 484bbdda7..bc1c5fd33 100644 --- a/service/common/src/main/java/org/apache/polaris/service/catalog/iceberg/SupportsCredentialDelegation.java +++ b/service/common/src/main/java/org/apache/polaris/service/catalog/iceberg/SupportsCredentialDelegation.java @@ -37,5 +37,5 @@ Map getCredentialConfig( TableMetadata tableMetadata, Set storageActions); - Map getVendedCredentialConfig(TableIdentifier tableIdentifier); + Map getVendedCredentialConfig(TableIdentifier tableIdentifier, String decodedCredentialsPath); } From bcea6c6be35c7e108a5c19a1974caa413df0b430 Mon Sep 17 00:00:00 2001 From: David Lu Date: Thu, 10 Apr 2025 17:37:20 -0400 Subject: [PATCH 17/20] pass down path --- .../catalog/IcebergCatalogHandlerAuthzTest.java | 16 ++++++++-------- .../catalog/iceberg/IcebergCatalogAdapter.java | 2 +- .../catalog/iceberg/IcebergCatalogHandler.java | 6 +++--- 3 files changed, 12 insertions(+), 12 deletions(-) diff --git a/quarkus/service/src/test/java/org/apache/polaris/service/quarkus/catalog/IcebergCatalogHandlerAuthzTest.java b/quarkus/service/src/test/java/org/apache/polaris/service/quarkus/catalog/IcebergCatalogHandlerAuthzTest.java index 0ae7bba23..211c21516 100644 --- a/quarkus/service/src/test/java/org/apache/polaris/service/quarkus/catalog/IcebergCatalogHandlerAuthzTest.java +++ b/quarkus/service/src/test/java/org/apache/polaris/service/quarkus/catalog/IcebergCatalogHandlerAuthzTest.java @@ -907,7 +907,7 @@ public void testLoadTableWithReadAccessDelegationSufficientPrivileges() { () -> newWrapper() .loadTableWithAccessDelegation( - TABLE_NS1A_2, "all", EnumSet.noneOf(AccessDelegationMode.class)), + TABLE_NS1A_2, "all", EnumSet.noneOf(AccessDelegationMode.class), null), null /* cleanupAction */); } @@ -926,7 +926,7 @@ public void testLoadTableWithReadAccessDelegationInsufficientPermissions() { () -> newWrapper() .loadTableWithAccessDelegation( - TABLE_NS1A_2, "all", EnumSet.noneOf(AccessDelegationMode.class))); + TABLE_NS1A_2, "all", EnumSet.noneOf(AccessDelegationMode.class), null)); } @Test @@ -942,7 +942,7 @@ public void testLoadTableWithWriteAccessDelegationSufficientPrivileges() { () -> newWrapper() .loadTableWithAccessDelegation( - TABLE_NS1A_2, "all", EnumSet.noneOf(AccessDelegationMode.class)), + TABLE_NS1A_2, "all", EnumSet.noneOf(AccessDelegationMode.class), null), null /* cleanupAction */); } @@ -961,7 +961,7 @@ public void testLoadTableWithWriteAccessDelegationInsufficientPermissions() { () -> newWrapper() .loadTableWithAccessDelegation( - TABLE_NS1A_2, "all", EnumSet.noneOf(AccessDelegationMode.class))); + TABLE_NS1A_2, "all", EnumSet.noneOf(AccessDelegationMode.class), null)); } @Test @@ -974,7 +974,7 @@ public void testLoadTableWithReadAccessDelegationIfStaleSufficientPrivileges() { () -> newWrapper() .loadTableWithAccessDelegationIfStale( - TABLE_NS1A_2, IfNoneMatch.fromHeader("W/\"0:0\""), "all", EnumSet.noneOf(AccessDelegationMode.class)), + TABLE_NS1A_2, IfNoneMatch.fromHeader("W/\"0:0\""), "all", EnumSet.noneOf(AccessDelegationMode.class), null), null /* cleanupAction */); } @@ -993,7 +993,7 @@ public void testLoadTableWithReadAccessDelegationIfStaleInsufficientPermissions( () -> newWrapper() .loadTableWithAccessDelegationIfStale( - TABLE_NS1A_2, IfNoneMatch.fromHeader("W/\"0:0\""), "all", EnumSet.noneOf(AccessDelegationMode.class))); + TABLE_NS1A_2, IfNoneMatch.fromHeader("W/\"0:0\""), "all", EnumSet.noneOf(AccessDelegationMode.class), null)); } @Test @@ -1009,7 +1009,7 @@ public void testLoadTableWithWriteAccessDelegationIfStaleSufficientPrivileges() () -> newWrapper() .loadTableWithAccessDelegationIfStale( - TABLE_NS1A_2, IfNoneMatch.fromHeader("W/\"0:0\""), "all", EnumSet.noneOf(AccessDelegationMode.class)), + TABLE_NS1A_2, IfNoneMatch.fromHeader("W/\"0:0\""), "all", EnumSet.noneOf(AccessDelegationMode.class), null), null /* cleanupAction */); } @@ -1028,7 +1028,7 @@ public void testLoadTableWithWriteAccessDelegationIfStaleInsufficientPermissions () -> newWrapper() .loadTableWithAccessDelegationIfStale( - TABLE_NS1A_2, IfNoneMatch.fromHeader("W/\"0:0\""), "all", EnumSet.noneOf(AccessDelegationMode.class))); + TABLE_NS1A_2, IfNoneMatch.fromHeader("W/\"0:0\""), "all", EnumSet.noneOf(AccessDelegationMode.class), null)); } @Test diff --git a/service/common/src/main/java/org/apache/polaris/service/catalog/iceberg/IcebergCatalogAdapter.java b/service/common/src/main/java/org/apache/polaris/service/catalog/iceberg/IcebergCatalogAdapter.java index c39df9358..f2e0ff820 100644 --- a/service/common/src/main/java/org/apache/polaris/service/catalog/iceberg/IcebergCatalogAdapter.java +++ b/service/common/src/main/java/org/apache/polaris/service/catalog/iceberg/IcebergCatalogAdapter.java @@ -555,7 +555,7 @@ public Response loadCredentials( catalog -> { LoadTableResponse loadTableResponse = catalog.loadTableWithAccessDelegation( - tableIdentifier, "all", EnumSet.noneOf(AccessDelegationMode.class)); + tableIdentifier, "all", EnumSet.noneOf(AccessDelegationMode.class), null); return Response.ok( ImmutableLoadCredentialsResponse.builder() .credentials(loadTableResponse.credentials()) diff --git a/service/common/src/main/java/org/apache/polaris/service/catalog/iceberg/IcebergCatalogHandler.java b/service/common/src/main/java/org/apache/polaris/service/catalog/iceberg/IcebergCatalogHandler.java index 73a6dea3c..994b4b91d 100644 --- a/service/common/src/main/java/org/apache/polaris/service/catalog/iceberg/IcebergCatalogHandler.java +++ b/service/common/src/main/java/org/apache/polaris/service/catalog/iceberg/IcebergCatalogHandler.java @@ -554,8 +554,8 @@ public Optional loadTableIfStale( } public LoadTableResponse loadTableWithAccessDelegation( - TableIdentifier tableIdentifier, String snapshots, EnumSet accessDelegationModes) { - return loadTableWithAccessDelegationIfStale(tableIdentifier, null, snapshots, accessDelegationModes).get(); + TableIdentifier tableIdentifier, String snapshots, EnumSet accessDelegationModes, String decodedCredentialsPath) { + return loadTableWithAccessDelegationIfStale(tableIdentifier, null, snapshots, accessDelegationModes, decodedCredentialsPath).get(); } /** @@ -653,7 +653,7 @@ public Optional loadTableWithAccessDelegationIfStale( tableIdentifier, tableMetadata, actionsRequested)); if (accessDelegationModes.contains(AccessDelegationMode.VENDED_CREDENTIALS)) { responseBuilder.addAllConfig( - credentialDelegation.getVendedCredentialConfig(tableIdentifier)); + credentialDelegation.getVendedCredentialConfig(tableIdentifier, decodedCredentialPath)); } } From 6ccc3e17d960edc82e42d639a94dfb0e16b71ae7 Mon Sep 17 00:00:00 2001 From: David Lu Date: Thu, 10 Apr 2025 20:35:52 -0400 Subject: [PATCH 18/20] spotlessApply --- .../auth/JWTSymmetricKeyGeneratorTest.java | 1 - .../IcebergCatalogHandlerAuthzTest.java | 24 +++++++++++++++---- .../catalog/iceberg/IcebergCatalog.java | 3 ++- .../iceberg/IcebergCatalogAdapter.java | 9 ++++--- .../iceberg/IcebergCatalogHandler.java | 18 ++++++++++---- .../iceberg/SupportsCredentialDelegation.java | 3 ++- .../service/catalog/io/FileIOFactoryTest.java | 1 - .../apache/polaris/service/TestServices.java | 1 - 8 files changed, 44 insertions(+), 16 deletions(-) diff --git a/quarkus/service/src/test/java/org/apache/polaris/service/quarkus/auth/JWTSymmetricKeyGeneratorTest.java b/quarkus/service/src/test/java/org/apache/polaris/service/quarkus/auth/JWTSymmetricKeyGeneratorTest.java index 17ae0a0d1..fed5d20db 100644 --- a/quarkus/service/src/test/java/org/apache/polaris/service/quarkus/auth/JWTSymmetricKeyGeneratorTest.java +++ b/quarkus/service/src/test/java/org/apache/polaris/service/quarkus/auth/JWTSymmetricKeyGeneratorTest.java @@ -24,7 +24,6 @@ import com.auth0.jwt.JWTVerifier; import com.auth0.jwt.algorithms.Algorithm; import com.auth0.jwt.interfaces.DecodedJWT; -import java.net.URI; import java.util.Map; import org.apache.polaris.core.PolarisCallContext; import org.apache.polaris.core.context.CallContext; diff --git a/quarkus/service/src/test/java/org/apache/polaris/service/quarkus/catalog/IcebergCatalogHandlerAuthzTest.java b/quarkus/service/src/test/java/org/apache/polaris/service/quarkus/catalog/IcebergCatalogHandlerAuthzTest.java index 211c21516..d6bf7dd5d 100644 --- a/quarkus/service/src/test/java/org/apache/polaris/service/quarkus/catalog/IcebergCatalogHandlerAuthzTest.java +++ b/quarkus/service/src/test/java/org/apache/polaris/service/quarkus/catalog/IcebergCatalogHandlerAuthzTest.java @@ -974,7 +974,11 @@ public void testLoadTableWithReadAccessDelegationIfStaleSufficientPrivileges() { () -> newWrapper() .loadTableWithAccessDelegationIfStale( - TABLE_NS1A_2, IfNoneMatch.fromHeader("W/\"0:0\""), "all", EnumSet.noneOf(AccessDelegationMode.class), null), + TABLE_NS1A_2, + IfNoneMatch.fromHeader("W/\"0:0\""), + "all", + EnumSet.noneOf(AccessDelegationMode.class), + null), null /* cleanupAction */); } @@ -993,7 +997,11 @@ public void testLoadTableWithReadAccessDelegationIfStaleInsufficientPermissions( () -> newWrapper() .loadTableWithAccessDelegationIfStale( - TABLE_NS1A_2, IfNoneMatch.fromHeader("W/\"0:0\""), "all", EnumSet.noneOf(AccessDelegationMode.class), null)); + TABLE_NS1A_2, + IfNoneMatch.fromHeader("W/\"0:0\""), + "all", + EnumSet.noneOf(AccessDelegationMode.class), + null)); } @Test @@ -1009,7 +1017,11 @@ public void testLoadTableWithWriteAccessDelegationIfStaleSufficientPrivileges() () -> newWrapper() .loadTableWithAccessDelegationIfStale( - TABLE_NS1A_2, IfNoneMatch.fromHeader("W/\"0:0\""), "all", EnumSet.noneOf(AccessDelegationMode.class), null), + TABLE_NS1A_2, + IfNoneMatch.fromHeader("W/\"0:0\""), + "all", + EnumSet.noneOf(AccessDelegationMode.class), + null), null /* cleanupAction */); } @@ -1028,7 +1040,11 @@ public void testLoadTableWithWriteAccessDelegationIfStaleInsufficientPermissions () -> newWrapper() .loadTableWithAccessDelegationIfStale( - TABLE_NS1A_2, IfNoneMatch.fromHeader("W/\"0:0\""), "all", EnumSet.noneOf(AccessDelegationMode.class), null)); + TABLE_NS1A_2, + IfNoneMatch.fromHeader("W/\"0:0\""), + "all", + EnumSet.noneOf(AccessDelegationMode.class), + null)); } @Test diff --git a/service/common/src/main/java/org/apache/polaris/service/catalog/iceberg/IcebergCatalog.java b/service/common/src/main/java/org/apache/polaris/service/catalog/iceberg/IcebergCatalog.java index 90ed958d6..04cb9bf84 100644 --- a/service/common/src/main/java/org/apache/polaris/service/catalog/iceberg/IcebergCatalog.java +++ b/service/common/src/main/java/org/apache/polaris/service/catalog/iceberg/IcebergCatalog.java @@ -858,7 +858,8 @@ public Map getCredentialConfig( } @Override - public Map getVendedCredentialConfig(TableIdentifier tableIdentifier, String decodedCredentialsPath) { + public Map getVendedCredentialConfig( + TableIdentifier tableIdentifier, String decodedCredentialsPath) { Map vendedCredentialConfig = new HashMap<>(); vendedCredentialConfig.put(AwsClientProperties.REFRESH_CREDENTIALS_ENABLED, "true"); vendedCredentialConfig.put( diff --git a/service/common/src/main/java/org/apache/polaris/service/catalog/iceberg/IcebergCatalogAdapter.java b/service/common/src/main/java/org/apache/polaris/service/catalog/iceberg/IcebergCatalogAdapter.java index f2e0ff820..a181d7faa 100644 --- a/service/common/src/main/java/org/apache/polaris/service/catalog/iceberg/IcebergCatalogAdapter.java +++ b/service/common/src/main/java/org/apache/polaris/service/catalog/iceberg/IcebergCatalogAdapter.java @@ -378,7 +378,6 @@ public Response loadTable( Namespace ns = decodeNamespace(namespace); TableIdentifier tableIdentifier = TableIdentifier.of(ns, RESTUtil.decodeString(table)); - // TODO: Populate with header value from parameter once the generated interface // contains the if-none-match header IfNoneMatch ifNoneMatch = IfNoneMatch.fromHeader(null); @@ -393,7 +392,6 @@ public Response loadTable( catalog -> { LoadTableResponse response; - if (delegationModes.isEmpty()) { response = catalog @@ -406,7 +404,12 @@ public Response loadTable( prefix, tableIdentifier.namespace().toString(), tableIdentifier.name()); response = catalog - .loadTableWithAccessDelegationIfStale(tableIdentifier, ifNoneMatch, snapshots, delegationModes, RESTUtil.decodeString(credentialsEndpoint)) + .loadTableWithAccessDelegationIfStale( + tableIdentifier, + ifNoneMatch, + snapshots, + delegationModes, + RESTUtil.decodeString(credentialsEndpoint)) .orElseThrow(() -> new WebApplicationException(Response.Status.NOT_MODIFIED)); } diff --git a/service/common/src/main/java/org/apache/polaris/service/catalog/iceberg/IcebergCatalogHandler.java b/service/common/src/main/java/org/apache/polaris/service/catalog/iceberg/IcebergCatalogHandler.java index 994b4b91d..9df23174a 100644 --- a/service/common/src/main/java/org/apache/polaris/service/catalog/iceberg/IcebergCatalogHandler.java +++ b/service/common/src/main/java/org/apache/polaris/service/catalog/iceberg/IcebergCatalogHandler.java @@ -554,8 +554,13 @@ public Optional loadTableIfStale( } public LoadTableResponse loadTableWithAccessDelegation( - TableIdentifier tableIdentifier, String snapshots, EnumSet accessDelegationModes, String decodedCredentialsPath) { - return loadTableWithAccessDelegationIfStale(tableIdentifier, null, snapshots, accessDelegationModes, decodedCredentialsPath).get(); + TableIdentifier tableIdentifier, + String snapshots, + EnumSet accessDelegationModes, + String decodedCredentialsPath) { + return loadTableWithAccessDelegationIfStale( + tableIdentifier, null, snapshots, accessDelegationModes, decodedCredentialsPath) + .get(); } /** @@ -569,7 +574,11 @@ public LoadTableResponse loadTableWithAccessDelegation( * load table response, otherwise */ public Optional loadTableWithAccessDelegationIfStale( - TableIdentifier tableIdentifier, IfNoneMatch ifNoneMatch, String snapshots, EnumSet accessDelegationModes, String decodedCredentialPath) { + TableIdentifier tableIdentifier, + IfNoneMatch ifNoneMatch, + String snapshots, + EnumSet accessDelegationModes, + String decodedCredentialPath) { // Here we have a single method that falls through multiple candidate // PolarisAuthorizableOperations because instead of identifying the desired operation up-front // and @@ -653,7 +662,8 @@ public Optional loadTableWithAccessDelegationIfStale( tableIdentifier, tableMetadata, actionsRequested)); if (accessDelegationModes.contains(AccessDelegationMode.VENDED_CREDENTIALS)) { responseBuilder.addAllConfig( - credentialDelegation.getVendedCredentialConfig(tableIdentifier, decodedCredentialPath)); + credentialDelegation.getVendedCredentialConfig( + tableIdentifier, decodedCredentialPath)); } } diff --git a/service/common/src/main/java/org/apache/polaris/service/catalog/iceberg/SupportsCredentialDelegation.java b/service/common/src/main/java/org/apache/polaris/service/catalog/iceberg/SupportsCredentialDelegation.java index bc1c5fd33..47ad7a20f 100644 --- a/service/common/src/main/java/org/apache/polaris/service/catalog/iceberg/SupportsCredentialDelegation.java +++ b/service/common/src/main/java/org/apache/polaris/service/catalog/iceberg/SupportsCredentialDelegation.java @@ -37,5 +37,6 @@ Map getCredentialConfig( TableMetadata tableMetadata, Set storageActions); - Map getVendedCredentialConfig(TableIdentifier tableIdentifier, String decodedCredentialsPath); + Map getVendedCredentialConfig( + TableIdentifier tableIdentifier, String decodedCredentialsPath); } diff --git a/service/common/src/test/java/org/apache/polaris/service/catalog/io/FileIOFactoryTest.java b/service/common/src/test/java/org/apache/polaris/service/catalog/io/FileIOFactoryTest.java index 15d8935b3..9a070273b 100644 --- a/service/common/src/test/java/org/apache/polaris/service/catalog/io/FileIOFactoryTest.java +++ b/service/common/src/test/java/org/apache/polaris/service/catalog/io/FileIOFactoryTest.java @@ -25,7 +25,6 @@ import com.google.common.collect.ImmutableMap; import jakarta.annotation.Nonnull; import java.lang.reflect.Method; -import java.net.URI; import java.time.Clock; import java.util.HashMap; import java.util.List; diff --git a/service/common/src/testFixtures/java/org/apache/polaris/service/TestServices.java b/service/common/src/testFixtures/java/org/apache/polaris/service/TestServices.java index 624f418fa..d55021792 100644 --- a/service/common/src/testFixtures/java/org/apache/polaris/service/TestServices.java +++ b/service/common/src/testFixtures/java/org/apache/polaris/service/TestServices.java @@ -21,7 +21,6 @@ import com.google.auth.oauth2.AccessToken; import com.google.auth.oauth2.GoogleCredentials; import jakarta.ws.rs.core.SecurityContext; -import java.net.URI; import java.security.Principal; import java.time.Clock; import java.time.Instant; From 54ef8786c6716c90ced3a168f929ed7419fe47f0 Mon Sep 17 00:00:00 2001 From: David Lu Date: Thu, 10 Apr 2025 20:43:32 -0400 Subject: [PATCH 19/20] remove unnecessary method --- .../service/catalog/iceberg/IcebergCatalog.java | 11 ----------- .../catalog/iceberg/IcebergCatalogHandler.java | 7 ++++--- .../catalog/iceberg/SupportsCredentialDelegation.java | 3 --- 3 files changed, 4 insertions(+), 17 deletions(-) diff --git a/service/common/src/main/java/org/apache/polaris/service/catalog/iceberg/IcebergCatalog.java b/service/common/src/main/java/org/apache/polaris/service/catalog/iceberg/IcebergCatalog.java index 04cb9bf84..3f9722f79 100644 --- a/service/common/src/main/java/org/apache/polaris/service/catalog/iceberg/IcebergCatalog.java +++ b/service/common/src/main/java/org/apache/polaris/service/catalog/iceberg/IcebergCatalog.java @@ -51,7 +51,6 @@ import org.apache.iceberg.TableMetadata; import org.apache.iceberg.TableMetadataParser; import org.apache.iceberg.TableOperations; -import org.apache.iceberg.aws.AwsClientProperties; import org.apache.iceberg.catalog.Namespace; import org.apache.iceberg.catalog.SupportsNamespaces; import org.apache.iceberg.catalog.TableIdentifier; @@ -857,16 +856,6 @@ public Map getCredentialConfig( storageInfo.get()); } - @Override - public Map getVendedCredentialConfig( - TableIdentifier tableIdentifier, String decodedCredentialsPath) { - Map vendedCredentialConfig = new HashMap<>(); - vendedCredentialConfig.put(AwsClientProperties.REFRESH_CREDENTIALS_ENABLED, "true"); - vendedCredentialConfig.put( - AwsClientProperties.REFRESH_CREDENTIALS_ENDPOINT, decodedCredentialsPath); - return vendedCredentialConfig; - } - /** * Based on configuration settings, for callsites that need to handle potentially setting a new * base location for a TableLike entity, produces the transformed location if applicable, or else diff --git a/service/common/src/main/java/org/apache/polaris/service/catalog/iceberg/IcebergCatalogHandler.java b/service/common/src/main/java/org/apache/polaris/service/catalog/iceberg/IcebergCatalogHandler.java index 9df23174a..a8d52a9cb 100644 --- a/service/common/src/main/java/org/apache/polaris/service/catalog/iceberg/IcebergCatalogHandler.java +++ b/service/common/src/main/java/org/apache/polaris/service/catalog/iceberg/IcebergCatalogHandler.java @@ -42,6 +42,7 @@ import org.apache.iceberg.TableMetadata; import org.apache.iceberg.TableOperations; import org.apache.iceberg.UpdateRequirement; +import org.apache.iceberg.aws.AwsClientProperties; import org.apache.iceberg.catalog.Catalog; import org.apache.iceberg.catalog.Namespace; import org.apache.iceberg.catalog.SupportsNamespaces; @@ -661,9 +662,9 @@ public Optional loadTableWithAccessDelegationIfStale( credentialDelegation.getCredentialConfig( tableIdentifier, tableMetadata, actionsRequested)); if (accessDelegationModes.contains(AccessDelegationMode.VENDED_CREDENTIALS)) { - responseBuilder.addAllConfig( - credentialDelegation.getVendedCredentialConfig( - tableIdentifier, decodedCredentialPath)); + responseBuilder.addConfig(AwsClientProperties.REFRESH_CREDENTIALS_ENABLED, "true"); + responseBuilder.addConfig( + AwsClientProperties.REFRESH_CREDENTIALS_ENDPOINT, decodedCredentialPath); } } diff --git a/service/common/src/main/java/org/apache/polaris/service/catalog/iceberg/SupportsCredentialDelegation.java b/service/common/src/main/java/org/apache/polaris/service/catalog/iceberg/SupportsCredentialDelegation.java index 47ad7a20f..06ca7fbde 100644 --- a/service/common/src/main/java/org/apache/polaris/service/catalog/iceberg/SupportsCredentialDelegation.java +++ b/service/common/src/main/java/org/apache/polaris/service/catalog/iceberg/SupportsCredentialDelegation.java @@ -36,7 +36,4 @@ Map getCredentialConfig( TableIdentifier tableIdentifier, TableMetadata tableMetadata, Set storageActions); - - Map getVendedCredentialConfig( - TableIdentifier tableIdentifier, String decodedCredentialsPath); } From 315afdafaf47692551a69e593496f96380a99611 Mon Sep 17 00:00:00 2001 From: David Lu Date: Tue, 15 Apr 2025 12:33:22 -0400 Subject: [PATCH 20/20] inject refresh credential properties at proper place --- .../IcebergCatalogHandlerAuthzTest.java | 46 ++++--------------- .../iceberg/IcebergCatalogAdapter.java | 31 +++++++++---- .../iceberg/IcebergCatalogHandler.java | 23 ++-------- 3 files changed, 33 insertions(+), 67 deletions(-) diff --git a/quarkus/service/src/test/java/org/apache/polaris/service/quarkus/catalog/IcebergCatalogHandlerAuthzTest.java b/quarkus/service/src/test/java/org/apache/polaris/service/quarkus/catalog/IcebergCatalogHandlerAuthzTest.java index d6bf7dd5d..207955c93 100644 --- a/quarkus/service/src/test/java/org/apache/polaris/service/quarkus/catalog/IcebergCatalogHandlerAuthzTest.java +++ b/quarkus/service/src/test/java/org/apache/polaris/service/quarkus/catalog/IcebergCatalogHandlerAuthzTest.java @@ -23,7 +23,6 @@ import io.quarkus.test.junit.TestProfile; import jakarta.ws.rs.core.SecurityContext; import java.time.Instant; -import java.util.EnumSet; import java.util.List; import java.util.Map; import java.util.Set; @@ -65,7 +64,6 @@ import org.apache.polaris.core.persistence.PolarisEntityManager; import org.apache.polaris.core.persistence.dao.entity.CreatePrincipalResult; import org.apache.polaris.core.persistence.resolver.PolarisResolutionManifest; -import org.apache.polaris.service.catalog.AccessDelegationMode; import org.apache.polaris.service.catalog.iceberg.IcebergCatalogHandler; import org.apache.polaris.service.catalog.io.DefaultFileIOFactory; import org.apache.polaris.service.config.RealmEntityManagerFactory; @@ -904,10 +902,7 @@ public void testLoadTableWithReadAccessDelegationSufficientPrivileges() { PolarisPrivilege.TABLE_READ_DATA, PolarisPrivilege.TABLE_WRITE_DATA, PolarisPrivilege.CATALOG_MANAGE_CONTENT), - () -> - newWrapper() - .loadTableWithAccessDelegation( - TABLE_NS1A_2, "all", EnumSet.noneOf(AccessDelegationMode.class), null), + () -> newWrapper().loadTableWithAccessDelegation(TABLE_NS1A_2, "all"), null /* cleanupAction */); } @@ -923,10 +918,7 @@ public void testLoadTableWithReadAccessDelegationInsufficientPermissions() { PolarisPrivilege.TABLE_CREATE, PolarisPrivilege.TABLE_LIST, PolarisPrivilege.TABLE_DROP), - () -> - newWrapper() - .loadTableWithAccessDelegation( - TABLE_NS1A_2, "all", EnumSet.noneOf(AccessDelegationMode.class), null)); + () -> newWrapper().loadTableWithAccessDelegation(TABLE_NS1A_2, "all")); } @Test @@ -939,10 +931,7 @@ public void testLoadTableWithWriteAccessDelegationSufficientPrivileges() { PolarisPrivilege.TABLE_READ_DATA, PolarisPrivilege.TABLE_WRITE_DATA, PolarisPrivilege.CATALOG_MANAGE_CONTENT), - () -> - newWrapper() - .loadTableWithAccessDelegation( - TABLE_NS1A_2, "all", EnumSet.noneOf(AccessDelegationMode.class), null), + () -> newWrapper().loadTableWithAccessDelegation(TABLE_NS1A_2, "all"), null /* cleanupAction */); } @@ -958,10 +947,7 @@ public void testLoadTableWithWriteAccessDelegationInsufficientPermissions() { PolarisPrivilege.TABLE_CREATE, PolarisPrivilege.TABLE_LIST, PolarisPrivilege.TABLE_DROP), - () -> - newWrapper() - .loadTableWithAccessDelegation( - TABLE_NS1A_2, "all", EnumSet.noneOf(AccessDelegationMode.class), null)); + () -> newWrapper().loadTableWithAccessDelegation(TABLE_NS1A_2, "all")); } @Test @@ -974,11 +960,7 @@ public void testLoadTableWithReadAccessDelegationIfStaleSufficientPrivileges() { () -> newWrapper() .loadTableWithAccessDelegationIfStale( - TABLE_NS1A_2, - IfNoneMatch.fromHeader("W/\"0:0\""), - "all", - EnumSet.noneOf(AccessDelegationMode.class), - null), + TABLE_NS1A_2, IfNoneMatch.fromHeader("W/\"0:0\""), "all"), null /* cleanupAction */); } @@ -997,11 +979,7 @@ public void testLoadTableWithReadAccessDelegationIfStaleInsufficientPermissions( () -> newWrapper() .loadTableWithAccessDelegationIfStale( - TABLE_NS1A_2, - IfNoneMatch.fromHeader("W/\"0:0\""), - "all", - EnumSet.noneOf(AccessDelegationMode.class), - null)); + TABLE_NS1A_2, IfNoneMatch.fromHeader("W/\"0:0\""), "all")); } @Test @@ -1017,11 +995,7 @@ public void testLoadTableWithWriteAccessDelegationIfStaleSufficientPrivileges() () -> newWrapper() .loadTableWithAccessDelegationIfStale( - TABLE_NS1A_2, - IfNoneMatch.fromHeader("W/\"0:0\""), - "all", - EnumSet.noneOf(AccessDelegationMode.class), - null), + TABLE_NS1A_2, IfNoneMatch.fromHeader("W/\"0:0\""), "all"), null /* cleanupAction */); } @@ -1040,11 +1014,7 @@ public void testLoadTableWithWriteAccessDelegationIfStaleInsufficientPermissions () -> newWrapper() .loadTableWithAccessDelegationIfStale( - TABLE_NS1A_2, - IfNoneMatch.fromHeader("W/\"0:0\""), - "all", - EnumSet.noneOf(AccessDelegationMode.class), - null)); + TABLE_NS1A_2, IfNoneMatch.fromHeader("W/\"0:0\""), "all")); } @Test diff --git a/service/common/src/main/java/org/apache/polaris/service/catalog/iceberg/IcebergCatalogAdapter.java b/service/common/src/main/java/org/apache/polaris/service/catalog/iceberg/IcebergCatalogAdapter.java index a181d7faa..f395157fb 100644 --- a/service/common/src/main/java/org/apache/polaris/service/catalog/iceberg/IcebergCatalogAdapter.java +++ b/service/common/src/main/java/org/apache/polaris/service/catalog/iceberg/IcebergCatalogAdapter.java @@ -37,6 +37,7 @@ import java.util.Optional; import java.util.Set; import java.util.function.Function; +import org.apache.iceberg.aws.AwsClientProperties; import org.apache.iceberg.catalog.Catalog; import org.apache.iceberg.catalog.Namespace; import org.apache.iceberg.catalog.TableIdentifier; @@ -402,21 +403,34 @@ public Response loadTable( String.format( "/v1/%s/namespaces/%s/tables/%s/credentials", prefix, tableIdentifier.namespace().toString(), tableIdentifier.name()); - response = + LoadTableResponse originalResponse = catalog - .loadTableWithAccessDelegationIfStale( - tableIdentifier, - ifNoneMatch, - snapshots, - delegationModes, - RESTUtil.decodeString(credentialsEndpoint)) + .loadTableWithAccessDelegationIfStale(tableIdentifier, ifNoneMatch, snapshots) .orElseThrow(() -> new WebApplicationException(Response.Status.NOT_MODIFIED)); + if (delegationModes.contains(VENDED_CREDENTIALS)) { + response = + injectRefreshVendedCredentialProperties(originalResponse, credentialsEndpoint); + } else { + response = originalResponse; + } } return tryInsertETagHeader(Response.ok(response), response, namespace, table).build(); }); } + private LoadTableResponse injectRefreshVendedCredentialProperties( + LoadTableResponse originalResponse, String credentialsEndpoint) { + LoadTableResponse.Builder loadResponseBuilder = + LoadTableResponse.builder().withTableMetadata(originalResponse.tableMetadata()); + loadResponseBuilder.addAllConfig(originalResponse.config()); + loadResponseBuilder.addAllCredentials(originalResponse.credentials()); + loadResponseBuilder.addConfig( + AwsClientProperties.REFRESH_CREDENTIALS_ENDPOINT, credentialsEndpoint); + loadResponseBuilder.addConfig(AwsClientProperties.REFRESH_CREDENTIALS_ENABLED, "true"); + return loadResponseBuilder.build(); + } + @Override public Response tableExists( String prefix, @@ -557,8 +571,7 @@ public Response loadCredentials( prefix, catalog -> { LoadTableResponse loadTableResponse = - catalog.loadTableWithAccessDelegation( - tableIdentifier, "all", EnumSet.noneOf(AccessDelegationMode.class), null); + catalog.loadTableWithAccessDelegation(tableIdentifier, "all"); return Response.ok( ImmutableLoadCredentialsResponse.builder() .credentials(loadTableResponse.credentials()) diff --git a/service/common/src/main/java/org/apache/polaris/service/catalog/iceberg/IcebergCatalogHandler.java b/service/common/src/main/java/org/apache/polaris/service/catalog/iceberg/IcebergCatalogHandler.java index a8d52a9cb..b5e1a0edb 100644 --- a/service/common/src/main/java/org/apache/polaris/service/catalog/iceberg/IcebergCatalogHandler.java +++ b/service/common/src/main/java/org/apache/polaris/service/catalog/iceberg/IcebergCatalogHandler.java @@ -26,7 +26,6 @@ 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; @@ -42,7 +41,6 @@ import org.apache.iceberg.TableMetadata; import org.apache.iceberg.TableOperations; import org.apache.iceberg.UpdateRequirement; -import org.apache.iceberg.aws.AwsClientProperties; import org.apache.iceberg.catalog.Catalog; import org.apache.iceberg.catalog.Namespace; import org.apache.iceberg.catalog.SupportsNamespaces; @@ -84,7 +82,6 @@ import org.apache.polaris.core.persistence.dao.entity.EntitiesResult; import org.apache.polaris.core.persistence.dao.entity.EntityWithPath; 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.context.CallContextCatalogFactory; @@ -555,13 +552,8 @@ public Optional loadTableIfStale( } public LoadTableResponse loadTableWithAccessDelegation( - TableIdentifier tableIdentifier, - String snapshots, - EnumSet accessDelegationModes, - String decodedCredentialsPath) { - return loadTableWithAccessDelegationIfStale( - tableIdentifier, null, snapshots, accessDelegationModes, decodedCredentialsPath) - .get(); + TableIdentifier tableIdentifier, String snapshots) { + return loadTableWithAccessDelegationIfStale(tableIdentifier, null, snapshots).get(); } /** @@ -575,11 +567,7 @@ public LoadTableResponse loadTableWithAccessDelegation( * load table response, otherwise */ public Optional loadTableWithAccessDelegationIfStale( - TableIdentifier tableIdentifier, - IfNoneMatch ifNoneMatch, - String snapshots, - EnumSet accessDelegationModes, - String decodedCredentialPath) { + TableIdentifier tableIdentifier, IfNoneMatch ifNoneMatch, String snapshots) { // Here we have a single method that falls through multiple candidate // PolarisAuthorizableOperations because instead of identifying the desired operation up-front // and @@ -661,11 +649,6 @@ public Optional loadTableWithAccessDelegationIfStale( responseBuilder.addAllConfig( credentialDelegation.getCredentialConfig( tableIdentifier, tableMetadata, actionsRequested)); - if (accessDelegationModes.contains(AccessDelegationMode.VENDED_CREDENTIALS)) { - responseBuilder.addConfig(AwsClientProperties.REFRESH_CREDENTIALS_ENABLED, "true"); - responseBuilder.addConfig( - AwsClientProperties.REFRESH_CREDENTIALS_ENDPOINT, decodedCredentialPath); - } } return Optional.of(responseBuilder.build());