Skip to content

fix(aws): honor DynamoDB endpoint credentials - #11292

Merged
ReubenBond merged 2 commits into
dotnet:mainfrom
ReubenBond:rb-fix-aws-honor-dynamodb-endpoint-credenti
Sep 17, 2026
Merged

ReubenBond merged 2 commits into
dotnet:mainfrom
ReubenBond:rb-fix-aws-honor-dynamodb-endpoint-credenti

Conversation

@ReubenBond

@ReubenBond ReubenBond commented Sep 17, 2026 •

Copy link
Copy Markdown
Member

Problem

Configuring Service with an HTTP or HTTPS URL currently makes the shared DynamoDB client use dummy credentials, even when access keys, session credentials, or a named profile are supplied. HTTPS endpoints therefore lose the intended AWS authentication, and DynamoDB Local can select a different database from clients using the configured access key.

Solution

Select the endpoint independently from credentials. Explicit access/secret keys, session credentials, and named profiles apply to endpoint URLs as well as AWS regions. HTTP endpoints retain the dummy/dummyKey local-development credentials when explicit keys and a profile are omitted; HTTPS endpoints and regions use the AWS SDK default credential chain in that case.

Align the transaction test client helper with this selection, add standalone tests inspecting the actual SDK client credentials and endpoint configuration, and update the clustering/persistence README recipes and authentication guidance. Profile tests use an isolated credentials file; default-chain tests inspect the selected SDK resolver without resolving ambient credentials or contacting AWS.

Scope and provenance

Extracted from #10798 at ee12e8b08d5fc9a0d514bd54f2e98f908e947847, preserving Reuben Bond's authorship. The shared runtime and transaction helper match that source verbatim. The tests extend its credential cases independently of Aspire, and the README changes retain only programmatic/authentication guidance.

This PR is based on current main (2e40fa8a3ba30a443a9e0332acea4f94733f5afb) and can be merged independently. #10798 should retain this runtime change as a prerequisite for generated HTTP configurations which supply explicit credentials until it can pick up this fix from main. Public API and package dependencies remain unchanged.

Microsoft Reviewers: Open in CodeFlow

Extract endpoint credential selection, coupled transaction test setup, and programmatic guidance from dotnet#10798 at ee12e8b. Preserve the original implementation by Reuben Bond and extend standalone tests to inspect SDK client credentials for HTTP, HTTPS, and AWS regions.

Original runtime commits: c5eae99, 61eefc2, c79e242.
Copilot AI lite review requested due to automatic review settings September 17, 2026 02:41
ReubenBond added a commit to ReubenBond/orleans that referenced this pull request Sep 17, 2026
Move independent NATS producer selection and dimension validation to dotnet#11287 and dotnet#11293. Move standalone redaction, credential, and journaling regression coverage to dotnet#11288, dotnet#11289, dotnet#11291, and dotnet#11292; move the independent DynamoDB README recipes to dotnet#11292.

Retain Token redaction, credential binding and endpoint behavior, journaling activation, and NATS credential-safe logging as prerequisites until dotnet#11288, dotnet#11290, dotnet#11292, dotnet#11289, and dotnet#11291 are merged by humans. The original PR remains focused on Aspire configuration, resource ownership, and integration coverage.

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.

Copilot review overview

🟡 Changes recommended

A critical issue remains because custom endpoints do not set AuthenticationRegion, potentially causing requests to use the wrong signing region.

Get a fresh assessment by requesting another Copilot review.

Review effort: Lite
Findings: 1 High severity

Open (1)
What changed in this PR

Fixes DynamoDB credential selection for custom HTTP/HTTPS endpoints, with expanded tests and documentation updates.

Changes:

  • Separates endpoint selection from credential resolution.
  • Aligns transaction test-client configuration and adds credential/endpoint coverage.
  • Updates DynamoDB clustering and persistence guidance.
File Description
test/​Transactions/​Orleans.Transactions.DynamoDB.Test/​DynamoDBTransactionalStateStorageTests.cs Aligns transaction client credentials.
test/​Extensions/​Orleans.AWS.Tests/​StorageTests/​DynamoDBStorageTestHooks.cs Exposes SDK client details for tests.
test/​Extensions/​Orleans.AWS.Tests/​StorageTests/​DynamoDBStorageCredentialTests.cs Tests credential and endpoint combinations.
src/​AWS/​Shared/​Storage/​DynamoDBStorage.cs Separates endpoint and credential selection.
src/​AWS/​Orleans.Persistence.DynamoDB/​README.md Updates persistence configuration guidance.
src/​AWS/​Orleans.Clustering.DynamoDB/​README.md Updates clustering configuration guidance.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread src/AWS/Shared/Storage/DynamoDBStorage.cs
ReubenBond added a commit to ReubenBond/orleans that referenced this pull request Sep 17, 2026
Move independent NATS producer selection and dimension validation to dotnet#11287 and dotnet#11293. Move standalone redaction, credential, and journaling regression coverage to dotnet#11288, dotnet#11289, dotnet#11291, and dotnet#11292; move the independent DynamoDB README recipes to dotnet#11292.

Retain Token redaction, credential binding and endpoint behavior, journaling activation, and NATS credential-safe logging as prerequisites until dotnet#11288, dotnet#11290, dotnet#11292, dotnet#11289, and dotnet#11291 are merged by humans. The original PR remains focused on Aspire configuration, resource ownership, and integration coverage.
@github-actions

github-actions Bot commented Sep 17, 2026 •

Copy link
Copy Markdown
Contributor

Code coverage

Metric Pull request Current main Variance
Lines 82.13% (112,286 / 136,710) 82.12% (112,272 / 136,709) +0.0096 pp
Branches 71.40% (32,375 / 45,345) 71.32% (32,306 / 45,295) +0.0735 pp

Report-only conclusion: improved.

The current-main baseline is commit 2e40fa8a3b and uses the same reviewed coverage matrix.

Coverage combines every CI test matrix job, including providers, CodeGen, .NET 8/10, Linux, Windows, and macOS, using canonical physical source and branch identities.

The comparison remains report-only while normal line and branch variance is calibrated.

Coverage details

Exercise the actual SDK signing pipeline through an in-memory HTTP transport with basic, session, and named-profile credentials. Cover regional AWS URLs, China and GovCloud endpoints, and existing region/custom endpoint behavior.

The SDK already derives the signing region for regional endpoint URLs when AuthenticationRegion is unset; keep production credential and transaction-helper behavior unchanged.
Copilot AI review requested due to automatic review settings September 17, 2026 03:38

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.

Copilot review overview

🔵 Needs a closer look

Regional endpoint authentication regions remain unset, which may cause requests to be signed for the wrong region.

Review effort: Lite
Findings: 1 High severity

Open (1)

ReubenBond added a commit to ReubenBond/orleans that referenced this pull request Sep 17, 2026
Move independent NATS producer selection and dimension validation to dotnet#11287 and dotnet#11293. Move standalone redaction, credential, and journaling regression coverage to dotnet#11288, dotnet#11289, dotnet#11291, and dotnet#11292; move the independent DynamoDB README recipes to dotnet#11292.

Retain Token redaction, credential binding and endpoint behavior, journaling activation, and NATS credential-safe logging as prerequisites until dotnet#11288, dotnet#11290, dotnet#11292, dotnet#11289, and dotnet#11291 are merged by humans. The original PR remains focused on Aspire configuration, resource ownership, and integration coverage.
@ReubenBond
ReubenBond requested a balanced review from Copilot September 17, 2026 14:26

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.

Copilot review overview

🟢 Approval recommended

The implementation consistently preserves credentials across regions and endpoints with comprehensive focused regression coverage.

Review effort: Balanced
Findings: None

Resolved since last review (1)

ReubenBond added a commit to ReubenBond/orleans that referenced this pull request Sep 17, 2026
Move independent NATS producer selection and dimension validation to dotnet#11287 and dotnet#11293. Move standalone redaction, credential, and journaling regression coverage to dotnet#11288, dotnet#11289, dotnet#11291, and dotnet#11292; move the independent DynamoDB README recipes to dotnet#11292.

Retain Token redaction, credential binding and endpoint behavior, journaling activation, and NATS credential-safe logging as prerequisites until dotnet#11288, dotnet#11290, dotnet#11292, dotnet#11289, and dotnet#11291 are merged by humans. The original PR remains focused on Aspire configuration, resource ownership, and integration coverage.
@ReubenBond
ReubenBond merged commit 0c3aee0 into dotnet:main Sep 17, 2026
73 checks passed
@ReubenBond
ReubenBond deleted the rb-fix-aws-honor-dynamodb-endpoint-credenti branch September 17, 2026 15:14
ReubenBond added a commit to ReubenBond/orleans that referenced this pull request Sep 17, 2026
Move independent NATS producer selection and dimension validation to dotnet#11287 and dotnet#11293. Move standalone redaction, credential, and journaling regression coverage to dotnet#11288, dotnet#11289, dotnet#11291, and dotnet#11292; move the independent DynamoDB README recipes to dotnet#11292.

Retain Token redaction, credential binding and endpoint behavior, journaling activation, and NATS credential-safe logging as prerequisites until dotnet#11288, dotnet#11290, dotnet#11292, dotnet#11289, and dotnet#11291 are merged by humans. The original PR remains focused on Aspire configuration, resource ownership, and integration coverage.
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.

2 participants