feat(sdk): add .NET SDK with AWS SSM and Azure Key Vault support#146
Merged
feat(sdk): add .NET SDK with AWS SSM and Azure Key Vault support#146
Conversation
Implement the Envilder .NET SDK targeting netstandard2.0 with hexagonal architecture (Domain/Application/Infrastructure). Includes map-file parsing, secret resolution, IConfiguration and IServiceCollection integration, plus unit and acceptance tests using LocalStack and Lowkey Vault via Testcontainers.
Implement the Envilder .NET SDK targeting netstandard2.0 with hexagonal architecture (Domain/Application/Infrastructure). Includes map-file parsing, secret resolution, IConfiguration and IServiceCollection integration, plus unit and acceptance tests using LocalStack and Lowkey Vault via Testcontainers.
…nvilder into macalbert/sdk-dotnet
Replaces string provider selection with SecretProviderType enum for strong typing and clarity. Refactors SecretProviderFactory and parsing logic accordingly. Updates tests to use the enum and improves LocalStack test reliability with health checks and embedded secrets.
Add comprehensive XML documentation for all public APIs to improve discoverability and maintainability. Introduce end-to-end tests using LocalStack and Lowkey Vault to verify integration with AWS SSM and Azure Key Vault, including configuration and error handling scenarios. Update test dependencies to Microsoft.Extensions.* v10 for .NET 8 support.
Allow MapFileConfig and ParsedMapFile properties to be set after construction. Remove IsExternalInit.cs as it is no longer needed.
Contributor
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (5)
WalkthroughAdds a .NET Envilder SDK: JSON map-file parser, secret-resolution client, AWS/Azure secret providers and factory, IConfiguration/IServiceCollection integrations, project/solution files, README, and extensive unit, acceptance, and end-to-end tests. Changes
Sequence Diagram(s)sequenceDiagram
participant App as App
participant Parser as MapFileParser
participant Parsed as ParsedMapFile
participant Factory as SecretProviderFactory
participant Provider as ISecretProvider
participant Client as EnvilderClient
App->>Parser: Parse(json map file)
Parser-->>App: ParsedMapFile
App->>Factory: Create(config, options?)
Factory-->>App: ISecretProvider (AWS/Azure)
App->>Client: new EnvilderClient(provider)
App->>Client: ResolveSecretsAsync(ParsedMapFile)
loop per mapping
Client->>Provider: GetSecretAsync(path)
Provider-->>Client: secret value or null
end
Client-->>App: Dictionary<key,value> (nulls omitted)
App->>Client: InjectIntoEnvironment(secrets)
loop per secret
Client->>App: Environment.SetEnvironmentVariable(key, value)
end
sequenceDiagram
participant App as .NET App
participant ConfigBuilder as IConfigurationBuilder
participant Ext as AddEnvilder
participant Source as EnvilderConfigurationSource
participant Provider as EnvilderConfigurationProvider
participant Client as EnvilderClient
participant SecretProvider as ISecretProvider
App->>Ext: AddEnvilder(mapFilePath, secretProvider)
Ext->>ConfigBuilder: builder.Add(source)
App->>ConfigBuilder: Build()
ConfigBuilder->>Provider: provider.Load()
Provider->>Client: ResolveSecretsAsync(mapFile)
loop per mapping
Client->>SecretProvider: GetSecretAsync(path)
SecretProvider-->>Client: value/null
end
Client-->>Provider: resolved secrets
Provider->>ConfigBuilder: populate Data (normalized keys)
ConfigBuilder-->>App: IConfiguration ready
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested labels
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (5)
src/sdks/dotnet/Infrastructure/Configuration/EnvilderConfigurationProvider.cs (1)
12-16: Consider adding null validation for constructor parameters.If
clientormapFileis null, theLoad()method will throw aNullReferenceExceptionwith minimal context. Adding guard clauses improves debuggability.🛡️ Proposed defensive validation
public EnvilderConfigurationProvider(EnvilderClient client, ParsedMapFile mapFile) { - _client = client; - _mapFile = mapFile; + _client = client ?? throw new ArgumentNullException(nameof(client)); + _mapFile = mapFile ?? throw new ArgumentNullException(nameof(mapFile)); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/sdks/dotnet/Infrastructure/Configuration/EnvilderConfigurationProvider.cs` around lines 12 - 16, Add defensive null validation in the EnvilderConfigurationProvider constructor: check that the incoming EnvilderClient parameter (client) and ParsedMapFile parameter (mapFile) are not null and throw ArgumentNullException with the appropriate parameter name if they are; this prevents obscure NullReferenceExceptions later in the Load() method and makes failures clearer by validating before assigning to the _client and _mapFile fields.tests/sdks/dotnet/Infrastructure/Configuration/EnvilderConfigurationSectionBindingTests.cs (1)
96-101: Consider usingintforMaxPoolSizeto test type conversion.
MaxPoolSizeis declared asstring, but in real-world scenarios it would typically be anint. The current test verifies string binding works, but doesn't exercise the configuration system's type conversion. If this is intentional for simplicity, it's fine; otherwise, using the actual expected type would provide additional coverage.💡 Optional: Test numeric binding
private class DatabaseConfig { public const string SectionName = "Database"; public string ConnectionString { get; set; } = string.Empty; - public string MaxPoolSize { get; set; } = string.Empty; + public int MaxPoolSize { get; set; } }Then update the assertion:
-actual.MaxPoolSize.Should().Be("100"); +actual.MaxPoolSize.Should().Be(100);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/sdks/dotnet/Infrastructure/Configuration/EnvilderConfigurationSectionBindingTests.cs` around lines 96 - 101, The test's DatabaseConfig declares MaxPoolSize as string which doesn't exercise configuration type conversion; change DatabaseConfig.MaxPoolSize to an int (e.g., public int MaxPoolSize { get; set; }) with a sensible default, update the test's configuration input to provide a numeric value (e.g., "10"), and adjust the assertion to expect the bound int value (e.g., Assert.Equal(10, bound.Database.MaxPoolSize)); keep SectionName and other properties unchanged so the binding call that uses DatabaseConfig (e.g., ConfigurationBinder/Bind/GetSection in the test) verifies numeric conversion.tests/sdks/dotnet/Infrastructure/SecretProviderFactoryTests.cs (2)
59-70: Consider verifying the exception message.The test confirms
InvalidOperationExceptionis thrown but doesn't assert on the message content. Verifying the message would catch regressions where the exception type is correct but the context is wrong.Suggested enhancement
// Assert - act.Should().Throw<InvalidOperationException>(); + act.Should().Throw<InvalidOperationException>() + .WithMessage("*Vault URL*");🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/sdks/dotnet/Infrastructure/SecretProviderFactoryTests.cs` around lines 59 - 70, Update the test Should_RequireVaultUrl_When_AzureProviderSelected to also assert the exception message content: when invoking SecretProviderFactory.Create with a MapFileConfig whose Provider is SecretProviderType.Azure, capture the InvalidOperationException and assert its Message contains the expected text (e.g., mentions missing VaultUrl or similar) so the test verifies both type and context; reference the existing act delegate and replace the simple Throw assertion with one that inspects the exception.Message returned by SecretProviderFactory.Create.
9-70: Add test coverage for null config and invalid AWS profile scenarios.The test suite covers the happy paths well but lacks edge cases:
- Null
configparameter (onceArgumentNullExceptionvalidation is added)- Invalid/missing AWS profile when profile is explicitly specified (once fail-fast behavior is implemented)
These tests would ensure the factory's defensive behaviors are exercised.
Suggested additional tests
[Fact] public void Should_ThrowArgumentNullException_When_ConfigIsNull() { // Act var act = () => SecretProviderFactory.Create(null!); // Assert act.Should().Throw<ArgumentNullException>() .And.ParamName.Should().Be("config"); } [Fact] public void Should_ThrowInvalidOperationException_When_AwsProfileNotFound() { // Arrange var config = new MapFileConfig { Provider = SecretProviderType.Aws, Profile = "nonexistent-profile-12345", }; // Act var act = () => SecretProviderFactory.Create(config); // Assert act.Should().Throw<InvalidOperationException>() .WithMessage("*profile*nonexistent-profile-12345*"); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/sdks/dotnet/Infrastructure/SecretProviderFactoryTests.cs` around lines 9 - 70, Add two edge-case unit tests in SecretProviderFactoryTests to validate defensive behavior: (1) a Should_ThrowArgumentNullException_When_ConfigIsNull test that calls SecretProviderFactory.Create(null!) and asserts an ArgumentNullException is thrown with ParamName "config"; and (2) a Should_ThrowInvalidOperationException_When_AwsProfileNotFound test that creates a MapFileConfig with Provider = SecretProviderType.Aws and Profile = "nonexistent-profile-12345", calls SecretProviderFactory.Create(config) and asserts an InvalidOperationException is thrown with a message containing the missing profile string; locate and add these tests alongside the existing tests referencing SecretProviderFactory.Create, MapFileConfig and SecretProviderType.src/sdks/dotnet/Infrastructure/SecretProviderFactory.cs (1)
45-45: Consider improving readability of nested constructor calls.The triple-nested target-typed
new()is syntactically valid but harder to parse at a glance. Explicit type names or intermediate variables would clarify intent.Suggested refactor
- return new(new(new(vaultUrl), new DefaultAzureCredential())); + var secretClient = new SecretClient(new Uri(vaultUrl), new DefaultAzureCredential()); + return new AzureKeyVaultSecretProvider(secretClient);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/sdks/dotnet/Infrastructure/SecretProviderFactory.cs` at line 45, The return statement in SecretProviderFactory that uses triple-nested target-typed new (return new(new(new(vaultUrl), new DefaultAzureCredential()));) is hard to read; refactor by introducing intermediate variables or using explicit type constructors to build the inner Vault/credential objects before returning the outer object (e.g., create a DefaultAzureCredential instance, create the VaultClient/Credential-wrapping object with vaultUrl, then pass those to the outer constructor) and replace the nested new(...) chain with those variables or explicit type-named new(...) calls to improve clarity.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In
`@src/sdks/dotnet/Infrastructure/Configuration/EnvilderConfigurationProvider.cs`:
- Around line 12-16: Add defensive null validation in the
EnvilderConfigurationProvider constructor: check that the incoming
EnvilderClient parameter (client) and ParsedMapFile parameter (mapFile) are not
null and throw ArgumentNullException with the appropriate parameter name if they
are; this prevents obscure NullReferenceExceptions later in the Load() method
and makes failures clearer by validating before assigning to the _client and
_mapFile fields.
In `@src/sdks/dotnet/Infrastructure/SecretProviderFactory.cs`:
- Line 45: The return statement in SecretProviderFactory that uses triple-nested
target-typed new (return new(new(new(vaultUrl), new DefaultAzureCredential()));)
is hard to read; refactor by introducing intermediate variables or using
explicit type constructors to build the inner Vault/credential objects before
returning the outer object (e.g., create a DefaultAzureCredential instance,
create the VaultClient/Credential-wrapping object with vaultUrl, then pass those
to the outer constructor) and replace the nested new(...) chain with those
variables or explicit type-named new(...) calls to improve clarity.
In
`@tests/sdks/dotnet/Infrastructure/Configuration/EnvilderConfigurationSectionBindingTests.cs`:
- Around line 96-101: The test's DatabaseConfig declares MaxPoolSize as string
which doesn't exercise configuration type conversion; change
DatabaseConfig.MaxPoolSize to an int (e.g., public int MaxPoolSize { get; set;
}) with a sensible default, update the test's configuration input to provide a
numeric value (e.g., "10"), and adjust the assertion to expect the bound int
value (e.g., Assert.Equal(10, bound.Database.MaxPoolSize)); keep SectionName and
other properties unchanged so the binding call that uses DatabaseConfig (e.g.,
ConfigurationBinder/Bind/GetSection in the test) verifies numeric conversion.
In `@tests/sdks/dotnet/Infrastructure/SecretProviderFactoryTests.cs`:
- Around line 59-70: Update the test
Should_RequireVaultUrl_When_AzureProviderSelected to also assert the exception
message content: when invoking SecretProviderFactory.Create with a MapFileConfig
whose Provider is SecretProviderType.Azure, capture the
InvalidOperationException and assert its Message contains the expected text
(e.g., mentions missing VaultUrl or similar) so the test verifies both type and
context; reference the existing act delegate and replace the simple Throw
assertion with one that inspects the exception.Message returned by
SecretProviderFactory.Create.
- Around line 9-70: Add two edge-case unit tests in SecretProviderFactoryTests
to validate defensive behavior: (1) a
Should_ThrowArgumentNullException_When_ConfigIsNull test that calls
SecretProviderFactory.Create(null!) and asserts an ArgumentNullException is
thrown with ParamName "config"; and (2) a
Should_ThrowInvalidOperationException_When_AwsProfileNotFound test that creates
a MapFileConfig with Provider = SecretProviderType.Aws and Profile =
"nonexistent-profile-12345", calls SecretProviderFactory.Create(config) and
asserts an InvalidOperationException is thrown with a message containing the
missing profile string; locate and add these tests alongside the existing tests
referencing SecretProviderFactory.Create, MapFileConfig and SecretProviderType.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 40506a65-b151-4dde-8ec0-adbbb6e666bb
⛔ Files ignored due to path filters (1)
e2e/sample/cli-validation.envis excluded by none and included by none
📒 Files selected for processing (19)
src/sdks/dotnet/Application/EnvilderClient.cssrc/sdks/dotnet/Application/MapFileParser.cssrc/sdks/dotnet/Domain/EnvilderOptions.cssrc/sdks/dotnet/Domain/ParsedMapFile.cssrc/sdks/dotnet/Infrastructure/Configuration/EnvilderConfigurationProvider.cssrc/sdks/dotnet/Infrastructure/DependencyInjection/ServiceCollectionExtensions.cssrc/sdks/dotnet/Infrastructure/SecretProviderFactory.cstests/sdks/dotnet/Application/EnvilderClientTests.cstests/sdks/dotnet/Application/MapFileParserTests.cstests/sdks/dotnet/CancellationTokenForTest.cstests/sdks/dotnet/EndToEnd/ConsumerExperienceTests.cstests/sdks/dotnet/Envilder.Tests.csprojtests/sdks/dotnet/Fixtures/LocalStackFixture.cstests/sdks/dotnet/Infrastructure/Aws/AwsSsmAcceptanceTests.cstests/sdks/dotnet/Infrastructure/Azure/AzureKeyVaultAcceptanceTests.cstests/sdks/dotnet/Infrastructure/Configuration/EnvilderConfigurationProviderTests.cstests/sdks/dotnet/Infrastructure/Configuration/EnvilderConfigurationSectionBindingTests.cstests/sdks/dotnet/Infrastructure/DependencyInjection/ServiceCollectionExtensionsTests.cstests/sdks/dotnet/Infrastructure/SecretProviderFactoryTests.cs
✅ Files skipped from review due to trivial changes (2)
- tests/sdks/dotnet/CancellationTokenForTest.cs
- tests/sdks/dotnet/Envilder.Tests.csproj
🚧 Files skipped from review as they are similar to previous changes (12)
- src/sdks/dotnet/Domain/EnvilderOptions.cs
- src/sdks/dotnet/Domain/ParsedMapFile.cs
- tests/sdks/dotnet/Infrastructure/Configuration/EnvilderConfigurationProviderTests.cs
- src/sdks/dotnet/Infrastructure/DependencyInjection/ServiceCollectionExtensions.cs
- src/sdks/dotnet/Application/MapFileParser.cs
- tests/sdks/dotnet/Infrastructure/DependencyInjection/ServiceCollectionExtensionsTests.cs
- src/sdks/dotnet/Application/EnvilderClient.cs
- tests/sdks/dotnet/Infrastructure/Azure/AzureKeyVaultAcceptanceTests.cs
- tests/sdks/dotnet/Infrastructure/Aws/AwsSsmAcceptanceTests.cs
- tests/sdks/dotnet/Application/EnvilderClientTests.cs
- tests/sdks/dotnet/Fixtures/LocalStackFixture.cs
- tests/sdks/dotnet/EndToEnd/ConsumerExperienceTests.cs
Enable slash-delimited keys in map file for hierarchical config binding (e.g., Database/ConnectionString). Refactor ParsedMapFile to use immutable properties. Add validation and error handling to AddEnvilder. Remove unused Region option. Update and add tests for section binding and error cases.
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
Add explicit null and argument validation to constructors and methods for improved robustness and fail-fast behavior. Update tests to cover new error cases. Update package versions and improve resource handling and code clarity.
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
|
|
||
| - name: 🍄 Run tests | ||
| if: steps.version-check.outputs.version_changed == 'true' | ||
| run: dotnet test ../../../tests/sdks/dotnet/ -c Release --filter "FullyQualifiedName!~Acceptance" --verbosity normal |
There was a problem hiding this comment.
🔴 CI test filter does not exclude E2E tests that depend on a developer-specific AWS profile
The CI workflow filter --filter "FullyQualifiedName!~Acceptance" at .github/workflows/publish-nuget.yml:58 excludes AwsSsmAcceptanceTests and AzureKeyVaultAcceptanceTests but does not exclude ConsumerExperienceTests (FQN: Envilder.Tests.EndToEnd.ConsumerExperienceTests) since its name doesn't contain "Acceptance".
Because ConsumerExperienceTests is in the ContainersCollection (tests/sdks/dotnet/Fixtures/ContainersCollection.cs:3-4), xUnit will initialize LocalStackFixture for this collection. During initialization, LocalStackFixture.LoadEnvironmentAsync() reads the embedded secrets-map.json which specifies "profile": "mac", then calls SecretProviderFactory.Create(mapFile.Config) (tests/sdks/dotnet/Fixtures/LocalStackFixture.cs:78). The factory's CreateAwsSecretProvider method at src/sdks/dotnet/Infrastructure/SecretProviderFactory.cs:63-72 throws InvalidOperationException when the "mac" profile is not found in the credential store — which it won't be in CI. This causes all E2E and acceptance tests in the collection to fail.
Prompt for agents
The CI test filter FullyQualifiedName!~Acceptance does not exclude ConsumerExperienceTests (in namespace Envilder.Tests.EndToEnd), which shares the ContainersCollection with the Acceptance tests. This collection triggers LocalStackFixture initialization, which reads secrets-map.json with a hardcoded "profile": "mac" AWS profile, causing SecretProviderFactory.Create to throw in CI.
Two approaches to fix:
1. Update the CI filter to also exclude EndToEnd tests: --filter "FullyQualifiedName!~Acceptance&FullyQualifiedName!~EndToEnd" (or rename ConsumerExperienceTests to include Acceptance in its name/namespace).
2. Make LocalStackFixture.LoadEnvironmentAsync resilient: catch the InvalidOperationException from SecretProviderFactory.Create when the AWS profile is missing and proceed with an empty environment dictionary, since LOCALSTACK_AUTH_TOKEN is only needed for LocalStack Pro features.
Files involved: .github/workflows/publish-nuget.yml (filter), tests/sdks/dotnet/Fixtures/LocalStackFixture.cs (LoadEnvironmentAsync), tests/sdks/dotnet/secrets-map.json (hardcoded profile).
Was this helpful? React with 👍 or 👎 to provide feedback.
4 tasks
6 tasks
This was referenced Apr 26, 2026
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.
Summary
Adds the .NET SDK for Envilder, enabling .NET applications to load secrets directly from AWS SSM Parameter Store or Azure Key Vault using the standard map-file format. Also includes shared Copilot skills for .NET architecture, testing, security, and observability, plus a NuGet publish workflow.
Changes
SDK — .NET (
src/sdks/dotnet/)EnvilderClientapplication service: parses map file and resolves secrets via providerMapFileParserfor reading and validating the JSON map-file formatSecretProviderFactorywith enum-based provider selection (Aws/Azure)EnvilderOptions,MapFileConfig,ParsedMapFile,ISecretProviderport,SecretProviderTypeenumAwsSsmSecretProvider(AWS SDK),AzureKeyVaultSecretProvider(Azure SDK)IConfigurationBuilder.AddEnvilder()extension,IServiceCollection.AddEnvilder()extensionEnvilderConfigurationProviderandEnvilderConfigurationSourcefor seamlessIConfigurationintegrationTests (
tests/sdks/dotnet/)EnvilderClient,MapFileParser,SecretProviderFactoryAwsSsmSecretProvider,AzureKeyVaultSecretProviderConfigurationBuilderExtensions,EnvilderConfigurationProvider,ServiceCollectionExtensionsAwsSsmAcceptanceTests(LocalStack via TestContainers),AzureKeyVaultAcceptanceTests(Lowkey Vault via TestContainers)ConsumerExperienceTestsverifying the full load-secrets pipelineLocalStackFixture,LowkeyVaultFixture,ContainersCollectionCI/CD
publish-nuget.ymlworkflow for publishing to NuGet on release tagsCopilot Skills (
.github/skills/)common-environments,common-git,common-observability,common-securitycommon-testing-conventions(with dotnet, python, typescript sub-files)dotnet-architecture,dotnet-dependency-injection,dotnet-integration-testingdotnet-libraries,dotnet-test-doublespython-testing,typescript-testingTesting
dotnet testpasses (unit + acceptance + e2e)pnpm testpasses (TypeScript core unchanged)pnpm lintpassesRelated
Implements .NET SDK from the Envilder SDK platform plan.
Summary by CodeRabbit
New Features
Documentation
Tests
Chores