Serverless Variables #1834

Merged
merged 26 commits into from Aug 31, 2016

Conversation

Projects
None yet
@eahefnawy
Member

eahefnawy commented Aug 12, 2016

This PR implements the new Serverelss Variables system as described here and discussed in #1801.

  • populating from env vars: ${env:my_arn}
  • populating from cli options: ${opt:stage}
  • populating from the self serverless.yml file: ${self:service}
  • move defaults into provider with backward compatibility
  • indefinite recursive nesting: ${env:${opt:stage}_arn}
  • populating from another file: ${file(./config.yml):data.subProp}
  • unit tests
  • update docs

cc/ @serverless/vip

@eahefnawy eahefnawy added this to the v1.0.0-beta.2 milestone Aug 12, 2016

@eahefnawy eahefnawy self-assigned this Aug 12, 2016

+
+```
+
+In the previous example you're dynamically adding a prefix to the function names by referencing the `stage` option that you pass in the CLI. So when you deploy, the function name will always include the stage you're deploying to.

This comment has been minimized.

@nicka

nicka Aug 15, 2016

Member

You might want to add an actual example CLI command. For example: sls deploy --stage=dev

@nicka

nicka Aug 15, 2016

Member

You might want to add an actual example CLI command. For example: sls deploy --stage=dev

This comment has been minimized.

@eahefnawy

eahefnawy Aug 16, 2016

Member

Sure thing. Note though that the command hasn't changed. It's still sls deploy --stage dev as usual, but what's new is that you have access to this stage CLI option in the configuration file.

@eahefnawy

eahefnawy Aug 16, 2016

Member

Sure thing. Note though that the command hasn't changed. It's still sls deploy --stage dev as usual, but what's new is that you have access to this stage CLI option in the configuration file.

+
+In the previous example you're dynamically adding a prefix to the function names by referencing the `stage` option that you pass in the CLI. So when you deploy, the function name will always include the stage you're deploying to.
+
+## Referencing CLI Options

This comment has been minimized.

@nicka

nicka Aug 15, 2016

Member

Think this should be about "Recursively reference properties of any type from the same serverless.yml file".

@nicka

nicka Aug 15, 2016

Member

Think this should be about "Recursively reference properties of any type from the same serverless.yml file".

This comment has been minimized.

@eahefnawy

eahefnawy Aug 16, 2016

Member

Yep! 👍

@eahefnawy

eahefnawy Aug 16, 2016

Member

Yep! 👍

lib/classes/Service.js
+ 'Deprecation Notice: the "defaults" property in serverless.yml',
+ ' is deprecated. The "stage", "region" & "variableSyntax" properties',
+ ' has been moved to the "provider" property instead. Please update',
+ ' your serverless.yml file asap.',

This comment has been minimized.

@nicka

nicka Aug 15, 2016

Member

Maybe add link to the readme which talks about the new variables structure?

@nicka

nicka Aug 15, 2016

Member

Maybe add link to the readme which talks about the new variables structure?

lib/classes/Service.js
- .then(() => {
- if (!options.stage) {
- options.stage = this.defaults.stage;
+ if (serverlessYml.defaults.stage) {

This comment has been minimized.

@nicka

nicka Aug 15, 2016

Member

Sorry for all the remarks. But first we trigger a deprecation warning and then we keep on using the serverlessYml.defaults?

@nicka

nicka Aug 15, 2016

Member

Sorry for all the remarks. But first we trigger a deprecation warning and then we keep on using the serverlessYml.defaults?

This comment has been minimized.

@ac360

ac360 Aug 16, 2016

Member

Also, serverlessYml is an inaccurate term, as we will introduce JSON support in the near future. Could we use serverlessService or something more general?

@ac360

ac360 Aug 16, 2016

Member

Also, serverlessYml is an inaccurate term, as we will introduce JSON support in the near future. Could we use serverlessService or something more general?

This comment has been minimized.

@eahefnawy

eahefnawy Aug 16, 2016

Member

@nicka we're not dropping support for defaults at this point. So in code defaults still has to exist/work for backward compatibility. In the future when we completely drop support for defaults, we'll remove it from the codebase.

@eahefnawy

eahefnawy Aug 16, 2016

Member

@nicka we're not dropping support for defaults at this point. So in code defaults still has to exist/work for backward compatibility. In the future when we completely drop support for defaults, we'll remove it from the codebase.

This comment has been minimized.

@eahefnawy

eahefnawy Aug 16, 2016

Member

@ac360 you're right! 👍 ... will update it now ... I'll go with serverlessFile as this variable represent the file right after it's loaded, the serverless service itself is represented in the Service class as a whole after loading & populating.

@eahefnawy

eahefnawy Aug 16, 2016

Member

@ac360 you're right! 👍 ... will update it now ... I'll go with serverlessFile as this variable represent the file right after it's loaded, the serverless service itself is represented in the Service class as a whole after loading & populating.

@pmuens

This comment has been minimized.

Show comment
Hide comment
@pmuens

pmuens Aug 16, 2016

Member

Just played around with it and it's really rock solid!
Great work 🎉

Here's just something I was wondering when using self. What about if we parse the file self references into JSON and use this JSON representation to find the correct value?

This way we can also e.g. access properties which are nested inside the events array like self.functions.hello.events[0].http.path. Not sure if it's necessary at all.

Another thing was that I still found several serverless.env.yml references (especially in the docs) while doing a global search (CMD + Shift + F in WebStorm). Those should be removed as well.

Member

pmuens commented Aug 16, 2016

Just played around with it and it's really rock solid!
Great work 🎉

Here's just something I was wondering when using self. What about if we parse the file self references into JSON and use this JSON representation to find the correct value?

This way we can also e.g. access properties which are nested inside the events array like self.functions.hello.events[0].http.path. Not sure if it's necessary at all.

Another thing was that I still found several serverless.env.yml references (especially in the docs) while doing a global search (CMD + Shift + F in WebStorm). Those should be removed as well.

@eahefnawy

This comment has been minimized.

Show comment
Hide comment
@eahefnawy

eahefnawy Aug 16, 2016

Member

@pmuens not sure I understand what you mean about JSON. The self is already accessing the parsed serverless.yml file as a Javascript Object, and we can access any value type. But just like you mention, the limitation is parsing the referencing string itself. So yeah we can't really go deep into arrays because that syntax it's not supported (not really sure if there's a need for that though), but how would JSON help here?

Member

eahefnawy commented Aug 16, 2016

@pmuens not sure I understand what you mean about JSON. The self is already accessing the parsed serverless.yml file as a Javascript Object, and we can access any value type. But just like you mention, the limitation is parsing the referencing string itself. So yeah we can't really go deep into arrays because that syntax it's not supported (not really sure if there's a need for that though), but how would JSON help here?

@pmuens

This comment has been minimized.

Show comment
Hide comment
@pmuens

pmuens Aug 16, 2016

Member

Yes. Just something random I stumbled upon when I tried to access the path property of the http event in the serverless.yml file.

Maybe a wrong usage on my side but also not a high priority IMHO.

Member

pmuens commented Aug 16, 2016

Yes. Just something random I stumbled upon when I tried to access the path property of the http event in the serverless.yml file.

Maybe a wrong usage on my side but also not a high priority IMHO.

@pmuens

This comment has been minimized.

Show comment
Hide comment
@pmuens

pmuens Aug 19, 2016

Member

Awesome! Will test it ASAP. Just one note. Could you do another global search for serverless.env.yml and serverless.env.yaml? There are still some references (e.g. in the root README.md file) in other files which might be removed.

Member

pmuens commented Aug 19, 2016

Awesome! Will test it ASAP. Just one note. Could you do another global search for serverless.env.yml and serverless.env.yaml? There are still some references (e.g. in the root README.md file) in other files which might be removed.

@eahefnawy

This comment has been minimized.

Show comment
Hide comment
@eahefnawy

eahefnawy Aug 19, 2016

Member

@pmuens Good catch! 👍 .... I was only searching in the docs folder

Member

eahefnawy commented Aug 19, 2016

@pmuens Good catch! 👍 .... I was only searching in the docs folder

@flomotlik

This comment has been minimized.

Show comment
Hide comment
@flomotlik

flomotlik Aug 22, 2016

Contributor

@eahefnawy please resolve conflicts

Contributor

flomotlik commented Aug 22, 2016

@eahefnawy please resolve conflicts

@ac360

This comment has been minimized.

Show comment
Hide comment
@ac360

ac360 Aug 23, 2016

Member

@eahefnawy You are so fantastic and talented, I bet you can still squeeze in support for var.myVar in populate(). Just imagine how simple and elegant var will look in serverless.yml, as an alternative to these other awesome variable sources you've implemented. It will be like art. Can you picture it?

Also, @maciej will give you a kiss.

Member

ac360 commented Aug 23, 2016

@eahefnawy You are so fantastic and talented, I bet you can still squeeze in support for var.myVar in populate(). Just imagine how simple and elegant var will look in serverless.yml, as an alternative to these other awesome variable sources you've implemented. It will be like art. Can you picture it?

Also, @maciej will give you a kiss.

@eahefnawy

This comment has been minimized.

Show comment
Hide comment
@eahefnawy

eahefnawy Aug 23, 2016

Member

@ac360 hahaha yes I can! It's just that I haven't got the "green light" yet 😛

Member

eahefnawy commented Aug 23, 2016

@ac360 hahaha yes I can! It's just that I haven't got the "green light" yet 😛

@ac360

This comment has been minimized.

Show comment
Hide comment
@ac360

ac360 Aug 23, 2016

Member

Oh. To help @flomotlik make the call here, can you verify that it will be an easy addition? I eyeballed it and think it can be done in <50 lines in the populate() method.

Member

ac360 commented Aug 23, 2016

Oh. To help @flomotlik make the call here, can you verify that it will be an easy addition? I eyeballed it and think it can be done in <50 lines in the populate() method.

@eahefnawy

This comment has been minimized.

Show comment
Hide comment
@eahefnawy

eahefnawy Aug 23, 2016

Member

yeah I think it'll be just another "source" just like env, opt, self & file. Should fit right in there.

Member

eahefnawy commented Aug 23, 2016

yeah I think it'll be just another "source" just like env, opt, self & file. Should fit right in there.

@flomotlik

This comment has been minimized.

Show comment
Hide comment
@flomotlik

flomotlik Aug 23, 2016

Contributor

What use case would var cover exactly? Is it an implementation to allow overriding? or to be able to specify vars inside the serverless.yml and then override them through the command line as well if necessary?

One use case outlined by @kennu that I'm not sure we got covered at this point is defining different variables and selecting them depending on the stage. In the specific case they want to define a profile to use for deployment and they are syncing the profiles. But they also want to be able to select a profile per stage.

So the serverless.yml could have:

provider:
  profile: ${self.vars.${self.provider.stage}.profile}
vars:
  staging:
    profile: some-specific-profile
  production:
    profile: some-other-specific-profile

And then they could set --stage as well and it should properly resolve. To add here though is that currently we don't have provider->profile implemented.

If we want overrides we could also have something like ${env.,self.} so if its defined in the environment pick from the environment, if not pick from self. But I'm not too sure what specific use case the var syntax would solve at the moment @ac360

Contributor

flomotlik commented Aug 23, 2016

What use case would var cover exactly? Is it an implementation to allow overriding? or to be able to specify vars inside the serverless.yml and then override them through the command line as well if necessary?

One use case outlined by @kennu that I'm not sure we got covered at this point is defining different variables and selecting them depending on the stage. In the specific case they want to define a profile to use for deployment and they are syncing the profiles. But they also want to be able to select a profile per stage.

So the serverless.yml could have:

provider:
  profile: ${self.vars.${self.provider.stage}.profile}
vars:
  staging:
    profile: some-specific-profile
  production:
    profile: some-other-specific-profile

And then they could set --stage as well and it should properly resolve. To add here though is that currently we don't have provider->profile implemented.

If we want overrides we could also have something like ${env.,self.} so if its defined in the environment pick from the environment, if not pick from self. But I'm not too sure what specific use case the var syntax would solve at the moment @ac360

@wedgybo

This comment has been minimized.

Show comment
Hide comment
@wedgybo

wedgybo Aug 24, 2016

Contributor

Just been giving this a bash and it's very nice. This is ideal for allowing devs to setup their own stage for testing and allowing people the choice between using env vars, or using serverless.env.yaml and not committing it to the repo. Good work guys! 👍

Contributor

wedgybo commented Aug 24, 2016

Just been giving this a bash and it's very nice. This is ideal for allowing devs to setup their own stage for testing and allowing people the choice between using env vars, or using serverless.env.yaml and not committing it to the repo. Good work guys! 👍

@svdgraaf

This comment has been minimized.

Show comment
Hide comment
@svdgraaf

svdgraaf Aug 24, 2016

Contributor

I tested it as well, I really like it :)

Contributor

svdgraaf commented Aug 24, 2016

I tested it as well, I really like it :)

@flomotlik

This comment has been minimized.

Show comment
Hide comment
@flomotlik

flomotlik Aug 25, 2016

Contributor

@wedgybo @svdgraaf @ac360 @eahefnawy @pmuens @rowanu Here is a further extension that imho the Variables system will need:

Basically one of the problems we have in the variables system is that overrides aren't implemented yet.

If we look at the use case that @rowanu and @kennu have:

They want to set up different AWS profiles and then depending on the stage that they are deploying to a different AWS profile should be used. The profile and stage don't necessarily have the same name so we need to be able to have a mapping between the stage and profile:

provider:
  stage: production
  profile: ${self.var.profiles.${self.provider.stage}}
var:
  profiles:
    production: my_prod_profile
    staging: my_staging_profile

Now the problem here is that provider->stage is hardcoded, so users have to go in and override it by hand to deploy somewhere else. It would be nice to be able to set it through --stage as well. For this we need overrides though. There are two options, first one is we add some built-in logical for overrides. While this would seem easier at first thought, with all the different ways we can load config from imho this will get painful very quickly if you do more complex configs.

The second option is we let you define the overrides, e.g.

provider:
  stage: production
  profile: ${self.var.profiles.${opts.stage, self.provider.stage}}
var:
  profiles:
    production: my_prod_profile
    staging: my_staging_profile

In this case we're loading from opts.stage first, so if you provide --stage it gets preference, then we use self.provider.stage. You can define your own override importance and could override from various sources. Now this is a bit more complex to understand and potentially use in very simple use cases, but it makes a lot more complex use cases possible and gives you a TON of flexibility to do things. It can even go as far as using a string in one case, but loading an object from a file in another case through the file syntax.

Whats your thoughts on this?

Contributor

flomotlik commented Aug 25, 2016

@wedgybo @svdgraaf @ac360 @eahefnawy @pmuens @rowanu Here is a further extension that imho the Variables system will need:

Basically one of the problems we have in the variables system is that overrides aren't implemented yet.

If we look at the use case that @rowanu and @kennu have:

They want to set up different AWS profiles and then depending on the stage that they are deploying to a different AWS profile should be used. The profile and stage don't necessarily have the same name so we need to be able to have a mapping between the stage and profile:

provider:
  stage: production
  profile: ${self.var.profiles.${self.provider.stage}}
var:
  profiles:
    production: my_prod_profile
    staging: my_staging_profile

Now the problem here is that provider->stage is hardcoded, so users have to go in and override it by hand to deploy somewhere else. It would be nice to be able to set it through --stage as well. For this we need overrides though. There are two options, first one is we add some built-in logical for overrides. While this would seem easier at first thought, with all the different ways we can load config from imho this will get painful very quickly if you do more complex configs.

The second option is we let you define the overrides, e.g.

provider:
  stage: production
  profile: ${self.var.profiles.${opts.stage, self.provider.stage}}
var:
  profiles:
    production: my_prod_profile
    staging: my_staging_profile

In this case we're loading from opts.stage first, so if you provide --stage it gets preference, then we use self.provider.stage. You can define your own override importance and could override from various sources. Now this is a bit more complex to understand and potentially use in very simple use cases, but it makes a lot more complex use cases possible and gives you a TON of flexibility to do things. It can even go as far as using a string in one case, but loading an object from a file in another case through the file syntax.

Whats your thoughts on this?

@wedgybo

This comment has been minimized.

Show comment
Hide comment
@wedgybo

wedgybo Aug 25, 2016

Contributor

I don't have much experience with multiple profiles. Looking at the use cases though is there another example where you might want to use this override system? The new setup is nice in but I worry that to do basic stuff it's starting to get pretty complex as it is and option 2 adds to that.

Could profile not be an arg option in the same way stage is if you want to override what you've done in option 1? Where stage and profile are overridden in the code rather than describing it in the config?

I can't see there being many cases where you would want to have the same stage deployed by different profiles except if you have multiple accounts for different regions? For those people you would either run multiple deploys using sls deploy --stage production --profile aws-uk, or create a new stage for each region to deploy to and define it in the variables as per option 1 you gave?

Contributor

wedgybo commented Aug 25, 2016

I don't have much experience with multiple profiles. Looking at the use cases though is there another example where you might want to use this override system? The new setup is nice in but I worry that to do basic stuff it's starting to get pretty complex as it is and option 2 adds to that.

Could profile not be an arg option in the same way stage is if you want to override what you've done in option 1? Where stage and profile are overridden in the code rather than describing it in the config?

I can't see there being many cases where you would want to have the same stage deployed by different profiles except if you have multiple accounts for different regions? For those people you would either run multiple deploys using sls deploy --stage production --profile aws-uk, or create a new stage for each region to deploy to and define it in the variables as per option 1 you gave?

+This overwrite functionality is super powerful. You can have as many variable references as you want, from any source you want, and each of them can be of different type and different name.
+
+# Migrating serverless.env.yml
+Previously we used `serverless.env.yml` file to track Serverless Variables. It was a completely different system with different concepts. To migrate your variables from `serverless.env.yml`, you'll need to decide where you want to store your variables.

This comment has been minimized.

@flomotlik

flomotlik Aug 30, 2016

Contributor

"we used" -> "we used the"

@flomotlik

flomotlik Aug 30, 2016

Contributor

"we used" -> "we used the"

+
+* Using a config file: You can still use `serverless.env.yml`, but the difference now is that you can structure the file however you want, and you'll need to reference each variable/property correctly in `serverless.yml`. For more info, you can check the file reference section above.
+* Using the same `serverless.yml` file: You can store your variables in `serverless.yml` if they don't contain sensitive data, and then reference them elsewhere in the file using `self:someProperty`. For more info, you can check the self reference section above.
+* Using environment variables: You ca instead store your variables in environment variables and reference them with `env.someEnvVar`. For more info, you can check the environment variable reference section above.

This comment has been minimized.

@flomotlik

flomotlik Aug 30, 2016

Contributor

"You ca" -> You can

@flomotlik

flomotlik Aug 30, 2016

Contributor

"You ca" -> You can

@@ -18,16 +18,14 @@ provider:
subnetIds:
- subnetId1
- subnetId2
+ stage: beta # Overwrite the default "dev" stage.

This comment has been minimized.

@flomotlik

flomotlik Aug 30, 2016

Contributor

Is this stage variable used by the aws deployment?

@flomotlik

flomotlik Aug 30, 2016

Contributor

Is this stage variable used by the aws deployment?

This comment has been minimized.

@eahefnawy

eahefnawy Aug 31, 2016

Member

yep...the default config in default and provider objects are an exact reflection of each other for backward compatibility, so aws deployment always have access to those values.

@eahefnawy

eahefnawy Aug 31, 2016

Member

yep...the default config in default and provider objects are an exact reflection of each other for backward compatibility, so aws deployment always have access to those values.

@@ -18,16 +18,14 @@ provider:
subnetIds:
- subnetId1
- subnetId2
+ stage: beta # Overwrite the default "dev" stage.
+ region: us-west-2 # Overwite the default "us-east-1" region.

This comment has been minimized.

@flomotlik

flomotlik Aug 30, 2016

Contributor

Same here is region already implemented here?

@flomotlik

flomotlik Aug 30, 2016

Contributor

Same here is region already implemented here?

@flomotlik

This comment has been minimized.

Show comment
Hide comment
@flomotlik

flomotlik Aug 30, 2016

Contributor

@eahefnawy reviewed the feature and code. There are a few things in the docs specifically that need to be fixed, but other than that we're nearly there for release. Can definitely do it tomorrow I think. Ping me once you've fixed the commented issues.

Contributor

flomotlik commented Aug 30, 2016

@eahefnawy reviewed the feature and code. There are a few things in the docs specifically that need to be fixed, but other than that we're nearly there for release. Can definitely do it tomorrow I think. Ping me once you've fixed the commented issues.

@flomotlik flomotlik merged commit e782df7 into master Aug 31, 2016

3 checks passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
continuous-integration/travis-ci/push The Travis CI build passed
Details
coverage/coveralls Coverage decreased (-0.1%) to 93.87%
Details

@flomotlik flomotlik deleted the sls-vars branch Aug 31, 2016

@WooDzu WooDzu referenced this pull request in dherault/serverless-offline Sep 8, 2016

Closed

TypeError: Cannot read property 'stages' of undefined #95

@hellolijin

This comment has been minimized.

Show comment
Hide comment
@hellolijin

hellolijin Nov 13, 2016

serverless.yml
....
config: ${opt:target}
custom: ${file(./${self:config}.yml)}
...

when I run,
$ sls deploy --target dev

error happens.

Serverless Error ---------------------------------------

 Trying to populate non string value into a string for
 variable ${self:config}. Please make sure the value
 of the property is a string.

...

serverless.yml
....
config: ${opt:target}
custom: ${file(./${self:config}.yml)}
...

when I run,
$ sls deploy --target dev

error happens.

Serverless Error ---------------------------------------

 Trying to populate non string value into a string for
 variable ${self:config}. Please make sure the value
 of the property is a string.

...

@hellolijin

This comment has been minimized.

Show comment
Hide comment
@hellolijin

hellolijin Nov 13, 2016

serverless.yml (the ./foo/function.yml is defined correctly)
...
functions:
foo: ${file(./foo/function.yml)}
...
When deploy, I got

Type Error ---------------------------------------------

 Cannot assign to read only property 'events' of ${file(./foo/function.yml)}

Could I know how to resolve it? I really want to define the functions in the sub folder.

serverless.yml (the ./foo/function.yml is defined correctly)
...
functions:
foo: ${file(./foo/function.yml)}
...
When deploy, I got

Type Error ---------------------------------------------

 Cannot assign to read only property 'events' of ${file(./foo/function.yml)}

Could I know how to resolve it? I really want to define the functions in the sub folder.

@mikerahmati mikerahmati referenced this pull request in cloudconformity/auto-remediate Nov 13, 2017

Merged

First weave of CloudTrail remediate function into project #1

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