diff --git a/.github/workflows/docker.yml b/.github/workflows/docker.yml new file mode 100644 index 0000000000..3af0f7929f --- /dev/null +++ b/.github/workflows/docker.yml @@ -0,0 +1,42 @@ +# For more information, refer to the "Dependent Docker images" section of +# DEVELOPMENT.md. +name: Publish Docker image dependencies + +# We only want to build on releases; this condition is 100% stolen from the +# goreleaser action. +on: + push: + tags: + - "*" + - "!latest" + +jobs: + publish: + runs-on: ubuntu-20.04 + steps: + - name: Checkout + uses: actions/checkout@v2 + + # We need buildx to be able to build a multi-architecture image. + - name: Set up Docker buildx + uses: docker/setup-buildx-action@v1 + + # We also need QEMU, since this is running on an AMD64 host and we want to + # build ARM64 images. + - name: Set up QEMU + uses: docker/setup-qemu-action@v1 + with: + platforms: arm64 + + - run: ./docker/campaign-volume-workspace/push.py -d ./docker/campaign-volume-workspace/Dockerfile -i sourcegraph/src-campaign-volume-workspace -p linux/amd64,linux/arm64,linux/386 + env: + DOCKER_PASSWORD: ${{ secrets.DOCKER_PASSWORD }} + DOCKER_USERNAME: sourcegraphci + + - name: Update Docker Hub description + uses: peter-evans/dockerhub-description@v2 + with: + username: sourcegraphci + password: ${{ secrets.DOCKER_PASSWORD }} + repository: sourcegraph/src-campaign-volume-workspace + readme-filepath: ./docker/campaign-volume-workspace/README.md diff --git a/CHANGELOG.md b/CHANGELOG.md index 853023b86d..a579b74e46 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -13,6 +13,8 @@ All notable changes to `src-cli` are documented in this file. ### Added +- `src campaign [apply|preview]` can now make use of Docker volumes, rather than bind-mounting the host filesystem. This is now the default on macOS, as volume mounts have generally better performance there. The optional `-workspace` flag can be used to override the default. [#412](https://github.com/sourcegraph/src-cli/pull/412) + ### Changed - `src login` now defaults to validating against `SRC_ENDPOINT` if configured. diff --git a/DEVELOPMENT.md b/DEVELOPMENT.md index 1d3f6a0178..8fc2a1d6b1 100644 --- a/DEVELOPMENT.md +++ b/DEVELOPMENT.md @@ -38,3 +38,19 @@ For example, suppose we have the the recommended versions. If a new feature is added to a new `3.91.6` release of src-cli and this change requires only features available in Sourcegraph `3.99`, then this feature should also be present in a new `3.85.8` release of src-cli. Because a Sourcegraph instance will automatically select the highest patch version, all non-breaking changes should increment only the patch version. Note that if instead the recommended src-cli version for Sourcegraph `3.99` was `3.90.4` in the example above, there is no additional step required, and the new patch version of src-cli will be available to both Sourcegraph versions. + +## Dependent Docker images + +`src campaign apply` and `src campaign preview` use a Docker image published as `sourcegraph/src-campaign-volume-workspace` for utility purposes when the volume workspace is selected, which is the default on macOS. This [Docker image](./docker/campaign-volume-workspace/Dockerfile) is built by [a Python script](./docker/campaign-volume-workspace/push.py) invoked by the GitHub Action workflow described in [`docker.yml`](.github/workflows/docker.yml). + +To build and develop this locally, you can build and tag the image with: + +```sh +docker build -t sourcegraph/src-campaign-volume-workspace - < docker/campaign-volume-workspace/Dockerfile +``` + +To remove it and then force the upstream image on Docker Hub to be used again: + +```sh +docker rmi sourcegraph/src-campaign-volume-workspace +``` diff --git a/cmd/src/campaigns_apply.go b/cmd/src/campaigns_apply.go index 958550ed67..bbafd5260e 100644 --- a/cmd/src/campaigns_apply.go +++ b/cmd/src/campaigns_apply.go @@ -70,6 +70,7 @@ Examples: svc := campaigns.NewService(&campaigns.ServiceOpts{ AllowUnsupported: flags.allowUnsupported, Client: cfg.apiClient(flags.api, flagSet.Output()), + Workspace: flags.workspace, }) if err := svc.DetermineFeatureFlags(ctx); err != nil { diff --git a/cmd/src/campaigns_common.go b/cmd/src/campaigns_common.go index 3c13e6c401..64fd032d19 100644 --- a/cmd/src/campaigns_common.go +++ b/cmd/src/campaigns_common.go @@ -40,6 +40,7 @@ type campaignsApplyFlags struct { namespace string parallelism int timeout time.Duration + workspace string cleanArchives bool skipErrors bool } @@ -100,6 +101,20 @@ func newCampaignsApplyFlags(flagSet *flag.FlagSet, cacheDir, tempDir string) *ca "If true, errors encountered while executing steps in a repository won't stop the execution of the campaign spec but only cause that repository to be skipped.", ) + // We default to bind workspaces on everything except ARM64 macOS at + // present. In the future, we'll likely want to switch the default for ARM64 + // macOS as well, but this requires access to the hardware for testing. + var defaultWorkspace string + if runtime.GOOS == "darwin" && runtime.GOARCH == "amd64" { + defaultWorkspace = "volume" + } else { + defaultWorkspace = "bind" + } + flagSet.StringVar( + &caf.workspace, "workspace", defaultWorkspace, + `Workspace mode to use ("bind" or "volume")`, + ) + flagSet.BoolVar(verbose, "v", false, "print verbose output") return caf @@ -176,18 +191,20 @@ func campaignsExecute(ctx context.Context, out *output.Output, svc *campaigns.Se } opts := campaigns.ExecutorOpts{ - Cache: svc.NewExecutionCache(flags.cacheDir), - Creator: svc.NewWorkspaceCreator(flags.cacheDir, flags.cleanArchives), - ClearCache: flags.clearCache, - KeepLogs: flags.keepLogs, - Timeout: flags.timeout, - TempDir: flags.tempDir, + Cache: svc.NewExecutionCache(flags.cacheDir), + Creator: svc.NewWorkspaceCreator(flags.cacheDir), + RepoFetcher: svc.NewRepoFetcher(flags.cacheDir, flags.cleanArchives), + ClearCache: flags.clearCache, + KeepLogs: flags.keepLogs, + Timeout: flags.timeout, + TempDir: flags.tempDir, } if flags.parallelism <= 0 { opts.Parallelism = runtime.GOMAXPROCS(0) } else { opts.Parallelism = flags.parallelism } + out.VerboseLine(output.Linef("🚧", output.StyleSuccess, "Workspace creator: %T", opts.Creator)) executor := svc.NewExecutor(opts) if errs != nil { @@ -210,10 +227,10 @@ func campaignsExecute(ctx context.Context, out *output.Output, svc *campaigns.Se imageProgress := out.Progress([]output.ProgressBar{{ Label: "Preparing container images", - Max: float64(len(campaignSpec.Steps)), + Max: 1.0, }}, nil) - err = svc.SetDockerImages(ctx, campaignSpec, func(step int) { - imageProgress.SetValue(0, float64(step)) + err = svc.SetDockerImages(ctx, opts.Creator, campaignSpec, func(perc float64) { + imageProgress.SetValue(0, perc) }) if err != nil { return "", "", err diff --git a/cmd/src/campaigns_preview.go b/cmd/src/campaigns_preview.go index 83ce4b98f3..fdd5ce7fd1 100644 --- a/cmd/src/campaigns_preview.go +++ b/cmd/src/campaigns_preview.go @@ -45,6 +45,7 @@ Examples: svc := campaigns.NewService(&campaigns.ServiceOpts{ AllowUnsupported: flags.allowUnsupported, Client: cfg.apiClient(flags.api, flagSet.Output()), + Workspace: flags.workspace, }) if err := svc.DetermineFeatureFlags(ctx); err != nil { diff --git a/docker/campaign-volume-workspace/Dockerfile b/docker/campaign-volume-workspace/Dockerfile new file mode 100644 index 0000000000..ce4cc19961 --- /dev/null +++ b/docker/campaign-volume-workspace/Dockerfile @@ -0,0 +1,13 @@ +# This Dockerfile builds the sourcegraph/src-campaign-volume-workspace image +# that we use to run curl, git, and unzip against a Docker volume when using +# the volume workspace. + +FROM alpine:3.12.3 + +# Note that we have to configure git's user.email and user.name settings to +# avoid issues when committing changes. These values are not used when creating +# changesets, since we only extract the diff from the container and not a full +# patch, but need to be set to avoid git erroring out. +RUN apk add --update curl git unzip && \ + git config --global user.email campaigns@sourcegraph.com && \ + git config --global user.name 'Sourcegraph Campaigns' diff --git a/docker/campaign-volume-workspace/README.md b/docker/campaign-volume-workspace/README.md new file mode 100644 index 0000000000..c4a4a9ed6a --- /dev/null +++ b/docker/campaign-volume-workspace/README.md @@ -0,0 +1,7 @@ +# `src` volume workspace base image + +Sourcegraph `src` executes campaigns using either a bind or volume workspace. In the latter case (which is the default on macOS), this utility image is used to initialise the volume workspace within Docker, and then to extract the diff used when creating the changeset. + +This image is based on Alpine, and adds the tools we need: curl, git, and unzip. + +For more information, please refer to the [`src-cli` repository](https://github.com/sourcegraph/src-cli/tree/main/docker/campaign-volume-workspace). diff --git a/docker/campaign-volume-workspace/push.py b/docker/campaign-volume-workspace/push.py new file mode 100755 index 0000000000..d230afb50c --- /dev/null +++ b/docker/campaign-volume-workspace/push.py @@ -0,0 +1,136 @@ +#!/usr/bin/env python3 + +# This is a very simple script to build and push the Docker image used by the +# Docker volume workspace driver. It's normally run from the "Publish Docker +# image dependencies" GitHub Action, but can be run locally if necessary. +# +# This script requires Python 3.8 or later, and Docker 19.03 (for buildx). You +# are strongly encouraged to use Black to format this file after modifying it. +# +# To run it locally, you'll need to be logged into Docker Hub, and create an +# image in a namespace that you have access to. For example, if your username is +# "alice", you could build and push the image as follows: +# +# $ ./push.py -d Dockerfile -i alice/src-campaign-volume-workspace +# +# By default, only the "latest" tag will be built and pushed. The script refers +# to the HEAD ref given to it, either via $GITHUB_REF or the -r argument. If +# this is in the form refs/tags/X.Y.Z, then we'll also push X, X.Y, and X.Y.Z +# tags. +# +# Finally, if you have your environment configured to allow multi-architecture +# builds with docker buildx, you can provide a --platform argument that will be +# passed through verbatim to docker buildx build. (This is how we build ARM64 +# builds in our CI.) For example, you could build ARM64 and AMD64 images with: +# +# $ ./push.py --platform linux/arm64,linux/amd64 ... +# +# For this to work, you will need a builder with the relevant platforms enabled. +# More instructions on this can be found at +# https://docs.docker.com/buildx/working-with-buildx/#build-multi-platform-images. + +import argparse +import itertools +import os +import subprocess + +from typing import BinaryIO, Optional, Sequence + + +def calculate_tags(ref: str) -> Sequence[str]: + # The tags always include latest. + tags = ["latest"] + + # If the ref is a tag ref, then we should parse the version out and add each + # component to our tag list: for example, for X.Y.Z, we want tags X, X.Y, + # and X.Y.Z. + if ref.startswith("refs/tags/"): + tags.extend( + [ + # Join the version components back into a string. + ".".join(vc) + for vc in itertools.accumulate( + # Split the version string into its components. + ref.split("/", 2)[2].split("."), + # Accumulate each component we've seen into a new list + # entry. + lambda vlist, v: vlist + [v], + initial=[], + ) + # We also get the initial value, so we need to skip that. + if len(vc) > 0 + ] + ) + + return tags + + +def docker_build( + dockerfile: BinaryIO, platform: Optional[str], image: str, tags: Sequence[str] +): + args = ["docker", "buildx", "build", "--push"] + + for tag in tags: + args.extend(["-t", f"{image}:{tag}"]) + + if platform is not None: + args.extend(["--platform", platform]) + + # Since we provide the Dockerfile via stdin, we don't need to provide it + # here. (Doing so means that we don't carry an unncessary context into the + # builder.) + args.append("-") + + run(args, stdin=dockerfile) + + +def docker_login(username: str, password: str): + run( + ["docker", "login", f"-u={username}", "--password-stdin"], + input=password, + text=True, + ) + + +def run(args: Sequence[str], /, **kwargs) -> subprocess.CompletedProcess: + print(f"+ {' '.join(args)}") + return subprocess.run(args, check=True, **kwargs) + + +def main(): + parser = argparse.ArgumentParser() + parser.add_argument( + "-d", "--dockerfile", required=True, help="the Dockerfile to build" + ) + parser.add_argument("-i", "--image", required=True, help="Docker image to push") + parser.add_argument( + "-p", + "--platform", + help="platforms to build using docker buildx (if omitted, the default will be used)", + ) + parser.add_argument( + "-r", + "--ref", + default=os.environ.get("GITHUB_REF"), + help="current ref in refs/heads/... or refs/tags/... form", + ) + args = parser.parse_args() + + tags = calculate_tags(args.ref) + print(f"will push tags: {', '.join(tags)}") + + print("logging into Docker Hub") + try: + docker_login(os.environ["DOCKER_USERNAME"], os.environ["DOCKER_PASSWORD"]) + except KeyError as e: + print(f"error retrieving environment variables: {e}") + raise + + print("building and pushing image") + docker_build(open(args.dockerfile, "rb"), args.platform, args.image, tags) + + print("success!") + + +if __name__ == "__main__": + main() diff --git a/go.mod b/go.mod index ad8ec67038..ebe9209de1 100644 --- a/go.mod +++ b/go.mod @@ -6,6 +6,7 @@ require ( github.com/Masterminds/semver v1.5.0 github.com/dustin/go-humanize v1.0.0 github.com/efritz/pentimento v0.0.0-20190429011147-ade47d831101 + github.com/gobwas/glob v0.2.3 github.com/google/go-cmp v0.5.2 github.com/hashicorp/errwrap v1.1.0 // indirect github.com/hashicorp/go-multierror v1.1.0 diff --git a/internal/campaigns/archive_fetcher.go b/internal/campaigns/bind_workspace.go similarity index 61% rename from internal/campaigns/archive_fetcher.go rename to internal/campaigns/bind_workspace.go index 69d4a61fb9..9e978037bf 100644 --- a/internal/campaigns/archive_fetcher.go +++ b/internal/campaigns/bind_workspace.go @@ -6,56 +6,103 @@ import ( "fmt" "io" "io/ioutil" - "net/http" "os" - "path" "path/filepath" "strings" "github.com/hashicorp/go-multierror" "github.com/pkg/errors" - "github.com/sourcegraph/src-cli/internal/api" "github.com/sourcegraph/src-cli/internal/campaigns/graphql" ) -type WorkspaceCreator struct { - dir string - client api.Client - - deleteZips bool +type dockerBindWorkspaceCreator struct { + dir string } -func (wc *WorkspaceCreator) Create(ctx context.Context, repo *graphql.Repository) (string, error) { - path := localRepositoryZipArchivePath(wc.dir, repo) +var _ WorkspaceCreator = &dockerBindWorkspaceCreator{} - exists, err := fileExists(path) +func (wc *dockerBindWorkspaceCreator) Create(ctx context.Context, repo *graphql.Repository, zip string) (Workspace, error) { + w, err := wc.unzipToWorkspace(ctx, repo, zip) if err != nil { - return "", err + return nil, errors.Wrap(err, "unzipping the repository") } - if !exists { - err = fetchRepositoryArchive(ctx, wc.client, repo, path) - if err != nil { - // If the context got cancelled, or we ran out of disk space, or - // ... while we were downloading the file, we remove the partially - // downloaded file. - os.Remove(path) + return w, errors.Wrap(wc.prepareGitRepo(ctx, w), "preparing local git repo") +} - return "", errors.Wrap(err, "fetching ZIP archive") - } +func (*dockerBindWorkspaceCreator) DockerImages() []string { return []string{} } + +func (*dockerBindWorkspaceCreator) prepareGitRepo(ctx context.Context, w *dockerBindWorkspace) error { + if _, err := runGitCmd(ctx, w.dir, "init"); err != nil { + return errors.Wrap(err, "git init failed") } - if wc.deleteZips { - defer os.Remove(path) + // --force because we want previously "gitignored" files in the repository + if _, err := runGitCmd(ctx, w.dir, "add", "--force", "--all"); err != nil { + return errors.Wrap(err, "git add failed") + } + if _, err := runGitCmd(ctx, w.dir, "commit", "--quiet", "--all", "--allow-empty", "-m", "src-action-exec"); err != nil { + return errors.Wrap(err, "git commit failed") } + return nil +} + +func (wc *dockerBindWorkspaceCreator) unzipToWorkspace(ctx context.Context, repo *graphql.Repository, zip string) (*dockerBindWorkspace, error) { prefix := "workspace-" + repo.Slug() - workspace, err := unzipToTempDir(ctx, path, wc.dir, prefix) + workspace, err := unzipToTempDir(ctx, zip, wc.dir, prefix) + if err != nil { + return nil, errors.Wrap(err, "unzipping the ZIP archive") + } + + return &dockerBindWorkspace{dir: workspace}, nil +} + +type dockerBindWorkspace struct { + dir string +} + +var _ Workspace = &dockerBindWorkspace{} + +func (w *dockerBindWorkspace) Close(ctx context.Context) error { + return os.RemoveAll(w.dir) +} + +func (w *dockerBindWorkspace) DockerRunOpts(ctx context.Context, target string) ([]string, error) { + return []string{ + "--mount", + fmt.Sprintf("type=bind,source=%s,target=%s", w.dir, target), + }, nil +} + +func (w *dockerBindWorkspace) WorkDir() *string { return &w.dir } + +func (w *dockerBindWorkspace) Changes(ctx context.Context) (*StepChanges, error) { + if _, err := runGitCmd(ctx, w.dir, "add", "--all"); err != nil { + return nil, errors.Wrap(err, "git add failed") + } + + statusOut, err := runGitCmd(ctx, w.dir, "status", "--porcelain") + if err != nil { + return nil, errors.Wrap(err, "git status failed") + } + + changes, err := parseGitStatus(statusOut) if err != nil { - return "", errors.Wrap(err, "unzipping the ZIP archive") + return nil, errors.Wrap(err, "parsing git status output") } - return workspace, nil + return &changes, nil +} + +func (w *dockerBindWorkspace) Diff(ctx context.Context) ([]byte, error) { + // As of Sourcegraph 3.14 we only support unified diff format. + // That means we need to strip away the `a/` and `/b` prefixes with `--no-prefix`. + // See: https://github.com/sourcegraph/sourcegraph/blob/82d5e7e1562fef6be5c0b17f18631040fd330835/enterprise/internal/campaigns/service.go#L324-L329 + // + // Also, we need to add --binary so binary file changes are inlined in the patch. + // + return runGitCmd(ctx, w.dir, "diff", "--cached", "--no-prefix", "--binary") } func fileExists(path string) (bool, error) { @@ -82,47 +129,6 @@ func unzipToTempDir(ctx context.Context, zipFile, tempDir, tempFilePrefix string return volumeDir, unzip(zipFile, volumeDir) } -func fetchRepositoryArchive(ctx context.Context, client api.Client, repo *graphql.Repository, dest string) error { - req, err := client.NewHTTPRequest(ctx, "GET", repositoryZipArchivePath(repo), nil) - if err != nil { - return err - } - req.Header.Set("Accept", "application/zip") - resp, err := http.DefaultClient.Do(req) - if err != nil { - return err - } - defer resp.Body.Close() - - if resp.StatusCode != http.StatusOK { - return fmt.Errorf("unable to fetch archive (HTTP %d from %s)", resp.StatusCode, req.URL.String()) - } - - // Unlike the mkdirAll() calls elsewhere in this file, this is only giving - // us a temporary place on the filesystem to keep the archive. Since it's - // never mounted into the containers being run, we can keep these - // directories 0700 without issue. - if err := os.MkdirAll(filepath.Dir(dest), 0700); err != nil { - return err - } - - f, err := os.Create(dest) - if err != nil { - return err - } - defer f.Close() - - if _, err := io.Copy(f, resp.Body); err != nil { - return err - } - - return nil -} - -func repositoryZipArchivePath(repo *graphql.Repository) string { - return path.Join("", repo.Name+"@"+repo.BaseRef(), "-", "raw") -} - func localRepositoryZipArchivePath(dir string, repo *graphql.Repository) string { return filepath.Join(dir, fmt.Sprintf("%s-%s.zip", repo.Slug(), repo.Rev())) } diff --git a/internal/campaigns/archive_fetcher_nonwin_test.go b/internal/campaigns/bind_workspace_nonwin_test.go similarity index 100% rename from internal/campaigns/archive_fetcher_nonwin_test.go rename to internal/campaigns/bind_workspace_nonwin_test.go diff --git a/internal/campaigns/archive_fetcher_test.go b/internal/campaigns/bind_workspace_test.go similarity index 54% rename from internal/campaigns/archive_fetcher_test.go rename to internal/campaigns/bind_workspace_test.go index 9e536173b7..2ee90e7a38 100644 --- a/internal/campaigns/archive_fetcher_test.go +++ b/internal/campaigns/bind_workspace_test.go @@ -1,22 +1,19 @@ package campaigns import ( - "bytes" + "archive/zip" "context" "io/ioutil" - "net/http" - "net/http/httptest" "os" "path/filepath" + "strings" "testing" - "time" "github.com/google/go-cmp/cmp" - "github.com/sourcegraph/src-cli/internal/api" "github.com/sourcegraph/src-cli/internal/campaigns/graphql" ) -func TestWorkspaceCreator_Create(t *testing.T) { +func TestDockerBindWorkspaceCreator_Create(t *testing.T) { workspaceTmpDir := func(t *testing.T) string { testTempDir, err := ioutil.TempDir("", "executor-integration-test-*") if err != nil { @@ -40,35 +37,36 @@ func TestWorkspaceCreator_Create(t *testing.T) { }, } - t.Run("success", func(t *testing.T) { - requestsReceived := 0 - callback := func(_ http.ResponseWriter, _ *http.Request) { - requestsReceived += 1 - } - - ts := httptest.NewServer(newZipArchivesMux(t, callback, archive)) - defer ts.Close() + // Create a zip file for all the other tests to use. + f, err := ioutil.TempFile(workspaceTmpDir(t), "repo-zip-*") + if err != nil { + t.Fatal(err) + } + archivePath := f.Name() + t.Cleanup(func() { os.Remove(archivePath) }) - var clientBuffer bytes.Buffer - client := api.NewClient(api.ClientOpts{Endpoint: ts.URL, Out: &clientBuffer}) + zw := zip.NewWriter(f) + for name, body := range archive.files { + f, err := zw.Create(name) + if err != nil { + t.Fatal(err) + } + if _, err := f.Write([]byte(body)); err != nil { + t.Fatal(err) + } + } + zw.Close() + f.Close() + t.Run("success", func(t *testing.T) { testTempDir := workspaceTmpDir(t) - creator := &WorkspaceCreator{dir: testTempDir, client: client} - workspace, err := creator.Create(context.Background(), repo) + creator := &dockerBindWorkspaceCreator{dir: testTempDir} + workspace, err := creator.Create(context.Background(), repo, archivePath) if err != nil { t.Fatalf("unexpected error: %s", err) } - wantZipFile := "github.com-sourcegraph-src-cli-d34db33f.zip" - ok, err := dirContains(creator.dir, wantZipFile) - if err != nil { - t.Fatal(err) - } - if !ok { - t.Fatalf("temp dir doesnt contain zip file") - } - haveUnzippedFiles, err := readWorkspaceFiles(workspace) if err != nil { t.Fatalf("error walking workspace: %s", err) @@ -77,121 +75,23 @@ func TestWorkspaceCreator_Create(t *testing.T) { if !cmp.Equal(archive.files, haveUnzippedFiles) { t.Fatalf("wrong files in workspace:\n%s", cmp.Diff(archive.files, haveUnzippedFiles)) } - - // Create it a second time and make sure that the server wasn't called - _, err = creator.Create(context.Background(), repo) - if err != nil { - t.Fatalf("unexpected error: %s", err) - } - - if requestsReceived != 1 { - t.Fatalf("wrong number of requests received: %d", requestsReceived) - } - - // Third time, but this time with cleanup, _after_ unzipping - creator.deleteZips = true - _, err = creator.Create(context.Background(), repo) - if err != nil { - t.Fatalf("unexpected error: %s", err) - } - - if requestsReceived != 1 { - t.Fatalf("wrong number of requests received: %d", requestsReceived) - } - - ok, err = dirContains(creator.dir, wantZipFile) - if err != nil { - t.Fatal(err) - } - if ok { - t.Fatalf("temp dir contains zip file but should not") - } }) - t.Run("canceled", func(t *testing.T) { - // We create a context that is canceled after the server sent down the - // first file to simulate a slow download that's aborted by the user hitting Ctrl-C. - - firstFileWritten := make(chan struct{}) - callback := func(w http.ResponseWriter, r *http.Request) { - // We flush the headers and the first file - w.(http.Flusher).Flush() - - // Wait a bit for the client to start writing the file - time.Sleep(50 * time.Millisecond) - - // Cancel the context to simulate the Ctrl-C - firstFileWritten <- struct{}{} - - <-r.Context().Done() - } - - ctx, cancel := context.WithCancel(context.Background()) - go func() { - <-firstFileWritten - cancel() - }() - - ts := httptest.NewServer(newZipArchivesMux(t, callback, archive)) - defer ts.Close() - - var clientBuffer bytes.Buffer - client := api.NewClient(api.ClientOpts{Endpoint: ts.URL, Out: &clientBuffer}) - + t.Run("failure", func(t *testing.T) { testTempDir := workspaceTmpDir(t) - creator := &WorkspaceCreator{dir: testTempDir, client: client} - - _, err := creator.Create(ctx, repo) - if err == nil { - t.Fatalf("error is nil") - } - - zipFile := "github.com-sourcegraph-src-cli-d34db33f.zip" - ok, err := dirContains(creator.dir, zipFile) + // Create an empty file (which is therefore a bad zip file). + badZip, err := ioutil.TempFile(testTempDir, "bad-zip-*") if err != nil { t.Fatal(err) } - if ok { - t.Fatalf("zip file in temp dir was not cleaned up") - } - }) - - t.Run("non-default branch", func(t *testing.T) { - otherBranchOID := "f00b4r" - repo := &graphql.Repository{ - ID: "src-cli-with-non-main-branch", - Name: "github.com/sourcegraph/src-cli", - DefaultBranch: &graphql.Branch{Name: "main", Target: graphql.Target{OID: "d34db33f"}}, - - Commit: graphql.Target{OID: otherBranchOID}, - Branch: graphql.Branch{Name: "other-branch", Target: graphql.Target{OID: otherBranchOID}}, - } + badZipFile := badZip.Name() + t.Cleanup(func() { os.Remove(badZipFile) }) + badZip.Close() - archive := mockRepoArchive{repo: repo, files: map[string]string{}} - - ts := httptest.NewServer(newZipArchivesMux(t, nil, archive)) - defer ts.Close() - - var clientBuffer bytes.Buffer - client := api.NewClient(api.ClientOpts{Endpoint: ts.URL, Out: &clientBuffer}) - - testTempDir := workspaceTmpDir(t) - - creator := &WorkspaceCreator{dir: testTempDir, client: client} - - _, err := creator.Create(context.Background(), repo) - if err != nil { - t.Fatalf("unexpected error: %s", err) - } - - wantZipFile := "github.com-sourcegraph-src-cli-" + otherBranchOID + ".zip" - ok, err := dirContains(creator.dir, wantZipFile) - if err != nil { - t.Fatal(err) - } - if !ok { - t.Fatalf("temp dir doesnt contain zip file") + creator := &dockerBindWorkspaceCreator{dir: testTempDir} + if _, err := creator.Create(context.Background(), repo, badZipFile); err == nil { + t.Error("unexpected nil error") } }) } @@ -329,10 +229,10 @@ func isDir(t *testing.T, path string) bool { return st.IsDir() } -func readWorkspaceFiles(workspace string) (map[string]string, error) { +func readWorkspaceFiles(workspace Workspace) (map[string]string, error) { files := map[string]string{} - - err := filepath.Walk(workspace, func(path string, info os.FileInfo, err error) error { + wdir := workspace.WorkDir() + err := filepath.Walk(*wdir, func(path string, info os.FileInfo, err error) error { if err != nil { return err } @@ -345,11 +245,15 @@ func readWorkspaceFiles(workspace string) (map[string]string, error) { return err } - rel, err := filepath.Rel(workspace, path) + rel, err := filepath.Rel(*wdir, path) if err != nil { return err } + if strings.HasPrefix(rel, ".git") { + return nil + } + files[rel] = string(content) return nil }) diff --git a/internal/campaigns/archive_fetcher_windows_test.go b/internal/campaigns/bind_workspace_windows_test.go similarity index 100% rename from internal/campaigns/archive_fetcher_windows_test.go rename to internal/campaigns/bind_workspace_windows_test.go diff --git a/internal/campaigns/executor.go b/internal/campaigns/executor.go index a9824f956a..c41dec49a0 100644 --- a/internal/campaigns/executor.go +++ b/internal/campaigns/executor.go @@ -143,7 +143,8 @@ type executor struct { client api.Client features featureFlags logger *LogManager - creator *WorkspaceCreator + creator WorkspaceCreator + fetcher RepoFetcher tasks []*Task statuses map[*Task]*TaskStatus @@ -165,6 +166,7 @@ func newExecutor(opts ExecutorOpts, client api.Client, features featureFlags) *e creator: opts.Creator, client: client, features: features, + fetcher: opts.RepoFetcher, doneEnqueuing: make(chan struct{}), logger: NewLogManager(opts.TempDir, opts.KeepLogs), tempDir: opts.TempDir, @@ -315,7 +317,7 @@ func (x *executor) do(ctx context.Context, task *Task) (err error) { defer cancel() // Actually execute the steps. - diff, err := runSteps(runCtx, x.creator, task.Repository, task.Steps, log, x.tempDir, func(currentlyExecuting string) { + diff, err := runSteps(runCtx, x.fetcher, x.creator, task.Repository, task.Steps, log, x.tempDir, func(currentlyExecuting string) { x.updateTaskStatus(task, func(status *TaskStatus) { status.CurrentlyExecuting = currentlyExecuting }) diff --git a/internal/campaigns/executor_test.go b/internal/campaigns/executor_test.go index afe2617eea..e280cebfaf 100644 --- a/internal/campaigns/executor_test.go +++ b/internal/campaigns/executor_test.go @@ -213,10 +213,14 @@ func TestExecutor_Integration(t *testing.T) { defer os.Remove(testTempDir) cache := newInMemoryExecutionCache() - creator := &WorkspaceCreator{dir: testTempDir, client: client} + creator := &dockerBindWorkspaceCreator{dir: testTempDir} opts := ExecutorOpts{ - Cache: cache, - Creator: creator, + Cache: cache, + Creator: creator, + RepoFetcher: &repoFetcher{ + client: client, + dir: testTempDir, + }, TempDir: testTempDir, Parallelism: runtime.GOMAXPROCS(0), Timeout: tc.executorTimeout, diff --git a/internal/campaigns/git.go b/internal/campaigns/git.go new file mode 100644 index 0000000000..5356e2cfcb --- /dev/null +++ b/internal/campaigns/git.go @@ -0,0 +1,33 @@ +package campaigns + +import ( + "context" + "os/exec" + "strings" + + "github.com/pkg/errors" +) + +func runGitCmd(ctx context.Context, dir string, args ...string) ([]byte, error) { + cmd := exec.CommandContext(ctx, "git", args...) + cmd.Env = []string{ + // Don't use the system wide git config. + "GIT_CONFIG_NOSYSTEM=1", + // And also not any other, because they can mess up output, change defaults, .. which can do unexpected things. + "GIT_CONFIG=/dev/null", + // Set user.name and user.email in the local repository. The user name and + // e-mail will eventually be ignored anyway, since we're just using the Git + // repository to generate diffs, but we don't want git to generate alarming + // looking warnings. + "GIT_AUTHOR_NAME=Sourcegraph", + "GIT_AUTHOR_EMAIL=campaigns@sourcegraph.com", + "GIT_COMMITTER_NAME=Sourcegraph", + "GIT_COMMITTER_EMAIL=campaigns@sourcegraph.com", + } + cmd.Dir = dir + out, err := cmd.CombinedOutput() + if err != nil { + return nil, errors.Wrapf(err, "'git %s' failed: %s", strings.Join(args, " "), out) + } + return out, nil +} diff --git a/internal/campaigns/main_test.go b/internal/campaigns/main_test.go new file mode 100644 index 0000000000..103b728a97 --- /dev/null +++ b/internal/campaigns/main_test.go @@ -0,0 +1,13 @@ +package campaigns + +import ( + "os" + "testing" + + "github.com/sourcegraph/src-cli/internal/exec/expect" +) + +func TestMain(m *testing.M) { + code := expect.Handle(m) + os.Exit(code) +} diff --git a/internal/campaigns/repo_fetcher.go b/internal/campaigns/repo_fetcher.go new file mode 100644 index 0000000000..0427517e05 --- /dev/null +++ b/internal/campaigns/repo_fetcher.go @@ -0,0 +1,131 @@ +package campaigns + +import ( + "context" + "fmt" + "io" + "net/http" + "os" + "path" + "path/filepath" + + "github.com/pkg/errors" + "github.com/sourcegraph/src-cli/internal/api" + "github.com/sourcegraph/src-cli/internal/campaigns/graphql" +) + +// RepoFetcher abstracts the process of retrieving an archive for the given +// repository. +type RepoFetcher interface { + // Fetch must retrieve the given repository and return it as a RepoZip. + // This will generally imply that the file should be written to a temporary + // location on the filesystem. + Fetch(context.Context, *graphql.Repository) (RepoZip, error) +} + +// repoFetcher is the concrete implementation of the RepoFetcher interface used +// outside of tests. +type repoFetcher struct { + client api.Client + dir string + deleteZips bool +} + +var _ RepoFetcher = &repoFetcher{} + +// RepoZip implementations represent a downloaded repository archive. +type RepoZip interface { + // Close must finalise the downloaded archive. If one or more temporary + // files were created, they should be deleted here. + Close() error + + // Path must return the path to the archive on the filesystem. + Path() string +} + +// repoZip is the concrete implementation of the RepoZip interface used outside +// of tests. +type repoZip struct { + path string + fetcher *repoFetcher +} + +var _ RepoZip = &repoZip{} + +func (rf *repoFetcher) Fetch(ctx context.Context, repo *graphql.Repository) (RepoZip, error) { + path := localRepositoryZipArchivePath(rf.dir, repo) + + exists, err := fileExists(path) + if err != nil { + return nil, err + } + + if !exists { + // Unlike the mkdirAll() calls elsewhere in this file, this is only + // giving us a temporary place on the filesystem to keep the archive. + // Since it's never mounted into the containers being run, we can keep + // these directories 0700 without issue. + if err := os.MkdirAll(filepath.Dir(path), 0700); err != nil { + return nil, err + } + + err = fetchRepositoryArchive(ctx, rf.client, repo, path) + if err != nil { + // If the context got cancelled, or we ran out of disk space, or ... + // while we were downloading the file, we remove the partially + // downloaded file. + os.Remove(path) + + return nil, errors.Wrap(err, "fetching ZIP archive") + } + } + + return &repoZip{ + path: path, + fetcher: rf, + }, nil +} + +func (rz *repoZip) Close() error { + if rz.fetcher.deleteZips { + return os.Remove(rz.path) + } + return nil +} + +func (rz *repoZip) Path() string { + return rz.path +} + +func fetchRepositoryArchive(ctx context.Context, client api.Client, repo *graphql.Repository, dest string) error { + req, err := client.NewHTTPRequest(ctx, "GET", repositoryZipArchivePath(repo), nil) + if err != nil { + return err + } + req.Header.Set("Accept", "application/zip") + resp, err := http.DefaultClient.Do(req) + if err != nil { + return err + } + defer resp.Body.Close() + + if resp.StatusCode != http.StatusOK { + return fmt.Errorf("unable to fetch archive (HTTP %d from %s)", resp.StatusCode, req.URL.String()) + } + + f, err := os.Create(dest) + if err != nil { + return err + } + defer f.Close() + + if _, err := io.Copy(f, resp.Body); err != nil { + return err + } + + return nil +} + +func repositoryZipArchivePath(repo *graphql.Repository) string { + return path.Join("", repo.Name+"@"+repo.BaseRef(), "-", "raw") +} diff --git a/internal/campaigns/repo_fetcher_test.go b/internal/campaigns/repo_fetcher_test.go new file mode 100644 index 0000000000..587b38913a --- /dev/null +++ b/internal/campaigns/repo_fetcher_test.go @@ -0,0 +1,201 @@ +package campaigns + +import ( + "bytes" + "context" + "io/ioutil" + "net/http" + "net/http/httptest" + "os" + "path" + "path/filepath" + "testing" + "time" + + "github.com/sourcegraph/src-cli/internal/api" + "github.com/sourcegraph/src-cli/internal/campaigns/graphql" +) + +func TestRepoFetcher_Fetch(t *testing.T) { + workspaceTmpDir := func(t *testing.T) string { + testTempDir, err := ioutil.TempDir("", "executor-integration-test-*") + if err != nil { + t.Fatal(err) + } + t.Cleanup(func() { os.Remove(testTempDir) }) + + return testTempDir + } + + repo := &graphql.Repository{ + ID: "src-cli", + Name: "github.com/sourcegraph/src-cli", + DefaultBranch: &graphql.Branch{Name: "main", Target: graphql.Target{OID: "d34db33f"}}, + } + + archive := mockRepoArchive{ + repo: repo, + files: map[string]string{ + "README.md": "# Welcome to the README\n", + }, + } + + t.Run("success", func(t *testing.T) { + requestsReceived := 0 + callback := func(_ http.ResponseWriter, _ *http.Request) { + requestsReceived++ + } + + ts := httptest.NewServer(newZipArchivesMux(t, callback, archive)) + defer ts.Close() + + var clientBuffer bytes.Buffer + client := api.NewClient(api.ClientOpts{Endpoint: ts.URL, Out: &clientBuffer}) + + rf := &repoFetcher{ + client: client, + dir: workspaceTmpDir(t), + deleteZips: false, + } + + rz, err := rf.Fetch(context.Background(), repo) + if err != nil { + t.Errorf("unexpected error: %s", err) + } + + wantZipFile := "github.com-sourcegraph-src-cli-d34db33f.zip" + ok, err := dirContains(rf.dir, wantZipFile) + if err != nil { + t.Fatal(err) + } + if !ok { + t.Fatalf("temp dir doesnt contain zip file") + } + + if have, want := rz.Path(), filepath.Join(path.Clean(rf.dir), wantZipFile); want != have { + t.Errorf("unexpected path: have=%q want=%q", have, want) + } + rz.Close() + + // Create it a second time and make sure that the server wasn't called + rz, err = rf.Fetch(context.Background(), repo) + if err != nil { + t.Fatalf("unexpected error: %s", err) + } + rz.Close() + + if requestsReceived != 1 { + t.Fatalf("wrong number of requests received: %d", requestsReceived) + } + + // Third time, but this time with cleanup, _after_ unzipping + rf.deleteZips = true + _, err = rf.Fetch(context.Background(), repo) + if err != nil { + t.Fatalf("unexpected error: %s", err) + } + + if requestsReceived != 1 { + t.Fatalf("wrong number of requests received: %d", requestsReceived) + } + rz.Close() + + ok, err = dirContains(rf.dir, wantZipFile) + if err != nil { + t.Fatal(err) + } + if ok { + t.Fatalf("temp dir contains zip file but should not") + } + }) + + t.Run("cancelled", func(t *testing.T) { + // We create a context that is canceled after the server sent down the + // first file to simulate a slow download that's aborted by the user hitting Ctrl-C. + + firstFileWritten := make(chan struct{}) + callback := func(w http.ResponseWriter, r *http.Request) { + // We flush the headers and the first file + w.(http.Flusher).Flush() + + // Wait a bit for the client to start writing the file + time.Sleep(50 * time.Millisecond) + + // Cancel the context to simulate the Ctrl-C + firstFileWritten <- struct{}{} + + <-r.Context().Done() + } + + ctx, cancel := context.WithCancel(context.Background()) + go func() { + <-firstFileWritten + cancel() + }() + + ts := httptest.NewServer(newZipArchivesMux(t, callback, archive)) + defer ts.Close() + + var clientBuffer bytes.Buffer + client := api.NewClient(api.ClientOpts{Endpoint: ts.URL, Out: &clientBuffer}) + + rf := &repoFetcher{ + client: client, + dir: workspaceTmpDir(t), + deleteZips: false, + } + + if _, err := rf.Fetch(ctx, repo); err == nil { + t.Error("error is nil") + } + + zipFile := "github.com-sourcegraph-src-cli-d34db33f.zip" + ok, err := dirContains(rf.dir, zipFile) + if err != nil { + t.Error(err) + } + if ok { + t.Errorf("zip file in temp dir was not cleaned up") + } + }) + + t.Run("non-default branch", func(t *testing.T) { + otherBranchOID := "f00b4r" + repo := &graphql.Repository{ + ID: "src-cli-with-non-main-branch", + Name: "github.com/sourcegraph/src-cli", + DefaultBranch: &graphql.Branch{Name: "main", Target: graphql.Target{OID: "d34db33f"}}, + + Commit: graphql.Target{OID: otherBranchOID}, + Branch: graphql.Branch{Name: "other-branch", Target: graphql.Target{OID: otherBranchOID}}, + } + + archive := mockRepoArchive{repo: repo, files: map[string]string{}} + + ts := httptest.NewServer(newZipArchivesMux(t, nil, archive)) + defer ts.Close() + + var clientBuffer bytes.Buffer + client := api.NewClient(api.ClientOpts{Endpoint: ts.URL, Out: &clientBuffer}) + + rf := &repoFetcher{ + client: client, + dir: workspaceTmpDir(t), + deleteZips: false, + } + + _, err := rf.Fetch(context.Background(), repo) + if err != nil { + t.Fatalf("unexpected error: %s", err) + } + + wantZipFile := "github.com-sourcegraph-src-cli-" + otherBranchOID + ".zip" + ok, err := dirContains(rf.dir, wantZipFile) + if err != nil { + t.Fatal(err) + } + if !ok { + t.Fatalf("temp dir doesnt contain zip file") + } + }) +} diff --git a/internal/campaigns/run_steps.go b/internal/campaigns/run_steps.go index d400961675..4e18e84b09 100644 --- a/internal/campaigns/run_steps.go +++ b/internal/campaigns/run_steps.go @@ -17,51 +17,20 @@ import ( "github.com/sourcegraph/src-cli/internal/campaigns/graphql" ) -func runSteps(ctx context.Context, wc *WorkspaceCreator, repo *graphql.Repository, steps []Step, logger *TaskLogger, tempDir string, reportProgress func(string)) ([]byte, error) { +func runSteps(ctx context.Context, rf RepoFetcher, wc WorkspaceCreator, repo *graphql.Repository, steps []Step, logger *TaskLogger, tempDir string, reportProgress func(string)) ([]byte, error) { reportProgress("Downloading archive") - - volumeDir, err := wc.Create(ctx, repo) + zip, err := rf.Fetch(ctx, repo) if err != nil { - return nil, errors.Wrap(err, "creating workspace") - } - defer os.RemoveAll(volumeDir) - - runGitCmd := func(args ...string) ([]byte, error) { - cmd := exec.CommandContext(ctx, "git", args...) - cmd.Env = []string{ - // Don't use the system wide git config. - "GIT_CONFIG_NOSYSTEM=1", - // And also not any other, because they can mess up output, change defaults, .. which can do unexpected things. - "GIT_CONFIG=/dev/null", - // Set user.name and user.email in the local repository. The user name and - // e-mail will eventually be ignored anyway, since we're just using the Git - // repository to generate diffs, but we don't want git to generate alarming - // looking warnings. - "GIT_AUTHOR_NAME=Sourcegraph", - "GIT_AUTHOR_EMAIL=campaigns@sourcegraph.com", - "GIT_COMMITTER_NAME=Sourcegraph", - "GIT_COMMITTER_EMAIL=campaigns@sourcegraph.com", - } - cmd.Dir = volumeDir - out, err := cmd.CombinedOutput() - if err != nil { - return nil, errors.Wrapf(err, "'git %s' failed: %s", strings.Join(args, " "), out) - } - return out, nil + return nil, errors.Wrap(err, "fetching repo") } + defer zip.Close() reportProgress("Initializing workspace") - if _, err := runGitCmd("init"); err != nil { - return nil, errors.Wrap(err, "git init failed") - } - - // --force because we want previously "gitignored" files in the repository - if _, err := runGitCmd("add", "--force", "--all"); err != nil { - return nil, errors.Wrap(err, "git add failed") - } - if _, err := runGitCmd("commit", "--quiet", "--all", "-m", "src-action-exec"); err != nil { - return nil, errors.Wrap(err, "git commit failed") + workspace, err := wc.Create(ctx, repo, zip.Path()) + if err != nil { + return nil, errors.Wrap(err, "creating workspace") } + defer workspace.Close(ctx) results := make([]StepResult, len(steps)) @@ -184,15 +153,18 @@ func runSteps(ctx context.Context, wc *WorkspaceCreator, repo *graphql.Repositor reportProgress(runScript.String()) const workDir = "/work" - args := []string{ + workspaceOpts, err := workspace.DockerRunOpts(ctx, workDir) + if err != nil { + return nil, errors.Wrap(err, "getting Docker options for workspace") + } + args := append([]string{ "run", "--rm", "--init", "--cidfile", cidFile.Name(), "--workdir", workDir, - "--mount", fmt.Sprintf("type=bind,source=%s,target=%s", volumeDir, workDir), "--mount", fmt.Sprintf("type=bind,source=%s,target=%s,ro", runScriptFile.Name(), containerTemp), - } + }, workspaceOpts...) for target, source := range filesToMount { args = append(args, "--mount", fmt.Sprintf("type=bind,source=%s,target=%s,ro", source.Name(), target)) } @@ -205,7 +177,9 @@ func runSteps(ctx context.Context, wc *WorkspaceCreator, repo *graphql.Repositor cmd := exec.CommandContext(ctx, "docker", args...) cmd.Args = append(cmd.Args, "--", step.image, containerTemp) - cmd.Dir = volumeDir + if dir := workspace.WorkDir(); dir != nil { + cmd.Dir = *dir + } var stdoutBuffer, stderrBuffer bytes.Buffer cmd.Stdout = io.MultiWriter(&stdoutBuffer, logger.PrefixWriter("stdout")) @@ -233,31 +207,16 @@ func runSteps(ctx context.Context, wc *WorkspaceCreator, repo *graphql.Repositor logger.Logf("[Step %d] complete in %s", i+1, elapsed) - if _, err := runGitCmd("add", "--all"); err != nil { - return nil, errors.Wrap(err, "git add failed") - } - - statusOut, err := runGitCmd("status", "--porcelain") - if err != nil { - return nil, errors.Wrap(err, "git status failed") - } - - changes, err := parseGitStatus(statusOut) + changes, err := workspace.Changes(ctx) if err != nil { - return nil, errors.Wrap(err, "parsing git status output") + return nil, errors.Wrap(err, "getting changed files in step") } - results[i] = StepResult{Files: changes, Stdout: &stdoutBuffer, Stderr: &stderrBuffer} + results[i] = StepResult{files: changes, Stdout: &stdoutBuffer, Stderr: &stderrBuffer} } reportProgress("Calculating diff") - // As of Sourcegraph 3.14 we only support unified diff format. - // That means we need to strip away the `a/` and `/b` prefixes with `--no-prefix`. - // See: https://github.com/sourcegraph/sourcegraph/blob/82d5e7e1562fef6be5c0b17f18631040fd330835/enterprise/internal/campaigns/service.go#L324-L329 - // - // Also, we need to add --binary so binary file changes are inlined in the patch. - // - diffOut, err := runGitCmd("diff", "--cached", "--no-prefix", "--binary") + diffOut, err := workspace.Diff(ctx) if err != nil { return nil, errors.Wrap(err, "git diff failed") } @@ -477,8 +436,8 @@ func (stepCtx *StepContext) ToFuncMap() template.FuncMap { // StepResult represents the result of a previously executed step. type StepResult struct { - // Files are the changes made to files by the step. - Files StepChanges + // files are the changes made to files by the step. + files *StepChanges // Stdout is the output produced by the step on standard out. Stdout *bytes.Buffer @@ -495,16 +454,36 @@ type StepChanges struct { } // ModifiedFiles returns the files modified by a step. -func (r StepResult) ModifiedFiles() []string { return r.Files.Modified } +func (r StepResult) ModifiedFiles() []string { + if r.files != nil { + return r.files.Modified + } + return []string{} +} // AddedFiles returns the files added by a step. -func (r StepResult) AddedFiles() []string { return r.Files.Added } +func (r StepResult) AddedFiles() []string { + if r.files != nil { + return r.files.Added + } + return []string{} +} // DeletedFiles returns the files deleted by a step. -func (r StepResult) DeletedFiles() []string { return r.Files.Deleted } +func (r StepResult) DeletedFiles() []string { + if r.files != nil { + return r.files.Deleted + } + return []string{} +} // RenamedFiles returns the new name of files that have been renamed by a step. -func (r StepResult) RenamedFiles() []string { return r.Files.Renamed } +func (r StepResult) RenamedFiles() []string { + if r.files != nil { + return r.files.Renamed + } + return []string{} +} func parseGitStatus(out []byte) (StepChanges, error) { result := StepChanges{} diff --git a/internal/campaigns/run_steps_test.go b/internal/campaigns/run_steps_test.go index 8d4db8415a..5beeb28d30 100644 --- a/internal/campaigns/run_steps_test.go +++ b/internal/campaigns/run_steps_test.go @@ -36,7 +36,7 @@ R README.md -> README.markdown func TestParsingAndRenderingTemplates(t *testing.T) { stepCtx := &StepContext{ PreviousStep: StepResult{ - Files: StepChanges{ + files: &StepChanges{ Modified: []string{"go.mod"}, Added: []string{"main.go.swp"}, Deleted: []string{".DS_Store"}, diff --git a/internal/campaigns/service.go b/internal/campaigns/service.go index d25dc9ec6a..0514f59f99 100644 --- a/internal/campaigns/service.go +++ b/internal/campaigns/service.go @@ -22,11 +22,13 @@ type Service struct { allowUnsupported bool client api.Client features featureFlags + workspace string } type ServiceOpts struct { AllowUnsupported bool Client api.Client + Workspace string } var ( @@ -37,6 +39,7 @@ func NewService(opts *ServiceOpts) *Service { return &Service{ allowUnsupported: opts.AllowUnsupported, client: opts.Client, + workspace: opts.Workspace, } } @@ -185,8 +188,9 @@ func (svc *Service) NewExecutionCache(dir string) ExecutionCache { type ExecutorOpts struct { Cache ExecutionCache - Creator *WorkspaceCreator + Creator WorkspaceCreator Parallelism int + RepoFetcher RepoFetcher Timeout time.Duration ClearCache bool @@ -199,19 +203,76 @@ func (svc *Service) NewExecutor(opts ExecutorOpts) Executor { return newExecutor(opts, svc.client, svc.features) } -func (svc *Service) NewWorkspaceCreator(dir string, cleanArchives bool) *WorkspaceCreator { - return &WorkspaceCreator{dir: dir, client: svc.client, deleteZips: cleanArchives} +func (svc *Service) NewRepoFetcher(dir string, cleanArchives bool) RepoFetcher { + return &repoFetcher{ + client: svc.client, + dir: dir, + deleteZips: cleanArchives, + } +} + +func (svc *Service) NewWorkspaceCreator(dir string) WorkspaceCreator { + if svc.workspace == "volume" { + return &dockerVolumeWorkspaceCreator{} + } + return &dockerBindWorkspaceCreator{dir: dir} +} + +// dockerImageSet represents a set of Docker images that need to be pulled. The +// keys are the Docker image names; the values are a slice of pointers to +// strings that should be set to the content digest of the image. +type dockerImageSet map[string][]*string + +func (dis dockerImageSet) add(image string, digestPtr *string) { + if digestPtr == nil { + // Since we don't have a digest pointer here, we just need to ensure + // that the image exists at all in the map. + if _, ok := dis[image]; !ok { + dis[image] = []*string{} + } + } else { + // Either append the digest pointer to an existing map entry, or add a + // new entry if required. + if digests, ok := dis[image]; ok { + dis[image] = append(digests, digestPtr) + } else { + dis[image] = []*string{digestPtr} + } + } } -func (svc *Service) SetDockerImages(ctx context.Context, spec *CampaignSpec, progress func(i int)) error { +// SetDockerImages updates the steps within the campaign spec to include the +// exact content digest to be used when running each step, and ensures that all +// Docker images are available, including any required by the workspace creator. +// +// Progress information is reported back to the given progress function: perc +// will be a value between 0.0 and 1.0, inclusive. +func (svc *Service) SetDockerImages(ctx context.Context, creator WorkspaceCreator, spec *CampaignSpec, progress func(perc float64)) error { + images := dockerImageSet{} for i, step := range spec.Steps { - image, err := getDockerImageContentDigest(ctx, step.Container) + images.add(step.Container, &spec.Steps[i].image) + } + + // The workspace creator may have its own dependencies. + for _, image := range creator.DockerImages() { + images.add(image, nil) + } + + progress(0) + i := 0 + for image, digests := range images { + digest, err := getDockerImageContentDigest(ctx, image) if err != nil { - return err + return errors.Wrapf(err, "getting content digest for image %q", image) } - spec.Steps[i].image = image - progress(i + 1) + for _, digestPtr := range digests { + *digestPtr = digest + } + + progress(float64(i) / float64(len(images))) + i++ } + progress(1) return nil } diff --git a/internal/campaigns/volume_workspace.go b/internal/campaigns/volume_workspace.go new file mode 100644 index 0000000000..70ef6fa518 --- /dev/null +++ b/internal/campaigns/volume_workspace.go @@ -0,0 +1,194 @@ +package campaigns + +import ( + "bytes" + "context" + "io/ioutil" + "os" + + "github.com/pkg/errors" + + "github.com/sourcegraph/src-cli/internal/campaigns/graphql" + "github.com/sourcegraph/src-cli/internal/exec" +) + +// dockerVolumeWorkspaceCreator creates dockerVolumeWorkspace instances. +type dockerVolumeWorkspaceCreator struct{} + +var _ WorkspaceCreator = &dockerVolumeWorkspaceCreator{} + +func (wc *dockerVolumeWorkspaceCreator) Create(ctx context.Context, repo *graphql.Repository, zip string) (Workspace, error) { + volume, err := wc.createVolume(ctx) + if err != nil { + return nil, errors.Wrap(err, "creating Docker volume") + } + + w := &dockerVolumeWorkspace{volume: volume} + if err := wc.unzipRepoIntoVolume(ctx, w, zip); err != nil { + return nil, errors.Wrap(err, "unzipping repo into workspace") + } + + return w, errors.Wrap(wc.prepareGitRepo(ctx, w), "preparing local git repo") +} + +func (*dockerVolumeWorkspaceCreator) DockerImages() []string { + return []string{dockerWorkspaceImage} +} + +func (*dockerVolumeWorkspaceCreator) createVolume(ctx context.Context) (string, error) { + out, err := exec.CommandContext(ctx, "docker", "volume", "create").CombinedOutput() + if err != nil { + return "", err + } + + return string(bytes.TrimSpace(out)), nil +} + +func (*dockerVolumeWorkspaceCreator) prepareGitRepo(ctx context.Context, w *dockerVolumeWorkspace) error { + script := `#!/bin/sh + +set -e +set -x + +git init +# --force because we want previously "gitignored" files in the repository +git add --force --all +git commit --quiet --all --allow-empty -m src-action-exec +` + + if _, err := w.runScript(ctx, "/work", script); err != nil { + return errors.Wrap(err, "preparing workspace") + } + return nil +} + +func (*dockerVolumeWorkspaceCreator) unzipRepoIntoVolume(ctx context.Context, w *dockerVolumeWorkspace, zip string) error { + // We want to mount that temporary file into a Docker container that has the + // workspace volume attached, and unzip it into the volume. + common, err := w.DockerRunOpts(ctx, "/work") + if err != nil { + return errors.Wrap(err, "generating run options") + } + + opts := append([]string{ + "run", + "--rm", + "--init", + "--workdir", "/work", + "--mount", "type=bind,source=" + zip + ",target=/tmp/zip,ro", + }, common...) + opts = append(opts, dockerWorkspaceImage, "unzip", "/tmp/zip") + + if out, err := exec.CommandContext(ctx, "docker", opts...).CombinedOutput(); err != nil { + return errors.Wrapf(err, "unzip output:\n\n%s\n\n", string(out)) + } + + return nil +} + +// dockerVolumeWorkspace workspaces are placed on Docker volumes (surprise!), +// and are therefore transparent to the host filesystem. This has performance +// advantages if bind mounts are slow, such as on Docker for Mac, but could make +// debugging harder and is slower when it's time to actually retrieve the diff. +type dockerVolumeWorkspace struct { + volume string +} + +var _ Workspace = &dockerVolumeWorkspace{} + +func (w *dockerVolumeWorkspace) Close(ctx context.Context) error { + // Cleanup here is easy: we just get rid of the Docker volume. + return exec.CommandContext(ctx, "docker", "volume", "rm", w.volume).Run() +} + +func (w *dockerVolumeWorkspace) DockerRunOpts(ctx context.Context, target string) ([]string, error) { + return []string{ + "--mount", "type=volume,source=" + w.volume + ",target=" + target, + }, nil +} + +func (w *dockerVolumeWorkspace) WorkDir() *string { return nil } + +func (w *dockerVolumeWorkspace) Changes(ctx context.Context) (*StepChanges, error) { + script := `#!/bin/sh + +set -e +# No set -x here, since we're going to parse the git status output. + +git add --all > /dev/null +exec git status --porcelain +` + + out, err := w.runScript(ctx, "/work", script) + if err != nil { + return nil, errors.Wrap(err, "running git status") + } + + changes, err := parseGitStatus(out) + if err != nil { + return nil, errors.Wrapf(err, "parsing git status output:\n\n%s", string(out)) + } + + return &changes, nil +} + +func (w *dockerVolumeWorkspace) Diff(ctx context.Context) ([]byte, error) { + // As of Sourcegraph 3.14 we only support unified diff format. + // That means we need to strip away the `a/` and `/b` prefixes with `--no-prefix`. + // See: https://github.com/sourcegraph/sourcegraph/blob/82d5e7e1562fef6be5c0b17f18631040fd330835/enterprise/internal/campaigns/service.go#L324-L329 + // + // Also, we need to add --binary so binary file changes are inlined in the patch. + script := `#!/bin/sh + +exec git diff --cached --no-prefix --binary +` + + out, err := w.runScript(ctx, "/work", script) + if err != nil { + return nil, errors.Wrapf(err, "git diff:\n\n%s", string(out)) + } + + return out, nil +} + +// dockerWorkspaceImage is the Docker image we'll run our unzip and git commands +// in. This needs to match the name defined in .github/workflows/docker.yml. +const dockerWorkspaceImage = "sourcegraph/src-campaign-volume-workspace" + +// runScript is a utility function to mount the given shell script into a Docker +// container started from the dockerWorkspaceImage, then run it and return the +// output. +func (w *dockerVolumeWorkspace) runScript(ctx context.Context, target, script string) ([]byte, error) { + f, err := ioutil.TempFile(os.TempDir(), "src-run-*") + if err != nil { + return nil, errors.Wrap(err, "creating run script") + } + name := f.Name() + defer os.Remove(name) + + if _, err := f.WriteString(script); err != nil { + return nil, errors.Wrap(err, "writing run script") + } + f.Close() + + common, err := w.DockerRunOpts(ctx, target) + if err != nil { + return nil, errors.Wrap(err, "generating run options") + } + + opts := append([]string{ + "run", + "--rm", + "--init", + "--workdir", target, + "--mount", "type=bind,source=" + name + ",target=/run.sh,ro", + }, common...) + opts = append(opts, dockerWorkspaceImage, "sh", "/run.sh") + + out, err := exec.CommandContext(ctx, "docker", opts...).CombinedOutput() + if err != nil { + return out, errors.Wrapf(err, "Docker output:\n\n%s\n\n", string(out)) + } + + return out, nil +} diff --git a/internal/campaigns/volume_workspace_test.go b/internal/campaigns/volume_workspace_test.go new file mode 100644 index 0000000000..3732cb6386 --- /dev/null +++ b/internal/campaigns/volume_workspace_test.go @@ -0,0 +1,393 @@ +package campaigns + +import ( + "bytes" + "context" + "io/ioutil" + "os" + "strings" + "testing" + + "github.com/google/go-cmp/cmp" + "github.com/pkg/errors" + + "github.com/sourcegraph/src-cli/internal/campaigns/graphql" + "github.com/sourcegraph/src-cli/internal/exec/expect" +) + +// We may as well use the same volume ID in all the subtests. +const volumeID = "VOLUME-ID" + +func TestVolumeWorkspaceCreator(t *testing.T) { + ctx := context.Background() + + // Create an empty file. It doesn't matter that it's an invalid zip, since + // we're mocking the unzip command anyway. + f, err := ioutil.TempFile(os.TempDir(), "volume-workspace-*") + if err != nil { + t.Fatal(err) + } + zip := f.Name() + f.Close() + defer os.Remove(zip) + + wc := &dockerVolumeWorkspaceCreator{} + + // We'll set up a fake repository with just enough fields defined for init() + // and friends. + repo := &graphql.Repository{ + DefaultBranch: &graphql.Branch{Name: "main"}, + } + + t.Run("success", func(t *testing.T) { + expect.Commands( + t, + expect.NewGlob( + expect.Behaviour{Stdout: []byte(volumeID)}, + "docker", "volume", "create", + ), + expect.NewGlob( + expect.Behaviour{}, + "docker", "run", "--rm", "--init", "--workdir", "/work", + "--mount", "type=bind,source=*,target=/tmp/zip,ro", + "--mount", "type=volume,source="+volumeID+",target=/work", + dockerWorkspaceImage, + "unzip", "/tmp/zip", + ), + expect.NewGlob( + expect.Behaviour{}, + "docker", "run", "--rm", "--init", "--workdir", "/work", + "--mount", "type=bind,source=*,target=/run.sh,ro", + "--mount", "type=volume,source="+volumeID+",target=/work", + dockerWorkspaceImage, + "sh", "/run.sh", + ), + ) + + if w, err := wc.Create(ctx, repo, zip); err != nil { + t.Errorf("unexpected error: %v", err) + } else if have := w.(*dockerVolumeWorkspace).volume; have != volumeID { + t.Errorf("unexpected volume: have=%q want=%q", have, volumeID) + } + }) + + t.Run("docker volume create failure", func(t *testing.T) { + expect.Commands( + t, + expect.NewGlob( + expect.Behaviour{ExitCode: 1}, + "docker", "volume", "create", + ), + ) + + if _, err := wc.Create(ctx, repo, zip); err == nil { + t.Error("unexpected nil error") + } + }) + + t.Run("unzip failure", func(t *testing.T) { + expect.Commands( + t, + expect.NewGlob( + expect.Behaviour{Stdout: []byte(volumeID)}, + "docker", "volume", "create", + ), + expect.NewGlob( + expect.Behaviour{ExitCode: 1}, + "docker", "run", "--rm", "--init", "--workdir", "/work", + "--mount", "type=bind,source=*,target=/tmp/zip,ro", + "--mount", "type=volume,source="+volumeID+",target=/work", + dockerWorkspaceImage, + "unzip", "/tmp/zip", + ), + ) + + if _, err := wc.Create(ctx, repo, zip); err == nil { + t.Error("unexpected nil error") + } + }) + + t.Run("git init failure", func(t *testing.T) { + expect.Commands( + t, + expect.NewGlob( + expect.Behaviour{Stdout: []byte(volumeID)}, + "docker", "volume", "create", + ), + expect.NewGlob( + expect.Behaviour{}, + "docker", "run", "--rm", "--init", "--workdir", "/work", + "--mount", "type=bind,source=*,target=/tmp/zip,ro", + "--mount", "type=volume,source="+volumeID+",target=/work", + dockerWorkspaceImage, + "unzip", "/tmp/zip", + ), + expect.NewGlob( + expect.Behaviour{ExitCode: 1}, + "docker", "run", "--rm", "--init", "--workdir", "/work", + "--mount", "type=bind,source=*,target=/run.sh,ro", + "--mount", "type=volume,source="+volumeID+",target=/work", + dockerWorkspaceImage, + "sh", "/run.sh", + ), + ) + + if _, err := wc.Create(ctx, repo, zip); err == nil { + t.Error("unexpected nil error") + } + }) +} + +func TestVolumeWorkspace_Close(t *testing.T) { + ctx := context.Background() + w := &dockerVolumeWorkspace{volume: volumeID} + + t.Run("success", func(t *testing.T) { + expect.Commands( + t, + expect.NewGlob( + expect.Behaviour{}, + "docker", "volume", "rm", volumeID, + ), + ) + + if err := w.Close(ctx); err != nil { + t.Errorf("unexpected error: %v", err) + } + }) + + t.Run("failure", func(t *testing.T) { + expect.Commands( + t, + expect.NewGlob( + expect.Behaviour{ExitCode: 1}, + "docker", "volume", "rm", volumeID, + ), + ) + + if err := w.Close(ctx); err == nil { + t.Error("unexpected nil error") + } + }) +} + +func TestVolumeWorkspace_DockerRunOpts(t *testing.T) { + ctx := context.Background() + w := &dockerVolumeWorkspace{volume: "VOLUME"} + + want := []string{ + "--mount", "type=volume,source=VOLUME,target=TARGET", + } + have, err := w.DockerRunOpts(ctx, "TARGET") + if err != nil { + t.Errorf("unexpected error: %+v", err) + } + if diff := cmp.Diff(have, want); diff != "" { + t.Errorf("unexpected options (-have +want):\n%s", diff) + } +} + +func TestVolumeWorkspace_WorkDir(t *testing.T) { + if have := (&dockerVolumeWorkspace{}).WorkDir(); have != nil { + t.Errorf("unexpected work dir: %q", *have) + } +} + +// For the below tests that essentially delegate to runScript, we're not going +// to test the content of the script file: we'll do that as a one off test at +// the bottom of runScript itself, rather than depending on script content that +// may drift over time. + +func TestVolumeWorkspace_Changes(t *testing.T) { + ctx := context.Background() + w := &dockerVolumeWorkspace{volume: volumeID} + + t.Run("success", func(t *testing.T) { + for name, tc := range map[string]struct { + stdout string + want *StepChanges + }{ + "empty": { + stdout: "", + want: &StepChanges{}, + }, + "valid": { + stdout: ` +M go.mod +M internal/campaigns/volume_workspace.go +M internal/campaigns/volume_workspace_test.go + `, + want: &StepChanges{Modified: []string{ + "go.mod", + "internal/campaigns/volume_workspace.go", + "internal/campaigns/volume_workspace_test.go", + }}, + }, + } { + t.Run(name, func(t *testing.T) { + expect.Commands( + t, + expect.NewGlob( + expect.Behaviour{Stdout: bytes.TrimSpace([]byte(tc.stdout))}, + "docker", "run", "--rm", "--init", "--workdir", "/work", + "--mount", "type=bind,source=*,target=/run.sh,ro", + "--mount", "type=volume,source="+volumeID+",target=/work", + dockerWorkspaceImage, + "sh", "/run.sh", + ), + ) + + have, err := w.Changes(ctx) + if err != nil { + t.Errorf("unexpected error: %v", err) + } + + if diff := cmp.Diff(have, tc.want); diff != "" { + t.Errorf("unexpected changes (-have +want):\n\n%s", diff) + } + + }) + } + }) + + t.Run("failure", func(t *testing.T) { + for name, behaviour := range map[string]expect.Behaviour{ + "exit code": {ExitCode: 1}, + "malformed status": {Stdout: []byte("Z")}, + } { + t.Run(name, func(t *testing.T) { + expect.Commands( + t, + expect.NewGlob( + behaviour, + "docker", "run", "--rm", "--init", "--workdir", "/work", + "--mount", "type=bind,source=*,target=/run.sh,ro", + "--mount", "type=volume,source="+volumeID+",target=/work", + dockerWorkspaceImage, + "sh", "/run.sh", + ), + ) + + if _, err := w.Changes(ctx); err == nil { + t.Error("unexpected nil error") + } + }) + } + }) +} + +func TestVolumeWorkspace_Diff(t *testing.T) { + ctx := context.Background() + w := &dockerVolumeWorkspace{volume: volumeID} + + t.Run("success", func(t *testing.T) { + for name, tc := range map[string]string{ + "empty": "", + "valid": ` +diff --git a/go.mod b/go.mod +index 06471f4..5f9d3fa 100644 +--- a/go.mod ++++ b/go.mod +@@ -7,6 +7,7 @@ require ( + github.com/alessio/shellescape v1.4.1 + github.com/dustin/go-humanize v1.0.0 + github.com/efritz/pentimento v0.0.0-20190429011147-ade47d831101 ++ github.com/gobwas/glob v0.2.3 + github.com/google/go-cmp v0.5.2 + github.com/hashicorp/errwrap v1.1.0 // indirect + github.com/hashicorp/go-multierror v1.1.0 + `, + } { + t.Run(name, func(t *testing.T) { + want := strings.TrimSpace(tc) + + expect.Commands( + t, + expect.NewGlob( + expect.Behaviour{Stdout: []byte(want)}, + "docker", "run", "--rm", "--init", "--workdir", "/work", + "--mount", "type=bind,source=*,target=/run.sh,ro", + "--mount", "type=volume,source="+volumeID+",target=/work", + dockerWorkspaceImage, + "sh", "/run.sh", + ), + ) + + have, err := w.Diff(ctx) + if err != nil { + t.Errorf("unexpected error: %v", err) + } + + if diff := cmp.Diff(string(have), want); diff != "" { + t.Errorf("unexpected changes (-have +want):\n\n%s", diff) + } + + }) + } + }) + + t.Run("failure", func(t *testing.T) { + expect.Commands( + t, + expect.NewGlob( + expect.Behaviour{ExitCode: 1}, + "docker", "run", "--rm", "--init", "--workdir", "/work", + "--mount", "type=bind,source=*,target=/run.sh,ro", + "--mount", "type=volume,source="+volumeID+",target=/work", + dockerWorkspaceImage, + "sh", "/run.sh", + ), + ) + + if _, err := w.Diff(ctx); err == nil { + t.Error("unexpected nil error") + } + }) +} + +func TestVolumeWorkspace_runScript(t *testing.T) { + // Since the above tests have thoroughly tested our error handling, this + // test just fills in the one logical gap we have in our test coverage: is + // the temporary script file correct? + const script = "#!/bin/sh\n\necho FOO" + ctx := context.Background() + w := &dockerVolumeWorkspace{volume: volumeID} + + expect.Commands( + t, + &expect.Expectation{ + Validator: func(name string, arg ...string) error { + // Run normal glob validation of the command line. + glob := expect.NewGlobValidator( + "docker", "run", "--rm", "--init", "--workdir", "/work", + "--mount", "type=bind,source=*,target=/run.sh,ro", + "--mount", "type=volume,source="+volumeID+",target=/work", + dockerWorkspaceImage, + "sh", "/run.sh", + ) + if err := glob(name, arg...); err != nil { + return err + } + + // OK, we know that the temporary file the script lives in can + // be parsed out of the seventh parameter, which provides the + // mount options. Let's go get it! + values := strings.Split(arg[6], ",") + source := strings.SplitN(values[1], "=", 2) + have, err := ioutil.ReadFile(source[1]) + if err != nil { + return errors.Errorf("error reading temporary file %q: %v", source[1], err) + } + + if string(have) != script { + return errors.Errorf("unexpected script: have=%q want=%q", string(have), script) + } + return nil + }, + }, + ) + + if _, err := w.runScript(ctx, "/work", script); err != nil { + t.Fatal(err) + } +} diff --git a/internal/campaigns/workspace.go b/internal/campaigns/workspace.go new file mode 100644 index 0000000000..45bd9797e1 --- /dev/null +++ b/internal/campaigns/workspace.go @@ -0,0 +1,46 @@ +package campaigns + +import ( + "context" + + "github.com/sourcegraph/src-cli/internal/campaigns/graphql" +) + +// WorkspaceCreator implementations are used to create workspaces, which manage +// per-changeset persistent storage when executing campaign steps and are +// responsible for ultimately generating a diff. +type WorkspaceCreator interface { + // Create creates a new workspace for the given repository and ZIP file. + Create(ctx context.Context, repo *graphql.Repository, zip string) (Workspace, error) + + // DockerImages returns any Docker images required to use workspaces created + // by this creator. + DockerImages() []string +} + +// Workspace implementations manage per-changeset storage when executing +// campaign step. +type Workspace interface { + // DockerRunOpts provides the options that should be given to `docker run` + // in order to use this workspace. Generally, this will be a set of mount + // options. + DockerRunOpts(ctx context.Context, target string) ([]string, error) + + // WorkDir allows workspaces to specify the working directory that should be + // used when running Docker. If no specific working directory is needed, + // then the function should return nil. + WorkDir() *string + + // Close is called once, after all steps have been executed and the diff has + // been calculated and stored outside the workspace. Implementations should + // delete the workspace when Close is called. + Close(ctx context.Context) error + + // Changes is called after each step is executed, and should return the + // cumulative file changes that have occurred since Prepare was called. + Changes(ctx context.Context) (*StepChanges, error) + + // Diff should return the total diff for the workspace. This may be called + // multiple times in the life of a workspace. + Diff(ctx context.Context) ([]byte, error) +} diff --git a/internal/exec/exec.go b/internal/exec/exec.go new file mode 100644 index 0000000000..1da35175d0 --- /dev/null +++ b/internal/exec/exec.go @@ -0,0 +1,31 @@ +// Package exec provides wrapped implementations of os/exec's Command and +// CommandContext functions that allow for command creation to be overridden, +// thereby allowing commands to be mocked. +package exec + +import ( + "context" + goexec "os/exec" +) + +// CmdCreator instances are used to create commands. os/exec.CommandContext is a +// valid CmdCreator. +type CmdCreator func(context.Context, string, ...string) *goexec.Cmd + +// creator is the singleton used to create a new command. +var creator CmdCreator = goexec.CommandContext + +// Command wraps os/exec.Command, and implements the same behaviour. +func Command(name string, arg ...string) *goexec.Cmd { + return CommandContext(nil, name, arg...) +} + +// CommandContext wraps os/exec.CommandContext, and implements the same +// behaviour. +func CommandContext(ctx context.Context, name string, arg ...string) *goexec.Cmd { + // TODO: if we add global logging infrastructure to cmd/src, we could + // leverage it here to log all commands that are executed in an appropriate + // verbose mode. + + return creator(ctx, name, arg...) +} diff --git a/internal/exec/expect/expect.go b/internal/exec/expect/expect.go new file mode 100644 index 0000000000..a63a0ab8b7 --- /dev/null +++ b/internal/exec/expect/expect.go @@ -0,0 +1,197 @@ +// Package expect uses the middleware concept in internal/exec to mock external +// commands. +// +// Generally, you only want to use this package in testing code. +// +// At a high level, this package operates by overriding created commands to +// invoke the current executable with a specific environment variable, which +// points to a temporary file with metadata on the behaviour that the command +// should implement. (This approach is shamelessly stolen from Go's os/exec test +// suite, but the details are somewhat different.) +// +// This means that the main() function of the executable _must_ check the +// relevant environment variable as early as possible, and not perform its usual +// logic if it's found. An implementation of this is provided for TestMain +// functions in the Handle function, which is normally how this package is used. +package expect + +import ( + "context" + "encoding/json" + "fmt" + "io/ioutil" + "os" + goexec "os/exec" + "testing" + + "github.com/gobwas/glob" + "github.com/hashicorp/go-multierror" + "github.com/pkg/errors" + + "github.com/sourcegraph/src-cli/internal/exec" +) + +// envBehaviourFile is the environment variable used to define the behaviour +// file. +const envBehaviourFile = "GO_EXEC_TESTING_BEHAVIOUR_FILE" + +// Behaviour defines the behaviour of the mocked command. +type Behaviour struct { + // Stdout defines the data that will be returned on stdout. + Stdout []byte + + // Stderr defines the data that will be returned on stderr. + Stderr []byte + + // ExitCode is the exit code that the process will exit with. + ExitCode int +} + +// Commands defines a set of expected commands for the given test. Commands may +// be called from any number of nested subtests, but must only be called once +// from a single test function, as it uses (*testing.T).Cleanup to manage +// per-test state. +func Commands(t *testing.T, exp ...*Expectation) { + t.Helper() + + i := 0 + mc := exec.NewMiddleware(func(ctx context.Context, previous exec.CmdCreator, name string, arg ...string) *goexec.Cmd { + if i >= len(exp) { + t.Fatalf("one or more extra commands created in spite of expecting %d command(s); attempting to create %q with arguments %v", len(exp), name, arg) + } + + if err := exp[i].Validator(name, arg...); err != nil { + t.Fatalf("unexpected command: %v", err) + } + + // Create the command using the next level of middleware. (Which is + // probably eventually os/exec.CommandContext().) + cmd := previous(ctx, name, arg...) + if cmd == nil { + t.Fatalf("unexpected nil *Cmd for %q with arguments %v", name, arg) + } + + // Now we modify the command to re-invoke this executable instead. We'll + // also blank out the arguments, since this should be controlled + // entirely by the presence of the behaviour file environment variable. + cmd.Path = os.Args[0] + cmd.Args = []string{} + + // Actually create the behaviour file. + f, err := ioutil.TempFile(os.TempDir(), "behaviour") + if err != nil { + t.Fatalf("error creating behaviour file: %v", err) + } + defer f.Close() + file := f.Name() + t.Cleanup(func() { os.Remove(file) }) + + data, err := json.Marshal(&exp[i].Behaviour) + if err != nil { + t.Fatalf("error marshalling behaviour data: %v", err) + } + f.Write(data) + + // Set the relevant environment variable. + cmd.Env = append(cmd.Env, envBehaviourFile+"="+file) + + i++ + return cmd + }) + + t.Cleanup(func() { + mc.Remove() + + if i != len(exp) { + t.Fatalf("unexpected number of commands executed: have=%d want=%d", i, len(exp)) + } + }) +} + +// Handle should be called from TestMain. It intercepts expected commands and +// implements the expected behaviour. +// +// m is defined as an interface rather than *testing.M to make this usable from +// outside of a testing context. +func Handle(m interface{ Run() int }) int { + if file := os.Getenv(envBehaviourFile); file != "" { + panicErr := func(err error) { + fmt.Fprintf(os.Stderr, "panic with error: %s", err.Error()) + // This exit code is chosen at random: obviously, a test that + // expects a failure might just swallow this and be happy. We do the + // best we can. + os.Exit(255) + } + + // Load up the expected behaviour of this command. + data, err := ioutil.ReadFile(file) + if err != nil { + panicErr(err) + } + + var b Behaviour + if err := json.Unmarshal(data, &b); err != nil { + panicErr(err) + } + + // Do it! + os.Stderr.Write(b.Stderr) + os.Stdout.Write(b.Stdout) + os.Exit(b.ExitCode) + } + + return m.Run() +} + +// Expectation represents a single command invocation. +type Expectation struct { + Validator CommandValidator + Behaviour Behaviour +} + +// CommandValidator is used to validate the command line that is would be +// executed. +type CommandValidator func(name string, arg ...string) error + +// NewGlob is a convenience function that creates an Expectation that validates +// commands using a glob validator (as created by NewGlobValidator) and +// implements the given behaviour. +// +// You don't need to use this, but it tends to make Commands() calls more +// readable. +func NewGlob(behaviour Behaviour, wantName string, wantArg ...string) *Expectation { + return &Expectation{ + Behaviour: behaviour, + Validator: NewGlobValidator(wantName, wantArg...), + } +} + +// NewGlobValidator creates a validation function that will validate a command +// using glob syntax against the given name and arguments. +func NewGlobValidator(wantName string, wantArg ...string) CommandValidator { + wantNameGlob := glob.MustCompile(wantName) + wantArgGlobs := make([]glob.Glob, len(wantArg)) + for i, a := range wantArg { + wantArgGlobs[i] = glob.MustCompile(a) + } + + return func(haveName string, haveArg ...string) error { + var errs *multierror.Error + + if !wantNameGlob.Match(haveName) { + errs = multierror.Append(errs, errors.Errorf("name does not match: have=%q want=%q", haveName, wantName)) + } + + if len(haveArg) != len(wantArgGlobs) { + errs = multierror.Append(errs, errors.Errorf("unexpected number of arguments: have=%v want=%v", haveArg, wantArg)) + } else { + for i, g := range wantArgGlobs { + if !g.Match(haveArg[i]) { + errs = multierror.Append(errs, errors.Errorf("unexpected argument at position %d: have=%q want=%q", i, haveArg[i], wantArg[i])) + } + } + } + + return errs.ErrorOrNil() + } +} diff --git a/internal/exec/middleware.go b/internal/exec/middleware.go new file mode 100644 index 0000000000..3236047eb7 --- /dev/null +++ b/internal/exec/middleware.go @@ -0,0 +1,24 @@ +package exec + +import ( + "context" + goexec "os/exec" +) + +// CmdCreatorMiddleware creates *exec.Cmd instances that delegate command +// creation to a provided callback. +type CmdCreatorMiddleware struct{ previous CmdCreator } + +// NewMiddleware adds a middleware to the command creation stack. +func NewMiddleware(mock func(context.Context, CmdCreator, string, ...string) *goexec.Cmd) CmdCreatorMiddleware { + mc := CmdCreatorMiddleware{previous: creator} + creator = func(ctx context.Context, name string, arg ...string) *goexec.Cmd { + return mock(ctx, mc.previous, name, arg...) + } + return mc +} + +// Remove removes the command creation middleware from the stack. +func (mc CmdCreatorMiddleware) Remove() { + creator = mc.previous +}