Skip to content

Commit 7300aed

Browse files
Copilotintel352
andauthored
fix(wfctl): add --push flag; fix hardened buildx push/load semantics (#546)
* Initial plan * fix: add --push flag to wfctl build and fix hardened buildx push/load semantics Bug 1: Add --push flag to wfctl build as the explicit-push counterpart to --no-push. --push=false is equivalent to --no-push. This allows dependent repos to use --push explicitly without a 'flag not defined' error. Bug 2: In buildWithDockerfile (hardened mode), add --push or --load to the docker buildx args. The docker-container driver required for --provenance/--sbom caches the build result in buildkit's cache but does not export it to the local daemon or registry unless --push or --load is specified. - hardened + push: passes --push to buildx (push directly from buildkit) - hardened + no-push: passes --load to buildx (load into local daemon) The orchestrator (runBuildOrchestrate) now passes --push to runBuildImage when hardened mode is active and skips the separate runBuildPush step (which would fail with 'image does not exist locally' since the hardened path never loads the image into the docker daemon). Tests: added TestRunBuildImage_HardenedPushFlag and TestRunBuildImage_HardenedNoPushAddsLoad to cover both paths. Fixes: wfctl build --push pipeline broken in hardened mode Agent-Logs-Url: https://github.com/GoCodeAlone/workflow/sessions/fecb8b30-963c-49ed-b4fb-db5028ae3afb Co-authored-by: intel352 <77607+intel352@users.noreply.github.com> * fix: address review feedback on hardened push/load logic - Only add --push to buildx when push=true AND push_to is non-empty; containers without push_to fall back to --load (single-platform) instead. - Add allImageRefsForContainer helper and use all push_to registries as --tag flags for multi-registry hardened pushes in a single buildx call. - Skip --load for multi-platform + no-push builds (unsupported by buildx); leave result in buildkit cache with a comment explaining why. - Replace fake fset regression test with a real TestRunBuild_PushFlagDefined that calls runBuild(["--push", "--dry-run"]) to exercise the real FlagSet. - Add new tests: HardenedPushNoPushTo, HardenedMultiRegistryPush, HardenedMultiPlatformNoPushNoLoad. - Update docs/manual/build-deploy/05-cli-reference.md: add --push flag to wfctl build table, expand wfctl build image synopsis with semantics note. Agent-Logs-Url: https://github.com/GoCodeAlone/workflow/sessions/eab2c034-93b1-4ef7-aac8-4f62c4744b84 Co-authored-by: intel352 <77607+intel352@users.noreply.github.com> --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: intel352 <77607+intel352@users.noreply.github.com>
1 parent 6bdacdf commit 7300aed

5 files changed

Lines changed: 302 additions & 24 deletions

File tree

cmd/wfctl/build.go

Lines changed: 21 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -57,10 +57,16 @@ func runBuild(args []string) error {
5757
fs.StringVar(&tag, "tag", "", "Override image tag for all container targets")
5858
fs.StringVar(&format, "format", "table", "Output format: table | json | yaml")
5959
fs.BoolVar(&noPush, "no-push", false, "Build but do not push images to registries")
60+
var push bool
61+
fs.BoolVar(&push, "push", true, "Push images to registries after build (default true; --push=false is equivalent to --no-push)")
6062
fs.StringVar(&envName, "env", "", "Environment name for per-env config overrides")
6163
if err := fs.Parse(args); err != nil {
6264
return err
6365
}
66+
// --push=false is equivalent to --no-push.
67+
if !push {
68+
noPush = true
69+
}
6470

6571
if dryRun {
6672
os.Setenv("WFCTL_BUILD_DRY_RUN", "1") //nolint:errcheck
@@ -105,6 +111,11 @@ type buildOpts struct {
105111
func runBuildOrchestrate(cfg *config.WorkflowConfig, opts buildOpts) error {
106112
build := cfg.CI.Build
107113

114+
// Detect hardened mode once; with buildx (docker-container driver) the push
115+
// must be done inside the buildx invocation itself — a separate docker push
116+
// would fail because the image is only in buildkit's cache, not the local daemon.
117+
hardened := cfg.CI.Build.Security != nil && cfg.CI.Build.Security.Hardened
118+
108119
// Go targets.
109120
for i := range build.Targets {
110121
t := &build.Targets[i]
@@ -157,13 +168,21 @@ func runBuildOrchestrate(cfg *config.WorkflowConfig, opts buildOpts) error {
157168
if opts.tag != "" {
158169
imgArgs = append(imgArgs, "--tag", opts.tag)
159170
}
171+
// For hardened builds (docker buildx with docker-container driver), pass
172+
// --push so buildx pushes directly from the buildkit cache. Without this,
173+
// buildx would silently cache the result and the subsequent docker push
174+
// would fail with "image does not exist locally".
175+
if !opts.noPush && hardened {
176+
imgArgs = append(imgArgs, "--push")
177+
}
160178
if err := runBuildImage(imgArgs); err != nil {
161179
return err
162180
}
163181
}
164182

165-
// Push step (unless --no-push).
166-
if !opts.noPush && !opts.dryRun {
183+
// Push step (unless --no-push, dry-run, or hardened mode where buildx
184+
// already pushed directly from the buildkit cache).
185+
if !opts.noPush && !opts.dryRun && !hardened {
167186
pushArgs := []string{}
168187
if opts.cfgPath != "" {
169188
pushArgs = append(pushArgs, "--config", opts.cfgPath)

cmd/wfctl/build_image.go

Lines changed: 42 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@ func runBuildImageWithOutput(args []string, out io.Writer) error {
2525
cfgPath := fs.String("config", "", "Config file")
2626
dryRun := fs.Bool("dry-run", false, "Print planned actions without executing")
2727
tagOverride := fs.String("tag", "", "Override image tag for all containers")
28+
pushImages := fs.Bool("push", false, "Push images directly via buildx (hardened mode: adds --push to buildx; non-hardened: no effect, separate push step handles it)")
2829
if err := fs.Parse(args); err != nil {
2930
return err
3031
}
@@ -84,15 +85,15 @@ func runBuildImageWithOutput(args []string, out io.Writer) error {
8485
return fmt.Errorf("ko build %q: %w", ctr.Name, err)
8586
}
8687
default: // dockerfile
87-
if err := buildWithDockerfile(ctr, tag, *dryRun, hardened, cfg.CI.Registries, out); err != nil {
88+
if err := buildWithDockerfile(ctr, tag, *dryRun, hardened, *pushImages, cfg.CI.Registries, out); err != nil {
8889
return fmt.Errorf("dockerfile build %q: %w", ctr.Name, err)
8990
}
9091
}
9192
}
9293
return nil
9394
}
9495

95-
func buildWithDockerfile(ctr config.CIContainerTarget, tag string, dryRun bool, hardened bool, registries []config.CIRegistry, out io.Writer) error {
96+
func buildWithDockerfile(ctr config.CIContainerTarget, tag string, dryRun bool, hardened bool, push bool, registries []config.CIRegistry, out io.Writer) error {
9697
dockerfile := ctr.Dockerfile
9798
if dockerfile == "" {
9899
dockerfile = "Dockerfile"
@@ -154,6 +155,21 @@ func buildWithDockerfile(ctr config.CIContainerTarget, tag string, dryRun bool,
154155
fmt.Fprintf(out, "warning: DOCKER_BUILDKIT is not set to 1; provenance attestation requires BuildKit\n")
155156
}
156157
args = append(args, "--provenance=mode=max", "--sbom=true")
158+
// The docker-container driver caches the build result but does not export
159+
// it unless --push or --load is explicitly specified.
160+
if push && len(ctr.PushTo) > 0 {
161+
// Multi-registry: add every push_to ref as a --tag flag so buildx
162+
// pushes to all registries in a single invocation.
163+
allRefs := allImageRefsForContainer(ctr, tag, registries)
164+
for _, ref := range allRefs[1:] {
165+
args = append(args, "--tag", ref)
166+
}
167+
args = append(args, "--push")
168+
} else if len(ctr.Platforms) <= 1 {
169+
// Single-platform with no push (or no push_to): load into local daemon.
170+
// Multi-platform + no-push: --load is unsupported; leave in buildkit cache.
171+
args = append(args, "--load")
172+
}
157173
}
158174

159175
args = append(args, ".")
@@ -267,3 +283,27 @@ func imageRefForContainer(ctr config.CIContainerTarget, tag string, registries [
267283
}
268284
return ctr.Name + ":" + tag
269285
}
286+
287+
// allImageRefsForContainer returns image refs for every registry listed in push_to[].
288+
// Falls back to local name:tag when push_to is empty.
289+
// Used by hardened buildx builds to populate multiple --tag flags so all registries
290+
// are pushed in a single buildx invocation.
291+
func allImageRefsForContainer(ctr config.CIContainerTarget, tag string, registries []config.CIRegistry) []string {
292+
if len(ctr.PushTo) == 0 {
293+
return []string{ctr.Name + ":" + tag}
294+
}
295+
regMap := make(map[string]string, len(registries))
296+
for _, reg := range registries {
297+
regMap[reg.Name] = reg.Path
298+
}
299+
refs := make([]string, 0, len(ctr.PushTo))
300+
for _, regName := range ctr.PushTo {
301+
if path, ok := regMap[regName]; ok {
302+
refs = append(refs, path+"/"+ctr.Name+":"+tag)
303+
} else {
304+
// Fall back to raw registry name when path not configured.
305+
refs = append(refs, regName+"/"+ctr.Name+":"+tag)
306+
}
307+
}
308+
return refs
309+
}

cmd/wfctl/build_image_test.go

Lines changed: 197 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -156,7 +156,203 @@ func TestRunBuildImage_NotHardenedNoProvenanceArgs(t *testing.T) {
156156
}
157157
}
158158

159-
// TestRunBuildImage_HardenedUsesBuildx verifies hardened=true produces "docker buildx build".
159+
// TestRunBuildImage_HardenedPushFlag verifies that --push passed to runBuildImage
160+
// adds --push to the docker buildx build args in hardened mode when push_to is set.
161+
func TestRunBuildImage_HardenedPushFlag(t *testing.T) {
162+
dir := t.TempDir()
163+
cfg := `ci:
164+
registries:
165+
- name: docr
166+
type: do
167+
path: registry.digitalocean.com/myorg
168+
build:
169+
security:
170+
hardened: true
171+
containers:
172+
- name: app
173+
method: dockerfile
174+
dockerfile: Dockerfile
175+
push_to:
176+
- docr
177+
`
178+
cfgPath := filepath.Join(dir, "ci.yaml")
179+
if err := os.WriteFile(cfgPath, []byte(cfg), 0o600); err != nil {
180+
t.Fatal(err)
181+
}
182+
t.Setenv("WFCTL_BUILD_DRY_RUN", "1")
183+
t.Setenv("DOCKER_BUILDKIT", "1")
184+
185+
var buf bytes.Buffer
186+
if err := runBuildImageWithOutput([]string{"--config", cfgPath, "--push"}, &buf); err != nil {
187+
t.Fatalf("hardened --push dry-run: %v", err)
188+
}
189+
190+
out := buf.String()
191+
if !strings.Contains(out, "--push") {
192+
t.Errorf("expected --push in hardened dry-run output with --push flag, got: %q", out)
193+
}
194+
if strings.Contains(out, "--load") {
195+
t.Errorf("expected no --load when --push is set, got: %q", out)
196+
}
197+
}
198+
199+
// TestRunBuildImage_HardenedPushNoPushTo verifies that without push_to, hardened
200+
// buildx falls back to --load (not --push), even when --push is requested.
201+
func TestRunBuildImage_HardenedPushNoPushTo(t *testing.T) {
202+
dir := t.TempDir()
203+
cfg := `ci:
204+
build:
205+
security:
206+
hardened: true
207+
containers:
208+
- name: app
209+
method: dockerfile
210+
dockerfile: Dockerfile
211+
`
212+
cfgPath := filepath.Join(dir, "ci.yaml")
213+
if err := os.WriteFile(cfgPath, []byte(cfg), 0o600); err != nil {
214+
t.Fatal(err)
215+
}
216+
t.Setenv("WFCTL_BUILD_DRY_RUN", "1")
217+
t.Setenv("DOCKER_BUILDKIT", "1")
218+
219+
var buf bytes.Buffer
220+
if err := runBuildImageWithOutput([]string{"--config", cfgPath, "--push"}, &buf); err != nil {
221+
t.Fatalf("hardened --push no-push_to dry-run: %v", err)
222+
}
223+
224+
out := buf.String()
225+
// No push_to → should not add --push (would try to push a local-only name).
226+
if strings.Contains(out, " --push") {
227+
t.Errorf("expected no --push when push_to is empty, got: %q", out)
228+
}
229+
// Single-platform fallback → should add --load.
230+
if !strings.Contains(out, "--load") {
231+
t.Errorf("expected --load for single-platform no-push_to, got: %q", out)
232+
}
233+
}
234+
235+
// TestRunBuildImage_HardenedMultiRegistryPush verifies that multi-registry push
236+
// (push_to: [docr, ghcr]) adds all registry refs as --tag flags so buildx pushes
237+
// to every registry in a single invocation.
238+
func TestRunBuildImage_HardenedMultiRegistryPush(t *testing.T) {
239+
dir := t.TempDir()
240+
cfg := `ci:
241+
registries:
242+
- name: docr
243+
type: do
244+
path: registry.digitalocean.com/myorg
245+
- name: ghcr
246+
type: github
247+
path: ghcr.io/myorg
248+
build:
249+
security:
250+
hardened: true
251+
containers:
252+
- name: app
253+
method: dockerfile
254+
dockerfile: Dockerfile
255+
push_to:
256+
- docr
257+
- ghcr
258+
`
259+
cfgPath := filepath.Join(dir, "ci.yaml")
260+
if err := os.WriteFile(cfgPath, []byte(cfg), 0o600); err != nil {
261+
t.Fatal(err)
262+
}
263+
t.Setenv("WFCTL_BUILD_DRY_RUN", "1")
264+
t.Setenv("DOCKER_BUILDKIT", "1")
265+
266+
var buf bytes.Buffer
267+
if err := runBuildImageWithOutput([]string{"--config", cfgPath, "--push"}, &buf); err != nil {
268+
t.Fatalf("hardened multi-registry push dry-run: %v", err)
269+
}
270+
271+
out := buf.String()
272+
if !strings.Contains(out, "registry.digitalocean.com/myorg/app") {
273+
t.Errorf("expected docr registry ref in output, got: %q", out)
274+
}
275+
if !strings.Contains(out, "ghcr.io/myorg/app") {
276+
t.Errorf("expected ghcr registry ref in output, got: %q", out)
277+
}
278+
if !strings.Contains(out, "--push") {
279+
t.Errorf("expected --push in multi-registry output, got: %q", out)
280+
}
281+
}
282+
283+
// TestRunBuildImage_HardenedNoPushAddsLoad verifies that without --push, hardened
284+
// single-platform buildx invocations include --load so the image is available in
285+
// the local daemon.
286+
func TestRunBuildImage_HardenedNoPushAddsLoad(t *testing.T) {
287+
dir := t.TempDir()
288+
cfg := `ci:
289+
build:
290+
security:
291+
hardened: true
292+
containers:
293+
- name: app
294+
method: dockerfile
295+
dockerfile: Dockerfile
296+
`
297+
cfgPath := filepath.Join(dir, "ci.yaml")
298+
if err := os.WriteFile(cfgPath, []byte(cfg), 0o600); err != nil {
299+
t.Fatal(err)
300+
}
301+
t.Setenv("WFCTL_BUILD_DRY_RUN", "1")
302+
t.Setenv("DOCKER_BUILDKIT", "1")
303+
304+
var buf bytes.Buffer
305+
// Note: no --push flag — default is no push (load only).
306+
if err := runBuildImageWithOutput([]string{"--config", cfgPath}, &buf); err != nil {
307+
t.Fatalf("hardened no-push dry-run: %v", err)
308+
}
309+
310+
out := buf.String()
311+
if !strings.Contains(out, "--load") {
312+
t.Errorf("expected --load in hardened dry-run output without --push flag, got: %q", out)
313+
}
314+
if strings.Contains(out, " --push") {
315+
t.Errorf("expected no --push when push flag is not set, got: %q", out)
316+
}
317+
}
318+
319+
// TestRunBuildImage_HardenedMultiPlatformNoPushNoLoad verifies that multi-platform
320+
// hardened builds without --push do NOT add --load (unsupported by buildx) and leave
321+
// the result in buildkit cache instead.
322+
func TestRunBuildImage_HardenedMultiPlatformNoPushNoLoad(t *testing.T) {
323+
dir := t.TempDir()
324+
cfg := `ci:
325+
build:
326+
security:
327+
hardened: true
328+
containers:
329+
- name: app
330+
method: dockerfile
331+
dockerfile: Dockerfile
332+
platforms:
333+
- linux/amd64
334+
- linux/arm64
335+
`
336+
cfgPath := filepath.Join(dir, "ci.yaml")
337+
if err := os.WriteFile(cfgPath, []byte(cfg), 0o600); err != nil {
338+
t.Fatal(err)
339+
}
340+
t.Setenv("WFCTL_BUILD_DRY_RUN", "1")
341+
t.Setenv("DOCKER_BUILDKIT", "1")
342+
343+
var buf bytes.Buffer
344+
if err := runBuildImageWithOutput([]string{"--config", cfgPath}, &buf); err != nil {
345+
t.Fatalf("hardened multi-platform no-push dry-run: %v", err)
346+
}
347+
348+
out := buf.String()
349+
if strings.Contains(out, "--load") {
350+
t.Errorf("expected no --load for multi-platform no-push (unsupported), got: %q", out)
351+
}
352+
if strings.Contains(out, " --push") {
353+
t.Errorf("expected no --push for multi-platform no-push, got: %q", out)
354+
}
355+
}
160356
func TestRunBuildImage_HardenedUsesBuildx(t *testing.T) {
161357
dir := t.TempDir()
162358
cfg := `ci:

0 commit comments

Comments
 (0)