Skip to content

Cache storage credentials independently of refresh endpoint - #5231

Open
iting0321 wants to merge 1 commit into
apache:mainfrom
iting0321:fix/cache-storage-credentials-independently-of-refresh-endpoint
Open

Cache storage credentials independently of refresh endpoint#5231
iting0321 wants to merge 1 commit into
apache:mainfrom
iting0321:fix/cache-storage-credentials-independently-of-refresh-endpoint

Conversation

@iting0321

Copy link
Copy Markdown
Contributor

Summary

Fixes #3292

A single loadTable request can request the same scoped storage credentials twice:

  1. Polaris loads table metadata using server-side FileIO, without a refresh credentials endpoint.
  2. Polaris builds the client response, with a refresh credentials endpoint.

Although the underlying credential vending inputs are otherwise identical, refreshCredentialsEndpoint was part of StorageCredentialCacheKey. This caused the second lookup to miss the cache and, for AWS storage, resulted in a second STS AssumeRole request.

Changes

This change treats the refresh endpoint as request specific response metadata rather than credential identity:

  • Removes refreshCredentialsEndpoint from StorageCredentialCacheKey.
  • Generates and caches an endpoint neutral StorageAccessConfig.
  • Attach the provider specific S3, GCS, or Azure refresh endpoint to each response.
  • Fail fast when the storage configuration needed to determine the provider is missing.

This preserves correct endpoint behavior across requests while allowing identical scoped credentials to reuse one cache entry. It also prevents one request's refresh endpoint from leaking into another response.

Checklist

  • 🛡️ Don't disclose security issues! (contact security@apache.org)
  • 🔗 Clearly explained why the changes are needed, or linked related issues: Fixes #
  • 🧪 Added/updated tests with good coverage, or manually tested (and explained how)
  • 💡 Added comments for complex logic
  • 🧾 Updated CHANGELOG.md (if needed)
  • 📚 Updated documentation in site/content/in-dev/unreleased (if needed)

Copilot AI review requested due to automatic review settings August 3, 2026 09:48
@github-project-automation github-project-automation Bot moved this to PRs In Progress in Basic Kanban Board Aug 3, 2026
@iting0321
iting0321 force-pushed the fix/cache-storage-credentials-independently-of-refresh-endpoint branch from 320e3a4 to e9f8a2f Compare August 3, 2026 09:59

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Note

Copilot couldn't run its full agentic review because it didn't start before the timeout. Make sure your repository has a runner available, or add a copilot-code-review.yml file specifying one with the runs-on attribute. See the docs for more details.

This PR changes the storage credential caching behavior so that scoped credentials are cached independently of the per-request refresh credentials endpoint, avoiding duplicate credential-vending calls (e.g., AWS STS AssumeRole) within a single request.

Changes:

  • Removes the refresh endpoint from the credential cache identity by always building cache keys without it.
  • Attaches provider-specific refresh endpoint metadata to the returned StorageAccessConfig per response.
  • Adds an integration test to verify one cache load across varying refresh endpoints and correct endpoint attachment.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.

File Description
polaris-core/src/main/java/org/apache/polaris/core/storage/CachingStorageIntegration.java Decouples cache key construction from refresh endpoint and injects refresh endpoint metadata into the response config.
polaris-core/src/test/java/org/apache/polaris/core/storage/CachingStorageIntegrationTest.java Adds coverage to ensure credentials are cached once while refresh endpoints remain request-specific.

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.

One load table API request results in two STS AssumeRole requests

2 participants