-
Notifications
You must be signed in to change notification settings - Fork 540
feat(misconf): Adds CloudFront standard logging v2 support to AVD-AWS-0010 #10848
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
Changes from 13 commits
4431e31
5469868
501e63c
ddf6ca5
2b97e16
6a4b3d5
fd3aa58
c64562c
f3cba5a
3b2afa1
2e07db1
f960b34
b3982d5
2946f5b
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -16,7 +16,14 @@ func adaptDistributions(modules terraform.Modules) []cloudfront.Distribution { | |
| var distributions []cloudfront.Distribution | ||
| for _, module := range modules { | ||
| for _, resource := range module.GetResourcesByType("aws_cloudfront_distribution") { | ||
| distributions = append(distributions, adaptDistribution(resource)) | ||
| distribution := adaptDistribution(resource) | ||
|
|
||
| distribution.Logging.V2 = cloudfront.LoggingV2{ | ||
| Metadata: resource.GetMetadata(), | ||
| Enabled: hasV2Logging(modules, resource), | ||
| } | ||
|
|
||
| distributions = append(distributions, distribution) | ||
| } | ||
| } | ||
| return distributions | ||
|
|
@@ -77,3 +84,22 @@ func adaptDistribution(resource *terraform.Block) cloudfront.Distribution { | |
|
|
||
| return distribution | ||
| } | ||
|
|
||
| func hasV2Logging(modules terraform.Modules, distributionBlock *terraform.Block) types.BoolValue { | ||
| metadata := distributionBlock.GetMetadata() | ||
|
|
||
| sources := modules.GetReferencingResources(distributionBlock, "aws_cloudwatch_log_delivery_source", "resource_arn") | ||
| for _, source := range sources { | ||
| if !source.GetAttribute("log_type").Equals("ACCESS_LOGS") { | ||
| continue | ||
| } | ||
|
|
||
| deliveries := modules.GetReferencingResources(source, "aws_cloudwatch_log_delivery", "delivery_source_name") | ||
| if len(deliveries) > 0 { | ||
| return types.Bool(true, metadata) | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Just as in CF, it's best to return the
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Until now it was under assumption that merely an existence of a
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes, that's fine, since there's supposed to be at least one delivery resource here.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. also just a note for both CF and Terraform in case of false, distribution metadata is returned |
||
| } | ||
| } | ||
|
|
||
| return types.Bool(false, metadata) | ||
|
|
||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -89,6 +89,99 @@ func Test_adaptDistribution(t *testing.T) { | |
| } | ||
| } | ||
|
|
||
| func Test_adaptDistributionV2(t *testing.T) { | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Could you add a negative case where the delivery source references the distribution and a delivery exists, but |
||
| tests := []struct { | ||
| name string | ||
| terraform string | ||
| expected cloudfront.Distribution | ||
| }{ | ||
| { | ||
| name: "v2 logging configured", | ||
| terraform: ` | ||
| resource "aws_cloudfront_distribution" "example" {} | ||
|
|
||
| resource "aws_cloudwatch_log_delivery_source" "example" { | ||
| log_type = "ACCESS_LOGS" | ||
| resource_arn = aws_cloudfront_distribution.example.arn | ||
| } | ||
|
|
||
| resource "aws_cloudwatch_log_delivery" "example" { | ||
| delivery_source_name = aws_cloudwatch_log_delivery_source.example.name | ||
| } | ||
| `, | ||
| expected: cloudfront.Distribution{ | ||
| Logging: cloudfront.Logging{ | ||
| V2: cloudfront.LoggingV2{ | ||
| Enabled: iacTypes.BoolTest(true), | ||
| }, | ||
| }, | ||
| DefaultCacheBehaviour: cloudfront.CacheBehaviour{}, | ||
| ViewerCertificate: cloudfront.ViewerCertificate{ | ||
| MinimumProtocolVersion: iacTypes.StringTest("TLSv1"), | ||
| }, | ||
| }, | ||
| }, | ||
| { | ||
| name: "v2 logging source exists but no delivery", | ||
| terraform: ` | ||
| resource "aws_cloudfront_distribution" "example" {} | ||
|
|
||
| resource "aws_cloudwatch_log_delivery_source" "example" { | ||
| log_type = "ACCESS_LOGS" | ||
| resource_arn = aws_cloudfront_distribution.example.arn | ||
| } | ||
| `, | ||
| expected: cloudfront.Distribution{ | ||
| Logging: cloudfront.Logging{ | ||
| V2: cloudfront.LoggingV2{ | ||
| Enabled: iacTypes.BoolTest(false), | ||
| }, | ||
| }, | ||
| DefaultCacheBehaviour: cloudfront.CacheBehaviour{}, | ||
| ViewerCertificate: cloudfront.ViewerCertificate{ | ||
| MinimumProtocolVersion: iacTypes.StringTest("TLSv1"), | ||
| }, | ||
| }, | ||
| }, | ||
|
|
||
| { | ||
| name: "v2 logging with non-access log_type", | ||
| terraform: ` | ||
| resource "aws_cloudfront_distribution" "example" {} | ||
|
|
||
| resource "aws_cloudwatch_log_delivery_source" "example" { | ||
| log_type = "ERROR_LOGS" | ||
| resource_arn = aws_cloudfront_distribution.example.arn | ||
| } | ||
|
|
||
| resource "aws_cloudwatch_log_delivery" "example" { | ||
| delivery_source_name = aws_cloudwatch_log_delivery_source.example.name | ||
| } | ||
| `, | ||
| expected: cloudfront.Distribution{ | ||
| Logging: cloudfront.Logging{ | ||
| V2: cloudfront.LoggingV2{ | ||
| Enabled: iacTypes.BoolTest(false), | ||
| }, | ||
| }, | ||
| DefaultCacheBehaviour: cloudfront.CacheBehaviour{}, | ||
| ViewerCertificate: cloudfront.ViewerCertificate{ | ||
| MinimumProtocolVersion: iacTypes.StringTest("TLSv1"), | ||
| }, | ||
| }, | ||
| }, | ||
| } | ||
|
|
||
| for _, test := range tests { | ||
| t.Run(test.name, func(t *testing.T) { | ||
| modules := tftestutil.CreateModulesFromSource(t, test.terraform, ".tf") | ||
| adapted := Adapt(modules) | ||
| require.Len(t, adapted.Distributions, 1) | ||
| testutil.AssertDefsecEqual(t, test.expected, adapted.Distributions[0]) | ||
| }) | ||
| } | ||
| } | ||
|
|
||
| func TestLines(t *testing.T) { | ||
| src := ` | ||
| resource "aws_cloudfront_distribution" "example" { | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Two cases missing here:
LogTypeother thanACCESS_LOGS→ expect false (covers theLogTypefilter).DeliverySourceNamegiven as!Ref CloudFrontAccessLogsDeliverySource(logical ID) instead of the literal name → expect true.