-
Notifications
You must be signed in to change notification settings - Fork 3.5k
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
Revert pr #16050 #16201
Revert pr #16050 #16201
Conversation
…es () inside config.string in the pipelines.yml file, but that created a regression, becuase env vars is expected to be replaced in all the other yaml keys
Quality Gate passedIssues Measures |
💚 Build Succeeded
cc @andsel |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm +1 to merging this revert on main
and back-porting to 8.14
, then following up to re-address the other issue separately.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
@logstashmachine backport 8.14 |
Release notes
Bugfix a regression which resulted in missed environment variables resolution inside
pipelines.yml
What does this PR do?
Revert pr #16050, which avoided to replace environment variables (${VAR}) inside config.string in the pipelines.yml file, but that created a regression, because env vars is expected to be replaced in all the other yaml keys.
Why is it important/What is the impact to the user?
Environment variables has to be resolved in
config/pipelines.yml
. With previous PR #16050 was tried to fix a problem linked also to resolution, which generated errors (and prohibited LS to start) if env vars were referred in commented code insideconfig.string
definition inpipelines.yml
. The fix was to avoid resolution at all inpipelines.yml
but that generated a regression, hence this revert PR.Checklist
Author's Checklist
How to test this PR locally
Related issues
config.string
contains ${VAR} in the comments. #16050Use cases
Screenshots
Logs