Merge pull request #6875 from mtrmac/TempDirForURL-API

Restore the previous TempDirForURL API

Author: Luap99

add.go

@@ -599,9 +599,9 @@ func (b *Builder) Add(destination string, extract bool, options AddAndCopyOption
 				go func() {
 					defer wg.Done()
 					defer pipeWriter.Close()
-					var repositoryDir string
-					// TODO: the returned tempDir is never cleaned up, leaking disk space.
-					_, repositoryDir, getErr = define.TempDirForURL(tmpdir.GetTempDir(), "", src)
+					// TODO: the returned cloneDir is never cleaned up, leaking disk space.
+					var cloneDir, subdir string
+					cloneDir, subdir, getErr = define.TempDirForURL(tmpdir.GetTempDir(), "", src)
 					if getErr != nil {
 						return
 					}
@@ -621,6 +621,7 @@ func (b *Builder) Add(destination string, extract bool, options AddAndCopyOption
 						Timestamp:          options.Timestamp,
 					}
 					writer := io.WriteCloser(pipeWriter)
+					repositoryDir := filepath.Join(cloneDir, subdir)
 					getErr = copier.Get(repositoryDir, repositoryDir, getOptions, []string{"."}, writer)
 				}()
 			} else {

define/types.go

@@ -189,11 +189,11 @@ type SBOMScanOptions struct {
 // TempDirForURL checks if the passed-in string looks like a URL or "-".  If it
 // is, TempDirForURL creates a temporary directory, arranges for its contents
 // to be the contents of that URL, and returns the temporary directory's path
-// (for cleanup) and the absolute path to the build context within it.
+// (for cleanup) and a relative subdirectory to the build context within it.
 // Removal of the temporary directory is the responsibility of the caller.
 // If the string doesn't look like a URL or "-", TempDirForURL returns empty
 // strings and a nil error code.
-func TempDirForURL(dir, prefix, url string) (tempDir string, contextDir string, err error) {
+func TempDirForURL(dir, prefix, url string) (tempDir string, relativeContextDir string, err error) {
 	if !urlsource.IsHTTPOrHTTPS(url) &&
 		!strings.HasPrefix(url, "git://") &&
 		!strings.HasPrefix(url, "github.com/") &&
@@ -250,13 +250,17 @@ func TempDirForURL(dir, prefix, url string) (tempDir string, contextDir string,
 		}
 	}
 
-	contextDir, err = securejoin.SecureJoin(downloadDir, contentSubdir)
+	contextDir, err := securejoin.SecureJoin(downloadDir, contentSubdir)
 	if err != nil {
 		return "", "", fmt.Errorf("resolving subdirectory %q in %q: %w", contentSubdir, downloadDir, err)
 	}
+	relativeContextDir, err = filepath.Rel(tempDir, contextDir)
+	if err != nil {
+		return "", "", err
+	}
 	logrus.Debugf("Build context is at %q", contextDir)
 	succeeded = true
-	return tempDir, contextDir, nil
+	return tempDir, relativeContextDir, nil
 }
 
 // parseGitBuildContext parses git build context to `repo`, `sub-dir`
@@ -372,7 +376,12 @@ func stdinToDirectory(dir string) error {
 
 // writeFileInRoot safely writes data to a file inside root, without following
 // symlinks that escape the root directory.
-func writeFileInRoot(root, name string, data []byte, perm os.FileMode) error {
+func writeFileInRoot(root, name string, data []byte, perm os.FileMode) error { //nolint:unparam,nolintlint
+	// Above:
+	// unparam: 'name' currently only receives "Dockerfile" but will potentially support other files later
+	// nolintlint: the unparam linter only triggers if there are ≥ 4 instances; we do have that
+	// with --tests defaulting to true, but not with --tests=false.
+
 	rootHandle, err := os.OpenRoot(root)
 	if err != nil {
 		return err

imagebuildah/stage_executor.go

@@ -512,13 +512,13 @@ func (s *stageExecutor) performCopy(excludes []string, copies ...imagebuilder.Co
 							// additional context contains a tar file
 							// so download and explode tar to buildah
 							// temp and point context to that.
-							// TODO: the returned tempDir is never cleaned up, leaking disk space.
-							_, contextPath, err := define.TempDirForURL(tmpdir.GetTempDir(), internal.BuildahExternalArtifactsDir, additionalBuildContext.Value)
+							// TODO: the returned path is never cleaned up, leaking disk space.
+							path, subdir, err := define.TempDirForURL(tmpdir.GetTempDir(), internal.BuildahExternalArtifactsDir, additionalBuildContext.Value)
 							if err != nil {
 								return fmt.Errorf("unable to download context from external source %q: %w", additionalBuildContext.Value, err)
 							}
 							// point context dir to the extracted path
-							contextDir = contextPath
+							contextDir = filepath.Join(path, subdir)
 							// populate cache for next RUN step
 							additionalBuildContext.DownloadedCache = contextDir
 						} else {
@@ -733,13 +733,13 @@ func (s *stageExecutor) runStageMountPoints(mountList []string) (map[string]inte
 								// additional context contains a tar file
 								// so download and explode tar to buildah
 								// temp and point context to that.
-								// TODO: the returned tempDir is never cleaned up, leaking disk space.
-								_, contextPath, err := define.TempDirForURL(tmpdir.GetTempDir(), internal.BuildahExternalArtifactsDir, additionalBuildContext.Value)
+								// TODO: the returned path is never cleaned up, leaking disk space.
+								path, subdir, err := define.TempDirForURL(tmpdir.GetTempDir(), internal.BuildahExternalArtifactsDir, additionalBuildContext.Value)
 								if err != nil {
 									return nil, fmt.Errorf("unable to download context from external source %q: %w", additionalBuildContext.Value, err)
 								}
 								// point context dir to the extracted path
-								mountPoint = contextPath
+								mountPoint = filepath.Join(path, subdir)
 								// populate cache for next RUN step
 								additionalBuildContext.DownloadedCache = mountPoint
 							} else {

pkg/cli/build.go

@@ -151,15 +151,15 @@ func GenBuildOptions(c *cobra.Command, inputArgs []string, iopts BuildOptions) (
 		}
 	} else {
 		// The context directory could be a URL.  Try to handle that.
-		tempDir, tempContextDir, err := define.TempDirForURL("", "buildah", cliArgs[0])
+		tempDir, subDir, err := define.TempDirForURL("", "buildah", cliArgs[0])
 		if err != nil {
 			return options, nil, nil, fmt.Errorf("prepping temporary context directory: %w", err)
 		}
 		if tempDir != "" {
 			// We had to download it to a temporary directory.
 			// Delete it later.
 			removeAll = append(removeAll, tempDir)
-			contextDir = tempContextDir
+			contextDir = filepath.Join(tempDir, subDir)
 		} else {
 			// Nope, it was local.  Use it as is.
 			absDir, err := filepath.Abs(cliArgs[0])

tests/bud.bats

@@ -8184,6 +8184,8 @@ _EOF
 }
 
 @test "bud with ADD with git repository source escape directory" {
+  skip 'TEMPORARY: needs a (git config user.{email,name})'
+
   _prefetch alpine
 
   local secretdir=${TEST_SCRATCH_DIR}/secretdir