Skip to content

Commit 7bc55ba

Browse files
andselandsel
committed
[Backport 8.x] Reimplement LogStash::String setting in Java (#16576) (#16959)
Non clean backport of #16576 ---- Reimplements `LogStash::Setting::String` Ruby setting class into the `org.logstash.settings.SettingString` and exposes it through `java_import` as `LogStash::Setting::SettingString`. Updates the rspec tests in two ways: - logging mock is now converted to real Log4J appender that spy log line that are later verified - verifies `java.lang.IllegalArgumentException` instead of `ArgumentError` is thrown because the kind of exception thrown by Java code, during verification. * Fixed the rename of NullableString to SettingNullableString * Fixed runner test to use real spy logger from Java Settings instead of mock test double
1 parent 1adecfe commit 7bc55ba

22 files changed

Lines changed: 390 additions & 117 deletions

File tree

‎logstash-core/lib/logstash/environment.rb‎

Lines changed: 25 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -35,10 +35,10 @@ module Environment
3535

3636
[
3737
Setting::Boolean.new("allow_superuser", true),
38-
Setting::String.new("node.name", Socket.gethostname),
39-
Setting::NullableString.new("path.config", nil, false),
38+
Setting::SettingString.new("node.name", Socket.gethostname),
39+
Setting::SettingNullableString.new("path.config", nil, false),
4040
Setting::WritableDirectory.new("path.data", ::File.join(LogStash::Environment::LOGSTASH_HOME, "data")),
41-
Setting::NullableString.new("config.string", nil, false),
41+
Setting::SettingNullableString.new("config.string", nil, false),
4242
Setting::Modules.new("modules.cli", LogStash::Util::ModulesSettingArray, []),
4343
Setting::Modules.new("modules", LogStash::Util::ModulesSettingArray, []),
4444
Setting.new("modules_list", Array, []),
@@ -50,10 +50,10 @@ module Environment
5050
Setting::Boolean.new("config.reload.automatic", false),
5151
Setting::TimeValue.new("config.reload.interval", "3s"), # in seconds
5252
Setting::Boolean.new("config.support_escapes", false),
53-
Setting::String.new("config.field_reference.escape_style", "none", true, %w(none percent ampersand)),
54-
Setting::String.new("event_api.tags.illegal", "rename", true, %w(rename warn)),
53+
Setting::SettingString.new("config.field_reference.escape_style", "none", true, %w(none percent ampersand)),
54+
Setting::SettingString.new("event_api.tags.illegal", "rename", true, %w(rename warn)),
5555
Setting::Boolean.new("metric.collect", true),
56-
Setting::String.new("pipeline.id", "main"),
56+
Setting::SettingString.new("pipeline.id", "main"),
5757
Setting::Boolean.new("pipeline.system", false),
5858
Setting::PositiveInteger.new("pipeline.workers", LogStash::Config::CpuCoreStrategy.maximum),
5959
Setting::PositiveInteger.new("pipeline.batch.size", 125),
@@ -65,32 +65,32 @@ module Environment
6565
Setting::CoercibleString.new("pipeline.ordered", "auto", true, ["auto", "true", "false"]),
6666
Setting::CoercibleString.new("pipeline.ecs_compatibility", "v8", true, %w(disabled v1 v8)),
6767
Setting.new("path.plugins", Array, []),
68-
Setting::NullableString.new("interactive", nil, false),
68+
Setting::SettingNullableString.new("interactive", nil, false),
6969
Setting::Boolean.new("config.debug", false),
70-
Setting::String.new("log.level", "info", true, ["fatal", "error", "warn", "debug", "info", "trace"]),
70+
Setting::SettingString.new("log.level", "info", true, ["fatal", "error", "warn", "debug", "info", "trace"]),
7171
Setting::Boolean.new("version", false),
7272
Setting::Boolean.new("help", false),
7373
Setting::Boolean.new("enable-local-plugin-development", false),
74-
Setting::String.new("log.format", "plain", true, ["json", "plain"]),
74+
Setting::SettingString.new("log.format", "plain", true, ["json", "plain"]),
7575
Setting::Boolean.new("log.format.json.fix_duplicate_message_fields", false),
7676
Setting::Boolean.new("api.enabled", true).with_deprecated_alias("http.enabled"),
77-
Setting::String.new("api.http.host", "127.0.0.1").with_deprecated_alias("http.host"),
77+
Setting::SettingString.new("api.http.host", "127.0.0.1").with_deprecated_alias("http.host"),
7878
Setting::PortRange.new("api.http.port", 9600..9700).with_deprecated_alias("http.port"),
79-
Setting::String.new("api.environment", "production").with_deprecated_alias("http.environment"),
80-
Setting::String.new("api.auth.type", "none", true, %w(none basic)),
81-
Setting::String.new("api.auth.basic.username", nil, false).nullable,
79+
Setting::SettingString.new("api.environment", "production").with_deprecated_alias("http.environment"),
80+
Setting::SettingString.new("api.auth.type", "none", true, %w(none basic)),
81+
Setting::SettingString.new("api.auth.basic.username", nil, false).nullable,
8282
Setting::Password.new("api.auth.basic.password", nil, false).nullable,
83-
Setting::String.new("api.auth.basic.password_policy.mode", "WARN", true, %w[WARN ERROR]),
83+
Setting::SettingString.new("api.auth.basic.password_policy.mode", "WARN", true, %w[WARN ERROR]),
8484
Setting::Numeric.new("api.auth.basic.password_policy.length.minimum", 8),
85-
Setting::String.new("api.auth.basic.password_policy.include.upper", "REQUIRED", true, %w[REQUIRED OPTIONAL]),
86-
Setting::String.new("api.auth.basic.password_policy.include.lower", "REQUIRED", true, %w[REQUIRED OPTIONAL]),
87-
Setting::String.new("api.auth.basic.password_policy.include.digit", "REQUIRED", true, %w[REQUIRED OPTIONAL]),
88-
Setting::String.new("api.auth.basic.password_policy.include.symbol", "OPTIONAL", true, %w[REQUIRED OPTIONAL]),
85+
Setting::SettingString.new("api.auth.basic.password_policy.include.upper", "REQUIRED", true, %w[REQUIRED OPTIONAL]),
86+
Setting::SettingString.new("api.auth.basic.password_policy.include.lower", "REQUIRED", true, %w[REQUIRED OPTIONAL]),
87+
Setting::SettingString.new("api.auth.basic.password_policy.include.digit", "REQUIRED", true, %w[REQUIRED OPTIONAL]),
88+
Setting::SettingString.new("api.auth.basic.password_policy.include.symbol", "OPTIONAL", true, %w[REQUIRED OPTIONAL]),
8989
Setting::Boolean.new("api.ssl.enabled", false),
9090
Setting::ExistingFilePath.new("api.ssl.keystore.path", nil, false).nullable,
9191
Setting::Password.new("api.ssl.keystore.password", nil, false).nullable,
9292
Setting::StringArray.new("api.ssl.supported_protocols", nil, true, %w[TLSv1 TLSv1.1 TLSv1.2 TLSv1.3]),
93-
Setting::String.new("queue.type", "memory", true, ["persisted", "memory"]),
93+
Setting::SettingString.new("queue.type", "memory", true, ["persisted", "memory"]),
9494
Setting::Boolean.new("queue.drain", false),
9595
Setting::Bytes.new("queue.page_capacity", "64mb"),
9696
Setting::Bytes.new("queue.max_bytes", "1024mb"),
@@ -102,16 +102,16 @@ module Environment
102102
Setting::Boolean.new("dead_letter_queue.enable", false),
103103
Setting::Bytes.new("dead_letter_queue.max_bytes", "1024mb"),
104104
Setting::Numeric.new("dead_letter_queue.flush_interval", 5000),
105-
Setting::String.new("dead_letter_queue.storage_policy", "drop_newer", true, ["drop_newer", "drop_older"]),
106-
Setting::NullableString.new("dead_letter_queue.retain.age"), # example 5d
105+
Setting::SettingString.new("dead_letter_queue.storage_policy", "drop_newer", true, ["drop_newer", "drop_older"]),
106+
Setting::SettingNullableString.new("dead_letter_queue.retain.age"), # example 5d
107107
Setting::TimeValue.new("slowlog.threshold.warn", "-1"),
108108
Setting::TimeValue.new("slowlog.threshold.info", "-1"),
109109
Setting::TimeValue.new("slowlog.threshold.debug", "-1"),
110110
Setting::TimeValue.new("slowlog.threshold.trace", "-1"),
111-
Setting::String.new("keystore.classname", "org.logstash.secret.store.backend.JavaKeyStore"),
112-
Setting::String.new("keystore.file", ::File.join(::File.join(LogStash::Environment::LOGSTASH_HOME, "config"), "logstash.keystore"), false), # will be populated on
113-
Setting::NullableString.new("monitoring.cluster_uuid"),
114-
Setting::String.new("pipeline.buffer.type", nil, false, ["direct", "heap"])
111+
Setting::SettingString.new("keystore.classname", "org.logstash.secret.store.backend.JavaKeyStore"),
112+
Setting::SettingString.new("keystore.file", ::File.join(::File.join(LogStash::Environment::LOGSTASH_HOME, "config"), "logstash.keystore"), false), # will be populated on
113+
Setting::SettingNullableString.new("monitoring.cluster_uuid"),
114+
Setting::SettingString.new("pipeline.buffer.type", nil, false, ["direct", "heap"])
115115
# post_process
116116
].each {|setting| SETTINGS.register(setting) }
117117

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -69,13 +69,13 @@ def setting(name, default)
6969
# validate the values and replace them in the template.
7070
case default
7171
when String
72-
get_setting(LogStash::Setting::NullableString.new(name, default.to_s))
72+
get_setting(LogStash::Setting::SettingNullableString.new(name, default.to_s))
7373
when Numeric
7474
get_setting(LogStash::Setting::Numeric.new(name, default))
7575
when true, false
7676
get_setting(LogStash::Setting::Boolean.new(name, default))
7777
else
78-
get_setting(LogStash::Setting::NullableString.new(name, default.to_s))
78+
get_setting(LogStash::Setting::SettingNullableString.new(name, default.to_s))
7979
end
8080
end
8181

‎logstash-core/lib/logstash/settings.rb‎

Lines changed: 5 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -523,27 +523,10 @@ def validate(value)
523523
@validator_class.validate(value)
524524
end
525525
end
526+
527+
java_import org.logstash.settings.SettingString
526528

527-
class String < Setting
528-
def initialize(name, default = nil, strict = true, possible_strings = [])
529-
@possible_strings = possible_strings
530-
super(name, ::String, default, strict)
531-
end
532-
533-
def validate(value)
534-
super(value)
535-
unless @possible_strings.empty? || @possible_strings.include?(value)
536-
raise ArgumentError.new("Invalid value \"#{name}: #{value}\". Options are: #{@possible_strings.inspect}")
537-
end
538-
end
539-
end
540-
541-
class NullableString < String
542-
def validate(value)
543-
return if value.nil?
544-
super(value)
545-
end
546-
end
529+
java_import org.logstash.settings.SettingNullableString
547530

548531
class Password < Coercible
549532
def initialize(name, default = nil, strict = true)
@@ -824,6 +807,8 @@ def validate(value)
824807
end
825808
end
826809

810+
java_import org.logstash.settings.NullableSetting
811+
827812
# @see Setting#nullable
828813
# @api internal
829814
class Nullable < SimpleDelegator

‎logstash-core/lib/logstash/util/settings_helper.rb‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -33,8 +33,8 @@ module LogStash::Util::SettingsHelper
3333
# The `path.settings` and `path.logs` can not be defined in "logstash/environment" since the `Environment::LOGSTASH_HOME` doesn't
3434
# exist unless launched via "bootstrap/environment"
3535
def self.pre_process
36-
LogStash::SETTINGS.register(LogStash::Setting::String.new("path.settings", ::File.join(LogStash::Environment::LOGSTASH_HOME, "config")))
37-
LogStash::SETTINGS.register(LogStash::Setting::String.new("path.logs", ::File.join(LogStash::Environment::LOGSTASH_HOME, "logs")))
36+
LogStash::SETTINGS.register(LogStash::Setting::SettingString.new("path.settings", ::File.join(LogStash::Environment::LOGSTASH_HOME, "config")))
37+
LogStash::SETTINGS.register(LogStash::Setting::SettingString.new("path.logs", ::File.join(LogStash::Environment::LOGSTASH_HOME, "logs")))
3838
end
3939

4040
# Ensure that any settings are re-calculated after loading yaml

‎logstash-core/spec/logstash/api/commands/default_metadata_spec.rb‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -35,8 +35,8 @@ def registerIfNot(setting)
3535
before :all do
3636
registerIfNot(LogStash::Setting::Boolean.new("xpack.monitoring.enabled", false))
3737
registerIfNot(LogStash::Setting::ArrayCoercible.new("xpack.monitoring.elasticsearch.hosts", String, ["http://localhost:9200"]))
38-
registerIfNot(LogStash::Setting::NullableString.new("xpack.monitoring.elasticsearch.username", "logstash_TEST system"))
39-
registerIfNot(LogStash::Setting::NullableString.new("xpack.monitoring.elasticsearch.username", "logstash_TEST system"))
38+
registerIfNot(LogStash::Setting::SettingNullableString.new("xpack.monitoring.elasticsearch.username", "logstash_TEST system"))
39+
registerIfNot(LogStash::Setting::SettingNullableString.new("xpack.monitoring.elasticsearch.username", "logstash_TEST system"))
4040
end
4141

4242
after :each do

‎logstash-core/spec/logstash/queue_factory_spec.rb‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -23,15 +23,15 @@
2323
let(:settings_array) do
2424
[
2525
LogStash::Setting::WritableDirectory.new("path.queue", Stud::Temporary.pathname),
26-
LogStash::Setting::String.new("queue.type", "memory", true, ["persisted", "memory"]),
26+
LogStash::Setting::SettingString.new("queue.type", "memory", true, ["persisted", "memory"]),
2727
LogStash::Setting::Bytes.new("queue.page_capacity", "8mb"),
2828
LogStash::Setting::Bytes.new("queue.max_bytes", "64mb"),
2929
LogStash::Setting::Numeric.new("queue.max_events", 0),
3030
LogStash::Setting::Numeric.new("queue.checkpoint.acks", 1024),
3131
LogStash::Setting::Numeric.new("queue.checkpoint.writes", 1024),
3232
LogStash::Setting::Numeric.new("queue.checkpoint.interval", 1000),
3333
LogStash::Setting::Boolean.new("queue.checkpoint.retry", false),
34-
LogStash::Setting::String.new("pipeline.id", pipeline_id),
34+
LogStash::Setting::SettingString.new("pipeline.id", pipeline_id),
3535
LogStash::Setting::PositiveInteger.new("pipeline.batch.size", 125),
3636
LogStash::Setting::PositiveInteger.new("pipeline.workers", LogStash::Config::CpuCoreStrategy.maximum)
3737
]

‎logstash-core/spec/logstash/runner_spec.rb‎

Lines changed: 34 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -265,15 +265,48 @@
265265

266266
context "when deprecated :http.host is defined by the user" do
267267
let(:args) { ["--http.host", "localhost", "-e", pipeline_string]}
268+
let(:events) { [] }
269+
270+
before(:each) do
271+
java_import org.apache.logging.log4j.LogManager
272+
logger = LogManager.getLogger("org.logstash.settings.DeprecatedAlias")
273+
deprecated_logger = LogManager.getLogger("org.logstash.deprecation.settings.DeprecatedAlias")
274+
275+
@custom_appender = CustomAppender.new(events).tap {|appender| appender.start }
276+
277+
java_import org.apache.logging.log4j.Level
278+
logger.addAppender(@custom_appender)
279+
deprecated_logger.addAppender(@custom_appender)
280+
# had to set level after appending as it was "error" for some reason
281+
logger.setLevel(Level::INFO)
282+
deprecated_logger.setLevel(Level::INFO)
283+
284+
expect(@custom_appender.started?).to be_truthy
285+
end
286+
287+
after(:each) do
288+
events.clear
289+
java_import org.apache.logging.log4j.LogManager
290+
logger = LogManager.getLogger("org.logstash.settings.DeprecatedAlias")
291+
deprecated_logger = LogManager.getLogger("org.logstash.deprecation.settings.DeprecatedAlias")
292+
# The Logger's AbstractConfiguration contains a cache of Appender, by class name. The cache is updated
293+
# iff is absent, so to make subsequent add_appender effective we have cleanup on teardown, else the new
294+
# appender instance is simply not used by the logger
295+
logger.remove_appender(@custom_appender)
296+
deprecated_logger.remove_appender(@custom_appender)
297+
end
298+
268299
it "creates an Agent whose `api.http.host` uses the provided value and provides helpful deprecation message" do
269-
expect(deprecation_logger_stub).to receive(:deprecated).with(a_string_including "`http.host` is a deprecated alias for `api.http.host`")
270300
expect(runner_deprecation_logger_stub).to receive(:deprecated).with(a_string_including 'The flag ["--http.host"] has been deprecated')
271301
expect(LogStash::Agent).to receive(:new) do |settings|
272302
expect(settings.set?("api.http.host")).to be(true)
273303
expect(settings.get("api.http.host")).to eq("localhost")
274304
end
275305

276306
subject.run("bin/logstash", args)
307+
308+
expect(events).not_to be_empty
309+
expect(events[0]).to match(/`http.host` is a deprecated alias for `api.http.host`/)
277310
end
278311
end
279312

‎logstash-core/spec/logstash/settings/nullable_spec.rb‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -20,13 +20,13 @@
2020

2121
describe LogStash::Setting::Nullable do
2222
let(:setting_name) { "this.that" }
23-
let(:normal_setting) { LogStash::Setting::String.new(setting_name, nil, false, possible_strings) }
23+
let(:normal_setting) { LogStash::Setting::SettingString.new(setting_name, nil, false, possible_strings) }
2424
let(:possible_strings) { [] } # empty means any string passes
2525

2626
subject(:nullable_setting) { normal_setting.nullable }
2727

2828
it 'is a kind of Nullable' do
29-
expect(nullable_setting).to be_a_kind_of(described_class)
29+
expect(nullable_setting).to be_a_kind_of(LogStash::Setting::NullableSetting)
3030
end
3131

3232
it "retains the wrapped setting's name" do
@@ -56,14 +56,14 @@
5656
context 'to an invalid wrong-type value' do
5757
let(:candidate_value) { 127 } # wrong type, expects String
5858
it 'is an invalid setting' do
59-
expect { nullable_setting.validate_value }.to raise_error(ArgumentError, a_string_including("Setting \"#{setting_name}\" must be a "))
59+
expect { nullable_setting.validate_value }.to raise_error(java.lang.ClassCastException, a_string_including("class java.lang.Long cannot be cast to class java.lang.String"))
6060
end
6161
end
6262
context 'to an invalid value not in the allow-list' do
6363
let(:possible_strings) { %w(this that)}
6464
let(:candidate_value) { "another" } # wrong type, expects String
6565
it 'is an invalid setting' do
66-
expect { nullable_setting.validate_value }.to raise_error(ArgumentError, a_string_including("Invalid value"))
66+
expect { nullable_setting.validate_value }.to raise_error(java.lang.IllegalArgumentException, a_string_including("Invalid value"))
6767
end
6868
end
6969
context 'to a valid value' do

0 commit comments

Comments
 (0)