add unit tests

pkg/dockerfile/dockerfile.go

@@ -22,9 +22,9 @@ import (
 	"io/ioutil"
 	"net/http"
 	"regexp"
-	"strconv"
 	"strings"
 
+	v1 "github.com/google/go-containerregistry/pkg/v1"
 	"github.com/sirupsen/logrus"
 
 	"github.com/GoogleContainerTools/kaniko/pkg/config"
@@ -34,60 +34,7 @@ import (
 	"github.com/pkg/errors"
 )
 
-// Stages parses a Dockerfile and returns an array of KanikoStage
-func Stages(opts *config.KanikoOptions) ([]config.KanikoStage, error) {
-	var err error
-	var d []uint8
-	match, _ := regexp.MatchString("^https?://", opts.DockerfilePath)
-	if match {
-		response, e := http.Get(opts.DockerfilePath)
-		if e != nil {
-			return nil, e
-		}
-		d, err = ioutil.ReadAll(response.Body)
-	} else {
-		d, err = ioutil.ReadFile(opts.DockerfilePath)
-	}
-
-	if err != nil {
-		return nil, errors.Wrap(err, fmt.Sprintf("reading dockerfile at path %s", opts.DockerfilePath))
-	}
-
-	stages, metaArgs, err := Parse(d)
-	if err != nil {
-		return nil, errors.Wrap(err, "parsing dockerfile")
-	}
-	targetStage, err := targetStage(stages, opts.Target)
-	if err != nil {
-		return nil, err
-	}
-	resolveStages(stages)
-	var kanikoStages []config.KanikoStage
-	for index, stage := range stages {
-		resolvedBaseName, err := util.ResolveEnvironmentReplacement(stage.BaseName, opts.BuildArgs, false)
-		if err != nil {
-			return nil, errors.Wrap(err, "resolving base name")
-		}
-		stage.Name = resolvedBaseName
-		logrus.Infof("Resolved base name %s to %s", stage.BaseName, stage.Name)
-		kanikoStages = append(kanikoStages, config.KanikoStage{
-			Stage:                  stage,
-			BaseImageIndex:         baseImageIndex(index, stages),
-			BaseImageStoredLocally: (baseImageIndex(index, stages) != -1),
-			SaveStage:              saveStage(index, stages),
-			Final:                  index == targetStage,
-			MetaArgs:               metaArgs,
-			Index:                  index,
-		})
-		if index == targetStage {
-			break
-		}
-	}
-
-	return kanikoStages, nil
-}
-
-func ParseInstructions(opts *config.KanikoOptions) ([]instructions.Stage, []instructions.ArgCommand, error) {
+func ParseStages(opts *config.KanikoOptions) ([]instructions.Stage, []instructions.ArgCommand, error) {
 	var err error
 	var d []uint8
 	match, _ := regexp.MatchString("^https?://", opts.DockerfilePath)
@@ -113,49 +60,6 @@ func ParseInstructions(opts *config.KanikoOptions) ([]instructions.Stage, []inst
 	return stages, metaArgs, nil
 }
 
-func ResolveCrossStageInstructions(stages []instructions.Stage) map[string]string {
-	nameToIndex := make(map[string]string)
-	for i, stage := range stages {
-		index := strconv.Itoa(i)
-		if stage.Name != "" {
-			nameToIndex[stage.Name] = index
-		}
-		ResolveCommands(stage.Commands, nameToIndex)
-	}
-
-	logrus.Debugf("Built stage name to index map: %v", nameToIndex)
-	return nameToIndex
-}
-
-func MakeKanikoStages(opts *config.KanikoOptions, stages []instructions.Stage, metaArgs []instructions.ArgCommand) ([]config.KanikoStage, error) {
-	targetStage, err := targetStage(stages, opts.Target)
-	if err != nil {
-		return nil, errors.Wrap(err, "Error finding target stage")
-	}
-	var kanikoStages []config.KanikoStage
-	for index, stage := range stages {
-		resolvedBaseName, err := util.ResolveEnvironmentReplacement(stage.BaseName, opts.BuildArgs, false)
-		if err != nil {
-			return nil, errors.Wrap(err, "resolving base name")
-		}
-		stage.Name = resolvedBaseName
-		logrus.Infof("Resolved base name %s to %s", stage.BaseName, stage.Name)
-		kanikoStages = append(kanikoStages, config.KanikoStage{
-			Stage:                  stage,
-			BaseImageIndex:         baseImageIndex(index, stages),
-			BaseImageStoredLocally: (baseImageIndex(index, stages) != -1),
-			SaveStage:              saveStage(index, stages),
-			Final:                  index == targetStage,
-			MetaArgs:               metaArgs,
-			Index:                  index,
-		})
-		if index == targetStage {
-			break
-		}
-	}
-	return kanikoStages, nil
-}
-
 // baseImageIndex returns the index of the stage the current stage is built off
 // returns -1 if the current stage isn't built off a previous stage
 func baseImageIndex(currentStage int, stages []instructions.Stage) int {
@@ -273,29 +177,6 @@ func targetStage(stages []instructions.Stage, target string) (int, error) {
 	return -1, fmt.Errorf("%s is not a valid target build stage", target)
 }
 
-// resolveStages resolves any calls to previous stages with names to indices
-// Ex. --from=second_stage should be --from=1 for easier processing later on
-// As third party library lowers stage name in FROM instruction, this function resolves stage case insensitively.
-func resolveStages(stages []instructions.Stage) {
-	nameToIndex := make(map[string]string)
-	for i, stage := range stages {
-		index := strconv.Itoa(i)
-		if stage.Name != index {
-			nameToIndex[stage.Name] = index
-		}
-		for _, cmd := range stage.Commands {
-			switch c := cmd.(type) {
-			case *instructions.CopyCommand:
-				if c.From != "" {
-					if val, ok := nameToIndex[strings.ToLower(c.From)]; ok {
-						c.From = val
-					}
-				}
-			}
-		}
-	}
-}
-
 // ParseCommands parses an array of commands into an array of instructions.Command; used for onbuild
 func ParseCommands(cmdArray []string) ([]instructions.Command, error) {
 	var cmds []instructions.Command
@@ -333,8 +214,10 @@ func saveStage(index int, stages []instructions.Stage) bool {
 	return false
 }
 
-// TODO move it to private method or put elsewhere
-func ResolveCommands(cmds []instructions.Command, stageNameToIdx map[string]string) () {
+// ResolveCrossStageCommands resolves any calls to previous stages with names to indices
+// Ex. --from=secondStage should be --from=1 for easier processing later on
+// As third party library lowers stage name in FROM instruction, this function resolves stage case insensitively.
+func ResolveCrossStageCommands(cmds []instructions.Command, stageNameToIdx map[string]string) {
 	for _, cmd := range cmds {
 		switch c := cmd.(type) {
 		case *instructions.CopyCommand:
@@ -345,5 +228,48 @@ func ResolveCommands(cmds []instructions.Command, stageNameToIdx map[string]stri
 			}
 		}
 	}
+}
 
-}
+func MakeKanikoStages(opts *config.KanikoOptions, stages []instructions.Stage, metaArgs []instructions.ArgCommand) ([]config.KanikoStage, error) {
+	targetStage, err := targetStage(stages, opts.Target)
+	if err != nil {
+		return nil, errors.Wrap(err, "Error finding target stage")
+	}
+	var kanikoStages []config.KanikoStage
+	for index, stage := range stages {
+		resolvedBaseName, err := util.ResolveEnvironmentReplacement(stage.BaseName, opts.BuildArgs, false)
+		if err != nil {
+			return nil, errors.Wrap(err, "resolving base name")
+		}
+		stage.Name = resolvedBaseName
+		logrus.Infof("Resolved base name %s to %s", stage.BaseName, stage.Name)
+		kanikoStages = append(kanikoStages, config.KanikoStage{
+			Stage:                  stage,
+			BaseImageIndex:         baseImageIndex(index, stages),
+			BaseImageStoredLocally: (baseImageIndex(index, stages) != -1),
+			SaveStage:              saveStage(index, stages),
+			Final:                  index == targetStage,
+			MetaArgs:               metaArgs,
+			Index:                  index,
+		})
+		if index == targetStage {
+			break
+		}
+	}
+	return kanikoStages, nil
+}
+
+func GetOnBuildInstructions(config *v1.Config, stageNameToIdx map[string]string) ([]instructions.Command, error) {
+	if config.OnBuild == nil || len(config.OnBuild) == 0 {
+		return nil, nil
+	}
+
+	cmds, err := ParseCommands(config.OnBuild)
+	if err != nil {
+		return nil, err
+	}
+
+	// Iterate over commands and replace references to other stages with their index
+	ResolveCrossStageCommands(cmds, stageNameToIdx)
+	return cmds, nil
+}

pkg/dockerfile/dockerfile_test.go

@@ -19,18 +19,19 @@ package dockerfile
 import (
 	"io/ioutil"
 	"os"
-	"strconv"
+	"reflect"
 	"testing"
 
 	"github.com/GoogleContainerTools/kaniko/pkg/config"
 	"github.com/GoogleContainerTools/kaniko/testutil"
+	v1 "github.com/google/go-containerregistry/pkg/v1"
 	"github.com/moby/buildkit/frontend/dockerfile/instructions"
-	"github.com/sirupsen/logrus"
 )
 
-func Test_Stages_ArgValueWithQuotes(t *testing.T) {
+func Test_ParseStages_ArgValueWithQuotes(t *testing.T) {
 	dockerfile := `
 	ARG IMAGE="ubuntu:16.04"
+	ARG FOO=bar
 	FROM ${IMAGE}
 	RUN echo hi > /hi
 	
@@ -54,25 +55,23 @@ func Test_Stages_ArgValueWithQuotes(t *testing.T) {
 		t.Fatal(err)
 	}
 
-	stages, err := Stages(&config.KanikoOptions{DockerfilePath: tmpfile.Name()})
+	stages, metaArgs, err := ParseStages(&config.KanikoOptions{DockerfilePath: tmpfile.Name()})
 	if err != nil {
 		t.Fatal(err)
 	}
 
 	if len(stages) == 0 {
 		t.Fatal("length of stages expected to be greater than zero, but was zero")
-
 	}
 
-	if len(stages[0].MetaArgs) == 0 {
-		t.Fatal("length of stage[0] meta args expected to be greater than zero, but was zero")
+	if len(metaArgs) != 2 {
+		t.Fatalf("length of stage meta args expected to be 2, but was %d", len(metaArgs))
 	}
 
-	expectedVal := "ubuntu:16.04"
-
-	arg := stages[0].MetaArgs[0]
-	if arg.ValueString() != expectedVal {
-		t.Fatalf("expected stages[0].MetaArgs[0] val to be %s but was %s", expectedVal, arg.ValueString())
+	for i, expectedVal := range []string{"ubuntu:16.04", "bar"} {
+		if metaArgs[i].ValueString() != expectedVal {
+			t.Fatalf("expected metaArg %d val to be %s but was %s", i, expectedVal, metaArgs[i].ValueString())
+		}
 	}
 }
 
@@ -190,40 +189,103 @@ func Test_stripEnclosingQuotes(t *testing.T) {
 	}
 }
 
-func Test_resolveStages(t *testing.T) {
-	dockerfile := `
-	FROM scratch
-	RUN echo hi > /hi
-	
-	FROM scratch AS second
-	COPY --from=0 /hi /hi2
-	
-	FROM scratch AS tHiRd
-	COPY --from=second /hi2 /hi3
-	COPY --from=1 /hi2 /hi3
-
-	FROM scratch
-	COPY --from=thIrD /hi3 /hi4
-	COPY --from=third /hi3 /hi4
-	COPY --from=2 /hi3 /hi4
-	`
-	stages, _, err := Parse([]byte(dockerfile))
-	if err != nil {
-		t.Fatal(err)
+func Test_ResolveCrossStageCommands(t *testing.T) {
+	type testCase struct {
+		name       string
+		cmd        instructions.CopyCommand
+		stageToIdx map[string]string
+		expFrom    string
 	}
-	resolveStages(stages)
-	for index, stage := range stages {
-		if index == 0 {
-			continue
-		}
-		expectedStage := strconv.Itoa(index - 1)
-		for _, command := range stage.Commands {
-			copyCmd := command.(*instructions.CopyCommand)
-			if copyCmd.From != expectedStage {
-				t.Fatalf("unexpected copy command: %s resolved to stage %s, expected %s", copyCmd.String(), copyCmd.From, expectedStage)
-			}
-		}
 
+	tests := []testCase{
+		{
+			name:       "resolve copy command",
+			cmd:        instructions.CopyCommand{From: "builder"},
+			stageToIdx: map[string]string{"builder": "0"},
+			expFrom:    "0",
+		},
+		{
+			name:       "resolve upper case FROM",
+			cmd:        instructions.CopyCommand{From: "BuIlDeR"},
+			stageToIdx: map[string]string{"builder": "0"},
+			expFrom:    "0",
+		},
+		{
+			name:       "nothing to resolve",
+			cmd:        instructions.CopyCommand{From: "downloader"},
+			stageToIdx: map[string]string{"builder": "0"},
+			expFrom:    "downloader",
+		},
+	}
+
+	for _, test := range tests {
+		t.Run(test.name, func(t *testing.T) {
+			cmds := []instructions.Command{&test.cmd}
+			ResolveCrossStageCommands(cmds, test.stageToIdx)
+			if test.cmd.From != test.expFrom {
+				t.Fatalf("Failed to resolve command: expected from %s, resolved to %s", test.expFrom, test.cmd.From)
+			}
+		})
+	}
+}
+
+func Test_GetOnBuildInstructions(t *testing.T) {
+	type testCase struct {
+		name        string
+		cfg         *v1.Config
+		stageToIdx  map[string]string
+		expCommands []instructions.Command
+	}
+
+	tests := []testCase{
+		{name: "no on-build on config",
+			cfg:         &v1.Config{},
+			stageToIdx:  map[string]string{"builder": "0"},
+			expCommands: nil,
+		},
+		{name: "onBuild on config, nothing to resolve",
+			cfg:         &v1.Config{OnBuild: []string{"WORKDIR /app"}},
+			stageToIdx:  map[string]string{"builder": "0", "temp": "1"},
+			expCommands: []instructions.Command{&instructions.WorkdirCommand{Path: "/app"}},
+		},
+		{name: "onBuild on config, resolve multiple stages",
+			cfg:        &v1.Config{OnBuild: []string{"COPY --from=builder a.txt b.txt", "COPY --from=temp /app /app"}},
+			stageToIdx: map[string]string{"builder": "0", "temp": "1"},
+			expCommands: []instructions.Command{
+				&instructions.CopyCommand{
+					SourcesAndDest: []string{"a.txt b.txt"},
+					From:           "0",
+				},
+				&instructions.CopyCommand{
+					SourcesAndDest: []string{"/app /app"},
+					From:           "1",
+				},
+			}},
+	}
+
+	for _, test := range tests {
+		t.Run(test.name, func(t *testing.T) {
+			cmds, err := GetOnBuildInstructions(test.cfg, test.stageToIdx)
+			if err != nil {
+				t.Fatalf("Failed to parse config for on-build instructions")
+			}
+			if len(cmds) != len(test.expCommands) {
+				t.Fatalf("Expected %d commands, got %d", len(test.expCommands), len(cmds))
+			}
+
+			for i, cmd := range cmds {
+				if reflect.TypeOf(cmd) != reflect.TypeOf(test.expCommands[i]) {
+					t.Fatalf("Got command %s, expected %s", cmd, test.expCommands[i])
+				}
+				switch c := cmd.(type) {
+				case *instructions.CopyCommand:
+					{
+						exp := test.expCommands[i].(*instructions.CopyCommand)
+						testutil.CheckDeepEqual(t, exp.From, c.From)
+					}
+				}
+			}
+		})
 	}
 }
 
@@ -365,30 +427,3 @@ func Test_baseImageIndex(t *testing.T) {
 		})
 	}
 }
-
-
-
-func Test_ResolveStages(t *testing.T) {
-	in := &instructions.CopyCommand{
-		SourcesAndDest: []string{
-			"/var/bo", "foo.txt",
-		},
-		From:  "boo",
-		Chown: "",
-	}
-	ibn := &instructions.CopyCommand{
-		SourcesAndDest: []string{
-			"/var/bo", "foo.txt",
-		},
-		From:  "poo",
-		Chown: "",
-	}
-
-	foo := []instructions.Command{in, ibn}
-	stageMap := map[string]string{"boo": "1"}
-	logrus.Infof("%#v", foo)
-	ResolveCommands(foo, stageMap)
-	logrus.Infof("%#v", foo)
-	logrus.Infof("%#v", foo[0])
-
-}