From 63780df0805270797bb6ae4a8d85ee7327a71682 Mon Sep 17 00:00:00 2001 From: Akihiro Suda Date: Tue, 20 Feb 2024 04:01:53 +0900 Subject: [PATCH] dockerfile: implement hooks for `RUN` instructions Close issue 4576 - - - e.g., ```bash buildctl build \ --frontend dockerfile.v0 \ --opt hook="$(cat hook.json)" ``` with `hook.json` as follows: ```json { "RUN": { "entrypoint": ["/dev/.dfhook/entrypoint"], "mounts": [ {"from": "example.com/hook", "target": "/dev/.dfhook"}, {"type": "secret", "source": "something", "target": "/etc/something"} ] } } ``` This will let the frontend treat `RUN foo` as: ```dockerfile RUN \ --mount=from=example.com/hook,target=/dev/.dfhook \ --mount=type=secret,source=something,target=/etc/something \ /dev/.dfhook/entrypoint foo ``` `docker history` will still show this as `RUN foo`. Signed-off-by: Akihiro Suda --- docs/reference/buildctl.md | 39 ++++++++++ frontend/dockerfile/dockerfile2llb/convert.go | 23 +++++- .../dockerfile/dockerfile_insthook_test.go | 76 +++++++++++++++++++ frontend/dockerfile/dockerfile_test.go | 1 + frontend/dockerfile/instructions/commands.go | 21 ++++- .../instructions/commands_runmount.go | 9 +++ frontend/dockerfile/instructions/parse.go | 18 +++-- .../instructions/parse_heredoc_test.go | 6 +- .../dockerfile/instructions/parse_test.go | 18 ++--- frontend/dockerui/config.go | 16 ++++ frontend/dockerui/types/hook.go | 12 +++ util/jsonutil/jsonutil.go | 13 ++++ 12 files changed, 230 insertions(+), 22 deletions(-) create mode 100644 frontend/dockerfile/dockerfile_insthook_test.go create mode 100644 frontend/dockerui/types/hook.go create mode 100644 util/jsonutil/jsonutil.go diff --git a/docs/reference/buildctl.md b/docs/reference/buildctl.md index 96e7e353980e9..3e3ecd555ad58 100644 --- a/docs/reference/buildctl.md +++ b/docs/reference/buildctl.md @@ -181,6 +181,45 @@ $ buildctl build --frontend dockerfile.v0 --local context=. --local dockerfile=. $ buildctl build --frontend dockerfile.v0 --local context=. --local dockerfile=. --oci-layout foo2=/home/dir/oci --opt context:alpine=oci-layout://foo2@sha256:bd04a5b26dec16579cd1d7322e949c5905c4742269663fcbc84dcb2e9f4592fb ``` +##### Instruction hooks + +In the master branch, the Dockerfile frontend also supports "instruction hooks". + +e.g., + +```bash +buildctl build \ + --frontend dockerfile.v0 \ + --opt hook="$(cat hook.json)" +``` +with `hook.json` as follows: +```json +{ + "RUN": { + "entrypoint": ["/dev/.dfhook/entrypoint"], + "mounts": [ + {"from": "example.com/hook", "target": "/dev/.dfhook"}, + {"type": "secret", "source": "something", "target": "/etc/something"} + ] + } +} +``` + +This will let the frontend treat `RUN foo` as: +```dockerfile +RUN \ + --mount=from=example.com/hook,target=/dev/.dfhook \ + --mount=type=secret,source=something,target=/etc/something \ + /dev/.dfhook/entrypoint foo +``` + +`docker history` will still show this as `RUN foo`. + + + #### gateway-specific options The `gateway.v0` frontend passes all of its `--opt` options on to the OCI image that is called to convert the diff --git a/frontend/dockerfile/dockerfile2llb/convert.go b/frontend/dockerfile/dockerfile2llb/convert.go index 531aa09484bd7..500b164e742f3 100644 --- a/frontend/dockerfile/dockerfile2llb/convert.go +++ b/frontend/dockerfile/dockerfile2llb/convert.go @@ -25,6 +25,7 @@ import ( "github.com/moby/buildkit/frontend/dockerfile/parser" "github.com/moby/buildkit/frontend/dockerfile/shell" "github.com/moby/buildkit/frontend/dockerui" + "github.com/moby/buildkit/frontend/dockerui/types" "github.com/moby/buildkit/frontend/subrequests/outline" "github.com/moby/buildkit/frontend/subrequests/targets" "github.com/moby/buildkit/identity" @@ -114,7 +115,7 @@ func ListTargets(ctx context.Context, dt []byte) (*targets.List, error) { if err != nil { return nil, err } - stages, _, err := instructions.Parse(dockerfile.AST) + stages, _, err := instructions.Parse(dockerfile.AST, instructions.ParseOpts{}) if err != nil { return nil, err } @@ -186,7 +187,10 @@ func toDispatchState(ctx context.Context, dt []byte, opt ConvertOpt) (*dispatchS proxyEnv := proxyEnvFromBuildArgs(opt.BuildArgs) - stages, metaArgs, err := instructions.Parse(dockerfile.AST) + parseOpts := instructions.ParseOpts{ + InstructionHook: opt.InstructionHook, + } + stages, metaArgs, err := instructions.Parse(dockerfile.AST, parseOpts) if err != nil { return nil, err } @@ -565,6 +569,7 @@ func toDispatchState(ctx context.Context, dt []byte, opt ConvertOpt) (*dispatchS cgroupParent: opt.CgroupParent, llbCaps: opt.LLBCaps, sourceMap: opt.SourceMap, + instHook: opt.InstructionHook, } if err = dispatchOnBuildTriggers(d, d.image.Config.OnBuild, opt); err != nil { @@ -700,6 +705,7 @@ type dispatchOpt struct { cgroupParent string llbCaps *apicaps.CapSet sourceMap *llb.SourceMap + instHook *types.InstructionHook } func dispatch(d *dispatchState, cmd command, opt dispatchOpt) error { @@ -910,6 +916,9 @@ type command struct { } func dispatchOnBuildTriggers(d *dispatchState, triggers []string, opt dispatchOpt) error { + parseOpts := instructions.ParseOpts{ + InstructionHook: opt.instHook, + } for _, trigger := range triggers { ast, err := parser.Parse(strings.NewReader(trigger)) if err != nil { @@ -918,7 +927,7 @@ func dispatchOnBuildTriggers(d *dispatchState, triggers []string, opt dispatchOp if len(ast.AST.Children) != 1 { return errors.New("onbuild trigger should be a single expression") } - ic, err := instructions.ParseCommand(ast.AST.Children[0]) + ic, err := instructions.ParseCommand(ast.AST.Children[0], parseOpts) if err != nil { return err } @@ -1008,6 +1017,12 @@ func dispatchRun(d *dispatchState, c *instructions.RunCommand, proxy *llb.ProxyE args = withShell(d.image, args) } + argsForHistory := args + if dopt.instHook != nil && dopt.instHook.Run != nil { + args = append(dopt.instHook.Run.Entrypoint, args...) + // leave argsForHistory unmodified + } + env, err := d.state.Env(context.TODO()) if err != nil { return err @@ -1074,7 +1089,7 @@ func dispatchRun(d *dispatchState, c *instructions.RunCommand, proxy *llb.ProxyE } d.state = d.state.Run(opt...).Root() - return commitToHistory(&d.image, "RUN "+runCommandString(args, d.buildArgs, shell.BuildEnvs(env)), true, &d.state, d.epoch) + return commitToHistory(&d.image, "RUN "+runCommandString(argsForHistory, d.buildArgs, shell.BuildEnvs(env)), true, &d.state, d.epoch) } func dispatchWorkdir(d *dispatchState, c *instructions.WorkdirCommand, commit bool, opt *dispatchOpt) error { diff --git a/frontend/dockerfile/dockerfile_insthook_test.go b/frontend/dockerfile/dockerfile_insthook_test.go new file mode 100644 index 0000000000000..137922bef9dd6 --- /dev/null +++ b/frontend/dockerfile/dockerfile_insthook_test.go @@ -0,0 +1,76 @@ +package dockerfile + +import ( + "os" + "path/filepath" + "strings" + "testing" + + "github.com/containerd/continuity/fs/fstest" + "github.com/moby/buildkit/client" + "github.com/moby/buildkit/frontend/dockerui" + "github.com/moby/buildkit/util/testutil/integration" + "github.com/stretchr/testify/require" + "github.com/tonistiigi/fsutil" +) + +var instHookTests = integration.TestFuncs( + testInstructionHook, +) + +func testInstructionHook(t *testing.T, sb integration.Sandbox) { + integration.SkipOnPlatform(t, "windows") + f := getFrontend(t, sb) + + dockerfile := []byte(` +FROM busybox AS base +RUN echo "$FOO" >/foo + +FROM scratch +COPY --from=base /foo /foo +`) + + dir := integration.Tmpdir( + t, + fstest.CreateFile("Dockerfile", dockerfile, 0600), + ) + destDir := t.TempDir() + + c, err := client.New(sb.Context(), sb.Address()) + require.NoError(t, err) + defer c.Close() + + build := func(attrs map[string]string) string { + _, err = f.Solve(sb.Context(), c, client.SolveOpt{ + FrontendAttrs: attrs, + Exports: []client.ExportEntry{ + { + Type: client.ExporterLocal, + OutputDir: destDir, + }, + }, + LocalMounts: map[string]fsutil.FS{ + dockerui.DefaultLocalNameDockerfile: dir, + dockerui.DefaultLocalNameContext: dir, + }, + }, nil) + require.NoError(t, err) + p := filepath.Join(destDir, "foo") + b, err := os.ReadFile(p) + require.NoError(t, err) + return strings.TrimSpace(string(b)) + } + + require.Equal(t, "", build(nil)) + + const hook = ` +{ + "RUN": { + "entrypoint": ["/dev/.dfhook/bin/busybox", "env", "FOO=BAR"], + "mounts": [ + {"from": "busybox:uclibc", "target": "/dev/.dfhook"} + ] + } +}` + require.Equal(t, "BAR", build(map[string]string{"hook": hook})) +} diff --git a/frontend/dockerfile/dockerfile_test.go b/frontend/dockerfile/dockerfile_test.go index d026fffc11185..ad0a66343ad19 100644 --- a/frontend/dockerfile/dockerfile_test.go +++ b/frontend/dockerfile/dockerfile_test.go @@ -254,6 +254,7 @@ func TestIntegration(t *testing.T) { "amd64/bullseye-20230109-slim": "docker.io/amd64/debian:bullseye-20230109-slim@sha256:1acb06a0c31fb467eb8327ad361f1091ab265e0bf26d452dea45dcb0c0ea5e75", }), )...) + integration.Run(t, instHookTests, opts...) } func testDefaultEnvWithArgs(t *testing.T, sb integration.Sandbox) { diff --git a/frontend/dockerfile/instructions/commands.go b/frontend/dockerfile/instructions/commands.go index 87252f9f17ddb..0078a618d4222 100644 --- a/frontend/dockerfile/instructions/commands.go +++ b/frontend/dockerfile/instructions/commands.go @@ -6,6 +6,7 @@ import ( "github.com/docker/docker/api/types/container" "github.com/docker/docker/api/types/strslice" "github.com/moby/buildkit/frontend/dockerfile/parser" + "github.com/moby/buildkit/frontend/dockerui/types" "github.com/pkg/errors" ) @@ -339,7 +340,7 @@ type ShellDependantCmdLine struct { // RUN ["echo", "hi"] # echo hi type RunCommand struct { withNameAndCode - withExternalData + WithInstructionHook ShellDependantCmdLine FlagsUsed []string } @@ -550,3 +551,21 @@ func (c *withExternalData) setExternalValue(k, v interface{}) { } c.m[k] = v } + +type WithInstructionHook struct { + withExternalData +} + +const instHookKey = "dockerfile/run/instruction-hook" + +func (c *WithInstructionHook) GetInstructionHook() *types.InstructionHook { + x := c.getExternalValue(instHookKey) + if x == nil { + return nil + } + return x.(*types.InstructionHook) +} + +func (c *WithInstructionHook) SetInstructionHook(h *types.InstructionHook) { + c.setExternalValue(instHookKey, h) +} diff --git a/frontend/dockerfile/instructions/commands_runmount.go b/frontend/dockerfile/instructions/commands_runmount.go index 591d3863282fc..3ad279fc51ad8 100644 --- a/frontend/dockerfile/instructions/commands_runmount.go +++ b/frontend/dockerfile/instructions/commands_runmount.go @@ -86,6 +86,15 @@ func setMountState(cmd *RunCommand, expander SingleWordExpander) error { return errors.Errorf("no mount state") } var mounts []*Mount + if hook := cmd.GetInstructionHook(); hook != nil && hook.Run != nil { + for _, m := range hook.Run.Mounts { + m := m + if err := validateMount(&m, false); err != nil { + return err + } + mounts = append(mounts, &m) + } + } for _, str := range st.flag.StringValues { m, err := parseMount(str, expander) if err != nil { diff --git a/frontend/dockerfile/instructions/parse.go b/frontend/dockerfile/instructions/parse.go index 1d41d9f2c224b..60df84664077b 100644 --- a/frontend/dockerfile/instructions/parse.go +++ b/frontend/dockerfile/instructions/parse.go @@ -16,12 +16,17 @@ import ( "github.com/docker/docker/api/types/strslice" "github.com/moby/buildkit/frontend/dockerfile/command" "github.com/moby/buildkit/frontend/dockerfile/parser" + "github.com/moby/buildkit/frontend/dockerui/types" "github.com/moby/buildkit/util/suggest" "github.com/pkg/errors" ) var excludePatternsEnabled = false +type ParseOpts struct { + InstructionHook *types.InstructionHook +} + type parseRequest struct { command string args []string @@ -31,6 +36,7 @@ type parseRequest struct { original string location []parser.Range comments []string + opts ParseOpts } var parseRunPreHooks []func(*RunCommand, parseRequest) error @@ -67,11 +73,12 @@ func newParseRequestFromNode(node *parser.Node) parseRequest { } // ParseInstruction converts an AST to a typed instruction (either a command or a build stage beginning when encountering a `FROM` statement) -func ParseInstruction(node *parser.Node) (v interface{}, err error) { +func ParseInstruction(node *parser.Node, opts ParseOpts) (v interface{}, err error) { defer func() { err = parser.WithLocation(err, node.Location()) }() req := newParseRequestFromNode(node) + req.opts = opts switch strings.ToLower(node.Value) { case command.Env: return parseEnv(req) @@ -114,8 +121,8 @@ func ParseInstruction(node *parser.Node) (v interface{}, err error) { } // ParseCommand converts an AST to a typed Command -func ParseCommand(node *parser.Node) (Command, error) { - s, err := ParseInstruction(node) +func ParseCommand(node *parser.Node, opts ParseOpts) (Command, error) { + s, err := ParseInstruction(node, opts) if err != nil { return nil, err } @@ -150,9 +157,9 @@ func (e *parseError) Unwrap() error { // Parse a Dockerfile into a collection of buildable stages. // metaArgs is a collection of ARG instructions that occur before the first FROM. -func Parse(ast *parser.Node) (stages []Stage, metaArgs []ArgCommand, err error) { +func Parse(ast *parser.Node, opts ParseOpts) (stages []Stage, metaArgs []ArgCommand, err error) { for _, n := range ast.Children { - cmd, err := ParseInstruction(n) + cmd, err := ParseInstruction(n, opts) if err != nil { return nil, nil, &parseError{inner: err, node: n} } @@ -472,6 +479,7 @@ func parseShellDependentCommand(req parseRequest, command string, emptyAsNil boo func parseRun(req parseRequest) (*RunCommand, error) { cmd := &RunCommand{} + cmd.SetInstructionHook(req.opts.InstructionHook) for _, fn := range parseRunPreHooks { if err := fn(cmd, req); err != nil { diff --git a/frontend/dockerfile/instructions/parse_heredoc_test.go b/frontend/dockerfile/instructions/parse_heredoc_test.go index 28cf034ee9ce7..36f993e57868a 100644 --- a/frontend/dockerfile/instructions/parse_heredoc_test.go +++ b/frontend/dockerfile/instructions/parse_heredoc_test.go @@ -29,7 +29,7 @@ func TestErrorCasesHeredoc(t *testing.T) { t.Fatalf("Error when parsing Dockerfile: %s", err) } n := ast.AST.Children[0] - _, err = ParseInstruction(n) + _, err = ParseInstruction(n, ParseOpts{}) require.Error(t, err) require.Contains(t, err.Error(), c.expectedError) } @@ -167,7 +167,7 @@ EOF`, require.NoError(t, err) n := ast.AST.Children[0] - comm, err := ParseInstruction(n) + comm, err := ParseInstruction(n, ParseOpts{}) require.NoError(t, err) sd := comm.(*CopyCommand).SourcesAndDest @@ -249,7 +249,7 @@ EOF`, require.NoError(t, err) n := ast.AST.Children[0] - comm, err := ParseInstruction(n) + comm, err := ParseInstruction(n, ParseOpts{}) require.NoError(t, err) require.Equal(t, c.shell, comm.(*RunCommand).PrependShell) require.Equal(t, c.command, comm.(*RunCommand).CmdLine) diff --git a/frontend/dockerfile/instructions/parse_test.go b/frontend/dockerfile/instructions/parse_test.go index 9101d3cd1dcd4..7ac240b8ebd2a 100644 --- a/frontend/dockerfile/instructions/parse_test.go +++ b/frontend/dockerfile/instructions/parse_test.go @@ -21,7 +21,7 @@ func TestCommandsExactlyOneArgument(t *testing.T) { for _, cmd := range commands { ast, err := parser.Parse(strings.NewReader(cmd)) require.NoError(t, err) - _, err = ParseInstruction(ast.AST.Children[0]) + _, err = ParseInstruction(ast.AST.Children[0], ParseOpts{}) require.EqualError(t, err, errExactlyOneArgument(cmd).Error()) } } @@ -39,7 +39,7 @@ func TestCommandsAtLeastOneArgument(t *testing.T) { for _, cmd := range commands { ast, err := parser.Parse(strings.NewReader(cmd)) require.NoError(t, err) - _, err = ParseInstruction(ast.AST.Children[0]) + _, err = ParseInstruction(ast.AST.Children[0], ParseOpts{}) require.EqualError(t, err, errAtLeastOneArgument(cmd).Error()) } } @@ -53,7 +53,7 @@ func TestCommandsNoDestinationArgument(t *testing.T) { for _, cmd := range commands { ast, err := parser.Parse(strings.NewReader(cmd + " arg1")) require.NoError(t, err) - _, err = ParseInstruction(ast.AST.Children[0]) + _, err = ParseInstruction(ast.AST.Children[0], ParseOpts{}) require.EqualError(t, err, errNoDestinationArgument(cmd).Error()) } } @@ -78,7 +78,7 @@ func TestCommandsTooManyArguments(t *testing.T) { }, }, } - _, err := ParseInstruction(node) + _, err := ParseInstruction(node, ParseOpts{}) require.EqualError(t, err, errTooManyArguments(cmd).Error()) } } @@ -100,7 +100,7 @@ func TestCommandsBlankNames(t *testing.T) { }, }, } - _, err := ParseInstruction(node) + _, err := ParseInstruction(node, ParseOpts{}) require.EqualError(t, err, errBlankCommandNames(cmd).Error()) } } @@ -118,7 +118,7 @@ func TestHealthCheckCmd(t *testing.T) { }, }, } - cmd, err := ParseInstruction(node) + cmd, err := ParseInstruction(node, ParseOpts{}) require.NoError(t, err) hc, ok := cmd.(*HealthCheckCommand) require.Equal(t, true, ok) @@ -161,7 +161,7 @@ ARG bar baz=123 ast, err := parser.Parse(bytes.NewBuffer([]byte(dt))) require.NoError(t, err) - stages, meta, err := Parse(ast.AST) + stages, meta, err := Parse(ast.AST, ParseOpts{}) require.NoError(t, err) require.Equal(t, "defines first stage", stages[0].Comment) @@ -225,7 +225,7 @@ func TestErrorCases(t *testing.T) { t.Fatalf("Error when parsing Dockerfile: %s", err) } n := ast.AST.Children[0] - _, err = ParseInstruction(n) + _, err = ParseInstruction(n, ParseOpts{}) require.ErrorContains(t, err, c.expectedError) } } @@ -237,7 +237,7 @@ func TestRunCmdFlagsUsed(t *testing.T) { require.NoError(t, err) n := ast.AST.Children[0] - c, err := ParseInstruction(n) + c, err := ParseInstruction(n, ParseOpts{}) require.NoError(t, err) require.IsType(t, c, &RunCommand{}) require.Equal(t, []string{"mount"}, c.(*RunCommand).FlagsUsed) diff --git a/frontend/dockerui/config.go b/frontend/dockerui/config.go index 476c9faf69e4b..b06a5ad2481c3 100644 --- a/frontend/dockerui/config.go +++ b/frontend/dockerui/config.go @@ -14,9 +14,11 @@ import ( controlapi "github.com/moby/buildkit/api/services/control" "github.com/moby/buildkit/client/llb" "github.com/moby/buildkit/frontend/attestations" + "github.com/moby/buildkit/frontend/dockerui/types" "github.com/moby/buildkit/frontend/gateway/client" "github.com/moby/buildkit/solver/pb" "github.com/moby/buildkit/util/flightcontrol" + "github.com/moby/buildkit/util/jsonutil" dockerspec "github.com/moby/docker-image-spec/specs-go/v1" "github.com/moby/patternmatcher/ignorefile" digest "github.com/opencontainers/go-digest" @@ -41,6 +43,7 @@ const ( keyUlimit = "ulimit" keyCacheFrom = "cache-from" // for registry only. deprecated in favor of keyCacheImports keyCacheImports = "cache-imports" // JSON representation of []CacheOptionsEntry + keyHook = "hook" // JSON representation of types.Hook // Don't forget to update frontend documentation if you add // a new build-arg: frontend/dockerfile/docs/reference.md @@ -70,6 +73,8 @@ type Config struct { BuildPlatforms []ocispecs.Platform MultiPlatformRequested bool SBOM *SBOM + + InstructionHook *types.InstructionHook } type Client struct { @@ -277,6 +282,17 @@ func (bc *Client) init() error { opts[keyHostname] = v } bc.Hostname = opts[keyHostname] + + if hookStr := opts[keyHook]; hookStr != "" { + var hook types.InstructionHook + // Using UnmarshalStrict is important to notify invalid hooks, + // because the JSON form of the mount struct is not as flexible as the CSV form. + // (e.g., "source" cannot be abbreviated as "src") + if err := jsonutil.UnmarshalStrict([]byte(hookStr), &hook); err != nil { + return errors.Wrapf(err, "failed to parse dockerfile hook") + } + bc.InstructionHook = &hook + } return nil } diff --git a/frontend/dockerui/types/hook.go b/frontend/dockerui/types/hook.go new file mode 100644 index 0000000000000..cb6a7fb38101c --- /dev/null +++ b/frontend/dockerui/types/hook.go @@ -0,0 +1,12 @@ +package types + +// InstructionHook provides a hooking mechanism for instructions of Dockerfile. +type InstructionHook struct { + Run *RunInstructionHook `json:"RUN,omitempty"` +} + +// RunInstructionHook provides a hooking mechanism for `RUN` instruction of Dockerfile. +type RunInstructionHook struct { + Entrypoint []string `json:"entrypoint"` + Mounts []Mount `json:"mounts"` +} diff --git a/util/jsonutil/jsonutil.go b/util/jsonutil/jsonutil.go new file mode 100644 index 0000000000000..3c86f39971513 --- /dev/null +++ b/util/jsonutil/jsonutil.go @@ -0,0 +1,13 @@ +package jsonutil + +import ( + "bytes" + "encoding/json" +) + +// UnmarshalStrict is similar to [json.Unmarshal] but strict. +func UnmarshalStrict(b []byte, v any) error { + d := json.NewDecoder(bytes.NewReader(b)) + d.DisallowUnknownFields() + return d.Decode(v) +}