fix(extensions): tolerate ClientFactory<T> constructor change in AWS SDK 4.0.4+ (#52) - #53
Merged
Merged
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
build/LocalStack.Build/LocalStack.Build.csproj
OpenSSF Scorecard
Scanned Files
|
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
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.
upload-artifact rejects '/' in artifact names, and the deploy job only ever runs on
master pushes or feature/* pull requests - so every feature branch deploy failed at
the upload step with "The artifact name is not valid ... Contains the following
character: Forward slash /", after the packages had already been published.
The branch name is now sanitised into SAFE_REF first, mirroring the Replace('/', '-')
that BuildContext already applies when composing the package version.
The pre-4.0.4 compatibility run shared the build-and-test job, which broke reporting.
TestTask names its TRX by target framework alone (net8-0_results.trx), so the second
pass overwrote the first and dorny/test-reporter published the legacy results instead
of the real ones - without the functional tests, which that leg skips. The badge took
the same wrong numbers.
Moved to its own sdk-compat job: separate workspace, no TRX collision, and no test
report at all, since these results are a compatibility gate rather than the suite's
outcome. Deployment now waits on it as well as build-and-test.
.mcp.json is per-developer configuration - it pins a machine-local Rider SSE port and
personal tooling unrelated to building this library - so it follows the convention the
repo already applies to .idea/, *.user and Properties/launchSettings.json. It stays on
disk, only untracked. No history rewrite: the file carries no secret, the GitHub token
is an ${ENV_VAR} reference.
The nuspec floor is deliberately raised to AWSSDK.Extensions.NETCore.Setup 4.0.100.5, so proving the pre-4.0.4 constructor shape still works tests a configuration no consumer can install. Drop the `legacy` track, the `sdk-compat` CI job and the `SdkCompat` traits that only existed to filter it, and trim the resolution tests to forward-drift coverage. A NuGet floor is a minimum, so consumers still float upward and will meet the next AWS change before the pinned build does. The canary is what catches that, so it now floats AWSSDK.Core as well - Core's move of constructor credentials to ExplicitAWSCredentials already bit us once, and the canary summary was printing a version that could never change. The canary had a second, quieter problem: --force-restore only means "do restore", it does not bypass NuGet's HTTP cache, and floating versions resolve against the cached feed index. Measured locally it resolved Setup 4.0.100.5 instead of 4.0.100.7 - reporting "AWS has not moved" while AWS had. Clear the HTTP cache before the run. Also: - Move $(AwsSetupTrack) and CentralPackageFloatingVersionsEnabled to Directory.Build.props; Directory.Packages.props keeps only conditional PackageVersion items. - Validate generated package versions through NuGet.Versioning instead of a hand-rolled SemVer rule. NuGetVersion decides whether a pre-release identifier is legal, the branch name is sanitised to [0-9A-Za-z-], and the composed version is parsed as a backstop so an invalid version fails the build with the offending value rather than at pack time. - Carry build.sh's executable bit in the index, removing five `chmod +x` steps across three workflows. - Give every workflow an explicit permissions block, resolving the CodeQL actions/missing-workflow-permissions alerts. build-and-test needs checks: write because dorny/test-reporter publishes a check run; the badge step uses its own PAT. - Pin global.json to SDK 9.0.316 and Microsoft.Extensions.*/System.Text.Json to 9.0.18. The highest TFM in the repo is net9.0 and there is no net10.0 target anywhere. Microsoft.SourceLink.GitHub stays on 10.0.301 - it has no 9.x release. Every Microsoft.Extensions.* reference in src/ carries an explicit VersionOverride, so no shipped dependency floor moves.
PackageExtensionVersion 2.0.0 -> 2.0.1. PackageMainVersion stays at 2.0.0: LocalStack.Client has no source change, and the advisory on its AWSSDK.Core >= 4.0.0.15 floor is unreachable in practice because AWSSDK.Extensions.NETCore.Setup 4.0.100.5 declares AWSSDK.Core >= 4.0.100.6, so NuGet lifts it anyway. PackageValidationBaselineVersion moves off the stale v1 baselines (1.4.1 / 1.2.2) to 2.0.0 for both projects. That makes every entry in LocalStack.Client's CompatibilitySuppressions.xml dead - they compare lib/net461, lib/net6.0 and lib/net7.0, target frameworks that only existed in the v1.4.1 package - and package validation fails the build while they remain, so the file goes. The CHANGELOG entry is rewritten as terse bullets matching the v2.0.0 section. The previous draft claimed a Microsoft.Extensions.* bump that consumers never see: every such reference in src/ carries an explicit VersionOverride, so the only consumer-visible dependency change is the AWSSDK.Extensions.NETCore.Setup floor.
There was a problem hiding this comment.
Pull request overview
This PR hardens LocalStack.Client.Extensions against AWS SDK v4 internal ClientFactory<T> constructor drift (4.0.4+), refreshes dependencies, and adds CI/build switches to continuously validate compatibility across multiple AWS SDK “shapes” (legacy/current/latest canary). It also improves functional-test isolation by introducing centralized per-test resource cleanup and updating sandboxes/tests for upstream dependency API changes.
Changes:
- Make
AwsClientFactoryWrapperselectClientFactory<T>constructors byAWSOptions-first matching (vs exact signature), defaulting trailing args and improving diagnostics; add SDK-shape resolution tests. - Introduce
AwsSetupTrack(current/legacy/latest) in CPM and propagate it through Cake + CI (including a scheduled canary workflow). - Refresh dependency graph and update tests/sandboxes (Testcontainers API changes, AWS SDK Core credential behavior, functional scenario teardown).
Reviewed changes
Copilot reviewed 41 out of 42 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/sandboxes/LocalStack.Container/Program.cs | Update Testcontainers LocalStack builder usage. |
| tests/sandboxes/LocalStack.Client.Sandbox/Program.cs | Adjust S3 bucket location access for newer SDK types. |
| tests/sandboxes/LocalStack.Client.Sandbox.WithGenericHost/SampleS3Service.cs | Adjust S3 bucket location access for newer SDK types. |
| tests/sandboxes/LocalStack.Client.Sandbox.WithGenericHost/LocalStack.Client.Sandbox.WithGenericHost.csproj | Add analyzer suppression for logging perf rule. |
| tests/sandboxes/LocalStack.Client.Sandbox.DependencyInjection/Program.cs | Adjust S3 bucket location access for newer SDK types. |
| tests/LocalStack.Client.Functional.Tests/TestContainers.cs | Update LocalStack builder construction for Testcontainers. |
| tests/LocalStack.Client.Functional.Tests/Scenarios/SQS/BaseSqsScenario.cs | Register created queues for centralized cleanup; untrack on explicit delete. |
| tests/LocalStack.Client.Functional.Tests/Scenarios/SNS/BaseSnsScenario.cs | Register created topics for centralized cleanup; untrack on explicit delete. |
| tests/LocalStack.Client.Functional.Tests/Scenarios/S3/BaseS3Scenario.cs | Register buckets for cleanup (including non-empty buckets); untrack on explicit delete. |
| tests/LocalStack.Client.Functional.Tests/Scenarios/RealLife/BaseRealLife.cs | Register SNS/SQS/subscription resources for cleanup ordering. |
| tests/LocalStack.Client.Functional.Tests/Scenarios/DynamoDb/BaseDynamoDbScenario.cs | Register DynamoDB tables for centralized cleanup; untrack on explicit delete. |
| tests/LocalStack.Client.Functional.Tests/Scenarios/CloudFormation/BaseCloudFormationScenario.cs | Track stacks for cleanup and wait for deletion completion. |
| tests/LocalStack.Client.Functional.Tests/Scenarios/BaseScenario.cs | Introduce per-test async teardown with tracked cleanup registrations. |
| tests/LocalStack.Client.Functional.Tests/LocalStack.Client.Functional.Tests.csproj | Add analyzer suppression for logging perf rule. |
| tests/LocalStack.Client.Functional.Tests/GlobalUsings.cs | Add S3 util global using for bucket-with-objects deletion helper. |
| tests/LocalStack.Client.Functional.Tests/Fixtures/TestFixture.cs | Fix nullability variance for in-memory configuration values. |
| tests/LocalStack.Client.Functional.Tests/CloudFormation/CloudFormationProvisioner.cs | Remove null-forgiving and adapt to updated nullability expectations. |
| tests/LocalStack.Client.Extensions.Tests/ServiceCollectionExtensionsTests.cs | Fix nullability variance for in-memory configuration values. |
| tests/LocalStack.Client.Extensions.Tests/AwsClientFactoryWrapperTests.cs | Add trait tagging for SDK-compat coverage. |
| tests/LocalStack.Client.Extensions.Tests/AwsClientFactoryWrapperResolutionTests.cs | New tests modeling multiple ClientFactory<T> constructor shapes. |
| tests/common/LocalStack.Tests.Common/Mocks/MockServiceClients/MockAmazonServiceClient.cs | Update credential exposure to match AWSSDK.Core >= 4.0.100 behavior. |
| src/LocalStack.Client/README.md | Add Discussions/community section. |
| src/LocalStack.Client/LICENSE.txt | Update copyright year. |
| src/LocalStack.Client.Extensions/README.md | Add Discussions/community section. |
| src/LocalStack.Client.Extensions/LocalStack.Client.Extensions.csproj | Expose internals to tests for constructor-resolution verification. |
| src/LocalStack.Client.Extensions/LICENSE.txt | Update copyright year. |
| src/LocalStack.Client.Extensions/GlobalUsings.cs | Add System.Text for constructor signature formatting. |
| src/LocalStack.Client.Extensions/AwsClientFactoryWrapper.cs | Implement constructor selection + argument building + improved diagnostics. |
| LICENSE | Update copyright year. |
| global.json | Bump pinned .NET SDK version. |
| Directory.Packages.props | Add AwsSetupTrack switch and refresh package versions across the graph. |
| CHANGELOG.md | Add unreleased entry documenting the fix + dependency/CI updates. |
| build/LocalStack.Build/GlobalUsings.cs | Add suppressions/attributes support for build tooling changes. |
| build/LocalStack.Build/CakeTasks/TestTask.cs | Ensure dotnet test uses the same AwsSetupTrack as build. |
| build/LocalStack.Build/CakeTasks/Nuget/Services/PackageOperations.cs | Add regex timeout to avoid potential pathological matches. |
| build/LocalStack.Build/CakeTasks/BuildTask.cs | Propagate AwsSetupTrack into MSBuild and log selected track. |
| build/LocalStack.Build/BuildContext.cs | Add aws-setup-track argument and harden SemVer pre-release identifier generation. |
| .gitignore | Ignore per-developer .mcp.json. |
| .github/workflows/publish-nuget.yml | Bump action versions and align setup-dotnet with global.json. |
| .github/workflows/dependency-review.yml | Bump action versions. |
| .github/workflows/ci-cd.yml | Bump action versions, install Mono on macOS, add legacy SDK compat job, sanitize artifact names. |
| .github/workflows/aws-sdk-canary.yml | New scheduled canary workflow floating AWS SDK versions. |
The slnx was added in 7669a28 but BuildContext still pointed Cake at LocalStack.sln, so every build, test and pack ran through the old file while the new one was only used by hand. The two listed exactly the same twelve projects, but that was maintained by hand - nothing enforced it, and a project added to one and not the other would have silently dropped out of CI. Point $(SlnFilePath) at LocalStack.slnx and delete the .sln. LocalStack.sln.DotSettings is renamed to LocalStack.slnx.DotSettings. Rider resolves shared settings by solution file name, so with the .slnx open the 24 KB of shared naming, formatting and inspection-severity settings were not being applied at all. The file has no internal reference to the solution name, so the rename is sufficient.
Anyone installing LocalStack.Client.Extensions 2.0.0 today still hits issue #52: its floor is [4.0.2, ), so NuGet floats them straight onto 4.0.4+ where the internal ClientFactory<T> constructor changed. The exception text is what people will search for, so the README is where the answer belongs - with the upgrade to 2.0.1 as the fix and pinning AWSSDK.Extensions.NETCore.Setup to 4.0.3.40 as the workaround for anyone who cannot upgrade yet. Also record why ClientFactoryGenericTypeName and CreateServiceClientMethodName are 'static readonly' and not 'const'. CA1802 and Sonar S3962 both flag them, and following that advice compiles but breaks two tests: AwsClientFactoryWrapperTests overwrites those fields by reflection to simulate AWS renaming its internals, and a const is inlined at every use site, so the overwrite would be a no-op and the failure-path tests would silently stop testing anything. The suppression is load-bearing; the comment says so. The two README copies under src/ are refreshed by the PreBuild target and ship inside the packages, so they move with the root file.
Blind-Striker
force-pushed
the
feature/issue-52-clientfactory-compat
branch
from
July 29, 2026 10:13
df66b27 to
d8c3de1
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 46 out of 49 changed files in this pull request and generated no new comments.
Comments suppressed due to low confidence (3)
tests/LocalStack.Client.Functional.Tests/Scenarios/CloudFormation/BaseCloudFormationScenario.cs:99
DeleteStackAndWaitAsynctreats anyAmazonCloudFormationExceptionduringDescribeStacksas a success signal. This can incorrectly report successful deletion on transient errors (throttling/network/auth), potentially leaking stacks and making later tests order-dependent again. It’s safer to only treat the specific “stack does not exist” case as success and let other exceptions fail the test (they’ll be aggregated byBaseScenario.DisposeAsync).
build/LocalStack.Build/BuildContext.cs:91- The
AwsSetupTrackXML doc comment still mentions alegacytrack (“pre-4.0.4”), but the PR description says the legacy/sdk-compat track was removed andDirectory.Build.propsonly documentscurrentandlatest. This comment is now misleading for contributors.
/// <summary>
/// Which AWSSDK.Extensions.NETCore.Setup constructor shape to build and test against
/// (current = post-4.0.4, legacy = pre-4.0.4, latest = floating canary).
/// </summary>
public string AwsSetupTrack { get; }
tests/LocalStack.Client.Extensions.Tests/AwsClientFactoryWrapperResolutionTests.cs:139
- The real-SDK resolution test uses the null-forgiving operator on
Assembly.GetType(...). If AWS renames/movesClientFactory<T>, this will fail with an unhelpfulNullReferenceExceptioninstead of a clear assertion failure explaining what type couldn’t be found.
// Deliberately shape-agnostic: this also runs on the floating canary track, where it must go red
// only when we genuinely cannot resolve - not merely because AWS appended another parameter.
Type factoryType = typeof(ConfigurationException).Assembly.GetType("Amazon.Extensions.NETCore.Setup.ClientFactory`1")!
.MakeGenericType(typeof(IAmazonS3));
ConstructorInfo[] candidates = factoryType.GetConstructors(BindingFlags.NonPublic | BindingFlags.Instance);
ConstructorInfo? selected = AwsClientFactoryWrapper.SelectFactoryConstructor(candidates);
DeleteStackAndWaitAsync caught every AmazonCloudFormationException from DescribeStacks and returned as though deletion had completed. Throttling, credential and service errors would all have been read as success, leaving the stack - and the SNS topic and SQS queue its template owns - behind in the shared container for whichever test ran next. That is the exact cross-test pollution this teardown was written to eliminate, so the swallow defeated its own purpose. Narrow the catch to a ValidationError, which is what CloudFormation answers with once the stack is gone, and let anything else fail the cleanup loudly. Verified against real LocalStack containers: CloudFormation scenarios 2/2 and the full functional suite 50/50 on net8.0, confirming LocalStack does report ValidationError for a deleted stack rather than some other error code. Raised by Copilot review on PR #53.
DescribeConstructors rendered parameter types with Type.Name, which collapses Action<ClientConfig, IServiceProvider> to "Action`2" - dropping precisely the part that identifies the parameter AWS added in 4.0.4. That text is what a bug report quotes, and diagnosing the next signature change without a version bisect is the whole reason the discovered signatures are included in the exception. Spell the arguments out, so the message reads .ctor(AWSOptions, Action<ClientConfig, IServiceProvider>). Uses Split rather than IndexOf because the StringComparison overload CA1307 asks for does not exist on netstandard2.0, which this project still targets. Covered by a test asserting the arguments survive and no backtick remains. Raised by Copilot review on PR #53.
The ValidationError filter added in cba7309 was justified with a claim the evidence did not actually support: the functional suite passing is also consistent with the catch never being entered, because the poll loop can exit through the DELETE_COMPLETE branch instead. A filter that always returned false would have passed the same tests. Measured it directly against both LocalStack versions these scenarios run on. Creating a stack, deleting it, then describing it by name gives, identically on 3.7.1 and 4.6.0: AmazonCloudFormationException ErrorCode = ValidationError StatusCode = 400 BadRequest Message = Stack with id <name> does not exist The throw also arrives on the very first poll - LocalStack drops the stack outright rather than parking it in DELETE_COMPLETE - so this catch is the loop's real exit path rather than a fallback, which is the part the passing suite could not have told us. Comment only; the filter itself is unchanged and now carries its evidence.
Blind-Striker
marked this pull request as ready for review
July 29, 2026 10:57
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
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, adds the safety net that would have caught this class of problem, and prepares theLocalStack.Client.Extensions2.0.1 release.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)📦 Release scope — Extensions 2.0.1 only
PackageExtensionVersion→ 2.0.1.PackageMainVersiondeliberately stays at 2.0.0:LocalStack.Clienthas no source change, and the advisory on itsAWSSDK.Core >= 4.0.0.15floor is unreachable in practice becauseAWSSDK.Extensions.NETCore.Setup 4.0.100.5itself declaresAWSSDK.Core >= 4.0.100.6, so NuGet lifts it anyway.The generated Extensions nuspec:
The minimum
AWSSDK.Extensions.NETCore.Setupis raised to 4.0.100.5 on purpose. Keeping consumers on a maintained AWS SDK is the intent; supporting the pre-4.0.4 shape while forbidding it in the nuspec would have been a contradiction. That decision is why this PR removes the backwards-compatibility test leg it originally introduced — see below.PackageValidationBaselineVersionmoved from the stale v1 baselines (1.4.1/1.2.2) to 2.0.0 for both projects. That made all five entries insrc/LocalStack.Client/CompatibilitySuppressions.xmlprovably dead — they comparelib/net461,lib/net6.0andlib/net7.0, target frameworks that only existed in the v1.4.1 package — so the file is deleted. Package validation reports them as unnecessary and fails the build if kept.🧪 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 on both tracks:
AWSSDK.Core/…NETCore.Setupresolvedcurrent(default, pinned)latest(floating canary)4.0.100.7is the reporter's exact version. Full matrix = Client.Tests 91 × net472/net8.0/net9.0, Integration.Tests 239 × net472/net8.0/net9.0, Extensions.Tests 51 × net8.0/net9.0. Clean--no-incrementalrebuild: 0 warnings, 0 errors. Both packages pack with package validation enabled.New tests model the current constructor shape — plus a shape AWS has not shipped, including a required value-type parameter — 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 the canary goes red only when resolution genuinely fails, not merely because AWS appended another parameter.
Test Environment:
📚 Documentation
✅ Code Quality Checklist
🔍 Additional Notes
Breaking Changes:
None to the public API; the new resolution helpers are
internal, exposed to tests viaInternalsVisibleTo. The raisedAWSSDK.Extensions.NETCore.Setupfloor is the one consumer-visible dependency change.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:
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 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.Microsoft.Extensions.*andSystem.Text.Jsontrack the .NET 9 servicing line (9.0.18), matching the highest TFM in the repo — there is nonet10.0target anywhere.Microsoft.SourceLink.GitHubstays on 10.0.301 because it has no 9.x release at all (it jumps 8.0.0 → 10.x). These central versions are consumed only by tests, sandboxes and the build project: everyMicrosoft.Extensions.*reference insrc/carries an explicitVersionOverride(3.1.32 / 8.0.0), so no shipped dependency floor moves.global.json→ SDK 9.0.316 for the same reason; the previous 10.0.302 pin had no target to justify it. LICENSE copyright range → 2026.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.Preventing recurrence — the
AwsSetupTrackswitch:Raising the floor does not stop this recurring: a NuGet floor is a minimum, so consumers still float upward and will meet the next AWS change before our pinned build does. The forgiving constructor resolution is the airbag; the canary is the alarm.
One MSBuild property moves both structurally-depended-on packages:
AWSSDK.Corefloats too — itsExplicitAWSCredentialschange bit us in this very PR, and the canary's summary step was printing a Core version that could never move.A
legacytrack and asdk-compatCI leg existed in earlier commits of this PR and are removed: once the nuspec floor is 4.0.100.5, proving the pre-4.0.4 shape works tests a configuration no consumer can install. The[Trait("Category", "SdkCompat")]attributes that only existed to filter that leg are gone with it.Two constraints worth knowing for review:
VersionOverridedoes not work for this —srcand tests must move together or NuGet reports NU1605 (package downgrade), an error underTreatWarningsAsErrors. Hence a central switch.CentralPackageFloatingVersionsEnabledis enabled only on the canary track. The property lives inDirectory.Build.props;Directory.Packages.propskeeps only the conditionalPackageVersionitems.A real bug found in the canary itself:
--force-restoreonly means "do restore" — it does not bypass NuGet's HTTP cache, and floating versions resolve against the cached feed index. Measured locally: the canary resolved…NETCore.Setup4.0.100.5 instead of 4.0.100.7, i.e. it would have silently reported "AWS has not moved" while AWS had. Fixed by clearing the HTTP cache before the run; it now resolves 4.0.100.9 / 4.0.100.7.Build reliability:
NuGet.Versioninginstead of a hand-rolled SemVer rule.NuGetVersiondecides whether a pre-release identifier is legal (it rejects…20260729.075853and accepts…g075853), the branch name is sanitised to the legal[0-9A-Za-z-]alphabet, and the composed version is parsed as a backstop so an invalid version fails the build with the offending value rather than atdotnet packtime.build.shnow carries the executable bit in the index (100755), so the fivechmod +x ./build.shsteps across three workflows are gone.CI/CD:
Every action bumped:
checkoutv4→v7,cachev4→v6,setup-dotnetv4→v6,upload-artifactv4→v7,dependency-review-actionv4→v5,dorny/test-reporterv1→v3.All four workflows now declare an explicit
permissionsblock, resolving the CodeQLactions/missing-workflow-permissionsalerts.build-and-testgetschecks: writebecausedorny/test-reporterpublishes a check run; the badge step authenticates with its own PAT and needs nothing fromGITHUB_TOKEN.setup-dotnetreadsglobal-json-file, while the explicit8.0.x/9.0.xentries are kept on purpose — runningnet8.0test assemblies needs the .NET 8 runtime, which the pinned SDK alone does not provide.🎯 Reviewer Focus Areas
Please pay special attention to:
Specifically worth a second pair of eyes:
4.0.2→4.0.100.5). Deliberate, and the reason thelegacytrack was dropped — but it is the one change consumers feel.SelectFactoryConstructortie-breaking — fewest-parameter-wins. Reasonable today, but it is the rule that decides behaviour if AWS ever ships twoAWSOptions-first overloads.CompatibilitySuppressions.xml. Provably dead against the 2.0.0 baseline, but it removes the record of those v1-era differences.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?>.Known follow-ups (deliberately not in this PR):
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. 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 4.0.4+ as well as the versions before it:
Running the suite the way the scheduled canary does:
By submitting this pull request, I confirm that: