From c73bfb8f2e57d853f7f88886ea7bd541210206bd Mon Sep 17 00:00:00 2001
From: Gil Portenseigne <pgil@apache.org>
Date: Tue, 23 Nov 2021 10:25:30 +0100
Subject: [PATCH 1/2] Implements getEnvironmentProperty to allow environment
 variable configuration

This will enable configuration of one OFBiz instance without modifying
source code, using environment variables.
Available in :
* property files
* serviceengine.xml
* entityengine.xml
* build.gradle (native)
---
 .../ofbiz/base/util/UtilProperties.java       | 31 ++++++++++++++++++-
 framework/common/config/general.properties    |  2 +-
 framework/entity/config/entityengine.xml      |  6 ++--
 .../ofbiz/entity/config/model/InlineJdbc.java |  7 +++--
 .../ofbiz/service/config/model/Parameter.java |  3 +-
 .../ofbiz/service/config/model/Server.java    | 17 +++++-----
 .../ofbiz/service/mail/JavaMailContainer.java |  4 ++-
 7 files changed, 52 insertions(+), 18 deletions(-)

diff --git a/framework/base/src/main/java/org/apache/ofbiz/base/util/UtilProperties.java b/framework/base/src/main/java/org/apache/ofbiz/base/util/UtilProperties.java
index 8b1f313ae7c..67e974b0c4e 100644
--- a/framework/base/src/main/java/org/apache/ofbiz/base/util/UtilProperties.java
+++ b/framework/base/src/main/java/org/apache/ofbiz/base/util/UtilProperties.java
@@ -285,7 +285,7 @@ public static String getPropertyValue(String resource, String name) {
         } catch (Exception e) {
             Debug.logInfo(e, MODULE);
         }
-        return value == null ? "" : value.trim();
+        return value == null ? "" : getEnvironmentProperty(value.trim());
     }
 
     /**
@@ -988,6 +988,35 @@ public static Properties xmlToProperties(InputStream in, Locale locale, Properti
         return properties;
     }
 
+    /**
+     * Resolve a property to check if it contains an environment variable
+     * represented by ${env:ENV_VARIABLE:DEFAULT_VALUE}
+     * @param value
+     * @return
+     */
+    public static String getEnvironmentProperty(String value) {
+        if (value != null) {
+            if (value.startsWith("${env:") && value.endsWith("}")) {
+                String envNameWithDefault = value.substring(6, value.length() - 1);
+                String environmentName = envNameWithDefault;
+                String defaultValue = null;
+                if (envNameWithDefault.contains(":")) {
+                    environmentName = envNameWithDefault.substring(0, envNameWithDefault.indexOf(":"));
+                    defaultValue = envNameWithDefault.substring(envNameWithDefault.indexOf(":") + 1);
+                }
+                String environmentValue = System.getenv(environmentName);
+                if (environmentValue != null) {
+                    return environmentValue;
+                }
+                if (defaultValue != null) {
+                    return defaultValue;
+                }
+                return "";
+            }
+        }
+        return value;
+    }
+
     /** Custom ResourceBundle class. This class extends ResourceBundle
      * to add custom bundle caching code and support for the OFBiz custom XML
      * properties file format.
diff --git a/framework/common/config/general.properties b/framework/common/config/general.properties
index a6f3c23da15..3889c961c3c 100644
--- a/framework/common/config/general.properties
+++ b/framework/common/config/general.properties
@@ -18,7 +18,7 @@
 ###############################################################################
 
 # -- unique instance id (20 char max)
-unique.instanceId=ofbiz1
+unique.instanceId=${env:OFB_INSTANCE_ID:ofbiz1}
 
 # -- the default currency to use for prices, etc
 currency.uom.id.default=USD
diff --git a/framework/entity/config/entityengine.xml b/framework/entity/config/entityengine.xml
index e9abf076320..47ebc21639e 100644
--- a/framework/entity/config/entityengine.xml
+++ b/framework/entity/config/entityengine.xml
@@ -489,9 +489,9 @@ access. For a detailed description see the core/docs/entityconfig.html file.
         <read-data reader-name="ext-demo"/>
         <inline-jdbc
                 jdbc-driver="org.postgresql.Driver"
-                jdbc-uri="jdbc:postgresql://127.0.0.1/ofbiz"
-                jdbc-username="ofbiz"
-                jdbc-password="ofbiz"
+                jdbc-uri="${env:OFB_POSTGRES_DB:jdbc:postgresql://127.0.0.1/ofbiz}"
+                jdbc-username="${env:OFB_POSTGRES_USER:ofbiz}"
+                jdbc-password="${env:OFB_POSTGRES_PASS:ofbiz}"
                 isolation-level="ReadCommitted"
                 pool-minsize="2"
                 pool-maxsize="250"
diff --git a/framework/entity/src/main/java/org/apache/ofbiz/entity/config/model/InlineJdbc.java b/framework/entity/src/main/java/org/apache/ofbiz/entity/config/model/InlineJdbc.java
index 6f621270f78..d4d7e1d3e9d 100644
--- a/framework/entity/src/main/java/org/apache/ofbiz/entity/config/model/InlineJdbc.java
+++ b/framework/entity/src/main/java/org/apache/ofbiz/entity/config/model/InlineJdbc.java
@@ -19,6 +19,7 @@
 package org.apache.ofbiz.entity.config.model;
 
 import org.apache.ofbiz.base.lang.ThreadSafe;
+import org.apache.ofbiz.base.util.UtilProperties;
 import org.apache.ofbiz.entity.GenericEntityConfException;
 import org.w3c.dom.Element;
 
@@ -59,17 +60,17 @@ public final class InlineJdbc extends JdbcElement {
             throw new GenericEntityConfException("<inline-jdbc> element jdbc-driver attribute is empty" + lineNumberText);
         }
         this.jdbcDriver = jdbcDriver;
-        String jdbcUri = element.getAttribute("jdbc-uri").intern();
+        String jdbcUri = UtilProperties.getEnvironmentProperty(element.getAttribute("jdbc-uri").intern());
         if (jdbcUri.isEmpty()) {
             throw new GenericEntityConfException("<inline-jdbc> element jdbc-uri attribute is empty" + lineNumberText);
         }
         this.jdbcUri = jdbcUri;
-        String jdbcUsername = element.getAttribute("jdbc-username").intern();
+        String jdbcUsername = UtilProperties.getEnvironmentProperty(element.getAttribute("jdbc-username").intern());
         if (jdbcUsername.isEmpty()) {
             throw new GenericEntityConfException("<inline-jdbc> element jdbc-username attribute is empty" + lineNumberText);
         }
         this.jdbcUsername = jdbcUsername;
-        this.jdbcPassword = element.getAttribute("jdbc-password").intern();
+        this.jdbcPassword = UtilProperties.getEnvironmentProperty(element.getAttribute("jdbc-password").intern());
         this.jdbcPasswordLookup = element.getAttribute("jdbc-password-lookup").intern();
         String poolMaxsize = element.getAttribute("pool-maxsize");
         if (poolMaxsize.isEmpty()) {
diff --git a/framework/service/src/main/java/org/apache/ofbiz/service/config/model/Parameter.java b/framework/service/src/main/java/org/apache/ofbiz/service/config/model/Parameter.java
index b3fe7eff2c9..9de680649bc 100644
--- a/framework/service/src/main/java/org/apache/ofbiz/service/config/model/Parameter.java
+++ b/framework/service/src/main/java/org/apache/ofbiz/service/config/model/Parameter.java
@@ -19,6 +19,7 @@
 package org.apache.ofbiz.service.config.model;
 
 import org.apache.ofbiz.base.lang.ThreadSafe;
+import org.apache.ofbiz.base.util.UtilProperties;
 import org.apache.ofbiz.service.config.ServiceConfigException;
 import org.w3c.dom.Element;
 
@@ -37,7 +38,7 @@ public final class Parameter {
             throw new ServiceConfigException("<parameter> element name attribute is empty");
         }
         this.name = name;
-        String value = parameterElement.getAttribute("value").intern();
+        String value = UtilProperties.getEnvironmentProperty(parameterElement.getAttribute("value").intern());
         if (value.isEmpty()) {
             throw new ServiceConfigException("<parameter> element value attribute is empty");
         }
diff --git a/framework/service/src/main/java/org/apache/ofbiz/service/config/model/Server.java b/framework/service/src/main/java/org/apache/ofbiz/service/config/model/Server.java
index 94bb716d162..3807ed7a604 100644
--- a/framework/service/src/main/java/org/apache/ofbiz/service/config/model/Server.java
+++ b/framework/service/src/main/java/org/apache/ofbiz/service/config/model/Server.java
@@ -19,6 +19,7 @@
 package org.apache.ofbiz.service.config.model;
 
 import org.apache.ofbiz.base.lang.ThreadSafe;
+import org.apache.ofbiz.base.util.UtilProperties;
 import org.apache.ofbiz.service.config.ServiceConfigException;
 import org.w3c.dom.Element;
 
@@ -39,17 +40,17 @@ public final class Server {
     private final String username;
 
     Server(Element serverElement) throws ServiceConfigException {
-        String jndiServerName = serverElement.getAttribute("jndi-server-name").intern();
+        String jndiServerName = UtilProperties.getEnvironmentProperty(serverElement.getAttribute("jndi-server-name").intern());
         if (jndiServerName.isEmpty()) {
             throw new ServiceConfigException("<server> element jndi-server-name attribute is empty");
         }
         this.jndiServerName = jndiServerName;
-        String jndiName = serverElement.getAttribute("jndi-name").intern();
+        String jndiName = UtilProperties.getEnvironmentProperty(serverElement.getAttribute("jndi-name").intern());
         if (jndiName.isEmpty()) {
             throw new ServiceConfigException("<server> element jndi-name attribute is empty");
         }
         this.jndiName = jndiName;
-        String topicQueue = serverElement.getAttribute("topic-queue").intern();
+        String topicQueue = UtilProperties.getEnvironmentProperty(serverElement.getAttribute("topic-queue").intern());
         if (topicQueue.isEmpty()) {
             throw new ServiceConfigException("<server> element topic-queue attribute is empty");
         }
@@ -59,11 +60,11 @@ public final class Server {
             throw new ServiceConfigException("<server> element type attribute is empty");
         }
         this.type = type;
-        this.username = serverElement.getAttribute("username").intern();
-        this.password = serverElement.getAttribute("password").intern();
-        this.clientId = serverElement.getAttribute("client-id").intern();
-        this.listen = "true".equals(serverElement.getAttribute("listen"));
-        this.listenerClass = serverElement.getAttribute("listener-class").intern();
+        this.username = UtilProperties.getEnvironmentProperty(serverElement.getAttribute("username").intern());
+        this.password = UtilProperties.getEnvironmentProperty(serverElement.getAttribute("password").intern());
+        this.clientId = UtilProperties.getEnvironmentProperty(serverElement.getAttribute("client-id").intern());
+        this.listen = "true".equals(UtilProperties.getEnvironmentProperty(serverElement.getAttribute("listen")));
+        this.listenerClass = UtilProperties.getEnvironmentProperty(serverElement.getAttribute("listener-class").intern());
     }
 
     public String getClientId() {
diff --git a/framework/service/src/main/java/org/apache/ofbiz/service/mail/JavaMailContainer.java b/framework/service/src/main/java/org/apache/ofbiz/service/mail/JavaMailContainer.java
index 08f8be371a7..68ee7a1b3bc 100644
--- a/framework/service/src/main/java/org/apache/ofbiz/service/mail/JavaMailContainer.java
+++ b/framework/service/src/main/java/org/apache/ofbiz/service/mail/JavaMailContainer.java
@@ -48,6 +48,7 @@
 import org.apache.ofbiz.base.start.StartupCommand;
 import org.apache.ofbiz.base.util.Debug;
 import org.apache.ofbiz.base.util.UtilMisc;
+import org.apache.ofbiz.base.util.UtilProperties;
 import org.apache.ofbiz.base.util.UtilValidate;
 import org.apache.ofbiz.entity.Delegator;
 import org.apache.ofbiz.entity.DelegatorFactory;
@@ -147,7 +148,8 @@ protected Session makeSession(Configuration.Property client) {
         Map<String, Configuration.Property> clientProps = client.properties();
         if (clientProps != null) {
             for (Configuration.Property p: clientProps.values()) {
-                props.setProperty(p.name().toLowerCase(Locale.getDefault()), p.value());
+                props.setProperty(p.name().toLowerCase(Locale.getDefault()),
+                        UtilProperties.getEnvironmentProperty(p.value()));
             }
         }
         return Session.getInstance(props);

From 8606b77f23c0de7f60149a502b62fa1e2edff35f Mon Sep 17 00:00:00 2001
From: Gil Portenseigne <pgil@apache.org>
Date: Mon, 3 Jan 2022 11:39:34 +0100
Subject: [PATCH 2/2] Implements getEnvironmentProperty to allow environment
 variable configuration

This commit add unit test and documentation regarding the improvement

Thanks Eugen for the review
---
 docs/asciidoc/developer-manual.adoc           |  1 +
 .../docs/asciidoc/_include/env-config.adoc    | 41 +++++++++++++++++++
 .../base/src/docs/asciidoc/configuration.adoc | 21 ++++++++++
 .../ofbiz/base/util/UtilProperties.java       | 19 +++++++--
 .../ofbiz/base/util/UtilPropertiesTests.java  | 24 +++++++++++
 5 files changed, 102 insertions(+), 4 deletions(-)
 create mode 100644 framework/base/src/docs/asciidoc/_include/env-config.adoc
 create mode 100644 framework/base/src/docs/asciidoc/configuration.adoc

diff --git a/docs/asciidoc/developer-manual.adoc b/docs/asciidoc/developer-manual.adoc
index a770ed20430..7604bb54085 100644
--- a/docs/asciidoc/developer-manual.adoc
+++ b/docs/asciidoc/developer-manual.adoc
@@ -289,6 +289,7 @@ For a core configuration guide check
 https://cwiki.apache.org/confluence/display/OFBIZ/Framework+Configuration+Guide[the OFBiz configuration Guide]
  (some points are not up to date).
 
+include::../../framework/base/src/docs/asciidoc/configuration.adoc[leveloffset=+2]
 
 include::../../framework/base/src/docs/asciidoc/email.adoc[leveloffset=+2]
 
diff --git a/framework/base/src/docs/asciidoc/_include/env-config.adoc b/framework/base/src/docs/asciidoc/_include/env-config.adoc
new file mode 100644
index 00000000000..95fa3efe130
--- /dev/null
+++ b/framework/base/src/docs/asciidoc/_include/env-config.adoc
@@ -0,0 +1,41 @@
+////
+Licensed to the Apache Software Foundation (ASF) under one
+or more contributor license agreements.  See the NOTICE file
+distributed with this work for additional information
+regarding copyright ownership.  The ASF 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.
+////
+= Configuration by environment variable
+
+https://issues.apache.org/jira/browse/OFBIZ-9498[OFBIZ-9498] introduce a new way to configure an OFBiz instance at
+launch using environment variable.
+
+This is a first intermediate step of the task, that allow configuration without modifying
+source code of a distributed OFBiz artefact.
+
+Currently, some configurations are designed to be set through environment variable :
+
+* framework/entity/config/entityengine.xml for database access credentials
+** jdbc-uri="${env:OFB_POSTGRES_DB:jdbc:postgresql://127.0.0.1/ofbiz}"
+** jdbc-username="${env:OFB_POSTGRES_USER:ofbiz}"
+** jdbc-password="${env:OFB_POSTGRES_PASS:ofbiz}"
+
+* framework/common/config/general.properties
+** unique.instanceId=${env:OFB_INSTANCE_ID:ofbiz1}
+
+Others could be designed using the same notation `${env:ENV_NAME:DEFAULT_VALUE}`, such as :
+
+* any property in the `*.properties` files
+* In `serviceengine.xml`, for new specific service engine, to allow api access credential configuration by the Ops.
+
diff --git a/framework/base/src/docs/asciidoc/configuration.adoc b/framework/base/src/docs/asciidoc/configuration.adoc
new file mode 100644
index 00000000000..40bdd9a25e4
--- /dev/null
+++ b/framework/base/src/docs/asciidoc/configuration.adoc
@@ -0,0 +1,21 @@
+////
+Licensed to the Apache Software Foundation (ASF) under one
+or more contributor license agreements.  See the NOTICE file
+distributed with this work for additional information
+regarding copyright ownership.  The ASF 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.
+////
+= Configuration System
+
+include::_include/env-config.adoc[leveloffset=+2]
\ No newline at end of file
diff --git a/framework/base/src/main/java/org/apache/ofbiz/base/util/UtilProperties.java b/framework/base/src/main/java/org/apache/ofbiz/base/util/UtilProperties.java
index 67e974b0c4e..ee8d08e5104 100644
--- a/framework/base/src/main/java/org/apache/ofbiz/base/util/UtilProperties.java
+++ b/framework/base/src/main/java/org/apache/ofbiz/base/util/UtilProperties.java
@@ -991,10 +991,11 @@ public static Properties xmlToProperties(InputStream in, Locale locale, Properti
     /**
      * Resolve a property to check if it contains an environment variable
      * represented by ${env:ENV_VARIABLE:DEFAULT_VALUE}
-     * @param value
-     * @return
+     * @param env : map that contains available env variables
+     * @param value : the property to resolve
+     * @return resolved value
      */
-    public static String getEnvironmentProperty(String value) {
+    public static String getEnvironmentProperty(Map<String, String> env, String value) {
         if (value != null) {
             if (value.startsWith("${env:") && value.endsWith("}")) {
                 String envNameWithDefault = value.substring(6, value.length() - 1);
@@ -1004,7 +1005,7 @@ public static String getEnvironmentProperty(String value) {
                     environmentName = envNameWithDefault.substring(0, envNameWithDefault.indexOf(":"));
                     defaultValue = envNameWithDefault.substring(envNameWithDefault.indexOf(":") + 1);
                 }
-                String environmentValue = System.getenv(environmentName);
+                String environmentValue = env.get(environmentName);
                 if (environmentValue != null) {
                     return environmentValue;
                 }
@@ -1017,6 +1018,16 @@ public static String getEnvironmentProperty(String value) {
         return value;
     }
 
+    /**
+     * Resolve a property to check if it contains an environment variable
+     * represented by ${env:ENV_VARIABLE:DEFAULT_VALUE}
+     * @param value : the property to resolve
+     * @return resolved value
+     */
+    public static String getEnvironmentProperty(String value) {
+        return getEnvironmentProperty(System.getenv(), value);
+    }
+
     /** Custom ResourceBundle class. This class extends ResourceBundle
      * to add custom bundle caching code and support for the OFBiz custom XML
      * properties file format.
diff --git a/framework/base/src/test/java/org/apache/ofbiz/base/util/UtilPropertiesTests.java b/framework/base/src/test/java/org/apache/ofbiz/base/util/UtilPropertiesTests.java
index 91b760c0c41..b03975c6e32 100644
--- a/framework/base/src/test/java/org/apache/ofbiz/base/util/UtilPropertiesTests.java
+++ b/framework/base/src/test/java/org/apache/ofbiz/base/util/UtilPropertiesTests.java
@@ -26,9 +26,12 @@
 import java.io.IOException;
 import java.io.InputStream;
 import java.nio.charset.Charset;
+import java.util.HashMap;
 import java.util.Locale;
+import java.util.Map;
 import java.util.Properties;
 
+import com.google.common.collect.ImmutableMap;
 import org.junit.Test;
 
 public class UtilPropertiesTests {
@@ -78,4 +81,25 @@ private Properties xmlToProperties(String separator) throws IOException {
             return UtilProperties.xmlToProperties(in, locale, null);
         }
     }
+
+    /**
+     * Environment Variable property retrieval
+     * Test that default value is retrieved if no variable set.
+     */
+    @Test
+    public void testEnvironmentPropertyDefaultValue() {
+        String value = UtilProperties.getEnvironmentProperty(new HashMap<>(), "${env:ENV_VARIABLE:DEFAULT_VALUE}");
+        assertEquals("DEFAULT_VALUE", value);
+    }
+
+    /**
+     * Environment Variable property retrieval
+     * Test that defined value is retrieved if env variable set.
+     */
+    @Test
+    public void testEnvironmentPropertyMatch() {
+        Map<String, String> env = ImmutableMap.of("ENV_VARIABLE", "SET_VALUE");
+        String value = UtilProperties.getEnvironmentProperty(env, "${env:ENV_VARIABLE:DEFAULT_VALUE}");
+        assertEquals("SET_VALUE", value);
+    }
 }