Repository: coder/envbuilder
Pull request #390 (2024-10-15 12:27:44)

fix: do not error if failed to push image
Relates to https://github.com/coder/envbuilder/issues/385

This adds an option `ENVBUILDER_EXIT_ON_PUSH_FAILURE` that controls the previous behaviour of exiting with an error if `ENVBUILDER_PUSH_IMAGE` was set to a truthy value and we failed to push the image.

Before, Envbuilder would always exit with an error on a failed push.
Now, Envbuilder will only exit with an error if `ENVBUILDER_EXIT_ON_PUSH_FAILURE` is set to a truthy value. Otherwise, Envbuilder will continue building the image.

```
#3: Pushing image to localhost:5000/cache
#3: ⚠️️ Failed to push image! [1.835942ms]
#4: 🔄 Updating the ownership of the workspace...
#4: 👤 Updated the ownership of the workspace! [406.355µs]
#5: 🔄 Updating ownership of /home/coder...
#5: 🏡 Updated ownership of /home/coder! [56.162µs]
=== Running init command as user "coder": ["/bin/sh" "-c" "bash"]
🐳 bedd93b40e11 envbuilder-starter-devcontainer # whoami
coder
```

johnstcn: Almost done here, just need to get the test for this working properly.

Patch:
diff --git a/docs/env-variables.md b/docs/env-variables.md
index 6f9653b..faca028 100644
--- a/docs/env-variables.md
+++ b/docs/env-variables.md
@@ -18,6 +18,7 @@
 | `--docker-config-base64` | `ENVBUILDER_DOCKER_CONFIG_BASE64` |  | The base64 encoded Docker config file that will be used to pull images from private container registries. When this is set, Docker configuration set via the DOCKER_CONFIG environment variable is ignored. |
 | `--fallback-image` | `ENVBUILDER_FALLBACK_IMAGE` |  | Specifies an alternative image to use when neither an image is declared in the devcontainer.json file nor a Dockerfile is present. If there's a build failure (from a faulty Dockerfile) or a misconfiguration, this image will be the substitute. Set ExitOnBuildFailure to true to halt the container if the build faces an issue. |
 | `--exit-on-build-failure` | `ENVBUILDER_EXIT_ON_BUILD_FAILURE` |  | Terminates the container upon a build failure. This is handy when preferring the FALLBACK_IMAGE in cases where no devcontainer.json or image is provided. However, it ensures that the container stops if the build process encounters an error. |
+| `--exit-on-push-failure` | `ENVBUILDER_EXIT_ON_PUSH_FAILURE` |  | ExitOnPushFailure terminates the container upon a push failure. This is useful if failure to push the built image should abort execution and result in an error. |
 | `--force-safe` | `ENVBUILDER_FORCE_SAFE` |  | Ignores any filesystem safety checks. This could cause serious harm to your system! This is used in cases where bypass is needed to unblock customers. |
 | `--insecure` | `ENVBUILDER_INSECURE` |  | Bypass TLS verification when cloning and pulling from container registries. |
 | `--ignore-paths` | `ENVBUILDER_IGNORE_PATHS` |  | The comma separated list of paths to ignore when building the workspace. |
diff --git a/envbuilder.go b/envbuilder.go
index 324a84c..e6f9c8d 100644
--- a/envbuilder.go
+++ b/envbuilder.go
@@ -583,10 +583,13 @@ func run(ctx context.Context, opts options.Options, execArgs *execArgsInfo) erro
 			endStage("🏗️ Built image!")
 			if opts.PushImage {
 				endStage = startStage("🏗️ Pushing image...")
-				if err := executor.DoPush(image, kOpts); err != nil {
+				if err := executor.DoPush(image, kOpts); err == nil {
+					endStage("🏗️ Pushed image!")
+				} else if !opts.ExitOnPushFailure {
+					endStage("⚠️️ Failed to push image!")
+				} else {
 					return nil, xerrors.Errorf("do push: %w", err)
 				}
-				endStage("🏗️ Pushed image!")
 			}
 
 			return image, err
diff --git a/options/options.go b/options/options.go
index 01ed510..ba26d1d 100644
--- a/options/options.go
+++ b/options/options.go
@@ -78,6 +78,10 @@ type Options struct {
 	// devcontainer.json or image is provided. However, it ensures that the
 	// container stops if the build process encounters an error.
 	ExitOnBuildFailure bool
+	// ExitOnPushFailure terminates the container upon a push failure. This is
+	// useful if failure to push the built image should abort execution
+	// and result in an error.
+	ExitOnPushFailure bool
 	// ForceSafe ignores any filesystem safety checks. This could cause serious
 	// harm to your system! This is used in cases where bypass is needed to
 	// unblock customers.
@@ -304,6 +308,14 @@ func (o *Options) CLI() serpent.OptionSet {
 				"no devcontainer.json or image is provided. However, it ensures " +
 				"that the container stops if the build process encounters an error.",
 		},
+		{
+			Flag:  "exit-on-push-failure",
+			Env:   WithEnvPrefix("EXIT_ON_PUSH_FAILURE"),
+			Value: serpent.BoolOf(&o.ExitOnPushFailure),
+			Description: "ExitOnPushFailure terminates the container upon a push failure. " +
+				"This is useful if failure to push the built image should abort execution " +
+				"and result in an error.",
+		},
 		{
 			Flag:  "force-safe",
 			Env:   WithEnvPrefix("FORCE_SAFE"),


Test patch:
diff --git a/integration/integration_test.go b/integration/integration_test.go
index 204ff90..75972ae 100644
--- a/integration/integration_test.go
+++ b/integration/integration_test.go
@@ -1949,9 +1949,10 @@ RUN date --utc > /root/date.txt`, testImageAlpine),
 		_, err = remote.Image(ref, remoteAuthOpt)
 		require.ErrorContains(t, err, "NAME_UNKNOWN", "expected image to not be present before build + push")
 
-		// When: we run envbuilder with PUSH_IMAGE set
+		// When: we run envbuilder with PUSH_IMAGE and EXIT_ON_PUSH_FAILURE set
 		_, err = runEnvbuilder(t, runOpts{env: append(opts,
 			envbuilderEnv("PUSH_IMAGE", "1"),
+			envbuilderEnv("EXIT_ON_PUSH_FAILURE", "1"),
 		)})
 		// Then: it should fail with an Unauthorized error
 		require.ErrorContains(t, err, "401 Unauthorized", "expected unauthorized error using no auth when cache repo requires it")
@@ -2144,7 +2145,7 @@ RUN date --utc > /root/date.txt`, testImageAlpine),
 		require.ErrorContains(t, err, "--cache-repo must be set when using --push-image")
 	})
 
-	t.Run("PushErr", func(t *testing.T) {
+	t.Run("PushErr/ExitOnPushFail", func(t *testing.T) {
 		t.Parallel()
 
 		srv := gittest.CreateGitServer(t, gittest.Options{
@@ -2174,12 +2175,50 @@ RUN date --utc > /root/date.txt`, testImageAlpine),
 			envbuilderEnv("GIT_URL", srv.URL),
 			envbuilderEnv("CACHE_REPO", notRegURL),
 			envbuilderEnv("PUSH_IMAGE", "1"),
+			envbuilderEnv("EXIT_ON_PUSH_FAILURE", "1"),
 		}})
 
 		// Then: envbuilder should fail with a descriptive error
 		require.ErrorContains(t, err, "failed to push to destination")
 	})
 
+	t.Run("PushErr/NoExitOnPushFail", func(t *testing.T) {
+		t.Parallel()
+
+		srv := gittest.CreateGitServer(t, gittest.Options{
+			Files: map[string]string{
+				".devcontainer/Dockerfile": fmt.Sprintf(`FROM %s
+USER root
+ARG WORKDIR=/
+WORKDIR $WORKDIR
+ENV FOO=bar
+RUN echo $FOO > /root/foo.txt
+RUN date --utc > /root/date.txt`, testImageAlpine),
+				".devcontainer/devcontainer.json": `{
+				"name": "Test",
+				"build": {
+					"dockerfile": "Dockerfile"
+				},
+			}`,
+			},
+		})
+
+		// Given: registry is not set up (in this case, not a registry)
+		notRegSrv := httptest.NewServer(http.NotFoundHandler())
+		notRegURL := strings.TrimPrefix(notRegSrv.URL, "http://") + "/test"
+
+		// When: we run envbuilder with PUSH_IMAGE set
+		_, err := runEnvbuilder(t, runOpts{env: []string{
+			envbuilderEnv("GIT_URL", srv.URL),
+			envbuilderEnv("CACHE_REPO", notRegURL),
+			envbuilderEnv("PUSH_IMAGE", "1"),
+			envbuilderEnv("EXIT_ON_PUSH_FAILURE", "0"),
+		}})
+
+		// Then: envbuilder should not fail
+		require.NoError(t, err)
+	})
+
 	t.Run("CacheAndPushDevcontainerFeatures", func(t *testing.T) {
 		t.Parallel()
 
@@ -2421,8 +2460,13 @@ func pushImage(t *testing.T, ref name.Reference, remoteOpt remote.Option, env ..
 	if remoteOpt != nil {
 		remoteOpts = append(remoteOpts, remoteOpt)
 	}
-
-	_, err := runEnvbuilder(t, runOpts{env: append(env, envbuilderEnv("PUSH_IMAGE", "1"))})
+	opts := runOpts{
+		env: append(env,
+			envbuilderEnv("PUSH_IMAGE", "1"),
+			envbuilderEnv("EXIT_ON_PUSH_FAILURE", "1"),
+		),
+	}
+	_, err := runEnvbuilder(t, opts)
 	require.NoError(t, err, "envbuilder push image failed")
 
 	img, err := remote.Image(ref, remoteOpts...)
diff --git a/options/testdata/options.golden b/options/testdata/options.golden
index 840b234..211305f 100644
--- a/options/testdata/options.golden
+++ b/options/testdata/options.golden
@@ -66,6 +66,11 @@ OPTIONS:
           image is provided. However, it ensures that the container stops if the
           build process encounters an error.
 
+      --exit-on-push-failure bool, $ENVBUILDER_EXIT_ON_PUSH_FAILURE
+          ExitOnPushFailure terminates the container upon a push failure. This
+          is useful if failure to push the built image should abort execution
+          and result in an error.
+
       --export-env-file string, $ENVBUILDER_EXPORT_ENV_FILE
           Optional file path to a .env file where envbuilder will dump
           environment variables from devcontainer.json and the built container