From a486b0a02ec5bde9c2e072d6b9f391f33cdb956c Mon Sep 17 00:00:00 2001 From: apostasie Date: Sat, 17 Aug 2024 14:11:00 -0700 Subject: [PATCH] Rewrite cp Signed-off-by: apostasie --- cmd/nerdctl/container_cp_linux_test.go | 1164 ++++++++++++++++-------- pkg/cmd/container/cp_linux.go | 6 +- pkg/containerutil/cp_linux.go | 366 ++++---- pkg/containerutil/resolve.go | 243 +++++ pkg/testutil/testutil.go | 2 + 5 files changed, 1258 insertions(+), 523 deletions(-) create mode 100644 pkg/containerutil/resolve.go diff --git a/cmd/nerdctl/container_cp_linux_test.go b/cmd/nerdctl/container_cp_linux_test.go index 33f59ba45d3..c115d7e328d 100644 --- a/cmd/nerdctl/container_cp_linux_test.go +++ b/cmd/nerdctl/container_cp_linux_test.go @@ -25,396 +25,838 @@ import ( "testing" "gotest.tools/v3/assert" + "gotest.tools/v3/icmd" + "github.com/containerd/nerdctl/v2/pkg/containerutil" "github.com/containerd/nerdctl/v2/pkg/rootlessutil" "github.com/containerd/nerdctl/v2/pkg/testutil" ) -func TestCopyToContainer(t *testing.T) { +// For the test matrix, see https://docs.docker.com/engine/reference/commandline/cp/ +// Obviously, none of this is fully windows ready - obviously `cp` itself is not either so, ok for now. +// srcDirName and srcFileName are tokens to be used by tests to build path specs +const ( + // TODO: fuzz this more seriously + // FIXME: the following are currently breaking (anything that will evaluate on the shell, obviously): + // - ` + // - $a, ${a}, etc + // Note that this currently break *the test*, but the code might also be broken + + complexify = "~a0-_.(){}[]*#! \"'∞" + + pathDoesNotExistRelative = "does-not-exist-" + complexify + pathDoesNotExistAbsolute = string(os.PathSeparator) + "does-not-exist-" + complexify + pathIsAFileRelative = "is-a-file-" + complexify + pathIsAFileAbsolute = string(os.PathSeparator) + "is-a-file-" + complexify + pathIsADirRelative = "is-a-dir-" + complexify + pathIsADirAbsolute = string(os.PathSeparator) + "is-a-dir-" + complexify + pathIsAVolumeMount = string(os.PathSeparator) + "is-a-volume-mount-" + complexify + + srcFileName = "test-file-" + complexify + + containerCwd = "/nerdctl/cp/test" +) + +var srcDirName = filepath.Join("three-levels-src-dir", "test-dir", "dir-"+complexify) + +type testgroup struct { + description string // parent test description + toContainer bool // copying to, or from container + + // sourceSpec as specified by the user (without the container: part) - can be relative or absolute - + // if sourceSpec points to a file, you must use srcFileName for filename + sourceSpec string + sourceIsAFile bool // whether the provided sourceSpec points to a file or a dir + testCases []testcases // testcases +} + +type testcases struct { + description string // textual description of what the test is doing + destinationSpec string // destination path as specified by the user (without the container: part) - can be relative or absolute + expect icmd.Expected // expectation + + // Optional + catFile string // path that we "cat" - defaults to destinationSpec if not specified + setup func(base *testutil.Base, container string, destPath string) // additional test setup if needed + tearDown func() // additional cleanup if needed + volume func(base *testutil.Base, id string) (string, string, bool) // volume creation function if needed (should return the volume name, mountPoint, readonly flag) +} + +func TestCopyToContainerNG(t *testing.T) { t.Parallel() - base := testutil.NewBase(t) - testContainer := testutil.Identifier(t) - testStoppedContainer := "stopped-container-" + testutil.Identifier(t) - - base.Cmd("run", "-d", "--name", testContainer, testutil.CommonImage, "sleep", "1h").AssertOK() - defer base.Cmd("rm", "-f", testContainer).Run() - - base.Cmd("run", "-d", "--name", testStoppedContainer, testutil.CommonImage, "sleep", "1h").AssertOK() - defer base.Cmd("rm", "-f", testStoppedContainer).Run() - // Stop container immediately after starting for testing copying into stopped container - base.Cmd("stop", testStoppedContainer).AssertOK() - srcUID := os.Geteuid() - srcDir := t.TempDir() - srcFile := filepath.Join(srcDir, "test-file") - srcFileContent := []byte("test-file-content") - err := os.WriteFile(srcFile, srcFileContent, 0o644) - assert.NilError(t, err) - - assertCat := func(catPath string, testContainer string, stopped bool) { - if stopped { - base.Cmd("start", testContainer).AssertOK() - defer base.Cmd("stop", testContainer).AssertOK() - } + + testGroups := []*testgroup{ + { + description: "Copying to container, SRC_PATH is a file, absolute", + sourceSpec: filepath.Join(string(os.PathSeparator), srcDirName, srcFileName), + sourceIsAFile: true, + toContainer: true, + testCases: []testcases{ + { + description: "DEST_PATH does not exist, relative", + destinationSpec: pathDoesNotExistRelative, + expect: icmd.Expected{ + ExitCode: 0, + }, + }, + { + description: "DEST_PATH does not exist, absolute", + destinationSpec: pathDoesNotExistAbsolute, + expect: icmd.Expected{ + ExitCode: 0, + }, + }, + { + description: "DEST_PATH does not exist, relative, and ends with " + string(os.PathSeparator), + destinationSpec: pathDoesNotExistRelative + string(os.PathSeparator), + expect: icmd.Expected{ + ExitCode: 1, + Err: containerutil.ErrDestinationDirMustExist.Error(), + }, + }, + { + description: "DEST_PATH does not exist, absolute, and ends with " + string(os.PathSeparator), + destinationSpec: pathDoesNotExistAbsolute + string(os.PathSeparator), + expect: icmd.Expected{ + ExitCode: 1, + Err: containerutil.ErrDestinationDirMustExist.Error(), + }, + }, + { + description: "DEST_PATH is a file, relative", + destinationSpec: pathIsAFileRelative, + expect: icmd.Expected{ + ExitCode: 0, + }, + setup: func(base *testutil.Base, container string, destPath string) { + base.Cmd("exec", container, "touch", destPath).AssertOK() + }, + }, + { + description: "DEST_PATH is a file, absolute", + destinationSpec: pathIsAFileAbsolute, + expect: icmd.Expected{ + ExitCode: 0, + }, + setup: func(base *testutil.Base, container string, destPath string) { + base.Cmd("exec", container, "touch", destPath).AssertOK() + }, + }, + { + description: "DEST_PATH is a file, relative, ends with improper " + string(os.PathSeparator), + destinationSpec: pathIsAFileRelative + string(os.PathSeparator), + expect: icmd.Expected{ + ExitCode: 1, + Err: containerutil.ErrDestinationDirMustExist.Error(), + }, + setup: func(base *testutil.Base, container string, destPath string) { + base.Cmd("exec", container, "touch", destPath).AssertOK() + }, + }, + { + description: "DEST_PATH is a file, absolute, ends with improper " + string(os.PathSeparator), + destinationSpec: pathIsAFileAbsolute + string(os.PathSeparator), + expect: icmd.Expected{ + ExitCode: 1, + // FIXME: it is unclear why the code path with absolute (this test) versus relative (just above) + // yields a different error. Both should ideally be ErrCannotCopyDirToFile + // This is probably happening somewhere in resolve. + // This is not a deal killer, as both DO error with a reasonable explanation, but a bit + // frustrating + Err: containerutil.ErrFilesystem.Error(), + }, + setup: func(base *testutil.Base, container string, destPath string) { + base.Cmd("exec", container, "touch", destPath).AssertOK() + }, + }, + { + description: "DEST_PATH is a directory, relative", + destinationSpec: pathIsADirRelative, + catFile: filepath.Join(pathIsADirRelative, srcFileName), + expect: icmd.Expected{ + ExitCode: 0, + }, + setup: func(base *testutil.Base, container string, destPath string) { + base.Cmd("exec", container, "mkdir", "-p", destPath).AssertOK() + }, + }, + { + description: "DEST_PATH is a directory, absolute", + destinationSpec: pathIsADirAbsolute, + catFile: filepath.Join(pathIsADirAbsolute, srcFileName), + expect: icmd.Expected{ + ExitCode: 0, + }, + setup: func(base *testutil.Base, container string, destPath string) { + base.Cmd("exec", container, "mkdir", "-p", destPath).AssertOK() + }, + }, + { + description: "DEST_PATH is a directory, relative, ends with " + string(os.PathSeparator), + destinationSpec: pathIsADirRelative + string(os.PathSeparator), + catFile: filepath.Join(pathIsADirRelative, srcFileName), + expect: icmd.Expected{ + ExitCode: 0, + }, + setup: func(base *testutil.Base, container string, destPath string) { + base.Cmd("exec", container, "mkdir", "-p", destPath).AssertOK() + }, + }, + { + description: "DEST_PATH is a directory, absolute, ends with " + string(os.PathSeparator), + destinationSpec: pathIsADirAbsolute + string(os.PathSeparator), + catFile: filepath.Join(pathIsADirAbsolute, srcFileName), + expect: icmd.Expected{ + ExitCode: 0, + }, + setup: func(base *testutil.Base, container string, destPath string) { + base.Cmd("exec", container, "mkdir", "-p", destPath).AssertOK() + }, + }, + { + description: "DEST_PATH is a volume mount-point", + destinationSpec: pathIsAVolumeMount, + catFile: filepath.Join(pathIsAVolumeMount, srcFileName), + expect: icmd.Expected{ + ExitCode: 0, + }, + // FIXME the way we handle volume is not right - too complicated for the test author + volume: func(base *testutil.Base, id string) (string, string, bool) { + base.Cmd("volume", "create", id).AssertOK() + return id, pathIsAVolumeMount, false + }, + }, + { + description: "Destination path is a read-only volume mount-point", + destinationSpec: pathIsAVolumeMount, + expect: icmd.Expected{ + ExitCode: 1, + Err: containerutil.ErrTargetIsReadOnly.Error(), + }, + volume: func(base *testutil.Base, id string) (string, string, bool) { + base.Cmd("volume", "create", id).AssertOK() + return id, pathIsAVolumeMount, true + }, + }, + }, + }, + { + description: "Copying to container, SRC_PATH is a directory", + sourceSpec: srcDirName, + toContainer: true, + testCases: []testcases{ + { + description: "DEST_PATH does not exist, relative", + destinationSpec: pathDoesNotExistRelative, + catFile: filepath.Join(pathDoesNotExistRelative, srcFileName), + expect: icmd.Expected{ + ExitCode: 0, + }, + }, + { + description: "DEST_PATH does not exist, absolute", + destinationSpec: pathDoesNotExistAbsolute, + catFile: filepath.Join(pathDoesNotExistAbsolute, srcFileName), + expect: icmd.Expected{ + ExitCode: 0, + }, + }, + { + description: "DEST_PATH does not exist, relative, and ends with " + string(os.PathSeparator), + destinationSpec: pathDoesNotExistRelative + string(os.PathSeparator), + catFile: filepath.Join(pathDoesNotExistRelative, srcFileName), + expect: icmd.Expected{ + ExitCode: 0, + }, + }, + { + description: "DEST_PATH does not exist, absolute, and ends with " + string(os.PathSeparator), + destinationSpec: pathDoesNotExistAbsolute + string(os.PathSeparator), + catFile: filepath.Join(pathDoesNotExistAbsolute, srcFileName), + expect: icmd.Expected{ + ExitCode: 0, + }, + }, + { + description: "DEST_PATH is a file, relative", + destinationSpec: pathIsAFileRelative, + expect: icmd.Expected{ + ExitCode: 1, + Err: containerutil.ErrCannotCopyDirToFile.Error(), + }, + setup: func(base *testutil.Base, container string, destPath string) { + base.Cmd("exec", container, "touch", destPath).AssertOK() + }, + }, + { + description: "DEST_PATH is a file, absolute", + destinationSpec: pathIsAFileAbsolute, + expect: icmd.Expected{ + ExitCode: 1, + Err: containerutil.ErrCannotCopyDirToFile.Error(), + }, + setup: func(base *testutil.Base, container string, destPath string) { + base.Cmd("exec", container, "touch", destPath).AssertOK() + }, + }, + { + description: "DEST_PATH is a file, relative, ends with improper " + string(os.PathSeparator), + destinationSpec: pathIsAFileRelative + string(os.PathSeparator), + expect: icmd.Expected{ + ExitCode: 1, + Err: containerutil.ErrCannotCopyDirToFile.Error(), + }, + setup: func(base *testutil.Base, container string, destPath string) { + base.Cmd("exec", container, "touch", destPath).AssertOK() + }, + }, + { + description: "DEST_PATH is a file, absolute, ends with improper " + string(os.PathSeparator), + destinationSpec: pathIsAFileAbsolute + string(os.PathSeparator), + expect: icmd.Expected{ + ExitCode: 1, + // FIXME: it is unclear why the code path with absolute (this test) versus relative (just above) + // yields a different error. Both should ideally be ErrCannotCopyDirToFile + // This is probably happening somewhere in resolve. + // This is not a deal killer, as both DO error with a reasonable explanation, but a bit + // frustrating + Err: containerutil.ErrFilesystem.Error(), + }, + setup: func(base *testutil.Base, container string, destPath string) { + base.Cmd("exec", container, "touch", destPath).AssertOK() + }, + }, + { + description: "DEST_PATH is a directory, relative", + destinationSpec: pathIsADirRelative, + catFile: filepath.Join(pathIsADirRelative, filepath.Base(srcDirName), srcFileName), + expect: icmd.Expected{ + ExitCode: 0, + }, + setup: func(base *testutil.Base, container string, destPath string) { + base.Cmd("exec", container, "mkdir", "-p", destPath).AssertOK() + }, + }, + { + description: "DEST_PATH is a directory, absolute", + destinationSpec: pathIsADirAbsolute, + catFile: filepath.Join(pathIsADirAbsolute, filepath.Base(srcDirName), srcFileName), + expect: icmd.Expected{ + ExitCode: 0, + }, + setup: func(base *testutil.Base, container string, destPath string) { + base.Cmd("exec", container, "mkdir", "-p", destPath).AssertOK() + }, + }, + { + description: "DEST_PATH is a directory, relative, ends with " + string(os.PathSeparator), + destinationSpec: pathIsADirRelative + string(os.PathSeparator), + catFile: filepath.Join(pathIsADirRelative, filepath.Base(srcDirName), srcFileName), + expect: icmd.Expected{ + ExitCode: 0, + }, + setup: func(base *testutil.Base, container string, destPath string) { + base.Cmd("exec", container, "mkdir", "-p", destPath).AssertOK() + }, + }, + { + description: "DEST_PATH is a directory, absolute, ends with " + string(os.PathSeparator), + destinationSpec: pathIsADirAbsolute + string(os.PathSeparator), + catFile: filepath.Join(pathIsADirAbsolute, filepath.Base(srcDirName), srcFileName), + expect: icmd.Expected{ + ExitCode: 0, + }, + setup: func(base *testutil.Base, container string, destPath string) { + base.Cmd("exec", container, "mkdir", "-p", destPath).AssertOK() + }, + }, + }, + }, + { + description: "Copying to container, SRC_PATH is a directory ending with /.", + sourceSpec: srcDirName + string(os.PathSeparator) + ".", + toContainer: true, + testCases: []testcases{ + { + description: "DEST_PATH is a directory, relative", + destinationSpec: pathIsADirRelative, + catFile: filepath.Join(pathIsADirRelative, srcFileName), + setup: func(base *testutil.Base, container string, destPath string) { + base.Cmd("exec", container, "mkdir", "-p", destPath).AssertOK() + }, + }, + { + description: "DEST_PATH is a directory, absolute", + destinationSpec: pathIsADirAbsolute, + catFile: filepath.Join(pathIsADirAbsolute, srcFileName), + setup: func(base *testutil.Base, container string, destPath string) { + base.Cmd("exec", container, "mkdir", "-p", destPath).AssertOK() + }, + }, + }, + }, + } + + assertCat := func(base *testutil.Base, catPath string, fileContent []byte, container string, expectedUid int, containerIsStopped bool) { t.Logf("catPath=%q", catPath) - base.Cmd("exec", testContainer, "cat", catPath).AssertOutExactly(string(srcFileContent)) - base.Cmd("exec", testContainer, "stat", "-c", "%u", catPath).AssertOutExactly(fmt.Sprintf("%d\n", srcUID)) + if container != "" && containerIsStopped { + base.Cmd("start", container).AssertOK() + defer base.Cmd("stop", container).AssertOK() + } + + if container == "" { + got, err := os.ReadFile(catPath) + assert.NilError(t, err, "Failed reading from file") + assert.DeepEqual(t, fileContent, got) + st, err := os.Stat(catPath) + assert.NilError(t, err) + stSys := st.Sys().(*syscall.Stat_t) + expected := uint32(expectedUid) + actual := stSys.Uid + assert.DeepEqual(t, expected, actual) + } else { + base.Cmd("exec", container, "sh", "-c", "--", fmt.Sprintf("cat %q", catPath)).AssertOutExactly(string(fileContent)) + base.Cmd("exec", container, "stat", "-c", "%u", catPath).AssertOutExactly(fmt.Sprintf("%d\n", expectedUid)) + } } - // For the test matrix, see https://docs.docker.com/engine/reference/commandline/cp/ - t.Run("SRC_PATH specifies a file", func(t *testing.T) { - srcPath := srcFile - t.Run("DEST_PATH does not exist", func(t *testing.T) { - destPath := "/dest-no-exist-no-slash" - base.Cmd("cp", srcPath, testContainer+":"+destPath).AssertOK() - catPath := destPath - assertCat(catPath, testContainer, false) - if rootlessutil.IsRootless() { - t.Skip("Test skipped in rootless mode for testStoppedContainer") - } - base.Cmd("cp", srcPath, testStoppedContainer+":"+destPath).AssertOK() - assertCat(catPath, testStoppedContainer, true) - }) - t.Run("DEST_PATH does not exist and ends with /", func(t *testing.T) { - destPath := "/dest-no-exist-with-slash/" - base.Cmd("cp", srcPath, testContainer+":"+destPath).AssertFail() - if rootlessutil.IsRootless() { - t.Skip("Test skipped in rootless mode for testStoppedContainer") - } - base.Cmd("cp", srcPath, testStoppedContainer+":"+destPath).AssertFail() - }) - t.Run("DEST_PATH exists and is a file", func(t *testing.T) { - destPath := "/dest-file-exists" - base.Cmd("exec", testContainer, "touch", destPath).AssertOK() - base.Cmd("cp", srcPath, testContainer+":"+destPath).AssertOK() - catPath := destPath - assertCat(catPath, testContainer, false) - if rootlessutil.IsRootless() { - t.Skip("Test skipped in rootless mode for testStoppedContainer") - } - base.Cmd("cp", srcPath, testStoppedContainer+":"+destPath).AssertOK() - assertCat(catPath, testStoppedContainer, true) - }) - t.Run("DEST_PATH exists and is a directory", func(t *testing.T) { - destPath := "/dest-dir-exists" - base.Cmd("exec", testContainer, "mkdir", "-p", destPath).AssertOK() - base.Cmd("cp", srcPath, testContainer+":"+destPath).AssertOK() - catPath := filepath.Join(destPath, filepath.Base(srcFile)) - assertCat(catPath, testContainer, false) - if rootlessutil.IsRootless() { - t.Skip("Test skipped in rootless mode for testStoppedContainer") - } - base.Cmd("start", testStoppedContainer).AssertOK() - base.Cmd("exec", testStoppedContainer, "mkdir", "-p", destPath).AssertOK() - base.Cmd("stop", testStoppedContainer).AssertOK() - base.Cmd("cp", srcPath, testStoppedContainer+":"+destPath).AssertOK() - assertCat(catPath, testStoppedContainer, true) - }) - t.Run("DEST_PATH is in a volume", func(t *testing.T) { - // Create a volume - vol := "somevol" - base.Cmd("volume", "create", vol).AssertOK() - defer base.Cmd("volume", "rm", vol).Run() - con := fmt.Sprintf("%s-with-volume", testContainer) - mountDir := "/some_dir" - base.Cmd("run", "-d", "--name", con, "-v", fmt.Sprintf("%s:%s", vol, mountDir), testutil.CommonImage, "sleep", "1h").AssertOK() - defer base.Cmd("rm", "-f", con).Run() - catPath := filepath.Join(mountDir, filepath.Base(srcFile)) - // Running container test - base.Cmd("cp", srcPath, con+":"+mountDir).AssertOK() - assertCat(catPath, con, false) - - // Skip for rootless - if rootlessutil.IsRootless() { - t.Skip("Test skipped in rootless mode for testStoppedContainer") - } - // Stopped container test - // Delete previously copied file - base.Cmd("exec", con, "rm", catPath).AssertOK() - base.Cmd("stop", con).AssertOK() - base.Cmd("cp", srcPath, con+":"+mountDir).AssertOK() - assertCat(catPath, con, true) - }) - t.Run("Destination path is a read-only", func(t *testing.T) { - vol := "somevol" - base.Cmd("volume", "create", vol).AssertOK() - defer base.Cmd("volume", "rm", vol).Run() - con := fmt.Sprintf("%s-with-read-only-volume", testContainer) - mountDir := "/some_dir" - // Create container with read-only volume mounted - base.Cmd("run", "-d", "--name", con, "-v", fmt.Sprintf("%s:%s:ro", vol, mountDir), testutil.CommonImage, "sleep", "1h").AssertOK() - defer base.Cmd("rm", "-f", con).Run() - base.Cmd("cp", srcPath, con+":"+mountDir).AssertFail() - - // Skip for rootless - if rootlessutil.IsRootless() { - t.Skip("Test skipped in rootless mode for testStoppedContainer") - } + for _, tg := range testGroups { + // Get the source path + groupSourceSpec := tg.sourceSpec + groupSourceDir := groupSourceSpec + if tg.sourceIsAFile { + groupSourceDir = filepath.Dir(groupSourceSpec) + } - // Stopped container test - // Delete previously copied file - base.Cmd("stop", con).AssertOK() - base.Cmd("cp", srcPath, con+":"+mountDir).AssertFail() - }) - t.Run("Destination path is a read-only and default tmpfs mount point", func(t *testing.T) { - vol := "somevol" - base.Cmd("volume", "create", vol).AssertOK() - defer base.Cmd("volume", "rm", vol).Run() - con := fmt.Sprintf("%s-with-read-only-volume", testContainer) - - // /tmp is from rootfs of alpine - mountDir := "/tmp" - // Create container with read-only mounted volume mounted at /tmp - base.Cmd("run", "-d", "--name", con, "-v", fmt.Sprintf("%s:%s:ro", vol, mountDir), testutil.CommonImage, "sleep", "1h").AssertOK() - defer base.Cmd("rm", "-f", con).Run() - base.Cmd("cp", srcPath, con+":"+mountDir).AssertFail() - - // Skip for rootless - if rootlessutil.IsRootless() { - t.Skip("Test skipped in rootless mode for testStoppedContainer") - } + // Copy direction + copyToContainer := tg.toContainer + // Description + description := tg.description + // Test cases + testCases := tg.testCases - // Stopped container test - // Delete previously copied file - base.Cmd("stop", con).AssertOK() - base.Cmd("cp", srcPath, con+":"+mountDir).AssertFail() - }) - }) - t.Run("SRC_PATH specifies a directory", func(t *testing.T) { - srcPath := srcDir - t.Run("DEST_PATH does not exist", func(t *testing.T) { - destPath := "/dest2-no-exist" - base.Cmd("cp", srcPath, testContainer+":"+destPath).AssertOK() - catPath := filepath.Join(destPath, filepath.Base(srcFile)) - assertCat(catPath, testContainer, false) - if rootlessutil.IsRootless() { - t.Skip("Test skipped in rootless mode for testStoppedContainer") - } - base.Cmd("cp", srcPath, testStoppedContainer+":"+destPath).AssertOK() - assertCat(catPath, testStoppedContainer, true) - }) - t.Run("DEST_PATH exists and is a file", func(t *testing.T) { - destPath := "/dest2-file-exists" - base.Cmd("exec", testContainer, "touch", destPath).AssertOK() - base.Cmd("cp", srcPath, testContainer+":"+destPath).AssertFail() - if rootlessutil.IsRootless() { - t.Skip("Test skipped in rootless mode for testStoppedContainer") + // Compute UIDs dependent on cp direction + var srcUID, destUID int + if copyToContainer { + srcUID = os.Geteuid() + destUID = srcUID + } else { + srcUID = 42 + destUID = os.Geteuid() + } + + t.Run(description, func(t *testing.T) { + t.Parallel() + + for _, tc := range testCases { + testCase := tc + + t.Run(testCase.description, func(t *testing.T) { + t.Parallel() + + // Compute test-specific values + testID := testutil.Identifier(t) + containerRunning := testID + "-r" + containerStopped := testID + "-s" + sourceFileContent := []byte(testID) + tempDir := t.TempDir() + + base := testutil.NewBase(t) + // Change working directory for commands to execute to the newly created temp directory on the host + // Note that ChDir won't do in a parallel context - and that setup func on the host below + // has to deal with that problem separately by making sure relative paths are resolved against temp + base.Dir = tempDir + + // Prepare the specs and derived variables + sourceSpec := groupSourceSpec + destinationSpec := testCase.destinationSpec + + // If the test case does not specify a catFile, start with the destination spec + catFile := testCase.catFile + if catFile == "" { + catFile = destinationSpec + } + + sourceFile := filepath.Join(groupSourceDir, srcFileName) + if copyToContainer { + // Use an absolute path for evaluation + if !filepath.IsAbs(catFile) { + catFile = filepath.Join(containerCwd, catFile) + } + // If the sourceFile is still relative, make it absolute to the temp + sourceFile = filepath.Join(tempDir, sourceFile) + // If the spec path for source on the host was absolute, make sure we put that under tempDir + if filepath.IsAbs(sourceSpec) { + sourceSpec = tempDir + sourceSpec + } + } else { + // If we are copying to host, we need to make sure we have an absolute path to cat, relative to temp, + // whether it is relative, or "absolute" + catFile = filepath.Join(tempDir, catFile) + // If the spec for destination on the host was absolute, make sure we put that under tempDir + if filepath.IsAbs(destinationSpec) { + destinationSpec = tempDir + destinationSpec + } + } + + // Teardown: clean-up containers and optional volume + tearDown := func() { + base.Cmd("rm", "-f", containerRunning).Run() + base.Cmd("rm", "-f", containerStopped).Run() + if testCase.volume != nil { + volID, _, _ := testCase.volume(base, testID) + base.Cmd("volume", "rm", volID).Run() + } + } + + // Setup: create volume, containers, create the source file + setup := func() { + args := []string{"run", "-d", "-w", containerCwd} + if testCase.volume != nil { + vol, mount, ro := testCase.volume(base, testID) + volArg := fmt.Sprintf("%s:%s", vol, mount) + if ro { + volArg += ":ro" + } + args = append(args, "-v", volArg) + } + base.Cmd(append(args, "--name", containerRunning, testutil.CommonImage, "sleep", "Inf")...).AssertOK() + base.Cmd(append(args, "--name", containerStopped, testutil.CommonImage, "sleep", "Inf")...).AssertOK() + + if copyToContainer { + // Create file on the host + err := os.MkdirAll(filepath.Dir(sourceFile), 0755) + assert.NilError(t, err) + err = os.WriteFile(sourceFile, sourceFileContent, 0o644) + assert.NilError(t, err) + } else { + // Create file content in the container + mkSrcScript := fmt.Sprintf("mkdir -p %q && echo -n %q >%q && chown %d %q", filepath.Dir(sourceFile), sourceFileContent, sourceFile, srcUID, sourceFile) + base.Cmd("exec", containerRunning, "sh", "-euc", mkSrcScript).AssertOK() + base.Cmd("exec", containerStopped, "sh", "-euc", mkSrcScript).AssertOK() + } + + // If we have optional setup, run that now + if testCase.setup != nil { + // Some specs may come with a trailing slash (proper or improper) + // Setup should still work in all cases (including if its a file), and get through to the actual test + setupDest := destinationSpec + setupDest = strings.TrimSuffix(setupDest, string(os.PathSeparator)) + testCase.setup(base, containerRunning, setupDest) + testCase.setup(base, containerStopped, setupDest) + } + + // Stop the "stopped" container + base.Cmd("stop", containerStopped).AssertOK() + } + + tearDown() + t.Cleanup(tearDown) + // If we have custom teardown, do that + if testCase.tearDown != nil { + testCase.tearDown() + t.Cleanup(testCase.tearDown) + } + + // Do the setup + setup() + + // Build the final src and dest specifiers, including `containerXYZ:` + container := "" + if copyToContainer { + container = containerRunning + base.Cmd("cp", sourceSpec, containerRunning+":"+destinationSpec).Assert(testCase.expect) + } else { + base.Cmd("cp", containerRunning+":"+sourceSpec, destinationSpec).Assert(testCase.expect) + } + + // Run the actual test for the running container + // If we expect the op to be a success, also check the destination file + if testCase.expect.ExitCode == 0 { + assertCat(base, catFile, sourceFileContent, container, destUID, false) + } + + // ... and for the stopped container + container = "" + var cmd *testutil.Cmd + if copyToContainer { + container = containerStopped + cmd = base.Cmd("cp", sourceSpec, containerStopped+":"+destinationSpec) + } else { + cmd = base.Cmd("cp", containerStopped+":"+sourceSpec, destinationSpec) + } + + if rootlessutil.IsRootless() { + cmd.Assert( + icmd.Expected{ + ExitCode: 1, + Err: containerutil.ErrRootlessCannotCp.Error(), + }) + return + } + + cmd.Assert(testCase.expect) + if testCase.expect.ExitCode == 0 { + assertCat(base, catFile, sourceFileContent, container, destUID, true) + } + }) } - base.Cmd("start", testStoppedContainer).AssertOK() - base.Cmd("exec", testStoppedContainer, "touch", destPath).AssertOK() - base.Cmd("stop", testStoppedContainer).AssertOK() - base.Cmd("cp", srcPath, testStoppedContainer+":"+destPath).AssertFail() }) - t.Run("DEST_PATH exists and is a directory", func(t *testing.T) { - t.Run("SRC_PATH does not end with `/.`", func(t *testing.T) { - destPath := "/dest2-dir-exists" - base.Cmd("exec", testContainer, "mkdir", "-p", destPath).AssertOK() - base.Cmd("cp", srcPath, testContainer+":"+destPath).AssertOK() - catPath := filepath.Join(destPath, strings.TrimPrefix(srcFile, filepath.Dir(srcDir)+"/")) - assertCat(catPath, testContainer, false) - if rootlessutil.IsRootless() { - t.Skip("Test skipped in rootless mode for testStoppedContainer") - } - base.Cmd("start", testStoppedContainer).AssertOK() - base.Cmd("exec", testStoppedContainer, "mkdir", "-p", destPath).AssertOK() - base.Cmd("stop", testStoppedContainer).AssertOK() - base.Cmd("cp", srcPath, testStoppedContainer+":"+destPath).AssertOK() - assertCat(catPath, testStoppedContainer, true) - }) - t.Run("SRC_PATH does end with `/.`", func(t *testing.T) { - srcPath += "/." - destPath := "/dest2-dir2-exists" - base.Cmd("exec", testContainer, "mkdir", "-p", destPath).AssertOK() - base.Cmd("cp", srcPath, testContainer+":"+destPath).AssertOK() - catPath := filepath.Join(destPath, filepath.Base(srcFile)) - t.Logf("catPath=%q", catPath) - assertCat(catPath, testContainer, false) - if rootlessutil.IsRootless() { - t.Skip("Test skipped in rootless mode for testStoppedContainer") - } - base.Cmd("start", testStoppedContainer).AssertOK() - base.Cmd("exec", testStoppedContainer, "mkdir", "-p", destPath).AssertOK() - base.Cmd("stop", testStoppedContainer).AssertOK() - base.Cmd("cp", srcPath, testStoppedContainer+":"+destPath).AssertOK() - assertCat(catPath, testStoppedContainer, true) - }) - }) - }) + } } +/* func TestCopyFromContainer(t *testing.T) { t.Parallel() - base := testutil.NewBase(t) - testContainer := testutil.Identifier(t) - testStoppedContainer := "stopped-container-" + testutil.Identifier(t) - base.Cmd("run", "-d", "--name", testContainer, testutil.CommonImage, "sleep", "1h").AssertOK() - defer base.Cmd("rm", "-f", testContainer).Run() - base.Cmd("run", "-d", "--name", testStoppedContainer, testutil.CommonImage, "sleep", "1h").AssertOK() - defer base.Cmd("rm", "-f", testStoppedContainer).Run() + base := testutil.NewBase(t) euid := os.Geteuid() srcUID := 42 + srcFileName := "test-file" + // FIXME: not portable srcDir := "/test-dir" - srcFile := filepath.Join(srcDir, "test-file") - srcFileContent := []byte("test-file-content") - mkSrcScript := fmt.Sprintf("mkdir -p %q && echo -n %q >%q && chown %d %q", srcDir, srcFileContent, srcFile, srcUID, srcFile) - base.Cmd("exec", testContainer, "sh", "-euc", mkSrcScript).AssertOK() - base.Cmd("exec", testStoppedContainer, "sh", "-euc", mkSrcScript).AssertOK() - // Stop container for testing copying out of stopped container - base.Cmd("stop", testStoppedContainer) - - assertCat := func(catPath string) { + + assertCat := func(catPath string, fileContent []byte, container string, expectedUid int, containerIsStopped bool) { t.Logf("catPath=%q", catPath) - got, err := os.ReadFile(catPath) - assert.NilError(t, err) - assert.DeepEqual(t, srcFileContent, got) - st, err := os.Stat(catPath) - assert.NilError(t, err) - stSys := st.Sys().(*syscall.Stat_t) - // stSys.Uid matches euid, not srcUID - assert.DeepEqual(t, uint32(euid), stSys.Uid) + if containerIsStopped { + base.Cmd("start", container).AssertOK() + defer base.Cmd("stop", container).AssertOK() + } + + if container == "" { + got, err := os.ReadFile(catPath) + assert.NilError(t, err, "Failed reading from file") + assert.DeepEqual(t, fileContent, got) + st, err := os.Stat(catPath) + assert.NilError(t, err) + stSys := st.Sys().(*syscall.Stat_t) + assert.DeepEqual(t, uint32(expectedUid), stSys.Uid) + } else { + base.Cmd("exec", container, "cat", catPath).AssertOutExactly(string(fileContent)) + base.Cmd("exec", container, "stat", "-c", "%u", catPath).AssertOutExactly(fmt.Sprintf("%d\n", expectedUid)) + } + } + + srcFile := filepath.Join(srcDir, srcFileName) + + testGroups := []*testgroup{ + { + description: "SRC_PATH specifies a file", + srcPath: srcFile, + testCases: []testcases{ + { + description: "DEST_PATH does not exist", + destPath: "dest-does-not-exist-no-slash", + expect: icmd.Expected{ + ExitCode: 0, + }, + }, + { + description: "DEST_PATH does not exist and ends with a path separator", + destPath: "dest-does-not-exist-with-slash" + string(os.PathSeparator), + expect: icmd.Expected{ + ExitCode: 1, + Err: containerutil.ErrDestinationDirMustExist.Error(), + }, + }, + { + description: "DEST_PATH exists and is a file", + destPath: "dest-file-exists", + expect: icmd.Expected{ + ExitCode: 0, + }, + setup: func(t *testing.T, destPath string) { + err := os.WriteFile(destPath, []byte(""), 0o644) + assert.NilError(t, err) + }, + }, + { + description: "DEST_PATH exists and is a directory", + destPath: "dest-dir-exists", + catFile: filepath.Join("dest-dir-exists", srcFileName), + expect: icmd.Expected{ + ExitCode: 0, + }, + setup: func(t *testing.T, destPath string) { + err := os.Mkdir(destPath, 0o755) + assert.NilError(t, err) + }, + }, + { + description: "DEST_PATH exists and is a directory, SRC_PATH is in a volume", + destPath: "dest-dir-exists", + catFile: filepath.Join("dest-dir-exists", srcFileName), + expect: icmd.Expected{ + ExitCode: 0, + }, + volume: func(id string) string { + base.Cmd("volume", "create", id).AssertOK() + return id + + }, + setup: func(t *testing.T, destPath string) { + err := os.Mkdir(destPath, 0o700) + assert.NilError(t, err) + }, + }, + }, + }, + { + description: "SRC_PATH specifies a dir", + srcPath: srcDir, + testCases: []testcases{ + { + description: "DEST_PATH does not exist", + destPath: "dest-does-not-exist-no-slash", + catFile: filepath.Join("dest-does-not-exist-no-slash", srcFileName), + expect: icmd.Expected{ + ExitCode: 0, + }, + }, + { + description: "DEST_PATH exists and is a file", + destPath: "dest-file-exists", + expect: icmd.Expected{ + ExitCode: 1, + Err: containerutil.ErrCannotCopyDirToFile.Error(), + }, + setup: func(t *testing.T, destPath string) { + err := os.WriteFile(destPath, []byte(""), 0o644) + assert.NilError(t, err) + }, + }, + { + description: "DEST_PATH exists, is a directory, SRC_PATH does not end with separator", + destPath: "dest-dir-exist", + catFile: filepath.Join("dest-dir-exist", filepath.Base(srcDir), srcFileName), + expect: icmd.Expected{ + ExitCode: 0, + }, + setup: func(t *testing.T, destPath string) { + err := os.Mkdir(destPath, 0o755) + assert.NilError(t, err) + }, + }, + }, + }, + { + description: "SRC_PATH specifies a dir, with a trailing slash/dot", + srcPath: srcDir + string(os.PathSeparator) + ".", + testCases: []testcases{ + { + description: "DEST_PATH exists, is a directory", + destPath: "dest-dir-exist", + catFile: filepath.Join("dest-dir-exist", srcFileName), + expect: icmd.Expected{ + ExitCode: 0, + }, + setup: func(t *testing.T, destPath string) { + err := os.Mkdir(destPath, 0o755) + assert.NilError(t, err) + }, + }, + }, + }, } - td := t.TempDir() // For the test matrix, see https://docs.docker.com/engine/reference/commandline/cp/ - t.Run("SRC_PATH specifies a file", func(t *testing.T) { - srcPath := srcFile - t.Run("DEST_PATH does not exist", func(t *testing.T) { - destPath := filepath.Join(td, "dest-no-exist-no-slash") - base.Cmd("cp", testContainer+":"+srcPath, destPath).AssertOK() - catPath := destPath - assertCat(catPath) - if rootlessutil.IsRootless() { - t.Skip("Test skipped in rootless mode for testStoppedContainer") - } - base.Cmd("cp", testStoppedContainer+":"+srcPath, destPath).AssertOK() - assertCat(catPath) - }) - t.Run("DEST_PATH does not exist and ends with /", func(t *testing.T) { - destPath := td + "/dest-no-exist-with-slash/" // Avoid filepath.Join, to forcibly append "/" - base.Cmd("cp", testContainer+":"+srcPath, destPath).AssertFail() - if rootlessutil.IsRootless() { - t.Skip("Test skipped in rootless mode for testStoppedContainer") - } - base.Cmd("cp", testStoppedContainer+":"+srcPath, destPath).AssertFail() - }) - t.Run("DEST_PATH exists and is a file", func(t *testing.T) { - destPath := filepath.Join(td, "dest-file-exists") - err := os.WriteFile(destPath, []byte(""), 0o644) - assert.NilError(t, err) - base.Cmd("cp", testContainer+":"+srcPath, destPath).AssertOK() - catPath := destPath - assertCat(catPath) - if rootlessutil.IsRootless() { - t.Skip("Test skipped in rootless mode for testStoppedContainer") - } - base.Cmd("cp", testStoppedContainer+":"+srcPath, destPath).AssertOK() - assertCat(catPath) - }) - t.Run("DEST_PATH exists and is a directory", func(t *testing.T) { - destPath := filepath.Join(td, "dest-dir-exists") - err := os.Mkdir(destPath, 0o755) - assert.NilError(t, err) - base.Cmd("cp", testContainer+":"+srcPath, destPath).AssertOK() - catPath := filepath.Join(destPath, filepath.Base(srcFile)) - assertCat(catPath) - if rootlessutil.IsRootless() { - t.Skip("Test skipped in rootless mode for testStoppedContainer") - } - base.Cmd("cp", testStoppedContainer+":"+srcPath, destPath).AssertOK() - assertCat(catPath) - }) - t.Run("SRC_PATH is in a volume", func(t *testing.T) { - // Setup - // Create a volume - vol := "somevol" - base.Cmd("volume", "create", vol).AssertOK() - defer base.Cmd("volume", "rm", "-f", vol).Run() - - // Create container for test - con := fmt.Sprintf("%s-with-volume", testContainer) - - mountDir := "/some_dir" - base.Cmd("run", "-d", "--name", con, "-v", fmt.Sprintf("%s:%s", vol, mountDir), testutil.CommonImage, "sleep", "1h").AssertOK() - defer base.Cmd("rm", "-f", con).Run() - - // Create a file to mounted volume - mountedVolFile := filepath.Join(mountDir, "test-file") - mkSrcScript = fmt.Sprintf("echo -n %q >%q && chown %d %q", srcFileContent, mountedVolFile, srcUID, mountedVolFile) - base.Cmd("exec", con, "sh", "-euc", mkSrcScript).AssertOK() - - // Create destination directory on host for copy - destPath := filepath.Join(td, "dest-dir") - err := os.Mkdir(destPath, 0o700) - assert.NilError(t, err) + for _, tg := range testGroups { + testGroup := tg + t.Run(testGroup.description, func(t *testing.T) { + t.Parallel() - catPath := filepath.Join(destPath, filepath.Base(mountedVolFile)) + srcPath := tg.srcPath - // Running container test - base.Cmd("cp", con+":"+mountedVolFile, destPath).AssertOK() - assertCat(catPath) + for _, tc := range tg.testCases { + testCase := tc - // Skip for rootless - if rootlessutil.IsRootless() { - t.Skip("Test skipped in rootless mode for testStoppedContainer") - } - // Stopped container test - base.Cmd("stop", con).AssertOK() - base.Cmd("cp", con+":"+mountedVolFile, destPath).AssertOK() - assertCat(catPath) - }) - }) - t.Run("SRC_PATH specifies a directory", func(t *testing.T) { - srcPath := srcDir - t.Run("DEST_PATH does not exist", func(t *testing.T) { - destPath := filepath.Join(td, "dest2-no-exist") - base.Cmd("cp", testContainer+":"+srcPath, destPath).AssertOK() - catPath := filepath.Join(destPath, filepath.Base(srcFile)) - assertCat(catPath) - if rootlessutil.IsRootless() { - t.Skip("Test skipped in rootless mode for testStoppedContainer") - } - base.Cmd("cp", testStoppedContainer+":"+srcPath, destPath).AssertOK() - assertCat(catPath) - }) - t.Run("DEST_PATH exists and is a file", func(t *testing.T) { - destPath := filepath.Join(td, "dest2-file-exists") - err := os.WriteFile(destPath, []byte(""), 0o644) - assert.NilError(t, err) - base.Cmd("cp", srcPath, testContainer+":"+destPath).AssertFail() - if rootlessutil.IsRootless() { - t.Skip("Test skipped in rootless mode for testStoppedContainer") + t.Run(testCase.description, func(t *testing.T) { + t.Parallel() + + testID := testutil.Identifier(t) + containerRunning := testID + "-r" + containerStopped := testID + "-s" + srcFileContent := []byte(testID) + tempDir := t.TempDir() + + // Do not use filepath.Join, as some path voluntarily end with PathSeparator that we want to preserve + destPath := tempDir + string(os.PathSeparator) + testCase.destPath + // If the test case does not specify a catFile, just use destPath + catFile := testCase.catFile + if catFile == "" { + catFile = testCase.destPath + } + catFile = filepath.Join(tempDir, catFile) + + // Teardown: clean-up containers and optional volume + tearDown := func() { + base.Cmd("rm", "-f", containerRunning).Run() + base.Cmd("rm", "-f", containerStopped).Run() + if testCase.volume != nil { + base.Cmd("volume", "rm", testCase.volume(testID)).Run() + } + } + + // Setup: create volume, containers, write the src file in containers + setup := func() { + if testCase.volume != nil { + vol := testCase.volume(testID) + base.Cmd("run", "-d", "-v", fmt.Sprintf("%s:%s", vol, srcDir), "--name", containerRunning, testutil.CommonImage, "sleep", "1h").AssertOK() + base.Cmd("run", "-d", "-v", fmt.Sprintf("%s:%s", vol, srcDir), "--name", containerStopped, testutil.CommonImage, "sleep", "1h").AssertOK() + } else { + base.Cmd("run", "-d", "--name", containerRunning, testutil.CommonImage, "sleep", "1h").AssertOK() + base.Cmd("run", "-d", "--name", containerStopped, testutil.CommonImage, "sleep", "1h").AssertOK() + } + mkSrcScript := fmt.Sprintf("mkdir -p %q && echo -n %q >%q && chown %d %q", srcDir, srcFileContent, srcFile, srcUID, srcFile) + base.Cmd("exec", containerRunning, "sh", "-euc", mkSrcScript).AssertOK() + base.Cmd("exec", containerStopped, "sh", "-euc", mkSrcScript).AssertOK() + base.Cmd("stop", containerStopped).AssertOK() + } + + tearDown() + t.Cleanup(tearDown) + setup() + + // If we have optional setup and teardown, run that + if testCase.setup != nil { + testCase.setup(base, destPath) + } + + if testCase.tearDown != nil { + t.Cleanup(testCase.tearDown) + } + + t.Logf("Copying %s:%s to %s", containerRunning, srcPath, destPath) + + // Run the actual test for running container + base.Cmd("cp", containerRunning+":"+srcPath, destPath).Assert(testCase.expect) + // If we expect the op to be a success, compare the file content + if testCase.expect.ExitCode == 0 { + assertCat(catFile, srcFileContent, "", euid, false) + } + + // ... and for the stopped container + cmd := base.Cmd("cp", containerStopped+":"+srcPath, destPath) + + if rootlessutil.IsRootless() { + cmd.Assert( + icmd.Expected{ + ExitCode: 1, + Err: containerutil.ErrRootlessCannotCp.Error(), + }) + return + } + + cmd.Assert(testCase.expect) + if testCase.expect.ExitCode == 0 { + assertCat(catFile, srcFileContent, "", euid, false) + } + }) } - base.Cmd("cp", srcPath, testStoppedContainer+":"+destPath).AssertFail() - }) - t.Run("DEST_PATH exists and is a directory", func(t *testing.T) { - t.Run("SRC_PATH does not end with `/.`", func(t *testing.T) { - destPath := filepath.Join(td, "dest2-dir-exists") - err := os.Mkdir(destPath, 0o755) - assert.NilError(t, err) - base.Cmd("cp", testContainer+":"+srcPath, destPath).AssertOK() - catPath := filepath.Join(destPath, strings.TrimPrefix(srcFile, filepath.Dir(srcDir)+"/")) - assertCat(catPath) - if rootlessutil.IsRootless() { - t.Skip("Test skipped in rootless mode for testStoppedContainer") - } - base.Cmd("cp", testStoppedContainer+":"+srcPath, destPath).AssertOK() - assertCat(catPath) - }) - t.Run("SRC_PATH does end with `/.`", func(t *testing.T) { - srcPath += "/." - destPath := filepath.Join(td, "dest2-dir2-exists") - err := os.Mkdir(destPath, 0o755) - assert.NilError(t, err) - base.Cmd("cp", testContainer+":"+srcPath, destPath).AssertOK() - catPath := filepath.Join(destPath, filepath.Base(srcFile)) - assertCat(catPath) - if rootlessutil.IsRootless() { - t.Skip("Test skipped in rootless mode for testStoppedContainer") - } - base.Cmd("cp", testStoppedContainer+":"+srcPath, destPath).AssertOK() - assertCat(catPath) - }) }) - }) + } } + + +*/ diff --git a/pkg/cmd/container/cp_linux.go b/pkg/cmd/container/cp_linux.go index 5d883a47565..5a3a26ac876 100644 --- a/pkg/cmd/container/cp_linux.go +++ b/pkg/cmd/container/cp_linux.go @@ -39,11 +39,7 @@ func Cp(ctx context.Context, client *containerd.Client, options types.ContainerC ctx, client, found.Container, - options.Container2Host, - options.DestPath, - options.SrcPath, - options.GOptions.Snapshotter, - options.FollowSymLink) + options) }, } count, err := walker.Walk(ctx, options.ContainerReq) diff --git a/pkg/containerutil/cp_linux.go b/pkg/containerutil/cp_linux.go index 2036b677630..78d71ced859 100644 --- a/pkg/containerutil/cp_linux.go +++ b/pkg/containerutil/cp_linux.go @@ -20,147 +20,247 @@ import ( "context" "errors" "fmt" - "io/fs" "os" "os/exec" - "path" "path/filepath" "strconv" "strings" - securejoin "github.com/cyphar/filepath-securejoin" - containerd "github.com/containerd/containerd/v2/client" + "github.com/containerd/containerd/v2/core/containers" "github.com/containerd/containerd/v2/core/mount" "github.com/containerd/errdefs" "github.com/containerd/log" + "github.com/containerd/nerdctl/v2/pkg/api/types" "github.com/containerd/nerdctl/v2/pkg/rootlessutil" "github.com/containerd/nerdctl/v2/pkg/tarutil" ) -// CopyFiles implements `nerdctl cp`. // See https://docs.docker.com/engine/reference/commandline/cp/ for the specification. -func CopyFiles(ctx context.Context, client *containerd.Client, container containerd.Container, container2host bool, dst, src string, snapshotter string, followSymlink bool) error { + +var ( + // Generic errors + ErrShouldNeverHappen = errors.New("an unexpected error happened - this should never happen - please report this as a bug") + ErrRootlessCannotCp = errors.New("cannot use cp with stopped containers in rootless mode") + ErrFailedMountingSnapshot = errors.New("failed mounting snapshot") + ErrFilesystem = errors.New("filesystem error") + ErrTargetIsReadOnly = errors.New("cannot copy into read-only location") + + // These are specific cp conditions as outlined in docker reference + ErrDestinationDirMustExist = errors.New("the destination directory in the container must exist") + ErrSourceIsNotADir = errors.New("source is not a directory") + ErrCannotCopyDirToFile = errors.New("cannot copy a directory to a file") +) + +// getRoot will tentatively return the root of the container on the host (/proc/pid/root), along with the pid, +// if it exists (eg: container is running) +func getRoot(ctx context.Context, container containerd.Container) (string, int, error) { + task, err := container.Task(ctx, nil) + if err != nil { + return "", 0, err + } + status, err := task.Status(ctx) + if err != nil { + return "", 0, err + } + if status.Status != containerd.Running { + return "", 0, nil + } + pid := int(task.Pid()) + return fmt.Sprintf("/proc/%d/root", pid), pid, nil +} + +// CopyFiles implements `nerdctl cp` +// It currently depends on the following assumptions: +// - linux only +// - tar binary exists on the system +// - nsenter binary exists on the system +// - if rootless, the container is running (aka: /proc/pid/root) +func CopyFiles(ctx context.Context, client *containerd.Client, container containerd.Container, options types.ContainerCpOptions) (err error) { + // We do rely on the tar binary as a shortcut - could also be replaced by archive/tar, though that would mean + // we need to replace nsenter calls with re-exec tarBinary, isGNUTar, err := tarutil.FindTarBinary() if err != nil { return err } + log.G(ctx).Debugf("Detected tar binary %q (GNU=%v)", tarBinary, isGNUTar) - var srcFull, dstFull, root, mountDestination, containerPath string - var cleanup func() - task, err := container.Task(ctx, nil) + + // Actually, this can happen if the container being passed has been deleted since in a racy way + conSpec, err := container.Spec(ctx) if err != nil { - // FIXME: Rootless does not support copying into/out of stopped/created containers as we need to nsenter into the user namespace of the - // pid of the running container with --preserve-credentials to preserve uid/gid mapping and copy files into the container. + return errors.Join(ErrShouldNeverHappen, err) + } + + // Try to get a running container root + root, pid, err := getRoot(ctx, container) + // If the task is "not found" (for example, if the container stopped), we will try to mount the snapshot + // Any other type of error from Task() is fatal here. + if err != nil && !errdefs.IsNotFound(err) { + return errors.Join(ErrShouldNeverHappen, err) + } + + log.G(ctx).Debugf("We have root %s and pid %d", root, pid) + + // If we have no root: + // - bail out for rootless + // - mount the snapshot for rootful + if root == "" { + // FIXME: Rootless does not support copying into/out of stopped/created containers as we need to nsenter into + // the user namespace of the pid of the running container with --preserve-credentials to preserve uid/gid + // mapping and copy files into the container. if rootlessutil.IsRootless() { - return errors.New("cannot use cp with stopped containers in rootless mode") + return ErrRootlessCannotCp } - // if the task is simply not found, we should try to mount the snapshot. any other type of error from Task() is fatal here. - if !errdefs.IsNotFound(err) { - return err + + // See similar situation above. This may happen if we are racing against container deletion + conInfo, err := container.Info(ctx) + if err != nil { + return errors.Join(ErrShouldNeverHappen, err) } - if container2host { - containerPath = src - } else { - containerPath = dst + + var cleanup func() error + root, cleanup, err = mountSnapshotForContainer(ctx, client, conInfo, options.GOptions.Snapshotter) + if cleanup != nil { + defer func() { + err = errors.Join(err, cleanup()) + }() } - // Check if containerPath is in a volume - root, mountDestination, err = getContainerMountInfo(ctx, container, containerPath, container2host) + if err != nil { - return err - } - // if containerPath is in a volume and not read-only in case of host2container copy then handle volume paths, - // else containerPath is not in volume so mount container snapshot for copy - if root != "" { - dst, src = handleVolumePaths(container2host, dst, src, mountDestination) - } else { - root, cleanup, err = mountSnapshotForContainer(ctx, client, container, snapshotter) - if cleanup != nil { - defer cleanup() - } - if err != nil { - return err - } + return errors.Join(ErrFailedMountingSnapshot, err) } + + log.G(ctx).Debugf("Got new root %s", root) + } + + // At this point, we have a host root: either /proc/pid/root for a running container, or the mounted snapshot + // Let's figure out where we are inside the container (root, or in a volume) + src := options.SrcPath + dst := options.DestPath + + var containerPath string + if options.Container2Host { + containerPath = src } else { - status, err := task.Status(ctx) - if err != nil { - return err + containerPath = dst + } + + pathResolver := newResolver(conSpec, root) + // Path may still be relative at this point. If it is, join it to the container cwd first + if !filepath.IsAbs(containerPath) { + containerPath = filepath.Join(conSpec.Process.Cwd, containerPath) + } + + // Now, fully resolve the path - resolving all symlinks and cleaning-up the end result, + // following across mounts + resolvedContainerPath, err := pathResolver.resolvePath(containerPath) + // Errors we get from that are from Lstat or Readlink + // Either the object does not exist, or we have a dangling symlink, or otherwise hosed filesystem entries + if err != nil { + log.G(ctx).Debugf("Got an error from the resolver %s (is 'not exist': %t)", err.Error(), errors.Is(err, os.ErrNotExist)) + // Any error (but does not exist), stop here. + // Also, if that was the source, then if it does not exist, stop as well. + if !errors.Is(err, os.ErrNotExist) || options.Container2Host { + return errors.Join(ErrFilesystem, err) } - if status.Status == containerd.Running { - root = fmt.Sprintf("/proc/%d/root", task.Pid()) - } else { - if rootlessutil.IsRootless() { - return fmt.Errorf("cannot use cp with stopped containers in rootless mode") - } - if container2host { - containerPath = src - } else { - containerPath = dst - } - root, mountDestination, err = getContainerMountInfo(ctx, container, containerPath, container2host) - if err != nil { - return err + + // If we have a trailing slash, and if the source (which is on the host) is a file, we must stop here as well + if containerPath[len(containerPath)-1:] == string(os.PathSeparator) { + stsrc, statErr := os.Stat(strings.TrimSuffix(src, string(os.PathSeparator))) + if statErr != nil { + return errors.Join(ErrFilesystem, statErr) } - // if containerPath is in a volume and not read-only in case of host2container copy then handle volume paths, - // else containerPath is not in volume so mount container snapshot for copy - if root != "" { - dst, src = handleVolumePaths(container2host, dst, src, mountDestination) - } else { - root, cleanup, err = mountSnapshotForContainer(ctx, client, container, snapshotter) - if cleanup != nil { - defer cleanup() - } - if err != nil { - return err - } + // If the source is not a dir, error + if !stsrc.IsDir() { + return errors.Join(ErrDestinationDirMustExist, err) } + // Trailing slash and is a dir? Retry with the "actual" parent dir + resolvedContainerPath, err = pathResolver.resolvePath(filepath.Dir(filepath.Dir(containerPath))) + } else { + // No trailing slash. Just retry with the parent dir + resolvedContainerPath, err = pathResolver.resolvePath(filepath.Dir(containerPath)) + } + + // Still erroring? Parent does not exist. Bail out. + if err != nil { + return errors.Join(ErrDestinationDirMustExist, err) } + // Otherwise, just re-add the final component + resolvedContainerPath = filepath.Join(resolvedContainerPath, filepath.Base(containerPath)) } - if container2host { - srcFull, err = securejoin.SecureJoin(root, src) - dstFull = dst + + log.G(ctx).Debugf("Resolved container path: %s", resolvedContainerPath) + + // Now, finally get the location of the fully resolved containerPath (in the root? in a volume?) + containerMount, relativePath := pathResolver.getMount(resolvedContainerPath) + + log.G(ctx).Debugf("Resolved into mount %s with rel %s", containerMount.hostPath, relativePath) + // ... and build the final paths for the container part + // Note that securejoin is unnecessary. Paths have been fully cleaned already. + if options.Container2Host { + src = filepath.Join(containerMount.hostPath, relativePath) } else { - srcFull = src - dstFull, err = securejoin.SecureJoin(root, dst) - } - if err != nil { - return err + // If we are copying into the container, and the locator is marked read-only, time to bail out + if containerMount.readonly { + return ErrTargetIsReadOnly + } + dst = filepath.Join(containerMount.hostPath, relativePath) } + + log.G(ctx).Debugf("Updated src: %s - and dest: %s", src, dst) + + // Now, resolve cp shenanigans var ( - srcIsDir bool - dstExists bool - dstExistsAsDir bool - st fs.FileInfo + srcIsDir bool + dstExists bool + dstExistsAsDir bool + dstEndsWithSep bool + srcEndsWithSlashDot bool + st os.FileInfo ) - st, err = os.Stat(srcFull) + + // Stat src to verify it exists and we can read it + log.L.Error("S+RC", src) + st, err = os.Stat(src) if err != nil { - return err + return errors.Join(ErrFilesystem, err) } + srcIsDir = st.IsDir() // dst may not exist yet, so err is negligible - if st, err := os.Stat(dstFull); err == nil { + if st, err = os.Stat(dst); err == nil { dstExists = true dstExistsAsDir = st.IsDir() } - dstEndsWithSep := strings.HasSuffix(dst, string(os.PathSeparator)) - srcEndsWithSlashDot := strings.HasSuffix(src, string(os.PathSeparator)+".") + + // We use the original values here, as src and dst have been cleaned-up + dstEndsWithSep = strings.HasSuffix(options.DestPath, string(os.PathSeparator)) + srcEndsWithSlashDot = strings.HasSuffix(options.SrcPath, string(os.PathSeparator)+".") + + log.L.Error("srcIsDir", srcIsDir) + log.L.Error("dstEndsWithSep", dstEndsWithSep) + log.L.Error("dstExistsAsDir", dstExistsAsDir) if !srcIsDir && dstEndsWithSep && !dstExistsAsDir { // The error is specified in https://docs.docker.com/engine/reference/commandline/cp/ // See the `DEST_PATH does not exist and ends with /` case. - return fmt.Errorf("the destination directory must exists: %w", err) + return errors.Join(ErrDestinationDirMustExist, err) } + if !srcIsDir && srcEndsWithSlashDot { - return fmt.Errorf("the source is not a directory") + return ErrSourceIsNotADir } + if srcIsDir && dstExists && !dstExistsAsDir { - return fmt.Errorf("cannot copy a directory to a file") + return ErrCannotCopyDirToFile } + if srcIsDir && !dstExists { - if err := os.MkdirAll(dstFull, 0o755); err != nil { - return err + // XXX FIXME: this seems wrong. What about ownership? We may be doing that inside a container + if err = os.MkdirAll(dst, 0o755); err != nil { + return errors.Join(ErrFilesystem, err) } } @@ -168,12 +268,12 @@ func CopyFiles(ctx context.Context, client *containerd.Client, container contain if srcIsDir { if !dstExists || srcEndsWithSlashDot { // the content of the source directory is copied into this directory - tarCDir = srcFull + tarCDir = src tarCArg = "." } else { // the source directory is copied into this directory - tarCDir = filepath.Dir(srcFull) - tarCArg = filepath.Base(srcFull) + tarCDir = filepath.Dir(src) + tarCArg = filepath.Base(src) } } else { // Prepare a single-file directory to create an archive of the source file @@ -184,16 +284,16 @@ func CopyFiles(ctx context.Context, client *containerd.Client, container contain defer os.RemoveAll(td) tarCDir = td cp := []string{"cp", "-a"} - if followSymlink { + if options.FollowSymLink { cp = append(cp, "-L") } if dstEndsWithSep || dstExistsAsDir { - tarCArg = filepath.Base(srcFull) + tarCArg = filepath.Base(src) } else { // Handle `nerdctl cp /path/to/file some-container:/path/to/file-with-another-name` - tarCArg = filepath.Base(dstFull) + tarCArg = filepath.Base(dst) } - cp = append(cp, srcFull, filepath.Join(td, tarCArg)) + cp = append(cp, src, filepath.Join(td, tarCArg)) cpCmd := exec.CommandContext(ctx, cp[0], cp[1:]...) log.G(ctx).Debugf("executing %v", cpCmd.Args) if out, err := cpCmd.CombinedOutput(); err != nil { @@ -201,24 +301,24 @@ func CopyFiles(ctx context.Context, client *containerd.Client, container contain } } tarC := []string{tarBinary} - if followSymlink { + if options.FollowSymLink { tarC = append(tarC, "-h") } tarC = append(tarC, "-c", "-f", "-", tarCArg) - tarXDir := dstFull + tarXDir := dst if !srcIsDir && !dstEndsWithSep && !dstExistsAsDir { - tarXDir = filepath.Dir(dstFull) + tarXDir = filepath.Dir(dst) } tarX := []string{tarBinary, "-x"} - if container2host && isGNUTar { + if options.Container2Host && isGNUTar { tarX = append(tarX, "--no-same-owner") } tarX = append(tarX, "-f", "-") if rootlessutil.IsRootless() { - nsenter := []string{"nsenter", "-t", strconv.Itoa(int(task.Pid())), "-U", "--preserve-credentials", "--"} - if container2host { + nsenter := []string{"nsenter", "-t", strconv.Itoa(pid), "-U", "--preserve-credentials", "--"} + if options.Container2Host { tarC = append(nsenter, tarC...) } else { tarX = append(nsenter, tarX...) @@ -256,78 +356,30 @@ func CopyFiles(ctx context.Context, client *containerd.Client, container contain return nil } -func mountSnapshotForContainer(ctx context.Context, client *containerd.Client, container containerd.Container, snapshotter string) (string, func(), error) { - cinfo, err := container.Info(ctx) - if err != nil { - return "", nil, err - } - snapKey := cinfo.SnapshotKey +func mountSnapshotForContainer(ctx context.Context, client *containerd.Client, conInfo containers.Container, snapshotter string) (string, func() error, error) { + snapKey := conInfo.SnapshotKey resp, err := client.SnapshotService(snapshotter).Mounts(ctx, snapKey) if err != nil { return "", nil, err } + tempDir, err := os.MkdirTemp("", "nerdctl-cp-") if err != nil { return "", nil, err } + err = mount.All(resp, tempDir) if err != nil { - return "", nil, fmt.Errorf("failed to mount snapshot with error %s", err.Error()) + return "", nil, errors.Join(errors.New("failed to mount snapshot"), err) } - cleanup := func() { + + cleanup := func() error { err = mount.Unmount(tempDir, 0) if err != nil { - log.G(ctx).Warnf("failed to unmount %s with error %s", tempDir, err.Error()) - return - } - os.RemoveAll(tempDir) - } - return tempDir, cleanup, nil -} - -func getContainerMountInfo(ctx context.Context, con containerd.Container, containerPath string, container2host bool) (string, string, error) { - filePath := filepath.Clean(containerPath) - spec, err := con.Spec(ctx) - if err != nil { - return "", "", err - } - // read-only applies only while copying into container from host - if !container2host && spec.Root.Readonly { - return "", "", fmt.Errorf("container rootfs: %s is marked read-only", spec.Root.Path) - } - - for _, mount := range spec.Mounts { - if isSelfOrAscendant(filePath, mount.Destination) { - // read-only applies only while copying into container from host - if !container2host { - for _, option := range mount.Options { - if option == "ro" { - return "", "", fmt.Errorf("mount point %s is marked read-only", filePath) - } - } - } - return mount.Source, mount.Destination, nil + return err } + return os.RemoveAll(tempDir) } - return "", "", nil -} -func isSelfOrAscendant(filePath, potentialAncestor string) bool { - if filePath == "/" || filePath == "" || potentialAncestor == "" { - return false - } - filePath = filepath.Clean(filePath) - potentialAncestor = filepath.Clean(potentialAncestor) - if filePath == potentialAncestor { - return true - } - return isSelfOrAscendant(path.Dir(filePath), potentialAncestor) -} - -// When the path is in volume remove directory that volume is mounted on from the path -func handleVolumePaths(container2host bool, dst string, src string, mountDestination string) (string, string) { - if container2host { - return dst, strings.TrimPrefix(filepath.Clean(src), mountDestination) - } - return strings.TrimPrefix(filepath.Clean(dst), mountDestination), src + return tempDir, cleanup, nil } diff --git a/pkg/containerutil/resolve.go b/pkg/containerutil/resolve.go new file mode 100644 index 00000000000..4436c510bb7 --- /dev/null +++ b/pkg/containerutil/resolve.go @@ -0,0 +1,243 @@ +package containerutil + +import ( + "errors" + "io/fs" + "os" + "path/filepath" + "runtime" + "slices" + "strings" + "syscall" + + "github.com/opencontainers/runtime-spec/specs-go" + + "github.com/containerd/containerd/v2/pkg/oci" +) + +// volumeNameLen returns length of the leading volume name on Windows. +// It returns 0 elsewhere. +// FIXME: whenever we will want to port cp to windows, we will need the windows implementation of volumeNameLen +func volumeNameLen(_ string) int { + return 0 +} + +func newResolver(conSpec *oci.Spec, hostRoot string) *resolver { + return &resolver{ + root: conSpec.Root, + mounts: conSpec.Mounts, + hostRoot: hostRoot, + } +} + +func isParent(child []string, candidate []string) (bool, []string) { + if len(child) < len(candidate) { + return false, child + } + return slices.Equal(child[0:len(candidate)], candidate), child[len(candidate):] +} + +// resolver provides method to fully resolve any given container given path to a host location +// accounting for rootfs and mounts location +type resolver struct { + root *specs.Root + mounts []specs.Mount + hostRoot string +} + +type locator struct { + containerPath string + hostPath string + readonly bool +} + +// pathOnHost will return the *host* location of a container path, accounting for volumes. +// The provided path must be absolute, as built by `resolvePath`. +func (res *resolver) pathOnHost(path string) string { + hostRoot := res.hostRoot + path = filepath.Clean(path) + itemized := strings.Split(path, string(os.PathSeparator)) + + containerRoot := "/" + sub := itemized + + for _, mnt := range res.mounts { + if candidateIsParent, subPath := isParent(itemized, strings.Split(mnt.Destination, string(os.PathSeparator))); candidateIsParent { + if len(mnt.Destination) > len(containerRoot) { + containerRoot = mnt.Destination + hostRoot = mnt.Source + sub = subPath + } + } + } + + return filepath.Join(append([]string{hostRoot}, sub...)...) +} + +func (res *resolver) getMount(path string) (*locator, string) { + itemized := strings.Split(path, string(os.PathSeparator)) + + loc := &locator{ + containerPath: "/", + hostPath: res.hostRoot, + readonly: res.root.Readonly, + } + + sub := itemized + + for _, mnt := range res.mounts { + if candidateIsParent, subPath := isParent(itemized, strings.Split(mnt.Destination, string(os.PathSeparator))); candidateIsParent { + if len(mnt.Destination) > len(loc.containerPath) { + loc.readonly = false + for _, option := range mnt.Options { + if option == "ro" { + loc.readonly = true + } + } + loc.containerPath = mnt.Destination + loc.hostPath = mnt.Source + sub = subPath + } + } + } + + return loc, filepath.Join(sub...) +} + +// resolvePath is adapted from https://cs.opensource.google/go/go/+/go1.23.0:src/path/filepath/path.go;l=147 +// The (only) changes are on Lstat and ReadLink, which are fed the actual host path, that is computed by `res.pathOnHost` +func (res *resolver) resolvePath(path string) (string, error) { + volLen := volumeNameLen(path) + pathSeparator := string(os.PathSeparator) + + if volLen < len(path) && os.IsPathSeparator(path[volLen]) { + volLen++ + } + vol := path[:volLen] + dest := vol + linksWalked := 0 + //nolint:ineffassign + for start, end := volLen, volLen; start < len(path); start = end { + for start < len(path) && os.IsPathSeparator(path[start]) { + start++ + } + end = start + for end < len(path) && !os.IsPathSeparator(path[end]) { + end++ + } + + // On Windows, "." can be a symlink. + // We look it up, and use the value if it is absolute. + // If not, we just return ".". + isWindowsDot := runtime.GOOS == "windows" && path[volumeNameLen(path):] == "." + + // The next path component is in path[start:end]. + if end == start { + // No more path components. + break + } else if path[start:end] == "." && !isWindowsDot { + // Ignore path component ".". + continue + } else if path[start:end] == ".." { + // Back up to previous component if possible. + // Note that volLen includes any leading slash. + + // Set r to the index of the last slash in dest, + // after the volume. + var r int + for r = len(dest) - 1; r >= volLen; r-- { + if os.IsPathSeparator(dest[r]) { + break + } + } + if r < volLen || dest[r+1:] == ".." { + // Either path has no slashes + // (it's empty or just "C:") + // or it ends in a ".." we had to keep. + // Either way, keep this "..". + if len(dest) > volLen { + dest += pathSeparator + } + dest += ".." + } else { + // Discard everything since the last slash. + dest = dest[:r] + } + continue + } + + // Ordinary path component. Add it to result. + + if len(dest) > volumeNameLen(dest) && !os.IsPathSeparator(dest[len(dest)-1]) { + dest += pathSeparator + } + + dest += path[start:end] + + // Resolve symlink. + hostPath := res.pathOnHost(dest) + fi, err := os.Lstat(hostPath) + if err != nil { + return "", err + } + + if fi.Mode()&fs.ModeSymlink == 0 { + if !fi.Mode().IsDir() && end < len(path) { + return "", syscall.ENOTDIR + } + continue + } + + // Found symlink. + linksWalked++ + if linksWalked > 255 { + return "", errors.New("EvalSymlinks: too many links") + } + + link, err := os.Readlink(hostPath) + if err != nil { + return "", err + } + + if isWindowsDot && !filepath.IsAbs(link) { + // On Windows, if "." is a relative symlink, + // just return ".". + break + } + + path = link + path[end:] + + v := volumeNameLen(link) + if v > 0 { + // Symlink to drive name is an absolute path. + if v < len(link) && os.IsPathSeparator(link[v]) { + v++ + } + vol = link[:v] + dest = vol + end = len(vol) + } else if len(link) > 0 && os.IsPathSeparator(link[0]) { + // Symlink to absolute path. + dest = link[:1] + end = 1 + vol = link[:1] + volLen = 1 + } else { + // Symlink to relative path; replace last + // path component in dest. + var r int + for r = len(dest) - 1; r >= volLen; r-- { + if os.IsPathSeparator(dest[r]) { + break + } + } + if r < volLen { + dest = vol + } else { + dest = dest[:r] + } + end = 0 + } + } + return filepath.Clean(dest), nil +} diff --git a/pkg/testutil/testutil.go b/pkg/testutil/testutil.go index 4d93fc4c915..eeccf508b17 100644 --- a/pkg/testutil/testutil.go +++ b/pkg/testutil/testutil.go @@ -58,6 +58,7 @@ type Base struct { Binary string Args []string Env []string + Dir string } // WithStdin sets the standard input of Cmd to the specified reader @@ -70,6 +71,7 @@ func WithStdin(r io.Reader) func(*Cmd) { func (b *Base) Cmd(args ...string) *Cmd { icmdCmd := icmd.Command(b.Binary, append(b.Args, args...)...) icmdCmd.Env = b.Env + icmdCmd.Dir = b.Dir cmd := &Cmd{ Cmd: icmdCmd, Base: b,