From 847cb28c81d6e0c67b45dec6e1f985b376ee5d6a Mon Sep 17 00:00:00 2001 From: Denis Kirillov Date: Tue, 16 Jul 2024 17:36:52 +0300 Subject: [PATCH] [#431] tree: Fix multipart having system name Previously if multipart key has the same name as some system node (e.g. bucket-settings, bucket-cors etc.) it shadows real system node and bucket started to be unversioned again for example. Signed-off-by: Denis Kirillov --- api/handler/delete_test.go | 10 ++++++ api/handler/multipart_upload_test.go | 13 +++++++ pkg/service/tree/tree.go | 54 ++++++++++++++++------------ 3 files changed, 55 insertions(+), 22 deletions(-) diff --git a/api/handler/delete_test.go b/api/handler/delete_test.go index 55cba0d..362a29a 100644 --- a/api/handler/delete_test.go +++ b/api/handler/delete_test.go @@ -458,6 +458,16 @@ func putBucketVersioning(t *testing.T, tc *handlerContext, bktName string, enabl assertStatus(t, w, http.StatusOK) } +func getBucketVersioning(hc *handlerContext, bktName string) *VersioningConfiguration { + w, r := prepareTestRequest(hc, bktName, "", nil) + hc.Handler().GetBucketVersioningHandler(w, r) + assertStatus(hc.t, w, http.StatusOK) + + res := &VersioningConfiguration{} + parseTestResponse(hc.t, w, res) + return res +} + func deleteObject(t *testing.T, tc *handlerContext, bktName, objName, version string) (string, bool) { query := make(url.Values) query.Add(api.QueryVersionID, version) diff --git a/api/handler/multipart_upload_test.go b/api/handler/multipart_upload_test.go index 5fdbe24..d0e6c85 100644 --- a/api/handler/multipart_upload_test.go +++ b/api/handler/multipart_upload_test.go @@ -68,6 +68,19 @@ func TestDeleteMultipartAllParts(t *testing.T) { require.Empty(t, hc.tp.Objects()) } +func TestSpecialMultipartName(t *testing.T) { + hc := prepareHandlerContextWithMinCache(t) + + bktName, objName := "bucket", "bucket-settings" + + createTestBucket(hc, bktName) + putBucketVersioning(t, hc, bktName, true) + + createMultipartUpload(hc, bktName, objName, nil) + res := getBucketVersioning(hc, bktName) + require.Equal(t, enabledValue, res.Status) +} + func TestMultipartReUploadPart(t *testing.T) { hc := prepareHandlerContext(t) diff --git a/pkg/service/tree/tree.go b/pkg/service/tree/tree.go index 1fa9809..3f390cb 100644 --- a/pkg/service/tree/tree.go +++ b/pkg/service/tree/tree.go @@ -395,8 +395,7 @@ func newPartInfo(node NodeResponse) (*data.PartInfo, error) { } func (c *Tree) GetSettingsNode(ctx context.Context, bktInfo *data.BucketInfo) (*data.BucketSettings, error) { - keysToReturn := []string{versioningKV, lockConfigurationKV, cannedACLKV, ownerKeyKV} - node, err := c.getSystemNode(ctx, bktInfo, []string{settingsFileName}, keysToReturn) + node, err := c.getSystemNode(ctx, bktInfo, []string{settingsFileName}) if err != nil { return nil, fmt.Errorf("couldn't get node: %w", err) } @@ -424,7 +423,7 @@ func (c *Tree) GetSettingsNode(ctx context.Context, bktInfo *data.BucketInfo) (* } func (c *Tree) PutSettingsNode(ctx context.Context, bktInfo *data.BucketInfo, settings *data.BucketSettings) error { - node, err := c.getSystemNode(ctx, bktInfo, []string{settingsFileName}, []string{}) + node, err := c.getSystemNode(ctx, bktInfo, []string{settingsFileName}) isErrNotFound := errors.Is(err, layer.ErrNodeNotFound) if err != nil && !isErrNotFound { return fmt.Errorf("couldn't get node: %w", err) @@ -446,7 +445,7 @@ func (c *Tree) PutSettingsNode(ctx context.Context, bktInfo *data.BucketInfo, se } func (c *Tree) GetNotificationConfigurationNode(ctx context.Context, bktInfo *data.BucketInfo) (oid.ID, error) { - node, err := c.getSystemNode(ctx, bktInfo, []string{notifConfFileName}, []string{oidKV}) + node, err := c.getSystemNode(ctx, bktInfo, []string{notifConfFileName}) if err != nil { return oid.ID{}, err } @@ -455,7 +454,7 @@ func (c *Tree) GetNotificationConfigurationNode(ctx context.Context, bktInfo *da } func (c *Tree) PutNotificationConfigurationNode(ctx context.Context, bktInfo *data.BucketInfo, objID oid.ID) (oid.ID, error) { - node, err := c.getSystemNode(ctx, bktInfo, []string{notifConfFileName}, []string{oidKV}) + node, err := c.getSystemNode(ctx, bktInfo, []string{notifConfFileName}) isErrNotFound := errors.Is(err, layer.ErrNodeNotFound) if err != nil && !isErrNotFound { return oid.ID{}, fmt.Errorf("couldn't get node: %w", err) @@ -481,7 +480,7 @@ func (c *Tree) PutNotificationConfigurationNode(ctx context.Context, bktInfo *da } func (c *Tree) GetBucketCORS(ctx context.Context, bktInfo *data.BucketInfo) (oid.ID, error) { - node, err := c.getSystemNode(ctx, bktInfo, []string{corsFilename}, []string{oidKV}) + node, err := c.getSystemNode(ctx, bktInfo, []string{corsFilename}) if err != nil { return oid.ID{}, err } @@ -490,7 +489,7 @@ func (c *Tree) GetBucketCORS(ctx context.Context, bktInfo *data.BucketInfo) (oid } func (c *Tree) PutBucketCORS(ctx context.Context, bktInfo *data.BucketInfo, objID oid.ID) (oid.ID, error) { - node, err := c.getSystemNode(ctx, bktInfo, []string{corsFilename}, []string{oidKV}) + node, err := c.getSystemNode(ctx, bktInfo, []string{corsFilename}) isErrNotFound := errors.Is(err, layer.ErrNodeNotFound) if err != nil && !isErrNotFound { return oid.ID{}, fmt.Errorf("couldn't get node: %w", err) @@ -516,7 +515,7 @@ func (c *Tree) PutBucketCORS(ctx context.Context, bktInfo *data.BucketInfo, objI } func (c *Tree) DeleteBucketCORS(ctx context.Context, bktInfo *data.BucketInfo) (oid.ID, error) { - node, err := c.getSystemNode(ctx, bktInfo, []string{corsFilename}, []string{oidKV}) + node, err := c.getSystemNode(ctx, bktInfo, []string{corsFilename}) if err != nil && !errors.Is(err, layer.ErrNodeNotFound) { return oid.ID{}, err } @@ -589,7 +588,7 @@ func (c *Tree) DeleteObjectTagging(ctx context.Context, bktInfo *data.BucketInfo } func (c *Tree) GetBucketTagging(ctx context.Context, bktInfo *data.BucketInfo) (map[string]string, error) { - node, err := c.getSystemNodeWithAllAttributes(ctx, bktInfo, []string{bucketTaggingFilename}) + node, err := c.getSystemNode(ctx, bktInfo, []string{bucketTaggingFilename}) if err != nil { return nil, err } @@ -606,7 +605,7 @@ func (c *Tree) GetBucketTagging(ctx context.Context, bktInfo *data.BucketInfo) ( } func (c *Tree) PutBucketTagging(ctx context.Context, bktInfo *data.BucketInfo, tagSet map[string]string) error { - node, err := c.getSystemNode(ctx, bktInfo, []string{bucketTaggingFilename}, []string{}) + node, err := c.getSystemNode(ctx, bktInfo, []string{bucketTaggingFilename}) isErrNotFound := errors.Is(err, layer.ErrNodeNotFound) if err != nil && !isErrNotFound { return fmt.Errorf("couldn't get node: %w", err) @@ -1595,27 +1594,21 @@ func metaFromMultipart(info *data.MultipartInfo, fileName string) map[string]str return info.Meta } -func (c *Tree) getSystemNode(ctx context.Context, bktInfo *data.BucketInfo, path, meta []string) (*treeNode, error) { - return c.getNode(ctx, bktInfo, systemTree, path, meta, false) -} - -func (c *Tree) getSystemNodeWithAllAttributes(ctx context.Context, bktInfo *data.BucketInfo, path []string) (*treeNode, error) { - return c.getNode(ctx, bktInfo, systemTree, path, []string{}, true) -} - -func (c *Tree) getNode(ctx context.Context, bktInfo *data.BucketInfo, treeID string, path, meta []string, allAttrs bool) (*treeNode, error) { +func (c *Tree) getSystemNode(ctx context.Context, bktInfo *data.BucketInfo, path []string) (*treeNode, error) { p := &GetNodesParams{ BktInfo: bktInfo, - TreeID: treeID, + TreeID: systemTree, Path: path, - Meta: meta, LatestOnly: false, - AllAttrs: allAttrs, + AllAttrs: true, } nodes, err := c.service.GetNodes(ctx, p) if err != nil { return nil, err } + + nodes = filterMultipartNodes(nodes) + if len(nodes) == 0 { return nil, layer.ErrNodeNotFound } @@ -1626,6 +1619,23 @@ func (c *Tree) getNode(ctx context.Context, bktInfo *data.BucketInfo, treeID str return newTreeNode(getLatestNode(nodes)) } +func filterMultipartNodes(nodes []NodeResponse) []NodeResponse { + res := make([]NodeResponse, 0, len(nodes)) + +LOOP: + for _, node := range nodes { + for _, meta := range node.GetMeta() { + if meta.GetKey() == uploadIDKV { + continue LOOP + } + } + + res = append(res, node) + } + + return res +} + func (c *Tree) reqLogger(ctx context.Context) *zap.Logger { reqLogger := middleware.GetReqLog(ctx) if reqLogger != nil {