From 8bfeafaaa1c184eecb67ff012cdd6044bc3fa738 Mon Sep 17 00:00:00 2001 From: Akihiro Suda Date: Tue, 12 Apr 2022 15:55:58 +0900 Subject: [PATCH] dockerfile (labs): implement `ADD ` e.g., # syntax=docker/dockerfile-upstream:master-labs FROM alpine ADD https://github.com/moby/buildkit.git#v0.10.1 /buildkit Close issue 775 Signed-off-by: Akihiro Suda --- .golangci.yml | 1 + frontend/dockerfile/builder/build.go | 45 +++---- frontend/dockerfile/dockerfile2llb/convert.go | 32 ++++- .../dockerfile2llb/convert_addgit.go | 6 + .../dockerfile2llb/convert_noaddgit.go | 6 + frontend/dockerfile/dockerfile_addgit_test.go | 115 ++++++++++++++++++ frontend/dockerfile/docs/reference.md | 38 ++++++ frontend/dockerfile/instructions/commands.go | 7 +- frontend/dockerfile/instructions/parse.go | 2 + frontend/dockerfile/release/labs/tags | 2 +- util/gitutil/git_ref.go | 95 +++++++++++++++ util/gitutil/git_ref_test.go | 77 ++++++++++++ 12 files changed, 391 insertions(+), 35 deletions(-) create mode 100644 frontend/dockerfile/dockerfile2llb/convert_addgit.go create mode 100644 frontend/dockerfile/dockerfile2llb/convert_noaddgit.go create mode 100644 frontend/dockerfile/dockerfile_addgit_test.go create mode 100644 util/gitutil/git_ref.go create mode 100644 util/gitutil/git_ref_test.go diff --git a/.golangci.yml b/.golangci.yml index 98590f082..17848dc28 100644 --- a/.golangci.yml +++ b/.golangci.yml @@ -7,6 +7,7 @@ run: build-tags: - dfrunsecurity + - dfaddgit linters: enable: diff --git a/frontend/dockerfile/builder/build.go b/frontend/dockerfile/builder/build.go index 0308c7c0c..4a5dcc3f1 100644 --- a/frontend/dockerfile/builder/build.go +++ b/frontend/dockerfile/builder/build.go @@ -27,6 +27,7 @@ import ( "github.com/moby/buildkit/solver/errdefs" "github.com/moby/buildkit/solver/pb" binfotypes "github.com/moby/buildkit/util/buildinfo/types" + "github.com/moby/buildkit/util/gitutil" digest "github.com/opencontainers/go-digest" ocispecs "github.com/opencontainers/image-spec/specs-go/v1" "github.com/pkg/errors" @@ -70,7 +71,6 @@ const ( ) var httpPrefix = regexp.MustCompile(`^https?://`) -var gitURLPathWithFragmentSuffix = regexp.MustCompile(`\.git(?:#.+)?$`) func Build(ctx context.Context, c client.Client) (*client.Result, error) { opts := c.BuildOpts().Opts @@ -181,7 +181,11 @@ func Build(ctx context.Context, c client.Client) (*client.Result, error) { var buildContext *llb.State isNotLocalContext := false - if st, ok := detectGitContext(opts[localNameContext], opts[keyContextKeepGitDirArg]); ok { + keepGit := false + if v, err := strconv.ParseBool(opts[keyContextKeepGitDirArg]); err == nil { + keepGit = v + } + if st, ok := detectGitContext(opts[localNameContext], keepGit); ok { if !forceLocalDockerfile { src = *st } @@ -599,40 +603,21 @@ func filter(opt map[string]string, key string) map[string]string { return m } -func detectGitContext(ref, gitContext string) (*llb.State, bool) { - found := false - if httpPrefix.MatchString(ref) && gitURLPathWithFragmentSuffix.MatchString(ref) { - found = true - } - - keepGit := false - if gitContext != "" { - if v, err := strconv.ParseBool(gitContext); err == nil { - keepGit = v - } - } - - for _, prefix := range []string{"git://", "github.com/", "git@"} { - if strings.HasPrefix(ref, prefix) { - found = true - break - } - } - if !found { +func detectGitContext(ref string, keepGit bool) (*llb.State, bool) { + g, err := gitutil.ParseGitRef(ref) + if err != nil { return nil, false } - - parts := strings.SplitN(ref, "#", 2) - branch := "" - if len(parts) > 1 { - branch = parts[1] + commit := g.Commit + if g.SubDir != "" { + commit += ":" + g.SubDir } gitOpts := []llb.GitOption{dockerfile2llb.WithInternalName("load git source " + ref)} if keepGit { gitOpts = append(gitOpts, llb.KeepGitDir()) } - st := llb.Git(parts[0], branch, gitOpts...) + st := llb.Git(g.Remote, commit, gitOpts...) return &st, true } @@ -870,13 +855,13 @@ func contextByName(ctx context.Context, c client.Client, sessionID, name string, } return &st, &img, nil, nil case "git": - st, ok := detectGitContext(v, "1") + st, ok := detectGitContext(v, true) if !ok { return nil, nil, nil, errors.Errorf("invalid git context %s", v) } return st, nil, nil, nil case "http", "https": - st, ok := detectGitContext(v, "1") + st, ok := detectGitContext(v, true) if !ok { httpst := llb.HTTP(v, llb.WithCustomName("[context "+name+"] "+v)) st = &httpst diff --git a/frontend/dockerfile/dockerfile2llb/convert.go b/frontend/dockerfile/dockerfile2llb/convert.go index 7d0b6c773..a5b7d3e5f 100644 --- a/frontend/dockerfile/dockerfile2llb/convert.go +++ b/frontend/dockerfile/dockerfile2llb/convert.go @@ -27,6 +27,7 @@ import ( "github.com/moby/buildkit/solver/pb" "github.com/moby/buildkit/util/apicaps" binfotypes "github.com/moby/buildkit/util/buildinfo/types" + "github.com/moby/buildkit/util/gitutil" "github.com/moby/buildkit/util/suggest" "github.com/moby/buildkit/util/system" "github.com/moby/sys/signal" @@ -652,6 +653,7 @@ func dispatch(d *dispatchState, cmd command, opt dispatchOpt) error { chown: c.Chown, chmod: c.Chmod, link: c.Link, + keepGitDir: c.KeepGitDir, location: c.Location(), opt: opt, }) @@ -1006,7 +1008,34 @@ func dispatchCopy(d *dispatchState, cfg copyConfig) error { for _, src := range cfg.params.SourcePaths { commitMessage.WriteString(" " + src) - if strings.HasPrefix(src, "http://") || strings.HasPrefix(src, "https://") { + gitRef, gitRefErr := gitutil.ParseGitRef(src) + if gitRefErr == nil && !gitRef.IndistinguishableFromLocal { + if !cfg.isAddCommand { + return errors.New("source can't be a git ref for COPY") + } + if !addGitEnabled { + return errors.New("instruction ADD requires the labs channel") + } + // TODO: print a warning (not an error) if gitRef.UnencryptedTCP is true + commit := gitRef.Commit + if gitRef.SubDir != "" { + commit += ":" + gitRef.SubDir + } + var gitOptions []llb.GitOption + if cfg.keepGitDir { + gitOptions = append(gitOptions, llb.KeepGitDir()) + } + st := llb.Git(gitRef.Remote, commit, gitOptions...) + opts := append([]llb.CopyOption{&llb.CopyInfo{ + Mode: mode, + CreateDestPath: true, + }}, copyOpt...) + if a == nil { + a = llb.Copy(st, "/", dest, opts...) + } else { + a = a.Copy(st, "/", dest, opts...) + } + } else if strings.HasPrefix(src, "http://") || strings.HasPrefix(src, "https://") { if !cfg.isAddCommand { return errors.New("source can't be a URL for COPY") } @@ -1129,6 +1158,7 @@ type copyConfig struct { chown string chmod string link bool + keepGitDir bool location []parser.Range opt dispatchOpt } diff --git a/frontend/dockerfile/dockerfile2llb/convert_addgit.go b/frontend/dockerfile/dockerfile2llb/convert_addgit.go new file mode 100644 index 000000000..9ccb7a20e --- /dev/null +++ b/frontend/dockerfile/dockerfile2llb/convert_addgit.go @@ -0,0 +1,6 @@ +//go:build dfaddgit +// +build dfaddgit + +package dockerfile2llb + +const addGitEnabled = true diff --git a/frontend/dockerfile/dockerfile2llb/convert_noaddgit.go b/frontend/dockerfile/dockerfile2llb/convert_noaddgit.go new file mode 100644 index 000000000..119bb32c8 --- /dev/null +++ b/frontend/dockerfile/dockerfile2llb/convert_noaddgit.go @@ -0,0 +1,6 @@ +//go:build !dfaddgit +// +build !dfaddgit + +package dockerfile2llb + +const addGitEnabled = false diff --git a/frontend/dockerfile/dockerfile_addgit_test.go b/frontend/dockerfile/dockerfile_addgit_test.go new file mode 100644 index 000000000..fa99dea56 --- /dev/null +++ b/frontend/dockerfile/dockerfile_addgit_test.go @@ -0,0 +1,115 @@ +//go:build dfaddgit +// +build dfaddgit + +package dockerfile + +import ( + "bytes" + "net/http" + "net/http/httptest" + "os" + "path/filepath" + "testing" + "text/template" + + "github.com/containerd/continuity/fs/fstest" + "github.com/moby/buildkit/client" + "github.com/moby/buildkit/frontend/dockerfile/builder" + "github.com/moby/buildkit/util/testutil/integration" + "github.com/stretchr/testify/require" +) + +var addGitTests = integration.TestFuncs( + testAddGit, +) + +func init() { + allTests = append(allTests, addGitTests...) +} + +func testAddGit(t *testing.T, sb integration.Sandbox) { + f := getFrontend(t, sb) + + gitDir, err := os.MkdirTemp("", "buildkit") + require.NoError(t, err) + defer os.RemoveAll(gitDir) + gitCommands := []string{ + "git init", + "git config --local user.email test", + "git config --local user.name test", + } + makeCommit := func(tag string) []string { + return []string{ + "echo foo of " + tag + " >foo", + "git add foo", + "git commit -m " + tag, + "git tag " + tag, + } + } + gitCommands = append(gitCommands, makeCommit("v0.0.1")...) + gitCommands = append(gitCommands, makeCommit("v0.0.2")...) + gitCommands = append(gitCommands, makeCommit("v0.0.3")...) + gitCommands = append(gitCommands, "git update-server-info") + err = runShell(gitDir, gitCommands...) + require.NoError(t, err) + + server := httptest.NewServer(http.FileServer(http.Dir(filepath.Join(gitDir)))) + defer server.Close() + serverURL := server.URL + t.Logf("serverURL=%q", serverURL) + + dockerfile, err := applyTemplate(` +FROM alpine + +# Basic case +ADD {{.ServerURL}}/.git#v0.0.1 /x +RUN cd /x && \ + [ "$(cat foo)" = "foo of v0.0.1" ] + +# Complicated case +ARG REPO="{{.ServerURL}}/.git" +ARG TAG="v0.0.2" +ADD --keep-git-dir=true --chown=4242:8484 ${REPO}#${TAG} /buildkit-chowned +RUN apk add git +USER 4242 +RUN cd /buildkit-chowned && \ + [ "$(cat foo)" = "foo of v0.0.2" ] && \ + [ "$(stat -c %u foo)" = "4242" ] && \ + [ "$(stat -c %g foo)" = "8484" ] && \ + [ -z "$(git status -s)" ] +`, map[string]string{ + "ServerURL": serverURL, + }) + require.NoError(t, err) + t.Logf("dockerfile=%s", dockerfile) + + dir, err := integration.Tmpdir(t, + fstest.CreateFile("Dockerfile", []byte(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() + + _, err = f.Solve(sb.Context(), c, client.SolveOpt{ + LocalDirs: map[string]string{ + builder.DefaultLocalNameDockerfile: dir, + builder.DefaultLocalNameContext: dir, + }, + }, nil) + require.NoError(t, err) +} + +func applyTemplate(tmpl string, x interface{}) (string, error) { + var buf bytes.Buffer + parsed, err := template.New("").Parse(tmpl) + if err != nil { + return "", err + } + if err := parsed.Execute(&buf, x); err != nil { + return "", err + } + return buf.String(), nil +} diff --git a/frontend/dockerfile/docs/reference.md b/frontend/dockerfile/docs/reference.md index ba579cc4b..0fa8a402d 100644 --- a/frontend/dockerfile/docs/reference.md +++ b/frontend/dockerfile/docs/reference.md @@ -1500,6 +1500,44 @@ guide – Leverage build cache](https://docs.docker.com/develop/develop-images/d - If `` doesn't exist, it is created along with all missing directories in its path. +### Adding a git repository `ADD ` + +> **Note** +> +> Available in [`docker/dockerfile-upstream:master-labs`](#syntax). +> Will be included in `docker/dockerfile:1.5-labs`. + +This form allows adding a git repository to an image directly, without using the `git` command inside the image: +``` +ADD [--keep-git-dir=] +``` + +```dockerfile +# syntax=docker/dockerfile-upstream:master-labs +FROM alpine +ADD --keep-git-dir=true https://github.com/moby/buildkit.git#v0.10.1 /buildkit +``` + +The `--keep-git-dir=true` flag adds the `.git` directory. This flag defaults to false. + +### Adding a private git repository +To add a private repo via SSH, create a Dockerfile with the following form: +```dockerfile +# syntax = docker/dockerfile-upstream:master-labs +FROM alpine +ADD git@git.example.com:foo/bar.git /bar +``` + +This Dockerfile can be built with `docker build --ssh` or `buildctl build --ssh`, e.g., + +```console +$ docker build --ssh default +``` + +```console +$ buildctl build --frontend=dockerfile.v0 --local context=. --local dockerfile=. --ssh default +``` + ## ADD --link See [`COPY --link`](#copy---link). diff --git a/frontend/dockerfile/instructions/commands.go b/frontend/dockerfile/instructions/commands.go index 48ebf183a..284a1a083 100644 --- a/frontend/dockerfile/instructions/commands.go +++ b/frontend/dockerfile/instructions/commands.go @@ -224,9 +224,10 @@ func (s *SourcesAndDest) ExpandRaw(expander SingleWordExpander) error { type AddCommand struct { withNameAndCode SourcesAndDest - Chown string - Chmod string - Link bool + Chown string + Chmod string + Link bool + KeepGitDir bool // whether to keep .git dir, only meaningful for git sources } // Expand variables diff --git a/frontend/dockerfile/instructions/parse.go b/frontend/dockerfile/instructions/parse.go index a04e9b9d4..ae5bdbb06 100644 --- a/frontend/dockerfile/instructions/parse.go +++ b/frontend/dockerfile/instructions/parse.go @@ -281,6 +281,7 @@ func parseAdd(req parseRequest) (*AddCommand, error) { flChown := req.flags.AddString("chown", "") flChmod := req.flags.AddString("chmod", "") flLink := req.flags.AddBool("link", false) + flKeepGitDir := req.flags.AddBool("keep-git-dir", false) if err := req.flags.Parse(); err != nil { return nil, err } @@ -296,6 +297,7 @@ func parseAdd(req parseRequest) (*AddCommand, error) { Chown: flChown.Value, Chmod: flChmod.Value, Link: flLink.Value == "true", + KeepGitDir: flKeepGitDir.Value == "true", }, nil } diff --git a/frontend/dockerfile/release/labs/tags b/frontend/dockerfile/release/labs/tags index 03dd8c3a5..7f2ea1cee 100644 --- a/frontend/dockerfile/release/labs/tags +++ b/frontend/dockerfile/release/labs/tags @@ -1 +1 @@ -dfrunsecurity +dfrunsecurity dfaddgit diff --git a/util/gitutil/git_ref.go b/util/gitutil/git_ref.go new file mode 100644 index 000000000..863691aeb --- /dev/null +++ b/util/gitutil/git_ref.go @@ -0,0 +1,95 @@ +package gitutil + +import ( + "regexp" + "strings" + + "github.com/containerd/containerd/errdefs" +) + +// GitRef represents a git ref. +// +// Examples: +// - "https://github.com/foo/bar.git#baz/qux:quux/quuz" is parsed into: +// {Remote: "https://github.com/foo/bar.git", ShortName: "bar", Commit:"baz/qux", SubDir: "quux/quuz"}. +type GitRef struct { + // Remote is the remote repository path. + Remote string + + // ShortName is the directory name of the repo. + // e.g., "bar" for "https://github.com/foo/bar.git" + ShortName string + + // Commit is a commit hash, a tag, or branch name. + // Commit is optional. + Commit string + + // SubDir is a directory path inside the repo. + // SubDir is optional. + SubDir string + + // IndistinguishableFromLocal is true for a ref that is indistinguishable from a local file path, + // e.g., "github.com/foo/bar". + // + // Deprecated. + // Instead, use a distinguishable form such as "https://github.com/foo/bar.git". + // + // The dockerfile frontend still accepts this form only for build contexts. + IndistinguishableFromLocal bool + + // UnencryptedTCP is true for a ref that needs an unencrypted TCP connection, + // e.g., "git://..." and "http://..." . + // + // Discouraged, although not deprecated. + // Instead, consider using an encrypted TCP connection such as "git@github.com/foo/bar.git" or "https://github.com/foo/bar.git". + UnencryptedTCP bool +} + +// var gitURLPathWithFragmentSuffix = regexp.MustCompile(`\.git(?:#.+)?$`) + +// ParseGitRef parses a git ref. +func ParseGitRef(ref string) (*GitRef, error) { + res := &GitRef{} + + if strings.HasPrefix(ref, "github.com/") { + res.IndistinguishableFromLocal = true // Deprecated + } else { + _, proto := ParseProtocol(ref) + switch proto { + case UnknownProtocol: + return nil, errdefs.ErrInvalidArgument + } + switch proto { + case HTTPProtocol, GitProtocol: + res.UnencryptedTCP = true // Discouraged, but not deprecated + } + switch proto { + // An HTTP(S) URL is considered to be a valid git ref only when it has the ".git[...]" suffix. + case HTTPProtocol, HTTPSProtocol: + var gitURLPathWithFragmentSuffix = regexp.MustCompile(`\.git(?:#.+)?$`) + if !gitURLPathWithFragmentSuffix.MatchString(ref) { + return nil, errdefs.ErrInvalidArgument + } + } + } + + refSplitBySharp := strings.SplitN(ref, "#", 2) + res.Remote = refSplitBySharp[0] + if len(res.Remote) == 0 { + return res, errdefs.ErrInvalidArgument + } + + if len(refSplitBySharp) > 1 { + refSplitBySharpSplitByColon := strings.SplitN(refSplitBySharp[1], ":", 2) + res.Commit = refSplitBySharpSplitByColon[0] + if len(res.Commit) == 0 { + return res, errdefs.ErrInvalidArgument + } + if len(refSplitBySharpSplitByColon) > 1 { + res.SubDir = refSplitBySharpSplitByColon[1] + } + } + repoSplitBySlash := strings.Split(res.Remote, "/") + res.ShortName = strings.TrimSuffix(repoSplitBySlash[len(repoSplitBySlash)-1], ".git") + return res, nil +} diff --git a/util/gitutil/git_ref_test.go b/util/gitutil/git_ref_test.go new file mode 100644 index 000000000..2c096cbe5 --- /dev/null +++ b/util/gitutil/git_ref_test.go @@ -0,0 +1,77 @@ +package gitutil + +import ( + "reflect" + "testing" +) + +func TestParseGitRef(t *testing.T) { + cases := map[string]*GitRef{ + "https://example.com/": nil, + "https://example.com/foo": nil, + "https://example.com/foo.git": { + Remote: "https://example.com/foo.git", + ShortName: "foo", + }, + "https://example.com/foo.git#deadbeef": { + Remote: "https://example.com/foo.git", + ShortName: "foo", + Commit: "deadbeef", + }, + "https://example.com/foo.git#release/1.2": { + Remote: "https://example.com/foo.git", + ShortName: "foo", + Commit: "release/1.2", + }, + "https://example.com/foo.git/": nil, + "https://example.com/foo.git.bar": nil, + "git://example.com/foo": { + Remote: "git://example.com/foo", + ShortName: "foo", + UnencryptedTCP: true, + }, + "github.com/moby/buildkit": { + Remote: "github.com/moby/buildkit", ShortName: "buildkit", + IndistinguishableFromLocal: true, + }, + "https://github.com/moby/buildkit": nil, + "https://github.com/moby/buildkit.git": { + Remote: "https://github.com/moby/buildkit.git", + ShortName: "buildkit", + }, + "git@github.com:moby/buildkit": { + Remote: "git@github.com:moby/buildkit", + ShortName: "buildkit", + }, + "git@github.com:moby/buildkit.git": { + Remote: "git@github.com:moby/buildkit.git", + ShortName: "buildkit", + }, + "git@bitbucket.org:atlassianlabs/atlassian-docker.git": { + Remote: "git@bitbucket.org:atlassianlabs/atlassian-docker.git", + ShortName: "atlassian-docker", + }, + "https://github.com/foo/bar.git#baz/qux:quux/quuz": { + Remote: "https://github.com/foo/bar.git", + ShortName: "bar", + Commit: "baz/qux", + SubDir: "quux/quuz", + }, + "http://github.com/docker/docker.git:#branch": nil, + } + for ref, expected := range cases { + got, err := ParseGitRef(ref) + if expected == nil { + if err == nil { + t.Errorf("expected an error for ParseGitRef(%q)", ref) + } + } else { + if err != nil { + t.Errorf("got an unexpected error: ParseGitRef(%q): %v", ref, err) + } + if !reflect.DeepEqual(got, expected) { + t.Errorf("expected ParseGitRef(%q) to return %#v, got %#v", ref, expected, got) + } + } + } +}