Run fetch and update in separate containers behind isolated_fetch_update

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