From ee22663dd3b033fad4b349925e7f45e220077183 Mon Sep 17 00:00:00 2001 From: Adam Peck Date: Wed, 7 Sep 2022 01:18:01 -0600 Subject: [PATCH] Add interpolation to JsonConfigurator (#13023) * Add interpolation to JsonConfigurator * Fix checkstyle * Fix tests by removing common-text override * Add back commons-text without version * Remove unused hadoopDir configs * Move some stuff to hopefully pass coverage --- core/pom.xml | 9 +++ .../apache/druid/guice/JsonConfigurator.java | 14 +++- .../druid/guice/JsonConfiguratorTest.java | 74 +++++++++++++++++++ core/src/test/resources/list.json | 5 ++ docs/configuration/index.md | 29 ++++++++ .../druid/testsEx/config/Initializer.java | 1 - integration-tests/pom.xml | 1 - licenses.yaml | 21 +++--- pom.xml | 7 +- server/pom.xml | 1 - 10 files changed, 145 insertions(+), 17 deletions(-) create mode 100644 core/src/test/resources/list.json diff --git a/core/pom.xml b/core/pom.xml index d588045cf54..2f8b9dc0b1d 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 61ef8d2067d..1e4f18dc1cd 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.fasterxml.jackson.databind.introspect.BeanPropertyDefinition; 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 + )).setEnableSubstitutionInVariables(true).setEnableUndefinedVariableException(true); @Inject public JsonConfigurator( @@ -89,7 +101,7 @@ public class JsonConfigurator 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 b9bfd126b4a..c9ec2a77726 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 com.google.common.collect.ImmutableSet; 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 class JsonConfiguratorTest 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 00000000000..5f91dc1efa6 --- /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 8d5723dae96..49ed052fbcb 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/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 a2899a08448..0c8df88686b 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 class Initializer // 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 73718324cef..5a3166e2731 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 diff --git a/licenses.yaml b/licenses.yaml index 50cddbdbbad..7e212eff1d6 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 ec6091c5835..1cde40ba61e 100644 --- a/pom.xml +++ b/pom.xml @@ -270,7 +270,12 @@ org.apache.commons commons-lang3 - 3.8.1 + 3.12.0 + + + org.apache.commons + commons-text + 1.9 com.amazonaws diff --git a/server/pom.xml b/server/pom.xml index 07c10ed97ac..499f7b54479 100644 --- a/server/pom.xml +++ b/server/pom.xml @@ -448,7 +448,6 @@ org.apache.commons commons-text - 1.3 test