diff --git a/pkg/build/builder/docker.go b/pkg/build/builder/docker.go index 0ff5859247d4..5d0a486c2424 100644 --- a/pkg/build/builder/docker.go +++ b/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.Image != "" { - newFileData = imageRegex.ReplaceAllLiteralString(string(fileData), fmt.Sprintf("FROM %s", d.build.Parameters.Strategy.DockerStrategy.Image)) + newFileData, err = replaceValidCmd(dockercmd.From, d.build.Parameters.Strategy.DockerStrategy.Image, fileData) + if err != nil { + return err + } } else { newFileData = newFileData + string(fileData) } @@ -171,6 +176,136 @@ func (d *DockerBuilder) addBuildParameters(dir string) error { return nil } +// 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()) + } + + pos := traverseAST(cmd, node) + if pos == 0 { + return "", replaceCmdErr + } + + // 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(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 + } + + // Normal ending line + if index == pos { + line = fmt.Sprintf("%s %s", strings.ToUpper(cmd), replaceArgs) + } + } + + // 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 + } + replaceNextLn = false + line = replaceArgs + } + + if err == io.EOF { + // Otherwise, the new Dockerfile will have one newline + // more in the end + newFileData += line + break + } + newFileData += line + "\n" + } + + // 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()) + } + + 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 { + 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++ + } + } + } + return index +} + // dockerBuild performs a docker build on the source that has been retrieved func (d *DockerBuilder) dockerBuild(dir string) error { var noCache bool diff --git a/pkg/build/builder/docker_test.go b/pkg/build/builder/docker_test.go new file mode 100644 index 000000000000..bc058ba1d7ad --- /dev/null +++ b/pkg/build/builder/docker_test.go @@ -0,0 +1,249 @@ +package builder + +import ( + "bytes" + "log" + "testing" + + dockercmd "github.com/docker/docker/builder/command" + "github.com/docker/docker/builder/parser" +) + +func TestReplaceValidCmd(t *testing.T) { + tests := []struct { + name string + cmd string + replaceArgs string + fileData []byte + edited bool + expectedOutput string + expectedErr error + }{ + { + name: "from-replacement", + cmd: dockercmd.From, + replaceArgs: "other/image", + fileData: []byte(dockerFile), + edited: true, + expectedOutput: expectedFROM, + 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), + edited: false, + 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), + edited: false, + expectedOutput: "", + expectedErr: invalidCmdErr, + }, + { + name: "no-cmd-in-dockerfile", + cmd: dockercmd.Cmd, + replaceArgs: "runme.sh", + fileData: []byte(dockerFile), + edited: false, + expectedOutput: "", + expectedErr: replaceCmdErr, + }, + { + name: "trailing-slash", + cmd: dockercmd.From, + replaceArgs: "rhel", + fileData: []byte(trSlashFile), + edited: true, + expectedOutput: expectedtrSlashFile, + expectedErr: nil, + }, + { + name: "multiple trailing slashes plus plus", + cmd: dockercmd.From, + replaceArgs: "scratch", + fileData: []byte(trickierFile), + edited: true, + expectedOutput: expectedTrickierFile, + 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) + + // Note that these tests will probably fail for Dockerfiles where we have replaced + // multiline command arguments + if test.edited && diff != 1 { + t.Errorf("%s: Edit mismatch, expected one edit, got %d", test.name, diff) + } else if !test.edited && diff > 0 { + t.Errorf("%s: Edit mismatch, expected no edit, got %d", test.name, 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: "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) + } + } +} + +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"] +` + + 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"] +` + + 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"] +` +)