diff --git a/build.gradle b/build.gradle index 7646bf5c3..6419c361a 100644 --- a/build.gradle +++ b/build.gradle @@ -2,7 +2,7 @@ plugins { id "io.codearte.nexus-staging" version "0.22.0" } -ext.projectVersion = '2.1.4' +ext.projectVersion = '2.1.5' ext.isReleaseVersion = !ext.projectVersion.endsWith('SNAPSHOT') ext.mavenRepoUrl = project.properties['mavenRepoUrl'] ?: 'https://oss.sonatype.org/service/local/staging/deploy/maven2/' diff --git a/docs/configuration.md b/docs/configuration.md index 10cf79905..19d3112fa 100644 --- a/docs/configuration.md +++ b/docs/configuration.md @@ -226,10 +226,11 @@ look like: } ``` -### Field interpolation using system properties -In both formats, you can use system properties as field values. For instance, `${user.home}` in any field will be -replaced by the actual value of system property `user.home`. If a property is not defined, no interpolation will be done -and string will be left as is. A default value can be provided after `:-`, for instance `${property.name:-defaultValue}`. +### Field interpolation using Java system properties or environment variables +In both formats, you can use Java system properties and system environment variables as field values. For instance, `${user.home}` in any field will be +replaced by the actual value of Java system property `user.home`. Likewise for `$HOME`, mapping the environment variable `HOME`. If a property is not defined, no interpolation will be done +and string will be left as is. If the same key is defined both as a Java system property and an environment variable, the Java property value will take precedence. +A default value can be provided after `:-`, for instance `${property.name:-defaultValue}`. Therefore, the following is a valid configuration file: ```json @@ -238,12 +239,14 @@ Therefore, the following is a valid configuration file: "bot": { "username": "bot-username", "privateKey": { - "path": "${user.home}/rsa-private-key.pem" + "path": "${HOME}/rsa-private-key.pem" } } } ``` Please mind that if you want to escape the `$` sign, `$${value}` will be replaced by `${value}`. +And as the matching of environment variables is done after Java system properties, if you have a system property with **value** `${value}` and en environment variable with **key** `value`, it will substitute the value of the environment variable + Reading a `JSON` configuration file is completely transparent: ```java diff --git a/symphony-bdk-core/src/main/java/com/symphony/bdk/core/config/BdkConfigLoader.java b/symphony-bdk-core/src/main/java/com/symphony/bdk/core/config/BdkConfigLoader.java index 5d592553b..7503cc9c8 100644 --- a/symphony-bdk-core/src/main/java/com/symphony/bdk/core/config/BdkConfigLoader.java +++ b/symphony-bdk-core/src/main/java/com/symphony/bdk/core/config/BdkConfigLoader.java @@ -52,7 +52,8 @@ public static BdkConfig loadFromFile(String configPath) throws BdkConfigExceptio * @return Symphony Bot Configuration */ public static BdkConfig loadFromInputStream(InputStream inputStream) throws BdkConfigException { - return parseConfig(BdkConfigParser.parse(inputStream)); + BdkConfigParser parser = new BdkConfigParser(); + return parseConfig(parser.parse(inputStream)); } private static BdkConfig parseConfig(JsonNode jsonNode) { diff --git a/symphony-bdk-core/src/main/java/com/symphony/bdk/core/config/BdkConfigParser.java b/symphony-bdk-core/src/main/java/com/symphony/bdk/core/config/BdkConfigParser.java index cde9292d3..7fcce8a3f 100644 --- a/symphony-bdk-core/src/main/java/com/symphony/bdk/core/config/BdkConfigParser.java +++ b/symphony-bdk-core/src/main/java/com/symphony/bdk/core/config/BdkConfigParser.java @@ -11,6 +11,7 @@ import com.fasterxml.jackson.dataformat.yaml.YAMLMapper; import lombok.extern.slf4j.Slf4j; import org.apache.commons.text.StringSubstitutor; +import org.apache.commons.text.lookup.StringLookupFactory; import org.apiguardian.api.API; import java.io.BufferedReader; @@ -25,68 +26,74 @@ @API(status = API.Status.INTERNAL) class BdkConfigParser { - private static final ObjectMapper JSON_MAPPER = new JsonMapper(); - private static final ObjectMapper YAML_MAPPER = new YAMLMapper(); + private static final ObjectMapper JSON_MAPPER = new JsonMapper(); + private static final ObjectMapper YAML_MAPPER = new YAMLMapper(); + private final StringSubstitutor envVarStringSubstitutor; - static { - JSON_MAPPER.configure(DeserializationFeature.FAIL_ON_UNKNOWN_PROPERTIES, false); - YAML_MAPPER.configure(DeserializationFeature.FAIL_ON_UNKNOWN_PROPERTIES, false); - } + static { + JSON_MAPPER.configure(DeserializationFeature.FAIL_ON_UNKNOWN_PROPERTIES, false); + YAML_MAPPER.configure(DeserializationFeature.FAIL_ON_UNKNOWN_PROPERTIES, false); + } - public static JsonNode parse(InputStream inputStream) throws BdkConfigException { - final JsonNode jsonNode = parseJsonNode(inputStream); - interpolateProperties(jsonNode); + public BdkConfigParser() { + envVarStringSubstitutor = new StringSubstitutor(StringLookupFactory.INSTANCE.environmentVariableStringLookup()); + } - return jsonNode; - } + public JsonNode parse(InputStream inputStream) throws BdkConfigException { + final JsonNode jsonNode = parseJsonNode(inputStream); + interpolateProperties(jsonNode); + return jsonNode; + } - public static JsonNode parseJsonNode(InputStream inputStream) throws BdkConfigException { - String content = new BufferedReader( - new InputStreamReader(inputStream, StandardCharsets.UTF_8)) - .lines() - .collect(Collectors.joining("\n")); - try { - return JSON_MAPPER.readTree(content); - } catch (IOException e) { - log.debug("Config file is not in JSON format, checking for YAML format."); - } + public JsonNode parseJsonNode(InputStream inputStream) throws BdkConfigException { + String content = new BufferedReader( + new InputStreamReader(inputStream, StandardCharsets.UTF_8)) + .lines() + .collect(Collectors.joining("\n")); + try { + return JSON_MAPPER.readTree(content); + } catch (IOException e) { + log.debug("Config file is not in JSON format, checking for YAML format."); + } - try { - JsonNode jsonNode = YAML_MAPPER.readTree(content); - if (jsonNode.isContainerNode()) { - log.debug("Config file found in YAML format."); - return jsonNode; - } - } catch (IOException e) { - log.debug("Config file is not in YAML format."); - } - throw new BdkConfigException("Given InputStream is not valid. Only YAML or JSON are allowed."); + try { + JsonNode jsonNode = YAML_MAPPER.readTree(content); + if (jsonNode.isContainerNode()) { + log.debug("Config file found in YAML format."); + return jsonNode; + } + } catch (IOException e) { + log.debug("Config file is not in YAML format."); } + throw new BdkConfigException("Given InputStream is not valid. Only YAML or JSON are allowed."); + } - public static void interpolateProperties(JsonNode jsonNode) { - if (jsonNode.isArray()) { - for (final JsonNode arrayItem : jsonNode) { - interpolateProperties(arrayItem); - } - } else if (jsonNode.isObject()) { - interpolatePropertiesInObject((ObjectNode) jsonNode); - } + public void interpolateProperties(JsonNode jsonNode) { + if (jsonNode.isArray()) { + for (final JsonNode arrayItem : jsonNode) { + interpolateProperties(arrayItem); + } + } else if (jsonNode.isObject()) { + interpolatePropertiesInObject((ObjectNode) jsonNode); } + } - private static void interpolatePropertiesInObject(ObjectNode objectNode) { - final Iterator fieldNames = objectNode.fieldNames(); - while (fieldNames.hasNext()) { - interpolatePropertyInField(objectNode, fieldNames.next()); - } + private void interpolatePropertiesInObject(ObjectNode objectNode) { + final Iterator fieldNames = objectNode.fieldNames(); + while (fieldNames.hasNext()) { + interpolatePropertyInField(objectNode, fieldNames.next()); } + } - private static void interpolatePropertyInField(ObjectNode objectNode, String field) { - final JsonNode node = objectNode.get(field); - if (node.isTextual()) { - final String interpolatedFieldValue = StringSubstitutor.replaceSystemProperties(node.asText()); - objectNode.set(field, new TextNode(interpolatedFieldValue)); - } else if (node.isObject() || node.isArray()) { - interpolateProperties(node); - } + private void interpolatePropertyInField(ObjectNode objectNode, String field) { + final JsonNode node = objectNode.get(field); + if (node.isTextual()) { + //Start by replacing any java system properties found, then match any remaining keys with environment variables + final String interpolatedFieldValue = + envVarStringSubstitutor.replace(StringSubstitutor.replaceSystemProperties(node.asText())); + objectNode.set(field, new TextNode(interpolatedFieldValue)); + } else if (node.isObject() || node.isArray()) { + interpolateProperties(node); } + } } diff --git a/symphony-bdk-core/src/test/java/com/symphony/bdk/core/config/BdkConfigParserTest.java b/symphony-bdk-core/src/test/java/com/symphony/bdk/core/config/BdkConfigParserTest.java index 21423e5fa..22f336747 100644 --- a/symphony-bdk-core/src/test/java/com/symphony/bdk/core/config/BdkConfigParserTest.java +++ b/symphony-bdk-core/src/test/java/com/symphony/bdk/core/config/BdkConfigParserTest.java @@ -11,112 +11,165 @@ import org.junit.jupiter.api.Test; import java.io.InputStream; +import java.lang.reflect.Field; +import java.util.Map; public class BdkConfigParserTest { - @BeforeEach - void tearDown() { - System.clearProperty("my.property"); - System.clearProperty("recursive"); + //this is a disgusting hack that should only be allowed in unit tests + void hackEnvVar(String key, String value) throws NoSuchFieldException, IllegalAccessException { + Map env = System.getenv(); + Field field = env.getClass().getDeclaredField("m"); + field.setAccessible(true); + if (value == null) { + ((Map) field.get(env)).remove(key); + } else { + ((Map) field.get(env)).put(key, value); } - - @Test - void parseJsonConfigTest() throws BdkConfigException { - String configPath = "/config/config.json"; - InputStream inputStream = BdkConfigLoaderTest.class.getResourceAsStream(configPath); - JsonNode jsonNode = BdkConfigParser.parse(inputStream); - assertEquals("tibot", jsonNode.at("/bot/username").asText()); - } - - @Test - void parseJsonConfigWithPropertyTest() throws BdkConfigException { - System.setProperty("my.property", "propvalue"); - String configPath = "/config/config_properties.json"; - InputStream inputStream = BdkConfigLoaderTest.class.getResourceAsStream(configPath); - JsonNode jsonNode = BdkConfigParser.parse(inputStream); - - assertEquals("${escaped}", jsonNode.at("/host").asText()); - assertEquals("propvalue/privatekey.pem", jsonNode.at("/bot/privateKeyPath").asText()); - assertEquals("default/value/privatekey.pem", jsonNode.at("/app/privateKeyPath").asText()); - } - - @Test - void parseJsonConfigWithPropertyContainingSpecialCharsTest() throws BdkConfigException { - System.setProperty("my.property", "propvalue:\"\n"); - String configPath = "/config/config_properties.json"; - InputStream inputStream = BdkConfigLoaderTest.class.getResourceAsStream(configPath); - JsonNode jsonNode = BdkConfigParser.parse(inputStream); - assertEquals("propvalue:\"\n/privatekey.pem", jsonNode.at("/bot/privateKeyPath").asText()); - } - - @Test - void parseJsonConfigWithRecursivePropertyShouldNotWorkTest() throws BdkConfigException { - System.setProperty("recursive", "my.property"); - System.setProperty("my.property", "propvalue"); - String configPath = "/config/config_recursive_props.json"; - InputStream inputStream = BdkConfigLoaderTest.class.getResourceAsStream(configPath); - JsonNode jsonNode = BdkConfigParser.parse(inputStream); - assertEquals("${${recursive}}/privatekey.pem", jsonNode.at("/bot/privateKeyPath").asText()); - } - - @Test - void parseJsonConfigWithUndefinedPropertyTest() throws BdkConfigException { - String configPath = "/config/config_properties.json"; - InputStream inputStream = BdkConfigLoaderTest.class.getResourceAsStream(configPath); - JsonNode jsonNode = BdkConfigParser.parse(inputStream); - assertEquals("${my.property}/privatekey.pem", jsonNode.at("/bot/privateKeyPath").asText()); - } - - @Test - void parseYamlConfigTest() throws BdkConfigException { - String configPath = "/config/config.yaml"; - InputStream inputStream = BdkConfigLoaderTest.class.getResourceAsStream(configPath); - JsonNode jsonNode = BdkConfigParser.parse(inputStream); - assertEquals("tibot", jsonNode.at("/bot/username").asText()); - } - - @Test - void parseYamlConfigWithPropertyTest() throws BdkConfigException { - System.setProperty("my.property", "propvalue"); - String configPath = "/config/config_properties.yaml"; - InputStream inputStream = BdkConfigLoaderTest.class.getResourceAsStream(configPath); - JsonNode jsonNode = BdkConfigParser.parse(inputStream); - assertEquals("propvalue/privatekey.pem", jsonNode.at("/bot/privateKey/path").asText()); - } - - @Test - void parseYamlConfigWithPropertiesInArray() throws BdkConfigException { - System.setProperty("my.property", "agent-lb.acme.org"); - String configPath = "/config/config_lb_properties.yaml"; - InputStream inputStream = BdkConfigLoaderTest.class.getResourceAsStream(configPath); - final JsonNode loadBalancing = BdkConfigParser.parse(inputStream).at("/agent/loadBalancing"); - - assertEquals("roundRobin", loadBalancing.at("/mode").asText()); - assertTrue(loadBalancing.at("/stickiness").asBoolean()); - - assertEquals("agent1.acme.org", loadBalancing.at("/nodes/0/host").asText()); - assertEquals(1234, loadBalancing.at("/nodes/0/port").asInt()); - - assertEquals("agent-lb.acme.org", loadBalancing.at("/nodes/1/host").asText()); - } - - @Test - void parseInvalidConfigTest() { - String configPath = "/config/invalid_config.html"; - BdkConfigException exception = assertThrows(BdkConfigException.class, () -> { - InputStream inputStream = BdkConfigLoaderTest.class.getResourceAsStream(configPath); - BdkConfigParser.parse(inputStream); - }); - assertEquals("Given InputStream is not valid. Only YAML or JSON are allowed.", exception.getMessage()); - } - - @Test - void parseInvalidYamlConfigTest() { - String configPath = "/config/invalid_config.yaml"; - BdkConfigException exception = assertThrows(BdkConfigException.class, () -> { - InputStream inputStream = BdkConfigLoaderTest.class.getResourceAsStream(configPath); - BdkConfigParser.parse(inputStream); - }); - assertEquals("Given InputStream is not valid. Only YAML or JSON are allowed.", exception.getMessage()); + } + + @BeforeEach + void setUp() { + System.clearProperty("my.property"); + System.clearProperty("recursive"); + try { + hackEnvVar("SYMPHONY_BDK_UNIT_TEST_TEMP", null); + hackEnvVar("my.property", null); + } catch (Exception e) { + throw new IllegalStateException("Couldn't hack environment variable", e); } + } + + @Test + void parseJsonConfigTest() throws BdkConfigException { + String configPath = "/config/config.json"; + InputStream inputStream = BdkConfigLoaderTest.class.getResourceAsStream(configPath); + BdkConfigParser parser = new BdkConfigParser(); + JsonNode jsonNode = parser.parse(inputStream); + assertEquals("tibot", jsonNode.at("/bot/username").asText()); + } + + @Test + void parseJsonConfigWithPropertyTest() throws BdkConfigException, NoSuchFieldException, IllegalAccessException { + System.setProperty("my.property", "propvalue"); + hackEnvVar("SYMPHONY_BDK_UNIT_TEST_TEMP", "envvalue"); + //java vars should take precedence over env vars + hackEnvVar("my.property", "invalid_value"); + String configPath = "/config/config_properties.json"; + InputStream inputStream = BdkConfigLoaderTest.class.getResourceAsStream(configPath); + BdkConfigParser parser = new BdkConfigParser(); + JsonNode jsonNode = parser.parse(inputStream); + assertEquals("${escaped}", jsonNode.at("/host").asText()); + assertEquals("envvalue/propvalue/privatekey.pem", jsonNode.at("/bot/privateKeyPath").asText()); + assertEquals("default/value/privatekey.pem", jsonNode.at("/app/privateKeyPath").asText()); + } + + @Test + void parseJsonConfigWithPropertyContainingSpecialCharsTest() + throws BdkConfigException, NoSuchFieldException, IllegalAccessException { + System.setProperty("my.property", "propvalue:\"\n"); + hackEnvVar("SYMPHONY_BDK_UNIT_TEST_TEMP", "envvalue"); + String configPath = "/config/config_properties.json"; + InputStream inputStream = BdkConfigLoaderTest.class.getResourceAsStream(configPath); + BdkConfigParser parser = new BdkConfigParser(); + JsonNode jsonNode = parser.parse(inputStream); + assertEquals("envvalue/propvalue:\"\n/privatekey.pem", jsonNode.at("/bot/privateKeyPath").asText()); + } + + @Test + void parseJsonConfigWithRecursivePropertyShouldNotWorkTest() throws BdkConfigException { + System.setProperty("recursive", "my.property"); + System.setProperty("my.property", "propvalue"); + String configPath = "/config/config_recursive_props.json"; + InputStream inputStream = BdkConfigLoaderTest.class.getResourceAsStream(configPath); + BdkConfigParser parser = new BdkConfigParser(); + JsonNode jsonNode = parser.parse(inputStream); + assertEquals("${${recursive}}/privatekey.pem", jsonNode.at("/bot/privateKeyPath").asText()); + } + + @Test + void parseJsonConfigWithUndefinedPropertyTest() throws BdkConfigException { + String configPath = "/config/config_properties.json"; + InputStream inputStream = BdkConfigLoaderTest.class.getResourceAsStream(configPath); + BdkConfigParser parser = new BdkConfigParser(); + JsonNode jsonNode = parser.parse(inputStream); + assertEquals("${SYMPHONY_BDK_UNIT_TEST_TEMP}/${my.property}/privatekey.pem", + jsonNode.at("/bot/privateKeyPath").asText()); + } + + @Test + void parseYamlConfigTest() throws BdkConfigException { + String configPath = "/config/config.yaml"; + InputStream inputStream = BdkConfigLoaderTest.class.getResourceAsStream(configPath); + BdkConfigParser parser = new BdkConfigParser(); + JsonNode jsonNode = parser.parse(inputStream); + assertEquals("tibot", jsonNode.at("/bot/username").asText()); + } + + @Test + void parseYamlConfigWithPropertyTest() throws BdkConfigException, NoSuchFieldException, IllegalAccessException { + System.setProperty("my.property", "propvalue"); + hackEnvVar("SYMPHONY_BDK_UNIT_TEST_TEMP", "envvalue"); + //java vars should take precedence over env vars + hackEnvVar("my.property", "invalid_value"); + String configPath = "/config/config_properties.yaml"; + InputStream inputStream = BdkConfigLoaderTest.class.getResourceAsStream(configPath); + BdkConfigParser parser = new BdkConfigParser(); + JsonNode jsonNode = parser.parse(inputStream); + assertEquals("envvalue/propvalue/privatekey.pem", jsonNode.at("/bot/privateKey/path").asText()); + ; + assertEquals("envvalue/propvalue/privatekey.pem", jsonNode.at("/bot/privateKey/path").asText()); + } + + @Test + void parseYamlConfigWithPropertiesInArrayTest() + throws BdkConfigException, NoSuchFieldException, IllegalAccessException { + System.setProperty("my.property", "agent-lb.acme.org"); + hackEnvVar("SYMPHONY_BDK_UNIT_TEST_TEMP", "valid_value"); + //java vars should take precedence over env vars + hackEnvVar("my.property", "invalid_value"); + String configPath = "/config/config_lb_properties.yaml"; + InputStream inputStream = BdkConfigLoaderTest.class.getResourceAsStream(configPath); + BdkConfigParser parser = new BdkConfigParser(); + final JsonNode loadBalancing = parser.parse(inputStream).at("/agent/loadBalancing"); + + assertEquals("roundRobin", loadBalancing.at("/mode").asText()); + assertTrue(loadBalancing.at("/stickiness").asBoolean()); + assertEquals("agent1.acme.org", loadBalancing.at("/nodes/0/host").asText()); + assertEquals(1234, loadBalancing.at("/nodes/0/port").asInt()); + assertEquals("valid_value/agent-lb.acme.org", loadBalancing.at("/nodes/1/host").asText()); + assertEquals("valid_value/hello", loadBalancing.at("/nodes/2/host").asText()); + } + + @Test + void parseYamlConfigWithUndefinedPropertyTest() throws BdkConfigException { + String configPath = "/config/config_properties.yaml"; + InputStream inputStream = BdkConfigLoaderTest.class.getResourceAsStream(configPath); + BdkConfigParser parser = new BdkConfigParser(); + JsonNode jsonNode = parser.parse(inputStream); + assertEquals("${SYMPHONY_BDK_UNIT_TEST_TEMP}/${my.property}/privatekey.pem", + jsonNode.at("/bot/privateKey/path").asText()); + } + + @Test + void parseInvalidConfigTest() { + String configPath = "/config/invalid_config.html"; + BdkConfigException exception = assertThrows(BdkConfigException.class, () -> { + InputStream inputStream = BdkConfigLoaderTest.class.getResourceAsStream(configPath); + BdkConfigParser parser = new BdkConfigParser(); + parser.parse(inputStream); + }); + assertEquals("Given InputStream is not valid. Only YAML or JSON are allowed.", exception.getMessage()); + } + + @Test + void parseInvalidYamlConfigTest() { + String configPath = "/config/invalid_config.yaml"; + BdkConfigException exception = assertThrows(BdkConfigException.class, () -> { + InputStream inputStream = BdkConfigLoaderTest.class.getResourceAsStream(configPath); + BdkConfigParser parser = new BdkConfigParser(); + parser.parse(inputStream); + }); + assertEquals("Given InputStream is not valid. Only YAML or JSON are allowed.", exception.getMessage()); + } } diff --git a/symphony-bdk-core/src/test/resources/config/config_lb_properties.yaml b/symphony-bdk-core/src/test/resources/config/config_lb_properties.yaml index 2e6f76028..f7828e55a 100644 --- a/symphony-bdk-core/src/test/resources/config/config_lb_properties.yaml +++ b/symphony-bdk-core/src/test/resources/config/config_lb_properties.yaml @@ -5,4 +5,5 @@ agent: nodes: - host: agent1.acme.org port: 1234 - - host: ${my.property} + - host: ${SYMPHONY_BDK_UNIT_TEST_TEMP}/${my.property} + - host: ${SYMPHONY_BDK_UNIT_TEST_TEMP}/hello diff --git a/symphony-bdk-core/src/test/resources/config/config_properties.json b/symphony-bdk-core/src/test/resources/config/config_properties.json index 1cd464348..46b6b0dab 100644 --- a/symphony-bdk-core/src/test/resources/config/config_properties.json +++ b/symphony-bdk-core/src/test/resources/config/config_properties.json @@ -5,7 +5,7 @@ "bot": { "username": "tibot", - "privateKeyPath": "${my.property}/privatekey.pem" + "privateKeyPath": "${SYMPHONY_BDK_UNIT_TEST_TEMP}/${my.property}/privatekey.pem" }, "app": { "appId": "tibapp", diff --git a/symphony-bdk-core/src/test/resources/config/config_properties.yaml b/symphony-bdk-core/src/test/resources/config/config_properties.yaml index c62d2282c..3045e3bf9 100644 --- a/symphony-bdk-core/src/test/resources/config/config_properties.yaml +++ b/symphony-bdk-core/src/test/resources/config/config_properties.yaml @@ -3,5 +3,6 @@ _version: '2.0' bot: username: tibot privateKey: - path: ${my.property}/privatekey.pem + path: ${SYMPHONY_BDK_UNIT_TEST_TEMP}/${my.property}/privatekey.pem +