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

${param} variables are not resolved in constructs #29

Closed
pedrodim opened this issue Jun 10, 2021 · 4 comments · Fixed by #50
Closed

${param} variables are not resolved in constructs #29

pedrodim opened this issue Jun 10, 2021 · 4 comments · Fixed by #50
Labels
bug Something isn't working

Comments

@pedrodim
Copy link

I just spent several hours trying to understand why the parameters from the serverless dashboard weren't correctly being passed to lift plugin until I came across this note on the serverless plugins' guide.

Note: Variable references in the serverless instance are not resolved before a Plugin's constructor is called, so if you need these, make sure to wait to access those from your hooks.

Which means that trying to do the following will result in an error "ARNs must start with "arn:" and have at least 6 components: ${param:CERTIFICATE}".

constructs:
  app:
    domain: ${param:DOMAIN}
    certificate: ${param:CERTIFICATE}
    type: static-website
    path: dist

I fixed it on a fork moving the Constructs loader to the initialize hook pedrodim@1c47638, which seems to work fine for the Static website construct (it does correctly deploy my website without errors) but obviously now some tests are broken and I have no idea how to fix those.

@mnapoli
Copy link
Member

mnapoli commented Jun 10, 2021

Hey @pedrodim, thanks for the report. This is indeed something that we need to clarify: because of the way Serverless loads all the plugins and resolves variables, this leads to some circular dependencies with constructs.

Using the initialize hook indeed messes up a few things.

I'll investigate right away if we can solve this specifically. If we can't, I'll document the limitation.

@pedrodim
Copy link
Author

Thank you so much!

An idea could be have each construct expose a initialize hook (like they do currently with afterDeploy) so that they could instantiate any dependency or internal config which could use variables there instead on the constructor.

From what I've seem on serverless/serverless#3911 other plugins have solved this by moving any logic that uses variables to a hook, maybe it is something that cannot be possible for all actual constructs but for some this could be a workaround.

@mnapoli
Copy link
Member

mnapoli commented Jun 11, 2021

constructs:
    jobs:
        type: queue
        worker:
            handler: worker.handler
        maxRetries: ${param:maxRetries}
        batchSize: ${env:BATCH_SIZE}

In the example above, ${env:BATCH_SIZE} will be resolved correctly, but not ${param:maxRetries}.

The reason (I suspect) is related to the order in which variables are resolved. I'm still digging into it, I may have a lead.

@mnapoli
Copy link
Member

mnapoli commented Jun 14, 2021

I am working on #31 that should fix this issue.

@mnapoli mnapoli changed the title Variable references are not resolved before plugin constructor is called ${param} variables are not resolved in constructs Jun 17, 2021
@mnapoli mnapoli added the bug Something isn't working label Jun 17, 2021
mnapoli added a commit that referenced this issue Jun 17, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
2 participants