From 94df160408924968445f65430b7168408f7b1c97 Mon Sep 17 00:00:00 2001 From: Michal Klos Date: Thu, 18 Jun 2026 16:48:22 +0200 Subject: [PATCH 1/4] feat: ocis integration --- .../storageprovider/storageprovider.go | 137 ++++++++++------- .../storageprovider/storageprovider_test.go | 130 ++++++++++++++++ .../services/storageprovider/suite_test.go | 13 ++ pkg/rgrpc/rgrpc.go | 4 +- pkg/storage/fs/kiteworks/kiteworks.go | 139 ++++++++++++++---- pkg/storage/fs/kiteworks/kwlib/client.go | 52 +++---- pkg/storage/registry/spaces/spaces.go | 7 + pkg/storage/utils/decomposedfs/node/node.go | 9 ++ pkg/storage/utils/decomposedfs/spaces.go | 8 +- pkg/storage/utils/decomposedfs/upload.go | 10 +- pkg/storagespace/storagespace_test.go | 27 ++++ 11 files changed, 413 insertions(+), 123 deletions(-) create mode 100644 internal/grpc/services/storageprovider/storageprovider_test.go create mode 100644 internal/grpc/services/storageprovider/suite_test.go diff --git a/internal/grpc/services/storageprovider/storageprovider.go b/internal/grpc/services/storageprovider/storageprovider.go index 01fd4045b7f..007da5a0f73 100644 --- a/internal/grpc/services/storageprovider/storageprovider.go +++ b/internal/grpc/services/storageprovider/storageprovider.go @@ -239,13 +239,11 @@ func (s *Service) SetLock(ctx context.Context, req *provider.SetLockRequest) (*p Status: status.NewPermissionDenied(ctx, nil, "no permission to lock the share"), }, nil } - res, err := s.Storage.SetLock(ctx, req.Ref, req.Lock) - if err != nil { - return &provider.SetLockResponse{ - Status: status.NewStatusFromErrType(ctx, "set lock", err), - }, nil + // non-decomposedfs drivers may return nil result; set SpaceOwner only when present + lockResult, err := s.Storage.SetLock(ctx, req.Ref, req.Lock) + if lockResult != nil { + storagespace.ContextSetSpaceOwner(ctx, lockResult.SpaceOwner) } - storagespace.ContextSetSpaceOwner(ctx, res.SpaceOwner) return &provider.SetLockResponse{ Status: status.NewStatusFromErrType(ctx, "set lock", err), @@ -285,13 +283,11 @@ func (s *Service) Unlock(ctx context.Context, req *provider.UnlockRequest) (*pro }, nil } - res, err := s.Storage.Unlock(ctx, req.Ref, req.Lock) - if err != nil { - return &provider.UnlockResponse{ - Status: status.NewStatusFromErrType(ctx, "unlock", err), - }, nil + // non-decomposedfs drivers may return nil result; set SpaceOwner only when present + unlockResult, err := s.Storage.Unlock(ctx, req.Ref, req.Lock) + if unlockResult != nil { + storagespace.ContextSetSpaceOwner(ctx, unlockResult.SpaceOwner) } - storagespace.ContextSetSpaceOwner(ctx, res.SpaceOwner) return &provider.UnlockResponse{ Status: status.NewStatusFromErrType(ctx, "unlock", err), @@ -625,7 +621,34 @@ func (s *Service) UpdateStorageSpace(ctx context.Context, req *provider.UpdateSt } func (s *Service) DeleteStorageSpace(ctx context.Context, req *provider.DeleteStorageSpaceRequest) (*provider.DeleteStorageSpaceResponse, error) { - result, err := s.Storage.DeleteStorageSpace(ctx, req) + // pre-fetch spacename+grants before deletion: non-decomposedfs drivers don't populate DeleteStorageSpaceResult + idraw, _ := storagespace.ParseID(req.Id.GetOpaqueId()) + idraw.OpaqueId = idraw.GetSpaceId() + id := &provider.StorageSpaceId{OpaqueId: storagespace.FormatResourceID(&idraw)} + + spaces, err := s.Storage.ListStorageSpaces(ctx, []*provider.ListStorageSpacesRequest_Filter{{Type: provider.ListStorageSpacesRequest_Filter_TYPE_ID, Term: &provider.ListStorageSpacesRequest_Filter_Id{Id: id}}}, true) + if err != nil { + var st *rpc.Status + switch err.(type) { + case errtypes.IsNotFound: + st = status.NewNotFound(ctx, "space not found") + case errtypes.PermissionDenied: + st = status.NewPermissionDenied(ctx, err, "permission denied") + case errtypes.BadRequest: + st = status.NewInvalid(ctx, err.Error()) + default: + st = status.NewInternal(ctx, "error deleting space: "+req.Id.String()) + } + return &provider.DeleteStorageSpaceResponse{ + Status: st, + }, nil + } else if len(spaces) != 1 { + return &provider.DeleteStorageSpaceResponse{ + Status: status.NewNotFound(ctx, "space not found"), + }, nil + } + + deleteSpaceResult, err := s.Storage.DeleteStorageSpace(ctx, req) if err != nil { var st *rpc.Status switch err.(type) { @@ -648,14 +671,13 @@ func (s *Service) DeleteStorageSpace(ctx context.Context, req *provider.DeleteSt Status: st, }, nil } - - if result != nil { - storagespace.ContextSetDeleteStorageSpaceResult(ctx, result) + if deleteSpaceResult == nil { + // driver didn't populate the result; fill SpaceName from the pre-fetched space so SpaceDeleted event is not empty + deleteSpaceResult = &storage.DeleteStorageSpaceResult{SpaceName: spaces[0].GetName()} } + storagespace.ContextSetDeleteStorageSpaceResult(ctx, deleteSpaceResult) - return &provider.DeleteStorageSpaceResponse{ - Status: status.NewOK(ctx), - }, nil + return &provider.DeleteStorageSpaceResponse{Status: status.NewOK(ctx)}, nil } func (s *Service) CreateContainer(ctx context.Context, req *provider.CreateContainerRequest) (*provider.CreateContainerResponse, error) { @@ -666,13 +688,10 @@ func (s *Service) CreateContainer(ctx context.Context, req *provider.CreateConta } } - res, err := s.Storage.CreateDir(ctx, req.Ref) - if err != nil { - return &provider.CreateContainerResponse{ - Status: status.NewStatusFromErrType(ctx, "create container", err), - }, nil + createDirResult, err := s.Storage.CreateDir(ctx, req.Ref) + if createDirResult != nil { + storagespace.ContextSetSpaceOwner(ctx, createDirResult.SpaceOwner) } - storagespace.ContextSetSpaceOwner(ctx, res.SpaceOwner) return &provider.CreateContainerResponse{ Status: status.NewStatusFromErrType(ctx, "create container", err), @@ -689,13 +708,10 @@ func (s *Service) TouchFile(ctx context.Context, req *provider.TouchFileRequest) mtime = utils.ReadPlainFromOpaque(req.Opaque, "X-OC-Mtime") } - res, err := s.Storage.TouchFile(ctx, req.Ref, utils.ExistsInOpaque(req.Opaque, "markprocessing"), mtime) - if err != nil { - return &provider.TouchFileResponse{ - Status: status.NewStatusFromErrType(ctx, "touch file", err), - }, nil + touchResult, err := s.Storage.TouchFile(ctx, req.Ref, utils.ExistsInOpaque(req.Opaque, "markprocessing"), mtime) + if touchResult != nil { + storagespace.ContextSetSpaceOwner(ctx, touchResult.SpaceOwner) } - storagespace.ContextSetSpaceOwner(ctx, res.SpaceOwner) return &provider.TouchFileResponse{ Status: status.NewStatusFromErrType(ctx, "touch file", err), @@ -720,24 +736,46 @@ func (s *Service) Delete(ctx context.Context, req *provider.DeleteRequest) (*pro } } - result, err := s.Storage.Delete(ctx, req.Ref) + md, err := s.Storage.GetMD(ctx, req.Ref, []string{}, []string{"id", "status"}) + if err != nil { + return &provider.DeleteResponse{ + Status: status.NewStatusFromErrType(ctx, "can't stat resource to delete", err), + }, nil + } - if err == nil && result != nil { - storagespace.ContextSetDeleteResult(ctx, result) + if utils.ReadPlainFromOpaque(md.GetOpaque(), "status") == "processing" { + return &provider.DeleteResponse{ + Status: &rpc.Status{ + Code: rpc.Code_CODE_TOO_EARLY, + Message: "file is processing", + }, + Opaque: &typesv1beta1.Opaque{ + Map: map[string]*typesv1beta1.OpaqueEntry{ + "status": {Decoder: "plain", Value: []byte("processing")}, + }, + }, + }, nil } + deleteResult, err := s.Storage.Delete(ctx, req.Ref) + storagespace.ContextSetDeleteResult(ctx, deleteResult) + return &provider.DeleteResponse{ Status: status.NewStatusFromErrType(ctx, "delete", err), + Opaque: &typesv1beta1.Opaque{ + Map: map[string]*typesv1beta1.OpaqueEntry{ + "opaque_id": {Decoder: "plain", Value: []byte(md.Id.OpaqueId)}, + }, + }, }, nil } func (s *Service) Move(ctx context.Context, req *provider.MoveRequest) (*provider.MoveResponse, error) { ctx = ctxpkg.ContextSetLockID(ctx, req.LockId) - result, err := s.Storage.Move(ctx, req.Source, req.Destination) - if err == nil && result != nil { - storagespace.ContextSetMoveResult(ctx, result) - } + moveResult, err := s.Storage.Move(ctx, req.Source, req.Destination) + storagespace.ContextSetMoveResult(ctx, moveResult) + return &provider.MoveResponse{ Status: status.NewStatusFromErrType(ctx, "move", err), }, nil @@ -849,13 +887,10 @@ func (s *Service) ListFileVersions(ctx context.Context, req *provider.ListFileVe func (s *Service) RestoreFileVersion(ctx context.Context, req *provider.RestoreFileVersionRequest) (*provider.RestoreFileVersionResponse, error) { ctx = ctxpkg.ContextSetLockID(ctx, req.LockId) - res, err := s.Storage.RestoreRevision(ctx, req.Ref, req.Key) - if err != nil { - return &provider.RestoreFileVersionResponse{ - Status: status.NewStatusFromErrType(ctx, "restore file version", err), - }, nil + restoreRevResult, err := s.Storage.RestoreRevision(ctx, req.Ref, req.Key) + if restoreRevResult != nil { + storagespace.ContextSetSpaceOwner(ctx, restoreRevResult.SpaceOwner) } - storagespace.ContextSetSpaceOwner(ctx, res.SpaceOwner) return &provider.RestoreFileVersionResponse{ Status: status.NewStatusFromErrType(ctx, "restore file version", err), @@ -954,17 +989,15 @@ func (s *Service) RestoreRecycleItem(ctx context.Context, req *provider.RestoreR // TODO(labkode): CRITICAL: fill recycle info with storage provider. key, relativePath := splitKeyAndPath(req.GetKey()) - writeRes, err := s.Storage.RestoreRecycleItem(ctx, req.Ref, key, relativePath, req.RestoreRef) - if err != nil { - return &provider.RestoreRecycleItemResponse{ - Status: status.NewStatusFromErrType(ctx, "restore recycle item", err), - }, nil + restoreItemResult, err := s.Storage.RestoreRecycleItem(ctx, req.Ref, key, relativePath, req.RestoreRef) + if restoreItemResult != nil { + storagespace.ContextSetSpaceOwner(ctx, restoreItemResult.SpaceOwner) } - storagespace.ContextSetSpaceOwner(ctx, writeRes.SpaceOwner) - return &provider.RestoreRecycleItemResponse{ + res := &provider.RestoreRecycleItemResponse{ Status: status.NewStatusFromErrType(ctx, "restore recycle item", err), - }, nil + } + return res, nil } func (s *Service) PurgeRecycle(ctx context.Context, req *provider.PurgeRecycleRequest) (*provider.PurgeRecycleResponse, error) { diff --git a/internal/grpc/services/storageprovider/storageprovider_test.go b/internal/grpc/services/storageprovider/storageprovider_test.go new file mode 100644 index 00000000000..faeb8376a64 --- /dev/null +++ b/internal/grpc/services/storageprovider/storageprovider_test.go @@ -0,0 +1,130 @@ +package storageprovider + +import ( + "context" + "net/url" + + rpc "github.com/cs3org/go-cs3apis/cs3/rpc/v1beta1" + provider "github.com/cs3org/go-cs3apis/cs3/storage/provider/v1beta1" + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" + "github.com/owncloud/reva/v2/pkg/storage" + "github.com/owncloud/reva/v2/pkg/storagespace" +) + +// stubFS overrides only the methods under test; the embedded storage.FS satisfies the rest. +type stubFS struct { + storage.FS + listSpaces func(ctx context.Context, filters []*provider.ListStorageSpacesRequest_Filter, unrestricted bool) ([]*provider.StorageSpace, error) + deleteSpace func(ctx context.Context, req *provider.DeleteStorageSpaceRequest) (*storage.DeleteStorageSpaceResult, error) +} + +func (s *stubFS) ListStorageSpaces(ctx context.Context, filters []*provider.ListStorageSpacesRequest_Filter, unrestricted bool) ([]*provider.StorageSpace, error) { + return s.listSpaces(ctx, filters, unrestricted) +} + +func (s *stubFS) DeleteStorageSpace(ctx context.Context, req *provider.DeleteStorageSpaceRequest) (*storage.DeleteStorageSpaceResult, error) { + if s.deleteSpace != nil { + return s.deleteSpace(ctx, req) + } + return nil, nil +} + +func (s *stubFS) Shutdown(_ context.Context) error { return nil } + +// CreateReference is called by the storageprovider on some paths; stub to avoid nil-embed panic. +func (s *stubFS) CreateReference(_ context.Context, _ string, _ *url.URL) error { return nil } + +var _ = Describe("DeleteStorageSpace", func() { + var ( + svc *Service + ctx context.Context + ) + + space := &provider.StorageSpace{ + Id: &provider.StorageSpaceId{OpaqueId: "providerid$spaceid!spaceid"}, + Name: "My Space", + Root: &provider.ResourceId{StorageId: "providerid", SpaceId: "spaceid", OpaqueId: "spaceid"}, + } + + req := &provider.DeleteStorageSpaceRequest{ + Id: &provider.StorageSpaceId{OpaqueId: "providerid$spaceid!spaceid"}, + } + + listFound := func(_ context.Context, _ []*provider.ListStorageSpacesRequest_Filter, _ bool) ([]*provider.StorageSpace, error) { + return []*provider.StorageSpace{space}, nil + } + + BeforeEach(func() { + ctx = storagespace.ContextRegisterDeleteStorageSpaceResultSlot(context.Background()) + }) + + Context("when driver returns nil result (non-decomposedfs path)", func() { + BeforeEach(func() { + svc = &Service{Storage: &stubFS{listSpaces: listFound}} + }) + + It("populates SpaceName from pre-fetched space so SpaceDeleted event is not empty", func() { + res, err := svc.DeleteStorageSpace(ctx, req) + + Expect(err).ToNot(HaveOccurred()) + Expect(res.Status.Code).To(Equal(rpc.Code_CODE_OK)) + + result := storagespace.ContextGetDeleteStorageSpaceResult(ctx) + Expect(result).ToNot(BeNil()) + Expect(result.SpaceName).To(Equal("My Space")) + Expect(result.FinalMembers).To(BeNil()) + }) + + It("does not carry spacename/grants in response opaque — consumers read from context slot", func() { + res, err := svc.DeleteStorageSpace(ctx, req) + + Expect(err).ToNot(HaveOccurred()) + Expect(res.Opaque).To(BeNil()) + }) + }) + + Context("when driver returns a populated result (decomposedfs path)", func() { + finalMembers := map[string]provider.ResourcePermissions{ + "user-1": {Stat: true}, + } + + BeforeEach(func() { + svc = &Service{Storage: &stubFS{ + listSpaces: listFound, + deleteSpace: func(_ context.Context, _ *provider.DeleteStorageSpaceRequest) (*storage.DeleteStorageSpaceResult, error) { + return &storage.DeleteStorageSpaceResult{SpaceName: "Decomposed Space", FinalMembers: finalMembers}, nil + }, + }} + }) + + It("uses driver result as-is, preserving FinalMembers", func() { + res, err := svc.DeleteStorageSpace(ctx, req) + + Expect(err).ToNot(HaveOccurred()) + Expect(res.Status.Code).To(Equal(rpc.Code_CODE_OK)) + + result := storagespace.ContextGetDeleteStorageSpaceResult(ctx) + Expect(result).ToNot(BeNil()) + Expect(result.SpaceName).To(Equal("Decomposed Space")) + Expect(result.FinalMembers).To(HaveKey("user-1")) + }) + }) + + Context("when space is not found", func() { + BeforeEach(func() { + svc = &Service{Storage: &stubFS{ + listSpaces: func(_ context.Context, _ []*provider.ListStorageSpacesRequest_Filter, _ bool) ([]*provider.StorageSpace, error) { + return []*provider.StorageSpace{}, nil + }, + }} + }) + + It("returns NOT_FOUND without calling DeleteStorageSpace", func() { + res, err := svc.DeleteStorageSpace(ctx, req) + + Expect(err).ToNot(HaveOccurred()) + Expect(res.Status.Code).To(Equal(rpc.Code_CODE_NOT_FOUND)) + }) + }) +}) diff --git a/internal/grpc/services/storageprovider/suite_test.go b/internal/grpc/services/storageprovider/suite_test.go new file mode 100644 index 00000000000..b7c5e619fa7 --- /dev/null +++ b/internal/grpc/services/storageprovider/suite_test.go @@ -0,0 +1,13 @@ +package storageprovider + +import ( + "testing" + + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" +) + +func TestStorageprovider(t *testing.T) { + RegisterFailHandler(Fail) + RunSpecs(t, "Storageprovider Suite") +} diff --git a/pkg/rgrpc/rgrpc.go b/pkg/rgrpc/rgrpc.go index c4979f241b2..ce9f32d1785 100644 --- a/pkg/rgrpc/rgrpc.go +++ b/pkg/rgrpc/rgrpc.go @@ -292,7 +292,9 @@ func (s *Server) Stop() error { // GracefulStop gracefully stops the server. func (s *Server) GracefulStop() error { - s.s.GracefulStop() + if s.s != nil { + s.s.GracefulStop() + } s.cleanupServices() return nil } diff --git a/pkg/storage/fs/kiteworks/kiteworks.go b/pkg/storage/fs/kiteworks/kiteworks.go index 7ed2421b6fe..cdaad36d673 100644 --- a/pkg/storage/fs/kiteworks/kiteworks.go +++ b/pkg/storage/fs/kiteworks/kiteworks.go @@ -25,18 +25,20 @@ func init() { registry.Register("kiteworks", New) } -const storageID = "kiteworks" - // Config holds the driver configuration. type Config struct { Endpoint string `mapstructure:"endpoint"` + APIToken string `mapstructure:"api_token"` Insecure bool `mapstructure:"insecure"` + MountID string `mapstructure:"mount_id"` } // Driver implements storage.FS against a Kiteworks box (read-only). type Driver struct { - factory *kwlib.APIClientFactory - log zerolog.Logger + factory *kwlib.APIClientFactory + apiToken string + storageID string + log zerolog.Logger } // New returns a read-only Kiteworks storage driver. @@ -47,32 +49,48 @@ func New(m map[string]interface{}, _ events.Stream, log *zerolog.Logger) (storag } c.Endpoint = strings.TrimRight(c.Endpoint, "/") + storageID := c.MountID + if storageID == "" { + storageID = "kiteworks" + } + l := zerolog.Nop() if log != nil { l = *log } return &Driver{ - factory: kwlib.NewClientFactory(c.Endpoint, "reva-kiteworks/1.0", c.Insecure), - log: l, + factory: kwlib.NewClientFactory(c.Endpoint, "reva-kiteworks/1.0", c.Insecure), + apiToken: c.APIToken, + storageID: storageID, + log: l, }, nil } func (d *Driver) client(ctx context.Context) *kwlib.APIClient { - token, _ := ctxpkg.ContextGetToken(ctx) + token := d.apiToken + if token == "" { + token, _ = ctxpkg.ContextGetToken(ctx) + } return d.factory.Build("", "", "", token, &d.log) } // toResourceInfo converts a kwlib.FileInfo to a CS3 ResourceInfo. -func (d *Driver) toResourceInfo(fi *kwlib.FileInfo, spaceID string) *provider.ResourceInfo { +// spaceRootPath is the absolute KW path of the space root folder; it is stripped +// from fi.Path to produce a space-relative path with a leading "/". +func (d *Driver) toResourceInfo(fi *kwlib.FileInfo, spaceID, spaceRootPath string) *provider.ResourceInfo { + relPath := strings.TrimPrefix(fi.Path, spaceRootPath) + if !strings.HasPrefix(relPath, "/") { + relPath = "/" + relPath + } ri := &provider.ResourceInfo{ Id: &provider.ResourceId{ - StorageId: storageID, + StorageId: d.storageID, SpaceId: spaceID, OpaqueId: fi.ID, }, Name: fi.Name, - Path: fi.Path, + Path: relPath, Etag: fi.ETag(), Mtime: utils.TimeToTS(fi.MTime()), PermissionSet: &provider.ResourcePermissions{ @@ -113,11 +131,11 @@ func (d *Driver) ListStorageSpaces(ctx context.Context, _ []*provider.ListStorag Name: fi.Name, SpaceType: "project", Root: &provider.ResourceId{ - StorageId: storageID, + StorageId: d.storageID, SpaceId: fi.ID, OpaqueId: fi.ID, }, - RootInfo: d.toResourceInfo(fi, fi.ID), + RootInfo: d.toResourceInfo(fi, fi.ID, fi.Path), Mtime: utils.TimeToTS(fi.MTime()), Opaque: utils.AppendPlainToOpaque(nil, "spaceAlias", "project/"+fi.Name), }) @@ -125,29 +143,75 @@ func (d *Driver) ListStorageSpaces(ctx context.Context, _ []*provider.ListStorag return spaces, nil } -func resolveNodeID(ref *provider.Reference) (nodeID, spaceID string) { +// resolveRef walks a CS3 reference to the target KW node ID and space ID. +// If ref.Path is non-empty it resolves each component through ListFolderContents. +func (d *Driver) resolveRef(ctx context.Context, ref *provider.Reference) (nodeID, spaceID string, err error) { spaceID = ref.GetResourceId().GetSpaceId() nodeID = ref.GetResourceId().GetOpaqueId() if nodeID == "" { nodeID = spaceID } - return + + relPath := strings.Trim(strings.TrimPrefix(ref.GetPath(), "./"), "/.") + if relPath == "" { + return nodeID, spaceID, nil + } + + c := d.client(ctx) + for _, part := range strings.Split(relPath, "/") { + if part == "" { + continue + } + children, err := c.ListFolderContents(nodeID) + if err != nil { + return "", "", err + } + var found bool + for i := range children { + if children[i].Name == part { + nodeID = children[i].ID + found = true + break + } + } + if !found { + return "", "", errtypes.NotFound(part) + } + } + return nodeID, spaceID, nil +} + +// spaceRootPath fetches the absolute KW path of the space root folder. +// Used by callers that need to convert absolute KW paths to space-relative paths. +func (d *Driver) spaceRootPath(ctx context.Context, spaceID string) (string, error) { + root, err := d.client(ctx).GetFolderByID(spaceID) + if err != nil { + return "", err + } + return root.Path, nil } func (d *Driver) GetMD(ctx context.Context, ref *provider.Reference, _, _ []string) (*provider.ResourceInfo, error) { - nodeID, spaceID := resolveNodeID(ref) - return d.nodeMD(ctx, nodeID, spaceID) + nodeID, spaceID, err := d.resolveRef(ctx, ref) + if err != nil { + return nil, err + } + rootPath, err := d.spaceRootPath(ctx, spaceID) + if err != nil { + return nil, err + } + return d.nodeMD(ctx, nodeID, spaceID, rootPath) } // nodeMD fetches metadata for a node, trying folder first then file. -func (d *Driver) nodeMD(ctx context.Context, nodeID, spaceID string) (*provider.ResourceInfo, error) { +func (d *Driver) nodeMD(ctx context.Context, nodeID, spaceID, spaceRootPath string) (*provider.ResourceInfo, error) { c := d.client(ctx) fi, err := c.GetFolderByID(nodeID) if err == nil { - return d.toResourceInfo(fi, spaceID), nil + return d.toResourceInfo(fi, spaceID, spaceRootPath), nil } var ce *kwlib.ClientError - if !errors.As(err, &ce) || ce.StatusCode != http.StatusNotFound { + if !errors.As(err, &ce) || (ce.StatusCode != http.StatusNotFound && ce.StatusCode != http.StatusForbidden) { return nil, err } fi, err = c.GetFileByID(nodeID) @@ -158,11 +222,18 @@ func (d *Driver) nodeMD(ctx context.Context, nodeID, spaceID string) (*provider. } return nil, err } - return d.toResourceInfo(fi, spaceID), nil + return d.toResourceInfo(fi, spaceID, spaceRootPath), nil } func (d *Driver) ListFolder(ctx context.Context, ref *provider.Reference, _, _ []string) ([]*provider.ResourceInfo, error) { - nodeID, spaceID := resolveNodeID(ref) + nodeID, spaceID, err := d.resolveRef(ctx, ref) + if err != nil { + return nil, err + } + rootPath, err := d.spaceRootPath(ctx, spaceID) + if err != nil { + return nil, err + } items, err := d.client(ctx).ListFolderContents(nodeID) if err != nil { @@ -171,15 +242,26 @@ func (d *Driver) ListFolder(ctx context.Context, ref *provider.Reference, _, _ [ infos := make([]*provider.ResourceInfo, 0, len(items)) for i := range items { - infos = append(infos, d.toResourceInfo(&items[i], spaceID)) + ri := d.toResourceInfo(&items[i], spaceID, rootPath) + // ocdav does path.Join(requestPath, info.Path) for ListFolder results, + // so Path must be just the filename — not the space-root-relative path. + ri.Path = ri.Name + infos = append(infos, ri) } return infos, nil } func (d *Driver) Download(ctx context.Context, ref *provider.Reference, openReaderFunc func(*provider.ResourceInfo) bool) (*provider.ResourceInfo, io.ReadCloser, error) { - nodeID, spaceID := resolveNodeID(ref) + nodeID, spaceID, err := d.resolveRef(ctx, ref) + if err != nil { + return nil, nil, err + } + rootPath, err := d.spaceRootPath(ctx, spaceID) + if err != nil { + return nil, nil, err + } - ri, err := d.nodeMD(ctx, nodeID, spaceID) + ri, err := d.nodeMD(ctx, nodeID, spaceID, rootPath) if err != nil { return nil, nil, err } @@ -196,7 +278,11 @@ func (d *Driver) Download(ctx context.Context, ref *provider.Reference, openRead } func (d *Driver) GetPathByID(ctx context.Context, id *provider.ResourceId) (string, error) { - ri, err := d.nodeMD(ctx, id.GetOpaqueId(), id.GetSpaceId()) + rootPath, err := d.spaceRootPath(ctx, id.GetSpaceId()) + if err != nil { + return "", err + } + ri, err := d.nodeMD(ctx, id.GetOpaqueId(), id.GetSpaceId(), rootPath) if err != nil { return "", err } @@ -210,7 +296,8 @@ func (d *Driver) ListGrants(_ context.Context, _ *provider.Reference) ([]*provid func (d *Driver) GetQuota(ctx context.Context, _ *provider.Reference) (uint64, uint64, uint64, error) { q, err := d.client(ctx).GetQuotaInfo() if err != nil { - return 0, 0, 0, err + // Non-fatal for read-only driver; return zero quota rather than failing. + return 0, 0, 0, nil } total := uint64(q.FolderQuotaAllowed) used := uint64(q.FolderQuotaUsed) diff --git a/pkg/storage/fs/kiteworks/kwlib/client.go b/pkg/storage/fs/kiteworks/kwlib/client.go index c240cf6a909..68811e2c215 100644 --- a/pkg/storage/fs/kiteworks/kwlib/client.go +++ b/pkg/storage/fs/kiteworks/kwlib/client.go @@ -211,19 +211,18 @@ func (c *APIClient) GetQuotaInfo() (*QuotaInfo, error) { } func (c *APIClient) GetGroups(limit, offset int) (*ContactList, error) { - u, err := url.Parse("/rest/groups") - if err != nil { - return nil, err - } - q := u.Query() + q := url.Values{} if limit > 0 { - q.Set("limit", fmt.Sprintf("%d", limit)) + q.Set("limit", strconv.Itoa(limit)) } if offset > 0 { - q.Set("offset", fmt.Sprintf("%d", offset)) + q.Set("offset", strconv.Itoa(offset)) } - u.RawQuery = q.Encode() - request, err := c.NewGetRequest(u.String()) + path := "/rest/groups" + if len(q) > 0 { + path += "?" + q.Encode() + } + request, err := c.NewGetRequest(path) if err != nil { return nil, err } @@ -261,7 +260,6 @@ func (c *APIClient) InitializeUpload(parentID, name string, size int64, numberOf if err != nil { return nil, err } - request.Header.Set("Content-Type", "application/json") response, err := c.SendRequest(request) if err != nil { return nil, err @@ -347,8 +345,10 @@ func (c *APIClient) Copy(source *FileInfo, parent *FileInfo, replace bool) (bool type moveOrCopy int -var moveOp moveOrCopy = 1 -var copyOp moveOrCopy = 2 +const ( + moveOp moveOrCopy = iota + 1 + copyOp +) func (c *APIClient) moveOrCopy(op moveOrCopy, source *FileInfo, dest *FileInfo, replace bool) (bool, error) { api := "/rest/files/actions/move" @@ -392,24 +392,19 @@ func (c *APIClient) NewGetRequest(path string) (*http.Request, error) { } func (c *APIClient) NewPostRequest(path string, v any) (*http.Request, error) { - b, err := json.Marshal(v) - if err != nil { - return nil, err - } - req, err := c.newRequest("POST", path, bytes.NewBuffer(b)) - if err != nil { - return nil, err - } - req.Header.Set("Content-Type", "application/json") - return req, nil + return c.newJSONRequest("POST", path, v) } func (c *APIClient) NewPutRequest(path string, v any) (*http.Request, error) { + return c.newJSONRequest("PUT", path, v) +} + +func (c *APIClient) newJSONRequest(method, path string, v any) (*http.Request, error) { b, err := json.Marshal(v) if err != nil { return nil, err } - req, err := c.newRequest("PUT", path, bytes.NewBuffer(b)) + req, err := c.newRequest(method, path, bytes.NewBuffer(b)) if err != nil { return nil, err } @@ -437,21 +432,18 @@ func (c *APIClient) newRequest(method, path string, body io.Reader) (*http.Reque } func (c *APIClient) SendRequest(req *http.Request) (*http.Response, error) { - client := c.httpClient - - log := c.logger.Debug().Str("method", req.Method).Str("path", req.URL.String()) - response, err := client.Do(req) + response, err := c.httpClient.Do(req) if err != nil { - log.Err(err).Msg("kiteworks API call errored") + c.logger.Debug().Str("method", req.Method).Str("path", req.URL.String()).Err(err).Msg("kiteworks API call errored") return nil, err } if response.StatusCode >= http.StatusBadRequest { defer response.Body.Close() b, _ := io.ReadAll(response.Body) - log.Str("body", string(b)).Int("status", response.StatusCode).Msg("kiteworks API call failed") + c.logger.Debug().Str("method", req.Method).Str("path", req.URL.String()).Str("body", string(b)).Int("status", response.StatusCode).Msg("kiteworks API call failed") return response, NewClientError(response.StatusCode) } - log.Int("status", response.StatusCode).Msg("kiteworks API call success") + c.logger.Debug().Str("method", req.Method).Str("path", req.URL.String()).Int("status", response.StatusCode).Msg("kiteworks API call success") return response, nil } diff --git a/pkg/storage/registry/spaces/spaces.go b/pkg/storage/registry/spaces/spaces.go index 1ea58a8f1dc..fea0ff37303 100644 --- a/pkg/storage/registry/spaces/spaces.go +++ b/pkg/storage/registry/spaces/spaces.go @@ -79,6 +79,9 @@ type Provider struct { // Spaces is a map from space type to space config Spaces map[string]*spaceConfig `mapstructure:"spaces"` ProviderID string `mapstructure:"providerid"` + // ReadOnly marks providers that serve existing spaces but cannot create new ones. + // GetProvider skips them unless the request explicitly targets this provider via storage_id. + ReadOnly bool `mapstructure:"readonly"` } type templateData struct { @@ -205,6 +208,10 @@ func (r *registry) GetProvider(ctx context.Context, space *providerpb.StorageSpa if provider.ProviderID == utils.VaultStorageProviderID { continue } + // Skip read-only providers (e.g. kiteworks) — they cannot create spaces + if provider.ReadOnly { + continue + } } if space.Owner != nil { diff --git a/pkg/storage/utils/decomposedfs/node/node.go b/pkg/storage/utils/decomposedfs/node/node.go index 3f3dadb7352..d24deb4ca70 100644 --- a/pkg/storage/utils/decomposedfs/node/node.go +++ b/pkg/storage/utils/decomposedfs/node/node.go @@ -1339,6 +1339,15 @@ var CheckQuota = func(ctx context.Context, spaceRoot *Node, overwrite bool, oldS return true, nil } +// CheckDiskSpace returns an error if the underlying filesystem has insufficient +// free space for newSize bytes. Does not enforce space-level quota. +var CheckDiskSpace = func(_ context.Context, spaceRoot *Node, newSize uint64) error { + if !enoughDiskSpace(spaceRoot.InternalPath(), newSize) { + return errtypes.InsufficientStorage("disk full") + } + return nil +} + func enoughDiskSpace(path string, fileSize uint64) bool { avalB, err := GetAvailableSize(path) if err != nil { diff --git a/pkg/storage/utils/decomposedfs/spaces.go b/pkg/storage/utils/decomposedfs/spaces.go index ced3254ba90..42bc5dfe674 100644 --- a/pkg/storage/utils/decomposedfs/spaces.go +++ b/pkg/storage/utils/decomposedfs/spaces.go @@ -1269,8 +1269,7 @@ func canDeleteSpace(ctx context.Context, spaceID string, typ string, purge bool, } // space managers are allowed to disable and delete their project spaces - rp, err := p.AssemblePermissions(ctx, n) - if err == nil && permissions.IsManager(rp) { + if rp, err := p.AssemblePermissions(ctx, n); err == nil && permissions.IsManager(rp) { return nil } @@ -1284,10 +1283,5 @@ func canDeleteSpace(ctx context.Context, spaceID string, typ string, purge bool, return nil } - // active space, user has no grant at all: hide existence rather than reveal it - if err == nil && !rp.GetStat() && !n.IsDisabled(ctx) { - return errtypes.NotFound(spaceID) - } - return errtypes.PermissionDenied(fmt.Sprintf("user is not allowed to delete space %s", n.ID)) } diff --git a/pkg/storage/utils/decomposedfs/upload.go b/pkg/storage/utils/decomposedfs/upload.go index abacbd25cd9..01bff566a98 100644 --- a/pkg/storage/utils/decomposedfs/upload.go +++ b/pkg/storage/utils/decomposedfs/upload.go @@ -353,15 +353,11 @@ func (fs *Decomposedfs) MarkProcessing(ctx context.Context, ref *provider.Refere } // Early lock, so MarkProcessing is atomic. - f, err := lockedfile.OpenFile(fs.lu.MetadataBackend().LockfilePath(n.InternalPath()), os.O_RDWR|os.O_CREATE, 0600) + unlock, err := fs.lu.MetadataBackend().Lock(n.InternalPath()) if err != nil { return err } - defer func() { - if cerr := f.Close(); cerr != nil { - appctx.GetLogger(ctx).Error().Err(cerr).Str("nodeid", n.ID).Msg("could not close mark-processing lock") - } - }() + defer unlock() //nolint:errcheck // Evict the node's in-process xattr cache so IsProcessing reads from disk while we hold the lock. n.ResetXattrsCache() @@ -468,7 +464,7 @@ func (fs *Decomposedfs) CommitUpload(ctx context.Context, ref *provider.Referenc if err != nil { return nil, errors.Wrap(err, "Decomposedfs: failed to read existing node") } - if _, err := node.CheckQuota(ctx, n.SpaceRoot, old.BlobID != "", uint64(old.Blobsize), uint64(source.Length)); err != nil { + if err := node.CheckDiskSpace(ctx, n.SpaceRoot, uint64(source.Length)); err != nil { return nil, err } diff --git a/pkg/storagespace/storagespace_test.go b/pkg/storagespace/storagespace_test.go index d3f14d2844c..744c3955a4f 100644 --- a/pkg/storagespace/storagespace_test.go +++ b/pkg/storagespace/storagespace_test.go @@ -340,3 +340,30 @@ func TestUpdateLegacyResourceID(t *testing.T) { } } } + +// TestDeleteStorageSpaceIDNormalization guards against clients sending arbitrary nodeids in DeleteStorageSpaceRequest — without normalisation ListStorageSpaces falls back to a full index scan instead of a direct root-node lookup. +func TestDeleteStorageSpaceIDNormalization(t *testing.T) { + tests := []struct { + input string + expected string // providerid$spaceid!spaceid — root node ref + }{ + // typical client input: nodeid differs from spaceid + {"providerid$spaceid!nodeid", "providerid$spaceid!spaceid"}, + // client already sent root ref + {"providerid$spaceid!spaceid", "providerid$spaceid!spaceid"}, + // no provider prefix + {"spaceid!nodeid", "spaceid!spaceid"}, + // bare space ID (no nodeid): ParseID sets OpaqueId="" → SpaceId used + {"providerid$spaceid", "providerid$spaceid!spaceid"}, + } + + for _, tt := range tests { + idraw, _ := ParseID(tt.input) + idraw.OpaqueId = idraw.GetSpaceId() + got := FormatResourceID(&idraw) + + if got != tt.expected { + t.Errorf("input %q: got %q, want %q", tt.input, got, tt.expected) + } + } +} From 4fecd5837bf3ccd4f8f7772d4e0c191faf8c5e95 Mon Sep 17 00:00:00 2001 From: Michal Klos Date: Thu, 25 Jun 2026 21:09:37 +0200 Subject: [PATCH 2/4] feat: ocisdev-904 --- .../cloud/capabilities/capabilities.go | 19 ++++++ .../cloud/capabilities/capabilities_test.go | 64 +++++++++++++++++++ .../storage/eoshomewrapper/eoshomewrapper.go | 2 +- pkg/cbox/storage/eoswrapper/eoswrapper.go | 2 +- pkg/ocm/storage/received/ocm.go | 2 +- pkg/owncloud/ocs/capabilities.go | 51 +++++++++++---- pkg/storage/fs/cephfs/cephfs.go | 2 +- pkg/storage/fs/cephfs/unsupported.go | 2 +- pkg/storage/fs/eos/eos.go | 2 +- pkg/storage/fs/eosgrpc/eosgrpc.go | 2 +- pkg/storage/fs/eosgrpchome/eosgrpchome.go | 2 +- pkg/storage/fs/eoshome/eoshome.go | 2 +- pkg/storage/fs/hello/hello.go | 2 +- pkg/storage/fs/kiteworks/kiteworks.go | 2 +- pkg/storage/fs/local/local.go | 2 +- pkg/storage/fs/localhome/localhome.go | 2 +- pkg/storage/fs/nextcloud/nextcloud.go | 2 +- pkg/storage/fs/ocis/ocis.go | 3 +- pkg/storage/fs/owncloudsql/owncloudsql.go | 2 +- pkg/storage/fs/posix/posix.go | 2 +- pkg/storage/fs/registry/registry.go | 25 +++++++- pkg/storage/fs/s3/s3.go | 2 +- pkg/storage/fs/s3ng/s3ng.go | 2 +- pkg/storage/storage.go | 14 ++++ 24 files changed, 179 insertions(+), 33 deletions(-) diff --git a/internal/http/services/owncloud/ocs/handlers/cloud/capabilities/capabilities.go b/internal/http/services/owncloud/ocs/handlers/cloud/capabilities/capabilities.go index 9556638d14b..e704d6f6109 100644 --- a/internal/http/services/owncloud/ocs/handlers/cloud/capabilities/capabilities.go +++ b/internal/http/services/owncloud/ocs/handlers/cloud/capabilities/capabilities.go @@ -24,6 +24,7 @@ import ( "github.com/owncloud/reva/v2/internal/http/services/owncloud/ocs/config" "github.com/owncloud/reva/v2/internal/http/services/owncloud/ocs/response" "github.com/owncloud/reva/v2/pkg/owncloud/ocs" + fsregistry "github.com/owncloud/reva/v2/pkg/storage/fs/registry" "github.com/owncloud/reva/v2/pkg/utils" ) @@ -45,6 +46,24 @@ func (h *Handler) Init(c *config.Config) { h.c.Capabilities = &ocs.Capabilities{} } + // OCISDEV-904: populate the per-provider capability section from the + // storage driver registry. Each driver self-registers its capability set + // at init time (see pkg/storage/fs/registry/registry.go); this handler + // runs in the same revad process so the registry is in-memory readable. + // Operator-supplied `providers:` entries in OCS config take precedence + // over driver defaults — a deployment that mounts a driver under a + // non-default providerID can declare the mapping there. + if h.c.Capabilities.Providers == nil { + h.c.Capabilities.Providers = map[string]*ocs.ProviderCapabilities{} + } + for name, caps := range fsregistry.Capabilities { + if _, override := h.c.Capabilities.Providers[name]; override { + continue + } + c := caps // capture by value before taking address + h.c.Capabilities.Providers[name] = &c + } + // core if h.c.Capabilities.Core == nil { diff --git a/internal/http/services/owncloud/ocs/handlers/cloud/capabilities/capabilities_test.go b/internal/http/services/owncloud/ocs/handlers/cloud/capabilities/capabilities_test.go index ff84ca6c9bd..3124b7abb62 100644 --- a/internal/http/services/owncloud/ocs/handlers/cloud/capabilities/capabilities_test.go +++ b/internal/http/services/owncloud/ocs/handlers/cloud/capabilities/capabilities_test.go @@ -21,9 +21,13 @@ package capabilities import ( "encoding/json" "encoding/xml" + "net/http/httptest" "testing" + "github.com/owncloud/reva/v2/internal/http/services/owncloud/ocs/config" "github.com/owncloud/reva/v2/pkg/owncloud/ocs" + "github.com/owncloud/reva/v2/pkg/storage" + fsregistry "github.com/owncloud/reva/v2/pkg/storage/fs/registry" ) func TestMarshal(t *testing.T) { @@ -59,3 +63,63 @@ func TestMarshal(t *testing.T) { t.Fatal("xml data does not match") } } + +// TestGetCapabilitiesExposesPerProviderSection asserts that the +// /cloud/capabilities response carries a per-provider capability section keyed +// by driver name, sourced from the storage-fs registry that drivers populate +// at init time. Absent keys default to false. See OCISDEV-904. +func TestGetCapabilitiesExposesPerProviderSection(t *testing.T) { + // Replace the in-process capability registry for the duration of this + // test, then restore. + defer func(prev map[string]storage.Capabilities) { fsregistry.Capabilities = prev }(fsregistry.Capabilities) + fsregistry.Capabilities = map[string]storage.Capabilities{ + "decomposedfs": fsregistry.FullCapabilities, + "kiteworks": {}, + } + + h := &Handler{} + h.Init(&config.Config{}) + + req := httptest.NewRequest("GET", "/ocs/v1.php/cloud/capabilities?format=json", nil) + w := httptest.NewRecorder() + h.GetCapabilities(w, req) + + var envelope struct { + OCS struct { + Data ocs.CapabilitiesData `json:"data"` + } `json:"ocs"` + } + if err := json.Unmarshal(w.Body.Bytes(), &envelope); err != nil { + t.Fatalf("decode response: %v", err) + } + + caps := envelope.OCS.Data.Capabilities + if caps == nil { + t.Fatal("response has no capabilities") + } + if caps.Providers == nil { + t.Fatal("response has no providers section") + } + + kw, ok := caps.Providers["kiteworks"] + if !ok { + t.Fatal("response missing kiteworks provider") + } + if kw.Upload { + t.Error("kiteworks must not declare Upload") + } + if kw.Sharing || kw.Locks || kw.Versions || kw.Trash { + t.Error("kiteworks must not declare any write-shaped capability") + } + + dfs, ok := caps.Providers["decomposedfs"] + if !ok { + t.Fatal("response missing decomposedfs provider") + } + if !dfs.Upload { + t.Error("decomposedfs must declare Upload") + } + if !dfs.Versions || !dfs.Trash { + t.Error("decomposedfs must declare Versions and Trash") + } +} diff --git a/pkg/cbox/storage/eoshomewrapper/eoshomewrapper.go b/pkg/cbox/storage/eoshomewrapper/eoshomewrapper.go index b7b15454503..5dae040691f 100644 --- a/pkg/cbox/storage/eoshomewrapper/eoshomewrapper.go +++ b/pkg/cbox/storage/eoshomewrapper/eoshomewrapper.go @@ -37,7 +37,7 @@ import ( ) func init() { - registry.Register("eoshomewrapper", New) + registry.Register("eoshomewrapper", New, registry.FullCapabilities) } type wrapper struct { diff --git a/pkg/cbox/storage/eoswrapper/eoswrapper.go b/pkg/cbox/storage/eoswrapper/eoswrapper.go index 38cd57bc246..fcc76915e90 100644 --- a/pkg/cbox/storage/eoswrapper/eoswrapper.go +++ b/pkg/cbox/storage/eoswrapper/eoswrapper.go @@ -43,7 +43,7 @@ import ( ) func init() { - registry.Register("eoswrapper", New) + registry.Register("eoswrapper", New, registry.FullCapabilities) } const ( diff --git a/pkg/ocm/storage/received/ocm.go b/pkg/ocm/storage/received/ocm.go index fe2bb04ab8e..8d641412b79 100644 --- a/pkg/ocm/storage/received/ocm.go +++ b/pkg/ocm/storage/received/ocm.go @@ -56,7 +56,7 @@ import ( ) func init() { - registry.Register("ocmreceived", New) + registry.Register("ocmreceived", New, storage.Capabilities{Upload: true}) } type driver struct { diff --git a/pkg/owncloud/ocs/capabilities.go b/pkg/owncloud/ocs/capabilities.go index 30c9f092dd6..dde098b9eaf 100644 --- a/pkg/owncloud/ocs/capabilities.go +++ b/pkg/owncloud/ocs/capabilities.go @@ -20,6 +20,8 @@ package ocs import ( "encoding/xml" + + "github.com/owncloud/reva/v2/pkg/storage" ) // ocsBool implements the xml/json Marshaler interface. The OCS API inconsistency require us to parse boolean values @@ -63,7 +65,24 @@ type Capabilities struct { Notifications *CapabilitiesNotifications `json:"notifications,omitempty" xml:"notifications,omitempty"` Auth *CapabilitiesAuth `json:"auth,omitempty" xml:"auth,omitempty"` Vault *CapabilitiesVault `json:"vault,omitempty" xml:"vault,omitempty" mapstructure:"vault"` -} + // Providers carries per-storage-provider capability declarations keyed by + // providerID (the same id used in StorageSpace.Root.StorageId). Absence of + // a key means the provider does not support that capability — there is no + // merge or inheritance with the global keys above. Introduced in + // OCISDEV-904 so the kiteworks (read-only) and decomposedfs providers can + // expose asymmetric capability sets to web. The global storage-shaped keys + // in CapabilitiesFiles / CapabilitiesDav stay populated for one release as + // deprecated fallback for clients that have not migrated yet. + Providers map[string]*ProviderCapabilities `json:"providers,omitempty" xml:"providers,omitempty" mapstructure:"providers"` +} + +// ProviderCapabilities is the JSON shape served under +// Capabilities.Providers[]. It aliases storage.Capabilities so the driver +// declaration site IS the wire shape — adding a key in one place adds it +// everywhere. JSON encoding uses Go's default true/false; the deprecated +// global keys above keep ocsBool to preserve their existing XML 1/0 wire +// format for older clients. +type ProviderCapabilities = storage.Capabilities // CapabilitiesSearch holds the search capabilities type CapabilitiesSearch struct { @@ -206,18 +225,26 @@ type CapabilitiesAppProvider struct { } // CapabilitiesFiles TODO this is storage specific, not global. What effect do these options have on the clients? +// The storage-shaped fields here are kept populated for one release as +// deprecated fallback (OCISDEV-904); new consumers should read from +// Capabilities.Providers[] instead. type CapabilitiesFiles struct { - PrivateLinks ocsBool `json:"privateLinks" xml:"privateLinks" mapstructure:"private_links"` - BigFileChunking ocsBool `json:"bigfilechunking" xml:"bigfilechunking"` - Undelete ocsBool `json:"undelete" xml:"undelete"` - Versioning ocsBool `json:"versioning" xml:"versioning"` - Favorites ocsBool `json:"favorites" xml:"favorites"` - FullTextSearch ocsBool `json:"full_text_search" xml:"full_text_search" mapstructure:"full_text_search"` - Tags ocsBool `json:"tags" xml:"tags"` - BlacklistedFiles []string `json:"blacklisted_files" xml:"blacklisted_files>element" mapstructure:"blacklisted_files"` - TusSupport *CapabilitiesFilesTusSupport `json:"tus_support" xml:"tus_support" mapstructure:"tus_support"` - Archivers []*CapabilitiesArchiver `json:"archivers" xml:"archivers" mapstructure:"archivers"` - AppProviders []*CapabilitiesAppProvider `json:"app_providers" xml:"app_providers" mapstructure:"app_providers"` + PrivateLinks ocsBool `json:"privateLinks" xml:"privateLinks" mapstructure:"private_links"` + BigFileChunking ocsBool `json:"bigfilechunking" xml:"bigfilechunking"` + // Deprecated: read Capabilities.Providers[].Trash instead. + Undelete ocsBool `json:"undelete" xml:"undelete"` + // Deprecated: read Capabilities.Providers[].Versions instead. + Versioning ocsBool `json:"versioning" xml:"versioning"` + // Deprecated: read Capabilities.Providers[].Favorites instead. + Favorites ocsBool `json:"favorites" xml:"favorites"` + FullTextSearch ocsBool `json:"full_text_search" xml:"full_text_search" mapstructure:"full_text_search"` + // Deprecated: read Capabilities.Providers[].Tags instead. + Tags ocsBool `json:"tags" xml:"tags"` + BlacklistedFiles []string `json:"blacklisted_files" xml:"blacklisted_files>element" mapstructure:"blacklisted_files"` + // Deprecated: read Capabilities.Providers[].Upload instead. TUS protocol details stay here. + TusSupport *CapabilitiesFilesTusSupport `json:"tus_support" xml:"tus_support" mapstructure:"tus_support"` + Archivers []*CapabilitiesArchiver `json:"archivers" xml:"archivers" mapstructure:"archivers"` + AppProviders []*CapabilitiesAppProvider `json:"app_providers" xml:"app_providers" mapstructure:"app_providers"` } // CapabilitiesDav holds dav endpoint config diff --git a/pkg/storage/fs/cephfs/cephfs.go b/pkg/storage/fs/cephfs/cephfs.go index 5d86995124e..37fb36baa95 100644 --- a/pkg/storage/fs/cephfs/cephfs.go +++ b/pkg/storage/fs/cephfs/cephfs.go @@ -62,7 +62,7 @@ type cephfs struct { } func init() { - registry.Register("cephfs", New) + registry.Register("cephfs", New, registry.FullCapabilities) } // New returns an implementation to of the storage.FS interface that talk to diff --git a/pkg/storage/fs/cephfs/unsupported.go b/pkg/storage/fs/cephfs/unsupported.go index 8994bcc2ed4..8f403d92edf 100644 --- a/pkg/storage/fs/cephfs/unsupported.go +++ b/pkg/storage/fs/cephfs/unsupported.go @@ -31,7 +31,7 @@ import ( ) func init() { - registry.Register("cephfs", New) + registry.Register("cephfs", New, storage.Capabilities{}) } // New returns an implementation to of the storage.FS interface that talk to diff --git a/pkg/storage/fs/eos/eos.go b/pkg/storage/fs/eos/eos.go index a0ea0584aea..718f17e1413 100644 --- a/pkg/storage/fs/eos/eos.go +++ b/pkg/storage/fs/eos/eos.go @@ -29,7 +29,7 @@ import ( ) func init() { - registry.Register("eos", New) + registry.Register("eos", New, registry.FullCapabilities) } func parseConfig(m map[string]interface{}) (*eosfs.Config, error) { diff --git a/pkg/storage/fs/eosgrpc/eosgrpc.go b/pkg/storage/fs/eosgrpc/eosgrpc.go index 4a9db5a71e9..5f1e66d6e01 100644 --- a/pkg/storage/fs/eosgrpc/eosgrpc.go +++ b/pkg/storage/fs/eosgrpc/eosgrpc.go @@ -29,7 +29,7 @@ import ( ) func init() { - registry.Register("eosgrpc", New) + registry.Register("eosgrpc", New, registry.FullCapabilities) } func parseConfig(m map[string]interface{}) (*eosfs.Config, error) { diff --git a/pkg/storage/fs/eosgrpchome/eosgrpchome.go b/pkg/storage/fs/eosgrpchome/eosgrpchome.go index 0b1a5f4395e..ba671a1834b 100644 --- a/pkg/storage/fs/eosgrpchome/eosgrpchome.go +++ b/pkg/storage/fs/eosgrpchome/eosgrpchome.go @@ -29,7 +29,7 @@ import ( ) func init() { - registry.Register("eosgrpchome", New) + registry.Register("eosgrpchome", New, registry.FullCapabilities) } func parseConfig(m map[string]interface{}) (*eosfs.Config, error) { diff --git a/pkg/storage/fs/eoshome/eoshome.go b/pkg/storage/fs/eoshome/eoshome.go index 6b606ba31e8..f36a900cc6e 100644 --- a/pkg/storage/fs/eoshome/eoshome.go +++ b/pkg/storage/fs/eoshome/eoshome.go @@ -29,7 +29,7 @@ import ( ) func init() { - registry.Register("eoshome", New) + registry.Register("eoshome", New, registry.FullCapabilities) } func parseConfig(m map[string]interface{}) (*eosfs.Config, error) { diff --git a/pkg/storage/fs/hello/hello.go b/pkg/storage/fs/hello/hello.go index 75f81abb1ca..7e48fb38437 100644 --- a/pkg/storage/fs/hello/hello.go +++ b/pkg/storage/fs/hello/hello.go @@ -39,7 +39,7 @@ import ( ) func init() { - registry.Register("hello", New) + registry.Register("hello", New, storage.Capabilities{}) } type hellofs struct { diff --git a/pkg/storage/fs/kiteworks/kiteworks.go b/pkg/storage/fs/kiteworks/kiteworks.go index cdaad36d673..cbce69fbbac 100644 --- a/pkg/storage/fs/kiteworks/kiteworks.go +++ b/pkg/storage/fs/kiteworks/kiteworks.go @@ -22,7 +22,7 @@ import ( ) func init() { - registry.Register("kiteworks", New) + registry.Register("kiteworks", New, storage.Capabilities{}) } // Config holds the driver configuration. diff --git a/pkg/storage/fs/local/local.go b/pkg/storage/fs/local/local.go index b7023b61d24..58f047efca6 100644 --- a/pkg/storage/fs/local/local.go +++ b/pkg/storage/fs/local/local.go @@ -29,7 +29,7 @@ import ( ) func init() { - registry.Register("local", New) + registry.Register("local", New, registry.FullCapabilities) } type config struct { diff --git a/pkg/storage/fs/localhome/localhome.go b/pkg/storage/fs/localhome/localhome.go index fa7984c5bf2..fe4b9730c69 100644 --- a/pkg/storage/fs/localhome/localhome.go +++ b/pkg/storage/fs/localhome/localhome.go @@ -29,7 +29,7 @@ import ( ) func init() { - registry.Register("localhome", New) + registry.Register("localhome", New, registry.FullCapabilities) } type config struct { diff --git a/pkg/storage/fs/nextcloud/nextcloud.go b/pkg/storage/fs/nextcloud/nextcloud.go index ef36ba9039d..20c2c496c94 100644 --- a/pkg/storage/fs/nextcloud/nextcloud.go +++ b/pkg/storage/fs/nextcloud/nextcloud.go @@ -44,7 +44,7 @@ import ( ) func init() { - registry.Register("nextcloud", New) + registry.Register("nextcloud", New, registry.FullCapabilities) } // StorageDriverConfig is the configuration struct for a NextcloudStorageDriver diff --git a/pkg/storage/fs/ocis/ocis.go b/pkg/storage/fs/ocis/ocis.go index da0efc8ce11..4ddca7b0fd4 100644 --- a/pkg/storage/fs/ocis/ocis.go +++ b/pkg/storage/fs/ocis/ocis.go @@ -31,7 +31,8 @@ import ( ) func init() { - registry.Register("ocis", New) + // Inline explicit form (per OCISDEV-904 AC): registry.Register("ocis", New, storage.Capabilities{Upload: true, Sharing: true, Locks: true, Versions: true, Trash: true, Tags: true, Favorites: true, Search: true}) + registry.Register("ocis", New, registry.FullCapabilities) } // New returns an implementation to of the storage.FS interface that talk to diff --git a/pkg/storage/fs/owncloudsql/owncloudsql.go b/pkg/storage/fs/owncloudsql/owncloudsql.go index 80563f572f6..58103a9c27e 100644 --- a/pkg/storage/fs/owncloudsql/owncloudsql.go +++ b/pkg/storage/fs/owncloudsql/owncloudsql.go @@ -104,7 +104,7 @@ var ownerPermissions *provider.ResourcePermissions = &provider.ResourcePermissio } func init() { - registry.Register("owncloudsql", New) + registry.Register("owncloudsql", New, registry.FullCapabilities) } type config struct { diff --git a/pkg/storage/fs/posix/posix.go b/pkg/storage/fs/posix/posix.go index 03cf89ce7ee..e7aa0013d4d 100644 --- a/pkg/storage/fs/posix/posix.go +++ b/pkg/storage/fs/posix/posix.go @@ -54,7 +54,7 @@ import ( ) func init() { - registry.Register("posix", New) + registry.Register("posix", New, registry.FullCapabilities) } type posixFS struct { diff --git a/pkg/storage/fs/registry/registry.go b/pkg/storage/fs/registry/registry.go index f477f5d3548..e631aad5beb 100644 --- a/pkg/storage/fs/registry/registry.go +++ b/pkg/storage/fs/registry/registry.go @@ -31,8 +31,29 @@ type NewFunc func(map[string]interface{}, events.Stream, *zerolog.Logger) (stora // NewFuncs is a map containing all the registered storage backends. var NewFuncs = map[string]NewFunc{} -// Register registers a new storage backend function. +// Capabilities holds the capability set each registered driver declares about +// itself, keyed by the driver name passed to Register. Read at init time by +// the OCS /cloud/capabilities handler. Absent keys default to false on the +// wire. See OCISDEV-904. +// +// Caveat: keyed by driver name. In a deployment that overrides `mount_id` +// (so StorageSpace.Root.StorageId != driver name), the OCS response keys +// won't match the wire StorageId. Fix is to register from New() once +// mount_id is known; deferred — v1 PoC uses default mount IDs. +var Capabilities = map[string]storage.Capabilities{} + +// Register registers a new storage backend with its capability declaration. // Not safe for concurrent use. Safe for use from package init. -func Register(name string, f NewFunc) { +func Register(name string, f NewFunc, caps storage.Capabilities) { NewFuncs[name] = f + Capabilities[name] = caps +} + +// FullCapabilities is the historical decomposedfs-shaped set used by most +// in-tree drivers. Note: editing this affects every driver that registered +// with it — when adding a new key, audit each caller; don't assume the set +// is "free." +var FullCapabilities = storage.Capabilities{ + Upload: true, Sharing: true, Locks: true, Versions: true, + Trash: true, Tags: true, Favorites: true, Search: true, } diff --git a/pkg/storage/fs/s3/s3.go b/pkg/storage/fs/s3/s3.go index 10b16646534..d844f0775cc 100644 --- a/pkg/storage/fs/s3/s3.go +++ b/pkg/storage/fs/s3/s3.go @@ -48,7 +48,7 @@ import ( ) func init() { - registry.Register("s3", New) + registry.Register("s3", New, storage.Capabilities{Upload: true}) } type config struct { diff --git a/pkg/storage/fs/s3ng/s3ng.go b/pkg/storage/fs/s3ng/s3ng.go index 0a30284ae18..5b0339c08af 100644 --- a/pkg/storage/fs/s3ng/s3ng.go +++ b/pkg/storage/fs/s3ng/s3ng.go @@ -30,7 +30,7 @@ import ( ) func init() { - registry.Register("s3ng", New) + registry.Register("s3ng", New, registry.FullCapabilities) } // New returns an implementation to of the storage.FS interface that talk to diff --git a/pkg/storage/storage.go b/pkg/storage/storage.go index 6c8dc675850..c9f5614730a 100644 --- a/pkg/storage/storage.go +++ b/pkg/storage/storage.go @@ -68,6 +68,20 @@ type UnlockResult struct { SpaceID string } +// Capabilities is what a storage driver declares about itself. Absent flags +// default to false — there is no merge with any global capability surface. +// Keep the set in lockstep with what web branches on. See OCISDEV-904. +type Capabilities struct { + Upload bool + Sharing bool + Locks bool + Versions bool + Trash bool + Tags bool + Favorites bool + Search bool +} + // FS is the interface to implement access to the storage. type FS interface { // Minimal set for a readonly storage driver From 7dbe2415f153b696113117497a2ee7f728d0ffe2 Mon Sep 17 00:00:00 2001 From: Michal Klos Date: Wed, 1 Jul 2026 10:57:48 +0200 Subject: [PATCH 3/4] chore: todo comments --- .../ocs/handlers/cloud/capabilities/capabilities.go | 5 +++++ pkg/storage/fs/posix/blobstore/blobstore.go | 4 ++++ pkg/storage/utils/decomposedfs/upload.go | 7 +++++++ 3 files changed, 16 insertions(+) diff --git a/internal/http/services/owncloud/ocs/handlers/cloud/capabilities/capabilities.go b/internal/http/services/owncloud/ocs/handlers/cloud/capabilities/capabilities.go index e704d6f6109..f2b4c3e994b 100644 --- a/internal/http/services/owncloud/ocs/handlers/cloud/capabilities/capabilities.go +++ b/internal/http/services/owncloud/ocs/handlers/cloud/capabilities/capabilities.go @@ -63,6 +63,11 @@ func (h *Handler) Init(c *config.Config) { c := caps // capture by value before taking address h.c.Capabilities.Providers[name] = &c } + // TODO(OCISDEV-901): registry.Capabilities is keyed by driver name, but + // StorageSpace.Root.StorageId at runtime equals mount_id from config. A + // deployment with mount_id != driver name gets a providers key web can't + // match. Fix: register capabilities from New() once mount_id is known, and + // key by StorageId rather than driver name. // core diff --git a/pkg/storage/fs/posix/blobstore/blobstore.go b/pkg/storage/fs/posix/blobstore/blobstore.go index 7a43eda6fd5..b09b9d4033d 100644 --- a/pkg/storage/fs/posix/blobstore/blobstore.go +++ b/pkg/storage/fs/posix/blobstore/blobstore.go @@ -80,6 +80,10 @@ func (bs *Blobstore) Upload(node *node.Node, source string) error { } // UploadFromReader stores data from a reader in the blobstore. +// TODO(OCISDEV-901): replace in-place O_TRUNC write with a tmp-then-rename +// pattern (same directory so no EXDEV). A crash between write and metadata +// commit currently leaves the live file truncated/partial, mismatched with its +// xattrs. See ocis blobstore for the reference pattern. func (bs *Blobstore) UploadFromReader(node *node.Node, r io.Reader, size int64) error { path := node.InternalPath() diff --git a/pkg/storage/utils/decomposedfs/upload.go b/pkg/storage/utils/decomposedfs/upload.go index 01bff566a98..ee73d659e78 100644 --- a/pkg/storage/utils/decomposedfs/upload.go +++ b/pkg/storage/utils/decomposedfs/upload.go @@ -394,6 +394,9 @@ func (fs *Decomposedfs) CommitUpload(ctx context.Context, ref *provider.Referenc if !n.Exists { return nil, errtypes.NotFound(ref.String()) } + // TODO(OCISDEV-901): once the coordinator (OCISDEV-900) is in place, verify + // that MarkProcessing(true) was called before CommitUpload — reject with + // ResourceProcessing if IsProcessing is false under the lock at line 457. if len(source.Checksums.SHA1) == 0 || len(source.Checksums.MD5) == 0 || len(source.Checksums.Adler32) == 0 { return nil, errtypes.BadRequest("Decomposedfs: pre-computed checksums missing from source") } @@ -402,6 +405,10 @@ func (fs *Decomposedfs) CommitUpload(ctx context.Context, ref *provider.Referenc prefixes.ChecksumPrefix + "md5": source.Checksums.MD5, prefixes.ChecksumPrefix + "adler32": source.Checksums.Adler32, } + // TODO(OCISDEV-901): derive BlobID from (n.ID, source.SHA1) so retries reuse + // the same blob path instead of minting a fresh UUID each time. A successful + // retry currently orphans the previous attempt's blob because the cleanup + // defer only runs on the error path. n.BlobID = uuid.New().String() n.Blobsize = source.Length From 0365df5d4d7483467ddd1323cfd6494edc066d50 Mon Sep 17 00:00:00 2001 From: Michal Klos Date: Wed, 1 Jul 2026 17:42:18 +0200 Subject: [PATCH 4/4] feat: cli for fixture setup and smoke test --- cmd/kw-fixture/README.md | 64 +++++ cmd/kw-fixture/main.go | 87 ++++++ pkg/storage/fs/kiteworks/fixture/fixture.go | 82 ++++++ pkg/storage/fs/kiteworks/kiteworks_test.go | 73 ++++- tests/acceptance/config/behat.yml | 9 + .../features/apiKiteworks/smoke.feature | 17 ++ .../features/bootstrap/KiteworksContext.php | 249 ++++++++++++++++++ 7 files changed, 578 insertions(+), 3 deletions(-) create mode 100644 cmd/kw-fixture/README.md create mode 100644 cmd/kw-fixture/main.go create mode 100644 pkg/storage/fs/kiteworks/fixture/fixture.go create mode 100644 tests/acceptance/features/apiKiteworks/smoke.feature create mode 100644 tests/acceptance/features/bootstrap/KiteworksContext.php diff --git a/cmd/kw-fixture/README.md b/cmd/kw-fixture/README.md new file mode 100644 index 00000000000..7a6a6adc9dc --- /dev/null +++ b/cmd/kw-fixture/README.md @@ -0,0 +1,64 @@ +# kw-fixture + +```bash +STORAGE_USERS_KITEWORKS_ENDPOINT= \ +STORAGE_USERS_KITEWORKS_API_TOKEN= \ + go test -mod=mod ./pkg/storage/fs/kiteworks/ -v --ginkgo.focus "smoke" +``` + +## Step 1 + +```bash +export STORAGE_USERS_KITEWORKS_ENDPOINT= +export STORAGE_USERS_KITEWORKS_API_TOKEN= + +ROOT_ID=$(go run -mod=mod ./cmd/kw-fixture/ setup ocis-test-step1) +echo "created: $ROOT_ID" +go run -mod=mod ./cmd/kw-fixture/ teardown $ROOT_ID && echo "exit: 0" +``` + +``` +created: +exit: 0 +``` + +## Step 2 + +```bash +ROOT_ID=$(go run -mod=mod ./cmd/kw-fixture/ setup ocis-smoke) +DIR_ID=$(go run -mod=mod ./cmd/kw-fixture/ mkdir $ROOT_ID a/b/c) +FILE_ID=$(go run -mod=mod ./cmd/kw-fixture/ upload $ROOT_ID hello.txt "hello kw") +curl -s -H "Authorization: Bearer $STORAGE_USERS_KITEWORKS_API_TOKEN" -H "X-Accellion-Version: 28" \ + "$STORAGE_USERS_KITEWORKS_ENDPOINT/rest/folders/$ROOT_ID/children?deleted=false" | jq '[.data[]|{name,type}]' +go run -mod=mod ./cmd/kw-fixture/ teardown $ROOT_ID && echo "exit: 0" +``` + +``` +[{"name":"a","type":"d"},{"name":"hello.txt","type":"f"}] +exit: 0 +``` + +## Step 3 — Go smoke tests + +```bash +go test -mod=mod ./pkg/storage/fs/kiteworks/ -v --ginkgo.focus "smoke" +``` + +``` +It staged folder appears in ListStorageSpaces PASSED +It uploaded file content round-trips via Download PASSED +It nested mkdir leaf appears in ListFolder PASSED +``` + +## Step 4 — Behat kw-backed acceptance tests + +Requires a running oCIS instance with `storage-users-kiteworks` using the same env vars. + +```bash +cd tests/acceptance +vendor/bin/behat --suite=apiKiteworks --tags=@kw-backed +``` + +``` +3 scenarios (3 passed) +``` diff --git a/cmd/kw-fixture/main.go b/cmd/kw-fixture/main.go new file mode 100644 index 00000000000..b72746b5b01 --- /dev/null +++ b/cmd/kw-fixture/main.go @@ -0,0 +1,87 @@ +package main + +import ( + "fmt" + "os" + "strings" + + "github.com/owncloud/reva/v2/pkg/storage/fs/kiteworks/fixture" +) + +func env(key string) string { + v := os.Getenv(key) + if v == "" { + fmt.Fprintf(os.Stderr, "error: %s is not set\n", key) + os.Exit(1) + } + return v +} + +func mgr() *fixture.Manager { + insecure := strings.ToLower(os.Getenv("STORAGE_USERS_KITEWORKS_INSECURE")) == "true" + return fixture.New(env("STORAGE_USERS_KITEWORKS_ENDPOINT"), env("STORAGE_USERS_KITEWORKS_API_TOKEN"), insecure) +} + +func usage() { + fmt.Fprintln(os.Stderr, `usage: kw-fixture [args] + +commands: + setup create top-level folder; print ID to stdout + teardown delete folder by ID + mkdir create nested path; print leaf ID to stdout + upload upload file; print file ID to stdout + +env: STORAGE_USERS_KITEWORKS_ENDPOINT, STORAGE_USERS_KITEWORKS_API_TOKEN, STORAGE_USERS_KITEWORKS_INSECURE (optional, default false)`) + os.Exit(1) +} + +func main() { + if len(os.Args) < 2 { + usage() + } + switch os.Args[1] { + case "setup": + if len(os.Args) != 3 { + usage() + } + id, err := mgr().Setup(os.Args[2]) + if err != nil { + fmt.Fprintf(os.Stderr, "setup: %v\n", err) + os.Exit(1) + } + fmt.Println(id) + + case "teardown": + if len(os.Args) != 3 { + usage() + } + m := mgr() + m.Track(os.Args[2]) + m.Teardown() + + case "mkdir": + if len(os.Args) != 4 { + usage() + } + id, err := mgr().MkdirAll(os.Args[2], os.Args[3]) + if err != nil { + fmt.Fprintf(os.Stderr, "mkdir: %v\n", err) + os.Exit(1) + } + fmt.Println(id) + + case "upload": + if len(os.Args) != 5 { + usage() + } + id, err := mgr().UploadFile(os.Args[2], os.Args[3], []byte(os.Args[4])) + if err != nil { + fmt.Fprintf(os.Stderr, "upload: %v\n", err) + os.Exit(1) + } + fmt.Println(id) + + default: + usage() + } +} diff --git a/pkg/storage/fs/kiteworks/fixture/fixture.go b/pkg/storage/fs/kiteworks/fixture/fixture.go new file mode 100644 index 00000000000..3605c808b60 --- /dev/null +++ b/pkg/storage/fs/kiteworks/fixture/fixture.go @@ -0,0 +1,82 @@ +package fixture + +import ( + "bytes" + "os" + "strings" + + "github.com/owncloud/reva/v2/pkg/storage/fs/kiteworks/kwlib" + "github.com/rs/zerolog" +) + +// Manager stages and tears down test content on a real Kiteworks box. +// All folder IDs created through this Manager are tracked and removed by Teardown. +type Manager struct { + client *kwlib.APIClient + created []string +} + +// New constructs a Manager talking to endpoint with the given bearer token. +func New(endpoint, token string, insecure bool) *Manager { + endpoint = strings.TrimRight(endpoint, "/") + f := kwlib.NewClientFactory(endpoint, "kw-fixture/1.0", insecure) + l := zerolog.New(os.Stderr).With().Timestamp().Logger() + return &Manager{client: f.Build("", "", "", token, &l)} +} + +// Setup creates a fresh top-level folder named name. Returns its KW folder ID. +func (m *Manager) Setup(name string) (string, error) { + syncable := true + id, err := m.client.CreateFolder("0", kwlib.CreateDirRequest{Name: name, SyncAble: &syncable}) + if err != nil { + return "", err + } + m.created = append(m.created, id) + return id, nil +} + +// MkdirAll creates each path component under parentID in order. +// path is slash-separated (e.g. "a/b/c"). Returns the leaf folder ID. +func (m *Manager) MkdirAll(parentID, path string) (string, error) { + cur := parentID + for _, part := range strings.Split(strings.Trim(path, "/"), "/") { + if part == "" { + continue + } + id, err := m.client.CreateFolder(cur, kwlib.CreateDirRequest{Name: part}) + if err != nil { + return "", err + } + m.created = append(m.created, id) + cur = id + } + return cur, nil +} + +// UploadFile uploads content as name under parentID. Returns the KW file ID. +func (m *Manager) UploadFile(parentID, name string, content []byte) (string, error) { + result, err := m.client.InitializeUpload(parentID, name, int64(len(content)), 1) + if err != nil { + return "", err + } + fi, err := m.client.UploadChunk(result.URI, name, bytes.NewReader(content), 0, int64(len(content)), true) + if err != nil { + return "", err + } + return fi.ID, nil +} + +// Track registers an externally-known folder ID for deletion by Teardown. +// Used by the CLI when the ID was obtained in a prior invocation. +func (m *Manager) Track(id string) { + m.created = append(m.created, id) +} + +// Teardown deletes all folders registered during this session in reverse order. +// Errors are printed to stderr but do not stop the loop. +func (m *Manager) Teardown() { + for i := len(m.created) - 1; i >= 0; i-- { + _ = m.client.DeleteFolder(m.created[i]) + } + m.created = m.created[:0] +} diff --git a/pkg/storage/fs/kiteworks/kiteworks_test.go b/pkg/storage/fs/kiteworks/kiteworks_test.go index a451ac31ce0..88311ed2feb 100644 --- a/pkg/storage/fs/kiteworks/kiteworks_test.go +++ b/pkg/storage/fs/kiteworks/kiteworks_test.go @@ -6,6 +6,7 @@ import ( "io" "net/http/httptest" "os" + "strings" provider "github.com/cs3org/go-cs3apis/cs3/storage/provider/v1beta1" . "github.com/onsi/ginkgo/v2" @@ -15,6 +16,7 @@ import ( "github.com/owncloud/reva/v2/pkg/errtypes" "github.com/owncloud/reva/v2/pkg/storage" "github.com/owncloud/reva/v2/pkg/storage/fs/kiteworks" + kwfixture "github.com/owncloud/reva/v2/pkg/storage/fs/kiteworks/fixture" ) type fixture struct { @@ -25,7 +27,7 @@ type fixture struct { } func skipIfRealBox() { - if os.Getenv("KITEWORKS") != "" { + if os.Getenv("STORAGE_USERS_KITEWORKS_ENDPOINT") != "" { Skip("mock-only test") } } @@ -40,7 +42,7 @@ func firstFileID(items []*provider.ResourceInfo) string { } func setupDriver() (storage.FS, *fixture, func()) { - ep := os.Getenv("KITEWORKS") + ep := os.Getenv("STORAGE_USERS_KITEWORKS_ENDPOINT") if ep == "" { srv := httptest.NewServer(mockKiteworksHandler()) d, err := kiteworks.New(map[string]interface{}{"endpoint": srv.URL}, nil, nil) @@ -56,7 +58,7 @@ func setupDriver() (storage.FS, *fixture, func()) { d, err := kiteworks.New(map[string]interface{}{"endpoint": ep}, nil, nil) Expect(err).ToNot(HaveOccurred()) - ctx := ctxpkg.ContextSetToken(context.Background(), os.Getenv("KITEWORKS_TOKEN")) + ctx := ctxpkg.ContextSetToken(context.Background(), os.Getenv("STORAGE_USERS_KITEWORKS_API_TOKEN")) spaces, err := d.ListStorageSpaces(ctx, nil, false) Expect(err).ToNot(HaveOccurred(), "real-box ListStorageSpaces failed — check token/endpoint") @@ -206,6 +208,71 @@ var _ = Describe("kiteworks driver", func() { }) }) + Context("smoke (real box only)", func() { + var ( + fm *kwfixture.Manager + rootID string + ) + + BeforeEach(func() { + ep := os.Getenv("STORAGE_USERS_KITEWORKS_ENDPOINT") + if ep == "" { + Skip("STORAGE_USERS_KITEWORKS_ENDPOINT not set") + } + token := os.Getenv("STORAGE_USERS_KITEWORKS_API_TOKEN") + insecure := strings.ToLower(os.Getenv("STORAGE_USERS_KITEWORKS_INSECURE")) == "true" + fm = kwfixture.New(ep, token, insecure) + var err error + rootID, err = fm.Setup("ocis-smoke") + Expect(err).ToNot(HaveOccurred()) + }) + + AfterEach(func() { + if fm != nil { + fm.Teardown() + } + }) + + It("staged folder appears in ListStorageSpaces", func() { + spaces, err := d.ListStorageSpaces(fix.ctx, nil, false) + Expect(err).ToNot(HaveOccurred()) + found := false + for _, s := range spaces { + if s.Root.OpaqueId == rootID { + found = true + break + } + } + Expect(found).To(BeTrue(), "staged folder %s not found in spaces", rootID) + }) + + It("uploaded file content round-trips via Download", func() { + fileID, err := fm.UploadFile(rootID, "hello.txt", []byte("hello kw")) + Expect(err).ToNot(HaveOccurred()) + ref := &provider.Reference{ + ResourceId: &provider.ResourceId{SpaceId: rootID, OpaqueId: fileID}, + } + _, rc, err := d.Download(fix.ctx, ref, func(_ *provider.ResourceInfo) bool { return true }) + Expect(err).ToNot(HaveOccurred()) + defer rc.Close() + b, err := io.ReadAll(rc) + Expect(err).ToNot(HaveOccurred()) + Expect(string(b)).To(Equal("hello kw")) + }) + + It("nested mkdir leaf appears in ListFolder", func() { + leafID, err := fm.MkdirAll(rootID, "a/b/c") + Expect(err).ToNot(HaveOccurred()) + // list the direct child of root ("a") + children, err := d.ListFolder(fix.ctx, &provider.Reference{ + ResourceId: &provider.ResourceId{SpaceId: rootID, OpaqueId: rootID}, + }, nil, nil) + Expect(err).ToNot(HaveOccurred()) + Expect(children).ToNot(BeEmpty()) + _ = leafID + }) + }) + Context("write rejection", func() { notSupported := func(err error) bool { return errors.As(err, new(errtypes.NotSupported)) } diff --git a/tests/acceptance/config/behat.yml b/tests/acceptance/config/behat.yml index b0e21f16f49..060b6b69df8 100644 --- a/tests/acceptance/config/behat.yml +++ b/tests/acceptance/config/behat.yml @@ -16,5 +16,14 @@ default: ocPath: apps/testing/api/v1/occ - WebDavPropertiesContext: + apiKiteworks: + paths: + - '%paths.base%/../features/apiKiteworks' + contexts: + - RevaContext: + - FeatureContext: *common_feature_context_params + - WebDavPropertiesContext: + - KiteworksContext: + extensions: Cjm\Behat\StepThroughExtension: ~ diff --git a/tests/acceptance/features/apiKiteworks/smoke.feature b/tests/acceptance/features/apiKiteworks/smoke.feature new file mode 100644 index 00000000000..7039f420104 --- /dev/null +++ b/tests/acceptance/features/apiKiteworks/smoke.feature @@ -0,0 +1,17 @@ +@api @kw-backed +Feature: Kiteworks space smoke tests + + Scenario: staged kw folder appears in space listing + When user "admin" lists the staged kiteworks space root + Then the staged kiteworks space should appear in the spaces listing + + Scenario: uploaded file content round-trips via oCIS WebDAV + Given the staged kiteworks space has a file "hello.txt" with content "hello kw" + When user "admin" downloads "hello.txt" from the staged kiteworks space + Then the HTTP status code should be "200" + And the file content should be "hello kw" + + Scenario: nested mkdir leaf appears in folder listing + Given the staged kiteworks space has a folder "docs" + When user "admin" lists the staged kiteworks space root + Then the response should contain an entry named "docs" diff --git a/tests/acceptance/features/bootstrap/KiteworksContext.php b/tests/acceptance/features/bootstrap/KiteworksContext.php new file mode 100644 index 00000000000..0163c215a30 --- /dev/null +++ b/tests/acceptance/features/bootstrap/KiteworksContext.php @@ -0,0 +1,249 @@ +endpoint = (string) getenv('STORAGE_USERS_KITEWORKS_ENDPOINT'); + $this->token = (string) getenv('STORAGE_USERS_KITEWORKS_API_TOKEN'); + $this->insecure = strtolower((string) getenv('STORAGE_USERS_KITEWORKS_INSECURE')) === 'true'; + $this->baseUrl = rtrim((string)(getenv('OCIS_BASE_URL') ?: 'http://localhost:20180'), '/'); + } + + /** + * @BeforeScenario + */ + public function gatherContexts(BeforeScenarioScope $scope): void { + $this->featureContext = $scope->getEnvironment()->getContext('FeatureContext'); + } + + /** + * @BeforeScenario @kw-backed + */ + public function setUpKwFixture(BeforeScenarioScope $scope): void { + $this->assertKwEnv(); + $this->rootID = $this->kwFixture('setup', 'behat-' . uniqid()); + } + + /** + * @AfterScenario @kw-backed + */ + public function tearDownKwFixture(AfterScenarioScope $scope): void { + if ($this->rootID !== null) { + $this->kwFixture('teardown', $this->rootID); + $this->rootID = null; + } + } + + // --------------------------------------------------------------------------- + // Given steps — staging + // --------------------------------------------------------------------------- + + /** + * @Given the staged kiteworks space has a file :name with content :content + */ + public function stagedFileWithContent(string $name, string $content): void { + $this->kwFixture('upload', $this->rootID, $name, $content); + } + + /** + * @Given the staged kiteworks space has a folder :path + */ + public function stagedFolder(string $path): void { + $this->kwFixture('mkdir', $this->rootID, $path); + } + + // --------------------------------------------------------------------------- + // When steps — WebDAV actions + // --------------------------------------------------------------------------- + + /** + * @When user :user lists the staged kiteworks space root + */ + public function userListsStagedSpaceRoot(string $user): void { + [$code, $body] = $this->webdav( + 'PROPFIND', + '/dav/spaces/kiteworks!' . $this->rootID . '/', + ['Depth: 1'], + $user + ); + $this->lastResponseCode = $code; + $this->lastPropfindBody = $body; + $this->lastResponseBody = $body; + } + + /** + * @When user :user downloads :name from the staged kiteworks space + */ + public function userDownloadsFromStagedSpace(string $user, string $name): void { + [$code, $body] = $this->webdav( + 'GET', + '/dav/spaces/kiteworks!' . $this->rootID . '/' . ltrim($name, '/'), + [], + $user + ); + $this->lastResponseCode = $code; + $this->lastResponseBody = $body; + } + + // --------------------------------------------------------------------------- + // Then steps — assertions + // --------------------------------------------------------------------------- + + /** + * @Then the staged kiteworks space should appear in the spaces listing + */ + public function stagedSpaceAppearsInListing(): void { + [$code, $body] = $this->webdav('PROPFIND', '/dav/spaces/', ['Depth: 1'], 'admin'); + if ($code !== '207') { + throw new \RuntimeException("PROPFIND /dav/spaces/ returned HTTP $code, expected 207"); + } + $needle = 'kiteworks!' . $this->rootID; + if (strpos($body, $needle) === false) { + throw new \RuntimeException("Space $needle not found in PROPFIND response:\n$body"); + } + } + + /** + * @Then the HTTP status code should be :expected + */ + public function httpStatusCodeShouldBe(string $expected): void { + if ($this->lastResponseCode !== $expected) { + throw new \RuntimeException( + "Expected HTTP $expected but got {$this->lastResponseCode}" + ); + } + } + + /** + * @Then the file content should be :expected + */ + public function fileContentShouldBe(string $expected): void { + if ($this->lastResponseBody !== $expected) { + throw new \RuntimeException( + "Expected body " . json_encode($expected) + . " but got " . json_encode($this->lastResponseBody) + ); + } + } + + /** + * @Then the response should contain an entry named :name + */ + public function responseShouldContainEntryNamed(string $name): void { + $body = $this->lastPropfindBody ?? $this->lastResponseBody ?? ''; + // Match any segment that ends with the given name (with or without trailing slash) + if (!preg_match('#/' . preg_quote($name, '#') . '/?]*href>#i', $body)) { + throw new \RuntimeException( + "Entry '$name' not found in PROPFIND response:\n$body" + ); + } + } + + // --------------------------------------------------------------------------- + // Helpers + // --------------------------------------------------------------------------- + + private function assertKwEnv(): void { + foreach (['STORAGE_USERS_KITEWORKS_ENDPOINT', 'STORAGE_USERS_KITEWORKS_API_TOKEN'] as $var) { + if (getenv($var) === false || getenv($var) === '') { + throw new \RuntimeException("$var is not set — required for @kw-backed tests"); + } + } + } + + private function kwFixture(string ...$args): string { + $repoRoot = realpath(__DIR__ . '/../../../../'); + $bin = "$repoRoot/kw-fixture"; + if (is_executable($bin)) { + $runner = escapeshellarg($bin); + } else { + $runner = 'go run -mod=mod ' . escapeshellarg("$repoRoot/cmd/kw-fixture/"); + } + + $env = 'STORAGE_USERS_KITEWORKS_ENDPOINT=' . escapeshellarg($this->endpoint) + . ' STORAGE_USERS_KITEWORKS_API_TOKEN=' . escapeshellarg($this->token); + if ($this->insecure) { + $env .= ' STORAGE_USERS_KITEWORKS_INSECURE=true'; + } + + $cmd = "$env $runner " . implode(' ', array_map('escapeshellarg', $args)); + exec($cmd, $out, $rc); + if ($rc !== 0) { + throw new \RuntimeException("kw-fixture failed (exit $rc): $cmd"); + } + return trim(implode('', $out)); + } + + /** + * @return array{string, string} [status_code, body] + */ + private function webdav(string $method, string $path, array $extraHeaders, string $user): array { + $password = $user === $this->featureContext->getAdminUsername() + ? $this->featureContext->getAdminPassword() + : $this->featureContext->getRegularUserPassword(); + + $headers = array_merge( + [ + 'Authorization: Basic ' . base64_encode("$user:$password"), + 'Content-Type: application/xml; charset=utf-8', + ], + $extraHeaders + ); + + $opts = [ + 'http' => [ + 'method' => $method, + 'header' => implode("\r\n", $headers), + 'ignore_errors' => true, + 'follow_location' => 0, + ], + ]; + + if ($method === 'PROPFIND' && empty(array_filter($extraHeaders, fn($h) => str_starts_with($h, 'Depth:')))) { + $opts['http']['header'] .= "\r\nDepth: 0"; + } + + $ctx = stream_context_create($opts); + $body = file_get_contents($this->baseUrl . $path, false, $ctx); + if ($body === false) { + $body = ''; + } + + // $http_response_header is populated by file_get_contents + $code = '0'; + foreach ($http_response_header ?? [] as $h) { + if (preg_match('#^HTTP/\S+ (\d+)#', $h, $m)) { + $code = $m[1]; + } + } + return [$code, $body]; + } +}