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

Issue in adding provisionedConcurrency or functionVersion set to true, Could not find reference to layer created in Resource section #9797

Closed
naveenbalaneni opened this issue Aug 2, 2021 · 13 comments · Fixed by #9826
Assignees

Comments

@naveenbalaneni
Copy link

naveenbalaneni commented Aug 2, 2021

Unable to find the reference to layer in resource section, especially when we enable the provisionedConcurrency or functionVersion set to true.

if we remove both provisionedConcurrency or functionVersion in serverless script, same code is working fine without any issues
Could not find reference to layer: rtnLayer. Ensure that you are referencing layer defined in your service.

serverless.yml
---
service:
  name: rtn
provider:
  name: aws
  region: us-west-2
  stackName: rtn-sls
  deploymentBucket:
    name: ${self:custom.S3BucketName.${opt:stage}}
    maxPreviousDeploymentArtifacts: 10
  deploymentPrefix: rtn/deployment
  runtime: nodejs12.x
  memorySize: 256
  timeout: 300

  # Lambda Configuration
  versionFunctions: false # Lambda Versioning

  # X-Ray Configuration
  tracing:
    lambda: true

custom:
  params: ${file(./env/${opt:stage}/parameters.json)}
  version: ${self:custom.params.Version}
  LayerVersion: ${self:custom.params.LayerVersion}
  s3Key: "https://s3.amazonaws.com/${self:custom.S3BucketName.${opt:stage}}/rtn-business-service/${self:custom.version}"

  S3BucketName:
    devint: rtn-devint
    prod: rtn

package:
  individually: true

functions:
  rtn-api-router:
    name: "rtn-api-router"
    handler: rtn-api-router.handler
    layers:
      - !Ref rtnLayer
    package:
      artifact: "${self:custom.s3Key}/rtn-api-router-v${self:custom.version}.zip"
    provisionedConcurrency: 1
    
resources:
  - Resources:
      rtnLayer:
        Type: AWS::Lambda::LayerVersion
        Properties:
          CompatibleRuntimes:
            - nodejs12.x
          Content:
            S3Bucket: ${self:custom.S3BucketName.${opt:stage}}
            S3Key: common-nodejs-lambda-layer/common-nodejs-lib-v${self:custom.LayerVersion}.zip
          LayerName: rtn-layer
serverless package -c serverless.yml --stage=devint output
Serverless: To ensure safe major version upgrades ensure "frameworkVersion" setting in service configuration (recommended setup: "frameworkVersion: ^2.52.1")

Serverless: Load command interactiveCli
Serverless: Load command config
Serverless: Load command config:credentials
Serverless: Load command config:tabcompletion
Serverless: Load command config:tabcompletion:install
Serverless: Load command config:tabcompletion:uninstall
Serverless: Load command create
Serverless: Load command install
Serverless: Load command package
Serverless: Load command deploy
Serverless: Load command deploy:function
Serverless: Load command deploy:list
Serverless: Load command deploy:list:functions
Serverless: Load command invoke
Serverless: Load command invoke:local
Serverless: Load command info
Serverless: Load command logs
Serverless: Load command metrics
Serverless: Load command print
Serverless: Load command remove
Serverless: Load command rollback
Serverless: Load command rollback:function
Serverless: Load command slstats
Serverless: Load command plugin
Serverless: Load command plugin
Serverless: Load command plugin:install
Serverless: Load command plugin
Serverless: Load command plugin:uninstall
Serverless: Load command plugin
Serverless: Load command plugin:list
Serverless: Load command plugin
Serverless: Load command plugin:search
Serverless: Load command config
Serverless: Load command config:credentials
Serverless: Load command upgrade
Serverless: Load command uninstall
Serverless: Load command login
Serverless: Load command logout
Serverless: Load command generate-event
Serverless: Load command test
Serverless: Load command dashboard
Serverless: Load command output
Serverless: Load command output:get
Serverless: Load command output:list
Serverless: Load command param
Serverless: Load command param:get
Serverless: Load command param:list
Serverless: Load command studio
Serverless: Skipping variables resolution with old resolver (new resolver reported no more variables to resolve)
Serverless: Configuration warning:
Serverless:   at 'functions['rtn-api-router']': unrecognized property 'logForwarding'
Serverless:   at 'service': unrecognized property 'configValidationMode'
Serverless:   at 'service': unrecognized property 'enableLocalInstallationFallback'
Serverless:  
Serverless: Learn more about configuration validation here: http://slss.io/configuration-validation
Serverless:  
Serverless: Invoke package
Serverless: Invoke aws:common:validate
Serverless: Invoke aws:common:cleanupTempDir
Serverless: Packaging service...
Serverless: Downloading rtn-api-router/1.0.1-2/rtn-api-router-v1.0.1-2.zip from bucket rtn-devint
 
 Serverless Error ----------------------------------------
 
  ServerlessError: Could not find reference to layer: rtnLayer. Ensure that you are referencing layer defined in your service.
      at /Users/naveen/rtn-api-router/node_modules/serverless/lib/plugins/aws/package/compile/functions.js:739:15
      at Array.forEach (<anonymous>)
      at extractLayerConfigurationsFromFunction (/Users/naveen/rtn-api-router/node_modules/serverless/lib/plugins/aws/package/compile/functions.js:735:29)
      at AwsCompileFunctions.compileFunction (/Users/naveen/rtn-api-router/node_modules/serverless/lib/plugins/aws/package/compile/functions.js:458:9)
      at /Users/naveen/rtn-api-router/node_modules/serverless/lib/plugins/aws/package/compile/functions.js:680:64
      at Array.map (<anonymous>)
      at AwsCompileFunctions.compileFunctions (/Users/naveen/rtn-api-router/node_modules/serverless/lib/plugins/aws/package/compile/functions.js:680:37)
      at AwsCompileFunctions.tryCatcher (/Users/naveen/rtn-api-router/node_modules/bluebird/js/release/util.js:16:23)
      at Promise._settlePromiseFromHandler (/Users/naveen/rtn-api-router/node_modules/bluebird/js/release/promise.js:547:31)
      at Promise._settlePromise (/Users/naveen/rtn-api-router/node_modules/bluebird/js/release/promise.js:604:18)
      at Promise._settlePromise0 (/Users/naveen/rtn-api-router/node_modules/bluebird/js/release/promise.js:649:10)
      at Promise._settlePromises (/Users/naveen/rtn-api-router/node_modules/bluebird/js/release/promise.js:729:18)
      at _drainQueueStep (/Users/naveen/rtn-api-router/node_modules/bluebird/js/release/async.js:93:12)
      at _drainQueue (/Users/naveen/rtn-api-router/node_modules/bluebird/js/release/async.js:86:9)
      at Async._drainQueues (/Users/naveen/rtn-api-router/node_modules/bluebird/js/release/async.js:102:5)
      at Immediate.Async.drainQueues (/Users/naveen/rtn-api-router/node_modules/bluebird/js/release/async.js:15:14)
      at processImmediate (internal/timers.js:456:21)
 
  Get Support --------------------------------------------
     Docs:          docs.serverless.com
     Bugs:          github.com/serverless/serverless/issues
     Issues:        forum.serverless.com
 
  Your Environment Information ---------------------------
     Operating System:          darwin
     Node Version:              12.18.3
     Framework Version:         2.52.1 (local)
     Plugin Version:            5.4.3
     SDK Version:               4.2.6
     Components Version:        3.14.2
 
Serverless: Deprecation warnings:

Starting from next major object notation for "service" property will no longer be recognized. Set "service" property directly with service name.
More Info: https://www.serverless.com/framework/docs/deprecations/#SERVICE_OBJECT_NOTATION

Starting with next major, Serverless will throw on configuration errors by default. Adapt to this behavior now by adding "configValidationMode: error" to service configuration
More Info: https://www.serverless.com/framework/docs/deprecations/#CONFIG_VALIDATION_MODE_DEFAULT```

</details>

<!--
Q4: Provide (in below placeholder) output of serverless --version
-->

<b>Installed version</b>

Serverless: Running "serverless" installed locally (in service node_modules)
Framework Core: 2.52.1 (local)
Plugin: 5.4.3
SDK: 4.2.6
Components: 3.14.2

@pgrzesik
Copy link
Contributor

pgrzesik commented Aug 3, 2021

Hello @naveenbalaneni - thanks for reporting, I have one extra question. When you don't use listed settings, does the layer get properly attached to your function?

@naveenbalaneni
Copy link
Author

naveenbalaneni commented Aug 3, 2021

@pgrzesik, thank you looking into the issue.

If I setup version versionFunctions: false and comment provisionedConcurrency. same code is working fine. If I enable one of versionFunctions or provisionedConcurrency, serverless unable to find the layer

@pgrzesik
Copy link
Contributor

pgrzesik commented Aug 4, 2021

Thanks a lot for providing that information, I believe I know what is the cause here and it is calculating hashes when provisionedConcurrency or function versioning is concerned. At the moment, referencing layers via !Ref assumes that the layer is locally defined and not a reference to a layer created outside of layers section of the Framework. We could skip erroring in such a situation and just pass forward, but the problem with that is it will not expose errors where someone by mistake tries to reference the local layer with the wrong ID. I think this might still be the way to go to potentially enable use case as yours.

We also have a ticket that would improve the way locally defined layers are referenced here: #8915

@naveenbalaneni
Copy link
Author

@pgrzesik Thank you for info. I have downgraded Serverless Version to 1.83.3. This version I am able to enable both Provisioned Concurrency and the layerVersion.

I will wait until this issue gets addressed to update serverless version to latest.

@medikoo
Copy link
Contributor

medikoo commented Aug 9, 2021

@pgrzesik do you know why it works in v1 and not in v2?

@pgrzesik
Copy link
Contributor

pgrzesik commented Aug 9, 2021

I was investigating it and I've noticed that it was due to the fact that in v1 the layers were not properly taken into account when computing version hashes - it was changed with #8066 and that logic does not handle well layers that are referenced as in the provided example.

In such situations, I believe we should just ignore layers that cannot be found, but by implementing such behavior, we introduce a risk that invalid references to locally defined layers will be passed silently, without any errors, which I believe is something that we should not allow (it is still a potential problem if I'm not mistaken but to lesser extent).
I think that in order to allow such behavior, we should first introduce a more robust way of referencing local layers and then we can safely "ignore" layers referenced with Ref in that logic.

What do you think @medikoo ?

@naveenbalaneni
Copy link
Author

naveenbalaneni commented Aug 9, 2021

@pgrzesik
Currently we have multiple stacks using the same layer. if you plan to remove Ref completely. Could you please enable to download package from S3. similar to lambda function.

package: individually: true artifact: "https://s3.amazonaws.com/rtn-devint/rtn-layer-1.0.0.zip"

so that we can clone the package from S3, instead from local.

Thank you,
Naveen

@medikoo
Copy link
Contributor

medikoo commented Aug 10, 2021

@pgrzesik great thanks for feedback.

One thing I miss, is why given configuration works for @naveenbalaneni while it's considered as invalid on our side?

If it works, shouldn't it be considered valid ?

@pgrzesik
Copy link
Contributor

pgrzesik commented Aug 10, 2021

It is considered invalid on our side due to a tradeoff. At the moment, the only way to reference local layers is to use Ref and Framework makes an assumption that if layer is referenced this way, it means that it's locally defined layer (managed by the Framework's support for layers), while in reality, the situation might be as in @naveenbalaneni case. In v1 it works due to a bug in version hashing logic, which does not take layers into consideration, so it doesn't even check if layer referenced by Ref is local or not.

So, the tradeoff that I mention here is this, we can either:

  1. Assume that if the layer referenced by Ref is not found locally, then it means that it is "external" or defined in resources and we ignore it when computing version hash. This opens up the possibility of not catching typos which will result in errors during CloudFormation deployment.
  2. Support only "local" layers via Ref and throw error if it cannot be found - this helps spotting typo issues earlier in the process

Currently, our logic supports the option number 2. One way would be to switch to option 1, which would unblock this specific case or implement a solution mentioned in #8915 which would allow for a less error-prone way to reference local layers, which in turn makes it safer to have option 1 for tradeoff above.

What is your opinion here? Maybe we should go with option 1 here to support all cases, while offering a little bit worse experience in case of typos when referencing local layers?

Note: While this layer is also local in the sense that it is a part of the same CloudFormation template, it hasn't been defined with layers functionality of the Framework.

@medikoo
Copy link
Contributor

medikoo commented Aug 10, 2021

@pgrzesik I assume a valid fix might be ignore the fact of missing layer, but only if we find the resources of given id in result template (and we may also then additionally validate its type).

Still I guess it's tricky to implement, as at given point we do not have yet access to fully resolved template.

@pgrzesik
Copy link
Contributor

Yes, I was thinking about that option as well, but I identified the same "issue" with the fact that we're adding resources in later processing stage. But I think it still might be a viable option. What do you think, should we consider that fix or rather instead resolve it by providing nicer API for referencing local layers?

@medikoo
Copy link
Contributor

medikoo commented Aug 11, 2021

What do you think, should we consider that fix or rather instead resolve it by providing nicer API for referencing local layers?

I assume that validation was added in context of improving hash generation, and back then we didn't specifally address a complaint that validation is missing.

We probably should remove that warning, as (1) if user is referencing by Ref not specifically layer create in scope of layers block but one configured manually in resources then we're doing wrong thing by throwing the error. While (2) if user have put wrong reference to Ref, then anyway will be quickly presented by error from validation on AWS side. While some disadvantage would be that message doesn't point exactly that probably layer is misnamed, I think it still will be meaningful enough to let user know what went wrong

@pgrzesik
Copy link
Contributor

I believe that's a fine tradeoff, I've just prepared a PR: #9826

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

Successfully merging a pull request may close this issue.

3 participants