Skip to content

Conversation

@HonahX
Copy link
Contributor

@HonahX HonahX commented Oct 9, 2025

This PR introduces credential vending support for passthrough-facade catalogs.

When creating a passthrough-facade catalog, the configuration currently requires two components:

  • StorageConfig – specifies the storage info for the remote catalog.
  • ConnectionInfo – defines connection parameters for the underlying remote catalog.

With this change, the StorageConfig is now also used to vend temporary credentials for user requests.
Credential vending honors table-level RBAC policies to determine whether to issue read-only or read-write credentials, ensuring access control consistency with Polaris authorization semantics.

A new test case validates the credential vending workflow, verifying both read and write credential vending.

Note: the remote catalog referenced by the passthrough-facade does not need to support IRC

Copy link
Contributor

@dimas-b dimas-b left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM overall 👍 just a few minor comments.

if (baseCatalog instanceof IcebergCatalog && resolvedStoragePath != null) {
if ((baseCatalog instanceof IcebergCatalog
|| realmConfig.getConfig(
ALLOW_EXTERNAL_CATALOG_CREDENTIAL_VENDING, getResolvedCatalogEntity()))
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we mention this in CHANGELOG.md?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good question. I'd prefer not, this PR seems a mix of bug fix and new feature, as we didn't clarify the behavior before.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why not clarify it now? My point that this condition change already affects Polaris behaviour, so we ought to notify users... which is what CHANGELOG is there for, right?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My concern is that users may not understand this behavior changes, it may confuse users even more, like what's the previous behavior? Is that supposed to be there already? Of course, we can explain it clearly in the changelog. I'm fine either way.

Copy link
Contributor

@dimas-b dimas-b Oct 9, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The ALLOW_EXTERNAL_CATALOG_CREDENTIAL_VENDING flag exists since #395. I believe we ought to inform users that existing flags start altering Polaris behaviour in new areas (where they did not work before).

Note that the default for ALLOW_EXTERNAL_CATALOG_CREDENTIAL_VENDING is true, so all users who have catalogs that are not instanceof IcebergCatalog will be affected by this PR.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for all the suggestions! After more thoughts, I realized that it better to have a separate param to control the credential vending for federated catalog. Plus we need to ensure ALLOW_EXTERNAL_CATALOG_CREDENTIAL_VENDING guard the passthrough-facade catalog in both loadTable and createTable call path.

The new experience is that we support credential vending for federated catalog if both ALLOW_EXTERNAL_CATALOG_CREDENTIAL_VENDING and the newly added ALLOW_FEDERATED_CATALOGS_CREDENTIAL_VENDING are set to true, which is the default behavior. It will make it easier to explain in the changelog

I've added a line in the CHANGELOG, but will wait for #2783 first

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thx - lgtm 👍

@HonahX HonahX requested a review from dennishuo October 9, 2025 22:53
dimas-b
dimas-b previously approved these changes Oct 9, 2025
@github-project-automation github-project-automation bot moved this from PRs In Progress to Ready to merge in Basic Kanban Board Oct 9, 2025
dennishuo
dennishuo previously approved these changes Oct 10, 2025
.defaultValue(true)
.buildFeatureConfiguration();

public static final FeatureConfiguration<Boolean> ALLOW_FEDERATED_CATALOGS_CREDENTIAL_VENDING =
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's reasonable to introduce the config now. It may not be necessary for long-term. We might think of a way to remove these configs. They makes system unnecessary complex. Can we mark it to be removed at certain versions, e.g., version 1.3/1.4? and file an issue to remove these in case we forgot it.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good suggestions! We may still want this new config but may need to scope it differently. Ideally we should let ALLOW_EXTERNAL_CATALOG_CREDENTIAL_VENDING only controls the behavior of static-facade EXTERNAL catalog and let ALLOW_FEDERATED_CATALOGS_CREDENTIAL_VENDING controls the behavior of passthrough-facade EXTERNAL catalog, since the implication of credential vending is different in these 2 cases. But that will be a behavior change and we'd better address that in a follow-up. I will drive that to make sure we stabilize this before 1.3

Copy link
Contributor

@flyrain flyrain Oct 10, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm still a bit confused by the long-term need of this config. Taking passthrough-facade EXTERNAL catalog as an example, why do we need a config to control the behavior? It should be the default behavior. Or is there any use case that user don't need credential vending, but still config the storage info.?

Copy link
Contributor Author

@HonahX HonahX Oct 10, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree this should be the default behavior. But I think it may be still good to preserve the ability to toggle the behavior until the new feature stabilized. And yes, may be several versions afterwards, we could remove this flag completely and make this a fundamental feature : ) I will have PR/issue to track this, thanks!

flyrain
flyrain previously approved these changes Oct 10, 2025
@HonahX HonahX dismissed stale reviews from flyrain, dennishuo, and dimas-b via 6428b9e October 10, 2025 17:08
@HonahX HonahX force-pushed the honahx-enable-credential-vending-for-federated-catalog branch from 4c904b5 to 6428b9e Compare October 10, 2025 17:08
@HonahX HonahX merged commit 04a9950 into apache:main Oct 10, 2025
16 checks passed
@github-project-automation github-project-automation bot moved this from Ready to merge to Done in Basic Kanban Board Oct 10, 2025
snazy added a commit to snazy/polaris that referenced this pull request Nov 20, 2025
* Build: remove code to post-process generated Quarkus jars (apache#2667)

Before Quarkus 3.28, the Quarkus generated jars used the "current" timestamp for all ZIP entries, which made the jars not-reproducible.
Since Quarkus 3.28, the generated jars use a fixed timestamp for all ZIP entries, so the custom code is no longer necessary.

This PR depends on Quarkus 3.28.

* Update docker.io/jaegertracing/all-in-one Docker tag to v1.74.0 (apache#2751)

* Updating metastore documentation with Aurora postgres example (apache#2706)

* added Aurora postgres to metastore documentation

* Service: Add events for APIs awaiting API changes (apache#2712)

* fix(enhancement): add .idea, .vscode, .venv to top level .gitignore (apache#2718)

fix(enhancement): add .idea, .vscode, .venv to top level .gitignore

* Fix javadocs of `PolarisPrincipal.getPrincipalRoles()` (apache#2752)

* fix(enhancement): squash commits (apache#2643)

* fix(deps): update dependency io.smallrye.config:smallrye-config-core to v3.14.1 (apache#2755)

* Extract interface for RequestIdGenerator (apache#2720)

Summary of changes:

1. Extracted an interface from `RequestIdGenerator`.
2. The `generateRequestId` method now returns a `Uni<String>` in case custom implementations need to perform I/O or other blocking calls during request ID generation.
3. Also addressed comments in apache#2602.

* JDBC: Handle schema evolution (apache#2714)

* Deprecate legacy management endpoints for removal (apache#2749)

* Deprecate LegacyManagementEndpoints for removal

* Add PolarisResolutionManifestCatalogView.getResolvedCatalogEntity helper (apache#2750)

this centralizes some common code and simplifies some test setups

* Enforce that S3 credentials are vended when requested (apache#2711)

This is a follow-up change to apache#2672 striving to improve user-facing error reporting for S3 storage systems without STS.

* Add property to `AccessConfig` to indicate whether the backing storage integration can produce credentials.

* Add a check to `IcebergCatalogHandler` (leading to 400) that storage credentials are vended when requested and the backend is capable of vending credentials in principle.

* Update `PolarisStorageIntegrationProviderImpl` to indicate that FILE storage does not support credential vending (requesitng redential vending with FILE storage does not produce any credentials and does not flag an error, which matches current Polaris behaviour).

* Only those S3 systems where STS is not available (or disabled / not permitted) are affected.

* Other storage integrations are not affected by this PR.

* [Catalog Federation] Ignore JIT entities when deleting federated catalogs, add integration test for namespace/table-level RBAC (apache#2690)

When enabling table/namespace level RBAC in federated catalog, JIT entities will be created during privilege grant. In the short term, we should ignore them when dropping the catalog. In the long term, we will clean-up those entities when deleting the catalog.

This will be the first step towards JIT entity clean-up:

1. Ignore JIT entities when dropping federated catalog (orphan entities)
2. Register tasks/in-place cleanup JIT entities during catalog drop
3. Add new functionality to PolarisMetastoreManager to support atomic delete non-used JIT entities during revoke.
4. Global Garbage Collector to clean-up unreachable entities (entities with non-existing catalog path/parent)

* SigV4 Auth Support for Catalog Federation - Part 3: Service Identity Info Injection (apache#2523)

This PR introduces service identity management for SigV4 Auth Support for Catalog Federation. Unlike user-supplied parameters, the service identity represents the identity of the Polaris service itself and should be managed by Polaris.

* Service Identity Injection

* Return injected service identity info in response

* Use AwsCredentialsProvider to retrieve the credentials

* Move some logic to ServiceIdentityConfiguration

* Rename ServiceIdentityRegistry to ServiceIdentityProvider

* Rename ResolvedServiceIdentity to ServiceIdentityCredential

* Simplify the logic and add more tests

* Use SecretReference and fix some small issues

* Disable Catalog Federation

* Update actions/stale digest to 5f858e3 (apache#2758)

* Service: RealmContextFilter test refactor (apache#2747)

* Update dependency software.amazon.awssdk:bom to v2.35.0 (apache#2760)

* Update apache/spark Docker tag to v3.5.7 (apache#2727)

* Update eric-maynard Team entry (apache#2763)

I'm no longer affiliated with Snowflake, so we should update this page accordingly

* Refactor resolutionManifest handling in PolarisAdminService (apache#2748)

- remove mutable `resolutionManifest` field in favor of letting the
  "authorize" methods return their `PolarisResolutionManifest`
- replace "find" helpers with "get" helpers that have built-in error
  handling

* Implement Finer Grained Operations and Privileges For Update Table (apache#2697)

This implements finer grained operations and privileges for update table in a backwards compatible way as discussed on the mailing list.

The idea is that all the existing privileges and operations will work and continue to work even after this change. (i.e. TABLE_WRITE_PROPERTIES will still ensure update table is authorized even after these changes).

However, because Polaris will now be able to identify each operation within an UpdateTable request and has a privilege model with inheritance that maps to each operation, users will now have the option of restricting permissions at a finer level if desired.

* [Python CLI][CI Failure] Pin pydantic version to < 2.12.0 to fix CI failure (apache#2770)

* Delete ServiceSecretReference (apache#2768)

* JDBC: Fix Bootstrap with schema options (apache#2762)

* Site: Add puppygraph integration (apache#2753)

* Update Changelog with finer grained authz (apache#2775)

* Add Arguments to Various Event Records (apache#2765)

* Update immutables to v2.11.5 (apache#2776)

* Client: add support for policy management (apache#2701)

Implementation for policy management via Polaris CLI (apache#1867).

Here are the subcommands to API mapping:

attach
 - PUT /polaris/v1/{prefix}/namespaces/{namespace}/policies/{policy-name}/mappings
create
 - POST /polaris/v1/{prefix}/namespaces/{namespace}/policies/{policy-name}/mappings
delete
 - DELETE /polaris/v1/{prefix}/namespaces/{namespace}/policies/{policy-name}
detach
 - POST /polaris/v1/{prefix}/namespaces/{namespace}/policies/{policy-name}/mappings
get
 - GET /polaris/v1/{prefix}/namespaces/{namespace}/policies/{policy-name}
list
 - GET /polaris/v1/{prefix}/namespaces/{namespace}/policies 
   - This is default for `list` operation
 - GET /polaris/v1/{prefix}/applicable-policies
   - This is when we have `--applicable` option provided
update
 - PUT /polaris/v1/{prefix}/namespaces/{namespace}/policies/{policy-name}

* Update dependency com.google.cloud:google-cloud-storage-bom to v2.58.1 (apache#2764)

* Update dependency org.jboss.weld:weld-junit5 to v5.0.3.Final (apache#2777)

* Update the LICENSE and NOTICE files in the runtime (apache#2779)

* SigV4 Auth Support for Catalog Federation - Part 4: Connection Credential Manager (apache#2759)

This PR introduces a flexible credential management system for Polaris. Building on Part 3's service identity management, this system combines Polaris service identities with user-provided authentication parameters to generate credentials for remote catalog access.
The core of this PR is the new ConnectionCredentialVendor interface, which:

Generates connection credentials by combining service identity with user auth parameters
Supports different authentication types (AWS SIGV4, AZURE Entra, GCP IAM) through CDI, currently only supports SigV4.
Provides on-demand credential generation
Enables easy extension for new authentication types
In the long term, we should move the storage credential management logic out of PolarisMetastoreManager, PolarisMetastoreManager should only provide persistence interfaces.

* Extract IcebergCatalog.getAccessConfig to a separate class AccessConfigProvider (apache#2736)

This PR extracts credential vending entrypoint getAccessConfig from IcebergCatalog into a new centralized AccessConfigProvider class, decoupling credential generation from catalog implementations.

The old SupportsCredentialVending is removed in this PR upon discussion

* Update immutables to v2.11.6 (apache#2780)

* Enhance Release docs (apache#2787)

* Spark: Remove unnecessary dependency (apache#2789)

* Update Pull Request Template (apache#2788)

* Freeze 1.2 change log (apache#2783)

* [Catalog Federation] Enable Credential Vending for Passthrough Facade Catalog (apache#2784)

This PR introduces credential vending support for passthrough-facade catalogs.

When creating a passthrough-facade catalog, the configuration currently requires two components:

StorageConfig – specifies the storage info for the remote catalog.
ConnectionInfo – defines connection parameters for the underlying remote catalog.

With this change, the StorageConfig is now also used to vend temporary credentials for user requests.
Credential vending honors table-level RBAC policies to determine whether to issue read-only or read-write credentials, ensuring access control consistency with Polaris authorization semantics.

A new test case validates the credential vending workflow, verifying both read and write credential vending.

Note: the remote catalog referenced by the passthrough-facade does not need to support IRC

* Site: Add docs for catalog federation (apache#2761)

* Python client: update CHANGELOG.MD for recent changes (apache#2796)

* Python client: remove Python 3.9 support (apache#2795)

* Update dependency software.amazon.awssdk:bom to v2.35.5 (apache#2799)

* FIX REG tests with cloud providers (apache#2793)

* [Catalog Federation] Block credential vending for remote tables outside allowed location list (apache#2791)

* Correct invalid example in management service OpenAPI spec (apache#2801)

The `example` was incorrectly placed as a sibling of `$ref` within a `schema` object in `polaris-management-service.yml`. According to the OpenAPI specification, properties that are siblings of a `$ref` are ignored.

This was causing a `NullPointerException` in OpenAPI Generator v7.13.0+ due to a change in how examples are processed. The generator now expects all `examples` to be valid and non-empty, and a misplaced `example` can lead to a null reference when the generator tries to access it (we are not yet using v7.13.0+, thus not a problem at the moment).

This commit moves the `example` to be a sibling of the `schema` object, which is the correct placement according to the OpenAPI specification.

Reference error when using newer version of openapi-generator-cli:
```
openapi-generator-cli generate -i spec/polaris-catalog-service.yaml -g python -o client/python --additional-properties=packageName=polaris.catalog --additional-properties=apiNameSuffix="" --skip-validate-spec --additional-properties=pythonVersion=3.13 --ignore-file-override /local/client/python/.openapi-generator-ignore 
...
  Exception: Cannot invoke "io.swagger.v3.oas.models.examples.Example.getValue()" because the return value of "java.util.Map.get(Object)" is null
	at org.openapitools.codegen.DefaultGenerator.processOperation(DefaultGenerator.java:1606)
	at org.openapitools.codegen.DefaultGenerator.processPaths(DefaultGenerator.java:1474)
	at org.openapitools.codegen.DefaultGenerator.generateApis(DefaultGenerator.java:663)
	at org.openapitools.codegen.DefaultGenerator.generate(DefaultGenerator.java:1296)
	at org.openapitools.codegen.cmd.Generate.execute(Generate.java:535)
	at org.openapitools.codegen.cmd.OpenApiGeneratorCommand.run(OpenApiGeneratorCommand.java:32)
	at org.openapitools.codegen.OpenAPIGenerator.main(OpenAPIGenerator.java:66)
Caused by: java.lang.NullPointerException: Cannot invoke "io.swagger.v3.oas.models.examples.Example.getValue()" because the return value of "java.util.Map.get(Object)" is null
	at org.openapitools.codegen.utils.ExamplesUtils.unaliasExamples(ExamplesUtils.java:75)
	at org.openapitools.codegen.DefaultCodegen.unaliasExamples(DefaultCodegen.java:2343)
	at org.openapitools.codegen.DefaultCodegen.fromResponse(DefaultCodegen.java:4934)
	at org.openapitools.codegen.DefaultCodegen.fromOperation(DefaultCodegen.java:4575)
	at org.openapitools.codegen.DefaultGenerator.processOperation(DefaultGenerator.java:1574)
	... 6 more
```

* Update dependency io.opentelemetry:opentelemetry-bom to v1.55.0 (apache#2804)

* Update dependency io.micrometer:micrometer-bom to v1.15.5 (apache#2806)

* Bump version for python deps (apache#2800)

* bump version for python deps

* bump version for python deps

* bump version for python deps

* Update openapi-generatr-cli from 7.11.0.post0 to 7.12.0

* Pin poetry version

* Pin poetry version

* Update dependency io.projectreactor.netty:reactor-netty-http to v1.2.11 (apache#2809)

* [Catalog Federation] Add Connection Credential Vendors for Other Auth Types (apache#2782)

Add Connection Credential Vendors for Other Auth Types

This change is a prerequisite for enabling connection credential caching.
By making PolarisCredentialManager the central entry point for obtaining connection credentials, we can introduce caching cleanly and manage all credential flows in a consistent way.

* Last merged commit 6b957ec

---------

Co-authored-by: Mend Renovate <bot@renovateapp.com>
Co-authored-by: fabio-rizzo-01 <fabio.rizzocascio@jpmorgan.com>
Co-authored-by: Adnan Hemani <adnan.h@berkeley.edu>
Co-authored-by: Artur Rakhmatulin <artur.rakhmatulin@gmail.com>
Co-authored-by: Alexandre Dutra <adutra@apache.org>
Co-authored-by: Prashant Singh <35593236+singhpk234@users.noreply.github.com>
Co-authored-by: Christopher Lambert <xn137@gmx.de>
Co-authored-by: Dmitri Bourlatchkov <dmitri.bourlatchkov@gmail.com>
Co-authored-by: Honah (Jonas) J. <honahx@apache.org>
Co-authored-by: Rulin Xing <xjdkcsq3@gmail.com>
Co-authored-by: Eric Maynard <eric.maynard+oss@snowflake.com>
Co-authored-by: Travis Bowen <122238243+travis-bowen@users.noreply.github.com>
Co-authored-by: Jaz Ku <jsku@dons.usfca.edu>
Co-authored-by: Yong Zheng <yongzheng0809@gmail.com>
Co-authored-by: JB Onofré <jbonofre@apache.org>
Co-authored-by: Yufei Gu <yufei@apache.org>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants