From fb202ed1b02672b269ffeb924536ac42925fbb10 Mon Sep 17 00:00:00 2001 From: Nick Josevski Date: Fri, 22 May 2026 22:02:14 +1000 Subject: [PATCH] fix: preserve --variable values through interactive runbook run (#582) Command-line --variable arguments to `octopus runbook run` were being silently dropped when the user went through the interactive prompts (without --no-prompt). Inside askRunbookPreviewVariables, the result map was rebuilt from the form preview's controls only, so any CLI variable whose name didn't match a control was discarded. The resulting runbook run fell back to the prompted variables' default values and the printed Automation Command was missing the -v args. This change extracts the variable-resolution logic into a pure helper, resolveRunbookPreviewVariables, and seeds the result map with the caller-supplied CLI variables before processing controls. Variables that do match a control are still canonicalised to the control's name (preserving the prior case-fixing behaviour). Variables without a matching control are now passed through unchanged. Adds unit tests covering: passthrough of unmatched CLI vars, canonicalisation of mismatched casing, no-prompt when CLI satisfies a required control, sensitive-variable tracking, and the still-working prompt path for unprovided required controls. A similar pattern exists in pkg/cmd/release/deploy/deploy.go around line 854; not changed here to keep this PR scoped to the reported bug. Co-Authored-By: Claude Opus 4.7 (1M context) --- pkg/cmd/runbook/run/preview_variables_test.go | 108 ++++++++++++++++++ pkg/cmd/runbook/run/run.go | 30 ++++- 2 files changed, 133 insertions(+), 5 deletions(-) create mode 100644 pkg/cmd/runbook/run/preview_variables_test.go diff --git a/pkg/cmd/runbook/run/preview_variables_test.go b/pkg/cmd/runbook/run/preview_variables_test.go new file mode 100644 index 00000000..dc185ca7 --- /dev/null +++ b/pkg/cmd/runbook/run/preview_variables_test.go @@ -0,0 +1,108 @@ +package run + +import ( + "testing" + + "github.com/AlecAivazis/survey/v2" + "github.com/AlecAivazis/survey/v2/core" + "github.com/OctopusDeploy/cli/pkg/question" + "github.com/OctopusDeploy/go-octopusdeploy/v2/pkg/deployments" + "github.com/OctopusDeploy/go-octopusdeploy/v2/pkg/resources" + "github.com/stretchr/testify/assert" +) + +func noPromptAsker(t *testing.T) question.Asker { + t.Helper() + return func(p survey.Prompt, response interface{}, opts ...survey.AskOpt) error { + t.Fatalf("unexpected prompt: %T %#v", p, p) + return nil + } +} + +func cannedAsker(answer interface{}) question.Asker { + return func(p survey.Prompt, response interface{}, opts ...survey.AskOpt) error { + return core.WriteAnswer(response, "", answer) + } +} + +// Regression test for issue #582: command-line --variable values that don't +// match a runbook form preview control were being silently dropped in +// interactive mode, causing the run to fall back to default values. +func TestResolveRunbookPreviewVariables_PreservesCmdVarWithoutMatchingControl(t *testing.T) { + controls := map[string]*deployments.Control{} + values := map[string]string{} + cmdVars := map[string]string{ + "Approver": "John", + "Signoff": "Jane", + } + + result, sensitive, err := resolveRunbookPreviewVariables(noPromptAsker(t), controls, values, cmdVars) + + assert.NoError(t, err) + assert.Equal(t, map[string]string{"Approver": "John", "Signoff": "Jane"}, result) + assert.Empty(t, sensitive) +} + +func TestResolveRunbookPreviewVariables_CanonicalisesCasingForMatchedControl(t *testing.T) { + approver := deployments.NewControl("VariableValue", "Approver", "", "", false, &resources.DisplaySettings{}) + controls := map[string]*deployments.Control{"elem-1": approver} + values := map[string]string{"elem-1": ""} + cmdVars := map[string]string{"APPROVER": "John"} + + result, _, err := resolveRunbookPreviewVariables(noPromptAsker(t), controls, values, cmdVars) + + assert.NoError(t, err) + assert.Equal(t, map[string]string{"Approver": "John"}, result) +} + +func TestResolveRunbookPreviewVariables_DoesNotPromptWhenCmdVarSuppliedForRequiredControl(t *testing.T) { + approver := deployments.NewControl("VariableValue", "Approver", "", "", true, &resources.DisplaySettings{}) + controls := map[string]*deployments.Control{"elem-1": approver} + values := map[string]string{"elem-1": ""} + cmdVars := map[string]string{"Approver": "John"} + + result, _, err := resolveRunbookPreviewVariables(noPromptAsker(t), controls, values, cmdVars) + + assert.NoError(t, err) + assert.Equal(t, map[string]string{"Approver": "John"}, result) +} + +func TestResolveRunbookPreviewVariables_TracksSensitiveControl(t *testing.T) { + token := deployments.NewControl("VariableValue", "Token", "", "", true, resources.NewDisplaySettings(resources.ControlTypeSensitive, nil)) + controls := map[string]*deployments.Control{"elem-1": token} + values := map[string]string{"elem-1": ""} + cmdVars := map[string]string{"Token": "secret"} + + result, sensitive, err := resolveRunbookPreviewVariables(noPromptAsker(t), controls, values, cmdVars) + + assert.NoError(t, err) + assert.Equal(t, map[string]string{"Token": "secret"}, result) + assert.Equal(t, []string{"Token"}, sensitive) +} + +func TestResolveRunbookPreviewVariables_PromptsForRequiredControlWithoutCmdVar(t *testing.T) { + approver := deployments.NewControl("VariableValue", "Approver", "", "", true, &resources.DisplaySettings{}) + controls := map[string]*deployments.Control{"elem-1": approver} + values := map[string]string{"elem-1": ""} + cmdVars := map[string]string{} + + result, _, err := resolveRunbookPreviewVariables(cannedAsker("John"), controls, values, cmdVars) + + assert.NoError(t, err) + assert.Equal(t, map[string]string{"Approver": "John"}, result) +} + +func TestResolveRunbookPreviewVariables_PreservesUnmatchedAndCanonicalisesMatched(t *testing.T) { + approver := deployments.NewControl("VariableValue", "Approver", "", "", false, &resources.DisplaySettings{}) + controls := map[string]*deployments.Control{"elem-1": approver} + values := map[string]string{"elem-1": ""} + cmdVars := map[string]string{ + "ApprOVER": "John", + "extra": "passthrough", + } + + result, _, err := resolveRunbookPreviewVariables(noPromptAsker(t), controls, values, cmdVars) + + assert.NoError(t, err) + assert.Equal(t, map[string]string{"Approver": "John", "extra": "passthrough"}, result) +} diff --git a/pkg/cmd/runbook/run/run.go b/pkg/cmd/runbook/run/run.go index a05f567a..8c4fcbfb 100644 --- a/pkg/cmd/runbook/run/run.go +++ b/pkg/cmd/runbook/run/run.go @@ -1076,8 +1076,24 @@ func askRunbookPreviewVariables( } } - // Process variables from command line and prompts - result := make(map[string]string) + return resolveRunbookPreviewVariables(asker, flattenedControls, flattenedValues, variablesFromCmd) +} + +// resolveRunbookPreviewVariables merges command-line --variable values with the +// runbook form preview, prompting for any required prompted variables not +// supplied on the command line. Command-line variables that don't match a +// preview control are passed through unchanged (issue #582). +func resolveRunbookPreviewVariables( + asker question.Asker, + flattenedControls map[string]*deployments.Control, + flattenedValues map[string]string, + variablesFromCmd map[string]string, +) (map[string]string, []string, error) { + result := make(map[string]string, len(variablesFromCmd)) + for k, v := range variablesFromCmd { + result[k] = v + } + lcaseVarsFromCmd := make(map[string]string, len(variablesFromCmd)) for k, v := range variablesFromCmd { lcaseVarsFromCmd[strings.ToLower(k)] = v @@ -1088,14 +1104,19 @@ func askRunbookPreviewVariables( return keys[i] > keys[j] }) - // Track sensitive variables sensitiveVars := make([]string, 0) for _, key := range keys { control := flattenedControls[key] valueFromCmd, foundValueOnCommandLine := lcaseVarsFromCmd[strings.ToLower(control.Name)] if foundValueOnCommandLine { - // implicitly fixes up variable casing + // Canonicalise to control.Name when the CLI used a different casing, + // so we don't end up with both spellings in the result map. + for k := range result { + if k != control.Name && strings.EqualFold(k, control.Name) { + delete(result, k) + } + } result[control.Name] = valueFromCmd } if control.Required == true && !foundValueOnCommandLine { @@ -1114,7 +1135,6 @@ func askRunbookPreviewVariables( result[control.Name] = responseString } - // Track sensitive variables from the preview if control.DisplaySettings.ControlType == "Sensitive" { sensitiveVars = append(sensitiveVars, control.Name) }