From 4108b37118c3abf199cf211eac4eba26b2ab10b6 Mon Sep 17 00:00:00 2001 From: Daniel J Walsh Date: Mon, 15 May 2023 16:18:21 -0400 Subject: [PATCH] Support podman --remote when Containerfile is not in context directory Fixes: https://github.com/containers/podman/issues/18239 [NO NEW TESTS NEEDED] @test "podman build -f test" in test/system/070-build.bats Will test this. This was passing when run on a local system since the remote end was using the clients path to read the Containerfile The issue is it would not work in a podman machine since the Containerfile would/should be a different path. Signed-off-by: Daniel J Walsh --- pkg/api/handlers/compat/images_build.go | 6 +++-- pkg/bindings/images/build.go | 33 +++++++++++++++---------- pkg/util/utils.go | 1 + test/e2e/build_test.go | 14 +++-------- 4 files changed, 29 insertions(+), 25 deletions(-) diff --git a/pkg/api/handlers/compat/images_build.go b/pkg/api/handlers/compat/images_build.go index 2c300e492b..984660c32c 100644 --- a/pkg/api/handlers/compat/images_build.go +++ b/pkg/api/handlers/compat/images_build.go @@ -221,9 +221,11 @@ func BuildImage(w http.ResponseWriter, r *http.Request) { var m = []string{} if err := json.Unmarshal([]byte(query.Dockerfile), &m); err != nil { // it's not json, assume just a string - m = []string{filepath.Join(contextDirectory, query.Dockerfile)} + m = []string{query.Dockerfile} + } + for _, containerfile := range m { + containerFiles = append(containerFiles, filepath.Join(contextDirectory, filepath.Clean(filepath.FromSlash(containerfile)))) } - containerFiles = m dockerFileSet = true } } diff --git a/pkg/bindings/images/build.go b/pkg/bindings/images/build.go index 59d626f20e..b4f7a87b94 100644 --- a/pkg/bindings/images/build.go +++ b/pkg/bindings/images/build.go @@ -650,14 +650,14 @@ func nTar(excludes []string, sources ...string) (io.ReadCloser, error) { defer gw.Close() defer tw.Close() seen := make(map[devino]string) - for _, src := range sources { - s, err := filepath.Abs(src) + for i, src := range sources { + source, err := filepath.Abs(src) if err != nil { logrus.Errorf("Cannot stat one of source context: %v", err) merr = multierror.Append(merr, err) return } - err = filepath.WalkDir(s, func(path string, d fs.DirEntry, err error) error { + err = filepath.WalkDir(source, func(path string, dentry fs.DirEntry, err error) error { if err != nil { return err } @@ -665,9 +665,9 @@ func nTar(excludes []string, sources ...string) (io.ReadCloser, error) { separator := string(filepath.Separator) // check if what we are given is an empty dir, if so then continue w/ it. Else return. // if we are given a file or a symlink, we do not want to exclude it. - if s == path { + if source == path { separator = "" - if d.IsDir() { + if dentry.IsDir() { var p *os.File p, err = os.Open(path) if err != nil { @@ -682,8 +682,15 @@ func nTar(excludes []string, sources ...string) (io.ReadCloser, error) { } } } - name := filepath.ToSlash(strings.TrimPrefix(path, s+separator)) - + var name string + if i == 0 { + name = filepath.ToSlash(strings.TrimPrefix(path, source+separator)) + } else { + if !dentry.Type().IsRegular() { + return fmt.Errorf("path %s must be a regular file", path) + } + name = filepath.ToSlash(path) + } excluded, err := pm.Matches(name) //nolint:staticcheck if err != nil { return fmt.Errorf("checking if %q is excluded: %w", name, err) @@ -695,8 +702,8 @@ func nTar(excludes []string, sources ...string) (io.ReadCloser, error) { return nil } switch { - case d.Type().IsRegular(): // add file item - info, err := d.Info() + case dentry.Type().IsRegular(): // add file item + info, err := dentry.Info() if err != nil { return err } @@ -735,8 +742,8 @@ func nTar(excludes []string, sources ...string) (io.ReadCloser, error) { seen[di] = name } return err - case d.IsDir(): // add folders - info, err := d.Info() + case dentry.IsDir(): // add folders + info, err := dentry.Info() if err != nil { return err } @@ -749,12 +756,12 @@ func nTar(excludes []string, sources ...string) (io.ReadCloser, error) { if lerr := tw.WriteHeader(hdr); lerr != nil { return lerr } - case d.Type()&os.ModeSymlink != 0: // add symlinks as it, not content + case dentry.Type()&os.ModeSymlink != 0: // add symlinks as it, not content link, err := os.Readlink(path) if err != nil { return err } - info, err := d.Info() + info, err := dentry.Info() if err != nil { return err } diff --git a/pkg/util/utils.go b/pkg/util/utils.go index 18998e6948..2e52180a07 100644 --- a/pkg/util/utils.go +++ b/pkg/util/utils.go @@ -94,6 +94,7 @@ func ParseDockerignore(containerfiles []string, root string) ([]string, string, // so remote must support parsing that. if dockerIgnoreErr != nil { for _, containerfile := range containerfiles { + containerfile = strings.TrimPrefix(containerfile, root) if _, err := os.Stat(filepath.Join(root, containerfile+".containerignore")); err == nil { path, symlinkErr = securejoin.SecureJoin(root, containerfile+".containerignore") if symlinkErr == nil { diff --git a/test/e2e/build_test.go b/test/e2e/build_test.go index 73d12a8dab..0434e1ab27 100644 --- a/test/e2e/build_test.go +++ b/test/e2e/build_test.go @@ -358,14 +358,9 @@ RUN exit 5`, ALPINE) if IsRemote() { podmanTest.StopRemoteService() podmanTest.StartRemoteService() - } else { - Skip("Only valid at remote test, case works fine for regular podman and buildah") } - cwd, err := os.Getwd() - Expect(err).ToNot(HaveOccurred()) - // Write target and fake files - targetSubPath := filepath.Join(cwd, "emptydir") + targetSubPath := filepath.Join(podmanTest.TempDir, "emptydir") if _, err = os.Stat(targetSubPath); err != nil { if os.IsNotExist(err) { err = os.Mkdir(targetSubPath, 0755) @@ -374,14 +369,14 @@ RUN exit 5`, ALPINE) } containerfile := fmt.Sprintf(`FROM %s -COPY /* /dir`, ALPINE) +COPY /emptydir/* /dir`, ALPINE) - containerfilePath := filepath.Join(cwd, "ContainerfilePathToCopier") + containerfilePath := filepath.Join(podmanTest.TempDir, "ContainerfilePathToCopier") err = os.WriteFile(containerfilePath, []byte(containerfile), 0644) Expect(err).ToNot(HaveOccurred()) defer os.Remove(containerfilePath) - session := podmanTest.Podman([]string{"build", "--pull-never", "-t", "test", "-f", "ContainerfilePathToCopier", targetSubPath}) + session := podmanTest.Podman([]string{"build", "--pull-never", "-t", "test", "-f", "ContainerfilePathToCopier", podmanTest.TempDir}) session.WaitWithDefaultTimeout() // NOTE: Docker and buildah both should error when `COPY /* /dir` is done on emptydir // as context. However buildkit simply ignores this so when buildah also starts ignoring @@ -614,7 +609,6 @@ subdir**` Expect(session).To(Exit(0)) output := session.OutputToString() - Expect(output).NotTo(ContainSubstring("Containerfile")) Expect(output).To(ContainSubstring("/testfilter/expected")) Expect(output).NotTo(ContainSubstring("subdir")) })