Merge pull request #324 from ijc/readonly-rootfs-execop

Make llb.ReadonlyRootFS usable with common container images

client/client_test.go

@@ -45,6 +45,7 @@ func TestClientIntegration(t *testing.T) {
 		testSchema1Image,
 		testMountWithNoSource,
 		testInvalidExporter,
+		testReadonlyRootFS,
 	})
 }
 
@@ -729,6 +730,35 @@ func testMountWithNoSource(t *testing.T, sb integration.Sandbox) {
 	checkAllReleasable(t, c, sb, true)
 }
 
+// #324
+func testReadonlyRootFS(t *testing.T, sb integration.Sandbox) {
+	t.Parallel()
+	c, err := New(sb.Address())
+	require.NoError(t, err)
+	defer c.Close()
+
+	busybox := llb.Image("docker.io/library/busybox:latest")
+	st := llb.Scratch()
+
+	// The path /foo should be unwriteable.
+	run := busybox.Run(
+		llb.ReadonlyRootFS(),
+		llb.Args([]string{"/bin/touch", "/foo"}))
+	st = run.AddMount("/mnt", st)
+
+	def, err := st.Marshal()
+	require.NoError(t, err)
+
+	err = c.Solve(context.TODO(), def, SolveOpt{}, nil)
+	require.Error(t, err)
+	// Would prefer to detect more specifically "Read-only file
+	// system" but that isn't exposed here (it is on the stdio
+	// which we don't see).
+	require.Contains(t, err.Error(), "executor failed running [/bin/touch /foo]:")
+
+	checkAllReleasable(t, c, sb, true)
+}
+
 func requiresLinux(t *testing.T) {
 	if runtime.GOOS != "linux" {
 		t.Skipf("unsupported GOOS: %s", runtime.GOOS)

executor/containerdexecutor/executor.go

@@ -62,7 +62,11 @@ func (w containerdExecutor) Exec(ctx context.Context, meta executor.Meta, root c
 		lm.Unmount()
 	}
 
-	spec, cleanup, err := oci.GenerateSpec(ctx, meta, mounts, id, resolvConf, hostsFile, containerdoci.WithUIDGID(uid, gid))
+	opts := []containerdoci.SpecOpts{containerdoci.WithUIDGID(uid, gid)}
+	if meta.ReadonlyRootFS {
+		opts = append(opts, containerdoci.WithRootFSReadonly())
+	}
+	spec, cleanup, err := oci.GenerateSpec(ctx, meta, mounts, id, resolvConf, hostsFile, opts...)
 	if err != nil {
 		return err
 	}

executor/executor.go

@@ -8,11 +8,12 @@ import (
 )
 
 type Meta struct {
-	Args []string
-	Env  []string
-	User string
-	Cwd  string
-	Tty  bool
+	Args           []string
+	Env            []string
+	User           string
+	Cwd            string
+	Tty            bool
+	ReadonlyRootFS bool
 	// DisableNetworking bool
 }
 

executor/runcexecutor/executor.go

@@ -99,7 +99,11 @@ func (w *runcExecutor) Exec(ctx context.Context, meta executor.Meta, root cache.
 		return err
 	}
 	defer f.Close()
-	spec, cleanup, err := oci.GenerateSpec(ctx, meta, mounts, id, resolvConf, hostsFile, containerdoci.WithUIDGID(uid, gid), seccomp.WithDefaultProfile())
+	opts := []containerdoci.SpecOpts{containerdoci.WithUIDGID(uid, gid), seccomp.WithDefaultProfile()}
+	if meta.ReadonlyRootFS {
+		opts = append(opts, containerdoci.WithRootFSReadonly())
+	}
+	spec, cleanup, err := oci.GenerateSpec(ctx, meta, mounts, id, resolvConf, hostsFile, opts...)
 	if err != nil {
 		return err
 	}

frontend/dockerfile/dockerfile2llb/convert.go

@@ -457,7 +457,7 @@ func dispatchCopy(d *dispatchState, c instructions.SourcesAndDest, sourceState l
 	commitMessage.WriteString(" " + c.Dest())
 
 	args = append(args, dest)
-	run := img.Run(append([]llb.RunOption{llb.Args(args), dfCmd(cmdToPrint)}, mounts...)...)
+	run := img.Run(append([]llb.RunOption{llb.Args(args), llb.ReadonlyRootFS(), dfCmd(cmdToPrint)}, mounts...)...)
 	d.state = run.AddMount("/dest", d.state)
 
 	return commitToHistory(&d.image, commitMessage.String(), true, &d.state)

frontend/gateway/gateway.go

@@ -64,6 +64,7 @@ func (gf *gatewayFrontend) Solve(ctx context.Context, llbBridge frontend.Fronten
 	_, isDevel := opts[keyDevel]
 	var img ocispec.Image
 	var rootFS cache.ImmutableRef
+	var readonly bool // TODO: try to switch to read-only by default.
 
 	if isDevel {
 		ref, exp, err := llbBridge.Solve(session.NewContext(ctx, "gateway:"+sid),
@@ -154,9 +155,10 @@ func (gf *gatewayFrontend) Solve(ctx context.Context, llbBridge frontend.Fronten
 	}()
 
 	err = llbBridge.Exec(ctx, executor.Meta{
-		Env:  env,
-		Args: args,
-		Cwd:  cwd,
+		Env:            env,
+		Args:           args,
+		Cwd:            cwd,
+		ReadonlyRootFS: readonly,
 	}, rootFS, lbf.Stdin, lbf.Stdout, os.Stderr)
 
 	if err != nil {

hack/test

@@ -8,7 +8,7 @@ docker build --iidfile $iidfile --target integration-tests -f ./hack/dockerfiles
 
 iid=$(cat $iidfile)
 
-docker run --rm -v /tmp --privileged $iid go test -tags no_containerd_worker ${TESTFLAGS:--v} ${TESTPKGS:-./...}
+docker run --rm -v /tmp --privileged $iid go test ${TESTFLAGS:--v} ${TESTPKGS:-./...}
 
 docker run --rm $iid go build ./frontend/gateway/client
 docker run --rm $iid go build ./frontend/dockerfile/cmd/dockerfile-frontend

solver/llbop/exec.go

@@ -59,6 +59,7 @@ func (e *execOp) Run(ctx context.Context, inputs []solver.Ref) ([]solver.Ref, er
 	var mounts []executor.Mount
 	var outputs []solver.Ref
 	var root cache.Mountable
+	var readonlyRootFS bool
 
 	defer func() {
 		for _, o := range outputs {
@@ -83,11 +84,16 @@ func (e *execOp) Run(ctx context.Context, inputs []solver.Ref) ([]solver.Ref, er
 			}
 			mountable = ref
 		}
+		activate := func(cache.ImmutableRef) (cache.MutableRef, error) {
+			desc := fmt.Sprintf("mount %s from exec %s", m.Dest, strings.Join(e.op.Meta.Args, " "))
+			return e.cm.New(ctx, ref, cache.WithDescription(desc)) // TODO: should be method `immutableRef.New() mutableRef`
+		}
+
 		if m.Output != pb.SkipOutput {
 			if m.Readonly && ref != nil && m.Dest != pb.RootMount { // exclude read-only rootfs
 				outputs = append(outputs, solver.NewSharedRef(ref).Clone())
 			} else {
-				active, err := e.cm.New(ctx, ref, cache.WithDescription(fmt.Sprintf("mount %s from exec %s", m.Dest, strings.Join(e.op.Meta.Args, " ")))) // TODO: should be method
+				active, err := activate(ref)
 				if err != nil {
 					return nil, err
 				}
@@ -102,6 +108,17 @@ func (e *execOp) Run(ctx context.Context, inputs []solver.Ref) ([]solver.Ref, er
 
 		if m.Dest == pb.RootMount {
 			root = mountable
+			readonlyRootFS = m.Readonly
+			if m.Output == pb.SkipOutput && readonlyRootFS {
+				active, err := activate(ref)
+				if err != nil {
+					return nil, err
+				}
+				defer func() {
+					go active.Release(context.TODO())
+				}()
+				root = active
+			}
 		} else {
 			mounts = append(mounts, executor.Mount{Src: mountable, Dest: m.Dest, Readonly: m.Readonly, Selector: m.Selector})
 		}
@@ -112,10 +129,11 @@ func (e *execOp) Run(ctx context.Context, inputs []solver.Ref) ([]solver.Ref, er
 	})
 
 	meta := executor.Meta{
-		Args: e.op.Meta.Args,
-		Env:  e.op.Meta.Env,
-		Cwd:  e.op.Meta.Cwd,
-		User: e.op.Meta.User,
+		Args:           e.op.Meta.Args,
+		Env:            e.op.Meta.Env,
+		Cwd:            e.op.Meta.Cwd,
+		User:           e.op.Meta.User,
+		ReadonlyRootFS: readonlyRootFS,
 	}
 
 	stdout, stderr := logs.NewLogStreams(ctx)

worker/runc/runc_test.go

@@ -1,4 +1,4 @@
-// +build linux,no_containerd_worker
+// +build linux,!no_runc_worker
 
 package runc