From 8ecf8180b30745ed04adf5cb203ca8111aeb345e Mon Sep 17 00:00:00 2001
From: gabemontero <gmontero@redhat.com>
Date: Fri, 29 Jan 2016 14:52:35 -0500
Subject: [PATCH] handle build image names ending in hex image ids

---
 pkg/build/strategies/layered/layered.go      | 22 +++++++-
 pkg/build/strategies/layered/layered_test.go | 57 ++++++++++++++++++++
 2 files changed, 78 insertions(+), 1 deletion(-)

diff --git a/pkg/build/strategies/layered/layered.go b/pkg/build/strategies/layered/layered.go
index 06ff07ead..5073b30ff 100644
--- a/pkg/build/strategies/layered/layered.go
+++ b/pkg/build/strategies/layered/layered.go
@@ -134,7 +134,27 @@ func (b *Layered) Build(config *api.Config) (*api.Result, error) {
 	}
 	defer tarStream.Close()
 
-	newBuilderImage := fmt.Sprintf("%s-%d", b.config.BuilderImage, time.Now().UnixNano())
+	dockerImageReference, err := docker.ParseDockerImageReference(b.config.BuilderImage)
+	if err != nil {
+		return nil, err
+	}
+	// if we fall down this path via oc new-app, the builder image will be a docker image ref ending
+	// with a @<hex image id> instead of a tag; simply appending the time stamp to the end of a
+	// hex image id ref is not kosher with the docker API; so we remove the ID piece, and then
+	// construct the new image name
+	var newBuilderImage string
+	if len(dockerImageReference.ID) == 0 {
+		newBuilderImage = fmt.Sprintf("%s-%d", b.config.BuilderImage, time.Now().UnixNano())
+	} else {
+		if len(dockerImageReference.Registry) > 0 {
+			newBuilderImage = fmt.Sprintf("%s/", dockerImageReference.Registry)
+		}
+		if len(dockerImageReference.Namespace) > 0 {
+			newBuilderImage = fmt.Sprintf("%s%s/", newBuilderImage, dockerImageReference.Namespace)
+		}
+		newBuilderImage = fmt.Sprintf("%s%s:s2i-layered-%d", newBuilderImage, dockerImageReference.Name, time.Now().UnixNano())
+	}
+
 	outReader, outWriter := io.Pipe()
 	defer outReader.Close()
 	defer outWriter.Close()
diff --git a/pkg/build/strategies/layered/layered_test.go b/pkg/build/strategies/layered/layered_test.go
index 59ab909be..4040c869e 100644
--- a/pkg/build/strategies/layered/layered_test.go
+++ b/pkg/build/strategies/layered/layered_test.go
@@ -6,6 +6,7 @@ import (
 	"os"
 	"path/filepath"
 	"regexp"
+	"strings"
 	"testing"
 
 	"github.com/openshift/source-to-image/pkg/api"
@@ -72,6 +73,53 @@ func TestBuildOK(t *testing.T) {
 	}
 }
 
+func TestBuildOKWithImageRef(t *testing.T) {
+	workDir, _ := ioutil.TempDir("", "sti")
+	scriptDir := filepath.Join(workDir, api.UploadScripts)
+	err := os.MkdirAll(scriptDir, 0700)
+	assemble := filepath.Join(scriptDir, api.Assemble)
+	file, err := os.Create(assemble)
+	if err != nil {
+		t.Errorf("Unexpected error returned: %v", err)
+	}
+	defer file.Close()
+	defer os.RemoveAll(workDir)
+	l := newFakeLayeredWithScripts(assemble, workDir)
+	l.config.BuilderImage = "docker.io/uptoknow/ruby-20-centos7@sha256:d6f5718b85126954d98931e654483ee794ac357e0a98f4a680c1e848d78863a1"
+	_, err = l.Build(l.config)
+	if err != nil {
+		t.Errorf("Unexpected error returned: %v", err)
+	}
+	if !l.config.LayeredBuild {
+		t.Errorf("Expected LayeredBuild to be true!")
+	}
+	if !strings.HasPrefix(l.config.BuilderImage, "docker.io/uptoknow/ruby-20-centos7:s2i-layered-") {
+		t.Errorf("Expected BuilderImage to start with docker.io/uptoknow/ruby-20-centos7:s2i-layered-, but got %s", l.config.BuilderImage)
+	}
+	l.config.BuilderImage = "uptoknow/ruby-20-centos7@sha256:d6f5718b85126954d98931e654483ee794ac357e0a98f4a680c1e848d78863a1"
+	_, err = l.Build(l.config)
+	if err != nil {
+		t.Errorf("Unexpected error returned: %v", err)
+	}
+	if !l.config.LayeredBuild {
+		t.Errorf("Expected LayeredBuild to be true!")
+	}
+	if !strings.HasPrefix(l.config.BuilderImage, "uptoknow/ruby-20-centos7:s2i-layered-") {
+		t.Errorf("Expected BuilderImage to start with uptoknow/ruby-20-centos7:s2i-layered-, but got %s", l.config.BuilderImage)
+	}
+	l.config.BuilderImage = "ruby-20-centos7@sha256:d6f5718b85126954d98931e654483ee794ac357e0a98f4a680c1e848d78863a1"
+	_, err = l.Build(l.config)
+	if err != nil {
+		t.Errorf("Unexpected error returned: %v", err)
+	}
+	if !l.config.LayeredBuild {
+		t.Errorf("Expected LayeredBuild to be true!")
+	}
+	if !strings.HasPrefix(l.config.BuilderImage, "ruby-20-centos7:s2i-layered-") {
+		t.Errorf("Expected BuilderImage to start with /ruby-20-centos7:s2i-layered-, but got %s", l.config.BuilderImage)
+	}
+}
+
 func TestBuildNoScriptsProvided(t *testing.T) {
 	l := newFakeLayered()
 	l.config.BuilderImage = "test/image"
@@ -119,9 +167,18 @@ func TestBuildErrorOpenTarFile(t *testing.T) {
 
 func TestBuildErrorBuildImage(t *testing.T) {
 	l := newFakeLayered()
+	l.config.BuilderImage = "test/image"
 	l.docker.(*docker.FakeDocker).BuildImageError = errors.New("BuildImageError")
 	_, err := l.Build(l.config)
 	if err == nil || err.Error() != "BuildImageError" {
 		t.Errorf("An error was expected for BuildImage, but got different: %v", err)
 	}
 }
+
+func TestBuildErrorBadImageName(t *testing.T) {
+	l := newFakeLayered()
+	_, err := l.Build(l.config)
+	if err == nil || !strings.Contains(err.Error(), "must be two or three segments separated by slashes") {
+		t.Errorf("An docker spec parse error was expected, but got different: %v", err)
+	}
+}