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

fix(cloudformation): fix some resources #69

Merged
merged 2 commits into from
Jan 17, 2024

Conversation

nikpivkin
Copy link
Collaborator

@nikpivkin nikpivkin commented Dec 28, 2023

Fix aquasecurity/trivy#5802

CloudFormation casts the types to the required types if possible. So a boolean property can be passed the string "false", which will be cast to bool. We could infer the type at the parsing stage so that we don't have to worry about it later, but we don't know the type of the property and so it's not safe. We must use GetBoolProperty to get the boolean value of the property, because this method converts the string representation of a boolean value to a boolean value.

Also fixed incorrect property names and default property values.

@@ -0,0 +1,66 @@
package ec2
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: typo in file name, should be adapt_test.go

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fixed 20a2496

Enabled: defsecTypes.BoolDefault(false, clusterResource.Metadata()),
KMSKeyID: defsecTypes.StringDefault("", clusterResource.Metadata()),
Enabled: clusterResource.GetBoolProperty("PerformanceInsightsEnabled"),
KMSKeyID: clusterResource.GetStringProperty("PerformanceInsightsKmsKeyId"),
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should this be the following?

Suggested change
KMSKeyID: clusterResource.GetStringProperty("PerformanceInsightsKmsKeyId"),
KMSKeyID: clusterResource.GetStringProperty("PerformanceInsightsKMSKeyId"),

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Either way, we should add it to the test case so we can cover it.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Collaborator Author

@nikpivkin nikpivkin Jan 17, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Updated the tests

@nikpivkin nikpivkin marked this pull request as ready for review January 17, 2024 20:07
@simar7 simar7 merged commit 3d568e9 into aquasecurity:main Jan 17, 2024
3 of 4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

bug(cloudformation): Trivy sometimes fails to recognise string interpretations of Boolean values
2 participants