TrueCloudLab/frostfs-s3-gw
17
3
Fork 19
Code Issues 28 Pull requests 7 Projects Releases 18 Packages Activity Actions

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
Conversation 0 Commits 3 Files changed 3 +173 -46
3 changed files with 173 additions and 46 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

1
api/data/lifecycle.go
View file

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

65
api/handler/lifecycle.go
View file

@ -17,6 +17,7 @@ import (
"git.frostfs.info/TrueCloudLab/frostfs-s3-gw/api/middleware"
"git.frostfs.info/TrueCloudLab/frostfs-s3-gw/internal/frostfs/util"
"git.frostfs.info/TrueCloudLab/frostfs-sdk-go/netmap"
"github.com/google/uuid"
)
const (
@ -97,7 +98,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 +141,67 @@ 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)
if 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{}{}
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,20 +214,29 @@ 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))
}
}
if err := checkLifecycleRuleFilter(rule.Filter); err != nil {
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
@ -239,9 +258,14 @@ func checkLifecycleRuleFilter(filter *data.LifecycleRuleFilter) error {
}
}
if filter.And.ObjectSizeGreaterThan != nil && filter.And.ObjectSizeLessThan != nil &&
*filter.And.ObjectSizeLessThan <= *filter.And.ObjectSizeGreaterThan {
return fmt.Errorf("the maximum object size must be larger than the minimum object size")
if filter.And.ObjectSizeLessThan != nil {
if *filter.And.ObjectSizeLessThan == 0 {
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 == 0 {
return fmt.Errorf("%w: the maximum object size must be more than 0", apierr.GetAPIError(apierr.ErrInvalidRequest))
}
fields++
}
@ -266,7 +293,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

149
api/handler/lifecycle_test.go
View file

@ -29,17 +29,14 @@ func TestPutBucketLifecycleConfiguration(t *testing.T) {
for _, tc := range []struct {
name string
body *data.LifecycleConfiguration
error bool
errorCode apierr.ErrorCode
dkirillov marked this conversation as resolved Outdated
dkirillov commented 2024-11-11 07:42:17 +00:00
Outdated
Review
Copy link

Let's rename this to errorCode or change type of field to error

Let's rename this to `errorCode` or change type of field to `error`
👀 1
}{
{
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 {
putBucketLifecycleConfigurationErr(hc, bktName, tc.body, apierr.GetAPIError(apierr.ErrMalformedXML))
if tc.errorCode > 0 {
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))
@ -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) {
hc := prepareHandlerContext(t)