From b75482b5310cdb82448e8869fecee90ada254e39 Mon Sep 17 00:00:00 2001 From: mitchell Date: Fri, 22 Nov 2024 17:18:13 -0500 Subject: [PATCH] When deploying via link, deploy symlinks directly instead of resolving them first. --- internal/smartlink/smartlink.go | 32 +++++++++++----------------- internal/smartlink/smartlink_test.go | 2 +- pkg/runtime/depot.go | 4 ++-- pkg/runtime/links_windows.go | 2 +- 4 files changed, 17 insertions(+), 23 deletions(-) diff --git a/internal/smartlink/smartlink.go b/internal/smartlink/smartlink.go index 2c62efd01f..f32c806a6b 100644 --- a/internal/smartlink/smartlink.go +++ b/internal/smartlink/smartlink.go @@ -10,7 +10,7 @@ import ( ) // LinkContents will link the contents of src to desc -func LinkContents(src, dest string, visited map[string]bool) error { +func LinkContents(src, dest string) error { if !fileutils.DirExists(src) { return errs.New("src dir does not exist: %s", src) } @@ -29,7 +29,7 @@ func LinkContents(src, dest string, visited map[string]bool) error { return errs.Wrap(err, "Reading dir %s failed", src) } for _, entry := range entries { - if err := Link(filepath.Join(src, entry.Name()), filepath.Join(dest, entry.Name()), visited); err != nil { + if err := Link(filepath.Join(src, entry.Name()), filepath.Join(dest, entry.Name())); err != nil { return errs.Wrap(err, "Link failed") } } @@ -39,27 +39,21 @@ func LinkContents(src, dest string, visited map[string]bool) error { // Link creates a link from src to target. MS decided to support Symlinks but only if you opt into developer mode (go figure), // which we cannot reasonably force on our users. So on Windows we will instead create dirs and hardlinks. -func Link(src, dest string, visited map[string]bool) error { - srcWasSymlink := isSymlink(src) - +func Link(src, dest string) error { var err error src, dest, err = resolvePaths(src, dest) if err != nil { return errs.Wrap(err, "Could not resolve src and dest paths") } - if visited == nil { - visited = make(map[string]bool) - } - if _, exists := visited[src]; exists && srcWasSymlink { - // We've encountered a recursive link. This is most often the case when the resolved src has - // already been visited. In that case, just link the dest to the src (which may be a directory; - // this is fine). - return linkFile(src, dest) - } - visited[src] = true - if fileutils.IsDir(src) { + if isSymlink(src) { + // Links to directories are okay on Linux and macOS, but will fail on Windows. + // If we ever get here on Windows, the artifact being deployed is bad and there's nothing we + // can do about it except receive the report from Rollbar and report it internally. + return linkFile(src, dest) + } + if err := fileutils.Mkdir(dest); err != nil { return errs.Wrap(err, "could not create directory %s", dest) } @@ -68,7 +62,7 @@ func Link(src, dest string, visited map[string]bool) error { return errs.Wrap(err, "could not read directory %s", src) } for _, entry := range entries { - if err := Link(filepath.Join(src, entry.Name()), filepath.Join(dest, entry.Name()), visited); err != nil { + if err := Link(filepath.Join(src, entry.Name()), filepath.Join(dest, entry.Name())); err != nil { return errs.Wrap(err, "sub link failed") } } @@ -153,11 +147,11 @@ func isSymlink(src string) bool { // This is to ensure that we're always comparing apples to apples when doing string comparisons on paths. func resolvePaths(src, dest string) (string, string, error) { var err error - src, err = fileutils.ResolveUniquePath(src) + src, err = filepath.Abs(filepath.Clean(src)) if err != nil { return "", "", errs.Wrap(err, "Could not resolve src path") } - dest, err = fileutils.ResolveUniquePath(dest) + dest, err = filepath.Abs(filepath.Clean(dest)) if err != nil { return "", "", errs.Wrap(err, "Could not resolve dest path") } diff --git a/internal/smartlink/smartlink_test.go b/internal/smartlink/smartlink_test.go index 5b52fbb0d3..50a772254f 100644 --- a/internal/smartlink/smartlink_test.go +++ b/internal/smartlink/smartlink_test.go @@ -42,7 +42,7 @@ func TestLinkContentsWithCircularLink(t *testing.T) { err = os.Symlink(subDir, circularLink) require.NoError(t, err) - err = LinkContents(srcDir, destDir, nil) + err = LinkContents(srcDir, destDir) if runtime.GOOS == "windows" { require.Error(t, err) return // hard links to directories are not allowed on Windows diff --git a/pkg/runtime/depot.go b/pkg/runtime/depot.go index c53a1ac1e6..610418e952 100644 --- a/pkg/runtime/depot.go +++ b/pkg/runtime/depot.go @@ -177,7 +177,7 @@ func (d *depot) DeployViaLink(id strfmt.UUID, relativeSrc, absoluteDest string) } // Copy or link the artifact files, depending on whether the artifact in question relies on file transformations - if err := smartlink.LinkContents(absoluteSrc, absoluteDest, nil); err != nil { + if err := smartlink.LinkContents(absoluteSrc, absoluteDest); err != nil { return errs.Wrap(err, "failed to link artifact") } @@ -295,7 +295,7 @@ func (d *depot) Undeploy(id strfmt.UUID, relativeSrc, path string) error { for sharedFile, relinkSrc := range redeploys { switch deploy.Type { case deploymentTypeLink: - if err := smartlink.Link(relinkSrc, sharedFile, nil); err != nil { + if err := smartlink.Link(relinkSrc, sharedFile); err != nil { return errs.Wrap(err, "failed to relink file") } case deploymentTypeCopy: diff --git a/pkg/runtime/links_windows.go b/pkg/runtime/links_windows.go index 907882a66f..0bdbbe79e8 100644 --- a/pkg/runtime/links_windows.go +++ b/pkg/runtime/links_windows.go @@ -43,7 +43,7 @@ func supportsHardLinks(path string) (supported bool) { } logging.Debug("Attempting to link '%s' to '%s'", lnk, target) - err = smartlink.Link(target, lnk, nil) + err = smartlink.Link(target, lnk) if err != nil { logging.Debug("Test link creation failed: %v", err) return false