Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Replace strings in Dancer2 config with environment variables #1628

Open
mikkoi opened this issue Jul 23, 2021 · 12 comments
Open

Replace strings in Dancer2 config with environment variables #1628

mikkoi opened this issue Jul 23, 2021 · 12 comments

Comments

@mikkoi
Copy link

mikkoi commented Jul 23, 2021

I want to run my Dancer2 application in a docker but I want to run the
container without mounting any local directories and I want to run the same
once-built container in all environments, not compile it separately for different
environments.

I would like to pass parts of the configuration as environment variables, e.g.
DB_HOST and SITE_URL, and then apply these over the values in
config.yaml when I start Dancer2 application.

Suggestion:
Change the command config (or the underlying file reader function)
so that it can replace environment variables in config strings, e.g.

plugins->database->connections->main->host: ${ENV:DB_HOST}
@mikkoi
Copy link
Author

mikkoi commented Aug 22, 2021

A script which creates config_local.yaml on the fly and then starts Dancer2 app would otherwise work but it means that container won't be immutable. Immutable is one of the architectural best practices listed by Google.

As it happens, this container is planned to run on Google Container Engine.

Using a tmpfs is also not an option because it would mean that container is dependent on the way it is being run (the runtime environment).

Substituting variables in config.yaml is not overly complicated in my mind. Many platforms use environment variables to pass information to containers, docker or others, e.g. Java jar. I have used Openshift platform and it gave me a very good example of how env vars are governed in order to provide secrets to the container.

By stating the env vars in the configuration, we make it clear to the programmer and to operations what is expected from the runtime environment. Besides this, it also makes local development easier if you "practice dotenv" - what The Twelve-Factor App recommends. I keep my environment in a .env file and I also use the same file when testing the container with local docker.

See:

@racke
Copy link
Member

racke commented Aug 23, 2021

Yes, I can see the need to override Dancer config with environment variables without a start script. From my view a better place for the transformation would be a Dancer plugin if that is technically possible instead of changing Dancer's core.

@mikkoi
Copy link
Author

mikkoi commented Aug 23, 2021

Plugin would be a great way to do it. Then there wouldn't be any compatibility issues. But the configuration system is not "pluggable", i.e. there is no hooks where a plugin could attach, AFAIK.

@cromedome
Copy link
Contributor

cromedome commented Aug 23, 2021 via email

@mikkoi
Copy link
Author

mikkoi commented Aug 23, 2021

Great to hear. I'd like to volunteer for that project.

@veryrusty
Copy link
Member

How much "configuration" does your app have ? You can define the settings inline:

package MyApp;
use Dancer2;

set foo => $ENV{FOO} // 'default-foo';
set bar => { baz => $ENV{BUZZ} };

1;

@mikkoi
Copy link
Author

mikkoi commented Aug 23, 2021

It has quite a lot of configuration. It interfaces with two web applications, there is JWT configuration and customizable urls.
And it also talks with a database, using Dancer2::Plugin::Database.

@xsawyerx
Copy link
Member

I think this is doable in core. It is unlikely to break any existing applications because I don't imagine many of them would be using something like $ENV{...} as a value. If any of them did, it is probably because they then eval that as code to run in the application itself to get the value - which is a major security issue.

The way I think this should work is by specifying in clear syntax how to retrieve something from ENV and not be an eval'ed piece of code. That's how security issues like Log4Shell happen.

plugins->database->connections->main->host: $ENV:DB_HOST

Here's how I see it:

  • Our configuration formats (YAML, JSON) have no support for variables, so this will not be interpolated.
  • Even if we were to store the configuration in a runtime system, it will not be able to interpolate this since $ENV:FOO is not valid Perl syntax. However, in a string "$ENV:FOO" is treated as "${ENV}:FOO" which is valid, but this can be escaped in any runtime configuration system: "\$ENV:FOO" or single-quoted as '$ENV:FOO'.
  • Any runtime configuration system can store this as a string.
  • We could do a quick check of whether the first argument is a $ sigil, then see if the next string (ENV) is a recognized "source of configuration." ENV would be the only source supported at the moment. We then know to go to $ENV{$next_token}.
  • This way, no actual parsing eval takes place so it stays safe.

I can imagine only two situations in which this is still a problem:

  1. Someone has "$ENV:FOO" in their config and do something with that string. We can release a version in which we warn of stuff like that, but I doubt this exists.

  2. If you have a system that processes the configuration and does a very simple substitution like:

{
    no strict;
    s/ ( \$ \B+  ) / *{$1} /xmseg;
}

(or something crazy like that)

It will not catch that this $whatever was actually \$whatever or that it was in single quotes.

In that case, I think you're doing something so wrong, there is no expectation for us to make it work.

@mikkoi
Copy link
Author

mikkoi commented Jan 23, 2022

I got a suggestion from @cromedome to change the ConfigReader role so that it would be possible for user to implement a hook which could do what I suggest above. I tried it but it was impossible or required more and deeper changes because using a hook created a circular dependency. I think the hooks are only usable after the App is setup and dancing.

So I went with a different approach. Configs are loaded by classes which implement the role Dancer2::Core::Role::ConfigReader.
User can create custom classes to use instead of Dancer2::ConfigReader::FileSimple.
Custom classes are activated via env var DANCER_CONFIG_READERS. If the env var is not set, app behaves like before.

There is an example where user's custom class is extending Dancer2::ConfigReader::FileSimple.

@mikkoi
Copy link
Author

mikkoi commented Jan 23, 2022

#1636

@xsawyerx
Copy link
Member

I can promise to review it this week, but not on which day.

I think having the config reader be a role is a good idea, but I'm also - aside from that - don't mind this functionality (populate config from an environment variable) be part of the core.

@cromedome
Copy link
Contributor

I will review this week, but like @xsawyerx I am not sure when yet.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants