[#536] Fix error codes in lifecycle configuration check

Signed-off-by: Marina Biryukova <m.biryukova@yadro.com>

api/handler/lifecycle.go

@@ -97,7 +97,7 @@ func (h *handler) PutBucketLifecycleHandler(w http.ResponseWriter, r *http.Reque
 	}
 
 	if err = checkLifecycleConfiguration(ctx, cfg, &networkInfo); err != nil {
-		h.logAndSendError(ctx, w, "invalid lifecycle configuration", reqInfo, fmt.Errorf("%w: %s", apierr.GetAPIError(apierr.ErrMalformedXML), err.Error()))
+		h.logAndSendError(ctx, w, "invalid lifecycle configuration", reqInfo, err)
 		return
 	}
 
@@ -140,58 +140,58 @@ func checkLifecycleConfiguration(ctx context.Context, cfg *data.LifecycleConfigu
 	now := layer.TimeNow(ctx)
 
 	if len(cfg.Rules) > maxRules {
-		return fmt.Errorf("number of rules cannot be greater than %d", maxRules)
+		return fmt.Errorf("%w: number of rules cannot be greater than %d", apierr.GetAPIError(apierr.ErrInvalidRequest), maxRules)
 	}
 
 	ids := make(map[string]struct{}, len(cfg.Rules))
 	for i, rule := range cfg.Rules {
 		if _, ok := ids[rule.ID]; ok && rule.ID != "" {
-			return fmt.Errorf("duplicate 'ID': %s", rule.ID)
+			return fmt.Errorf("%w: duplicate 'ID': %s", apierr.GetAPIError(apierr.ErrInvalidArgument), rule.ID)
 		}
 		ids[rule.ID] = struct{}{}
 
 		if len(rule.ID) > maxRuleIDLen {
-			return fmt.Errorf("'ID' value cannot be longer than %d characters", maxRuleIDLen)
+			return fmt.Errorf("%w: 'ID' value cannot be longer than %d characters", apierr.GetAPIError(apierr.ErrInvalidArgument), maxRuleIDLen)
 		}
 
 		if rule.Status != data.LifecycleStatusEnabled && rule.Status != data.LifecycleStatusDisabled {
-			return fmt.Errorf("invalid lifecycle status: %s", rule.Status)
+			return fmt.Errorf("%w: invalid lifecycle status: %s", apierr.GetAPIError(apierr.ErrMalformedXML), rule.Status)
 		}
 
 		if rule.AbortIncompleteMultipartUpload == nil && rule.Expiration == nil && rule.NonCurrentVersionExpiration == nil {
-			return fmt.Errorf("at least one action needs to be specified in a rule")
+			return fmt.Errorf("%w: at least one action needs to be specified in a rule", apierr.GetAPIError(apierr.ErrInvalidRequest))
 		}
 
 		if rule.AbortIncompleteMultipartUpload != nil {
 			if rule.AbortIncompleteMultipartUpload.DaysAfterInitiation != nil &&
 				*rule.AbortIncompleteMultipartUpload.DaysAfterInitiation <= 0 {
-				return fmt.Errorf("days after initiation must be a positive integer: %d", *rule.AbortIncompleteMultipartUpload.DaysAfterInitiation)
+				return fmt.Errorf("%w: days after initiation must be a positive integer", apierr.GetAPIError(apierr.ErrInvalidArgument))
 			}
 
 			if rule.Filter != nil && (rule.Filter.Tag != nil || (rule.Filter.And != nil && len(rule.Filter.And.Tags) > 0)) {
-				return fmt.Errorf("abort incomplete multipart upload cannot be specified with tags")
+				return fmt.Errorf("%w: abort incomplete multipart upload cannot be specified with tags", apierr.GetAPIError(apierr.ErrInvalidRequest))
 			}
 		}
 
 		if rule.Expiration != nil {
 			if rule.Expiration.ExpiredObjectDeleteMarker != nil {
 				if rule.Expiration.Days != nil || rule.Expiration.Date != "" {
-					return fmt.Errorf("expired object delete marker cannot be specified with days or date")
+					return fmt.Errorf("%w: expired object delete marker cannot be specified with days or date", apierr.GetAPIError(apierr.ErrMalformedXML))
 				}
 
 				if rule.Filter != nil && (rule.Filter.Tag != nil || (rule.Filter.And != nil && len(rule.Filter.And.Tags) > 0)) {
-					return fmt.Errorf("expired object delete marker cannot be specified with tags")
+					return fmt.Errorf("%w: expired object delete marker cannot be specified with tags", apierr.GetAPIError(apierr.ErrInvalidRequest))
 				}
 			}
 
 			if rule.Expiration.Days != nil && *rule.Expiration.Days <= 0 {
-				return fmt.Errorf("expiration days must be a positive integer: %d", *rule.Expiration.Days)
+				return fmt.Errorf("%w: expiration days must be a positive integer", apierr.GetAPIError(apierr.ErrInvalidArgument))
 			}
 
 			if rule.Expiration.Date != "" {
 				parsedTime, err := time.Parse("2006-01-02T15:04:05Z", rule.Expiration.Date)
 				if err != nil {
-					return fmt.Errorf("invalid value of expiration date: %s", rule.Expiration.Date)
+					return fmt.Errorf("%w: invalid value of expiration date: %s", apierr.GetAPIError(apierr.ErrInvalidArgument), rule.Expiration.Date)
 				}
 
 				epoch, err := util.TimeToEpoch(ni, now, parsedTime)
@@ -204,14 +204,19 @@ func checkLifecycleConfiguration(ctx context.Context, cfg *data.LifecycleConfigu
 		}
 
 		if rule.NonCurrentVersionExpiration != nil {
+			if rule.NonCurrentVersionExpiration.NewerNonCurrentVersions != nil && rule.NonCurrentVersionExpiration.NonCurrentDays == nil {
+				return fmt.Errorf("%w: newer noncurrent versions cannot be specified without noncurrent days", apierr.GetAPIError(apierr.ErrMalformedXML))
+			}
+
 			if rule.NonCurrentVersionExpiration.NewerNonCurrentVersions != nil &&
 				(*rule.NonCurrentVersionExpiration.NewerNonCurrentVersions > maxNewerNoncurrentVersions ||
 					*rule.NonCurrentVersionExpiration.NewerNonCurrentVersions <= 0) {
-				return fmt.Errorf("invalid value of newer noncurrent versions: %d", *rule.NonCurrentVersionExpiration.NewerNonCurrentVersions)
+				return fmt.Errorf("%w: newer noncurrent versions must be a positive integer up to %d", apierr.GetAPIError(apierr.ErrInvalidArgument),
+					maxNewerNoncurrentVersions)
 			}
 
 			if rule.NonCurrentVersionExpiration.NonCurrentDays != nil && *rule.NonCurrentVersionExpiration.NonCurrentDays <= 0 {
-				return fmt.Errorf("invalid value of noncurrent days: %d", *rule.NonCurrentVersionExpiration.NonCurrentDays)
+				return fmt.Errorf("%w: noncurrent days must be a positive integer", apierr.GetAPIError(apierr.ErrInvalidArgument))
 			}
 		}
 
@@ -243,9 +248,14 @@ func checkLifecycleRuleFilter(filter *data.LifecycleRuleFilter) error {
 			}
 		}
 
-		if filter.And.ObjectSizeGreaterThan != nil && filter.And.ObjectSizeLessThan != nil &&
+		if filter.And.ObjectSizeLessThan != nil {
-			*filter.And.ObjectSizeLessThan <= *filter.And.ObjectSizeGreaterThan {
+			if *filter.And.ObjectSizeLessThan == 0 {
-			return fmt.Errorf("the maximum object size must be larger than the minimum object size")
+				return fmt.Errorf("%w: the maximum object size must be more than 0", apierr.GetAPIError(apierr.ErrInvalidRequest))
+			}
+
+			if filter.And.ObjectSizeGreaterThan != nil && *filter.And.ObjectSizeLessThan <= *filter.And.ObjectSizeGreaterThan {
+				return fmt.Errorf("%w: the maximum object size must be larger than the minimum object size", apierr.GetAPIError(apierr.ErrInvalidRequest))
+			}
 		}
 	}
 
@@ -254,6 +264,9 @@ func checkLifecycleRuleFilter(filter *data.LifecycleRuleFilter) error {
 	}
 
 	if filter.ObjectSizeLessThan != nil {
+		if *filter.ObjectSizeLessThan == 0 {
+			return fmt.Errorf("%w: the maximum object size must be more than 0", apierr.GetAPIError(apierr.ErrInvalidRequest))
+		}
 		fields++
 	}
 
@@ -270,7 +283,7 @@ func checkLifecycleRuleFilter(filter *data.LifecycleRuleFilter) error {
 	}
 
 	if fields > 1 {
-		return fmt.Errorf("filter cannot have more than one field")
+		return fmt.Errorf("%w: filter cannot have more than one field", apierr.GetAPIError(apierr.ErrMalformedXML))
 	}
 
 	return nil

api/handler/lifecycle_test.go

@@ -27,19 +27,16 @@ func TestPutBucketLifecycleConfiguration(t *testing.T) {
 	createBucket(hc, bktName)
 
 	for _, tc := range []struct {
-		name  string
+		name      string
-		body  *data.LifecycleConfiguration
+		body      *data.LifecycleConfiguration
-		error bool
+		errorCode apierr.ErrorCode
 	}{
 		{
 			name: "correct configuration",
 			body: &data.LifecycleConfiguration{
-				XMLName: xml.Name{
-					Space: `http://s3.amazonaws.com/doc/2006-03-01/`,
-					Local: "LifecycleConfiguration",
-				},
 				Rules: []data.LifecycleRule{
 					{
+						ID:     "rule-1",
 						Status: data.LifecycleStatusEnabled,
 						Expiration: &data.LifecycleExpiration{
 							Days: ptr(21),
@@ -54,6 +51,7 @@ func TestPutBucketLifecycleConfiguration(t *testing.T) {
 						},
 					},
 					{
+						ID:     "rule-2",
 						Status: data.LifecycleStatusEnabled,
 						AbortIncompleteMultipartUpload: &data.AbortIncompleteMultipartUpload{
 							DaysAfterInitiation: ptr(14),
@@ -83,7 +81,7 @@ func TestPutBucketLifecycleConfiguration(t *testing.T) {
 				}
 				return lifecycle
 			}(),
-			error: true,
+			errorCode: apierr.ErrInvalidRequest,
 		},
 		{
 			name: "duplicate rule ID",
@@ -105,7 +103,7 @@ func TestPutBucketLifecycleConfiguration(t *testing.T) {
 					},
 				},
 			},
-			error: true,
+			errorCode: apierr.ErrInvalidArgument,
 		},
 		{
 			name: "too long rule ID",
@@ -121,7 +119,7 @@ func TestPutBucketLifecycleConfiguration(t *testing.T) {
 					},
 				}
 			}(),
-			error: true,
+			errorCode: apierr.ErrInvalidArgument,
 		},
 		{
 			name: "invalid status",
@@ -132,7 +130,7 @@ func TestPutBucketLifecycleConfiguration(t *testing.T) {
 					},
 				},
 			},
-			error: true,
+			errorCode: apierr.ErrMalformedXML,
 		},
 		{
 			name: "no actions",
@@ -146,7 +144,7 @@ func TestPutBucketLifecycleConfiguration(t *testing.T) {
 					},
 				},
 			},
-			error: true,
+			errorCode: apierr.ErrInvalidRequest,
 		},
 		{
 			name: "invalid days after initiation",
@@ -160,7 +158,7 @@ func TestPutBucketLifecycleConfiguration(t *testing.T) {
 					},
 				},
 			},
-			error: true,
+			errorCode: apierr.ErrInvalidArgument,
 		},
 		{
 			name: "invalid expired object delete marker declaration",
@@ -175,7 +173,7 @@ func TestPutBucketLifecycleConfiguration(t *testing.T) {
 					},
 				},
 			},
-			error: true,
+			errorCode: apierr.ErrMalformedXML,
 		},
 		{
 			name: "invalid expiration days",
@@ -189,7 +187,7 @@ func TestPutBucketLifecycleConfiguration(t *testing.T) {
 					},
 				},
 			},
-			error: true,
+			errorCode: apierr.ErrInvalidArgument,
 		},
 		{
 			name: "invalid expiration date",
@@ -203,7 +201,7 @@ func TestPutBucketLifecycleConfiguration(t *testing.T) {
 					},
 				},
 			},
-			error: true,
+			errorCode: apierr.ErrInvalidArgument,
 		},
 		{
 			name: "newer noncurrent versions is too small",
@@ -212,12 +210,13 @@ func TestPutBucketLifecycleConfiguration(t *testing.T) {
 					{
 						Status: data.LifecycleStatusEnabled,
 						NonCurrentVersionExpiration: &data.NonCurrentVersionExpiration{
+							NonCurrentDays:          ptr(1),
 							NewerNonCurrentVersions: ptr(0),
 						},
 					},
 				},
 			},
-			error: true,
+			errorCode: apierr.ErrInvalidArgument,
 		},
 		{
 			name: "newer noncurrent versions is too large",
@@ -226,12 +225,13 @@ func TestPutBucketLifecycleConfiguration(t *testing.T) {
 					{
 						Status: data.LifecycleStatusEnabled,
 						NonCurrentVersionExpiration: &data.NonCurrentVersionExpiration{
+							NonCurrentDays:          ptr(1),
 							NewerNonCurrentVersions: ptr(101),
 						},
 					},
 				},
 			},
-			error: true,
+			errorCode: apierr.ErrInvalidArgument,
 		},
 		{
 			name: "invalid noncurrent days",
@@ -245,7 +245,7 @@ func TestPutBucketLifecycleConfiguration(t *testing.T) {
 					},
 				},
 			},
-			error: true,
+			errorCode: apierr.ErrInvalidArgument,
 		},
 		{
 			name: "more than one filter field",
@@ -263,7 +263,7 @@ func TestPutBucketLifecycleConfiguration(t *testing.T) {
 					},
 				},
 			},
-			error: true,
+			errorCode: apierr.ErrMalformedXML,
 		},
 		{
 			name: "invalid tag in filter",
@@ -280,7 +280,7 @@ func TestPutBucketLifecycleConfiguration(t *testing.T) {
 					},
 				},
 			},
-			error: true,
+			errorCode: apierr.ErrInvalidTagKey,
 		},
 		{
 			name: "abort incomplete multipart upload with tag",
@@ -297,7 +297,7 @@ func TestPutBucketLifecycleConfiguration(t *testing.T) {
 					},
 				},
 			},
-			error: true,
+			errorCode: apierr.ErrInvalidRequest,
 		},
 		{
 			name: "expired object delete marker with tag",
@@ -316,7 +316,7 @@ func TestPutBucketLifecycleConfiguration(t *testing.T) {
 					},
 				},
 			},
-			error: true,
+			errorCode: apierr.ErrInvalidRequest,
 		},
 		{
 			name: "invalid size range",
@@ -336,19 +336,88 @@ func TestPutBucketLifecycleConfiguration(t *testing.T) {
 					},
 				},
 			},
-			error: true,
+			errorCode: apierr.ErrInvalidRequest,
+		},
+		{
+			name: "two prefixes",
+			body: &data.LifecycleConfiguration{
+				Rules: []data.LifecycleRule{
+					{
+						Status: data.LifecycleStatusEnabled,
+						Expiration: &data.LifecycleExpiration{
+							Days: ptr(21),
+						},
+						Filter: &data.LifecycleRuleFilter{
+							Prefix: "prefix-1/",
+						},
+						Prefix: "prefix-2/",
+					},
+				},
+			},
+			errorCode: apierr.ErrMalformedXML,
+		},
+		{
+			name: "newer noncurrent versions without noncurrent days",
+			body: &data.LifecycleConfiguration{
+				Rules: []data.LifecycleRule{
+					{
+						Status: data.LifecycleStatusEnabled,
+						NonCurrentVersionExpiration: &data.NonCurrentVersionExpiration{
+							NewerNonCurrentVersions: ptr(10),
+						},
+					},
+				},
+			},
+			errorCode: apierr.ErrMalformedXML,
+		},
+		{
+			name: "invalid maximum object size in filter",
+			body: &data.LifecycleConfiguration{
+				Rules: []data.LifecycleRule{
+					{
+						Status: data.LifecycleStatusEnabled,
+						Expiration: &data.LifecycleExpiration{
+							Days: ptr(21),
+						},
+						Filter: &data.LifecycleRuleFilter{
+							ObjectSizeLessThan: ptr(uint64(0)),
+						},
+					},
+				},
+			},
+			errorCode: apierr.ErrInvalidRequest,
+		},
+		{
+			name: "invalid maximum object size in filter and",
+			body: &data.LifecycleConfiguration{
+				Rules: []data.LifecycleRule{
+					{
+						Status: data.LifecycleStatusEnabled,
+						Expiration: &data.LifecycleExpiration{
+							Days: ptr(21),
+						},
+						Filter: &data.LifecycleRuleFilter{
+							And: &data.LifecycleRuleAndOperator{
+								Prefix:             "prefix/",
+								ObjectSizeLessThan: ptr(uint64(0)),
+							},
+						},
+					},
+				},
+			},
+			errorCode: apierr.ErrInvalidRequest,
 		},
 	} {
 		t.Run(tc.name, func(t *testing.T) {
-			if tc.error {
+			if tc.errorCode > 0 {
-				putBucketLifecycleConfigurationErr(hc, bktName, tc.body, apierr.GetAPIError(apierr.ErrMalformedXML))
+				putBucketLifecycleConfigurationErr(hc, bktName, tc.body, apierr.GetAPIError(tc.errorCode))
 				return
 			}
 
 			putBucketLifecycleConfiguration(hc, bktName, tc.body)
 
 			cfg := getBucketLifecycleConfiguration(hc, bktName)
-			require.Equal(t, *tc.body, *cfg)
+			require.Equal(t, tc.body.Rules, cfg.Rules)
 
 			deleteBucketLifecycleConfiguration(hc, bktName)
 			getBucketLifecycleConfigurationErr(hc, bktName, apierr.GetAPIError(apierr.ErrNoSuchLifecycleConfiguration))