fix(extensions): tolerate ClientFactory<T> constructor change in AWS SDK 4.0.4+ (#52) - #53
Draft
Blind-Striker wants to merge 6 commits into
Draft
fix(extensions): tolerate ClientFactory<T> constructor change in AWS SDK 4.0.4+ (#52)#53Blind-Striker wants to merge 6 commits into
Blind-Striker wants to merge 6 commits into
Conversation
Updates AWSSDK.Core to 4.0.100.6 and AWSSDK.Extensions.NETCore.Setup to 4.0.100.5 (near-latest, well-adopted, both published 2026-07-17), 120 AWSSDK service packages, Microsoft.Extensions.* to 10.0.10, and the analyzer/test toolchain to latest. Resolves NU1901 (GHSA-9cvc-h2w8-phrp) on AWSSDK.Core 4.0.0.15, which was failing restore outright because TreatWarningsAsErrors promotes the advisory to an error. The previous pin was 403 days old. Adds the $(AwsSetupTrack) switch to Directory.Packages.props so the whole package graph can move between AWSSDK.Extensions.NETCore.Setup shapes at once: current (default) - post-4.0.4 ClientFactory<T>(AWSOptions, Action<...>) legacy - pre-4.0.4 ClientFactory<T>(AWSOptions) latest - floating, scheduled canary only A per-project VersionOverride does not work here: src and tests must move together or NuGet reports NU1605. Floating versions are opt-in under CPM (NU1011), so they are enabled only on the canary track. Exposed through Cake as --aws-setup-track. Two upstream behaviour changes required source adaptations, none in src/: - Testcontainers 4.13.0 obsoleted the parameterless LocalStackBuilder() ctor. - AWSSDK.Core moved constructor-supplied credentials from Config.DefaultAWSCredentials to the new protected internal ExplicitAWSCredentials, leaving the old property null. Caught immediately by the existing unit tests. New analyzer diagnostics fixed rather than suppressed, except CA1873 which joins the CA1848/CA2254 logging-performance opt-out already present in those test projects. Note S8969 flagged eight null-forgiving operators of which seven were NOT redundant - they masked CS8620 nullability variance; the correct fix was Dictionary<string, string?>. Also updates global.json to SDK 10.0.302 and the LICENSE copyright range to 2026.
….4+ (#52) AWSSDK.Extensions.NETCore.Setup 4.0.4 (2026-05-20) changed the internal ClientFactory<T> constructor: <= 4.0.3.40 ClientFactory(AWSOptions awsOptions) >= 4.0.4 ClientFactory(AWSOptions awsOptions, Action<ClientConfig, IServiceProvider> configAction = null) The new parameter is optional, so this was source-compatible for AWS and shipped as a patch-level bump with nothing in the release notes. It is binary-breaking for our exact-signature reflection lookup, which returned null and threw LocalStackClientConfigurationException. Only the UseLocalStack:false path was affected, which is why it went unreported for ~70 days while consumers floated past 4.0.4 (our nuspec declares [4.0.2, )) and CI stayed pinned at 4.0.2. AwsClientFactoryWrapper now matches on the AWSOptions parameter rather than the full signature, prefers the fewest-parameter overload so the choice stays deterministic if AWS appends more optional parameters, and defaults any trailing arguments. Passing null for configAction is what the SDK itself does - the parameter is declared = null and null-guarded before use. Requiring AWSOptions in first position also excludes the private parameterless ctor present in every version. Failure messages now include the constructor signatures actually discovered, so the next drift is diagnosable from a bug report instead of needing a version bisect. Tests model both known shapes - plus a shape AWS has not shipped - with local stand-ins, because a test bound to the real SDK can only ever see the pinned version. The real-SDK check is deliberately shape-agnostic so it passes on either track. Verified green against 4.0.3.40, 4.0.100.5 and 4.0.100.7 (the reporter's version). Public API unchanged; the resolution helpers are internal via InternalsVisibleTo.
Updates every action to latest: checkout v4->v7, cache v4->v6, setup-dotnet v4->v6, upload-artifact v4->v7, dependency-review-action v4->v5, dorny/test-reporter v1->v3. setup-dotnet now also reads global-json-file so the SDK matches the repo pin, while the explicit 8.0.x/9.0.x entries are kept because the multi-TFM test run needs those runtimes present - the .NET 10 SDK alone cannot execute net8.0/net9.0 test assemblies. No setup-node pinning: nothing in the workflows uses setup-node, and update-test-badge is a composite action running bash steps. The JS action runtimes come from the actions themselves, so bumping them is what moves Node forward. Adds a Linux leg running the pre-4.0.4 AWS SDK track, so backwards compatibility is proven on every push rather than assumed. Adds a scheduled canary that floats AWSSDK.Extensions.NETCore.Setup to the newest 4.x and runs the SDK-sensitive tests against it, deliberately without NuGet caching. The pinned build is reproducible by design, which is exactly why the 4.0.4 break went unnoticed for 70 days - CI only ever saw the pinned version while consumers floated. A failure there means "AWS moved", not "we broke something".
Dependency ReviewThe following issues were found:
License Issues.github/workflows/dependency-review.yml
OpenSSF Scorecard
Scanned Files
|
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
Comment on lines
+25
to
+65
| name: "Latest AWS SDK" | ||
| runs-on: ubuntu-22.04 | ||
| if: github.repository == 'localstack-dotnet/localstack-dotnet-client' | ||
|
|
||
| steps: | ||
| - name: "Checkout" | ||
| uses: actions/checkout@v7 | ||
|
|
||
| - name: "Setup .NET SDK" | ||
| uses: actions/setup-dotnet@v6 | ||
| with: | ||
| dotnet-version: | | ||
| 8.0.x | ||
| 9.0.x | ||
| global-json-file: global.json | ||
|
|
||
| - name: "Make build script executable" | ||
| run: chmod +x ./build.sh | ||
|
|
||
| # Deliberately no NuGet cache: the point is to resolve the newest AWSSDK packages every run. | ||
| - name: "Build & test against floating AWS SDK" | ||
| run: ./build.sh --target tests --skipFunctionalTest true --aws-setup-track latest --force-restore true --exclusive | ||
|
|
||
| - name: "Report resolved AWS SDK versions" | ||
| if: always() | ||
| run: | | ||
| echo "### Resolved AWS SDK versions" >> $GITHUB_STEP_SUMMARY | ||
| echo '```' >> $GITHUB_STEP_SUMMARY | ||
| grep -rhoE '"AWSSDK\.(Extensions\.NETCore\.Setup|Core)/[^"]+"' \ | ||
| tests/*/obj/project.assets.json 2>/dev/null | sort -u >> $GITHUB_STEP_SUMMARY || true | ||
| echo '```' >> $GITHUB_STEP_SUMMARY | ||
|
|
||
| - name: "Upload test artifacts" | ||
| uses: actions/upload-artifact@v7 | ||
| if: failure() | ||
| with: | ||
| name: canary-test-results | ||
| path: | | ||
| **/*.trx | ||
| **/TestResults/**/* | ||
| retention-days: 7 |
The macos-latest image no longer ships Mono, so VSTest aborts the net472 run with "Could not find 'mono' host". Linux is unaffected - ubuntu-22.04 still has it. Guarded by a presence check so the step becomes a no-op if a future image brings Mono back.
…ainers
Scenario classes in a collection share one LocalStack container, so anything a test
leaves behind is visible to every test that runs afterwards - including in other
classes. That made assertions about container state order dependent:
Multi_Region_Tests_Async("eu-central-1") asserts Assert.Single over ListTopics and
failed once another class left a topic alive. The same test passed or failed
depending on which subset was run, which is the definition of a non-isolated test.
The assertion is deliberately left untouched. Asserting more than strictly necessary
is what surfaced the leak in the first place; weakening it would hide the defect
rather than fix it.
Cleanup previously sat at the end of the test body, so it never ran when an assertion
threw - one red test would poison every test after it. BaseScenario now implements
IAsyncLifetime: xUnit builds a fresh instance per test method and awaits DisposeAsync
regardless of outcome, which is exactly per-test teardown.
TrackForCleanup(resourceId, cleanup) - registers a resource for removal
UntrackCleanup(resourceId) - drops it when deletion is the behaviour under test
Teardown runs in reverse registration order, so a subscription goes before the topic
it depends on, and reports failures via AggregateException instead of swallowing them -
a leaked resource is a defect and should be loud.
Applied across every scenario that creates resources:
- BaseRealLife created a topic, a queue and a subscription and deleted none of them.
- BaseCloudFormationScenario never deleted its stack, so the template's ChatTopic
survived. Deletion now waits for DELETE_COMPLETE, because DeleteStack returns before
the resources it owns are actually gone.
- SNS/SQS/S3/DynamoDB create helpers register cleanup; delete helpers de-register.
- S3 buckets are emptied via AmazonS3Util.DeleteS3BucketWithObjectsAsync, since S3
refuses to drop a non-empty bucket and these scenarios upload into it.
Verified: full functional suite 50/50 on net8.0 and net9.0, and the previously
order-dependent subsets (~SNS, ~Scenarios.SNS, ~CloudFormation) now agree.
Two defects in the dynamic version generated for nightly/feature packages.
1. Invalid SemVer between 00:00 and 09:59 UTC.
The commit-SHA fallback emits DateTime.UtcNow.ToString("HHmmss"), so a build at
07:58 produced "2.0.0-<branch>.20260729.075853". SemVer 2.0.0 forbids leading
zeroes in numeric pre-release identifiers and NuGet enforces it, so dotnet pack
failed with "is not a valid version string" - purely as a function of the time of
day. Master's last successful deploy ran at 10:38 UTC, which is why this never
surfaced. Numeric identifiers with a leading zero are now prefixed, keeping them
alphanumeric, which SemVer permits. A git short SHA that happens to be all digits
hits the same trap and is covered by the same guard.
2. The commit SHA was never actually captured.
Cake's StartProcess returned exit code 0 with empty stdout, so every build silently
fell through to the timestamp branch and no published package has ever identified
its commit. Reading the process directly makes the capture explicit and independent
of Cake's redirection behaviour, and the fallback now logs a warning instead of
passing silently.
Verified locally: dotnet pack now produces 2.0.0-test.20260729.c3176a1, matching
git rev-parse --short HEAD.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
📝 Description
What does this PR do?
Fixes the
LocalStackClientConfigurationExceptionthrown whenUseLocalStackisfalse, refreshes the dependency graph, and adds the missing safety net that would have caught this class of problem.Root cause.
AWSSDK.Extensions.NETCore.Setup4.0.4 (published 2026-05-20) changed the internalClientFactory<T>constructor:The added parameter is optional, so from AWS's side this is source-compatible and shipped as a patch-level bump with nothing in the release notes. It is binary-breaking for our exact-signature reflection lookup in
AwsClientFactoryWrapper, which returnednulland threw.Three things kept it quiet for ~70 days:
UseLocalStack: falsepath goes through this wrapper — LocalStack-only users never hit it.[4.0.2, ), so consumers float to the newest version and break.4.0.2exactly, so the pipeline only ever saw the one version that still worked.The existing test
CreateServiceClient_Should_Create_Client_When_UseLocalStack_Falsewas always correct — it goes red the moment the pin moves past 4.0.4. The suite was never the gap; the pin was.The fix.
AwsClientFactoryWrappernow matches on theAWSOptionsparameter rather than the full signature, prefers the fewest-parameter overload so the choice stays deterministic if AWS appends more optional parameters, and defaults any trailing arguments. PassingnullforconfigActionis not a workaround — it is exactly what the SDK itself passes (= null, null-guarded before use). RequiringAWSOptionsin first position also naturally excludes the private parameterless ctor present in every version.Failure messages now include the constructor signatures actually discovered, so the next drift is diagnosable from a bug report instead of requiring a version bisect.
Related Issue(s):
🔄 Type of Change
🎯 Target Version Track
sdkv3-lts)master)🧪 Testing
How has this been tested?
Functional/container tests were not run locally (Docker); they run on the Linux CI leg, which is why this is opened as a draft — letting the pipeline exercise everything.
Reproduced the original failure first using the reporter's own repro, then verified the fix against all three SDK shapes:
AWSSDK.Extensions.NETCore.Setupcurrent(default)legacylatest(canary)Full matrix on the default track: 1094 tests passing, 0 warnings, 0 errors.
New tests model both known constructor shapes — plus a shape AWS has not shipped — with local stand-ins, because a test bound to the real SDK can only ever see whichever version is pinned. The real-SDK check is deliberately shape-agnostic so it passes on either track.
Test Environment:
📚 Documentation
✅ Code Quality Checklist
🔍 Additional Notes
Breaking Changes:
None. Public API is unchanged; the new resolution helpers are
internal, exposed to tests viaInternalsVisibleTo.Performance Impact:
Negligible. Constructor selection is a one-time reflection scan per service registration, replacing a single
GetConstructorcall with a short loop over the same candidate set.Dependencies:
Refreshed the whole graph.
AWSSDK.Core→ 4.0.100.6 andAWSSDK.Extensions.NETCore.Setup→ 4.0.100.5 (near-latest, well-adopted, both published 2026-07-17 — a coherent pair), plus 120 AWSSDK service packages,Microsoft.Extensions.*→ 10.0.10, and the analyzer/test toolchain to latest.This also resolves NU1901 (GHSA-9cvc-h2w8-phrp) on
AWSSDK.Core4.0.0.15, which was failing restore outright becauseTreatWarningsAsErrorspromotes the advisory to an error. The previous pin was 403 days old.Two upstream behaviour changes required source adaptations — none in
src/:LocalStackBuilder()ctor (CS0618 → error).Config.DefaultAWSCredentialsto the newprotected internal ExplicitAWSCredentials, leaving the old propertynull. Caught immediately by the existing unit tests.Also:
global.json→ SDK 10.0.302, LICENSE copyright range → 2026.Preventing recurrence — the
AwsSetupTrackswitch:A new MSBuild property in
Directory.Packages.propsmoves the entire package graph between SDK shapes:Exposed through Cake as
--aws-setup-track, with a Linux CI leg onlegacyand a scheduled canary that floats to the newest AWS SDK (deliberately without NuGet caching). A canary failure means "AWS moved", not "we broke something".Two constraints worth knowing for review:
VersionOverridedoes not work here —srcand tests must move together or NuGet reports NU1605 (package downgrade), which is an error underTreatWarningsAsErrors. Hence a central switch.CentralPackageFloatingVersionsEnabledis enabled only on the canary track.currentandlegacystay strictly reproducible.CI/CD modernisation:
Every action bumped:
checkoutv4→v7,cachev4→v6,setup-dotnetv4→v6,upload-artifactv4→v7,dependency-review-actionv4→v5,dorny/test-reporterv1→v3.setup-dotnetnow also readsglobal-json-file, while the explicit8.0.x/9.0.xentries are kept on purpose — the .NET 10 SDK alone cannot execute net8.0/net9.0 test assemblies, so dropping them would break the multi-TFM run.No
setup-nodepinning was added: nothing in the workflows uses it, andupdate-test-badgeis a composite action running bash steps. The JS action runtimes come from the actions themselves, so bumping them is what moves Node forward.🎯 Reviewer Focus Areas
Please pay special attention to:
Specifically worth a second pair of eyes:
SelectFactoryConstructortie-breaking — fewest-parameter-wins. Reasonable today, but it is the rule that decides behaviour if AWS ever ships twoAWSOptions-first overloads.S8969fixes. SonarAnalyzer flagged 8 null-forgiving operators as redundant; 7 were not — they were maskingCS8620nullability variance. Following the analyzer literally broke the build. The correct fix wasDictionary<string, string?>. Worth confirming you agree with that reading.CA1873was added to the existingCA1848/CA2254logging-performance opt-out in the test/sandbox projects rather than restructuring demo logging calls.src/has zero occurrences.Known follow-ups (deliberately not in this PR):
PackageExtensionVersionbump 2.0.0 → 2.0.1 — a release decision.LocalStack.Client.Functional.Tests(AutoFixture 4.18.1→Fare→NETStandard.Library 1.6.1→System.Net.Http 4.3.0/System.Text.RegularExpressions 4.3.0). Test-only, never shipped, and AutoFixture 4.18.1 is already latest so it cannot be fixed by upgrading. Note these are invisible todotnet buildbecauseNuGetAuditModedefaults todirect.AWSOptions.CreateServiceClient<T>()is public and has existed in every release since3.2.8-rc(2016-09-08). It would remove this reflection entirely — and with it the[RequiresDynamicCode]/[RequiresUnreferencedCode]annotations andILLink.Descriptors.xml— but it has noIServiceProvideroverload, so theAWSOptionsfallback chain and logger wiring would need reimplementing. Tracked for 2.1 alongside the AOT work.📸 Screenshots/Examples
The failure this fixes, from the original report:
Unchanged user code — this now works on both old and new AWS SDK versions:
Running the suite against the older SDK shape:
./build.sh --target tests --aws-setup-track legacy --force-restore trueBy submitting this pull request, I confirm that: