Add Settings#validate_all and NullableString setting

This allows us to validate all settings after all the settings sources
have been processed (logstash.yml, flags, environment variables, etc)

NullableString is required for validation to pass on what were
previously String settings with nil defaults.

WritableDirectory's strict now defaults false to help with a problem
where the default path.data might not be writable *and* the user could
be specifying --path.data on the command line to compensate. Prior to
this, the default value would be validated and cause Logstash to
terminate on startup because of the default data directory was validated
before the flag override was applied.

To make this validate_all feature more useful, Setting#set will only
call validate if `strict` is true.

Fixes #6004

Fixes #6008

logstash-core/lib/logstash/environment.rb

@@ -17,9 +17,9 @@ module LogStash
 
   [
             Setting::String.new("node.name", Socket.gethostname),
-            Setting::String.new("path.config", nil, false),
+    Setting::NullableString.new("path.config", nil, false),
  Setting::WritableDirectory.new("path.data", ::File.join(LogStash::Environment::LOGSTASH_HOME, "data")),
-            Setting::String.new("config.string", nil, false),
+    Setting::NullableString.new("config.string", nil, false),
            Setting::Boolean.new("config.test_and_exit", false),
            Setting::Boolean.new("config.reload.automatic", false),
            Setting::Numeric.new("config.reload.interval", 3), # in seconds
@@ -31,7 +31,7 @@ module LogStash
            Setting::Numeric.new("pipeline.batch.delay", 5), # in milliseconds
            Setting::Boolean.new("pipeline.unsafe_shutdown", false),
                     Setting.new("path.plugins", Array, []),
-            Setting::String.new("interactive", nil, false),
+    Setting::NullableString.new("interactive", nil, false),
            Setting::Boolean.new("config.debug", false),
             Setting::String.new("log.level", "info", true, ["fatal", "error", "warn", "debug", "info", "trace"]),
            Setting::Boolean.new("version", false),

logstash-core/lib/logstash/runner.rb

@@ -226,6 +226,8 @@ class LogStash::Runner < Clamp::StrictCommand
 
     return start_shell(setting("interactive"), binding) if setting("interactive")
 
+    @settings.validate_all
+
     @settings.format_settings.each {|line| logger.debug(line) }
 
     if setting("config.string").nil? && setting("path.config").nil?

logstash-core/lib/logstash/settings.rb

@@ -95,6 +95,12 @@ module LogStash
       self.merge(flatten_hash(settings))
     end
 
+    def validate_all
+      @settings.each do |name, setting|
+        setting.validate_value
+      end
+    end
+
     private
     def read_yaml(path)
       YAML.safe_load(IO.read(path)) || {}
@@ -123,8 +129,9 @@ module LogStash
       @validator_proc = validator_proc
       @value = nil
       @value_is_set = false
+      @strict = strict
 
-      validate(default) if strict
+      validate(default) if @strict
       @default = default
     end
 
@@ -136,8 +143,12 @@ module LogStash
       @value_is_set
     end
 
+    def strict?
+      @strict
+    end
+
     def set(value)
-      validate(value)
+      validate(value) if @strict
       @value = value
       @value_is_set = true
       @value
@@ -167,12 +178,18 @@ module LogStash
       self.to_hash == other.to_hash
     end
 
-    private
-    def validate(value)
-      if !value.is_a?(@klass)
-        raise ArgumentError.new("Setting \"#{@name}\" must be a #{@klass}. Received: #{value} (#{value.class})")
-      elsif @validator_proc && !@validator_proc.call(value)
-        raise ArgumentError.new("Failed to validate setting \"#{@name}\" with value: #{value}")
+    def validate_value
+      validate(value)
+    end
+
+    protected
+    def validate(input)
+      if !input.is_a?(@klass)
+        raise ArgumentError.new("Setting \"#{@name}\" must be a #{@klass}. Received: #{input} (#{input.class})")
+      end
+
+      if @validator_proc && !@validator_proc.call(input)
+        raise ArgumentError.new("Failed to validate setting \"#{@name}\" with value: #{input}")
       end
     end
 
@@ -351,6 +368,13 @@ module LogStash
       end
     end
 
+    class NullableString < String
+      def validate(value)
+        return if value.nil?
+        super(value)
+      end
+    end
+
     class ExistingFilePath < Setting
       def initialize(name, default=nil, strict=true)
         super(name, ::String, default, strict) do |file_path|
@@ -364,7 +388,7 @@ module LogStash
     end
 
     class WritableDirectory < Setting
-      def initialize(name, default=nil, strict=true)
+      def initialize(name, default=nil, strict=false)
         super(name, ::String, default, strict) do |path|
           if ::File.directory?(path) && ::File.writable?(path)
             true
@@ -378,3 +402,4 @@ module LogStash
 
   SETTINGS = Settings.new
 end
+

logstash-core/spec/logstash/setting_spec.rb

@@ -63,6 +63,7 @@ describe LogStash::Setting do
       end
     end
   end
+
   describe "#set" do
     subject { described_class.new("number", Numeric, 1) }
     it "should change the value of a setting" do
@@ -77,12 +78,33 @@ describe LogStash::Setting do
         expect(subject.set?).to eq(true)
       end
     end
-
     context "when the argument's class does not match @klass" do
       it "should throw an exception" do
         expect { subject.set("not a number") }.to raise_error
       end
     end
+    context "when strict=false" do
+      let(:strict) { false }
+      subject { described_class.new("number", Numeric, 1, strict) }
+      before do
+        expect(subject).not_to receive(:validate)
+      end
+
+      it "should not call #validate" do
+        subject.set(123)
+      end
+    end
+    context "when strict=true" do
+      let(:strict) { true }
+      subject { described_class.new("number", Numeric, 1, strict) }
+      before do
+        expect(subject).to receive(:validate)
+      end
+
+      it "should call #validate" do
+        subject.set(123)
+      end
+    end
   end
 
   describe "#reset" do

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

@@ -4,7 +4,7 @@ require "logstash/settings"
 
 describe LogStash::Setting::String do
   let(:possible_values) { ["a", "b", "c"] }
-  subject { described_class.new("mytext", nil, false, possible_values) }
+  subject { described_class.new("mytext", possible_values.first, true, possible_values) }
   describe "#set" do
     context "when a value is given outside of possible_values" do
       it "should raise an ArgumentError" do

logstash-core/spec/logstash/settings_spec.rb

@@ -59,4 +59,31 @@ describe LogStash::Settings do
       expect(subset.get("num.2")).to eq(1000)
     end
   end
+
+  describe "#validate_all" do
+    subject { described_class.new }
+    let(:numeric_setting_name) { "example" }
+    let(:numeric_setting) { LogStash::Setting.new(numeric_setting_name, Numeric, 1, false) }
+
+    before do
+      subject.register(numeric_setting)
+      subject.set_value(numeric_setting_name, value)
+    end
+
+    context "when any setting is invalid" do
+      let(:value) { "some string" }
+
+      it "should fail" do
+        expect { subject.validate_all }.to raise_error
+      end
+    end
+
+    context "when all settings are valid" do
+      let(:value) { 123 }
+
+      it "should succeed" do
+        expect { subject.validate_all }.not_to raise_error
+      end
+    end
+  end
 end