strongloop / loopback-boot

Convention-based bootstrapper for LoopBack applications
Other
62 stars 71 forks source link

Environment-specific config: replace the whole config, do not merge #93

Closed bajtos closed 6 years ago

bajtos commented 9 years ago

@raymondfeng and @dashby3000 suggested we should change the way how environment-specific configurations are applied.

At the moment, values from datasources.production.json are merged with datasources.json.

The proposal is to drop the merging algorithm and always use only one file. I.e. only load datasources.production.json and completely ignore datasources.json & datasources.local.json when running in production.

The reasoning is that keeping the configuration spread across multiple files makes it more difficult for operations people to quickly see the full configuration that will be applied. It also makes it difficult to switch from host/port/database config to url based config.

Related issues: #82 #4

@rmg @sam-github @ritch @simoami @doublemarked What is your opinion about such change?

bajtos commented 9 years ago

Below are few use-cases from @raymondfeng where the current approach does not work well.

1) The unwanted configuration properties from base can be leaked into production and there is no way to shield them, for example.

base:
{
  mongodb: {
    url: ‘dev-url'
  }
} 

prod:
{
  mongodb: {
    host; ‘prod-server'
  }
}
  1. The datasource can be backed by different DBs between dev and prod and each DB has different configuration properties, for example:
base:
{
  accountDB: {
    connector: ‘mongodb’,
    url: ‘dev-url'
  }
} 

prod:
{
  accountDB: {
    connector: ‘mysql’,
    host: ’mysql-server'
  }
} 

3) Unwanted datasources from base can be leaked into prod. For example, the base might have an Oracle DB for discovery and it’s not needed for prod.

4) Difficulty for devops to find out or script the resolved configuration across multiple files with merging.

bajtos commented 9 years ago

The original complaint from @dashby3000:

Let's say in 'datasources.json', I have the following:

{
"foodb": {
  "url":"mongodb://admin:changeme@candidate.44.mongolayer.com:10308/foo",
  "name": "foodb",
  "connector": "loopback-connector-mongodb",
  "debug": true
}
}

Then in a file 'datasources.dev.json', I have the following:

{
"foodb": {
  "name": "foodb",
  "connector": "loopback-connector-mongodb",
  "database": "local_foodb",
 "host": "localhost",
  "port": 27017
}
}

When I run NODE_ENV=dev slc run it will always connect using the URL property defined in the 'datasources.json' file.

rmg commented 9 years ago

For environment based JSON config files like this I think it makes sense to override the whole config rather than merge them. So :+1: from me on this change.

If these config files were JS instead, then it would make sense to simply execute them in order (default, $env, $host, etc) and let the code in each handle whether it merges or replaces.

altsang commented 9 years ago

+1 as well, I would prefer that the merging happen optionally with another module if that's require or in the case where we want to be copacetic with an configuration management utility like Chef, Puppet, Ansible etc, those systems would handle the final output of the merge through templatization and variables defined

STRML commented 9 years ago

If this is done, I think it should be opt-in or at least opt-out. Otherwise, there is a lot of copy/paste dancing going on and the possibility for configuration to be missed. Considering this is javascript, and undefined property accesses don't throw, I can imagine this causing a lot of hard-to-find bugs.

:+1: on making configuration files JS instead of JSON though. The flexibility is very, very nice to have.

I simply use a configuration file format like:

'use strict';

module.exports = {
  // ...
};

And the exports can be massaged by living code rather than being strict configuration.

codyolsen commented 8 years ago

+1 for making this optional.

MerrickClark commented 7 years ago

Was caught off guard by the default merge of config files & would like to see this feature get launched! Optional of-course to prevent breaking all the things.

bajtos commented 6 years ago

In October 2018, LoopBack 4.0 GA was released. LoopBack 3 and its supporting modules have entered LTS mode. Per our Long Term Support Policy, this means new features are no longer accepted.

I am closing this GitHub issue as "wont fix".