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

Feature Request: Using both external and serverless.yml env vars #4571

Open
simlu opened this issue Dec 13, 2017 · 42 comments
Open

Feature Request: Using both external and serverless.yml env vars #4571

simlu opened this issue Dec 13, 2017 · 42 comments

Comments

@simlu
Copy link

simlu commented Dec 13, 2017

I'm trying to use external variables from an external configuration file and internal variables from serverless.yml as environment variables.

I've tried

  environment:
    STAGE: ${self:provider.stage}
    REGION: ${self:provider.region}
    <<: ${file(config/${self:provider.stage}.yml)

However this gives me the error YAMLException: cannot merge mappings; the provided source object is unacceptable

Is there currently a way to achieve this or is this a feature request?


Note from maintainers:

You might be interested in Stage Parameters.


Related (also unanswered questions):

@horike37
Copy link
Member

Thank you for reporting @simlu 👍
This would be a feature request since currently, it seems that double left arrow is not supported yet.

@simlu
Copy link
Author

simlu commented Dec 16, 2017

Is there a way to achieve this by using different contexts? E.g. you could use function + service environment variables. Is there a more generic way?

@simlu
Copy link
Author

simlu commented Jan 10, 2018

@horike37 Would you mind pointing me to the relevant file(s)? I'd like to make this my first contribution! Cheers

@horike37
Copy link
Member

AWESOME @simlu 👍 💯 It would be great if you can implement this improvement!

In general, the yaml file is parsed within YamlParser.js, which actually leverage js-yaml as a YAML parser library. Therefore, I think the base way is that js-yaml has supported for the merge syntax(I'm not sure if so or not). First, you might want to investigate that.

If not, we should consider other solution for that.

@HyperBrain
Copy link
Member

js-yaml is currently invoked with "safeLoad". This is safe but disallows YAML references that load multiple files as far as I know.
You can try to use the "load" function instead and see if all YAML references magically start to work :)

@simlu
Copy link
Author

simlu commented Jan 11, 2018

@horike37 Thank you for the input!
@HyperBrain As far as I can see currently load is used. Where are you seeing safeLoad?

@simlu
Copy link
Author

simlu commented Jan 12, 2018

Seems like https://github.com/whitlockjc/json-refs is used to resolve the dependencies after the yaml is parsed. It makes sense that we are seeing this error. Any objections to restructuring it so that the resolve of dependencies happens before parsing the yaml?

@HyperBrain
Copy link
Member

@erikerikson , do you have any opinion here regarding the json-refs location? It might work only partially if resolved before parsing the yaml.

@erikerikson
Copy link
Member

Sorry, no opinion in particular.

It's not clear to me that json-ref will deal with unparsed yaml. That module was originally introduced in order to help deal with circular references in the service object (i.e. ${self:}).

It seems there is some confusion here though. Variable resolution happens at a whole different phase than yaml parsing but @simlu, you're trying to mix the two here. The yaml is parsed into a JavaScript object and only after that parsing is complete are any variable populations attempted. Therefore as written, you are not attempting to merge an object but a string (i.e. '${file(config/${self:provider.stage}.yml)') into your environment object. I don't know the defined behavior for this attempt but I would not expect it to yield what you would seem to like to result.

Adding a merge capability into the variable system doesn't seem unreasonable so you could accomplish this but it seems that it would be easy to accomplish the same result now albeit less tersely using overrides on individual values. Alternatively, unless you're actually merging over top of other values, you could simply be more explicit about where the data is coming from.

@simlu
Copy link
Author

simlu commented Jan 12, 2018

@erikerikson Ah yes. Sorry I wasn't clear at all.

There are three stages;
(1) yaml parsing
(2) json-ref processing and
(3) variable resolution

Basically the syntax we want to support works fine for (1). However it leaves the data invalid for (2). So my suggestion it to move (3) before (2) or even before (1).

From what you are saying it sounds like that defeats the purpose of (2) though?

Do you have a better idea? I can always hack something in for this one case. But ideally we solve this properly :)

@erikerikson
Copy link
Member

I won't be responding fully just now (I'm on vacation) but I think it would be helpful to be absolutely crisp about the use case you're trying to enable. Is it simply the merge? Is there something you are blocked from accomplishing? Et cetera...

@erikerikson
Copy link
Member

Following up here...

js-yaml supports the merge operator. however, in your case you want it to merge the object that would result from replacing not yet rendered variable. yaml parsing/resolution of the merge operator happens at file loading time, prior to variable resolution.

Due to this and no plan to adjust the ordering because of the complexity of properly altering YAML (consider arbitrary indentation contexts when inserting a complex object), I think that this is asking for a new variable capability. Which is why I recommended that if you want change you ought to be very crisp about the use case and exactly the scenario(s) you want to enable, how you would expect it to work, et cetera.

FWIW, I did actually encounter a case where a merge could have been helpful, so it's not entirely insane. I'm fixing #4687 currently and will have familiarity if you can provide a legible notion of the feature request.

@simonbuchan
Copy link
Contributor

In order to support the expected syntax, you would probably have to parse the config file using yaml-ast-parser only, then implement transforming the nodes to their value, then inject variable substitution when transforming a YAMLScalar. Sounds like a lot of work!

It might be simpler to consider permitting yaml tags for variables, similar to the cloudformation shorthands, for example:

  environment:
    STAGE: !self provider.stage
    REGION: !self provider.region
    <<: !file ['config/', !self provider.stage, '.yml']

js-yaml allows you to provide a custom schema where you can essentially define callbacks for when such tags are seen, but the API is pretty annoying and is underdocumented: https://github.com/nodeca/js-yaml/wiki/Custom-types - in particular, AFAICT it forces each tag to only accept one of str, seq or map for the value, where you would want both str and seq so you could nest variables, like above. So this may need some lower-level work.

@erikerikson
Copy link
Member

@simonbuchan thanks for sharing your knowledge! Interesting option that I was ignorant of but find interesting. This could imply ceding some UX control and potentially break custom formats or the like but maybe I'm just exposing more ignorance: interesting nonetheless.

@simonbuchan
Copy link
Contributor

On the plus side, the use-case for custom formats was a conflict with cloudformation Sub templates, adding the ability to specify variables with YAML tags wouldn't hit the same issue since the syntax is already reserved by YAML itself. Tag names could conflict, true, but js-yaml doesn't support parsing unknown tags anyway, and if it did you may (or may not?) be able to work around a conflict with something like !quote !file not-serverless-file-var -> !file not-serverless-file-var. There's also a whole thing with global (uri) tags vs local (app-specific) tags, but heck if I know how those work :)

@erikerikson
Copy link
Member

BTW, a work around here is to move your STAGE and REGION into the config/${self:provider.stage}.yml file. They can continue to be defined as given and will be rendered in the context of the loading serverless.yml.

environment: ${file(config/${self:provider.stage}.yml)

@erikerikson
Copy link
Member

@thomasmichaelwallace provided an interesting option elsewhere:

we do the equivalent of cat serverless.common.yml serverless.project.yml > serverless.yml && serverless deploy

You can then use yaml anchors to pull in bits from .common file.
(notice that serverless ignores top level yaml objects that are not special to it)
So your example would look like:

.provider: &provider
  name: aws
  runtime: python3.6
  stage: ${opt:stage, 'dev'}
  region: us-east-1
provider:
  <<: *provider
  customThing: "my custom thing"```

Particularly if you were to do something like define stage as an environment variable:

export STAGE=dev
cat config/$STAGE.yml serverless.project.yml > serverless.yml && serverless deploy

@thomasmichaelwallace
Copy link
Contributor

Although I do use this in production, it's a bit of a hack.

Can I suggest we take a leaf out of webpack's book and support serverless.config.js, which exports a common is config object.

That effectively allows you to do anything, as you're free to code various solutions- and use common plugins.

Assuming I haven't missed that this already exists, and people think it could be a good idea, I'm happy to put together a PR.

@simonbuchan
Copy link
Contributor

Well you might be able to do something like:

.realconfig: &realconfig
  ${file(./serverless.js)}

<<: *realconfig

But you're probably better off just using custom: ${file(./serverless.config.js)} then splatting bits in where needed with ${self:custom.some.custom.value}

@thomasmichaelwallace
Copy link
Contributor

Interesting- I don't think that approach would have ever occured to me.

The splatting bits works up to a point (that's what we do with the concat; as the common file is centralised); but yaml has it's limits (like no list merging, or conditional anchor references).

I'm still fairly taken with the idea of webpack-like configuration, so I've raised a separate issue (#4787) as to not clog up this commentary.

@simlu
Copy link
Author

simlu commented Mar 12, 2018

The plugin serverless-merge-config should resolve this. Big thanks to @Omicron7 - However there seem to be other problems with parameter resolutions.

Considering how important config resolution is and how many critical issues surfaced around it, it might be worth taking another high level look at how the different resolution steps work and what is resolved where.

After digging through the code it feels like there are a bunch of patches implemented, rather than having a high level design that solves the problems more generically. But unfortunately I didn't get super deep into untangling.

References:

Related: #4817, #4814, #3511, #2997

@simonbuchan
Copy link
Contributor

I feel doing the at the AST level is definitely the right way to go, it's just it's rather a pain to do at the moment with the APIs available.
It seems there's definitely discontent about the yaml parsers out there: mulesoft-labs/yaml-ast-parser#27 -
https://github.com/eemeli/yaml looks quite nice so far.

@thomasmichaelwallace
Copy link
Contributor

thomasmichaelwallace commented Mar 12, 2018 via email

@simlu
Copy link
Author

simlu commented Mar 12, 2018

@simonbuchan @thomasmichaelwallace Might be interesting for reference: #4817 (comment)

Edit: Here is the important file https://github.com/simlu/lambda-serverless-api/blob/master/lib/util/yaml.js

@simlu
Copy link
Author

simlu commented Mar 14, 2018

Based on discussion with @erikerikson I'm detailing our use case a bit more.

We have two separate stacks that we deploy with serverless: One data stack that we deploy when the service gets initialised and an api stack that our CD deploys. These are different stacks, but share the same code and some configuration.

We define our serverless.yml as ${file(./stacks/${opt:stack, 'api'}.yml)} (this also only works because we use custom logic in serverless.js to load our yml).

Ideally we would share common information in the serverless.yml instead of duplicating it (e.g. provider), but as this thread highlights this is not easily possible.

Discussion in the other thread would indicate that our use case is not really what serverless configuration is designed for - hence using serverless.js might be the viable option as it allows us to support the syntax mentioned here and others. E.g. I'd love to use something like

provider:
  name: aws
  runtime: nodejs6.10

.stack: &stack
  ${file(./stacks/${opt:stack, 'api'}.yml)}
<<: *stack

but that won't work for various reasons.

@simlu
Copy link
Author

simlu commented Mar 18, 2018

We are now using yaml-boost and it works really well.

Basically it replaces serverless.yml with serverless.core.yml and creates a new serverless.js that loads the rename yml file and parses is as appropriate. Benefit for us is that we get full transparency into what is passed to serverless and can extend/improve config loading functionality as appropriate.

@iDVB
Copy link

iDVB commented Oct 25, 2018

@simlu are you still using yaml-boost to resolve this limitation?
Seems like your project https://github.com/simlu/lambda-monitor is not working with the latest SLS?

@simlu
Copy link
Author

simlu commented Oct 25, 2018

@iDVB Yes, we are still using yaml-boost in a lot of projects (it has become our bread and butter).

I will take a look at the ticket you opened in lambda-moitor in a bit.

@piokania
Copy link

BTW, a work around here is to move your STAGE and REGION into the config/${self:provider.stage}.yml file. They can continue to be defined as given and will be rendered in the context of the loading serverless.yml.

environment: ${file(config/${self:provider.stage}.yml)

That worked for me, thanks!
Here's a sample as I couldn't find it in docs:

serverless.yml

provider:
  (...)
  region: us-east-1
  stage: dev
  environment: ${file(environment.yml)}

environment.yml

STAGE: ${opt:stage, self:provider.stage}
REGION: ${opt:region, self:provider.region}
other_env_variable: 123

that allows to pass region/stage through cli:

serverless deploy --stage qa --region eu-west-1

@extJo
Copy link

extJo commented Feb 11, 2020

@horike37

Hi, back to the title. any plan to support using both external and serverless.yml env vars?

@erikerikson
Copy link
Member

@extJo maybe I'm missing something but try looking into the ${env:VAR_NAME} variable type. This issue is about a configuration merge operator (i.e. merging the object in the referenced file into the object at the same level).

@extJo
Copy link

extJo commented Feb 11, 2020

@erikerikson i know that way. the point is that plan to support merging the object in the referenced file into the object at the same level.

@erikerikson
Copy link
Member

Thanks, I was confused about you language and wanted to be sure. Glad you were aware.

@extJo
Copy link

extJo commented Feb 11, 2020

@erikerikson Sorry about that :).
anyway, are you plan to support merging operation?

@erikerikson
Copy link
Member

I'm not nearly the regular contributor to the framework I once was. I'd love to be but anyway... I doubt the maintainers would turn down a good PR. ;D

@atz
Copy link
Contributor

atz commented Dec 3, 2020

The thing I need is a yaml anchor attached to the top of an imported file object. I understand the complexity with phases and varying syntaxes, but this should not still be intractable now three years later!

FAILS

These are non-working examples, but represent the 3 parts that need to fit together:

environment: &environment ${file(config/${self:provider.stage}.yml)
environment: &environment 
  <<: ${file(config/${self:provider.stage}.yml)
environment: 
  <<: &environment 
  ${file(config/${self:provider.stage}.yml)

IDEAS

IMO, it would be sufficient to allow a workaround syntax:

environment: ${anchored_file(config/${self:provider.stage}.yml, environment)}

AFAICT, This would reduce the number of different tools in play and put the control in the hands of the implementer of anchored_file.

More atomically, one would really only need to produce a serverless function anchor like:

environment: ${anchor(file(config/${self:provider.stage}.yml), environment}

Which then might see reuse with other config retrieving functions besides file. The YAML anchor is the payoff.

Obviously, this still conflicts with the presumption that YAML parsing is complete before any serverless function is allowed to execute.

@ccmattr
Copy link

ccmattr commented Jun 22, 2021

Would it be possible to be able to do something like this:

functions:
  hello:
    handler: handlers.hello
    environment:
      <<: *db_env
      SOME_OTHER_KEY: foo
  world:
    handler: handlers.world
    environment:
      <<:  [ *db_env, *redis_env ]
      SOME_OTHER_KEY: bar
custom:
  dbEnv: &db_env
    PGHOST: foo
    PGPASSWORD: bar
  redisENV: &redis_env
    REDIS_HOST: foo

I have a number of functions and I don't want all of them to get db config/redis details. At the moment I'm redefining each variable individually in the lambda environments, it would be useful to be able to define different groups of configs in custom and include groups depending on the requirement.

@mnapoli
Copy link
Contributor

mnapoli commented Dec 6, 2021

I've been reading the history of this issue and it seems there are 3 different root use cases:

  1. change some configuration based on the stage
  2. reuse blocks of config (to avoid copy-pasting)
  3. deal with multi-service (or "multi-stack") applications

About 1, we've opened #10264 (Stage-specific configuration) to address this. Let us know if that would help!

@enapupe
Copy link

enapupe commented May 30, 2022

Perhaps my case is a different than 2? I'm trying to mix both envs from AWS Secrets Manager and locally defined via yml (or whatever, really).
To be more specific, I want my fns to get the general envs from SecretManager + I want to define the project version as an env var (error reporting enhancement).

@simlu
Copy link
Author

simlu commented May 30, 2022

We solved this now by compiling the yml into a single file automatically before every deploy. This is relatively easy to do and has the added benefit that you know exactly what you're deploying

@enapupe
Copy link

enapupe commented May 30, 2022

I thought about that but could not quickly find any docs on how to deploy on top of a static yml config file.
I'm doing it using a regular js file, it's looking like this so far:

const AWS = require('aws-sdk')

const package = require('./package.json')

const SSM_NAME_MAP = {
  staging: 'secretname',
  prod: 'othersecretname',
}

const getVersionOrHash = (stage) => {
  if (stage === 'staging' && process.env.CI) {
    return process.env.CIRCLE_SHA1
  }
  return package.version
}

module.exports = ({ options, resolveVariable }) =>
  new Promise(async (resolve, reject) => {
    const stage = await resolveVariable('sls:stage')
    const region = await resolveVariable('opt:region, self:provider.region')

    new AWS.SecretsManager({ region }).getSecretValue(
      { SecretId: SSM_NAME_MAP[stage] },
      (err, { SecretString }) => {
        if (err) {
          return reject(err)
        }
        resolve({
          ...JSON.parse(SecretString),
          VERSION: getVersionOrHash(stage),
        })
      },
    )
  })

and then on serverless.yml

environment: ${file(./envs.js)}

@simlu
Copy link
Author

simlu commented May 30, 2022

Definitely in the docs, i.e. sls deploy --config config-file-name-here.yml

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests