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

Handle self variable resolution #49

Open
simlu opened this issue May 6, 2018 · 11 comments
Open

Handle self variable resolution #49

simlu opened this issue May 6, 2018 · 11 comments

Comments

@simlu
Copy link
Collaborator

simlu commented May 6, 2018

Should be relatively easy to implement. However can cause issues with recursion.

We might not want to support this and instead fix the readme?

@simlu simlu mentioned this issue Jun 28, 2018
@roni-frantchi
Copy link

Would love to have this one. Any works or considerations on how to have this one implemented?..
I may be able to help out.

@simlu
Copy link
Collaborator Author

simlu commented Jun 25, 2019

@roni-frantchi Can you describe your use case more? We are using this library in a lot of complex cases and haven't really had this requirement.

@roni-frantchi
Copy link

@simlu In my case, I'm using the Serverless Framework, (in which to load, I use yaml-boost as described in this example).

In Serverless, I have a DynamoDB set up like so:

resources:
  Resources:
    ConfigurationTable:
      Type: AWS::DynamoDB::Table
      Properties:
        TableName: ${self:provider.environment.CONFIGURATION_TABLE_NAME}
        ...

Which works great.

For unit tests, I use jest-dynamodb, which requires an object identical in structure to the one I wrote on my serverless.yml describing DynamoDB's table for it to set up in tests.

I don't want to repeat this same setup, as I already have it in my Serverless yaml, so I figured I could just require(serverless.js) (the configuration file as suggested in the example).

However, that doesn't work - TableName doesn't get resolved, because it is Serverless who does the resolution of ${self}, and not yaml-boost.

Therefor my only option was to open this PR, and load the yaml using Serverless to be able to resolve the ${self} variables like so:

await serverless.init();
const service = await serverless.variables.populateService();
...

It's works well enough, but is less than ideal because serverless.init() may take a while, it also requires to have Serverless installed in our CI for unit tests.

@simlu
Copy link
Collaborator Author

simlu commented Jun 25, 2019

You should separate your config into multiple files and use the file require syntax. I don't think there is a reason here to use self?

@roni-frantchi
Copy link

roni-frantchi commented Jun 25, 2019

@simlu They are in fact separated to several files, but I can't see how that would help.
I need to have the table name available for the lambda as an environment variable, as well as set it up for the Dynamo table resource:

provider:
  environment:
    CONFIGURATION_TABLE_NAME: MyTableName  # this is resolved from a whole other `env.yml` file, inlining for simplicity of the example

# this is part is in `dynamodb.yaml` and is imported to `serverless.yml` using yaml-boost, which works great, I'm inlining it here just for simplicity
resources:
  Resources:
    ConfigurationTable:
      Type: AWS::DynamoDB::Table
      Properties:
        TableName: ${self:provider.environment.CONFIGURATION_TABLE_NAME}
        ...

As I wrote the above, I'm guessing you suggest I refer from dynamodb.yml back to serverless.yml to get that environment variable?.. But serverless.yml imports from dynamodb.yml so wouldn't that be cyclic file reference?
Or would yaml-boost be able to handle this:

# in serverless.yml:
<<<:
  - ${file(dynamodb.yml)}
provider:
  environment:
    CONFIGURATION_TABLE_NAME: MyTableName  # this is resolved from a whole other `env.yml` file, inlining for simplicity of the example

# in dynamodb.yml:

resources:
  Resources:
    ConfigurationTable:
      Type: AWS::DynamoDB::Table
      Properties:
        TableName: ${file(serverless.yml).provider.environment.CONFIGURATION_TABLE_NAME}
        ...

Even if it does work, I think in some cases you don't want to have separate files just for the sake of referencing a single variable - I usually separate base on other needs, not based on my need to refer between items, not sure that should be what drive my decision to split files.

@simlu
Copy link
Collaborator Author

simlu commented Jun 25, 2019

You have a weird use case as in that you want self to resolve, but not cf. How would it work when you mix them?

Don't have a lot of time atm, will get back with more details later

@simlu
Copy link
Collaborator Author

simlu commented Jun 26, 2019

Spent some time thinking about how to best implement. Might find time to continue on a pr tomorrow.

The change isn't hard, just also want to do some refactoring

@simlu
Copy link
Collaborator Author

simlu commented Jun 27, 2019

I'm thinking something like:

module.exports.load = (filePath, vars = {}) => {
  const result = loadRecursive(
    path.dirname(filePath),
    path.dirname(filePath),
    yaml.safeLoad(fs.readFileSync(filePath, 'utf8')),
    vars
  );
  // todo: resolve self rec
  return result;
};

@simlu
Copy link
Collaborator Author

simlu commented Jun 27, 2019

@roni-frantchi The biggest problem I have with the self resolution is that it's very complex since it can happen at the same time as file resolution. So one needs to continuously resolve. Example:

provider:
  name: aws
  stage: ${opt:stage, 'dev'}  # or this could be a file resolution
  environment:
    MY_SECRET: ${file(../config.${self:provider.stage}.json):CREDS}

@roni-frantchi
Copy link

@simlu right, we would have to continue resolving file while not all self variables have been resolved or the same variable was already visited

@ZebraFlesh
Copy link

This is the missing feature that would make this package usable for me. I have various bits of configuration defined in my serverless.yml that I want to access in JS (instead of repeating the configuration). Example:

custom:
  cdn:
    dev: https://dev.mycdn.com/${self:service}
    prod: https://www.mycdn.com/${self:service}

I suppose I could do something weird like putting the service name in a JS file, then using a variable for serverless's service element ... but that seems obtuse and unnecessary.

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

No branches or pull requests

3 participants