Preserve recursive FileTransfer paths

docs/methods.rst

@@ -33,9 +33,8 @@ absolute-path validation or normalize interior path components.
 normalization applies to ``targetPath``; tracked host destinations must still
 be canonical absolute paths without symlink components.
 
-FileTransfer continues to copy each file by its basename into
-``destinationDirectory``; adding a trailing slash does not preserve nested source
-directories or change its destination layout.
+FileTransfer preserves directories below ``targetPath``. Adding a trailing slash
+does not change the destination layout. Use ``glob`` to select a single file.
 
 Dynamic Configuration Reload
 ----------------------------
@@ -123,7 +122,7 @@ revision-pinned builds and :doc:`release_notes` for unreleased changes.
      - Authored host ``.service`` files
      - Optional tracked-service cleanup
    * - ``filetransfer``
-     - Copy files into an existing host directory
+     - Copy files and nested directories into an existing host directory
      - Optional tracked-file cleanup
    * - ``ansible``
      - Host configuration through playbooks
@@ -350,11 +349,14 @@ The File Transfer method will copy files from the container to the host. This me
        schedule: "*/5 * * * *"
      branch: main
 
-The ``destinationDirectory`` field is the directory on the host where files
-will be copied. Set ``cleanupOnRemoval: true`` to track individual copied files
-and remove them when the method leaves configuration. The default is false.
-Tracked deployments require an existing, absolute destination directory and
-refuse to overwrite untracked files. See :doc:`lifecycle` for setup and recovery.
+The ``destinationDirectory`` field is the directory on the host where files and
+nested directories below ``targetPath`` will be copied. Source directory
+structure is preserved, while ``glob`` can still select a single file. Set
+``cleanupOnRemoval: true`` to track individual copied files and remove them when
+the method leaves configuration. The default is false. Tracked deployments
+require an existing, absolute destination directory, create missing parent
+directories for tracked files, and refuse to overwrite untracked files. See
+:doc:`lifecycle` for setup and recovery.
 
 During Git reconciliation, deletion removes the old destination filename, and
 rename removes the old filename before installing the new one. Filenames with

docs/release_notes.rst

@@ -4,6 +4,12 @@ Release notes
 Unreleased
 ----------
 
+* Recursive FileTransfer (issue #236): files below ``targetPath`` retain their
+  relative directory structure in ``destinationDirectory``. Single-file ``glob``
+  selections continue to copy directly into the destination. Tracked cleanup
+  supports nested destination paths and creates missing parent directories for
+  owned files. See :doc:`methods`.
+
 * Graceful engine shutdown (issue #287): SIGTERM and SIGINT stop scheduling
   work and cancel running operations, with a five-second grace period before
   exit. The entry script forwards signals by replacing itself with FetchIt.
@@ -21,8 +27,8 @@ Trailing slashes in Git target paths
 ``targetPath: examples/filetransfer`` (issue #201). Trailing slashes are removed
 before Git change filtering and ownership calculations for Git methods. This
 also makes configuration reloads between these spellings retain the same
-identity. FileTransfer's existing basename-based destination layout is unchanged;
-this does not add recursive directory preservation. See :doc:`methods`.
+identity. FileTransfer preserves paths relative to ``targetPath``; trailing
+slashes do not add another directory level. See :doc:`methods`.
 
 Tracked host file and service cleanup
 ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

pkg/engine/container.go

@@ -6,6 +6,7 @@ import (
 	"errors"
 	"fmt"
 	"net/http"
+	"path/filepath"
 	"strings"
 
 	"github.com/containers/fetchit/pkg/engine/utils"
@@ -35,14 +36,14 @@ func validateShellParam(param string, paramName string) error {
 
 func generateSpec(method, file, copyFile, dest string, name string) *specgen.SpecGenerator {
 	s := specgen.NewSpecGenerator(fetchitImage, false)
-	s.Name = method + "-" + name + "-" + file
+	s.Name = method + "-" + name + "-" + strings.ReplaceAll(file, "/", "-")
 	privileged := true
 	s.Privileged = &privileged
 	s.PidNS = specgen.Namespace{
 		NSMode: "host",
 		Value:  "",
 	}
-	s.Command = []string{"rsync", "-avz", "--", copyFile, dest}
+	s.Command = []string{"rsync", "-avz", "--mkpath", "--", copyFile, filepath.Join(dest, file)}
 	s.Mounts = []specs.Mount{{Source: dest, Destination: dest, Type: "bind", Options: []string{"rw"}}}
 	s.Volumes = []*specgen.NamedVolume{{Name: fetchitVolume, Dest: "/opt", Options: []string{"rw"}}}
 	return s

pkg/engine/filetransfer.go

@@ -2,7 +2,9 @@ package engine
 
 import (
 	"context"
+	"fmt"
 	"path/filepath"
+	"strings"
 	"time"
 
 	"github.com/go-git/go-git/v5/plumbing"
@@ -57,40 +59,64 @@ func (ft *FileTransfer) Process(ctx, conn context.Context, skew int) {
 }
 
 func (ft *FileTransfer) MethodEngine(ctx, conn context.Context, change *object.Change, path string) error {
+	targetRoot := filepath.Join(getDirectory(ft.GetTarget()), ft.GetTargetPath())
 	var prev *string = nil
+	var previousPath string
 	if change != nil {
 		if change.From.Name != "" {
 			prev = &change.From.Name
 		}
 	}
+	if prev != nil {
+		relativePath, err := fileTransferRelativePath(filepath.Join(targetRoot, *prev), targetRoot)
+		if err != nil {
+			return err
+		}
+		previousPath = relativePath
+	}
 	if ft.CleanupOnRemoval {
 		receipt, _, err := hostMethodReceipt(ft)
 		if err != nil {
 			return err
 		}
 		plan := hostArtifactPlan{Receipt: receipt, Action: "apply"}
 		if prev != nil {
-			plan.Previous = filepath.Base(*prev)
+			plan.Previous = previousPath
 		}
 		if path == deleteFile {
 			plan.Action = "delete"
 		} else {
+			relativePath, err := fileTransferRelativePath(path, targetRoot)
+			if err != nil {
+				return err
+			}
 			plan.Source = filepath.Join("/opt", path)
+			plan.DestinationPath = relativePath
 		}
 		return deployHostArtifact(conn, plan)
 	}
+
+	relativePath := ""
+	if path != deleteFile {
+		var err error
+		relativePath, err = fileTransferRelativePath(path, targetRoot)
+		if err != nil {
+			return err
+		}
+	}
+
 	dest := ft.DestinationDirectory
-	return ft.fileTransferPodman(ctx, conn, path, dest, prev)
+	return ft.fileTransferPodman(ctx, conn, path, relativePath, dest, previousPath)
 }
 
 func (ft *FileTransfer) Apply(ctx, conn context.Context, currentState, desiredState plumbing.Hash, tags *[]string) error {
 	return ft.applyGitChanges(ctx, conn, ft, currentState, desiredState, tags)
 }
 
-func (ft *FileTransfer) fileTransferPodman(ctx, conn context.Context, path, dest string, prev *string) error {
-	if prev != nil {
-		pathToRemove := filepath.Join(dest, filepath.Base(*prev))
-		s := generateSpecRemove(filetransferMethod, filepath.Base(pathToRemove), pathToRemove, dest, ft.Name)
+func (ft *FileTransfer) fileTransferPodman(ctx, conn context.Context, path, relativePath, dest, previousPath string) error {
+	if previousPath != "" {
+		pathToRemove := filepath.Join(dest, previousPath)
+		s := generateSpecRemove(filetransferMethod, previousPath, pathToRemove, dest, ft.Name)
 		createResponse, err := createAndStartContainer(conn, s)
 		if err != nil {
 			return err
@@ -108,11 +134,9 @@ func (ft *FileTransfer) fileTransferPodman(ctx, conn context.Context, path, dest
 
 	logger.Infof("Deploying file(s) %s", path)
 
-	file := filepath.Base(path)
+	source := "/opt/" + filepath.ToSlash(path)
 
-	source := filepath.Join("/opt", path)
-
-	s := generateSpec(filetransferMethod, file, source, dest, ft.Name)
+	s := generateSpec(filetransferMethod, relativePath, source, dest, ft.Name)
 	createResponse, err := createAndStartContainer(conn, s)
 	if err != nil {
 		return err
@@ -121,3 +145,12 @@ func (ft *FileTransfer) fileTransferPodman(ctx, conn context.Context, path, dest
 	// Wait for the container to exit
 	return waitAndRemoveContainer(conn, createResponse.ID)
 }
+
+func fileTransferRelativePath(path, repository string) (string, error) {
+	relativePath, err := filepath.Rel(repository, path)
+	if err != nil || relativePath == "" || relativePath == "." || relativePath == ".." ||
+		strings.HasPrefix(relativePath, ".."+string(filepath.Separator)) {
+		return "", fmt.Errorf("file transfer path %q is outside repository %q", path, repository)
+	}
+	return relativePath, nil
+}

pkg/engine/filetransfer_lifecycle_test.go

@@ -7,6 +7,7 @@ import (
 	"github.com/go-git/go-git/v5/plumbing/object"
 	"go.uber.org/zap"
 	"net/http"
+	"path/filepath"
 	"reflect"
 	"strings"
 	"testing"
@@ -17,14 +18,15 @@ func TestFileTransferCreateRenameDeleteAndRetry(t *testing.T) {
 	logger = zap.NewNop().Sugar()
 	t.Cleanup(func() { logger = old })
 	for _, tc := range []struct {
-		name, from, to string
-		fail           bool
-		want           [][]string
+		name, targetPath, from, to string
+		fail                       bool
+		want                       [][]string
 	}{
-		{name: "create", to: "new file.txt", want: [][]string{{"rsync", "-avz", "--", "/opt/repo/new file.txt", "/host/dest"}}},
-		{name: "rename", from: "old file.txt", to: "new file.txt", want: [][]string{{"rm", "-f", "--", "/host/dest/old file.txt"}, {"rsync", "-avz", "--", "/opt/repo/new file.txt", "/host/dest"}}},
-		{name: "delete", from: "old file.txt", want: [][]string{{"rm", "-f", "--", "/host/dest/old file.txt"}}},
-		{name: "failed removal", from: "old file.txt", to: "new file.txt", fail: true, want: [][]string{{"rm", "-f", "--", "/host/dest/old file.txt"}}},
+		{name: "create single file", to: "new file.txt", want: [][]string{{"rsync", "-avz", "--mkpath", "--", "/opt/repo/new file.txt", "/host/dest/new file.txt"}}},
+		{name: "create nested file", targetPath: "source", to: "nested/deep/new file.txt", want: [][]string{{"rsync", "-avz", "--mkpath", "--", "/opt/repo/source/nested/deep/new file.txt", "/host/dest/nested/deep/new file.txt"}}},
+		{name: "rename nested file", targetPath: "source", from: "nested/old file.txt", to: "nested/deep/new file.txt", want: [][]string{{"rm", "-f", "--", "/host/dest/nested/old file.txt"}, {"rsync", "-avz", "--mkpath", "--", "/opt/repo/source/nested/deep/new file.txt", "/host/dest/nested/deep/new file.txt"}}},
+		{name: "delete nested file", targetPath: "source", from: "nested/old file.txt", want: [][]string{{"rm", "-f", "--", "/host/dest/nested/old file.txt"}}},
+		{name: "failed nested removal", targetPath: "source", from: "nested/old file.txt", to: "nested/deep/new file.txt", fail: true, want: [][]string{{"rm", "-f", "--", "/host/dest/nested/old file.txt"}}},
 	} {
 		t.Run(tc.name, func(t *testing.T) {
 			var commands [][]string
@@ -54,11 +56,14 @@ func TestFileTransferCreateRenameDeleteAndRetry(t *testing.T) {
 				}
 			})
 			change := &object.Change{From: object.ChangeEntry{Name: tc.from}, To: object.ChangeEntry{Name: tc.to}}
-			path := "repo/" + tc.to
+			path := filepath.Join("repo", tc.targetPath, tc.to)
 			if tc.to == "" {
 				path = deleteFile
 			}
-			method := &FileTransfer{DestinationDirectory: "/host/dest", CommonMethod: CommonMethod{Name: "copy"}}
+			method := &FileTransfer{
+				DestinationDirectory: "/host/dest",
+				CommonMethod:         CommonMethod{Name: "copy", TargetPath: tc.targetPath, target: &Target{url: "https://example.com/repo.git"}},
+			}
 			err := method.MethodEngine(context.Background(), conn, change, path)
 			if (err != nil) != tc.fail || !reflect.DeepEqual(commands, tc.want) {
 				t.Fatalf("commands=%v err=%v", commands, err)

pkg/engine/host_artifact_helper.go

@@ -58,13 +58,26 @@ func hostSystemctl(host string, h *hostRemoval, args ...string) (string, error)
 }
 
 func validArtifactName(name, kind string) bool {
-	if name == "" || name == "." || name == ".." || name == artifactJournalDirectory || filepath.Base(name) != name || strings.ContainsAny(name, "\x00\n\r") {
+	if name == "" || name == "." || name == ".." || strings.ContainsAny(name, "\x00\n\r") {
 		return false
 	}
-	if kind == systemdMethod {
-		return quadletSafeName.MatchString(name) && strings.HasSuffix(name, ".service") && !strings.HasPrefix(name, "-") && name != ".service"
+	if kind == filetransferMethod {
+		return validFileTransferArtifactPath(name)
 	}
-	return kind == filetransferMethod
+	return kind == systemdMethod && filepath.Base(name) == name && name != artifactJournalDirectory &&
+		quadletSafeName.MatchString(name) && strings.HasSuffix(name, ".service") && !strings.HasPrefix(name, "-") && name != ".service"
+}
+
+func validFileTransferArtifactPath(name string) bool {
+	if filepath.IsAbs(name) || filepath.Clean(name) != name {
+		return false
+	}
+	for _, part := range strings.Split(name, string(filepath.Separator)) {
+		if part == "" || part == "." || part == ".." || part == artifactJournalDirectory {
+			return false
+		}
+	}
+	return true
 }
 
 // Validate every destination component, rather than following a host symlink.
@@ -114,6 +127,31 @@ func fingerprintArtifact(path string) (*artifactFingerprint, error) {
 	stat := opened.Sys().(*syscall.Stat_t)
 	return &artifactFingerprint{Digest: hex.EncodeToString(digest.Sum(nil)), Mode: uint32(opened.Mode().Perm()), Device: uint64(stat.Dev), Inode: uint64(stat.Ino), UID: stat.Uid, GID: stat.Gid}, nil
 }
+
+func artifactTargetPath(dest, name string, createParents bool) (string, error) {
+	current := dest
+	components := strings.Split(name, string(filepath.Separator))
+	for _, component := range components[:len(components)-1] {
+		current = filepath.Join(current, component)
+		info, err := os.Lstat(current)
+		if errors.Is(err, os.ErrNotExist) && createParents {
+			if err := os.Mkdir(current, 0755); err != nil {
+				return "", err
+			}
+			info, err = os.Lstat(current)
+		}
+		if err != nil {
+			if errors.Is(err, os.ErrNotExist) {
+				return filepath.Join(dest, name), nil
+			}
+			return "", err
+		}
+		if !info.IsDir() || info.Mode()&os.ModeSymlink != 0 {
+			return "", fmt.Errorf("artifact destination is not a real directory: %s", current)
+		}
+	}
+	return filepath.Join(dest, name), nil
+}
 func sameFingerprint(a, b *artifactFingerprint) bool { return a != nil && b != nil && *a == *b }
 
 func validArtifactRecord(name string, record artifactRecord) bool {
@@ -213,11 +251,19 @@ func executeHostArtifactPlan(plan hostArtifactPlan, host, sourceRoot string, con
 	if plan.Action != "apply" && plan.Action != "delete" && plan.Action != "cleanup" {
 		return errors.New("invalid host artifact action")
 	}
+	if plan.DestinationPath != "" && !validArtifactName(plan.DestinationPath, r.Kind) {
+		return errors.New("invalid artifact destination path")
+	}
 	if plan.Previous != "" && !validArtifactName(plan.Previous, r.Kind) {
 		return errors.New("invalid previous artifact name")
 	}
-	if plan.Action == "apply" && (!cleanHostPath(plan.Source) || !strings.HasPrefix(plan.Source, "/opt/") || !validArtifactName(filepath.Base(plan.Source), r.Kind)) {
-		return errors.New("invalid artifact source")
+	if plan.Action == "apply" {
+		if plan.DestinationPath == "" {
+			plan.DestinationPath = filepath.Base(plan.Source)
+		}
+		if !cleanHostPath(plan.Source) || !strings.HasPrefix(plan.Source, "/opt/") || !validArtifactName(plan.DestinationPath, r.Kind) {
+			return errors.New("invalid artifact source")
+		}
 	}
 	dest, err := artifactDestination(host, r.Host.Destination)
 	if errors.Is(err, os.ErrNotExist) && plan.Action == "cleanup" {
@@ -265,7 +311,11 @@ func executeHostArtifactPlan(plan hostArtifactPlan, host, sourceRoot string, con
 		return err
 	}
 	check := func(name string) (artifactRecord, *artifactFingerprint, error) {
-		existing, err := fingerprintArtifact(filepath.Join(dest, name))
+		target, err := artifactTargetPath(dest, name, false)
+		if err != nil {
+			return artifactRecord{}, nil, err
+		}
+		existing, err := fingerprintArtifact(target)
 		if err != nil {
 			return artifactRecord{}, nil, err
 		}
@@ -320,7 +370,11 @@ func executeHostArtifactPlan(plan hostArtifactPlan, host, sourceRoot string, con
 			}
 		}
 		if existing != nil {
-			if err := os.Remove(filepath.Join(dest, name)); err != nil {
+			target, err := artifactTargetPath(dest, name, false)
+			if err != nil {
+				return err
+			}
+			if err := os.Remove(target); err != nil {
 				return err
 			}
 		}
@@ -375,7 +429,7 @@ func executeHostArtifactPlan(plan hostArtifactPlan, host, sourceRoot string, con
 		}
 		return nil
 	}
-	name := filepath.Base(plan.Source)
+	name := plan.DestinationPath
 	record, _, err := check(name)
 	if err != nil {
 		return err
@@ -406,7 +460,11 @@ func executeHostArtifactPlan(plan hostArtifactPlan, host, sourceRoot string, con
 	}
 	// An interrupted copy can own either the original inode or the staged inode.
 	if record.Pending != nil {
-		installed, err := fingerprintArtifact(filepath.Join(dest, name))
+		target, err := artifactTargetPath(dest, name, true)
+		if err != nil {
+			return err
+		}
+		installed, err := fingerprintArtifact(target)
 		if err != nil {
 			return err
 		}
@@ -465,7 +523,11 @@ func executeHostArtifactPlan(plan hostArtifactPlan, host, sourceRoot string, con
 			return err
 		}
 	}
-	if err := os.Rename(stagePath, filepath.Join(dest, name)); err != nil {
+	target, err := artifactTargetPath(dest, name, true)
+	if err != nil {
+		return err
+	}
+	if err := os.Rename(stagePath, target); err != nil {
 		return err
 	}
 	if err := syncArtifactDirectory(dest); err != nil {