source: context: Optimize fetch by path

This makes sure that if a path is specified on a context source that we
we only ever fetch the content from beneath that path.

Before this change we were doing the context path filtering after the
content was fetched.

This also makes so that include/exclude patterns are automatically
namespaced by the source path, since it doesn't make sense to include
files that are outside of the source path.

Signed-off-by: Brian Goff <[email protected]>

frontend/debug/handle_sources.go

@@ -24,6 +24,11 @@ func Sources(ctx context.Context, client gwclient.Client) (*client.Result, error
 			return nil, nil, err
 		}
 
+		for k, v := range sources {
+			st := llb.Scratch().File(llb.Copy(v, "/", k))
+			sources[k] = st
+		}
+
 		def, err := dalec.MergeAtPath(llb.Scratch(), dalec.SortedMapValues(sources), "/").Marshal(ctx)
 		if err != nil {
 			return nil, nil, err

frontend/gateway.go

@@ -116,6 +116,9 @@ func SourceOptFromClient(ctx context.Context, c gwclient.Client) (dalec.SourceOp
 			}
 			st, _, err := dc.NamedContext(ctx, ref, dockerui.ContextOpt{
 				ResolveMode: dc.ImageResolveMode.String(),
+				AsyncLocalOpts: func() []llb.LocalOption {
+					return opts
+				},
 			})
 			if err != nil {
 				return nil, err

frontend/request.go

@@ -143,7 +143,7 @@ func marshalDockerfile(ctx context.Context, dt []byte, opts ...llb.ConstraintsOp
 
 func getSigningConfigFromContext(ctx context.Context, client gwclient.Client, cfgPath string, configCtxName string, sOpt dalec.SourceOpts) (*dalec.PackageSigner, error) {
 	sc := dalec.SourceContext{Name: configCtxName}
-	signConfigState, err := sc.AsState([]string{cfgPath}, nil, sOpt)
+	signConfigState, err := sc.AsState(cfgPath, []string{cfgPath}, nil, sOpt)
 	if err != nil {
 		return nil, err
 	}

source.go

@@ -3,6 +3,7 @@ package dalec
 import (
 	"bufio"
 	"bytes"
+	"encoding/json"
 	"fmt"
 	"io"
 	"path/filepath"
@@ -36,11 +37,9 @@ func getFilter(src Source, forMount bool, opts ...llb.ConstraintsOpt) llb.StateO
 	case src.HTTP != nil,
 		src.Git != nil,
 		src.Build != nil,
+		src.Context != nil,
 		src.Inline != nil:
 		return filterState(path, src.Includes, src.Excludes, opts...)
-	case src.Context != nil:
-		// context sources handle includes and excludes
-		return filterState(path, []string{}, []string{})
 	case src.DockerImage != nil:
 		if src.DockerImage.Cmd != nil {
 			// if a docker image source has a command,
@@ -62,7 +61,7 @@ func getSource(src Source, name string, sOpt SourceOpts, opts ...llb.Constraints
 	case src.Git != nil:
 		st, err = src.Git.AsState(opts...)
 	case src.Context != nil:
-		st, err = src.Context.AsState(src.Includes, src.Excludes, sOpt, opts...)
+		st, err = src.Context.AsState(src.Path, src.Includes, src.Excludes, sOpt, opts...)
 	case src.DockerImage != nil:
 		st, err = src.DockerImage.AsState(name, src.Path, sOpt, opts...)
 	case src.Build != nil:
@@ -83,8 +82,32 @@ func (src *SourceInline) AsState(name string) (llb.State, error) {
 	return llb.Scratch().With(src.Dir.PopulateAt("/")), nil
 }
 
-func (src *SourceContext) AsState(includes []string, excludes []string, sOpt SourceOpts, opts ...llb.ConstraintsOpt) (llb.State, error) {
-	st, err := sOpt.GetContext(src.Name, localIncludeExcludeMerge(includes, excludes), withConstraints(opts))
+// withFollowPath similar to using [llb.IncludePatterns] except that it will
+// follow symlinks at the provided path.
+func withFollowPath(p string) localOptionFunc {
+	return func(li *llb.LocalInfo) {
+		if isRoot(p) {
+			return
+		}
+
+		paths := []string{p}
+		if li.FollowPaths != "" {
+			var ls []string
+			if err := json.Unmarshal([]byte(li.FollowPaths), &ls); err != nil {
+				panic(err)
+			}
+			paths = append(ls, paths...)
+		}
+		llb.FollowPaths(paths).SetLocalOption(li)
+	}
+}
+
+func (src *SourceContext) AsState(path string, includes []string, excludes []string, sOpt SourceOpts, opts ...llb.ConstraintsOpt) (llb.State, error) {
+	if !isRoot(path) {
+		excludes = append(excludeAllButPath(path), excludes...)
+	}
+
+	st, err := sOpt.GetContext(src.Name, localIncludeExcludeMerge(includes, excludes), withFollowPath(path), withConstraints(opts))
 	if err != nil {
 		return llb.Scratch(), err
 	}
@@ -96,6 +119,13 @@ func (src *SourceContext) AsState(includes []string, excludes []string, sOpt Sou
 	return *st, nil
 }
 
+func excludeAllButPath(p string) []string {
+	return []string{
+		"*",
+		"!" + filepath.ToSlash(filepath.Clean(p)),
+	}
+}
+
 func (src *SourceGit) AsState(opts ...llb.ConstraintsOpt) (llb.State, error) {
 	ref, err := gitutil.ParseGitRef(src.URL)
 	if err != nil {

source_test.go

@@ -20,6 +20,7 @@ import (
 	"github.com/opencontainers/go-digest"
 	v1 "github.com/opencontainers/image-spec/specs-go/v1"
 	"gotest.tools/v3/assert"
+	"gotest.tools/v3/assert/cmp"
 )
 
 func TestSourceGitSSH(t *testing.T) {
@@ -527,9 +528,12 @@ func TestSourceContext(t *testing.T) {
 				ops := getSourceOp(ctx, t, src)
 				checkContext(t, ops[0].GetSource(), &src)
 				// With include/exclude only, this should be handled with just one op.
-				if len(ops) != 1 {
+				// except... there are optimizations to prevent fetching the same context multiple times
+				// As such we need to make sure filters are applied correctly.
+				if len(ops) != 2 {
 					t.Fatalf("expected 1 op, got %d\n%s", len(ops), ops)
 				}
+				checkFilter(t, ops[1].GetFile(), &src)
 			})
 
 			t.Run("subpath with include-exclude", func(t *testing.T) {
@@ -540,10 +544,6 @@ func TestSourceContext(t *testing.T) {
 				ops := getSourceOp(ctx, t, src)
 				checkContext(t, ops[0].GetSource(), &src)
 				// for context source, we expect to have a copy operation as the last op when subdir is used
-
-				// set includes, excludes to nil before checking against filter, as includes and excludes are
-				// handled before filter operation for context sources
-				src.Includes, src.Excludes = nil, nil
 				checkFilter(t, ops[1].GetFile(), &src)
 			})
 		})
@@ -879,6 +879,7 @@ func checkGitOp(t *testing.T, ops []*pb.Op, src *Source) {
 }
 
 func checkFilter(t *testing.T, op *pb.FileOp, src *Source) {
+	t.Helper()
 	if op == nil {
 		t.Fatal("expected file op")
 	}
@@ -996,17 +997,32 @@ func checkContext(t *testing.T, op *pb.SourceOp, src *Source) {
 		if string(includesJson) != localIncludes {
 			t.Errorf("expected includes %q on local op, got %q", includesJson, localIncludes)
 		}
+
+	}
+
+	if !isRoot(src.Path) {
+		expect := append(excludeAllButPath(src.Path), src.Excludes...)
+
+		var actual []string
+		localExcludes := op.Attrs["local.excludepatterns"]
+		err := json.Unmarshal([]byte(localExcludes), &actual)
+		assert.NilError(t, err)
+		assert.Check(t, cmp.DeepEqual(actual, expect))
 	}
 
 	if src.Excludes != nil {
-		excludesJson, err := json.Marshal(src.Excludes)
+		v := src.Excludes
+		if src.Path != "" {
+			v = append(excludeAllButPath(src.Path), v...)
+		}
+		excludesJson, err := json.Marshal(v)
 		if err != nil {
 			t.Fatal(err)
 		}
 		localExcludes := op.Attrs["local.excludepatterns"]
 
 		if string(excludesJson) != localExcludes {
-			t.Errorf("expected includes %q on local op, got %q", excludesJson, localExcludes)
+			t.Errorf("expected excludes %q on local op, got %q", excludesJson, localExcludes)
 		}
 	}
 }

test/source_test.go

@@ -3,10 +3,14 @@ package test
 import (
 	"context"
 	"io/fs"
+	"os"
+	"path/filepath"
 	"strings"
 	"testing"
 
 	"github.com/Azure/dalec"
+	"github.com/Azure/dalec/frontend/pkg/bkfs"
+	"github.com/moby/buildkit/client/llb"
 	gwclient "github.com/moby/buildkit/frontend/gateway/client"
 	"github.com/opencontainers/go-digest"
 )
@@ -66,8 +70,8 @@ func TestSourceCmd(t *testing.T) {
 		req := newSolveRequest(withBuildTarget("debug/sources"), withSpec(ctx, t, spec))
 		res := solveT(ctx, t, gwc, req)
 
-		checkFile(ctx, t, "foo", res, []byte("foo bar\n"))
-		checkFile(ctx, t, "hello", res, []byte("hello\n"))
+		checkFile(ctx, t, filepath.Join(sourceName, "foo"), res, []byte("foo bar\n"))
+		checkFile(ctx, t, filepath.Join(sourceName, "hello"), res, []byte("hello\n"))
 	})
 
 	t.Run("with mounted file", func(t *testing.T) {
@@ -97,7 +101,7 @@ func TestSourceCmd(t *testing.T) {
 			req := newSolveRequest(withBuildTarget("debug/sources"), withSpec(ctx, t, spec))
 			res := solveT(ctx, t, gwc, req)
 
-			checkFile(ctx, t, "foo", res, []byte("foo bar"))
+			checkFile(ctx, t, filepath.Join(sourceName, "foo"), res, []byte("foo bar"))
 		})
 	})
 }
@@ -113,7 +117,7 @@ func TestSourceBuild(t *testing.T) {
 				ro := newSolveRequest(withBuildTarget("debug/sources"), withSpec(ctx, t, spec))
 
 				res := solveT(ctx, t, gwc, ro)
-				checkFile(ctx, t, "hello", res, []byte("hello\n"))
+				checkFile(ctx, t, "test/hello", res, []byte("hello\n"))
 			})
 		})
 	}
@@ -370,3 +374,106 @@ index ea874f5..ba38f84 100644
 		})
 	})
 }
+
+func TestSourceContext(t *testing.T) {
+	t.Parallel()
+
+	contextSt := llb.Scratch().
+		File(llb.Mkfile("/base", 0o644, nil)).
+		File(llb.Mkdir("/foo/bar", 0o755, llb.WithParents(true))).
+		File(llb.Mkfile("/foo/file", 0o644, nil)).
+		File(llb.Mkfile("/foo/bar/another", 0o644, nil))
+
+	spec := &dalec.Spec{
+		Name: "test-dalec-context-source",
+		Sources: map[string]dalec.Source{
+			"basic":         {Context: &dalec.SourceContext{}},
+			"with-path":     {Path: "/foo/bar", Context: &dalec.SourceContext{}},
+			"with-includes": {Includes: []string{"foo/**/*"}, Context: &dalec.SourceContext{}},
+			"with-excludes": {Excludes: []string{"foo/**/*"}, Context: &dalec.SourceContext{}},
+			"with-path-and-includes-excludes": {
+				Path:     "/foo",
+				Includes: []string{"file", "bar"},
+				Excludes: []string{"bar/another"},
+				Context:  &dalec.SourceContext{},
+			},
+		},
+	}
+
+	runTest(t, func(ctx context.Context, gwc gwclient.Client) {
+		req := newSolveRequest(withSpec(ctx, t, spec), withBuildContext(ctx, t, "context", contextSt), withBuildTarget("debug/sources"))
+		res := solveT(ctx, t, gwc, req)
+		ref, err := res.SingleRef()
+		if err != nil {
+			t.Fatal(err)
+		}
+
+		dir := bkfs.FromRef(ctx, ref)
+
+		existsNotDir := checkFileStatOpt{Exists: true}
+		existsDir := checkFileStatOpt{Exists: true, IsDir: true}
+		notExists := checkFileStatOpt{}
+
+		checkFileStat(t, dir, "basic/base", existsNotDir)
+		checkFileStat(t, dir, "basic/foo/bar", existsDir)
+		checkFileStat(t, dir, "basic/foo/file", existsNotDir)
+		checkFileStat(t, dir, "basic/foo/bar/another", existsNotDir)
+
+		checkFileStat(t, dir, "with-path/base", notExists)
+		checkFileStat(t, dir, "with-path/foo", notExists)
+		checkFileStat(t, dir, "with-path/another", existsNotDir)
+
+		checkFileStat(t, dir, "with-includes/base", notExists)
+		checkFileStat(t, dir, "with-includes/foo/bar", existsDir)
+		checkFileStat(t, dir, "with-includes/foo/file", existsNotDir)
+		checkFileStat(t, dir, "with-includes/foo/bar/another", existsNotDir)
+
+		checkFileStat(t, dir, "with-excludes/base", existsNotDir)
+		checkFileStat(t, dir, "with-excludes/foo", existsDir)
+		checkFileStat(t, dir, "with-excludes/foo/file", notExists)
+		checkFileStat(t, dir, "with-excludes/foo/bar", notExists)
+
+		checkFileStat(t, dir, "with-path-and-includes-excludes/base", notExists)
+		checkFileStat(t, dir, "with-path-and-includes-excludes/foo", notExists)
+		checkFileStat(t, dir, "with-path-and-includes-excludes/file", existsNotDir)
+		checkFileStat(t, dir, "with-path-and-includes-excludes/bar", existsDir)
+		checkFileStat(t, dir, "with-path-and-includes-excludes/bar/another", notExists)
+	})
+}
+
+type checkFileStatOpt struct {
+	IsDir  bool
+	Exists bool
+}
+
+func checkFileStat(t *testing.T, dir fs.FS, p string, opt checkFileStatOpt) {
+	t.Helper()
+
+	stat, err := fs.Stat(dir, p)
+	if err != nil && !os.IsNotExist(err) {
+		// TODO: the error returned from the buildkit client is not giving us what we want here.
+		// So we need to check the error string for now
+		if !strings.Contains(err.Error(), "no such file or directory") {
+			t.Error(err)
+			return
+		}
+
+		if opt.Exists {
+			t.Errorf("file %q should exist", p)
+		}
+		return
+	}
+
+	if !opt.Exists {
+		t.Errorf("file %q should not exist", p)
+		return
+	}
+
+	if stat == nil {
+		return
+	}
+
+	if stat.IsDir() != opt.IsDir {
+		t.Errorf("expected file %q isDir=%v, got %v", p, opt.IsDir, stat.IsDir())
+	}
+}