fix: use StringMap for build_args instead of StringSlice (#277)

BREAKING CHANGE: The type of the `build_args` property was changed from `list` to `map` to support the use of secrets for certain build arguments. During this refactoring, the `build_args_from_env` behavior was also changed to achieve the intended behavior. Environment variable names passed to this list will be used as build arguments if the environment variable is set.

docker/docker.go

@@ -2,6 +2,7 @@ package docker
 
 import (
 	"fmt"
+	"maps"
 	"os"
 	"strings"
 	"time"
@@ -32,7 +33,7 @@ type Build struct {
 	Tags          cli.StringSlice   // Docker build tags
 	ExtraTags     cli.StringSlice   // Docker build tags including registry
 	Platforms     cli.StringSlice   // Docker build target platforms
-	Args          cli.StringSlice // Docker build args
+	Args          map[string]string // Docker build args
 	ArgsEnv       cli.StringSlice   // Docker build args from env
 	Target        string            // Docker build target
 	Pull          bool              // Docker build pull
@@ -100,10 +101,12 @@ func (b *Build) Run() *plugin_exec.Cmd {
 		"-f", b.Containerfile,
 	}
 
-	defaultBuildArgs := []string{
-		fmt.Sprintf("DOCKER_IMAGE_CREATED=%s", time.Now().Format(time.RFC3339)),
+	defaultBuildArgs := map[string]string{
+		"DOCKER_IMAGE_CREATED": time.Now().Format(time.RFC3339),
 	}
 
+	maps.Copy(b.Args, defaultBuildArgs)
+
 	args = append(args, b.Context)
 	if !b.Dryrun && b.Output == "" && len(b.Tags.Value()) > 0 {
 		args = append(args, "--push")
@@ -130,11 +133,11 @@ func (b *Build) Run() *plugin_exec.Cmd {
 	}
 
 	for _, arg := range b.ArgsEnv.Value() {
-		b.addProxyValue(arg)
+		b.addArgFromEnv(arg)
 	}
 
-	for _, arg := range append(defaultBuildArgs, b.Args.Value()...) {
-		args = append(args, "--build-arg", arg)
+	for key, value := range b.Args {
+		args = append(args, "--build-arg", fmt.Sprintf("%s=%s", key, value))
 	}
 
 	for _, host := range b.AddHost.Value() {
@@ -203,9 +206,19 @@ func (b *Build) AddProxyBuildArgs() {
 func (b *Build) addProxyValue(key string) {
 	value := b.getProxyValue(key)
 
-	if len(value) > 0 && !b.hasProxyBuildArg(key) {
-		b.Args = *cli.NewStringSlice(append(b.Args.Value(), fmt.Sprintf("%s=%s", key, value))...)
-		b.Args = *cli.NewStringSlice(append(b.Args.Value(), fmt.Sprintf("%s=%s", strings.ToUpper(key), value))...)
+	if value != "" && !b.hasProxyBuildArg(key) {
+		b.Args[key] = value
+		b.Args[strings.ToUpper(key)] = value
+	}
+}
+
+func (b *Build) addArgFromEnv(key string) {
+	if value, ok := b.Args[key]; ok && value != "" {
+		return
+	}
+
+	if value, ok := os.LookupEnv(key); ok && value != "" {
+		b.Args[key] = value
 	}
 }
 
@@ -215,7 +228,7 @@ func (b *Build) addProxyValue(key string) {
 func (b *Build) getProxyValue(key string) string {
 	value := os.Getenv(key)
 
-	if len(value) > 0 {
+	if value != "" {
 		return value
 	}
 
@@ -224,12 +237,12 @@ func (b *Build) getProxyValue(key string) string {
 
 // helper function that looks to see if a proxy value was set in the build args.
 func (b *Build) hasProxyBuildArg(key string) bool {
-	keyUpper := strings.ToUpper(key)
-
-	for _, s := range b.Args.Value() {
-		if strings.HasPrefix(s, key) || strings.HasPrefix(s, keyUpper) {
+	if _, ok := b.Args[key]; ok {
 		return true
 	}
+
+	if _, ok := b.Args[strings.ToUpper(key)]; ok {
+		return true
 	}
 
 	return false

docker/docker_test.go

@@ -0,0 +1,197 @@
+package docker
+
+import (
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+)
+
+func TestHasProxyBuildArg(t *testing.T) {
+	tests := []struct {
+		name     string
+		args     map[string]string
+		key      string
+		expected bool
+	}{
+		{
+			name:     "lowercase key exists",
+			args:     map[string]string{"http_proxy": "http://proxy.example.com"},
+			key:      "http_proxy",
+			expected: true,
+		},
+		{
+			name:     "uppercase key exists",
+			args:     map[string]string{"HTTP_PROXY": "http://proxy.example.com"},
+			key:      "http_proxy",
+			expected: true,
+		},
+		{
+			name:     "key does not exist",
+			args:     map[string]string{},
+			key:      "http_proxy",
+			expected: false,
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			b := &Build{Args: tt.args}
+			result := b.hasProxyBuildArg(tt.key)
+			assert.Equal(t, tt.expected, result)
+		})
+	}
+}
+
+func TestGetProxyValue(t *testing.T) {
+	tests := []struct {
+		name     string
+		envVars  map[string]string
+		key      string
+		expected string
+	}{
+		{
+			name: "lowercase env var exists",
+			envVars: map[string]string{
+				"http_proxy": "http://proxy.local",
+			},
+			key:      "http_proxy",
+			expected: "http://proxy.local",
+		},
+		{
+			name: "uppercase env var exists",
+			envVars: map[string]string{
+				"HTTP_PROXY": "http://proxy.upper.local",
+			},
+			key:      "http_proxy",
+			expected: "http://proxy.upper.local",
+		},
+		{
+			name: "both cases exist, lowercase preferred",
+			envVars: map[string]string{
+				"http_proxy": "http://proxy.lower.local",
+				"HTTP_PROXY": "http://proxy.upper.local",
+			},
+			key:      "http_proxy",
+			expected: "http://proxy.lower.local",
+		},
+		{
+			name:     "no env vars exist",
+			envVars:  map[string]string{},
+			key:      "http_proxy",
+			expected: "",
+		},
+		{
+			name: "different proxy type",
+			envVars: map[string]string{
+				"https_proxy": "https://secure.proxy.local",
+				"HTTPS_PROXY": "https://secure.proxy.upper.local",
+			},
+			key:      "https_proxy",
+			expected: "https://secure.proxy.local",
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			// Set test environment variables
+			for k, v := range tt.envVars {
+				t.Setenv(k, v)
+			}
+
+			b := &Build{}
+			result := b.getProxyValue(tt.key)
+			assert.Equal(t, tt.expected, result)
+		})
+	}
+}
+
+func TestAddArgFromEnv(t *testing.T) {
+	tests := []struct {
+		name     string
+		envVars  map[string]string
+		key      string
+		existing map[string]string
+		expected map[string]string
+	}{
+		{
+			name: "add new env var",
+			envVars: map[string]string{
+				"NEW_VAR": "test_value",
+			},
+			key: "NEW_VAR",
+			expected: map[string]string{
+				"NEW_VAR": "test_value",
+			},
+		},
+		{
+			name:     "empty env var",
+			envVars:  map[string]string{},
+			key:      "MISSING_VAR",
+			expected: map[string]string{},
+		},
+		{
+			name: "env var with empty value",
+			envVars: map[string]string{
+				"EMPTY_VAR": "",
+			},
+			key:      "EMPTY_VAR",
+			expected: map[string]string{},
+		},
+		{
+			name: "multiple env vars",
+			envVars: map[string]string{
+				"VAR1": "value1",
+				"VAR2": "value2",
+			},
+			key: "VAR1",
+			expected: map[string]string{
+				"VAR1": "value1",
+			},
+		},
+		{
+			name: "special characters in value",
+			envVars: map[string]string{
+				"SPECIAL_VAR": "!@#$%^&*()",
+			},
+			key: "SPECIAL_VAR",
+			expected: map[string]string{
+				"SPECIAL_VAR": "!@#$%^&*()",
+			},
+		},
+		{
+			name: "preserve existing args",
+			envVars: map[string]string{
+				"TEST_VAR": "new_value",
+			},
+			key: "TEST_VAR",
+			existing: map[string]string{
+				"TEST_VAR": "old_value",
+			},
+			expected: map[string]string{
+				"TEST_VAR": "old_value",
+			},
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			// Clear environment before each test
+			for k := range tt.envVars {
+				t.Setenv(k, "")
+			}
+
+			// Set test environment variables
+			for k, v := range tt.envVars {
+				t.Setenv(k, v)
+			}
+
+			b := &Build{Args: make(map[string]string)}
+			if tt.existing != nil {
+				b.Args = tt.existing
+			}
+
+			b.addArgFromEnv(tt.key)
+			assert.Equal(t, tt.expected, b.Args)
+		})
+	}
+}

docs/data/data.yaml

@@ -34,13 +34,38 @@ properties:
 
   - name: build_args
     description: |
-      Custom build arguments for the build.
-    type: list
+      Custom build arguments for the build. Example:
+
+      ```yaml
+      steps:
+        - name: Build
+          image: quay.io/thegeeklab/wp-docker-buildx
+          settings:
+            repo: example/repo
+            build_args:
+              FOO: bar
+              API_KEY:
+                from_secret: API_KEY
+      ```
+    type: map
     required: false
 
   - name: build_args_from_env
     description: |
-      Forward environment variables as custom arguments to the build.
+      Forward environment variables to the build as build arguments. If the same key
+      already exists in `build_args`, it will not be overwritten. Example:
+
+      ```yaml
+      steps:
+        - name: Build
+          image: quay.io/thegeeklab/wp-docker-buildx
+          environment:
+            CUSTOM_ENVIRONMENT: custom_value
+          settings:
+            repo: example/repo
+            build_args_from_env:
+              - CUSTOM_ENVIRONMENT
+      ```
     type: list
     required: false
 

plugin/impl.go

@@ -210,6 +210,13 @@ func (p *Plugin) FlagsFromContext() error {
 
 	p.Settings.Build.Secrets = secrets.Get()
 
+	args, ok := p.Context.Generic("args").(*plugin_types.StringMapFlag)
+	if !ok {
+		return fmt.Errorf("%w: failed to read args input", ErrTypeAssertionFailed)
+	}
+
+	p.Settings.Build.Args = args.Get()
+
 	return nil
 }
 

plugin/plugin.go

@@ -227,11 +227,11 @@ func Flags(settings *Settings, category string) []cli.Flag {
 			Destination: &settings.Build.ExtraTags,
 			Category:    category,
 		},
-		&cli.StringSliceFlag{
+		&cli.GenericFlag{
 			Name:     "args",
 			EnvVars:  []string{"PLUGIN_BUILD_ARGS"},
 			Usage:    "custom build arguments for the build",
-			Destination: &settings.Build.Args,
+			Value:    &plugin_types.StringMapFlag{},
 			Category: category,
 		},
 		&cli.StringSliceFlag{