From b1b7a79e76a4f25912198f4f5bb54ac9a79db6af Mon Sep 17 00:00:00 2001 From: Ry Biesemeyer Date: Mon, 6 Nov 2017 19:20:00 +0000 Subject: [PATCH] LIR: merge hash attributes of same name to support legacy configurations A [build failure][] of the grok plugin when run through LIR/lscl indicates that there is an expectation for multiple Attributes of the same name to be merged together. This commit ports the failing spec in the plugin to an abstraction that can be tested within logstash-core, and adds a caveat to the LSCL LIR-builder to ensure that we merge the hashes in a way that is compatible with legacy behaviour. NOTE: when multiple Attributes of the same name are used in a single config, it's possible to create configurations that circumvent `AST::Hash`'s ability to report duplicate keys. [build failure]: https://travis-ci.org/logstash-plugins/logstash-filter-grok/builds/293778268 Backports #8597 Fixes #8602 --- logstash-core/lib/logstash/compiler/lscl.rb | 18 ++++++++++-- .../spec/logstash/compiler/compiler_spec.rb | 29 +++++++++++++++++++ 2 files changed, 44 insertions(+), 3 deletions(-) diff --git a/logstash-core/lib/logstash/compiler/lscl.rb b/logstash-core/lib/logstash/compiler/lscl.rb index 6bbba9c8ea1..0516da50339 100644 --- a/logstash-core/lib/logstash/compiler/lscl.rb +++ b/logstash-core/lib/logstash/compiler/lscl.rb @@ -101,9 +101,21 @@ def expr_attributes else [k,v] end - }.reduce({}) do |hash,kv| - k,v = kv - hash[k] = v + }.reduce({}) do |hash, kv| + k, v = kv + existing = hash[k] + if existing.nil? + hash[k] = v + elsif existing.kind_of?(::Hash) + # For legacy reasons, a config can contain multiple `AST::Attribute`s with the same name + # and a hash-type value (e.g., "match" in the grok filter), which are merged into a single + # hash value; e.g., `{"match" => {"baz" => "bar"}, "match" => {"foo" => "bulb"}}` is + # interpreted as `{"match" => {"baz" => "bar", "foo" => "blub"}}`. + # (NOTE: this bypasses `AST::Hash`'s ability to detect duplicate keys) + hash[k] = existing.merge(v) + else + hash[k] = existing + v + end hash end diff --git a/logstash-core/spec/logstash/compiler/compiler_spec.rb b/logstash-core/spec/logstash/compiler/compiler_spec.rb index da018062011..64f11d9ce62 100644 --- a/logstash-core/spec/logstash/compiler/compiler_spec.rb +++ b/logstash-core/spec/logstash/compiler/compiler_spec.rb @@ -193,6 +193,35 @@ def j expect(c_plugin).to ir_eql(j.iPlugin(INPUT, "generator", expected_plugin_args)) end end + + describe "a filter plugin that repeats a Hash directive" do + let(:source) { "input { } filter { #{plugin_source} } output { } " } + subject(:c_plugin) { compiled[:filter] } + + let(:plugin_source) do + %q[ + grok { + match => { "message" => "%{WORD:word}" } + match => { "examplefield" => "%{NUMBER:num}" } + break_on_match => false + } + ] + end + + let(:expected_plugin_args) do + { + "match" => { + "message" => "%{WORD:word}", + "examplefield" => "%{NUMBER:num}" + }, + "break_on_match" => "false" + } + end + + it "should merge the contents of the individual directives" do + expect(c_plugin).to ir_eql(j.iPlugin(FILTER, "grok", expected_plugin_args)) + end + end end context "inputs" do