Merge pull request #3348 from opencloud-eu/fix/posix_cli_commands

fix: posix cli commands

.woodpecker.star

@@ -331,6 +331,8 @@ config = {
                 "ANTIVIRUS_CLAMAV_SOCKET": "tcp://clamav:3310",
                 "OC_ASYNC_UPLOADS": True,
                 "OC_ADD_RUN_SERVICES": "antivirus",
+                "OC_SERVICE_ACCOUNT_ID": "service-account-id",
+                "OC_SERVICE_ACCOUNT_SECRET": "service-account-secret",
             },
         },
         "multiTenancy": {

opencloud/pkg/command/revisions.go

@@ -25,6 +25,8 @@ var (
 	_nodesGlobPattern = "spaces/*/*/nodes/"
 )
 
+const posixDriver = "posix"
+
 // RevisionsCommand is the entrypoint for the revisions command.
 func RevisionsCommand(cfg *config.Config) *cobra.Command {
 	revCmd := &cobra.Command{
@@ -86,23 +88,28 @@ func PurgeRevisionsCommand(cfg *config.Config) *cobra.Command {
 				mechanism = "glob"
 			}
 
+			var posix = cfg.StorageUsers.Driver == posixDriver
+
 			var ch <-chan string
 			switch mechanism {
 			default:
 				fallthrough
 			case "glob":
-				p := generatePath(basePath, rid)
+				p := generatePath(basePath, rid, posix)
 				if rid.GetOpaqueId() == "" {
 					p = filepath.Join(p, "*/*/*/*/*")
 				}
 				ch = revisions.Glob(p)
 			case "workers":
-				p := generatePath(basePath, rid)
+				p := generatePath(basePath, rid, posix)
 				ch = revisions.GlobWorkers(p, "/*", "/*/*/*/*")
 			case "list":
-				p := filepath.Join(basePath, "spaces")
+				p := basePath
+				if !posix {
+					p = filepath.Join(basePath, "spaces")
+				}
 				if rid != nil {
-					p = generatePath(basePath, rid)
+					p = generatePath(basePath, rid, posix)
 				}
 				ch = revisions.List(p, 10)
 			}
@@ -144,22 +151,44 @@ func printResults(countFiles, countBlobs, countRevisions int, dryRun bool) {
 	}
 }
 
-func generatePath(basePath string, rid *provider.ResourceId) string {
-	if rid == nil {
-		return filepath.Join(basePath, _nodesGlobPattern)
-	}
+func generatePath(basePath string, rid *provider.ResourceId, posix bool) string {
+	// decomposedfs and posix store the revisions of a node at different
+	// locations on disk, so the path has to be built per driver:
+	//   - decomposedfs: <basePath>/spaces/<pathified spaceID>/nodes/<pathified nodeID>.REV.<ts>
+	//   - posix:        <basePath>/<users|projects>/<spaceID>/.oc-nodes/<pathified nodeID>.REV.<ts>
+	if posix {
+		if rid == nil {
+			return filepath.Join(basePath, "*", "*", ".oc-nodes")
+		}
 
-	sid := lookup.Pathify(rid.GetSpaceId(), 1, 2)
-	if sid == "" {
-		return ""
-	}
+		nid := lookup.Pathify(rid.GetOpaqueId(), 4, 2)
+		if nid != "" {
+			return filepath.Join(basePath, "*", "*", ".oc-nodes", nid+"*")
+		}
 
-	nid := lookup.Pathify(rid.GetOpaqueId(), 4, 2)
-	if nid == "" {
-		return filepath.Join(basePath, "spaces", sid, "nodes")
-	}
+		if rid.GetSpaceId() == "" {
+			return ""
+		}
+		return filepath.Join(basePath, "*", rid.GetSpaceId(), ".oc-nodes")
+	} else {
+		// decomposedfs
+		if rid == nil {
 
-	return filepath.Join(basePath, "spaces", sid, "nodes", nid+"*")
+			return filepath.Join(basePath, _nodesGlobPattern)
+		}
+
+		sid := lookup.Pathify(rid.GetSpaceId(), 1, 2)
+		if sid == "" {
+			return ""
+		}
+
+		nid := lookup.Pathify(rid.GetOpaqueId(), 4, 2)
+		if nid == "" {
+			return filepath.Join(basePath, "spaces", sid, "nodes")
+		}
+
+		return filepath.Join(basePath, "spaces", sid, "nodes", nid+"*")
+	}
 }
 
 func init() {

opencloud/pkg/command/shares.go

@@ -1,7 +1,9 @@
 package command
 
 import (
+	"context"
 	"errors"
+	"time"
 
 	"github.com/spf13/viper"
 
@@ -21,6 +23,14 @@ import (
 	"github.com/spf13/cobra"
 )
 
+// need to be discussed, for now I will let it here
+//
+// Problem:
+// in reva on CleanupStaleShares call there is migrations invokation, which at leat in tests takes some time,
+// reva code was modified to wait until migrations are done, to prevent cases when migrations are stuck and the
+// this executions is not returned this timeout is needed
+const cleanupTimeout = 1 * time.Minute
+
 // SharesCommand is the entrypoint for the groups command.
 func SharesCommand(cfg *config.Config) *cobra.Command {
 	sharesCmd := &cobra.Command{
@@ -120,7 +130,11 @@ func cleanup(_ *cobra.Command, cfg *config.Config) error {
 	}
 	serviceUserCtx = l.WithContext(serviceUserCtx)
 
-	mgr.(*jsoncs3.Manager).CleanupStaleShares(serviceUserCtx)
+	cleanupCtx, cancel := context.WithTimeout(serviceUserCtx, cleanupTimeout)
+	defer cancel()
+	if err := mgr.(*jsoncs3.Manager).CleanupStaleShares(cleanupCtx); err != nil {
+		return configlog.ReturnError(err)
+	}
 
 	return nil
 }
@@ -158,11 +172,13 @@ func revaShareConfig(cfg *sharing.Config) map[string]any {
 			"machine_auth_apikey": cfg.UserSharingDrivers.CS3.SystemUserAPIKey,
 		},
 		"jsoncs3": map[string]any{
-			"gateway_addr":        cfg.Reva.Address,
-			"provider_addr":       cfg.UserSharingDrivers.JSONCS3.ProviderAddr,
-			"service_user_id":     cfg.UserSharingDrivers.JSONCS3.SystemUserID,
-			"service_user_idp":    cfg.UserSharingDrivers.JSONCS3.SystemUserIDP,
-			"machine_auth_apikey": cfg.UserSharingDrivers.JSONCS3.SystemUserAPIKey,
+			"gateway_addr":           cfg.Reva.Address,
+			"provider_addr":          cfg.UserSharingDrivers.JSONCS3.ProviderAddr,
+			"system_user_id":         cfg.UserSharingDrivers.JSONCS3.SystemUserID,
+			"system_user_idp":        cfg.UserSharingDrivers.JSONCS3.SystemUserIDP,
+			"machine_auth_apikey":    cfg.UserSharingDrivers.JSONCS3.SystemUserAPIKey,
+			"service_account_id":     cfg.ServiceAccount.ServiceAccountID,
+			"service_account_secret": cfg.ServiceAccount.ServiceAccountSecret,
 		},
 	}
 }

opencloud/pkg/command/trash.go

@@ -36,7 +36,8 @@ func TrashPurgeEmptyDirsCommand(cfg *config.Config) *cobra.Command {
 		RunE: func(cmd *cobra.Command, args []string) error {
 			basePath, _ := cmd.Flags().GetString("basepath")
 			dryRun, _ := cmd.Flags().GetBool("dry-run")
-			if err := trash.PurgeTrashEmptyPaths(basePath, dryRun); err != nil {
+			posix := cfg.StorageUsers.Driver == posixDriver
+			if err := trash.PurgeTrashEmptyPaths(basePath, dryRun, posix); err != nil {
 				fmt.Println(err)
 				return err
 			}

opencloud/pkg/trash/trash.go

@@ -10,12 +10,23 @@ import (
 const (
 	// _trashGlobPattern is the glob pattern to find all trash items
 	_trashGlobPattern = "spaces/*/*/trash/*/*/*/*"
+	// _trashRootPattern is the glob pattern of the trash container root
+	_trashRootPattern = "spaces/*/*/trash"
+	// _posixTrashGlobPattern is the glob pattern to find all trash items on posix
+	_posixTrashGlobPattern = "*/*/.Trash/files/*"
+	// _posixTrashRootPattern is the glob pattern of the trash container root on posix
+	_posixTrashRootPattern = "*/*/.Trash/files"
 )
 
 // PurgeTrashEmptyPaths purges empty paths in the trash
-func PurgeTrashEmptyPaths(p string, dryRun bool) error {
+func PurgeTrashEmptyPaths(p string, dryRun bool, posix bool) error {
+	pattern := _trashGlobPattern
+	if posix {
+		pattern = _posixTrashGlobPattern
+	}
+
 	// we have all trash nodes in all spaces now
-	dirs, err := filepath.Glob(filepath.Join(p, _trashGlobPattern))
+	dirs, err := filepath.Glob(filepath.Join(p, pattern))
 	if err != nil {
 		return err
 	}
@@ -25,15 +36,26 @@ func PurgeTrashEmptyPaths(p string, dryRun bool) error {
 	}
 
 	for _, d := range dirs {
-		if err := removeEmptyFolder(d, dryRun); err != nil {
+		if err := removeEmptyFolder(d, dryRun, posix, p); err != nil {
 			return err
 		}
 	}
 	return nil
 }
 
-func removeEmptyFolder(path string, dryRun bool) error {
+func removeEmptyFolder(path string, dryRun bool, posix bool, basePath string) error {
+
 	if dryRun {
+		if posix {
+			// on posix the ".trashitem" entries are the actual data and can be
+			// files, which are skipped, so we need to check here if the path
+			// is the actual dir, the same check for real removal part
+			fi, err := os.Stat(path)
+			if err != nil || !fi.IsDir() {
+				return nil
+			}
+		}
+
 		f, err := os.ReadDir(path)
 		if err != nil {
 			return err
@@ -43,6 +65,14 @@ func removeEmptyFolder(path string, dryRun bool) error {
 		}
 		return nil
 	}
+
+	if posix {
+		fi, err := os.Stat(path)
+		if err != nil || !fi.IsDir() {
+			return nil
+		}
+	}
+
 	if err := os.Remove(path); err != nil {
 		// we do not really care about the error here
 		// if the folder is not empty we will get an error,
@@ -50,8 +80,17 @@ func removeEmptyFolder(path string, dryRun bool) error {
 		return nil
 	}
 	nd := filepath.Dir(path)
-	if filepath.Base(nd) == "trash" {
+	if isTrashRoot(nd, basePath, posix) {
 		return nil
 	}
-	return removeEmptyFolder(nd, dryRun)
+	return removeEmptyFolder(nd, dryRun, posix, basePath)
+}
+
+func isTrashRoot(path, basePath string, posix bool) bool {
+	rootPattern := _trashRootPattern
+	if posix {
+		rootPattern = _posixTrashRootPattern
+	}
+	matched, _ := filepath.Match(filepath.Join(basePath, rootPattern), path)
+	return matched
 }

opencloud/pkg/trash/trash_test.go

@@ -0,0 +1,109 @@
+package trash
+
+import (
+	"os"
+	"testing"
+
+	"github.com/test-go/testify/require"
+)
+
+func TestIsTrashRootDecomposed(t *testing.T) {
+	storageRoot := "test_temp_" + t.Name()
+	defer os.RemoveAll(storageRoot)
+
+	require.True(t, isTrashRoot(storageRoot+"/spaces/id/id/trash", storageRoot, false))
+	require.False(t, isTrashRoot(storageRoot+"/spaces/id/id/trash/s1/s2/s3/node", storageRoot, false))
+	require.False(t, isTrashRoot(storageRoot+"/spaces/id/id/trash/s1/s2/trash/node", storageRoot, false))
+}
+
+func TestIsTrashRootPosix(t *testing.T) {
+	storageRoot := "test_temp_" + t.Name()
+	defer os.RemoveAll(storageRoot)
+
+	require.True(t, isTrashRoot(storageRoot+"/users/alice/.Trash/files", storageRoot, true))
+	require.False(t, isTrashRoot(storageRoot+"/users/alice/.Trash/files/item.trashitem", storageRoot, true))
+	require.False(t, isTrashRoot(storageRoot+"/users/alice/.Trash/files/item.trashitem/files", storageRoot, true))
+}
+
+func TestRemoveEmptyFolderPosix(t *testing.T) {
+	storageRoot := "test_temp_" + t.Name()
+	base := storageRoot + "/users/alice/.Trash/files"
+	defer os.RemoveAll(storageRoot)
+
+	emptyChain := base + "/empty.trashitem/sub/subsub"
+	require.NoError(t, os.MkdirAll(emptyChain, os.ModePerm))
+
+	nonEmpty := base + "/keep.trashitem"
+	require.NoError(t, os.MkdirAll(nonEmpty, os.ModePerm))
+	require.NoError(t, os.WriteFile(nonEmpty+"/file.txt", []byte("some text"), os.ModePerm))
+
+	require.NoError(t, removeEmptyFolder(emptyChain, false, true, storageRoot))
+
+	assertNoDirExists(t, emptyChain)
+	assertNoDirExists(t, base+"/empty.trashitem")
+	assertDirExists(t, base)
+	assertDirExists(t, nonEmpty)
+}
+
+func TestRemoveEmptyFolderPosixUserDirNamedFiles(t *testing.T) {
+	storageRoot := "test_temp_" + t.Name()
+	base := storageRoot + "/users/alice/.Trash/files"
+	defer os.RemoveAll(storageRoot)
+
+	nestedFilesDir := base + "/folder.trashitem/files/files"
+	require.NoError(t, os.MkdirAll(nestedFilesDir, os.ModePerm))
+
+	require.NoError(t, removeEmptyFolder(nestedFilesDir, false, true, storageRoot))
+
+	assertNoDirExists(t, nestedFilesDir)
+	assertNoDirExists(t, base+"/folder.trashitem/files")
+	assertNoDirExists(t, base+"/folder.trashitem")
+	assertDirExists(t, base)
+}
+
+func TestRemoveEmptyFolderDecomposed(t *testing.T) {
+	storageRoot := "test_temp_" + t.Name()
+	base := storageRoot + "/spaces/id/id/trash"
+	defer os.RemoveAll(storageRoot)
+
+	emptyChain := base + "/s1/s2/s3/node"
+	require.NoError(t, os.MkdirAll(emptyChain, os.ModePerm))
+
+	require.NoError(t, removeEmptyFolder(emptyChain, false, false, storageRoot))
+
+	assertNoDirExists(t, emptyChain)
+	assertNoDirExists(t, base+"/s1/s2/s3")
+	assertNoDirExists(t, base+"/s1/s2")
+	assertNoDirExists(t, base+"/s1")
+	assertDirExists(t, base)
+}
+
+func TestRemoveEmptyFolderDecomposedUserDirNamedTrash(t *testing.T) {
+	storageRoot := "test_temp_" + t.Name()
+	base := storageRoot + "/spaces/id/id/trash"
+	defer os.RemoveAll(storageRoot)
+
+	nestedTrashDir := base + "/s1/trash/s2/node"
+	require.NoError(t, os.MkdirAll(nestedTrashDir, os.ModePerm))
+
+	require.NoError(t, removeEmptyFolder(nestedTrashDir, false, false, storageRoot))
+
+	assertNoDirExists(t, nestedTrashDir)
+	assertNoDirExists(t, base+"/s1/trash/s2")
+	assertNoDirExists(t, base+"/s1/trash")
+	assertNoDirExists(t, base+"/s1")
+	assertDirExists(t, base)
+}
+
+func assertNoDirExists(t *testing.T, path string) {
+	t.Helper()
+	_, err := os.Stat(path)
+	require.True(t, os.IsNotExist(err))
+}
+
+func assertDirExists(t *testing.T, path string) {
+	t.Helper()
+	fi, err := os.Stat(path)
+	require.NoError(t, err)
+	require.True(t, fi.IsDir())
+}

services/storage-users/pkg/command/trash_bin.go

@@ -306,8 +306,13 @@ func restoreTrashBinItem(cfg *config.Config) *cobra.Command {
 
 			var found bool
 			var itemRef *provider.RecycleItem
+			rid, err := storagespace.ParseID(itemID)
+			if err != nil {
+				return err
+			}
+			opaqueID := rid.GetOpaqueId()
 			for _, item := range res.GetRecycleItems() {
-				if item.GetKey() == itemID {
+				if item.GetKey() == opaqueID {
 					itemRef = item
 					found = true
 					break
@@ -350,10 +355,6 @@ func listRecycle(ctx context.Context, client gateway.GatewayAPIClient, ref provi
 	if res.Status.Code != rpc.Code_CODE_OK {
 		return nil, fmt.Errorf("%s %s", _retrievingErrorMsg, res.Status.Code)
 	}
-	if len(res.GetRecycleItems()) == 0 {
-		fmt.Errorf("The trash-bin is empty. Nothing to restore")
-		os.Exit(0)
-	}
 	return res, nil
 }
 

tests/acceptance/docker/Makefile

@@ -54,6 +54,11 @@ else ifeq ($(STORAGE_DRIVER),decomposeds3)
 	COMPOSE_FILE := src/ceph.yml:$(COMPOSE_FILE)
 endif
 
+# CLI commands use fixed service-account credentials expected by the CLI tests.
+ifneq ($(or $(filter cliCommands,$(BEHAT_SUITE)),$(findstring cliCommands/,$(BEHAT_FEATURE))),)
+	COMPOSE_FILE := $(COMPOSE_FILE):src/cliCommands.yml
+endif
+
 # use latest as default tag if OC_IMAGE is provided but no tag is set
 ifneq ($(strip $(OC_IMAGE)),)
 	ifeq ($(strip $(OC_IMAGE_TAG)),)

tests/acceptance/docker/src/acceptance.yml

@@ -10,6 +10,7 @@ services:
       STORAGE_DRIVER: ${STORAGE_DRIVER:-posix}
       BEHAT_SUITE: ${BEHAT_SUITE:-}
       BEHAT_FEATURE: ${BEHAT_FEATURE:-}
+      OC_BASE_DATA_PATH: ${OC_BASE_DATA_PATH:-/var/lib/opencloud}
       USE_BEARER_TOKEN: ${USE_BEARER_TOKEN:-false}
       # email
       EMAIL_HOST: email

tests/acceptance/docker/src/cliCommands.yml

@@ -0,0 +1,5 @@
+services:
+  opencloud-server:
+    environment:
+      OC_SERVICE_ACCOUNT_ID: "service-account-id"
+      OC_SERVICE_ACCOUNT_SECRET: "service-account-secret"

tests/acceptance/expected-failures-decomposed-storage.md

@@ -335,12 +335,6 @@ _ocdav: api compatibility, return correct status code_
 - [coreApiWebdavPreviews/previews.feature:265](https://github.com/opencloud-eu/opencloud/blob/main/tests/acceptance/features/coreApiWebdavPreviews/previews.feature#L265)
 
 
-#### [POSIX: failing CLI commands](https://github.com/opencloud-eu/opencloud/issues/3095)
-
-- [cliCommands/restoreTrashBinItems.feature:11](https://github.com/opencloud-eu/opencloud/blob/main/tests/acceptance/features/cliCommands/restoreTrashBinItems.feature#L11)
-- [cliCommands/restoreTrashBinItems.feature:23](https://github.com/opencloud-eu/opencloud/blob/main/tests/acceptance/features/cliCommands/restoreTrashBinItems.feature#L23)
-- [cliCommands/sharesCleanup.feature:12](https://github.com/opencloud-eu/opencloud/blob/main/tests/acceptance/features/cliCommands/sharesCleanup.feature#L12)
-
 ### notification issue #323
 - [apiNotification/emailNotification.feature:280](https://github.com/opencloud-eu/opencloud/blob/main/tests/acceptance/features/apiNotification/emailNotification.feature#L280)
 - [apiNotification/emailNotification.feature:298](https://github.com/opencloud-eu/opencloud/blob/main/tests/acceptance/features/apiNotification/emailNotification.feature#L298)

tests/acceptance/expected-failures-posix-storage.md

@@ -337,11 +337,6 @@ _ocdav: api compatibility, return correct status code_
 - [coreApiWebdavPreviews/previews.feature:264](https://github.com/opencloud-eu/opencloud/blob/main/tests/acceptance/features/coreApiWebdavPreviews/previews.feature#L264)
 - [coreApiWebdavPreviews/previews.feature:265](https://github.com/opencloud-eu/opencloud/blob/main/tests/acceptance/features/coreApiWebdavPreviews/previews.feature#L265)
 
-#### [POSIX: failing CLI commands](https://github.com/opencloud-eu/opencloud/issues/3095)
-
-- [cliCommands/restoreTrashBinItems.feature:11](https://github.com/opencloud-eu/opencloud/blob/main/tests/acceptance/features/cliCommands/restoreTrashBinItems.feature#L11)
-- [cliCommands/restoreTrashBinItems.feature:23](https://github.com/opencloud-eu/opencloud/blob/main/tests/acceptance/features/cliCommands/restoreTrashBinItems.feature#L23)
-- [cliCommands/sharesCleanup.feature:12](https://github.com/opencloud-eu/opencloud/blob/main/tests/acceptance/features/cliCommands/sharesCleanup.feature#L12)
 
 ### notification issue #323
 - [apiNotification/emailNotification.feature:280](https://github.com/opencloud-eu/opencloud/blob/main/tests/acceptance/features/apiNotification/emailNotification.feature#L280)

tests/acceptance/features/cliCommands/cleanupEmptyTrashBin.feature

@@ -1,4 +1,4 @@
-@env-config @skipOnOpencloud-posix-Storage
+@env-config
 Feature: delete empty trash bin folder via CLI command
 
 

tests/acceptance/features/cliCommands/removeFileVersions.feature

@@ -1,4 +1,4 @@
-@env-config @skipOnOpencloud-posix-Storage
+@env-config
 Feature: remove file versions via CLI command
 
   Background:
@@ -11,7 +11,7 @@ Feature: remove file versions via CLI command
     And user "Alice" has uploaded file with content "This is version 3" to "textfile.txt"
     When the administrator removes all the file versions using the CLI
     Then the command should be successful
-    And the command output should contain "✅ Deleted 2 revisions (6 files / 2 blobs)"
+    And the command output should contain "✅ Deleted 2 revisions"
     When user "Alice" gets the number of versions of file "textfile.txt"
     Then the HTTP status code should be "207"
     And the number of versions should be "0"
@@ -26,7 +26,7 @@ Feature: remove file versions via CLI command
     And user "Alice" has uploaded file with content "This is version 3" to "anotherFile.txt"
     When the administrator removes the versions of file "randomFile.txt" of user "Alice" from space "Personal" using the CLI
     Then the command should be successful
-    And the command output should contain "✅ Deleted 2 revisions (6 files / 2 blobs)"
+    And the command output should contain "✅ Deleted 2 revisions"
     When user "Alice" gets the number of versions of file "randomFile.txt"
     Then the HTTP status code should be "207"
     And the number of versions should be "0"
@@ -52,7 +52,7 @@ Feature: remove file versions via CLI command
     And we save it into "EPSUM_FILEID"
     When the administrator removes the file versions of space "projectSpace" using the CLI
     Then the command should be successful
-    And the command output should contain "✅ Deleted 4 revisions (12 files / 4 blobs)"
+    And the command output should contain "✅ Deleted 4 revisions"
     When user "Alice" gets the number of versions of file "file.txt"
     Then the HTTP status code should be "207"
     And the number of versions should be "2"