From 87300694138dab81b473a0d484a92c66f43c697a Mon Sep 17 00:00:00 2001 From: Mashhur Date: Sun, 3 Nov 2024 08:49:57 -0800 Subject: [PATCH 1/2] If config.string contains env in its comments, pipeline may not start if VAR is not set which is meaningless. This commit introduces a logic to remove comments of config.string. Note that other keys, such as pipeline.id are YAML definitions where their comments will be wiped out by . --- .../lib/logstash/config/source/multi_local.rb | 28 +++++++++++++++++- .../config/source/multi_local_spec.rb | 29 ++++++++++++------- 2 files changed, 45 insertions(+), 12 deletions(-) diff --git a/logstash-core/lib/logstash/config/source/multi_local.rb b/logstash-core/lib/logstash/config/source/multi_local.rb index 14697835cba..1ea2b8deaef 100644 --- a/logstash-core/lib/logstash/config/source/multi_local.rb +++ b/logstash-core/lib/logstash/config/source/multi_local.rb @@ -23,6 +23,8 @@ class MultiLocal < Local include LogStash::Util::SubstitutionVariables include LogStash::Util::Loggable + REMOVE_COMMENTS_CONFIG_KEYS = %w(config.string) + def initialize(settings) @original_settings = settings super(settings) @@ -30,7 +32,8 @@ def initialize(settings) end def pipeline_configs - pipelines = deep_replace(retrieve_yaml_pipelines) + yaml_config_without_comments = wipeout_comments(retrieve_yaml_pipelines) + pipelines = deep_replace(yaml_config_without_comments) pipelines_settings = pipelines.map do |pipeline_settings| clone = @original_settings.clone clone.merge_pipeline_settings(pipeline_settings) @@ -134,5 +137,28 @@ def do_warning? end !done end + + # @param [Object] `yaml_config` the input object + # @return [Object] Updated object where its key etries which match the `CONFIG_KEYS_TO_WIPE_OUT_COMMENTS` do not contain comments + def wipeout_comments(yaml_config) + case yaml_config + when Hash + yaml_config.each do |key, val| + yaml_config[key.to_s] = wipeout_comments(val) if REMOVE_COMMENTS_CONFIG_KEYS.include?(key) + end + when Array + yaml_config.map { |val| wipeout_comments(val) } + when String + yaml_config.lines.map do |line| + if line.strip.start_with?("#") + "" + else + line.match?(/(? 5555 # intentional comment contains \"${UDP_DEV_PORT}\" variable, shouldn't break functionalities -# host => \"127.0.0.1\" -# } -# # another intentional comment contains \"${UDP_PROD_HOST}\" variable, shouldn't break functionalities -# } -# output {}" -# } - let(:config_string) { "input {} output {}" } + let(:config_string) { + "input { + udp { + port => 5555 # intentional comment contains \"${UDP_DEV_PORT}\" variable, shouldn't break functionalities + host => \"127.0.0.1\" + } + # another intentional comment contains \"${UDP_PROD_HOST}\" variable, shouldn't break functionalities + } + output {}" + } let(:retrieved_pipelines) do [ { "pipeline.id" => "main", "config.string" => config_string }, @@ -175,6 +174,14 @@ expect(configs.last).to be_a(::LogStash::Config::PipelineConfig) end + it "wipes out the comments of `config.string`" do + expectation = "input {\n\n udp {\n\n port => 5555\n\n host => \"127.0.0.1\"\n\n }\n\n\n }\n\n output {}" + config_without_comments = subject.send(:wipeout_comments, retrieved_pipelines) + config_without_comments.each do |config_line| + expect(config_line['config.string']).to match expectation + end + end + context "using non pipeline related settings" do let(:retrieved_pipelines) do [ { "pipeline.id" => "main", "config.string" => "", "api.http.port" => 22222 }, From 1615cb724bac8d0ef366b0b74fb927fa758ed650 Mon Sep 17 00:00:00 2001 From: Mashhur Date: Sat, 16 Nov 2024 08:47:34 -0800 Subject: [PATCH 2/2] Consider if config contains comment like symbol ' #' in its real fields but not a comment in a reality. --- .../lib/logstash/config/source/multi_local.rb | 53 +++++++++++++++---- .../config/source/multi_local_spec.rb | 38 ++++++++++--- 2 files changed, 72 insertions(+), 19 deletions(-) diff --git a/logstash-core/lib/logstash/config/source/multi_local.rb b/logstash-core/lib/logstash/config/source/multi_local.rb index 1ea2b8deaef..bac97dca95c 100644 --- a/logstash-core/lib/logstash/config/source/multi_local.rb +++ b/logstash-core/lib/logstash/config/source/multi_local.rb @@ -24,6 +24,7 @@ class MultiLocal < Local include LogStash::Util::Loggable REMOVE_COMMENTS_CONFIG_KEYS = %w(config.string) + CONFIG_LINE_COMMENT = " #" def initialize(settings) @original_settings = settings @@ -32,7 +33,7 @@ def initialize(settings) end def pipeline_configs - yaml_config_without_comments = wipeout_comments(retrieve_yaml_pipelines) + yaml_config_without_comments = remove_comments(retrieve_yaml_pipelines) pipelines = deep_replace(yaml_config_without_comments) pipelines_settings = pipelines.map do |pipeline_settings| clone = @original_settings.clone @@ -138,24 +139,54 @@ def do_warning? !done end + ESCAPE_CHAR = "\\" + + def remove_comment(config_line = "") + refined_line = config_line + comment_indices = [] + comment_idx = -1 + while (comment_idx = config_line.index(CONFIG_LINE_COMMENT, comment_idx + 1)) != nil + comment_indices << comment_idx + end + + double_quotes_count = 0 + single_quotes_count = 0 + config_line.chars.each_with_index do |char, i| + if char == '"' && (config_line[i - ESCAPE_CHAR.length] != ESCAPE_CHAR || (config_line[i - ESCAPE_CHAR.length] == ESCAPE_CHAR && config_line[i - 2 * ESCAPE_CHAR.length] == ESCAPE_CHAR)) && double_quotes_count == 0 && single_quotes_count == 0 + double_quotes_count += 1 unless comment_indices.include?(i) + elsif char == '"' && (config_line[i - ESCAPE_CHAR.length] != ESCAPE_CHAR || (config_line[i - ESCAPE_CHAR.length] == ESCAPE_CHAR && config_line[i - 2 * ESCAPE_CHAR.length] == ESCAPE_CHAR)) && double_quotes_count == 1 && single_quotes_count == 0 + double_quotes_count -= 1 unless comment_indices.include?(i) + end + + if char == "'" && (config_line[i - ESCAPE_CHAR.length] != ESCAPE_CHAR || (config_line[i - ESCAPE_CHAR.length] == ESCAPE_CHAR && config_line[i - 2 * ESCAPE_CHAR.length] == ESCAPE_CHAR)) && double_quotes_count == 0 && single_quotes_count == 0 + single_quotes_count += 1 unless comment_indices.include?(i) + elsif char == "'" && (config_line[i - ESCAPE_CHAR.length] != ESCAPE_CHAR || (config_line[i - ESCAPE_CHAR.length] == ESCAPE_CHAR && config_line[i - 2 * ESCAPE_CHAR.length] == ESCAPE_CHAR)) && double_quotes_count == 0 && single_quotes_count == 1 + single_quotes_count -= 1 unless comment_indices.include?(i) + end + + if comment_indices.include?(i) && double_quotes_count == 0 && single_quotes_count == 0 + refined_line = config_line[0...i] + break + end + end + refined_line + end + # @param [Object] `yaml_config` the input object # @return [Object] Updated object where its key etries which match the `CONFIG_KEYS_TO_WIPE_OUT_COMMENTS` do not contain comments - def wipeout_comments(yaml_config) + def remove_comments(yaml_config) case yaml_config when Hash yaml_config.each do |key, val| - yaml_config[key.to_s] = wipeout_comments(val) if REMOVE_COMMENTS_CONFIG_KEYS.include?(key) + yaml_config[key.to_s] = remove_comments(val) if REMOVE_COMMENTS_CONFIG_KEYS.include?(key) end when Array - yaml_config.map { |val| wipeout_comments(val) } + yaml_config.map { |val| remove_comments(val) } when String - yaml_config.lines.map do |line| - if line.strip.start_with?("#") - "" - else - line.match?(/(? 5555 # intentional comment contains \"${UDP_DEV_PORT}\" variable, shouldn't break functionalities + beats { + port => 5555 # intentional comment contains \"${BEATS_DEV_PORT}\" variable, shouldn't break functionalities host => \"127.0.0.1\" } - # another intentional comment contains \"${UDP_PROD_HOST}\" variable, shouldn't break functionalities + # another intentional comment contains \"${BEATS_PROD_HOST}\" variable, shouldn't break functionalities + } + filter { + mutate { add_field => { \"oopsy\" => \"This is real data # not a ' comment, oopsy\" } } + mutate { + add_field => { + 'doopsy' => \"This is real data # not a ' comment, doopsy\" # pipeline.workers: \"${PIPELINE_WORKERS}\" + } + add_tag => [ \"doopsy_tag\", \"${ENV_TAG}\" ] # \"${ENV_TAG_ANOTHER} + } + mutate { + add_field => { + \"hoopsy\" => + 'This is real data # not a \" comment, hoopsy' + } + } + mutate { add_field => { \"loopsy\" => \"This is real data # not a ' escape comment, \\\" loopsy\" } } + mutate { add_field => { \"woopsy\" => 'This is real data # not a \" comment, ${ENV_TAG_ANOTHER} whoopsy' } } # \"${ENV_TAG_ANOTHER} } } } - output {}" + output { + stdout { } # output + } # pipeline.workers: \"${PIPELINE_WORKERS}\" + # pipeline.workers: \"${PIPELINE_WORKERS}\" + " } let(:retrieved_pipelines) do [ @@ -174,9 +196,9 @@ expect(configs.last).to be_a(::LogStash::Config::PipelineConfig) end - it "wipes out the comments of `config.string`" do - expectation = "input {\n\n udp {\n\n port => 5555\n\n host => \"127.0.0.1\"\n\n }\n\n\n }\n\n output {}" - config_without_comments = subject.send(:wipeout_comments, retrieved_pipelines) + it "removes comments from `config.string`" do + expectation = "input {\n beats {\n port => 5555 host => \"127.0.0.1\"\n }\n\n }\n filter {\n mutate { add_field => { \"oopsy\" => \"This is real data # not a ' comment, oopsy\" } }\n mutate {\n add_field => {\n 'doopsy' => \"This is real data # not a ' comment, doopsy\" }\n add_tag => [ \"doopsy_tag\", \"${ENV_TAG}\" ] }\n mutate {\n add_field => {\n \"hoopsy\" =>\n 'This is real data # not a \" comment, hoopsy'\n }\n }\n mutate { add_field => { \"loopsy\" => \"This is real data # not a ' escape comment, \\\" loopsy\" } }\n mutate { add_field => { \"woopsy\" => 'This is real data # not a \" comment, ${ENV_TAG_ANOTHER} whoopsy' } } }\n output {\n stdout { } }\n " + config_without_comments = subject.send(:remove_comments, retrieved_pipelines) config_without_comments.each do |config_line| expect(config_line['config.string']).to match expectation end