Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ensure --dry-run doesn't report expanded config values #2439

Merged
merged 3 commits into from
Jan 3, 2023

Conversation

rmfitzpatrick
Copy link
Contributor

@rmfitzpatrick rmfitzpatrick commented Jan 3, 2023

Currently --dry-run will provide the final config values with all config source and environment variables having been resolved/expanded. This is not desirable for dynamic* content. These changes add a new configprovider.Hook mechanism to provide additional components access of pre-resolving config content like* the config server currently does. The config server converter is updated to conform to the new interface.

These also pick up existing missing imports and add an integration test with an additional testutil container helper.

@rmfitzpatrick rmfitzpatrick requested review from a team as code owners January 3, 2023 21:51
@@ -126,13 +145,26 @@ func TestConfigSourceConfigMapProvider(t *testing.T) {
} else {
assert.ErrorContains(t, err, tt.wantErr)
assert.Nil(t, r)
return
break
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this was an existing bug in the test that missed shutdown coverage

@rmfitzpatrick rmfitzpatrick merged commit 7362b99 into main Jan 3, 2023
@delete-merged-branch delete-merged-branch bot deleted the unexpandeddryrun branch January 3, 2023 22:57
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.

3 participants