Add more options for the modules templates (#7582)

* Add more options for the modules templates

This commits add the following new features:

Allow users to use the internal settings classes to have better handling
over the primitives values that a user can use in the modules templates,
by default we are using a `NullableString` which is the more flexible
type.

This change open the way to have more strict validation on the module
templates and will allow us to preprocess configuration to retrieve the
list of the settings and their settings types.

We also add a new type of settings called `SplittableStringArray`, this
class is a subclass or `ArrayCoercible` but target module CLI
configuration. It will take the following form `-M
"arcsight.var.input.hosts=127.0.0.1:3333,192.168.1.23:9999"` and
transform it into a ruby array of string.

We can also add alias of settings values in the configuration by using the
`alias_settings_keys!()`, this is usefull when the language domain is
different from the logstash domain.

* adjust test

logstash-core/lib/logstash/modules/logstash_config.rb

@@ -1,9 +1,9 @@
 # encoding: utf-8
 require "logstash/namespace"
 require_relative "file_reader"
+require "logstash/settings"
 
 module LogStash module Modules class LogStashConfig
-
   # We name it `modul` here because `module` has meaning in Ruby.
   def initialize(modul, settings)
     @directory = ::File.join(modul.directory, "logstash")
@@ -15,21 +15,58 @@ module LogStash module Modules class LogStashConfig
     ::File.join(@directory, "#{@name}.conf.erb")
   end
 
-  def setting(value, default)
-    @settings.fetch(value, default)
+  def configured_inputs(default = [], aliases = {})
+    name = "var.inputs"
+    values = get_setting(LogStash::Setting::SplittableStringArray.new(name, String, default))
+
+    aliases.each { |k,v| values << v if values.include?(k) }
+    aliases.invert.each { |k,v| values << v if values.include?(k) }
+    values.flatten.uniq
+  end
+
+  def alias_settings_keys!(aliases)
+    aliased_settings = alias_matching_keys(aliases, @settings)
+    @settings = alias_matching_keys(aliases.invert, aliased_settings)
+  end
+
+  def array_to_string(array)
+    "[#{array.collect { |i| "'#{i}'" }.join(", ")}]"
+  end
+
+  def get_setting(setting_class)
+    raw_value = @settings[setting_class.name]
+    # If we dont check for NIL, the Settings class will try to coerce the value
+    # and most of the it will fails when a NIL value is explicitely set.
+    # This will be fixed once we wrap the plugins settings into a Settings class
+    setting_class.set(raw_value) unless raw_value.nil?
+    setting_class.value
+  end
+
+  def setting(name, default)
+    # by default we use the more permissive setting which is a `NullableString`
+    # This is fine because the end format of the logstash configuration is a string representation
+    # of the pipeline. There is a good reason why I think we should use the settings classes, we
+    # can `preprocess` a template and generate a configuration from the defined settings
+    # validate the values and replace them in the template.
+    case default
+      when String
+        get_setting(LogStash::Setting::NullableString.new(name, default.to_s))
+      when Numeric
+        get_setting(LogStash::Setting::Numeric.new(name, default))
+      else
+        get_setting(LogStash::Setting::NullableString.new(name, default.to_s))
+      end
   end
 
   def elasticsearch_output_config(type_string = nil)
-    hosts = setting("var.output.elasticsearch.hosts", "localhost:9200").split(',').map do |s|
-      '"' + s.strip + '"'
-    end.join(',')
+    hosts = array_to_string(get_setting(LogStash::Setting::SplittableStringArray.new("var.output.elasticsearch.hosts", String, ["localhost:9200"])))
     index = "#{@name}-#{setting("var.output.elasticsearch.index_suffix", "%{+YYYY.MM.dd}")}"
     password = "#{setting("var.output.elasticsearch.password", "changeme")}"
     user = "#{setting("var.output.elasticsearch.user", "elastic")}"
     document_type_line = type_string ? "document_type => #{type_string}" : ""
     <<-CONF
 elasticsearch {
-hosts => [#{hosts}]
+hosts => #{hosts}
 index => "#{index}"
 password => "#{password}"
 user => "#{user}"
@@ -45,4 +82,31 @@ CONF
     renderer = ERB.new(FileReader.read(template))
     renderer.result(binding)
   end
+
+  private
+  # For a first version we are copying the values of the original hash,
+  # this might become problematic if we users changes the values of the
+  # settings in the template, which could result in an inconsistent view of the original data
+  #
+  # For v1 of the feature I think its an OK compromise, v2 we have a more advanced hash that
+  # support alias.
+  def alias_matching_keys(aliases, target)
+    aliased_target = target.dup
+
+    aliases.each do |matching_key_prefix, new_key_prefix|
+      target.each do |k, v|
+        re = /^#{matching_key_prefix}\./
+
+        if k =~ re
+          alias_key = k.gsub(re, "#{new_key_prefix}.")
+
+          # If the user setup the same values twices with different values lets just halt.
+          raise "Cannot create an alias, the destination key has already a value set: original key: #{k}, alias key: #{alias_key}" if (!aliased_target[alias_key].nil? && aliased_target[alias_key] != v)
+          aliased_target[alias_key] = v unless v.nil?
+        end
+      end
+    end
+
+    aliased_target
+  end
 end end end

logstash-core/lib/logstash/settings.rb

@@ -537,8 +537,27 @@ module LogStash
         end
       end
     end
-  end
 
+    class SplittableStringArray < ArrayCoercible
+      DEFAULT_TOKEN = ","
+
+      def initialize(name, klass, default, strict=true, tokenizer = DEFAULT_TOKEN, &validator_proc)
+        @element_class = klass
+        @token = tokenizer
+        super(name, klass, default, strict, &validator_proc)
+      end
+
+      def coerce(value)
+        if value.is_a?(Array)
+          value
+        elsif value.nil?
+          []
+        else
+          value.split(@token).map(&:strip)
+        end
+      end
+    end
+  end
 
   SETTINGS = Settings.new
 end

logstash-core/spec/logstash/modules/logstash_config_spec.rb

@@ -0,0 +1,56 @@
+# encoding: utf-8
+require "logstash/modules/logstash_config"
+
+describe LogStash::Modules::LogStashConfig do
+  let(:mod) { instance_double("module", :directory => Stud::Temporary.directory, :module_name => "testing") }
+  let(:settings) { {"var.logstash.testing.pants" => "fancy" }}
+  subject { described_class.new(mod, settings) }
+
+  describe "configured inputs" do
+    context "when no inputs is send" do
+      it "returns the default" do
+        expect(subject.configured_inputs(["kafka"])).to include("kafka")
+      end
+    end
+
+    context "when inputs are send" do
+      let(:settings) { { "var.inputs" => "tcp" } }
+
+      it "returns the configured inputs" do
+        expect(subject.configured_inputs(["kafka"])).to include("tcp")
+      end
+
+      context "when alias is specified" do
+        let(:settings) { { "var.inputs" => "smartconnector" } }
+
+        it "returns the configured inputs" do
+          expect(subject.configured_inputs(["kafka"], { "smartconnector" => "tcp"  })).to include("tcp", "smartconnector")
+        end
+      end
+    end
+  end
+
+  describe "array to logstash array string" do
+    it "return an escaped string" do
+      expect(subject.array_to_string(["hello", "ninja"])).to eq("['hello', 'ninja']")
+    end
+  end
+
+  describe "alias modules options" do
+    let(:alias_table) do
+      { "var.logstash.testing" => "var.logstash.better" }
+    end
+
+    before do
+      subject.alias_settings_keys!(alias_table)
+    end
+
+    it "allow to retrieve settings" do
+      expect(subject.setting("var.logstash.better.pants", "dont-exist")).to eq("fancy")
+    end
+
+    it "allow to retrieve settings with the original name" do
+      expect(subject.setting("var.logstash.testing.pants", "dont-exist")).to eq("fancy")
+    end
+  end
+end

logstash-core/spec/logstash/modules/scaffold_spec.rb

@@ -80,7 +80,7 @@ ERB
       expect(test_module.logstash_configuration).not_to be_nil
       config_string = test_module.config_string
       expect(config_string).to include("port => 5606")
-      expect(config_string).to include('hosts => ["es.mycloud.com:9200"]')
+      expect(config_string).to include("hosts => ['es.mycloud.com:9200']")
     end
   end
 

logstash-core/spec/logstash/settings/splittable_string_array_spec.rb

@@ -0,0 +1,51 @@
+# encoding: utf-8
+require "spec_helper"
+require "logstash/settings"
+
+describe LogStash::Setting::SplittableStringArray do
+  let(:element_class) { String }
+  let(:default_value) { [] }
+
+  subject { described_class.new("testing", element_class, default_value) }
+
+  before do
+    subject.set(candidate)
+  end
+
+  context "when giving an array" do
+    let(:candidate) { ["hello,", "ninja"] }
+
+    it "returns the same elements" do
+      expect(subject.value).to match(candidate)
+    end
+  end
+
+  context "when given a string" do
+    context "with 1 element" do
+      let(:candidate) { "hello" }
+
+      it "returns 1 element" do
+        expect(subject.value).to match(["hello"])
+      end
+    end
+
+    context "with multiple element" do
+      let(:candidate) { "hello,ninja" }
+
+      it "returns an array of string" do
+        expect(subject.value).to match(["hello", "ninja"])
+      end
+    end
+  end
+
+  context "when defining a custom tokenizer" do
+    subject { described_class.new("testing", element_class, default_value, strict=true, ";") }
+
+    let(:candidate) { "hello;ninja" }
+
+    it "returns an array of string" do
+      expect(subject.value).to match(["hello", "ninja"])
+    end
+  end
+end
+