fix(lance-io): request vended credentials explicitly in namespace provider - #7978
fix(lance-io): request vended credentials explicitly in namespace provider#7978rpgreen wants to merge 1 commit into
Conversation
…vider `LanceNamespaceStorageOptionsProvider` fetches storage options precisely because it needs credentials to open a namespace-backed dataset, yet it left `vend_credentials` unset. Per the lance-namespace spec, an unset flag lets the server decide whether to vend, and some implementations only vend when asked. When such a server returns no credentials, `build_aws_credential` falls through to the AWS SDK `DefaultCredentialsChain`, which fails in environments without ambient credentials -- even though the namespace could have vended scoped ones. Set `vend_credentials: Some(true)` so the provider always opts in explicitly. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
github-actions
Bot
added
A-encoding
📝 WalkthroughWalkthroughThe namespace storage options provider now explicitly requests credential vending when issuing a table description request. ChangesNamespace storage credentials
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@rust/lance-io/src/object_store/storage_options.rs`:
- Around line 141-142: Add a mock-client regression test covering the
storage-options path that builds DescribeTableRequest, capture the outgoing
request, and assert its vend_credentials field is Some(true). Keep the test
focused on explicit credential vending and ensure it guards against fallback to
ambient AWS credentials.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: QUIET
Plan: Pro Plus
Run ID: cb567c80-b29a-45d0-91ab-f2c008e475f6
📒 Files selected for processing (1)
rust/lance-io/src/object_store/storage_options.rs
| // Some server implementations may not return credentials unless explicitly requested | ||
| vend_credentials: Some(true), |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Add a regression test for explicit credential vending.
Capture the outgoing DescribeTableRequest in a mock-client test and assert vend_credentials == Some(true). This prevents a future change from silently reintroducing ambient AWS credential fallback.
As per coding guidelines: “Every bugfix and feature must have corresponding tests.”
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@rust/lance-io/src/object_store/storage_options.rs` around lines 141 - 142,
Add a mock-client regression test covering the storage-options path that builds
DescribeTableRequest, capture the outgoing request, and assert its
vend_credentials field is Some(true). Keep the test focused on explicit
credential vending and ensure it guards against fallback to ambient AWS
credentials.
Source: Coding guidelines
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
westonpace
left a comment
westonpace
left a comment
There was a problem hiding this comment.
In what cases is the field ever set to false? Just when it is used for the initial load?
2 participants
What
LanceNamespaceStorageOptionsProvider::fetch_storage_optionsnow sendsvend_credentials: Some(true)in itsdescribe_tablerequest.Why
This provider is invoked specifically to obtain credentials for opening a namespace-backed dataset, yet it left
vend_credentialsunset. Per the lance-namespace spec, an unset flag lets the server decide whether to vend, and some server implementations only vend credentials when explicitly asked.When such a server returns no credentials,
build_aws_credential(lance-io/src/object_store/providers/aws.rs) falls through its resolution chain to the AWS SDKDefaultCredentialsChain. In an environment without ambient AWS credentials this fails (e.g.Failed to get AWS credentials: ... FailedToLoadToken), even though the namespace could have vended scoped credentials.Explicitly requesting vending makes the client's intent unambiguous and avoids the silent fall-through to the default chain.
Impact
Low risk. The provider is only used when opening namespace-backed datasets that need credentials. Servers that already vend on an unset flag are unaffected (they vend either way); servers that gate on the flag now correctly vend.
🤖 Generated with Claude Code