TryGhost / Ignition

Basic configuration and tooling shared across applications
MIT License
16 stars 21 forks source link

Improved support for environment variables #99

Closed allouis closed 4 years ago

allouis commented 4 years ago

no-issue

When running applications with docker-compose it is nice to pass config as environment variables defined in the docker-compose.yml

This change will allow an environment variable like DATABASE__HOST (note double underscores) To be read like Ignition.config().get('database:host')

allouis commented 4 years ago

When passing config as environment variables, we can't pass nested config.

e.g. running database:host=something node index.js fails

A more concrete example of the docker compose stuff is like so:

// some service
const config = Ignition.config();

makeANewDb({
  host: config.get('db:host'),
  pass: config.get('db:pass')
});

And in a docker-compose.yml I would have something like

services:
  db:
    image: some-db-image
  my-service:
    build: ./my-service
    environment:
      DB__HOST: 'db',
      DB__PASS: 'some-password'
ErisDS commented 4 years ago

Isn't this just because you have to match case?

services:
  db:
    image: some-db-image
  my-service:
    build: ./my-service
    environment:
      db__host: 'db',
      db__pass: 'some-password'

https://ghost.org/docs/concepts/config/#running-ghost-with-config-env-variables

More relevant context: https://github.com/TryGhost/Ghost/pull/8960 & https://github.com/indexzero/nconf/issues/271#issuecomment-337667124

allouis commented 4 years ago

I got round to testing this, and even matching case doesn't work - I looked at how/why this works in Ghost - and it's because Ghost isn't using Ignition to load the config, it uses nconf directly and sets the separator key here https://github.com/TryGhost/Ghost/blob/master/core/server/config/index.js#L27-L33

So now I think that we should at least pass the separator so that we can do the case matching version of this