From 168ab7b8cea319947dfea2552ac00cc27dae03df Mon Sep 17 00:00:00 2001 From: Ryan Cook Date: Tue, 13 Oct 2026 09:00:01 -0400 Subject: [PATCH] Preserve recursive FileTransfer paths Signed-off-by: Ryan Cook --- docs/methods.rst | 20 +++-- docs/release_notes.rst | 10 ++- pkg/engine/container.go | 5 +- pkg/engine/filetransfer.go | 53 +++++++++--- pkg/engine/filetransfer_lifecycle_test.go | 23 +++-- pkg/engine/host_artifact_helper.go | 84 ++++++++++++++++--- pkg/engine/host_artifacts.go | 13 +-- pkg/engine/host_artifacts_integration_test.go | 28 +++++-- pkg/engine/host_artifacts_test.go | 40 ++++++++- pkg/engine/systemd.go | 10 ++- 10 files changed, 228 insertions(+), 58 deletions(-) diff --git a/docs/methods.rst b/docs/methods.rst index 0f47e2e9..df69a283 100644 --- a/docs/methods.rst +++ b/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 diff --git a/docs/release_notes.rst b/docs/release_notes.rst index 51feb272..d5b59658 100644 --- a/docs/release_notes.rst +++ b/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 ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ diff --git a/pkg/engine/container.go b/pkg/engine/container.go index d13b9f6a..2e5662b2 100644 --- a/pkg/engine/container.go +++ b/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 diff --git a/pkg/engine/filetransfer.go b/pkg/engine/filetransfer.go index 43b16776..cebc3284 100644 --- a/pkg/engine/filetransfer.go +++ b/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,12 +59,21 @@ 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 { @@ -70,27 +81,42 @@ func (ft *FileTransfer) MethodEngine(ctx, conn context.Context, change *object.C } 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 +} diff --git a/pkg/engine/filetransfer_lifecycle_test.go b/pkg/engine/filetransfer_lifecycle_test.go index d607b7ff..ba891f83 100644 --- a/pkg/engine/filetransfer_lifecycle_test.go +++ b/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) diff --git a/pkg/engine/host_artifact_helper.go b/pkg/engine/host_artifact_helper.go index 88646a6d..49572a1c 100644 --- a/pkg/engine/host_artifact_helper.go +++ b/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 { diff --git a/pkg/engine/host_artifacts.go b/pkg/engine/host_artifacts.go index 93fd7bc6..9c3c0117 100644 --- a/pkg/engine/host_artifacts.go +++ b/pkg/engine/host_artifacts.go @@ -25,12 +25,13 @@ type hostRemoval struct { } type hostArtifactPlan struct { - Receipt removalReceipt `json:"receipt"` - Action string `json:"action"` - Source string `json:"source,omitempty"` - Previous string `json:"previous,omitempty"` - Enable bool `json:"enable,omitempty"` - Restart bool `json:"restart,omitempty"` + Receipt removalReceipt `json:"receipt"` + Action string `json:"action"` + Source string `json:"source,omitempty"` + DestinationPath string `json:"destinationPath,omitempty"` + Previous string `json:"previous,omitempty"` + Enable bool `json:"enable,omitempty"` + Restart bool `json:"restart,omitempty"` } func init() { diff --git a/pkg/engine/host_artifacts_integration_test.go b/pkg/engine/host_artifacts_integration_test.go index 3ac2be53..bf3e9d69 100644 --- a/pkg/engine/host_artifacts_integration_test.go +++ b/pkg/engine/host_artifacts_integration_test.go @@ -52,12 +52,23 @@ func TestHostArtifactsPodmanLifecycle(t *testing.T) { wantedBy = "multi-user.target" } unitText := "[Unit]\nDescription=FetchIt artifact lifecycle fixture\n[Service]\nExecStart=/usr/bin/sleep infinity\n[Install]\nWantedBy=" + wantedBy + "\n" - files := map[string]string{"owned file.txt": "original", unit: unitText, "legacy.txt": "legacy"} + fileSource := filepath.Join(source, "artifact-fixture", "files", "nested") + serviceSource := filepath.Join(source, "artifact-fixture", "services") + if err := os.MkdirAll(fileSource, 0755); err != nil { + t.Fatal(err) + } + if err := os.MkdirAll(serviceSource, 0755); err != nil { + t.Fatal(err) + } + files := map[string]string{"owned file.txt": "original", "legacy.txt": "legacy"} for file, contents := range files { - if err := os.WriteFile(filepath.Join(source, file), []byte(contents), 0644); err != nil { + if err := os.WriteFile(filepath.Join(fileSource, file), []byte(contents), 0644); err != nil { t.Fatal(err) } } + if err := os.WriteFile(filepath.Join(serviceSource, unit), []byte(unitText), 0644); err != nil { + t.Fatal(err) + } uploader := command("create", "--network=none", "--entrypoint", "/bin/sleep", "-v", fetchitVolume+":/opt", fetchitImage, "infinity") t.Cleanup(func() { exec.Command("podman", "rm", "-f", uploader).Run() }) command("cp", source+"/.", uploader+":/opt/") @@ -76,10 +87,12 @@ func TestHostArtifactsPodmanLifecycle(t *testing.T) { if err := store.configure(methods); err != nil { t.Fatal(err) } - if err := ft.MethodEngine(context.Background(), conn, nil, "owned file.txt"); err != nil { + ownedPathInRepo := filepath.Join("artifact-fixture", "files", "nested", "owned file.txt") + if err := ft.MethodEngine(context.Background(), conn, nil, ownedPathInRepo); err != nil { t.Fatal(err) } - if err := legacy.MethodEngine(context.Background(), conn, nil, "legacy.txt"); err != nil { + legacyPathInRepo := filepath.Join("artifact-fixture", "files", "nested", "legacy.txt") + if err := legacy.MethodEngine(context.Background(), conn, nil, legacyPathInRepo); err != nil { t.Fatal(err) } unrelated := filepath.Join(dest, "unrelated.txt") @@ -104,7 +117,8 @@ func TestHostArtifactsPodmanLifecycle(t *testing.T) { systemctl("daemon-reload") } }) - if err := sd.MethodEngine(context.Background(), conn, nil, unit); err != nil { + unitPathInRepo := filepath.Join("artifact-fixture", "services", unit) + if err := sd.MethodEngine(context.Background(), conn, nil, unitPathInRepo); err != nil { t.Fatal(err) } installed = true @@ -113,7 +127,7 @@ func TestHostArtifactsPodmanLifecycle(t *testing.T) { } // Stop relying on Git/source files before reconstructing removal receipts. command("volume", "rm", fetchitVolume) - ownedPath := filepath.Join(dest, "owned file.txt") + ownedPath := filepath.Join(dest, "nested", "owned file.txt") if err := os.WriteFile(ownedPath, []byte("external edit"), 0644); err != nil { t.Fatal(err) } @@ -155,7 +169,7 @@ func TestHostArtifactsPodmanLifecycle(t *testing.T) { if _, err := os.Stat(ownedPath); !errors.Is(err, os.ErrNotExist) { t.Fatal("owned file remains after retry", err) } - for _, path := range []string{unrelated, filepath.Join(legacyDest, "legacy.txt")} { + for _, path := range []string{unrelated, filepath.Join(legacyDest, "nested", "legacy.txt")} { if _, err := os.Stat(path); err != nil { t.Fatal("retained file removed", path, err) } diff --git a/pkg/engine/host_artifacts_test.go b/pkg/engine/host_artifacts_test.go index 1df96431..60e29fb3 100644 --- a/pkg/engine/host_artifacts_test.go +++ b/pkg/engine/host_artifacts_test.go @@ -30,7 +30,11 @@ func artifactFixture(t *testing.T, systemd bool) (hostArtifactPlan, string, stri plan := hostArtifactPlan{Receipt: removalReceipt{Kind: kind, Owner: kubeOwner("fixture", kind, "", ""), Host: h}, Action: "apply"} write := func(name, text string) { t.Helper() - if err := os.WriteFile(filepath.Join(sourceRoot, "opt", "repo", name), []byte(text), 0644); err != nil { + path := filepath.Join(sourceRoot, "opt", "repo", name) + if err := os.MkdirAll(filepath.Dir(path), 0755); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(path, []byte(text), 0644); err != nil { t.Fatal(err) } } @@ -94,6 +98,40 @@ func TestHostArtifactsChangedFilesRetained(t *testing.T) { }) } } + +func TestHostArtifactsPreserveNestedDestinationPaths(t *testing.T) { + plan, host, sourceRoot, write := artifactFixture(t, false) + write("nested/old.txt", "old") + plan.Source = "/opt/repo/nested/old.txt" + plan.DestinationPath = "nested/old.txt" + if err := executeHostArtifactPlan(plan, host, sourceRoot, noArtifactControl); err != nil { + t.Fatal(err) + } + + write("nested/deep/new.txt", "new") + plan.Source = "/opt/repo/nested/deep/new.txt" + plan.DestinationPath = "nested/deep/new.txt" + plan.Previous = "nested/old.txt" + if err := executeHostArtifactPlan(plan, host, sourceRoot, noArtifactControl); err != nil { + t.Fatal(err) + } + + dest := filepath.Join(host, plan.Receipt.Host.Destination) + if _, err := os.Stat(filepath.Join(dest, "nested", "old.txt")); !errors.Is(err, os.ErrNotExist) { + t.Fatal("rename left old nested file", err) + } + if data, err := os.ReadFile(filepath.Join(dest, "nested", "deep", "new.txt")); err != nil || string(data) != "new" { + t.Fatalf("new nested file missing or wrong: %q %v", data, err) + } + + plan.Action, plan.Source, plan.DestinationPath, plan.Previous = "delete", "", "", "nested/deep/new.txt" + if err := executeHostArtifactPlan(plan, host, sourceRoot, noArtifactControl); err != nil { + t.Fatal(err) + } + if _, err := os.Stat(filepath.Join(dest, "nested", "deep", "new.txt")); !errors.Is(err, os.ErrNotExist) { + t.Fatal("nested file remains", err) + } +} func TestHostArtifactsRefuseUnmanagedAndConflictingOwners(t *testing.T) { plan, host, sourceRoot, write := artifactFixture(t, false) write("file.txt", "managed") diff --git a/pkg/engine/systemd.go b/pkg/engine/systemd.go index b7fd5352..72e72963 100644 --- a/pkg/engine/systemd.go +++ b/pkg/engine/systemd.go @@ -206,7 +206,15 @@ func (sd *Systemd) systemdPodman(ctx context.Context, conn context.Context, path Name: sd.Name, }, } - if err := ft.fileTransferPodman(ctx, conn, path, dest, prev); err != nil { + relativePath := "" + if path != deleteFile { + relativePath = filepath.Base(path) + } + previousPath := "" + if prev != nil { + previousPath = filepath.Base(*prev) + } + if err := ft.fileTransferPodman(ctx, conn, path, relativePath, dest, previousPath); err != nil { return utils.WrapErr(err, "Error deploying systemd %s file(s), Path: %s", sd.Name, sd.TargetPath) } }