Skip to content

#76 clear Data before refill#77

Merged
MrZoidberg merged 3 commits intoMrZoidberg:masterfrom
k0st1x:fix/76-delete-keys-on-reload
Mar 25, 2026
Merged

#76 clear Data before refill#77
MrZoidberg merged 3 commits intoMrZoidberg:masterfrom
k0st1x:fix/76-delete-keys-on-reload

Conversation

@k0st1x
Copy link
Copy Markdown
Contributor

@k0st1x k0st1x commented Mar 13, 2026

No description provided.

@MrZoidberg
Copy link
Copy Markdown
Owner

I'll need some tests to cover this bug and that nothing is broken with the fix. Feel free to add them or I can help over the weekend. Thanks for you contribution

@MrZoidberg
Copy link
Copy Markdown
Owner

Some tests are failing

@k0st1x k0st1x force-pushed the fix/76-delete-keys-on-reload branch from b7d1d4b to 52af3d9 Compare March 17, 2026 06:38
@k0st1x
Copy link
Copy Markdown
Contributor Author

k0st1x commented Mar 17, 2026

@MrZoidberg
how to rerun tests?

@MrZoidberg
Copy link
Copy Markdown
Owner

MrZoidberg commented Mar 22, 2026

@k0st1x In PR with forks, only I can run workflows to mitigate any security risks. LGTM, the only thing you haven't updated is the integration tests to check that old keys are removed. would you mind adding this to tests and I will merge/create new release

[Fact]
        public async Task Success_DeletedKeyRemovedFromVersionsCache()
        {
            // arrange
            var values = new Dictionary<string, IEnumerable<KeyValuePair<string, object>>>
            {
                { "test", new[] { new KeyValuePair<string, object>("option1", "value1") } },
                { "test/subsection", new[] { new KeyValuePair<string, object>("option2", "value2") } },
            };

            var container = this.PrepareVaultContainer();
            try
            {
                await container.StartAsync();
                await this.LoadDataAsync("http://localhost:8200", values);

                var builder = new ConfigurationBuilder();
                builder.AddVaultConfiguration(
                    () => new VaultOptions("http://localhost:8200", "root"),
                    "test",
                    "secret",
                    this.logger);
                var configurationRoot = builder.Build();

                // assert initial state
                configurationRoot.GetValue<string>("option1").Should().Be("value1");
                configurationRoot.GetSection("subsection").GetValue<string>("option2").Should().Be("value2");

                var provider = configurationRoot.Providers.OfType<VaultConfigurationProvider>().First();
                var versionsCacheField = typeof(VaultConfigurationProvider)
                    .GetField("versionsCache", BindingFlags.NonPublic | BindingFlags.Instance);
                var versionsCache = (Dictionary<string, int>)versionsCacheField!.GetValue(provider)!;

                versionsCache.Should().ContainKey("subsection");

                // delete test/subsection from Vault
                var vaultClientSettings = new VaultClientSettings("http://localhost:8200", new TokenAuthMethodInfo("root"))
                {
                    SecretsEngineMountPoints = { KeyValueV2 = "secret" }
                };
                IVaultClient vaultClient = new VaultClient(vaultClientSettings);
                await vaultClient.V1.Secrets.KeyValue.V2.DeleteSecretAsync("test/subsection", "secret");

                // reload the configuration provider
                provider.Load();

                // assert the deleted key is removed from versionsCache
                versionsCache.Should().NotContainKey("subsection");

                // assert the deleted key's value is no longer present in configuration
                configurationRoot.GetSection("subsection").GetValue<string>("option2").Should().BeNull();
            }
            finally
            {
                await container.DisposeAsync();
            }
        }

additional property Vaultconfigurationprovider.VersionsCache_TEST only for testing purpose
@k0st1x
Copy link
Copy Markdown
Contributor Author

k0st1x commented Mar 23, 2026

@MrZoidberg thank you for being completed working unit-test.
I've just added a bit - using of reflection replaced by the "_TEST" property - to avoid failed unit-test after some refactoring. Such property will not be published in the release-package.

@MrZoidberg
Copy link
Copy Markdown
Owner

@k0st1x, the tests are failing. Please check them

@k0st1x
Copy link
Copy Markdown
Contributor Author

k0st1x commented Mar 25, 2026

@MrZoidberg but locally it works fine.
Are tests run on Release configuration?

@MrZoidberg
Copy link
Copy Markdown
Owner

Looks good now

@MrZoidberg MrZoidberg merged commit 2e4edbc into MrZoidberg:master Mar 25, 2026
1 check failed
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