From 57a144a93683076df5a1455a711d0ed1609a0466 Mon Sep 17 00:00:00 2001 From: Vishal Rana Date: Thu, 29 Oct 2026 08:41:56 -0700 Subject: [PATCH 1/2] fix: make no-op deploy output match its plan --- cmd/ob/commands.go | 9 +- cmd/ob/deploy_noop_test.go | 162 +++++++++++++++++++++++++++++++++ internal/onebox/plan_deploy.go | 12 +++ 3 files changed, 182 insertions(+), 1 deletion(-) create mode 100644 cmd/ob/deploy_noop_test.go diff --git a/cmd/ob/commands.go b/cmd/ob/commands.go index 39b675d7..d0a47c55 100644 --- a/cmd/ob/commands.go +++ b/cmd/ob/commands.go @@ -341,7 +341,11 @@ func renderDeployPlan(cmd *cobra.Command, u *ui.UI, plan onebox.DeployPlan) { u.Infof("proxy: %s", plan.ProxyAction) } - u.Println(u.Bold("operation:")) + if plan.NoOp { + u.Println(u.Bold("operation (with --redeploy only):")) + } else { + u.Println(u.Bold("operation:")) + } u.Println(fmt.Sprintf(" risk=%s reversibility=%s approval=%s", plan.Operation.Risk, plan.Operation.Reversibility, plan.Operation.Approval)) for _, step := range plan.Operation.Steps { @@ -594,6 +598,9 @@ func runMutation(cmd *cobra.Command, g *globalFlags, request onebox.ExecuteReque } } result, err := operationsService(cmd, g).Execute(cmd.Context(), request) + if err == nil && result.NoOp && request.Kind == onebox.KindDeploy && structured == nil { + newUI(cmd, g).Successf("nothing to deploy — %s is current (`--redeploy` forces a fresh roll)", request.Plan.Artifact.HostState.CurrentRelease) + } notifyOperationOutcome(cmd.Context(), cfg, g, verb, result, err) if structured != nil { if outputErr := structured.finish(&result, err); outputErr != nil { diff --git a/cmd/ob/deploy_noop_test.go b/cmd/ob/deploy_noop_test.go new file mode 100644 index 00000000..336b5518 --- /dev/null +++ b/cmd/ob/deploy_noop_test.go @@ -0,0 +1,162 @@ +package main + +import ( + "bytes" + "context" + "encoding/json" + "fmt" + "os" + "path/filepath" + "strings" + "testing" + + "github.com/labstack/onebox/internal/app" + "github.com/labstack/onebox/internal/onebox" + "github.com/labstack/onebox/internal/transport" + "github.com/labstack/onebox/internal/ui" + "github.com/spf13/cobra" +) + +// Use the real planner and executor; only the SSH/host boundary is simulated. +func cliNoOpDeployPlan(t *testing.T) (string, string, onebox.DeployPlan, *transport.Fake) { + t.Helper() + dir := t.TempDir() + config := filepath.Join(dir, "ob.yml") + project := `apiVersion: onebox.run/v1alpha2 +kind: Application +metadata: {name: demo} +spec: + environments: + production: {server: deploy@example.invalid} + workloads: + temporal: {image: 'example/app@sha256:` + strings.Repeat("1", 64) + `', strategy: Recreate} + ui: {image: 'example/app@sha256:` + strings.Repeat("2", 64) + `', strategy: Recreate} + gate: {role: Job, image: 'example/app@sha256:` + strings.Repeat("3", 64) + `', command: 'true', deploymentPhase: PreRelease, dataEffect: None} + deployment: {order: [temporal, ui]} +` + if err := os.WriteFile(config, []byte(project), 0o600); err != nil { + t.Fatal(err) + } + liveCompose, livePayload, containers := "services: {}\n", strings.Repeat("0", 64), "" + fake := &transport.Fake{HostName: "example.invalid", TargetName: "deploy@example.invalid"} + fake.Dynamic = func(command string) (transport.Result, bool) { + switch { + case strings.Contains(command, "/_host/owner"): + return transport.Result{Stdout: "demo production\n"}, true + case strings.Contains(command, "readlink"): + return transport.Result{Stdout: "releases/R0\n"}, true + case strings.Contains(command, "docker ps") && strings.Contains(command, "--format"): + return transport.Result{Stdout: containers}, true + case strings.Contains(command, "docker ps -q"): + return transport.Result{Stdout: "C1\n"}, true + case strings.Contains(command, "docker inspect") && strings.Contains(command, "{{.Image}}"): + return transport.Result{Stdout: "sha256:" + strings.Repeat("a", 64)}, true + case strings.Contains(command, "cat ") && strings.Contains(command, "compose.yaml"): + return transport.Result{Stdout: liveCompose}, true + case strings.Contains(command, "find . -type f"): + return transport.Result{Stdout: livePayload + "\n"}, true + case command == app.BuildxCapabilityCommand: + return transport.Result{Stdout: "Usage: docker buildx imagetools inspect [OPTIONS] NAME\n --format string\n"}, true + case strings.HasPrefix(command, ": onebox-epoch-probe;"): + return transport.Result{ExitCode: app.ProbeAbsent}, true + } + return transport.Result{}, false + } + previous := cliConnector + cliConnector = func(context.Context, transport.Route) (transport.Transport, error) { return fake, nil } + t.Cleanup(func() { cliConnector = previous }) + svc := onebox.New(onebox.Options{ConfigPath: config, Connect: cliConnector}) + first, err := svc.PlanDeploy(t.Context(), onebox.PlanDeployRequest{}) + if err != nil { + t.Fatal(err) + } + liveCompose, livePayload = first.Artifact.RenderedCompose, first.Operation.Binding.PayloadDigest + for _, step := range first.Operation.Steps { + if step.Kind == onebox.StepWorkloadRelease { + containers += fmt.Sprintf("%s|%s|R0|%s|Up (healthy)\n", step.Service, step.Service, step.Revision) + } + } + plan, err := svc.PlanDeploy(t.Context(), onebox.PlanDeployRequest{}) + if err != nil || !plan.NoOp { + t.Fatalf("plan unchanged runtime: no_op=%t err=%v", plan.NoOp, err) + } + path := filepath.Join(dir, "plan.json") + if err := plan.Save(path); err != nil { + t.Fatal(err) + } + fake.Commands = nil + return config, path, plan, fake +} + +func TestNoOpDeployReportsOutcome(t *testing.T) { + for _, mode := range []string{"saved human", "interactive human", "json", "ndjson"} { + t.Run(mode, func(t *testing.T) { + config, path, _, fake := cliNoOpDeployPlan(t) + var stdout, stderr bytes.Buffer + root := newRootCmd() + root.SetOut(&stdout) + root.SetErr(&stderr) + args := []string{"--config", config, "deploy"} + if mode != "interactive human" { + args = append(args, "--plan", path) + } + if mode == "json" || mode == "ndjson" { + args = append(args, "--output", mode) + } + root.SetArgs(args) + if err := root.Execute(); err != nil { + t.Fatalf("deploy: %v\nstderr: %s", err, &stderr) + } + if strings.HasSuffix(mode, "human") { + if !strings.Contains(stdout.String(), "\n✓ nothing to deploy — R0 is current (`--redeploy` forces a fresh roll)\n") { + t.Errorf("missing standalone no-op outcome:\n%s", &stdout) + } + } else { + lines := bytes.Split(bytes.TrimSpace(stdout.Bytes()), []byte("\n")) + body := stdout.Bytes() + if mode == "ndjson" { + body = lines[len(lines)-1] + } + var envelope cliEnvelope + if err := json.Unmarshal(body, &envelope); err != nil || envelope.Outcome != cliOutcomeNoOp { + t.Errorf("no-op envelope = %+v, decode=%v\n%s", envelope, err, &stdout) + } + } + if len(fake.Uploads) != 0 || len(fake.Inputs) != 0 { + t.Errorf("no-op transferred a release: uploads=%v inputs=%v", fake.Uploads, fake.Inputs) + } + for _, command := range fake.Commands { + if strings.Contains(command, "--force-recreate") || strings.Contains(command, " run --rm") || strings.Contains(command, "ln -sfn") { + t.Errorf("no-op mutated application runtime: %s", command) + } + } + }) + } +} + +func TestNoOpPlanShowsRedeployAsConditional(t *testing.T) { + _, _, plan, _ := cliNoOpDeployPlan(t) + // This list is also persisted in the sealed JSON plan, so the condition must + // be established by the planner, not just added by the human CLI renderer. + commands := strings.Join(plan.Artifact.Commands, "\n") + if !strings.HasPrefix(commands, "without --redeploy: no changes — R0 remains current\nwith --redeploy only:\n") { + t.Errorf("no-op plan lists unconditional runtime mutations:\n%s", commands) + } + for _, command := range plan.Artifact.Commands[2:] { + if !strings.HasPrefix(command, " ") { + t.Errorf("command escaped the conditional redeploy branch: %q", command) + } + } + for _, workload := range []string{"temporal", "ui"} { + if !strings.Contains(commands, " release "+workload+" (recreate):") { + t.Errorf("missing conditional fresh roll for %s:\n%s", workload, commands) + } + } + var out bytes.Buffer + cmd := &cobra.Command{} + cmd.SetOut(&out) + renderDeployPlan(cmd, ui.New(&out, false), plan) + if !strings.Contains(out.String(), "operation (with --redeploy only):") { + t.Errorf("no-op graph does not state its execution condition:\n%s", &out) + } +} diff --git a/internal/onebox/plan_deploy.go b/internal/onebox/plan_deploy.go index 1f1439bf..f7f1a282 100644 --- a/internal/onebox/plan_deploy.go +++ b/internal/onebox/plan_deploy.go @@ -180,6 +180,18 @@ func (s *Service) PlanDeploy(ctx context.Context, _ PlanDeployRequest) (DeployPl release.PathsFor(e.Names()).Releases+"/"+releaseID+"/compose.yaml", workloadPlans, ) + if noOp { + // The sealed graph authorizes a fresh roll with --redeploy. Ordinary + // execution takes the no-op branch, so show both branches in the bound + // command list rather than presenting that roll as unconditional. + for index := range commands { + commands[index] = " " + commands[index] + } + commands = append([]string{ + fmt.Sprintf("without --redeploy: no changes — %s remains current", hostState.CurrentRelease), + "with --redeploy only:", + }, commands...) + } artifact := engine.Artifact{ ID: releaseID, App: lp.resolved.Name, Env: s.environment, CreatedAt: now, GitSHA: gitSHA, ConfigHash: configDigest, HostState: hostState, From 9bfda47c1cfe6d572916121453392b4ad0ab1476 Mon Sep 17 00:00:00 2001 From: Vishal Rana Date: Thu, 29 Oct 2026 08:48:35 -0700 Subject: [PATCH 2/2] refactor: share the no-op deploy message --- cmd/ob/commands.go | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/cmd/ob/commands.go b/cmd/ob/commands.go index d0a47c55..a56ed444 100644 --- a/cmd/ob/commands.go +++ b/cmd/ob/commands.go @@ -581,6 +581,10 @@ func sortedNames[V any](m map[string]V) []string { return out } +func renderDeployNoOp(u *ui.UI, currentRelease string) { + u.Successf("nothing to deploy — %s is current (`--redeploy` forces a fresh roll)", currentRelease) +} + func runMutation(cmd *cobra.Command, g *globalFlags, request onebox.ExecuteRequest, verb string) error { cfg, err := notificationConfig(g) if err != nil { @@ -599,7 +603,7 @@ func runMutation(cmd *cobra.Command, g *globalFlags, request onebox.ExecuteReque } result, err := operationsService(cmd, g).Execute(cmd.Context(), request) if err == nil && result.NoOp && request.Kind == onebox.KindDeploy && structured == nil { - newUI(cmd, g).Successf("nothing to deploy — %s is current (`--redeploy` forces a fresh roll)", request.Plan.Artifact.HostState.CurrentRelease) + renderDeployNoOp(newUI(cmd, g), request.Plan.Artifact.HostState.CurrentRelease) } notifyOperationOutcome(cmd.Context(), cfg, g, verb, result, err) if structured != nil { @@ -790,7 +794,7 @@ func runDeploy(cmd *cobra.Command, g *globalFlags, planFile, approvalFile, backu NoRollback: g.NoRollback, Redeploy: redeploy, }) if err == nil && result.NoOp { - pl.ui.Successf("nothing to deploy — %s is current (`--redeploy` forces a fresh roll)", pl.plan.Artifact.HostState.CurrentRelease) + renderDeployNoOp(pl.ui, pl.plan.Artifact.HostState.CurrentRelease) return nil } notifyOperationOutcome(cmd.Context(), pl.cfg, g, "deploy", result, err)