From 86eca8e3f710e2ffd043d493c47d97de7b35962d Mon Sep 17 00:00:00 2001 From: Adam Peck Date: Sat, 3 Sep 2022 15:31:04 -0600 Subject: [PATCH 1/6] Add interpolation to JsonConfigurator --- core/pom.xml | 9 +++ .../apache/druid/guice/JsonConfigurator.java | 16 +++- .../druid/guice/JsonConfiguratorTest.java | 74 +++++++++++++++++++ core/src/test/resources/list.json | 5 ++ docs/configuration/index.md | 29 ++++++++ licenses.yaml | 21 +++--- pom.xml | 7 +- 7 files changed, 147 insertions(+), 14 deletions(-) create mode 100644 core/src/test/resources/list.json diff --git a/core/pom.xml b/core/pom.xml index d588045cf54e..2f8b9dc0b1d2 100644 --- a/core/pom.xml +++ b/core/pom.xml @@ -67,6 +67,10 @@ org.apache.commons commons-compress + + org.apache.commons + commons-text + org.skife.config config-magic @@ -368,6 +372,11 @@ ${postgresql.version} test + + com.github.stefanbirkner + system-rules + test + diff --git a/core/src/main/java/org/apache/druid/guice/JsonConfigurator.java b/core/src/main/java/org/apache/druid/guice/JsonConfigurator.java index 61ef8d2067d3..2fb666857f2a 100644 --- a/core/src/main/java/org/apache/druid/guice/JsonConfigurator.java +++ b/core/src/main/java/org/apache/druid/guice/JsonConfigurator.java @@ -27,10 +27,13 @@ import com.google.common.annotations.VisibleForTesting; import com.google.common.base.Function; import com.google.common.base.Strings; +import com.google.common.collect.ImmutableMap; import com.google.common.collect.Iterables; import com.google.inject.Inject; import com.google.inject.ProvisionException; import com.google.inject.spi.Message; +import org.apache.commons.text.StringSubstitutor; +import org.apache.commons.text.lookup.StringLookupFactory; import org.apache.druid.java.util.common.StringUtils; import org.apache.druid.java.util.common.logger.Logger; @@ -58,6 +61,15 @@ public class JsonConfigurator private final ObjectMapper jsonMapper; private final Validator validator; + private final StringSubstitutor stringSubstitutor = new StringSubstitutor(StringLookupFactory.INSTANCE.interpolatorStringLookup( + ImmutableMap.of( + StringLookupFactory.KEY_SYS, StringLookupFactory.INSTANCE.systemPropertyStringLookup(), + StringLookupFactory.KEY_ENV, StringLookupFactory.INSTANCE.environmentVariableStringLookup(), + StringLookupFactory.KEY_FILE, StringLookupFactory.INSTANCE.fileStringLookup() + ), + null, + false + ));; @Inject public JsonConfigurator( @@ -67,6 +79,8 @@ public JsonConfigurator( { this.jsonMapper = jsonMapper; this.validator = validator; + this.stringSubstitutor.setEnableSubstitutionInVariables(true); + this.stringSubstitutor.setEnableUndefinedVariableException(true); } public T configurate(Properties props, String propertyPrefix, Class clazz) throws ProvisionException @@ -89,7 +103,7 @@ public T configurate( Map jsonMap = new HashMap<>(); for (String prop : props.stringPropertyNames()) { if (prop.startsWith(propertyBase)) { - final String propValue = props.getProperty(prop); + final String propValue = stringSubstitutor.replace(props.getProperty(prop)); Object value; try { // If it's a String Jackson wants it to be quoted, so check if it's not an object or array and quote. diff --git a/core/src/test/java/org/apache/druid/guice/JsonConfiguratorTest.java b/core/src/test/java/org/apache/druid/guice/JsonConfiguratorTest.java index b9bfd126b4ae..c9ec2a777267 100644 --- a/core/src/test/java/org/apache/druid/guice/JsonConfiguratorTest.java +++ b/core/src/test/java/org/apache/druid/guice/JsonConfiguratorTest.java @@ -27,7 +27,10 @@ import org.apache.druid.TestObjectMapper; import org.junit.Assert; import org.junit.Before; +import org.junit.Rule; import org.junit.Test; +import org.junit.contrib.java.lang.system.EnvironmentVariables; +import org.junit.contrib.java.lang.system.RestoreSystemProperties; import javax.validation.ConstraintViolation; import javax.validation.Validator; @@ -43,6 +46,12 @@ public class JsonConfiguratorTest private final ObjectMapper mapper = new TestObjectMapper(); private final Properties properties = new Properties(); + @Rule + public final RestoreSystemProperties restoreSystemProperties = new RestoreSystemProperties(); + + @Rule + public final EnvironmentVariables environmentVariables = new EnvironmentVariables(); + @Before public void setUp() { @@ -159,6 +168,71 @@ public void testPropertyWithDot() Assert.assertEquals("prop1", obj.prop1); } + + @Test + public void testPropertyInterpolation() + { + System.setProperty("my.property", "value1"); + List list = ImmutableList.of("list", "of", "strings"); + environmentVariables.set("MY_VAR", "value2"); + + final JsonConfigurator configurator = new JsonConfigurator(mapper, validator); + properties.setProperty(PROP_PREFIX + "prop1", "${sys:my.property}"); + properties.setProperty(PROP_PREFIX + "prop1List", "${file:UTF-8:src/test/resources/list.json}"); + properties.setProperty(PROP_PREFIX + "prop2.prop.2", "${env:MY_VAR}"); + final MappableObject obj = configurator.configurate(properties, PROP_PREFIX, MappableObject.class); + Assert.assertEquals(System.getProperty("my.property"), obj.prop1); + Assert.assertEquals(list, obj.prop1List); + Assert.assertEquals("value2", obj.prop2); + } + + @Test + public void testPropertyInterpolationInName() + { + System.setProperty("my.property", "value1"); + List list = ImmutableList.of("list", "of", "strings"); + environmentVariables.set("MY_VAR", "value2"); + + environmentVariables.set("SYS_PROP", "my.property"); + System.setProperty("json.path", "src/test/resources/list.json"); + environmentVariables.set("PROP2_NAME", "MY_VAR"); + + final JsonConfigurator configurator = new JsonConfigurator(mapper, validator); + properties.setProperty(PROP_PREFIX + "prop1", "${sys:${env:SYS_PROP}}"); + properties.setProperty(PROP_PREFIX + "prop1List", "${file:UTF-8:${sys:json.path}}"); + properties.setProperty(PROP_PREFIX + "prop2.prop.2", "${env:${env:PROP2_NAME}}"); + final MappableObject obj = configurator.configurate(properties, PROP_PREFIX, MappableObject.class); + Assert.assertEquals(System.getProperty("my.property"), obj.prop1); + Assert.assertEquals(list, obj.prop1List); + Assert.assertEquals("value2", obj.prop2); + } + + @Test + public void testPropertyInterpolationFallback() + { + List list = ImmutableList.of("list", "of", "strings"); + + final JsonConfigurator configurator = new JsonConfigurator(mapper, validator); + properties.setProperty(PROP_PREFIX + "prop1", "${sys:my.property:-value1}"); + properties.setProperty(PROP_PREFIX + "prop1List", "${unknown:-[\"list\", \"of\", \"strings\"]}"); + properties.setProperty(PROP_PREFIX + "prop2.prop.2", "${MY_VAR:-value2}"); + final MappableObject obj = configurator.configurate(properties, PROP_PREFIX, MappableObject.class); + Assert.assertEquals("value1", obj.prop1); + Assert.assertEquals(list, obj.prop1List); + Assert.assertEquals("value2", obj.prop2); + } + + @Test + public void testPropertyInterpolationUndefinedException() + { + final JsonConfigurator configurator = new JsonConfigurator(mapper, validator); + properties.setProperty(PROP_PREFIX + "prop1", "${sys:my.property}"); + + Assert.assertThrows( + IllegalArgumentException.class, + () -> configurator.configurate(properties, PROP_PREFIX, MappableObject.class) + ); + } } class MappableObject diff --git a/core/src/test/resources/list.json b/core/src/test/resources/list.json new file mode 100644 index 000000000000..5f91dc1efa60 --- /dev/null +++ b/core/src/test/resources/list.json @@ -0,0 +1,5 @@ +[ + "list", + "of", + "strings" +] \ No newline at end of file diff --git a/docs/configuration/index.md b/docs/configuration/index.md index 8d5723dae96a..49ed052fbcbc 100644 --- a/docs/configuration/index.md +++ b/docs/configuration/index.md @@ -61,6 +61,35 @@ The `jvm.config` files contain JVM flags such as heap sizing properties for each Common properties shared by all services are placed in `_common/common.runtime.properties`. +## Configuration Interpolation + +Configuration values can be interpolated from System Properties, Environment Variables, or local files. Below is an example of how this can be used: + +``` +druid.metadata.storage.type=${env:METADATA_STORAGE_TYPE} +druid.processing.tmpDir=${sys:java.io.tmpdir} +druid.segmentCache.locations=${file:UTF-8:/config/segment-cache-def.json} +``` + +Interpolation is also recursive so you can do: + +``` +druid.segmentCache.locations=${file:UTF-8:${env:SEGMENT_DEF_LOCATION}} +``` + +If the property is not set an exception will be thrown on startup, but a default can be provided if desired. Setting a default value will not work with file interpolation as an exception will be thrown if the file does not exist. + +``` +druid.metadata.storage.type=${env:METADATA_STORAGE_TYPE:-mysql} +druid.processing.tmpDir=${sys:java.io.tmpdir:-/tmp} +``` + +If you need to set a variable that is wrapped by `${...}` but do not want it to be interpolated you can escape it by adding another `$`. For example: + +``` +config.name=$${value} +``` + ## Common Configurations The properties under this section are common configurations that should be shared across all Druid services in a cluster. diff --git a/licenses.yaml b/licenses.yaml index 50cddbdbbad4..7e212eff1d6b 100644 --- a/licenses.yaml +++ b/licenses.yaml @@ -668,13 +668,16 @@ name: Apache Commons Lang license_category: binary module: java-core license_name: Apache License version 2.0 -version: 3.8.1 +version: 3.12.0 libraries: - org.apache.commons: commons-lang3 notices: - commons-lang3: | Apache Commons Lang - Copyright 2001-2018 The Apache Software Foundation + Copyright 2001-2021 The Apache Software Foundation + + This product includes software developed at + The Apache Software Foundation (https://www.apache.org/). --- @@ -719,23 +722,17 @@ name: Apache Commons Text license_category: binary module: java-core license_name: Apache License version 2.0 -version: 1.3 +version: 1.9 libraries: - org.apache.commons: commons-text notices: - commons-text: | Apache Commons Text - Copyright 2001-2018 The Apache Software Foundation + Copyright 2014-2020 The Apache Software Foundation ---- + This product includes software developed at + The Apache Software Foundation (https://www.apache.org/). -name: Apache Commons Text -license_category: binary -module: java-core -license_name: Apache License version 2.0 -version: 1.4 -libraries: - - org.apache.commons: commons-text --- name: Airline diff --git a/pom.xml b/pom.xml index 36e39d543f0c..01dedbcb276a 100644 --- a/pom.xml +++ b/pom.xml @@ -269,7 +269,12 @@ org.apache.commons commons-lang3 - 3.8.1 + 3.12.0 + + + org.apache.commons + commons-text + 1.9 com.amazonaws From c610567b0e5964c9086553602936a8648fd0843f Mon Sep 17 00:00:00 2001 From: Adam Peck Date: Sat, 3 Sep 2022 20:23:43 -0600 Subject: [PATCH 2/6] Fix checkstyle --- core/src/main/java/org/apache/druid/guice/JsonConfigurator.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/core/src/main/java/org/apache/druid/guice/JsonConfigurator.java b/core/src/main/java/org/apache/druid/guice/JsonConfigurator.java index 2fb666857f2a..a61f0dafbe0b 100644 --- a/core/src/main/java/org/apache/druid/guice/JsonConfigurator.java +++ b/core/src/main/java/org/apache/druid/guice/JsonConfigurator.java @@ -69,7 +69,7 @@ public class JsonConfigurator ), null, false - ));; + )); @Inject public JsonConfigurator( From bb43ab2a311f731d32afdcf979462b232abc3895 Mon Sep 17 00:00:00 2001 From: Adam Peck Date: Sun, 4 Sep 2022 00:19:11 -0600 Subject: [PATCH 3/6] Fix tests by removing common-text override --- server/pom.xml | 6 ------ 1 file changed, 6 deletions(-) diff --git a/server/pom.xml b/server/pom.xml index 07c10ed97acb..57f8bfff55c3 100644 --- a/server/pom.xml +++ b/server/pom.xml @@ -445,12 +445,6 @@ equalsverifier test - - org.apache.commons - commons-text - 1.3 - test - From 9f55784462e39aae2fafc2bbd11ec34e9817df1c Mon Sep 17 00:00:00 2001 From: Adam Peck Date: Sun, 4 Sep 2022 00:46:16 -0600 Subject: [PATCH 4/6] Add back commons-text without version --- server/pom.xml | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/server/pom.xml b/server/pom.xml index 57f8bfff55c3..499f7b544795 100644 --- a/server/pom.xml +++ b/server/pom.xml @@ -445,6 +445,11 @@ equalsverifier test + + org.apache.commons + commons-text + test + From 99d64b4e960b4d8c70bfe09a1be6e552ccd58990 Mon Sep 17 00:00:00 2001 From: Adam Peck Date: Sun, 4 Sep 2022 10:55:23 -0600 Subject: [PATCH 5/6] Remove unused hadoopDir configs --- .../test/java/org/apache/druid/testsEx/config/Initializer.java | 1 - integration-tests/pom.xml | 1 - 2 files changed, 2 deletions(-) diff --git a/integration-tests-ex/cases/src/test/java/org/apache/druid/testsEx/config/Initializer.java b/integration-tests-ex/cases/src/test/java/org/apache/druid/testsEx/config/Initializer.java index a2899a084488..0c8df88686b2 100644 --- a/integration-tests-ex/cases/src/test/java/org/apache/druid/testsEx/config/Initializer.java +++ b/integration-tests-ex/cases/src/test/java/org/apache/druid/testsEx/config/Initializer.java @@ -260,7 +260,6 @@ public Builder(String clusterName) // previously set in Maven. propertyEnvVarBinding("druid.test.config.dockerIp", "DOCKER_IP"); propertyEnvVarBinding("druid.zk.service.host", "DOCKER_IP"); - propertyEnvVarBinding("druid.test.config.hadoopDir", "HADOOP_DIR"); property("druid.client.https.trustStorePath", "client_tls/truststore.jks"); property("druid.client.https.trustStorePassword", "druid123"); property("druid.client.https.keyStorePath", "client_tls/client.jks"); diff --git a/integration-tests/pom.xml b/integration-tests/pom.xml index 73718324cefa..5a3166e2731e 100644 --- a/integration-tests/pom.xml +++ b/integration-tests/pom.xml @@ -651,7 +651,6 @@ -Duser.timezone=UTC -Dfile.encoding=UTF-8 -Ddruid.test.config.dockerIp=${env.DOCKER_IP} - -Ddruid.test.config.hadoopDir=${env.HADOOP_DIR} -Ddruid.test.config.extraDatasourceNameSuffix=${extra.datasource.name.suffix} -Ddruid.zk.service.host=${env.DOCKER_IP} -Ddruid.client.https.trustStorePath=client_tls/truststore.jks From 986bf2ebb7844e153cbff5996ef646007fb5f0da Mon Sep 17 00:00:00 2001 From: Adam Peck Date: Sun, 4 Sep 2022 14:14:24 -0600 Subject: [PATCH 6/6] Move some stuff to hopefully pass coverage --- .../main/java/org/apache/druid/guice/JsonConfigurator.java | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/core/src/main/java/org/apache/druid/guice/JsonConfigurator.java b/core/src/main/java/org/apache/druid/guice/JsonConfigurator.java index a61f0dafbe0b..1e4f18dc1cd7 100644 --- a/core/src/main/java/org/apache/druid/guice/JsonConfigurator.java +++ b/core/src/main/java/org/apache/druid/guice/JsonConfigurator.java @@ -69,7 +69,7 @@ public class JsonConfigurator ), null, false - )); + )).setEnableSubstitutionInVariables(true).setEnableUndefinedVariableException(true); @Inject public JsonConfigurator( @@ -79,8 +79,6 @@ public JsonConfigurator( { this.jsonMapper = jsonMapper; this.validator = validator; - this.stringSubstitutor.setEnableSubstitutionInVariables(true); - this.stringSubstitutor.setEnableUndefinedVariableException(true); } public T configurate(Properties props, String propertyPrefix, Class clazz) throws ProvisionException