Fixes the issue where LS wipes out all quotes from docker env variabl…

…es. (#16456)

* Fixes the issue where LS wipes out all quotes from docker env variables. This is an issue when running LS on docker with CONFIG_STRING, needs to keep quotes with env variable.

* Add a docker acceptance integration test.

logstash-core/lib/logstash/util/substitution_variables.rb

@@ -87,15 +87,33 @@ def replace_placeholders(value, refine)
 
     # ENV ${var} value may carry single quote or escaped double quote
     # or single/double quoted entries in array string, needs to be refined
-    refined_value = placeholder_value.gsub(/[\\"\\']/, '')
+    refined_value = strip_enclosing_char(strip_enclosing_char(placeholder_value, "'"), '"')
     if refined_value.start_with?('[') && refined_value.end_with?(']')
       # remove square brackets, split by comma and cleanup leading/trailing whitespace
-      refined_value[1..-2].split(',').map(&:strip)
+      refined_array = refined_value[1..-2].split(',').map(&:strip)
+      refined_array.each_with_index do |str, index|
+        refined_array[index] = strip_enclosing_char(strip_enclosing_char(str, "'"), '"')
+      end
+      refined_array
     else
       refined_value
     end
   end # def replace_placeholders
 
+  private
+
+  # removes removed_char of string_value if string_value is wrapped with removed_char
+  def strip_enclosing_char(string_value, remove_char)
+    return string_value unless string_value.is_a?(String)
+    return string_value if string_value.empty?
+
+    if string_value.start_with?(remove_char) && string_value.end_with?(remove_char)
+      string_value[1..-2] # Remove the first and last characters
+    else
+      string_value
+    end
+  end
+
   class << self
     private
 

logstash-core/spec/logstash/util/substitution_variables_spec.rb

@@ -0,0 +1,63 @@
+# Licensed to Elasticsearch B.V. under one or more contributor
+# license agreements. See the NOTICE file distributed with
+# this work for additional information regarding copyright
+# ownership. Elasticsearch B.V. licenses this file to you under
+# the Apache License, Version 2.0 (the "License"); you may
+# not use this file except in compliance with the License.
+# You may obtain a copy of the License at
+#
+#  http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied.  See the License for the
+# specific language governing permissions and limitations
+# under the License.
+
+require "spec_helper"
+require "logstash/util/substitution_variables"
+
+describe LogStash::Util::SubstitutionVariables do
+
+  subject { Class.new { extend LogStash::Util::SubstitutionVariables } }
+
+  context "ENV or Keystore ${VAR} with single/double quotes" do
+    # single or double quotes come from ENV/Keystore ${VAR} value
+    let(:xpack_monitoring_host) { '"http://node1:9200"' }
+    let(:xpack_monitoring_hosts) { "'[\"http://node3:9200\", \"http://node4:9200\"]'" }
+    let(:xpack_management_pipeline_id) { '"*"' }
+    let(:config_string) {
+      "'input {
+        stdin { }
+        beats { port => 5040 }
+      }
+      output {
+        elasticsearch {
+          hosts => [\"https://es:9200\"]
+          user => \"elastic\"
+          password => 'changeme'
+        }
+      }'"
+    }
+
+    # this happens mostly when running LS with docker
+    it "stripes out quotes" do
+      expect(subject.send(:strip_enclosing_char, xpack_monitoring_host, '"')).to eql('http://node1:9200')
+      expect(subject.send(:strip_enclosing_char, xpack_monitoring_hosts, "'")).to eql('["http://node3:9200", "http://node4:9200"]')
+      expect(subject.send(:strip_enclosing_char, xpack_management_pipeline_id, '"')).to eql('*')
+      # make sure we keep the hosts, user and password param enclosed quotes
+      expect(subject.send(:strip_enclosing_char, config_string, "'")).to eql('input {
+        stdin { }
+        beats { port => 5040 }
+      }
+      output {
+        elasticsearch {
+          hosts => ["https://es:9200"]
+          user => "elastic"
+          password => \'changeme\'
+        }
+      }')
+    end
+  end
+end

qa/docker/shared_examples/container_options.rb

@@ -56,4 +56,40 @@
       expect(get_settings(@container)['pipeline.unsafe_shutdown']).to be_truthy
     end
   end
+
+  context 'when setting config.string' do
+    let(:options) {
+      {
+        'ENV' => [
+          'USER=kimchy',
+          'CONFIG_STRING=input {
+              beats { port => 5040 }
+            }
+            output {
+              elasticsearch {
+                hosts => ["https://es:9200"]
+                user => "${USER}"
+                password => \'changeme\'
+              }
+            }'
+        ]
+      }
+    }
+
+    it "persists ${CONFIG_STRING} key in logstash.yml, resolves when running and spins up without issue" do
+      settings = get_settings(@container)
+      expect(settings['config.string']).to eq("${CONFIG_STRING}")
+
+      pipeline_config = get_pipeline_stats(@container)
+      input_plugins = pipeline_config.dig('plugins', 'inputs')
+      expect(input_plugins[0].dig('name')).to eql('beats')
+
+      output_plugins = pipeline_config.dig('plugins', 'outputs')
+      expect(output_plugins[0].dig('name')).to eql('elasticsearch')
+
+      # check if logs contain the ES request with the resolved ${USER}
+      container_logs = @container.logs(stdout: true)
+      expect(container_logs.include?('https://kimchy:xxxxxx@es:9200')).to be true
+    end
+  end
 end

qa/docker/shared_examples/xpack.rb

@@ -35,7 +35,7 @@
         ]
       }
 
-      it 'persists var keys into logstas.yaml and uses their resolved actual values' do
+      it 'persists var keys into logstash.yml and uses their resolved actual values' do
         container = create_container(@image, {'ENV' => env})
 
         sleep(15) # wait for container run