Detector changes for run image extension

acceptance/detector_test.go

@@ -362,11 +362,13 @@ fail: fail_detect_buildpack@some_version
 					"-extensions=/cnb/extensions",
 					"-generated=/layers/generated",
 					"-log-level=debug",
+					"-run=/layers/run.toml", // /cnb/run.toml is the default location of run.toml
 				),
 			)
 
 			t.Log("runs /bin/detect for buildpacks and extensions")
 			h.AssertStringContains(t, output, "Platform requested experimental feature 'Dockerfiles'")
+			h.AssertStringContains(t, output, "FOO=val-from-build-config")
 			h.AssertStringContains(t, output, "simple_extension: output from /bin/detect")
 			t.Log("writes group.toml")
 			foundGroupTOML := filepath.Join(copyDir, "layers", "group.toml")

acceptance/testdata/analyzer/container/cnb/run.toml

@@ -1,5 +1,5 @@
-[[image]]
+[[images]]
  image = "some-run-image-from-run-toml"
 
-[[image]]
+[[images]]
  image = "some-other-run-image"

acceptance/testdata/creator/container/cnb/run.toml

@@ -1,5 +1,5 @@
-[[image]]
+[[images]]
  image = "some-run-image-from-run-toml"
 
-[[image]]
+[[images]]
  image = "some-other-run-image"

acceptance/testdata/detector/container/cnb/build-config/env/FOO

@@ -0,0 +1 @@
+val-from-build-config

acceptance/testdata/detector/container/cnb/buildpacks/buildpack_for_ext/buildpack_for_ext_version/bin/detect

@@ -1,5 +1,8 @@
 #!/usr/bin/env bash
 
+echo "ENV"
+env
+
 plan_path=$2
 
 cat >> "${plan_path}" <<EOL

acceptance/testdata/detector/container/cnb/run.toml

@@ -0,0 +1,5 @@
+[[images]]
+ image = "some-run-image-from-extension"
+
+[[images]]
+ image = "some-other-run-image"

acceptance/testdata/detector/container/layers/analyzed.toml

@@ -0,0 +1,2 @@
+[run-image]
+reference = "some-old-run-image"

analyzer.go

@@ -189,15 +189,15 @@ func (f *AnalyzerFactory) setRun(analyzer *Analyzer, imageRef string) error {
 // Analyze fetches the layers metadata from the previous image and writes analyzed.toml.
 func (a *Analyzer) Analyze() (platform.AnalyzedMetadata, error) {
 	var (
-		err                      error
-		appMeta                  platform.LayersMetadata
-		cacheMeta                platform.CacheMetadata
-		previousImageIDReference string
-		runImageIDReference      string
+		err              error
+		appMeta          platform.LayersMetadata
+		cacheMeta        platform.CacheMetadata
+		previousImageRef string
+		runImageRef      string
 	)
 
 	if a.PreviousImage != nil { // Previous image is optional in Platform API >= 0.7
-		if previousImageIDReference, err = a.getImageIdentifier(a.PreviousImage); err != nil {
+		if previousImageRef, err = a.getImageIdentifier(a.PreviousImage); err != nil {
 			return platform.AnalyzedMetadata{}, errors.Wrap(err, "identifying previous image")
 		}
 
@@ -214,7 +214,7 @@ func (a *Analyzer) Analyze() (platform.AnalyzedMetadata, error) {
 	}
 
 	if a.RunImage != nil {
-		runImageIDReference, err = a.getImageIdentifier(a.RunImage)
+		runImageRef, err = a.getImageIdentifier(a.RunImage)
 		if err != nil {
 			return platform.AnalyzedMetadata{}, errors.Wrap(err, "identifying run image")
 		}
@@ -233,11 +233,9 @@ func (a *Analyzer) Analyze() (platform.AnalyzedMetadata, error) {
 	}
 
 	return platform.AnalyzedMetadata{
-		PreviousImage: &platform.ImageIdentifier{
-			Reference: previousImageIDReference,
-		},
-		RunImage: &platform.RunImage{Reference: runImageIDReference},
-		Metadata: appMeta,
+		PreviousImage: &platform.ImageIdentifier{Reference: previousImageRef},
+		RunImage:      &platform.RunImage{Reference: runImageRef},
+		Metadata:      appMeta,
 	}, nil
 }
 

api/apis.go

@@ -9,7 +9,7 @@ import (
 
 var (
 	Platform  = newApisMustParse([]string{"0.3", "0.4", "0.5", "0.6", "0.7", "0.8", "0.9", "0.10", "0.11", "0.12"}, []string{"0.3", "0.4", "0.5", "0.6"})
-	Buildpack = newApisMustParse([]string{"0.2", "0.3", "0.4", "0.5", "0.6", "0.7", "0.8", "0.9"}, []string{"0.2", "0.3", "0.4", "0.5", "0.6"})
+	Buildpack = newApisMustParse([]string{"0.2", "0.3", "0.4", "0.5", "0.6", "0.7", "0.8", "0.9", "0.10"}, []string{"0.2", "0.3", "0.4", "0.5", "0.6"})
 )
 
 type APIs struct {

buildpack/dockerfile.go

@@ -2,27 +2,45 @@ package buildpack
 
 import (
 	"bytes"
+	"errors"
 	"fmt"
 	"os"
 	"strings"
 
 	"github.com/moby/buildkit/frontend/dockerfile/instructions"
 	"github.com/moby/buildkit/frontend/dockerfile/parser"
 
+	"github.com/buildpacks/lifecycle/api"
 	"github.com/buildpacks/lifecycle/log"
 )
 
 const (
 	DockerfileKindBuild = "build"
 	DockerfileKindRun   = "run"
+
+	buildDockerfileName = "build.Dockerfile"
+	runDockerfileName   = "run.Dockerfile"
+
+	baseImageArgName = "base_image"
+	baseImageArgRef  = "${base_image}"
+
+	errArgumentsNotPermitted            = "run.Dockerfile should not expect arguments"
+	errBuildMissingRequiredARGCommand   = "build.Dockerfile did not start with required ARG command"
+	errBuildMissingRequiredFROMCommand  = "build.Dockerfile did not contain required FROM ${base_image} command"
+	errMissingRequiredStage             = "%s should have at least one stage"
+	errMissingRequiredInstruction       = "%s should have at least one instruction"
+	errMultiStageNotPermitted           = "%s is not permitted to use multistage build"
+	errRunOtherInstructionsNotPermitted = "run.Dockerfile is not permitted to have instructions other than FROM"
+	warnCommandNotRecommended           = "%s command %s on line %d is not recommended"
 )
 
-var permittedCommandsBuild = []string{"FROM", "ADD", "ARG", "COPY", "ENV", "LABEL", "RUN", "SHELL", "USER", "WORKDIR"}
+var permittedCommands = []string{"FROM", "ADD", "ARG", "COPY", "ENV", "LABEL", "RUN", "SHELL", "USER", "WORKDIR"}
 
 type DockerfileInfo struct {
 	ExtensionID string
 	Kind        string
 	Path        string
+	NewBase     string
 }
 
 type ExtendConfig struct {
@@ -64,80 +82,113 @@ func VerifyBuildDockerfile(dockerfile string, logger log.Logger) error {
 
 	// validate only 1 FROM
 	if len(stages) > 1 {
-		return fmt.Errorf("build.Dockerfile is not permitted to use multistage build")
+		return fmt.Errorf(errMultiStageNotPermitted, buildDockerfileName)
 	}
 
 	// validate only permitted Commands
 	for _, stage := range stages {
 		for _, command := range stage.Commands {
 			found := false
-			for _, permittedCommand := range permittedCommandsBuild {
+			for _, permittedCommand := range permittedCommands {
 				if permittedCommand == strings.ToUpper(command.Name()) {
 					found = true
 					break
 				}
 			}
 			if !found {
-				logger.Warnf("build.Dockerfile command %s on line %d is not recommended", strings.ToUpper(command.Name()), command.Location()[0].Start.Line)
+				logger.Warnf(warnCommandNotRecommended, buildDockerfileName, strings.ToUpper(command.Name()), command.Location()[0].Start.Line)
 			}
 		}
 	}
 
 	// validate build.Dockerfile preamble
 	if len(margs) != 1 {
-		return fmt.Errorf("build.Dockerfile did not start with required ARG command")
+		return errors.New(errBuildMissingRequiredARGCommand)
 	}
-	if margs[0].Args[0].Key != "base_image" {
-		return fmt.Errorf("build.Dockerfile did not start with required ARG base_image command")
+	if margs[0].Args[0].Key != baseImageArgName {
+		return errors.New(errBuildMissingRequiredARGCommand)
 	}
 	// sanity check to prevent panic
 	if len(stages) == 0 {
-		return fmt.Errorf("build.Dockerfile should have at least one stage")
+		return fmt.Errorf(errMissingRequiredStage, buildDockerfileName)
 	}
-	if stages[0].BaseName != "${base_image}" {
-		return fmt.Errorf("build.Dockerfile did not contain required FROM ${base_image} command")
+
+	if stages[0].BaseName != baseImageArgRef {
+		return errors.New(errBuildMissingRequiredFROMCommand)
 	}
 
 	return nil
 }
 
-func VerifyRunDockerfile(dockerfile string) error {
+func VerifyRunDockerfile(dockerfile string, buildpackAPI *api.Version, logger log.Logger) (string, error) {
+	if buildpackAPI.LessThan("0.10") {
+		return verifyRunDockerfile09(dockerfile)
+	}
+	return verifyRunDockerfile(dockerfile, logger)
+}
+
+func verifyRunDockerfile(dockerfile string, logger log.Logger) (string, error) {
+	stages, _, err := parseDockerfile(dockerfile)
+	if err != nil {
+		return "", err
+	}
+
+	// validate only 1 FROM
+	if len(stages) > 1 {
+		return "", fmt.Errorf(errMultiStageNotPermitted, runDockerfileName)
+	}
+	if len(stages) == 0 {
+		return "", fmt.Errorf(errMissingRequiredStage, runDockerfileName)
+	}
+
+	var newBase string
+	// validate only permitted Commands
+	for _, stage := range stages {
+		if stage.BaseName != baseImageArgRef {
+			newBase = stage.BaseName
+		}
+		for _, command := range stage.Commands {
+			found := false
+			for _, permittedCommand := range permittedCommands {
+				if permittedCommand == strings.ToUpper(command.Name()) {
+					found = true
+					break
+				}
+			}
+			if !found {
+				logger.Warnf(warnCommandNotRecommended, runDockerfileName, strings.ToUpper(command.Name()), command.Location()[0].Start.Line)
+			}
+		}
+	}
+
+	return newBase, nil
+}
+
+func verifyRunDockerfile09(dockerfile string) (string, error) {
 	stages, margs, err := parseDockerfile(dockerfile)
 	if err != nil {
-		return err
+		return "", err
 	}
 
 	// validate only 1 FROM
 	if len(stages) > 1 {
-		return fmt.Errorf("run.Dockerfile is not permitted to use multistage build")
+		return "", fmt.Errorf(errMultiStageNotPermitted, runDockerfileName)
 	}
 
 	// validate FROM does not expect argument
 	if len(margs) > 0 {
-		return fmt.Errorf("run.Dockerfile should not expect arguments")
+		return "", errors.New(errArgumentsNotPermitted)
 	}
 
 	// sanity check to prevent panic
 	if len(stages) == 0 {
-		return fmt.Errorf("run.Dockerfile should have at least one stage")
+		return "", fmt.Errorf(errMissingRequiredStage, runDockerfileName)
 	}
 
 	// validate no instructions in stage
 	if len(stages[0].Commands) != 0 {
-		return fmt.Errorf("run.Dockerfile is not permitted to have instructions other than FROM")
+		return "", fmt.Errorf(errRunOtherInstructionsNotPermitted)
 	}
 
-	return nil
-}
-
-func RetrieveFirstFromImageNameFromDockerfile(dockerfile string) (string, error) {
-	ins, _, err := parseDockerfile(dockerfile)
-	if err != nil {
-		return "", err
-	}
-	// sanity check to prevent panic
-	if len(ins) == 0 {
-		return "", fmt.Errorf("expected at least one instruction")
-	}
-	return ins[0].BaseName, nil
+	return stages[0].BaseName, nil
 }