Throw an error if a scripts URL is specified but no scripts can be downloaded from it

pkg/api/types.go

@@ -261,6 +261,10 @@ type InstallResult struct {
 
 	// Error describes last error encountered during install operation
 	Error error
+
+	// FailedSources is a list of sources that were attempted but failed
+	// when downloading this script
+	FailedSources []string
 }
 
 // SourceInfo stores information about the source code

pkg/build/strategies/sti/sti.go

@@ -218,6 +218,24 @@ func (b *STI) Prepare(config *api.Config) error {
 	}
 	optional := b.installer.InstallOptional(b.optionalScripts, config.WorkingDir)
 
+	// If a ScriptsURL was specified, but no scripts were downloaded from it, throw an error
+	if len(config.ScriptsURL) > 0 {
+		failedCount := 0
+		for _, result := range required {
+			if includes(result.FailedSources, scripts.ScriptURLHandler) {
+				failedCount++
+			}
+		}
+		for _, result := range optional {
+			if includes(result.FailedSources, scripts.ScriptURLHandler) {
+				failedCount++
+			}
+		}
+		if failedCount == len(required)+len(optional) {
+			return fmt.Errorf("Could not download any scripts from URL %v", config.ScriptsURL)
+		}
+	}
+
 	for _, r := range append(required, optional...) {
 		if r.Error == nil {
 			b.externalScripts[r.Script] = r.Downloaded
@@ -594,3 +612,12 @@ func isMissingRequirements(text string) bool {
 	sh, _ := regexp.MatchString(`.*/bin/sh.*no such file or directory`, text)
 	return tar || sh
 }
+
+func includes(arr []string, str string) bool {
+	for _, s := range arr {
+		if s == str {
+			return true
+		}
+	}
+	return false
+}

pkg/scripts/install.go

@@ -29,29 +29,41 @@ type ScriptHandler interface {
 	String() string
 }
 
-// UrlScriptHandler handles script download using URL.
-type UrlScriptHandler struct {
+// URLScriptHandler handles script download using URL.
+type URLScriptHandler struct {
 	URL            string
 	DestinationDir string
 	download       Downloader
 	fs             util.FileSystem
+	name           string
 }
 
-const sourcesRootAbbrev = "<source-dir>/"
+const (
+	sourcesRootAbbrev = "<source-dir>/"
+
+	// ScriptURLHandler is the name of the script URL handler
+	ScriptURLHandler = "script URL handler"
+
+	// ImageURLHandler is the name of the image URL handler
+	ImageURLHandler = "image URL handler"
+
+	// SourceHandler is the name of the source script handler
+	SourceHandler = "source handler"
+)
 
 // SetDestinationDir sets the destination where the scripts should be
 // downloaded.
-func (s *UrlScriptHandler) SetDestinationDir(baseDir string) {
+func (s *URLScriptHandler) SetDestinationDir(baseDir string) {
 	s.DestinationDir = filepath.Join(baseDir, api.UploadScripts)
 }
 
 // String implements the String() function.
-func (s *UrlScriptHandler) String() string {
-	return "url handler"
+func (s *URLScriptHandler) String() string {
+	return s.name
 }
 
 // Get parses the provided URL and the script name.
-func (s *UrlScriptHandler) Get(script string) *api.InstallResult {
+func (s *URLScriptHandler) Get(script string) *api.InstallResult {
 	if len(s.URL) == 0 {
 		return nil
 	}
@@ -67,7 +79,7 @@ func (s *UrlScriptHandler) Get(script string) *api.InstallResult {
 }
 
 // Install downloads the script and fix its permissions.
-func (s *UrlScriptHandler) Install(r *api.InstallResult) error {
+func (s *URLScriptHandler) Install(r *api.InstallResult) error {
 	downloadURL, err := url.Parse(r.URL)
 	if err != nil {
 		return err
@@ -116,7 +128,7 @@ func (s *SourceScriptHandler) Get(script string) *api.InstallResult {
 
 // String implements the String() function.
 func (s *SourceScriptHandler) String() string {
-	return "source handler"
+	return SourceHandler
 }
 
 // Install copies the script into upload directory and fix its permissions.
@@ -184,7 +196,7 @@ func NewInstaller(image string, scriptsURL string, proxyConfig *api.ProxyConfig,
 	// Order is important here, first we try to get the scripts from provided URL,
 	// then we look into sources and check for .s2i/bin scripts.
 	if len(m.ScriptsURL) > 0 {
-		m.Add(&UrlScriptHandler{URL: m.ScriptsURL, download: m.download, fs: m.fs})
+		m.Add(&URLScriptHandler{URL: m.ScriptsURL, download: m.download, fs: m.fs, name: ScriptURLHandler})
 	}
 
 	m.Add(&SourceScriptHandler{fs: m.fs})
@@ -193,7 +205,7 @@ func NewInstaller(image string, scriptsURL string, proxyConfig *api.ProxyConfig,
 	// docker image itself.
 	defaultURL, err := m.docker.GetScriptsURL(m.Image)
 	if err == nil && defaultURL != "" {
-		m.Add(&UrlScriptHandler{URL: defaultURL, download: m.download, fs: m.fs})
+		m.Add(&URLScriptHandler{URL: defaultURL, download: m.download, fs: m.fs, name: ImageURLHandler})
 	}
 	return &m
 }
@@ -222,14 +234,17 @@ func (i *DefaultScriptSourceManager) InstallOptional(scripts []string, dstDir st
 	result := []api.InstallResult{}
 	for _, script := range scripts {
 		installed := false
+		failedSources := []string{}
 		for _, e := range i.sources {
 			detected := false
 			h := e.(ScriptHandler)
 			h.SetDestinationDir(dstDir)
 			if r := h.Get(script); r != nil {
 				if err := h.Install(r); err != nil {
+					failedSources = append(failedSources, h.String())
 					glog.Errorf("script %q found by the %s, but failed to install: %v", script, h, err)
 				} else {
+					r.FailedSources = failedSources
 					result = append(result, *r)
 					installed = true
 					detected = true
@@ -242,8 +257,9 @@ func (i *DefaultScriptSourceManager) InstallOptional(scripts []string, dstDir st
 		}
 		if !installed {
 			result = append(result, api.InstallResult{
-				Script: script,
-				Error:  fmt.Errorf("script %q not installed", script),
+				FailedSources: failedSources,
+				Script:        script,
+				Error:         fmt.Errorf("script %q not installed", script),
 			})
 		}
 	}

pkg/scripts/install_test.go

@@ -3,6 +3,7 @@ package scripts
 import (
 	"fmt"
 	"path/filepath"
+	"reflect"
 	"strings"
 	"testing"
 
@@ -37,11 +38,11 @@ func newFakeInstaller(config *fakeScriptManagerConfig) Installer {
 		fs:         config.fs,
 		download:   config.download,
 	}
-	m.Add(&UrlScriptHandler{URL: m.ScriptsURL, download: m.download, fs: m.fs})
+	m.Add(&URLScriptHandler{URL: m.ScriptsURL, download: m.download, fs: m.fs, name: ScriptURLHandler})
 	m.Add(&SourceScriptHandler{fs: m.fs})
 	defaultURL, err := m.docker.GetScriptsURL(m.Image)
 	if err == nil && defaultURL != "" {
-		m.Add(&UrlScriptHandler{URL: defaultURL, download: m.download, fs: m.fs})
+		m.Add(&URLScriptHandler{URL: defaultURL, download: m.download, fs: m.fs, name: ImageURLHandler})
 	}
 	return &m
 }
@@ -264,14 +265,14 @@ func TestNewInstaller(t *testing.T) {
 	docker := &dockerpkg.FakeDocker{DefaultURLResult: "image://docker"}
 	inst := NewInstaller("test-image", "http://foo.bar", nil, docker, dockerClient.AuthConfiguration{})
 	sources := inst.(*DefaultScriptSourceManager).sources
-	firstHandler, ok := sources[0].(*UrlScriptHandler)
+	firstHandler, ok := sources[0].(*URLScriptHandler)
 	if !ok {
 		t.Errorf("expected first handler to be script url handler, got %#v", inst.(*DefaultScriptSourceManager).sources)
 	}
 	if firstHandler.URL != "http://foo.bar" {
 		t.Errorf("expected first handler to handle the script url, got %+v", firstHandler)
 	}
-	lastHandler, ok := sources[len(sources)-1].(*UrlScriptHandler)
+	lastHandler, ok := sources[len(sources)-1].(*URLScriptHandler)
 	if !ok {
 		t.Errorf("expected last handler to be docker url handler, got %#v", inst.(*DefaultScriptSourceManager).sources)
 	}
@@ -279,3 +280,45 @@ func TestNewInstaller(t *testing.T) {
 		t.Errorf("expected last handler to handle the docker default url, got %+v", lastHandler)
 	}
 }
+
+type fakeSource struct {
+	name   string
+	failOn map[string]struct{}
+}
+
+func (f *fakeSource) Get(script string) *api.InstallResult {
+	return &api.InstallResult{Script: script}
+}
+
+func (f *fakeSource) Install(r *api.InstallResult) error {
+	if _, fail := f.failOn[r.Script]; fail {
+		return fmt.Errorf("error")
+	}
+	return nil
+}
+
+func (f *fakeSource) SetDestinationDir(string) {}
+
+func (f *fakeSource) String() string {
+	return f.name
+}
+
+func TestInstallOptionalFailedSources(t *testing.T) {
+
+	m := DefaultScriptSourceManager{}
+	m.Add(&fakeSource{name: "failing1", failOn: map[string]struct{}{"one": {}, "two": {}, "three": {}}})
+	m.Add(&fakeSource{name: "failing2", failOn: map[string]struct{}{"one": {}, "two": {}, "three": {}}})
+	m.Add(&fakeSource{name: "almostpassing", failOn: map[string]struct{}{"three": {}}})
+
+	expect := map[string][]string{
+		"one":   {"failing1", "failing2"},
+		"two":   {"failing1", "failing2"},
+		"three": {"failing1", "failing2", "almostpassing"},
+	}
+	results := m.InstallOptional([]string{"one", "two", "three"}, "foo")
+	for _, result := range results {
+		if !reflect.DeepEqual(result.FailedSources, expect[result.Script]) {
+			t.Errorf("Did not get expected failed sources: %#v", result)
+		}
+	}
+}

test/integration/integration_test.go

@@ -58,7 +58,7 @@ const (
 	// scripts are made available for integration testing.
 	//
 	// Port 23456 must match the port used in the fake image Dockerfiles
-	FakeScriptsHttpURL = "http://127.0.0.1:23456/sti-fake/.s2i/bin"
+	FakeScriptsHttpURL = "http://127.0.0.1:23456/.s2i/bin"
 )
 
 // TestInjectionBuild tests the build where we inject files to assemble script.