Run fetch and update in separate containers behind isolated_fetch_update

With the experiment on, the CLI clones in one updater container and updates in
another. The clone travels between them on a Docker volume mounted at
DEPENDABOT_REPO_CONTENTS_PATH in both, so it is made once rather than twice.
Each side gets its own proxy on its own network pair, so the update container
has no route to the fetch proxy and the two credential sets can diverge later.
They are identical for now; the call site marks the seam.

`--local` populates the fetch container only. Debug shells, the OpenTelemetry
collector and use_case_insensitive_filesystem are rejected rather than silently
misbehaving under the split.

Combined mode drops the `bin/run fetch_files &&` prefix. That entrypoint used to
be a no-op stub, so this is byte-identical to today, and it avoids fetching
twice now that dependabot-core makes it do real work.

Also fixes container output being truncated: RunCmd piped through io.Copy and
prefixer, but Prefixer.WriteTo returns without writing pending data at EOF and
io.Copy only calls WriteTo once, so the tail of every command was dropped.
Running `bin/run` twice used to mask it, which is what the "unless I echo here
the json doesn't output" comment in input.txt was working around.

Author: v-robaiken

docs/debugging.md

@@ -10,14 +10,13 @@ Next, clone https://github.com/dependabot/dependabot-core. This project contains
 
 Try opening a terminal and run `script/dependabot update go_modules dependabot/cli --debug` in the `dependabot-core` project directory. This will drop you in an interactive session with the update ready to proceed.
 
-To perform the update, you need to run two commands:
+To perform the update, run:
 
-- `bin/run fetch_files`
 - `bin/run update_files`
 
-If the problem you are debugging is during the fetch step, the fetch_files command will be all you need to run.
+This fetches the dependency files and then updates them.
 
-If the problem is after the fetch step, you can repeatedly run update_files while you're debugging. 
+If the problem you are debugging is during the fetch step, run `bin/run fetch_files` instead. It needs somewhere to write the fetched file set, so set `DEPENDABOT_HANDOFF_PATH` first, for example `export DEPENDABOT_HANDOFF_PATH=/tmp/handoff.json`. `update_files` reads that file instead of fetching again when the variable is set, so you can repeatedly run `update_files` against a single fetch while you're debugging.
 
 In the example `script/dependabot` command above, try running an update in the container. 
 
@@ -32,7 +31,7 @@ Next, let's try adding a `debugger` statement. Open the `dependabot-core` projec
 
 > **Note** You don't have to restart your CLI session, the changes are automatically synced to the container!
 
-In the interactive debugging session, run `bin/run fetch_files` and `bin/run update_files`. During the update_files command, the Ruby debugger will open. It should look something like this:
+In the interactive debugging session, run `bin/run update_files`. During that command, the Ruby debugger will open. It should look something like this:
 
 ```ruby
 [11, 20] in ~/go_modules/lib/dependabot/go_modules/update_checker.rb
@@ -60,7 +59,7 @@ At this prompt, you can run [debugger commands](https://github.com/ruby/debug) t
 
 If your Dependabot job is hanging and would like to figure out why, the CLI is the perfect tool for the job. 
 
-Start by running the update that recreates the hang with `dependabot update <ecosystem> <org/repo>`. Once the hang is reproducible, run with the `--debug` flag and the run the `fetch_files` and `update_files` commands and wait until the job hangs.
+Start by running the update that recreates the hang with `dependabot update <ecosystem> <org/repo>`. Once the hang is reproducible, run with the `--debug` flag and then run the `update_files` command and wait until the job hangs.
 
 Once it does hang, hit CTL-C, and you'll get a stack trace leading you to the problematic code.
 

internal/infra/proxy.go

@@ -10,6 +10,7 @@ import (
 	"path"
 	"path/filepath"
 
+	"github.com/dependabot/cli/internal/model"
 	"github.com/docker/docker/api/types/container"
 	"github.com/docker/docker/api/types/mount"
 	"github.com/docker/docker/api/types/network"
@@ -32,6 +33,12 @@ type Proxy struct {
 }
 
 func NewProxy(ctx context.Context, cli *client.Client, params *RunParams, nets *Networks) (*Proxy, error) {
+	return newProxyWithCreds(ctx, cli, params, nets, params.Creds)
+}
+
+// newProxyWithCreds builds a proxy serving a specific credential set, so the fetch
+// and update sides can be given different ones.
+func newProxyWithCreds(ctx context.Context, cli *client.Client, params *RunParams, nets *Networks, creds []model.Credential) (*Proxy, error) {
 	// Generate secrets:
 	ca, err := GenerateCertificateAuthority()
 	if err != nil {
@@ -40,7 +47,7 @@ func NewProxy(ctx context.Context, cli *client.Client, params *RunParams, nets *
 
 	// Generate and write configuration to disk:
 	proxyConfig := &Config{
-		Credentials: params.Creds,
+		Credentials: creds,
 		CA:          ca,
 	}
 

internal/infra/run.go

@@ -18,6 +18,7 @@ import (
 	"time"
 
 	"github.com/docker/docker/api/types/container"
+	"github.com/docker/docker/api/types/volume"
 
 	"github.com/dependabot/cli/internal/model"
 	"github.com/dependabot/cli/internal/server"
@@ -31,12 +32,16 @@ import (
 	"gopkg.in/yaml.v3"
 )
 
-var runCmds = map[model.RunCommand]string{
-	model.VersionCommand:     "bin/run fetch_files && bin/run update_files",
-	model.UpdateFilesCommand: "bin/run fetch_files && bin/run update_files",
-	model.RecreateCommand:    "bin/run fetch_files && bin/run update_files",
-	model.SecurityCommand:    "bin/run fetch_files && bin/run update_files",
-	model.UpdateGraphCommand: "bin/run fetch_files && bin/run update_graph",
+// fetchCmd fetches the dependency files. In the combined topology the update
+// commands fetch for themselves, so it only runs in the split topology.
+const fetchCmd = "bin/run fetch_files"
+
+var updateCmds = map[model.RunCommand]string{
+	model.VersionCommand:     "bin/run update_files",
+	model.UpdateFilesCommand: "bin/run update_files",
+	model.RecreateCommand:    "bin/run update_files",
+	model.SecurityCommand:    "bin/run update_files",
+	model.UpdateGraphCommand: "bin/run update_graph",
 }
 
 type RunParams struct {
@@ -445,7 +450,16 @@ func runContainers(ctx context.Context, params RunParams) (err error) {
 		defer collector.Close()
 	}
 
-	updater, err := NewUpdater(ctx, cli, networks, &params, prox, collector)
+	if params.Job.IsolatedFetchUpdate() {
+		return runIsolated(ctx, cli, networks, &params, prox, collector)
+	}
+
+	return runCombined(ctx, cli, networks, &params, prox, collector)
+}
+
+// runCombined runs fetch and update in a single container sharing a repo clone.
+func runCombined(ctx context.Context, cli *client.Client, networks *Networks, params *RunParams, prox *Proxy, collector *Collector) (err error) {
+	updater, err := NewUpdater(ctx, cli, networks, params, prox, collector, "")
 	if err != nil {
 		return err
 	}
@@ -455,16 +469,8 @@ func runContainers(ctx context.Context, params RunParams) (err error) {
 		}
 	}()
 
-	// put the clone dir in the updater container to be used by during the update
-	if params.LocalDir != "" {
-		containerDir := guestRepoDir
-		if params.Job.UseCaseInsensitiveFileSystem() {
-			// since the updater is using the storage container, we need to populate the repo on that device because that's the directory that will be used for the update
-			containerDir = caseSensitiveRepoContentsPath
-		}
-		if err = putCloneDir(ctx, cli, updater, params.LocalDir, containerDir); err != nil {
-			return err
-		}
+	if err = placeCloneDir(ctx, cli, params, updater); err != nil {
+		return err
 	}
 
 	// update CA certificates as root prior to start debug shell or running dependabot commands
@@ -473,25 +479,145 @@ func runContainers(ctx context.Context, params RunParams) (err error) {
 	}
 
 	if params.Debug {
-		if err := updater.RunShell(ctx, prox.url, params.ApiUrl, params.Job, params.UpdaterEnvironmentVariables); err != nil {
-			return err
+		return updater.RunShell(ctx, prox.url, params.ApiUrl, params.Job, params.UpdaterEnvironmentVariables)
+	}
+
+	// Run dependabot commands as a dependabot user
+	env := userEnv(prox.url, params.ApiUrl, params.Job, params.UpdaterEnvironmentVariables)
+	if params.Flamegraph {
+		env = append(env, "FLAMEGRAPH=1")
+	}
+	if err := updater.RunCmd(ctx, updateCmds[params.Job.Command], dependabot, env...); err != nil {
+		return err
+	}
+	if params.Flamegraph {
+		getFromContainer(ctx, cli, updater.containerID, "/tmp/dependabot-flamegraph.html")
+	}
+
+	return checkExitCode(params, updater)
+}
+
+// runIsolated clones in one container and updates in another. The clone travels
+// between them on a shared volume rather than being made twice. Each side gets its
+// own proxy on its own network, so the update side cannot reach the fetch proxy.
+func runIsolated(ctx context.Context, cli *client.Client, networks *Networks, params *RunParams, prox *Proxy, collector *Collector) (err error) {
+	if params.Debug {
+		return fmt.Errorf("--debug is not supported with the isolated_fetch_update experiment")
+	}
+	if params.Job.UseCaseInsensitiveFileSystem() {
+		return fmt.Errorf("isolated_fetch_update is not supported with use_case_insensitive_filesystem")
+	}
+	if params.CollectorConfigPath != "" {
+		return fmt.Errorf("the OpenTelemetry collector is not supported with the isolated_fetch_update experiment")
+	}
+
+	repoVolume, err := cli.VolumeCreate(ctx, volume.CreateOptions{Labels: map[string]string{"dependabot-cli": "repo"}})
+	if err != nil {
+		return fmt.Errorf("failed to create repo volume: %w", err)
+	}
+	defer func() {
+		if volumeErr := cli.VolumeRemove(context.Background(), repoVolume.Name, true); volumeErr != nil {
+			err = volumeErr
 		}
-	} else {
-		// Run dependabot commands as a dependabot user
-		env := userEnv(prox.url, params.ApiUrl, params.Job, params.UpdaterEnvironmentVariables)
-		if params.Flamegraph {
-			env = append(env, "FLAMEGRAPH=1")
+	}()
+
+	fetcher, err := NewUpdater(ctx, cli, networks, params, prox, collector, repoVolume.Name)
+	if err != nil {
+		return err
+	}
+	defer func() {
+		if fetcherErr := fetcher.Close(); fetcherErr != nil {
+			err = fetcherErr
 		}
-		if err := updater.RunCmd(ctx, runCmds[params.Job.Command], dependabot, env...); err != nil {
-			return err
+	}()
+
+	if err = placeCloneDir(ctx, cli, params, fetcher); err != nil {
+		return err
+	}
+
+	if err = fetcher.RunCmd(ctx, "update-ca-certificates", root); err != nil {
+		return err
+	}
+
+	fetchEnv := userEnv(prox.url, params.ApiUrl, params.Job, params.UpdaterEnvironmentVariables)
+	if err = fetcher.RunCmd(ctx, fetchCmd, dependabot, fetchEnv...); err != nil {
+		return err
+	}
+	if *fetcher.ExitCode != 0 {
+		return fmt.Errorf("fetch exited with code %d", *fetcher.ExitCode)
+	}
+
+	// The update side gets its own network and proxy so its credentials can diverge
+	// from the fetch side's without the two containers being able to swap proxies.
+	updateNetworks, err := NewNetworks(ctx, cli)
+	if err != nil {
+		return fmt.Errorf("failed to create update networks: %w", err)
+	}
+	defer func() {
+		if netErr := updateNetworks.Close(); netErr != nil {
+			err = netErr
 		}
-		if params.Flamegraph {
-			getFromContainer(ctx, cli, updater.containerID, "/tmp/dependabot-flamegraph.html")
+	}()
+
+	// Same credentials as the fetch side for now. This is the seam where the repo-read
+	// credential gets dropped, once the update side no longer calls the target repo.
+	updateProxy, err := newProxyWithCreds(ctx, cli, params, updateNetworks, params.Creds)
+	if err != nil {
+		return fmt.Errorf("failed to create update proxy: %w", err)
+	}
+	defer func() {
+		if proxyErr := updateProxy.Close(); proxyErr != nil {
+			err = proxyErr
 		}
-		// If the exit code is non-zero, error when using the `update` subcommand, but not the `test` subcommand.
-		if params.Expected == nil && *updater.ExitCode != 0 {
-			return fmt.Errorf("updater exited with code %d", *updater.ExitCode)
+	}()
+	go updateProxy.TailLogs(ctx, cli)
+
+	updater, err := NewUpdater(ctx, cli, updateNetworks, params, updateProxy, collector, repoVolume.Name)
+	if err != nil {
+		return err
+	}
+	defer func() {
+		if updaterErr := updater.Close(); updaterErr != nil {
+			err = updaterErr
 		}
+	}()
+
+	if err = updater.RunCmd(ctx, "update-ca-certificates", root); err != nil {
+		return err
+	}
+
+	env := userEnv(updateProxy.url, params.ApiUrl, params.Job, params.UpdaterEnvironmentVariables)
+	if params.Flamegraph {
+		env = append(env, "FLAMEGRAPH=1")
+	}
+	if err = updater.RunCmd(ctx, updateCmds[params.Job.Command], dependabot, env...); err != nil {
+		return err
+	}
+	if params.Flamegraph {
+		getFromContainer(ctx, cli, updater.containerID, "/tmp/dependabot-flamegraph.html")
+	}
+
+	return checkExitCode(params, updater)
+}
+
+func placeCloneDir(ctx context.Context, cli *client.Client, params *RunParams, updater *Updater) error {
+	if params.LocalDir == "" {
+		return nil
+	}
+
+	containerDir := guestRepoDir
+	if params.Job.UseCaseInsensitiveFileSystem() {
+		// since the updater is using the storage container, we need to populate the repo on that device because that's the directory that will be used for the update
+		containerDir = caseSensitiveRepoContentsPath
+	}
+
+	return putCloneDir(ctx, cli, updater, params.LocalDir, containerDir)
+}
+
+// checkExitCode errors when using the `update` subcommand, but not the `test` subcommand.
+func checkExitCode(params *RunParams, updater *Updater) error {
+	if params.Expected == nil && *updater.ExitCode != 0 {
+		return fmt.Errorf("updater exited with code %d", *updater.ExitCode)
 	}
 
 	return nil