adjust tests + fix import

pkg/plugins/storage/ownership_nop.go

@@ -1,8 +1,9 @@
 //go:build windows
+// +build windows
 
 package storage
 
 // no-op for Windows
-func matchOwnershipToParent(_ , _ string) error {
-    return nil
+func matchOwnershipToParent(_, _ string) error {
+	return nil
 }

pkg/plugins/storage/ownership_unix.go

@@ -3,13 +3,30 @@
 package storage
 
 import (
-    "fmt"
-    "os"
-    "syscall"
+	"fmt"
+	"os"
+	"path/filepath"
+	"syscall"
 )
 
+// chownFunc is a function type for changing file ownership
+type chownFunc func(name string, uid, gid int) error
+
+// ownershipChanger handles changing file ownership
+type ownershipChanger struct {
+	chown chownFunc
+}
+
+// defaultOwnershipChanger uses the real os.Chown function
+var defaultOwnershipChanger = &ownershipChanger{chown: os.Chown}
+
 // matchOwnershipToParent ensures child directory and all its contents inherit ownership from parent dir (UID/GID).
 func matchOwnershipToParent(child, parent string) error {
+	return matchOwnershipToParentWithChanger(child, parent, defaultOwnershipChanger)
+}
+
+// matchOwnershipToParentWithChanger is the testable version that accepts a custom ownership changer
+func matchOwnershipToParentWithChanger(child, parent string, changer *ownershipChanger) error {
 	parentInfo, err := os.Stat(parent)
 	if err != nil {
 		return err
@@ -30,6 +47,6 @@ func matchOwnershipToParent(child, parent string) error {
 		if err != nil {
 			return err
 		}
-		return os.Chown(path, uid, gid)
+		return changer.chown(path, uid, gid)
 	})
 }

pkg/plugins/storage/ownership_unix_test.go

@@ -1,47 +1,140 @@
 //go:build !windows
-// +build !windows
 
 package storage
 
 import (
+	"errors"
 	"os"
 	"path/filepath"
+	"syscall"
 	"testing"
+
+	"github.com/stretchr/testify/require"
 )
 
-// Basic tests for matchOwnershipToParent on Unix-like systems.
-// These avoid requiring root and are safe to run in CI.
+func TestMatchOwnershipToParent(t *testing.T) {
+	t.Run("Parent dir does not exist", func(t *testing.T) {
+		childDir := t.TempDir()
+		err := matchOwnershipToParent(childDir, filepath.Join(childDir, "no-such-parent"))
+		require.ErrorIs(t, err, os.ErrNotExist)
+	})
 
-func TestMatchOwnershipToParent_NoErrorAndRecurses(t *testing.T) {
-	parent := t.TempDir()
-	childParent := t.TempDir()
-	child := filepath.Join(childParent, "childdir")
-	if err := os.MkdirAll(filepath.Join(child, "nested"), 0o755); err != nil {
-		t.Fatalf("failed to create child nested dir: %v", err)
-	}
+	t.Run("Child directory does not exist", func(t *testing.T) {
+		parent := t.TempDir()
+		nonExistentChild := filepath.Join(parent, "does-not-exist")
+		err := matchOwnershipToParent(nonExistentChild, parent)
+		require.ErrorIs(t, err, os.ErrNotExist)
+	})
 
-	fpath := filepath.Join(child, "nested", "file.txt")
-	if err := os.WriteFile(fpath, []byte("hello"), 0o644); err != nil {
-		t.Fatalf("failed to write file: %v", err)
-	}
+	t.Run("Should call chown for all files and directories", func(t *testing.T) {
+		parent := t.TempDir()
+		parentUID, parentGID := getOwnership(t, parent)
 
-	// Should not error and must recurse into child directories.
-	if err := matchOwnershipToParent(child, parent); err != nil {
-		t.Fatalf("matchOwnershipToParent returned error: %v", err)
-	}
+		// Create test structure
+		child := filepath.Join(parent, "child")
+		err := os.MkdirAll(child, 0o750)
+		require.NoError(t, err)
 
-	// File must still exist after call.
-	if _, err := os.Stat(fpath); err != nil {
-		t.Fatalf("expected file to exist after ownership change, got: %v", err)
-	}
+		nestedDir := filepath.Join(child, "nested")
+		err = os.MkdirAll(nestedDir, 0o750)
+		require.NoError(t, err)
+
+		file1 := filepath.Join(child, "file1.txt")
+		err = os.WriteFile(file1, []byte("test"), 0o644)
+		require.NoError(t, err)
+
+		file2 := filepath.Join(nestedDir, "file2.txt")
+		err = os.WriteFile(file2, []byte("test"), 0o644)
+		require.NoError(t, err)
+
+		mock := &mockChowner{}
+		err = matchOwnershipToParentWithChanger(child, parent, &ownershipChanger{chown: mock.chown})
+		require.NoError(t, err)
+
+		// Verify chown was called for all paths with correct UID/GID
+		require.Len(t, mock.calls, 4)
+
+		expectedPaths := []string{child, file1, nestedDir, file2}
+		actualPaths := make([]string, len(mock.calls))
+		for i, call := range mock.calls {
+			actualPaths[i] = call.path
+			require.Equal(t, parentUID, call.uid)
+			require.Equal(t, parentGID, call.gid)
+		}
+
+		require.ElementsMatch(t, expectedPaths, actualPaths)
+	})
+
+	t.Run("Should handle chown errors", func(t *testing.T) {
+		parent := t.TempDir()
+
+		child := filepath.Join(parent, "child")
+		err := os.MkdirAll(child, 0o750)
+		require.NoError(t, err)
+
+		// Mock chown to return an error
+		chownErr := errors.New("permission denied")
+		mock := &mockChowner{err: chownErr}
+		err = matchOwnershipToParentWithChanger(child, parent, &ownershipChanger{chown: mock.chown})
+		require.ErrorIs(t, err, chownErr)
+	})
+
+	t.Run("Should handle single file", func(t *testing.T) {
+		parent := t.TempDir()
+		parentUID, parentGID := getOwnership(t, parent)
+
+		singleFile := filepath.Join(parent, "single-file.txt")
+		err := os.WriteFile(singleFile, []byte("content"), 0o644)
+		require.NoError(t, err)
+
+		mock := &mockChowner{}
+		err = matchOwnershipToParentWithChanger(singleFile, parent, &ownershipChanger{chown: mock.chown})
+		require.NoError(t, err)
+
+		// Should call chown once for the single file
+		require.Len(t, mock.calls, 1)
+		require.Equal(t, singleFile, mock.calls[0].path)
+		require.Equal(t, parentUID, mock.calls[0].uid)
+		require.Equal(t, parentGID, mock.calls[0].gid)
+	})
+
+	t.Run("Should not error with non mock", func(t *testing.T) {
+		parent := t.TempDir()
+
+		singleFile := filepath.Join(parent, "single-file.txt")
+		err := os.WriteFile(singleFile, []byte("content"), 0o644)
+		require.NoError(t, err)
+
+		err = matchOwnershipToParent(singleFile, parent)
+		require.NoError(t, err)
+	})
 }
 
-func TestMatchOwnershipToParent_MissingParentReturnsError(t *testing.T) {
-	child := t.TempDir()
-	missingParent := filepath.Join(child, "no-such-parent")
+// getOwnership returns the UID and GID of a file or directory
+func getOwnership(t *testing.T, path string) (uid, gid int) {
+	t.Helper()
+	info, err := os.Stat(path)
+	require.NoError(t, err)
 
-	// Should error if parent does not exist.
-	if err := matchOwnershipToParent(child, missingParent); err == nil {
-		t.Fatalf("expected an error when parent does not exist, got nil")
-	}
+	stat, ok := info.Sys().(*syscall.Stat_t)
+	require.True(t, ok)
+
+	return int(stat.Uid), int(stat.Gid)
+}
+
+// mockChowner records calls to chown for testing
+type mockChowner struct {
+	calls []chownCall
+	err   error // error to return from chown calls
+}
+
+type chownCall struct {
+	path string
+	uid  int
+	gid  int
+}
+
+func (m *mockChowner) chown(path string, uid, gid int) error {
+	m.calls = append(m.calls, chownCall{path: path, uid: uid, gid: gid})
+	return m.err
 }