#507 backport to 2.1 rc (#508)

* #506 Allow environment variables in config file (#507) * Allow environment variables in config file * documentation for 506 * changes requested in PR review * more changes requested in PR review * #506 Bumped project version to 2.1.5 Co-authored-by: Yannick <80749842+Yannick-Symphony@users.noreply.github.com>
finos · Apr 27, 2021 · c0dca85 · c0dca85
1 parent fa66974
commit c0dca85
Show file tree

Hide file tree

Showing 8 changed files with 231 additions and 165 deletions.
diff --git 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
@@ -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
@@ -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
@@ -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<String> fieldNames = objectNode.fieldNames();
-        while (fieldNames.hasNext()) {
-            interpolatePropertyInField(objectNode, fieldNames.next());
-        }
+  private void interpolatePropertiesInObject(ObjectNode objectNode) {
+    final Iterator<String> 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);
     }
+  }
 }