From d25b40c20bbde9417d234542014a7050e059b2d9 Mon Sep 17 00:00:00 2001 From: Nick Josevski Date: Wed, 19 Aug 2026 11:42:06 +1000 Subject: [PATCH 1/6] fix: accept comma-separated values on deployment target and scope flags `--deployment-target "ABC,XYZ"` was sent to the server as a single target name because the flag is a pflag StringArray, while its legacy aliases (`--target`, `--specificMachines`) are StringSlice and already split on commas. Expand comma-separated values for the environment, tenant, tenant-tag and target flags on `release deploy` and `runbook run`, so the comma form matches the repeat-the-flag form. Values that can legitimately contain a comma (--variable, --skip, package/git-resource specs) are left alone. Fixes #556 Co-Authored-By: Claude Opus 5 (1M context) --- pkg/cmd/release/deploy/deploy.go | 17 +++- pkg/cmd/release/deploy/deploy_test.go | 95 +++++++++++++++++++ pkg/cmd/runbook/run/run.go | 17 +++- pkg/cmd/runbook/run/run_test.go | 48 ++++++++++ pkg/executionscommon/executionscommon.go | 23 +++++ pkg/executionscommon/executionscommon_test.go | 30 ++++++ 6 files changed, 220 insertions(+), 10 deletions(-) diff --git a/pkg/cmd/release/deploy/deploy.go b/pkg/cmd/release/deploy/deploy.go index 0df6d614..0bb5c4ea 100644 --- a/pkg/cmd/release/deploy/deploy.go +++ b/pkg/cmd/release/deploy/deploy.go @@ -160,9 +160,9 @@ func NewCmdDeploy(f factory.Factory) *cobra.Command { flags := cmd.Flags() flags.StringVarP(&deployFlags.Project.Value, deployFlags.Project.Name, "p", "", "Name or ID of the project to deploy the release from") flags.StringVarP(&deployFlags.ReleaseVersion.Value, deployFlags.ReleaseVersion.Name, "", "", "Release version to deploy") - flags.StringArrayVarP(&deployFlags.Environments.Value, deployFlags.Environments.Name, "e", nil, "Deploy to this environment (can be specified multiple times)") - flags.StringArrayVarP(&deployFlags.Tenants.Value, deployFlags.Tenants.Name, "", nil, "Deploy to this tenant (can be specified multiple times)") - flags.StringArrayVarP(&deployFlags.TenantTags.Value, deployFlags.TenantTags.Name, "", nil, "Deploy to tenants matching this tag (can be specified multiple times). Format is 'Tag Set Name/Tag Name', such as 'Regions/South'.") + flags.StringArrayVarP(&deployFlags.Environments.Value, deployFlags.Environments.Name, "e", nil, "Deploy to this environment (can be specified multiple times, or as a comma-separated list)") + flags.StringArrayVarP(&deployFlags.Tenants.Value, deployFlags.Tenants.Name, "", nil, "Deploy to this tenant (can be specified multiple times, or as a comma-separated list)") + flags.StringArrayVarP(&deployFlags.TenantTags.Value, deployFlags.TenantTags.Name, "", nil, "Deploy to tenants matching this tag (can be specified multiple times, or as a comma-separated list). Format is 'Tag Set Name/Tag Name', such as 'Regions/South'.") flags.StringVarP(&deployFlags.DeployAt.Value, deployFlags.DeployAt.Name, "", "", "Deploy at a later time. Deploy now if omitted. TODO date formats and timezones!") flags.StringVarP(&deployFlags.MaxQueueTime.Value, deployFlags.MaxQueueTime.Name, "", "", "Cancel the deployment if it hasn't started within this time period.") flags.StringArrayVarP(&deployFlags.Variables.Value, deployFlags.Variables.Name, "v", nil, "Set the value for a prompted variable in the format Label:Value") @@ -170,8 +170,8 @@ func NewCmdDeploy(f factory.Factory) *cobra.Command { flags.StringArrayVarP(&deployFlags.ExcludedSteps.Value, deployFlags.ExcludedSteps.Name, "", nil, "Exclude specific steps from the deployment") flags.StringVarP(&deployFlags.GuidedFailureMode.Value, deployFlags.GuidedFailureMode.Name, "", "", "Enable Guided failure mode (true/false/default)") flags.BoolVarP(&deployFlags.ForcePackageDownload.Value, deployFlags.ForcePackageDownload.Name, "", false, "Force re-download of packages") - flags.StringArrayVarP(&deployFlags.DeploymentTargets.Value, deployFlags.DeploymentTargets.Name, "", nil, "Deploy to this target (can be specified multiple times)") - flags.StringArrayVarP(&deployFlags.ExcludeTargets.Value, deployFlags.ExcludeTargets.Name, "", nil, "Deploy to targets except for this (can be specified multiple times)") + flags.StringArrayVarP(&deployFlags.DeploymentTargets.Value, deployFlags.DeploymentTargets.Name, "", nil, "Deploy to this target (can be specified multiple times, or as a comma-separated list)") + flags.StringArrayVarP(&deployFlags.ExcludeTargets.Value, deployFlags.ExcludeTargets.Name, "", nil, "Deploy to targets except for this (can be specified multiple times, or as a comma-separated list)") flags.StringArrayVarP(&deployFlags.DeploymentFreezeNames.Value, deployFlags.DeploymentFreezeNames.Name, "", nil, "Override this deployment freeze (can be specified multiple times)") flags.StringVarP(&deployFlags.DeploymentFreezeOverrideReason.Value, deployFlags.DeploymentFreezeOverrideReason.Name, "", "", "Reason for overriding a deployment freeze") @@ -198,6 +198,13 @@ func NewCmdDeploy(f factory.Factory) *cobra.Command { } func deployRun(cmd *cobra.Command, f factory.Factory, flags *DeployFlags) error { + // these flags accept a comma-separated list as well as being specified multiple times + flags.Environments.Value = executionscommon.ExpandCommaSeparated(flags.Environments.Value) + flags.Tenants.Value = executionscommon.ExpandCommaSeparated(flags.Tenants.Value) + flags.TenantTags.Value = executionscommon.ExpandCommaSeparated(flags.TenantTags.Value) + flags.DeploymentTargets.Value = executionscommon.ExpandCommaSeparated(flags.DeploymentTargets.Value) + flags.ExcludeTargets.Value = executionscommon.ExpandCommaSeparated(flags.ExcludeTargets.Value) + outputFormat, err := cmd.Flags().GetString(constants.FlagOutputFormat) if err != nil { // should never happen, but fallback if it does outputFormat = constants.OutputFormatTable diff --git a/pkg/cmd/release/deploy/deploy_test.go b/pkg/cmd/release/deploy/deploy_test.go index fde01017..618ec30e 100644 --- a/pkg/cmd/release/deploy/deploy_test.go +++ b/pkg/cmd/release/deploy/deploy_test.go @@ -2006,6 +2006,101 @@ func TestDeployCreate_AutomationMode(t *testing.T) { assert.Equal(t, "ServerTasks-29394\n", stdOut.String()) assert.Equal(t, "", stdErr.String()) }}, + + {"release deploy accepts comma-separated targets and environments; untenanted", func(t *testing.T, api *testutil.MockHttpServer, rootCmd *cobra.Command, stdOut *bytes.Buffer, stdErr *bytes.Buffer) { + cmdReceiver := testutil.GoBegin2(func() (*cobra.Command, error) { + defer api.Close() + rootCmd.SetArgs([]string{ + "release", "deploy", + "--project", fireProject.Name, + "--version", "1.0", + "--environment", "dev,test", // comma form + // mixed form; names containing spaces are preserved, whitespace around the comma is not + "--deployment-target", "first Machine, second Machine", "--deployment-target", "third Machine", + "--exclude-deployment-target", "fourthMachine,fifthMachine", + "--output-format", "basic", // not neccessary, just means we don't need the follow up HTTP requests at the end to print the web link + }) + return rootCmd.ExecuteC() + }) + + api.ExpectRequest(t, "GET", "/api/").RespondWith(rootResource) + api.ExpectRequest(t, "GET", "/api/Spaces-1").RespondWith(rootResource) + api.ExpectRequest(t, "GET", "/api/Spaces-1/projects/"+fireProject.GetName()).RespondWith(fireProject) + + req := api.ExpectRequest(t, "POST", "/api/Spaces-1/deployments/create/untenanted/v1") + requestBody, err := testutil.ReadJson[deployments.CreateDeploymentUntenantedCommandV1](req.Request.Body) + assert.Nil(t, err) + + assert.Equal(t, deployments.CreateDeploymentUntenantedCommandV1{ + ReleaseVersion: "1.0", + EnvironmentNames: []string{"dev", "test"}, + CreateExecutionAbstractCommandV1: deployments.CreateExecutionAbstractCommandV1{ + SpaceID: "Spaces-1", + ProjectIDOrName: fireProject.Name, + SpecificMachineNames: []string{"first Machine", "second Machine", "third Machine"}, + ExcludedMachineNames: []string{"fourthMachine", "fifthMachine"}, + }, + }, requestBody) + + req.RespondWith(&deployments.CreateDeploymentResponseV1{ + DeploymentServerTasks: []*deployments.DeploymentServerTask{ + {DeploymentID: "Deployments-203", ServerTaskID: "ServerTasks-29394"}, + }, + }) + + _, err = testutil.ReceivePair(cmdReceiver) + assert.Nil(t, err) + + assert.Equal(t, "ServerTasks-29394\n", stdOut.String()) + assert.Equal(t, "", stdErr.String()) + }}, + + {"release deploy accepts comma-separated tenants and tenant tags; tenanted", func(t *testing.T, api *testutil.MockHttpServer, rootCmd *cobra.Command, stdOut *bytes.Buffer, stdErr *bytes.Buffer) { + cmdReceiver := testutil.GoBegin2(func() (*cobra.Command, error) { + defer api.Close() + rootCmd.SetArgs([]string{ + "release", "deploy", + "--project", fireProject.Name, + "--version", "1.0", + "--environment", "dev", + "--tenant", "Coke,Pepsi", // comma form + "--tenant-tag", "Region/us-east", "--tenant-tag", "Region/us-west,Region/eu", // mixed form + "--output-format", "basic", + }) + return rootCmd.ExecuteC() + }) + + api.ExpectRequest(t, "GET", "/api/").RespondWith(rootResource) + api.ExpectRequest(t, "GET", "/api/Spaces-1").RespondWith(rootResource) + api.ExpectRequest(t, "GET", "/api/Spaces-1/projects/"+fireProject.GetName()).RespondWith(fireProject) + + req := api.ExpectRequest(t, "POST", "/api/Spaces-1/deployments/create/tenanted/v1") + requestBody, err := testutil.ReadJson[deployments.CreateDeploymentTenantedCommandV1](req.Request.Body) + assert.Nil(t, err) + + assert.Equal(t, deployments.CreateDeploymentTenantedCommandV1{ + ReleaseVersion: "1.0", + EnvironmentName: "dev", + Tenants: []string{"Coke", "Pepsi"}, + TenantTags: []string{"Region/us-east", "Region/us-west", "Region/eu"}, + CreateExecutionAbstractCommandV1: deployments.CreateExecutionAbstractCommandV1{ + SpaceID: "Spaces-1", + ProjectIDOrName: fireProject.Name, + }, + }, requestBody) + + req.RespondWith(&deployments.CreateDeploymentResponseV1{ + DeploymentServerTasks: []*deployments.DeploymentServerTask{ + {DeploymentID: "Deployments-203", ServerTaskID: "ServerTasks-29394"}, + }, + }) + + _, err = testutil.ReceivePair(cmdReceiver) + assert.Nil(t, err) + + assert.Equal(t, "ServerTasks-29394\n", stdOut.String()) + assert.Equal(t, "", stdErr.String()) + }}, } for _, test := range tests { diff --git a/pkg/cmd/runbook/run/run.go b/pkg/cmd/runbook/run/run.go index ad57eb89..83959392 100644 --- a/pkg/cmd/runbook/run/run.go +++ b/pkg/cmd/runbook/run/run.go @@ -162,9 +162,9 @@ func NewCmdRun(f factory.Factory) *cobra.Command { flags.StringVarP(&runFlags.Project.Value, runFlags.Project.Name, "p", "", "Name or ID of the project to run the runbook from") flags.StringVarP(&runFlags.RunbookName.Value, runFlags.RunbookName.Name, "n", "", "Name of the runbook to run") flags.StringArrayVarP(&runFlags.RunbookTags.Value, runFlags.RunbookTags.Name, "", nil, "Run all runbooks matching this tag (can be specified multiple times). Format is 'Tag Set Name/Tag Name'. Mutually exclusive with --name.") - flags.StringArrayVarP(&runFlags.Environments.Value, runFlags.Environments.Name, "e", nil, "Run in this environment (can be specified multiple times)") - flags.StringArrayVarP(&runFlags.Tenants.Value, runFlags.Tenants.Name, "", nil, "Run for this tenant (can be specified multiple times)") - flags.StringArrayVarP(&runFlags.TenantTags.Value, runFlags.TenantTags.Name, "", nil, "Run for tenants matching this tag (can be specified multiple times). Format is 'Tag Set Name/Tag Name', such as 'Regions/South'.") + flags.StringArrayVarP(&runFlags.Environments.Value, runFlags.Environments.Name, "e", nil, "Run in this environment (can be specified multiple times, or as a comma-separated list)") + flags.StringArrayVarP(&runFlags.Tenants.Value, runFlags.Tenants.Name, "", nil, "Run for this tenant (can be specified multiple times, or as a comma-separated list)") + flags.StringArrayVarP(&runFlags.TenantTags.Value, runFlags.TenantTags.Name, "", nil, "Run for tenants matching this tag (can be specified multiple times, or as a comma-separated list). Format is 'Tag Set Name/Tag Name', such as 'Regions/South'.") flags.StringVarP(&runFlags.RunAt.Value, runFlags.RunAt.Name, "", "", "Run at a later time. Run now if omitted. TODO date formats and timezones!") flags.StringVarP(&runFlags.MaxQueueTime.Value, runFlags.MaxQueueTime.Name, "", "", "Cancel a scheduled run if it hasn't started within this time period.") flags.StringArrayVarP(&runFlags.Variables.Value, runFlags.Variables.Name, "v", nil, "Set the value for a prompted variable in the format Label:Value") @@ -172,8 +172,8 @@ func NewCmdRun(f factory.Factory) *cobra.Command { flags.StringArrayVarP(&runFlags.ExcludedSteps.Value, runFlags.ExcludedSteps.Name, "", nil, "Exclude specific steps from the runbook") flags.StringVarP(&runFlags.GuidedFailureMode.Value, runFlags.GuidedFailureMode.Name, "", "", "Enable Guided failure mode (true/false/default)") flags.BoolVarP(&runFlags.ForcePackageDownload.Value, runFlags.ForcePackageDownload.Name, "", false, "Force re-download of packages") - flags.StringArrayVarP(&runFlags.RunTargets.Value, runFlags.RunTargets.Name, "", nil, "Run on this target (can be specified multiple times)") - flags.StringArrayVarP(&runFlags.ExcludeTargets.Value, runFlags.ExcludeTargets.Name, "", nil, "Run on targets except for this (can be specified multiple times)") + flags.StringArrayVarP(&runFlags.RunTargets.Value, runFlags.RunTargets.Name, "", nil, "Run on this target (can be specified multiple times, or as a comma-separated list)") + flags.StringArrayVarP(&runFlags.ExcludeTargets.Value, runFlags.ExcludeTargets.Name, "", nil, "Run on targets except for this (can be specified multiple times, or as a comma-separated list)") flags.StringVarP(&runFlags.GitRef.Value, runFlags.GitRef.Name, "", "", "Git Reference e.g. refs/heads/main. Only relevant for config-as-code projects where runbooks are stored in Git.") flags.StringVarP(&runFlags.PackageVersion.Value, runFlags.PackageVersion.Name, "", "", "Default version to use for all packages. Only relevant for config-as-code projects where runbooks are stored in Git.") flags.StringArrayVarP(&runFlags.PackageVersionSpec.Value, runFlags.PackageVersionSpec.Name, "", nil, "Version specification for a specific package.\nFormat as {package}:{version}, {step}:{version} or {package-ref-name}:{packageOrStep}:{version}\nYou may specify this multiple times.\nOnly relevant for config-as-code projects where runbooks are stored in Git.") @@ -201,6 +201,13 @@ func NewCmdRun(f factory.Factory) *cobra.Command { } func runbookRun(cmd *cobra.Command, f factory.Factory, flags *RunFlags) error { + // these flags accept a comma-separated list as well as being specified multiple times + flags.Environments.Value = executionscommon.ExpandCommaSeparated(flags.Environments.Value) + flags.Tenants.Value = executionscommon.ExpandCommaSeparated(flags.Tenants.Value) + flags.TenantTags.Value = executionscommon.ExpandCommaSeparated(flags.TenantTags.Value) + flags.RunTargets.Value = executionscommon.ExpandCommaSeparated(flags.RunTargets.Value) + flags.ExcludeTargets.Value = executionscommon.ExpandCommaSeparated(flags.ExcludeTargets.Value) + if flags.RunbookName.Value != "" && len(flags.RunbookTags.Value) > 0 { return errors.New("--name and --runbook-tag are mutually exclusive. Please specify either a runbook name or runbook tags, not both") } diff --git a/pkg/cmd/runbook/run/run_test.go b/pkg/cmd/runbook/run/run_test.go index 33c1904d..fc5b872b 100644 --- a/pkg/cmd/runbook/run/run_test.go +++ b/pkg/cmd/runbook/run/run_test.go @@ -338,6 +338,54 @@ func TestRunbookRun_AutomationMode(t *testing.T) { assert.Contains(t, stdOut.String(), "ServerTasks-29394\n") assert.Equal(t, "", stdErr.String()) }}, + + {"runbook run accepts comma-separated environments and targets", func(t *testing.T, api *testutil.MockHttpServer, rootCmd *cobra.Command, stdOut *bytes.Buffer, stdErr *bytes.Buffer) { + cmdReceiver := testutil.GoBegin2(func() (*cobra.Command, error) { + defer api.Close() + rootCmd.SetArgs([]string{ + "runbook", "run", + "--project", "Fire Project", + "--runbook", "Provision Database", + "--environment", "dev,test", // comma form + // mixed form; names containing spaces are preserved, whitespace around the comma is not + "--run-target", "first Machine, second Machine", "--run-target", "third Machine", + "--exclude-run-target", "fourthMachine,fifthMachine", + "--output-format", "basic", + }) + return rootCmd.ExecuteC() + }) + + api.ExpectRequest(t, "GET", "/api/").RespondWith(rootResource) + api.ExpectRequest(t, "GET", "/api/Spaces-1").RespondWith(rootResource) + api.ExpectRequest(t, "GET", "/api/Spaces-1/projects/Fire Project").RespondWithJSON(fixtures.AsServerResponse(fireProject)) + + req := api.ExpectRequest(t, "POST", "/api/Spaces-1/runbook-runs/create/v1") + requestBody, err := testutil.ReadJson[runbooks.RunbookRunCommandV1](req.Request.Body) + assert.Nil(t, err) + + assert.Equal(t, runbooks.RunbookRunCommandV1{ + RunbookName: "Provision Database", + EnvironmentNames: []string{"dev", "test"}, + CreateExecutionAbstractCommandV1: deployments.CreateExecutionAbstractCommandV1{ + SpaceID: "Spaces-1", + ProjectIDOrName: fireProject.Name, + SpecificMachineNames: []string{"first Machine", "second Machine", "third Machine"}, + ExcludedMachineNames: []string{"fourthMachine", "fifthMachine"}, + }, + }, requestBody) + + req.RespondWith(&runbooks.RunbookRunResponseV1{ + RunbookRunServerTasks: []*runbooks.RunbookRunServerTask{ + {RunbookRunID: "RunbookRun-203", ServerTaskID: "ServerTasks-29394"}, + }, + }) + + _, err = testutil.ReceivePair(cmdReceiver) + assert.Nil(t, err) + + assert.Contains(t, stdOut.String(), "ServerTasks-29394\n") + assert.Equal(t, "", stdErr.String()) + }}, } for _, test := range tests { diff --git a/pkg/executionscommon/executionscommon.go b/pkg/executionscommon/executionscommon.go index 4348d7bd..e875557e 100644 --- a/pkg/executionscommon/executionscommon.go +++ b/pkg/executionscommon/executionscommon.go @@ -301,6 +301,29 @@ func AskVariableSpecificPrompt(asker question.Asker, message string, variableTyp } } +// ExpandCommaSeparated splits each entry on commas so `--flag "A,B"` behaves the same as +// `--flag A --flag B`. Whitespace around each entry is trimmed and blank entries are dropped. +// Only apply this to flags whose values cannot legitimately contain a comma; notably NOT to +// --variable, --skip or the package/git-resource specs. +func ExpandCommaSeparated(values []string) []string { + if len(values) == 0 { + return values + } + result := make([]string, 0, len(values)) + for _, value := range values { + for _, component := range strings.Split(value, ",") { + component = strings.TrimSpace(component) + if component != "" { + result = append(result, component) + } + } + } + if len(result) == 0 { + return nil + } + return result +} + func ParseVariableStringArray(variables []string) (map[string]string, error) { result := make(map[string]string, len(variables)) for _, v := range variables { diff --git a/pkg/executionscommon/executionscommon_test.go b/pkg/executionscommon/executionscommon_test.go index 72604be2..26db6a16 100644 --- a/pkg/executionscommon/executionscommon_test.go +++ b/pkg/executionscommon/executionscommon_test.go @@ -412,3 +412,33 @@ func TestToVariableStringArray(t *testing.T) { }) } } + +func TestExpandCommaSeparated(t *testing.T) { + tests := []struct { + name string + input []string + expect []string + }{ + {name: "nil stays nil", input: nil, expect: nil}, + {name: "single value", input: []string{"ABC"}, expect: []string{"ABC"}}, + + {name: "comma form", input: []string{"ABC,XYZ"}, expect: []string{"ABC", "XYZ"}}, + {name: "repeated form", input: []string{"ABC", "XYZ"}, expect: []string{"ABC", "XYZ"}}, + {name: "mixed form", input: []string{"ABC,XYZ", "DEF"}, expect: []string{"ABC", "XYZ", "DEF"}}, + + {name: "preserves spaces within values", input: []string{"Web Server 01,Web Server 02"}, expect: []string{"Web Server 01", "Web Server 02"}}, + {name: "trims spaces around values", input: []string{" ABC ,\tXYZ "}, expect: []string{"ABC", "XYZ"}}, + + {name: "preserves order and duplicates", input: []string{"ABC,ABC"}, expect: []string{"ABC", "ABC"}}, + {name: "tenant tags", input: []string{"Regions/us-east,Regions/us-west"}, expect: []string{"Regions/us-east", "Regions/us-west"}}, + + {name: "drops blank entries", input: []string{"ABC,,XYZ"}, expect: []string{"ABC", "XYZ"}}, + {name: "all blank entries returns nil", input: []string{"", " , "}, expect: nil}, + } + + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + assert.Equal(t, test.expect, executionscommon.ExpandCommaSeparated(test.input)) + }) + } +} From 073cf922d745bc065f75bd4e7802858527935e4c Mon Sep 17 00:00:00 2001 From: Nick Josevski Date: Wed, 19 Aug 2026 11:44:08 +1000 Subject: [PATCH 2/6] fix: report missing package versions instead of a server null reference `release create --no-prompt` sends the create request straight to the server without resolving package versions first. When a package has no version in its feed the server raises a null reference exception, which surfaces as "Octopus API error: Object reference not set to an instance of an object. []". On a 5xx failure the CLI now repeats the package version resolution the server does, and reports the packages, steps and feeds that have no version available. Where it can't identify a specific package, an unhandled server error now carries a hint about the likely causes. Fixes #426 Co-Authored-By: Claude Opus 5 (1M context) --- pkg/cmd/release/create/create.go | 93 +++++++++++- pkg/cmd/release/create/create_test.go | 207 ++++++++++++++++++++++++++ pkg/packages/packages.go | 101 ++++++++++--- 3 files changed, 382 insertions(+), 19 deletions(-) diff --git a/pkg/cmd/release/create/create.go b/pkg/cmd/release/create/create.go index b4b50caa..2f1d835b 100644 --- a/pkg/cmd/release/create/create.go +++ b/pkg/cmd/release/create/create.go @@ -28,6 +28,7 @@ import ( "github.com/OctopusDeploy/cli/pkg/util/flag" "github.com/OctopusDeploy/go-octopusdeploy/v2/pkg/channels" octopusApiClient "github.com/OctopusDeploy/go-octopusdeploy/v2/pkg/client" + "github.com/OctopusDeploy/go-octopusdeploy/v2/pkg/core" "github.com/OctopusDeploy/go-octopusdeploy/v2/pkg/deployments" "github.com/OctopusDeploy/go-octopusdeploy/v2/pkg/feeds" "github.com/OctopusDeploy/go-octopusdeploy/v2/pkg/projects" @@ -318,7 +319,7 @@ func createRun(cmd *cobra.Command, f factory.Factory, flags *CreateFlags) error executor.NewTask(executor.TaskTypeCreateRelease, options), }) if err != nil { - return err + return DiagnoseCreateReleaseFailure(octopus, options, err) } if options.Response != nil { @@ -420,6 +421,96 @@ func BuildPackageVersionBaselineForChannel(octopus *octopusApiClient.Client, dep return result, nil } +// serverNullReferenceMessage is what an Octopus Server sends back when it hits an unhandled +// null reference exception; it carries no information about what actually went wrong. +const serverNullReferenceMessage = "Object reference not set to an instance of an object" + +// DiagnoseCreateReleaseFailure replaces an opaque server-side failure with an actionable message where +// it can. The server raises a null reference exception, surfaced as a bare 500, when it can't select a +// version for a package; see https://github.com/OctopusDeploy/cli/issues/426 +func DiagnoseCreateReleaseFailure(octopus *octopusApiClient.Client, options *executor.TaskOptionsCreateRelease, cause error) error { + var apiError *core.APIError + if !errors.As(cause, &apiError) || apiError.StatusCode < 500 { + return cause + } + + // diagnosis is best-effort; if any part of it fails we must not mask the original failure + if octopus != nil && options != nil { + if missingPackages, findErr := findPackagesWithoutVersions(octopus, options); findErr == nil && len(missingPackages) > 0 { + return packages.NewMissingPackageVersionsError(missingPackages, cause) + } + } + + if strings.Contains(apiError.ErrorMessage, serverNullReferenceMessage) { + return fmt.Errorf("%w\nthe server failed with an unhandled error; this usually means it could not resolve the packages, channel or git reference for the release", cause) + } + return cause +} + +// findPackagesWithoutVersions repeats the package version resolution the server does when it assembles a +// release, so we can report which packages have no version available in their feed. +func findPackagesWithoutVersions(octopus *octopusApiClient.Client, options *executor.TaskOptionsCreateRelease) ([]releases.ReleaseTemplatePackage, error) { + project, err := selectors.FindProject(octopus, options.ProjectName) + if err != nil { + return nil, err + } + + gitReferenceKey := "" + if project.PersistenceSettings != nil && project.PersistenceSettings.Type() == projects.PersistenceSettingsTypeVersionControlled { + gitReferenceKey = options.GitReference + if options.GitCommit != "" { // prefer a specific git commit if one was specified + gitReferenceKey = options.GitCommit + } + } + + deploymentProcess, err := octopus.DeploymentProcesses.Get(project, gitReferenceKey) + if err != nil { + return nil, err + } + + channel, err := findChannelForDiagnosis(octopus, project, options.ChannelName) + if err != nil { + return nil, err + } + + deploymentProcessTemplate, err := octopus.DeploymentProcesses.GetTemplate(deploymentProcess, channel.ID, "") + if err != nil { + return nil, err + } + + packageVersionBaseline, err := BuildPackageVersionBaselineForChannel(octopus, deploymentProcessTemplate, channel) + if err != nil { + return nil, err + } + + overrides := packages.BuildPackageVersionOverrides(packageVersionBaseline, options.DefaultPackageVersion, options.PackageVersionOverrides) + resolvedVersions := packages.ApplyPackageOverrides(packageVersionBaseline, overrides) + + return packages.FindPackagesWithoutVersions(deploymentProcessTemplate.Packages, resolvedVersions), nil +} + +// findChannelForDiagnosis locates the channel the server would have used. When no channel was specified we +// can only guess; the default channel is the best approximation available to us. +func findChannelForDiagnosis(octopus *octopusApiClient.Client, project *projects.Project, channelName string) (*channels.Channel, error) { + if channelName != "" { + return selectors.FindChannel(octopus, project, channelName) + } + + existingChannels, err := octopus.Projects.GetChannels(project) + if err != nil { + return nil, err + } + if len(existingChannels) == 1 { + return existingChannels[0], nil + } + for _, c := range existingChannels { + if c.IsDefault { + return c, nil + } + } + return nil, fmt.Errorf("cannot determine the default channel for project %s", project.GetName()) +} + func AskQuestions(octopus *octopusApiClient.Client, stdout io.Writer, asker question.Asker, options *executor.TaskOptionsCreateRelease) error { if octopus == nil { return cliErrors.NewArgumentNullOrEmptyError("octopus") diff --git a/pkg/cmd/release/create/create_test.go b/pkg/cmd/release/create/create_test.go index 65ee97c7..87078d8e 100644 --- a/pkg/cmd/release/create/create_test.go +++ b/pkg/cmd/release/create/create_test.go @@ -3,6 +3,7 @@ package create_test import ( "bytes" "errors" + "net/http" "net/url" "os" "testing" @@ -19,6 +20,7 @@ import ( "github.com/OctopusDeploy/go-octopusdeploy/v2/pkg/channels" octopusApiClient "github.com/OctopusDeploy/go-octopusdeploy/v2/pkg/client" "github.com/OctopusDeploy/go-octopusdeploy/v2/pkg/constants" + "github.com/OctopusDeploy/go-octopusdeploy/v2/pkg/core" "github.com/OctopusDeploy/go-octopusdeploy/v2/pkg/credentials" "github.com/OctopusDeploy/go-octopusdeploy/v2/pkg/deployments" "github.com/OctopusDeploy/go-octopusdeploy/v2/pkg/feeds" @@ -2829,3 +2831,208 @@ func TestReleaseCreate_ApplyPackageOverride(t *testing.T) { }, result) }) } + +func TestReleaseCreate_FindPackagesWithoutVersions(t *testing.T) { + resolvable := releases.ReleaseTemplatePackage{ + ActionName: "Deploy Website", + FeedID: "feeds-builtin", + FeedName: "Octopus Server (built-in)", + PackageID: "acme-web", + PackageReferenceName: "acme-web", + IsResolvable: true, + } + + t.Run("reports a resolvable package with no version", func(t *testing.T) { + missing := packages.FindPackagesWithoutVersions( + []releases.ReleaseTemplatePackage{resolvable}, + []*packages.StepPackageVersion{{PackageID: "acme-web", ActionName: "Deploy Website", PackageReferenceName: "acme-web", Version: ""}}) + + assert.Equal(t, []releases.ReleaseTemplatePackage{resolvable}, missing) + }) + + t.Run("ignores a package which has a version", func(t *testing.T) { + missing := packages.FindPackagesWithoutVersions( + []releases.ReleaseTemplatePackage{resolvable}, + []*packages.StepPackageVersion{{PackageID: "acme-web", ActionName: "Deploy Website", PackageReferenceName: "acme-web", Version: "1.0.0"}}) + + assert.Equal(t, []releases.ReleaseTemplatePackage{}, missing) + }) + + t.Run("ignores packages which don't need a version at release creation time", func(t *testing.T) { + fixed := resolvable + fixed.FixedVersion = "1.0.0" + unresolvable := resolvable + unresolvable.IsResolvable = false + + missing := packages.FindPackagesWithoutVersions( + []releases.ReleaseTemplatePackage{fixed, unresolvable}, + []*packages.StepPackageVersion{{PackageID: "acme-web", ActionName: "Deploy Website", PackageReferenceName: "acme-web", Version: ""}}) + + assert.Equal(t, []releases.ReleaseTemplatePackage{}, missing) + }) + + t.Run("matches on step and package reference, not just package ID", func(t *testing.T) { + secondStep := resolvable + secondStep.ActionName = "Deploy Worker" + + missing := packages.FindPackagesWithoutVersions( + []releases.ReleaseTemplatePackage{resolvable, secondStep}, + []*packages.StepPackageVersion{ + {PackageID: "acme-web", ActionName: "Deploy Website", PackageReferenceName: "acme-web", Version: "1.0.0"}, + {PackageID: "acme-web", ActionName: "Deploy Worker", PackageReferenceName: "acme-web", Version: ""}, + }) + + assert.Equal(t, []releases.ReleaseTemplatePackage{secondStep}, missing) + }) +} + +func TestReleaseCreate_MissingPackageVersionsError(t *testing.T) { + cause := errors.New("Octopus API error: Object reference not set to an instance of an object. []") + + t.Run("names the package, step and feed", func(t *testing.T) { + err := packages.NewMissingPackageVersionsError([]releases.ReleaseTemplatePackage{{ + ActionName: "Deploy Website", + FeedID: "feeds-builtin", + FeedName: "Octopus Server (built-in)", + PackageID: "acme-web", + PackageReferenceName: "acme-web", + }}, cause) + + assert.EqualError(t, err, heredoc.Doc(` + cannot create release; no version could be found for the following packages: + - 'acme-web' in step 'Deploy Website' (feed 'Octopus Server (built-in)') + push the package(s) to the feed, or supply a version with --package or --package-version`)) + + assert.Equal(t, cause, errors.Unwrap(err)) + }) + + t.Run("qualifies the package with its reference name where they differ", func(t *testing.T) { + err := packages.NewMissingPackageVersionsError([]releases.ReleaseTemplatePackage{{ + ActionName: "Deploy Website", + FeedID: "Feeds-1001", + PackageID: "acme-web", + PackageReferenceName: "extra-config", + }}, cause) + + // no FeedName in this response, so it falls back to the feed ID + assert.EqualError(t, err, heredoc.Doc(` + cannot create release; no version could be found for the following packages: + - 'acme-web/extra-config' in step 'Deploy Website' (feed 'Feeds-1001') + push the package(s) to the feed, or supply a version with --package or --package-version`)) + }) +} + +func TestReleaseCreate_DiagnoseCreateReleaseFailure(t *testing.T) { + t.Run("passes through errors which aren't server faults", func(t *testing.T) { + cause := errors.New("no such host") + assert.Equal(t, cause, create.DiagnoseCreateReleaseFailure(nil, nil, cause)) + + badRequest := &core.APIError{ErrorMessage: "release version 1.0.0 already exists", StatusCode: http.StatusBadRequest} + assert.Equal(t, error(badRequest), create.DiagnoseCreateReleaseFailure(nil, nil, badRequest)) + }) +} + +// issue #426: the server raises a null reference exception rather than telling us that a package +// referenced by the deployment process has no version available in its feed +func TestReleaseCreate_AutomationMode_MissingPackageDiagnosis(t *testing.T) { + const spaceID = "Spaces-1" + const fireProjectID = "Projects-22" + const builtinFeedID = "feeds-builtin" + + space1 := fixtures.NewSpace(spaceID, "Default Space") + depProcess := fixtures.NewDeploymentProcessForProject(spaceID, fireProjectID) + fireProject := fixtures.NewProject(spaceID, fireProjectID, "Fire Project", "Lifecycles-1", "ProjectGroups-1", depProcess.ID) + defaultChannel := fixtures.NewChannel(spaceID, "Channels-1", "Default", fireProjectID) + + nullReferenceError := &core.APIError{ErrorMessage: "Object reference not set to an instance of an object."} + + tests := []struct { + name string + run func(t *testing.T, api *testutil.MockHttpServer, rootCmd *cobra.Command, stdOut *bytes.Buffer, stdErr *bytes.Buffer) + }{ + {"reports the package which has no version in its feed", func(t *testing.T, api *testutil.MockHttpServer, rootCmd *cobra.Command, stdOut *bytes.Buffer, stdErr *bytes.Buffer) { + cmdReceiver := testutil.GoBegin2(func() (*cobra.Command, error) { + defer api.Close() + rootCmd.SetArgs([]string{"release", "create", "--project", fireProject.Name, "--version", "1.0.0"}) + return rootCmd.ExecuteC() + }) + + api.ExpectRequest(t, "GET", "/api/").RespondWith(rootResource) + api.ExpectRequest(t, "GET", "/api/Spaces-1").RespondWith(rootResource) + api.ExpectRequest(t, "GET", "/api/Spaces-1/projects/Fire Project").RespondWith(fireProject) + + api.ExpectRequest(t, "POST", "/api/Spaces-1/releases/create/v1"). + RespondWithStatus(http.StatusInternalServerError, "500 Internal Server Error", nullReferenceError) + + // the CLI now goes back to the server to work out what the real problem was + api.ExpectRequest(t, "GET", "/api/Spaces-1/projects/Fire Project").RespondWith(fireProject) + api.ExpectRequest(t, "GET", "/api/Spaces-1/deploymentprocesses/"+depProcess.ID).RespondWith(depProcess) + api.ExpectRequest(t, "GET", "/api/Spaces-1/projects/"+fireProjectID+"/channels").RespondWith(resources.Resources[*channels.Channel]{ + Items: []*channels.Channel{defaultChannel}, + }) + api.ExpectRequest(t, "GET", "/api/Spaces-1/projects/"+fireProjectID+"/deploymentprocesses/template?channel=Channels-1"). + RespondWith(&deployments.DeploymentProcessTemplate{ + Packages: []releases.ReleaseTemplatePackage{{ + ActionName: "Deploy Website", + FeedID: builtinFeedID, + FeedName: "Octopus Server (built-in)", + PackageID: "acme-web", + PackageReferenceName: "acme-web", + IsResolvable: true, + }}, + }) + api.ExpectRequest(t, "GET", "/api/Spaces-1/feeds?ids="+builtinFeedID+"&take=1").RespondWith(&feeds.Feeds{Items: []feeds.IFeed{ + &feeds.FeedResource{Name: "Octopus Server (built-in)", FeedType: feeds.FeedTypeBuiltIn, Resource: resources.Resource{ + ID: builtinFeedID, + Links: map[string]string{ + constants.LinkSearchPackageVersionsTemplate: "/api/Spaces-1/feeds/feeds-builtin/packages/versions{?packageId,take,skip,includePreRelease,versionRange,preReleaseTag,filter,includeReleaseNotes}", + }}}, + }}) + api.ExpectRequest(t, "GET", "/api/Spaces-1/feeds/feeds-builtin/packages/versions?packageId=acme-web&take=1"). + RespondWith(&resources.Resources[*octopusPackages.PackageVersion]{Items: []*octopusPackages.PackageVersion{}}) + + _, err := testutil.ReceivePair(cmdReceiver) + assert.EqualError(t, err, heredoc.Doc(` + cannot create release; no version could be found for the following packages: + - 'acme-web' in step 'Deploy Website' (feed 'Octopus Server (built-in)') + push the package(s) to the feed, or supply a version with --package or --package-version`)) + + assert.Equal(t, "", stdOut.String()) + }}, + + {"falls back to a hint when it can't identify a missing package", func(t *testing.T, api *testutil.MockHttpServer, rootCmd *cobra.Command, stdOut *bytes.Buffer, stdErr *bytes.Buffer) { + cmdReceiver := testutil.GoBegin2(func() (*cobra.Command, error) { + defer api.Close() + rootCmd.SetArgs([]string{"release", "create", "--project", fireProject.Name}) + return rootCmd.ExecuteC() + }) + + api.ExpectRequest(t, "GET", "/api/").RespondWith(rootResource) + api.ExpectRequest(t, "GET", "/api/Spaces-1").RespondWith(rootResource) + api.ExpectRequest(t, "GET", "/api/Spaces-1/projects/Fire Project").RespondWith(fireProject) + + api.ExpectRequest(t, "POST", "/api/Spaces-1/releases/create/v1"). + RespondWithStatus(http.StatusInternalServerError, "500 Internal Server Error", nullReferenceError) + + // the diagnosis is best-effort; this server can't tell us about the deployment process + api.ExpectRequest(t, "GET", "/api/Spaces-1/projects/Fire Project").RespondWith(fireProject) + api.ExpectRequest(t, "GET", "/api/Spaces-1/deploymentprocesses/"+depProcess.ID).RespondWithStatus(http.StatusNotFound, "404 Not Found", nil) + + _, err := testutil.ReceivePair(cmdReceiver) + assert.EqualError(t, err, "Octopus API error: Object reference not set to an instance of an object. [] \nthe server failed with an unhandled error; this usually means it could not resolve the packages, channel or git reference for the release") + }}, + } + + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + stdout, stderr := &bytes.Buffer{}, &bytes.Buffer{} + api := testutil.NewMockHttpServer() + + rootCmd := cmdRoot.NewCmdRoot(testutil.NewMockFactoryWithSpace(api, space1), nil, nil) + rootCmd.SetOut(stdout) + rootCmd.SetErr(stderr) + + test.run(t, api, rootCmd, stdout, stderr) + }) + } +} diff --git a/pkg/packages/packages.go b/pkg/packages/packages.go index 3eff889a..e32d0986 100644 --- a/pkg/packages/packages.go +++ b/pkg/packages/packages.go @@ -180,6 +180,88 @@ func BuildPackageVersionBaseline(octopus *octopusApiClient.Client, packages []re return result, nil } +// FindPackagesWithoutVersions returns the deployment process template packages which the server +// expects to have a version at release creation time, but for which no version could be found in the feed. +// Packages with a fixed version, or which aren't resolvable until deployment time, are excluded because +// they don't need one. +func FindPackagesWithoutVersions(templatePackages []releases.ReleaseTemplatePackage, resolvedVersions []*StepPackageVersion) []releases.ReleaseTemplatePackage { + result := make([]releases.ReleaseTemplatePackage, 0) + for _, templatePackage := range templatePackages { + if templatePackage.FixedVersion != "" || !templatePackage.IsResolvable { + continue + } + for _, resolved := range resolvedVersions { + if resolved.PackageID == templatePackage.PackageID && + resolved.ActionName == templatePackage.ActionName && + resolved.PackageReferenceName == templatePackage.PackageReferenceName { + if strings.TrimSpace(resolved.Version) == "" { + result = append(result, templatePackage) + } + break + } + } + } + return result +} + +// MissingPackageVersionsError is raised when one or more packages referenced by the deployment process +// have no version available in their feed. The server can't assemble a release in this state; rather than +// reporting that, it raises a null reference exception, so the CLI detects the situation itself. +type MissingPackageVersionsError struct { + Packages []releases.ReleaseTemplatePackage + cause error +} + +func NewMissingPackageVersionsError(missingPackages []releases.ReleaseTemplatePackage, cause error) *MissingPackageVersionsError { + return &MissingPackageVersionsError{Packages: missingPackages, cause: cause} +} + +func (e *MissingPackageVersionsError) Unwrap() error { return e.cause } + +func (e *MissingPackageVersionsError) Error() string { + sb := &strings.Builder{} + sb.WriteString("cannot create release; no version could be found for the following packages:") + for _, p := range e.Packages { + packageName := p.PackageID + if p.PackageReferenceName != "" && p.PackageReferenceName != p.PackageID { + packageName = fmt.Sprintf("%s/%s", packageName, p.PackageReferenceName) + } + feedName := p.FeedName + if feedName == "" { + feedName = p.FeedID + } + sb.WriteString(fmt.Sprintf("\n - '%s' in step '%s' (feed '%s')", packageName, p.ActionName, feedName)) + } + sb.WriteString("\npush the package(s) to the feed, or supply a version with --package or --package-version") + return sb.String() +} + +// BuildPackageVersionOverrides converts the --package-version and --package command line flags into +// resolved overrides, using the baseline to work out which step or package each override refers to. +// Anything that can't be parsed or resolved is ignored; the server reports those. +func BuildPackageVersionOverrides(packageVersionBaseline []*StepPackageVersion, defaultPackageVersion string, packageOverrideFlags []string) []*PackageVersionOverride { + packageVersionOverrides := make([]*PackageVersionOverride, 0, len(packageOverrideFlags)+1) + + if defaultPackageVersion != "" { + // blind apply to everything + packageVersionOverrides = append(packageVersionOverrides, &PackageVersionOverride{Version: defaultPackageVersion}) + } + + for _, s := range packageOverrideFlags { + ambOverride, err := ParsePackageOverrideString(s) + if err != nil { + continue // silently ignore anything that wasn't parseable (should we emit a warning?) + } + resolvedOverride, err := ResolvePackageOverride(ambOverride, packageVersionBaseline) + if err != nil { + continue // silently ignore anything that wasn't parseable (should we emit a warning?) + } + packageVersionOverrides = append(packageVersionOverrides, resolvedOverride) + } + + return packageVersionOverrides +} + type PackageVersionOverride struct { ActionName string // optional, but one or both of ActionName or PackageID must be supplied PackageID string // optional, but one or both of ActionName or PackageID must be supplied @@ -539,25 +621,8 @@ func AskPackageOverrideLoop( initialPackageOverrideFlags []string, // the --package command line flag (multiple occurrences) asker question.Asker, stdout io.Writer) ([]*StepPackageVersion, []*PackageVersionOverride, error) { - packageVersionOverrides := make([]*PackageVersionOverride, 0) - // pickup any partial package specifications that may have arrived on the commandline - if defaultPackageVersion != "" { - // blind apply to everything - packageVersionOverrides = append(packageVersionOverrides, &PackageVersionOverride{Version: defaultPackageVersion}) - } - - for _, s := range initialPackageOverrideFlags { - ambOverride, err := ParsePackageOverrideString(s) - if err != nil { - continue // silently ignore anything that wasn't parseable (should we emit a warning?) - } - resolvedOverride, err := ResolvePackageOverride(ambOverride, packageVersionBaseline) - if err != nil { - continue // silently ignore anything that wasn't parseable (should we emit a warning?) - } - packageVersionOverrides = append(packageVersionOverrides, resolvedOverride) - } + packageVersionOverrides := BuildPackageVersionOverrides(packageVersionBaseline, defaultPackageVersion, initialPackageOverrideFlags) overriddenPackageVersions := ApplyPackageOverrides(packageVersionBaseline, packageVersionOverrides) From af509e5b63b4cb4edf1b754af1a67fc45561ed46 Mon Sep 17 00:00:00 2001 From: Nick Josevski Date: Wed, 19 Aug 2026 11:45:14 +1000 Subject: [PATCH 3/6] fix: report unknown release versions instead of a server null reference `release deploy` passed --version straight to the executions API, which answers an unknown version with "Object reference not set to an instance of an object". Resolve the release before deploying so a version that doesn't exist is reported by name, and call out `latest` explicitly since it is not a supported alias. Refs #294 Co-Authored-By: Claude Opus 5 (1M context) --- pkg/cmd/release/deploy/deploy.go | 12 +++- pkg/cmd/release/deploy/deploy_test.go | 76 +++++++++++++------- pkg/cmd/release/progression/shared/shared.go | 11 +-- pkg/question/selectors/releases.go | 45 ++++++++++++ 4 files changed, 109 insertions(+), 35 deletions(-) create mode 100644 pkg/question/selectors/releases.go diff --git a/pkg/cmd/release/deploy/deploy.go b/pkg/cmd/release/deploy/deploy.go index 0df6d614..cc314649 100644 --- a/pkg/cmd/release/deploy/deploy.go +++ b/pkg/cmd/release/deploy/deploy.go @@ -317,6 +317,16 @@ func deployRun(cmd *cobra.Command, f factory.Factory, flags *DeployFlags) error return err } options.ProjectName = project.GetName() + + if options.ReleaseVersion != "" { + // resolve the release up front; the executions API reports an unknown version as an + // unhelpful null reference error, and having the ID saves looking it up again later + release, err := selectors.FindRelease(octopus, f.GetCurrentSpace().ID, project, options.ReleaseVersion) + if err != nil { + return err + } + options.ReleaseID = release.ID + } } } @@ -426,7 +436,7 @@ func AskQuestions(octopus *octopusApiClient.Client, stdout io.Writer, asker ques return err } } else { - selectedRelease, err = releases.GetReleaseInProject(octopus, space.ID, selectedProject.ID, options.ReleaseVersion) + selectedRelease, err = selectors.FindRelease(octopus, space.ID, selectedProject, options.ReleaseVersion) if err != nil { return err } diff --git a/pkg/cmd/release/deploy/deploy_test.go b/pkg/cmd/release/deploy/deploy_test.go index fde01017..5baade63 100644 --- a/pkg/cmd/release/deploy/deploy_test.go +++ b/pkg/cmd/release/deploy/deploy_test.go @@ -1594,6 +1594,7 @@ func TestDeployCreate_AutomationMode(t *testing.T) { api.ExpectRequest(t, "GET", "/api/").RespondWith(rootResource) api.ExpectRequest(t, "GET", "/api/Spaces-1").RespondWith(rootResource) api.ExpectRequest(t, "GET", "/api/Spaces-1/projects/"+fireProject.GetName()).RespondWith(fireProject) + api.ExpectRequest(t, "GET", "/api/Spaces-1/projects/"+fireProjectID+"/releases/1.9").RespondWith(release10) _, err := testutil.ReceivePair(cmdReceiver) assert.EqualError(t, err, "environment(s) must be specified") @@ -1602,6 +1603,45 @@ func TestDeployCreate_AutomationMode(t *testing.T) { assert.Equal(t, "", stdErr.String()) }}, + {"release deploy reports a release version that doesn't exist", func(t *testing.T, api *testutil.MockHttpServer, rootCmd *cobra.Command, stdOut *bytes.Buffer, stdErr *bytes.Buffer) { + cmdReceiver := testutil.GoBegin2(func() (*cobra.Command, error) { + defer api.Close() + rootCmd.SetArgs([]string{"release", "deploy", "--project", fireProject.Name, "--version", "9.9", "--environment", "dev"}) + return rootCmd.ExecuteC() + }) + + api.ExpectRequest(t, "GET", "/api/").RespondWith(rootResource) + api.ExpectRequest(t, "GET", "/api/Spaces-1").RespondWith(rootResource) + api.ExpectRequest(t, "GET", "/api/Spaces-1/projects/"+fireProject.GetName()).RespondWith(fireProject) + api.ExpectRequest(t, "GET", "/api/Spaces-1/projects/"+fireProjectID+"/releases/9.9"). + RespondWithStatus(404, "404 Not Found", &core.APIError{ErrorMessage: "The resource you requested was not found."}) + + _, err := testutil.ReceivePair(cmdReceiver) + assert.EqualError(t, err, "cannot find a release with version '9.9' in project 'Fire Project'") + + assert.Equal(t, "", stdOut.String()) + assert.Equal(t, "", stdErr.String()) + }}, + + {"release deploy explains that 'latest' is not a supported release version", func(t *testing.T, api *testutil.MockHttpServer, rootCmd *cobra.Command, stdOut *bytes.Buffer, stdErr *bytes.Buffer) { + cmdReceiver := testutil.GoBegin2(func() (*cobra.Command, error) { + defer api.Close() + rootCmd.SetArgs([]string{"release", "deploy", "--project", fireProject.Name, "--version", "latest", "--environment", "dev"}) + return rootCmd.ExecuteC() + }) + + api.ExpectRequest(t, "GET", "/api/").RespondWith(rootResource) + api.ExpectRequest(t, "GET", "/api/Spaces-1").RespondWith(rootResource) + api.ExpectRequest(t, "GET", "/api/Spaces-1/projects/"+fireProject.GetName()).RespondWith(fireProject) + api.ExpectRequest(t, "GET", "/api/Spaces-1/projects/"+fireProjectID+"/releases/latest").RespondWithStatus(404, "NotFound", nil) + + _, err := testutil.ReceivePair(cmdReceiver) + assert.EqualError(t, err, "cannot find a release with version 'latest' in project 'Fire Project'; 'latest' is not a supported alias, specify an exact version. Run 'octopus release list --project \"Fire Project\"' to see the available versions") + + assert.Equal(t, "", stdOut.String()) + assert.Equal(t, "", stdErr.String()) + }}, + {"release deploy specifying project, version, env only (bare minimum) assuming untenanted", func(t *testing.T, api *testutil.MockHttpServer, rootCmd *cobra.Command, stdOut *bytes.Buffer, stdErr *bytes.Buffer) { cmdReceiver := testutil.GoBegin2(func() (*cobra.Command, error) { defer api.Close() @@ -1612,6 +1652,7 @@ func TestDeployCreate_AutomationMode(t *testing.T) { api.ExpectRequest(t, "GET", "/api/").RespondWith(rootResource) api.ExpectRequest(t, "GET", "/api/Spaces-1").RespondWith(rootResource) api.ExpectRequest(t, "GET", "/api/Spaces-1/projects/"+fireProject.GetName()).RespondWith(fireProject) + api.ExpectRequest(t, "GET", "/api/Spaces-1/projects/"+fireProjectID+"/releases/1.0").RespondWith(release10) // Note: because we didn't specify --tenant or --tenant-tag, automation-mode code is going to assume untenanted req := api.ExpectRequest(t, "POST", "/api/Spaces-1/deployments/create/untenanted/v1") @@ -1634,12 +1675,7 @@ func TestDeployCreate_AutomationMode(t *testing.T) { }, }) - // now it's going to try and look up the project/version to generate the web URL - api.ExpectRequest(t, "GET", "/api/Spaces-1/projects/Fire Project").RespondWithStatus(404, "NotFound", nil) - api.ExpectRequest(t, "GET", "/api/Spaces-1/projects?partialName=Fire+Project").RespondWith(resources.Resources[*projects.Project]{ - Items: []*projects.Project{fireProject}, - }) - api.ExpectRequest(t, "GET", "/api/Spaces-1/projects/"+fireProjectID+"/releases/1.0").RespondWith(release10) + // no lookup to generate the web URL; the release was already resolved before deploying _, err = testutil.ReceivePair(cmdReceiver) assert.Nil(t, err) @@ -1662,6 +1698,7 @@ func TestDeployCreate_AutomationMode(t *testing.T) { api.ExpectRequest(t, "GET", "/api/").RespondWith(rootResource) api.ExpectRequest(t, "GET", "/api/Spaces-1").RespondWith(rootResource) api.ExpectRequest(t, "GET", "/api/Spaces-1/projects/"+fireProject.GetName()).RespondWith(fireProject) + api.ExpectRequest(t, "GET", "/api/Spaces-1/projects/"+fireProjectID+"/releases/2.1").RespondWith(release10) // Note: because we didn't specify --tenant or --tenant-tag, automation-mode code is going to assume untenanted req := api.ExpectRequest(t, "POST", "/api/Spaces-1/deployments/create/untenanted/v1") @@ -1684,12 +1721,7 @@ func TestDeployCreate_AutomationMode(t *testing.T) { }, }) - // now it's going to try and look up the project/version to generate the web URL - api.ExpectRequest(t, "GET", "/api/Spaces-1/projects/Fire Project").RespondWithStatus(404, "NotFound", nil) - api.ExpectRequest(t, "GET", "/api/Spaces-1/projects?partialName=Fire+Project").RespondWith(resources.Resources[*projects.Project]{ - Items: []*projects.Project{fireProject}, - }) - api.ExpectRequest(t, "GET", "/api/Spaces-1/projects/"+fireProjectID+"/releases/2.1").RespondWith(release10) + // no lookup to generate the web URL; the release was already resolved before deploying _, err = testutil.ReceivePair(cmdReceiver) assert.Nil(t, err) @@ -1712,6 +1744,7 @@ func TestDeployCreate_AutomationMode(t *testing.T) { api.ExpectRequest(t, "GET", "/api/").RespondWith(rootResource) api.ExpectRequest(t, "GET", "/api/Spaces-1").RespondWith(rootResource) api.ExpectRequest(t, "GET", "/api/Spaces-1/projects/"+fireProject.GetName()).RespondWith(fireProject) + api.ExpectRequest(t, "GET", "/api/Spaces-1/projects/"+fireProjectID+"/releases/1.0").RespondWith(release10) // Note: because we didn't specify --tenant or --tenant-tag, automation-mode code is going to assume untenanted api.ExpectRequest(t, "POST", "/api/Spaces-1/deployments/create/untenanted/v1").RespondWith(&deployments.CreateDeploymentResponseV1{ @@ -1742,6 +1775,7 @@ func TestDeployCreate_AutomationMode(t *testing.T) { api.ExpectRequest(t, "GET", "/api/").RespondWith(rootResource) api.ExpectRequest(t, "GET", "/api/Spaces-1").RespondWith(rootResource) api.ExpectRequest(t, "GET", "/api/Spaces-1/projects/"+fireProject.GetName()).RespondWith(fireProject) + api.ExpectRequest(t, "GET", "/api/Spaces-1/projects/"+fireProjectID+"/releases/1.0").RespondWith(release10) // Note: because we didn't specify --tenant or --tenant-tag, automation-mode code is going to assume untenanted serverTasks := []*deployments.DeploymentServerTask{ @@ -1773,6 +1807,7 @@ func TestDeployCreate_AutomationMode(t *testing.T) { api.ExpectRequest(t, "GET", "/api/").RespondWith(rootResource) api.ExpectRequest(t, "GET", "/api/Spaces-1").RespondWith(rootResource) api.ExpectRequest(t, "GET", "/api/Spaces-1/projects/"+fireProject.GetName()).RespondWith(fireProject) + api.ExpectRequest(t, "GET", "/api/Spaces-1/projects/"+fireProjectID+"/releases/1.0").RespondWith(release10) req := api.ExpectRequest(t, "POST", "/api/Spaces-1/deployments/create/tenanted/v1") requestBody, err := testutil.ReadJson[deployments.CreateDeploymentTenantedCommandV1](req.Request.Body) @@ -1794,12 +1829,7 @@ func TestDeployCreate_AutomationMode(t *testing.T) { }, }) - // now it's going to try and look up the project/version to generate the web URL - api.ExpectRequest(t, "GET", "/api/Spaces-1/projects/Fire Project").RespondWithStatus(404, "NotFound", nil) - api.ExpectRequest(t, "GET", "/api/Spaces-1/projects?partialName=Fire+Project").RespondWith(resources.Resources[*projects.Project]{ - Items: []*projects.Project{fireProject}, - }) - api.ExpectRequest(t, "GET", "/api/Spaces-1/projects/"+fireProjectID+"/releases/1.0").RespondWith(release10) + // no lookup to generate the web URL; the release was already resolved before deploying _, err = testutil.ReceivePair(cmdReceiver) assert.Nil(t, err) @@ -1822,6 +1852,7 @@ func TestDeployCreate_AutomationMode(t *testing.T) { api.ExpectRequest(t, "GET", "/api/").RespondWith(rootResource) api.ExpectRequest(t, "GET", "/api/Spaces-1").RespondWith(rootResource) api.ExpectRequest(t, "GET", "/api/Spaces-1/projects/"+fireProject.GetName()).RespondWith(fireProject) + api.ExpectRequest(t, "GET", "/api/Spaces-1/projects/"+fireProjectID+"/releases/1.0").RespondWith(release10) req := api.ExpectRequest(t, "POST", "/api/Spaces-1/deployments/create/tenanted/v1") requestBody, err := testutil.ReadJson[deployments.CreateDeploymentTenantedCommandV1](req.Request.Body) @@ -1843,12 +1874,7 @@ func TestDeployCreate_AutomationMode(t *testing.T) { }, }) - // now it's going to try and look up the project/version to generate the web URL - api.ExpectRequest(t, "GET", "/api/Spaces-1/projects/Fire Project").RespondWithStatus(404, "NotFound", nil) - api.ExpectRequest(t, "GET", "/api/Spaces-1/projects?partialName=Fire+Project").RespondWith(resources.Resources[*projects.Project]{ - Items: []*projects.Project{fireProject}, - }) - api.ExpectRequest(t, "GET", "/api/Spaces-1/projects/"+fireProjectID+"/releases/1.0").RespondWith(release10) + // no lookup to generate the web URL; the release was already resolved before deploying _, err = testutil.ReceivePair(cmdReceiver) assert.Nil(t, err) @@ -1888,6 +1914,7 @@ func TestDeployCreate_AutomationMode(t *testing.T) { api.ExpectRequest(t, "GET", "/api/").RespondWith(rootResource) api.ExpectRequest(t, "GET", "/api/Spaces-1").RespondWith(rootResource) api.ExpectRequest(t, "GET", "/api/Spaces-1/projects/"+fireProject.GetName()).RespondWith(fireProject) + api.ExpectRequest(t, "GET", "/api/Spaces-1/projects/"+fireProjectID+"/releases/1.0").RespondWith(release10) // Note: because we didn't specify --tenant or --tenant-tag, automation-mode code is going to assume untenanted req := api.ExpectRequest(t, "POST", "/api/Spaces-1/deployments/create/untenanted/v1") @@ -1962,6 +1989,7 @@ func TestDeployCreate_AutomationMode(t *testing.T) { api.ExpectRequest(t, "GET", "/api/").RespondWith(rootResource) api.ExpectRequest(t, "GET", "/api/Spaces-1").RespondWith(rootResource) api.ExpectRequest(t, "GET", "/api/Spaces-1/projects/"+fireProject.GetName()).RespondWith(fireProject) + api.ExpectRequest(t, "GET", "/api/Spaces-1/projects/"+fireProjectID+"/releases/1.0").RespondWith(release10) req := api.ExpectRequest(t, "POST", "/api/Spaces-1/deployments/create/tenanted/v1") requestBody, err := testutil.ReadJson[deployments.CreateDeploymentTenantedCommandV1](req.Request.Body) diff --git a/pkg/cmd/release/progression/shared/shared.go b/pkg/cmd/release/progression/shared/shared.go index 94f669b3..a181a771 100644 --- a/pkg/cmd/release/progression/shared/shared.go +++ b/pkg/cmd/release/progression/shared/shared.go @@ -40,14 +40,5 @@ func SelectRelease(octopus *client.Client, project *projects.Project, ask questi } func FindRelease(octopus *client.Client, project *projects.Project, version string) (*releases.Release, error) { - existingRelease, err := releases.GetReleaseInProject(octopus, octopus.GetSpaceID(), project.GetID(), version) - if err != nil { - return nil, err - } - - if existingRelease == nil { - return nil, fmt.Errorf("unable to locate a release with version/release number '%s'", version) - } - - return existingRelease, nil + return selectors.FindRelease(octopus, octopus.GetSpaceID(), project, version) } diff --git a/pkg/question/selectors/releases.go b/pkg/question/selectors/releases.go new file mode 100644 index 00000000..04e44cb0 --- /dev/null +++ b/pkg/question/selectors/releases.go @@ -0,0 +1,45 @@ +package selectors + +import ( + "errors" + "fmt" + "net/http" + "strings" + + "github.com/OctopusDeploy/cli/pkg/constants" + octopusApiClient "github.com/OctopusDeploy/go-octopusdeploy/v2/pkg/client" + "github.com/OctopusDeploy/go-octopusdeploy/v2/pkg/core" + "github.com/OctopusDeploy/go-octopusdeploy/v2/pkg/projects" + "github.com/OctopusDeploy/go-octopusdeploy/v2/pkg/releases" +) + +// latestReleaseAlias is the value the old `octo` CLI accepted to mean "the newest release". +// This CLI has no equivalent, so it is called out explicitly when the lookup fails. +const latestReleaseAlias = "latest" + +// FindRelease looks up a release by version within a project. A version that doesn't exist is +// reported here, because the executions API answers one with a null reference error instead. +func FindRelease(octopus *octopusApiClient.Client, spaceID string, project *projects.Project, releaseVersion string) (*releases.Release, error) { + release, err := releases.GetReleaseInProject(octopus, spaceID, project.GetID(), releaseVersion) + if err != nil { + var apiError *core.APIError + if errors.As(err, &apiError) && apiError.StatusCode == http.StatusNotFound { + return nil, releaseNotFoundError(project, releaseVersion) + } + return nil, err + } + // a 404 with an empty body doesn't reach the error path above; it decodes as an empty release + if release == nil || release.GetID() == "" { + return nil, releaseNotFoundError(project, releaseVersion) + } + + return release, nil +} + +func releaseNotFoundError(project *projects.Project, releaseVersion string) error { + if strings.EqualFold(releaseVersion, latestReleaseAlias) { + return fmt.Errorf("cannot find a release with version '%s' in project '%s'; '%s' is not a supported alias, specify an exact version. Run '%s release list --project \"%s\"' to see the available versions", + releaseVersion, project.GetName(), releaseVersion, constants.ExecutableName, project.GetName()) + } + return fmt.Errorf("cannot find a release with version '%s' in project '%s'", releaseVersion, project.GetName()) +} From e083816260f4429a38444d19d8a86bfb11beb069 Mon Sep 17 00:00:00 2001 From: Nick Josevski Date: Wed, 19 Aug 2026 12:24:27 +1000 Subject: [PATCH 4/6] fix: accept IDs as well as names for --channel, --environment and --tenant The executions API only matches channels, environments and tenants by name, so `release create`, `release deploy` and `runbook run` passed whatever the caller typed straight through and the server rejected IDs. `--project` already worked because the server accepts a project ID or name. Resolve those identifiers client side through the shared selectors package before handing them to the executor, preferring an ID match over a name match so it behaves the same way as `--project`. Fixes #250 Co-Authored-By: Claude Opus 5 (1M context) --- pkg/cmd/channel/delete/delete_test.go | 2 +- pkg/cmd/channel/view/view_test.go | 4 +- pkg/cmd/release/create/create.go | 8 + pkg/cmd/release/create/create_test.go | 64 ++++++++ pkg/cmd/release/deploy/deploy.go | 53 +++++- pkg/cmd/release/deploy/deploy_test.go | 87 +++++++++- pkg/cmd/runbook/run/run.go | 18 +++ pkg/cmd/runbook/run/run_test.go | 78 +++++++++ pkg/executionscommon/executionscommon.go | 39 +---- pkg/question/selectors/channels.go | 13 +- pkg/question/selectors/environments.go | 54 +++++-- pkg/question/selectors/find_test.go | 198 +++++++++++++++++++++++ pkg/question/selectors/tenants.go | 38 +++++ 13 files changed, 590 insertions(+), 66 deletions(-) create mode 100644 pkg/question/selectors/find_test.go create mode 100644 pkg/question/selectors/tenants.go diff --git a/pkg/cmd/channel/delete/delete_test.go b/pkg/cmd/channel/delete/delete_test.go index d4a57197..eab6c2ce 100644 --- a/pkg/cmd/channel/delete/delete_test.go +++ b/pkg/cmd/channel/delete/delete_test.go @@ -167,7 +167,7 @@ func TestChannelDelete(t *testing.T) { // No DELETE request is expected; api.Close() asserts nothing further was requested. _, err := testutil.ReceivePair(cmdReceiver) - assert.EqualError(t, err, "no channel found with name of Channels-99") + assert.EqualError(t, err, "cannot find a channel in project 'Fire Project' with the ID or name of 'Channels-99'") assert.Equal(t, "", stdErr.String()) }}, diff --git a/pkg/cmd/channel/view/view_test.go b/pkg/cmd/channel/view/view_test.go index 556f85f5..96e69fe2 100644 --- a/pkg/cmd/channel/view/view_test.go +++ b/pkg/cmd/channel/view/view_test.go @@ -238,7 +238,7 @@ func TestChannelView(t *testing.T) { }) _, err := testutil.ReceivePair(cmdReceiver) - assert.EqualError(t, err, "no channel found with name of Channels-99") + assert.EqualError(t, err, "cannot find a channel in project 'Fire Project' with the ID or name of 'Channels-99'") assert.Equal(t, "", stdOut.String()) assert.Equal(t, "", stdErr.String()) @@ -262,7 +262,7 @@ func TestChannelView(t *testing.T) { }) _, err := testutil.ReceivePair(cmdReceiver) - assert.EqualError(t, err, "no channel found with name of Nonexistent") + assert.EqualError(t, err, "cannot find a channel in project 'Fire Project' with the ID or name of 'Nonexistent'") assert.Equal(t, "", stdOut.String()) assert.Equal(t, "", stdErr.String()) diff --git a/pkg/cmd/release/create/create.go b/pkg/cmd/release/create/create.go index b4b50caa..a9bfd01c 100644 --- a/pkg/cmd/release/create/create.go +++ b/pkg/cmd/release/create/create.go @@ -310,6 +310,14 @@ func createRun(cmd *cobra.Command, f factory.Factory, flags *CreateFlags) error return err } options.ProjectName = project.GetName() + + if options.ChannelName != "" { // the executions API only matches channels by name, so resolve any ID we were given + channel, err := selectors.FindChannel(octopus, project, options.ChannelName) + if err != nil { + return err + } + options.ChannelName = channel.Name + } } } diff --git a/pkg/cmd/release/create/create_test.go b/pkg/cmd/release/create/create_test.go index 65ee97c7..872b87b3 100644 --- a/pkg/cmd/release/create/create_test.go +++ b/pkg/cmd/release/create/create_test.go @@ -1209,6 +1209,7 @@ func TestReleaseCreate_AutomationMode(t *testing.T) { protectedBranchNamePatterns := []string{} cacProject := fixtures.NewProject(space1.ID, cacProjectID, "CaC Project", "Lifecycles-1", "ProjectGroups-1", depProcess.ID) + betaChannel := fixtures.NewChannel(space1.ID, "Channels-31", "BetaChannel", cacProjectID) cacProject.PersistenceSettings = projects.NewGitPersistenceSettings( ".octopus", credentials.NewAnonymous(), @@ -1588,6 +1589,53 @@ func TestReleaseCreate_AutomationMode(t *testing.T) { assert.EqualError(t, err, "cannot specify both --release-notes and --release-notes-file at the same time") }}, + {"release creation specifying the project and channel by ID", func(t *testing.T, api *testutil.MockHttpServer, rootCmd *cobra.Command, stdOut *bytes.Buffer, stdErr *bytes.Buffer) { + cmdReceiver := testutil.GoBegin2(func() (*cobra.Command, error) { + defer api.Close() + rootCmd.SetArgs([]string{"release", "create", "--project", cacProjectID, "--channel", betaChannel.ID}) + return rootCmd.ExecuteC() + }) + + api.ExpectRequest(t, "GET", "/api/").RespondWith(rootResource) + api.ExpectRequest(t, "GET", "/api/Spaces-1").RespondWith(rootResource) + api.ExpectRequest(t, "GET", "/api/Spaces-1/projects/"+cacProjectID).RespondWith(cacProject) + + api.ExpectRequest(t, "GET", "/api/Spaces-1/projects/"+cacProjectID+"/channels").RespondWith(resources.Resources[*channels.Channel]{ + Items: []*channels.Channel{betaChannel}, + }) + + req := api.ExpectRequest(t, "POST", "/api/Spaces-1/releases/create/v1") + + // the executions API only matches channels by name, so the ID must have been resolved before we got here + requestBody, err := testutil.ReadJson[releases.CreateReleaseCommandV1](req.Request.Body) + assert.Nil(t, err) + + assert.Equal(t, releases.CreateReleaseCommandV1{ + SpaceID: "Spaces-1", + ProjectIDOrName: cacProject.Name, + ChannelIDOrName: betaChannel.Name, + }, requestBody) + + req.RespondWith(&releases.CreateReleaseResponseV1{ + ReleaseID: "Releases-999", + ReleaseVersion: "1.2.3", + }) + + releaseInfo := releases.NewRelease(betaChannel.ID, cacProject.ID, "1.2.3") + api.ExpectRequest(t, "GET", "/api/Spaces-1/releases/Releases-999").RespondWith(releaseInfo) + api.ExpectRequest(t, "GET", "/api/Spaces-1/channels/"+betaChannel.ID).RespondWith(betaChannel) + + _, err = testutil.ReceivePair(cmdReceiver) + assert.Nil(t, err) + + assert.Equal(t, heredoc.Doc(` + Successfully created release version 1.2.3 using channel BetaChannel + + View this release on Octopus Deploy: http://server/app#/Spaces-1/releases/Releases-999 + `), stdOut.String()) + assert.Equal(t, "", stdErr.String()) + }}, + {"release creation with all the flags", func(t *testing.T, api *testutil.MockHttpServer, rootCmd *cobra.Command, stdOut *bytes.Buffer, stdErr *bytes.Buffer) { cmdReceiver := testutil.GoBegin2(func() (*cobra.Command, error) { defer api.Close() @@ -1611,6 +1659,10 @@ func TestReleaseCreate_AutomationMode(t *testing.T) { api.ExpectRequest(t, "GET", "/api/Spaces-1").RespondWith(rootResource) api.ExpectRequest(t, "GET", "/api/Spaces-1/projects/"+cacProject.GetName()).RespondWith(cacProject) + api.ExpectRequest(t, "GET", "/api/Spaces-1/projects/"+cacProjectID+"/channels").RespondWith(resources.Resources[*channels.Channel]{ + Items: []*channels.Channel{betaChannel}, + }) + req := api.ExpectRequest(t, "POST", "/api/Spaces-1/releases/create/v1") // check that it sent the server the right request body @@ -1682,6 +1734,10 @@ func TestReleaseCreate_AutomationMode(t *testing.T) { api.ExpectRequest(t, "GET", "/api/Spaces-1").RespondWith(rootResource) api.ExpectRequest(t, "GET", "/api/Spaces-1/projects/"+cacProject.GetName()).RespondWith(cacProject) + api.ExpectRequest(t, "GET", "/api/Spaces-1/projects/"+cacProjectID+"/channels").RespondWith(resources.Resources[*channels.Channel]{ + Items: []*channels.Channel{betaChannel}, + }) + req := api.ExpectRequest(t, "POST", "/api/Spaces-1/releases/create/v1") // check that it sent the server the right request body @@ -1748,6 +1804,10 @@ func TestReleaseCreate_AutomationMode(t *testing.T) { api.ExpectRequest(t, "GET", "/api/Spaces-1").RespondWith(rootResource) api.ExpectRequest(t, "GET", "/api/Spaces-1/projects/"+cacProject.GetName()).RespondWith(cacProject) + api.ExpectRequest(t, "GET", "/api/Spaces-1/projects/"+cacProjectID+"/channels").RespondWith(resources.Resources[*channels.Channel]{ + Items: []*channels.Channel{betaChannel}, + }) + req := api.ExpectRequest(t, "POST", "/api/Spaces-1/releases/create/v1") // check that it sent the server the right request body @@ -1817,6 +1877,10 @@ func TestReleaseCreate_AutomationMode(t *testing.T) { api.ExpectRequest(t, "GET", "/api/Spaces-1").RespondWith(rootResource) api.ExpectRequest(t, "GET", "/api/Spaces-1/projects/"+cacProject.GetName()).RespondWith(cacProject) + api.ExpectRequest(t, "GET", "/api/Spaces-1/projects/"+cacProjectID+"/channels").RespondWith(resources.Resources[*channels.Channel]{ + Items: []*channels.Channel{betaChannel}, + }) + req := api.ExpectRequest(t, "POST", "/api/Spaces-1/releases/create/v1") // check that it sent the server the right request body diff --git a/pkg/cmd/release/deploy/deploy.go b/pkg/cmd/release/deploy/deploy.go index 0df6d614..9779dd6e 100644 --- a/pkg/cmd/release/deploy/deploy.go +++ b/pkg/cmd/release/deploy/deploy.go @@ -36,6 +36,7 @@ import ( "github.com/OctopusDeploy/go-octopusdeploy/v2/pkg/projects" "github.com/OctopusDeploy/go-octopusdeploy/v2/pkg/releases" "github.com/OctopusDeploy/go-octopusdeploy/v2/pkg/spaces" + "github.com/OctopusDeploy/go-octopusdeploy/v2/pkg/tenants" "github.com/OctopusDeploy/go-octopusdeploy/v2/pkg/variables" "github.com/spf13/cobra" ) @@ -237,6 +238,15 @@ func deployRun(cmd *cobra.Command, f factory.Factory, flags *DeployFlags) error options.ForcePackageDownloadWasSpecified = true } + // the executions API only matches tenants by name, so resolve any IDs we were given + if len(options.Tenants) > 0 { + selectedTenants, err := selectors.FindTenants(octopus, options.Tenants) + if err != nil { + return err + } + options.Tenants = util.SliceTransform(selectedTenants, func(t *tenants.Tenant) string { return t.Name }) + } + if f.IsPromptEnabled() { now := time.Now if cmd.Context() != nil { // allow context to override the definition of 'now' for testing @@ -319,6 +329,13 @@ func deployRun(cmd *cobra.Command, f factory.Factory, flags *DeployFlags) error options.ProjectName = project.GetName() } + // the executions API only matches environments by name, so resolve any IDs we were given + if len(options.Environments) > 0 { + options.Environments, err = resolveEnvironmentNames(octopus, f.GetCurrentSpace(), options.Environments) + if err != nil { + return err + } + } } // the executor will raise errors if any required options are missing @@ -474,18 +491,21 @@ func AskQuestions(octopus *octopusApiClient.Client, stdout io.Writer, asker ques if len(deploymentEnvironmentIDs) == 0 { // if the Q&A process earlier hasn't loaded environments already, we need to load them now if selectedChannel.Type == channels.ChannelTypeLifecycle { - selectedEnvironments, err := executionscommon.FindEnvironments(octopus, options.Environments) + selectedEnvironments, err := selectors.FindEnvironments(octopus, options.Environments) if err != nil { return err } deploymentEnvironmentIDs = util.SliceTransform(selectedEnvironments, func(env *environments.Environment) string { return env.ID }) + options.Environments = util.SliceTransform(selectedEnvironments, func(env *environments.Environment) string { return env.Name }) } else if selectedChannel.Type == channels.ChannelTypeEphemeral { - deploymentEnvironmentIDs, err = findEphemeralEnvironmentIDs(octopus, space, options.Environments) - + selectedEnvironments, err := findEphemeralEnvironments(octopus, space, options.Environments) if err != nil { return err } + + deploymentEnvironmentIDs = util.SliceTransform(selectedEnvironments, func(env *ephemeralenvironments.EphemeralEnvironment) string { return env.ID }) + options.Environments = util.SliceTransform(selectedEnvironments, func(env *ephemeralenvironments.EphemeralEnvironment) string { return env.Name }) } } @@ -622,7 +642,7 @@ func AskQuestions(octopus *octopusApiClient.Client, stdout io.Writer, asker ques return nil } -func findEphemeralEnvironmentIDs(octopus *octopusApiClient.Client, space *spaces.Space, environments []string) ([]string, error) { +func findEphemeralEnvironments(octopus *octopusApiClient.Client, space *spaces.Space, environmentIdentifiers []string) ([]*ephemeralenvironments.EphemeralEnvironment, error) { allEphemeralEnvironments, err := ephemeralenvironments.GetAll(octopus, space.ID) if err != nil { return nil, err @@ -632,8 +652,8 @@ func findEphemeralEnvironmentIDs(octopus *octopusApiClient.Client, space *spaces return nil, errors.New("no ephemeral environments exist to deploy to") } - var selectedEnvironments []string - if len(environments) == 0 { + var selectedEnvironments []*ephemeralenvironments.EphemeralEnvironment + if len(environmentIdentifiers) == 0 { return nil, nil } @@ -643,17 +663,33 @@ func findEphemeralEnvironmentIDs(octopus *octopusApiClient.Client, space *spaces envMap[strings.ToLower(ephemeralEnv.Name)] = ephemeralEnv } - for _, envIdentifier := range environments { + for _, envIdentifier := range environmentIdentifiers { ephemeralEnv, found := envMap[strings.ToLower(envIdentifier)] if !found { return nil, fmt.Errorf("environment '%s' not found in ephemeral environments", envIdentifier) } - selectedEnvironments = append(selectedEnvironments, ephemeralEnv.ID) + selectedEnvironments = append(selectedEnvironments, ephemeralEnv) } return selectedEnvironments, nil } +// resolveEnvironmentNames maps environment names or IDs onto canonical environment names, because +// the executions API only matches environments by name. Ephemeral environments aren't part of the +// regular environment list, so they're looked up separately when the regular lookup comes up empty. +func resolveEnvironmentNames(octopus *octopusApiClient.Client, space *spaces.Space, environmentIdentifiers []string) ([]string, error) { + selectedEnvironments, err := selectors.FindEnvironments(octopus, environmentIdentifiers) + if err == nil { + return util.SliceTransform(selectedEnvironments, func(env *environments.Environment) string { return env.Name }), nil + } + + ephemeralEnvironments, ephemeralErr := findEphemeralEnvironments(octopus, space, environmentIdentifiers) + if ephemeralErr != nil { + return nil, err // ephemeral environments are the rarer case; report why the regular lookup failed + } + return util.SliceTransform(ephemeralEnvironments, func(env *ephemeralenvironments.EphemeralEnvironment) string { return env.Name }), nil +} + func selectDeploymentEnvironmentsForEphemeralChannel(octopus *octopusApiClient.Client, stdout io.Writer, asker question.Asker, options *executor.TaskOptionsDeployRelease, selectedRelease *releases.Release) ([]string, error) { var deploymentEnvironmentIds []string var selectedEnvironments []*ephemeralenvironments.EphemeralEnvironment @@ -721,6 +757,7 @@ func selectDeploymentEnvironmentsForLifecycleChannel(octopus *octopusApiClient.C if err != nil { return nil, err } + options.Environments = []string{selectedEnvironment.Name} _, _ = fmt.Fprintf(stdout, "Environment %s\n", output.Cyan(selectedEnvironment.Name)) } selectedEnvironments = []*environments.Environment{selectedEnvironment} diff --git a/pkg/cmd/release/deploy/deploy_test.go b/pkg/cmd/release/deploy/deploy_test.go index fde01017..c2eab692 100644 --- a/pkg/cmd/release/deploy/deploy_test.go +++ b/pkg/cmd/release/deploy/deploy_test.go @@ -508,7 +508,7 @@ func TestDeployCreate_AskQuestions(t *testing.T) { assert.Equal(t, &executor.TaskOptionsDeployRelease{ ProjectName: "Fire Project", ReleaseVersion: "2.1", - Environments: []string{"ephemeral environment"}, + Environments: []string{"Ephemeral Environment"}, // the identifier from the command line is resolved to the canonical name GuidedFailureMode: "", Variables: make(map[string]string, 0), ReleaseID: release21.ID, @@ -1542,7 +1542,12 @@ func TestDeployCreate_AutomationMode(t *testing.T) { ////release20.ProjectDeploymentProcessSnapshotID = depProcessSnapshot.ID //release20.ProjectVariableSetSnapshotID = variableSnapshotWithPromptedVariables.ID // - //devEnvironment := fixtures.NewEnvironment(spaceID, "Environments-12", "dev") + devEnvironment := fixtures.NewEnvironment(spaceID, "Environments-12", "dev") + testEnvironment := fixtures.NewEnvironment(spaceID, "Environments-13", "test") + ephemeralEnvironment := fixtures.NewEphemeralEnvironment(spaceID, "Environments-123", "Ephemeral Environment", "Environments-12") + + cokeTenant := fixtures.NewTenant(spaceID, "Tenants-29", "Coke", "Regions/us-east", "Importance/High") + pepsiTenant := fixtures.NewTenant(spaceID, "Tenants-37", "Pepsi", "Regions/us-east", "Importance/Low") // TEST STARTS HERE tests := []struct { @@ -1612,6 +1617,7 @@ func TestDeployCreate_AutomationMode(t *testing.T) { api.ExpectRequest(t, "GET", "/api/").RespondWith(rootResource) api.ExpectRequest(t, "GET", "/api/Spaces-1").RespondWith(rootResource) api.ExpectRequest(t, "GET", "/api/Spaces-1/projects/"+fireProject.GetName()).RespondWith(fireProject) + api.ExpectRequest(t, "GET", "/api/Spaces-1/environments/all").RespondWith([]*environments.Environment{devEnvironment, testEnvironment}) // Note: because we didn't specify --tenant or --tenant-tag, automation-mode code is going to assume untenanted req := api.ExpectRequest(t, "POST", "/api/Spaces-1/deployments/create/untenanted/v1") @@ -1652,6 +1658,60 @@ func TestDeployCreate_AutomationMode(t *testing.T) { assert.Equal(t, "", stdErr.String()) }}, + {"release deploy specifying project, environment and tenant by ID", func(t *testing.T, api *testutil.MockHttpServer, rootCmd *cobra.Command, stdOut *bytes.Buffer, stdErr *bytes.Buffer) { + cmdReceiver := testutil.GoBegin2(func() (*cobra.Command, error) { + defer api.Close() + rootCmd.SetArgs([]string{"release", "deploy", "--project", fireProjectID, "--version", "1.0", "--environment", devEnvironment.ID, "--tenant", cokeTenant.ID}) + return rootCmd.ExecuteC() + }) + + api.ExpectRequest(t, "GET", "/api/").RespondWith(rootResource) + api.ExpectRequest(t, "GET", "/api/Spaces-1").RespondWith(rootResource) + + api.ExpectRequest(t, "GET", "/api/Spaces-1/tenants/"+cokeTenant.ID).RespondWith(cokeTenant) + api.ExpectRequest(t, "GET", "/api/Spaces-1/projects/"+fireProjectID).RespondWith(fireProject) + api.ExpectRequest(t, "GET", "/api/Spaces-1/environments/all").RespondWith([]*environments.Environment{devEnvironment, testEnvironment}) + + req := api.ExpectRequest(t, "POST", "/api/Spaces-1/deployments/create/tenanted/v1") + + // the executions API only matches environments and tenants by name, so the IDs must have been resolved before we got here + requestBody, err := testutil.ReadJson[deployments.CreateDeploymentTenantedCommandV1](req.Request.Body) + assert.Nil(t, err) + + assert.Equal(t, deployments.CreateDeploymentTenantedCommandV1{ + ReleaseVersion: "1.0", + EnvironmentName: devEnvironment.Name, + Tenants: []string{cokeTenant.Name}, + CreateExecutionAbstractCommandV1: deployments.CreateExecutionAbstractCommandV1{ + SpaceID: "Spaces-1", + ProjectIDOrName: fireProject.Name, + }, + }, requestBody) + + req.RespondWith(&deployments.CreateDeploymentResponseV1{ + DeploymentServerTasks: []*deployments.DeploymentServerTask{ + {DeploymentID: "Deployments-203", ServerTaskID: "ServerTasks-29394"}, + }, + }) + + // now it's going to try and look up the project/version to generate the web URL + api.ExpectRequest(t, "GET", "/api/Spaces-1/projects/Fire Project").RespondWithStatus(404, "NotFound", nil) + api.ExpectRequest(t, "GET", "/api/Spaces-1/projects?partialName=Fire+Project").RespondWith(resources.Resources[*projects.Project]{ + Items: []*projects.Project{fireProject}, + }) + api.ExpectRequest(t, "GET", "/api/Spaces-1/projects/"+fireProjectID+"/releases/1.0").RespondWith(release10) + + _, err = testutil.ReceivePair(cmdReceiver) + assert.Nil(t, err) + + assert.Equal(t, heredoc.Docf(` + Successfully started 1 deployment(s) + + View this release on Octopus Deploy: http://server/app#/Spaces-1/releases/%s + `, release10.ID), stdOut.String()) + assert.Equal(t, "", stdErr.String()) + }}, + {"release deploy specifying project, version, ephemeral env only (bare minimum)", func(t *testing.T, api *testutil.MockHttpServer, rootCmd *cobra.Command, stdOut *bytes.Buffer, stdErr *bytes.Buffer) { cmdReceiver := testutil.GoBegin2(func() (*cobra.Command, error) { defer api.Close() @@ -1662,6 +1722,13 @@ func TestDeployCreate_AutomationMode(t *testing.T) { api.ExpectRequest(t, "GET", "/api/").RespondWith(rootResource) api.ExpectRequest(t, "GET", "/api/Spaces-1").RespondWith(rootResource) api.ExpectRequest(t, "GET", "/api/Spaces-1/projects/"+fireProject.GetName()).RespondWith(fireProject) + api.ExpectRequest(t, "GET", "/api/Spaces-1/environments/all").RespondWith([]*environments.Environment{devEnvironment, testEnvironment}) + api.ExpectRequest(t, "GET", "/api/Spaces-1/environments/v2?skip=0&take=2147483647&type=Ephemeral").RespondWith(resources.Resources[*ephemeralenvironments.EphemeralEnvironment]{ + Items: []*ephemeralenvironments.EphemeralEnvironment{ephemeralEnvironment}, + PagedResults: resources.PagedResults{ + TotalResults: 1, + }, + }) // Note: because we didn't specify --tenant or --tenant-tag, automation-mode code is going to assume untenanted req := api.ExpectRequest(t, "POST", "/api/Spaces-1/deployments/create/untenanted/v1") @@ -1712,6 +1779,7 @@ func TestDeployCreate_AutomationMode(t *testing.T) { api.ExpectRequest(t, "GET", "/api/").RespondWith(rootResource) api.ExpectRequest(t, "GET", "/api/Spaces-1").RespondWith(rootResource) api.ExpectRequest(t, "GET", "/api/Spaces-1/projects/"+fireProject.GetName()).RespondWith(fireProject) + api.ExpectRequest(t, "GET", "/api/Spaces-1/environments/all").RespondWith([]*environments.Environment{devEnvironment, testEnvironment}) // Note: because we didn't specify --tenant or --tenant-tag, automation-mode code is going to assume untenanted api.ExpectRequest(t, "POST", "/api/Spaces-1/deployments/create/untenanted/v1").RespondWith(&deployments.CreateDeploymentResponseV1{ @@ -1742,6 +1810,7 @@ func TestDeployCreate_AutomationMode(t *testing.T) { api.ExpectRequest(t, "GET", "/api/").RespondWith(rootResource) api.ExpectRequest(t, "GET", "/api/Spaces-1").RespondWith(rootResource) api.ExpectRequest(t, "GET", "/api/Spaces-1/projects/"+fireProject.GetName()).RespondWith(fireProject) + api.ExpectRequest(t, "GET", "/api/Spaces-1/environments/all").RespondWith([]*environments.Environment{devEnvironment, testEnvironment}) // Note: because we didn't specify --tenant or --tenant-tag, automation-mode code is going to assume untenanted serverTasks := []*deployments.DeploymentServerTask{ @@ -1772,7 +1841,13 @@ func TestDeployCreate_AutomationMode(t *testing.T) { api.ExpectRequest(t, "GET", "/api/").RespondWith(rootResource) api.ExpectRequest(t, "GET", "/api/Spaces-1").RespondWith(rootResource) + api.ExpectRequest(t, "GET", "/api/Spaces-1/tenants/Coke").RespondWithStatus(404, "NotFound", nil) + api.ExpectRequest(t, "GET", "/api/Spaces-1/tenants?partialName=Coke").RespondWith(resources.Resources[*tenants.Tenant]{Items: []*tenants.Tenant{cokeTenant}}) + api.ExpectRequest(t, "GET", "/api/Spaces-1/tenants/Pepsi").RespondWithStatus(404, "NotFound", nil) + api.ExpectRequest(t, "GET", "/api/Spaces-1/tenants?partialName=Pepsi").RespondWith(resources.Resources[*tenants.Tenant]{Items: []*tenants.Tenant{pepsiTenant}}) + api.ExpectRequest(t, "GET", "/api/Spaces-1/projects/"+fireProject.GetName()).RespondWith(fireProject) + api.ExpectRequest(t, "GET", "/api/Spaces-1/environments/all").RespondWith([]*environments.Environment{devEnvironment, testEnvironment}) req := api.ExpectRequest(t, "POST", "/api/Spaces-1/deployments/create/tenanted/v1") requestBody, err := testutil.ReadJson[deployments.CreateDeploymentTenantedCommandV1](req.Request.Body) @@ -1822,6 +1897,7 @@ func TestDeployCreate_AutomationMode(t *testing.T) { api.ExpectRequest(t, "GET", "/api/").RespondWith(rootResource) api.ExpectRequest(t, "GET", "/api/Spaces-1").RespondWith(rootResource) api.ExpectRequest(t, "GET", "/api/Spaces-1/projects/"+fireProject.GetName()).RespondWith(fireProject) + api.ExpectRequest(t, "GET", "/api/Spaces-1/environments/all").RespondWith([]*environments.Environment{devEnvironment, testEnvironment}) req := api.ExpectRequest(t, "POST", "/api/Spaces-1/deployments/create/tenanted/v1") requestBody, err := testutil.ReadJson[deployments.CreateDeploymentTenantedCommandV1](req.Request.Body) @@ -1888,6 +1964,7 @@ func TestDeployCreate_AutomationMode(t *testing.T) { api.ExpectRequest(t, "GET", "/api/").RespondWith(rootResource) api.ExpectRequest(t, "GET", "/api/Spaces-1").RespondWith(rootResource) api.ExpectRequest(t, "GET", "/api/Spaces-1/projects/"+fireProject.GetName()).RespondWith(fireProject) + api.ExpectRequest(t, "GET", "/api/Spaces-1/environments/all").RespondWith([]*environments.Environment{devEnvironment, testEnvironment}) // Note: because we didn't specify --tenant or --tenant-tag, automation-mode code is going to assume untenanted req := api.ExpectRequest(t, "POST", "/api/Spaces-1/deployments/create/untenanted/v1") @@ -1961,7 +2038,13 @@ func TestDeployCreate_AutomationMode(t *testing.T) { api.ExpectRequest(t, "GET", "/api/").RespondWith(rootResource) api.ExpectRequest(t, "GET", "/api/Spaces-1").RespondWith(rootResource) + api.ExpectRequest(t, "GET", "/api/Spaces-1/tenants/Coke").RespondWithStatus(404, "NotFound", nil) + api.ExpectRequest(t, "GET", "/api/Spaces-1/tenants?partialName=Coke").RespondWith(resources.Resources[*tenants.Tenant]{Items: []*tenants.Tenant{cokeTenant}}) + api.ExpectRequest(t, "GET", "/api/Spaces-1/tenants/Pepsi").RespondWithStatus(404, "NotFound", nil) + api.ExpectRequest(t, "GET", "/api/Spaces-1/tenants?partialName=Pepsi").RespondWith(resources.Resources[*tenants.Tenant]{Items: []*tenants.Tenant{pepsiTenant}}) + api.ExpectRequest(t, "GET", "/api/Spaces-1/projects/"+fireProject.GetName()).RespondWith(fireProject) + api.ExpectRequest(t, "GET", "/api/Spaces-1/environments/all").RespondWith([]*environments.Environment{devEnvironment, testEnvironment}) req := api.ExpectRequest(t, "POST", "/api/Spaces-1/deployments/create/tenanted/v1") requestBody, err := testutil.ReadJson[deployments.CreateDeploymentTenantedCommandV1](req.Request.Body) diff --git a/pkg/cmd/runbook/run/run.go b/pkg/cmd/runbook/run/run.go index ad57eb89..000c665e 100644 --- a/pkg/cmd/runbook/run/run.go +++ b/pkg/cmd/runbook/run/run.go @@ -38,6 +38,7 @@ import ( "github.com/OctopusDeploy/go-octopusdeploy/v2/pkg/releases" "github.com/OctopusDeploy/go-octopusdeploy/v2/pkg/runbooks" "github.com/OctopusDeploy/go-octopusdeploy/v2/pkg/spaces" + "github.com/OctopusDeploy/go-octopusdeploy/v2/pkg/tenants" "github.com/spf13/cobra" ) @@ -228,6 +229,23 @@ func runbookRun(cmd *cobra.Command, f factory.Factory, flags *RunFlags) error { flags.Project.Value = project.Name + // the executions API only matches environments and tenants by name, so resolve any IDs we were given + if len(flags.Environments.Value) > 0 { + selectedEnvironments, err := selectors.FindEnvironments(octopus, flags.Environments.Value) + if err != nil { + return err + } + flags.Environments.Value = util.SliceTransform(selectedEnvironments, func(env *environments.Environment) string { return env.Name }) + } + + if len(flags.Tenants.Value) > 0 { + selectedTenants, err := selectors.FindTenants(octopus, flags.Tenants.Value) + if err != nil { + return err + } + flags.Tenants.Value = util.SliceTransform(selectedTenants, func(t *tenants.Tenant) string { return t.Name }) + } + if f.IsPromptEnabled() && flags.RunbookName.Value == "" && len(flags.RunbookTags.Value) == 0 { var runBySelection string err = f.Ask(&survey.Select{ diff --git a/pkg/cmd/runbook/run/run_test.go b/pkg/cmd/runbook/run/run_test.go index 33c1904d..82173da2 100644 --- a/pkg/cmd/runbook/run/run_test.go +++ b/pkg/cmd/runbook/run/run_test.go @@ -15,8 +15,11 @@ import ( "github.com/OctopusDeploy/cli/test/fixtures" "github.com/OctopusDeploy/cli/test/testutil" "github.com/OctopusDeploy/go-octopusdeploy/v2/pkg/deployments" + "github.com/OctopusDeploy/go-octopusdeploy/v2/pkg/environments" "github.com/OctopusDeploy/go-octopusdeploy/v2/pkg/projects" + "github.com/OctopusDeploy/go-octopusdeploy/v2/pkg/resources" "github.com/OctopusDeploy/go-octopusdeploy/v2/pkg/runbooks" + "github.com/OctopusDeploy/go-octopusdeploy/v2/pkg/tenants" "github.com/spf13/cobra" "github.com/stretchr/testify/assert" ) @@ -39,6 +42,12 @@ func TestRunbookRun_AutomationMode(t *testing.T) { fireProject := fixtures.NewProject(spaceID, fireProjectID, "Fire Project", "Lifecycles-1", "ProjectGroups-1", "deploymentprocess-"+fireProjectID) _ = fireProject + devEnvironment := fixtures.NewEnvironment(spaceID, "Environments-12", "dev") + testEnvironment := fixtures.NewEnvironment(spaceID, "Environments-13", "test") + + cokeTenant := fixtures.NewTenant(spaceID, "Tenants-29", "Coke", "Regions/us-east", "Importance/High") + pepsiTenant := fixtures.NewTenant(spaceID, "Tenants-37", "Pepsi", "Regions/us-east", "Importance/Low") + // TEST STARTS HERE tests := []struct { name string @@ -107,6 +116,7 @@ func TestRunbookRun_AutomationMode(t *testing.T) { api.ExpectRequest(t, "GET", "/api/").RespondWith(rootResource) api.ExpectRequest(t, "GET", "/api/Spaces-1").RespondWith(rootResource) api.ExpectRequest(t, "GET", "/api/Spaces-1/projects/Fire Project").RespondWithJSON(fixtures.AsServerResponse(fireProject)) + api.ExpectRequest(t, "GET", "/api/Spaces-1/environments/all").RespondWith([]*environments.Environment{devEnvironment, testEnvironment}) // Note: because we didn't specify --tenant or --tenant-tag, automation-mode code is going to assume untenanted req := api.ExpectRequest(t, "POST", "/api/Spaces-1/runbook-runs/create/v1") @@ -146,6 +156,7 @@ func TestRunbookRun_AutomationMode(t *testing.T) { api.ExpectRequest(t, "GET", "/api/").RespondWith(rootResource) api.ExpectRequest(t, "GET", "/api/Spaces-1").RespondWith(rootResource) api.ExpectRequest(t, "GET", "/api/Spaces-1/projects/Fire Project").RespondWithJSON(fixtures.AsServerResponse(fireProject)) + api.ExpectRequest(t, "GET", "/api/Spaces-1/environments/all").RespondWith([]*environments.Environment{devEnvironment, testEnvironment}) // Note: because we didn't specify --tenant or --tenant-tag, automation-mode code is going to assume untenanted api.ExpectRequest(t, "POST", "/api/Spaces-1/runbook-runs/create/v1").RespondWith(&runbooks.RunbookRunResponseV1{ @@ -175,6 +186,7 @@ func TestRunbookRun_AutomationMode(t *testing.T) { api.ExpectRequest(t, "GET", "/api/").RespondWith(rootResource) api.ExpectRequest(t, "GET", "/api/Spaces-1").RespondWith(rootResource) api.ExpectRequest(t, "GET", "/api/Spaces-1/projects/Fire Project").RespondWithJSON(fixtures.AsServerResponse(fireProject)) + api.ExpectRequest(t, "GET", "/api/Spaces-1/environments/all").RespondWith([]*environments.Environment{devEnvironment, testEnvironment}) serverTasks := []*runbooks.RunbookRunServerTask{ {RunbookRunID: "RunbookRun-203", ServerTaskID: "ServerTasks-29394"}, @@ -196,6 +208,48 @@ func TestRunbookRun_AutomationMode(t *testing.T) { assert.Equal(t, "", stdErr.String()) }}, + {"runbook run specifying project, environment and tenant by ID", func(t *testing.T, api *testutil.MockHttpServer, rootCmd *cobra.Command, stdOut *bytes.Buffer, stdErr *bytes.Buffer) { + cmdReceiver := testutil.GoBegin2(func() (*cobra.Command, error) { + defer api.Close() + rootCmd.SetArgs([]string{"runbook", "run", "--project", fireProjectID, "--runbook", "Provision Database", "--environment", devEnvironment.ID, "--tenant", cokeTenant.ID}) + return rootCmd.ExecuteC() + }) + + api.ExpectRequest(t, "GET", "/api/").RespondWith(rootResource) + api.ExpectRequest(t, "GET", "/api/Spaces-1").RespondWith(rootResource) + api.ExpectRequest(t, "GET", "/api/Spaces-1/projects/"+fireProjectID).RespondWithJSON(fixtures.AsServerResponse(fireProject)) + api.ExpectRequest(t, "GET", "/api/Spaces-1/environments/all").RespondWith([]*environments.Environment{devEnvironment, testEnvironment}) + api.ExpectRequest(t, "GET", "/api/Spaces-1/tenants/"+cokeTenant.ID).RespondWith(cokeTenant) + + req := api.ExpectRequest(t, "POST", "/api/Spaces-1/runbook-runs/create/v1") + + // the executions API only matches environments and tenants by name, so the IDs must have been resolved before we got here + requestBody, err := testutil.ReadJson[runbooks.RunbookRunCommandV1](req.Request.Body) + assert.Nil(t, err) + + assert.Equal(t, runbooks.RunbookRunCommandV1{ + RunbookName: "Provision Database", + EnvironmentNames: []string{devEnvironment.Name}, + Tenants: []string{cokeTenant.Name}, + CreateExecutionAbstractCommandV1: deployments.CreateExecutionAbstractCommandV1{ + SpaceID: "Spaces-1", + ProjectIDOrName: fireProject.Name, + }, + }, requestBody) + + req.RespondWith(&runbooks.RunbookRunResponseV1{ + RunbookRunServerTasks: []*runbooks.RunbookRunServerTask{ + {RunbookRunID: "RunbookRun-203", ServerTaskID: "ServerTasks-29394"}, + }, + }) + + _, err = testutil.ReceivePair(cmdReceiver) + assert.Nil(t, err) + + assert.Equal(t, "Successfully started 1 runbook run(s)\n", stdOut.String()) + assert.Equal(t, "", stdErr.String()) + }}, + {"runbook run specifying project, runbook, env only (bare minimum) assuming tenanted", func(t *testing.T, api *testutil.MockHttpServer, rootCmd *cobra.Command, stdOut *bytes.Buffer, stdErr *bytes.Buffer) { cmdReceiver := testutil.GoBegin2(func() (*cobra.Command, error) { defer api.Close() @@ -206,6 +260,11 @@ func TestRunbookRun_AutomationMode(t *testing.T) { api.ExpectRequest(t, "GET", "/api/").RespondWith(rootResource) api.ExpectRequest(t, "GET", "/api/Spaces-1").RespondWith(rootResource) api.ExpectRequest(t, "GET", "/api/Spaces-1/projects/Fire Project").RespondWithJSON(fixtures.AsServerResponse(fireProject)) + api.ExpectRequest(t, "GET", "/api/Spaces-1/environments/all").RespondWith([]*environments.Environment{devEnvironment, testEnvironment}) + api.ExpectRequest(t, "GET", "/api/Spaces-1/tenants/Coke").RespondWithStatus(404, "NotFound", nil) + api.ExpectRequest(t, "GET", "/api/Spaces-1/tenants?partialName=Coke").RespondWith(resources.Resources[*tenants.Tenant]{Items: []*tenants.Tenant{cokeTenant}}) + api.ExpectRequest(t, "GET", "/api/Spaces-1/tenants/Pepsi").RespondWithStatus(404, "NotFound", nil) + api.ExpectRequest(t, "GET", "/api/Spaces-1/tenants?partialName=Pepsi").RespondWith(resources.Resources[*tenants.Tenant]{Items: []*tenants.Tenant{pepsiTenant}}) req := api.ExpectRequest(t, "POST", "/api/Spaces-1/runbook-runs/create/v1") requestBody, err := testutil.ReadJson[runbooks.RunbookRunCommandV1](req.Request.Body) @@ -245,6 +304,7 @@ func TestRunbookRun_AutomationMode(t *testing.T) { api.ExpectRequest(t, "GET", "/api/").RespondWith(rootResource) api.ExpectRequest(t, "GET", "/api/Spaces-1").RespondWith(rootResource) api.ExpectRequest(t, "GET", "/api/Spaces-1/projects/Fire Project").RespondWithJSON(fixtures.AsServerResponse(fireProject)) + api.ExpectRequest(t, "GET", "/api/Spaces-1/environments/all").RespondWith([]*environments.Environment{devEnvironment, testEnvironment}) req := api.ExpectRequest(t, "POST", "/api/Spaces-1/runbook-runs/create/v1") requestBody, err := testutil.ReadJson[runbooks.RunbookRunCommandV1](req.Request.Body) @@ -299,6 +359,7 @@ func TestRunbookRun_AutomationMode(t *testing.T) { api.ExpectRequest(t, "GET", "/api/").RespondWith(rootResource) api.ExpectRequest(t, "GET", "/api/Spaces-1").RespondWith(rootResource) api.ExpectRequest(t, "GET", "/api/Spaces-1/projects/Fire Project").RespondWithJSON(fixtures.AsServerResponse(fireProject)) + api.ExpectRequest(t, "GET", "/api/Spaces-1/environments/all").RespondWith([]*environments.Environment{devEnvironment, testEnvironment}) req := api.ExpectRequest(t, "POST", "/api/Spaces-1/runbook-runs/create/v1") requestBody, err := testutil.ReadJson[runbooks.RunbookRunCommandV1](req.Request.Body) @@ -367,6 +428,12 @@ func TestGitRunbookRun_AutomationMode(t *testing.T) { fireProject.PersistenceSettings.(projects.GitPersistenceSettings).SetRunbooksAreInGit() _ = fireProject + devEnvironment := fixtures.NewEnvironment(spaceID, "Environments-12", "dev") + testEnvironment := fixtures.NewEnvironment(spaceID, "Environments-13", "test") + + cokeTenant := fixtures.NewTenant(spaceID, "Tenants-29", "Coke", "Regions/us-east", "Importance/High") + pepsiTenant := fixtures.NewTenant(spaceID, "Tenants-37", "Pepsi", "Regions/us-east", "Importance/Low") + // TEST STARTS HERE tests := []struct { name string @@ -435,6 +502,7 @@ func TestGitRunbookRun_AutomationMode(t *testing.T) { api.ExpectRequest(t, "GET", "/api/").RespondWith(rootResource) api.ExpectRequest(t, "GET", "/api/Spaces-1").RespondWith(rootResource) api.ExpectRequest(t, "GET", "/api/Spaces-1/projects/Fire Project").RespondWithJSON(fixtures.AsServerResponse(fireProject)) + api.ExpectRequest(t, "GET", "/api/Spaces-1/environments/all").RespondWith([]*environments.Environment{devEnvironment, testEnvironment}) _, err := testutil.ReceivePair(cmdReceiver) assert.EqualError(t, err, "git reference must be specified") @@ -453,6 +521,7 @@ func TestGitRunbookRun_AutomationMode(t *testing.T) { api.ExpectRequest(t, "GET", "/api/").RespondWith(rootResource) api.ExpectRequest(t, "GET", "/api/Spaces-1").RespondWith(rootResource) api.ExpectRequest(t, "GET", "/api/Spaces-1/projects/Fire Project").RespondWithJSON(fixtures.AsServerResponse(fireProject)) + api.ExpectRequest(t, "GET", "/api/Spaces-1/environments/all").RespondWith([]*environments.Environment{devEnvironment, testEnvironment}) // Note: because we didn't specify --tenant or --tenant-tag, automation-mode code is going to assume untenanted req := api.ExpectRequest(t, "POST", "/api/Spaces-1/runbook-runs/git/create/v1") @@ -493,6 +562,7 @@ func TestGitRunbookRun_AutomationMode(t *testing.T) { api.ExpectRequest(t, "GET", "/api/").RespondWith(rootResource) api.ExpectRequest(t, "GET", "/api/Spaces-1").RespondWith(rootResource) api.ExpectRequest(t, "GET", "/api/Spaces-1/projects/Fire Project").RespondWithJSON(fixtures.AsServerResponse(fireProject)) + api.ExpectRequest(t, "GET", "/api/Spaces-1/environments/all").RespondWith([]*environments.Environment{devEnvironment, testEnvironment}) // Note: because we didn't specify --tenant or --tenant-tag, automation-mode code is going to assume untenanted api.ExpectRequest(t, "POST", "/api/Spaces-1/runbook-runs/git/create/v1").RespondWith(&runbooks.GitRunbookRunResponseV1{ @@ -522,6 +592,7 @@ func TestGitRunbookRun_AutomationMode(t *testing.T) { api.ExpectRequest(t, "GET", "/api/").RespondWith(rootResource) api.ExpectRequest(t, "GET", "/api/Spaces-1").RespondWith(rootResource) api.ExpectRequest(t, "GET", "/api/Spaces-1/projects/Fire Project").RespondWithJSON(fixtures.AsServerResponse(fireProject)) + api.ExpectRequest(t, "GET", "/api/Spaces-1/environments/all").RespondWith([]*environments.Environment{devEnvironment, testEnvironment}) serverTasks := []*runbooks.RunbookRunServerTask{ {RunbookRunID: "RunbookRun-203", ServerTaskID: "ServerTasks-29394"}, @@ -553,6 +624,11 @@ func TestGitRunbookRun_AutomationMode(t *testing.T) { api.ExpectRequest(t, "GET", "/api/").RespondWith(rootResource) api.ExpectRequest(t, "GET", "/api/Spaces-1").RespondWith(rootResource) api.ExpectRequest(t, "GET", "/api/Spaces-1/projects/Fire Project").RespondWithJSON(fixtures.AsServerResponse(fireProject)) + api.ExpectRequest(t, "GET", "/api/Spaces-1/environments/all").RespondWith([]*environments.Environment{devEnvironment, testEnvironment}) + api.ExpectRequest(t, "GET", "/api/Spaces-1/tenants/Coke").RespondWithStatus(404, "NotFound", nil) + api.ExpectRequest(t, "GET", "/api/Spaces-1/tenants?partialName=Coke").RespondWith(resources.Resources[*tenants.Tenant]{Items: []*tenants.Tenant{cokeTenant}}) + api.ExpectRequest(t, "GET", "/api/Spaces-1/tenants/Pepsi").RespondWithStatus(404, "NotFound", nil) + api.ExpectRequest(t, "GET", "/api/Spaces-1/tenants?partialName=Pepsi").RespondWith(resources.Resources[*tenants.Tenant]{Items: []*tenants.Tenant{pepsiTenant}}) req := api.ExpectRequest(t, "POST", "/api/Spaces-1/runbook-runs/git/create/v1") requestBody, err := testutil.ReadJson[runbooks.GitRunbookRunCommandV1](req.Request.Body) @@ -593,6 +669,7 @@ func TestGitRunbookRun_AutomationMode(t *testing.T) { api.ExpectRequest(t, "GET", "/api/").RespondWith(rootResource) api.ExpectRequest(t, "GET", "/api/Spaces-1").RespondWith(rootResource) api.ExpectRequest(t, "GET", "/api/Spaces-1/projects/Fire Project").RespondWithJSON(fixtures.AsServerResponse(fireProject)) + api.ExpectRequest(t, "GET", "/api/Spaces-1/environments/all").RespondWith([]*environments.Environment{devEnvironment, testEnvironment}) req := api.ExpectRequest(t, "POST", "/api/Spaces-1/runbook-runs/git/create/v1") requestBody, err := testutil.ReadJson[runbooks.GitRunbookRunCommandV1](req.Request.Body) @@ -651,6 +728,7 @@ func TestGitRunbookRun_AutomationMode(t *testing.T) { api.ExpectRequest(t, "GET", "/api/").RespondWith(rootResource) api.ExpectRequest(t, "GET", "/api/Spaces-1").RespondWith(rootResource) api.ExpectRequest(t, "GET", "/api/Spaces-1/projects/Fire Project").RespondWithJSON(fixtures.AsServerResponse(fireProject)) + api.ExpectRequest(t, "GET", "/api/Spaces-1/environments/all").RespondWith([]*environments.Environment{devEnvironment, testEnvironment}) req := api.ExpectRequest(t, "POST", "/api/Spaces-1/runbook-runs/git/create/v1") requestBody, err := testutil.ReadJson[runbooks.GitRunbookRunCommandV1](req.Request.Body) diff --git a/pkg/executionscommon/executionscommon.go b/pkg/executionscommon/executionscommon.go index 4348d7bd..8e3d9827 100644 --- a/pkg/executionscommon/executionscommon.go +++ b/pkg/executionscommon/executionscommon.go @@ -8,6 +8,7 @@ import ( "github.com/AlecAivazis/survey/v2" cliErrors "github.com/OctopusDeploy/cli/pkg/errors" "github.com/OctopusDeploy/cli/pkg/question" + "github.com/OctopusDeploy/cli/pkg/question/selectors" "github.com/OctopusDeploy/cli/pkg/surveyext" "github.com/OctopusDeploy/cli/pkg/util" octopusApiClient "github.com/OctopusDeploy/go-octopusdeploy/v2/pkg/client" @@ -431,40 +432,8 @@ func ScheduledStartTimeAnswerFormatter(datePicker *surveyext.DatePicker, t time. } } -// given an array of environment names, maps these all to actual objects by querying the server +// FindEnvironments maps an array of environment names or IDs onto the matching objects. +// Kept as an alias so existing callers don't have to change; selectors owns the lookup. func FindEnvironments(client *octopusApiClient.Client, environmentNamesOrIds []string) ([]*environments.Environment, error) { - if len(environmentNamesOrIds) == 0 { - return nil, nil - } - // there's no "bulk lookup" API, so we either need to do a foreach loop to find each environment individually, or load the entire server's worth of environments - // it's probably going to be cheaper to just list out all the environments and match them client side, so we'll do that for simplicity's sake - allEnvs, err := client.Environments.GetAll() - if err != nil { - return nil, err - } - - nameLookup := make(map[string]*environments.Environment, len(allEnvs)) - idLookup := make(map[string]*environments.Environment, len(allEnvs)) - - for _, env := range allEnvs { - nameLookup[strings.ToLower(env.GetName())] = env - idLookup[strings.ToLower(env.GetID())] = env - } - - var result []*environments.Environment - for _, n := range environmentNamesOrIds { - nameOrId := strings.ToLower(n) - env := nameLookup[nameOrId] - if env != nil { - result = append(result, env) - } else { - env = idLookup[nameOrId] - if env != nil { - result = append(result, env) - } else { - return nil, fmt.Errorf("cannot find environment %s", nameOrId) - } - } - } - return result, nil + return selectors.FindEnvironments(client, environmentNamesOrIds) } diff --git a/pkg/question/selectors/channels.go b/pkg/question/selectors/channels.go index 7a5452b6..59f330e9 100644 --- a/pkg/question/selectors/channels.go +++ b/pkg/question/selectors/channels.go @@ -26,15 +26,22 @@ func Channel(octopus *octopusApiClient.Client, ask question.Asker, io io.Writer, }) } -func FindChannel(octopus *octopusApiClient.Client, project *projects.Project, channelName string) (*channels.Channel, error) { +// FindChannel looks a channel up within a project by either its ID or its name. An ID match +// wins over a name match, so it stays consistent with how projects and tenants resolve. +func FindChannel(octopus *octopusApiClient.Client, project *projects.Project, channelIdentifier string) (*channels.Channel, error) { foundChannels, err := octopus.Projects.GetChannels(project) // TODO change this to channel partial name search on server; will require go client update if err != nil { return nil, err } + for _, c := range foundChannels { + if strings.EqualFold(c.ID, channelIdentifier) { + return c, nil + } + } for _, c := range foundChannels { // server doesn't support channel search by exact name so we must emulate it - if strings.EqualFold(c.Name, channelName) { + if strings.EqualFold(c.Name, channelIdentifier) { return c, nil } } - return nil, fmt.Errorf("no channel found with name of %s", channelName) + return nil, fmt.Errorf("cannot find a channel in project '%s' with the ID or name of '%s'", project.GetName(), channelIdentifier) } diff --git a/pkg/question/selectors/environments.go b/pkg/question/selectors/environments.go index 0570782f..2176b400 100644 --- a/pkg/question/selectors/environments.go +++ b/pkg/question/selectors/environments.go @@ -2,10 +2,11 @@ package selectors import ( "fmt" + "strings" + "github.com/OctopusDeploy/cli/pkg/question" "github.com/OctopusDeploy/go-octopusdeploy/v2/pkg/client" "github.com/OctopusDeploy/go-octopusdeploy/v2/pkg/environments" - "strings" ) type GetAllEnvironmentsCallback func() ([]*environments.Environment, error) @@ -34,25 +35,48 @@ func EnvironmentSelect(ask question.Asker, getAllEnvironmentsCallback GetAllEnvi }) } -func FindEnvironment(octopus *client.Client, environmentName string) (*environments.Environment, error) { - resultPage, err := octopus.Environments.Get(environments.EnvironmentsQuery{PartialName: environmentName}) +// FindEnvironment looks an environment up by either its ID or its name. +func FindEnvironment(octopus *client.Client, environmentIdentifier string) (*environments.Environment, error) { + found, err := FindEnvironments(octopus, []string{environmentIdentifier}) if err != nil { return nil, err } - // environmentsQuery has "Name" but it's just an alias in the server for PartialName; we need to filter client side - for resultPage != nil && len(resultPage.Items) > 0 { - for _, c := range resultPage.Items { // server doesn't support search by exact name so we must emulate it - if strings.EqualFold(c.Name, environmentName) { - return c, nil - } - } - resultPage, err = resultPage.GetNextPage(octopus.Environments.GetClient()) - if err != nil { - return nil, err - } // if there are no more pages, then GetNextPage will return nil, which breaks us out of the loop + return found[0], nil +} + +// FindEnvironments looks environments up by either their IDs or their names. An ID match +// wins over a name match, so it stays consistent with how projects and tenants resolve. +func FindEnvironments(octopus *client.Client, environmentIdentifiers []string) ([]*environments.Environment, error) { + if len(environmentIdentifiers) == 0 { + return nil, nil + } + // there's no "bulk lookup" API, so we either need to do a foreach loop to find each environment individually, or load the entire server's worth of environments + // it's probably going to be cheaper to just list out all the environments and match them client side, so we'll do that for simplicity's sake + allEnvs, err := octopus.Environments.GetAll() + if err != nil { + return nil, err + } + + idLookup := make(map[string]*environments.Environment, len(allEnvs)) + nameLookup := make(map[string]*environments.Environment, len(allEnvs)) + for _, env := range allEnvs { + idLookup[strings.ToLower(env.GetID())] = env + nameLookup[strings.ToLower(env.GetName())] = env } - return nil, fmt.Errorf("no environment found with name of %s", environmentName) + result := make([]*environments.Environment, 0, len(environmentIdentifiers)) + for _, identifier := range environmentIdentifiers { + key := strings.ToLower(identifier) + env, found := idLookup[key] + if !found { + env, found = nameLookup[key] + } + if !found { + return nil, fmt.Errorf("cannot find an environment with the ID or name of '%s'", identifier) + } + result = append(result, env) + } + return result, nil } func EnvironmentsMultiSelect(ask question.Asker, getAllEnvironmentsCallback GetAllEnvironmentsCallback, message string, required bool) ([]*environments.Environment, error) { diff --git a/pkg/question/selectors/find_test.go b/pkg/question/selectors/find_test.go new file mode 100644 index 00000000..0ff5612f --- /dev/null +++ b/pkg/question/selectors/find_test.go @@ -0,0 +1,198 @@ +package selectors_test + +import ( + "net/url" + "testing" + + "github.com/OctopusDeploy/cli/pkg/question/selectors" + "github.com/OctopusDeploy/cli/pkg/util" + "github.com/OctopusDeploy/cli/test/fixtures" + "github.com/OctopusDeploy/cli/test/testutil" + "github.com/OctopusDeploy/go-octopusdeploy/v2/pkg/channels" + octopusApiClient "github.com/OctopusDeploy/go-octopusdeploy/v2/pkg/client" + "github.com/OctopusDeploy/go-octopusdeploy/v2/pkg/environments" + "github.com/OctopusDeploy/go-octopusdeploy/v2/pkg/resources" + "github.com/OctopusDeploy/go-octopusdeploy/v2/pkg/tenants" + "github.com/stretchr/testify/assert" +) + +var serverUrl, _ = url.Parse("http://server") + +const placeholderApiKey = "API-XXXXXXXXXXXXXXXXXXXXXXXXXXXXX" + +var findRootResource = testutil.NewRootResource() + +const findSpaceID = "Spaces-1" +const findProjectID = "Projects-22" + +// beginRequest spins up a mock server and hands back the client to run `action` against; +// the octopus client makes network calls on construction so it has to live in the goroutine +func beginRequest[T any](api *testutil.MockHttpServer, action func(octopus *octopusApiClient.Client) (T, error)) chan testutil.Pair[T, error] { + return testutil.GoBegin2(func() (T, error) { + defer api.Close() + octopus, _ := octopusApiClient.NewClient(testutil.NewMockHttpClientWithTransport(api), serverUrl, placeholderApiKey, "") + return action(octopus) + }) +} + +func TestFindEnvironments(t *testing.T) { + devEnvironment := fixtures.NewEnvironment(findSpaceID, "Environments-12", "dev") + prodEnvironment := fixtures.NewEnvironment(findSpaceID, "Environments-13", "production") + + // an environment which is *named* like an ID, to prove the precedence rule + decoyEnvironment := fixtures.NewEnvironment(findSpaceID, "Environments-99", "Environments-13") + + allEnvironments := []*environments.Environment{devEnvironment, prodEnvironment, decoyEnvironment} + + tests := []struct { + name string + identifiers []string + expectedIDs []string + expectedErr string + }{ + {"finds an environment by name", []string{"dev"}, []string{devEnvironment.ID}, ""}, + {"finds an environment by name, ignoring case", []string{"DEV"}, []string{devEnvironment.ID}, ""}, + {"finds an environment by ID", []string{"Environments-12"}, []string{devEnvironment.ID}, ""}, + {"finds several environments at once", []string{"Environments-12", "production"}, []string{devEnvironment.ID, prodEnvironment.ID}, ""}, + {"prefers an ID match over a name match", []string{"Environments-13"}, []string{prodEnvironment.ID}, ""}, + {"errors when nothing matches", []string{"Environments-404"}, nil, "cannot find an environment with the ID or name of 'Environments-404'"}, + } + + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + api := testutil.NewMockHttpServer() + receiver := beginRequest(api, func(octopus *octopusApiClient.Client) ([]*environments.Environment, error) { + return selectors.FindEnvironments(octopus, test.identifiers) + }) + + api.ExpectRequest(t, "GET", "/api/").RespondWith(findRootResource) + api.ExpectRequest(t, "GET", "/api/spaces").RespondWith(findRootResource) + api.ExpectRequest(t, "GET", "/api/Spaces-1/environments/all").RespondWith(allEnvironments) + + result, err := testutil.ReceivePair(receiver) + if test.expectedErr == "" { + assert.Nil(t, err) + assert.Equal(t, test.expectedIDs, util.SliceTransform(result, func(env *environments.Environment) string { return env.ID })) + } else { + assert.EqualError(t, err, test.expectedErr) + } + }) + } +} + +func TestFindEnvironment(t *testing.T) { + devEnvironment := fixtures.NewEnvironment(findSpaceID, "Environments-12", "dev") + + api := testutil.NewMockHttpServer() + receiver := beginRequest(api, func(octopus *octopusApiClient.Client) (*environments.Environment, error) { + return selectors.FindEnvironment(octopus, "Environments-12") + }) + + api.ExpectRequest(t, "GET", "/api/").RespondWith(findRootResource) + api.ExpectRequest(t, "GET", "/api/spaces").RespondWith(findRootResource) + api.ExpectRequest(t, "GET", "/api/Spaces-1/environments/all").RespondWith([]*environments.Environment{devEnvironment}) + + result, err := testutil.ReceivePair(receiver) + assert.Nil(t, err) + assert.Equal(t, devEnvironment.ID, result.ID) +} + +func TestFindChannel(t *testing.T) { + project := fixtures.NewProject(findSpaceID, findProjectID, "Fire Project", "Lifecycles-1", "ProjectGroups-1", "deploymentprocess-"+findProjectID) + + defaultChannel := fixtures.NewChannel(findSpaceID, "Channels-1", "Default", findProjectID) + betaChannel := fixtures.NewChannel(findSpaceID, "Channels-2", "Beta", findProjectID) + + // a channel which is *named* like an ID, to prove the precedence rule + decoyChannel := fixtures.NewChannel(findSpaceID, "Channels-3", "Channels-2", findProjectID) + + allChannels := []*channels.Channel{defaultChannel, betaChannel, decoyChannel} + + tests := []struct { + name string + identifier string + expectedID string + expectedErr string + }{ + {"finds a channel by name", "Beta", betaChannel.ID, ""}, + {"finds a channel by name, ignoring case", "beta", betaChannel.ID, ""}, + {"finds a channel by ID", "Channels-1", defaultChannel.ID, ""}, + {"prefers an ID match over a name match", "Channels-2", betaChannel.ID, ""}, + {"errors when nothing matches", "Channels-404", "", "cannot find a channel in project 'Fire Project' with the ID or name of 'Channels-404'"}, + } + + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + api := testutil.NewMockHttpServer() + receiver := beginRequest(api, func(octopus *octopusApiClient.Client) (*channels.Channel, error) { + return selectors.FindChannel(octopus, project, test.identifier) + }) + + api.ExpectRequest(t, "GET", "/api/").RespondWith(findRootResource) + api.ExpectRequest(t, "GET", "/api/spaces").RespondWith(findRootResource) + api.ExpectRequest(t, "GET", "/api/Spaces-1/projects/"+findProjectID+"/channels").RespondWith(resources.Resources[*channels.Channel]{ + Items: allChannels, + }) + + result, err := testutil.ReceivePair(receiver) + if test.expectedErr == "" { + assert.Nil(t, err) + assert.Equal(t, test.expectedID, result.ID) + } else { + assert.EqualError(t, err, test.expectedErr) + } + }) + } +} + +func TestFindTenants(t *testing.T) { + cokeTenant := fixtures.NewTenant(findSpaceID, "Tenants-29", "Coke", "Regions/us-east") + + t.Run("finds a tenant by ID", func(t *testing.T) { + api := testutil.NewMockHttpServer() + receiver := beginRequest(api, func(octopus *octopusApiClient.Client) ([]*tenants.Tenant, error) { + return selectors.FindTenants(octopus, []string{"Tenants-29"}) + }) + + api.ExpectRequest(t, "GET", "/api/").RespondWith(findRootResource) + api.ExpectRequest(t, "GET", "/api/spaces").RespondWith(findRootResource) + api.ExpectRequest(t, "GET", "/api/Spaces-1/tenants/Tenants-29").RespondWith(cokeTenant) + + result, err := testutil.ReceivePair(receiver) + assert.Nil(t, err) + assert.Equal(t, []string{cokeTenant.ID}, util.SliceTransform(result, func(tenant *tenants.Tenant) string { return tenant.ID })) + }) + + t.Run("falls back to a name lookup when the ID doesn't exist", func(t *testing.T) { + api := testutil.NewMockHttpServer() + receiver := beginRequest(api, func(octopus *octopusApiClient.Client) ([]*tenants.Tenant, error) { + return selectors.FindTenants(octopus, []string{"Coke"}) + }) + + api.ExpectRequest(t, "GET", "/api/").RespondWith(findRootResource) + api.ExpectRequest(t, "GET", "/api/spaces").RespondWith(findRootResource) + api.ExpectRequest(t, "GET", "/api/Spaces-1/tenants/Coke").RespondWithStatus(404, "NotFound", nil) + api.ExpectRequest(t, "GET", "/api/Spaces-1/tenants?partialName=Coke").RespondWith(resources.Resources[*tenants.Tenant]{ + Items: []*tenants.Tenant{cokeTenant}, + }) + + result, err := testutil.ReceivePair(receiver) + assert.Nil(t, err) + assert.Equal(t, []string{cokeTenant.ID}, util.SliceTransform(result, func(tenant *tenants.Tenant) string { return tenant.ID })) + }) + + t.Run("errors when nothing matches", func(t *testing.T) { + api := testutil.NewMockHttpServer() + receiver := beginRequest(api, func(octopus *octopusApiClient.Client) ([]*tenants.Tenant, error) { + return selectors.FindTenants(octopus, []string{"Tenants-404"}) + }) + + api.ExpectRequest(t, "GET", "/api/").RespondWith(findRootResource) + api.ExpectRequest(t, "GET", "/api/spaces").RespondWith(findRootResource) + api.ExpectRequest(t, "GET", "/api/Spaces-1/tenants/Tenants-404").RespondWithStatus(404, "NotFound", nil) + api.ExpectRequest(t, "GET", "/api/Spaces-1/tenants?partialName=Tenants-404").RespondWith(resources.Resources[*tenants.Tenant]{}) + + _, err := testutil.ReceivePair(receiver) + assert.EqualError(t, err, "cannot find a tenant with the ID or name of 'Tenants-404'") + }) +} diff --git a/pkg/question/selectors/tenants.go b/pkg/question/selectors/tenants.go new file mode 100644 index 00000000..6b51d26d --- /dev/null +++ b/pkg/question/selectors/tenants.go @@ -0,0 +1,38 @@ +package selectors + +import ( + "errors" + "fmt" + + octopusApiClient "github.com/OctopusDeploy/go-octopusdeploy/v2/pkg/client" + "github.com/OctopusDeploy/go-octopusdeploy/v2/pkg/services" + "github.com/OctopusDeploy/go-octopusdeploy/v2/pkg/tenants" +) + +// FindTenant looks a tenant up by either its ID or its name. +func FindTenant(octopus *octopusApiClient.Client, tenantIdentifier string) (*tenants.Tenant, error) { + tenant, err := octopus.Tenants.GetByIdentifier(tenantIdentifier) + if err != nil { + if errors.Is(err, services.ErrItemNotFound) { + return nil, fmt.Errorf("cannot find a tenant with the ID or name of '%s'", tenantIdentifier) + } + return nil, err + } + return tenant, nil +} + +// FindTenants looks tenants up by either their IDs or their names. +func FindTenants(octopus *octopusApiClient.Client, tenantIdentifiers []string) ([]*tenants.Tenant, error) { + if len(tenantIdentifiers) == 0 { + return nil, nil + } + result := make([]*tenants.Tenant, 0, len(tenantIdentifiers)) + for _, identifier := range tenantIdentifiers { + tenant, err := FindTenant(octopus, identifier) + if err != nil { + return nil, err + } + result = append(result, tenant) + } + return result, nil +} From 7195c0ff3d5321a4f7cb91fa71c19478d27ab004 Mon Sep 17 00:00:00 2001 From: Nick Josevski Date: Wed, 19 Aug 2026 15:06:34 +1000 Subject: [PATCH 5/6] test: reconcile release deploy expectations across the tier 1 fixes The four fixes are green individually but their mock request sequences disagree once merged: #294 adds a release pre-flight lookup and removes the post-deploy web URL lookups, #250 adds an environment lookup, and the tests #250 and #556 introduce expect neither. Two of those cases deadlock the mock server rather than failing. Co-Authored-By: Claude Opus 5 (1M context) --- pkg/cmd/release/deploy/deploy_test.go | 17 ++++++++++------- pkg/cmd/runbook/run/run_test.go | 1 + 2 files changed, 11 insertions(+), 7 deletions(-) diff --git a/pkg/cmd/release/deploy/deploy_test.go b/pkg/cmd/release/deploy/deploy_test.go index 9fb52877..bcb9b68f 100644 --- a/pkg/cmd/release/deploy/deploy_test.go +++ b/pkg/cmd/release/deploy/deploy_test.go @@ -1706,6 +1706,7 @@ func TestDeployCreate_AutomationMode(t *testing.T) { api.ExpectRequest(t, "GET", "/api/Spaces-1/tenants/"+cokeTenant.ID).RespondWith(cokeTenant) api.ExpectRequest(t, "GET", "/api/Spaces-1/projects/"+fireProjectID).RespondWith(fireProject) + api.ExpectRequest(t, "GET", "/api/Spaces-1/projects/"+fireProjectID+"/releases/1.0").RespondWith(release10) api.ExpectRequest(t, "GET", "/api/Spaces-1/environments/all").RespondWith([]*environments.Environment{devEnvironment, testEnvironment}) req := api.ExpectRequest(t, "POST", "/api/Spaces-1/deployments/create/tenanted/v1") @@ -1730,13 +1731,6 @@ func TestDeployCreate_AutomationMode(t *testing.T) { }, }) - // now it's going to try and look up the project/version to generate the web URL - api.ExpectRequest(t, "GET", "/api/Spaces-1/projects/Fire Project").RespondWithStatus(404, "NotFound", nil) - api.ExpectRequest(t, "GET", "/api/Spaces-1/projects?partialName=Fire+Project").RespondWith(resources.Resources[*projects.Project]{ - Items: []*projects.Project{fireProject}, - }) - api.ExpectRequest(t, "GET", "/api/Spaces-1/projects/"+fireProjectID+"/releases/1.0").RespondWith(release10) - _, err = testutil.ReceivePair(cmdReceiver) assert.Nil(t, err) @@ -2137,6 +2131,8 @@ func TestDeployCreate_AutomationMode(t *testing.T) { api.ExpectRequest(t, "GET", "/api/").RespondWith(rootResource) api.ExpectRequest(t, "GET", "/api/Spaces-1").RespondWith(rootResource) api.ExpectRequest(t, "GET", "/api/Spaces-1/projects/"+fireProject.GetName()).RespondWith(fireProject) + api.ExpectRequest(t, "GET", "/api/Spaces-1/projects/"+fireProjectID+"/releases/1.0").RespondWith(release10) + api.ExpectRequest(t, "GET", "/api/Spaces-1/environments/all").RespondWith([]*environments.Environment{devEnvironment, testEnvironment}) req := api.ExpectRequest(t, "POST", "/api/Spaces-1/deployments/create/untenanted/v1") requestBody, err := testutil.ReadJson[deployments.CreateDeploymentUntenantedCommandV1](req.Request.Body) @@ -2183,7 +2179,14 @@ func TestDeployCreate_AutomationMode(t *testing.T) { api.ExpectRequest(t, "GET", "/api/").RespondWith(rootResource) api.ExpectRequest(t, "GET", "/api/Spaces-1").RespondWith(rootResource) + api.ExpectRequest(t, "GET", "/api/Spaces-1/tenants/Coke").RespondWithStatus(404, "NotFound", nil) + api.ExpectRequest(t, "GET", "/api/Spaces-1/tenants?partialName=Coke").RespondWith(resources.Resources[*tenants.Tenant]{Items: []*tenants.Tenant{cokeTenant}}) + api.ExpectRequest(t, "GET", "/api/Spaces-1/tenants/Pepsi").RespondWithStatus(404, "NotFound", nil) + api.ExpectRequest(t, "GET", "/api/Spaces-1/tenants?partialName=Pepsi").RespondWith(resources.Resources[*tenants.Tenant]{Items: []*tenants.Tenant{pepsiTenant}}) + api.ExpectRequest(t, "GET", "/api/Spaces-1/projects/"+fireProject.GetName()).RespondWith(fireProject) + api.ExpectRequest(t, "GET", "/api/Spaces-1/projects/"+fireProjectID+"/releases/1.0").RespondWith(release10) + api.ExpectRequest(t, "GET", "/api/Spaces-1/environments/all").RespondWith([]*environments.Environment{devEnvironment, testEnvironment}) req := api.ExpectRequest(t, "POST", "/api/Spaces-1/deployments/create/tenanted/v1") requestBody, err := testutil.ReadJson[deployments.CreateDeploymentTenantedCommandV1](req.Request.Body) diff --git a/pkg/cmd/runbook/run/run_test.go b/pkg/cmd/runbook/run/run_test.go index d1c61af5..ff5f6453 100644 --- a/pkg/cmd/runbook/run/run_test.go +++ b/pkg/cmd/runbook/run/run_test.go @@ -419,6 +419,7 @@ func TestRunbookRun_AutomationMode(t *testing.T) { api.ExpectRequest(t, "GET", "/api/").RespondWith(rootResource) api.ExpectRequest(t, "GET", "/api/Spaces-1").RespondWith(rootResource) api.ExpectRequest(t, "GET", "/api/Spaces-1/projects/Fire Project").RespondWithJSON(fixtures.AsServerResponse(fireProject)) + api.ExpectRequest(t, "GET", "/api/Spaces-1/environments/all").RespondWith([]*environments.Environment{devEnvironment, testEnvironment}) req := api.ExpectRequest(t, "POST", "/api/Spaces-1/runbook-runs/create/v1") requestBody, err := testutil.ReadJson[runbooks.RunbookRunCommandV1](req.Request.Body) From ceae659f870a244d8fdd9bae10e21f898d8e74a5 Mon Sep 17 00:00:00 2001 From: Nick Josevski Date: Wed, 19 Aug 2026 15:06:35 +1000 Subject: [PATCH 6/6] test: add integration tests for the tier 1 release fixes Covers behaviour that only a real server exercises: unknown release versions, packages with no version in their feed, channel and environment IDs on the executions API, and comma-separated deployment targets. Refs #294, #426, #250, #556 Co-Authored-By: Claude Opus 5 (1M context) --- test/integration/release_test.go | 299 +++++++++++++++++++++++++++++++ 1 file changed, 299 insertions(+) diff --git a/test/integration/release_test.go b/test/integration/release_test.go index b6f476f3..5f439555 100644 --- a/test/integration/release_test.go +++ b/test/integration/release_test.go @@ -8,13 +8,19 @@ import ( octopusApiClient "github.com/OctopusDeploy/go-octopusdeploy/v2/pkg/client" "github.com/OctopusDeploy/go-octopusdeploy/v2/pkg/core" "github.com/OctopusDeploy/go-octopusdeploy/v2/pkg/deployments" + "github.com/OctopusDeploy/go-octopusdeploy/v2/pkg/environments" + "github.com/OctopusDeploy/go-octopusdeploy/v2/pkg/lifecycles" + "github.com/OctopusDeploy/go-octopusdeploy/v2/pkg/machines" + "github.com/OctopusDeploy/go-octopusdeploy/v2/pkg/packages" "github.com/OctopusDeploy/go-octopusdeploy/v2/pkg/projects" "github.com/OctopusDeploy/go-octopusdeploy/v2/pkg/releases" + "github.com/OctopusDeploy/go-octopusdeploy/v2/pkg/tasks" "github.com/google/uuid" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" "os/exec" "testing" + "time" ) const space1ID = "Spaces-1" @@ -256,3 +262,296 @@ func TestReleaseListAndDelete(t *testing.T) { // the error struct contains an error message, but the server can/will change this over time, and we don't particularly care about it; 404 statuscode is the important bit }) } + +func createEnvironment(t *testing.T, apiClient *octopusApiClient.Client, name string) *environments.Environment { + environment, err := apiClient.Environments.Add(environments.NewEnvironment(name)) + if !testutil.AssertSuccess(t, err) { + return nil + } + t.Cleanup(func() { assert.Nil(t, apiClient.Environments.DeleteByID(environment.GetID())) }) + return environment +} + +func createCloudRegionTarget(t *testing.T, apiClient *octopusApiClient.Client, name string, environmentID string) *machines.DeploymentTarget { + target, err := apiClient.Machines.Add(machines.NewDeploymentTarget(name, machines.NewCloudRegionEndpoint(), []string{environmentID}, []string{"deploy"})) + if !testutil.AssertSuccess(t, err) { + return nil + } + t.Cleanup(func() { assert.Nil(t, apiClient.Machines.DeleteByID(target.GetID())) }) + return target +} + +// allowDeploymentsTo replaces the fixture lifecycle's phases with a single phase for the +// given environment, so releases in the project can be deployed to it. +func allowDeploymentsTo(t *testing.T, apiClient *octopusApiClient.Client, lifecycle *lifecycles.Lifecycle, environmentID string) bool { + phase := lifecycles.NewPhase("phase1") + phase.OptionalDeploymentTargets = []string{environmentID} + lifecycle.Phases = []*lifecycles.Phase{phase} + updated, err := apiClient.Lifecycles.Update(lifecycle) + if !testutil.AssertSuccess(t, err) { + return false + } + t.Cleanup(func() { + updated.Phases = nil + _, err := apiClient.Lifecycles.Update(updated) + assert.Nil(t, err) + }) + return true +} + +// waitForTaskToComplete blocks until the deployment's server task finishes; the project cannot be +// deleted while it is still running. Whether it succeeded is not this test's concern. +func waitForTaskToComplete(t *testing.T, apiClient *octopusApiClient.Client, taskID string) { + deadline := time.Now().Add(2 * time.Minute) + for time.Now().Before(deadline) { + found, err := apiClient.Tasks.Get(tasks.TasksQuery{IDs: []string{taskID}}) + if !testutil.AssertSuccess(t, err) { + return + } + if len(found.Items) == 1 && found.Items[0].IsCompleted != nil && *found.Items[0].IsCompleted { + return + } + time.Sleep(2 * time.Second) + } + t.Errorf("timed out waiting for task %s to complete", taskID) +} + +// scriptStep builds a single inline script step. With no target roles it runs on the server, so +// the project is deployable without any deployment targets. +func scriptStep(name string, targetRoles string) *deployments.DeploymentStep { + stepProperties := map[string]core.PropertyValue{} + action := &deployments.DeploymentAction{ + ActionType: "Octopus.Script", + Name: name, + Properties: map[string]core.PropertyValue{ + "Octopus.Action.Script.ScriptBody": core.NewPropertyValue("echo 'hello'", false), + }, + } + if targetRoles != "" { + stepProperties["Octopus.Action.TargetRoles"] = core.NewPropertyValue(targetRoles, false) + } else { + action.Properties["Octopus.Action.RunOnServer"] = core.NewPropertyValue("true", false) + } + return &deployments.DeploymentStep{Name: name, Properties: stepProperties, Actions: []*deployments.DeploymentAction{action}} +} + +// packageStep builds a single package step. The server rejects a package on an inline script, +// so a release that needs a package version has to go through this step type. +func packageStep(name string, targetRoles string, packageID string) *deployments.DeploymentStep { + return &deployments.DeploymentStep{ + Name: name, + Properties: map[string]core.PropertyValue{"Octopus.Action.TargetRoles": core.NewPropertyValue(targetRoles, false)}, + Actions: []*deployments.DeploymentAction{ + { + ActionType: "Octopus.TentaclePackage", + Name: name, + Properties: map[string]core.PropertyValue{}, + Packages: []*packages.PackageReference{ + { + PackageID: packageID, + FeedID: "feeds-builtin", + AcquisitionLocation: "Server", + Properties: map[string]string{"SelectionMode": "immediate"}, + }, + }, + }, + }, + } +} + +func setDeploymentProcess(t *testing.T, apiClient *octopusApiClient.Client, project *projects.Project, step *deployments.DeploymentStep) bool { + deploymentProcess, err := apiClient.DeploymentProcesses.Get(project, "") + if !testutil.AssertSuccess(t, err) { + return false + } + deploymentProcess.Steps = []*deployments.DeploymentStep{step} + _, err = apiClient.DeploymentProcesses.Update(deploymentProcess) + return testutil.AssertSuccess(t, err) +} + +func onlyReleaseInProject(t *testing.T, apiClient *octopusApiClient.Client, project *projects.Project) *releases.Release { + projectReleases, err := apiClient.Projects.GetReleases(project) + if !testutil.AssertSuccess(t, err) { + return nil + } + require.Equal(t, 1, len(projectReleases)) + return projectReleases[0] +} + +func onlyDeploymentOfRelease(t *testing.T, apiClient *octopusApiClient.Client, release *releases.Release) *deployments.Deployment { + releaseDeployments, err := apiClient.Deployments.GetDeployments(release) + if !testutil.AssertSuccess(t, err) { + return nil + } + require.Equal(t, 1, len(releaseDeployments.Items)) + return releaseDeployments.Items[0] +} + +// The executions API reports an unknown release version poorly - as a null reference error on the +// servers in issue #294, and as a bare "was not found" on current ones - so the CLI resolves the +// version up front and says what it looked for. +func TestReleaseDeployUnknownVersion(t *testing.T) { + runId := uuid.New() + apiClient, err := integration.GetApiClient(space1ID) + testutil.RequireSuccess(t, err) + + fx, err := integration.CreateCommonProject(t, apiClient, runId) + testutil.RequireSuccess(t, err) + project := fx.Project + + environment := createEnvironment(t, apiClient, fmt.Sprintf("env-%s", runId)) + if environment == nil { + return + } + + t.Run("the API does not answer with a usable release", func(t *testing.T) { + release, err := releases.GetReleaseInProject(apiClient, space1ID, project.GetID(), "9.9.9") + assert.True(t, err != nil || release == nil || release.GetID() == "") + }) + + t.Run("deploy names the version it could not find", func(t *testing.T) { + _, stdErr, err := integration.RunCli(space1ID, "release", "deploy", "--project", project.Name, "--version", "9.9.9", "--environment", environment.Name) + assert.Error(t, err) + assert.Contains(t, stdErr, fmt.Sprintf("cannot find a release with version '9.9.9' in project '%s'", project.Name)) + assert.NotContains(t, stdErr, "Object reference not set") + }) + + t.Run("deploy reports that latest is not an alias", func(t *testing.T) { + _, stdErr, err := integration.RunCli(space1ID, "release", "deploy", "--project", project.Name, "--version", "latest", "--environment", environment.Name) + assert.Error(t, err) + assert.Contains(t, stdErr, "'latest' is not a supported alias") + assert.NotContains(t, stdErr, "Object reference not set") + }) +} + +// A package with no version in its feed fails the release with no indication of which package is +// at fault, so the CLI diagnoses the failure and names them. See issue #426. +func TestReleaseCreateMissingPackageVersion(t *testing.T) { + runId := uuid.New() + apiClient, err := integration.GetApiClient(space1ID) + testutil.RequireSuccess(t, err) + + fx, err := integration.CreateCommonProject(t, apiClient, runId) + testutil.RequireSuccess(t, err) + project := fx.Project + + stepName := fmt.Sprintf("step-%s", runId) + packageID := fmt.Sprintf("package-%s", runId) + if !setDeploymentProcess(t, apiClient, project, packageStep(stepName, "deploy", packageID)) { + return + } + + t.Run("create names the package that has no version", func(t *testing.T) { + _, stdErr, err := integration.RunCli(space1ID, "release", "create", "--project", project.Name) + assert.Error(t, err) + assert.Contains(t, stdErr, "no version could be found for the following packages") + assert.Contains(t, stdErr, packageID) + assert.Contains(t, stdErr, stepName) + assert.NotContains(t, stdErr, "Object reference not set") + }) +} + +// The executions API matches channels and environments by name only, so the CLI resolves IDs +// before sending them. See issue #250. +func TestReleaseCreateAndDeployByID(t *testing.T) { + runId := uuid.New() + apiClient, err := integration.GetApiClient(space1ID) + testutil.RequireSuccess(t, err) + + fx, err := integration.CreateCommonProject(t, apiClient, runId) + testutil.RequireSuccess(t, err) + project := fx.Project + + environment := createEnvironment(t, apiClient, fmt.Sprintf("env-%s", runId)) + if environment == nil { + return + } + if !allowDeploymentsTo(t, apiClient, fx.Lifecycle, environment.GetID()) { + return + } + if !setDeploymentProcess(t, apiClient, project, scriptStep(fmt.Sprintf("step-%s", runId), "")) { + return + } + t.Cleanup(func() { deleteAllReleasesInProject(t, apiClient, project) }) + + t.Run("create accepts a channel ID", func(t *testing.T) { + _, stdErr, err := integration.RunCli(space1ID, "release", "create", "--project", project.Name, "--channel", fx.ProjectDefaultChannel.GetID(), "--version", "1.0.0") + if !testutil.AssertSuccess(t, err, stdErr) { + return + } + release := onlyReleaseInProject(t, apiClient, project) + if release == nil { + return + } + assert.Equal(t, fx.ProjectDefaultChannel.GetID(), release.ChannelID) + }) + + t.Run("deploy accepts an environment ID", func(t *testing.T) { + _, stdErr, err := integration.RunCli(space1ID, "release", "deploy", "--project", project.Name, "--version", "1.0.0", "--environment", environment.GetID()) + if !testutil.AssertSuccess(t, err, stdErr) { + return + } + release := onlyReleaseInProject(t, apiClient, project) + if release == nil { + return + } + deployment := onlyDeploymentOfRelease(t, apiClient, release) + if deployment == nil { + return + } + assert.Equal(t, environment.GetID(), deployment.EnvironmentID) + waitForTaskToComplete(t, apiClient, deployment.TaskID) + }) +} + +// Comma-separated values are split before they reach the executions API, which otherwise reports +// the whole string as one unknown target. See issue #556. +func TestReleaseDeployCommaSeparatedTargets(t *testing.T) { + runId := uuid.New() + apiClient, err := integration.GetApiClient(space1ID) + testutil.RequireSuccess(t, err) + + fx, err := integration.CreateCommonProject(t, apiClient, runId) + testutil.RequireSuccess(t, err) + project := fx.Project + + environment := createEnvironment(t, apiClient, fmt.Sprintf("env-%s", runId)) + if environment == nil { + return + } + if !allowDeploymentsTo(t, apiClient, fx.Lifecycle, environment.GetID()) { + return + } + if !setDeploymentProcess(t, apiClient, project, scriptStep(fmt.Sprintf("step-%s", runId), "deploy")) { + return + } + + targetA := createCloudRegionTarget(t, apiClient, fmt.Sprintf("target-a-%s", runId), environment.GetID()) + targetB := createCloudRegionTarget(t, apiClient, fmt.Sprintf("target-b-%s", runId), environment.GetID()) + if targetA == nil || targetB == nil { + return + } + t.Cleanup(func() { deleteAllReleasesInProject(t, apiClient, project) }) + + _, stdErr, err := integration.RunCli(space1ID, "release", "create", "--project", project.Name, "--version", "1.0.0") + if !testutil.AssertSuccess(t, err, stdErr) { + return + } + + t.Run("deploy splits a comma-separated target list", func(t *testing.T) { + _, stdErr, err := integration.RunCli(space1ID, "release", "deploy", "--project", project.Name, "--version", "1.0.0", "--environment", environment.Name, "--deployment-target", fmt.Sprintf("%s,%s", targetA.Name, targetB.Name)) + if !testutil.AssertSuccess(t, err, stdErr) { + return + } + release := onlyReleaseInProject(t, apiClient, project) + if release == nil { + return + } + deployment := onlyDeploymentOfRelease(t, apiClient, release) + if deployment == nil { + return + } + assert.ElementsMatch(t, []string{targetA.GetID(), targetB.GetID()}, deployment.SpecificMachineIDs) + waitForTaskToComplete(t, apiClient, deployment.TaskID) + }) +}