Skip to content

Commit

Permalink
make e and f flags mutually exclusive
Browse files Browse the repository at this point in the history
  • Loading branch information
jsvd committed May 8, 2017
1 parent 6b28223 commit a20d2df
Show file tree
Hide file tree
Showing 6 changed files with 38 additions and 58 deletions.
4 changes: 4 additions & 0 deletions logstash-core/lib/logstash/bootstrap_check/default_config.rb
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,10 @@ def self.check(settings)
raise LogStash::BootstrapCheckError, I18n.t("logstash.runner.missing-configuration")
end

if settings.get("config.string") && settings.get("path.config")
raise LogStash::BootstrapCheckError, I18n.t("logstash.runner.config-string-path-exclusive")
end

if settings.get("config.reload.automatic") && settings.get("path.config").nil?
# there's nothing to reload
raise LogStash::BootstrapCheckError, I18n.t("logstash.runner.reload-without-config-path")
Expand Down
25 changes: 16 additions & 9 deletions logstash-core/lib/logstash/config/source/local.rb
Original file line number Diff line number Diff line change
Expand Up @@ -132,20 +132,23 @@ def self.read(uri)
OUTPUT_BLOCK_RE = /output *{/

def pipeline_configs
config_parts = []

config_parts.concat(ConfigStringLoader.read(config_string)) if config_string?
if local_config?
local_config_parts = ConfigPathLoader.read(config_path)
config_parts.concat(local_config_parts)
else
local_config_parts = []
unless mutually_exclusive(config_string?, local_config?, remote_config?)
raise ConfigurationError.new("Settings 'config.string' and 'path.config' can't be used simultaneously.")
end

config_parts.concat(ConfigRemoteLoader.read(config_path)) if remote_config?
config_parts = if config_string?
ConfigStringLoader.read(config_string)
elsif local_config?
ConfigPathLoader.read(config_path)
elsif remote_config?
ConfigRemoteLoader.read(config_path)
else
[]
end

return if config_parts.empty?
return if config_string? && config_string.strip.empty? && local_config? && local_config_parts.empty?
return if config_string? && config_string.strip.empty?

add_missing_default_inputs_or_outputs(config_parts)

Expand Down Expand Up @@ -211,5 +214,9 @@ def remote_config?
false
end
end

def mutually_exclusive(a, b, c)
(a ^ b ^ c) && !(a && b && c)
end
end
end end end
2 changes: 2 additions & 0 deletions logstash-core/locales/en.yml
Original file line number Diff line number Diff line change
Expand Up @@ -98,6 +98,8 @@ en:
missing-configuration: >-
No configuration file was specified. Perhaps you forgot to provide
the '-f yourlogstash.conf' flag?
config-string-path-exclusive:
Settings 'path.config' (-f) and 'config.string' (-e) can't be used simultaneously.
reload-without-config-path: >-
Configuration reloading also requires passing a configuration path with '-f yourlogstash.conf'
locked-data-path: >-
Expand Down
23 changes: 6 additions & 17 deletions logstash-core/spec/logstash/agent_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -100,13 +100,8 @@
end

context "when auto_reload is false" do
let(:agent_args) do
{
"config.reload.automatic" => false,
"path.config" => config_file
}
end

let(:agent_settings) { mock_settings("config.reload.automatic" => false) }
let(:agent_args) { { "path.config" => config_file } }

context "if state is clean" do
before :each do
Expand Down Expand Up @@ -222,16 +217,10 @@
end

context "when auto_reload is true" do
let(:agent_settings) { mock_settings("config.reload.automatic" => true, "config.reload.interval" => 0.01) }
subject { described_class.new(agent_settings, default_source_loader) }

let(:agent_args) do
{
"config.string" => "",
"config.reload.automatic" => true,
"config.reload.interval" => 0.01,
"path.config" => config_file
}
end
let(:agent_args) { { "path.config" => config_file } }

context "if state is clean" do
it "should periodically reload_state" do
Expand Down Expand Up @@ -406,8 +395,8 @@
end

context "metrics after config reloading" do
let(:temporary_file) { Stud::Temporary.file.path }
let(:config) { "input { generator { } } output { file { path => '#{temporary_file}' } }" }
let(:agent_settings) { mock_settings({}) }
let!(:config) { "input { generator { } } output { dummyoutput { } }" }

let(:config_path) do
f = Stud::Temporary.file
Expand Down
9 changes: 5 additions & 4 deletions logstash-core/spec/logstash/config/source/local_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -292,8 +292,9 @@
)
end

it "returns a merged config" do
expect(subject.pipeline_configs.first.config_string).to include(input_block, output_block, filter_block)
# this should be impossible as the bootstrap checks should catch this
it "raises an exception" do
expect { subject.pipeline_configs }.to raise_error
end
end

Expand Down Expand Up @@ -353,8 +354,8 @@
)
end

it "returns a merged config" do
expect(subject.pipeline_configs.first.config_string).to include(input_block, filter_block)
it "raises an exception" do
expect { subject.pipeline_configs }.to raise_error
end
end
end
Expand Down
33 changes: 5 additions & 28 deletions qa/integration/specs/01_logstash_bin_smoke_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -128,36 +128,13 @@
expect(@ls1.get_version.strip).to eq("logstash #{expected['logstash']}")
end

it "should still merge when -e is specified and -f has no valid config files" do
it "should abort if both -f and -e are specified" do
config_string = "input { tcp { port => #{port1} } }"
@ls1.spawn_logstash("-e", config_string, "-f" "/tmp/foobartest")
@ls1.wait_for_logstash

try(20) do
expect(is_port_open?(port1)).to be true
end
end

it "should not start when -e is not specified and -f has no valid config files" do
@ls2.spawn_logstash("-e", "", "-f" "/tmp/foobartest")
try(num_retries) do
expect(is_port_open?(9600)).to be_falsey
end
end

it "should merge config_string when both -f and -e is specified" do
config_string = "input { tcp { port => #{port1} } }"
@ls1.spawn_logstash("-e", config_string, "-f", config3)
@ls1.wait_for_logstash

# Both ports should be reachable
try(20) do
expect(is_port_open?(port1)).to be true
end

@ls1.spawn_logstash("-e", config_string, "-f", config2)
try(20) do
expect(is_port_open?(port3)).to be true
expect(@ls1.exited?).to be(true)
end
expect(@ls1.exit_code).to be(1)
end

def get_id
Expand All @@ -168,7 +145,7 @@ def get_id
config_string = "input { tcp { port => #{port1} } }"

start_ls = lambda {
@ls1.spawn_logstash("-e", config_string, "-f", config3)
@ls1.spawn_logstash("-e", config_string)
@ls1.wait_for_logstash
}
start_ls.call()
Expand Down

0 comments on commit a20d2df

Please sign in to comment.