diff --git a/pkg/build/strategies/dockerfile/dockerfile.go b/pkg/build/strategies/dockerfile/dockerfile.go index d8e66009b..a25ac42ab 100644 --- a/pkg/build/strategies/dockerfile/dockerfile.go +++ b/pkg/build/strategies/dockerfile/dockerfile.go @@ -127,11 +127,7 @@ func (builder *Dockerfile) CreateDockerfile(config *api.Config) error { artifactsDestDir := filepath.Join(getDestination(config), "artifacts") artifactsTar := sanitize(filepath.ToSlash(filepath.Join(defaultDestination, "artifacts.tar"))) // hasAllScripts indicates that we blindly trust all scripts are provided in the image scripts dir - imageScriptsDir, hasAllScripts := getImageScriptsDir(config) - var providedScripts map[string]bool - if !hasAllScripts { - providedScripts = scanScripts(filepath.Join(config.WorkingDir, builder.uploadScriptsDir)) - } + imageScriptsDir, providedScripts := getImageScriptsDir(config, builder) if config.Incremental { imageTag := util.FirstNonEmpty(config.IncrementalFromTag, config.Tag) @@ -422,20 +418,29 @@ func getDestination(config *api.Config) string { return destination } -// getImageScriptsDir returns the directory containing the builder image scripts and a bool -// indicating that the directory is expected to contain all s2i scripts -func getImageScriptsDir(config *api.Config) (string, bool) { +// getImageScriptsDir returns the default directory which should contain the builder image scripts +// as well as a map of booleans identifying individual scripts provided in the repository as overrides +func getImageScriptsDir(config *api.Config, builder *Dockerfile) (string, map[string]bool) { + + // 1st priority is the command line parameter (pointing to an image, overrides it all) if strings.HasPrefix(config.ScriptsURL, "image://") { - return strings.TrimPrefix(config.ScriptsURL, "image://"), true + return strings.TrimPrefix(config.ScriptsURL, "image://"), make(map[string]bool) } + + // 2nd priority (the source code repository), collect the locations + providedScripts := scanScripts(filepath.Join(config.WorkingDir, builder.uploadScriptsDir)) + + // 3rd priority (the builder image), collect the locations scriptsURL, _ := util.AdjustConfigWithImageLabels(config) if strings.HasPrefix(scriptsURL, "image://") { - return strings.TrimPrefix(scriptsURL, "image://"), true + return strings.TrimPrefix(scriptsURL, "image://"), providedScripts } if strings.HasPrefix(config.ImageScriptsURL, "image://") { - return strings.TrimPrefix(config.ImageScriptsURL, "image://"), false + return strings.TrimPrefix(config.ImageScriptsURL, "image://"), providedScripts } - return defaultScriptsDir, false + + // If all else fails, use the default scripts dir + return defaultScriptsDir, providedScripts } // scanScripts returns a map of provided s2i scripts diff --git a/pkg/build/strategies/dockerfile/dockerfile_test.go b/pkg/build/strategies/dockerfile/dockerfile_test.go index a56312e34..e9801ad8e 100644 --- a/pkg/build/strategies/dockerfile/dockerfile_test.go +++ b/pkg/build/strategies/dockerfile/dockerfile_test.go @@ -28,8 +28,7 @@ func TestGetImageScriptsDir(t *testing.T) { Config: &api.Config{ ScriptsURL: "image:///usr/some/dir", }, - ExpectedDir: "/usr/some/dir", - HasAllScripts: true, + ExpectedDir: "/usr/some/dir", }, { Config: &api.Config{ @@ -61,8 +60,7 @@ func TestGetImageScriptsDir(t *testing.T) { ScriptsURL: "image:///usr/some/dir", ImageScriptsURL: "image:///usr/other/dir", }, - ExpectedDir: "/usr/some/dir", - HasAllScripts: true, + ExpectedDir: "/usr/some/dir", }, { Config: &api.Config{ @@ -70,8 +68,7 @@ func TestGetImageScriptsDir(t *testing.T) { constants.ScriptsURLLabel: "image:///usr/some/dir", }, }, - ExpectedDir: "/usr/some/dir", - HasAllScripts: true, + ExpectedDir: "/usr/some/dir", }, { Config: &api.Config{ @@ -80,8 +77,7 @@ func TestGetImageScriptsDir(t *testing.T) { constants.DeprecatedScriptsURLLabel: "image:///usr/other/dir", }, }, - ExpectedDir: "/usr/some/dir", - HasAllScripts: true, + ExpectedDir: "/usr/some/dir", }, { Config: &api.Config{ @@ -89,18 +85,14 @@ func TestGetImageScriptsDir(t *testing.T) { constants.DeprecatedScriptsURLLabel: "image:///usr/some/dir", }, }, - ExpectedDir: "/usr/some/dir", - HasAllScripts: true, + ExpectedDir: "/usr/some/dir", }, } for _, tc := range cases { - output, hasScripts := getImageScriptsDir(tc.Config) + output, _ := getImageScriptsDir(tc.Config, &(Dockerfile{})) if output != tc.ExpectedDir { t.Errorf("Expected image scripts dir %s to be %s", output, tc.ExpectedDir) } - if hasScripts != tc.HasAllScripts { - t.Errorf("Expected has all scripts indicator:\n%v\nto be: %v", hasScripts, tc.HasAllScripts) - } } }