diff --git a/logstash-core/lib/logstash/settings.rb b/logstash-core/lib/logstash/settings.rb index b24bd6d2692..f9d74df9845 100644 --- a/logstash-core/lib/logstash/settings.rb +++ b/logstash-core/lib/logstash/settings.rb @@ -458,6 +458,8 @@ def validate(value) java_import org.logstash.settings.TimeValueSetting + java_import org.logstash.settings.ArrayCoercibleSetting + class ArrayCoercible < Coercible def initialize(name, klass, default, strict = true, &validator_proc) @element_class = klass diff --git a/logstash-core/spec/logstash/api/commands/default_metadata_spec.rb b/logstash-core/spec/logstash/api/commands/default_metadata_spec.rb index 9fe73b1e99f..7d0f0f64e1c 100644 --- a/logstash-core/spec/logstash/api/commands/default_metadata_spec.rb +++ b/logstash-core/spec/logstash/api/commands/default_metadata_spec.rb @@ -34,7 +34,7 @@ def registerIfNot(setting) before :all do registerIfNot(LogStash::Setting::BooleanSetting.new("xpack.monitoring.enabled", false)) - registerIfNot(LogStash::Setting::ArrayCoercible.new("xpack.monitoring.elasticsearch.hosts", String, ["http://localhost:9200"])) + registerIfNot(LogStash::Setting::ArrayCoercibleSetting.new("xpack.monitoring.elasticsearch.hosts", java.lang.String.java_class, ["http://localhost:9200"])) registerIfNot(LogStash::Setting::NullableStringSetting.new("xpack.monitoring.elasticsearch.username", "logstash_TEST system")) registerIfNot(LogStash::Setting::NullableStringSetting.new("xpack.monitoring.elasticsearch.username", "logstash_TEST system")) end diff --git a/logstash-core/spec/logstash/settings/array_coercible_spec.rb b/logstash-core/spec/logstash/settings/array_coercible_spec.rb index db0999f1095..c73e3bfc3f6 100644 --- a/logstash-core/spec/logstash/settings/array_coercible_spec.rb +++ b/logstash-core/spec/logstash/settings/array_coercible_spec.rb @@ -18,10 +18,10 @@ require "spec_helper" require "logstash/settings" -describe LogStash::Setting::ArrayCoercible do +describe LogStash::Setting::ArrayCoercibleSetting do subject { described_class.new("option", element_class, value) } let(:value) { [] } - let(:element_class) { Object } + let(:element_class) { java.lang.Object.java_class } context "when given a non array value" do let(:value) { "test" } @@ -43,16 +43,16 @@ describe "initialization" do subject { described_class } - let(:element_class) { Integer } + let(:element_class) { java.lang.Integer.java_class } context "when given values of incorrect element class" do let(:value) { "test" } it "will raise an exception" do - expect { described_class.new("option", element_class, value) }.to raise_error(ArgumentError) + expect { described_class.new("option", element_class, value) }.to raise_error(java.lang.IllegalArgumentException) end end context "when given values of correct element class" do - let(:value) { 1 } + let(:value) { java.lang.Integer.new(1) } it "will not raise an exception" do expect { described_class.new("option", element_class, value) }.not_to raise_error @@ -63,9 +63,9 @@ describe "#==" do context "when comparing two settings" do let(:setting_1) { described_class.new("option_1", element_class_1, value_1) } - let(:element_class_1) { String } + let(:element_class_1) { java.lang.String.java_class } let(:setting_2) { described_class.new("option_1", element_class_2, value_2) } - let(:element_class_2) { String } + let(:element_class_2) { java.lang.String.java_class } context "where one was given a non array value" do let(:value_1) { "a string" } diff --git a/logstash-core/spec/logstash/settings_spec.rb b/logstash-core/spec/logstash/settings_spec.rb index d5bee93f1f4..dc782b88d77 100644 --- a/logstash-core/spec/logstash/settings_spec.rb +++ b/logstash-core/spec/logstash/settings_spec.rb @@ -325,7 +325,7 @@ subject do settings = described_class.new - settings.register(LogStash::Setting::ArrayCoercible.new("host", String, [])) + settings.register(LogStash::Setting::ArrayCoercibleSetting.new("host", java.lang.String.java_class, [])) settings end diff --git a/logstash-core/src/main/java/org/logstash/settings/ArrayCoercibleSetting.java b/logstash-core/src/main/java/org/logstash/settings/ArrayCoercibleSetting.java new file mode 100644 index 00000000000..e61c36bfbe0 --- /dev/null +++ b/logstash-core/src/main/java/org/logstash/settings/ArrayCoercibleSetting.java @@ -0,0 +1,106 @@ +/* + * Licensed to Elasticsearch B.V. under one or more contributor + * license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright + * ownership. Elasticsearch B.V. licenses this file to you under + * the Apache License, Version 2.0 (the "License"); you may + * not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +package org.logstash.settings; + +import java.util.*; +import java.util.function.Predicate; + +public class ArrayCoercibleSetting extends Coercible { + + private final Class elementClass; + + @SuppressWarnings("this-escape") + public ArrayCoercibleSetting(String name, Class elementClass, Object defaultValue, boolean strict, + Predicate validator) { + super(name, defaultValue, false, validator); + this.elementClass = elementClass; + + if (strict) { + List coercedDefault = coerce(defaultValue); + validate(coercedDefault); + this.defaultValue = coercedDefault; + } else { + this.defaultValue = defaultValue; + } + } + + public ArrayCoercibleSetting(String name, Class elementClass, Object defaultValue, boolean strict) { + this(name, elementClass, defaultValue, strict, noValidator()); + } + + public ArrayCoercibleSetting(String name, Class elementClass, Object defaultValue) { + this(name, elementClass, defaultValue, true); + } + + @Override + public List coerce(Object value) { + if (value == null) { + return Collections.emptyList(); + } + if (value instanceof List) { + // We need to move away from RubyArray instances, because they equals method does a strong type checking. + // See https://github.com/jruby/jruby/blob/9.4.14.0/core/src/main/java/org/jruby/RubyArray.java#L5988-L5994 + // If a RubyArray is compared to something else that's not a RubyArray, return false, but this doesn't + // take care of cases when it's compared to a List, which is the case for our settings. + // So we need to create a new ArrayList with the same content. + return new ArrayList<>((List) value); + } + if (value instanceof Object[]) { + return Arrays.asList((Object[]) value); + } + return Collections.singletonList(value); + } + + @Override + public void validate(Object input) throws IllegalArgumentException { + super.validate(input); + + if (input == null) { + return; + } + + List inputList = (List) input; + for (Object element : inputList) { + if (!elementClass.isInstance(element)) { + throw new IllegalArgumentException( + String.format("Values of setting \"%s\" must be %s. Received: %s (%s)", + getName(), elementClass.getSimpleName(), element, + element != null ? element.getClass().getSimpleName() : "null")); + } + } + } + + @Override + public boolean equals(Object o) { + if (this == o) { + return true; + } + if (o == null || getClass() != o.getClass()) { + return false; + } + + ArrayCoercibleSetting that = (ArrayCoercibleSetting) o; + return this.value().equals(that.value()); + } + + @Override + public int hashCode() { + return Objects.hashCode(this.value()); + } +} diff --git a/logstash-core/src/test/java/org/logstash/settings/ArrayCoercibleSettingTest.java b/logstash-core/src/test/java/org/logstash/settings/ArrayCoercibleSettingTest.java new file mode 100644 index 00000000000..2961fa2b819 --- /dev/null +++ b/logstash-core/src/test/java/org/logstash/settings/ArrayCoercibleSettingTest.java @@ -0,0 +1,144 @@ +/* + * Licensed to Elasticsearch B.V. under one or more contributor + * license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright + * ownership. Elasticsearch B.V. licenses this file to you under + * the Apache License, Version 2.0 (the "License"); you may + * not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +package org.logstash.settings; + +import org.junit.Test; + +import java.util.Collections; +import java.util.List; + +import static org.hamcrest.MatcherAssert.assertThat; +import static org.hamcrest.Matchers.contains; +import static org.hamcrest.Matchers.containsString; +import static org.hamcrest.Matchers.empty; +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertNotEquals; +import static org.junit.Assert.assertThrows; + +public class ArrayCoercibleSettingTest { + + @SuppressWarnings("unchecked") + @Test + public void givenNonArrayValueWhenCoercedThenConvertedToSingleElementArray() { + ArrayCoercibleSetting sut = new ArrayCoercibleSetting("option", String.class, Collections.emptyList(), false); + + sut.set("test"); + + assertThat((List) sut.value(), contains("test")); + } + + @Test + public void givenArrayValueWhenCoercedThenNotModified() { + List initialValue = List.of("test"); + ArrayCoercibleSetting sut = new ArrayCoercibleSetting("option", String.class, initialValue, true); + + assertEquals(initialValue, sut.value()); + } + + @Test + public void givenValuesOfIncorrectElementClassWhenInitializedThenThrowsException() { + IllegalArgumentException ex = assertThrows(IllegalArgumentException.class, () -> { + new ArrayCoercibleSetting("option", Integer.class, Collections.singletonList("test")); + }); + + assertThat(ex.getMessage(), containsString("Values of setting \"option\" must be Integer")); + } + + @SuppressWarnings("unchecked") + @Test + public void givenValuesOfCorrectElementClassWhenInitializedThenNoException() { + ArrayCoercibleSetting sut = new ArrayCoercibleSetting("option", Integer.class, Collections.singletonList(1)); + + assertThat((List) sut.value(), contains(1)); + } + + @SuppressWarnings("unchecked") + @Test + public void givenNullValueWhenCoercedThenReturnsEmptyList() { + ArrayCoercibleSetting sut = new ArrayCoercibleSetting("option", String.class, Collections.emptyList(), false); + + sut.set(null); + + assertThat((List) sut.value(), empty()); + } + + @Test + public void givenTwoSettingsWithSameNonArrayValueThenAreEqual() { + ArrayCoercibleSetting setting1 = new ArrayCoercibleSetting("option_1", String.class, "a string"); + ArrayCoercibleSetting setting2 = new ArrayCoercibleSetting("option_1", String.class, "a string"); + + assertEquals(setting1, setting2); + } + + @Test + public void givenTwoSettingsOneNonArrayAndOneArrayWithSameValueThenAreEqual() { + ArrayCoercibleSetting setting1 = new ArrayCoercibleSetting("option_1", String.class, "a string"); + ArrayCoercibleSetting setting2 = new ArrayCoercibleSetting("option_1", String.class, List.of("a string")); + + assertEquals(setting1, setting2); + } + + @Test + public void givenTwoSettingsWithDifferentNonArrayValuesThenAreNotEqual() { + ArrayCoercibleSetting setting1 = new ArrayCoercibleSetting("option_1", String.class, "a string"); + ArrayCoercibleSetting setting2 = new ArrayCoercibleSetting("option_1", String.class, "a different string"); + + assertNotEquals(setting1, setting2); + } + + @Test + public void givenTwoSettingsWithNonArrayAndDifferentValueInArrayThenAreNotEqual() { + ArrayCoercibleSetting setting1 = new ArrayCoercibleSetting("option_1", String.class, "a string"); + ArrayCoercibleSetting setting2 = new ArrayCoercibleSetting("option_1", String.class, List.of("a different string")); + + assertNotEquals(setting1, setting2); + } + + @Test + public void givenTwoSettingsBothHaveSameArrayValueThenAreEqual() { + ArrayCoercibleSetting setting1 = new ArrayCoercibleSetting("option_1", String.class, List.of("a string")); + ArrayCoercibleSetting setting2 = new ArrayCoercibleSetting("option_1", String.class, List.of("a string")); + + assertEquals(setting1, setting2); + } + + @Test + public void givenTwoSettingsArrayAndNonArrayWithSameValueThenAreEqual() { + ArrayCoercibleSetting setting1 = new ArrayCoercibleSetting("option_1", String.class, List.of("a string")); + ArrayCoercibleSetting setting2 = new ArrayCoercibleSetting("option_1", String.class, "a string"); + + assertEquals(setting1, setting2); + } + + @Test + public void givenTwoSettingsWithDifferentArrayValuesThenAreNotEqual() { + ArrayCoercibleSetting setting1 = new ArrayCoercibleSetting("option_1", String.class, List.of("a string")); + ArrayCoercibleSetting setting2 = new ArrayCoercibleSetting("option_1", String.class, List.of("a different string")); + + assertNotEquals(setting1, setting2); + } + + @Test + public void givenTwoSettingsWithArrayAndDifferentNonArrayValueThenAreNotEqual() { + ArrayCoercibleSetting setting1 = new ArrayCoercibleSetting("option_1", String.class, List.of("a string")); + ArrayCoercibleSetting setting2 = new ArrayCoercibleSetting("option_1", String.class, "a different string"); + + assertNotEquals(setting1, setting2); + } +} diff --git a/x-pack/lib/config_management/extension.rb b/x-pack/lib/config_management/extension.rb index 1f5c6844248..844e5ceeee5 100644 --- a/x-pack/lib/config_management/extension.rb +++ b/x-pack/lib/config_management/extension.rb @@ -23,10 +23,10 @@ def additionals_settings(settings) settings.register(LogStash::Setting::BooleanSetting.new("xpack.management.enabled", false)) settings.register(LogStash::Setting::TimeValueSetting.new("xpack.management.logstash.poll_interval", "5s")) - settings.register(LogStash::Setting::ArrayCoercible.new("xpack.management.pipeline.id", String, ["main"])) + settings.register(LogStash::Setting::ArrayCoercibleSetting.new("xpack.management.pipeline.id", java.lang.String.java_class, ["main"])) settings.register(LogStash::Setting::NullableStringSetting.new("xpack.management.elasticsearch.username", "logstash_system")) settings.register(LogStash::Setting::NullableStringSetting.new("xpack.management.elasticsearch.password")) - settings.register(LogStash::Setting::ArrayCoercible.new("xpack.management.elasticsearch.hosts", String, ["https://localhost:9200"])) + settings.register(LogStash::Setting::ArrayCoercibleSetting.new("xpack.management.elasticsearch.hosts", java.lang.String.java_class, ["https://localhost:9200"])) settings.register(LogStash::Setting::NullableStringSetting.new("xpack.management.elasticsearch.cloud_id")) settings.register(LogStash::Setting::NullableStringSetting.new("xpack.management.elasticsearch.cloud_auth")) settings.register(LogStash::Setting::NullableStringSetting.new("xpack.management.elasticsearch.api_key")) @@ -39,7 +39,7 @@ def additionals_settings(settings) settings.register(LogStash::Setting::NullableStringSetting.new("xpack.management.elasticsearch.ssl.keystore.password")) settings.register(LogStash::Setting::NullableStringSetting.new("xpack.management.elasticsearch.ssl.certificate")) settings.register(LogStash::Setting::NullableStringSetting.new("xpack.management.elasticsearch.ssl.key")) - settings.register(LogStash::Setting::ArrayCoercible.new("xpack.management.elasticsearch.ssl.cipher_suites", String, [])) + settings.register(LogStash::Setting::ArrayCoercibleSetting.new("xpack.management.elasticsearch.ssl.cipher_suites", java.lang.String.java_class, [])) settings.register(LogStash::Setting::StringSetting.new("xpack.management.elasticsearch.ssl.verification_mode", "full", true, %w[none certificate full])) settings.register(LogStash::Setting::BooleanSetting.new("xpack.management.elasticsearch.sniffing", false)) rescue => e