fix: do not error if failed to push image (#390)

Relates to #385

This commit 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 and executing ENVBUILDER_INIT_SCRIPT even if push fails.

(cherry picked from commit b18cd0e)

Author: johnstcn

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. |

envbuilder.go

@@ -582,10 +582,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

integration/integration_test.go

@@ -1879,9 +1879,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")
@@ -2074,7 +2075,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{
@@ -2104,12 +2105,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()
 
@@ -2351,8 +2390,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...)

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.
@@ -301,6 +305,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"),

options/testdata/options.golden

@@ -62,6 +62,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