From 7c034baa84e1d354c18681b54f5016c7af40228c Mon Sep 17 00:00:00 2001 From: Nikita Pivkin <100182843+nikpivkin@users.noreply.github.com> Date: Thu, 27 Jul 2023 06:35:25 +0300 Subject: [PATCH] fix(cloudformation): s3 object_lock_configuration sets versioning to true (#1398) * fix(cloudformation): s3 object_lock_configuration sets versioning to true * fix lint errors --- internal/adapters/terraform/aws/s3/bucket.go | 153 +++++++++--------- .../adapters/terraform/aws/s3/bucket_test.go | 64 ++++++++ 2 files changed, 137 insertions(+), 80 deletions(-) diff --git a/internal/adapters/terraform/aws/s3/bucket.go b/internal/adapters/terraform/aws/s3/bucket.go index 4d776ce33..4464049a1 100644 --- a/internal/adapters/terraform/aws/s3/bucket.go +++ b/internal/adapters/terraform/aws/s3/bucket.go @@ -51,31 +51,15 @@ func getEncryption(block *terraform.Block, a *adapter) s3.Encryption { KMSKeyId: block.GetNestedAttribute("server_side_encryption_configuration.rule.apply_server_side_encryption_by_default.kms_master_key_id").AsStringValueOrDefault("", block), } } - for _, encryptionResource := range a.modules.GetResourcesByType("aws_s3_bucket_server_side_encryption_configuration") { - bucketAttr := encryptionResource.GetAttribute("bucket") - if bucketAttr.IsNotNil() { - if bucketAttr.IsString() { - actualBucketName := block.GetAttribute("bucket").AsStringValueOrDefault("", block).Value() - if bucketAttr.Equals(block.ID()) || bucketAttr.Equals(actualBucketName) { - return s3.Encryption{ - Metadata: encryptionResource.GetMetadata(), - Enabled: isEncrypted(encryptionResource), - Algorithm: encryptionResource.GetNestedAttribute("rule.apply_server_side_encryption_by_default.sse_algorithm").AsStringValueOrDefault("", block), - KMSKeyId: encryptionResource.GetNestedAttribute("rule.apply_server_side_encryption_by_default.kms_master_key_id").AsStringValueOrDefault("", block), - } - } - } - if referencedBlock, err := a.modules.GetReferencedBlock(bucketAttr, encryptionResource); err == nil { - if referencedBlock.ID() == block.ID() { - return s3.Encryption{ - Metadata: encryptionResource.GetMetadata(), - Enabled: isEncrypted(encryptionResource), - Algorithm: encryptionResource.GetNestedAttribute("rule.apply_server_side_encryption_by_default.sse_algorithm").AsStringValueOrDefault("", block), - KMSKeyId: encryptionResource.GetNestedAttribute("rule.apply_server_side_encryption_by_default.kms_master_key_id").AsStringValueOrDefault("", block), - } - } - } + if val, ok := applyForBucketRelatedResource(a, block, "aws_s3_bucket_server_side_encryption_configuration", func(resource *terraform.Block) s3.Encryption { + return s3.Encryption{ + Metadata: resource.GetMetadata(), + Enabled: isEncrypted(resource), + Algorithm: resource.GetNestedAttribute("rule.apply_server_side_encryption_by_default.sse_algorithm").AsStringValueOrDefault("", block), + KMSKeyId: resource.GetNestedAttribute("rule.apply_server_side_encryption_by_default.kms_master_key_id").AsStringValueOrDefault("", block), } + }); ok { + return val } return s3.Encryption{ Metadata: block.GetMetadata(), @@ -91,29 +75,43 @@ func getVersioning(block *terraform.Block, a *adapter) s3.Versioning { Enabled: defsecTypes.BoolDefault(false, block.GetMetadata()), MFADelete: defsecTypes.BoolDefault(false, block.GetMetadata()), } + if lockBlock := block.GetBlock("object_lock_configuration"); lockBlock != nil { + if enabled := isObjeckLockEnabled(lockBlock); enabled != nil { + versioning.Enabled = *enabled + } + } if vBlock := block.GetBlock("versioning"); vBlock != nil { versioning.Enabled = vBlock.GetAttribute("enabled").AsBoolValueOrDefault(true, vBlock) versioning.MFADelete = vBlock.GetAttribute("mfa_delete").AsBoolValueOrDefault(false, vBlock) } - for _, versioningResource := range a.modules.GetResourcesByType("aws_s3_bucket_versioning") { - bucketAttr := versioningResource.GetAttribute("bucket") - if bucketAttr.IsNotNil() { - if bucketAttr.IsString() { - actualBucketName := block.GetAttribute("bucket").AsStringValueOrDefault("", block).Value() - if bucketAttr.Equals(block.ID()) || bucketAttr.Equals(actualBucketName) { - return getVersioningFromResource(versioningResource) - } - } - if referencedBlock, err := a.modules.GetReferencedBlock(bucketAttr, versioningResource); err == nil { - if referencedBlock.ID() == block.ID() { - return getVersioningFromResource(versioningResource) - } - } + + if enabled, ok := applyForBucketRelatedResource(a, block, "aws_s3_bucket_object_lock_configuration", func(resource *terraform.Block) *defsecTypes.BoolValue { + if block.GetAttribute("object_lock_enabled").IsTrue() { + return isObjeckLockEnabled(resource) } + return nil + }); ok && enabled != nil { + versioning.Enabled = *enabled + } + + if val, ok := applyForBucketRelatedResource(a, block, "aws_s3_bucket_versioning", getVersioningFromResource); ok { + return val } return versioning } +func isObjeckLockEnabled(resource *terraform.Block) *defsecTypes.BoolValue { + var val defsecTypes.BoolValue + attr := resource.GetAttribute("object_lock_enabled") + switch { + case attr.IsNil(): // enabled by default + val = defsecTypes.BoolDefault(true, resource.GetMetadata()) + case attr.Equals("Enabled"): + val = defsecTypes.Bool(true, attr.GetMetadata()) + } + return &val +} + // from aws_s3_bucket_versioning func getVersioningFromResource(block *terraform.Block) s3.Versioning { versioning := s3.Versioning{ @@ -145,33 +143,18 @@ func getLogging(block *terraform.Block, a *adapter) s3.Logging { } } - for _, loggingResource := range a.modules.GetResourcesByType("aws_s3_bucket_logging") { - bucketAttr := loggingResource.GetAttribute("bucket") - if bucketAttr.IsNotNil() { - targetBucket := loggingResource.GetAttribute("target-bucket").AsStringValueOrDefault("", loggingResource) - if referencedBlock, err := a.modules.GetReferencedBlock(loggingResource.GetAttribute("target_bucket"), loggingResource); err == nil { - targetBucket = defsecTypes.String(referencedBlock.FullName(), loggingResource.GetAttribute("target_bucket").GetMetadata()) - } - if bucketAttr.IsString() { - actualBucketName := block.GetAttribute("bucket").AsStringValueOrDefault("", block).Value() - if bucketAttr.Equals(block.ID()) || bucketAttr.Equals(actualBucketName) { - return s3.Logging{ - Metadata: loggingResource.GetMetadata(), - Enabled: hasLogging(loggingResource), - TargetBucket: targetBucket, - } - } - } - if referencedBlock, err := a.modules.GetReferencedBlock(bucketAttr, loggingResource); err == nil { - if referencedBlock.ID() == block.ID() { - return s3.Logging{ - Metadata: loggingResource.GetMetadata(), - Enabled: hasLogging(loggingResource), - TargetBucket: targetBucket, - } - } - } + if val, ok := applyForBucketRelatedResource(a, block, "aws_s3_bucket_logging", func(resource *terraform.Block) s3.Logging { + targetBucket := resource.GetAttribute("target-bucket").AsStringValueOrDefault("", resource) + if referencedBlock, err := a.modules.GetReferencedBlock(resource.GetAttribute("target_bucket"), resource); err == nil { + targetBucket = defsecTypes.String(referencedBlock.FullName(), resource.GetAttribute("target_bucket").GetMetadata()) } + return s3.Logging{ + Metadata: resource.GetMetadata(), + Enabled: hasLogging(resource), + TargetBucket: targetBucket, + } + }); ok { + return val } return s3.Logging{ @@ -187,22 +170,10 @@ func getBucketAcl(block *terraform.Block, a *adapter) defsecTypes.StringValue { return aclAttr.AsStringValueOrDefault("private", block) } - for _, aclResource := range a.modules.GetResourcesByType("aws_s3_bucket_acl") { - bucketAttr := aclResource.GetAttribute("bucket") - - if bucketAttr.IsNotNil() { - if bucketAttr.IsString() { - actualBucketName := block.GetAttribute("bucket").AsStringValueOrDefault("", block).Value() - if bucketAttr.Equals(block.ID()) || bucketAttr.Equals(actualBucketName) { - return aclResource.GetAttribute("acl").AsStringValueOrDefault("private", aclResource) - } - } - if referencedBlock, err := a.modules.GetReferencedBlock(bucketAttr, aclResource); err == nil { - if referencedBlock.ID() == block.ID() { - return aclResource.GetAttribute("acl").AsStringValueOrDefault("private", aclResource) - } - } - } + if val, ok := applyForBucketRelatedResource(a, block, "aws_s3_bucket_acl", func(resource *terraform.Block) defsecTypes.StringValue { + return resource.GetAttribute("acl").AsStringValueOrDefault("private", resource) + }); ok { + return val } return defsecTypes.StringDefault("private", block.GetMetadata()) } @@ -280,3 +251,25 @@ func getaccelerateStatus(b *terraform.Block, a *adapter) defsecTypes.StringValue } return status } + +func applyForBucketRelatedResource[T any](a *adapter, block *terraform.Block, resType string, fn func(resource *terraform.Block) T) (T, bool) { + for _, resource := range a.modules.GetResourcesByType(resType) { + bucketAttr := resource.GetAttribute("bucket") + if bucketAttr.IsNotNil() { + if bucketAttr.IsString() { + actualBucketName := block.GetAttribute("bucket").AsStringValueOrDefault("", block).Value() + if bucketAttr.Equals(block.ID()) || bucketAttr.Equals(actualBucketName) { + return fn(resource), true + } + } + if referencedBlock, err := a.modules.GetReferencedBlock(bucketAttr, resource); err == nil { + if referencedBlock.ID() == block.ID() { + return fn(resource), true + } + } + } + + } + var res T + return res, false +} diff --git a/internal/adapters/terraform/aws/s3/bucket_test.go b/internal/adapters/terraform/aws/s3/bucket_test.go index 86b170f99..a11afbb61 100644 --- a/internal/adapters/terraform/aws/s3/bucket_test.go +++ b/internal/adapters/terraform/aws/s3/bucket_test.go @@ -162,6 +162,70 @@ resource "aws_s3_bucket_versioning" "example" { assert.True(t, s3.Buckets[0].Versioning.Enabled.Value()) } +func Test_BucketGetVersioningWithLockDeprecated(t *testing.T) { + source := ` +resource "aws_s3_bucket" "example" { + bucket = "mybucket" + object_lock_configuration { + object_lock_enabled = "Enabled" + } +} +` + modules := tftestutil.CreateModulesFromSource(t, source, ".tf") + + s3 := Adapt(modules) + + assert.Equal(t, 1, len(s3.Buckets)) + assert.True(t, s3.Buckets[0].Versioning.Enabled.Value()) + +} + +func Test_BucketGetVersioningWithLockForNewBucket(t *testing.T) { + source := ` +resource "aws_s3_bucket" "example" { + bucket = "mybucket" + object_lock_enabled = true +} + +resource "aws_s3_bucket_object_lock_configuration" "example" { + bucket = aws_s3_bucket.example.id +} +` + modules := tftestutil.CreateModulesFromSource(t, source, ".tf") + + s3 := Adapt(modules) + + assert.Equal(t, 1, len(s3.Buckets)) + assert.True(t, s3.Buckets[0].Versioning.Enabled.Value()) + +} + +func Test_BucketGetVersioningWhenLockDisabledButVersioningEnabled(t *testing.T) { + source := ` +resource "aws_s3_bucket" "example" { + bucket = "mybucket" +} + +resource "aws_s3_bucket_object_lock_configuration" "example" { + bucket = aws_s3_bucket.example.id +} + +resource "aws_s3_bucket_versioning" "example" { + bucket = aws_s3_bucket.example.id + versioning_configuration { + status = "Enabled" + } +} +` + modules := tftestutil.CreateModulesFromSource(t, source, ".tf") + + s3 := Adapt(modules) + + assert.Equal(t, 1, len(s3.Buckets)) + assert.True(t, s3.Buckets[0].Versioning.Enabled.Value()) + +} + func Test_BucketGetEncryption(t *testing.T) { source := `