From 135fda27796e58214b7e70e6fb111dd48f06b63b Mon Sep 17 00:00:00 2001 From: Christian Richter Date: Wed, 6 Jul 2022 14:19:51 +0200 Subject: [PATCH] implement filter & refactor spacetypes handling Signed-off-by: Christian Richter --- changelog/unreleased/add-user-filter.md | 5 + pkg/storage/fs/owncloudsql/spaces.go | 2 + pkg/storage/registry/spaces/spaces.go | 2 +- pkg/storage/utils/decomposedfs/spaces.go | 126 ++++++++++------- pkg/storage/utils/decomposedfs/spaces_test.go | 2 +- .../utils/decomposedfs/tree/migrations.go | 101 ++++++++++++++ pkg/storage/utils/decomposedfs/tree/tree.go | 130 +++++++++++++----- 7 files changed, 282 insertions(+), 86 deletions(-) create mode 100644 changelog/unreleased/add-user-filter.md create mode 100644 pkg/storage/utils/decomposedfs/tree/migrations.go diff --git a/changelog/unreleased/add-user-filter.md b/changelog/unreleased/add-user-filter.md new file mode 100644 index 00000000000..32df2521648 --- /dev/null +++ b/changelog/unreleased/add-user-filter.md @@ -0,0 +1,5 @@ +Feature: Add user filter + +This PR adds the ability to filter spaces by user-id + +https://github.com/owncloud/ocis/pull/4072 \ No newline at end of file diff --git a/pkg/storage/fs/owncloudsql/spaces.go b/pkg/storage/fs/owncloudsql/spaces.go index 9b07348b9c8..2cc5cad9f9b 100644 --- a/pkg/storage/fs/owncloudsql/spaces.go +++ b/pkg/storage/fs/owncloudsql/spaces.go @@ -48,6 +48,8 @@ func (fs *owncloudsqlfs) ListStorageSpaces(ctx context.Context, filter []*provid filteringUnsupportedSpaceTypes = (t != "personal" && !strings.HasPrefix(t, "+")) case provider.ListStorageSpacesRequest_Filter_TYPE_ID: spaceID, _, _ = storagespace.SplitID(filter[i].GetId().OpaqueId) + case provider.ListStorageSpacesRequest_Filter_TYPE_USER: + spaceID, _, _ = storagespace.SplitID(filter[i].GetId().OpaqueId) } } if filteringUnsupportedSpaceTypes { diff --git a/pkg/storage/registry/spaces/spaces.go b/pkg/storage/registry/spaces/spaces.go index c1f517d6251..373bba6fc85 100644 --- a/pkg/storage/registry/spaces/spaces.go +++ b/pkg/storage/registry/spaces/spaces.go @@ -305,7 +305,7 @@ func (r *registry) buildFilters(filterMap map[string]string) []*providerpb.ListS }) } } - if filterMap["user_id"] != "" && filterMap["user_idp"] != "" { + if filterMap["user_id"] != "" { filters = append(filters, &providerpb.ListStorageSpacesRequest_Filter{ Type: providerpb.ListStorageSpacesRequest_Filter_TYPE_USER, Term: &providerpb.ListStorageSpacesRequest_Filter_User{ diff --git a/pkg/storage/utils/decomposedfs/spaces.go b/pkg/storage/utils/decomposedfs/spaces.go index 680ced16806..766f71e558b 100644 --- a/pkg/storage/utils/decomposedfs/spaces.go +++ b/pkg/storage/utils/decomposedfs/spaces.go @@ -160,7 +160,7 @@ func (fs *Decomposedfs) CreateStorageSpace(ctx context.Context, req *provider.Cr } } - space, err := fs.storageSpaceFromNode(ctx, root, root.InternalPath(), false, false) + space, err := fs.storageSpaceFromNode(ctx, root, false, false) if err != nil { return nil, err } @@ -215,21 +215,17 @@ func (fs *Decomposedfs) canCreateSpace(ctx context.Context, spaceID string) bool return checkRes.Status.Code == v1beta11.Code_CODE_OK } -// ReadSpaceAndNodeFromSpaceTypeLink reads a symlink and parses space and node id if the link has the correct format, eg: +// ReadSpaceAndNodeFromIndexLink reads a symlink and parses space and node id if the link has the correct format, eg: // ../../spaces/4c/510ada-c86b-4815-8820-42cdf82c3d51/nodes/4c/51/0a/da/-c86b-4815-8820-42cdf82c3d51 // ../../spaces/4c/510ada-c86b-4815-8820-42cdf82c3d51/nodes/4c/51/0a/da/-c86b-4815-8820-42cdf82c3d51.T.2022-02-24T12:35:18.196484592Z -func ReadSpaceAndNodeFromSpaceTypeLink(path string) (string, string, error) { - link, err := os.Readlink(path) - if err != nil { - return "", "", err - } - // ../../spaces/sp/ace-id/nodes/sh/or/tn/od/eid - // 0 1 2 3 4 5 6 7 8 9 10 +func ReadSpaceAndNodeFromIndexLink(link string) (string, string, error) { + // ../../../spaces/sp/ace-id/nodes/sh/or/tn/od/eid + // 0 1 2 3 4 5 6 7 8 9 10 11 parts := strings.Split(link, string(filepath.Separator)) - if len(parts) != 11 || parts[0] != ".." || parts[1] != ".." || parts[2] != "spaces" || parts[5] != "nodes" { + if len(parts) != 12 || parts[0] != ".." || parts[1] != ".." || parts[2] != ".." || parts[3] != "spaces" || parts[6] != "nodes" { return "", "", errtypes.InternalError("malformed link") } - return strings.Join(parts[3:5], ""), strings.Join(parts[6:11], ""), nil + return strings.Join(parts[4:6], ""), strings.Join(parts[7:12], ""), nil } // ListStorageSpaces returns a list of StorageSpaces. @@ -250,10 +246,10 @@ func (fs *Decomposedfs) ListStorageSpaces(ctx context.Context, filter []*provide var ( spaceID = spaceIDAny nodeID = spaceIDAny - userID = ctxpkg.ContextMustGetUser(ctx).GetId().GetOpaqueId() + userID = spaceIDAny ) - spaceTypes := []string{} + spaceTypes := map[string]struct{}{} for i := range filter { switch filter[i].Type { @@ -264,25 +260,28 @@ func (fs *Decomposedfs) ListStorageSpaces(ctx context.Context, filter []*provide case "+grant": // TODO include grants default: - spaceTypes = append(spaceTypes, filter[i].GetSpaceType()) + spaceTypes[filter[i].GetSpaceType()] = struct{}{} } case provider.ListStorageSpacesRequest_Filter_TYPE_ID: spaceID, nodeID, _ = storagespace.SplitID(filter[i].GetId().OpaqueId) if strings.Contains(nodeID, "/") { return []*provider.StorageSpace{}, nil } - case provider.ListStorageSpacesRequest_Filter_TYPE_USER: // TODO: refactor this to GetUserId() in cs3 - userID = filter[i].GetUser() + userID = filter[i].GetUser().GetOpaqueId() } } if len(spaceTypes) == 0 { - spaceTypes = []string{spaceTypeAny} + spaceTypes[spaceTypeAny] = struct{}{} } canListAllSpaces := fs.canListAllSpaces(ctx) + if userID != spaceTypeAny && !canListAllSpaces { + return nil, errtypes.PermissionDenied(fmt.Sprintf("user %s is not allowed to list spaces of other users", ctxpkg.ContextMustGetUser(ctx).GetId().GetOpaqueId())) + } + spaces := []*provider.StorageSpace{} // build the glob path, eg. // /path/to/root/spaces/{spaceType}/{spaceId} @@ -300,28 +299,52 @@ func (fs *Decomposedfs) ListStorageSpaces(ctx context.Context, filter []*provide // return empty list return spaces, nil } - space, err := fs.storageSpaceFromNode(ctx, n, n.InternalPath(), canListAllSpaces, unrestricted) + space, err := fs.storageSpaceFromNode(ctx, n, canListAllSpaces, unrestricted) if err != nil { return nil, err } // filter space types - for _, spaceType := range spaceTypes { - if spaceType == spaceTypeAny || spaceType == space.SpaceType { - spaces = append(spaces, space) - } + _, ok1 := spaceTypes[spaceTypeAny] + _, ok2 := spaceTypes[space.SpaceType] + if ok1 || ok2 { + spaces = append(spaces, space) } - + // TODO: filter user id return spaces, nil } - matches := []string{} - for _, spaceType := range spaceTypes { - path := filepath.Join(fs.o.Root, "spacetypes", spaceType, nodeID) + matches := map[string]struct{}{} + + if userID != spaceTypeAny { + path := filepath.Join(fs.o.Root, "indexes", "by-user-id", userID, nodeID) m, err := filepath.Glob(path) if err != nil { return nil, err } - matches = append(matches, m...) + for _, match := range m { + link, err := os.Readlink(match) + if err != nil { + continue + } + matches[link] = struct{}{} + } + } + + if userID == spaceTypeAny { + for spaceType := range spaceTypes { + path := filepath.Join(fs.o.Root, "indexes", "by-type", spaceType, nodeID) + m, err := filepath.Glob(path) + if err != nil { + return nil, err + } + for _, match := range m { + link, err := os.Readlink(match) + if err != nil { + continue + } + matches[link] = struct{}{} + } + } } // FIXME if the space does not exist try a node as the space root. @@ -337,16 +360,16 @@ func (fs *Decomposedfs) ListStorageSpaces(ctx context.Context, filter []*provide numShares := 0 - for i := range matches { + for match := range matches { var err error // do not investigate flock files any further. They indicate file locks but are not relevant here. - if strings.HasSuffix(matches[i], ".flock") { + if strings.HasSuffix(match, ".flock") { continue } // always read link in case storage space id != node id - spaceID, nodeID, err = ReadSpaceAndNodeFromSpaceTypeLink(matches[i]) + spaceID, nodeID, err = ReadSpaceAndNodeFromIndexLink(match) if err != nil { - appctx.GetLogger(ctx).Error().Err(err).Str("match", matches[i]).Msg("could not read link, skipping") + appctx.GetLogger(ctx).Error().Err(err).Str("match", match).Msg("could not read link, skipping") continue } @@ -360,25 +383,27 @@ func (fs *Decomposedfs) ListStorageSpaces(ctx context.Context, filter []*provide continue } - spaceType := filepath.Base(filepath.Dir(matches[i])) + space, err := fs.storageSpaceFromNode(ctx, n, canListAllSpaces, unrestricted) + if err != nil { + if _, ok := err.(errtypes.IsPermissionDenied); !ok { + appctx.GetLogger(ctx).Error().Err(err).Interface("node", n).Msg("could not convert to storage space") + } + continue + } // FIXME type share evolved to grant on the edge branch ... make it configurable if the driver should support them or not for now ... ignore type share - if spaceType == spaceTypeShare { + if space.SpaceType == spaceTypeShare { numShares++ // do not list shares as spaces for the owner continue } // TODO apply more filters - space, err := fs.storageSpaceFromNode(ctx, n, matches[i], canListAllSpaces, unrestricted) - if err != nil { - if _, ok := err.(errtypes.IsPermissionDenied); !ok { - appctx.GetLogger(ctx).Error().Err(err).Interface("node", n).Msg("could not convert to storage space") - } - continue + _, ok1 := spaceTypes[spaceTypeAny] + _, ok2 := spaceTypes[space.SpaceType] + if ok1 || ok2 { + spaces = append(spaces, space) } - spaces = append(spaces, space) - } // if there are no matches (or they happened to be spaces for the owner) and the node is a child return a space if len(matches) <= numShares && nodeID != spaceID { @@ -388,7 +413,7 @@ func (fs *Decomposedfs) ListStorageSpaces(ctx context.Context, filter []*provide return nil, err } if n.Exists { - space, err := fs.storageSpaceFromNode(ctx, n, n.InternalPath(), canListAllSpaces, unrestricted) + space, err := fs.storageSpaceFromNode(ctx, n, canListAllSpaces, unrestricted) if err != nil { return nil, err } @@ -496,7 +521,7 @@ func (fs *Decomposedfs) UpdateStorageSpace(ctx context.Context, req *provider.Up } // send back the updated data from the storage - updatedSpace, err := fs.storageSpaceFromNode(ctx, node, node.InternalPath(), false, false) + updatedSpace, err := fs.storageSpaceFromNode(ctx, node, false, false) if err != nil { return nil, err } @@ -570,6 +595,7 @@ func (fs *Decomposedfs) linkSpaceByUser(ctx context.Context, userID, spaceID str return nil } // create user index dir + // TODO: pathify userID if err := os.MkdirAll(filepath.Join(fs.o.Root, "indexes", "by-user-id", userID), 0700); err != nil { return err } @@ -577,12 +603,12 @@ func (fs *Decomposedfs) linkSpaceByUser(ctx context.Context, userID, spaceID str err := os.Symlink("../../../spaces/"+lookup.Pathify(spaceID, 1, 2)+"/nodes/"+lookup.Pathify(spaceID, 4, 2), filepath.Join(fs.o.Root, "indexes/by-user-id", userID, spaceID)) if err != nil { if isAlreadyExists(err) { - appctx.GetLogger(ctx).Debug().Err(err).Str("space", spaceID).Str("indexes/by-user-id", userID).Msg("symlink already exists") + appctx.GetLogger(ctx).Debug().Err(err).Str("space", spaceID).Str("user-id", userID).Msg("symlink already exists") // FIXME: is it ok to wipe this err if the symlink already exists? err = nil } else { // TODO how should we handle error cases here? - appctx.GetLogger(ctx).Error().Err(err).Str("space", spaceID).Str("indexes/by-user-id", userID).Msg("could not create symlink") + appctx.GetLogger(ctx).Error().Err(err).Str("space", spaceID).Str("user-id", userID).Msg("could not create symlink") } } return nil @@ -595,12 +621,12 @@ func (fs *Decomposedfs) linkStorageSpaceType(ctx context.Context, spaceType stri return nil } // create space type dir - if err := os.MkdirAll(filepath.Join(fs.o.Root, "spacetypes", spaceType), 0700); err != nil { + if err := os.MkdirAll(filepath.Join(fs.o.Root, "indexes", "by-type", spaceType), 0700); err != nil { return err } // link space in spacetypes - err := os.Symlink("../../spaces/"+lookup.Pathify(spaceID, 1, 2)+"/nodes/"+lookup.Pathify(spaceID, 4, 2), filepath.Join(fs.o.Root, "spacetypes", spaceType, spaceID)) + err := os.Symlink("../../../spaces/"+lookup.Pathify(spaceID, 1, 2)+"/nodes/"+lookup.Pathify(spaceID, 4, 2), filepath.Join(fs.o.Root, "spacetypes", spaceType, spaceID)) if err != nil { if isAlreadyExists(err) { appctx.GetLogger(ctx).Debug().Err(err).Str("space", spaceID).Str("spacetype", spaceType).Msg("symlink already exists") @@ -615,7 +641,7 @@ func (fs *Decomposedfs) linkStorageSpaceType(ctx context.Context, spaceType stri return err } -func (fs *Decomposedfs) storageSpaceFromNode(ctx context.Context, n *node.Node, nodePath string, canListAllSpaces bool, unrestricted bool) (*provider.StorageSpace, error) { +func (fs *Decomposedfs) storageSpaceFromNode(ctx context.Context, n *node.Node, canListAllSpaces bool, unrestricted bool) (*provider.StorageSpace, error) { user := ctxpkg.ContextMustGetUser(ctx) if !canListAllSpaces || !unrestricted { ok, err := node.NewPermissions(fs.lu).HasPermission(ctx, n, func(p *provider.ResourcePermissions) bool { @@ -723,7 +749,7 @@ func (fs *Decomposedfs) storageSpaceFromNode(ctx context.Context, n *node.Node, Seconds: uint64(un / 1000000000), Nanos: uint32(un % 1000000000), } - } else if fi, err := os.Stat(nodePath); err == nil { + } else if fi, err := os.Stat(n.InternalPath()); err == nil { // fall back to stat mtime tmtime = fi.ModTime() un := fi.ModTime().UnixNano() @@ -742,7 +768,7 @@ func (fs *Decomposedfs) storageSpaceFromNode(ctx context.Context, n *node.Node, Value: []byte(etag), } - spaceAttributes, err := xattrs.All(nodePath) + spaceAttributes, err := xattrs.All(n.InternalPath()) if err != nil { return nil, err } diff --git a/pkg/storage/utils/decomposedfs/spaces_test.go b/pkg/storage/utils/decomposedfs/spaces_test.go index d4bfbd0cef3..8ca2154fa15 100644 --- a/pkg/storage/utils/decomposedfs/spaces_test.go +++ b/pkg/storage/utils/decomposedfs/spaces_test.go @@ -133,7 +133,7 @@ var _ = Describe("Spaces", func() { err := os.Symlink(link, filepath.Join(tmpdir, "link")) Expect(err).ToNot(HaveOccurred()) - space, node, err := decomposedfs.ReadSpaceAndNodeFromSpaceTypeLink(filepath.Join(tmpdir, "link")) + space, node, err := decomposedfs.ReadSpaceAndNodeFromIndexLink(filepath.Join(tmpdir, "link")) if shouldErr { Expect(err).To(HaveOccurred()) } else { diff --git a/pkg/storage/utils/decomposedfs/tree/migrations.go b/pkg/storage/utils/decomposedfs/tree/migrations.go new file mode 100644 index 00000000000..5cf8bf1b642 --- /dev/null +++ b/pkg/storage/utils/decomposedfs/tree/migrations.go @@ -0,0 +1,101 @@ +// Copyright 2018-2021 CERN +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. +// +// In applying this license, CERN does not waive the privileges and immunities +// granted to it by virtue of its status as an Intergovernmental Organization +// or submit itself to any jurisdiction. + +package tree + +import ( + "os" + "path/filepath" + + "github.com/cs3org/reva/v2/pkg/logger" +) + +/** + * This function runs all migrations in sequence. + * Note this sequence must not be changed or it might + * damage existing decomposed fs. + */ +func (t *Tree) runMigrations() error { + if err := t.migration0001Nodes(); err != nil { + return err + } + if err := t.migration0002SpaceTypes(); err != nil { + return err + } + return nil +} + +func (t *Tree) migration0001Nodes() error { + // create spaces folder and iterate over existing nodes to populate it + nodesPath := filepath.Join(t.root, "nodes") + fi, err := os.Stat(nodesPath) + if err == nil && fi.IsDir() { + + f, err := os.Open(nodesPath) + if err != nil { + return err + } + nodes, err := f.Readdir(0) + if err != nil { + return err + } + + for _, node := range nodes { + nodePath := filepath.Join(nodesPath, node.Name()) + + if isRootNode(nodePath) { + if err := t.moveNode(node.Name(), node.Name()); err != nil { + logger.New().Error().Err(err). + Str("space", node.Name()). + Msg("could not move space") + continue + } + t.linkSpaceNode("personal", node.Name()) + } + } + // TODO delete nodesPath if empty + } + return nil +} + +func (t *Tree) migration0002SpaceTypes() error { + spaceTypesPath := filepath.Join(t.root, "spacetypes") + fi, err := os.Stat(spaceTypesPath) + if err == nil && fi.IsDir() { + + f, err := os.Open(spaceTypesPath) + if err != nil { + return err + } + spaceTypes, err := f.Readdir(0) + if err != nil { + return err + } + + for _, st := range spaceTypes { + err := t.moveSpaceType(st.Name()) + if err != nil { + logger.New().Error().Err(err). + Str("space", st.Name()). + Msg("could not move space") + continue + } + } + } + return nil +} diff --git a/pkg/storage/utils/decomposedfs/tree/tree.go b/pkg/storage/utils/decomposedfs/tree/tree.go index 2e21b77d06c..a8defc30fff 100644 --- a/pkg/storage/utils/decomposedfs/tree/tree.go +++ b/pkg/storage/utils/decomposedfs/tree/tree.go @@ -102,40 +102,10 @@ func (t *Tree) Setup() error { return err } } - - // create spaces folder and iterate over existing nodes to populate it - nodesPath := filepath.Join(t.root, "nodes") - fi, err := os.Stat(nodesPath) - if err == nil && fi.IsDir() { - - f, err := os.Open(nodesPath) - if err != nil { - return err - } - nodes, err := f.Readdir(0) - if err != nil { - return err - } - - for _, node := range nodes { - nodePath := filepath.Join(nodesPath, node.Name()) - - if isRootNode(nodePath) { - if err := t.moveNode(node.Name(), node.Name()); err != nil { - logger.New().Error().Err(err). - Str("space", node.Name()). - Msg("could not move space") - continue - } - t.linkSpace("personal", node.Name()) - } - } - // TODO delete nodesPath if empty - - } - - return nil + // Run migrations & return + return t.runMigrations() } + func (t *Tree) moveNode(spaceID, nodeID string) error { dirPath := filepath.Join(t.root, "nodes", nodeID) f, err := os.Open(dirPath) @@ -166,8 +136,100 @@ func (t *Tree) moveNode(spaceID, nodeID string) error { return nil } +func (t *Tree) moveSpaceType(spaceType string) error { + dirPath := filepath.Join(t.root, "spacetypes", spaceType) + f, err := os.Open(dirPath) + if err != nil { + return err + } + children, err := f.Readdir(0) + if err != nil { + return err + } + for _, child := range children { + old := filepath.Join(t.root, "spacetypes", spaceType, child.Name()) + target, err := os.Readlink(old) + if err != nil { + logger.New().Error().Err(err). + Str("space", spaceType). + Str("nodes", child.Name()). + Str("oldLink", old). + Msg("could not read old symplink") + continue + } + newDir := filepath.Join(t.root, "indexes", "by-type", spaceType) + if err := os.MkdirAll(newDir, 0700); err != nil { + logger.New().Error().Err(err). + Str("space", spaceType). + Str("nodes", child.Name()). + Str("targetDir", newDir). + Msg("could not read old symplink") + } + newLink := filepath.Join(newDir, child.Name()) + if err := os.Symlink(filepath.Join("..", target), newLink); err != nil { + logger.New().Error().Err(err). + Str("space", spaceType). + Str("nodes", child.Name()). + Str("oldpath", old). + Str("newpath", newLink). + Msg("could not rename node") + continue + } + if err := os.Remove(old); err != nil { + logger.New().Error().Err(err). + Str("space", spaceType). + Str("nodes", child.Name()). + Str("oldLink", old). + Msg("could not remove old symlink") + continue + } + } + if err := os.Remove(dirPath); err != nil { + logger.New().Error().Err(err). + Str("space", spaceType). + Str("dir", dirPath). + Msg("could not remove spaces folder, folder probably not empty") + } + return nil +} + +// linkSpace creates a new symbolic link for a space with the given type st, and node id +func (t *Tree) linkSpaceType(spaceType, spaceID string) error { + if err := os.MkdirAll(filepath.Join(t.root, "indexes", "by-type"), 0700); err != nil { + return err + } + spaceTypesPath := filepath.Join(t.root, "indexes", "by-type", spaceType, spaceID) + expectedTarget := "../../spaces/" + lookup.Pathify(spaceID, 1, 2) + "/nodes/" + lookup.Pathify(spaceID, 4, 2) + linkTarget, err := os.Readlink(spaceTypesPath) + if errors.Is(err, os.ErrNotExist) { + err = os.Symlink(expectedTarget, spaceTypesPath) + if err != nil { + logger.New().Error().Err(err). + Str("space_type", spaceType). + Str("space", spaceID). + Msg("could not create symlink") + } + } else { + if err != nil { + logger.New().Error().Err(err). + Str("space_type", spaceType). + Str("space", spaceID). + Msg("could not read symlink") + } + if linkTarget != expectedTarget { + logger.New().Warn(). + Str("space_type", spaceType). + Str("space", spaceID). + Str("expected", expectedTarget). + Str("actual", linkTarget). + Msg("expected a different link target") + } + } + return nil +} + // linkSpace creates a new symbolic link for a space with the given type st, and node id -func (t *Tree) linkSpace(spaceType, spaceID string) { +func (t *Tree) linkSpaceNode(spaceType, spaceID string) { spaceTypesPath := filepath.Join(t.root, "spacetypes", spaceType, spaceID) expectedTarget := "../../spaces/" + lookup.Pathify(spaceID, 1, 2) + "/nodes/" + lookup.Pathify(spaceID, 4, 2) linkTarget, err := os.Readlink(spaceTypesPath)