bugfix/lifecycle #544

Merged
alexvanin merged 3 commits from mbiryukova/frostfs-s3-gw:bugfix/lifecycle into master 2024-11-12 12:51:03 +00:00
3 changed files with 173 additions and 46 deletions

View file

@ -20,6 +20,7 @@ type (
Filter *LifecycleRuleFilter `xml:"Filter,omitempty"` Filter *LifecycleRuleFilter `xml:"Filter,omitempty"`
ID string `xml:"ID,omitempty"` ID string `xml:"ID,omitempty"`
NonCurrentVersionExpiration *NonCurrentVersionExpiration `xml:"NoncurrentVersionExpiration,omitempty"` NonCurrentVersionExpiration *NonCurrentVersionExpiration `xml:"NoncurrentVersionExpiration,omitempty"`
Prefix string `xml:"Prefix,omitempty"`
} }
AbortIncompleteMultipartUpload struct { AbortIncompleteMultipartUpload struct {

View file

@ -17,6 +17,7 @@ import (
"git.frostfs.info/TrueCloudLab/frostfs-s3-gw/api/middleware" "git.frostfs.info/TrueCloudLab/frostfs-s3-gw/api/middleware"
"git.frostfs.info/TrueCloudLab/frostfs-s3-gw/internal/frostfs/util" "git.frostfs.info/TrueCloudLab/frostfs-s3-gw/internal/frostfs/util"
"git.frostfs.info/TrueCloudLab/frostfs-sdk-go/netmap" "git.frostfs.info/TrueCloudLab/frostfs-sdk-go/netmap"
"github.com/google/uuid"
) )
const ( const (
@ -97,7 +98,7 @@ func (h *handler) PutBucketLifecycleHandler(w http.ResponseWriter, r *http.Reque
} }
if err = checkLifecycleConfiguration(ctx, cfg, &networkInfo); err != nil { 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 return
} }
@ -140,58 +141,67 @@ func checkLifecycleConfiguration(ctx context.Context, cfg *data.LifecycleConfigu
now := layer.TimeNow(ctx) now := layer.TimeNow(ctx)
if len(cfg.Rules) > maxRules { 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)) ids := make(map[string]struct{}, len(cfg.Rules))
for i, rule := range cfg.Rules { for i, rule := range cfg.Rules {
if _, ok := ids[rule.ID]; ok && rule.ID != "" { if rule.ID == "" {
return fmt.Errorf("duplicate 'ID': %s", rule.ID) id, err := uuid.NewRandom()
if err != nil {
return fmt.Errorf("generate uuid: %w", err)
}
cfg.Rules[i].ID = id.String()
rule.ID = id.String()
}
if _, ok := ids[rule.ID]; ok {
return fmt.Errorf("%w: duplicate 'ID': %s", apierr.GetAPIError(apierr.ErrInvalidArgument), rule.ID)
} }
ids[rule.ID] = struct{}{} ids[rule.ID] = struct{}{}
if len(rule.ID) > maxRuleIDLen { 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 { 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 { 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 != nil {
if rule.AbortIncompleteMultipartUpload.DaysAfterInitiation != nil && if rule.AbortIncompleteMultipartUpload.DaysAfterInitiation != nil &&
*rule.AbortIncompleteMultipartUpload.DaysAfterInitiation <= 0 { *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)) { 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 != nil {
if rule.Expiration.ExpiredObjectDeleteMarker != nil { if rule.Expiration.ExpiredObjectDeleteMarker != nil {
if rule.Expiration.Days != nil || rule.Expiration.Date != "" { 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)) { 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 { 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 != "" { if rule.Expiration.Date != "" {
parsedTime, err := time.Parse("2006-01-02T15:04:05Z", rule.Expiration.Date) parsedTime, err := time.Parse("2006-01-02T15:04:05Z", rule.Expiration.Date)
if err != nil { 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) epoch, err := util.TimeToEpoch(ni, now, parsedTime)
@ -204,20 +214,29 @@ func checkLifecycleConfiguration(ctx context.Context, cfg *data.LifecycleConfigu
} }
if rule.NonCurrentVersionExpiration != nil { 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 && if rule.NonCurrentVersionExpiration.NewerNonCurrentVersions != nil &&
(*rule.NonCurrentVersionExpiration.NewerNonCurrentVersions > maxNewerNoncurrentVersions || (*rule.NonCurrentVersionExpiration.NewerNonCurrentVersions > maxNewerNoncurrentVersions ||
*rule.NonCurrentVersionExpiration.NewerNonCurrentVersions <= 0) { *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 { 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))
} }
} }
if err := checkLifecycleRuleFilter(rule.Filter); err != nil { if err := checkLifecycleRuleFilter(rule.Filter); err != nil {
return err return err
} }
if rule.Filter != nil && rule.Filter.Prefix != "" && rule.Prefix != "" {
return fmt.Errorf("%w: rule cannot have two prefixes", apierr.GetAPIError(apierr.ErrMalformedXML))
}
} }
return nil return nil
@ -239,9 +258,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))
}
} }
} }
@ -250,6 +274,9 @@ func checkLifecycleRuleFilter(filter *data.LifecycleRuleFilter) error {
} }
if filter.ObjectSizeLessThan != nil { 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++ fields++
} }
@ -266,7 +293,7 @@ func checkLifecycleRuleFilter(filter *data.LifecycleRuleFilter) error {
} }
if fields > 1 { 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 return nil

View file

@ -27,19 +27,16 @@ func TestPutBucketLifecycleConfiguration(t *testing.T) {
createBucket(hc, bktName) createBucket(hc, bktName)
for _, tc := range []struct { for _, tc := range []struct {
name string name string
body *data.LifecycleConfiguration body *data.LifecycleConfiguration
error bool errorCode apierr.ErrorCode
}{ }{
{ {
name: "correct configuration", name: "correct configuration",
body: &data.LifecycleConfiguration{ body: &data.LifecycleConfiguration{
XMLName: xml.Name{
Space: `http://s3.amazonaws.com/doc/2006-03-01/`,
Local: "LifecycleConfiguration",
},
Rules: []data.LifecycleRule{ Rules: []data.LifecycleRule{
{ {
ID: "rule-1",
Status: data.LifecycleStatusEnabled, Status: data.LifecycleStatusEnabled,
Expiration: &data.LifecycleExpiration{ Expiration: &data.LifecycleExpiration{
Days: ptr(21), Days: ptr(21),
@ -54,6 +51,7 @@ func TestPutBucketLifecycleConfiguration(t *testing.T) {
}, },
}, },
{ {
ID: "rule-2",
Status: data.LifecycleStatusEnabled, Status: data.LifecycleStatusEnabled,
AbortIncompleteMultipartUpload: &data.AbortIncompleteMultipartUpload{ AbortIncompleteMultipartUpload: &data.AbortIncompleteMultipartUpload{
DaysAfterInitiation: ptr(14), DaysAfterInitiation: ptr(14),
@ -83,7 +81,7 @@ func TestPutBucketLifecycleConfiguration(t *testing.T) {
} }
return lifecycle return lifecycle
}(), }(),
error: true, errorCode: apierr.ErrInvalidRequest,
}, },
{ {
name: "duplicate rule ID", name: "duplicate rule ID",
@ -105,7 +103,7 @@ func TestPutBucketLifecycleConfiguration(t *testing.T) {
}, },
}, },
}, },
error: true, errorCode: apierr.ErrInvalidArgument,
}, },
{ {
name: "too long rule ID", name: "too long rule ID",
@ -121,7 +119,7 @@ func TestPutBucketLifecycleConfiguration(t *testing.T) {
}, },
} }
}(), }(),
error: true, errorCode: apierr.ErrInvalidArgument,
}, },
{ {
name: "invalid status", name: "invalid status",
@ -132,7 +130,7 @@ func TestPutBucketLifecycleConfiguration(t *testing.T) {
}, },
}, },
}, },
error: true, errorCode: apierr.ErrMalformedXML,
}, },
{ {
name: "no actions", name: "no actions",
@ -146,7 +144,7 @@ func TestPutBucketLifecycleConfiguration(t *testing.T) {
}, },
}, },
}, },
error: true, errorCode: apierr.ErrInvalidRequest,
}, },
{ {
name: "invalid days after initiation", 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", 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", name: "invalid expiration days",
@ -189,7 +187,7 @@ func TestPutBucketLifecycleConfiguration(t *testing.T) {
}, },
}, },
}, },
error: true, errorCode: apierr.ErrInvalidArgument,
}, },
{ {
name: "invalid expiration date", 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", name: "newer noncurrent versions is too small",
@ -212,12 +210,13 @@ func TestPutBucketLifecycleConfiguration(t *testing.T) {
{ {
Status: data.LifecycleStatusEnabled, Status: data.LifecycleStatusEnabled,
NonCurrentVersionExpiration: &data.NonCurrentVersionExpiration{ NonCurrentVersionExpiration: &data.NonCurrentVersionExpiration{
NonCurrentDays: ptr(1),
NewerNonCurrentVersions: ptr(0), NewerNonCurrentVersions: ptr(0),
}, },
}, },
}, },
}, },
error: true, errorCode: apierr.ErrInvalidArgument,
}, },
{ {
name: "newer noncurrent versions is too large", name: "newer noncurrent versions is too large",
@ -226,12 +225,13 @@ func TestPutBucketLifecycleConfiguration(t *testing.T) {
{ {
Status: data.LifecycleStatusEnabled, Status: data.LifecycleStatusEnabled,
NonCurrentVersionExpiration: &data.NonCurrentVersionExpiration{ NonCurrentVersionExpiration: &data.NonCurrentVersionExpiration{
NonCurrentDays: ptr(1),
NewerNonCurrentVersions: ptr(101), NewerNonCurrentVersions: ptr(101),
}, },
}, },
}, },
}, },
error: true, errorCode: apierr.ErrInvalidArgument,
}, },
{ {
name: "invalid noncurrent days", name: "invalid noncurrent days",
@ -245,7 +245,7 @@ func TestPutBucketLifecycleConfiguration(t *testing.T) {
}, },
}, },
}, },
error: true, errorCode: apierr.ErrInvalidArgument,
}, },
{ {
name: "more than one filter field", 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", 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", 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", 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", 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) { 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 return
} }
putBucketLifecycleConfiguration(hc, bktName, tc.body) putBucketLifecycleConfiguration(hc, bktName, tc.body)
cfg := getBucketLifecycleConfiguration(hc, bktName) cfg := getBucketLifecycleConfiguration(hc, bktName)
require.Equal(t, *tc.body, *cfg) require.Equal(t, tc.body.Rules, cfg.Rules)
deleteBucketLifecycleConfiguration(hc, bktName) deleteBucketLifecycleConfiguration(hc, bktName)
getBucketLifecycleConfigurationErr(hc, bktName, apierr.GetAPIError(apierr.ErrNoSuchLifecycleConfiguration)) getBucketLifecycleConfigurationErr(hc, bktName, apierr.GetAPIError(apierr.ErrNoSuchLifecycleConfiguration))
@ -356,6 +425,36 @@ func TestPutBucketLifecycleConfiguration(t *testing.T) {
} }
} }
func TestPutBucketLifecycleIDGeneration(t *testing.T) {
hc := prepareHandlerContext(t)
bktName := "bucket-lifecycle-id"
createBucket(hc, bktName)
lifecycle := &data.LifecycleConfiguration{
Rules: []data.LifecycleRule{
{
Status: data.LifecycleStatusEnabled,
Expiration: &data.LifecycleExpiration{
Days: ptr(21),
},
},
{
Status: data.LifecycleStatusEnabled,
AbortIncompleteMultipartUpload: &data.AbortIncompleteMultipartUpload{
DaysAfterInitiation: ptr(14),
},
},
},
}
putBucketLifecycleConfiguration(hc, bktName, lifecycle)
cfg := getBucketLifecycleConfiguration(hc, bktName)
require.Len(t, cfg.Rules, 2)
require.NotEmpty(t, cfg.Rules[0].ID)
require.NotEmpty(t, cfg.Rules[1].ID)
}
func TestPutBucketLifecycleInvalidMD5(t *testing.T) { func TestPutBucketLifecycleInvalidMD5(t *testing.T) {
hc := prepareHandlerContext(t) hc := prepareHandlerContext(t)