From de7fcce614aa1445b337890f65f5c12f125c491d Mon Sep 17 00:00:00 2001 From: Justin Chadwell Date: Wed, 9 Jun 2021 10:50:31 +0100 Subject: [PATCH] Integrate heredoc support into ADD/COPY and RUN This modifies the command structures to support inline files, as well as provides the logic to compile them down into appropriate LLB definitions. Signed-off-by: Justin Chadwell --- frontend/dockerfile/dockerfile2llb/convert.go | 93 ++++++- frontend/dockerfile/instructions/commands.go | 62 ++++- frontend/dockerfile/instructions/parse.go | 106 +++++++- .../instructions/parse_heredoc_test.go | 245 ++++++++++++++++++ .../dockerfile/instructions/parse_test.go | 10 +- 5 files changed, 474 insertions(+), 42 deletions(-) create mode 100644 frontend/dockerfile/instructions/parse_heredoc_test.go diff --git a/frontend/dockerfile/dockerfile2llb/convert.go b/frontend/dockerfile/dockerfile2llb/convert.go index 02f08f48..c26d0856 100644 --- a/frontend/dockerfile/dockerfile2llb/convert.go +++ b/frontend/dockerfile/dockerfile2llb/convert.go @@ -507,7 +507,7 @@ func dispatch(d *dispatchState, cmd command, opt dispatchOpt) error { case *instructions.AddCommand: err = dispatchCopy(d, c.SourcesAndDest, opt.buildContext, true, c, c.Chown, c.Chmod, c.Location(), opt) if err == nil { - for _, src := range c.Sources() { + for _, src := range c.SourcePaths { if !strings.HasPrefix(src, "http://") && !strings.HasPrefix(src, "https://") { d.ctxPaths[path.Join("/", filepath.ToSlash(src))] = struct{}{} } @@ -542,7 +542,7 @@ func dispatch(d *dispatchState, cmd command, opt dispatchOpt) error { } err = dispatchCopy(d, c.SourcesAndDest, l, false, c, c.Chown, c.Chmod, c.Location(), opt) if err == nil && len(cmd.sources) == 0 { - for _, src := range c.Sources() { + for _, src := range c.SourcePaths { d.ctxPaths[path.Join("/", filepath.ToSlash(src))] = struct{}{} } } @@ -647,15 +647,63 @@ func dispatchEnv(d *dispatchState, c *instructions.EnvCommand) error { } func dispatchRun(d *dispatchState, c *instructions.RunCommand, proxy *llb.ProxyEnv, sources []*dispatchState, dopt dispatchOpt) error { + var opt []llb.RunOption + var args []string = c.CmdLine + if len(c.Files) > 0 { + if len(args) != 1 { + return fmt.Errorf("parsing produced an invalid run command: %v", args) + } + + if heredoc := parser.MustParseHeredoc(args[0]); heredoc != nil { + if d.image.OS != "windows" && strings.HasPrefix(c.Files[0].Data, "#!") { + // This is a single heredoc with a shebang, so create a file + // and run it. + // NOTE: choosing to expand doesn't really make sense here, so + // we silently ignore that option if it was provided. + sourcePath := "/" + destPath := "/dev/pipes/" + + f := c.Files[0].Name + data := c.Files[0].Data + if c.Files[0].Chomp { + data = parser.ChompHeredocContent(data) + } + st := llb.Scratch().Dir(sourcePath).File(llb.Mkfile(f, 0755, []byte(data))) + + mount := llb.AddMount(destPath, st, llb.SourcePath(sourcePath), llb.Readonly) + opt = append(opt, mount) + + args[0] = path.Join(destPath, f) + } else { + // Just a simple heredoc, so just run the contents in the + // shell: this creates the effect of a "fake"-heredoc, so that + // the syntax can still be used for shells that don't support + // heredocs directly. + // NOTE: like above, we ignore the expand option. + data := c.Files[0].Data + if c.Files[0].Chomp { + data = parser.ChompHeredocContent(data) + } + args[0] = data + } + } else { + // More complex heredoc, so reconstitute it, and pass it to the + // shell to handle. + for _, file := range c.Files { + args[0] += "\n" + file.Data + file.Name + } + } + } if c.PrependShell { args = withShell(d.image, args) } + env, err := d.state.Env(context.TODO()) if err != nil { return err } - opt := []llb.RunOption{llb.Args(args), dfCmd(c), location(dopt.sourceMap, c.Location())} + opt = append(opt, llb.Args(args), dfCmd(c), location(dopt.sourceMap, c.Location())) if d.ignoreCache { opt = append(opt, llb.IgnoreCache) } @@ -735,12 +783,12 @@ func dispatchWorkdir(d *dispatchState, c *instructions.WorkdirCommand, commit bo } func dispatchCopyFileOp(d *dispatchState, c instructions.SourcesAndDest, sourceState llb.State, isAddCommand bool, cmdToPrint fmt.Stringer, chown string, chmod string, loc []parser.Range, opt dispatchOpt) error { - pp, err := pathRelativeToWorkingDir(d.state, c.Dest()) + pp, err := pathRelativeToWorkingDir(d.state, c.DestPath) if err != nil { return err } dest := path.Join("/", pp) - if c.Dest() == "." || c.Dest() == "" || c.Dest()[len(c.Dest())-1] == filepath.Separator { + if c.DestPath == "." || c.DestPath == "" || c.DestPath[len(c.DestPath)-1] == filepath.Separator { dest += string(filepath.Separator) } @@ -768,7 +816,7 @@ func dispatchCopyFileOp(d *dispatchState, c instructions.SourcesAndDest, sourceS var a *llb.FileAction - for _, src := range c.Sources() { + for _, src := range c.SourcePaths { commitMessage.WriteString(" " + src) if strings.HasPrefix(src, "http://") || strings.HasPrefix(src, "https://") { if !isAddCommand { @@ -818,7 +866,24 @@ func dispatchCopyFileOp(d *dispatchState, c instructions.SourcesAndDest, sourceS } } - commitMessage.WriteString(" " + c.Dest()) + for _, src := range c.SourceContents { + data := src.Data + f := src.Path + st := llb.Scratch().Dir("/").File(llb.Mkfile(f, 0664, []byte(data))) + + opts := append([]llb.CopyOption{&llb.CopyInfo{ + Mode: mode, + CreateDestPath: true, + }}, copyOpt...) + + if a == nil { + a = llb.Copy(st, f, dest, opts...) + } else { + a = a.Copy(st, f, dest, opts...) + } + } + + commitMessage.WriteString(" " + c.DestPath) platform := opt.targetPlatform if d.platform != nil { @@ -847,6 +912,10 @@ func dispatchCopy(d *dispatchState, c instructions.SourcesAndDest, sourceState l return dispatchCopyFileOp(d, c, sourceState, isAddCommand, cmdToPrint, chown, chmod, loc, opt) } + if len(c.SourceContents) > 0 { + return errors.New("inline content copy is not supported") + } + if chmod != "" { if opt.llbCaps != nil && opt.llbCaps.Supports(pb.CapFileBase) != nil { return errors.Wrap(opt.llbCaps.Supports(pb.CapFileBase), "chmod is not supported") @@ -855,18 +924,18 @@ func dispatchCopy(d *dispatchState, c instructions.SourcesAndDest, sourceState l } img := llb.Image(opt.copyImage, llb.MarkImageInternal, llb.Platform(opt.buildPlatforms[0]), WithInternalName("helper image for file operations")) - pp, err := pathRelativeToWorkingDir(d.state, c.Dest()) + pp, err := pathRelativeToWorkingDir(d.state, c.DestPath) if err != nil { return err } dest := path.Join(".", pp) - if c.Dest() == "." || c.Dest() == "" || c.Dest()[len(c.Dest())-1] == filepath.Separator { + if c.DestPath == "." || c.DestPath == "" || c.DestPath[len(c.DestPath)-1] == filepath.Separator { dest += string(filepath.Separator) } args := []string{"copy"} unpack := isAddCommand - mounts := make([]llb.RunOption, 0, len(c.Sources())) + mounts := make([]llb.RunOption, 0, len(c.SourcePaths)) if chown != "" { args = append(args, fmt.Sprintf("--chown=%s", chown)) _, _, err := parseUser(chown) @@ -883,7 +952,7 @@ func dispatchCopy(d *dispatchState, c instructions.SourcesAndDest, sourceState l commitMessage.WriteString("COPY") } - for i, src := range c.Sources() { + for i, src := range c.SourcePaths { commitMessage.WriteString(" " + src) if strings.HasPrefix(src, "http://") || strings.HasPrefix(src, "https://") { if !isAddCommand { @@ -920,7 +989,7 @@ func dispatchCopy(d *dispatchState, c instructions.SourcesAndDest, sourceState l } } - commitMessage.WriteString(" " + c.Dest()) + commitMessage.WriteString(" " + c.DestPath) args = append(args, dest) if unpack { diff --git a/frontend/dockerfile/instructions/commands.go b/frontend/dockerfile/instructions/commands.go index b576552f..c7d5c495 100644 --- a/frontend/dockerfile/instructions/commands.go +++ b/frontend/dockerfile/instructions/commands.go @@ -165,19 +165,45 @@ func (c *LabelCommand) Expand(expander SingleWordExpander) error { return expandKvpsInPlace(c.Labels, expander) } -// SourcesAndDest represent a list of source files and a destination -type SourcesAndDest []string - -// Sources list the source paths -func (s SourcesAndDest) Sources() []string { - res := make([]string, len(s)-1) - copy(res, s[:len(s)-1]) - return res +// SourceContent represents an anonymous file object +type SourceContent struct { + Path string + Data string + Expand bool } -// Dest path of the operation -func (s SourcesAndDest) Dest() string { - return s[len(s)-1] +// SourcesAndDest represent a collection of sources and a destination +type SourcesAndDest struct { + DestPath string + SourcePaths []string + SourceContents []SourceContent +} + +func (s *SourcesAndDest) Expand(expander SingleWordExpander) error { + for i, content := range s.SourceContents { + if !content.Expand { + continue + } + + expandedData, err := expander(content.Data) + if err != nil { + return err + } + s.SourceContents[i].Data = expandedData + } + + err := expandSliceInPlace(s.SourcePaths, expander) + if err != nil { + return err + } + + expandedDestPath, err := expander(s.DestPath) + if err != nil { + return err + } + s.DestPath = expandedDestPath + + return nil } // AddCommand : ADD foo /path @@ -199,7 +225,8 @@ func (c *AddCommand) Expand(expander SingleWordExpander) error { return err } c.Chown = expandedChown - return expandSliceInPlace(c.SourcesAndDest, expander) + + return c.SourcesAndDest.Expand(expander) } // CopyCommand : COPY foo /path @@ -221,7 +248,8 @@ func (c *CopyCommand) Expand(expander SingleWordExpander) error { return err } c.Chown = expandedChown - return expandSliceInPlace(c.SourcesAndDest, expander) + + return c.SourcesAndDest.Expand(expander) } // OnbuildCommand : ONBUILD @@ -249,9 +277,17 @@ func (c *WorkdirCommand) Expand(expander SingleWordExpander) error { return nil } +// ShellInlineFile represents an inline file created for a shell command +type ShellInlineFile struct { + Name string + Data string + Chomp bool +} + // ShellDependantCmdLine represents a cmdline optionally prepended with the shell type ShellDependantCmdLine struct { CmdLine strslice.StrSlice + Files []ShellInlineFile PrependShell bool } diff --git a/frontend/dockerfile/instructions/parse.go b/frontend/dockerfile/instructions/parse.go index 18e72722..7a6c69e3 100644 --- a/frontend/dockerfile/instructions/parse.go +++ b/frontend/dockerfile/instructions/parse.go @@ -18,6 +18,7 @@ import ( type parseRequest struct { command string args []string + heredocs []parser.Heredoc attributes map[string]bool flags *BFlags original string @@ -47,6 +48,7 @@ func newParseRequestFromNode(node *parser.Node) parseRequest { return parseRequest{ command: node.Value, args: nodeArgs(node), + heredocs: node.Heredocs, attributes: node.Attributes, original: node.Original, flags: NewBFlagsWithArgs(node.Flags), @@ -236,6 +238,45 @@ func parseLabel(req parseRequest) (*LabelCommand, error) { }, nil } +func parseSourcesAndDest(req parseRequest, command string) (*SourcesAndDest, error) { + srcs := req.args[:len(req.args)-1] + dest := req.args[len(req.args)-1] + if heredoc := parser.MustParseHeredoc(dest); heredoc != nil { + return nil, errBadHeredoc(command, "a destination") + } + + heredocLookup := make(map[string]parser.Heredoc) + for _, heredoc := range req.heredocs { + heredocLookup[heredoc.Name] = heredoc + } + + var sourcePaths []string + var sourceContents []SourceContent + for _, src := range srcs { + if heredoc := parser.MustParseHeredoc(src); heredoc != nil { + content := heredocLookup[heredoc.Name].Content + if heredoc.Chomp { + content = parser.ChompHeredocContent(content) + } + sourceContents = append(sourceContents, + SourceContent{ + Data: content, + Path: heredoc.Name, + Expand: heredoc.Expand, + }, + ) + } else { + sourcePaths = append(sourcePaths, src) + } + } + + return &SourcesAndDest{ + DestPath: dest, + SourcePaths: sourcePaths, + SourceContents: sourceContents, + }, nil +} + func parseAdd(req parseRequest) (*AddCommand, error) { if len(req.args) < 2 { return nil, errNoDestinationArgument("ADD") @@ -245,9 +286,15 @@ func parseAdd(req parseRequest) (*AddCommand, error) { if err := req.flags.Parse(); err != nil { return nil, err } + + sourcesAndDest, err := parseSourcesAndDest(req, "ADD") + if err != nil { + return nil, err + } + return &AddCommand{ - SourcesAndDest: SourcesAndDest(req.args), withNameAndCode: newWithNameAndCode(req), + SourcesAndDest: *sourcesAndDest, Chown: flChown.Value, Chmod: flChmod.Value, }, nil @@ -263,10 +310,16 @@ func parseCopy(req parseRequest) (*CopyCommand, error) { if err := req.flags.Parse(); err != nil { return nil, err } + + sourcesAndDest, err := parseSourcesAndDest(req, "COPY") + if err != nil { + return nil, err + } + return &CopyCommand{ - SourcesAndDest: SourcesAndDest(req.args), - From: flFrom.Value, withNameAndCode: newWithNameAndCode(req), + SourcesAndDest: *sourcesAndDest, + From: flFrom.Value, Chown: flChown.Value, Chmod: flChmod.Value, }, nil @@ -351,7 +404,17 @@ func parseWorkdir(req parseRequest) (*WorkdirCommand, error) { } -func parseShellDependentCommand(req parseRequest, emptyAsNil bool) ShellDependantCmdLine { +func parseShellDependentCommand(req parseRequest, command string, emptyAsNil bool) (ShellDependantCmdLine, error) { + var files []ShellInlineFile + for _, heredoc := range req.heredocs { + file := ShellInlineFile{ + Name: heredoc.Name, + Data: heredoc.Content, + Chomp: heredoc.Chomp, + } + files = append(files, file) + } + args := handleJSONArgs(req.args, req.attributes) cmd := strslice.StrSlice(args) if emptyAsNil && len(cmd) == 0 { @@ -359,8 +422,9 @@ func parseShellDependentCommand(req parseRequest, emptyAsNil bool) ShellDependan } return ShellDependantCmdLine{ CmdLine: cmd, + Files: files, PrependShell: !req.attributes["json"], - } + }, nil } func parseRun(req parseRequest) (*RunCommand, error) { @@ -376,7 +440,13 @@ func parseRun(req parseRequest) (*RunCommand, error) { return nil, err } cmd.FlagsUsed = req.flags.Used() - cmd.ShellDependantCmdLine = parseShellDependentCommand(req, false) + + cmdline, err := parseShellDependentCommand(req, "RUN", false) + if err != nil { + return nil, err + } + cmd.ShellDependantCmdLine = cmdline + cmd.withNameAndCode = newWithNameAndCode(req) for _, fn := range parseRunPostHooks { @@ -392,11 +462,16 @@ func parseCmd(req parseRequest) (*CmdCommand, error) { if err := req.flags.Parse(); err != nil { return nil, err } + + cmdline, err := parseShellDependentCommand(req, "CMD", false) + if err != nil { + return nil, err + } + return &CmdCommand{ - ShellDependantCmdLine: parseShellDependentCommand(req, false), + ShellDependantCmdLine: cmdline, withNameAndCode: newWithNameAndCode(req), }, nil - } func parseEntrypoint(req parseRequest) (*EntrypointCommand, error) { @@ -404,12 +479,15 @@ func parseEntrypoint(req parseRequest) (*EntrypointCommand, error) { return nil, err } - cmd := &EntrypointCommand{ - ShellDependantCmdLine: parseShellDependentCommand(req, true), - withNameAndCode: newWithNameAndCode(req), + cmdline, err := parseShellDependentCommand(req, "ENTRYPOINT", true) + if err != nil { + return nil, err } - return cmd, nil + return &EntrypointCommand{ + ShellDependantCmdLine: cmdline, + withNameAndCode: newWithNameAndCode(req), + }, nil } // parseOptInterval(flag) is the duration of flag.Value, or 0 if @@ -651,6 +729,10 @@ func errNoDestinationArgument(command string) error { return errors.Errorf("%s requires at least two arguments, but only one was provided. Destination could not be determined.", command) } +func errBadHeredoc(command string, option string) error { + return errors.Errorf("%s cannot accept a heredoc as %s", command, option) +} + func errBlankCommandNames(command string) error { return errors.Errorf("%s names can not be blank", command) } diff --git a/frontend/dockerfile/instructions/parse_heredoc_test.go b/frontend/dockerfile/instructions/parse_heredoc_test.go new file mode 100644 index 00000000..7bc5a98a --- /dev/null +++ b/frontend/dockerfile/instructions/parse_heredoc_test.go @@ -0,0 +1,245 @@ +// +build dfheredoc + +package instructions + +import ( + "strings" + "testing" + + "github.com/docker/docker/api/types/strslice" + "github.com/moby/buildkit/frontend/dockerfile/parser" + "github.com/stretchr/testify/require" +) + +func TestErrorCasesHeredoc(t *testing.T) { + cases := []struct { + name string + dockerfile string + expectedError string + }{ + { + name: "COPY heredoc destination", + dockerfile: "COPY /foo <