Skip to content

Commit

Permalink
fix: use StringMap for build_args instead of StringSlice (#277)
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
xoxys authored Nov 15, 2024
1 parent b213fca commit a2bc086
Show file tree
Hide file tree
Showing 5 changed files with 292 additions and 50 deletions.
95 changes: 54 additions & 41 deletions docker/docker.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ package docker

import (
"fmt"
"maps"
"os"
"strings"
"time"
Expand All @@ -23,33 +24,33 @@ type Registry struct {

// Build defines Docker build parameters.
type Build struct {
Ref string // Git commit ref
Branch string // Git repository branch
Containerfile string // Docker build Containerfile
Context string // Docker build context
TagsAuto bool // Docker build auto tag
TagsSuffix string // Docker build tags with suffix
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
ArgsEnv cli.StringSlice // Docker build args from env
Target string // Docker build target
Pull bool // Docker build pull
CacheFrom []string // Docker build cache-from
CacheTo string // Docker build cache-to
Compress bool // Docker build compress
Repo string // Docker build repository
NoCache bool // Docker build no-cache
AddHost cli.StringSlice // Docker build add-host
Quiet bool // Docker build quiet
Output string // Docker build output folder
NamedContext cli.StringSlice // Docker build named context
Labels cli.StringSlice // Docker build labels
Provenance string // Docker build provenance attestation
SBOM string // Docker build sbom attestation
Secrets []string // Docker build secrets
Dryrun bool // Docker build dryrun
Ref string // Git commit ref
Branch string // Git repository branch
Containerfile string // Docker build Containerfile
Context string // Docker build context
TagsAuto bool // Docker build auto tag
TagsSuffix string // Docker build tags with suffix
Tags cli.StringSlice // Docker build tags
ExtraTags cli.StringSlice // Docker build tags including registry
Platforms cli.StringSlice // Docker build target platforms
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
CacheFrom []string // Docker build cache-from
CacheTo string // Docker build cache-to
Compress bool // Docker build compress
Repo string // Docker build repository
NoCache bool // Docker build no-cache
AddHost cli.StringSlice // Docker build add-host
Quiet bool // Docker build quiet
Output string // Docker build output folder
NamedContext cli.StringSlice // Docker build named context
Labels cli.StringSlice // Docker build labels
Provenance string // Docker build provenance attestation
SBOM string // Docker build sbom attestation
Secrets []string // Docker build secrets
Dryrun bool // Docker build dryrun
}

// helper function to create the docker login command.
Expand Down Expand Up @@ -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")
Expand All @@ -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() {
Expand Down Expand Up @@ -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
}
}

Expand All @@ -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
}

Expand All @@ -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)
if _, ok := b.Args[key]; ok {
return true
}

for _, s := range b.Args.Value() {
if strings.HasPrefix(s, key) || strings.HasPrefix(s, keyUpper) {
return true
}
if _, ok := b.Args[strings.ToUpper(key)]; ok {
return true
}

return false
Expand Down
197 changes: 197 additions & 0 deletions docker/docker_test.go
Original file line number Diff line number Diff line change
@@ -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)
})
}
}
31 changes: 28 additions & 3 deletions docs/data/data.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
Loading

0 comments on commit a2bc086

Please sign in to comment.