Skip to content

Commit

Permalink
Support build args without explicit value
Browse files Browse the repository at this point in the history
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]: moby/moby#24101

Signed-off-by: Caleb Xu <calebcenter@live.com>
  • Loading branch information
alebcay committed Oct 6, 2022
1 parent 86f7cf4 commit 0e41485
Show file tree
Hide file tree
Showing 3 changed files with 79 additions and 13 deletions.
38 changes: 26 additions & 12 deletions cmd/nerdctl/build.go
Original file line number Diff line number Diff line change
Expand Up @@ -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, "--opt=build-arg:"+ba+"="+val)
} else {
logrus.WithError(err).Warnf("invalid BUILDKIT_INLINE_CACHE: %q", bic)
logrus.Warnf("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 {
logrus.Warnf("ignoring invalid build arg %q", ba)
}
}

Expand Down
41 changes: 41 additions & 0 deletions cmd/nerdctl/build_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -215,6 +215,47 @@ 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
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")

base.Cmd("build", buildCtx, "-t", imageName, "--build-arg", "TEST_STRING=2").AssertOK()
base.Cmd("run", "--rm", imageName).AssertOutExactly("2\n")

base.Cmd("build", buildCtx, "-t", imageName, "--build-arg", "TEST_STRING=").AssertOK()
base.Cmd("run", "--rm", imageName).AssertOutExactly("\n")

base.Cmd("build", buildCtx, "-t", imageName, "--build-arg", "TEST_STRING").AssertOK()
base.Cmd("run", "--rm", imageName).AssertOutExactly("1\n")

err = os.Setenv("TEST_STRING", "3")
assert.NilError(t, err)
base.Cmd("build", buildCtx, "-t", imageName, "--build-arg", "TEST_STRING").AssertOK()
base.Cmd("run", "--rm", imageName).AssertOutExactly("3\n")

err = os.Setenv("TEST_STRING", "")
assert.NilError(t, err)
base.Cmd("build", buildCtx, "-t", imageName, "--build-arg", "TEST_STRING").AssertOK()
base.Cmd("run", "--rm", imageName).AssertOutExactly("\n")
err = os.Unsetenv("TEST_STRING")
assert.NilError(t, err)
}

func TestBuildWithIIDFile(t *testing.T) {
t.Parallel()
testutil.RequiresBuild(t)
Expand Down
13 changes: 12 additions & 1 deletion cmd/nerdctl/builder.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ import (
"fmt"
"os"
"os/exec"
"strings"

"github.com/containerd/nerdctl/pkg/buildkitutil"
"github.com/containerd/nerdctl/pkg/defaults"
Expand Down Expand Up @@ -128,7 +129,17 @@ 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, "--build-arg="+v+"="+val)
}
} else if len(arr) > 1 && len(arr[0]) > 0 {
buildgArgs = append(buildgArgs, "--build-arg="+v)
}
}
}

Expand Down

0 comments on commit 0e41485

Please sign in to comment.