feathersjs / feathers

The API and real-time application framework
https://feathersjs.com
MIT License
14.97k stars 744 forks source link

feathers configuration does not load environment variable with empty string #3309

Closed elvince closed 8 months ago

elvince commented 9 months ago

Steps to reproduce

(First please check that this issue is not already solved as described here - if it is a general question or suggestion please start a Discussion)

If feathers configuration file has a variable set to an environment variable (defined in .env file and loaded by dotenv) and that environment variable is an empty string, feathers will not set the variable to empty string but instead to the name of that environment variable.

For example:

// config/default.json
{
    "connection": {
      "user": "DB_USER_DEV",
      "password": "DB_PASSWORD_DEV"
    }
}

//custom-environment-variables
{
    "connection": {
      "user": "DB_USER_DEV",
      "password": "DB_PASSWORD_DEV"
    }
}
// .env
DB_USER_DEV = "root"
DB_PASSWORD_DEV = ""

Expected behavior

Expect user: root, password: ''

It is exactly #2512 that should have been fixed in v5.X but this is not the case. It seems that node_config had been updated to fix that type of issue, but not sure what version you are using or if it is another bug.

Thanks,

Actual behavior

Actual: user: root, password: 'DB_PASSWORD_DEV'

System configuration

FeathersJS v5.x

daffl commented 9 months ago

The main goal was to avoid ambiguity and have the same behaviour as node-config which seems to be the case here. https://github.com/node-config/node-config/wiki/Environment-Variables#custom-environment-variables states that

Empty environment variables are ignored, and their mappings have no effect on your config.

One option would be to keep it empty by default but you can also update the configuration explicitly via app.set before registering the database connection.

if (process.env.DB_PASSWORD_DEV !== undefined) {
  app.set('connection', {
    ...app.get('connection'),
    password: process.env.DB_PASSWORD_DEV
  })
}
elvince commented 8 months ago

Thanks for the feedback.

I didn't saw that sentence.