roadrunner icon indicating copy to clipboard operation
roadrunner copied to clipboard

[💡 FEATURE REQUEST]: Allow defaults in env var expansion of server.env config

Open devnev opened this issue 1 year ago • 2 comments
trafficstars

Plugin

Server

I have an idea!

The current handling of server.env configuration does a plain os.Expand(_, os.Getenv()). Unlike other parts of the roadrunner config, this doesn't allow for defaults in the form ${ENV:-default}.

It would be great to allow defaults, or even go one step further and use https://github.com/drone/envsubst both here and in the general configuration for some more advanced substitution options.

devnev avatar Aug 16 '24 23:08 devnev

Hey @devnev 👋 Yeah, you're right, I forgot about these envs to standardize them. It would be nice if you'd be able to send us a PR. I wrote a custom env parser (std+some additions): link. The steps for anyone wanted to contribute:

  1. Copy that file to the server plugin.
  2. Replace this and this env parsers from the default os.Expand to custom.
  3. Write tests for that 😃

rustatian avatar Aug 17 '24 08:08 rustatian

@rustatian I started implementing this in the server plugin, but was then wondering why config plugin wasn't already doing this. It turns out I was using a slice-of-maps in the config for server.env, which viper with WeaklyTypedInput=true will merge into an output map when required, but isn't included in the results of viper's AllKeys method when the config plugin does its env expansion. Thet led me down a rabbit hole of how viper decodes things, and I've raised https://github.com/roadrunner-server/config/pull/60 that should make it consistent, but might also be enough of a breaking change to warrant more careful consideration.

devnev avatar Aug 21 '24 03:08 devnev