diff --git a/builder/dockerfile/buildargs.go b/builder/dockerfile/buildargs.go index 93794044e9..232f9d23f6 100644 --- a/builder/dockerfile/buildargs.go +++ b/builder/dockerfile/buildargs.go @@ -21,8 +21,8 @@ var builtinAllowedBuildArgs = map[string]bool{ "no_proxy": true, } -// buildArgs manages arguments used by the builder -type buildArgs struct { +// BuildArgs manages arguments used by the builder +type BuildArgs struct { // args that are allowed for expansion/substitution and passing to commands in 'run'. allowedBuildArgs map[string]*string // args defined before the first `FROM` in a Dockerfile @@ -33,8 +33,9 @@ type buildArgs struct { argsFromOptions map[string]*string } -func newBuildArgs(argsFromOptions map[string]*string) *buildArgs { - return &buildArgs{ +// NewBuildArgs creates a new BuildArgs type +func NewBuildArgs(argsFromOptions map[string]*string) *BuildArgs { + return &BuildArgs{ allowedBuildArgs: make(map[string]*string), allowedMetaArgs: make(map[string]*string), referencedArgs: make(map[string]struct{}), @@ -42,8 +43,9 @@ func newBuildArgs(argsFromOptions map[string]*string) *buildArgs { } } -func (b *buildArgs) Clone() *buildArgs { - result := newBuildArgs(b.argsFromOptions) +// Clone returns a copy of the BuildArgs type +func (b *BuildArgs) Clone() *BuildArgs { + result := NewBuildArgs(b.argsFromOptions) for k, v := range b.allowedBuildArgs { result.allowedBuildArgs[k] = v } @@ -56,7 +58,9 @@ func (b *buildArgs) Clone() *buildArgs { return result } -func (b *buildArgs) MergeReferencedArgs(other *buildArgs) { +// MergeReferencedArgs merges referenced args from another BuildArgs +// object into the current one +func (b *BuildArgs) MergeReferencedArgs(other *BuildArgs) { for k := range other.referencedArgs { b.referencedArgs[k] = struct{}{} } @@ -64,7 +68,7 @@ func (b *buildArgs) MergeReferencedArgs(other *buildArgs) { // WarnOnUnusedBuildArgs checks if there are any leftover build-args that were // passed but not consumed during build. Print a warning, if there are any. -func (b *buildArgs) WarnOnUnusedBuildArgs(out io.Writer) { +func (b *BuildArgs) WarnOnUnusedBuildArgs(out io.Writer) { leftoverArgs := []string{} for arg := range b.argsFromOptions { _, isReferenced := b.referencedArgs[arg] @@ -80,17 +84,17 @@ func (b *buildArgs) WarnOnUnusedBuildArgs(out io.Writer) { // ResetAllowed clears the list of args that are allowed to be used by a // directive -func (b *buildArgs) ResetAllowed() { +func (b *BuildArgs) ResetAllowed() { b.allowedBuildArgs = make(map[string]*string) } // AddMetaArg adds a new meta arg that can be used by FROM directives -func (b *buildArgs) AddMetaArg(key string, value *string) { +func (b *BuildArgs) AddMetaArg(key string, value *string) { b.allowedMetaArgs[key] = value } // AddArg adds a new arg that can be used by directives -func (b *buildArgs) AddArg(key string, value *string) { +func (b *BuildArgs) AddArg(key string, value *string) { b.allowedBuildArgs[key] = value b.referencedArgs[key] = struct{}{} } @@ -98,23 +102,23 @@ func (b *buildArgs) AddArg(key string, value *string) { // IsReferencedOrNotBuiltin checks if the key is a built-in arg, or if it has been // referenced by the Dockerfile. Returns true if the arg is not a builtin or // if the builtin has been referenced in the Dockerfile. -func (b *buildArgs) IsReferencedOrNotBuiltin(key string) bool { +func (b *BuildArgs) IsReferencedOrNotBuiltin(key string) bool { _, isBuiltin := builtinAllowedBuildArgs[key] _, isAllowed := b.allowedBuildArgs[key] return isAllowed || !isBuiltin } // GetAllAllowed returns a mapping with all the allowed args -func (b *buildArgs) GetAllAllowed() map[string]string { +func (b *BuildArgs) GetAllAllowed() map[string]string { return b.getAllFromMapping(b.allowedBuildArgs) } // GetAllMeta returns a mapping with all the meta meta args -func (b *buildArgs) GetAllMeta() map[string]string { +func (b *BuildArgs) GetAllMeta() map[string]string { return b.getAllFromMapping(b.allowedMetaArgs) } -func (b *buildArgs) getAllFromMapping(source map[string]*string) map[string]string { +func (b *BuildArgs) getAllFromMapping(source map[string]*string) map[string]string { m := make(map[string]string) keys := keysFromMaps(source, builtinAllowedBuildArgs) @@ -128,7 +132,7 @@ func (b *buildArgs) getAllFromMapping(source map[string]*string) map[string]stri } // FilterAllowed returns all allowed args without the filtered args -func (b *buildArgs) FilterAllowed(filter []string) []string { +func (b *BuildArgs) FilterAllowed(filter []string) []string { envs := []string{} configEnv := opts.ConvertKVStringsToMap(filter) @@ -140,7 +144,7 @@ func (b *buildArgs) FilterAllowed(filter []string) []string { return envs } -func (b *buildArgs) getBuildArg(key string, mapping map[string]*string) (string, bool) { +func (b *BuildArgs) getBuildArg(key string, mapping map[string]*string) (string, bool) { defaultValue, exists := mapping[key] // Return override from options if one is defined if v, ok := b.argsFromOptions[key]; ok && v != nil { diff --git a/builder/dockerfile/buildargs_test.go b/builder/dockerfile/buildargs_test.go index ae00e3b650..c3f6104862 100644 --- a/builder/dockerfile/buildargs_test.go +++ b/builder/dockerfile/buildargs_test.go @@ -14,7 +14,7 @@ func strPtr(source string) *string { } func TestGetAllAllowed(t *testing.T) { - buildArgs := newBuildArgs(map[string]*string{ + buildArgs := NewBuildArgs(map[string]*string{ "ArgNotUsedInDockerfile": strPtr("fromopt1"), "ArgOverriddenByOptions": strPtr("fromopt2"), "ArgNoDefaultInDockerfileFromOptions": strPtr("fromopt3"), @@ -45,7 +45,7 @@ func TestGetAllAllowed(t *testing.T) { } func TestGetAllMeta(t *testing.T) { - buildArgs := newBuildArgs(map[string]*string{ + buildArgs := NewBuildArgs(map[string]*string{ "ArgNotUsedInDockerfile": strPtr("fromopt1"), "ArgOverriddenByOptions": strPtr("fromopt2"), "ArgNoDefaultInMetaFromOptions": strPtr("fromopt3"), @@ -67,7 +67,7 @@ func TestGetAllMeta(t *testing.T) { } func TestWarnOnUnusedBuildArgs(t *testing.T) { - buildArgs := newBuildArgs(map[string]*string{ + buildArgs := NewBuildArgs(map[string]*string{ "ThisArgIsUsed": strPtr("fromopt1"), "ThisArgIsNotUsed": strPtr("fromopt2"), "HTTPS_PROXY": strPtr("referenced builtin"), @@ -86,7 +86,7 @@ func TestWarnOnUnusedBuildArgs(t *testing.T) { } func TestIsUnreferencedBuiltin(t *testing.T) { - buildArgs := newBuildArgs(map[string]*string{ + buildArgs := NewBuildArgs(map[string]*string{ "ThisArgIsUsed": strPtr("fromopt1"), "ThisArgIsNotUsed": strPtr("fromopt2"), "HTTPS_PROXY": strPtr("referenced builtin"), diff --git a/builder/dockerfile/builder.go b/builder/dockerfile/builder.go index cd2fa7c56b..21d84cb513 100644 --- a/builder/dockerfile/builder.go +++ b/builder/dockerfile/builder.go @@ -250,7 +250,7 @@ func emitImageID(aux *streamformatter.AuxFormatter, state *dispatchState) error return aux.Emit(types.BuildResult{ID: state.imageID}) } -func processMetaArg(meta instructions.ArgCommand, shlex *shell.Lex, args *buildArgs) error { +func processMetaArg(meta instructions.ArgCommand, shlex *shell.Lex, args *BuildArgs) error { // shell.Lex currently only support the concatenated string format envs := convertMapToEnvList(args.GetAllAllowed()) if err := meta.Expand(func(word string) (string, error) { @@ -271,7 +271,7 @@ func printCommand(out io.Writer, currentCommandIndex int, totalCommands int, cmd func (b *Builder) dispatchDockerfileWithCancellation(parseResult []instructions.Stage, metaArgs []instructions.ArgCommand, escapeToken rune, source builder.Source) (*dispatchState, error) { dispatchRequest := dispatchRequest{} - buildArgs := newBuildArgs(b.options.BuildArgs) + buildArgs := NewBuildArgs(b.options.BuildArgs) totalCommands := len(metaArgs) + len(parseResult) currentCommandIndex := 1 for _, stage := range parseResult { @@ -388,7 +388,7 @@ func BuildFromConfig(config *container.Config, changes []string, os string) (*co commands = append(commands, cmd) } - dispatchRequest := newDispatchRequest(b, dockerfile.EscapeToken, nil, newBuildArgs(b.options.BuildArgs), newStagesBuildResults()) + dispatchRequest := newDispatchRequest(b, dockerfile.EscapeToken, nil, NewBuildArgs(b.options.BuildArgs), newStagesBuildResults()) // We make mutations to the configuration, ensure we have a copy dispatchRequest.state.runConfig = copyRunConfig(config) dispatchRequest.state.imageID = config.Image diff --git a/builder/dockerfile/dispatchers.go b/builder/dockerfile/dispatchers.go index 991c433b2b..9dd7502453 100644 --- a/builder/dockerfile/dispatchers.go +++ b/builder/dockerfile/dispatchers.go @@ -399,7 +399,7 @@ func dispatchRun(d dispatchRequest, c *instructions.RunCommand) error { // remove any unreferenced built-in args from the environment variables. // These args are transparent so resulting image should be the same regardless // of the value. -func prependEnvOnCmd(buildArgs *buildArgs, buildArgVars []string, cmd strslice.StrSlice) strslice.StrSlice { +func prependEnvOnCmd(buildArgs *BuildArgs, buildArgVars []string, cmd strslice.StrSlice) strslice.StrSlice { var tmpBuildEnv []string for _, env := range buildArgVars { key := strings.SplitN(env, "=", 2)[0] diff --git a/builder/dockerfile/dispatchers_test.go b/builder/dockerfile/dispatchers_test.go index 6ddde82d43..8eed646a70 100644 --- a/builder/dockerfile/dispatchers_test.go +++ b/builder/dockerfile/dispatchers_test.go @@ -41,7 +41,7 @@ func newBuilderWithMockBackend() *Builder { func TestEnv2Variables(t *testing.T) { b := newBuilderWithMockBackend() - sb := newDispatchRequest(b, '\\', nil, newBuildArgs(make(map[string]*string)), newStagesBuildResults()) + sb := newDispatchRequest(b, '\\', nil, NewBuildArgs(make(map[string]*string)), newStagesBuildResults()) envCommand := &instructions.EnvCommand{ Env: instructions.KeyValuePairs{ instructions.KeyValuePair{Key: "var1", Value: "val1"}, @@ -60,7 +60,7 @@ func TestEnv2Variables(t *testing.T) { func TestEnvValueWithExistingRunConfigEnv(t *testing.T) { b := newBuilderWithMockBackend() - sb := newDispatchRequest(b, '\\', nil, newBuildArgs(make(map[string]*string)), newStagesBuildResults()) + sb := newDispatchRequest(b, '\\', nil, NewBuildArgs(make(map[string]*string)), newStagesBuildResults()) sb.state.runConfig.Env = []string{"var1=old", "var2=fromenv"} envCommand := &instructions.EnvCommand{ Env: instructions.KeyValuePairs{ @@ -79,7 +79,7 @@ func TestEnvValueWithExistingRunConfigEnv(t *testing.T) { func TestMaintainer(t *testing.T) { maintainerEntry := "Some Maintainer " b := newBuilderWithMockBackend() - sb := newDispatchRequest(b, '\\', nil, newBuildArgs(make(map[string]*string)), newStagesBuildResults()) + sb := newDispatchRequest(b, '\\', nil, NewBuildArgs(make(map[string]*string)), newStagesBuildResults()) cmd := &instructions.MaintainerCommand{Maintainer: maintainerEntry} err := dispatch(sb, cmd) assert.NilError(t, err) @@ -91,7 +91,7 @@ func TestLabel(t *testing.T) { labelValue := "value" b := newBuilderWithMockBackend() - sb := newDispatchRequest(b, '\\', nil, newBuildArgs(make(map[string]*string)), newStagesBuildResults()) + sb := newDispatchRequest(b, '\\', nil, NewBuildArgs(make(map[string]*string)), newStagesBuildResults()) cmd := &instructions.LabelCommand{ Labels: instructions.KeyValuePairs{ instructions.KeyValuePair{Key: labelName, Value: labelValue}, @@ -106,7 +106,7 @@ func TestLabel(t *testing.T) { func TestFromScratch(t *testing.T) { b := newBuilderWithMockBackend() - sb := newDispatchRequest(b, '\\', nil, newBuildArgs(make(map[string]*string)), newStagesBuildResults()) + sb := newDispatchRequest(b, '\\', nil, NewBuildArgs(make(map[string]*string)), newStagesBuildResults()) cmd := &instructions.Stage{ BaseName: "scratch", } @@ -133,7 +133,7 @@ func TestFromWithArg(t *testing.T) { } b := newBuilderWithMockBackend() b.docker.(*MockBackend).getImageFunc = getImage - args := newBuildArgs(make(map[string]*string)) + args := NewBuildArgs(make(map[string]*string)) val := "sometag" metaArg := instructions.ArgCommand{ @@ -165,7 +165,7 @@ func TestFromWithUndefinedArg(t *testing.T) { } b := newBuilderWithMockBackend() b.docker.(*MockBackend).getImageFunc = getImage - sb := newDispatchRequest(b, '\\', nil, newBuildArgs(make(map[string]*string)), newStagesBuildResults()) + sb := newDispatchRequest(b, '\\', nil, NewBuildArgs(make(map[string]*string)), newStagesBuildResults()) b.options.BuildArgs = map[string]*string{"THETAG": &tag} @@ -182,8 +182,8 @@ func TestFromMultiStageWithNamedStage(t *testing.T) { firstFrom := &instructions.Stage{BaseName: "someimg", Name: "base"} secondFrom := &instructions.Stage{BaseName: "base"} previousResults := newStagesBuildResults() - firstSB := newDispatchRequest(b, '\\', nil, newBuildArgs(make(map[string]*string)), previousResults) - secondSB := newDispatchRequest(b, '\\', nil, newBuildArgs(make(map[string]*string)), previousResults) + firstSB := newDispatchRequest(b, '\\', nil, NewBuildArgs(make(map[string]*string)), previousResults) + secondSB := newDispatchRequest(b, '\\', nil, NewBuildArgs(make(map[string]*string)), previousResults) err := initializeStage(firstSB, firstFrom) assert.NilError(t, err) assert.Check(t, firstSB.state.hasFromImage()) @@ -196,7 +196,7 @@ func TestFromMultiStageWithNamedStage(t *testing.T) { func TestOnbuild(t *testing.T) { b := newBuilderWithMockBackend() - sb := newDispatchRequest(b, '\\', nil, newBuildArgs(make(map[string]*string)), newStagesBuildResults()) + sb := newDispatchRequest(b, '\\', nil, NewBuildArgs(make(map[string]*string)), newStagesBuildResults()) cmd := &instructions.OnbuildCommand{ Expression: "ADD . /app/src", } @@ -207,7 +207,7 @@ func TestOnbuild(t *testing.T) { func TestWorkdir(t *testing.T) { b := newBuilderWithMockBackend() - sb := newDispatchRequest(b, '`', nil, newBuildArgs(make(map[string]*string)), newStagesBuildResults()) + sb := newDispatchRequest(b, '`', nil, NewBuildArgs(make(map[string]*string)), newStagesBuildResults()) sb.state.baseImage = &mockImage{} workingDir := "/app" if runtime.GOOS == "windows" { @@ -224,7 +224,7 @@ func TestWorkdir(t *testing.T) { func TestCmd(t *testing.T) { b := newBuilderWithMockBackend() - sb := newDispatchRequest(b, '`', nil, newBuildArgs(make(map[string]*string)), newStagesBuildResults()) + sb := newDispatchRequest(b, '`', nil, NewBuildArgs(make(map[string]*string)), newStagesBuildResults()) sb.state.baseImage = &mockImage{} command := "./executable" @@ -250,7 +250,7 @@ func TestCmd(t *testing.T) { func TestHealthcheckNone(t *testing.T) { b := newBuilderWithMockBackend() - sb := newDispatchRequest(b, '`', nil, newBuildArgs(make(map[string]*string)), newStagesBuildResults()) + sb := newDispatchRequest(b, '`', nil, NewBuildArgs(make(map[string]*string)), newStagesBuildResults()) cmd := &instructions.HealthCheckCommand{ Health: &container.HealthConfig{ Test: []string{"NONE"}, @@ -266,7 +266,7 @@ func TestHealthcheckNone(t *testing.T) { func TestHealthcheckCmd(t *testing.T) { b := newBuilderWithMockBackend() - sb := newDispatchRequest(b, '`', nil, newBuildArgs(make(map[string]*string)), newStagesBuildResults()) + sb := newDispatchRequest(b, '`', nil, NewBuildArgs(make(map[string]*string)), newStagesBuildResults()) expectedTest := []string{"CMD-SHELL", "curl -f http://localhost/ || exit 1"} cmd := &instructions.HealthCheckCommand{ Health: &container.HealthConfig{ @@ -282,7 +282,7 @@ func TestHealthcheckCmd(t *testing.T) { func TestEntrypoint(t *testing.T) { b := newBuilderWithMockBackend() - sb := newDispatchRequest(b, '`', nil, newBuildArgs(make(map[string]*string)), newStagesBuildResults()) + sb := newDispatchRequest(b, '`', nil, NewBuildArgs(make(map[string]*string)), newStagesBuildResults()) sb.state.baseImage = &mockImage{} entrypointCmd := "/usr/sbin/nginx" @@ -307,7 +307,7 @@ func TestEntrypoint(t *testing.T) { func TestExpose(t *testing.T) { b := newBuilderWithMockBackend() - sb := newDispatchRequest(b, '`', nil, newBuildArgs(make(map[string]*string)), newStagesBuildResults()) + sb := newDispatchRequest(b, '`', nil, NewBuildArgs(make(map[string]*string)), newStagesBuildResults()) exposedPort := "80" cmd := &instructions.ExposeCommand{ @@ -326,7 +326,7 @@ func TestExpose(t *testing.T) { func TestUser(t *testing.T) { b := newBuilderWithMockBackend() - sb := newDispatchRequest(b, '`', nil, newBuildArgs(make(map[string]*string)), newStagesBuildResults()) + sb := newDispatchRequest(b, '`', nil, NewBuildArgs(make(map[string]*string)), newStagesBuildResults()) cmd := &instructions.UserCommand{ User: "test", @@ -338,7 +338,7 @@ func TestUser(t *testing.T) { func TestVolume(t *testing.T) { b := newBuilderWithMockBackend() - sb := newDispatchRequest(b, '`', nil, newBuildArgs(make(map[string]*string)), newStagesBuildResults()) + sb := newDispatchRequest(b, '`', nil, NewBuildArgs(make(map[string]*string)), newStagesBuildResults()) exposedVolume := "/foo" @@ -358,7 +358,7 @@ func TestStopSignal(t *testing.T) { return } b := newBuilderWithMockBackend() - sb := newDispatchRequest(b, '`', nil, newBuildArgs(make(map[string]*string)), newStagesBuildResults()) + sb := newDispatchRequest(b, '`', nil, NewBuildArgs(make(map[string]*string)), newStagesBuildResults()) sb.state.baseImage = &mockImage{} signal := "SIGKILL" @@ -372,7 +372,7 @@ func TestStopSignal(t *testing.T) { func TestArg(t *testing.T) { b := newBuilderWithMockBackend() - sb := newDispatchRequest(b, '`', nil, newBuildArgs(make(map[string]*string)), newStagesBuildResults()) + sb := newDispatchRequest(b, '`', nil, NewBuildArgs(make(map[string]*string)), newStagesBuildResults()) argName := "foo" argVal := "bar" @@ -386,7 +386,7 @@ func TestArg(t *testing.T) { func TestShell(t *testing.T) { b := newBuilderWithMockBackend() - sb := newDispatchRequest(b, '`', nil, newBuildArgs(make(map[string]*string)), newStagesBuildResults()) + sb := newDispatchRequest(b, '`', nil, NewBuildArgs(make(map[string]*string)), newStagesBuildResults()) shellCmd := "powershell" cmd := &instructions.ShellCommand{Shell: strslice.StrSlice{shellCmd}} @@ -399,7 +399,7 @@ func TestShell(t *testing.T) { } func TestPrependEnvOnCmd(t *testing.T) { - buildArgs := newBuildArgs(nil) + buildArgs := NewBuildArgs(nil) buildArgs.AddArg("NO_PROXY", nil) args := []string{"sorted=nope", "args=not", "http_proxy=foo", "NO_PROXY=YA"} @@ -412,7 +412,7 @@ func TestPrependEnvOnCmd(t *testing.T) { func TestRunWithBuildArgs(t *testing.T) { b := newBuilderWithMockBackend() - args := newBuildArgs(make(map[string]*string)) + args := NewBuildArgs(make(map[string]*string)) args.argsFromOptions["HTTP_PROXY"] = strPtr("FOO") b.disableCommit = false sb := newDispatchRequest(b, '`', nil, args, newStagesBuildResults()) diff --git a/builder/dockerfile/evaluator.go b/builder/dockerfile/evaluator.go index 0f76845086..75073cec6d 100644 --- a/builder/dockerfile/evaluator.go +++ b/builder/dockerfile/evaluator.go @@ -111,11 +111,11 @@ type dispatchState struct { imageID string baseImage builder.Image stageName string - buildArgs *buildArgs + buildArgs *BuildArgs operatingSystem string } -func newDispatchState(baseArgs *buildArgs) *dispatchState { +func newDispatchState(baseArgs *BuildArgs) *dispatchState { args := baseArgs.Clone() args.ResetAllowed() return &dispatchState{runConfig: &container.Config{}, buildArgs: args} @@ -193,7 +193,7 @@ type dispatchRequest struct { stages *stagesBuildResults } -func newDispatchRequest(builder *Builder, escapeToken rune, source builder.Source, buildArgs *buildArgs, stages *stagesBuildResults) dispatchRequest { +func newDispatchRequest(builder *Builder, escapeToken rune, source builder.Source, buildArgs *BuildArgs, stages *stagesBuildResults) dispatchRequest { return dispatchRequest{ state: newDispatchState(buildArgs), shlex: shell.NewLex(escapeToken), diff --git a/builder/dockerfile/evaluator_test.go b/builder/dockerfile/evaluator_test.go index db88f36c2d..4d0f6c30ec 100644 --- a/builder/dockerfile/evaluator_test.go +++ b/builder/dockerfile/evaluator_test.go @@ -137,7 +137,7 @@ func executeTestCase(t *testing.T, testCase dispatchTestCase) { }() b := newBuilderWithMockBackend() - sb := newDispatchRequest(b, '`', context, newBuildArgs(make(map[string]*string)), newStagesBuildResults()) + sb := newDispatchRequest(b, '`', context, NewBuildArgs(make(map[string]*string)), newStagesBuildResults()) err = dispatch(sb, testCase.cmd) testutil.ErrorContains(t, err, testCase.expectedError) }