new file mode 100644

@@ -0,0 +1,32 @@

+package builder

+

+import "github.com/openshift/origin/pkg/build/api"

+

+// A KeyValue can be used to build ordered lists of key-value pairs.

+type KeyValue struct {

+	Key   string

+	Value string

+}

+

+// buildInfo returns a slice of KeyValue pairs with build metadata to be

+// inserted into Docker images produced by build.

+func buildInfo(build *api.Build) []KeyValue {

+	kv := []KeyValue{

+		{"OPENSHIFT_BUILD_NAME", build.Name},

+		{"OPENSHIFT_BUILD_NAMESPACE", build.Namespace},

+		{"OPENSHIFT_BUILD_SOURCE", build.Spec.Source.Git.URI},

+	}

+	if build.Spec.Source.Git.Ref != "" {

+		kv = append(kv, KeyValue{"OPENSHIFT_BUILD_REFERENCE", build.Spec.Source.Git.Ref})

+	}

+	if build.Spec.Revision != nil && build.Spec.Revision.Git != nil && build.Spec.Revision.Git.Commit != "" {

+		kv = append(kv, KeyValue{"OPENSHIFT_BUILD_COMMIT", build.Spec.Revision.Git.Commit})

+	}

+	if build.Spec.Strategy.Type == api.SourceBuildStrategyType {

+		env := build.Spec.Strategy.SourceStrategy.Env

+		for _, e := range env {

+			kv = append(kv, KeyValue{e.Name, e.Value})

+		}

+	}

+	return kv

+}

new file mode 100644

@@ -0,0 +1,52 @@

+package builder

+

+import (

+	"reflect"

+	"testing"

+

+	kapi "k8s.io/kubernetes/pkg/api"

+

+	"github.com/openshift/origin/pkg/build/api"

+)

+

+func TestBuildInfo(t *testing.T) {

+	b := &api.Build{

+		ObjectMeta: kapi.ObjectMeta{

+			Name:      "sample-app",

+			Namespace: "default",

+		},

+		Spec: api.BuildSpec{

+			Source: api.BuildSource{

+				Git: &api.GitBuildSource{

+					URI: "github.com/openshift/sample-app",

+					Ref: "master",

+				},

+			},

+			Strategy: api.BuildStrategy{

+				Type: api.SourceBuildStrategyType,

+				SourceStrategy: &api.SourceBuildStrategy{

+					Env: []kapi.EnvVar{

+						{Name: "RAILS_ENV", Value: "production"},

+					},

+				},

+			},

+			Revision: &api.SourceRevision{

+				Git: &api.GitSourceRevision{

+					Commit: "1575a90c569a7cc0eea84fbd3304d9df37c9f5ee",

+				},

+			},

+		},

+	}

+	got := buildInfo(b)

+	want := []KeyValue{

+		{"OPENSHIFT_BUILD_NAME", "sample-app"},

+		{"OPENSHIFT_BUILD_NAMESPACE", "default"},

+		{"OPENSHIFT_BUILD_SOURCE", "github.com/openshift/sample-app"},

+		{"OPENSHIFT_BUILD_REFERENCE", "master"},

+		{"OPENSHIFT_BUILD_COMMIT", "1575a90c569a7cc0eea84fbd3304d9df37c9f5ee"},

+		{"RAILS_ENV", "production"},

+	}

+	if !reflect.DeepEqual(got, want) {

+		t.Errorf("buildInfo(%+v) = %+v; want %+v", b, got, want)

+	}

+}

@@ -1,10 +1,7 @@

 package builder

 import (

-	"bytes"

-	"errors"

 	"fmt"

-	"io"

 	"io/ioutil"

 	"net"

 	"net/url"

@@ -19,13 +16,14 @@ import (

 	"github.com/golang/glog"

 	kapi "k8s.io/kubernetes/pkg/api"

-	"github.com/openshift/origin/pkg/build/api"

-	"github.com/openshift/origin/pkg/build/builder/cmd/dockercfg"

-	"github.com/openshift/origin/pkg/util/docker/dockerfile"

 	s2iapi "github.com/openshift/source-to-image/pkg/api"

 	"github.com/openshift/source-to-image/pkg/scm/git"

 	"github.com/openshift/source-to-image/pkg/tar"

 	"github.com/openshift/source-to-image/pkg/util"

+

+	"github.com/openshift/origin/pkg/build/api"

+	"github.com/openshift/origin/pkg/build/builder/cmd/dockercfg"

+	"github.com/openshift/origin/pkg/util/docker/dockerfile"

 )

 const (

@@ -48,17 +46,6 @@ type DockerBuilder struct {

 	urlTimeout   time.Duration

 }

-// MetaInstuction represent an Docker instruction used for adding metadata

-// to Dockerfile

-type MetaInstruction string

-

-const (

-	// Label represents the LABEL Docker instruction

-	Label MetaInstruction = "LABEL"

-	// Env represents the ENV Docker instruction

-	Env MetaInstruction = "ENV"

-)

-

 // NewDockerBuilder creates a new instance of DockerBuilder

 func NewDockerBuilder(dockerClient DockerClient, build *api.Build) *DockerBuilder {

 	return &DockerBuilder{

@@ -229,44 +216,38 @@ func (d *DockerBuilder) addBuildParameters(dir string) error {

 		dockerfilePath = filepath.Join(dir, d.build.Spec.Source.ContextDir, "Dockerfile")

 	}

-	fileStat, err := os.Lstat(dockerfilePath)

+	f, err := os.Open(dockerfilePath)

 	if err != nil {

 		return err

 	}

-	filePerm := fileStat.Mode()

-

-	fileData, err := ioutil.ReadFile(dockerfilePath)

+	// Parse the Dockerfile.

+	node, err := parser.Parse(f)

 	if err != nil {

 		return err

 	}

-	var newFileData string

+	// Update base image if build strategy specifies the From field.

 	if d.build.Spec.Strategy.DockerStrategy.From != nil && d.build.Spec.Strategy.DockerStrategy.From.Kind == "DockerImage" {

-		newFileData, err = replaceValidCmd(dockercmd.From, d.build.Spec.Strategy.DockerStrategy.From.Name, fileData)

+		err := replaceLastFrom(node, d.build.Spec.Strategy.DockerStrategy.From.Name)

 		if err != nil {

 			return err

 		}

-	} else {

-		newFileData = newFileData + string(fileData)

 	}

-	envVars := getBuildEnvVars(d.build)

-	newFileData = appendMetadata(Env, newFileData, envVars)

-

-	labels := map[string]string{}

-	sourceInfo := d.git.GetInfo(dir)

-	if len(d.build.Spec.Source.ContextDir) > 0 {

-		sourceInfo.ContextDir = d.build.Spec.Source.ContextDir

+	// Append build info as environment variables.

+	err = appendEnv(node, d.buildInfo())

+	if err != nil {

+		return err

 	}

-	labels = util.GenerateLabelsFromSourceInfo(labels, sourceInfo, api.DefaultDockerLabelNamespace)

-	newFileData = appendMetadata(Label, newFileData, labels)

-	node, err := parser.Parse(strings.NewReader(newFileData))

+	// Append build labels.

+	err = appendLabel(node, d.buildLabels(dir))

 	if err != nil {

 		return err

 	}

+	// Insert environment variables defined in the build strategy.

 	err = insertEnvAfterFrom(node, d.build.Spec.Strategy.DockerStrategy.Env)

 	if err != nil {

 		return err

@@ -274,156 +255,123 @@ func (d *DockerBuilder) addBuildParameters(dir string) error {

 	instructions := dockerfile.ParseTreeToDockerfile(node)

-	return ioutil.WriteFile(dockerfilePath, instructions, filePerm)

+	// Overwrite the Dockerfile.

+	fi, err := f.Stat()

+	if err != nil {

+		return err

+	}

+	return ioutil.WriteFile(dockerfilePath, instructions, fi.Mode())

 }

-// appendMetadata appends a Docker instruction that adds metadata values

-// to a string containing a valid Dockerfile.

-func appendMetadata(inst MetaInstruction, fileData string, envVars map[string]string) string {

-	if !strings.HasSuffix(fileData, "\n") {

-		fileData += "\n"

-	}

-	first := true

-	for k, v := range envVars {

-		if first {

-			fileData += fmt.Sprintf("%s %s=%+q", inst, k, v)

-			first = false

-		} else {

-			fileData += fmt.Sprintf(" \\\n\t%s=%+q", k, v)

-		}

+// buildInfo converts the buildInfo output to a format that appendEnv can

+// consume.

+func (d *DockerBuilder) buildInfo() []dockerfile.KeyValue {

+	bi := buildInfo(d.build)

+	kv := make([]dockerfile.KeyValue, len(bi))

+	for i, item := range bi {

+		kv[i] = dockerfile.KeyValue{Key: item.Key, Value: item.Value}

 	}

-	fileData += "\n"

-	return fileData

+	return kv

 }

-// invalidCmdErr represents an error returned from replaceValidCmd

-// when an invalid Dockerfile command has been passed to

-// replaceValidCmd

-var invalidCmdErr = errors.New("invalid Dockerfile command")

-

-// replaceCmdErr represents an error returned from replaceValidCmd

-// when a command which has more than one valid occurrences inside

-// a Dockerfile has been passed or the specified command cannot

-// be found

-var replaceCmdErr = errors.New("cannot replace given Dockerfile command")

-

-// replaceValidCmd replaces the valid occurrence of a command

-// in a Dockerfile with the given replaceArgs

-func replaceValidCmd(cmd, replaceArgs string, fileData []byte) (string, error) {

-	if _, ok := dockercmd.Commands[cmd]; !ok {

-		return "", invalidCmdErr

-	}

-	buf := bytes.NewBuffer(fileData)

-	// Parse with Docker parser

-	node, err := parser.Parse(buf)

-	if err != nil {

-		return "", errors.New("cannot parse Dockerfile: " + err.Error())

+// buildLabels returns a slice of KeyValue pairs in a format that appendEnv can

+// consume.

+func (d *DockerBuilder) buildLabels(dir string) []dockerfile.KeyValue {

+	labels := map[string]string{}

+	sourceInfo := d.git.GetInfo(dir)

+	if len(d.build.Spec.Source.ContextDir) > 0 {

+		sourceInfo.ContextDir = d.build.Spec.Source.ContextDir

 	}

+	labels = util.GenerateLabelsFromSourceInfo(labels, sourceInfo, api.DefaultDockerLabelNamespace)

+	kv := make([]dockerfile.KeyValue, len(labels))

+	i := 0

+	for k, v := range labels {

+		kv[i] = dockerfile.KeyValue{Key: k, Value: v}

+		i++

+	}

+	return kv

+}

-	pos := traverseAST(cmd, node)

-	if pos == 0 {

-		return "", replaceCmdErr

+// setupPullSecret provides a Docker authentication configuration when the

+// PullSecret is specified.

+func (d *DockerBuilder) setupPullSecret() (*docker.AuthConfigurations, error) {

+	if len(os.Getenv(dockercfg.PullAuthType)) == 0 {

+		return nil, nil

 	}

+	r, err := os.Open(os.Getenv(dockercfg.PullAuthType))

+	if err != nil {

+		return nil, fmt.Errorf("'%s': %s", os.Getenv(dockercfg.PullAuthType), err)

+	}

+	return docker.NewAuthConfigurations(r)

+}

-	// Re-initialize the buffer

-	buf = bytes.NewBuffer(fileData)

-	var newFileData string

-	var index int

-	var replaceNextLn bool

-	for {

-		line, err := buf.ReadString('\n')

-		if err != nil && err != io.EOF {

-			return "", err

+// dockerBuild performs a docker build on the source that has been retrieved

+func (d *DockerBuilder) dockerBuild(dir string) error {

+	var noCache bool

+	var forcePull bool

+	if d.build.Spec.Strategy.DockerStrategy != nil {

+		if d.build.Spec.Source.ContextDir != "" {

+			dir = filepath.Join(dir, d.build.Spec.Source.ContextDir)

 		}

-		line = strings.TrimSpace(line)

-

-		// The current line starts with the specified command (cmd)

-		if strings.HasPrefix(strings.ToUpper(line), strings.ToUpper(cmd)) {

-			index++

-

-			// The current line finishes on a backslash.

-			// All we need to do is replace the next line

-			// with our specified replaceArgs

-			if line[len(line)-1:] == "\\" && index == pos {

-				replaceNextLn = true

-

-				args := strings.Split(line, " ")

-				if len(args) > 2 {

-					// Keep just our Dockerfile command and the backslash

-					newFileData += args[0] + " \\" + "\n"

-				} else {

-					newFileData += line + "\n"

-				}

-				continue

-			}

+		noCache = d.build.Spec.Strategy.DockerStrategy.NoCache

+		forcePull = d.build.Spec.Strategy.DockerStrategy.ForcePull

+	}

+	auth, err := d.setupPullSecret()

+	if err != nil {

+		return err

+	}

+	return buildImage(d.dockerClient, dir, noCache, d.build.Spec.Output.To.Name, d.tar, auth, forcePull)

+}

-			// Normal ending line

-			if index == pos {

-				line = fmt.Sprintf("%s %s", strings.ToUpper(cmd), replaceArgs)

+// replaceLastFrom changes the last FROM instruction of node to point to the

+// base image image.

+func replaceLastFrom(node *parser.Node, image string) error {

+	if node == nil {

+		return nil

+	}

+	for i := len(node.Children) - 1; i >= 0; i-- {

+		child := node.Children[i]

+		if child != nil && child.Value == dockercmd.From {

+			from, err := dockerfile.From(image)

+			if err != nil {

+				return err

 			}

-		}

-

-		// Previous line ended on a backslash

-		// This line contains command arguments

-		if replaceNextLn {

-			if line[len(line)-1:] == "\\" {

-				// Ignore all successive lines terminating on a backslash

-				// since they all are going to be replaced by replaceArgs

-				continue

+			fromTree, err := parser.Parse(strings.NewReader(from))

+			if err != nil {

+				return err

 			}

-			replaceNextLn = false

-			line = replaceArgs

+			node.Children[i] = fromTree.Children[0]

+			return nil

 		}

-

-		if err == io.EOF {

-			// Otherwise, the new Dockerfile will have one newline

-			// more in the end

-			newFileData += line

-			break

-		}

-		newFileData += line + "\n"

 	}

+	return nil

+}

-	// Parse output for validation

-	buf = bytes.NewBuffer([]byte(newFileData))

-	if _, err := parser.Parse(buf); err != nil {

-		return "", errors.New("cannot parse new Dockerfile: " + err.Error())

-	}

+// appendEnv appends an ENV Dockerfile instruction as the last child of node

+// with keys and values from m.

+func appendEnv(node *parser.Node, m []dockerfile.KeyValue) error {

+	return appendKeyValueInstruction(dockerfile.Env, node, m)

+}

-	return newFileData, nil

+// appendLabel appends a LABEL Dockerfile instruction as the last child of node

+// with keys and values from m.

+func appendLabel(node *parser.Node, m []dockerfile.KeyValue) error {

+	return appendKeyValueInstruction(dockerfile.Label, node, m)

 }

-// traverseAST traverses the Abstract Syntax Tree output

-// from the Docker parser and returns the valid position

-// of the command it was requested to look for.

-//

-// Note that this function is intended to be used with

-// Dockerfile commands that should be specified only once

-// in a Dockerfile (FROM, CMD, ENTRYPOINT)

-func traverseAST(cmd string, node *parser.Node) int {

-	switch cmd {

-	case dockercmd.From, dockercmd.Entrypoint, dockercmd.Cmd:

-	default:

-		return 0

-	}

-

-	index := 0

-	if node.Value == cmd {

-		index++

-	}

-	for _, n := range node.Children {

-		index += traverseAST(cmd, n)

-	}

-	if node.Next != nil {

-		for n := node.Next; n != nil; n = n.Next {

-			if len(n.Children) > 0 {

-				index += traverseAST(cmd, n)

-			} else if n.Value == cmd {

-				index++

-			}

-		}

+// appendKeyValueInstruction is a primitive used to avoid code duplication.

+// Callers should use a derivative of this such as appendEnv or appendLabel.

+// appendKeyValueInstruction appends a Dockerfile instruction with key-value

+// syntax created by f as the last child of node with keys and values from m.

+func appendKeyValueInstruction(f func([]dockerfile.KeyValue) (string, error), node *parser.Node, m []dockerfile.KeyValue) error {

+	if node == nil {

+		return nil

 	}

-	return index

+	instruction, err := f(m)

+	if err != nil {

+		return err

+	}

+	return dockerfile.InsertInstructions(node, len(node.Children), instruction)

 }

 // insertEnvAfterFrom inserts an ENV instruction with the environment variables

@@ -456,34 +404,3 @@ func insertEnvAfterFrom(node *parser.Node, env []kapi.EnvVar) error {

 	return nil

 }

-

-// setupPullSecret provides a Docker authentication configuration when the

-// PullSecret is specified.

-func (d *DockerBuilder) setupPullSecret() (*docker.AuthConfigurations, error) {

-	if len(os.Getenv(dockercfg.PullAuthType)) == 0 {

-		return nil, nil

-	}

-	r, err := os.Open(os.Getenv(dockercfg.PullAuthType))

-	if err != nil {

-		return nil, fmt.Errorf("'%s': %s", os.Getenv(dockercfg.PullAuthType), err)

-	}

-	return docker.NewAuthConfigurations(r)

-}

-

-// dockerBuild performs a docker build on the source that has been retrieved

-func (d *DockerBuilder) dockerBuild(dir string) error {

-	var noCache bool

-	var forcePull bool

-	if d.build.Spec.Strategy.DockerStrategy != nil {

-		if d.build.Spec.Source.ContextDir != "" {

-			dir = filepath.Join(dir, d.build.Spec.Source.ContextDir)

-		}

-		noCache = d.build.Spec.Strategy.DockerStrategy.NoCache

-		forcePull = d.build.Spec.Strategy.DockerStrategy.ForcePull

-	}

-	auth, err := d.setupPullSecret()

-	if err != nil {

-		return err

-	}

-	return buildImage(d.dockerClient, dir, noCache, d.build.Spec.Output.To.Name, d.tar, auth, forcePull)

-}

@@ -1,259 +1,16 @@

 package builder

 import (

-	"bytes"

-	"log"

 	"reflect"

 	"strings"

 	"testing"

-	dockercmd "github.com/docker/docker/builder/command"

 	"github.com/docker/docker/builder/parser"

 	kapi "k8s.io/kubernetes/pkg/api"

 	"github.com/openshift/origin/pkg/util/docker/dockerfile"

 )

-func TestReplaceValidCmd(t *testing.T) {

-	tests := []struct {

-		name           string

-		cmd            string

-		replaceArgs    string

-		fileData       []byte

-		expectedOutput string

-		expectedDiffs  int

-		expectedErr    error

-	}{

-		{

-			name:           "from-replacement",

-			cmd:            dockercmd.From,

-			replaceArgs:    "other/image",

-			fileData:       []byte(dockerFile),

-			expectedOutput: expectedFROM,

-			expectedDiffs:  1,

-			expectedErr:    nil,

-		},

-		{

-			name:           "run-replacement",

-			cmd:            dockercmd.Run,

-			replaceArgs:    "This test kind-of-fails before string replacement so this string won't be used",

-			fileData:       []byte(dockerFile),

-			expectedOutput: "",

-			expectedErr:    replaceCmdErr,

-		},

-		{

-			name:           "invalid-dockerfile-cmd",

-			cmd:            "blabla",

-			replaceArgs:    "This test fails at start so this string won't be used",

-			fileData:       []byte(dockerFile),

-			expectedOutput: "",

-			expectedErr:    invalidCmdErr,

-		},

-		{

-			name:           "no-cmd-in-dockerfile",

-			cmd:            dockercmd.Cmd,

-			replaceArgs:    "runme.sh",

-			fileData:       []byte(dockerFile),

-			expectedOutput: "",

-			expectedErr:    replaceCmdErr,

-		},

-		{

-			name:           "trailing-slash",

-			cmd:            dockercmd.From,

-			replaceArgs:    "rhel",

-			fileData:       []byte(trSlashFile),

-			expectedOutput: expectedtrSlashFile,

-			expectedDiffs:  1,

-			expectedErr:    nil,

-		},

-		{

-			name:           "multiple trailing slashes plus plus",

-			cmd:            dockercmd.From,

-			replaceArgs:    "scratch",

-			fileData:       []byte(trickierFile),

-			expectedOutput: expectedTrickierFile,

-			expectedDiffs:  1,

-			expectedErr:    nil,

-		},

-	}

-

-	for _, test := range tests {

-		out, err := replaceValidCmd(test.cmd, test.replaceArgs, test.fileData)

-		if err != test.expectedErr {

-			t.Errorf("%s: Unexpected error: Expected %v, got %v", test.name, test.expectedErr, err)

-		}

-		if out != test.expectedOutput {

-			t.Errorf("%s: Unexpected output:\n\nExpected:\n%s\n(length: %d)\n\ngot:\n%s\n(length: %d)",

-				test.name, test.expectedOutput, len(test.expectedOutput), out, len(out))

-		}

-	}

-

-	// Re-use the tests above

-	var buf *bytes.Buffer

-	for _, test := range tests {

-		buf = bytes.NewBuffer([]byte(test.fileData))

-		original, err := parser.Parse(buf)

-		if err != nil {

-			log.Println(err)

-		}

-		repl, err := replaceValidCmd(test.cmd, test.replaceArgs, test.fileData)

-		if err != nil {

-			log.Println(err)

-		}

-		buf = bytes.NewBuffer([]byte(repl))

-		edited, err := parser.Parse(buf)

-		if err != nil {

-			log.Println(err)

-		}

-

-		diff := cmpASTs(original, edited)

-

-		if diff != test.expectedDiffs {

-			t.Errorf("%s: Edit mismatch, expected %d edit(s), got %d", test.name, test.expectedDiffs, diff)

-		}

-	}

-}

-

-// cmpASTs compares two Abstract Syntax Trees and returns the

-// amount of differences between them

-func cmpASTs(original *parser.Node, edited *parser.Node) int {

-	index := 0

-	if original.Value != edited.Value {

-		index++

-	}

-

-	originalChildren := make([]*parser.Node, 0)

-	for _, n := range original.Children {

-		originalChildren = append(originalChildren, n)

-	}

-	editedChildren := make([]*parser.Node, 0)

-	for _, n := range edited.Children {

-		editedChildren = append(editedChildren, n)

-	}

-	for i := 0; i < len(editedChildren); i++ {

-		index += cmpASTs(originalChildren[i], editedChildren[i])

-	}

-

-	if original.Next != nil && edited.Next != nil {

-		index += cmpASTs(original.Next, edited.Next)

-	} else if original.Next != edited.Next {

-		index++

-	}

-	return index

-}

-

-func TestTraverseAST(t *testing.T) {

-	tests := []struct {

-		name     string

-		cmd      string

-		fileData []byte

-		expected int

-	}{

-		{

-			name:     "dockerFile",

-			cmd:      dockercmd.Entrypoint,

-			fileData: []byte(dockerFile),

-			expected: 1,

-		},

-		{

-			name:     "dockerFile no newline",

-			cmd:      dockercmd.Entrypoint,

-			fileData: []byte(dockerFileNoNewline),

-			expected: 1,

-		},

-		{

-			name:     "expectedFROM",

-			cmd:      dockercmd.From,

-			fileData: []byte(expectedFROM),

-			expected: 2,

-		},

-		{

-			name:     "trSlashFile",

-			cmd:      dockercmd.Entrypoint,

-			fileData: []byte(trSlashFile),

-			expected: 0,

-		},

-		{

-			name:     "expectedtrSlashFile",

-			cmd:      dockercmd.Cmd,

-			fileData: []byte(expectedtrSlashFile),

-			expected: 1,

-		},

-	}

-

-	var buf *bytes.Buffer

-	for _, test := range tests {

-		buf = bytes.NewBuffer([]byte(test.fileData))

-		node, err := parser.Parse(buf)

-		if err != nil {

-			log.Println(err)

-		}

-

-		howMany := traverseAST(test.cmd, node)

-		if howMany != test.expected {

-			t.Errorf("Wrong result, expected %d, got %d", test.expected, howMany)

-		}

-	}

-}

-

-func TestAppendEnvVars(t *testing.T) {

-	tests := []struct {

-		name       string

-		dockerFile string

-		envVars    map[string]string

-		expected   string

-	}{

-		{

-			name:       "regular dockerFile",

-			dockerFile: dockerFile,

-			envVars:    map[string]string{"VAR1": "value1"},

-			expected:   dockerFile + "ENV VAR1=\"value1\"\n",

-		},

-		{

-			name:       "dockerFile with no newline",

-			dockerFile: dockerFileNoNewline,

-			envVars:    map[string]string{"VAR1": "value1"},

-			expected:   dockerFileNoNewline + "\nENV VAR1=\"value1\"\n",

-		},

-	}

-

-	for _, test := range tests {

-		result := appendMetadata(Env, test.dockerFile, test.envVars)

-		if result != test.expected {

-			t.Errorf("%s: unexpected result.\n\tExpected: %s\n\tGot: %s\n", test.name, test.expected, result)

-		}

-	}

-}

-

-func TestAppendLabels(t *testing.T) {

-	tests := []struct {

-		name       string

-		dockerFile string

-		labels     map[string]string

-		expected   string

-	}{

-		{

-			name:       "regular dockerFile",

-			dockerFile: dockerFile,

-			labels:     map[string]string{"LABEL1": "value1"},

-			expected:   dockerFile + "LABEL LABEL1=\"value1\"\n",

-		},

-		{

-			name:       "dockerFile with no newline",

-			dockerFile: dockerFileNoNewline,

-			labels:     map[string]string{"LABEL1": "value1"},

-			expected:   dockerFileNoNewline + "\nLABEL LABEL1=\"value1\"\n",

-		},

-	}

-

-	for _, test := range tests {

-		result := appendMetadata(Label, test.dockerFile, test.labels)

-		if result != test.expected {

-			t.Errorf("%s: unexpected result.\n\tExpected: %s\n\tGot: %s\n", test.name, test.expected, result)

-		}

-	}

-}

-

 func TestInsertEnvAfterFrom(t *testing.T) {

 	tests := map[string]struct {

 		original string

@@ -321,69 +78,54 @@ RUN echo "hello world"

 	}

 }

-const (

-	dockerFile = `

-FROM openshift/origin-base

-FROM candidate

-

-RUN mkdir -p /var/lib/openshift

-

-ADD bin/openshift        /usr/bin/openshift

-RUN ln -s /usr/bin/openshift /usr/bin/oc && \

-

-ENV HOME /root

-WORKDIR /var/lib/openshift

-ENTRYPOINT ["/usr/bin/openshift"]

-`

-	dockerFileNoNewline = `

-FROM openshift/origin-base

-FROM candidate

-

-RUN mkdir -p /var/lib/openshift

-

-ADD bin/openshift        /usr/bin/openshift

-RUN ln -s /usr/bin/openshift /usr/bin/oc && \

-

-ENV HOME /root

-WORKDIR /var/lib/openshift

-ENTRYPOINT ["/usr/bin/openshift"]`

-

-	expectedFROM = `

-FROM openshift/origin-base

-FROM other/image

-

-RUN mkdir -p /var/lib/openshift

-

-ADD bin/openshift        /usr/bin/openshift

-RUN ln -s /usr/bin/openshift /usr/bin/oc && \

-

-ENV HOME /root

-WORKDIR /var/lib/openshift

-ENTRYPOINT ["/usr/bin/openshift"]

-`

-

-	trSlashFile = `

-from \

-centos

-CMD "cat /etc/passwd"`

-

-	expectedtrSlashFile = `

-from \

-rhel

-CMD "cat /etc/passwd"`

-

-	trickierFile = `

-from centos \

-rhel \

-ubuntu

-

-CMD ["executable","param1","param2"]

-`

-

-	expectedTrickierFile = `

-from \

-scratch

-

-CMD ["executable","param1","param2"]

-`

-)

+func TestReplaceLastFrom(t *testing.T) {

+	tests := []struct {

+		original string

+		image    string

+		want     string

+	}{

+		{

+			original: `# no FROM instruction`,

+			image:    "centos",

+			want:     ``,

+		},

+		{

+			original: `FROM scratch

+# FROM busybox

+RUN echo "hello world"

+`,

+			image: "centos",

+			want: `FROM centos

+RUN echo "hello world"

+`,

+		},

+		{

+			original: `FROM scratch

+FROM busybox

+RUN echo "hello world"

+`,

+			image: "centos",

+			want: `FROM scratch

+FROM centos

+RUN echo "hello world"

+`,

+		},

+	}

+	for i, test := range tests {

+		got, err := parser.Parse(strings.NewReader(test.original))

+		if err != nil {

+			t.Errorf("test[%d]: %v", i, err)

+			continue

+		}

+		want, err := parser.Parse(strings.NewReader(test.want))

+		if err != nil {

+			t.Errorf("test[%d]: %v", i, err)

+			continue

+		}

+		replaceLastFrom(got, test.image)

+		if !reflect.DeepEqual(got, want) {

+			t.Errorf("test[%d]: replaceLastFrom(node, %+v) = %+v; want %+v", i, test.image, got, want)

+			t.Logf("resulting Dockerfile:\n%s", dockerfile.ParseTreeToDockerfile(got))

+		}

+	}

+}

@@ -58,7 +58,7 @@ func (s *STIBuilder) Build() error {

 		DockerCfgPath:  os.Getenv(dockercfg.PullAuthType),

 		Tag:            tag,

 		ScriptsURL:     s.build.Spec.Strategy.SourceStrategy.Scripts,

-		Environment:    getBuildEnvVars(s.build),

+		Environment:    buildEnvVars(s.build),

 		LabelNamespace: api.DefaultDockerLabelNamespace,

 		Incremental:    s.build.Spec.Strategy.SourceStrategy.Incremental,

 		ForcePull:      s.build.Spec.Strategy.SourceStrategy.ForcePull,

@@ -157,3 +157,20 @@ func (s *STIBuilder) Build() error {

 	}

 	return nil

 }

+

+// buildEnvVars returns a map with build metadata to be inserted into Docker

+// images produced by build. It transforms the output from buildInfo into the

+// input format expected by stiapi.Config.Environment.