Config: Support multiple source extension in any object configuration

[sajithneyo]

Use case description

Currently, environment option in provider and in function level can only accept a single JSON object. It's better to have multiple object support.

Maintainer's note:

As we receive requests to support this notation also in other places, it'll probably be good to solve with one generic solution, instead of hard-coding support for array notation in all needed places one by one.

Proposed solution (Strategy)

1. Universal solution (not specific to configuration file type)

Introduce a merge variable source, so single object or array can be constructed from multiple sources as follows:

custom:
  environment:
    FOO: value

provider:
  environment: ${merge(${self:custom.environment}, ${file(common-env.json)}})}

2. YAML configuration specific

Introduce !merge YAML function, which could work as follows

provider:
  environment: !merge
    - FOO: value
    - ${file(common-env.json)}

How to implement it? (Tactics)

Note: Solution for both methods should be proposed with two different PR's

1. Universal solution (not specific to configuration file type)

1. In lib/configuration/variables/sources Configure merge source (it should be accompanied with tests that provide 100% coverage). On how to implement it, some hint could be taken from file source configuration. Still in case of merge I believe we will only need to process params argument.
   Important constraints to met:
   • Support constructing plain objects and arrays exclusively
   • Imply a top level merge. In plain object case it should work similarly to Object.assign({}, param1, param2, ..). In case of arrays: [].push(...param1, ...param2)
   • In case of plain object construction throw an exception if two input objects share same key (let's not silently override them)
   • Ensure that all input params are either exclusively plain objects, or exclusively arrays (throw otherwise)
2. Add source to variables resolution in:
   • scripts/serverless.js
   • lib/configuration/variables/index.js
3. Document new variables source in docs/providers/aws/guide/variables.md

2. YAML configuration specific

1. Configure schema extension which adds !merge support to YAML. Hint on how such extension can be configured can probably be taken from @serverless/utils/cloudformation-schema.js which add support for functions as !Ref , !GetAtt etc.. Let's put definition of new YAML types to lib/configuration/yaml-merge.js.
   As items passed to !merge may be constructed with Serverless Variables, which cannot be resolved at the point of YAML resolution. We need to convert this notation into merge variable source (implemented at step 1). Additionally as merge source cannot accept inline object or array structures, we need to build some temporary collection attached directly to configuration, in which we can store items, and which we can address in merge sources. To achieve that I propose that:
   1. lib/configuration/yaml-merge.js when resolving the !merge constructs, builds in a memory a temporary object with all referenced items, and in merge variable sources address them via ${self:_yamlMerge.<configuration-path>}, e.g. if we list item that was configured at foo.bar[0] property, then we should address it in merge source param as ${self:_yamlMerge.foo.bar[0]} (Reason for preserving same paths, it to ensure meaningful error if e.g. variable resolution fails at those places)
   2. In lib/configuration/read.js once YAML is resolved into JSON, we should retrieve temporary object from lib/configuration/yaml-merge.js and add it to result configuration at _yamlMerge property. Additional notes:
      1. If there were no !merge tags, no _yamlMerge object should be created
      2. Once temporary object is retrieved from lib/configuration/yaml-merge.js, it should be discarded in context of lib/configuration/yaml-merge.js (so e.g. repeated configuration reads, do not build up on same temporary object)
   3. In scripts/serverless.js (before we do serverless.run()), delete the _yamlMerge property from the configuration
2. Let's ensure YAML schema is extended with new types at: lib/configuration/read.js
3. Document this YAML extension in docs/providers/aws/guide/variables.md

[medikoo]

@sajithneyo great thanks for proposal!

It definitely makes sense to support input from many sources there.

Still we have many places like that, and I wonder whether we can come up with some global solution that will allow us to use it everywhere, and not address each property like that individually.

It'll probably have to be some variables resolution extension

[sajithneyo]

@medikoo yeah, it's better to have a global solution for that. but until that, it's better to have this feature for envs.

[medikoo]

I was thinking about sorting this with notation as following:

environment:
  FOO: someVal
  $extend:
    - ${self:custom.obj1}
    - ${self:custom.obj2}

Still one problem I see, is that setting $exclude key (literally) would be impossible with that. Ideally we should not limit what keys can be used in objects (still that problem is probably already shared by var resolvers, which do not offer any escape mechanism to ${)

[glb]

@medikoo it looks like the gitlab folks are doing something similar in their config by extending YAML syntax, you may want to check out their proposal.

[medikoo]

@glb great thanks for linking that. That looks very neat. Indeed maybe then it'll be better to do it as follows:

environment:
  FOO: someVal
  !merge:
    - ${self:custom.obj1}
    - ${self:custom.obj2}

What do you think?

Note: I've put it slightly differently, as I think notation they propose (in rules: !merge example), will not really work, as it doesn't seem as valid YAML structure (but I didn't tested that through)

[glb]

@medikoo I don't know enough about how tags work in YAML processing to comment. I liked their proposal, if that's something that can actually be done.

[medikoo]

@glb I've replaced $extend with !merge in implementation proposal, as I think it's more intuitive, and as it's something else as our variables, I think it's ok to use ! not $ as prefix.

PR that implements that is welcome!

[medikoo]

On revisiting this, I have following thoughts:

A solution that works across all types of configs (YAML, JSON etc), is probably best if configured with our variables syntax. We can introduce e.g. ${merge(obj1, obj2, obj3)} which will do single level merge on either plain objects or arrays.

Downside is that we cannot merge inline part with externally configured parts. ${merge}could only work with externally configured parts. Therefore eventual inline part if needed will have to be configured in some side property, e.g.:

custom:
  environment:
    FOO: value

provider:
  environment: ${merge(${self:custom.environment}, ${file(common-env.json)}})}

Another downside of that, is that in context of YAML, it's not as convenient to use as e.g. would be a solution based on YAML function. Still I think we may also add support for such YAML function which will work only in context of YAML confiugurations, and may look as:

custom:
  environment:
    FOO: value

provider:
  environment: !merge
    - FOO: value
    - ${file(common-env.json)}

Both methods I think should be relatively easy to implement. @pgrzesik @glb what do you think?

[pgrzesik]

Thanks for proposal @medikoo. I like the approach based on variable syntax as it's the most universal, minus the downside that you highlighted. Then, if there's a need for that, we could also add the second, yaml-specific syntax with !merge

[medikoo]

Thanks @pgrzesik I've updated main description with instructions how we can implement it. Let me know what you think about it.

As you mention I would definitely first got with merge variable source, and then eventually also !merge YAML extension can be provided

[glb]

Still required after serverless/utils#79 and #9314 are merged (if they're acceptable):

1. version bump for serverless/utils
2. update serverless/serverless to pick up new serverless/utils version, document !merge tag. Is variables.md the right place as stated in the description or is there somewhere else that makes sense?

[glb]

@medikoo I am running into some complexity with the !merge version of this, which I think is actually the most useful one. The problem is that the !merge processing happens during YAML parsing, but if someone has something like:

resources:
  Resources: !merge
    - ${file(file1.yml)}
    - ${file(file2.yml)}

we won't know the resolved values of ${file(file1.yml)} and ${file(file2.yml)} until after variable resolution.

I tried making the !merge args implementation simply insert ${merge(args)} but it doesn't work with inputs like:

resources:
  Resources: !merge
    - a: 1
    - b: 2

because Variable syntax error at "test": Invalid variable param at index 8 in "${merge({"a":"1"},{"b":"2"}}). What I'm currently considering is remembering the inputs, replacing with placeholders, and transforming into something like ${merge(${_placeholder(1),${_placeholder(2)}} where _placeholder retrieves the stored values. _placeholder is probably not the right name for this.

Thoughts?

[medikoo]

@glb thanks for sharing. Indeed, when specyfing I didn't think of that culprit, and that's definitely tricky to solve, while needs to be solved

I think your idea with _placeholder is interesting, still (putting naming aside for now), all those items in an array need to have variables resolved naturally, so question is, where values behind placeholders will be stored, and how will variables be resolved in them (?)

Idea that comes to my mind (not sure if it's best we can do), is to:

• Introduce a temporary top level configuration property, e.g. named _temp
• When resolving !merge, put into _temp all its items, and in result ${merge(..) simply reference them via self:_temp....
• After variables resolution is done, have _temp property automatically removed from configuration

What do you think? If that's really best we can do, I can outline on how we can implement that.

[glb]

@medikoo that's likely one way to go about it, sure.

[medikoo]

@glb I've updated spec in main description to reflect this solution. Let me know what you think

[throrin19]

Any news about the merge ?

[medikoo]

@throrin19 are you interested in submitting the PR?