Repository: openllb/hlb
Pull request #119 (2020-04-22 07:50:35)

allow for directory in arguments to `secret`
Also automatically add mount directories for secrets/mounts on top of `readonly` fs

I will add some codegen tests, just wanted to get your thoughts on this.  It seems to work, greatly simplified the mkdir/mkfile copy/paste I had all over so that I could get caching on builds from local inputs.


Patch:
diff --git a/builtin/builtin.go b/builtin/builtin.go
index d07bdae..fe411e6 100644
--- a/builtin/builtin.go
+++ b/builtin/builtin.go
@@ -385,6 +385,16 @@ var (
 							parser.NewField(parser.Int, "filemode", false),
 						},
 					},
+					"includePatterns": FuncLookup{
+						Params: []*parser.Field{
+							parser.NewField(parser.Str, "pattern", true),
+						},
+					},
+					"excludePatterns": FuncLookup{
+						Params: []*parser.Field{
+							parser.NewField(parser.Str, "pattern", true),
+						},
+					},
 				},
 			},
 			"option::ssh": LookupByType{
diff --git a/codegen/chain.go b/codegen/chain.go
index 2a436ee..271dd77 100644
--- a/codegen/chain.go
+++ b/codegen/chain.go
@@ -5,6 +5,7 @@ import (
 	"context"
 	"fmt"
 	"os"
+	"path/filepath"
 	"runtime"
 	"strings"
 	"text/template"
@@ -333,6 +334,11 @@ func (cg *CodeGen) EmitFilesystemBuiltinChainStmt(ctx context.Context, scope *pa
 		customName := strings.ReplaceAll(shlex, "\n", "")
 		opts = append(opts, llb.Shlex(shlex), llb.WithCustomName(customName))
 
+		err = fixReadonlyMounts(opts)
+		if err != nil {
+			return nil, err
+		}
+
 		fc = func(st llb.State) (llb.State, error) {
 			exec := st.Run(opts...)
 
@@ -588,6 +594,166 @@ func (cg *CodeGen) EmitFilesystemBuiltinChainStmt(ctx context.Context, scope *pa
 	return fc, nil
 }
 
+// fixReadonlyMounts will modify the source for readonly mounts so subsequent mounts that
+// mount onto the readonly-mounts will have the mountpoint present.  For example if we
+// have this code:
+//
+// 		run "make" with option {
+//			dir "/src"
+//			mount fs {
+//				local "."
+//			} "/src" with readonly
+//			mount scratch "/src/output" as buildOutput
+//			# ^^^^^ FAIL cannot create `output` directory for mount on readonly FS
+//			secret "./secret/foo.pem" "/src/secret/foo.pem"
+//			# ^^^^^ FAIL cannot create `./secret/foo.pm` for secret on readonly FS
+//		}
+//
+// when containerd tries to mount /src/output on top of the /src mountpoint it will
+// fail because /src is mounted as readonly.  The work around for this is to
+// inline create the mountpoints so that they pre-exist and containerd will not have
+// to create them.  It can be done with HLB like:
+//
+//		run "make" with option {
+//			dir "/src"
+//			mount fs {
+//				local "."
+//				mkdir "output" 0o755 # <-- this is added to ensure mountpoint exists
+//				mkdir "secret" 0o755            # <-- added so the secret can be mounted
+//				mkfile "secret/foo.pm" 0o644 "" # <-- added so the secret can be mounted
+//			} "/src" with readonly
+//			mount scratch "/src/output" as buildOutput
+//		}
+//
+// So this function is effectively automatically adding the `mkdir` and `mkfile` instructions
+// when it detects that a mountpoint is required to be on a readonly FS.
+func fixReadonlyMounts(opts []llb.RunOption) error {
+	// short-circuit if we don't have any readonly mounts
+	haveReadonly := false
+	for _, opt := range opts {
+		if mnt, ok := opt.(*mountRunOption); ok {
+			haveReadonly = mnt.IsReadonly()
+			if haveReadonly {
+				break
+			}
+		}
+	}
+	if !haveReadonly {
+		return nil
+	}
+
+	// collecting run options to look for targets (secrets, mounts) so we can
+	// determine if there are overlapping mounts with readonly attributes
+	mountDetails := make([]struct {
+		Target string
+		Mount  *mountRunOption
+	}, len(opts))
+
+	for i, opt := range opts {
+		switch runOpt := opt.(type) {
+		case *mountRunOption:
+			mountDetails[i].Target = runOpt.Target
+			mountDetails[i].Mount = runOpt
+		case llb.RunOption:
+			ei := llb.ExecInfo{}
+			runOpt.SetRunOption(&ei)
+			if len(ei.Secrets) > 0 {
+				// we only processed one option, so can have at most one secret
+				mountDetails[i].Target = ei.Secrets[0].Target
+				continue
+			}
+		}
+	}
+
+	// madeDirs will keep track of directories we have had to create
+	// so we don't duplicate instructions
+	madeDirs := map[string]struct{}{}
+
+	// if we have readonly mounts and then secrets or other mounts on top of the
+	// readonly mounts then we have to run a mkdir or mkfile on the mount first
+	// before it become readonly
+
+	// now walk the mountDetails backwards and look for common target paths
+	// in prior mounts (mount ordering is significant).
+	for i := len(mountDetails) - 1; i >= 0; i-- {
+		src := mountDetails[i]
+		if src.Target == "" {
+			// not a target option, like `dir "foo"`, so just skip
+			continue
+		}
+		for j := i - 1; j >= 0; j-- {
+			dest := mountDetails[j]
+			if !strings.HasPrefix(src.Target, dest.Target) {
+				// paths not common, skip
+				continue
+			}
+			if dest.Mount == nil {
+				// dest is not a mount, so skip
+				continue
+			}
+			if !dest.Mount.IsReadonly() {
+				// not mounting into readonly fs, so we are good with this mount
+				break
+			}
+
+			// we need to rewrite the mount at opts[j] so that that we mkdir and/or mkfile
+			st := dest.Mount.Source
+			if src.Mount != nil {
+				// this is a mount, so we need to ensure the mount point
+				// directory has been created
+				if _, ok := madeDirs[src.Target]; ok {
+					// already created the dir
+					break
+				}
+				// update local cache so we don't make this dir again
+				madeDirs[dest.Target] = struct{}{}
+
+				relativeDir, err := filepath.Rel(dest.Target, src.Target)
+				if err != nil {
+					return err
+				}
+				st = st.File(
+					llb.Mkdir(relativeDir, os.FileMode(0755), llb.WithParents(true)),
+				)
+			} else {
+				// not a mount, so must be a `secret` which will be a path
+				// to a file, we will need to make the directory for the
+				// secret as well as an empty file to be mounted over
+				dir := filepath.Dir(src.Target)
+				relativeDir := strings.TrimPrefix(dir, dest.Target)
+
+				if _, ok := madeDirs[dir]; !ok {
+					// update local cache so we don't make this dir again
+					madeDirs[dir] = struct{}{}
+
+					st = st.File(
+						llb.Mkdir(relativeDir, os.FileMode(0755), llb.WithParents(true)),
+					)
+				}
+				relativeFile, err := filepath.Rel(dest.Target, src.Target)
+				if err != nil {
+					return err
+				}
+				st = st.File(
+					llb.Mkfile(relativeFile, os.FileMode(0644), []byte{}),
+				)
+			}
+
+			// reset the mount option to include our state with mkdir/mkfile actions
+			opts[j] = &mountRunOption{
+				Target: dest.Target,
+				Source: st,
+				Opts:   dest.Mount.Opts,
+			}
+
+			// save the state for later in case we need to add more mkdir/mkfile actions
+			mountDetails[j].Mount.Source = st
+			break
+		}
+	}
+	return nil
+}
+
 func (cg *CodeGen) EmitStringChainStmt(ctx context.Context, scope *parser.Scope, expr *parser.Expr, args []*parser.Expr, with *parser.WithOpt, chainStart interface{}) (StringChain, error) {
 	var iopts []interface{}
 	var err error
diff --git a/codegen/codegen.go b/codegen/codegen.go
index 7f30911..fecf537 100644
--- a/codegen/codegen.go
+++ b/codegen/codegen.go
@@ -1060,38 +1060,121 @@ func (cg *CodeGen) EmitExecOptions(ctx context.Context, scope *parser.Scope, op
 
 				opts = append(opts, llb.AddSSHSocket(sshOpts...))
 			case "secret":
-				localPath, err := cg.EmitStringExpr(ctx, scope, args[0])
+				localPathArg, err := cg.EmitStringExpr(ctx, scope, args[0])
 				if err != nil {
 					return opts, err
 				}
 
-				localPath, err = ResolvePathForNode(scope.Node, localPath)
+				localPathArg, err = ResolvePathForNode(scope.Node, localPathArg)
 				if err != nil {
 					return opts, err
 				}
 
-				mountPoint, err := cg.EmitStringExpr(ctx, scope, args[1])
-				if err != nil {
-					return opts, err
-				}
+				var includePatterns []string
+				var excludePatterns []string
 
-				id := SecretID(localPath)
+				secretOpts := []llb.SecretOption{}
 
-				// Register path as an allowed file source for the session.
-				cg.fileSourceByID[id] = secretsprovider.FileSource{
-					ID:       id,
-					FilePath: localPath,
+				for _, iopt := range iopts {
+					switch opt := iopt.(type) {
+					case *secretIncludePatterns:
+						includePatterns = opt.Patterns
+					case *secretExcludePatterns:
+						excludePatterns = opt.Patterns
+					case llb.SecretOption:
+						secretOpts = append(secretOpts, opt)
+					}
 				}
 
-				secretOpts := []llb.SecretOption{
-					llb.SecretID(id),
+				localPaths := []string{}
+				if st, err := os.Stat(localPathArg); err != nil {
+					return opts, err
+				} else {
+					switch {
+					case st.Mode().IsRegular():
+						localPaths = append(localPaths, localPathArg)
+					case st.Mode().IsDir():
+						err := filepath.Walk(localPathArg, func(walkPath string, info os.FileInfo, err error) error {
+							if err != nil {
+								return err
+							}
+							relPath, err := filepath.Rel(localPathArg, walkPath)
+							if err != nil {
+								return err
+							}
+							if relPath == "." {
+								return nil
+							}
+							if len(includePatterns) > 0 {
+								for _, pattern := range includePatterns {
+									if ok, err := filepath.Match(pattern, relPath); ok && err == nil {
+										if info.Mode().IsRegular() {
+											localPaths = append(localPaths, walkPath)
+										}
+										return nil
+									} else if err != nil {
+										return err
+									}
+								}
+								// didn't match include, so skip directory
+								if info.Mode().IsDir() {
+									return filepath.SkipDir
+								}
+								return nil
+							} else if len(excludePatterns) > 0 {
+								for _, pattern := range excludePatterns {
+									if ok, err := filepath.Match(pattern, relPath); !ok && err == nil {
+										if info.Mode().IsDir() {
+											return filepath.SkipDir
+										}
+										return nil
+									} else if err != nil {
+										return err
+									}
+								}
+								// didn't match exclude to add it to list
+								if info.Mode().IsRegular() {
+									localPaths = append(localPaths, walkPath)
+								}
+								return nil
+							}
+							if info.Mode().IsRegular() {
+								localPaths = append(localPaths, walkPath)
+							}
+							return nil
+						})
+						if err != nil {
+							return opts, err
+						}
+					default:
+						return opts, errors.Errorf("Unexpected secret file type at %s", localPathArg)
+					}
 				}
-				for _, iopt := range iopts {
-					opt := iopt.(llb.SecretOption)
-					secretOpts = append(secretOpts, opt)
+
+				mountPointArg, err := cg.EmitStringExpr(ctx, scope, args[1])
+				if err != nil {
+					return opts, err
 				}
 
-				opts = append(opts, llb.AddSecret(mountPoint, secretOpts...))
+				for _, localPath := range localPaths {
+					mountPoint := filepath.Join(
+						mountPointArg,
+						strings.TrimPrefix(localPath, localPathArg),
+					)
+
+					id := SecretID(localPath)
+
+					// Register path as an allowed file source for the session.
+					cg.fileSourceByID[id] = secretsprovider.FileSource{
+						ID:       id,
+						FilePath: localPath,
+					}
+
+					opts = append(opts, llb.AddSecret(
+						mountPoint,
+						append(secretOpts, llb.SecretID(id))...,
+					))
+				}
 			case "mount":
 				input, err := cg.EmitFilesystemExpr(ctx, scope, args[0], ac)
 				if err != nil {
@@ -1103,13 +1186,11 @@ func (cg *CodeGen) EmitExecOptions(ctx context.Context, scope *parser.Scope, op
 					return opts, err
 				}
 
-				var mountOpts []llb.MountOption
-				for _, iopt := range iopts {
-					opt := iopt.(llb.MountOption)
-					mountOpts = append(mountOpts, opt)
-				}
-
-				opts = append(opts, llb.AddMount(target, input, mountOpts...))
+				opts = append(opts, &mountRunOption{
+					Target: target,
+					Source: input,
+					Opts:   iopts,
+				})
 			default:
 				iopts, err := cg.EmitOptionLookup(ctx, scope, expr, args, op)
 				if err != nil {
@@ -1122,6 +1203,37 @@ func (cg *CodeGen) EmitExecOptions(ctx context.Context, scope *parser.Scope, op
 	return
 }
 
+// mountRunOption gives access to capture custom MountOptions so we
+// can easily capture if the mount is to be readonly
+type mountRunOption struct {
+	Target string
+	Source llb.State
+	Opts   []interface{}
+}
+
+type readonlyMount struct{}
+
+func (m *mountRunOption) SetRunOption(es *llb.ExecInfo) {
+	opts := []llb.MountOption{}
+	for _, opt := range m.Opts {
+		if _, ok := opt.(*readonlyMount); ok {
+			opts = append(opts, llb.Readonly)
+			continue
+		}
+		opts = append(opts, opt.(llb.MountOption))
+	}
+	llb.AddMount(m.Target, m.Source, opts...).SetRunOption(es)
+}
+
+func (m *mountRunOption) IsReadonly() bool {
+	for _, opt := range m.Opts {
+		if _, ok := opt.(*readonlyMount); ok {
+			return true
+		}
+	}
+	return false
+}
+
 type sshSocketOpt struct {
 	target string
 	uid    int
@@ -1215,6 +1327,14 @@ type secretOpt struct {
 	mode os.FileMode
 }
 
+type secretIncludePatterns struct {
+	Patterns []string
+}
+
+type secretExcludePatterns struct {
+	Patterns []string
+}
+
 func (cg *CodeGen) EmitSecretOptions(ctx context.Context, scope *parser.Scope, op string, stmts []*parser.Stmt) (opts []interface{}, err error) {
 	var sopt *secretOpt
 	for _, stmt := range stmts {
@@ -1254,6 +1374,24 @@ func (cg *CodeGen) EmitSecretOptions(ctx context.Context, scope *parser.Scope, o
 					sopt = &secretOpt{}
 				}
 				sopt.mode = os.FileMode(mode)
+			case "includePatterns":
+				patterns := make([]string, len(args))
+				for i, arg := range args {
+					patterns[i], err = cg.EmitStringExpr(ctx, scope, arg)
+					if err != nil {
+						return opts, err
+					}
+				}
+				opts = append(opts, &secretIncludePatterns{patterns})
+			case "excludePatterns":
+				patterns := make([]string, len(args))
+				for i, arg := range args {
+					patterns[i], err = cg.EmitStringExpr(ctx, scope, arg)
+					if err != nil {
+						return opts, err
+					}
+				}
+				opts = append(opts, &secretExcludePatterns{patterns})
 			default:
 				iopts, err := cg.EmitOptionLookup(ctx, scope, stmt.Call.Func, args, op)
 				if err != nil {
@@ -1286,7 +1424,7 @@ func (cg *CodeGen) EmitMountOptions(ctx context.Context, scope *parser.Scope, op
 					return opts, err
 				}
 				if v {
-					opts = append(opts, llb.MountOption(llb.Readonly))
+					opts = append(opts, &readonlyMount{})
 				}
 			case "tmpfs":
 				v, err := cg.MaybeEmitBoolExpr(ctx, scope, args)
diff --git a/language/builtin.hlb b/language/builtin.hlb
index 6d1ea6e..d07c85a 100644
--- a/language/builtin.hlb
+++ b/language/builtin.hlb
@@ -268,6 +268,18 @@ option::secret gid(int id)
 # @return an option to set the permissions of the secure file.
 option::secret mode(int filemode)
 
+# Attach secrets only for files that match any of the included patterns.
+#
+# @param pattern a list of patterns for files that should be attached as secrets
+# @return an option to attach files that match any pattern.
+option::secret includePatterns(variadic string pattern)
+
+# Attach secrets only for files that do not match any of the excluded patterns.
+#
+# @param pattern a list of patterns for files that should not be attached as secrets
+# @return an option to attach files that don't match any pattern.
+option::secret excludePatterns(variadic string pattern)
+
 # Sets the mount to be attached as a read-only filesystem.
 #
 # @return an option to attach the mount as a read-only filesystem..


Test patch:
diff --git a/codegen/codegen_test.go b/codegen/codegen_test.go
index e7c4da5..aae4e04 100644
--- a/codegen/codegen_test.go
+++ b/codegen/codegen_test.go
@@ -317,6 +317,55 @@ func TestCodeGen(t *testing.T) {
 				solver.WithEntitlement(entitlements.EntitlementSecurityInsecure),
 			)
 		},
+	}, {
+		"mount over readonly",
+		[]string{"default"},
+		`
+		fs default() {
+			image "busybox"
+			run "find ." with option {
+				dir "/foo"
+				mount fs {
+					local "."
+				} "/foo" with readonly
+				mount scratch "/foo/bar"
+				secret "codegen_test.go" "/foo/secret/codegen_test.go"
+			}
+		}
+		`,
+		func(t *testing.T, cg *CodeGen) solver.Request {
+			id, err := cg.LocalID(".")
+			require.NoError(t, err)
+			sid := SecretID("codegen_test.go")
+
+			return Expect(t, llb.Image("busybox").Run(
+				llb.Shlex("find ."),
+				llb.Dir("/foo"),
+				llb.AddMount(
+					"/foo",
+					llb.Local(
+						id,
+						llb.SessionID(cg.SessionID()),
+						llb.SharedKeyHint("."),
+					).File(
+						// this Mkdir is made implicitly due to /foo/secret
+						// secret over readonly FS
+						llb.Mkdir("secret", 0755, llb.WithParents(true)),
+					).File(
+						// this Mkfile is made implicitly due to /foo/secret
+						// secret over readonly FS
+						llb.Mkfile("secret/codegen_test.go", 0644, []byte{}),
+					).File(
+						// this Mkdir is made implicitly due to /foo/bar
+						// mount over readonly FS
+						llb.Mkdir("bar", 0755, llb.WithParents(true)),
+					),
+					llb.Readonly,
+				),
+				llb.AddMount("/foo/bar", llb.Scratch()),
+				llb.AddSecret("/foo/secret/codegen_test.go", llb.SecretID(sid)),
+			).Root())
+		},
 	}} {
 		tc := tc
 		t.Run(tc.name, func(t *testing.T) {