Merge pull request #2209 from jedevc/dockerfile-heredocs-onbuild

Add support for heredocs in the ONBUILD command
moby · Jun 30, 2021 · 921b0de · 921b0de
2 parents bb6f11c + 5ad1ff3
commit 921b0de
Show file tree

Hide file tree

Showing 4 changed files with 120 additions and 10 deletions.
diff --git a/frontend/dockerfile/dockerfile_heredoc_test.go b/frontend/dockerfile/dockerfile_heredoc_test.go
@@ -3,6 +3,7 @@
 package dockerfile
 
 import (
+	"fmt"
 	"io/ioutil"
 	"os"
 	"path/filepath"
@@ -12,6 +13,7 @@ import (
 	"github.com/moby/buildkit/client"
 	"github.com/moby/buildkit/frontend/dockerfile/builder"
 	"github.com/moby/buildkit/util/testutil/integration"
+	"github.com/pkg/errors"
 	"github.com/stretchr/testify/require"
 )
 
@@ -23,6 +25,7 @@ var hdTests = []integration.Test{
 	testRunComplexHeredoc,
 	testHeredocIndent,
 	testHeredocVarSubstitution,
+	testOnBuildHeredoc,
 }
 
 func init() {
@@ -500,3 +503,92 @@ COPY --from=build /dest /
 		require.Equal(t, content, string(dt))
 	}
 }
+
+func testOnBuildHeredoc(t *testing.T, sb integration.Sandbox) {
+	f := getFrontend(t, sb)
+
+	registry, err := sb.NewRegistry()
+	if errors.Is(err, integration.ErrorRequirements) {
+		t.Skip(err.Error())
+	}
+	require.NoError(t, err)
+
+	dockerfile := []byte(`
+FROM busybox
+ONBUILD RUN <<EOF
+echo "hello world" >> /dest
+EOF
+`)
+
+	dir, err := tmpdir(
+		fstest.CreateFile("Dockerfile", dockerfile, 0600),
+	)
+	require.NoError(t, err)
+	defer os.RemoveAll(dir)
+
+	c, err := client.New(sb.Context(), sb.Address())
+	require.NoError(t, err)
+	defer c.Close()
+
+	target := registry + "/buildkit/testonbuildheredoc:base"
+	_, err = f.Solve(sb.Context(), c, client.SolveOpt{
+		Exports: []client.ExportEntry{
+			{
+				Type: client.ExporterImage,
+				Attrs: map[string]string{
+					"push": "true",
+					"name": target,
+				},
+			},
+		},
+		LocalDirs: map[string]string{
+			builder.DefaultLocalNameDockerfile: dir,
+			builder.DefaultLocalNameContext:    dir,
+		},
+	}, nil)
+	require.NoError(t, err)
+
+	dockerfile = []byte(fmt.Sprintf(`
+	FROM %s 
+	`, target))
+
+	dir, err = tmpdir(
+		fstest.CreateFile("Dockerfile", dockerfile, 0600),
+	)
+	require.NoError(t, err)
+	defer os.RemoveAll(dir)
+
+	dockerfile = []byte(fmt.Sprintf(`
+	FROM %s AS base
+	FROM scratch
+	COPY --from=base /dest /dest
+	`, target))
+
+	dir, err = tmpdir(
+		fstest.CreateFile("Dockerfile", dockerfile, 0600),
+	)
+	require.NoError(t, err)
+	defer os.RemoveAll(dir)
+
+	destDir, err := ioutil.TempDir("", "buildkit")
+	require.NoError(t, err)
+	defer os.RemoveAll(destDir)
+
+	_, err = f.Solve(sb.Context(), c, client.SolveOpt{
+		Exports: []client.ExportEntry{
+			{
+				Type:      client.ExporterLocal,
+				OutputDir: destDir,
+			},
+		},
+		LocalDirs: map[string]string{
+			builder.DefaultLocalNameDockerfile: dir,
+			builder.DefaultLocalNameContext:    dir,
+		},
+	}, nil)
+	require.NoError(t, err)
+
+	dt, err := ioutil.ReadFile(filepath.Join(destDir, "dest"))
+	require.NoError(t, err)
+	require.Equal(t, "hello world\n", string(dt))
+}
diff --git a/frontend/dockerfile/instructions/parse.go b/frontend/dockerfile/instructions/parse.go
@@ -381,11 +381,14 @@ func parseOnBuild(req parseRequest) (*OnbuildCommand, error) {
 	}
 
 	original := regexp.MustCompile(`(?i)^\s*ONBUILD\s*`).ReplaceAllString(req.original, "")
+	for _, heredoc := range req.heredocs {
+		original += "\n" + heredoc.Content + heredoc.Name
+	}
+
 	return &OnbuildCommand{
 		Expression:      original,
 		withNameAndCode: newWithNameAndCode(req),
 	}, nil
-
 }
 
 func parseWorkdir(req parseRequest) (*WorkdirCommand, error) {

diff --git a/frontend/dockerfile/parser/parser.go b/frontend/dockerfile/parser/parser.go
@@ -77,10 +77,17 @@ func (node *Node) lines(start, end int) {
 }
 
 func (node *Node) canContainHeredoc() bool {
-	if _, allowedDirective := heredocDirectives[node.Value]; !allowedDirective {
+	// check for compound commands, like ONBUILD
+	if ok := heredocCompoundDirectives[node.Value]; ok {
+		if node.Next != nil && len(node.Next.Children) > 0 {
+			node = node.Next.Children[0]
+		}
+	}
+
+	if ok := heredocDirectives[node.Value]; !ok {
 		return false
 	}
-	if _, isJSON := node.Attributes["json"]; isJSON {
+	if isJSON := node.Attributes["json"]; isJSON {
 		return false
 	}
 
@@ -106,13 +113,12 @@ type Heredoc struct {
 }
 
 var (
-	dispatch          map[string]func(string, *directives) (*Node, map[string]bool, error)
-	heredocDirectives map[string]bool
-	reWhitespace      = regexp.MustCompile(`[\t\v\f\r ]+`)
-	reDirectives      = regexp.MustCompile(`^#\s*([a-zA-Z][a-zA-Z0-9]*)\s*=\s*(.+?)\s*$`)
-	reComment         = regexp.MustCompile(`^#.*$`)
-	reHeredoc         = regexp.MustCompile(`^(\d*)<<(-?)(['"]?)([a-zA-Z][a-zA-Z0-9]*)(['"]?)$`)
-	reLeadingTabs     = regexp.MustCompile(`(?m)^\t+`)
+	dispatch      map[string]func(string, *directives) (*Node, map[string]bool, error)
+	reWhitespace  = regexp.MustCompile(`[\t\v\f\r ]+`)
+	reDirectives  = regexp.MustCompile(`^#\s*([a-zA-Z][a-zA-Z0-9]*)\s*=\s*(.+?)\s*$`)
+	reComment     = regexp.MustCompile(`^#.*$`)
+	reHeredoc     = regexp.MustCompile(`^(\d*)<<(-?)(['"]?)([a-zA-Z][a-zA-Z0-9]*)(['"]?)$`)
+	reLeadingTabs = regexp.MustCompile(`(?m)^\t+`)
 )
 
 // DefaultEscapeToken is the default escape token
@@ -123,6 +129,11 @@ var validDirectives = map[string]struct{}{
 	"syntax": {},
 }
 
+var (
+	heredocDirectives         map[string]bool // directives allowed to contain heredocs
+	heredocCompoundDirectives map[string]bool // directives allowed to contain directives containing heredocs
+)
+
 // directive is the structure used during a build run to hold the state of
 // parsing directives.
 type directives struct {

diff --git a/frontend/dockerfile/parser/parser_heredoc.go b/frontend/dockerfile/parser/parser_heredoc.go
@@ -10,4 +10,8 @@ func init() {
 		command.Copy: true,
 		command.Run:  true,
 	}
+
+	heredocCompoundDirectives = map[string]bool{
+		command.Onbuild: true,
+	}
 }