Config.Builder and Config.create() give environment variables precedence over system properties

[pfmackin]

Helidon Version: 1.2.1

Helidon MP
 and SE
JDK version: 1.8.0_211

OS: MacOS 10.13.6

Problem Description
:
The Config.Builder and Config.create() give environment variables precedence over system properties. The config spec has the opposite. See https://download.eclipse.org/microprofile/microprofile-config-1.3/microprofile-config-spec.pdf

Note: The Helidon Javadoc actually documents this incorrect behavior: See Config.Buider at https://helidon.io/docs/latest/apidocs/index.html?overview-summary.html

Furthermore, If the Server is created without using the Config.Builder then the behavior is correct. This works:
static Server startServer() {
return Server.create().start();
}

Steps to reproduce
:
Create a a project using MP archetype to test this.

1. Change app.greeting to appGreeting in GreetingProvider.java and META-INF/microprofile-config.properties.
2. Build and run the application as follows:

export appGreeting=HelloFromEnvironment
mvn package -DskipTests=true
java -DappGreeting="HelloFromJavaProperty" -jar target/helidon-quickstart-mp.jar

3. Test the app
   curl http://localhost:8080/greet
   {"message":"HelloFromJavaProperty World!"}

---

Now change Main.java as follows:

    return Server.builder()
        .config(Config.builder()
            .sources(classpath("META-INF/microprofile-config.properties"))
            .build())
        .build()
        .start();

Retest the app and you will see the problem:

curl http://localhost:8080/greet
{"message":"HelloFromEnvironment World!"}

[batsatt]

There are a couple of things going on here...

First, the Config.builder() and Config.create() methods are actually SE specific, though this is not readily apparent: the call to Server.builder().config(...) takes the SE config and converts it to an MP config using MpConfig.builder().config(…).build(). This conversion does not add the default MP config sources, assuming that the ones provided by the SE config are desired.

Second, sys prop and env var precedence is inverted in SE relative to MP (see #507). Taken together with the above, the behavior you are seeing is actually expected, but… confusing, for sure.

IMO, aligning the default behavior in SE with MP is the right approach, but it is a breaking change. We can certainly consider doing this for the next major release.

[tomas-langer]

You should have the option to define order of env and sys properties (another issue & PR I think).
This behavior is expected (although a bit confusing as @batsatt said).
Explanation:
In Helidon, we have decided to prioritize sources as follows:

1. environment variables
2. system properties
3. other config sources (file, class path etc.)

This is in the order of how hard it is to change these

• environment variables can be easily modified when running in a cloud environment (e.g. K8S)
• system properties are harder to change, though do not require repackaging of application
• config sources may require change to sources (e.g. classpath)

So the ordering is logical and from my point of view the reasoning is sound.

Unfortunately the MicroProfile specification has decided to use a different order - they consider system properties are more significant than environment variables.

You have the option to use both - either MP approach or Helidon approach.
In case you want to combine these, you need to explicitly order these sources if you want MicroProfile behavior.

[pfmackin]

Even if I explicitly order them with system properties first, the env vars take precedence: see issue 977. It would be good if you could fix that. Note, that meta-config works correctly. The precedence follows the order of appearance, e.g. I can make system properties have the highest precedence.

[batsatt]

In progress, see PR #1002.

[batsatt]

PR #1002 merged. Closing this; leaving open #507 to consider the merits of changing precedence.