Commented out variable substitutions may still be evaluated in pipelines.yml

[robbavey]

1. Logstash version (e.g. bin/logstash --version) Latest versions of 7.17, 8.12 and main
2. Logstash installation source (e.g. built from source, with a package manager: DEB/RPM, expanded from tar or zip archive, docker) Built from source, tar.gz and Logstash on ECK (docker image)

JVM (e.g. java -version): Bundled JVM

Description of the problem including expected versus actual behavior: When running a pipeline defined in pipelines.yml, pipelines defined in config.string that have commented out lines, will have any variable substitution (ENV or keystore) present in those commented out lines, evaluated.

This was discovered during ECK testing when commenting out a plugin definition that contained a non-existent ENV variable normally provided by ElasticsearchRef

Steps to reproduce:

Create a pipelines.yml that looks like:

- pipeline.id: test
  config.string: |
    input { generator {} } 
    output { 
      nil {}
      # ${I_AM_NOT_HERE}
    }   

and run bin/logstash.

Logstash will fail to start (regardless of whether there are also valid pipelines specified), giving the following error:

[2024-03-14T14:15:16,947][ERROR][logstash.config.sourceloader] Could not fetch all the sources {:exception=>LogStash::ConfigurationError, :message=>"Cannot evaluate `${I_AM_NOT_HERE}`. Replacement variable `I_AM_NOT_HERE` is not defined in a Logstash secret store or as an Environment entry and there is no default value given.", :backtrace=>["/Users/robbavey/code/logstash/logstash-core/lib/logstash/util/substitution_variables.rb:75:in `block in replace_placeholders'", "org/jruby/RubyString.java:3186:in `gsub'", "/Users/robbavey/code/logstash/logstash-core/lib/logstash/util/substitution_variables.rb:60:in `replace_placeholders'", "/Users/robbavey/code/logstash/logstash-core/lib/logstash/util/substitution_variables.rb:44:in `deep_replace'", "/Users/robbavey/code/logstash/logstash-core/lib/logstash/util/substitution_variables.rb:35:in `block in deep_replace'", "org/jruby/RubyHash.java:1610:in `each'", "/Users/robbavey/code/logstash/logstash-core/lib/logstash/util/substitution_variables.rb:34:in `deep_replace'", "/Users/robbavey/code/logstash/logstash-core/lib/logstash/util/substitution_variables.rb:41:in `block in deep_replace'", "org/jruby/RubyArray.java:1983:in `each'", "org/jruby/RubyEnumerable.java:1212:in `each_with_index'", "/Users/robbavey/code/logstash/logstash-core/lib/logstash/util/substitution_variables.rb:40:in `deep_replace'", "/Users/robbavey/code/logstash/logstash-core/lib/logstash/config/source/multi_local.rb:33:in `pipeline_configs'", "/Users/robbavey/code/logstash/logstash-core/lib/logstash/config/source_loader.rb:76:in `block in fetch'", "org/jruby/RubyArray.java:2800:in `collect'", "/Users/robbavey/code/logstash/logstash-core/lib/logstash/config/source_loader.rb:75:in `fetch'", "/Users/robbavey/code/logstash/logstash-core/lib/logstash/agent.rb:179:in `converge_state_and_update'", "/Users/robbavey/code/logstash/logstash-core/lib/logstash/agent.rb:117:in `execute'", "/Users/robbavey/code/logstash/logstash-core/lib/logstash/runner.rb:431:in `block in execute'", "/Users/robbavey/code/logstash/vendor/bundle/jruby/3.1.0/gems/stud-0.0.23/lib/stud/task.rb:24:in `block in initialize'"]}
[2024-03-14T14:15:16,949][ERROR][logstash.agent           ] An exception happened when converging configuration {:exception=>RuntimeError, :message=>"Could not fetch the configuration, message: Cannot evaluate `${I_AM_NOT_HERE}`. Replacement variable `I_AM_NOT_HERE` is not defined in a Logstash secret store or as an Environment entry and there is no default value given."}

Note that this does not happen when pipelines are configured in config files, either directly via -f, or referred to in pipelines.yml

[yaauie]

🧐 so we do two passes of substitutions, once on the decoded YAML (where the entire pipeline.string value is just a string), and then again on the pipeline definition (where the variable, if it were left in expanded, would be a comment). In the first pass, we don't have enough context to know that the variable is in a comment.

[robbavey]

The reversion in #16201 means this issue is still present