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.

Sign up for GitHub

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

Jump to bottom

aws-lambda: incorrect securityGroups check #27147

Closed
TheWarrenG opened this issue Sep 14, 2023 · 2 comments · Fixed by #27157
Closed

aws-lambda: incorrect securityGroups check #27147

TheWarrenG opened this issue Sep 14, 2023 · 2 comments · Fixed by #27157
Labels
@aws-cdk/aws-lambda Related to AWS Lambda bug This issue is a bug. effort/small Small work item – less than a day of effort p1

Comments

@TheWarrenG
Copy link

TheWarrenG commented Sep 14, 2023 •
edited
Loading

Describe the bug

This commit in aws-lambda adds a check for the presence of a security groups parameter if the VPC parameter is not present. Since TypeScript treats empty arrays as true, the check for the securityGroups parameter evaluates to true when an empty array is passed in for FunctionProps.securityGroups.

Expected Behavior

This condition checking for the presence of FunctionProps.securityGroups should evaluate to false if FunctionProps.securityGroups is an empty array.

Current Behavior

This condition checking for the presence of FunctionProps.securityGroups should evaluate to true if FunctionProps.securityGroups is an empty array.

Reproduction Steps

Pass in an empty array for the securityGroups field and no value for the vpc field in FunctionProps when calling Function::configureVpc method.

Possible Solution

Explicitly check for empty arrays in addition to the current truthiness check.

Additional Information/Context

No response

CDK CLI Version

2.93.0

Framework Version

No response

Node.js Version

14.19.1

OS

Linux

Language

Typescript

Language Version

No response

Other information

No response
@TheWarrenG TheWarrenG added bug This issue is a bug. needs-triage This issue or PR still needs to be triaged. labels Sep 14, 2023
@github-actions github-actions bot added the @aws-cdk/aws-lambda Related to AWS Lambda label Sep 14, 2023
@cgarvis cgarvis added the p1 label Sep 14, 2023
@TheWarrenG TheWarrenG changed the title aws-lambda: incorrect VPC check aws-lambda: incorrect securityGroups check Sep 14, 2023
@peterwoodworth peterwoodworth added effort/small Small work item – less than a day of effort and removed needs-triage This issue or PR still needs to be triaged. labels Sep 14, 2023
@peterwoodworth
Copy link
Contributor

I can reproduce this, thanks for reporting

@lpizzinidev lpizzinidev mentioned this issue Sep 15, 2023
Merged
mergify bot added a commit to lpizzinidev/aws-cdk that referenced this issue Sep 15, 2023
@mergify
Merge branch 'main' into awsgh-27147
58754e1
@mergify mergify bot closed this as completed in #27157 Sep 15, 2023
mergify bot pushed a commit that referenced this issue Sep 15, 2023
@lpizzinidev
refactor(lambda): ignore empty securityGroups property (#27157)
f1948a9 
Changes the checks on `securityGroups` to avoid throwing errors, if `vpc` is not specified, or create a default security group when an empty array is passed.

Closes #27147.

----

*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.

@github-actions github-actions bot mentioned this issue Oct 1, 2023
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
@aws-cdk/aws-lambda Related to AWS Lambda bug This issue is a bug. effort/small Small work item – less than a day of effort p1
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

refactor(lambda): ignore empty securityGroups property lpizzinidev/aws-cdk
3 participants
@cgarvis @TheWarrenG @peterwoodworth