dockerfile (labs): implement ADD <git ref>

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 <akihiro.suda.cz@hco.ntt.co.jp>

.golangci.yml

@@ -7,6 +7,7 @@ run:
 
   build-tags:
     - dfrunsecurity
+    - dfaddgit
 
 linters:
   enable:

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

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 <git ref> 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
 }

frontend/dockerfile/dockerfile2llb/convert_addgit.go

@@ -0,0 +1,6 @@
+//go:build dfaddgit
+// +build dfaddgit
+
+package dockerfile2llb
+
+const addGitEnabled = true

frontend/dockerfile/dockerfile2llb/convert_noaddgit.go

@@ -0,0 +1,6 @@
+//go:build !dfaddgit
+// +build !dfaddgit
+
+package dockerfile2llb
+
+const addGitEnabled = false

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
+}

frontend/dockerfile/docs/reference.md

@@ -1500,6 +1500,44 @@ guide – Leverage build cache](https://docs.docker.com/develop/develop-images/d
 - If `<dest>` doesn't exist, it is created along with all missing directories
   in its path.
 
+### Adding a git repository `ADD <git ref> <dir>`
+
+> **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=<boolean>] <git ref> <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).

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

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
 }
 

frontend/dockerfile/release/labs/tags

@@ -1 +1 @@
-dfrunsecurity
+dfrunsecurity dfaddgit

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
+}

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)
+			}
+		}
+	}
+}