Skip to content

Commit 90c383a

Browse files
WVerlaekona-agent
andcommitted
fix: support comma-containing values in --docker-build-options
Change flag type from StringToString to StringArray to prevent comma-based splitting of option values. This allows docker buildx cache options like cache-to=type=gha,mode=max to work correctly. Co-authored-by: Ona <no-reply@ona.com>
1 parent 85c4c42 commit 90c383a

3 files changed

Lines changed: 62 additions & 7 deletions

File tree

cmd/build.go

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -231,7 +231,7 @@ func addBuildFlags(cmd *cobra.Command) {
231231
cmd.Flags().String("coverage-output-path", "", "Output path where test coverage file will be copied after running tests")
232232
cmd.Flags().Bool("disable-coverage", false, "Disable test coverage collection (defaults to false)")
233233
cmd.Flags().Bool("enable-test-tracing", false, "Enable per-test OpenTelemetry span creation (defaults to false)")
234-
cmd.Flags().StringToString("docker-build-options", nil, "Options passed to all 'docker build' commands")
234+
cmd.Flags().StringArray("docker-build-options", nil, "Options passed to all 'docker build' commands (can be repeated, e.g., --docker-build-options=cache-to=type=gha,mode=max)")
235235
cmd.Flags().Bool("slsa-cache-verification", false, "Enable SLSA verification for cached artifacts")
236236
cmd.Flags().String("slsa-source-uri", "", "Expected source URI for SLSA verification (required when verification enabled)")
237237
cmd.Flags().Bool("slsa-require-attestation", false, "Require SLSA attestations (missing/invalid → build locally)")
@@ -367,11 +367,11 @@ func getBuildOpts(cmd *cobra.Command) ([]leeway.BuildOption, cache.LocalCache) {
367367
disableCoverage, _ := cmd.Flags().GetBool("disable-coverage")
368368
enableTestTracing, _ := cmd.Flags().GetBool("enable-test-tracing")
369369

370-
var dockerBuildOptions leeway.DockerBuildOptions
371-
dockerBuildOptions, err = cmd.Flags().GetStringToString("docker-build-options")
370+
dockerBuildOptionsSlice, err := cmd.Flags().GetStringArray("docker-build-options")
372371
if err != nil {
373372
log.Fatal(err)
374373
}
374+
dockerBuildOptions := leeway.DockerBuildOptions(dockerBuildOptionsSlice)
375375

376376
jailedExecution, err := cmd.Flags().GetBool("jailed-execution")
377377
if err != nil {

pkg/leeway/build.go

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -495,8 +495,9 @@ type buildOptions struct {
495495
context *buildContext
496496
}
497497

498-
// DockerBuildOptions are options passed to "docker build"
499-
type DockerBuildOptions map[string]string
498+
// DockerBuildOptions are options passed to "docker build".
499+
// Each entry is a "key=value" string that becomes "--key=value" in the docker command.
500+
type DockerBuildOptions []string
500501

501502
// BuildOption configures the build behaviour
502503
type BuildOption func(*buildOptions) error
@@ -2408,8 +2409,8 @@ func (p *Package) buildDocker(buildctx *buildContext, wd, result string) (res *p
24082409
buildcmd = append(buildcmd, "--squash")
24092410
}
24102411
if buildctx.DockerBuildOptions != nil {
2411-
for opt, v := range *buildctx.DockerBuildOptions {
2412-
buildcmd = append(buildcmd, fmt.Sprintf("--%s=%s", opt, v))
2412+
for _, opt := range *buildctx.DockerBuildOptions {
2413+
buildcmd = append(buildcmd, fmt.Sprintf("--%s", opt))
24132414
}
24142415
}
24152416
buildcmd = append(buildcmd, ".")

pkg/leeway/build_internal_test.go

Lines changed: 54 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -333,3 +333,57 @@ func TestYarnAppExtraction_ScopedPackage(t *testing.T) {
333333
})
334334
}
335335
}
336+
337+
func TestDockerBuildOptions_CommaInValue(t *testing.T) {
338+
// DockerBuildOptions should preserve comma-containing values intact.
339+
// This is the fix for: https://github.com/gitpod-io/leeway/issues/XXX
340+
// Previously, using StringToString flag type would split "cache-to=type=gha,mode=max"
341+
// into two separate options, breaking docker buildx cache options.
342+
343+
tests := []struct {
344+
name string
345+
opts DockerBuildOptions
346+
expected []string
347+
}{
348+
{
349+
name: "single option with comma in value",
350+
opts: DockerBuildOptions{"cache-to=type=gha,mode=max"},
351+
expected: []string{"--cache-to=type=gha,mode=max"},
352+
},
353+
{
354+
name: "multiple options with commas",
355+
opts: DockerBuildOptions{"cache-to=type=gha,mode=max", "cache-from=type=gha"},
356+
expected: []string{"--cache-to=type=gha,mode=max", "--cache-from=type=gha"},
357+
},
358+
{
359+
name: "simple option without comma",
360+
opts: DockerBuildOptions{"no-cache=true"},
361+
expected: []string{"--no-cache=true"},
362+
},
363+
{
364+
name: "empty options",
365+
opts: DockerBuildOptions{},
366+
expected: []string{},
367+
},
368+
}
369+
370+
for _, tt := range tests {
371+
t.Run(tt.name, func(t *testing.T) {
372+
var result []string
373+
for _, opt := range tt.opts {
374+
result = append(result, fmt.Sprintf("--%s", opt))
375+
}
376+
377+
if len(result) != len(tt.expected) {
378+
t.Errorf("got %d options, want %d", len(result), len(tt.expected))
379+
return
380+
}
381+
382+
for i, got := range result {
383+
if got != tt.expected[i] {
384+
t.Errorf("option %d: got %q, want %q", i, got, tt.expected[i])
385+
}
386+
}
387+
})
388+
}
389+
}

0 commit comments

Comments
 (0)