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

'CodeUri' requires Bucket and Key properties to be specified using Fn::ImportValue. #3264

Closed
garretwilson opened this issue Jul 15, 2023 · 5 comments
Labels
area/intrinsics Ref, If, Sub, GetAtt, ... stage/needs-triage Automatically applied to new issues and PRs, indicating they haven't been looked at.

Comments

@garretwilson
Copy link

I'm trying to use a combination of Fn::Sub and Fn::ImportValue to specify CodeUri for AWS::Serverless::Function. I'm doing this because the staging bucket name is exported from another stack.

Let's start from the basics. I can do this with no problem:

      CodeUri: !Sub "s3://my-staging-bucket/foo-${ver}-aws-lambda.zip"

I can also substitute the entire bucket name, like this:

      CodeUri: !Sub
        - "s3://${bucket}/foo-${ver}-aws-lambda.zip"
        - bucket: my-staging-bucket

Great—now all I need to do is import my bucket name from the other stack. It turns out the other stack name is special based upon the name of the environment the stack is using, so I have to substitute parameters for the stack name that exports the value. Here goes:

      CodeUri: !Sub
        - "s3://${bucket}/foo-${ver}-aws-lambda.zip"
        - bucket: my-staging-bucket
            Fn::ImportValue:
              !Sub "other-stack-${env}:stagingBucket"

I've been using this technique for months with no problem across pure CloudFormation templates. Unfortunately SAM doesn't like it, and says:

Error: Failed to create changeset for the stack: my-stack, ex: Waiter ChangeSetCreateComplete failed: Waiter encountered a terminal failure state: For expression "Status" we matched expected path: "FAILED" Status: FAILED. Reason: Transform AWS::Serverless-2016-10-31 failed with: Invalid Serverless Application Specification document. Number of errors found: 1. Resource with id [MyFunction] is invalid. 'CodeUri' requires Bucket and Key properties to be specified.

My best guess is that SAM is using some naive validation on the expression, instead of just letting me use what I want to use, and this expression only allows limited expressions that the designers thought of.

Note that in pure CloudFormation, I can do this just fine using AWS::Lambda::Function, which uses Code instead of CodeUri. It looks like this:

      Code:
        S3Bucket:
          Fn::ImportValue:
            !Sub "other-stack-${env}:stagingBucket"
        S3Key: !Sub "foo-${ver}-aws-lambda.zip"

That works fine. But when I tried to do the equivalent thing with SAM, it breaks.

It's one thing that SAM doesn't understand expression, as in aws/aws-sam-cli#5249, and won't help me out by uploading a Zip file when using an expression. I've worked around that by manually uploading the file. But SAM shouldn't prevent me from using an expression—that is much worse. I shouldn't be deprived of normal CloudFormation expression handling (which is already limited to begin with) by switching to SAM. I'm deciding now whether I should just switch back to CloudFormation for function definitions. I don't want to hard-code the staging bucket, but I don't know of a workaround at the moment.

Note: It's possible I'm in error here and have incorrectly included Fn::ImportValue, because the way CloudFormation using Fn::Sub and Fn::ImportValue is very clunky and error-prone. But I don't think I've made a mistake, because I've copied the expression almost verbatim, and with the same layout, from another pure CloudFormation template.

@hawflau
Copy link
Contributor

hawflau commented Jul 18, 2023

Hi @garretwilson Thanks for raising the issue. From your error, it seems the template was already submitted to CreateChangeSet. Despite sam deploy was run, the error came from CreateChangeSet, in which SAM Transform was involved. Moving it to SAM-T repo.

I wonder if AWS::LanguageExtensions can handle it. Skimming through its doc, I don't think it can...

@hawflau hawflau transferred this issue from aws/aws-sam-cli Jul 18, 2023
@garretwilson
Copy link
Author

garretwilson commented Jul 18, 2023

I wonder if AWS::LanguageExtensions can handle it.

I'm already using AWS::LanguageExtensions. Do you realize how absurd it is we even have to add AWS::LanguageExtensions in the first place, to place bandages everywhere to try to get things working that worked fine in CloudFormation but then break in SAM?

Someone will probably mention #2533 at some point and say, "yeah, that's still broken". Undoubtedly we'll get the same with #3260.

As I mentioned in aws/aws-sam-cli#5254:

The whole CloudFormation template thing SAM is built on is very brittle to begin with, as illustrated by aws/aws-sam-cli#5249, and in fact the very need to have the AWS::LanguageExtensions hack in the first place, which itself doesn't even work all the time; see aws-cloudformation/cfn-language-discussion#127 .

@aaythapa
Copy link
Contributor

Hi there, thanks for opening the ticket and the thorough description. I can replicate the issue you are having and I don't think you're using Fn:ImportValue incorrectly.

Unfortunately, theres not much we can do at this time. We don't resolve intrinsics in SAM and this comment from another issue you opened explains well why we don't do so. Our go to solution for intrinsic has been to add AWS::LanguageExtensions as it resolves most of our intrinsic issues but didn't help in this case. I see you've already opened a ticket with the CFN team, fwiw I've +1'd the ticket and will continue to track it.

As for now there are other workaround besides AWS::LanguageExtensions if you want to continue using SAM. This issue lists some as does this comment.

@aaythapa aaythapa added the area/intrinsics Ref, If, Sub, GetAtt, ... label Jul 21, 2023
@garretwilson
Copy link
Author

garretwilson commented Sep 23, 2023

@aaythapa one of the workarounds mentioned in the links you gave is to use a pass-through property.

Fine! If AWS::Serverless::Function provided a Code pass-through property, I could use that. I want to use that!

But AWS::Serverless::Function does not provide Code. It only provides CodeUri.

The only other realistic option is to use raw CloudFormation AWS::Lambda::Function. But doing that defeats the purpose of SAM altogether, as now I have to manually create all the policies and permissions. Going that route removes all reason to use SAM. I might as well ditch SAM and use raw CloudFormation. (Or write my own deployment tool that competes with SAM.)

Could you please add a Code pass-through property to AWS::Serverless::Function so we can at least work around this shortcoming in the design of SAM?

@garretwilson
Copy link
Author

garretwilson commented Sep 29, 2023

Update: It turns out that there are actually passthrough properties already—they are just "hidden" under the CodeUri rather than at the same level. See the FunctionCode object and the discussion in #3358.

With this new information, this particular bug isn't really a problem—I'll just avoid CodeUri as a string and use a FunctionCode object instead.

I'm not sure why this wasn't evident earlier. Perhaps at the time I was still trying to use SAM's own upload facility (aws/aws-sam-cli#5249); but now that I've essentially rewrote the entire SAM upload logic with improved features, I can use any form of the CodeUri that I like.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/intrinsics Ref, If, Sub, GetAtt, ... stage/needs-triage Automatically applied to new issues and PRs, indicating they haven't been looked at.
Projects
None yet
Development

No branches or pull requests

3 participants