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

(aws-sam): cannot generate string policies for CfnFunction #12854

Closed
spalladino opened this issue Feb 3, 2021 · 8 comments · Fixed by #12954
Closed

(aws-sam): cannot generate string policies for CfnFunction #12854

spalladino opened this issue Feb 3, 2021 · 8 comments · Fixed by #12954
Assignees
Labels
@aws-cdk/aws-sam Related to AWS Serverless Application Model guidance Question that needs advice or information.

Comments

@spalladino
Copy link

The property CfnFunctionProps.policies accepts a list of both objects and strings. However, when I try to generate a list with a string, an empty object is generated instead.

Reproduction Steps

Running cdk synth with the following construct:

new CfnFunction(scope, 'MyFunction', {
  policies: ['AWSLambdaExecute']
});

What did you expect to happen?

  MyFunction:
    Type: AWS::Serverless::Function
    Properties:
      Policies:
        - AWSLambdaExecute

What actually happened?

  MyFunction:
    Type: AWS::Serverless::Function
    Properties:
      Policies:
        - {}

Environment

  • CDK CLI Version : 1.80.0
  • Framework Version:
  • Node.js Version: 12.18.3
  • OS : Ubuntu 19.10
  • Language (Version):

Other

Note that if I use a string, instead of a list of strings, the output is correct.

new CfnFunction(scope, 'MyFunction', {
  policies: 'AWSLambdaExecute'
});
  MyFunction:
    Type: AWS::Serverless::Function
    Properties:
      Policies: 'AWSLambdaExecute'

This is 🐛 Bug Report

@spalladino spalladino added bug This issue is a bug. needs-triage This issue or PR still needs to be triaged. labels Feb 3, 2021
@github-actions github-actions bot added the @aws-cdk/aws-sam Related to AWS Serverless Application Model label Feb 3, 2021
@skinny85
Copy link
Contributor

skinny85 commented Feb 3, 2021

@spalladino you're missing a bunch of required properties in your code, I'm surprised it compiles...

This:

        new sam.CfnFunction(this, 'MyFunction', {
            codeUri: {
                bucket: 'my-bucket',
                key: 'my-key',
            },
            runtime: 'nodejs-12.x',
            handler: 'index.handler',
            policies: 'AWSLambdaExecute',
        });

Produces this for me:

Transform: AWS::Serverless-2016-10-31
  MyFunction:
    Type: AWS::Serverless::Function
    Properties:
      CodeUri:
        Bucket: my-bucket
        Key: my-key
      Handler: index.handler
      Runtime: nodejs-12.x
      Policies: AWSLambdaExecute
    Metadata:
      aws:cdk:path: TestStack/MyFunction

@skinny85 skinny85 added guidance Question that needs advice or information. response-requested Waiting on additional info and feedback. Will move to "closing-soon" in 7 days. and removed bug This issue is a bug. needs-triage This issue or PR still needs to be triaged. labels Feb 3, 2021
@skinny85 skinny85 assigned skinny85 and unassigned njlynch Feb 3, 2021
@skinny85
Copy link
Contributor

skinny85 commented Feb 3, 2021

Oh, I see what you're saying. The problem is for an array of strings, not a single string. Let me see.

@spalladino
Copy link
Author

Sorry, I guess I went with a too minimal example - I only included the property that was causing trouble. Let me know if you need a more complete example to reproduce.

@skinny85
Copy link
Contributor

skinny85 commented Feb 3, 2021

No, I'm good, thanks. Let me dig into this one a little bit more. I suspect it might be a bug with our code generation.

@spalladino
Copy link
Author

spalladino commented Feb 3, 2021

If I had to bet, I'd say the culprit is the SAMPolicyTemplateProperty defined as one of the possible property types before string. Since it doesn't have any required properties, the code generator may be thinking the string can be a valid SAMPolicyTemplateProperty, and thus outputs the empty object. And this doesn't happen when the string is not in an array because the SAMPolicyTemplateProperty is not a possible type then.

@skinny85
Copy link
Contributor

skinny85 commented Feb 3, 2021

I think you're exactly right. I feel like I'm barely needed here 😃.

@skinny85
Copy link
Contributor

skinny85 commented Feb 3, 2021

Which means we need to make sure that a string is not a valid object in our validation logic.

@github-actions github-actions bot removed the response-requested Waiting on additional info and feedback. Will move to "closing-soon" in 7 days. label Feb 4, 2021
skinny85 added a commit to skinny85/aws-cdk that referenced this issue Feb 9, 2021
…red properties in a union

In our code generation, in the validation functions for CFN complex types,
we never checked whether the argument we received is an object.
Because of that, passing a string was actually a correct candidate for a union with a complex type that had no required properties.
Fix that by checking for that error explicitly,
which removes the complex type without required properties from the candidates when a string is passed.

Fixes aws#12854
skinny85 added a commit to skinny85/aws-cdk that referenced this issue Feb 11, 2021
…red properties in a union

In our code generation, in the validation functions for CFN complex types,
we never checked whether the argument we received is an object.
Because of that, passing a string was actually a correct candidate for a union with a complex type that had no required properties.
Fix that by checking for that error explicitly,
which removes the complex type without required properties from the candidates when a string is passed.

Fixes aws#12854
@mergify mergify bot closed this as completed in #12954 Feb 12, 2021
mergify bot pushed a commit that referenced this issue Feb 12, 2021
…red properties in a union (#12954)

In our code generation, in the validation functions for CFN complex types,
we never checked whether the argument we received is an object.
Because of that, passing a string was actually a correct candidate for a union with a complex type that had no required properties.
Fix that by checking for that error explicitly,
which removes the complex type without required properties from the candidates for the union type when a string is passed.

Fixes #12854

----

*By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
@github-actions
Copy link

⚠️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.

NovakGu pushed a commit to NovakGu/aws-cdk that referenced this issue Feb 18, 2021
…red properties in a union (aws#12954)

In our code generation, in the validation functions for CFN complex types,
we never checked whether the argument we received is an object.
Because of that, passing a string was actually a correct candidate for a union with a complex type that had no required properties.
Fix that by checking for that error explicitly,
which removes the complex type without required properties from the candidates for the union type when a string is passed.

Fixes aws#12854

----

*By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
@aws-cdk/aws-sam Related to AWS Serverless Application Model guidance Question that needs advice or information.
Projects
None yet
3 participants