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

Add config validation and coercion like in the nest configuration documentation #33

Open
ghost opened this issue Oct 2, 2018 · 9 comments
Labels
enhancement New feature or request

Comments

@ghost
Copy link

ghost commented Oct 2, 2018

In https://docs.nestjs.com/techniques/configuration it suggests to use joi.

@bashleigh bashleigh self-assigned this Oct 2, 2018
@bashleigh bashleigh added enhancement New feature or request good first issue Good for newcomers labels Oct 2, 2018
@bashleigh
Copy link
Collaborator

yea that's a good idea! However I'm not sure what would define the required valid keys? Another glob path perhaps? What do you think?

@ghost
Copy link
Author

ghost commented Oct 2, 2018

The closest thing to this design wise that I know of is the symfony config component.

It uses a a resource locator to define where to find the config sources: https://symfony.com/doc/current/components/config/resources.html
And a definition class to define the schema validator:
https://symfony.com/doc/current/components/config/definition.html

Of course most of what it does is overkill since it's for supporting reusable libraries, multiple config defintion styles (yaml, php, ini, xml, etc) and working around php issues (like arrays vs objects), but it's still a neat design.

@bashleigh
Copy link
Collaborator

Ok, so! @shekohex and I had a discussion about this. He's suggested using a key value type validator that will:

  • prevent your script from compiling if an incorrect key has been requested
  • prevent your script from compiling if an invalid type of default value is passed into the get func
  • prevent your script from compiling if an invalid type of value is used in your config files/ envs etc

He's added a basic gist here :)
https://gist.github.com/shekohex/f051aac07850ee96d496818629845ef9

Let me know what you think. Personally I prefer this because it will prevent my jenkins pipelines from completing so it won't deploy the image to production or staging.

@ghost
Copy link
Author

ghost commented Oct 5, 2018

How about numbers within range though? It'd be nice if the validator could accept function so you could say for example that a port is within valid port range, or simply that the source of truth is somewhere else.

@bashleigh bashleigh removed the good first issue Good for newcomers label Oct 8, 2018
@bashleigh
Copy link
Collaborator

Ok so after discussing this again we've decided to add this as an optional feature to v2. Using both a key manager and using class-validator to offer support for decorated value validation.

@moltar
Copy link

moltar commented Nov 4, 2018

There's also https://github.com/arvitaly/typio

@hejkerooo
Copy link

https://www.npmjs.com/package/env-var

@bashleigh
Copy link
Collaborator

bashleigh commented Jun 17, 2019

In v2 of nestjs-config I've added the ability to use classes and interfaces in your config files. I know it's not a true validation but it's a start! I want to add a package agnostic implemention of validation to the nestjs-config package so you can use joi or coercion or whatever. But I feel like this is a lot of work and maintenance. What do you guys think of the v2 implementation of interfaces?

Another idea I've had is to create a metadata on a service/provider and call a method on the provider after the env providers have been validated. Then returning true or false determines whether the configurations are valid.

Something like

import {ConfigValidator, IConfigValidator, ConifgProvider} from 'nestjs-config';

@ConfigValidator()
export class ConfigValidatorProvider implements IConfigValidator {
  constructor(private readonly providers: ConfigProvider[]) {}

  validate(): boolean {
    this.providers.forEach(provider => {
      // validate the provider
    });

    return true;
  }
}

@bashleigh bashleigh pinned this issue Jun 17, 2019
@cheelahim
Copy link

I second @hejkerooo recommendation. The env-var plays well with current config implementation. Here is what you can do to both validate and coerse the env variable values.

import * as env from 'env-var';

export default {
  name: env.get('APP_NAME', 'Application').asString(),
  port: env.get('APP_PORT').required().asIntPositive(),
  ...
}

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

No branches or pull requests

4 participants