diff --git a/pkg/build/builder/common.go b/pkg/build/builder/common.go index 59fbeb3817e..81204fa756d 100644 --- a/pkg/build/builder/common.go +++ b/pkg/build/builder/common.go @@ -392,6 +392,21 @@ func addBuildParameters(dir string, build *buildapiv1.Build, sourceInfo *git.Sou return err } + // Arguments set before the first stage get discarded when we split the + // tree up into stages in replaceImagesFromSource. Other instructions + // are preserved. Preserve the arguments here so that we can prepend + // them to the replacement Dockerfile that we generate. + var preambleArgs []byte + for _, child := range node.Children { + if child.Value == dockercmd.Arg { + preambleArgs = append(preambleArgs, []byte(child.Original+"\n")...) + } else { + if child.Value == dockercmd.From { + break + } + } + } + // Update base image if build strategy specifies the From field. if build.Spec.Strategy.DockerStrategy != nil && build.Spec.Strategy.DockerStrategy.From != nil && build.Spec.Strategy.DockerStrategy.From.Kind == "DockerImage" { // Reduce the name to a minimal canonical form for the daemon @@ -425,11 +440,15 @@ func addBuildParameters(dir string, build *buildapiv1.Build, sourceInfo *git.Sou return err } - if err := replaceImagesFromSource(node, build.Spec.Source.Images); err != nil { + var buildArgs []corev1.EnvVar + if build.Spec.Strategy.DockerStrategy != nil { + buildArgs = build.Spec.Strategy.DockerStrategy.BuildArgs + } + if err := replaceImagesFromSource(node, build.Spec.Source.Images, buildArgs); err != nil { return err } - out := dockerfile.Write(node) + out := append(preambleArgs, dockerfile.Write(node)...) log.V(4).Infof("Replacing dockerfile\n%s\nwith:\n%s", string(in), string(out)) return overwriteFile(dockerfilePath, out) } @@ -437,7 +456,7 @@ func addBuildParameters(dir string, build *buildapiv1.Build, sourceInfo *git.Sou // replaceImagesFromSource updates a single or multi-stage Dockerfile with any replacement // image sources ('FROM ' and 'COPY --from='). It operates on exact string matches // and performs no interpretation of names from the Dockerfile. -func replaceImagesFromSource(node *parser.Node, imageSources []buildapiv1.ImageSource) error { +func replaceImagesFromSource(node *parser.Node, imageSources []buildapiv1.ImageSource, buildArgs []corev1.EnvVar) error { replacements := make(map[string]string) for _, image := range imageSources { if image.From.Kind != "DockerImage" || len(image.From.Name) == 0 { @@ -448,7 +467,11 @@ func replaceImagesFromSource(node *parser.Node, imageSources []buildapiv1.ImageS } } names := make(map[string]string) - stages, err := imagebuilder.NewStages(node, imagebuilder.NewBuilder(make(map[string]string))) + buildArgMap := make(map[string]string) + for _, ba := range buildArgs { + buildArgMap[ba.Name] = ba.Value + } + stages, err := imagebuilder.NewStages(node, imagebuilder.NewBuilder(buildArgMap)) if err != nil { return err } @@ -478,7 +501,7 @@ func replaceImagesFromSource(node *parser.Node, imageSources []buildapiv1.ImageS } // findReferencedImages returns all qualified images referenced by the Dockerfile, or returns an error. -func findReferencedImages(dockerfilePath string) ([]string, error) { +func findReferencedImages(dockerfilePath string, buildArgs []corev1.EnvVar) ([]string, error) { if len(dockerfilePath) == 0 { return nil, nil } @@ -488,7 +511,11 @@ func findReferencedImages(dockerfilePath string) ([]string, error) { } names := make(map[string]string) images := sets.NewString() - stages, err := imagebuilder.NewStages(node, imagebuilder.NewBuilder(make(map[string]string))) + buildArgMap := make(map[string]string) + for _, ba := range buildArgs { + buildArgMap[ba.Name] = ba.Value + } + stages, err := imagebuilder.NewStages(node, imagebuilder.NewBuilder(buildArgMap)) if err != nil { return nil, err } diff --git a/pkg/build/builder/common_test.go b/pkg/build/builder/common_test.go index a56be920bfb..2d5afacf843 100644 --- a/pkg/build/builder/common_test.go +++ b/pkg/build/builder/common_test.go @@ -205,6 +205,30 @@ func Test_addBuildParameters(t *testing.T) { `), want: want{ Out: heredoc.Doc(` + ARG GOLANG_CONTAINER=golang:latest + FROM $GOLANG_CONTAINER + RUN echo "hello world" + `), + }, + }, + { + // won't actually build: only ARG is allowed before the + // first FROM. preserving and then re-prepending ARG + // instructions effectively reorders them before any + // non-ARG instructions that also come before the first + // FROM. + original: heredoc.Doc(` + ARG GOLANG_CONTAINER=golang:latest + LABEL this=error + ARG GOLANG_CONTAINER2=golang:1.11 + FROM $GOLANG_CONTAINER + RUN echo "hello world" + `), + want: want{ + Out: heredoc.Doc(` + ARG GOLANG_CONTAINER=golang:latest + ARG GOLANG_CONTAINER2=golang:1.11 + LABEL this=error FROM $GOLANG_CONTAINER RUN echo "hello world" `), @@ -628,7 +652,7 @@ func Test_findReferencedImages(t *testing.T) { if _, err := dockerfile.Parse(strings.NewReader(test.original)); err != nil { t.Fatal(err) } - images, err := findReferencedImages(f.Name()) + images, err := findReferencedImages(f.Name(), nil) got := want{ Images: images, Err: err != nil, diff --git a/pkg/build/builder/docker.go b/pkg/build/builder/docker.go index 94c5393d3c1..b03f154ceb0 100644 --- a/pkg/build/builder/docker.go +++ b/pkg/build/builder/docker.go @@ -79,7 +79,7 @@ func (d *DockerBuilder) Build() error { buildTag := randomBuildTag(d.build.Namespace, d.build.Name) dockerfilePath := getDockerfilePath(buildDir, d.build) - imageNames, err := findReferencedImages(dockerfilePath) + imageNames, err := findReferencedImages(dockerfilePath, d.build.Spec.Strategy.DockerStrategy.BuildArgs) if err != nil { return err } @@ -95,7 +95,8 @@ func (d *DockerBuilder) Build() error { _, err = d.dockerClient.InspectImage(imageName) if err != nil { if err != docker.ErrNoSuchImage { - return err + log.V(4).Infof("\nError inspecting image \"%s\": %v, continuing", imageName, err) + continue } imageExists = false }