From a9dda5285da5c72fcd6baea6ecac66bde85cf874 Mon Sep 17 00:00:00 2001 From: Caleb Xu Date: Thu, 6 Oct 2022 09:28:49 -0400 Subject: [PATCH] Support build args without explicit value When a build arg key is specified without an explicit value, the value should be taken from the corresponding environment variable on the host [1]. When the corresponding environment variable on the host is unset, the build arg should be ignored [2] to avoid masking a possible default value specified in the Dockerfile [3]. To pass an empty string as build arg, use the form "VAR=" on the CLI/as an environment variable. [1]: https://docs.docker.com/engine/reference/commandline/build/#set-build-time-variables---build-arg [2]: https://docs.docker.com/engine/reference/commandline/run/#set-environment-variables--e---env---env-file [3]: https://github.com/moby/moby/issues/24101 Signed-off-by: Caleb Xu --- cmd/nerdctl/build.go | 38 +++++++++++++++++++--------- cmd/nerdctl/build_test.go | 52 +++++++++++++++++++++++++++++++++++++++ cmd/nerdctl/builder.go | 15 ++++++++++- 3 files changed, 92 insertions(+), 13 deletions(-) diff --git a/cmd/nerdctl/build.go b/cmd/nerdctl/build.go index ab21f8115cc..42fddfd1ae6 100644 --- a/cmd/nerdctl/build.go +++ b/cmd/nerdctl/build.go @@ -382,20 +382,34 @@ func generateBuildctlArgs(cmd *cobra.Command, buildkitHost string, platform, arg return "", nil, false, "", nil, cleanup, err } for _, ba := range strutil.DedupeStrSlice(buildArgsValue) { - buildctlArgs = append(buildctlArgs, "--opt=build-arg:"+ba) - - // Support `--build-arg BUILDKIT_INLINE_CACHE=1` for compatibility with `docker buildx build` - // https://github.com/docker/buildx/blob/v0.6.3/docs/reference/buildx_build.md#-export-build-cache-to-an-external-cache-destination---cache-to - if strings.HasPrefix(ba, "BUILDKIT_INLINE_CACHE=") { - bic := strings.TrimPrefix(ba, "BUILDKIT_INLINE_CACHE=") - bicParsed, err := strconv.ParseBool(bic) - if err == nil { - if bicParsed { - buildctlArgs = append(buildctlArgs, "--export-cache=type=inline") - } + arr := strings.Split(ba, "=") + if len(arr) == 1 && len(arr[0]) > 0 { + // Avoid masking default build arg value from Dockerfile if environment variable is not set + // https://github.com/moby/moby/issues/24101 + val, ok := os.LookupEnv(arr[0]) + if ok { + buildctlArgs = append(buildctlArgs, fmt.Sprintf("--opt=build-arg:%s=%s", ba, val)) } else { - logrus.WithError(err).Warnf("invalid BUILDKIT_INLINE_CACHE: %q", bic) + logrus.Debugf("ignoring unset build arg %q", ba) + } + } else if len(arr) > 1 && len(arr[0]) > 0 { + buildctlArgs = append(buildctlArgs, "--opt=build-arg:"+ba) + + // Support `--build-arg BUILDKIT_INLINE_CACHE=1` for compatibility with `docker buildx build` + // https://github.com/docker/buildx/blob/v0.6.3/docs/reference/buildx_build.md#-export-build-cache-to-an-external-cache-destination---cache-to + if strings.HasPrefix(ba, "BUILDKIT_INLINE_CACHE=") { + bic := strings.TrimPrefix(ba, "BUILDKIT_INLINE_CACHE=") + bicParsed, err := strconv.ParseBool(bic) + if err == nil { + if bicParsed { + buildctlArgs = append(buildctlArgs, "--export-cache=type=inline") + } + } else { + logrus.WithError(err).Warnf("invalid BUILDKIT_INLINE_CACHE: %q", bic) + } } + } else { + return "", nil, false, "", nil, nil, fmt.Errorf("invalid build arg %q", ba) } } diff --git a/cmd/nerdctl/build_test.go b/cmd/nerdctl/build_test.go index f2d330f434a..25092f287b1 100644 --- a/cmd/nerdctl/build_test.go +++ b/cmd/nerdctl/build_test.go @@ -215,6 +215,58 @@ func createBuildContext(dockerfile string) (string, error) { return tmpDir, nil } +func TestBuildWithBuildArg(t *testing.T) { + testutil.RequiresBuild(t) + base := testutil.NewBase(t) + defer base.Cmd("builder", "prune").Run() + imageName := testutil.Identifier(t) + defer base.Cmd("rmi", imageName).Run() + + dockerfile := fmt.Sprintf(`FROM %s +ARG TEST_STRING=1 +ENV TEST_STRING=$TEST_STRING +CMD echo $TEST_STRING + `, testutil.CommonImage) + + buildCtx, err := createBuildContext(dockerfile) + assert.NilError(t, err) + defer os.RemoveAll(buildCtx) + + base.Cmd("build", buildCtx, "-t", imageName).AssertOK() + base.Cmd("run", "--rm", imageName).AssertOutExactly("1\n") + + validCases := []struct { + name string + arg string + envValue string + envSet bool + expected string + }{ + {"ArgValueOverridesDefault", "TEST_STRING=2", "", false, "2\n"}, + {"EmptyArgValueOverridesDefault", "TEST_STRING=", "", false, "\n"}, + {"UnsetArgKeyPreservesDefault", "TEST_STRING", "", false, "1\n"}, + {"EnvValueOverridesDefault", "TEST_STRING", "3", true, "3\n"}, + {"EmptyEnvValueOverridesDefault", "TEST_STRING", "", true, "\n"}, + } + + for _, tc := range validCases { + t.Run(tc.name, func(t *testing.T) { + if tc.envSet { + err := os.Setenv("TEST_STRING", tc.envValue) + assert.NilError(t, err) + defer os.Unsetenv("TEST_STRING") + } + + base.Cmd("build", buildCtx, "-t", imageName, "--build-arg", tc.arg).AssertOK() + base.Cmd("run", "--rm", imageName).AssertOutExactly(tc.expected) + }) + } + + t.Run("InvalidBuildArgCausesError", func(t *testing.T) { + base.Cmd("build", buildCtx, "-t", imageName, "--build-arg", "=TEST_STRING").AssertFail() + }) +} + func TestBuildWithIIDFile(t *testing.T) { t.Parallel() testutil.RequiresBuild(t) diff --git a/cmd/nerdctl/builder.go b/cmd/nerdctl/builder.go index 6c2c618149f..ba9d265ef8b 100644 --- a/cmd/nerdctl/builder.go +++ b/cmd/nerdctl/builder.go @@ -20,6 +20,7 @@ import ( "fmt" "os" "os/exec" + "strings" "github.com/containerd/nerdctl/pkg/buildkitutil" "github.com/containerd/nerdctl/pkg/defaults" @@ -128,7 +129,19 @@ func builderDebugAction(cmd *cobra.Command, args []string) error { return err } else if len(buildArgsValue) > 0 { for _, v := range buildArgsValue { - buildgArgs = append(buildgArgs, "--build-arg="+v) + arr := strings.Split(v, "=") + if len(arr) == 1 && len(arr[0]) > 0 { + // Avoid masking default build arg value from Dockerfile if environment variable is not set + // https://github.com/moby/moby/issues/24101 + val, ok := os.LookupEnv(arr[0]) + if ok { + buildgArgs = append(buildgArgs, fmt.Sprintf("--build-arg=%s=%s", v, val)) + } + } else if len(arr) > 1 && len(arr[0]) > 0 { + buildgArgs = append(buildgArgs, "--build-arg="+v) + } else { + return fmt.Errorf("invalid build arg %q", v) + } } }