Allow environment vars to be resolved to list #427

psliwa · 2016-09-25T16:20:49Z

It has been requested few times so far: #320, #168. Is there any contraindication why env vars cannot be treated the same way as system properties (in case of syntax)?

havocp · 2016-09-27T14:35:54Z

Thanks!

I would really like the "fork in test" fix, regardless of the env var thing, if you are interested in making that its own PR?

Re: the env vars, it seems like there are two issues with this.

One is that it isn't quite backward compatible, though we could probably fudge it, mostly because I doubt anyone is using dots in env vars due to the second issue -

Second issue is that bash and other tools really don't support this. http://unix.stackexchange.com/questions/93532/exporting-a-variable-with-dot-in-it

$ export foo.0=bar
bash: export: `foo.0=bar': not a valid identifier
$ echo ${foo.0}
bash: ${foo.0}: bad substitution
$ echo "$foo.0"
.0

It looks like libc is maybe ok with it (if you can arrange to call setenv() programmatically), but if bash chokes a bunch of other ops tools like Docker, Ansible, stuff like that probably choke too. (Those are just speculative examples, I haven't gone and tried them.)

Anyway, it doesn't seem like a great solution for that reason. I don't know if any of the other issues have discussion of more ideas; I wonder if a special env var that could contain HOCON directly, like CONFIG_SNIPPET="foo.bar: [1,2,3]" might be a possibility (this would get parsed then merged in somewhere in the stack, maybe just above all the other env vars?). We could even do separate special env vars for places in the stack, like CONFIG_DEFAULTS= and CONFIG_OVERRIDES= maybe? That's just one idea. These env vars would then NOT be included in the config the way other env vars are, they'd be filtered out of the regular env var config object.

psliwa · 2016-09-27T20:39:19Z

Env var with . in name is able to be set in docker via --env-file option - I tested this and it works (I am not sure if such variable can be set via -e option). In the project that I am working on, we use docker and pass envs via file, so it would suit us perfectly :P I don't know how this looks like in other tools.

I think hacks like CONFIG_SNIPPET are not a good idea. The origin use case of support list for env variables is incorporeality with system properties. In our case for some reason we wanted to move some configuration values from system properties to env variables (some of them were lists) and unfortunatelly it didn't work and we had to use workaround.

psliwa · 2016-11-23T11:04:58Z

@havocp Is there a chance to make system properties and env variables interchangeable? As I said using docker there is a easy way to define env variables with dots and it works, using docker-compose also there is a way to define such env vars.

psliwa · 2017-03-01T19:01:39Z

@havocp is there any chance to merge this?

havocp

Sorry for the delay, I just couldn't immediately decide what to do about it. I guess it can't really hurt to support this.

In a perfect world this PR would be split into separate commits for the fork in Test fix and the properties-parsing feature, using rebase -i or add -p for example.

I'd like to avoid creating a Properties and going through the Parseable code, I think it's a matter of adding a short method to PropertiesParser.java that takes a Map<String,String> instead of a Properties. Otherwise I'm afraid of surprises where someone naturally assumes that the ParseableProperties only handles properties and makes changes that should not be applied to env vars.

Thank you for your patience and help.

havocp · 2017-03-01T19:18:13Z

config/src/main/java/com/typesafe/config/impl/ConfigImpl.java

+        props.putAll(System.getenv());
+
+        return (AbstractConfigObject) Parseable.newProperties(props,
+            ConfigParseOptions.defaults().setOriginDescription("env variables")).parse();


It would be a little cleaner I think to add a static method in PropertiesParser.java akin to fromProperties but instead it would be fromStringMap(Map<String,String>), to avoid going through the Parseable indirection and avoid creating a Properties object here.

psliwa · 2017-03-02T21:55:41Z

Thanks for feedback. I will split those changes into two commits and apply your suggestions in few days.

psliwa · 2017-03-06T20:17:07Z

@havocp I've rebased, split this MR into two commits and applied changes that you suggested. Also I found the reason why build was falling on windows machines (TimeZone.getDefault issue described in a comment).

havocp

Thanks so much for updating the PR, looks good!

havocp · 2017-03-06T20:43:12Z

config/src/test/scala/com/typesafe/config/impl/PublicApiTest.scala

+    def before(): Unit = {
+        // TimeZone.getDefault internally invokes System.setProperty("user.timezone", <default time zone>) and it may
+        // cause flaky tests depending on tests order and jvm options. This method is invoked
+        // eg. by URLConnection.getContentType (it reads headers and gets default time zone).


good find! I imagine that was some work to track down...

Yes, it took few hours. I had to use windows OS because on linux I wasn't able to reproduce the issue - it was horrible :)

ouch, I know the feeling :-)

flyinprogrammer · 2018-07-30T06:28:59Z

Today while trying to use this PR I learned a few things:

Using this: System.out.println(conf.root().render()); to debug configuration is beyond helpful.
The human(s) that made the annotations feature in conf.root().render() really deserve a badge of troubleshooting honor.
The default script that the Gradle Application Plugin generates via CreateStartScripts has a shebang of: #!/usr/bin/env sh, which will often times resolve to /bin/sh.
On debian/ubuntu systems, /bin/sh is typically symbolically linked to dash: /bin/dash
As far as I can tell dash really does not allow you to have environment variables containing dots. In my experience when launching a container with dot environment variables into a dash shell, none of the environment variables were listed when calling env. Conversely, the same test when using a bash shell showed all the environment variables.
After changing the symbolic link of /bin/sh to point to /bin/bash: ln -sf /bin/bash /bin/sh - the substitution of configuration lists via environment variable started working.

I hope this saves future folks some time.

- lightbend/config#427 added support to the Java version of Hocon for lists of environment variables. For instance, given a Hocon configuration file like: "a": ${testList} And the environment variables: testList.0=0 testList.1=1 Then the Hocon configuration file will resolve to the values: "a": ["0", "1"] - Implement the behavior in the Ruby code and Add specs demonstrating this new behavior

havocp reviewed Mar 1, 2017

View reviewed changes

psliwa force-pushed the patch-1 branch 11 times, most recently from 9b7cc1e to 13ae0dd Compare March 5, 2017 21:04

psliwa added 2 commits March 6, 2017 21:12

fork jvm in tests

38dce62

Allow environment vars to be resolved to list

7473d8c

psliwa force-pushed the patch-1 branch from 13ae0dd to 7473d8c Compare March 6, 2017 20:13

havocp approved these changes Mar 6, 2017

View reviewed changes

havocp merged commit 180d66b into lightbend:master Mar 6, 2017

talpr mentioned this pull request Sep 5, 2017

Configuration lists and environment variables akka/akka#23600

Closed

2m mentioned this pull request Oct 2, 2017

Allow environnement variables to be resolved to lists #320

Closed

havocp mentioned this pull request Dec 2, 2017

Exact Match for Environment Variables #488

Open

havocp mentioned this pull request Jun 1, 2018

Nested configuration from environment variables #559

Closed

havocp mentioned this pull request Mar 1, 2019

Add a config.strategy to enable overwrite of properties from env vars #620

Merged

Iristyle mentioned this pull request Oct 30, 2019

(HC-104) Support environment variable lists puppetlabs/ruby-hocon#117

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Allow environment vars to be resolved to list #427

Allow environment vars to be resolved to list #427

psliwa commented Sep 25, 2016 •

edited

Loading

havocp commented Sep 27, 2016

psliwa commented Sep 27, 2016 •

edited

Loading

psliwa commented Nov 23, 2016

psliwa commented Mar 1, 2017

havocp left a comment

havocp Mar 1, 2017

psliwa commented Mar 2, 2017 •

edited

Loading

psliwa commented Mar 6, 2017 •

edited

Loading

havocp left a comment

havocp Mar 6, 2017

psliwa Mar 6, 2017

havocp Mar 6, 2017

flyinprogrammer commented Jul 30, 2018

Allow environment vars to be resolved to list #427

Allow environment vars to be resolved to list #427

Conversation

psliwa commented Sep 25, 2016 • edited Loading

havocp commented Sep 27, 2016

psliwa commented Sep 27, 2016 • edited Loading

psliwa commented Nov 23, 2016

psliwa commented Mar 1, 2017

havocp left a comment

Choose a reason for hiding this comment

havocp Mar 1, 2017

Choose a reason for hiding this comment

psliwa commented Mar 2, 2017 • edited Loading

psliwa commented Mar 6, 2017 • edited Loading

havocp left a comment

Choose a reason for hiding this comment

havocp Mar 6, 2017

Choose a reason for hiding this comment

psliwa Mar 6, 2017

Choose a reason for hiding this comment

havocp Mar 6, 2017

Choose a reason for hiding this comment

flyinprogrammer commented Jul 30, 2018

psliwa commented Sep 25, 2016 •

edited

Loading

psliwa commented Sep 27, 2016 •

edited

Loading

psliwa commented Mar 2, 2017 •

edited

Loading

psliwa commented Mar 6, 2017 •

edited

Loading