[WIP] Use Docker parser when manipulating Dockerfiles

pkg/build/builder/docker.go

@@ -1,17 +1,22 @@
 package builder
 
 import (
+	"bytes"
+	"errors"
 	"fmt"
+	"io"
 	"io/ioutil"
 	"net"
 	"net/url"
 	"os"
 	"path/filepath"
-	"regexp"
 	"strings"
 	"time"
 
+	dockercmd "github.com/docker/docker/builder/command"
+	"github.com/docker/docker/builder/parser"
 	"github.com/fsouza/go-dockerclient"
+
 	"github.com/openshift/origin/pkg/build/api"
 	"github.com/openshift/source-to-image/pkg/git"
 	"github.com/openshift/source-to-image/pkg/tar"
@@ -22,9 +27,6 @@ import (
 // not proceed further and stop
 const urlCheckTimeout = 16 * time.Second
 
-// imageRegex is used to substitute image names in buildconfigs with immutable image ids at build time.
-var imageRegex = regexp.MustCompile(`(?mi)^\s*FROM\s+\w+.+`)
-
 // DockerBuilder builds Docker images given a git repository URL
 type DockerBuilder struct {
 	dockerClient DockerClient
@@ -154,7 +156,10 @@ func (d *DockerBuilder) addBuildParameters(dir string) error {
 
 	var newFileData string
 	if d.build.Parameters.Strategy.DockerStrategy.BaseImage != "" {
-		newFileData = imageRegex.ReplaceAllLiteralString(string(fileData), fmt.Sprintf("FROM %s", d.build.Parameters.Strategy.DockerStrategy.BaseImage))
+		newFileData, err = replaceValidCmd(dockercmd.From, d.build.Parameters.Strategy.DockerStrategy.BaseImage, fileData)
+		if err != nil {
+			return err
+		}
 	} else {
 		newFileData = newFileData + string(fileData)
 	}
@@ -171,6 +176,107 @@ func (d *DockerBuilder) addBuildParameters(dir string) error {
 	return nil
 }
 
+// invalidCmdErr repesents an error returned from replaceValidCmd
+// when an invalid Dockerfile command has been passed to
+// replaceValidCmd
+var invalidCmdErr = errors.New("invalid Dockerfile command")
+
+// replaceValidCmd replaces the valid occurrence of cmd
+// 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")
+	}
+
+	var pos int
+	switch cmd {
+	case dockercmd.From, dockercmd.Entrypoint, dockercmd.Cmd:
+		pos = traverseAST(cmd, node)
+		if pos == 0 {
+			fallthrough
+		}
+	default:
+		return string(fileData), nil
+	}
+
+	// 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
+		}
+		line = strings.TrimSpace(line)
+
+		// The current line starts with the specified command (cmd)
+		if strings.HasPrefix(line, cmd) || strings.HasPrefix(line, strings.ToUpper(cmd)) {
+			index++
+
+			// The current line finishes on a backslash
+			// All we need to do is to replace the next
+			// line with our specified replaceArgs
+			if line[len(line)-1:] == "\\" && index == pos {
+				replaceNextLn = true
+				newFileData += line + "\n"
+				continue
+			}
+
+			// Normal ending line
+			if index == pos {
+				line = fmt.Sprintf("%s %s\n", strings.ToUpper(cmd), replaceArgs)
+			}
+		}
+
+		// Previous line ended on a backslash
+		// This line contains command arguments
+		if replaceNextLn {
+			replaceNextLn = false
+			line = replaceArgs + "\n"
+		}
+
+		newFileData += line
+		if err == io.EOF {
+			break
+		}
+	}
+	return newFileData, nil
+}
+
+// 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 {
+	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++
+			}
+		}
+	}
+	return index
+}
+
 // dockerBuild performs a docker build on the source that has been retrieved
 func (d *DockerBuilder) dockerBuild(dir string) error {
 	var noCache bool

pkg/build/builder/docker_test.go

@@ -0,0 +1,87 @@
+package builder
+
+import (
+	"testing"
+)
+
+func TestReplaceValidCmd(t *testing.T) {
+	tests := []struct {
+		name           string
+		cmd            string
+		replaceStr     string
+		fileData       []byte
+		expectedOutput string
+		expectedErr    error
+	}{
+		{
+			name:           "from-replacement",
+			cmd:            "from",
+			replaceStr:     "FROM other/image",
+			fileData:       []byte(dockerFile),
+			expectedOutput: expectedFROM,
+			expectedErr:    nil,
+		},
+		{
+			name:           "run-replacement",
+			cmd:            "run",
+			replaceStr:     "This test kind-of-fails before string replacement so this string won't be used",
+			fileData:       []byte(dockerFile),
+			expectedOutput: dockerFile,
+			expectedErr:    nil,
+		},
+		{
+			name:           "invalid-dockerfile-cmd",
+			cmd:            "blabla",
+			replaceStr:     "This test fails at start so this string won't be used",
+			fileData:       []byte(dockerFile),
+			expectedOutput: "",
+			expectedErr:    invalidCmdErr,
+		},
+		{
+			name:           "no-cmd-in-dockerfile",
+			cmd:            "cmd",
+			replaceStr:     "CMD runme.sh",
+			fileData:       []byte(dockerFile),
+			expectedOutput: dockerFile,
+			expectedErr:    nil,
+		},
+	}
+
+	for _, test := range tests {
+		out, err := replaceValidCmd(test.cmd, test.replaceStr, 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: Expected %s, got %s", test.name, test.expectedOutput, out)
+		}
+	}
+}
+
+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/osc && \
+
+ENV HOME /root
+WORKDIR /var/lib/openshift
+ENTRYPOINT ["/usr/bin/openshift"]
+`
+
+const 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/osc && \
+
+ENV HOME /root
+WORKDIR /var/lib/openshift
+ENTRYPOINT ["/usr/bin/openshift"]
+`