Skip to content

Commit

Permalink
buildah: add heredoc support for RUN, COPY and ADD
Browse files Browse the repository at this point in the history
Following PR is a attempt to add `Heredoc` support to buildah.

Once this PR is merged buildah is supposed to honor heredoc syntax while
processing containerfiles

Expected syntax to work

```Dockerfile
FROM docker.io/library/python:latest
RUN <<EOF
echo "Hello" >> /hello
echo "World!" >> /hello
EOF

RUN python3 <<EOF
with open("/hello", "w") as f:
    print("Hello", file=f)
    print("Something", file=f)
EOF

RUN ls -a
RUN cat hello
```

Signed-off-by: Aditya R <arajan@redhat.com>
  • Loading branch information
flouthoc committed Nov 16, 2023
1 parent 7cd1f62 commit 34a2111
Show file tree
Hide file tree
Showing 9 changed files with 288 additions and 3 deletions.
2 changes: 1 addition & 1 deletion go.mod
Expand Up @@ -18,6 +18,7 @@ require (
github.com/fsouza/go-dockerclient v1.9.7
github.com/hashicorp/go-multierror v1.1.1
github.com/mattn/go-shellwords v1.0.12
github.com/moby/buildkit v0.10.6
github.com/onsi/ginkgo v1.16.5
github.com/onsi/gomega v1.29.0
github.com/opencontainers/go-digest v1.0.0
Expand Down Expand Up @@ -98,7 +99,6 @@ require (
github.com/miekg/pkcs11 v1.1.1 // indirect
github.com/mistifyio/go-zfs/v3 v3.0.1 // indirect
github.com/mitchellh/mapstructure v1.5.0 // indirect
github.com/moby/buildkit v0.10.6 // indirect
github.com/moby/patternmatcher v0.5.0 // indirect
github.com/moby/sys/mountinfo v0.6.2 // indirect
github.com/moby/sys/sequential v0.5.0 // indirect
Expand Down
119 changes: 117 additions & 2 deletions imagebuildah/stage_executor.go
Expand Up @@ -7,6 +7,7 @@ import (
"fmt"
"io"
"os"
"path"
"path/filepath"
"sort"
"strconv"
Expand Down Expand Up @@ -35,6 +36,7 @@ import (
"github.com/containers/storage/pkg/chrootarchive"
"github.com/containers/storage/pkg/unshare"
docker "github.com/fsouza/go-dockerclient"
buildkitparser "github.com/moby/buildkit/frontend/dockerfile/parser"
digest "github.com/opencontainers/go-digest"
v1 "github.com/opencontainers/image-spec/specs-go/v1"
"github.com/opencontainers/runtime-spec/specs-go"
Expand Down Expand Up @@ -348,6 +350,7 @@ func (s *StageExecutor) volumeCacheRestore() error {
// imagebuilder tells us the instruction was "ADD" and not "COPY".
func (s *StageExecutor) Copy(excludes []string, copies ...imagebuilder.Copy) error {
s.builder.ContentDigester.Restart()
copiesExtend := []imagebuilder.Copy{}
for _, copy := range copies {
if err := s.volumeCacheInvalidate(copy.Dest); err != nil {
return err
Expand All @@ -362,7 +365,61 @@ func (s *StageExecutor) Copy(excludes []string, copies ...imagebuilder.Copy) err
stripSetgid := false
preserveOwnership := false
contextDir := s.executor.contextDir
if len(copy.From) > 0 {
// If we are copying files via heredoc syntax, then
// its time to create these temporary files on host
// and copy these to container
if len(copy.Files) > 0 {
// If we are copying files from heredoc syntax, there
// maybe regular files from context as well so split and
// process them differently
if len(copy.Src) > len(copy.Files) {
regularSources := []string{}
for _, src := range copy.Src {
// If this source is not a heredoc, then it is a regular file from
// build context or from another stage (`--from=`) so treat this differently.
if !strings.HasPrefix(src, "<<") {
regularSources = append(regularSources, src)
}
}
copyEntry := copy
// Remove heredoc if any, since we are already processing them
// so create new entry with sources containing regular files
// only, since regular files can have different context then
// heredoc files.
copyEntry.Files = nil
copyEntry.Src = regularSources
copiesExtend = append(copiesExtend, copyEntry)
}
copySources := []string{}
for _, file := range copy.Files {
data := file.Data
// remove first break line added while parsing heredoc
data = strings.TrimPrefix(data, "\n")
// add breakline when heredoc ends for docker compat
data = data + "\n"
tmpFile, err := os.Create(filepath.Join(parse.GetTempDir(), path.Base(filepath.ToSlash(file.Name))))
if err != nil {
return fmt.Errorf("unable to create tmp file for COPY instruction at %q: %w", parse.GetTempDir(), err)
}
err = tmpFile.Chmod(0644) // 644 is consistent with buildkit
if err != nil {
tmpFile.Close()
return fmt.Errorf("unable to chmod tmp file created for COPY instruction at %q: %w", tmpFile.Name(), err)
}
defer os.Remove(tmpFile.Name())
_, err = tmpFile.WriteString(data)
if err != nil {
tmpFile.Close()
return fmt.Errorf("unable to write contents of heredoc file at %q: %w", tmpFile.Name(), err)
}
copySources = append(copySources, filepath.Base(tmpFile.Name()))
tmpFile.Close()
}
contextDir = parse.GetTempDir()
copy.Src = copySources
}

if len(copy.From) > 0 && len(copy.Files) == 0 {
// If from has an argument within it, resolve it to its
// value. Otherwise just return the value found.
from, fromErr := imagebuilder.ProcessWord(copy.From, s.stage.Builder.Arguments())
Expand Down Expand Up @@ -485,6 +542,13 @@ func (s *StageExecutor) Copy(excludes []string, copies ...imagebuilder.Copy) err
return err
}
}
if len(copiesExtend) > 0 {
// If we found heredocs and regularfiles together
// in same statement then we produced new copies to
// process regular files separately since they need
// different context.
return s.Copy(excludes, copiesExtend...)
}
return nil
}

Expand Down Expand Up @@ -590,10 +654,59 @@ func (s *StageExecutor) runStageMountPoints(mountList []string) (map[string]inte
return stageMountPoints, nil
}

func (s *StageExecutor) createNeededHeredocMountsForRun(files []imagebuilder.File) ([]Mount, error) {
mountResult := []Mount{}
for _, file := range files {
f, err := os.CreateTemp("", "buildahheredoc")
if err != nil {
return nil, err
}
if _, err := f.WriteString(file.Data); err != nil {
f.Close()
return nil, err
}
err = f.Chmod(0755)
if err != nil {
f.Close()
return nil, err
}
// dest path is same as buildkit for compat
dest := filepath.Join("/dev/pipes/", filepath.Base(f.Name()))
mount := Mount{Destination: dest, Type: define.TypeBind, Source: f.Name(), Options: append(define.BindOptions, "rprivate", "z", "Z")}
mountResult = append(mountResult, mount)
f.Close()
}
return mountResult, nil
}

// Run executes a RUN instruction using the stage's current working container
// as a root directory.
func (s *StageExecutor) Run(run imagebuilder.Run, config docker.Config) error {
logrus.Debugf("RUN %#v, %#v", run, config)
args := run.Args
heredocMounts := []Mount{}
if len(run.Files) > 0 {
if heredoc := buildkitparser.MustParseHeredoc(args[0]); heredoc != nil {
if strings.HasPrefix(run.Files[0].Data, "#!") || strings.HasPrefix(run.Files[0].Data, "\n#!") {
// This is a single heredoc with a shebang, so create a file
// and run it.
heredocMount, err := s.createNeededHeredocMountsForRun(run.Files)
if err != nil {
return err
}
args = []string{heredocMount[0].Destination}
heredocMounts = append(heredocMounts, heredocMount...)
} else {
args = []string{run.Files[0].Data}
}
} else {
full := args[0]
for _, file := range run.Files {
full += file.Data + "\n" + file.Name
}
args = []string{full}
}
}
stageMountPoints, err := s.runStageMountPoints(run.Mounts)
if err != nil {
return err
Expand Down Expand Up @@ -657,7 +770,6 @@ func (s *StageExecutor) Run(run imagebuilder.Run, config docker.Config) error {
options.ConfigureNetwork = buildah.NetworkDisabled
}

args := run.Args
if run.Shell {
if len(config.Shell) > 0 && s.builder.Format == define.Dockerv2ImageManifest {
args = append(config.Shell, args...)
Expand All @@ -670,6 +782,9 @@ func (s *StageExecutor) Run(run imagebuilder.Run, config docker.Config) error {
return err
}
options.Mounts = append(options.Mounts, mounts...)
if len(heredocMounts) > 0 {
options.Mounts = append(options.Mounts, heredocMounts...)
}
err = s.builder.Run(args, options)
if err2 := s.volumeCacheRestore(); err2 != nil {
if err == nil {
Expand Down
35 changes: 35 additions & 0 deletions tests/bud.bats
Expand Up @@ -267,6 +267,41 @@ _EOF
run_buildah 1 run myctr ls -l subdir/
}

@test "bud build with heredoc content" {
run_buildah build -t heredoc $WITH_POLICY_JSON -f $BUDFILES/heredoc/Containerfile .
expect_output --substring "print first line from heredoc"
expect_output --substring "print second line from heredoc"
expect_output --substring "Heredoc writing first file"
expect_output --substring "some text of first file"
expect_output --substring "file2 from python"
expect_output --substring "(your index page goes here)"
expect_output --substring "(robots content)"
expect_output --substring "(humans content)"
expect_output --substring "this is the output of test6 part1"
expect_output --substring "this is the output of test6 part2"
expect_output --substring "this is the output of test7 part1"
expect_output --substring "this is the output of test7 part2"
expect_output --substring "this is the output of test7 part3"
expect_output --substring "this is the output of test8 part1"
expect_output --substring "this is the output of test8 part2"
}

@test "bud build with heredoc content which is a bash file" {
skip_if_in_container
_prefetch busybox
run_buildah build -t heredoc $WITH_POLICY_JSON -f $BUDFILES/heredoc/Containerfile.bash_file .
expect_output --substring "this is the output of test9"
expect_output --substring "this is the output of test10"
}

@test "bud build with heredoc verify mount leak" {
skip_if_in_container
_prefetch alpine
run_buildah 1 build -t heredoc $WITH_POLICY_JSON -f $BUDFILES/heredoc/Containerfile.verify_mount_leak .
expect_output --substring "this is the output of test"
expect_output --substring "ls: /dev/pipes: No such file or directory"
}

@test "bud with .containerignore" {
_prefetch alpine busybox
run_buildah 125 build -t testbud $WITH_POLICY_JSON -f $BUDFILES/containerignore/Dockerfile $BUDFILES/containerignore
Expand Down
59 changes: 59 additions & 0 deletions tests/bud/heredoc/Containerfile
@@ -0,0 +1,59 @@
FROM docker.io/library/python:latest

RUN <<EOF
echo "print first line from heredoc"
echo "print second line from heredoc"
EOF

RUN <<EOF
echo "Heredoc writing first file" >> /file1
echo "some text of first file" >> /file1
EOF

RUN cat file1

RUN python3 <<EOF
with open("/file2", "w") as f:
print("file2 from python", file=f)
EOF

RUN cat file2

ADD <<EOF /index.html
(your index page goes here)
EOF

RUN cat index.html

COPY <<robots.txt <<humans.txt /test/
(robots content)
robots.txt
(humans content)
humans.txt

RUN cat /proc/self/fd/5 /proc/self/fd/6 5<<FILE1 6<<FILE2 > test6.txt
this is the output of test6 part1
FILE1
this is the output of test6 part2
FILE2

RUN 5<<file cat /proc/self/fd/5 /proc/self/fd/6 6<<FILE | cat /dev/stdin /proc/self/fd/6 6<<File > test7.txt
this is the output of test7 part1
file
this is the output of test7 part2
FILE
this is the output of test7 part3
File

RUN <<FILE1 cat > test8.1 && <<FILE2 cat > test8.2
this is the output of test8 part1
FILE1
this is the output of test8 part2
FILE2

RUN cat /test/robots.txt
RUN cat /test/humans.txt
RUN cat test6.txt
RUN cat test7.txt
RUN cat test8.1
RUN cat test8.2
15 changes: 15 additions & 0 deletions tests/bud/heredoc/Containerfile.bash_file
@@ -0,0 +1,15 @@
FROM busybox
RUN <<EOF
#!/bin/sh
echo "
this is the output of test9" > test9.txt
EOF

RUN <<-EOF
#!/bin/sh
echo "
this is the output of test10" > test10.txt
EOF

RUN cat test9.txt
RUN cat test10.txt
17 changes: 17 additions & 0 deletions tests/bud/heredoc/Containerfile.verify_mount_leak
@@ -0,0 +1,17 @@
FROM alpine

RUN <<EOF
#!/bin/sh
echo "
this is the output of test" > test.txt
# Mount of this file must exists till this run step
# so this `ls` command should not fail
ls -a /dev/pipes/
EOF

RUN cat test.txt

# This ls command must fail, since mount is removed in this step
RUN ls -a /dev/pipes


20 changes: 20 additions & 0 deletions tests/conformance/conformance_test.go
Expand Up @@ -3003,6 +3003,26 @@ var internalTestCases = []testCase{
dockerUseBuildKit: true,
},

{
name: "heredoc-copy",
dockerfile: "Dockerfile.heredoc_copy",
dockerUseBuildKit: true,
contextDir: "heredoc",
fsSkip: []string{"(dir):test:mtime",
"(dir):test2:mtime",
"(dir):test:(dir):humans.txt:mtime",
"(dir):test:(dir):robots.txt:mtime",
"(dir):test:(dir):file:mtime",
"(dir):test:(dir):file:uid", /* buildkit's regular COPY sets UID which is inconsistent with docker and buildah*/
"(dir):test:(dir):file:gid", /* buildkit's regular COPY sets GID which is inconsistent with docker and buildah*/
"(dir):test2:(dir):humans.txt:mtime",
"(dir):test2:(dir):robots.txt:mtime",
"(dir):test2:(dir):image_file:mtime",
"(dir):test2:(dir):image_file:uid", /* buildkit's regular COPY sets UID which is inconsistent with docker and buildah*/
"(dir):test2:(dir):image_file:gid", /* buildkit's regular COPY sets GID which is inconsistent with docker and buildah*/
"(dir):etc:(dir):hostname" /* buildkit does not contains /etc/hostname like buildah */},
},

{
name: "replace-symlink-with-directory",
contextDir: "replace/symlink-with-directory",
Expand Down
23 changes: 23 additions & 0 deletions tests/conformance/testdata/heredoc/Dockerfile.heredoc_copy
@@ -0,0 +1,23 @@
# syntax=docker/dockerfile:1.3-labs
FROM busybox as one
RUN echo helloworld > image_file
FROM busybox
RUN echo hello
# copy two heredoc and one from context
COPY <<robots.txt <<humans.txt file /test/
(robots content)
Long file with random text
Random line
HelloWorld
robots.txt
(humans content)
humans.txt
# copy two heredoc and one from another stage
COPY --from=one image_file <<robots.txt <<humans.txt /test2/
(robots content)
Long file with random text
Random line
HelloWorld
robots.txt
(humans content)
humans.txt
1 change: 1 addition & 0 deletions tests/conformance/testdata/heredoc/file
@@ -0,0 +1 @@
somefile

0 comments on commit 34a2111

Please sign in to comment.