dimas-b commented on code in PR #2280:
URL: https://github.com/apache/polaris/pull/2280#discussion_r2500660568
##########
runtime/service/src/main/java/org/apache/polaris/service/catalog/io/AccessConfigProvider.java:
##########
@@ -101,4 +120,58 @@ public AccessConfig getAccessConfig(
storageInfo.get(),
refreshCredentialsEndpoint);
}
+
+ /**
+ * Generates a remote signing configuration for accessing table storage at
explicit locations.
+ *
+ * @param callContext the call context containing realm, principal, and
security context
+ * @param catalogName the name of the catalog
+ * @param tableIdentifier the table identifier, used for logging and refresh
endpoint construction
+ * @param resolvedPath the entity hierarchy to search for storage
configuration
+ * @return {@link AccessConfig} with scoped credentials and metadata; empty
if no storage config
+ * found
+ */
+ public AccessConfig getAccessConfigForRemoteSigning(
+ @Nonnull CallContext callContext,
+ @Nonnull String catalogName,
+ @Nonnull TableIdentifier tableIdentifier,
+ @Nonnull PolarisResolvedPathWrapper resolvedPath) {
+ LOGGER
+ .atDebug()
+ .addKeyValue("tableIdentifier", tableIdentifier)
+ .log("Fetching remote signing config for table");
+
+ Optional<PolarisEntity> storageInfo =
FileIOUtil.findStorageInfoFromHierarchy(resolvedPath);
+ Optional<PolarisStorageConfigurationInfo> configurationInfo =
+ storageInfo
+ .map(PolarisEntity::getInternalPropertiesAsMap)
+ .map(info ->
info.get(PolarisEntityConstants.getStorageConfigInfoPropertyName()))
+ .map(PolarisStorageConfigurationInfo::deserialize);
+
+ if (configurationInfo.isEmpty()) {
+ LOGGER
+ .atWarn()
+ .addKeyValue("tableIdentifier", tableIdentifier)
+ .log("Table entity has no storage configuration in its hierarchy");
+ return AccessConfig.EMPTY;
+ }
+
+ PolarisStorageIntegration<AwsStorageConfigurationInfo> storageIntegration =
+
storageIntegrationProvider.getStorageIntegrationForConfig(configurationInfo.get());
+
+ if (!(storageIntegration
+ instanceof AwsCredentialsStorageIntegration
awsCredentialsStorageIntegration)) {
+ LOGGER
+ .atWarn()
+ .addKeyValue("tableIdentifier", tableIdentifier)
+ .log("Table entity storage integration is not an AWS credentials
storage integration");
+ return AccessConfig.EMPTY;
+ }
+
+ String prefix =
prefixParser.catalogNameToPrefix(callContext.getRealmContext(), catalogName);
+ URI signerUri = uriInfo.getBaseUri().resolve("api/");
Review Comment:
I'm not sure about the `api/` constant... It related to our `@Path`
annotations in the generated REST API classes, so it's usage is a bit obscure
here. Could we make it a constant in the `s3-sign-service` module?
##########
polaris-core/src/main/java/org/apache/polaris/core/storage/StorageAccessProperty.java:
##########
@@ -52,6 +54,21 @@ public enum StorageAccessProperty {
"the endpoint to load vended credentials for a table from the catalog",
false,
false),
+ AWS_REMOTE_SIGNING_ENABLED(
+ Boolean.class,
+ S3FileIOProperties.REMOTE_SIGNING_ENABLED,
+ "whether to enable remote signing for S3 requests",
+ false),
+ AWS_REMOTE_SIGNER_URI(
+ String.class,
+ S3V4RestSignerClient.S3_SIGNER_URI,
+ "the base URI for the remote signer service, used for signing S3
requests",
Review Comment:
Does this have a reference Iceberg version number where the property becomes
respected?
##########
polaris-core/src/main/java/org/apache/polaris/core/rest/PolarisResourcePaths.java:
##########
@@ -78,4 +82,15 @@ public String genericTable(TableIdentifier ident) {
"generic-tables",
RESTUtil.encodeString(ident.name()));
}
+
+ public String s3RemoteSigning(TableIdentifier ident) {
Review Comment:
Is this a `core` concern? I suppose it is only relevant to the REST layer 🤔
##########
runtime/service/src/main/java/org/apache/polaris/service/catalog/iceberg/IcebergCatalogAdapter.java:
##########
@@ -828,6 +830,9 @@ public Response getConfig(
.addAll(VIEW_ENDPOINTS)
.addAll(PolarisEndpoints.getSupportedGenericTableEndpoints(realmConfig))
.addAll(PolarisEndpoints.getSupportedPolicyEndpoints(realmConfig))
+ .addAll(
+
PolarisEndpoints.getSupportedRemoteSigningEndpoints(
Review Comment:
I'm not sure it's worth advertising in `ConfigResponse` because it's not a
standard endpoint, for which clients might want to check the endpoints list.
Clients learn about it via `loadTable` responses, I guess 🤔
##########
runtime/service/src/main/java/org/apache/polaris/service/catalog/iceberg/IcebergCatalogHandler.java:
##########
@@ -801,6 +806,33 @@ private LoadTableResponse.Builder
buildLoadTableResponseWithDelegationCredential
return responseBuilder;
}
+ AccessDelegationMode delegationMode =
selectAccessDelegationMode(delegationModes);
+
+ if (delegationMode == REMOTE_SIGNING) {
+
+ S3RemoteSigningCatalogHandler.throwIfRemoteSigningNotEnabled(
+ callContext.getRealmConfig(), getResolvedCatalogEntity());
+
+ AccessConfig accessConfig =
+ accessConfigProvider.getAccessConfigForRemoteSigning(
+ callContext, catalogName, tableIdentifier, resolvedStoragePath);
+
+ Map<String, String> credentialConfig = accessConfig.credentials();
+
+ if (!credentialConfig.isEmpty()) {
+ responseBuilder.addAllConfig(credentialConfig);
+ responseBuilder.addCredential(
Review Comment:
Why `addCredential()` when we're remote-signing?
##########
spec/README.md:
##########
@@ -17,14 +17,40 @@
under the License.
-->
-# Polaris API Specifications
-
-Polaris provides two sets of OpenAPI specifications:
-- `polaris-management-service.yml` - Defines the management APIs for using
Polaris to create and manage Iceberg catalogs and their principals
-- `polaris-catalog-service.yaml` - Defines the specification for the Polaris
Catalog API, which encompasses both the Iceberg REST Catalog API
- and Polaris-native API.
- - `polaris-catalog-apis` - Contains the specification for Polaris-specific
Catalog APIs
- - `iceberg-rest-catalog-open-api.yaml` - Contains the specification for
Iceberg Rest Catalog API
+# Apache Polaris API Specifications
+
+Apache Polaris provides the following OpenAPI specifications:
+
+- [polaris-management-service.yml](polaris-management-service.yml) - Defines
the management APIs for using Apache
+ Polaris to create and manage Apache Iceberg catalogs and their principals
+
+- [polaris-catalog-service.yaml](polaris-catalog-service.yaml) - Defines the
specification for the Apache Polaris
+ Catalog API, which encompasses both the Apache Iceberg REST Catalog API and
Apache Polaris-native APIs:
+
+ - [iceberg-rest-catalog-open-api.yaml](iceberg-rest-catalog-open-api.yaml) -
Contains the specification for
+ Apache Iceberg Rest Catalog API.
+
+ - [polaris-catalog-apis](polaris-catalog-apis) - This folder contains the
specifications for Apache
+ Polaris-specific Catalog APIs:
+
+ - [generic-tables-api.yaml](polaris-catalog-apis/generic-tables-api.yaml)
- Contains the specification for
+ the Generic Tables API
+
+ - [notifications-api.yaml](polaris-catalog-apis/notifications-api.yaml) -
Contains the specification for
+ the Notifications API
+
+ - [oauth-tokens-api.yaml](polaris-catalog-apis/oauth-tokens-api.yaml) -
Contains the specification for the
+ internal OAuth Token endpoint, extracted from the Apache Iceberg REST
Catalog API.
+
+ - [policy-apis.yaml](polaris-catalog-apis/policy-apis.yaml) - Contains the
specification for the Policy APIs.
+
+- [s3-sign](s3-sign) - This folder contains the specifications for S3 remote
signing:
+
+ - [iceberg-s3-signer-open-api.yaml](s3-sign/iceberg-s3-signer-open-api.yaml)
- Contains the specification of the
Review Comment:
nit: maybe clarify that Polaris uses only type definitions from the spec
file?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
