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

[confmap] Fix expansion of escaped environment variables #10716

Merged
merged 1 commit into from
Jul 24, 2024

Conversation

dmitryax
Copy link
Member

@dmitryax dmitryax commented Jul 24, 2024

This change fixes the issue where environment variables escaped with $$ are expanded. The collector now converts $${ENV_VAR} to ${ENV_VAR} and $$ENV_VAR to $ENV_VAR without further expansion (or the expansion failure in case of $ENV_VAR).

Fixes #10713

Copy link

codecov bot commented Jul 24, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 92.22%. Comparing base (49ea32b) to head (9ceeda2).

Additional details and impacted files
@@            Coverage Diff             @@
##             main   #10716      +/-   ##
==========================================
- Coverage   92.24%   92.22%   -0.03%     
==========================================
  Files         403      403              
  Lines       18720    18717       -3     
==========================================
- Hits        17268    17261       -7     
- Misses       1097     1099       +2     
- Partials      355      357       +2     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@mx-psi mx-psi requested a review from TylerHelmuth July 24, 2024 08:25
@mx-psi mx-psi added the release:blocker The issue must be resolved before cutting the next release label Jul 24, 2024
@dmitryax dmitryax changed the title [confmap] Fix double-expansion of escaped environment variables [confmap] Fix expansion of escaped environment variables Jul 24, 2024
@dmitryax dmitryax force-pushed the fix-double-expansion branch 4 times, most recently from 06db8d7 to bd85bb6 Compare July 24, 2024 20:51
This change fixes the issue where environment variables escaped with $$ were expanded. The collector now converts `$${ENV_VAR}` to `${ENV_VAR}` and `$$ENV_VAR` to `$ENV_VAR` without further expansion.
@dmitryax dmitryax merged commit 1c96225 into open-telemetry:main Jul 24, 2024
50 checks passed
@github-actions github-actions bot added this to the next release milestone Jul 24, 2024
@dmitryax dmitryax deleted the fix-double-expansion branch July 24, 2024 22:51
jvoravong added a commit to jvoravong/splunk-otel-collector that referenced this pull request Jul 25, 2024
jvoravong added a commit to signalfx/splunk-otel-collector that referenced this pull request Jul 25, 2024
…x expansion of escaped environment variables
jvoravong added a commit to signalfx/splunk-otel-collector that referenced this pull request Jul 25, 2024
…x expansion of escaped environment variables
dmitryax added a commit to signalfx/splunk-otel-collector that referenced this pull request Jul 29, 2024
* [chore] Upgrade core dependencies

Update to 0.105.0 version + cherry pick open-telemetry/opentelemetry-collector#10716

Also, disabling both confmap upstream feature gates for now. Enabling confmap.unifyEnvVarExpansion doesn't have any effect other than introducing a bug. The warning messages that are expected to show with the FG are ignored in our repo

We have duplicated functionality in our distro which needs to be deprecated first. Then we can remove it and align with upstream.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
release:blocker The issue must be resolved before cutting the next release
Projects
None yet
5 participants