Support podman --remote when Containerfile is not in context directory

Fixes: #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 <[email protected]>
containers · May 22, 2023 · 4108b37 · 4108b37
1 parent b6a406b
commit 4108b37
Show file tree

Hide file tree

Showing 4 changed files with 29 additions and 25 deletions.
diff --git 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
@@ -650,24 +650,24 @@ 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
 				}
 
 				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
@@ -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
@@ -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"))
 	})