Skip to content

Commit

Permalink
fix: do not error if failed to push image (#390)
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
johnstcn authored Nov 5, 2024
1 parent c16ae9f commit b18cd0e
Show file tree
Hide file tree
Showing 5 changed files with 71 additions and 6 deletions.
1 change: 1 addition & 0 deletions docs/env-variables.md
Original file line number Diff line number Diff line change
Expand Up @@ -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. |
Expand Down
7 changes: 5 additions & 2 deletions envbuilder.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
52 changes: 48 additions & 4 deletions integration/integration_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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")
Expand Down Expand Up @@ -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{
Expand Down Expand Up @@ -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()

Expand Down Expand Up @@ -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...)
Expand Down
12 changes: 12 additions & 0 deletions options/options.go
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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"),
Expand Down
5 changes: 5 additions & 0 deletions options/testdata/options.golden
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down

0 comments on commit b18cd0e

Please sign in to comment.