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

Rely on upstream confmap implementation to expand env variables #5206

Merged
merged 1 commit into from
Aug 14, 2024

Conversation

dmitryax
Copy link
Contributor

@dmitryax dmitryax commented Aug 9, 2024

After recent changes in the collector core, we can rely on the upstream to expand environmental variables along with $$ escaping. Unfortunately, we cannot do that for the config_sources section as it's custom for our distro. So we need to split the behavior for a while to unblock the 0.107.0 release.

This change also fixes the $$ expansion bug. See the corrected test data for TestExpandedDollarSignsViaEnvConfigSource, which previously validate invalid behavior.

Later, we can refactor it further and migrate the config sources to the config providers to be handled upstream, significantly reducing the amount of code we maintain.

@dmitryax dmitryax marked this pull request as ready for review August 9, 2024 19:20
@dmitryax dmitryax requested review from a team as code owners August 9, 2024 19:20
@dmitryax dmitryax force-pushed the unblock-107 branch 3 times, most recently from a5ebf3d to 7e3de9a Compare August 9, 2024 22:49
assert.Equal(t, expectedParser.ToStringMap(), res.ToStringMap())
assert.NoError(t, callClose(closeFunc))
}

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The test has been removed since the expansion was disabled in favor of the upstream implementation. e2e functionality is covered in the integration tests

@dmitryax dmitryax force-pushed the unblock-107 branch 6 times, most recently from d67af1b to b98d271 Compare August 13, 2024 19:01
@dmitryax dmitryax force-pushed the unblock-107 branch 2 times, most recently from e5e0ded to 3119ba4 Compare August 13, 2024 21:11
@dmitryax dmitryax changed the title Do not expand env variables for regular config section Rely on upstream confmap implementation to expand env variables Aug 13, 2024
@dmitryax dmitryax force-pushed the unblock-107 branch 2 times, most recently from e355dd8 to a11e3a9 Compare August 14, 2024 05:26
Rely on the envprovider instead. This change resolves two blocked feature gates and prepares for 0.107.0 release
@dmitryax dmitryax merged commit 3c7ebe6 into main Aug 14, 2024
54 checks passed
@dmitryax dmitryax deleted the unblock-107 branch August 14, 2024 15:34
@github-actions github-actions bot locked and limited conversation to collaborators Aug 14, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants