feat(pkg/commands): add support for build secrets

integration/dockerfiles/Dockerfile_test_run_mount

@@ -19,6 +19,9 @@ RUN --mount=type=secret,id=FOO,env=FOO,required=true echo "$FOO" > /etc/foo
 RUN --mount=type=secret,id=BAR cat /run/secrets/BAR > /etc/bar
 # Make sure we set secrets to custom disk location the same way docker does
 RUN --mount=type=secret,id=BAZ,target=/baz.secret cat /baz.secret > /etc/baz
+# Make sure relative targets for secret mounts work:
+WORKDIR /etc
+RUN --mount=type=secret,id=QUX,target=qux.secret cat qux.secret > /etc/qux
 
 # Test with ARG so that we know our implementation of secrets don't override args
 ARG file

integration/images.go

@@ -79,6 +79,7 @@ var secretsMap = map[string][]buildSecret{
 		{name: "FOO", value: "foo"},
 		{name: "BAR", value: "bar"},
 		{name: "BAZ", value: "baz"},
+		{name: "QUX", value: "qux"},
 	},
 }
 

integration/integration_test.go

@@ -591,8 +591,9 @@ func TestBuildWithHTTPError(t *testing.T) {
 
 func TestLayers(t *testing.T) {
 	offset := map[string]int{
-		"Dockerfile_test_add":     12,
-		"Dockerfile_test_scratch": 3,
+		"Dockerfile_test_add":       12,
+		"Dockerfile_test_scratch":   3,
+		"Dockerfile_test_run_mount": 1, // The WORKDIR layer is not present in the kaniko image
 	}
 
 	if os.Getenv("CI") == "true" {

pkg/commands/run.go

@@ -64,7 +64,7 @@ func (r *RunCommand) ExecuteCommand(config *v1.Config, buildArgs *dockerfile.Bui
 	return runCommandInExec(config, buildArgs, r.cmd, r.output, r.buildSecrets)
 }
 
-func runCommandInExec(config *v1.Config, buildArgs *dockerfile.BuildArgs, cmdRun *instructions.RunCommand, output *RunOutput, buildSecrets []string) error {
+func runCommandInExec(config *v1.Config, buildArgs *dockerfile.BuildArgs, cmdRun *instructions.RunCommand, output *RunOutput, buildSecrets []string) (err error) {
 	if output == nil {
 		output = &RunOutput{}
 	}
@@ -152,7 +152,14 @@ func runCommandInExec(config *v1.Config, buildArgs *dockerfile.BuildArgs, cmdRun
 		buildSecretsMap[secretName] = secretValue
 	}
 
-	var buildSecretDirOrFilesToClean []string
+	secretFileManager := FileCreatorCleaner{}
+	defer func() {
+		cleanupErr := secretFileManager.Clean()
+		if err == nil {
+			err = cleanupErr
+		}
+	}()
+
 	mounts := instructions.GetMounts(cmdRun)
 	for _, mount := range mounts {
 		switch mount.Type {
@@ -174,17 +181,10 @@ func runCommandInExec(config *v1.Config, buildArgs *dockerfile.BuildArgs, cmdRun
 				if targetFile == "" {
 					targetFile = filepath.Join(secretsDir, mount.CacheID)
 				}
-
-				createdDirs, err := mkdirAllAndListCreated(filepath.Dir(targetFile), 0700)
-				if err != nil {
-					return errors.Wrap(err, "creating directories for secret file")
-				}
-				buildSecretDirOrFilesToClean = append(buildSecretDirOrFilesToClean, createdDirs...)
-
-				if err := os.WriteFile(targetFile, []byte(secret), 0600); err != nil {
-					return errors.Wrap(err, "writing secret to file")
+				if !filepath.IsAbs(targetFile) {
+					targetFile = filepath.Join(config.WorkingDir, targetFile)
 				}
-				buildSecretDirOrFilesToClean = append(buildSecretDirOrFilesToClean, targetFile)
+				secretFileManager.MkdirAndWriteFile(targetFile, []byte(secret), 0700, 0600)
 			}
 
 			// We don't return in the block above, because its possible to have both a target and an env.
@@ -226,68 +226,69 @@ func runCommandInExec(config *v1.Config, buildArgs *dockerfile.BuildArgs, cmdRun
 		return errors.Wrap(err, "waiting for process to exit")
 	}
 
-	// We need to recursively clean up after any secrets that were written to disk.
-	// But also, we need to clean up any directories that we created to house those
-	// secrets. But also we should only clean up that directory if the build process
-	// hasn't put anything else in there. If it has put anything else non secret in
-	// there, then that directory is now meant to make it into the container and we
-	// shouldn't remove it. This loop does that. We iterate in reverse because that
-	// way we can remove the deepest directories first.
-	for i := len(buildSecretDirOrFilesToClean) - 1; i >= 0; i-- {
-		dir := buildSecretDirOrFilesToClean[i]
-
-		info, err := filesystem.FS.Stat(dir)
-		if os.IsNotExist(err) {
-			continue
-		} else if err != nil {
-			return errors.Wrap(err, "statting directory")
-		}
-
-		if info.IsDir() {
-			err := os.Remove(dir)
-			if err != nil {
-				if os.IsExist(err) {
-					continue
-				}
-				return err
-			}
-		} else {
-			err := os.Remove(dir)
-			if err != nil {
-				return err
-			}
-		}
-	}
-
 	// it's not an error if there are no grandchildren
 	if err := syscall.Kill(-pgid, syscall.SIGKILL); err != nil && err.Error() != "no such process" {
 		return err
 	}
 	return nil
 }
 
-func mkdirAllAndListCreated(path string, perm os.FileMode) ([]string, error) {
-	var createdDirs []string
-	absPath, err := filepath.Abs(path)
-	if err != nil {
-		return nil, err
-	}
+// FileCreatorCleaner keeps tracks of all files and directories that it created in the order that they were created.
+// Once asked to clean up, it will remove all files and directories in the reverse order that they were created.
+type FileCreatorCleaner struct {
+	filesToClean []string
+	dirsToClean  []string
+}
+
+func (s *FileCreatorCleaner) MkdirAndWriteFile(path string, data []byte, dirPerm, filePerm os.FileMode) error {
+	dirPath := filepath.Dir(path)
+	parentDirs := filepath.SplitList(dirPath)
 
-	dirs := filepath.SplitList(absPath)
+	// Start at the root directory
 	currentPath := string(os.PathSeparator)
 
-	for _, dir := range dirs {
-		currentPath = filepath.Join(currentPath, dir)
+	for _, nextDirDown := range parentDirs {
+		// Traverse one level down
+		currentPath = filepath.Join(currentPath, nextDirDown)
 
 		if _, err := filesystem.FS.Stat(currentPath); os.IsNotExist(err) {
-			if err := os.Mkdir(currentPath, perm); err != nil {
-				return createdDirs, err
+			if err := os.Mkdir(currentPath, dirPerm); err != nil {
+				return err
 			}
-			createdDirs = append(createdDirs, currentPath)
+			s.dirsToClean = append(s.dirsToClean, currentPath)
 		}
 	}
 
-	return createdDirs, nil
+	// With all parent directories created, we can now create the actual secret file
+	if err := os.WriteFile(path, []byte(data), 0600); err != nil {
+		return errors.Wrap(err, "writing secret to file")
+	}
+	s.filesToClean = append(s.filesToClean, path)
+
+	return nil
+}
+
+func (s *FileCreatorCleaner) Clean() error {
+	for i := len(s.filesToClean) - 1; i >= 0; i-- {
+		if err := os.Remove(s.filesToClean[i]); err != nil {
+			return err
+		}
+	}
+
+	for i := len(s.dirsToClean) - 1; i >= 0; i-- {
+		if err := os.Remove(s.dirsToClean[i]); err != nil {
+			pathErr := os.PathError{}
+			// If a path that we need to clean up is not empty, then that means
+			// that a third party has placed something in there since we created it.
+			// In that case, we should not remove it, because it no longer belongs exclusively to us.
+			if errors.As(err, &pathErr); pathErr.Err == syscall.ENOTEMPTY {
+				continue
+			}
+			return err
+		}
+	}
+
+	return nil
 }
 
 // addDefaultHOME adds the default value for HOME if it isn't already set