Merge pull request #25701 from danegsta/danegsta/windowsCpPath

Fix windows path handling in `podman cp`
This commit is contained in:
openshift-merge-bot[bot] 2025-04-10 16:53:23 +00:00 committed by GitHub
commit 48423a615d
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
5 changed files with 272 additions and 120 deletions

View file

@ -5,6 +5,7 @@ import (
"io"
"os"
"os/user"
"path"
"path/filepath"
"strconv"
"strings"
@ -147,7 +148,7 @@ func copyContainerToContainer(sourceContainer string, sourcePath string, destCon
sourceContainerTarget := sourceContainerInfo.LinkTarget
destContainerTarget := destContainerInfo.LinkTarget
if !destContainerInfo.IsDir {
destContainerTarget = filepath.Dir(destPath)
destContainerTarget = path.Dir(destPath)
}
// If we copy a directory via the "." notation and the container path
@ -158,8 +159,8 @@ func copyContainerToContainer(sourceContainer string, sourcePath string, destCon
// Hence, whenever "." is the source and the destination does not
// exist, we copy the source's parent and let the copier package create
// the destination via the Rename option.
if destResolvedToParentDir && sourceContainerInfo.IsDir && filepath.Base(sourcePath) == "." {
sourceContainerTarget = filepath.Dir(sourceContainerTarget)
if destResolvedToParentDir && sourceContainerInfo.IsDir && path.Base(sourcePath) == "." {
sourceContainerTarget = path.Dir(sourceContainerTarget)
}
reader, writer := io.Pipe()
@ -183,7 +184,7 @@ func copyContainerToContainer(sourceContainer string, sourcePath string, destCon
if (!sourceContainerInfo.IsDir && !destContainerInfo.IsDir) || destResolvedToParentDir {
// If we're having a file-to-file copy, make sure to
// rename accordingly.
copyOptions.Rename = map[string]string{filepath.Base(sourceContainerTarget): destContainerBaseName}
copyOptions.Rename = map[string]string{path.Base(sourceContainerTarget): destContainerBaseName}
}
copyFunc, err := registry.ContainerEngine().ContainerCopyFromArchive(registry.Context(), destContainer, destContainerTarget, reader, copyOptions)
@ -261,8 +262,8 @@ func copyFromContainer(container string, containerPath string, hostPath string)
// we copy the source's parent and let the copier package create the
// destination via the Rename option.
containerTarget := containerInfo.LinkTarget
if resolvedToHostParentDir && containerInfo.IsDir && filepath.Base(containerTarget) == "." {
containerTarget = filepath.Dir(containerTarget)
if resolvedToHostParentDir && containerInfo.IsDir && path.Base(containerTarget) == "." {
containerTarget = path.Dir(containerTarget)
}
if !isStdout && containerInfo.IsDir && !hostInfo.IsDir {
@ -307,7 +308,7 @@ func copyFromContainer(container string, containerPath string, hostPath string)
if (!containerInfo.IsDir && !hostInfo.IsDir) || resolvedToHostParentDir {
// If we're having a file-to-file copy, make sure to
// rename accordingly.
putOptions.Rename = map[string]string{filepath.Base(containerTarget): hostBaseName}
putOptions.Rename = map[string]string{path.Base(containerTarget): hostBaseName}
}
dir := hostInfo.LinkTarget
if !hostInfo.IsDir {
@ -428,7 +429,11 @@ func copyToContainer(container string, containerPath string, hostPath string) er
// rename accordingly.
getOptions.Rename = map[string]string{filepath.Base(hostTarget): containerBaseName}
}
if err := buildahCopiah.Get("/", "", getOptions, []string{hostTarget}, writer); err != nil {
// On Windows, the root path needs to be <drive>:\, while otherwise
// it needs to be /. Combining filepath.VolumeName() + string(os.PathSeparator)
// gives us the correct path for the current OS.
if err := buildahCopiah.Get(filepath.VolumeName(hostTarget)+string(os.PathSeparator), "", getOptions, []string{hostTarget}, writer); err != nil {
return fmt.Errorf("copying from host: %w", err)
}
return nil
@ -438,7 +443,7 @@ func copyToContainer(container string, containerPath string, hostPath string) er
defer reader.Close()
target := containerInfo.FileInfo.LinkTarget
if !containerInfo.IsDir {
target = filepath.Dir(target)
target = path.Dir(target)
}
copyFunc, err := registry.ContainerEngine().ContainerCopyFromArchive(registry.Context(), container, target, reader, entities.CopyOptions{Chown: chown, NoOverwriteDirNonDir: !cpOpts.OverwriteDirNonDir})
@ -460,7 +465,7 @@ func copyToContainer(container string, containerPath string, hostPath string) er
func resolvePathOnDestinationContainer(container string, containerPath string, isStdin bool) (baseName string, containerInfo *entities.ContainerStatReport, resolvedToParentDir bool, err error) {
containerInfo, err = registry.ContainerEngine().ContainerStat(registry.Context(), container, containerPath)
if err == nil {
baseName = filepath.Base(containerInfo.LinkTarget)
baseName = path.Base(containerInfo.LinkTarget)
return //nolint: nilerr
}
@ -476,17 +481,17 @@ func resolvePathOnDestinationContainer(container string, containerPath string, i
// NOTE: containerInfo may actually be set. That happens when
// the container path is a symlink into nirvana. In that case,
// we must use the symlinked path instead.
path := containerPath
parentPath := containerPath
if containerInfo != nil {
baseName = filepath.Base(containerInfo.LinkTarget)
path = containerInfo.LinkTarget
baseName = path.Base(containerInfo.LinkTarget)
parentPath = containerInfo.LinkTarget
} else {
baseName = filepath.Base(containerPath)
baseName = path.Base(containerPath)
}
parentDir, err := containerParentDir(container, path)
parentDir, err := containerParentDir(container, parentPath)
if err != nil {
err = fmt.Errorf("could not determine parent dir of %q on container %s: %w", path, container, err)
err = fmt.Errorf("could not determine parent dir of %q on container %s: %w", parentPath, container, err)
return
}
@ -504,8 +509,11 @@ func resolvePathOnDestinationContainer(container string, containerPath string, i
// container. If the path is relative, it will be resolved relative to the
// container's working directory (or "/" if the work dir isn't set).
func containerParentDir(container string, containerPath string) (string, error) {
if filepath.IsAbs(containerPath) {
return filepath.Dir(containerPath), nil
// This is specifically a path in the (linux) container, so we need to intentionally use
// path instead of filepath to ensure we don't try to parse container paths using the
// host OS conventions.
if path.IsAbs(containerPath) {
return path.Dir(containerPath), nil
}
inspectData, _, err := registry.ContainerEngine().ContainerInspect(registry.Context(), []string{container}, entities.InspectOptions{})
if err != nil {
@ -514,9 +522,9 @@ func containerParentDir(container string, containerPath string) (string, error)
if len(inspectData) != 1 {
return "", fmt.Errorf("inspecting container %q: expected 1 data item but got %d", container, len(inspectData))
}
workDir := filepath.Join("/", inspectData[0].Config.WorkingDir)
workDir = filepath.Join(workDir, containerPath)
return filepath.Dir(workDir), nil
workDir := path.Join("/", inspectData[0].Config.WorkingDir)
workDir = path.Join(workDir, containerPath)
return path.Dir(workDir), nil
}
// validateFileInfo returns an error if the specified FileInfo doesn't point to

View file

@ -83,23 +83,28 @@ func ResolveHostPath(path string) (*FileInfo, error) {
// is preserved. The filepath API among tends to clean up a bit too much but
// we *must* preserve this data by all means.
func PreserveBasePath(original, resolved string) string {
// Handle "/"
if strings.HasSuffix(original, "/") {
if !strings.HasSuffix(resolved, "/") {
resolved += "/"
// Ensure paths are in platform semantics (replace / with \ on Windows)
resolved = filepath.FromSlash(resolved)
original = filepath.FromSlash(original)
if filepath.Base(resolved) != "." && filepath.Base(original) == "." {
if !hasTrailingPathSeparator(resolved) {
// Add a separator if it doesn't already end with one (a cleaned
// path would only end in a separator if it is the root).
resolved += string(filepath.Separator)
}
return resolved
resolved += "."
}
// Handle "/."
if strings.HasSuffix(original, "/.") {
if strings.HasSuffix(resolved, "/") { // could be root!
resolved += "."
} else if !strings.HasSuffix(resolved, "/.") {
resolved += "/."
}
return resolved
if !hasTrailingPathSeparator(resolved) && hasTrailingPathSeparator(original) {
resolved += string(filepath.Separator)
}
return resolved
}
// hasTrailingPathSeparator returns whether the given
// path ends with the system's path separator character.
func hasTrailingPathSeparator(path string) bool {
return len(path) > 0 && os.IsPathSeparator(path[len(path)-1])
}

View file

@ -2,6 +2,7 @@ package copy
import (
"fmt"
"path/filepath"
"strings"
)
@ -40,6 +41,14 @@ func parseUserInput(input string) (container string, path string) {
return
}
// If the input is an absolute path, it cannot refer to a container.
// This is necessary because absolute paths on Windows will include
// a colon, which would cause the drive letter to be parsed as a
// container name.
if filepath.IsAbs(input) {
return
}
if parsedContainer, parsedPath, ok := strings.Cut(path, ":"); ok {
container = parsedContainer
path = parsedPath

View file

@ -1,11 +1,8 @@
package e2e_test
import (
"archive/tar"
"bytes"
"fmt"
"io"
"io/fs"
"net"
"net/http"
"net/url"
@ -288,87 +285,6 @@ var _ = Describe("run basic podman commands", func() {
Expect(run).To(Exit(0))
Expect(build.outputToString()).To(ContainSubstring(name))
})
It("Copy ops", func() {
var (
stdinDirectory = "stdin-dir"
stdinFile = "file.txt"
)
now := time.Now()
tarBuffer := &bytes.Buffer{}
tw := tar.NewWriter(tarBuffer)
// Write a directory header to the tar
err := tw.WriteHeader(&tar.Header{
Name: stdinDirectory,
Mode: int64(0640 | fs.ModeDir),
Gid: 1000,
ModTime: now,
ChangeTime: now,
AccessTime: now,
Typeflag: tar.TypeDir,
})
Expect(err).ToNot(HaveOccurred())
// Write a file header to the tar
err = tw.WriteHeader(&tar.Header{
Name: path.Join(stdinDirectory, stdinFile),
Mode: 0755,
Uid: 1000,
ModTime: now,
ChangeTime: now,
AccessTime: now,
})
Expect(err).ToNot(HaveOccurred())
err = tw.Close()
Expect(err).ToNot(HaveOccurred())
name := randomString()
i := new(initMachine)
session, err := mb.setName(name).setCmd(i.withImage(mb.imagePath).withNow()).run()
Expect(err).ToNot(HaveOccurred())
Expect(session).To(Exit(0))
bm := basicMachine{}
newImgs, err := mb.setCmd(bm.withPodmanCommand([]string{"pull", TESTIMAGE})).run()
Expect(err).ToNot(HaveOccurred())
Expect(newImgs).To(Exit(0))
Expect(newImgs.outputToStringSlice()).To(HaveLen(1))
createAlp, err := mb.setCmd(bm.withPodmanCommand([]string{"create", TESTIMAGE, "top"})).run()
Expect(err).ToNot(HaveOccurred())
Expect(createAlp).To(Exit(0))
Expect(createAlp.outputToStringSlice()).To(HaveLen(1))
// Testing stdin copy with archive mode disabled (ownership will be determined by the tar file)
containerID := createAlp.outputToStringSlice()[0]
cpTar, err := mb.setCmd(bm.withPodmanCommand([]string{"cp", "-a=false", "-", containerID + ":/tmp"})).setStdin(tarBuffer).run()
Expect(err).ToNot(HaveOccurred())
Expect(cpTar).To(Exit(0))
start, err := mb.setCmd(bm.withPodmanCommand([]string{"start", containerID})).run()
Expect(err).ToNot(HaveOccurred())
Expect(start).To(Exit(0))
// Check the directory is created with the appropriate mode, uid, gid
exec, err := mb.setCmd(bm.withPodmanCommand([]string{"exec", containerID, "stat", "-c", "%a %u %g", "/tmp/stdin-dir"})).run()
Expect(err).ToNot(HaveOccurred())
Expect(exec).To(Exit(0))
execStdOut := exec.outputToStringSlice()
Expect(execStdOut).To(HaveLen(1))
Expect(execStdOut[0]).To(Equal("640 0 1000"))
// Check the file is created with the appropriate mode, uid, gid
exec, err = mb.setCmd(bm.withPodmanCommand([]string{"exec", containerID, "stat", "-c", "%a %u %g", "/tmp/stdin-dir/file.txt"})).run()
Expect(err).ToNot(HaveOccurred())
Expect(exec).To(Exit(0))
execStdOut = exec.outputToStringSlice()
Expect(execStdOut).To(HaveLen(1))
Expect(execStdOut[0]).To(Equal("755 1000 0"))
})
})
func testHTTPServer(port string, shouldErr bool, expectedResponse string) {

View file

@ -1,18 +1,232 @@
package e2e_test
import (
"archive/tar"
"bytes"
"fmt"
"io/fs"
"os"
"path"
"path/filepath"
"runtime"
"time"
. "github.com/onsi/ginkgo/v2"
. "github.com/onsi/gomega"
. "github.com/onsi/gomega/gexec"
)
var _ = Describe("podman machine cp", func() {
It("all tests", func() {
var _ = Describe("run cp commands", func() {
It("podman cp", func() {
const (
file = "foo.txt"
directory = "foo-dir"
fileInDirectory = "bar.txt"
stdinFile = "file.txt"
stdinDirectory = "stdin-dir"
containerName = "podman-cp-test"
)
sourceDir := GinkgoT().TempDir()
destinationDir := GinkgoT().TempDir()
f, err := os.Create(filepath.Join(sourceDir, file))
Expect(err).ToNot(HaveOccurred())
err = f.Close()
Expect(err).ToNot(HaveOccurred())
// Get the file stat to check permissions later
sourceFileStat, err := os.Stat(filepath.Join(sourceDir, file))
Expect(err).ToNot(HaveOccurred())
err = os.MkdirAll(filepath.Join(sourceDir, directory), 0755)
Expect(err).ToNot(HaveOccurred())
// Get the directory stat to check permissions later
sourceDirStat, err := os.Stat(filepath.Join(sourceDir, directory))
Expect(err).ToNot(HaveOccurred())
f, err = os.Create(filepath.Join(sourceDir, directory, fileInDirectory))
Expect(err).ToNot(HaveOccurred())
err = f.Close()
Expect(err).ToNot(HaveOccurred())
// Get the file in directory stat to check permissions later
sourceFileInDirStat, err := os.Stat(filepath.Join(sourceDir, directory, fileInDirectory))
Expect(err).ToNot(HaveOccurred())
name := randomString()
i := new(initMachine)
session, err := mb.setName(name).setCmd(i.withImage(mb.imagePath).withNow()).run()
Expect(err).ToNot(HaveOccurred())
Expect(session).To(Exit(0))
bm := basicMachine{}
newImgs, err := mb.setCmd(bm.withPodmanCommand([]string{"pull", TESTIMAGE})).run()
Expect(err).ToNot(HaveOccurred())
Expect(newImgs).To(Exit(0))
Expect(newImgs.outputToStringSlice()).To(HaveLen(1))
createAlp, err := mb.setCmd(bm.withPodmanCommand([]string{"create", "--name", containerName, TESTIMAGE, "top"})).run()
Expect(err).ToNot(HaveOccurred())
Expect(createAlp).To(Exit(0))
Expect(createAlp.outputToStringSlice()).To(HaveLen(1))
containerID := createAlp.outputToStringSlice()[0]
// Create a second container to test copying between containers
// This container is named "C" to also test that Windows prefers
// to treat C:\ as a local file path instead of a container name
createAlp, err = mb.setCmd(bm.withPodmanCommand([]string{"create", "--name", "C", TESTIMAGE, "top"})).run()
Expect(err).ToNot(HaveOccurred())
Expect(createAlp).To(Exit(0))
Expect(createAlp.outputToStringSlice()).To(HaveLen(1))
destinationContainerID := createAlp.outputToStringSlice()[0]
By("copy from host to container by id")
// Copy a single file into the container
cpFile, err := mb.setCmd(bm.withPodmanCommand([]string{"cp", filepath.Join(sourceDir, file), containerID + ":/tmp/"})).run()
Expect(err).ToNot(HaveOccurred())
Expect(cpFile).To(Exit(0))
// Copy a directory into the container
cpDir, err := mb.setCmd(bm.withPodmanCommand([]string{"cp", filepath.Join(sourceDir, directory), containerID + ":/tmp"})).run()
Expect(err).ToNot(HaveOccurred())
Expect(cpDir).To(Exit(0))
start, err := mb.setCmd(bm.withPodmanCommand([]string{"start", containerID})).run()
Expect(err).ToNot(HaveOccurred())
Expect(start).To(Exit(0))
// Check the single file is created with the appropriate mode, uid, gid
exec, err := mb.setCmd(bm.withPodmanCommand([]string{"exec", containerID, "stat", "-c", "%a %u %g", path.Join("/tmp", file)})).run()
Expect(err).ToNot(HaveOccurred())
Expect(exec).To(Exit(0))
Expect(exec.outputToString()).To(Equal(fmt.Sprintf("%o %d %d", sourceFileStat.Mode().Perm(), 0, 0)))
// Check the directory is created with the appropriate mode, uid, gid
exec, err = mb.setCmd(bm.withPodmanCommand([]string{"exec", containerID, "stat", "-c", "%a %u %g", path.Join("/tmp", directory)})).run()
Expect(err).ToNot(HaveOccurred())
Expect(exec).To(Exit(0))
Expect(exec.outputToString()).To(Equal(fmt.Sprintf("%o %d %d", sourceDirStat.Mode().Perm(), 0, 0)))
// Check the file in the directory is created with the appropriate mode, uid, gid
exec, err = mb.setCmd(bm.withPodmanCommand([]string{"exec", containerID, "stat", "-c", "%a %u %g", path.Join("/tmp", directory, fileInDirectory)})).run()
Expect(err).ToNot(HaveOccurred())
Expect(exec).To(Exit(0))
Expect(exec.outputToString()).To(Equal(fmt.Sprintf("%o %d %d", sourceFileInDirStat.Mode().Perm(), 0, 0)))
By("copy from host to container by name")
// Copy a single renamed file into the container
cpFile, err = mb.setCmd(bm.withPodmanCommand([]string{"cp", filepath.Join(sourceDir, file), containerName + ":/tmp/rename.txt"})).run()
Expect(err).ToNot(HaveOccurred())
Expect(cpFile).To(Exit(0))
// Check the single file is created with the appropriate mode, uid, gid
exec, err = mb.setCmd(bm.withPodmanCommand([]string{"exec", containerID, "stat", "-c", "%a %u %g", "/tmp/rename.txt"})).run()
Expect(err).ToNot(HaveOccurred())
Expect(exec).To(Exit(0))
Expect(exec.outputToString()).To(Equal(fmt.Sprintf("%o %d %d", sourceFileStat.Mode().Perm(), 0, 0)))
By("copy from container to host")
// Copy the file back from the container to the host
cpFile, err = mb.setCmd(bm.withPodmanCommand([]string{"cp", containerID + ":" + path.Join("/tmp", file), destinationDir + string(os.PathSeparator)})).run()
Expect(err).ToNot(HaveOccurred())
Expect(cpFile).To(Exit(0))
// Get the file stat of the copied file to compare against the original
destinationFileStat, err := os.Stat(filepath.Join(destinationDir, file))
Expect(err).ToNot(HaveOccurred())
Expect(destinationFileStat.Mode()).To(Equal(sourceFileStat.Mode()))
// Compare the modification time of the file in the container and the host (with second level precision)
Expect(destinationFileStat.ModTime()).To(BeTemporally("~", sourceFileStat.ModTime(), time.Second))
// Copy a directory back from the container to the host
cpDir, err = mb.setCmd(bm.withPodmanCommand([]string{"cp", containerID + ":" + path.Join("/tmp", directory), destinationDir + string(os.PathSeparator)})).run()
Expect(err).ToNot(HaveOccurred())
Expect(cpDir).To(Exit(0))
// Get the stat of the copied directory to compare against the original
destinationDirStat, err := os.Stat(filepath.Join(destinationDir, directory))
Expect(err).ToNot(HaveOccurred())
Expect(destinationDirStat.Mode()).To(Equal(sourceDirStat.Mode()))
// Compare the modification time of the folder in the container and the host (with second level precision)
Expect(destinationDirStat.ModTime()).To(BeTemporally("~", sourceDirStat.ModTime(), time.Second))
// Get the stat of the copied file in the directory to compare against the original
destinationFileInDirStat, err := os.Stat(filepath.Join(sourceDir, directory, fileInDirectory))
Expect(err).ToNot(HaveOccurred())
Expect(destinationFileInDirStat.Mode()).To(Equal(sourceFileInDirStat.Mode()))
// Compare the modification time of the file in the container and the host (with second level precision)
Expect(destinationFileInDirStat.ModTime()).To(BeTemporally("~", sourceFileInDirStat.ModTime(), time.Second))
By("copy stdin to container")
now := time.Now()
tarBuffer := &bytes.Buffer{}
tw := tar.NewWriter(tarBuffer)
// Write a directory header to the tar
err = tw.WriteHeader(&tar.Header{
Name: stdinDirectory,
Mode: int64(0640 | fs.ModeDir),
Gid: 1000,
ModTime: now,
ChangeTime: now,
AccessTime: now,
Typeflag: tar.TypeDir,
})
Expect(err).ToNot(HaveOccurred())
// Write a file header to the tar
err = tw.WriteHeader(&tar.Header{
Name: path.Join(stdinDirectory, stdinFile),
Mode: 0755,
Uid: 1000,
ModTime: now,
ChangeTime: now,
AccessTime: now,
})
Expect(err).ToNot(HaveOccurred())
err = tw.Close()
Expect(err).ToNot(HaveOccurred())
// Testing stdin copy with archive mode disabled (ownership will be determined by the tar file)
cpTar, err := mb.setCmd(bm.withPodmanCommand([]string{"cp", "-a=false", "-", containerID + ":/tmp"})).setStdin(tarBuffer).run()
Expect(err).ToNot(HaveOccurred())
Expect(cpTar).To(Exit(0))
// Check the directory is created with the appropriate mode, uid, gid
exec, err = mb.setCmd(bm.withPodmanCommand([]string{"exec", containerID, "stat", "-c", "%a %u %g", "/tmp/stdin-dir"})).run()
Expect(err).ToNot(HaveOccurred())
Expect(exec).To(Exit(0))
Expect(exec.outputToString()).To(Equal("640 0 1000"))
// Check the file is created with the appropriate mode, uid, gid
exec, err = mb.setCmd(bm.withPodmanCommand([]string{"exec", containerID, "stat", "-c", "%a %u %g", "/tmp/stdin-dir/file.txt"})).run()
Expect(err).ToNot(HaveOccurred())
Expect(exec).To(Exit(0))
Expect(exec.outputToString()).To(Equal("755 1000 0"))
By("copy from container to container")
// Copy the file from the first container to the second container (with renaming)
cpFile, err = mb.setCmd(bm.withPodmanCommand([]string{"cp", containerID + ":" + path.Join("/tmp", file), destinationContainerID + ":" + path.Join("/tmp", "destination.txt")})).run()
Expect(err).ToNot(HaveOccurred())
Expect(cpFile).To(Exit(0))
start, err = mb.setCmd(bm.withPodmanCommand([]string{"start", destinationContainerID})).run()
Expect(err).ToNot(HaveOccurred())
Expect(start).To(Exit(0))
// Check the single file is created with the appropriate mode, uid, gid
exec, err = mb.setCmd(bm.withPodmanCommand([]string{"exec", destinationContainerID, "stat", "-c", "%a %u %g", path.Join("/tmp", "destination.txt")})).run()
Expect(err).ToNot(HaveOccurred())
Expect(exec).To(Exit(0))
Expect(exec.outputToString()).To(Equal(fmt.Sprintf("%o %d %d", sourceFileStat.Mode().Perm(), 0, 0)))
})
It("podman machine cp", func() {
// HOST FILE SYSTEM
// ~/<ginkgo_tmp>
// * foo.txt