From 5d24f0a5875d3d5d498a582e0f401e4323966809 Mon Sep 17 00:00:00 2001 From: Cian Johnston Date: Tue, 5 Nov 2024 12:50:24 +0000 Subject: [PATCH 1/2] 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 b18cd0e8308bd83e6b1453ebb46f64d64c435f85) --- docs/env-variables.md | 1 + envbuilder.go | 7 +++-- integration/integration_test.go | 52 ++++++++++++++++++++++++++++++--- options/options.go | 12 ++++++++ options/testdata/options.golden | 5 ++++ 5 files changed, 71 insertions(+), 6 deletions(-) diff --git a/docs/env-variables.md b/docs/env-variables.md index cb7054f3..c198c705 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 028a7bb2..e99637f7 100644 --- a/envbuilder.go +++ b/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 diff --git a/integration/integration_test.go b/integration/integration_test.go index 46e0e88c..8d96a071 100644 --- a/integration/integration_test.go +++ b/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...) diff --git a/options/options.go b/options/options.go index ebda2eeb..b2497d3c 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. @@ -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"), diff --git a/options/testdata/options.golden b/options/testdata/options.golden index 397d5e0b..f5c5729d 100644 --- a/options/testdata/options.golden +++ b/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 From dfa8129afa688db9928bd8ef2cce45a405f0b30e Mon Sep 17 00:00:00 2001 From: Mathias Fredriksson Date: Thu, 7 Nov 2024 11:13:34 +0200 Subject: [PATCH 2/2] feat: allow changing default workspaces folder (#406) Fixes #384 (cherry picked from commit 24ef801bcf2f47c25e76daf13b3707b653d23270) --- docs/env-variables.md | 3 +- integration/integration_test.go | 22 ++++++++++++++ options/defaults.go | 23 ++++++++------- options/defaults_test.go | 52 +++++++++++++++++++++++++-------- options/options.go | 19 ++++++++++-- options/testdata/options.golden | 6 ++++ 6 files changed, 99 insertions(+), 26 deletions(-) diff --git a/docs/env-variables.md b/docs/env-variables.md index c198c705..427a3f63 100644 --- a/docs/env-variables.md +++ b/docs/env-variables.md @@ -31,7 +31,8 @@ | `--git-ssh-private-key-path` | `ENVBUILDER_GIT_SSH_PRIVATE_KEY_PATH` | | Path to an SSH private key to be used for Git authentication. If this is set, then GIT_SSH_PRIVATE_KEY_BASE64 cannot be set. | | `--git-ssh-private-key-base64` | `ENVBUILDER_GIT_SSH_PRIVATE_KEY_BASE64` | | Base64 encoded SSH private key to be used for Git authentication. If this is set, then GIT_SSH_PRIVATE_KEY_PATH cannot be set. | | `--git-http-proxy-url` | `ENVBUILDER_GIT_HTTP_PROXY_URL` | | The URL for the HTTP proxy. This is optional. | -| `--workspace-folder` | `ENVBUILDER_WORKSPACE_FOLDER` | | The path to the workspace folder that will be built. This is optional. | +| `--workspace-base-dir` | `ENVBUILDER_WORKSPACE_BASE_DIR` | `/workspaces` | The path under which workspaces will be placed when workspace folder option is not given. | +| `--workspace-folder` | `ENVBUILDER_WORKSPACE_FOLDER` | | The path to the workspace folder that will be built. This is optional. Defaults to `[workspace base dir]/[name]` where name is the name of the repository or `empty`. | | `--ssl-cert-base64` | `ENVBUILDER_SSL_CERT_BASE64` | | The content of an SSL cert file. This is useful for self-signed certificates. | | `--export-env-file` | `ENVBUILDER_EXPORT_ENV_FILE` | | Optional file path to a .env file where envbuilder will dump environment variables from devcontainer.json and the built container image. | | `--post-start-script-path` | `ENVBUILDER_POST_START_SCRIPT_PATH` | | The path to a script that will be created by envbuilder based on the postStartCommand in devcontainer.json, if any is specified (otherwise the script is not created). If this is set, the specified InitCommand should check for the presence of this script and execute it after successful startup. | diff --git a/integration/integration_test.go b/integration/integration_test.go index 8d96a071..09db0ce3 100644 --- a/integration/integration_test.go +++ b/integration/integration_test.go @@ -818,6 +818,28 @@ func TestBuildFromDevcontainerInCustomPath(t *testing.T) { require.Equal(t, "hello", strings.TrimSpace(output)) } +func TestBuildFromCustomWorkspaceBaseDir(t *testing.T) { + t.Parallel() + + // Ensures that a Git repository with a devcontainer.json is cloned and built. + srv := gittest.CreateGitServer(t, gittest.Options{ + Files: map[string]string{ + "Dockerfile": "FROM " + testImageUbuntu, + }, + }) + ctr, err := runEnvbuilder(t, runOpts{ + env: []string{ + envbuilderEnv("DOCKERFILE_PATH", "Dockerfile"), + envbuilderEnv("WORKSPACE_BASE_DIR", "/foo"), + envbuilderEnv("GIT_URL", srv.URL), + }, + }) + require.NoError(t, err) + + output := execContainer(t, ctr, "readlink /proc/1/cwd") + require.Contains(t, output, "/foo/") +} + func TestBuildFromDevcontainerInSubfolder(t *testing.T) { t.Parallel() diff --git a/options/defaults.go b/options/defaults.go index b9e47a80..a6fd145b 100644 --- a/options/defaults.go +++ b/options/defaults.go @@ -12,29 +12,29 @@ import ( "github.com/coder/envbuilder/internal/workingdir" ) -// EmptyWorkspaceDir is the path to a workspace that has -// nothing going on... it's empty! -var EmptyWorkspaceDir = "/workspaces/empty" - // DefaultWorkspaceFolder returns the default workspace folder // for a given repository URL. -func DefaultWorkspaceFolder(repoURL string) string { +func DefaultWorkspaceFolder(workspacesFolder, repoURL string) string { + // emptyWorkspaceDir is the path to a workspace that has + // nothing going on... it's empty! + emptyWorkspaceDir := workspacesFolder + "/empty" + if repoURL == "" { - return EmptyWorkspaceDir + return emptyWorkspaceDir } parsed, err := giturls.Parse(repoURL) if err != nil { - return EmptyWorkspaceDir + return emptyWorkspaceDir } repo := path.Base(parsed.Path) // Giturls parsing never actually fails since ParseLocal never // errors and places the entire URL in the Path field. This check // ensures it's at least a Unix path containing forwardslash. if repo == repoURL || repo == "/" || repo == "." || repo == "" { - return EmptyWorkspaceDir + return emptyWorkspaceDir } repo = strings.TrimSuffix(repo, ".git") - return fmt.Sprintf("/workspaces/%s", repo) + return fmt.Sprintf("%s/%s", workspacesFolder, repo) } func (o *Options) SetDefaults() { @@ -59,8 +59,11 @@ func (o *Options) SetDefaults() { if o.Filesystem == nil { o.Filesystem = chmodfs.New(osfs.New("/")) } + if o.WorkspaceBaseDir == "" { + o.WorkspaceBaseDir = "/workspaces" + } if o.WorkspaceFolder == "" { - o.WorkspaceFolder = DefaultWorkspaceFolder(o.GitURL) + o.WorkspaceFolder = DefaultWorkspaceFolder(o.WorkspaceBaseDir, o.GitURL) } if o.BinaryPath == "" { o.BinaryPath = "/.envbuilder/bin/envbuilder" diff --git a/options/defaults_test.go b/options/defaults_test.go index de57365d..3efd8f25 100644 --- a/options/defaults_test.go +++ b/options/defaults_test.go @@ -17,83 +17,110 @@ func TestDefaultWorkspaceFolder(t *testing.T) { successTests := []struct { name string + baseDir string gitURL string expected string }{ { name: "HTTP", + baseDir: "/workspaces", gitURL: "https://github.com/coder/envbuilder.git", expected: "/workspaces/envbuilder", }, { name: "SSH", + baseDir: "/workspaces", gitURL: "git@github.com:coder/envbuilder.git", expected: "/workspaces/envbuilder", }, { name: "username and password", + baseDir: "/workspaces", gitURL: "https://username:password@github.com/coder/envbuilder.git", expected: "/workspaces/envbuilder", }, { name: "trailing", + baseDir: "/workspaces", gitURL: "https://github.com/coder/envbuilder.git/", expected: "/workspaces/envbuilder", }, { name: "trailing-x2", + baseDir: "/workspaces", gitURL: "https://github.com/coder/envbuilder.git//", expected: "/workspaces/envbuilder", }, { name: "no .git", + baseDir: "/workspaces", gitURL: "https://github.com/coder/envbuilder", expected: "/workspaces/envbuilder", }, { name: "trailing no .git", + baseDir: "/workspaces", gitURL: "https://github.com/coder/envbuilder/", expected: "/workspaces/envbuilder", }, { name: "fragment", + baseDir: "/workspaces", gitURL: "https://github.com/coder/envbuilder.git#feature-branch", expected: "/workspaces/envbuilder", }, { name: "fragment-trailing", + baseDir: "/workspaces", gitURL: "https://github.com/coder/envbuilder.git/#refs/heads/feature-branch", expected: "/workspaces/envbuilder", }, { name: "fragment-trailing no .git", + baseDir: "/workspaces", gitURL: "https://github.com/coder/envbuilder/#refs/heads/feature-branch", expected: "/workspaces/envbuilder", }, { name: "space", + baseDir: "/workspaces", gitURL: "https://github.com/coder/env%20builder.git", expected: "/workspaces/env builder", }, { name: "Unix path", + baseDir: "/workspaces", gitURL: "/repo", expected: "/workspaces/repo", }, { name: "Unix subpath", + baseDir: "/workspaces", gitURL: "/path/to/repo", expected: "/workspaces/repo", }, { name: "empty", + baseDir: "/workspaces", gitURL: "", - expected: options.EmptyWorkspaceDir, + expected: "/workspaces/empty", + }, + { + name: "non default workspaces folder", + baseDir: "/foo", + gitURL: "https://github.com/coder/envbuilder.git", + expected: "/foo/envbuilder", + }, + { + name: "non default workspaces folder empty git URL", + baseDir: "/foo", + gitURL: "", + expected: "/foo/empty", }, } for _, tt := range successTests { t.Run(tt.name, func(t *testing.T) { - dir := options.DefaultWorkspaceFolder(tt.gitURL) + dir := options.DefaultWorkspaceFolder(tt.baseDir, tt.gitURL) require.Equal(t, tt.expected, dir) }) } @@ -125,8 +152,8 @@ func TestDefaultWorkspaceFolder(t *testing.T) { } for _, tt := range invalidTests { t.Run(tt.name, func(t *testing.T) { - dir := options.DefaultWorkspaceFolder(tt.invalidURL) - require.Equal(t, options.EmptyWorkspaceDir, dir) + dir := options.DefaultWorkspaceFolder("/workspaces", tt.invalidURL) + require.Equal(t, "/workspaces/empty", dir) }) } } @@ -135,14 +162,15 @@ func TestOptions_SetDefaults(t *testing.T) { t.Parallel() expected := options.Options{ - InitScript: "sleep infinity", - InitCommand: "/bin/sh", - IgnorePaths: []string{"/var/run", "/product_uuid", "/product_name"}, - Filesystem: chmodfs.New(osfs.New("/")), - GitURL: "", - WorkspaceFolder: options.EmptyWorkspaceDir, - MagicDirBase: "/.envbuilder", - BinaryPath: "/.envbuilder/bin/envbuilder", + InitScript: "sleep infinity", + InitCommand: "/bin/sh", + IgnorePaths: []string{"/var/run", "/product_uuid", "/product_name"}, + Filesystem: chmodfs.New(osfs.New("/")), + GitURL: "", + WorkspaceBaseDir: "/workspaces", + WorkspaceFolder: "/workspaces/empty", + MagicDirBase: "/.envbuilder", + BinaryPath: "/.envbuilder/bin/envbuilder", } var actual options.Options diff --git a/options/options.go b/options/options.go index b2497d3c..9b9edda2 100644 --- a/options/options.go +++ b/options/options.go @@ -117,8 +117,12 @@ type Options struct { GitSSHPrivateKeyBase64 string // GitHTTPProxyURL is the URL for the HTTP proxy. This is optional. GitHTTPProxyURL string + // WorkspaceBaseDir is the path under which workspaces will be placed when + // workspace folder option is not given. + WorkspaceBaseDir string // WorkspaceFolder is the path to the workspace folder that will be built. - // This is optional. + // This is optional. Defaults to `[workspace base dir]/[name]` where name is + // the name of the repository or "empty". WorkspaceFolder string // SSLCertBase64 is the content of an SSL cert file. This is useful for // self-signed certificates. @@ -394,12 +398,21 @@ func (o *Options) CLI() serpent.OptionSet { Value: serpent.StringOf(&o.GitHTTPProxyURL), Description: "The URL for the HTTP proxy. This is optional.", }, + { + Flag: "workspace-base-dir", + Env: WithEnvPrefix("WORKSPACE_BASE_DIR"), + Value: serpent.StringOf(&o.WorkspaceBaseDir), + Default: "/workspaces", + Description: "The path under which workspaces will be placed when " + + "workspace folder option is not given.", + }, { Flag: "workspace-folder", Env: WithEnvPrefix("WORKSPACE_FOLDER"), Value: serpent.StringOf(&o.WorkspaceFolder), - Description: "The path to the workspace folder that will " + - "be built. This is optional.", + Description: "The path to the workspace folder that will be built. " + + "This is optional. Defaults to `[workspace base dir]/[name]` where " + + "name is the name of the repository or `empty`.", }, { Flag: "ssl-cert-base64", diff --git a/options/testdata/options.golden b/options/testdata/options.golden index f5c5729d..203c197b 100644 --- a/options/testdata/options.golden +++ b/options/testdata/options.golden @@ -177,6 +177,12 @@ OPTIONS: --verbose bool, $ENVBUILDER_VERBOSE Enable verbose logging. + --workspace-base-dir string, $ENVBUILDER_WORKSPACE_BASE_DIR (default: /workspaces) + The path under which workspaces will be placed when workspace folder + option is not given. + --workspace-folder string, $ENVBUILDER_WORKSPACE_FOLDER The path to the workspace folder that will be built. This is optional. + Defaults to `[workspace base dir]/[name]` where name is the name of + the repository or `empty`.