From 1489f8db571ed294248e4c3dc6f73b88f3224884 Mon Sep 17 00:00:00 2001 From: Adam Kaplan Date: Tue, 1 May 2018 14:26:52 -0400 Subject: [PATCH] Keep Option for Injected Files * Adding a Keep option to VolumeSpec so that files can be kept in s2i builds. * This will be used to support the injection of ConfigMap build sources, which should not be truncated. Trello card: https://trello.com/c/RMKJxJUm/1020-5-allow-using-a-configmap-as-an-input-to-a-build-builds --- pkg/api/types.go | 6 +- pkg/build/strategies/sti/sti.go | 4 +- pkg/util/injection.go | 106 +++++++++++++++------------ pkg/util/injection_test.go | 19 +++-- test/integration/integration_test.go | 32 +++++++- 5 files changed, 110 insertions(+), 57 deletions(-) diff --git a/pkg/api/types.go b/pkg/api/types.go index 4a22854c4..9c5a134e9 100644 --- a/pkg/api/types.go +++ b/pkg/api/types.go @@ -266,8 +266,12 @@ type CGroupLimits struct { // VolumeSpec represents a single volume mount point. type VolumeSpec struct { - Source string + // Source is a reference to the volume source. + Source string + // Destination is the path to mount the volume to - absolute or relative. Destination string + // Keep indicates if the mounted data should be kept in the final image. + Keep bool } // VolumeList contains list of VolumeSpec. diff --git a/pkg/build/strategies/sti/sti.go b/pkg/build/strategies/sti/sti.go index 26c87289c..83d508202 100644 --- a/pkg/build/strategies/sti/sti.go +++ b/pkg/build/strategies/sti/sti.go @@ -613,7 +613,7 @@ func (builder *STI) Execute(command string, user string, config *api.Config) err return err } config.Injections = util.FixInjectionsWithRelativePath(workdir, config.Injections) - injectedFiles, err := util.ExpandInjectedFiles(builder.fs, config.Injections) + truncatedFiles, err := util.ListFilesToTruncate(builder.fs, config.Injections) if err != nil { builder.result.BuildInfo.FailureReason = utilstatus.NewFailureReason( utilstatus.ReasonInstallScriptsFailed, @@ -621,7 +621,7 @@ func (builder *STI) Execute(command string, user string, config *api.Config) err ) return err } - rmScript, err := util.CreateInjectedFilesRemovalScript(injectedFiles, "/tmp/rm-injections") + rmScript, err := util.CreateTruncateFilesScript(truncatedFiles, "/tmp/rm-injections") if len(rmScript) != 0 { defer os.Remove(rmScript) } diff --git a/pkg/util/injection.go b/pkg/util/injection.go index 2a76842d6..3df9aec64 100644 --- a/pkg/util/injection.go +++ b/pkg/util/injection.go @@ -37,65 +37,79 @@ func FixInjectionsWithRelativePath(workdir string, injections api.VolumeList) ap return newList } -// ExpandInjectedFiles returns a flat list of all files that are injected into a -// container. All files from nested directories are returned in the list. -func ExpandInjectedFiles(fs fs.FileSystem, injections api.VolumeList) ([]string, error) { +// ListFilesToTruncate returns a flat list of all files that are injected into a +// container which need to be truncated. All files from nested directories are returned in the list. +func ListFilesToTruncate(fs fs.FileSystem, injections api.VolumeList) ([]string, error) { result := []string{} for _, s := range injections { - if _, err := os.Stat(s.Source); err != nil { - return nil, err + if s.Keep { + continue } - err := fs.Walk(s.Source, func(path string, f os.FileInfo, err error) error { - if err != nil { - return err - } - - // Detected files will be truncated. k8s' AtomicWriter creates - // directories and symlinks to directories in order to inject files. - // An attempt to truncate either a dir or symlink to a dir will fail. - // Thus, we need to dereference symlinks to see if they might point - // to a directory. - // Do not try to simplify this logic to simply return nil if a symlink - // is detected. During the tar transfer to an assemble image, symlinked - // files are turned concrete (i.e. they will be turned into regular files - // containing the content of their target). These newly concrete files - // need to be truncated as well. - - if f.Mode()&os.ModeSymlink != 0 { - linkDest, err := filepath.EvalSymlinks(path) - if err != nil { - return fmt.Errorf("unable to evaluate symlink [%v]: %v", path, err) - } - // Evaluate the destination of the link. - f, err = os.Lstat(linkDest) - if err != nil { - // This is not a fatal error. If AtomicWrite tried multiple times, a symlink might not point - // to a valid destination. - glog.Warningf("Unable to lstat symlink destination [%s]->[%s]. err: %v. Partial atomic write?", path, linkDest, err) - return nil - } - } - - if f.IsDir() { - return nil - } - - newPath := filepath.ToSlash(filepath.Join(s.Destination, strings.TrimPrefix(path, s.Source))) - result = append(result, newPath) - return nil - }) + files, err := ListFiles(fs, s) if err != nil { return nil, err } + result = append(result, files...) } return result, nil } -// CreateInjectedFilesRemovalScript creates a shell script that contains truncation +// ListFiles returns a flat list of all files injected into a container for the given `VolumeSpec`. +func ListFiles(fs fs.FileSystem, spec api.VolumeSpec) ([]string, error) { + result := []string{} + if _, err := os.Stat(spec.Source); err != nil { + return nil, err + } + err := fs.Walk(spec.Source, func(path string, f os.FileInfo, err error) error { + if err != nil { + return err + } + + // Detected files will be truncated. k8s' AtomicWriter creates + // directories and symlinks to directories in order to inject files. + // An attempt to truncate either a dir or symlink to a dir will fail. + // Thus, we need to dereference symlinks to see if they might point + // to a directory. + // Do not try to simplify this logic to simply return nil if a symlink + // is detected. During the tar transfer to an assemble image, symlinked + // files are turned concrete (i.e. they will be turned into regular files + // containing the content of their target). These newly concrete files + // need to be truncated as well. + + if f.Mode()&os.ModeSymlink != 0 { + linkDest, err := filepath.EvalSymlinks(path) + if err != nil { + return fmt.Errorf("unable to evaluate symlink [%v]: %v", path, err) + } + // Evaluate the destination of the link. + f, err = os.Lstat(linkDest) + if err != nil { + // This is not a fatal error. If AtomicWrite tried multiple times, a symlink might not point + // to a valid destination. + glog.Warningf("Unable to lstat symlink destination [%s]->[%s]. err: %v. Partial atomic write?", path, linkDest, err) + return nil + } + } + + if f.IsDir() { + return nil + } + + newPath := filepath.ToSlash(filepath.Join(spec.Destination, strings.TrimPrefix(path, spec.Source))) + result = append(result, newPath) + return nil + }) + if err != nil { + return nil, err + } + return result, nil +} + +// CreateTruncateFilesScript creates a shell script that contains truncation // of all files we injected into the container. The path to the script is returned. // When the scriptName is provided, it is also truncated together with all // secrets. -func CreateInjectedFilesRemovalScript(files []string, scriptName string) (string, error) { +func CreateTruncateFilesScript(files []string, scriptName string) (string, error) { rmScript := "set -e\n" for _, s := range files { rmScript += fmt.Sprintf("truncate -s0 %q\n", s) diff --git a/pkg/util/injection_test.go b/pkg/util/injection_test.go index e1784bcc6..6c88c4aaa 100644 --- a/pkg/util/injection_test.go +++ b/pkg/util/injection_test.go @@ -12,12 +12,12 @@ import ( "github.com/openshift/source-to-image/pkg/util/fs" ) -func TestCreateInjectedFilesRemovalScript(t *testing.T) { +func TestCreateTruncateFilesScript(t *testing.T) { files := []string{ "/foo", "/bar/bar", } - name, err := CreateInjectedFilesRemovalScript(files, "/tmp/rm-foo") + name, err := CreateTruncateFilesScript(files, "/tmp/rm-foo") defer os.Remove(name) if err != nil { t.Errorf("Unexpected error: %v", name) @@ -38,21 +38,30 @@ func TestCreateInjectedFilesRemovalScript(t *testing.T) { } } -func TestExpandInjectedFiles(t *testing.T) { +func TestListFilesToTruncate(t *testing.T) { tmp, err := ioutil.TempDir("", "s2i-test-") + tmpKeep, err := ioutil.TempDir("", "s2i-test-") tmpNested, err := ioutil.TempDir(tmp, "nested") if err != nil { t.Errorf("Unable to create temp directory: %v", err) } defer os.RemoveAll(tmp) - list := api.VolumeList{{Source: tmp, Destination: "/foo"}} + defer os.RemoveAll(tmpKeep) + list := api.VolumeList{ + {Source: tmp, Destination: "/foo"}, + {Source: tmpKeep, Destination: "/this", Keep: true}, + } f1, _ := ioutil.TempFile(tmp, "foo") f2, _ := ioutil.TempFile(tmpNested, "bar") - files, err := ExpandInjectedFiles(fs.NewFileSystem(), list) + ioutil.TempFile(tmpKeep, "that") + files, err := ListFilesToTruncate(fs.NewFileSystem(), list) if err != nil { t.Errorf("Unexpected error: %v", err) } expected := []string{"/foo/" + filepath.Base(f1.Name()), "/foo/" + filepath.Base(tmpNested) + "/" + filepath.Base(f2.Name())} + if len(expected) != len(files) { + t.Errorf("Expected %d files in resulting list, got %+v", len(expected), files) + } for _, exp := range expected { found := false for _, f := range files { diff --git a/test/integration/integration_test.go b/test/integration/integration_test.go index 1ff95e26d..a6a59c4d3 100644 --- a/test/integration/integration_test.go +++ b/test/integration/integration_test.go @@ -403,6 +403,12 @@ func (i *integrationTest) exerciseInjectionBuild(tag, imageName string, injectio t.Errorf("injectionList.Set() failed with error %s\n", err) } } + // For test purposes, keep at least one injected source + var keptVolume *api.VolumeSpec + if len(injectionList) > 0 { + injectionList[0].Keep = true + keptVolume = &injectionList[0] + } config := &api.Config{ DockerConfig: docker.GetDefaultDockerConfig(), BuilderImage: imageName, @@ -432,15 +438,35 @@ func (i *integrationTest) exerciseInjectionBuild(tag, imageName string, injectio i.fileExists(containerID, "/sti-fake/relative-secret-delivered") // Make sure the injected file does not exists in resulting image - files, err := util.ExpandInjectedFiles(fs.NewFileSystem(), injectionList) + testFs := fs.NewFileSystem() + files, err := util.ListFilesToTruncate(testFs, injectionList) if err != nil { t.Errorf("Unexpected error: %v", err) } for _, f := range files { - if exitCode := i.runInImage(tag, "test -s "+f); exitCode == 0 { - t.Errorf("The file %q must be empty", f) + if err = i.testFile(tag, f); err == nil { + t.Errorf("The file %q must be empty or not exist", f) } } + if keptVolume != nil { + keptFiles, err := util.ListFiles(testFs, *keptVolume) + if err != nil { + t.Errorf("Unexpected error: %v", err) + } + for _, f := range keptFiles { + if err = i.testFile(tag, f); err != nil { + t.Errorf("The file %q must exist and not be empty", f) + } + } + } +} + +func (i *integrationTest) testFile(tag, path string) error { + exitCode := i.runInImage(tag, "test -s "+path) + if exitCode != 0 { + return fmt.Errorf("file %s does not exist or is empty in the container %s", path, tag) + } + return nil } func (i *integrationTest) exerciseIncrementalBuild(tag, imageName string, removePreviousImage bool, expectClean bool, checkOnBuild bool) {