Skip to content
This repository has been archived by the owner on Jan 10, 2022. It is now read-only.

Improved support for environment variables #99

Merged
merged 2 commits into from
Dec 3, 2019

Conversation

allouis
Copy link
Contributor

@allouis allouis commented Nov 26, 2019

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')

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
Copy link
Contributor Author

allouis commented Nov 26, 2019

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
Copy link
Member

ErisDS commented Nov 26, 2019

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: TryGhost/Ghost#8960 & indexzero/nconf#271 (comment)

@allouis allouis closed this Nov 26, 2019
@allouis allouis deleted the improve-config-from-env branch November 26, 2019 12:21
@allouis allouis restored the improve-config-from-env branch December 3, 2019 11:30
@allouis
Copy link
Contributor Author

allouis commented Dec 3, 2019

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

@allouis allouis reopened this Dec 3, 2019
lib/config/index.js Outdated Show resolved Hide resolved
lib/config/index.js Outdated Show resolved Hide resolved
no-issue

We only need the functionality of the separator
@allouis allouis merged commit 1a6a271 into master Dec 3, 2019
@allouis allouis deleted the improve-config-from-env branch December 3, 2019 12:03
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants