fix(s3): dont set ExpiredObjectDeleteMarker in lifecycle policy if da…

…ys or date is set Signed-off-by: Kevin Kendzia <[email protected]>
crossplane-contrib · Nov 4, 2024 · 1bf5a43 · 1bf5a43
1 parent 64d009a
commit 1bf5a43
Show file tree

Hide file tree

Showing 2 changed files with 37 additions and 8 deletions.
diff --git a/pkg/controller/s3/bucket/lifecycleConfig.go b/pkg/controller/s3/bucket/lifecycleConfig.go
@@ -161,13 +161,19 @@ func GenerateLifecycleRules(in []v1beta1.LifecycleRule) []types.LifecycleRule {
 			}
 		}
 		if local.Expiration != nil {
-			rule.Expiration = &types.LifecycleExpiration{
-				Days:                      ptr.To(local.Expiration.Days),
-				ExpiredObjectDeleteMarker: ptr.To(local.Expiration.ExpiredObjectDeleteMarker),
+			rule.Expiration = &types.LifecycleExpiration{}
+			if local.Expiration.Days > 0 {
+				rule.Expiration.Days = ptr.To(local.Expiration.Days)
 			}
 			if local.Expiration.Date != nil {
 				rule.Expiration.Date = ptr.To(local.Expiration.Date.Time)
 			}
+			// NOTE(kkendzia): ExpiredObjectDeleteMarker must not be set when Days or Date is specified
+			// This behaviour is in line with terraform provider aws and avoids a MalformedXML Error
+			// When both is set, ExpiredObjectDeleteMarker will be ignored.
+			if local.Expiration.Date == nil && local.Expiration.Days == 0 {
+				rule.Expiration.ExpiredObjectDeleteMarker = ptr.To(local.Expiration.ExpiredObjectDeleteMarker)
+			}
 		}
 		if local.NoncurrentVersionExpiration != nil {
 			rule.NoncurrentVersionExpiration = &types.NoncurrentVersionExpiration{NoncurrentDays: ptr.To(local.NoncurrentVersionExpiration.NoncurrentDays)}

diff --git a/pkg/controller/s3/bucket/lifecycleConfig_test.go b/pkg/controller/s3/bucket/lifecycleConfig_test.go
@@ -44,7 +44,7 @@ var (
 	location, _       = time.LoadLocation("UTC")
 	date              = metav1.Date(2020, time.September, 25, 11, 40, 0, 0, location)
 	awsDate           = time.Date(2020, time.September, 25, 11, 40, 0, 0, location)
-	marker            = false
+	marker            = true
 	prefix            = "test-"
 	id                = "test-id"
 	storage           = "ONEZONE_IA"
@@ -58,9 +58,8 @@ func generateLifecycleConfig() *v1beta1.BucketLifecycleConfiguration {
 			{
 				AbortIncompleteMultipartUpload: &v1beta1.AbortIncompleteMultipartUpload{DaysAfterInitiation: 1},
 				Expiration: &v1beta1.LifecycleExpiration{
-					Date:                      &date,
-					Days:                      days,
-					ExpiredObjectDeleteMarker: marker,
+					Date: &date,
+					Days: days,
 				},
 				Filter: &v1beta1.LifecycleRuleFilter{
 					And: &v1beta1.LifecycleRuleAndOperator{
@@ -85,6 +84,14 @@ func generateLifecycleConfig() *v1beta1.BucketLifecycleConfiguration {
 	}
 }
 
+func generateLifecycleConfigWithMarker() *v1beta1.BucketLifecycleConfiguration {
+	res := generateLifecycleConfig()
+	res.Rules[0].Expiration = &v1beta1.LifecycleExpiration{
+		ExpiredObjectDeleteMarker: marker,
+	}
+	return res
+}
+
 func generateAWSLifecycle(sortTag bool) *s3types.BucketLifecycleConfiguration {
 	conf := &s3types.BucketLifecycleConfiguration{
 		Rules: []s3types.LifecycleRule{
@@ -93,7 +100,7 @@ func generateAWSLifecycle(sortTag bool) *s3types.BucketLifecycleConfiguration {
 				Expiration: &s3types.LifecycleExpiration{
 					Date:                      &awsDate,
 					Days:                      &days,
-					ExpiredObjectDeleteMarker: &marker,
+					ExpiredObjectDeleteMarker: nil,
 				},
 				Filter: &s3types.LifecycleRuleFilterMemberAnd{
 					Value: s3types.LifecycleRuleAndOperator{
@@ -122,6 +129,14 @@ func generateAWSLifecycle(sortTag bool) *s3types.BucketLifecycleConfiguration {
 	return conf
 }
 
+func generateAWSLifecycleWithMarker(sortTag bool) *s3types.BucketLifecycleConfiguration {
+	res := generateAWSLifecycle(sortTag)
+	res.Rules[0].Expiration = &s3types.LifecycleExpiration{
+		ExpiredObjectDeleteMarker: &marker,
+	}
+	return res
+}
+
 func TestGenerateLifecycleConfiguration(t *testing.T) {
 	type args struct {
 		b *v1beta1.Bucket
@@ -143,6 +158,14 @@ func TestGenerateLifecycleConfiguration(t *testing.T) {
 				input: generateAWSLifecycle(true).Rules,
 			},
 		},
+		"Marker": {
+			args: args{
+				b: s3testing.Bucket(s3testing.WithLifecycleConfig(generateLifecycleConfigWithMarker())),
+			},
+			want: want{
+				input: generateAWSLifecycleWithMarker(true).Rules,
+			},
+		},
 	}
 
 	for name, tc := range cases {