Dockerfile Injection Removal

* Add script to remove secrets injected into Dockerfile builds
* Ensure dockerfile failure reason is reported in build result

pkg/api/constants/scripts.go

@@ -50,4 +50,7 @@ const (
 
 	// IgnoreFile is the s2i version for ignore files like we see with .gitignore or .dockerignore .. initial impl mirrors documented .dockerignore capabilities
 	IgnoreFile = ".s2iignore"
+
+	// ClearInjections is the s2i script which removes injected content
+	ClearInjections = "clear-injections"
 )

pkg/build/strategies/dockerfile/dockerfile.go

@@ -79,7 +79,8 @@ func (builder *Dockerfile) Build(config *api.Config) (*api.Result, error) {
 		config.AssembleUser = "1001"
 	}
 	if !user.IsUserAllowed(config.AssembleUser, &config.AllowedUIDs) {
-		return nil, s2ierr.NewUserNotAllowedError(config.AssembleUser, false)
+		builder.setFailureReason(utilstatus.ReasonAssembleUserForbidden, utilstatus.ReasonMessageAssembleUserForbidden)
+		return builder.result, s2ierr.NewUserNotAllowedError(config.AssembleUser, false)
 	}
 
 	dir, _ := filepath.Split(config.AsDockerfile)
@@ -90,10 +91,7 @@ func (builder *Dockerfile) Build(config *api.Config) (*api.Result, error) {
 	config.WorkingDir = dir
 
 	if config.BuilderImage == "" {
-		builder.result.BuildInfo.FailureReason = utilstatus.NewFailureReason(
-			utilstatus.ReasonGenericS2IBuildFailed,
-			utilstatus.ReasonMessageGenericS2iBuildFailed,
-		)
+		builder.setFailureReason(utilstatus.ReasonGenericS2IBuildFailed, utilstatus.ReasonMessageGenericS2iBuildFailed)
 		return builder.result, errors.New("builder image name cannot be empty")
 	}
 
@@ -102,10 +100,7 @@ func (builder *Dockerfile) Build(config *api.Config) (*api.Result, error) {
 	}
 
 	if err := builder.CreateDockerfile(config); err != nil {
-		builder.result.BuildInfo.FailureReason = utilstatus.NewFailureReason(
-			utilstatus.ReasonDockerfileCreateFailed,
-			utilstatus.ReasonMessageDockerfileCreateFailed,
-		)
+		builder.setFailureReason(utilstatus.ReasonDockerfileCreateFailed, utilstatus.ReasonMessageDockerfileCreateFailed)
 		return builder.result, err
 	}
 
@@ -230,24 +225,19 @@ func (builder *Dockerfile) CreateDockerfile(config *api.Config) error {
 		buffer.WriteString(fmt.Sprintf("RUN %s\n", sanitize(filepath.ToSlash(filepath.Join(imageScriptsDir, "assemble")))))
 	}
 
-	// Remove any secrets that were copied into the image,
-	// but leave configmap content alone.
-	wroteRun := false
-	for _, injection := range config.Injections {
-		if injection.Keep {
-			continue
-		}
-		if !wroteRun {
-			buffer.WriteString("# Cleaning up injected secret content\n")
-			buffer.WriteString("RUN ")
-			wroteRun = true
-		} else {
-			buffer.WriteString(" && \\\n    ")
-		}
-		buffer.WriteString(fmt.Sprintf("rm -rf %s", sanitize(filepath.ToSlash(injection.Destination))))
+	filesToDelete, err := util.ListFilesToTruncate(builder.fs, config.Injections)
+	if err != nil {
+		return err
 	}
-	if wroteRun {
-		buffer.WriteString("\n")
+	if len(filesToDelete) > 0 {
+		_, err := util.CreateDeleteFilesScript(filesToDelete, filepath.Join(config.WorkingDir, builder.uploadScriptsDir))
+		if err != nil {
+			return err
+		}
+		buffer.WriteString("# Cleaning up injected secret content\n")
+		rmDestination := filepath.Join(scriptsDestDir, constants.ClearInjections)
+		buffer.WriteString(fmt.Sprintf("COPY --chown=%s:0 %s %s\n", sanitize(imageUser), sanitize(filepath.ToSlash(filepath.Join(constants.UploadScripts, constants.ClearInjections))), filepath.ToSlash(rmDestination)))
+		buffer.WriteString(fmt.Sprintf("RUN %[1]s && rm %[1]s\n", filepath.ToSlash(rmDestination)))
 	}
 
 	if _, provided := providedScripts[constants.Run]; provided {
@@ -273,10 +263,7 @@ func (builder *Dockerfile) Prepare(config *api.Config) error {
 
 	if len(config.WorkingDir) == 0 {
 		if config.WorkingDir, err = builder.fs.CreateWorkingDirectory(); err != nil {
-			builder.result.BuildInfo.FailureReason = utilstatus.NewFailureReason(
-				utilstatus.ReasonFSOperationFailed,
-				utilstatus.ReasonMessageFSOperationFailed,
-			)
+			builder.setFailureReason(utilstatus.ReasonFSOperationFailed, utilstatus.ReasonMessageFSOperationFailed)
 			return err
 		}
 	}
@@ -286,10 +273,7 @@ func (builder *Dockerfile) Prepare(config *api.Config) error {
 	// Setup working directories
 	for _, v := range workingDirs {
 		if err = builder.fs.MkdirAllWithPermissions(filepath.Join(config.WorkingDir, v), 0755); err != nil {
-			builder.result.BuildInfo.FailureReason = utilstatus.NewFailureReason(
-				utilstatus.ReasonFSOperationFailed,
-				utilstatus.ReasonMessageFSOperationFailed,
-			)
+			builder.setFailureReason(utilstatus.ReasonFSOperationFailed, utilstatus.ReasonMessageFSOperationFailed)
 			return err
 		}
 	}
@@ -303,13 +287,11 @@ func (builder *Dockerfile) Prepare(config *api.Config) error {
 	if config.Source != nil {
 		downloader, err := scm.DownloaderForSource(builder.fs, config.Source, config.ForceCopy)
 		if err != nil {
+			builder.setFailureReason(utilstatus.ReasonFetchSourceFailed, utilstatus.ReasonMessageFetchSourceFailed)
 			return err
 		}
 		if builder.sourceInfo, err = downloader.Download(config); err != nil {
-			builder.result.BuildInfo.FailureReason = utilstatus.NewFailureReason(
-				utilstatus.ReasonFetchSourceFailed,
-				utilstatus.ReasonMessageFetchSourceFailed,
-			)
+			builder.setFailureReason(utilstatus.ReasonFetchSourceFailed, utilstatus.ReasonMessageFetchSourceFailed)
 			switch err.(type) {
 			case file.RecursiveCopyError:
 				return fmt.Errorf("input source directory contains the target directory for the build, check that your Dockerfile output path does not reside within your input source path: %v", err)
@@ -333,6 +315,7 @@ func (builder *Dockerfile) Prepare(config *api.Config) error {
 		dst := filepath.Join(config.WorkingDir, constants.Injections, trimmedSrc)
 		glog.V(4).Infof("Copying injection content from %s to %s", injection.Source, dst)
 		if err := builder.fs.CopyContents(injection.Source, dst); err != nil {
+			builder.setFailureReason(utilstatus.ReasonGenericS2IBuildFailed, utilstatus.ReasonMessageGenericS2iBuildFailed)
 			return err
 		}
 		config.Injections[i].Source = trimmedSrc
@@ -340,7 +323,12 @@ func (builder *Dockerfile) Prepare(config *api.Config) error {
 
 	// see if there is a .s2iignore file, and if so, read in the patterns and then
 	// search and delete on them.
-	return builder.ignorer.Ignore(config)
+	err = builder.ignorer.Ignore(config)
+	if err != nil {
+		builder.setFailureReason(utilstatus.ReasonGenericS2IBuildFailed, utilstatus.ReasonMessageGenericS2iBuildFailed)
+		return err
+	}
+	return nil
 }
 
 // installScripts installs scripts at the provided URL to the Dockerfile context
@@ -359,6 +347,11 @@ func (builder *Dockerfile) installScripts(scriptsURL string, config *api.Config)
 	return scriptInstaller.InstallOptional(append(scripts.RequiredScripts, scripts.OptionalScripts...), config.WorkingDir)
 }
 
+// setFailureReason sets the builder's failure reason with the given reason and message.
+func (builder *Dockerfile) setFailureReason(reason api.StepFailureReason, message api.StepFailureMessage) {
+	builder.result.BuildInfo.FailureReason = utilstatus.NewFailureReason(reason, message)
+}
+
 // getDestination returns the destination directory from the config.
 func getDestination(config *api.Config) string {
 	destination := config.Destination

pkg/util/injection.go

@@ -1,6 +1,7 @@
 package util
 
 import (
+	"bytes"
 	"fmt"
 	"io/ioutil"
 	"os"
@@ -8,6 +9,7 @@ import (
 	"strings"
 
 	"github.com/openshift/source-to-image/pkg/api"
+	"github.com/openshift/source-to-image/pkg/api/constants"
 	"github.com/openshift/source-to-image/pkg/util/fs"
 )
 
@@ -127,6 +129,20 @@ func CreateTruncateFilesScript(files []string, scriptName string) (string, error
 	return f.Name(), err
 }
 
+// CreateDeleteFilesScript creates a shell script that contains removal
+// of the provided files injected into the container. The path to the script is returned.
+func CreateDeleteFilesScript(files []string, dir string) (string, error) {
+	rmScript := &bytes.Buffer{}
+	rmScript.WriteString("set -e\n")
+	for _, s := range files {
+		rmScript.WriteString(fmt.Sprintf("rm %q\n", s))
+	}
+	rmScript.WriteString("set +e\n")
+	scriptName := filepath.Join(dir, constants.ClearInjections)
+	err := ioutil.WriteFile(scriptName, rmScript.Bytes(), 0700)
+	return scriptName, err
+}
+
 // CreateInjectionResultFile creates a result file with the message from the provided injection
 // error. The path to the result file is returned. If the provided error is nil, an empty file is
 // created.

pkg/util/injection_test.go

@@ -31,9 +31,12 @@ func TestCreateTruncateFilesScript(t *testing.T) {
 	if err != nil {
 		t.Errorf("Unable to read %q: %v", name, err)
 	}
-	if !strings.Contains(string(data), fmt.Sprintf("truncate -s0 %q", "/foo")) {
-		t.Errorf("Expected script to contain truncate -s0 \"/foo\", got: %q", string(data))
+	for _, f := range files {
+		if !strings.Contains(string(data), fmt.Sprintf("truncate -s0 %q", f)) {
+			t.Errorf("Expected script to contain truncate -s0 %q, got: %q", f, string(data))
+		}
 	}
+
 	if !strings.Contains(string(data), fmt.Sprintf("truncate -s0 %q", "/tmp/rm-foo")) {
 		t.Errorf("Expected script to truncate itself, got: %q", string(data))
 	}
@@ -110,3 +113,33 @@ func TestCreateInjectionResultFile(t *testing.T) {
 		}
 	}
 }
+
+func TestCreateDeleteFilesScript(t *testing.T) {
+	files := []string{
+		"/foo",
+		"/bar/bar",
+	}
+	dir, err := ioutil.TempDir("", "s2i-delete-files-script")
+	if err != nil {
+		t.Fatalf("failed to create temp dir: %v", err)
+	}
+	defer os.RemoveAll(dir)
+	name, err := CreateDeleteFilesScript(files, dir)
+	if err != nil {
+		t.Errorf("Unexpected error: %v", name)
+	}
+	_, err = os.Stat(name)
+	if err != nil {
+		t.Errorf("Expected file %q to exists, got: %v", name, err)
+	}
+	data, err := ioutil.ReadFile(name)
+	if err != nil {
+		t.Errorf("Unable to read %q: %v", name, err)
+	}
+	for _, f := range files {
+		if !strings.Contains(string(data), fmt.Sprintf("rm %q", f)) {
+			t.Errorf("Expected script to contain rm %q, got: %q", f, string(data))
+		}
+	}
+
+}

pkg/util/status/build_status.go

@@ -111,6 +111,14 @@ const (
 	// ReasonMessageOnBuildForbidden is the message associated with an image that
 	// uses the ONBUILD instruction when it's not allowed.
 	ReasonMessageOnBuildForbidden api.StepFailureMessage = "ONBUILD instructions not allowed in this context."
+
+	// ReasonAssembleUserForbidden is the failure reason associated with an image that
+	// uses a forbidden AssembleUser.
+	ReasonAssembleUserForbidden api.StepFailureReason = "AssembleUserForbidden"
+
+	// ReasonMessageAssembleUserForbidden is the failure reason associated with an image that
+	// uses a forbidden AssembleUser.
+	ReasonMessageAssembleUserForbidden api.StepFailureMessage = "Assemble user for S2I build is forbidden."
 )
 
 // NewFailureReason initializes a new failure reason that contains both the

test/integration/integration_test.go

@@ -751,12 +751,20 @@ func TestDockerfileBuildInjections(t *testing.T) {
 	if err != nil {
 		t.Errorf("Unable to create injection dir: %v", err)
 	}
+	_, err = ioutil.TempFile(injection1, "injectfile-1")
+	if err != nil {
+		t.Errorf("Unable to create injection file: %v", err)
+	}
 
 	injection2 := filepath.Join(tempdir, "injection2")
 	err = os.Mkdir(injection2, 0777)
 	if err != nil {
 		t.Errorf("Unable to create injection dir: %v", err)
 	}
+	_, err = ioutil.TempFile(injection2, "injectfile-2")
+	if err != nil {
+		t.Errorf("Unable to create injection file: %v", err)
+	}
 
 	config := &api.Config{
 		BuilderImage: "docker.io/centos/nodejs-8-centos7",
@@ -792,7 +800,8 @@ func TestDockerfileBuildInjections(t *testing.T) {
 	expected := []string{
 		"COPY --chown=1001:0 upload/injections" + trimmedInjection1 + " /workdir/injection1",
 		"COPY --chown=1001:0 upload/injections" + trimmedInjection2 + " /destination/injection2",
-		"rm -rf /workdir/injection1",
+		"COPY --chown=1001:0 upload/scripts/clear-injections /tmp/scripts/clear-injections",
+		"RUN /tmp/scripts/clear-injections && rm /tmp/scripts/clear-injections",
 	}
 	notExpected := []string{
 		"rm -rf /destination/injection2",
@@ -801,6 +810,7 @@ func TestDockerfileBuildInjections(t *testing.T) {
 		filepath.Join(tempdir, "upload/src/server.js"),
 		filepath.Join(tempdir, "upload/injections"+trimmedInjection1),
 		filepath.Join(tempdir, "upload/injections"+trimmedInjection2),
+		filepath.Join(tempdir, "upload/scripts/clear-injections"),
 	}
 	runDockerfileTest(t, config, expected, notExpected, expectedFiles)
 }