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 c20c6fe
Show file tree
Hide file tree
Showing 3 changed files with 86 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, fmt.Sprintf("--opt=build-arg:%s=%s", 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
48 changes: 48 additions & 0 deletions cmd/nerdctl/build_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -215,6 +215,54 @@ 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)
})
}
}

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, fmt.Sprintf("--build-arg=%s=%s", v, val))
}
} else if len(arr) > 1 && len(arr[0]) > 0 {
buildgArgs = append(buildgArgs, "--build-arg="+v)
}
}
}

Expand Down

0 comments on commit c20c6fe

Please sign in to comment.