From 19442d1ba101f08e5d4cb8d6c44da426130849aa Mon Sep 17 00:00:00 2001 From: gabemontero Date: Mon, 20 Jul 2015 11:05:59 -0400 Subject: [PATCH 1/2] issue197: implement .dockerignore style function via .s2iignore file and code changes, with updated doc and tests; investigation and work around for hang with very quick git clones; multiple revisions based on comments from Ben and Maciej --- .gitignore | 1 + README.md | 36 ++- pkg/api/scripts.go | 20 +- pkg/api/types.go | 4 + pkg/build/ignore/doc.go | 3 + pkg/build/interfaces.go | 9 + pkg/build/strategies/onbuild/onbuild.go | 3 + pkg/build/strategies/onbuild/onbuild_test.go | 4 + pkg/build/strategies/sti/sti.go | 10 +- pkg/build/strategies/sti/sti_test.go | 2 + pkg/git/clone.go | 7 +- pkg/git/git.go | 24 +- pkg/ignore/ignore.go | 128 +++++++++++ pkg/ignore/ignore_test.go | 224 +++++++++++++++++++ 14 files changed, 453 insertions(+), 22 deletions(-) create mode 100644 pkg/build/ignore/doc.go create mode 100644 pkg/ignore/ignore.go create mode 100644 pkg/ignore/ignore_test.go diff --git a/.gitignore b/.gitignore index 125bf420b..1a67f577b 100644 --- a/.gitignore +++ b/.gitignore @@ -1,3 +1,4 @@ /_output /.vagrant /.project +*~ diff --git a/README.md b/README.md index c568e2b5a..de62cd396 100644 --- a/README.md +++ b/README.md @@ -40,6 +40,40 @@ image: Additionally for the best user experience and optimized `sti` operation we suggest images to have `/bin/sh` and `tar` commands available. +Filtering the contents of the source tree is possible if the user supplies a +`.s2iignore` file in the root directory of the source repository, where `.s2iignore` contains regular +expressions that capture the set of files and directories you want filtered from the image s2i produces. + +Specifically: + +1. Specify one rule per line, with each line terminating in `\n`. +1. Filepaths are appended to the absolute path of the root of the source tree (either the local directory supplied, or the target destination of the clone of the remote source repository s2i creates). +1. Wildcards and globbing (file name expansion) leverage Go's `filepath.Match` and `filepath.Glob` functions. +1. Search is not recursive. Subdirectory paths must be specified (though wildcards and regular expressions can be used in the subdirectory specifications). +1. If the first character is the `#` character, the line is treated as a comment. +1. If the first character is the `!`, the rule is an exception rule, and can undo candidates selected for filtering by prior rules (but only prior rules). + +Here are some examples to help illustrate: + +With specifying subdirectories, the `*/temp*` rule prevents the filtering of any files starting with `temp` that are in any subdirectory that is immediately (or one level) below the root directory. +And the `*/*/temp*` rule prevents the filtering of any files starting with `temp` that are in any subdirectory that is two levels below the root directory. + +Next, to illustrate exception rules, first consider the following example snippet of a `.s2iignore` file: + + + *.md + !README.md + + +With this exception rule example, README.md will not be filtered, and remain in the image s2i produces. However, with this snippet: + + + !README.md + *.md + + +README.md, if filtered by any prior rules, but then put back in by `!README.md`, would be filtered, and not part of the resulting image s2i produces. Since `*.md` follows `!README.md`, `*.md` takes precedence. + Users can also set extra environment variables in the application source code. They are passed to the build, and the `assemble` script consumes them. All environment variables are also present in the output application image. These @@ -61,7 +95,7 @@ See [here](https://github.com/openshift/source-to-image/blob/master/docs/builder The `sti build` workflow is: 1. `sti` creates a container based on the build image and passes it a tar file that contains: - 1. The application source in `src` + 1. The application source in `src`, excluding any files selected by `.s2iignore` 1. The build artifacts in `artifacts` (if applicable - see [incremental builds](#incremental-builds)) 1. `sti` sets the environment variables from `.sti/environment` (optional) 1. `sti` starts the container and runs its `assemble` script diff --git a/pkg/api/scripts.go b/pkg/api/scripts.go index ebb69aceb..ece54ead0 100644 --- a/pkg/api/scripts.go +++ b/pkg/api/scripts.go @@ -1,5 +1,9 @@ package api +import ( + "os" +) + const ( // Assemble is the name of the script responsible for build process of the resulting image. Assemble = "assemble" @@ -18,14 +22,20 @@ const ( const ( // UserScripts is the location of scripts downloaded from user provided URL (-s flag). - UserScripts = "downloads/scripts" + UserScripts = "downloads" + string(os.PathSeparator) + "scripts" // DefaultScripts is the location of scripts downloaded from default location (io.openshift.s2i.scripts-url label). - DefaultScripts = "downloads/defaultScripts" + DefaultScripts = "downloads" + string(os.PathSeparator) + "defaultScripts" // SourceScripts is the location of scripts downloaded with application sources. - SourceScripts = "upload/src/.sti/bin" + SourceScripts = "upload" + string(os.PathSeparator) + "src" + string(os.PathSeparator) + ".sti" + string(os.PathSeparator) + "bin" // UploadScripts is the location of scripts that will be uploaded to the image during STI build. - UploadScripts = "upload/scripts" + UploadScripts = "upload" + string(os.PathSeparator) + "scripts" // Source is the location of application sources. - Source = "upload/src" + Source = "upload" + string(os.PathSeparator) + "src" + + // ContextTmp is the location of applications sources off of a supplied context dir + ContextTmp = "upload" + string(os.PathSeparator) + "tmp" + + // Ignorefile is the s2i version for ignore files like we see with .gitignore or .dockerignore .. initial impl mirrors documented .dockerignore capabilities + IgnoreFile = ".s2iignore" ) diff --git a/pkg/api/types.go b/pkg/api/types.go index ad375fa30..401e75986 100644 --- a/pkg/api/types.go +++ b/pkg/api/types.go @@ -77,6 +77,10 @@ type Config struct { // WorkingDir describes temporary directory used for downloading sources, scripts and tar operations. WorkingDir string + // WorkingSourceDir describes the subdirectory off of WorkingDir set up during the repo download + // that is later used as the root for ignore processing + WorkingSourceDir string + // LayeredBuild describes if this is build which layered scripts and sources on top of BuilderImage. LayeredBuild bool diff --git a/pkg/build/ignore/doc.go b/pkg/build/ignore/doc.go new file mode 100644 index 000000000..7883c76d9 --- /dev/null +++ b/pkg/build/ignore/doc.go @@ -0,0 +1,3 @@ +// implements an interface around providing ignore file capabillities like seen in .dockerignore or .gitignore + +package ignore diff --git a/pkg/build/interfaces.go b/pkg/build/interfaces.go index e5684d829..99549b24b 100644 --- a/pkg/build/interfaces.go +++ b/pkg/build/interfaces.go @@ -41,11 +41,20 @@ type Downloader interface { Download(*api.Config) (*api.SourceInfo, error) } +// Implement ignore file type processing on source tree +// NOTE: raised to this level for possible future extensions to +// support say both .gitignore and .dockerignore level functionality +// ( currently do .dockerignore) +type Ignorer interface { + Ignore(*api.Config) error +} + // SourceHandler is a wrapper for STI strategy Downloader and Preparer which // allows to use Download and Prepare functions from the STI strategy. type SourceHandler interface { Downloader Preparer + Ignorer } // LayeredDockerBuilder represents a minimal Docker builder interface that is diff --git a/pkg/build/strategies/onbuild/onbuild.go b/pkg/build/strategies/onbuild/onbuild.go index 141be790a..f27906fcc 100644 --- a/pkg/build/strategies/onbuild/onbuild.go +++ b/pkg/build/strategies/onbuild/onbuild.go @@ -10,6 +10,7 @@ import ( "github.com/golang/glog" "github.com/openshift/source-to-image/pkg/api" "github.com/openshift/source-to-image/pkg/build" + "github.com/openshift/source-to-image/pkg/ignore" "github.com/openshift/source-to-image/pkg/build/strategies/sti" "github.com/openshift/source-to-image/pkg/docker" "github.com/openshift/source-to-image/pkg/git" @@ -32,6 +33,7 @@ type OnBuild struct { type onBuildSourceHandler struct { build.Downloader build.Preparer + build.Ignorer } // New returns a new instance of OnBuild builder @@ -53,6 +55,7 @@ func New(config *api.Config) (*OnBuild, error) { b.source = onBuildSourceHandler{ &git.Clone{b.git, b.fs}, s, + &ignore.DockerIgnorer{}, } b.garbage = &build.DefaultCleaner{b.fs, b.docker} return b, nil diff --git a/pkg/build/strategies/onbuild/onbuild_test.go b/pkg/build/strategies/onbuild/onbuild_test.go index 88eb31bf5..ee1a5fc03 100644 --- a/pkg/build/strategies/onbuild/onbuild_test.go +++ b/pkg/build/strategies/onbuild/onbuild_test.go @@ -19,6 +19,10 @@ func (*fakeSourceHandler) Prepare(r *api.Config) error { return nil } +func (*fakeSourceHandler) Ignore(r *api.Config) error { + return nil +} + func (*fakeSourceHandler) Download(r *api.Config) (*api.SourceInfo, error) { return &api.SourceInfo{}, nil } diff --git a/pkg/build/strategies/sti/sti.go b/pkg/build/strategies/sti/sti.go index 4daf6e72d..bddc3e895 100644 --- a/pkg/build/strategies/sti/sti.go +++ b/pkg/build/strategies/sti/sti.go @@ -14,6 +14,7 @@ import ( "github.com/openshift/source-to-image/pkg/docker" "github.com/openshift/source-to-image/pkg/errors" "github.com/openshift/source-to-image/pkg/git" + "github.com/openshift/source-to-image/pkg/ignore" "github.com/openshift/source-to-image/pkg/scripts" "github.com/openshift/source-to-image/pkg/tar" "github.com/openshift/source-to-image/pkg/util" @@ -56,6 +57,7 @@ type STI struct { // Interfaces preparer build.Preparer + ignorer build.Ignorer artifacts build.IncrementalBuilder scripts build.ScriptsHandler source build.Downloader @@ -97,6 +99,9 @@ func New(req *api.Config) (*STI, error) { // Set interfaces b.preparer = b + // later on, if we support say .gitignore func in addition to .dockerignore func, setting + // ignorer will be based on config setting + b.ignorer = &ignore.DockerIgnorer{} b.artifacts = b b.scripts = b b.postExecutor = b @@ -149,6 +154,8 @@ func (b *STI) Build(config *api.Config) (*api.Result, error) { } // Prepare prepares the source code and tar for build +// NOTE, this func serves both the sti and onbuild strategies, as the OnBuild +// struct Build func leverages the STI struct Prepare func directly below func (b *STI) Prepare(config *api.Config) error { var err error if config.WorkingDir, err = b.fs.CreateWorkingDirectory(); err != nil { @@ -192,7 +199,8 @@ func (b *STI) Prepare(config *api.Config) error { } } - return nil + // see if there is a .s2iignore file, and if so, read in the patterns an then search and delete on + return b.ignorer.Ignore(config) } // SetScripts allows to override default required and optional scripts diff --git a/pkg/build/strategies/sti/sti_test.go b/pkg/build/strategies/sti/sti_test.go index 048f185ea..51c7ff33f 100644 --- a/pkg/build/strategies/sti/sti_test.go +++ b/pkg/build/strategies/sti/sti_test.go @@ -10,6 +10,7 @@ import ( "github.com/openshift/source-to-image/pkg/build" stierr "github.com/openshift/source-to-image/pkg/errors" "github.com/openshift/source-to-image/pkg/git" + "github.com/openshift/source-to-image/pkg/ignore" "github.com/openshift/source-to-image/pkg/test" ) @@ -59,6 +60,7 @@ func newFakeSTI(f *FakeSTI) *STI { fs: &test.FakeFileSystem{}, tar: &test.FakeTar{}, preparer: f, + ignorer: &ignore.DockerIgnorer{}, artifacts: f, scripts: f, garbage: f, diff --git a/pkg/git/clone.go b/pkg/git/clone.go index 64b0f868e..ce8cde3c4 100644 --- a/pkg/git/clone.go +++ b/pkg/git/clone.go @@ -17,12 +17,13 @@ type Clone struct { // Download downloads the application source code from the GIT repository // and checkout the Ref specified in the config. func (c *Clone) Download(config *api.Config) (*api.SourceInfo, error) { - targetSourceDir := filepath.Join(config.WorkingDir, "upload", "src") + targetSourceDir := filepath.Join(config.WorkingDir, api.Source) + config.WorkingSourceDir = targetSourceDir var info *api.SourceInfo if c.ValidCloneSpec(config.Source) { if len(config.ContextDir) > 0 { - targetSourceDir = filepath.Join(config.WorkingDir, "upload", "tmp") + targetSourceDir = filepath.Join(config.WorkingDir, api.ContextTmp) } glog.V(2).Infof("Cloning into %s", targetSourceDir) if err := c.Clone(config.Source, targetSourceDir); err != nil { @@ -38,7 +39,7 @@ func (c *Clone) Download(config *api.Config) (*api.SourceInfo, error) { } if len(config.ContextDir) > 0 { - originalTargetDir := filepath.Join(config.WorkingDir, "upload", "src") + originalTargetDir := filepath.Join(config.WorkingDir, api.Source) c.RemoveDirectory(originalTargetDir) // we want to copy entire dir contents, thus we need to use dir/. construct path := filepath.Join(targetSourceDir, config.ContextDir) + string(filepath.Separator) + "." diff --git a/pkg/git/git.go b/pkg/git/git.go index bf1922ace..50123bbdb 100644 --- a/pkg/git/git.go +++ b/pkg/git/git.go @@ -66,20 +66,20 @@ func (h *stiGit) ValidCloneSpec(source string) bool { // Clone clones a git repository to a specific target directory func (h *stiGit) Clone(source, target string) error { - outReader, outWriter := io.Pipe() - errReader, errWriter := io.Pipe() - defer func() { - outReader.Close() - outWriter.Close() - errReader.Close() - errWriter.Close() - }() + + // NOTE, we don NOT pass in both stdout and stderr, because + // with running with --quiet, and no output heading to stdout, hangs were occurring with the coordination + // of underlying channel management in the Go layer when dealing with the Go Cmd wrapper around + // git, sending of stdout/stderr to the Pipes created here, and the glog routines sent to pipeToLog + // + // It was agreed that we wanted to keep --quiet and no stdout output ....leaving stderr only since + // --quiet does not surpress that anyway reduced the frequency of the hang, but it still occurred. + // the pipeToLog method has been left for now for historical purposes, but if this implemenetation + // of git clone holds, we'll want to delete that at some point. + opts := util.CommandOpts{ - Stdout: outWriter, - Stderr: errWriter, } - go pipeToLog(outReader, glog.Info) - go pipeToLog(errReader, glog.Error) + return h.runner.RunWithOptions(opts, "git", "clone", "--quiet", "--recursive", source, target) } diff --git a/pkg/ignore/ignore.go b/pkg/ignore/ignore.go new file mode 100644 index 000000000..22bd1352b --- /dev/null +++ b/pkg/ignore/ignore.go @@ -0,0 +1,128 @@ +package ignore + +import ( + "bufio" + "io" + "os" + "path/filepath" + "strings" + + "github.com/golang/glog" + "github.com/openshift/source-to-image/pkg/api" +) + +type DockerIgnorer struct {} + +func (b *DockerIgnorer) Ignore(config *api.Config) error { + // so, to duplicate the .dockerignore capabilities (https://docs.docker.com/reference/builder/#dockerignore-file) + // we have a flow that follows: + /* + + 0) First note, .dockerignore rules are NOT recursive (unlike .gitignore) .. you have to list subdir explicitly + 1) Read in the exclusion patterns + 2) Skip over comments (noted by #) + 3) note overrides (via exclamation sign i.e. !) and reinstate files (don't remove) as needed + 4) leverage Glob matching to build list, as .dockerignore is documented as following filepath.Match / filepath.Glob + 5) del files + + 1 to 4 is in getListOfFilesToIgnore + + */ + + filesToDel, lerr := getListOfFilesToIgnore(config) + if lerr != nil { + return lerr + } + + if filesToDel == nil { + return nil + } + + // delete compiled list of files + for _, fileToDel := range filesToDel { + glog.V(5).Infof("attempting to remove file %s \n", fileToDel) + rerr := os.RemoveAll(fileToDel) + if rerr != nil { + glog.Errorf("error removing file %s because of %v \n", fileToDel, rerr) + return rerr + } + } + + return nil +} + +func getListOfFilesToIgnore(config *api.Config) (map[string]string, error) { + path := filepath.Join(config.WorkingSourceDir, api.IgnoreFile) + file, err := os.Open(path) + if err != nil { + if !os.IsNotExist(err) { + glog.Errorf("Ignore processing, problem opening %s because of %v\n", path, err) + return nil, err + } + glog.V(4).Info(".s2iignore file does not exist") + return nil, nil + } + defer file.Close() + + filesToDel := make(map[string]string) + scanner := bufio.NewScanner(file) + for scanner.Scan() { + filespec := strings.Trim(scanner.Text(), " ") + + if strings.HasPrefix(filespec, "#") { + continue + } + + glog.V(4).Infof(".s2iignore lists a file spec of %s \n", filespec) + + if strings.HasPrefix(filespec, "!") { + //remove any existing files to del that the override covers + // and patterns later on that undo this take precedence + + // first, remove ! ... note, replace ! with */ did not have + // expected effect with filepath.Match + filespec = strings.Replace(filespec, "!", "", 1) + + // iterate through and determine ones to leave in + dontDel := make([]string, 0) + for candidate, _ := range filesToDel { + compare := filepath.Join(config.WorkingSourceDir, filespec) + glog.V(5).Infof("For %s and %s see if it matches the spec %s which means that we leave in\n", filespec, candidate, compare) + leaveIn, _ := filepath.Match(compare, candidate) + if leaveIn { + glog.V(5).Infof("Not removing %s \n", candidate) + dontDel = append(dontDel, candidate) + } else { + glog.V(5).Infof("No match for %s and %s \n", filespec, candidate) + } + } + + // now remove any matches from files to delete list + for _, leaveIn := range dontDel { + delete(filesToDel, leaveIn) + } + continue + } + + globspec := filepath.Join(config.WorkingSourceDir, filespec) + glog.V(4).Infof("using globspec %s \n", globspec) + list, gerr := filepath.Glob(globspec) + if gerr != nil { + glog.V(4).Infof("Glob failed with %v \n", gerr) + } else { + for _, globresult := range list { + glog.V(5).Infof("Glob result %s \n", globresult) + filesToDel[globresult] = globresult + + } + } + + } + + if err := scanner.Err(); err != nil && err != io.EOF { + glog.Errorf("Problem processing .s2iignore %v \n", err) + return nil, err + } + + return filesToDel, nil +} diff --git a/pkg/ignore/ignore_test.go b/pkg/ignore/ignore_test.go new file mode 100644 index 000000000..44d2ff540 --- /dev/null +++ b/pkg/ignore/ignore_test.go @@ -0,0 +1,224 @@ +package ignore + +import ( + "path/filepath" + "strings" + "testing" + + "github.com/openshift/source-to-image/pkg/api" + "github.com/openshift/source-to-image/pkg/util" + "github.com/golang/glog" + "os" +) + +func getLogLevel() (level int) { + for level = 5; level >= 0; level-- { + if glog.V(glog.Level(level)) == true { + break + } + } + return +} + + +func baseTest(t *testing.T, patterns []string, filesToDel []string, filesToKeep []string) { + + // create working dir + workingDir, werr := util.NewFileSystem().CreateWorkingDirectory() + if werr != nil { + t.Errorf("problem allocating working dir %v \n", werr) + } else { + t.Logf("working directory is %s \n", workingDir) + } + defer func() { + // clean up test + cleanerr := os.RemoveAll(workingDir) + if cleanerr != nil { + t.Errorf("problem cleaning up %v \n", cleanerr) + } + }() + + + c := &api.Config{WorkingDir: workingDir} + + // create source repo dir for .s2iignore that matches where ignore.go looks + dpath := filepath.Join(c.WorkingDir,"upload","src") + derr := os.MkdirAll(dpath,0777) + if derr != nil { + t.Errorf("Problem creating source repo dir %s with %v \n", dpath, derr) + } + + c.WorkingSourceDir = dpath + t.Logf("working source dir %s \n", dpath) + + // create s2iignore file + ipath := filepath.Join(dpath, api.IgnoreFile) + ifile, ierr := os.Create(ipath) + defer ifile.Close() + if ierr != nil { + t.Errorf("Problem creating .s2iignore at %s with %v \n",ipath, ierr) + } + + // write patterns to remove into s2ignore, but save ! exclusions + filesToIgnore := make(map[string]string) + for _,pattern := range patterns { + t.Logf("storing pattern %s \n",pattern) + _, serr := ifile.WriteString(pattern) + + if serr != nil { + t.Errorf("Problem setting .s2iignore %v \n", serr) + } + if strings.HasPrefix(pattern, "!") { + pattern = strings.Replace(pattern, "!","",1) + t.Logf("Noting ignore pattern %s \n", pattern) + filesToIgnore[pattern] = pattern + } + } + + // create slices the store files to create, maps for files which should be deleted, files which should be kept + filesToCreate := make([]string, 0) + filesToDelCheck := make(map[string]string) + for _, fileToDel := range filesToDel { + filesToDelCheck[fileToDel] = fileToDel + filesToCreate = append(filesToCreate, fileToDel) + } + filesToKeepCheck := make(map[string]string) + for _,fileToKeep :=range filesToKeep { + filesToKeepCheck[fileToKeep] = fileToKeep + filesToCreate = append(filesToCreate, fileToKeep) + } + + + // create files for test + for _, fileToCreate := range filesToCreate { + fbpath := filepath.Join(dpath, fileToCreate) + + // ensure any subdirs off working dir exist + dirpath := filepath.Dir(fbpath) + derr := os.MkdirAll(dirpath,0777) + if derr != nil && !os.IsExist(derr) { + t.Errorf("Problem creating subdirs %s with %v \n", dirpath, derr) + } + t.Logf("Going to create file %s given supplied suffix %s \n", fbpath, fileToCreate) + fbfile, fberr := os.Create(fbpath) + defer fbfile.Close() + if fberr != nil { + t.Errorf("Problem creating test file %v \n", fberr) + } + } + + + // run ignorer algorithm + ignorer := &DockerIgnorer{} + ignorer.Ignore(c) + + // check if filesToDel, minus ignores, are gone, and filesToKeep are still there + for _,fileToCheck := range filesToCreate { + fbpath := filepath.Join(dpath, fileToCheck) + t.Logf("Evaluating file %s from dir %s and file to check %s \n", fbpath, dpath, fileToCheck) + + // see if file still exists or not + ofile, oerr := os.Open(fbpath) + defer ofile.Close() + var fileExists bool + if oerr == nil { + fileExists = true + t.Logf("The file %s exists after Ignore was run \n", fbpath) + } else { + if os.IsNotExist(oerr) { + t.Logf("The file %s does not exist after Ignore was run \n", fbpath) + fileExists = false + } else { + t.Errorf("Could not verify existence of %s because of %v \n", fbpath, oerr) + } + } + + _, iok := filesToIgnore[fileToCheck] + _, kok := filesToKeepCheck[fileToCheck] + _, dok := filesToDelCheck[fileToCheck] + + // if file present, verify it is in ignore or keep list, and not in del list + if fileExists { + if iok { + t.Logf("validated ignored file is still present %s \n ", fileToCheck) + continue + } + + if kok { + t.Logf("validated file to keep is still present %s \n", fileToCheck) + continue + } + + if dok { + t.Errorf("file which was cited to be deleted by caller to runTest exists %s \n", fileToCheck) + continue + } + + // if here, something unexpected + t.Errorf("file not in ignore / keep / del list !?!?!?!? %s \n", fileToCheck) + + } else { + if dok { + t.Logf("file which should have been deleted is in fact gone %s \n", fileToCheck) + continue + } + + if iok { + t.Errorf("file put into ignore list does not exist %s \n ", fileToCheck) + continue + } + + if kok { + t.Errorf("file passed in with keep list does not exist %s \n", fileToCheck) + continue + } + + // if here, then something unexpected happened + t.Errorf("file not in ignore / keep / del list !?!?!?!? %s \n", fileToCheck) + + } + + } + +} + +func TestSingleIgnore(t *testing.T) { + baseTest(t, []string{"foo.bar\n"}, []string{"foo.bar"}, []string{}) +} + +func TestWildcardIgnore(t *testing.T) { + baseTest(t, []string{"foo.*\n"}, []string{"foo.a", "foo.b"}, []string{}) +} + +func TestExclusion(t *testing.T) { + baseTest(t, []string{"foo.*\n", "!foo.a"}, []string{"foo.b"}, []string{"foo.a"}) +} + +func TestSubDirs(t *testing.T) { + baseTest(t, []string{"*/foo.a\n"}, []string{"asdf/foo.a"}, []string{"foo.a"}) +} + +func TestBasicDelKeepMix(t *testing.T) { + baseTest(t, []string{"foo.bar\n"}, []string{"foo.bar"}, []string{"bar.foo"}) +} + +/* +Per the docker user guide, with a docker ignore list of: + + LICENCSE.* + !LICENCSE.md + *.md + +LICENSE.MD will NOT be kept, as *.md overrides !LICENCSE.md +*/ +func TestExcludeOverride(t *testing.T) { + baseTest(t, []string{"LICENCSE.*\n", "!LICENCSE.md\n", "*.md"}, []string{"LICENCSE.foo", "LICENCSE.md"}, []string{"foo.bar"}) +} + +func TestExclusionWithWildcard(t *testing.T) { + baseTest(t, []string{"*.bar\n", "!foo.*"}, []string{"boo.bar", "bar.bar"}, []string{"foo.bar"}) +} + +func TestHopelessExclusion(t *testing.T) { + baseTest(t, []string{"!LICENSE.md\n", "LICENSE.*"}, []string{"LICENSE.md"}, []string{}) +} From 47bb7fb5e0113685c097f4de5663fee588b04eb0 Mon Sep 17 00:00:00 2001 From: gabemontero Date: Wed, 29 Jul 2015 14:18:30 -0400 Subject: [PATCH 2/2] issue197: fix gofmt complaints found by verify-gofmt.sh --- pkg/api/scripts.go | 4 +-- pkg/build/strategies/onbuild/onbuild.go | 2 +- pkg/git/git.go | 5 ++- pkg/ignore/ignore.go | 20 +++++------ pkg/ignore/ignore_test.go | 48 ++++++++++++------------- 5 files changed, 35 insertions(+), 44 deletions(-) diff --git a/pkg/api/scripts.go b/pkg/api/scripts.go index ece54ead0..d30061255 100644 --- a/pkg/api/scripts.go +++ b/pkg/api/scripts.go @@ -29,7 +29,7 @@ const ( SourceScripts = "upload" + string(os.PathSeparator) + "src" + string(os.PathSeparator) + ".sti" + string(os.PathSeparator) + "bin" // UploadScripts is the location of scripts that will be uploaded to the image during STI build. - UploadScripts = "upload" + string(os.PathSeparator) + "scripts" + UploadScripts = "upload" + string(os.PathSeparator) + "scripts" // Source is the location of application sources. Source = "upload" + string(os.PathSeparator) + "src" @@ -37,5 +37,5 @@ const ( ContextTmp = "upload" + string(os.PathSeparator) + "tmp" // Ignorefile is the s2i version for ignore files like we see with .gitignore or .dockerignore .. initial impl mirrors documented .dockerignore capabilities - IgnoreFile = ".s2iignore" + IgnoreFile = ".s2iignore" ) diff --git a/pkg/build/strategies/onbuild/onbuild.go b/pkg/build/strategies/onbuild/onbuild.go index f27906fcc..8ecf18d03 100644 --- a/pkg/build/strategies/onbuild/onbuild.go +++ b/pkg/build/strategies/onbuild/onbuild.go @@ -10,10 +10,10 @@ import ( "github.com/golang/glog" "github.com/openshift/source-to-image/pkg/api" "github.com/openshift/source-to-image/pkg/build" - "github.com/openshift/source-to-image/pkg/ignore" "github.com/openshift/source-to-image/pkg/build/strategies/sti" "github.com/openshift/source-to-image/pkg/docker" "github.com/openshift/source-to-image/pkg/git" + "github.com/openshift/source-to-image/pkg/ignore" "github.com/openshift/source-to-image/pkg/scripts" "github.com/openshift/source-to-image/pkg/tar" "github.com/openshift/source-to-image/pkg/util" diff --git a/pkg/git/git.go b/pkg/git/git.go index 50123bbdb..b1f71eb38 100644 --- a/pkg/git/git.go +++ b/pkg/git/git.go @@ -76,9 +76,8 @@ func (h *stiGit) Clone(source, target string) error { // --quiet does not surpress that anyway reduced the frequency of the hang, but it still occurred. // the pipeToLog method has been left for now for historical purposes, but if this implemenetation // of git clone holds, we'll want to delete that at some point. - - opts := util.CommandOpts{ - } + + opts := util.CommandOpts{} return h.runner.RunWithOptions(opts, "git", "clone", "--quiet", "--recursive", source, target) } diff --git a/pkg/ignore/ignore.go b/pkg/ignore/ignore.go index 22bd1352b..afaf586e0 100644 --- a/pkg/ignore/ignore.go +++ b/pkg/ignore/ignore.go @@ -11,24 +11,20 @@ import ( "github.com/openshift/source-to-image/pkg/api" ) -type DockerIgnorer struct {} +type DockerIgnorer struct{} func (b *DockerIgnorer) Ignore(config *api.Config) error { - // so, to duplicate the .dockerignore capabilities (https://docs.docker.com/reference/builder/#dockerignore-file) - // we have a flow that follows: /* - + so, to duplicate the .dockerignore capabilities (https://docs.docker.com/reference/builder/#dockerignore-file) + we have a flow that follows: 0) First note, .dockerignore rules are NOT recursive (unlike .gitignore) .. you have to list subdir explicitly 1) Read in the exclusion patterns 2) Skip over comments (noted by #) 3) note overrides (via exclamation sign i.e. !) and reinstate files (don't remove) as needed 4) leverage Glob matching to build list, as .dockerignore is documented as following filepath.Match / filepath.Glob 5) del files - - 1 to 4 is in getListOfFilesToIgnore - + 1 to 4 is in getListOfFilesToIgnore */ - filesToDel, lerr := getListOfFilesToIgnore(config) if lerr != nil { return lerr @@ -38,7 +34,7 @@ func (b *DockerIgnorer) Ignore(config *api.Config) error { return nil } - // delete compiled list of files + // delete compiled list of files for _, fileToDel := range filesToDel { glog.V(5).Infof("attempting to remove file %s \n", fileToDel) rerr := os.RemoveAll(fileToDel) @@ -72,9 +68,9 @@ func getListOfFilesToIgnore(config *api.Config) (map[string]string, error) { if strings.HasPrefix(filespec, "#") { continue } - + glog.V(4).Infof(".s2iignore lists a file spec of %s \n", filespec) - + if strings.HasPrefix(filespec, "!") { //remove any existing files to del that the override covers // and patterns later on that undo this take precedence @@ -85,7 +81,7 @@ func getListOfFilesToIgnore(config *api.Config) (map[string]string, error) { // iterate through and determine ones to leave in dontDel := make([]string, 0) - for candidate, _ := range filesToDel { + for candidate := range filesToDel { compare := filepath.Join(config.WorkingSourceDir, filespec) glog.V(5).Infof("For %s and %s see if it matches the spec %s which means that we leave in\n", filespec, candidate, compare) leaveIn, _ := filepath.Match(compare, candidate) diff --git a/pkg/ignore/ignore_test.go b/pkg/ignore/ignore_test.go index 44d2ff540..d6990d250 100644 --- a/pkg/ignore/ignore_test.go +++ b/pkg/ignore/ignore_test.go @@ -5,9 +5,9 @@ import ( "strings" "testing" + "github.com/golang/glog" "github.com/openshift/source-to-image/pkg/api" "github.com/openshift/source-to-image/pkg/util" - "github.com/golang/glog" "os" ) @@ -20,7 +20,6 @@ func getLogLevel() (level int) { return } - func baseTest(t *testing.T, patterns []string, filesToDel []string, filesToKeep []string) { // create working dir @@ -38,16 +37,15 @@ func baseTest(t *testing.T, patterns []string, filesToDel []string, filesToKeep } }() - c := &api.Config{WorkingDir: workingDir} - + // create source repo dir for .s2iignore that matches where ignore.go looks - dpath := filepath.Join(c.WorkingDir,"upload","src") - derr := os.MkdirAll(dpath,0777) + dpath := filepath.Join(c.WorkingDir, "upload", "src") + derr := os.MkdirAll(dpath, 0777) if derr != nil { t.Errorf("Problem creating source repo dir %s with %v \n", dpath, derr) } - + c.WorkingSourceDir = dpath t.Logf("working source dir %s \n", dpath) @@ -56,20 +54,20 @@ func baseTest(t *testing.T, patterns []string, filesToDel []string, filesToKeep ifile, ierr := os.Create(ipath) defer ifile.Close() if ierr != nil { - t.Errorf("Problem creating .s2iignore at %s with %v \n",ipath, ierr) + t.Errorf("Problem creating .s2iignore at %s with %v \n", ipath, ierr) } - + // write patterns to remove into s2ignore, but save ! exclusions filesToIgnore := make(map[string]string) - for _,pattern := range patterns { - t.Logf("storing pattern %s \n",pattern) + for _, pattern := range patterns { + t.Logf("storing pattern %s \n", pattern) _, serr := ifile.WriteString(pattern) - + if serr != nil { t.Errorf("Problem setting .s2iignore %v \n", serr) } if strings.HasPrefix(pattern, "!") { - pattern = strings.Replace(pattern, "!","",1) + pattern = strings.Replace(pattern, "!", "", 1) t.Logf("Noting ignore pattern %s \n", pattern) filesToIgnore[pattern] = pattern } @@ -83,19 +81,18 @@ func baseTest(t *testing.T, patterns []string, filesToDel []string, filesToKeep filesToCreate = append(filesToCreate, fileToDel) } filesToKeepCheck := make(map[string]string) - for _,fileToKeep :=range filesToKeep { + for _, fileToKeep := range filesToKeep { filesToKeepCheck[fileToKeep] = fileToKeep filesToCreate = append(filesToCreate, fileToKeep) } - // create files for test - for _, fileToCreate := range filesToCreate { + for _, fileToCreate := range filesToCreate { fbpath := filepath.Join(dpath, fileToCreate) // ensure any subdirs off working dir exist dirpath := filepath.Dir(fbpath) - derr := os.MkdirAll(dirpath,0777) + derr := os.MkdirAll(dirpath, 0777) if derr != nil && !os.IsExist(derr) { t.Errorf("Problem creating subdirs %s with %v \n", dirpath, derr) } @@ -105,15 +102,14 @@ func baseTest(t *testing.T, patterns []string, filesToDel []string, filesToKeep if fberr != nil { t.Errorf("Problem creating test file %v \n", fberr) } - } - + } // run ignorer algorithm - ignorer := &DockerIgnorer{} + ignorer := &DockerIgnorer{} ignorer.Ignore(c) // check if filesToDel, minus ignores, are gone, and filesToKeep are still there - for _,fileToCheck := range filesToCreate { + for _, fileToCheck := range filesToCreate { fbpath := filepath.Join(dpath, fileToCheck) t.Logf("Evaluating file %s from dir %s and file to check %s \n", fbpath, dpath, fileToCheck) @@ -153,10 +149,10 @@ func baseTest(t *testing.T, patterns []string, filesToDel []string, filesToKeep t.Errorf("file which was cited to be deleted by caller to runTest exists %s \n", fileToCheck) continue } - - // if here, something unexpected + + // if here, something unexpected t.Errorf("file not in ignore / keep / del list !?!?!?!? %s \n", fileToCheck) - + } else { if dok { t.Logf("file which should have been deleted is in fact gone %s \n", fileToCheck) @@ -175,9 +171,9 @@ func baseTest(t *testing.T, patterns []string, filesToDel []string, filesToKeep // if here, then something unexpected happened t.Errorf("file not in ignore / keep / del list !?!?!?!? %s \n", fileToCheck) - + } - + } }