From 7c64c7394bf47e8b5316710876ed55350df46d61 Mon Sep 17 00:00:00 2001 From: Mashhur <99575341+mashhurs@users.noreply.github.com> Date: Tue, 17 Sep 2024 06:46:19 -0700 Subject: [PATCH] Fixes the issue where LS wipes out all quotes from docker env variables. (#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/util/substitution_variables.rb | 22 ++++++- .../util/substitution_variables_spec.rb | 63 +++++++++++++++++++ .../shared_examples/container_options.rb | 36 +++++++++++ qa/docker/shared_examples/xpack.rb | 2 +- 4 files changed, 120 insertions(+), 3 deletions(-) create mode 100644 logstash-core/spec/logstash/util/substitution_variables_spec.rb diff --git a/logstash-core/lib/logstash/util/substitution_variables.rb b/logstash-core/lib/logstash/util/substitution_variables.rb index 4319a38235f..61b938503e5 100644 --- a/logstash-core/lib/logstash/util/substitution_variables.rb +++ b/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 diff --git a/logstash-core/spec/logstash/util/substitution_variables_spec.rb b/logstash-core/spec/logstash/util/substitution_variables_spec.rb new file mode 100644 index 00000000000..13c5c93c43b --- /dev/null +++ b/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 \ No newline at end of file diff --git a/qa/docker/shared_examples/container_options.rb b/qa/docker/shared_examples/container_options.rb index 1d5108bff94..48c51d382b9 100644 --- a/qa/docker/shared_examples/container_options.rb +++ b/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 diff --git a/qa/docker/shared_examples/xpack.rb b/qa/docker/shared_examples/xpack.rb index 82ff64ae065..254e0354ca9 100644 --- a/qa/docker/shared_examples/xpack.rb +++ b/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