campaigns: handle non-root volume use cases more gracefully (#434)

This reverts commit a35c2e7.

Co-authored-by: Thorsten Ball <[email protected]>

Author: LawnGnome

internal/campaigns/action.go

@@ -21,7 +21,7 @@ type dockerBindWorkspaceCreator struct {
 
 var _ WorkspaceCreator = &dockerBindWorkspaceCreator{}
 
-func (wc *dockerBindWorkspaceCreator) Create(ctx context.Context, repo *graphql.Repository, zip string) (Workspace, error) {
+func (wc *dockerBindWorkspaceCreator) Create(ctx context.Context, repo *graphql.Repository, steps []Step, zip string) (Workspace, error) {
 	w, err := wc.unzipToWorkspace(ctx, repo, zip)
 	if err != nil {
 		return nil, errors.Wrap(err, "unzipping the repository")

internal/campaigns/bind_workspace.go

@@ -62,7 +62,7 @@ func TestDockerBindWorkspaceCreator_Create(t *testing.T) {
 		testTempDir := workspaceTmpDir(t)
 
 		creator := &dockerBindWorkspaceCreator{dir: testTempDir}
-		workspace, err := creator.Create(context.Background(), repo, archivePath)
+		workspace, err := creator.Create(context.Background(), repo, nil, archivePath)
 		if err != nil {
 			t.Fatalf("unexpected error: %s", err)
 		}
@@ -90,7 +90,7 @@ func TestDockerBindWorkspaceCreator_Create(t *testing.T) {
 		badZip.Close()
 
 		creator := &dockerBindWorkspaceCreator{dir: testTempDir}
-		if _, err := creator.Create(context.Background(), repo, badZipFile); err == nil {
+		if _, err := creator.Create(context.Background(), repo, nil, badZipFile); err == nil {
 			t.Error("unexpected nil error")
 		}
 	})

internal/campaigns/bind_workspace_test.go

@@ -8,6 +8,7 @@ import (
 	"github.com/sourcegraph/campaignutils/env"
 	"github.com/sourcegraph/campaignutils/overridable"
 	"github.com/sourcegraph/campaignutils/yaml"
+	"github.com/sourcegraph/src-cli/internal/campaigns/docker"
 	"github.com/sourcegraph/src-cli/schema"
 )
 
@@ -73,7 +74,7 @@ type Step struct {
 	Files     map[string]string `json:"files,omitempty" yaml:"files,omitempty"`
 	Outputs   Outputs           `json:"outputs,omitempty" yaml:"outputs,omitempty"`
 
-	image string
+	image docker.Image
 }
 
 type Outputs map[string]Output

internal/campaigns/campaign_spec.go

@@ -0,0 +1,32 @@
+package docker
+
+import "sync"
+
+// ImageCache is a cache of metadata about Docker images, indexed by name.
+type ImageCache struct {
+	images   map[string]Image
+	imagesMu sync.Mutex
+}
+
+// NewImageCache creates a new image cache.
+func NewImageCache() *ImageCache {
+	return &ImageCache{
+		images: make(map[string]Image),
+	}
+}
+
+// Get returns the image cache entry for the given Docker image. The name may be
+// anything the Docker command line will accept as an image name: this will
+// generally be IMAGE or IMAGE:TAG.
+func (ic *ImageCache) Get(name string) Image {
+	ic.imagesMu.Lock()
+	defer ic.imagesMu.Unlock()
+
+	if image, ok := ic.images[name]; ok {
+		return image
+	}
+
+	image := &image{name: name}
+	ic.images[name] = image
+	return image
+}

internal/campaigns/docker/cache.go

@@ -0,0 +1,23 @@
+package docker
+
+import "testing"
+
+func TestImageCache(t *testing.T) {
+	cache := NewImageCache()
+	if cache == nil {
+		t.Error("unexpected nil cache")
+	}
+
+	have := cache.Get("foo")
+	if have == nil {
+		t.Error("unexpected nil error")
+	}
+	if name := have.(*image).name; name != "foo" {
+		t.Errorf("invalid name: have=%q want=%q", name, "foo")
+	}
+
+	again := cache.Get("foo")
+	if have != again {
+		t.Errorf("invalid memoisation: first=%v second=%v", have, again)
+	}
+}

internal/campaigns/docker/cache_test.go

@@ -0,0 +1,147 @@
+package docker
+
+import (
+	"bytes"
+	"context"
+	"fmt"
+	"strings"
+	"sync"
+
+	"github.com/pkg/errors"
+
+	"github.com/sourcegraph/src-cli/internal/exec"
+)
+
+// UIDGID represents a UID:GID pair.
+type UIDGID struct {
+	UID int
+	GID int
+}
+
+func (ug UIDGID) String() string {
+	return fmt.Sprintf("%d:%d", ug.UID, ug.GID)
+}
+
+// Root is a root:root user.
+var Root = UIDGID{UID: 0, GID: 0}
+
+// Image represents a Docker image, hopefully stored in the local cache.
+type Image interface {
+	Digest(context.Context) (string, error)
+	Ensure(context.Context) error
+	UIDGID(context.Context) (UIDGID, error)
+}
+
+type image struct {
+	name string
+
+	// There are lots of once fields below: basically, we're going to try fairly
+	// hard to prevent performing the same operations on the same image over and
+	// over, since some of them are expensive.
+
+	digest     string
+	digestErr  error
+	digestOnce sync.Once
+
+	ensureErr  error
+	ensureOnce sync.Once
+
+	uidGid     UIDGID
+	uidGidErr  error
+	uidGidOnce sync.Once
+}
+
+// Digest gets and returns the content digest for the image. Note that this is
+// different from the "distribution digest" (which is what you can use to
+// specify an image to `docker run`, as in `my/image@sha256:xxx`). We need to
+// use the content digest because the distribution digest is only computed for
+// images that have been pulled from or pushed to a registry. See
+// https://windsock.io/explaining-docker-image-ids/ under "A Final Twist" for a
+// good explanation.
+func (image *image) Digest(ctx context.Context) (string, error) {
+	image.digestOnce.Do(func() {
+		image.digest, image.digestErr = func() (string, error) {
+			if err := image.Ensure(ctx); err != nil {
+				return "", err
+			}
+
+			// TODO!(sqs): is image id the right thing to use here? it is NOT
+			// the digest. but the digest is not calculated for all images
+			// (unless they are pulled/pushed from/to a registry), see
+			// https://github.com/moby/moby/issues/32016.
+			out, err := exec.CommandContext(ctx, "docker", "image", "inspect", "--format", "{{.Id}}", "--", image.name).CombinedOutput()
+			if err != nil {
+				return "", errors.Wrapf(err, "inspecting docker image: %s", string(bytes.TrimSpace(out)))
+			}
+			id := string(bytes.TrimSpace(out))
+			if id == "" {
+				return "", errors.Errorf("unexpected empty docker image content ID for %q", image.name)
+			}
+			return id, nil
+		}()
+	})
+
+	return image.digest, image.digestErr
+}
+
+// Ensure ensures that the image has been pulled by Docker. Note that it does
+// not attempt to pull a newer version of the image if it exists locally.
+func (image *image) Ensure(ctx context.Context) error {
+	image.ensureOnce.Do(func() {
+		image.ensureErr = func() error {
+			// docker image inspect will return a non-zero exit code if the image and
+			// tag don't exist locally, regardless of the format.
+			if err := exec.CommandContext(ctx, "docker", "image", "inspect", "--format", "1", image.name).Run(); err != nil {
+				// Let's try pulling the image.
+				if err := exec.CommandContext(ctx, "docker", "image", "pull", image.name).Run(); err != nil {
+					return errors.Wrap(err, "pulling image")
+				}
+			}
+
+			return nil
+		}()
+	})
+
+	return image.ensureErr
+}
+
+// UIDGID returns the user and group the container is configured to run as.
+func (image *image) UIDGID(ctx context.Context) (UIDGID, error) {
+	image.uidGidOnce.Do(func() {
+		image.uidGid, image.uidGidErr = func() (UIDGID, error) {
+			stdout := new(bytes.Buffer)
+
+			// Digest also implicitly means Ensure has been called.
+			digest, err := image.Digest(ctx)
+			if err != nil {
+				return UIDGID{}, errors.Wrap(err, "getting digest")
+			}
+
+			args := []string{
+				"run",
+				"--rm",
+				"--entrypoint", "/bin/sh",
+				digest,
+				"-c", "id -u; id -g",
+			}
+			cmd := exec.CommandContext(ctx, "docker", args...)
+			cmd.Stdout = stdout
+
+			if err := cmd.Run(); err != nil {
+				return UIDGID{}, errors.Wrap(err, "running id")
+			}
+
+			// POSIX specifies the output of `id -u` as the effective UID,
+			// terminated by a newline. `id -g` is the same, just for the GID.
+			raw := strings.TrimSpace(stdout.String())
+			var res UIDGID
+			_, err = fmt.Sscanf(raw, "%d\n%d", &res.UID, &res.GID)
+			if err != nil {
+				return res, errors.Wrapf(err, "malformed uid/gid: %q", raw)
+			}
+			return res, nil
+		}()
+	})
+
+	return image.uidGid, image.uidGidErr
+}