Don’t compute onbuild triggers for images that are stage names

Fix #502
GoogleContainerTools · Aug 29, 2018 · ae712da · ae712da
1 parent cea1e93
commit ae712da
Show file tree

Hide file tree

Showing 2 changed files with 36 additions and 8 deletions.
diff --git a/pkg/skaffold/docker/parse.go b/pkg/skaffold/docker/parse.go
@@ -102,10 +102,23 @@ func readDockerfile(workspace, absDockerfilePath string, buildArgs map[string]*s
 
 	// Then process onbuilds, if present.
 	onbuildsImages := [][]string{}
+	stages := map[string]bool{}
 	for _, value := range res.AST.Children {
 		switch value.Value {
 		case command.From:
-			onbuilds, err := processBaseImage(value)
+			imageName := value.Next.Value
+			if _, found := stages[imageName]; found {
+				continue
+			}
+
+			next := value.Next.Next
+			if next != nil && strings.ToLower(next.Value) == "as" {
+				if next.Next != nil {
+					stages[next.Next.Value] = true
+				}
+			}
+
+			onbuilds, err := processBaseImage(imageName)
 			if err != nil {
 				logrus.Warnf("Error processing base image for onbuild triggers: %s. Dependencies may be incomplete.", err)
 			}
@@ -287,18 +300,18 @@ func GetDependencies(workspace string, a *v1alpha2.DockerArtifact) ([]string, er
 	return dependencies, nil
 }
 
-func processBaseImage(value *parser.Node) ([]string, error) {
-	base := value.Next.Value
-	logrus.Debugf("Checking base image %s for ONBUILD triggers.", base)
-	if strings.ToLower(base) == "scratch" {
-		logrus.Debugf("SCRATCH base image found, skipping check: %s", base)
+func processBaseImage(baseImageName string) ([]string, error) {
+	if strings.ToLower(baseImageName) == "scratch" {
 		return nil, nil
 	}
-	img, err := RetrieveImage(base)
+
+	logrus.Debugf("Checking base image %s for ONBUILD triggers.", baseImageName)
+	img, err := RetrieveImage(baseImageName)
 	if err != nil {
 		return nil, err
 	}
-	logrus.Debugf("Found onbuild triggers %v in image %s", img.Config.OnBuild, base)
+
+	logrus.Debugf("Found onbuild triggers %v in image %s", img.Config.OnBuild, baseImageName)
 	return img.Config.OnBuild, nil
 }
 

diff --git a/pkg/skaffold/docker/parse_test.go b/pkg/skaffold/docker/parse_test.go
@@ -140,6 +140,11 @@ COPY $FOO .
 CMD $FOO
 `
 
+const fromStage = `
+FROM ubuntu:14.04 as base
+FROM base as dist
+`
+
 var fooArg = "server.go" // used for build args
 
 var ImageConfigs = map[string]*v1.ConfigFile{
@@ -171,6 +176,10 @@ var ImageConfigs = map[string]*v1.ConfigFile{
 }
 
 func mockRetrieveImage(image string) (*v1.ConfigFile, error) {
+	if image == "base" {
+		panic("we should never retrieve images which are stage names")
+	}
+
 	if cfg, ok := ImageConfigs[image]; ok {
 		return cfg, nil
 	}
@@ -328,6 +337,12 @@ func TestGetDependencies(t *testing.T) {
 			buildArgs:   map[string]*string{"FOO": &fooArg},
 			expected:    []string{"Dockerfile", "server.go"},
 		},
+		{
+			description: "fromStage",
+			dockerfile:  fromStage,
+			workspace:   ".",
+			expected:    []string{"Dockerfile"},
+		},
 	}
 
 	RetrieveImage = mockRetrieveImage