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..93f098b4 100644 --- a/pkg/cmd/release/create/create.go +++ b/pkg/cmd/release/create/create.go @@ -5,6 +5,7 @@ import ( "errors" "fmt" "io" + "net/http" "os" "strings" "time" @@ -28,6 +29,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" @@ -310,6 +312,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 + } } } @@ -318,7 +328,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 +430,113 @@ func BuildPackageVersionBaselineForChannel(octopus *octopusApiClient.Client, dep return result, nil } +// 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 +// +// Any 500 is diagnosed, not just the null reference one, because the message a server sends for this +// varies by version: current servers report "no viable release plans" instead. The cost of being wrong +// is bounded, since MissingPackageVersionsError reports what the server actually said alongside the +// diagnosis. Other 5xx codes are excluded: the failure we are looking for is always raised by the API +// itself as a 500, so a 502/503/504 is something in front of the server and never worth replaying. +func DiagnoseCreateReleaseFailure(octopus *octopusApiClient.Client, options *executor.TaskOptionsCreateRelease, cause error) error { + var apiError *core.APIError + if !errors.As(cause, &apiError) || apiError.StatusCode != http.StatusInternalServerError { + 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, packages.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 + } + + // mirror what the server did: with --ignore-channel-rules it selects versions without applying the + // channel's version rules, so applying them here would report packages as missing when they only + // failed the rules. + var packageVersionBaseline []*packages.StepPackageVersion + if options.IgnoreChannelRules { + packageVersionBaseline, err = packages.BuildPackageVersionBaseline(octopus, deploymentProcessTemplate.Packages, nil) + } else { + 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. --channel reaches the server as +// ChannelIDOrName, so we match on either. 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, channelIDOrName string) (*channels.Channel, error) { + existingChannels, err := octopus.Projects.GetChannels(project) + if err != nil { + return nil, err + } + + if channelIDOrName != "" { + for _, c := range existingChannels { + if strings.EqualFold(c.Name, channelIDOrName) || c.ID == channelIDOrName { + return c, nil + } + } + return nil, fmt.Errorf("no channel found with name or ID of %s", channelIDOrName) + } + + 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..3792b867 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" @@ -1209,6 +1211,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 +1591,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 +1661,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 +1736,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 +1806,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 +1879,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 @@ -2829,3 +2895,344 @@ 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)) + }) + + t.Run("passes through server faults it cannot diagnose", func(t *testing.T) { + // a 500 is only replaced when the CLI can positively name the packages behind it. With no + // client to go and look, and no null reference message to explain, the server error stands. + serverError := &core.APIError{ErrorMessage: "The database is unavailable", StatusCode: http.StatusInternalServerError} + assert.Equal(t, error(serverError), create.DiagnoseCreateReleaseFailure(nil, nil, serverError)) + }) + + t.Run("passes through 5xx codes which aren't the API's own failure", func(t *testing.T) { + // the release plan failure is always raised by the API itself as a 500; anything else in the + // 5xx range came from in front of the server, so there is nothing worth replaying + for _, statusCode := range []int{http.StatusBadGateway, http.StatusServiceUnavailable, http.StatusGatewayTimeout} { + // carrying the null reference message, to show it's the status code doing the work here + gatewayError := &core.APIError{ErrorMessage: "Object reference not set to an instance of an object.", StatusCode: statusCode} + assert.Equal(t, error(gatewayError), create.DiagnoseCreateReleaseFailure(nil, nil, gatewayError)) + } + }) + + t.Run("explains a bare null reference fault even when no packages are missing", func(t *testing.T) { + nullRef := &core.APIError{ErrorMessage: "Object reference not set to an instance of an object.", StatusCode: http.StatusInternalServerError} + err := create.DiagnoseCreateReleaseFailure(nil, nil, nullRef) + assert.ErrorIs(t, err, nullRef) + assert.Contains(t, err.Error(), "the server failed with an unhandled error") + }) +} + +// 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()) + }}, + + {"doesn't apply channel version rules when --ignore-channel-rules was specified", func(t *testing.T, api *testutil.MockHttpServer, rootCmd *cobra.Command, stdOut *bytes.Buffer, stdErr *bytes.Buffer) { + // the server resolved versions without the channel rules, so the diagnosis must too; + // otherwise a package which only fails the rules gets reported as having no version at all + ruledChannel := fixtures.NewChannel(spaceID, "Channels-1", "Default", fireProjectID) + ruledChannel.Rules = []channels.ChannelRule{{ + Tag: "^pre$", + VersionRange: "[5.0,6.0)", + ActionPackages: []octopusPackages.DeploymentActionPackage{ + {DeploymentAction: "Deploy Website", PackageReference: "acme-web"}, + }, + }} + + cmdReceiver := testutil.GoBegin2(func() (*cobra.Command, error) { + defer api.Close() + rootCmd.SetArgs([]string{"release", "create", "--project", fireProject.Name, "--ignore-channel-rules"}) + 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) + + 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{ruledChannel}, + }) + 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}", + }}}, + }}) + // no versionRange or preReleaseTag in the query, despite the channel carrying a rule for this package + 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`)) + }}, + + {"finds the channel when --channel was given as an ID rather than a name", func(t *testing.T, api *testutil.MockHttpServer, rootCmd *cobra.Command, stdOut *bytes.Buffer, stdErr *bytes.Buffer) { + // --channel reaches the server as ChannelIDOrName with no client-side resolution, so an ID + // is a legitimate input; matching on name alone would silently drop the diagnosis + cmdReceiver := testutil.GoBegin2(func() (*cobra.Command, error) { + defer api.Close() + rootCmd.SetArgs([]string{"release", "create", "--project", fireProject.Name, "--channel", defaultChannel.ID, "--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, "GET", "/api/Spaces-1/projects/"+fireProjectID+"/channels").RespondWith(resources.Resources[*channels.Channel]{ + Items: []*channels.Channel{defaultChannel}, + }) + + api.ExpectRequest(t, "POST", "/api/Spaces-1/releases/create/v1"). + RespondWithStatus(http.StatusInternalServerError, "500 Internal Server Error", nullReferenceError) + + 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`)) + }}, + + {"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/cmd/release/deploy/deploy.go b/pkg/cmd/release/deploy/deploy.go index 835c523a..bb34ca83 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" ) @@ -171,9 +172,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; escape a comma inside a value as '\\,')") + flags.StringArrayVarP(&deployFlags.Tenants.Value, deployFlags.Tenants.Name, "", nil, "Deploy to this tenant (can be specified multiple times, or as a comma-separated list; escape a comma inside a value as '\\,')") + 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; escape a comma inside a value as '\\,'). 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") @@ -182,8 +183,8 @@ func NewCmdDeploy(f factory.Factory) *cobra.Command { flags.StringVarP(&deployFlags.GuidedFailureMode.Value, deployFlags.GuidedFailureMode.Name, "", "", "Enable Guided failure mode (true/false/default)") flags.StringVarP(&deployFlags.Priority.Value, deployFlags.Priority.Name, "", "", "Jump the task queue ahead of other queued tasks (true/false/default). Requires the Priority Tasks feature, and the TaskPrioritize permission to set true.") 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; escape a comma inside a value as '\\,')") + 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; escape a comma inside a value as '\\,')") flags.StringArrayVarP(&deployFlags.SpecificTargetTagNames.Value, deployFlags.SpecificTargetTagNames.Name, "", nil, "Deploy to targets matching this tag (can be specified multiple times)") flags.StringArrayVarP(&deployFlags.ExcludedTargetTagNames.Value, deployFlags.ExcludedTargetTagNames.Name, "", nil, "Deploy to targets except for those matching this tag (can be specified multiple times)") flags.StringArrayVarP(&deployFlags.DeploymentFreezeNames.Value, deployFlags.DeploymentFreezeNames.Name, "", nil, "Override this deployment freeze (can be specified multiple times)") @@ -212,6 +213,17 @@ 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 + if err := executionscommon.ExpandCommaSeparatedFlags( + flags.Environments, + flags.Tenants, + flags.TenantTags, + flags.DeploymentTargets, + flags.ExcludeTargets, + ); err != nil { + return err + } + outputFormat, err := cmd.Flags().GetString(constants.FlagOutputFormat) if err != nil { // should never happen, but fallback if it does outputFormat = constants.OutputFormatTable @@ -258,6 +270,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 @@ -276,16 +297,16 @@ func deployRun(cmd *cobra.Command, f factory.Factory, flags *DeployFlags) error resolvedFlags := NewDeployFlags() resolvedFlags.Project.Value = options.ProjectName resolvedFlags.ReleaseVersion.Value = options.ReleaseVersion - resolvedFlags.Environments.Value = options.Environments - resolvedFlags.Tenants.Value = options.Tenants - resolvedFlags.TenantTags.Value = options.TenantTags + resolvedFlags.Environments.Value = executionscommon.EscapeCommas(options.Environments) + resolvedFlags.Tenants.Value = executionscommon.EscapeCommas(options.Tenants) + resolvedFlags.TenantTags.Value = executionscommon.EscapeCommas(options.TenantTags) resolvedFlags.DeployAt.Value = options.ScheduledStartTime resolvedFlags.MaxQueueTime.Value = options.ScheduledExpiryTime resolvedFlags.ExcludedSteps.Value = options.ExcludedSteps resolvedFlags.GuidedFailureMode.Value = options.GuidedFailureMode + resolvedFlags.DeploymentTargets.Value = executionscommon.EscapeCommas(options.DeploymentTargets) + resolvedFlags.ExcludeTargets.Value = executionscommon.EscapeCommas(options.ExcludeTargets) resolvedFlags.Priority.Value = options.Priority - resolvedFlags.DeploymentTargets.Value = options.DeploymentTargets - resolvedFlags.ExcludeTargets.Value = options.ExcludeTargets resolvedFlags.SpecificTargetTagNames.Value = options.SpecificTargetTagNames resolvedFlags.ExcludedTargetTagNames.Value = options.ExcludedTargetTagNames resolvedFlags.DeploymentFreezeNames.Value = options.DeploymentFreezeNames @@ -344,8 +365,34 @@ 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. + // Only a *confirmed* "no such release" is fatal: this lookup is new to the deploy path, + // so anything else must not fail a deploy that would previously have succeeded. That + // covers both a non-not-found error (no ReleaseView permission, a transient 5xx) and an + // unconfirmed ReleaseNotFoundError, which is what an empty response body decodes to + // whatever its status code was - a bodyless 403 or a proxy's 502 included. In those + // cases the server stays the authority and we simply go without the release ID. + release, err := selectors.FindRelease(octopus, f.GetCurrentSpace().ID, project, options.ReleaseVersion) + var releaseNotFound *selectors.ReleaseNotFoundError + if errors.As(err, &releaseNotFound) && releaseNotFound.Confirmed { + return err + } + if err == nil { + options.ReleaseID = release.ID + } + } } + // the executions API only matches environments by name, so resolve any IDs we were given + if len(options.Environments) > 0 { + options.Environments, err = selectors.ResolveEnvironmentNames(octopus, f.GetCurrentSpace(), options.Environments) + if err != nil { + return err + } + } } // the executor will raise errors if any required options are missing @@ -377,20 +424,10 @@ func deployRun(cmd *cobra.Command, f factory.Factory, flags *DeployFlags) error // output web URL all the time, so long as output format is not JSON or basic if err == nil && !constants.IsProgrammaticOutputFormat(outputFormat) { - releaseID := options.ReleaseID - if releaseID == "" { - // we may already have the release ID from AskQuestions. If not, we need to go and look up the release ID to link to it - // which needs the project ID. Errors here are ignorable; it's not the end of the world if we can't print the web link - prj, err := selectors.FindProject(octopus, options.ProjectName) - if err == nil { - rel, err := releases.GetReleaseInProject(octopus, f.GetCurrentSpace().ID, prj.ID, options.ReleaseVersion) - if err == nil { - releaseID = rel.ID - } - } - } - - if releaseID != "" { + // both paths that reach here have already resolved the release: AskQuestions in interactive + // mode, the pre-flight lookup in automation mode. It stays empty only when that lookup failed + // for a reason we deliberately ignored, in which case repeating it here would fail too. + if releaseID := options.ReleaseID; releaseID != "" { link := output.Bluef("%s/app#/%s/releases/%s", f.GetCurrentHost(), f.GetCurrentSpace().ID, releaseID) cmd.Printf("\nView this release on Octopus Deploy: %s\n", link) } @@ -453,7 +490,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 } @@ -501,18 +538,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 }) } } @@ -664,7 +704,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 @@ -674,23 +714,25 @@ 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 } envMap := make(map[string]*ephemeralenvironments.EphemeralEnvironment, len(allEphemeralEnvironments.Items)*2) for _, ephemeralEnv := range allEphemeralEnvironments.Items { - envMap[strings.ToLower(ephemeralEnv.ID)] = ephemeralEnv envMap[strings.ToLower(ephemeralEnv.Name)] = ephemeralEnv } + for _, ephemeralEnv := range allEphemeralEnvironments.Items { // IDs go in second so an ID match wins a collision with another environment's name + envMap[strings.ToLower(ephemeralEnv.ID)] = 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 @@ -763,6 +805,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 24d8d238..d4ddb489 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, @@ -1728,7 +1728,59 @@ 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") + + // deploysDespiteFailedReleaseLookup asserts that a deployment still reaches the executions API when + // the pre-flight release lookup fails. Only a confirmed "no such release" may stop a deployment that + // would previously have gone straight to the server, so the cases using this differ only in how the + // lookup fails; respondToLookup answers the release request. + deploysDespiteFailedReleaseLookup := func(respondToLookup func(lookup *testutil.RequestWrapper)) func(t *testing.T, api *testutil.MockHttpServer, rootCmd *cobra.Command, stdOut *bytes.Buffer, stdErr *bytes.Buffer) { + return 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"}) + 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) + + respondToLookup(api.ExpectRequest(t, "GET", "/api/Spaces-1/projects/"+fireProjectID+"/releases/1.0")) + 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) + assert.Nil(t, err) + + assert.Equal(t, deployments.CreateDeploymentUntenantedCommandV1{ + ReleaseVersion: "1.0", + EnvironmentNames: []string{"dev"}, + 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) + + // no release ID, so no web link; the deployment itself still went ahead + assert.Equal(t, "Successfully started 1 deployment(s)\n", stdOut.String()) + assert.Equal(t, "", stdErr.String()) + } + } // TEST STARTS HERE tests := []struct { @@ -1780,6 +1832,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") @@ -1788,6 +1841,67 @@ 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) + // a real server answers this with a 404 carrying an APIError body, so the version is + // confirmed missing and the pre-flight is allowed to stop the deployment + api.ExpectRequest(t, "GET", "/api/Spaces-1/projects/"+fireProjectID+"/releases/latest"). + 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 '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 proceeds when the release lookup is forbidden with an error body", deploysDespiteFailedReleaseLookup( + // an account allowed to deploy but not to read releases must not be blocked by the pre-flight lookup + func(lookup *testutil.RequestWrapper) { + lookup.RespondWithStatus(403, "403 Forbidden", &core.APIError{ErrorMessage: "You do not have permission to perform this action."}) + })}, + + {"release deploy proceeds when the release lookup is forbidden with an empty body", deploysDespiteFailedReleaseLookup( + // an empty body decodes as a zero-valued release with no error, so this arrives as an + // unconfirmed ReleaseNotFoundError rather than on the error path + func(lookup *testutil.RequestWrapper) { + lookup.RespondWithStatus(403, "403 Forbidden", nil) + })}, + + {"release deploy proceeds when the release lookup hits an empty-bodied gateway error", deploysDespiteFailedReleaseLookup( + // e.g. a reverse proxy in front of the server answering with Content-Length: 0 + func(lookup *testutil.RequestWrapper) { + lookup.RespondWithStatus(502, "502 Bad Gateway", nil) + })}, + {"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() @@ -1798,6 +1912,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}) // 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") @@ -1820,18 +1936,63 @@ 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}, + // no lookup to generate the web URL; the release was already resolved before deploying + + _, err = testutil.ReceivePair(cmdReceiver) + assert.Nil(t, err) + + assert.Equal(t, heredoc.Docf(` + Successfully started 2 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, 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/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") + + // 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"}, + }, + }) + + // no further requests: the pre-flight lookup already gave us the release ID for the web URL _, err = testutil.ReceivePair(cmdReceiver) assert.Nil(t, err) assert.Equal(t, heredoc.Docf(` - Successfully started 2 deployment(s) + Successfully started 1 deployment(s) View this release on Octopus Deploy: http://server/app#/Spaces-1/releases/%s `, release10.ID), stdOut.String()) @@ -1848,6 +2009,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/projects/"+fireProject.GetName()).RespondWith(fireProject) + api.ExpectRequest(t, "GET", "/api/Spaces-1/projects/"+fireProjectID+"/releases/2.1").RespondWith(release10) + 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") @@ -1870,12 +2039,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) @@ -1898,6 +2062,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}) // 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{ @@ -1928,6 +2094,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}) // Note: because we didn't specify --tenant or --tenant-tag, automation-mode code is going to assume untenanted serverTasks := []*deployments.DeploymentServerTask{ @@ -1958,7 +2126,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) @@ -1980,12 +2155,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) @@ -2008,6 +2178,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/tenanted/v1") requestBody, err := testutil.ReadJson[deployments.CreateDeploymentTenantedCommandV1](req.Request.Body) @@ -2029,12 +2201,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) @@ -2057,6 +2224,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) @@ -2095,6 +2264,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) @@ -2166,6 +2337,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}) // 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") @@ -2245,7 +2418,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) @@ -2293,6 +2473,171 @@ 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) + 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) + 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) + // the comma form must resolve each tenant individually, exactly as the repeated form does + 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) + 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()) + }}, + + {"release deploy treats a backslash-escaped comma as part of the value", 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", + "--deployment-target", `Web\, Prod,Other`, + "--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) + 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) + assert.Nil(t, err) + + assert.Equal(t, []string{"Web, Prod", "Other"}, requestBody.SpecificMachineNames) + + 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()) + }}, + + // a --tenant that expands to nothing must not fall through to an untenanted deployment + {"release deploy rejects a blank comma-separated value rather than silently dropping it", 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", ",", // e.g. "$TENANT_A,$TENANT_B" where both are unset + "--output-format", "basic", + }) + return rootCmd.ExecuteC() + }) + + _, err := testutil.ReceivePair(cmdReceiver) + assert.ErrorContains(t, err, "--tenant has a blank value") + + assert.Equal(t, "", stdOut.String()) + }}, } for _, test := range tests { diff --git a/pkg/cmd/release/progression/shared/shared.go b/pkg/cmd/release/progression/shared/shared.go index 94f669b3..e9d7249d 100644 --- a/pkg/cmd/release/progression/shared/shared.go +++ b/pkg/cmd/release/progression/shared/shared.go @@ -16,7 +16,7 @@ func GetReleaseID(octopus *client.Client, spaceID string, projectIdentifier stri return "", err } - selectedRelease, err := FindRelease(octopus, selectedProject, version) + selectedRelease, err := selectors.FindRelease(octopus, spaceID, selectedProject, version) if err != nil { return "", err } @@ -38,16 +38,3 @@ func SelectRelease(octopus *client.Client, project *projects.Project, ask questi return selectedRelease, nil } - -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 -} diff --git a/pkg/cmd/runbook/run/run.go b/pkg/cmd/runbook/run/run.go index b0a5fa40..8f451701 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" ) @@ -173,9 +174,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; escape a comma inside a value as '\\,')") + flags.StringArrayVarP(&runFlags.Tenants.Value, runFlags.Tenants.Name, "", nil, "Run for this tenant (can be specified multiple times, or as a comma-separated list; escape a comma inside a value as '\\,')") + 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; escape a comma inside a value as '\\,'). 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") @@ -184,8 +185,8 @@ func NewCmdRun(f factory.Factory) *cobra.Command { flags.StringVarP(&runFlags.GuidedFailureMode.Value, runFlags.GuidedFailureMode.Name, "", "", "Enable Guided failure mode (true/false/default)") flags.StringVarP(&runFlags.Priority.Value, runFlags.Priority.Name, "", "", "Jump the task queue ahead of other queued tasks (true/false/default). Requires the Priority Tasks feature. For runbook runs, 'default' is the same as 'false'.") 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; escape a comma inside a value as '\\,')") + 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; escape a comma inside a value as '\\,')") flags.StringArrayVarP(&runFlags.SpecificTargetTagNames.Value, runFlags.SpecificTargetTagNames.Name, "", nil, "Run on targets matching this tag (can be specified multiple times)") flags.StringArrayVarP(&runFlags.ExcludedTargetTagNames.Value, runFlags.ExcludedTargetTagNames.Name, "", nil, "Run on targets except for those matching this tag (can be specified multiple times)") 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.") @@ -215,6 +216,17 @@ 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 + if err := executionscommon.ExpandCommaSeparatedFlags( + flags.Environments, + flags.Tenants, + flags.TenantTags, + flags.RunTargets, + flags.ExcludeTargets, + ); err != nil { + return err + } + 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") } @@ -246,6 +258,27 @@ 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. + // Run previews and target selection need the IDs, so keep the whole resolved identity rather than + // looking the names up again later - an ID-first lookup of a name that collides with another + // environment's ID would land on the other environment. + var resolvedEnvironments []*selectors.ResolvedEnvironment + if len(flags.Environments.Value) > 0 { + resolvedEnvironments, err = selectors.ResolveEnvironments(octopus, f.GetCurrentSpace(), flags.Environments.Value) + if err != nil { + return err + } + flags.Environments.Value = util.SliceTransform(resolvedEnvironments, func(env *selectors.ResolvedEnvironment) 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{ @@ -282,20 +315,20 @@ func runbookRun(cmd *cobra.Command, f factory.Factory, flags *RunFlags) error { if len(flags.RunbookTags.Value) > 0 { if shared.AreRunbooksInGit(project) { - return runRunbooksByTag(cmd, f, flags, octopus, project, parsedVariables, outputFormat, true) + return runRunbooksByTag(cmd, f, flags, octopus, project, resolvedEnvironments, parsedVariables, outputFormat, true) } else { - return runRunbooksByTag(cmd, f, flags, octopus, project, parsedVariables, outputFormat, false) + return runRunbooksByTag(cmd, f, flags, octopus, project, resolvedEnvironments, parsedVariables, outputFormat, false) } } if shared.AreRunbooksInGit(project) { - return runGitRunbook(cmd, f, flags, octopus, project, parsedVariables, outputFormat) + return runGitRunbook(cmd, f, flags, octopus, project, resolvedEnvironments, parsedVariables, outputFormat) } else { - return runDbRunbook(cmd, f, flags, octopus, project, parsedVariables, outputFormat) + return runDbRunbook(cmd, f, flags, octopus, project, resolvedEnvironments, parsedVariables, outputFormat) } } -func runDbRunbook(cmd *cobra.Command, f factory.Factory, flags *RunFlags, octopus *octopusApiClient.Client, project *projects.Project, parsedVariables map[string]string, outputFormat string) error { +func runDbRunbook(cmd *cobra.Command, f factory.Factory, flags *RunFlags, octopus *octopusApiClient.Client, project *projects.Project, resolvedEnvironments []*selectors.ResolvedEnvironment, parsedVariables map[string]string, outputFormat string) error { commonOptions := &executor.TaskOptionsRunbookRunBase{ ProjectName: project.Name, @@ -334,7 +367,7 @@ func runDbRunbook(cmd *cobra.Command, f factory.Factory, flags *RunFlags, octopu } } - err := AskDbRunbookRunQuestions(octopus, cmd.OutOrStdout(), f.Ask, f.GetCurrentSpace(), project, options, now) + err := AskDbRunbookRunQuestions(octopus, cmd.OutOrStdout(), f.Ask, f.GetCurrentSpace(), project, options, resolvedEnvironments, now) if err != nil { return err } @@ -344,16 +377,16 @@ func runDbRunbook(cmd *cobra.Command, f factory.Factory, flags *RunFlags, octopu resolvedFlags := NewRunFlags() resolvedFlags.Project.Value = options.ProjectName resolvedFlags.RunbookName.Value = options.RunbookName - resolvedFlags.Environments.Value = options.Environments - resolvedFlags.Tenants.Value = options.Tenants - resolvedFlags.TenantTags.Value = options.TenantTags + resolvedFlags.Environments.Value = executionscommon.EscapeCommas(options.Environments) + resolvedFlags.Tenants.Value = executionscommon.EscapeCommas(options.Tenants) + resolvedFlags.TenantTags.Value = executionscommon.EscapeCommas(options.TenantTags) resolvedFlags.RunAt.Value = options.ScheduledStartTime resolvedFlags.MaxQueueTime.Value = options.ScheduledExpiryTime resolvedFlags.ExcludedSteps.Value = options.ExcludedSteps resolvedFlags.GuidedFailureMode.Value = options.GuidedFailureMode + resolvedFlags.RunTargets.Value = executionscommon.EscapeCommas(options.RunTargets) + resolvedFlags.ExcludeTargets.Value = executionscommon.EscapeCommas(options.ExcludeTargets) resolvedFlags.Priority.Value = options.Priority - resolvedFlags.RunTargets.Value = options.RunTargets - resolvedFlags.ExcludeTargets.Value = options.ExcludeTargets resolvedFlags.SpecificTargetTagNames.Value = options.SpecificTargetTagNames resolvedFlags.ExcludedTargetTagNames.Value = options.ExcludedTargetTagNames @@ -435,7 +468,7 @@ func runDbRunbook(cmd *cobra.Command, f factory.Factory, flags *RunFlags, octopu return nil } -func runGitRunbook(cmd *cobra.Command, f factory.Factory, flags *RunFlags, octopus *octopusApiClient.Client, project *projects.Project, parsedVariables map[string]string, outputFormat string) error { +func runGitRunbook(cmd *cobra.Command, f factory.Factory, flags *RunFlags, octopus *octopusApiClient.Client, project *projects.Project, resolvedEnvironments []*selectors.ResolvedEnvironment, parsedVariables map[string]string, outputFormat string) error { commonOptions := &executor.TaskOptionsRunbookRunBase{ ProjectName: project.Name, @@ -477,7 +510,7 @@ func runGitRunbook(cmd *cobra.Command, f factory.Factory, flags *RunFlags, octop } } - err := AskGitRunbookRunQuestions(octopus, cmd.OutOrStdout(), f.Ask, f.GetCurrentSpace(), project, options, now) + err := AskGitRunbookRunQuestions(octopus, cmd.OutOrStdout(), f.Ask, f.GetCurrentSpace(), project, options, resolvedEnvironments, now) if err != nil { return err } @@ -487,16 +520,16 @@ func runGitRunbook(cmd *cobra.Command, f factory.Factory, flags *RunFlags, octop resolvedFlags := NewRunFlags() resolvedFlags.Project.Value = options.ProjectName resolvedFlags.RunbookName.Value = options.RunbookName - resolvedFlags.Environments.Value = options.Environments - resolvedFlags.Tenants.Value = options.Tenants - resolvedFlags.TenantTags.Value = options.TenantTags + resolvedFlags.Environments.Value = executionscommon.EscapeCommas(options.Environments) + resolvedFlags.Tenants.Value = executionscommon.EscapeCommas(options.Tenants) + resolvedFlags.TenantTags.Value = executionscommon.EscapeCommas(options.TenantTags) resolvedFlags.RunAt.Value = options.ScheduledStartTime resolvedFlags.MaxQueueTime.Value = options.ScheduledExpiryTime resolvedFlags.ExcludedSteps.Value = options.ExcludedSteps resolvedFlags.GuidedFailureMode.Value = options.GuidedFailureMode + resolvedFlags.RunTargets.Value = executionscommon.EscapeCommas(options.RunTargets) + resolvedFlags.ExcludeTargets.Value = executionscommon.EscapeCommas(options.ExcludeTargets) resolvedFlags.Priority.Value = options.Priority - resolvedFlags.RunTargets.Value = options.RunTargets - resolvedFlags.ExcludeTargets.Value = options.ExcludeTargets resolvedFlags.SpecificTargetTagNames.Value = options.SpecificTargetTagNames resolvedFlags.ExcludedTargetTagNames.Value = options.ExcludedTargetTagNames resolvedFlags.GitRef.Value = options.GitReference @@ -682,7 +715,7 @@ func askCommonAdvancedOptions( return nil } -func AskDbRunbookRunQuestions(octopus *octopusApiClient.Client, stdout io.Writer, asker question.Asker, space *spaces.Space, project *projects.Project, options *executor.TaskOptionsRunbookRun, now func() time.Time) error { +func AskDbRunbookRunQuestions(octopus *octopusApiClient.Client, stdout io.Writer, asker question.Asker, space *spaces.Space, project *projects.Project, options *executor.TaskOptionsRunbookRun, resolvedEnvironments []*selectors.ResolvedEnvironment, now func() time.Time) error { if octopus == nil { return cliErrors.NewArgumentNullOrEmptyError("octopus") } @@ -720,14 +753,18 @@ func AskDbRunbookRunQuestions(octopus *octopusApiClient.Client, stdout io.Writer return err } - // machine selection later on needs to refer back to the environments. + // machine selection later on needs to refer back to the environments. When the environments came + // from the command line the caller has already resolved them, so reuse those IDs rather than + // resolving options.Environments - now canonical names - a second time. var selectedEnvironments []*environments.Environment + environmentIDs := util.SliceTransform(resolvedEnvironments, func(env *selectors.ResolvedEnvironment) string { return env.ID }) if len(options.Environments) == 0 { selectedEnvironments, err = selectRunEnvironments(asker, octopus, space, project, selectedRunbook) if err != nil { return err } options.Environments = util.SliceTransform(selectedEnvironments, func(env *environments.Environment) string { return env.Name }) + environmentIDs = util.SliceTransform(selectedEnvironments, func(env *environments.Environment) string { return env.ID }) } else { _, _ = fmt.Fprintf(stdout, "Environments %s\n", output.Cyan(strings.Join(options.Environments, ","))) } @@ -829,14 +866,15 @@ func AskDbRunbookRunQuestions(octopus *octopusApiClient.Client, stdout io.Writer } if !isRunTargetsSpecified { - if len(selectedEnvironments) == 0 { // if the Q&A process earlier hasn't loaded environments already, we need to load them now - selectedEnvironments, err = executionscommon.FindEnvironments(octopus, options.Environments) + if len(environmentIDs) == 0 { // if the Q&A process earlier hasn't loaded environments already, we need to load them now + envs, err := executionscommon.FindEnvironments(octopus, options.Environments) if err != nil { return err } + environmentIDs = util.SliceTransform(envs, func(env *environments.Environment) string { return env.ID }) } - options.RunTargets, err = askRunbookTargets(octopus, asker, space.ID, selectedSnapshot.ID, selectedEnvironments) + options.RunTargets, err = askRunbookTargets(octopus, asker, space.ID, selectedSnapshot.ID, environmentIDs) if err != nil { return err } @@ -860,7 +898,7 @@ func AskDbRunbookRunQuestions(octopus *octopusApiClient.Client, stdout io.Writer return nil } -func AskGitRunbookRunQuestions(octopus *octopusApiClient.Client, stdout io.Writer, asker question.Asker, space *spaces.Space, project *projects.Project, options *executor.TaskOptionsGitRunbookRun, now func() time.Time) error { +func AskGitRunbookRunQuestions(octopus *octopusApiClient.Client, stdout io.Writer, asker question.Asker, space *spaces.Space, project *projects.Project, options *executor.TaskOptionsGitRunbookRun, resolvedEnvironments []*selectors.ResolvedEnvironment, now func() time.Time) error { if octopus == nil { return cliErrors.NewArgumentNullOrEmptyError("octopus") } @@ -909,14 +947,18 @@ func AskGitRunbookRunQuestions(octopus *octopusApiClient.Client, stdout io.Write return err } - // machine selection later on needs to refer back to the environments. + // machine selection later on needs to refer back to the environments. When the environments came + // from the command line the caller has already resolved them, so reuse those IDs rather than + // resolving options.Environments - now canonical names - a second time. var selectedEnvironments []*environments.Environment + environmentIDs := util.SliceTransform(resolvedEnvironments, func(env *selectors.ResolvedEnvironment) string { return env.ID }) if len(options.Environments) == 0 { selectedEnvironments, err = selectGitRunEnvironments(asker, octopus, space, project, selectedRunbook, options.GitReference) if err != nil { return err } options.Environments = util.SliceTransform(selectedEnvironments, func(env *environments.Environment) string { return env.Name }) + environmentIDs = util.SliceTransform(selectedEnvironments, func(env *environments.Environment) string { return env.ID }) } else { _, _ = fmt.Fprintf(stdout, "Environments %s\n", output.Cyan(strings.Join(options.Environments, ","))) } @@ -1052,14 +1094,15 @@ func AskGitRunbookRunQuestions(octopus *octopusApiClient.Client, stdout io.Write } if !isRunTargetsSpecified { - if len(selectedEnvironments) == 0 { // if the Q&A process earlier hasn't loaded environments already, we need to load them now - selectedEnvironments, err = executionscommon.FindEnvironments(octopus, options.Environments) + if len(environmentIDs) == 0 { // if the Q&A process earlier hasn't loaded environments already, we need to load them now + envs, err := executionscommon.FindEnvironments(octopus, options.Environments) if err != nil { return err } + environmentIDs = util.SliceTransform(envs, func(env *environments.Environment) string { return env.ID }) } - options.RunTargets, err = askGitRunbookTargets(octopus, asker, space.ID, project.ID, selectedRunbook.ID, options.GitReference, selectedEnvironments) + options.RunTargets, err = askGitRunbookTargets(octopus, asker, space.ID, project.ID, selectedRunbook.ID, options.GitReference, environmentIDs) if err != nil { return err } @@ -1191,11 +1234,11 @@ func resolveRunbookPreviewVariables( return result, sensitiveVars, nil } -func askRunbookTargets(octopus *octopusApiClient.Client, asker question.Asker, spaceID string, runbookSnapshotID string, selectedEnvironments []*environments.Environment) ([]string, error) { +func askRunbookTargets(octopus *octopusApiClient.Client, asker question.Asker, spaceID string, runbookSnapshotID string, environmentIDs []string) ([]string, error) { var results []string - for _, env := range selectedEnvironments { - preview, err := runbooks.GetRunbookSnapshotRunPreview(octopus, spaceID, runbookSnapshotID, env.ID, true) + for _, environmentID := range environmentIDs { + preview, err := runbooks.GetRunbookSnapshotRunPreview(octopus, spaceID, runbookSnapshotID, environmentID, true) if err != nil { return nil, err } @@ -1228,11 +1271,11 @@ func askRunbookTargets(octopus *octopusApiClient.Client, asker question.Asker, s return nil, nil } -func askGitRunbookTargets(octopus *octopusApiClient.Client, asker question.Asker, spaceID string, projectID string, runbookID string, gitRef string, selectedEnvironments []*environments.Environment) ([]string, error) { +func askGitRunbookTargets(octopus *octopusApiClient.Client, asker question.Asker, spaceID string, projectID string, runbookID string, gitRef string, environmentIDs []string) ([]string, error) { var results []string - for _, env := range selectedEnvironments { - preview, err := runbooks.GetGitRunbookRunPreview(octopus, spaceID, projectID, runbookID, gitRef, env.ID, true) + for _, environmentID := range environmentIDs { + preview, err := runbooks.GetGitRunbookRunPreview(octopus, spaceID, projectID, runbookID, gitRef, environmentID, true) if err != nil { return nil, err } diff --git a/pkg/cmd/runbook/run/run_by_tag.go b/pkg/cmd/runbook/run/run_by_tag.go index d281bff5..88195d85 100644 --- a/pkg/cmd/runbook/run/run_by_tag.go +++ b/pkg/cmd/runbook/run/run_by_tag.go @@ -16,6 +16,7 @@ import ( "github.com/OctopusDeploy/cli/pkg/factory" "github.com/OctopusDeploy/cli/pkg/output" "github.com/OctopusDeploy/cli/pkg/question" + "github.com/OctopusDeploy/cli/pkg/question/selectors" "github.com/OctopusDeploy/cli/pkg/util" "github.com/OctopusDeploy/cli/pkg/util/flag" "github.com/OctopusDeploy/go-octopusdeploy/v2/pkg/core" @@ -166,7 +167,7 @@ func processRunbookTasks(octopus *octopusApiClient.Client, space *spaces.Space, return results } -func runRunbooksByTag(cmd *cobra.Command, f factory.Factory, flags *RunFlags, octopus *octopusApiClient.Client, project *projects.Project, parsedVariables map[string]string, outputFormat string, isGit bool) error { +func runRunbooksByTag(cmd *cobra.Command, f factory.Factory, flags *RunFlags, octopus *octopusApiClient.Client, project *projects.Project, resolvedEnvironments []*selectors.ResolvedEnvironment, parsedVariables map[string]string, outputFormat string, isGit bool) error { var allRunbooks []*runbooks.Runbook var err error @@ -197,6 +198,10 @@ func runRunbooksByTag(cmd *cobra.Command, f factory.Factory, flags *RunFlags, oc cmd.Println() } + // the caller has already resolved any environments given on the command line; keep their IDs so + // the run previews below don't have to resolve the canonical names a second time + environmentIDs := util.SliceTransform(resolvedEnvironments, func(env *selectors.ResolvedEnvironment) string { return env.ID }) + var selectedEnvironments []*environments.Environment if f.IsPromptEnabled() { if len(flags.Environments.Value) == 0 { @@ -209,6 +214,7 @@ func runRunbooksByTag(cmd *cobra.Command, f factory.Factory, flags *RunFlags, oc return err } flags.Environments.Value = util.SliceTransform(selectedEnvironments, func(env *environments.Environment) string { return env.Name }) + environmentIDs = util.SliceTransform(selectedEnvironments, func(env *environments.Environment) string { return env.ID }) } if len(flags.Tenants.Value) == 0 && len(flags.TenantTags.Value) == 0 { @@ -226,27 +232,27 @@ func runRunbooksByTag(cmd *cobra.Command, f factory.Factory, flags *RunFlags, oc // Check if any runbooks have prompted variables - block execution if found if len(parsedVariables) == 0 { + if len(environmentIDs) == 0 { // nothing was pre-resolved, so fall back to a name lookup + envs, err := executionscommon.FindEnvironments(octopus, flags.Environments.Value[:1]) + if err == nil { + environmentIDs = util.SliceTransform(envs, func(env *environments.Environment) string { return env.ID }) + } + } + // one preview per runbook is enough to spot prompted variables, so only the first environment is used + previewEnvironmentID := "" + if len(environmentIDs) > 0 { + previewEnvironmentID = environmentIDs[0] + } + hasPromptedVars := false var runbookWithPrompts string for _, runbook := range matchingRunbooks { var preview *runbooks.RunPreview - if isGit { - // Get preview for first environment to check for prompted variables - if len(flags.Environments.Value) > 0 { - envs, err := executionscommon.FindEnvironments(octopus, flags.Environments.Value[:1]) - if err == nil && len(envs) > 0 { - preview, _ = runbooks.GetGitRunbookRunPreview(octopus, f.GetCurrentSpace().ID, project.ID, runbook.ID, flags.GitRef.Value, envs[0].ID, true) - } - } - } else { - // For DB runbooks, we need the published snapshot - if runbook.PublishedRunbookSnapshotID != "" { - if len(flags.Environments.Value) > 0 { - envs, err := executionscommon.FindEnvironments(octopus, flags.Environments.Value[:1]) - if err == nil && len(envs) > 0 { - preview, _ = runbooks.GetRunbookSnapshotRunPreview(octopus, f.GetCurrentSpace().ID, runbook.PublishedRunbookSnapshotID, envs[0].ID, true) - } - } + if previewEnvironmentID != "" { + if isGit { + preview, _ = runbooks.GetGitRunbookRunPreview(octopus, f.GetCurrentSpace().ID, project.ID, runbook.ID, flags.GitRef.Value, previewEnvironmentID, true) + } else if runbook.PublishedRunbookSnapshotID != "" { // for DB runbooks, we need the published snapshot + preview, _ = runbooks.GetRunbookSnapshotRunPreview(octopus, f.GetCurrentSpace().ID, runbook.PublishedRunbookSnapshotID, previewEnvironmentID, true) } } if preview != nil && len(preview.Form.Elements) > 0 { @@ -321,9 +327,9 @@ func runRunbooksByTag(cmd *cobra.Command, f factory.Factory, flags *RunFlags, oc resolvedFlags := NewRunFlags() resolvedFlags.Project.Value = flags.Project.Value resolvedFlags.RunbookTags.Value = flags.RunbookTags.Value - resolvedFlags.Environments.Value = flags.Environments.Value - resolvedFlags.Tenants.Value = flags.Tenants.Value - resolvedFlags.TenantTags.Value = flags.TenantTags.Value + resolvedFlags.Environments.Value = executionscommon.EscapeCommas(flags.Environments.Value) + resolvedFlags.Tenants.Value = executionscommon.EscapeCommas(flags.Tenants.Value) + resolvedFlags.TenantTags.Value = executionscommon.EscapeCommas(flags.TenantTags.Value) spaceName := "" if s := f.GetCurrentSpace(); s != nil { diff --git a/pkg/cmd/runbook/run/run_test.go b/pkg/cmd/runbook/run/run_test.go index a3d50e83..8bae3e7a 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) @@ -302,6 +362,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) @@ -344,6 +405,55 @@ 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)) + 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) + 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 { @@ -373,6 +483,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 @@ -441,6 +557,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") @@ -459,6 +576,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") @@ -499,6 +617,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{ @@ -528,6 +647,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"}, @@ -559,6 +679,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) @@ -599,6 +724,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) @@ -660,6 +786,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) @@ -726,6 +853,66 @@ func TestGitRunbookRun_AutomationMode(t *testing.T) { } } +// --environment is resolved once, up front, and the resolved identity has to be carried through to +// the run preview. Looking the canonical name up again would go through an ID-first lookup and land +// on whichever environment happens to have that name as its ID. +func TestRunbookRunByTag_UsesTheResolvedEnvironmentForThePreview(t *testing.T) { + const spaceID = "Spaces-1" + const fireProjectID = "Projects-22" + + space1 := fixtures.NewSpace(spaceID, "Default Space") + fireProject := fixtures.NewProject(spaceID, fireProjectID, "Fire Project", "Lifecycles-1", "ProjectGroups-1", "deploymentprocess-"+fireProjectID) + + devEnvironment := fixtures.NewEnvironment(spaceID, "Environments-12", "dev") + testEnvironment := fixtures.NewEnvironment(spaceID, "Environments-13", "test") + // an environment which is *named* like another environment's ID + decoyEnvironment := fixtures.NewEnvironment(spaceID, "Environments-99", "Environments-13") + + nightlyRunbook := fixtures.NewRunbook(spaceID, fireProjectID, "Runbooks-1", "Provision Database") + nightlyRunbook.RunbookTags = []string{"nightly"} + nightlyRunbook.PublishedRunbookSnapshotID = "RunbookSnapshots-1" + + stdout, stderr := &bytes.Buffer{}, &bytes.Buffer{} + api := testutil.NewMockHttpServer() + rootCmd := cmdRoot.NewCmdRoot(testutil.NewMockFactoryWithSpace(api, space1), nil, nil) + rootCmd.SetContext(ctxWithFakeNow) + rootCmd.SetOut(stdout) + rootCmd.SetErr(stderr) + + cmdReceiver := testutil.GoBegin2(func() (*cobra.Command, error) { + defer api.Close() + rootCmd.SetArgs([]string{"runbook", "run", "--project", "Fire Project", "--runbook-tag", "nightly", "--environment", "Environments-99"}) + 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)) + // the one and only environment lookup; the decoy wins because an ID match beats a name match + api.ExpectRequest(t, "GET", "/api/Spaces-1/environments/all").RespondWith([]*environments.Environment{devEnvironment, testEnvironment, decoyEnvironment}) + api.ExpectRequest(t, "GET", "/api/Spaces-1/projects/Projects-22/runbooks?take=2147483647").RespondWith(resources.Resources[*runbooks.Runbook]{ + Items: []*runbooks.Runbook{nightlyRunbook}, + }) + // Environments-99, not Environments-13: the preview must use the environment we actually resolved + api.ExpectRequest(t, "GET", "/api/Spaces-1/runbookSnapshots/RunbookSnapshots-1/runbookRuns/preview/Environments-99?includeDisabledSteps=true"). + RespondWith(&runbooks.RunPreview{Form: deployments.NewFormWithValuesAndElements(map[string]string{}, []*deployments.Element{})}) + + 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) + // the executions API only matches by name, so the decoy's name is what gets submitted + assert.Equal(t, []string{"Environments-13"}, requestBody.EnvironmentNames) + req.RespondWith(&runbooks.RunbookRunResponseV1{ + RunbookRunServerTasks: []*runbooks.RunbookRunServerTask{ + {RunbookRunID: "RunbookRun-203", ServerTaskID: "ServerTasks-29394"}, + }, + }) + + _, err = testutil.ReceivePair(cmdReceiver) + assert.Nil(t, err) + assert.Equal(t, "", stderr.String()) +} + func TestRunbookRun_PrintAdvancedSummary(t *testing.T) { tests := []struct { name string diff --git a/pkg/executionscommon/executionscommon.go b/pkg/executionscommon/executionscommon.go index 91af36fa..e3dc8e58 100644 --- a/pkg/executionscommon/executionscommon.go +++ b/pkg/executionscommon/executionscommon.go @@ -8,8 +8,10 @@ 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" + "github.com/OctopusDeploy/cli/pkg/util/flag" octopusApiClient "github.com/OctopusDeploy/go-octopusdeploy/v2/pkg/client" "github.com/OctopusDeploy/go-octopusdeploy/v2/pkg/deployments" "github.com/OctopusDeploy/go-octopusdeploy/v2/pkg/environments" @@ -301,6 +303,84 @@ 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. +// +// A comma that is part of a value can be escaped with a backslash, so +// `--deployment-target 'Web\, Prod'` yields the single value `Web, Prod`. A backslash in any +// other position is left alone, so target names such as `DOMAIN\host` are unaffected. +// +// Blank entries are rejected rather than silently dropped. A value such as "," or "A,,B" +// almost always means a caller-side variable substitution produced nothing, and quietly +// dropping it would change the scope of the deployment: an empty --tenant list, for example, +// turns a tenanted deployment into an untenanted one rather than failing. +// +// Only apply this to flags whose values cannot legitimately contain an unescaped comma; +// notably NOT to --variable, --skip or the package/git-resource specs. +func ExpandCommaSeparated(flagName string, values []string) ([]string, error) { + if len(values) == 0 { + return values, nil + } + result := make([]string, 0, len(values)) + for _, value := range values { + for _, component := range splitOnUnescapedCommas(value) { + component = strings.TrimSpace(component) + if component == "" { + return nil, fmt.Errorf("--%s has a blank value; check for an empty variable or a stray comma in %q. Use '\\,' to include a comma in a value", flagName, value) + } + result = append(result, component) + } + } + return result, nil +} + +// splitOnUnescapedCommas splits on commas, treating `\,` as an escaped literal comma. +// Any other backslash is preserved verbatim. +func splitOnUnescapedCommas(value string) []string { + var result []string + var current strings.Builder + for i := 0; i < len(value); i++ { + switch { + case value[i] == '\\' && i+1 < len(value) && value[i+1] == ',': + current.WriteByte(',') + i++ + case value[i] == ',': + result = append(result, current.String()) + current.Reset() + default: + current.WriteByte(value[i]) + } + } + return append(result, current.String()) +} + +// ExpandCommaSeparatedFlags applies ExpandCommaSeparated in place to each of the given flags, +// so callers don't have to keep a hand-maintained list of assignments in sync. +func ExpandCommaSeparatedFlags(flags ...*flag.Flag[[]string]) error { + for _, f := range flags { + expanded, err := ExpandCommaSeparated(f.Name, f.Value) + if err != nil { + return err + } + f.Value = expanded + } + return nil +} + +// EscapeCommas escapes any comma within each value so that the result survives a round trip +// back through ExpandCommaSeparated. Used when echoing user selections into the generated +// automation command, which emits values verbatim. +func EscapeCommas(values []string) []string { + if len(values) == 0 { + return values + } + result := make([]string, 0, len(values)) + for _, value := range values { + result = append(result, strings.ReplaceAll(value, ",", "\\,")) + } + return result +} + func ParseVariableStringArray(variables []string) (map[string]string, error) { result := make(map[string]string, len(variables)) for _, v := range variables { @@ -462,40 +542,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/executionscommon/executionscommon_test.go b/pkg/executionscommon/executionscommon_test.go index 72604be2..701cf942 100644 --- a/pkg/executionscommon/executionscommon_test.go +++ b/pkg/executionscommon/executionscommon_test.go @@ -8,6 +8,7 @@ import ( "github.com/AlecAivazis/survey/v2" "github.com/OctopusDeploy/cli/pkg/executionscommon" + "github.com/OctopusDeploy/cli/pkg/util/flag" "github.com/OctopusDeploy/cli/test/fixtures" "github.com/OctopusDeploy/cli/test/testutil" "github.com/OctopusDeploy/go-octopusdeploy/v2/pkg/resources" @@ -412,3 +413,91 @@ 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: "escaped comma is a literal comma", input: []string{`Web\, Prod`}, expect: []string{"Web, Prod"}}, + {name: "escaped and unescaped commas mix", input: []string{`Web\, Prod,Other`}, expect: []string{"Web, Prod", "Other"}}, + {name: "backslash not before a comma is preserved", input: []string{`DOMAIN\host,Other`}, expect: []string{`DOMAIN\host`, "Other"}}, + {name: "trailing backslash is preserved", input: []string{`ABC\`}, expect: []string{`ABC\`}}, + } + + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + result, err := executionscommon.ExpandCommaSeparated("environment", test.input) + assert.NoError(t, err) + assert.Equal(t, test.expect, result) + }) + } +} + +// a blank component almost always means a caller-side variable expanded to nothing; dropping it +// silently would narrow the scope of a deployment (or flip a tenanted deploy to untenanted) +func TestExpandCommaSeparated_RejectsBlankValues(t *testing.T) { + tests := []struct { + name string + input []string + }{ + {name: "empty string", input: []string{""}}, + {name: "lone comma", input: []string{","}}, + {name: "whitespace only", input: []string{" , "}}, + {name: "blank in the middle", input: []string{"ABC,,XYZ"}}, + {name: "trailing comma", input: []string{"ABC,"}}, + {name: "blank alongside a good repeat", input: []string{"ABC", ""}}, + } + + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + result, err := executionscommon.ExpandCommaSeparated("tenant", test.input) + assert.Nil(t, result) + assert.ErrorContains(t, err, "--tenant has a blank value") + }) + } +} + +func TestExpandCommaSeparatedFlags(t *testing.T) { + environments := flag.New[[]string]("environment", false) + environments.Value = []string{"dev,test"} + tenants := flag.New[[]string]("tenant", false) + tenants.Value = []string{"Tenant A", "Tenant B,Tenant C"} + + assert.NoError(t, executionscommon.ExpandCommaSeparatedFlags(environments, tenants)) + assert.Equal(t, []string{"dev", "test"}, environments.Value) + assert.Equal(t, []string{"Tenant A", "Tenant B", "Tenant C"}, tenants.Value) + + bad := flag.New[[]string]("deployment-target", false) + bad.Value = []string{"ABC,"} + err := executionscommon.ExpandCommaSeparatedFlags(environments, bad) + assert.ErrorContains(t, err, "--deployment-target has a blank value") +} + +// values chosen interactively are echoed back as an automation command verbatim, so any comma +// inside them has to be escaped or the replayed command would split it back apart +func TestEscapeCommas_RoundTripsThroughExpand(t *testing.T) { + assert.Nil(t, executionscommon.EscapeCommas(nil)) + + input := []string{"Web, Prod", "Plain", `Already\, Escaped`} + escaped := executionscommon.EscapeCommas(input) + assert.Equal(t, []string{`Web\, Prod`, "Plain", `Already\\, Escaped`}, escaped) + + expanded, err := executionscommon.ExpandCommaSeparated("deployment-target", escaped) + assert.NoError(t, err) + assert.Equal(t, []string{"Web, Prod", "Plain", `Already\, Escaped`}, expanded) +} diff --git a/pkg/packages/packages.go b/pkg/packages/packages.go index 3eff889a..52f38eed 100644 --- a/pkg/packages/packages.go +++ b/pkg/packages/packages.go @@ -180,6 +180,101 @@ 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 +} + +// 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, so it is worth +// replacing rather than reporting. +const ServerNullReferenceMessage = "Object reference not set to an instance of an object" + +// 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") + // this diagnosis is inferred from a failure the server doesn't describe, so it can be wrong. + // Report what the server actually said too, unless that's the null reference message, which + // says nothing the lines above don't already say better. + if e.cause != nil { + if causeText := e.cause.Error(); !strings.Contains(causeText, ServerNullReferenceMessage) { + sb.WriteString(fmt.Sprintf("\nthe server reported: %s", causeText)) + } + } + 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 +634,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) diff --git a/pkg/packages/packages_test.go b/pkg/packages/packages_test.go new file mode 100644 index 00000000..c6e9a813 --- /dev/null +++ b/pkg/packages/packages_test.go @@ -0,0 +1,40 @@ +package packages_test + +import ( + "errors" + "testing" + + "github.com/OctopusDeploy/cli/pkg/packages" + "github.com/OctopusDeploy/go-octopusdeploy/v2/pkg/releases" + "github.com/stretchr/testify/assert" +) + +func TestMissingPackageVersionsError_Error(t *testing.T) { + missing := []releases.ReleaseTemplatePackage{ + {PackageID: "acme.web", ActionName: "Deploy Web", FeedID: "feeds-builtin", FeedName: "Octopus Server (built-in)"}, + } + + t.Run("names the package, the step and the feed", func(t *testing.T) { + err := packages.NewMissingPackageVersionsError(missing, nil) + assert.Contains(t, err.Error(), "no version could be found for the following packages") + assert.Contains(t, err.Error(), "'acme.web' in step 'Deploy Web' (feed 'Octopus Server (built-in)')") + assert.Contains(t, err.Error(), "push the package(s) to the feed") + }) + + // the diagnosis is inferred from a failure the server doesn't describe, so if we guessed wrong + // the user still needs to be able to see what actually went wrong + t.Run("reports what the server said alongside the diagnosis", func(t *testing.T) { + cause := errors.New("There are no viable release plans in any channels") + err := packages.NewMissingPackageVersionsError(missing, cause) + assert.Contains(t, err.Error(), "the server reported: There are no viable release plans in any channels") + assert.ErrorIs(t, err, cause) + }) + + t.Run("omits the null reference message, which explains nothing", func(t *testing.T) { + cause := errors.New("Octopus API error: " + packages.ServerNullReferenceMessage + " []") + err := packages.NewMissingPackageVersionsError(missing, cause) + assert.NotContains(t, err.Error(), packages.ServerNullReferenceMessage) + assert.NotContains(t, err.Error(), "the server reported") + assert.ErrorIs(t, err, cause) // still unwrappable, just not printed + }) +} 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..1419a10d 100644 --- a/pkg/question/selectors/environments.go +++ b/pkg/question/selectors/environments.go @@ -2,10 +2,14 @@ package selectors import ( "fmt" + "strings" + "github.com/OctopusDeploy/cli/pkg/question" + "github.com/OctopusDeploy/cli/pkg/util" "github.com/OctopusDeploy/go-octopusdeploy/v2/pkg/client" "github.com/OctopusDeploy/go-octopusdeploy/v2/pkg/environments" - "strings" + "github.com/OctopusDeploy/go-octopusdeploy/v2/pkg/environments/v2/ephemeralenvironments" + "github.com/OctopusDeploy/go-octopusdeploy/v2/pkg/spaces" ) type GetAllEnvironmentsCallback func() ([]*environments.Environment, error) @@ -34,25 +38,141 @@ 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 + } + 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 + } + lookup := newIdentifierLookup(allEnvs, + func(env *environments.Environment) string { return env.GetID() }, + func(env *environments.Environment) string { return env.GetName() }) + + result := make([]*environments.Environment, 0, len(environmentIdentifiers)) + for _, identifier := range environmentIdentifiers { + env, found := lookup.find(identifier) + 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 +} + +// ResolvedEnvironment is an environment identified by both its ID and its name, so callers that +// need the name (the executions API only matches environments by name) and callers that need the +// ID (run/deployment previews, target selection) can share a single lookup. Resolving twice isn't +// safe: an ID-first lookup of a name that happens to be another environment's ID lands on the +// other environment. +type ResolvedEnvironment struct { + ID string + Name string +} + +// ResolveEnvironmentNames maps environment names or IDs onto canonical environment names, because +// the executions API only matches environments by name. Prefer ResolveEnvironments when the caller +// also needs the environment's ID later on. +func ResolveEnvironmentNames(octopus *client.Client, space *spaces.Space, environmentIdentifiers []string) ([]string, error) { + resolved, err := ResolveEnvironments(octopus, space, environmentIdentifiers) + if err != nil { + return nil, err + } + return util.SliceTransform(resolved, func(env *ResolvedEnvironment) string { return env.Name }), nil +} + +// ResolveEnvironments maps environment names or IDs onto the ID and name of the environment each +// one picks out. +// +// Ephemeral environments aren't part of the regular environment list, so that list is consulted - +// once, lazily - for any identifier the regular list doesn't have. Resolving one identifier at a +// time means a list mixing the two kinds still reports the identifier that actually went missing. +func ResolveEnvironments(octopus *client.Client, space *spaces.Space, environmentIdentifiers []string) ([]*ResolvedEnvironment, error) { + if len(environmentIdentifiers) == 0 { + return nil, nil + } + allEnvs, err := octopus.Environments.GetAll() 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 + regular := newIdentifierLookup(allEnvs, + func(env *environments.Environment) string { return env.GetID() }, + func(env *environments.Environment) string { return env.GetName() }) + + var ephemeral *identifierLookup[*ephemeralenvironments.EphemeralEnvironment] + + resolved := make([]*ResolvedEnvironment, 0, len(environmentIdentifiers)) + for _, identifier := range environmentIdentifiers { + if env, found := regular.find(identifier); found { + resolved = append(resolved, &ResolvedEnvironment{ID: env.GetID(), Name: env.GetName()}) + continue + } + + if ephemeral == nil { + if space == nil { + return nil, fmt.Errorf("cannot find an environment with the ID or name of '%s'", identifier) + } + allEphemeral, ephemeralErr := ephemeralenvironments.GetAll(octopus, space.ID) + if ephemeralErr != nil { + // ephemeral environments are the rarer case, and the endpoint doesn't exist on + // every server; either way the identifier is genuinely not a regular environment + return nil, fmt.Errorf("cannot find an environment with the ID or name of '%s'", identifier) } + lookup := newIdentifierLookup(allEphemeral.Items, + func(env *ephemeralenvironments.EphemeralEnvironment) string { return env.ID }, + func(env *ephemeralenvironments.EphemeralEnvironment) string { return env.Name }) + ephemeral = &lookup } - 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 + + if env, found := ephemeral.find(identifier); found { + resolved = append(resolved, &ResolvedEnvironment{ID: env.ID, Name: env.Name}) + continue + } + return nil, fmt.Errorf("cannot find an environment with the ID or name of '%s'", identifier) + } + return resolved, nil +} + +// identifierLookup indexes items by both ID and name so an identifier can be matched against +// either, with an ID match winning when an item's name collides with another item's ID. +type identifierLookup[T any] struct { + byID map[string]T + byName map[string]T +} + +func newIdentifierLookup[T any](items []T, id func(T) string, name func(T) string) identifierLookup[T] { + lookup := identifierLookup[T]{ + byID: make(map[string]T, len(items)), + byName: make(map[string]T, len(items)), + } + for _, item := range items { + lookup.byID[strings.ToLower(id(item))] = item + lookup.byName[strings.ToLower(name(item))] = item } + return lookup +} - return nil, fmt.Errorf("no environment found with name of %s", environmentName) +func (l identifierLookup[T]) find(identifier string) (T, bool) { + key := strings.ToLower(identifier) + if item, found := l.byID[key]; found { + return item, true + } + item, found := l.byName[key] + return item, found } 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..e813b263 --- /dev/null +++ b/pkg/question/selectors/find_test.go @@ -0,0 +1,306 @@ +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/environments/v2/ephemeralenvironments" + "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 TestResolveEnvironmentNames(t *testing.T) { + findSpace := fixtures.NewSpace(findSpaceID, "Default") + devEnvironment := fixtures.NewEnvironment(findSpaceID, "Environments-12", "dev") + ephemeralEnvironment := fixtures.NewEphemeralEnvironment(findSpaceID, "Environments-123", "Ephemeral Environment", "Environments-12") + + t.Run("resolves a mix of regular and ephemeral environments", func(t *testing.T) { + api := testutil.NewMockHttpServer() + receiver := beginRequest(api, func(octopus *octopusApiClient.Client) ([]string, error) { + return selectors.ResolveEnvironmentNames(octopus, findSpace, []string{"DEV", "Environments-123"}) + }) + + 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}) + api.ExpectRequest(t, "GET", "/api/Spaces-1/environments/v2?skip=0&take=2147483647&type=Ephemeral").RespondWith(resources.Resources[*ephemeralenvironments.EphemeralEnvironment]{ + Items: []*ephemeralenvironments.EphemeralEnvironment{ephemeralEnvironment}, + }) + + result, err := testutil.ReceivePair(receiver) + assert.Nil(t, err) + assert.Equal(t, []string{"dev", "Ephemeral Environment"}, result) + }) + + t.Run("doesn't look at ephemeral environments when everything resolves", func(t *testing.T) { + api := testutil.NewMockHttpServer() + receiver := beginRequest(api, func(octopus *octopusApiClient.Client) ([]string, error) { + return selectors.ResolveEnvironmentNames(octopus, findSpace, []string{"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, []string{"dev"}, result) + }) + + t.Run("names the environment that is actually missing", func(t *testing.T) { + api := testutil.NewMockHttpServer() + receiver := beginRequest(api, func(octopus *octopusApiClient.Client) ([]string, error) { + return selectors.ResolveEnvironmentNames(octopus, findSpace, []string{"dev", "Environments-404"}) + }) + + 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}) + api.ExpectRequest(t, "GET", "/api/Spaces-1/environments/v2?skip=0&take=2147483647&type=Ephemeral").RespondWith(resources.Resources[*ephemeralenvironments.EphemeralEnvironment]{ + Items: []*ephemeralenvironments.EphemeralEnvironment{ephemeralEnvironment}, + }) + + _, err := testutil.ReceivePair(receiver) + assert.EqualError(t, err, "cannot find an environment with the ID or name of 'Environments-404'") + }) +} + +func TestResolveEnvironments(t *testing.T) { + findSpace := fixtures.NewSpace(findSpaceID, "Default") + prodEnvironment := fixtures.NewEnvironment(findSpaceID, "Environments-13", "production") + // an environment which is *named* like another environment's ID + decoyEnvironment := fixtures.NewEnvironment(findSpaceID, "Environments-99", "Environments-13") + + // resolving the returned name a second time would hit the ID index and land on prodEnvironment, + // which is why callers need to keep the ID alongside the name + api := testutil.NewMockHttpServer() + receiver := beginRequest(api, func(octopus *octopusApiClient.Client) ([]*selectors.ResolvedEnvironment, error) { + return selectors.ResolveEnvironments(octopus, findSpace, []string{"Environments-99"}) + }) + + 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{prodEnvironment, decoyEnvironment}) + + result, err := testutil.ReceivePair(receiver) + assert.Nil(t, err) + assert.Equal(t, []*selectors.ResolvedEnvironment{{ID: "Environments-99", Name: "Environments-13"}}, result) +} + +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("finds an exact name match beyond the first page of the partial name search", func(t *testing.T) { + // `partialName` is a contains filter, so a tenant exactly named "Smith" can be pushed off + // the first page by every other tenant whose name also contains "Smith" + aaronSmith := fixtures.NewTenant(findSpaceID, "Tenants-30", "Aaron Smith") + smith := fixtures.NewTenant(findSpaceID, "Tenants-31", "Smith") + + api := testutil.NewMockHttpServer() + receiver := beginRequest(api, func(octopus *octopusApiClient.Client) ([]*tenants.Tenant, error) { + return selectors.FindTenants(octopus, []string{"Smith"}) + }) + + api.ExpectRequest(t, "GET", "/api/").RespondWith(findRootResource) + api.ExpectRequest(t, "GET", "/api/spaces").RespondWith(findRootResource) + api.ExpectRequest(t, "GET", "/api/Spaces-1/tenants/Smith").RespondWithStatus(404, "NotFound", nil) + api.ExpectRequest(t, "GET", "/api/Spaces-1/tenants?partialName=Smith").RespondWith(resources.Resources[*tenants.Tenant]{ + Items: []*tenants.Tenant{aaronSmith}, + PagedResults: resources.PagedResults{ + Links: resources.Links{PageNext: "/api/Spaces-1/tenants?partialName=Smith&skip=1"}, + }, + }) + api.ExpectRequest(t, "GET", "/api/Spaces-1/tenants?partialName=Smith&skip=1").RespondWith(resources.Resources[*tenants.Tenant]{ + Items: []*tenants.Tenant{smith}, + }) + + result, err := testutil.ReceivePair(receiver) + assert.Nil(t, err) + assert.Equal(t, []string{smith.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/releases.go b/pkg/question/selectors/releases.go new file mode 100644 index 00000000..56e8026e --- /dev/null +++ b/pkg/question/selectors/releases.go @@ -0,0 +1,70 @@ +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" + +// ReleaseNotFoundError reports a release version that the server didn't return a release for. +// +// Confirmed distinguishes the two ways that answer arrives. A 404 carrying an APIError body is a +// definite "no such release". An empty response body is not: the SDK's DoRawJsonRequest short-circuits +// on `resp.ContentLength == 0` and returns (resp, nil) for *any* status code, so a 403 with no body, or +// a 502 from a proxy, decodes into a zero-valued Release with a nil error and is indistinguishable from +// a 404 by the time it reaches us. The status code isn't recoverable at this layer, so the message +// hedges rather than asserting the release is missing. +type ReleaseNotFoundError struct { + ProjectName string + ReleaseVersion string + Confirmed bool +} + +func (e *ReleaseNotFoundError) Error() string { + var message string + if e.Confirmed { + message = fmt.Sprintf("cannot find a release with version '%s' in project '%s'", e.ReleaseVersion, e.ProjectName) + } else { + message = fmt.Sprintf("could not resolve a release with version '%s' in project '%s'; the server returned an empty response, which usually means there is no such release, but can also mean the lookup itself failed", e.ReleaseVersion, e.ProjectName) + } + + if strings.EqualFold(e.ReleaseVersion, latestReleaseAlias) { + message += fmt.Sprintf(". '%s' is not a supported alias, specify an exact version. Run '%s release list --project \"%s\"' to see the available versions", + e.ReleaseVersion, constants.ExecutableName, e.ProjectName) + } + + return message +} + +// 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. +// Anything else (a permissions failure, a transport error) is returned untouched, so callers that +// would rather let the server be the authority can tell the two apart with errors.As. +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{ProjectName: project.GetName(), ReleaseVersion: releaseVersion, Confirmed: true} + } + return nil, err + } + // an empty response body doesn't reach the error path above; it decodes as an empty release. + // See ReleaseNotFoundError for why this can't be reported as a definite "not found". + if release == nil || release.GetID() == "" { + return nil, &ReleaseNotFoundError{ProjectName: project.GetName(), ReleaseVersion: releaseVersion} + } + + return release, nil +} diff --git a/pkg/question/selectors/tenants.go b/pkg/question/selectors/tenants.go new file mode 100644 index 00000000..ffb0af34 --- /dev/null +++ b/pkg/question/selectors/tenants.go @@ -0,0 +1,69 @@ +package selectors + +import ( + "errors" + "fmt" + "strings" + + octopusApiClient "github.com/OctopusDeploy/go-octopusdeploy/v2/pkg/client" + "github.com/OctopusDeploy/go-octopusdeploy/v2/pkg/core" + "github.com/OctopusDeploy/go-octopusdeploy/v2/pkg/tenants" +) + +// FindTenant looks a tenant up by either its ID or its name. An ID match wins over a name +// match, so it stays consistent with how projects, environments and channels resolve. +// +// Deliberately not Tenants.GetByIdentifier: its name fallback issues a single `partialName` +// (i.e. contains) query and only scans the first page of the result, so an exact name that +// sorts past that page is reported as not found. Names are on the deploy hot path and used +// to be resolved server side, so a miss here is a regression rather than an inconvenience. +func FindTenant(octopus *octopusApiClient.Client, tenantIdentifier string) (*tenants.Tenant, error) { + if tenantIdentifier == "" { + return nil, errors.New("cannot find a tenant without an ID or name") + } + + tenant, err := octopus.Tenants.GetByID(tenantIdentifier) + if err != nil { + var apiError *core.APIError + if errors.As(err, &apiError) && apiError.StatusCode != 404 { + return nil, err + } + // a 404 (or an identifier that doesn't look like an ID at all) just means "try the name" + } else if tenant != nil { + return tenant, nil + } + + resultPage, err := octopus.Tenants.Get(tenants.TenantsQuery{PartialName: tenantIdentifier}) + if err != nil { + return nil, err + } + for resultPage != nil && len(resultPage.Items) > 0 { + for _, t := range resultPage.Items { // the server has no exact-name search, so we emulate one + if strings.EqualFold(t.Name, tenantIdentifier) { + return t, nil + } + } + resultPage, err = resultPage.GetNextPage(octopus.Tenants.GetClient()) + if err != nil { + return nil, err + } // if there are no more pages, GetNextPage returns nil, which breaks us out of the loop + } + + return nil, fmt.Errorf("cannot find a tenant with the ID or name of '%s'", tenantIdentifier) +} + +// 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 +} diff --git a/test/integration/release_test.go b/test/integration/release_test.go index b6f476f3..7c0bbf19 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,317 @@ 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 +} + +// createSecondaryChannel adds a non-default channel to the project. It carries no lifecycle of its +// own, so it inherits the project's, and no version rules, so any release version is valid in it. +// It exists separately from the fixture's default channel so that a test asserting on the channel a +// release landed in can tell a channel the CLI asked for from the one the server would have picked. +func createSecondaryChannel(t *testing.T, apiClient *octopusApiClient.Client, project *projects.Project, name string) *channels.Channel { + channel, err := apiClient.Channels.Add(channels.NewChannel(name, project.GetID())) + if !testutil.AssertSuccess(t, err) { + return nil + } + t.Cleanup(func() { assert.Nil(t, apiClient.Channels.DeleteByID(channel.GetID())) }) + require.False(t, channel.IsDefault, "the project's default channel must stay the one the fixture created") + return channel +} + +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 + } + // the release has to land in a channel the server wouldn't have chosen by itself, otherwise the + // assertion below passes just as well when --channel is dropped on the floor + secondaryChannel := createSecondaryChannel(t, apiClient, project, fmt.Sprintf("channel-%s", runId)) + if secondaryChannel == nil { + 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", secondaryChannel.GetID(), "--version", "1.0.0") + if !testutil.AssertSuccess(t, err, stdErr) { + return + } + release := onlyReleaseInProject(t, apiClient, project) + if release == nil { + return + } + assert.Equal(t, secondaryChannel.GetID(), release.ChannelID) + assert.NotEqual(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) + }) +}