From 0d2dc1477be71e5539e45f00266b10df1b2b88ef Mon Sep 17 00:00:00 2001 From: TomSweeneyRedHat Date: Fri, 16 Nov 2018 17:43:48 -0500 Subject: [PATCH] Handle 'COPY --from' in Dockerfile Signed-off-by: TomSweeneyRedHat Closes: #1181 Approved by: vrothberg --- imagebuildah/build.go | 93 ++++++++++++++++++- tests/bud.bats | 57 ++++++++++++ tests/bud/copy-from/Dockerfile | 2 + .../Dockerfile.index | 5 + .../Dockerfile.mixed | 13 +++ .../Dockerfile.name | 5 + tests/bud/multi-stage-builds/Dockerfile.mixed | 4 +- tests/bud/multi-stage-builds/Dockerfile.name | 2 +- 8 files changed, 175 insertions(+), 6 deletions(-) create mode 100644 tests/bud/copy-from/Dockerfile create mode 100644 tests/bud/multi-stage-builds-small-as/Dockerfile.index create mode 100644 tests/bud/multi-stage-builds-small-as/Dockerfile.mixed create mode 100644 tests/bud/multi-stage-builds-small-as/Dockerfile.name diff --git a/imagebuildah/build.go b/imagebuildah/build.go index 7182337788..2681bc198a 100644 --- a/imagebuildah/build.go +++ b/imagebuildah/build.go @@ -10,6 +10,7 @@ import ( "os" "os/exec" "path/filepath" + "regexp" "strconv" "strings" "time" @@ -242,7 +243,7 @@ var builtinAllowedBuildArgs = map[string]bool{ } // withName creates a new child executor that will be used whenever a COPY statement uses --from=NAME. -func (b *Executor) withName(name string, index int) *Executor { +func (b *Executor) withName(name string, index int, from string) *Executor { if b.named == nil { b.named = make(map[string]*Executor) } @@ -251,6 +252,7 @@ func (b *Executor) withName(name string, index int) *Executor { copied.name = name child := &copied b.named[name] = child + b.named[from] = child if idx := strconv.Itoa(index); idx != name { b.named[idx] = child } @@ -1258,8 +1260,16 @@ func (b *Executor) Build(ctx context.Context, stages imagebuilder.Stages) (strin b.imageMap = make(map[string]string) stageCount := 0 for _, stage := range stages { - stageExecutor = b.withName(stage.Name, stage.Position) - if err := stageExecutor.Prepare(ctx, stage, ""); err != nil { + ib := stage.Builder + node := stage.Node + base, err := ib.From(node) + if err != nil { + logrus.Debugf("Build(node.Children=%#v)", node.Children) + return "", nil, err + } + + stageExecutor = b.withName(stage.Name, stage.Position, base) + if err := stageExecutor.Prepare(ctx, stage, base); err != nil { return "", nil, err } // Always remove the intermediate/build containers, even if the build was unsuccessful. @@ -1398,6 +1408,9 @@ func BuildDockerfiles(ctx context.Context, store storage.Store, options BuildOpt dockerfiles = append(dockerfiles, data) } + + dockerfiles = processCopyFrom(dockerfiles) + mainNode, err := imagebuilder.ParseDockerfile(dockerfiles[0]) if err != nil { return "", nil, errors.Wrapf(err, "error parsing main Dockerfile") @@ -1421,6 +1434,80 @@ func BuildDockerfiles(ctx context.Context, store storage.Store, options BuildOpt return exec.Build(ctx, stages) } +// processCopyFrom goes through the Dockerfiles and handles any 'COPY --from' instances +// prepending a new FROM statement the Dockerfile that do not already have a corresponding +// FROM command within them. +func processCopyFrom(dockerfiles []io.ReadCloser) []io.ReadCloser { + + var newDockerfiles []io.ReadCloser + // fromMap contains the names of the images seen in a FROM + // line in the Dockerfiles. The boolean value just completes the map object. + fromMap := make(map[string]bool) + // asMap contains the names of the images seen after a "FROM image AS" + // line in the Dockefiles. The boolean value just completes the map object. + asMap := make(map[string]bool) + + copyRE := regexp.MustCompile(`\s*COPY\s+--from=`) + fromRE := regexp.MustCompile(`\s*FROM\s+`) + asRE := regexp.MustCompile(`(?i)\s+as\s+`) + for _, dfile := range dockerfiles { + if dfileBinary, err := ioutil.ReadAll(dfile); err == nil { + dfileString := fmt.Sprintf("%s", dfileBinary) + copyFromContent := copyRE.Split(dfileString, -1) + // no "COPY --from=", just continue + if len(copyFromContent) < 2 { + newDockerfiles = append(newDockerfiles, ioutil.NopCloser(strings.NewReader(dfileString))) + continue + } + // Load all image names in our Dockerfiles into a map + // for easy reference later. + fromContent := fromRE.Split(dfileString, -1) + for i := 0; i < len(fromContent); i++ { + imageName := strings.Split(fromContent[i], " ") + if len(imageName) > 0 { + finalImage := strings.Split(imageName[0], "\n") + if finalImage[0] != "" { + fromMap[strings.TrimSpace(finalImage[0])] = true + } + } + } + logrus.Debug("fromMap: ", fromMap) + + // Load all image names associated with an 'as' or 'AS' in + // our Dockerfiles into a map for easy reference later. + asContent := asRE.Split(dfileString, -1) + // Skip the first entry in the array as it's stuff before + // the " as " and we don't care. + for i := 1; i < len(asContent); i++ { + asName := strings.Split(asContent[i], " ") + if len(asName) > 0 { + finalAsImage := strings.Split(asName[0], "\n") + if finalAsImage[0] != "" { + asMap[strings.TrimSpace(finalAsImage[0])] = true + } + } + } + logrus.Debug("asMap: ", asMap) + + for i := 1; i < len(copyFromContent); i++ { + fromArray := strings.Split(copyFromContent[i], " ") + // If the image isn't a stage number or already declared, + // add a FROM statement for it to the top of our Dockerfile. + trimmedFrom := strings.TrimSpace(fromArray[0]) + _, okFrom := fromMap[trimmedFrom] + _, okAs := asMap[trimmedFrom] + _, err := strconv.Atoi(trimmedFrom) + if !okFrom && !okAs && err != nil { + from := "FROM " + trimmedFrom + newDockerfiles = append(newDockerfiles, ioutil.NopCloser(strings.NewReader(from))) + } + } + newDockerfiles = append(newDockerfiles, ioutil.NopCloser(strings.NewReader(dfileString))) + } // End if dfileBinary, err := ioutil.ReadAll(dfile); err == nil + } // End for _, dfile := range dockerfiles { + return newDockerfiles +} + // deleteSuccessfulIntermediateCtrs goes through the container IDs in b.containerIDs // and deletes the containers associated with that ID. func (b *Executor) deleteSuccessfulIntermediateCtrs() error { diff --git a/tests/bud.bats b/tests/bud.bats index abab8812e3..60ead7eee8 100644 --- a/tests/bud.bats +++ b/tests/bud.bats @@ -336,6 +336,50 @@ load helpers [ "$status" -eq 0 ] } +@test "bud-multi-stage-builds-small-as" { + target=multi-stage-index + buildah bud --signature-policy ${TESTSDIR}/policy.json -t ${target} -f ${TESTSDIR}/bud/multi-stage-builds-small-as/Dockerfile.index ${TESTSDIR}/bud/multi-stage-builds-small-as + cid=$(buildah from ${target}) + root=$(buildah mount ${cid}) + cmp $root/Dockerfile.index ${TESTSDIR}/bud/multi-stage-builds-small-as/Dockerfile.index + run test -s $root/etc/passwd + [ "$status" -eq 0 ] + buildah rm ${cid} + buildah rmi -a + run buildah --debug=false images -q + echo "$output" + [ "$status" -eq 0 ] + [ "$output" = "" ] + + target=multi-stage-name + buildah bud --signature-policy ${TESTSDIR}/policy.json -t ${target} -f Dockerfile.name ${TESTSDIR}/bud/multi-stage-builds-small-as + cid=$(buildah from ${target}) + root=$(buildah mount ${cid}) + cmp $root/Dockerfile.name ${TESTSDIR}/bud/multi-stage-builds-small-as/Dockerfile.name + run test -s $root/etc/passwd + [ "$status" -ne 0 ] + buildah rm ${cid} + buildah rmi $(buildah --debug=false images -q) + run buildah --debug=false images -q + echo "$output" + [ "$output" = "" ] + [ "$status" -eq 0 ] + + target=multi-stage-mixed + buildah bud --signature-policy ${TESTSDIR}/policy.json -t ${target} -f ${TESTSDIR}/bud/multi-stage-builds-small-as/Dockerfile.mixed ${TESTSDIR}/bud/multi-stage-builds-small-as + cid=$(buildah from ${target}) + root=$(buildah mount ${cid}) + cmp $root/Dockerfile.name ${TESTSDIR}/bud/multi-stage-builds-small-as/Dockerfile.name + cmp $root/Dockerfile.index ${TESTSDIR}/bud/multi-stage-builds-small-as/Dockerfile.index + cmp $root/Dockerfile.mixed ${TESTSDIR}/bud/multi-stage-builds-small-as/Dockerfile.mixed + buildah rm ${cid} + buildah rmi $(buildah --debug=false images -q) + run buildah --debug=false images -q + echo "$output" + [ "$output" = "" ] + [ "$status" -eq 0 ] +} + @test "bud-preserve-subvolumes" { # This Dockerfile needs us to be able to handle a working RUN instruction. if ! which runc ; then @@ -1078,3 +1122,16 @@ load helpers out=$(buildah bud --signature-policy ${TESTSDIR}/policy.json -t ${target} --build-arg foo=bar --build-arg foo2=bar2 -f ${TESTSDIR}/bud/build-arg ${TESTSDIR}/bud/build-arg | grep "Warning" | wc -l) [ "$out" -ne 0 ] } + +@test "bud with copy-from in Dockerfile no prior FROM" { + target=php-image + run buildah --debug=false bud --signature-policy ${TESTSDIR}/policy.json -t ${target} -f ${TESTSDIR}/bud/copy-from ${TESTSDIR}/bud/copy-from + echo "$output" + [ "$status" -eq 0 ] + + ctr=$(buildah --debug=false from --signature-policy ${TESTSDIR}/policy.json ${target}) + mnt=$(buildah --debug=false mount ${ctr}) + + run test -e $mnt/usr/local/bin/composer + [ "$status" -eq 0 ] +} diff --git a/tests/bud/copy-from/Dockerfile b/tests/bud/copy-from/Dockerfile new file mode 100644 index 0000000000..39145ae6fb --- /dev/null +++ b/tests/bud/copy-from/Dockerfile @@ -0,0 +1,2 @@ +FROM php:7.2 +COPY --from=composer:latest /usr/bin/composer /usr/local/bin/composer diff --git a/tests/bud/multi-stage-builds-small-as/Dockerfile.index b/tests/bud/multi-stage-builds-small-as/Dockerfile.index new file mode 100644 index 0000000000..f3ee8c7a6a --- /dev/null +++ b/tests/bud/multi-stage-builds-small-as/Dockerfile.index @@ -0,0 +1,5 @@ +FROM scratch +COPY Dockerfile.index / + +FROM alpine +COPY --from=0 /Dockerfile.index /Dockerfile.index diff --git a/tests/bud/multi-stage-builds-small-as/Dockerfile.mixed b/tests/bud/multi-stage-builds-small-as/Dockerfile.mixed new file mode 100644 index 0000000000..9323c23b35 --- /dev/null +++ b/tests/bud/multi-stage-builds-small-as/Dockerfile.mixed @@ -0,0 +1,13 @@ +FROM scratch as myname +COPY Dockerfile.name / + +FROM scratch as myname2 +COPY Dockerfile.index / + +FROM scratch +COPY Dockerfile.mixed / + +FROM scratch +COPY --from=myname /Dockerfile.name /Dockerfile.name +COPY --from=1 /Dockerfile.index /Dockerfile.index +COPY --from=2 /Dockerfile.mixed /Dockerfile.mixed diff --git a/tests/bud/multi-stage-builds-small-as/Dockerfile.name b/tests/bud/multi-stage-builds-small-as/Dockerfile.name new file mode 100644 index 0000000000..8908f5c829 --- /dev/null +++ b/tests/bud/multi-stage-builds-small-as/Dockerfile.name @@ -0,0 +1,5 @@ +FROM alpine as myname +COPY Dockerfile.name / + +FROM scratch +COPY --from=myname /Dockerfile.name /Dockerfile.name diff --git a/tests/bud/multi-stage-builds/Dockerfile.mixed b/tests/bud/multi-stage-builds/Dockerfile.mixed index 9323c23b35..b436a50936 100644 --- a/tests/bud/multi-stage-builds/Dockerfile.mixed +++ b/tests/bud/multi-stage-builds/Dockerfile.mixed @@ -1,7 +1,7 @@ -FROM scratch as myname +FROM scratch AS myname COPY Dockerfile.name / -FROM scratch as myname2 +FROM scratch AS myname2 COPY Dockerfile.index / FROM scratch diff --git a/tests/bud/multi-stage-builds/Dockerfile.name b/tests/bud/multi-stage-builds/Dockerfile.name index 8908f5c829..ab22ce9ebc 100644 --- a/tests/bud/multi-stage-builds/Dockerfile.name +++ b/tests/bud/multi-stage-builds/Dockerfile.name @@ -1,4 +1,4 @@ -FROM alpine as myname +FROM alpine AS myname COPY Dockerfile.name / FROM scratch