aws / aws-sam-cli

CLI tool to build, test, debug, and deploy Serverless applications using AWS SAM
https://aws.amazon.com/serverless/sam/
Apache License 2.0
6.51k stars 1.17k forks source link

sam build fails when providing a function LayerArn with a reference to a SSM::Parameter CF param #1069

Closed ericallam closed 3 years ago

ericallam commented 5 years ago

Description

When running sam build on a template that includes a Serverless::Function resource that specifies a LayerArn property with a reference to a CF Parameter name that is of Type AWS::SSM::Parameter<string> with a default value of the name of the param in param store, sam build reports an invalid Arn.

Steps to reproduce

  1. git clone https://github.com/ericallam/sam-build-ssm-parameter-layer-arn-issue.git
  2. cd sam-build-ssm-parameter-layer-arn-issue
  3. sam build --debug

Observed result

View debug output here

Expected result

sam build should resolve the actual value of the param in param store and use that to determine if the LayerArn is valid.

Additional environment details (Ex: Windows, Mac, Amazon Linux etc)

  1. OS: macOS Mojave
  2. sam --version: 0.13.0
jfuss commented 5 years ago

@ericallam Thanks for bringing this up. Currently SAM CLI does not do deep inspection of Parameters. I can see some value in doing this inspection and fetching the value but that means we are in this continually re-implement CloudFormation Features locally. You can use the --parameter-overrides and pass in the value of the Parameter. This is my recommendation on a path forward and is already supported.

It may make sense for sam buildto ignore some validation. To build, we don't need this Ref resolved. I think we change this issue to capture not failing build on unresolved Refs (specifically for Layers, but there could be other cases). Thoughts?

r-kuhr commented 5 years ago

I am another wishing for a fix/change for this as well...

sam --version: 0.17.0

Using SSM parameters for SAM template parameters is VERY nice. Not validating the Ref would be very OK in the sam build step since I ASSUME the build is there to pack up your code nicely into a package to be upload-able to S3.

Since you are not bundling up the Layers themselves in the code in the SAM Build Step, it makes sense to at least ignore the Layers property of AWS::Serverless::Function.Properties.

So, I hope to hear that this is something to be fixed in a new-er version in the near future. I prefer to keep my sam build steps pretty clean and in line with all of the 'default' parameter settings.

chizou commented 5 years ago

I get that using --parameter-overrides works, but it's a really non-intuitive way to need to get sam build to work when we want to pass any AWS::SSM parameter types.

It may make sense for sam build to ignore some validation. To build, we don't need this Ref resolved.

This is exactly how it should behave if the Ref is an AWS::SSM::* parameter. I would imagine a reasonable person wouldn't expect the parameter value be resolved at build time. Or at the very least, if sam build detects this type of parameter, it should output some kind of warning about needing to use a --parameter-override to pass the validation.

Otherwise, one has to search Github issues to find this solution.

brysontyrrell commented 5 years ago

I ran into this today and it's causing me headaches. The workaround is not as desirable as having sam build pass on parameters that reference SSM and continue with the build.

The workaround also exposes something annoying with sam build I wasn't aware of before:

Error: Invalid value for "--parameter-overrides": MyParam=MyValue is not in valid format. It must look something like 'ParameterKey=KeyPairName,ParameterValue=MyKey ParameterKey=InstanceType,ParameterValue=t1.micro'

This is now how the aws cloudformation CLI handles overrides (which means it is not the was sam deploy handles them). Parameter overrides when deploying follow this pattern:

--parameter-overrides MyParam1=MyValue1 MyParam2=MyValue2

This is far less verbose and cleaner than how sam build chose to implement:

--parameter-overrides ParameterKey=MyParam1,ParameterValue=MyValue1

Why the difference?

mindrunner commented 5 years ago

Took me a while to figure out where the problem is. Tried finding an existing bug, but could not. I was already starting to write my bug report when I eventually found this thread.

Same happens for me, but not on all environments. I figured out, that my local sam package is quite old:

SAM CLI, version 0.16.1

Whereas most of our developers are on a newer version:

SAM CLI, version 0.22.0

CI/CD is on version:

SAM CLI, version 0.22.0

It fails on our developers-environment as well as on the CI/CD system. But on my local box it runs through. So it seems like a regression to me.

Edit: @ericallam mentioned, he is using 0.13.0. Looks even more weird to me...

mindrunner commented 5 years ago

I am trying to work around this to get our CI/CD running again. However, passing --parameter-overrides to sam build does not help at all.

Error: <string> is an Invalid Layer Arn. is what I always get. No matter what parameters I pass for overriding.

My deployment script is bloated by workarounds for sam and cloudformation. Is this supposed to be production ready? I cannot believe, I need to maintain two different versions of --parameter-overrides for sam and aws-cli.

Is there any option to not use sam? Can I use other tools for building?

mindrunner commented 5 years ago

I solved my problem by just kicking out sam

Before:

Now:

I am a little bit lost now. Can someone explain to me, why we actually need SAM? I was thinking, it is the build-chain for serverless applications on AWS. But why can I deploy my apps without building them before? Is there any manual about what is actually happening here?

christophsteiner commented 4 years ago

Can somebody point me to the solution with using the parameter-overrides? I am currently struggling. I have the same issue with not a valid ARN. What do I provide as override because if I provide the SSM Parameter as override, I get the same error

mindrunner commented 4 years ago

I can't sorry. I stopped using SAM for production. It is marked as Beta and there seems no support from the devs here. It might even be a stale/dead project.

Maybe think if you really need SAM @christophsteiner, depending what you are intending to do.

Drop me a line if you or your project needs any help

jfuss commented 4 years ago

@mindrunner

It is marked as Beta and there seems no support from the devs here. It might even be a stale/dead project.

We are doing our best to keep all issues updated. We also made a release on Friday: https://github.com/awslabs/aws-sam-cli/releases So the project is far from dead.

@christophsteiner Assuming the SSM Value is a Parameter in the CloudFormation Template, you can add --parameter-overrides ParameterKey=MyParam1,ParameterValue=MyValue1 or --parameter-overrides MyParam1=MyValue1. For build, it just has to be a valid layer ARN. It does not have to exist, since we do not do anything with it but validate it is an ARN.

mindrunner commented 4 years ago

@jfuss thanks for your help :)

christophsteiner commented 4 years ago

@jfuss Thanks. Providing a valid ARN was what I was missing to pass sam build. @mindrunner Thank you for you offer to help. We are totally fine with SAM at the moment and I am personally very pleased with the rate of improvement it shows.

shanesiebken commented 4 years ago

this solution works in 0.47.0 but no longer works in 0.48.0. I also tried feeding the parameter override to deployment but it won't deploy with a parameter path for a default on an SSM parameter

jfuss commented 4 years ago

@shanesiebken See #1958, you are seeing a different issue that is already resolved in develop.

shanesiebken commented 4 years ago

Thanks @jfuss. I didn't think to look for a new issue since I've been tracking this conversation.

jbpratt commented 4 years ago

I believe I am dealing with a sort of similar problem but I am happy to open a separate issue as this isn't a parameter issue and feels a bit different.

Transform:
  - "AWS::Serverless-2016-10-31"
  - PipLayer
Resources:
  DepLayer:
    Type: "AWS::Lambda::PipLayer"
    Properties:
      BucketName: test
      CompatibleRuntimes:
        - python3.8
      LayerName: camelcase
      Package:
        name: camelcase
        version: "0.2"
  Function:
    Type: "AWS::Serverless::Function"
    Properties:
      CodeUri: src/
      Handler: app.handler
      Layers:
        - !Ref DepLayer

Error: DepLayer is an Invalid Layer Arn. I have a custom macro for provisioning Layers from Pip packages, the above code fails to build. I'm not really sure of what the best behavior, if any, for SAM would be here.

obsequiouswoe commented 4 years ago

For clarity for anyone else fighting this bug the following literally got me working:

sam build --parameter-overrides param1=arn:::::::1 param2=arn:::::::1

rrlino-sage commented 4 years ago

Hey guys, just curious what is the stage/pm-review? should we expect this soon?

PierreKiwi commented 4 years ago

So @jfuss, if my understanding is correct, you are saying that we can't use AWS::SSM::Parameter::Value<String> to specify the ARN of a Lambda Layer when using SAM?

My ARNs are saved in SSM because my layers do not have the save versioning for all the regions so I can't use the ARN directly.

Why does SAM not interpolate the parameters like CloudFormation does? I thought SAM was a wrapper around CFn (so basically CFn + extra features).

awsjeffg commented 3 years ago

Hey guys, just curious what is the stage/pm-review? should we expect this soon?

Sorry for the delay in replying - we're still evaluating customer impact.

Nr18 commented 3 years ago

So this behaviour is also applied to the deploy target but where you have the ability to overwrite the parameter when building during deployment you cannot use that workaround I created #2474 for that.

This basically means that I build and package using SAM but I deploy using the AWS CLI, loosing all the nice visual feedback from SAM :-(

nkeating-mutualofenumclaw commented 3 years ago

Just my two cents. This is extremely impactful to major deployments which share a common library, and which won't all be deployed with substacks. This should be a must have for enterprise organizations which want to have organizational reference layers.

GregTurner commented 3 years ago

Same error on sam deploy but using the resolve function:

      Layers:
        - '{{resolve:ssm:MyLayerArn:1}}'
dougmolineux commented 3 years ago

This seems to be still happening with version 1.18.1

Error: arn:aws:lambda:us-east-1:123456789012:layer:my-layer:MyVersionNumberParameter is an Invalid Layer Arn.

With it defined like:

Layers:
   - !Sub 'arn:aws:lambda:${AWS::Region}:${AWS::AccountId}:layer:my-layer:${MyVersionNumberParameter}'
sachrajneesh commented 3 years ago

@dougmolineux

Actually this does work in version 1.18 for me

If you add a default numeric value for MyVersionNumberParameter in your template.

dougmolineux commented 3 years ago

Thank you for the response! I will try this tomorrow

Sent from my iPhone

On Feb 16, 2021, at 8:40 PM, sachrajneesh notifications@github.com wrote:

 @dougmolineux

Actually this does work in version 1.18 for me

!Sub 'arn:aws:lambda:${AWS::Region}:${AWS::AccountId}:layer:my-layer:${MyVersionNumberParameter}' If you add a default numeric value for MyVersionNumberParameter in your template.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub, or unsubscribe.

dougmolineux commented 3 years ago

@sachrajneesh adding a default value does indeed make it work, thank you so much!

aahung commented 3 years ago

This issue is more impacting after nested stack feature was released in 1.20.0, see #2763

JordanShurmer commented 3 years ago

Just adding my note that SAM is still reporting these Refs as invalid at build time. It would be great if SAM would just no validate Refs at build time

JordanShurmer commented 3 years ago

This comment comes from https://github.com/aws/aws-sam-cli/issues/2474

If anyone is looking for a workaround.. my solution is to lookup the SSM value elsewhere in the build process and pass it in using --parameter-overrides

Workaround Example

For example, here's a makefile using this method for both the build and the deploy steps

SHARED_LAYER := $(shell  aws ssm get-parameter --name '/path/to/lambda/arn' --query 'Parameter.Value' --output text)

deploy: .aws-sam/build/template.yaml
    sam deploy --template .aws-sam/build/template.yaml --parameter-overrides LambdaArnParam=${SHARED_LAYER}

.aws-sam/build/template.yaml:
    sam build --parameter-overrides LambdaArnParam=${SHARED_LAYER

Note that you'll have to change the Parameter type to a simple String

aahung commented 3 years ago

This comment comes from #2474

If anyone is looking for a workaround.. my solution is to lookup the SSM value elsewhere in the build process and pass it in using --parameter-overrides

Workaround Example

For example, here's a makefile using this method for both the build and the deploy steps

SHARED_LAYER := $(shell  aws ssm get-parameter --name '/path/to/lambda/arn' --query 'Parameter.Value' --output text)

deploy: .aws-sam/build/template.yaml
    sam deploy --template .aws-sam/build/template.yaml --parameter-overrides LambdaArnParam=${SHARED_LAYER}

.aws-sam/build/template.yaml:
    sam build --parameter-overrides LambdaArnParam=${SHARED_LAYER

Note that you'll have to change the Parameter type to a simple String

@JordanShurmer in the latest 1.22.0 release, unresolvable layers under a function are ignored in sam build, can you retry using 1.22.0?

JordanShurmer commented 3 years ago

Great! thanks for the update @aahung. I'll try to get our CI env updated with this version too :)

Was the deploy command updated as well?

aahung commented 3 years ago

Was the deploy command updated as well?

Deploy should always work

aahung commented 3 years ago

Closing, 1.22 ignores unresolvable layers so sam build won't fail. Please let us know if the issue still persists.

github-actions[bot] commented 3 years ago

⚠️COMMENT VISIBILITY WARNING⚠️

Comments on closed issues are hard for our team to see. If you need more assistance, please either tag a team member or open a new issue that references this one. If you wish to keep having a conversation with other community members under this issue feel free to do so.