From d57433388c3754b086b14f3a5c329a9e8aa8507e Mon Sep 17 00:00:00 2001 From: Nick Josevski Date: Wed, 19 Aug 2026 11:50:51 +1000 Subject: [PATCH] feat: add --dry-run to release create and release delete Declares --dry-run per command rather than persistently, so a command that hasn't implemented it rejects the flag instead of silently ignoring it. A client-level guard refuses any non-read-only request once a dry run is under way, so a half-implemented dry run fails loudly. Refs #63 Co-Authored-By: Claude Opus 5 (1M context) --- pkg/apiclient/client_factory.go | 27 +++ pkg/apiclient/client_factory_test.go | 32 ++++ pkg/cmd/release/create/create.go | 247 ++++++++++++++++++++++++-- pkg/cmd/release/create/create_test.go | 152 ++++++++++++++++ pkg/cmd/release/delete/delete.go | 51 ++++-- pkg/cmd/release/delete/delete_test.go | 54 ++++++ pkg/cmd/root/root.go | 10 +- pkg/constants/constants.go | 1 + pkg/dryrun/dryrun.go | 95 ++++++++++ pkg/dryrun/dryrun_test.go | 101 +++++++++++ pkg/packages/packages.go | 28 +-- 11 files changed, 760 insertions(+), 38 deletions(-) create mode 100644 pkg/dryrun/dryrun.go create mode 100644 pkg/dryrun/dryrun_test.go diff --git a/pkg/apiclient/client_factory.go b/pkg/apiclient/client_factory.go index 93c73abe..12c10ea7 100644 --- a/pkg/apiclient/client_factory.go +++ b/pkg/apiclient/client_factory.go @@ -9,6 +9,7 @@ import ( "github.com/MakeNowJust/heredoc/v2" "github.com/OctopusDeploy/cli/pkg/constants" + "github.com/OctopusDeploy/cli/pkg/dryrun" "github.com/OctopusDeploy/cli/pkg/output" "github.com/OctopusDeploy/cli/pkg/question" "github.com/OctopusDeploy/go-octopusdeploy/v2/pkg/spaces" @@ -45,6 +46,12 @@ type ClientFactory interface { // GetHttpClient returns a raw http client which can be used to query Octopus GetHttpClient() (*http.Client, error) + + // SetDryRun puts the client into dry-run mode, where any request that would change + // server state is refused before it is sent. It backstops the per-command --dry-run + // implementations; a command which hasn't finished implementing dry run fails loudly + // rather than mutating Octopus while claiming it did not. + SetDryRun(enabled bool) } type Client struct { @@ -73,6 +80,9 @@ type Client struct { ActiveSpace *spaces.Space Ask question.AskProvider + + // true once the dry-run guard has been installed on HttpClient + dryRun bool } func NewClientFactory(httpClient *http.Client, host string, credentials octopusApiClient.ICredential, spaceNameOrID string, ask question.AskProvider) (ClientFactory, error) { @@ -257,6 +267,21 @@ func (c *Client) GetHttpClient() (*http.Client, error) { return c.HttpClient, nil } +// SetDryRun wraps the transport in the dry-run guard. It must be called before the +// space-scoped or system clients are created, which is why the root command arms it +// from PersistentPreRun; both clients are built lazily during RunE. +func (c *Client) SetDryRun(enabled bool) { + if !enabled || c.dryRun { + return + } + c.dryRun = true + + if c.HttpClient == nil { + c.HttpClient = &http.Client{} + } + c.HttpClient.Transport = dryrun.NewGuardRoundTripper(c.HttpClient.Transport) +} + func (c *Client) SetSpaceNameOrId(spaceNameOrId string) { // technically don't need to nil out the SystemClient, but it's cleaner that way // because a SpaceScopedClient can also be a SystemClient @@ -408,3 +433,5 @@ func (s *stubClientFactory) GetHostUrl() string { return "" } func (s *stubClientFactory) GetHttpClient() (*http.Client, error) { return nil, nil } + +func (s *stubClientFactory) SetDryRun(_ bool) {} diff --git a/pkg/apiclient/client_factory_test.go b/pkg/apiclient/client_factory_test.go index 4cbc542e..892d35c5 100644 --- a/pkg/apiclient/client_factory_test.go +++ b/pkg/apiclient/client_factory_test.go @@ -1,6 +1,9 @@ package apiclient_test import ( + "bytes" + "io" + "net/http" "testing" "github.com/OctopusDeploy/cli/pkg/apiclient" @@ -67,3 +70,32 @@ func TestNewClientFactory_WhenHostAndAccessTokenAreSupplied_ReturnsClientFactory testutil.RequireSuccess(t, err) assert.NotNil(t, factory) } + +type recordingRoundTripper struct { + Requests []*http.Request +} + +func (r *recordingRoundTripper) RoundTrip(req *http.Request) (*http.Response, error) { + r.Requests = append(r.Requests, req) + return &http.Response{StatusCode: http.StatusOK, Body: io.NopCloser(bytes.NewReader(nil))}, nil +} + +func TestClientFactory_SetDryRun_RefusesMutatingRequests(t *testing.T) { + transport := &recordingRoundTripper{} + apiKeyCredential, _ := client.NewApiKey(apiKey) + clientFactory, err := apiclient.NewClientFactory(&http.Client{Transport: transport}, hostUrl, apiKeyCredential, "", qa) + testutil.RequireSuccess(t, err) + + clientFactory.SetDryRun(true) + + httpClient, err := clientFactory.GetHttpClient() + testutil.RequireSuccess(t, err) + + _, err = httpClient.Post(hostUrl+"/api/Spaces-1/releases/create/v1", "application/json", nil) + assert.ErrorContains(t, err, "dry run blocked a POST request to /api/Spaces-1/releases/create/v1") + assert.Empty(t, transport.Requests, "a mutating request must not reach the server") + + _, err = httpClient.Get(hostUrl + "/api/Spaces-1/projects/all") + assert.Nil(t, err) + assert.Len(t, transport.Requests, 1, "read-only requests still go through") +} diff --git a/pkg/cmd/release/create/create.go b/pkg/cmd/release/create/create.go index b4b50caa..8b84757d 100644 --- a/pkg/cmd/release/create/create.go +++ b/pkg/cmd/release/create/create.go @@ -15,6 +15,7 @@ import ( "github.com/MakeNowJust/heredoc/v2" "github.com/OctopusDeploy/cli/pkg/cmd/release/list" "github.com/OctopusDeploy/cli/pkg/constants" + "github.com/OctopusDeploy/cli/pkg/dryrun" cliErrors "github.com/OctopusDeploy/cli/pkg/errors" "github.com/OctopusDeploy/cli/pkg/executor" "github.com/OctopusDeploy/cli/pkg/factory" @@ -32,6 +33,7 @@ import ( "github.com/OctopusDeploy/go-octopusdeploy/v2/pkg/feeds" "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/spf13/cobra" ) @@ -127,6 +129,7 @@ type CreateFlags struct { PackageVersionSpec *flag.Flag[[]string] GitResourceRefsSpec *flag.Flag[[]string] CustomFields *flag.Flag[[]string] + DryRun *flag.Flag[bool] } func NewCreateFlags() *CreateFlags { @@ -144,6 +147,7 @@ func NewCreateFlags() *CreateFlags { PackageVersionSpec: flag.New[[]string](FlagPackageVersionSpec, false), GitResourceRefsSpec: flag.New[[]string](FlagGitResourceRefSpec, false), CustomFields: flag.New[[]string](FlagCustomField, false), + DryRun: flag.New[bool](constants.FlagDryRun, false), } } @@ -160,6 +164,7 @@ func NewCmdCreate(f factory.Factory) *cobra.Command { %[1]s release create -p MyProject -c default --package "utils:1.2.3" --package "utils:InstallOnly:5.6.7" %[1]s release create -p MyProject --package "com.example\:my-artifact:1.0" %[1]s release create -p MyProject -c Beta --no-prompt + %[1]s release create -p MyProject -c Beta --dry-run `, constants.ExecutableName), RunE: func(cmd *cobra.Command, args []string) error { return createRun(cmd, f, createFlags) }, } @@ -179,6 +184,7 @@ func NewCmdCreate(f factory.Factory) *cobra.Command { flags.StringArrayVarP(&createFlags.PackageVersionSpec.Value, createFlags.PackageVersionSpec.Name, "", []string{}, "Version specification for a specific package. You may specify this multiple times.\nFormat as {package}:{version}, {step}:{version} or {package-ref-name}:{packageOrStep}:{version}\nIf the package ID or step name contains a colon, slash, or equals sign (such as Maven coordinates like com.example:my-artifact), escape that character with a backslash:\n --package \"com.example\\:my-artifact:1.0\"\nThis escape syntax requires Octopus CLI 2.21.2 or later and Octopus Server 2025.4.10680 or later.") flags.StringArrayVarP(&createFlags.GitResourceRefsSpec.Value, createFlags.GitResourceRefsSpec.Name, "", []string{}, "Git reference for a specific Git resource.\nFormat as {step}:{git-ref}, {step}:{git-resource-name}:{git-ref}\nYou may specify this multiple times") flags.StringArrayVarP(&createFlags.CustomFields.Value, createFlags.CustomFields.Name, "", []string{}, "Custom field value to set on the release.\nFormat as {name}:{value}. You may specify multiple times") + dryrun.AddFlag(flags, &createFlags.DryRun.Value) // we want the help text to display in the above order, rather than alphabetical flags.SortFlags = false @@ -258,6 +264,9 @@ func createRun(cmd *cobra.Command, f factory.Factory, flags *CreateFlags) error return err } + // only populated in automation mode; the interactive Q&A resolves everything as it goes + var resolvedProject *projects.Project + if f.IsPromptEnabled() { err = AskQuestions(octopus, cmd.OutOrStdout(), f.Ask, options) if err != nil { @@ -310,9 +319,18 @@ func createRun(cmd *cobra.Command, f factory.Factory, flags *CreateFlags) error return err } options.ProjectName = project.GetName() + resolvedProject = project } } + if flags.DryRun.Value { + preview, err := buildReleasePreview(octopus, f.GetCurrentSpace(), options, resolvedProject) + if err != nil { + return err + } + return printReleasePreview(cmd, preview, outputFormat) + } + // the executor will raise errors if any required options are missing err = executor.ProcessTasks(octopus, f.GetCurrentSpace(), []*executor.Task{ executor.NewTask(executor.TaskTypeCreateRelease, options), @@ -382,6 +400,220 @@ func createRun(cmd *cobra.Command, f factory.Factory, flags *CreateFlags) error return nil } +// resolveVersioningStrategy loads the project's versioning strategy. Config-as-code projects +// don't inline it in the project resource, so it has to come from the deployment settings. +func resolveVersioningStrategy(octopus *octopusApiClient.Client, project *projects.Project, gitReferenceKey string) (*projects.VersioningStrategy, error) { + if project.VersioningStrategy != nil { + return project.VersioningStrategy, nil + } + + deploymentSettings, err := octopus.Deployments.GetDeploymentSettings(project, gitReferenceKey) + if err != nil { + return nil, err + } + if deploymentSettings.VersioningStrategy == nil { // not sure if this should ever happen, but best to be defensive + return nil, cliErrors.NewInvalidResponseError(fmt.Sprintf("cannot determine versioning strategy for project %s", project.Name)) + } + return deploymentSettings.VersioningStrategy, nil +} + +// ReleasePreview is the release that a dry run would have created, resolved as far as the +// CLI can resolve it. An empty Channel or Version means the Octopus Server decides it. +type ReleasePreview struct { + // always true; it marks machine-readable output as a plan rather than a result + DryRun bool + Space string + Project string + Channel string + GitReference string `json:",omitempty"` + GitCommit string `json:",omitempty"` + Version string + ReleaseNotes string `json:",omitempty"` + PackageVersions []*packages.StepPackageVersion `json:",omitempty"` + PackageOverrides []string `json:",omitempty"` + GitResources []string `json:",omitempty"` + CustomFields map[string]string `json:",omitempty"` + IgnoreExisting bool + IgnoreChannelRules bool +} + +// buildReleasePreview describes the release that would be created, without creating it. +// In interactive mode the Q&A has already resolved everything, so resolvedProject is nil +// and the options are the answer. In automation mode only the project has been resolved, +// so we go back to the server (read-only) for the channel, packages and version. +func buildReleasePreview(octopus *octopusApiClient.Client, space *spaces.Space, options *executor.TaskOptionsCreateRelease, resolvedProject *projects.Project) (*ReleasePreview, error) { + if options.ProjectName == "" { + return nil, errors.New("project must be specified") + } + + preview := &ReleasePreview{ + DryRun: true, + Project: options.ProjectName, + Channel: options.ChannelName, + GitReference: options.GitReference, + GitCommit: options.GitCommit, + Version: options.Version, + ReleaseNotes: options.ReleaseNotes, + PackageOverrides: options.PackageVersionOverrides, + GitResources: options.GitResourceRefs, + CustomFields: options.CustomFields, + IgnoreExisting: options.IgnoreIfAlreadyExists, + IgnoreChannelRules: options.IgnoreChannelRules, + } + if space != nil { + preview.Space = space.GetName() + } + + if resolvedProject == nil { + return preview, nil + } + + // without an explicit channel the server picks one by applying the channel rules, and + // both the package versions and the release version follow from that choice; guessing + // which channel it would pick risks showing a plan that doesn't match what happens + if options.ChannelName == "" { + return preview, nil + } + + channel, err := selectors.FindChannel(octopus, resolvedProject, options.ChannelName) + if err != nil { + return nil, err + } + preview.Channel = channel.Name + + gitReferenceKey := "" + if resolvedProject.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(resolvedProject, gitReferenceKey) + if err != nil { + return nil, err + } + + deploymentProcessTemplate, err := octopus.DeploymentProcesses.GetTemplate(deploymentProcess, channel.ID, "") + if err != nil { + return nil, err + } + + baseline, err := BuildPackageVersionBaselineForChannel(octopus, deploymentProcessTemplate, channel) + if err != nil { + return nil, err + } + overrides := packages.BuildPackageVersionOverrides(baseline, options.DefaultPackageVersion, options.PackageVersionOverrides) + preview.PackageVersions = packages.ApplyPackageOverrides(baseline, overrides) + + if preview.Version == "" { + preview.Version, err = determineReleaseVersion(octopus, resolvedProject, gitReferenceKey, deploymentProcessTemplate, preview.PackageVersions) + if err != nil { + return nil, err + } + } + + return preview, nil +} + +// determineReleaseVersion works out the version the server would assign, following the same +// rules as the interactive prompt but without asking anything. An empty string means the +// version can't be determined up front. +func determineReleaseVersion(octopus *octopusApiClient.Client, project *projects.Project, gitReferenceKey string, deploymentProcessTemplate *deployments.DeploymentProcessTemplate, packageVersions []*packages.StepPackageVersion) (string, error) { + versioningStrategy, err := resolveVersioningStrategy(octopus, project, gitReferenceKey) + if err != nil { + return "", err + } + + if donor := versioningStrategy.DonorPackage; donor != nil { + for _, pkg := range packageVersions { + if pkg.PackageReferenceName == donor.PackageReference && pkg.ActionName == donor.DeploymentAction { + return pkg.Version, nil + } + } + return "", nil + } + if versioningStrategy.DonorPackageStepID != nil { // a donor step with no package reference; nothing to read a version from + return "", nil + } + + if versioningStrategy.Template != "" { + return deploymentProcessTemplate.NextVersionIncrement, nil + } + + return "", nil +} + +func printReleasePreview(cmd *cobra.Command, preview *ReleasePreview, outputFormat string) error { + if outputFormat == constants.OutputFormatJson { + data, err := json.Marshal(preview) + if err != nil { + return err + } + _, _ = cmd.OutOrStdout().Write(data) + cmd.Println() + return nil + } + + dryrun.Header(cmd) + cmd.Printf("Would create a release with:\n") + + byServer := output.Dim("(determined by the Octopus Server)") + rows := []*output.DataRow{ + output.NewDataRow("Space", preview.Space), + output.NewDataRow("Project", preview.Project), + output.NewDataRow("Channel", orDefault(preview.Channel, byServer)), + output.NewDataRow("Version", orDefault(preview.Version, byServer)), + } + if preview.GitReference != "" { + rows = append(rows, output.NewDataRow("Git Reference", preview.GitReference)) + } + if preview.GitCommit != "" { + rows = append(rows, output.NewDataRow("Git Commit", preview.GitCommit)) + } + rows = append(rows, output.NewDataRow("Release Notes", orDefault(preview.ReleaseNotes, output.Dim("(none)")))) + for _, ref := range preview.GitResources { + rows = append(rows, output.NewDataRow("Git Resource", ref)) + } + for name, value := range preview.CustomFields { + rows = append(rows, output.NewDataRow("Custom Field", fmt.Sprintf("%s: %s", name, value))) + } + if preview.IgnoreExisting { + rows = append(rows, output.NewDataRow("Ignore Existing", "true")) + } + if preview.IgnoreChannelRules { + rows = append(rows, output.NewDataRow("Ignore Channel Rules", "true")) + } + output.PrintRows(rows, cmd.OutOrStdout()) + + if len(preview.PackageVersions) > 0 { + cmd.Printf("\nPackages:\n") + t := output.NewTable(cmd.OutOrStdout()) + t.AddRow(output.Bold("PACKAGE"), output.Bold("VERSION"), output.Bold("STEP NAME/PACKAGE REFERENCE")) + for _, pkg := range preview.PackageVersions { + t.AddRow(pkg.PackageID, orDefault(pkg.Version, output.Yellow("unknown")), fmt.Sprintf("%s/%s", pkg.ActionName, pkg.PackageReferenceName)) + } + if err := t.Print(); err != nil { + return err + } + } else if len(preview.PackageOverrides) > 0 { + cmd.Printf("\nPackage overrides:\n") + for _, ov := range preview.PackageOverrides { + cmd.Printf(" %s\n", ov) + } + } + + dryrun.Footer(cmd, "no release was created.") + return nil +} + +func orDefault(value string, fallback string) string { + if value == "" { + return fallback + } + return value +} + // BuildPackageVersionBaselineForChannel loads the deployment process template from the server, and for each step+package therein, // finds the latest available version satisfying the channel version rules. Result is the list of step+package+versions // to use as a baseline. The package version override process takes this as an input and layers on top of it @@ -567,18 +799,9 @@ func AskQuestions(octopus *octopusApiClient.Client, stdout io.Writer, asker ques // - but we must allow the user to override package versions first. // If the project's VersioningStrategy is null, it means this is a Config-as-code project and we need to // additionally load the deployment settings because the API doesn't inline the strategy in the main project resource for some reason - var versioningStrategy *projects.VersioningStrategy - if selectedProject.VersioningStrategy != nil { - versioningStrategy = selectedProject.VersioningStrategy - } else { - deploymentSettings, err := octopus.Deployments.GetDeploymentSettings(selectedProject, gitReferenceKey) - if err != nil { - return err - } - versioningStrategy = deploymentSettings.VersioningStrategy - } - if versioningStrategy == nil { // not sure if this should ever happen, but best to be defensive - return cliErrors.NewInvalidResponseError(fmt.Sprintf("cannot determine versioning strategy for project %s", selectedProject.Name)) + versioningStrategy, err := resolveVersioningStrategy(octopus, selectedProject, gitReferenceKey) + if err != nil { + return err } if versioningStrategy.DonorPackageStepID != nil || versioningStrategy.DonorPackage != nil { diff --git a/pkg/cmd/release/create/create_test.go b/pkg/cmd/release/create/create_test.go index 65ee97c7..6681e5ad 100644 --- a/pkg/cmd/release/create/create_test.go +++ b/pkg/cmd/release/create/create_test.go @@ -2829,3 +2829,155 @@ func TestReleaseCreate_ApplyPackageOverride(t *testing.T) { }, result) }) } + +func TestReleaseCreate_DryRun(t *testing.T) { + const spaceID = "Spaces-1" + const fireProjectID = "Projects-22" + + 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", "Fire Project Default Channel", fireProjectID) + + tests := []struct { + name string + run func(t *testing.T, api *testutil.MockHttpServer, rootCmd *cobra.Command, stdOut *bytes.Buffer, stdErr *bytes.Buffer) + }{ + {"dry run without a channel says what the server would decide, and creates nothing", 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, "--dry-run"}) + 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) + + // note the absence of a POST to /releases/create/v1; an unexpected request + // would leave the mock server with nothing to respond to it + _, err := testutil.ReceivePair(cmdReceiver) + assert.Nil(t, err) + assert.Equal(t, 0, api.GetPendingMessageCount()) + + assert.Equal(t, heredoc.Doc(` + DRY RUN: no changes will be made in Octopus. + + Would create a release with: + Space Default Space + Project Fire Project + Channel (determined by the Octopus Server) + Version (determined by the Octopus Server) + Release Notes (none) + + DRY RUN: no release was created. + `), stdOut.String()) + assert.Equal(t, "", stdErr.String()) + }}, + + {"dry run with a channel resolves the version and package versions, and creates nothing", 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, + "--channel", defaultChannel.Name, + "--package", "pterm:9.9", + "--release-notes", "Some notes", + "--dry-run", + }) + 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, "GET", "/api/Spaces-1/deploymentprocesses/deploymentprocess-"+fireProjectID).RespondWith(depProcess) + + api.ExpectRequest(t, "GET", "/api/Spaces-1/projects/"+fireProjectID+"/deploymentprocesses/template?channel=Channels-1"). + RespondWith(&deployments.DeploymentProcessTemplate{ + Packages: []releases.ReleaseTemplatePackage{ + { + ActionName: "Install", + FeedID: "feeds-builtin", + PackageID: "pterm", + PackageReferenceName: "pterm-on-install", + IsResolvable: true, + }, + }, + NextVersionIncrement: "27.9.33", + }) + + api.ExpectRequest(t, "GET", "/api/Spaces-1/feeds?ids=feeds-builtin&take=1").RespondWith(&feeds.Feeds{Items: []feeds.IFeed{ + &feeds.FeedResource{Name: "Builtin", FeedType: feeds.FeedTypeBuiltIn, Resource: resources.Resource{ + ID: "feeds-builtin", + 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=pterm&take=1").RespondWith(&resources.Resources[*octopusPackages.PackageVersion]{ + Items: []*octopusPackages.PackageVersion{{PackageID: "pterm", Version: "0.12.51"}}, + }) + + _, err := testutil.ReceivePair(cmdReceiver) + assert.Nil(t, err) + assert.Equal(t, 0, api.GetPendingMessageCount()) + + assert.Equal(t, heredoc.Doc(` + DRY RUN: no changes will be made in Octopus. + + Would create a release with: + Space Default Space + Project Fire Project + Channel Fire Project Default Channel + Version 27.9.33 + Release Notes Some notes + + Packages: + PACKAGE VERSION STEP NAME/PACKAGE REFERENCE + pterm 9.9 Install/pterm-on-install + + DRY RUN: no release was created. + `), stdOut.String()) + assert.Equal(t, "", stdErr.String()) + }}, + + {"dry run with json output emits a machine readable plan flagged as a dry run", 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, "--dry-run", "--output-format", "json"}) + 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) + + _, err := testutil.ReceivePair(cmdReceiver) + assert.Nil(t, err) + assert.Equal(t, 0, api.GetPendingMessageCount()) + + assert.Equal(t, `{"DryRun":true,"Space":"Default Space","Project":"Fire Project","Channel":"","Version":"","IgnoreExisting":false,"IgnoreChannelRules":false}`+"\n", stdOut.String()) + assert.Equal(t, "", stdErr.String()) + }}, + } + + 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/delete/delete.go b/pkg/cmd/release/delete/delete.go index a387a270..7aa5e118 100644 --- a/pkg/cmd/release/delete/delete.go +++ b/pkg/cmd/release/delete/delete.go @@ -9,6 +9,7 @@ import ( "github.com/AlecAivazis/survey/v2" "github.com/MakeNowJust/heredoc/v2" "github.com/OctopusDeploy/cli/pkg/constants" + "github.com/OctopusDeploy/cli/pkg/dryrun" "github.com/OctopusDeploy/cli/pkg/factory" "github.com/OctopusDeploy/cli/pkg/output" "github.com/OctopusDeploy/cli/pkg/question" @@ -30,12 +31,14 @@ const ( type Flags struct { Project *flag.Flag[string] Version *flag.Flag[[]string] + DryRun *flag.Flag[bool] } func NewFlags() *Flags { return &Flags{ Project: flag.New[string](FlagProject, false), Version: flag.New[[]string](FlagVersion, false), + DryRun: flag.New[bool](constants.FlagDryRun, false), } } @@ -49,6 +52,7 @@ func NewCmdDelete(f factory.Factory) *cobra.Command { %[1]s release delete myProject 2.0 %[1]s release delete --project myProject --version 2.0 %[1]s release rm "Other Project" -v 2.0 + %[1]s release delete --project myProject --version 2.0 --dry-run `, constants.ExecutableName), Aliases: []string{"del", "rm"}, RunE: func(cmd *cobra.Command, args []string) error { @@ -59,6 +63,7 @@ func NewCmdDelete(f factory.Factory) *cobra.Command { flags := cmd.Flags() flags.StringVarP(&cmdFlags.Project.Value, cmdFlags.Project.Name, "p", "", "Name or ID of the project to delete releases in") flags.StringArrayVarP(&cmdFlags.Version.Value, cmdFlags.Version.Name, "v", make([]string, 0), "Release version to delete, can be specified multiple times") + dryrun.AddFlag(flags, &cmdFlags.DryRun.Value) return cmd } @@ -126,23 +131,25 @@ func deleteRun(cmd *cobra.Command, f factory.Factory, flags *Flags, args []strin return nil // no work to do, just exit } - // prompt for confirmation - cmd.Printf("You are about to delete the following releases:\n") - for _, r := range releasesToDelete { - cmd.Printf("%s\n", r.Version) - } + // a dry run never deletes anything, so there is nothing to confirm; the plan + // printed below says what would have happened instead + if !flags.DryRun.Value { + cmd.Printf("You are about to delete the following releases:\n") + for _, r := range releasesToDelete { + cmd.Printf("%s\n", r.Version) + } - var isConfirmed bool - if err = f.Ask(&survey.Confirm{ - Message: fmt.Sprintf("Confirm delete of %d release(s)", len(releasesToDelete)), - Default: false, - }, &isConfirmed); err != nil { - return err - } - if !isConfirmed { - return nil // nothing to be done here + var isConfirmed bool + if err = f.Ask(&survey.Confirm{ + Message: fmt.Sprintf("Confirm delete of %d release(s)", len(releasesToDelete)), + Default: false, + }, &isConfirmed); err != nil { + return err + } + if !isConfirmed { + return nil // nothing to be done here + } } - } else { // we don't have the executions API backing us and allowing NameOrID; we need to do the lookups ourselves releasesToDelete, err = findReleases(octopus, selectedProject, versionsToDelete) if err != nil { @@ -155,6 +162,11 @@ func deleteRun(cmd *cobra.Command, f factory.Factory, flags *Flags, args []strin return nil } + if flags.DryRun.Value { + printDeletePlan(cmd, selectedProject, releasesToDelete) + return nil + } + var releaseDeleteErrors = &multierror.Error{} for _, r := range releasesToDelete { err = octopus.Releases.DeleteByID(r.ID) @@ -178,6 +190,15 @@ func deleteRun(cmd *cobra.Command, f factory.Factory, flags *Flags, args []strin return releaseDeleteErrors.ErrorOrNil() } +func printDeletePlan(cmd *cobra.Command, project *projects.Project, releasesToDelete []*releases.Release) { + dryrun.Header(cmd) + cmd.Printf("Would delete %d release(s) from project %s:\n", len(releasesToDelete), output.Cyan(project.GetName())) + for _, r := range releasesToDelete { + cmd.Printf(" %s\n", r.Version) + } + dryrun.Footer(cmd, "no releases were deleted.") +} + func selectReleases(octopus *octopusApiClient.Client, project *projects.Project, ask question.Asker) ([]*releases.Release, error) { existingReleases, err := octopus.Projects.GetReleases(project) // gets all of them, no paging if err != nil { diff --git a/pkg/cmd/release/delete/delete_test.go b/pkg/cmd/release/delete/delete_test.go index 3e77fda2..6b780b4e 100644 --- a/pkg/cmd/release/delete/delete_test.go +++ b/pkg/cmd/release/delete/delete_test.go @@ -176,6 +176,60 @@ func TestReleaseDelete(t *testing.T) { assert.Equal(t, "", stdErr.String()) }}, + // ----- dry run ------ + + {"noprompt: dry run reports what would be deleted and doesn't delete anything", func(t *testing.T, api *testutil.MockHttpServer, qa *testutil.AskMocker, rootCmd *cobra.Command, stdOut *bytes.Buffer, stdErr *bytes.Buffer) { + cmdReceiver := testutil.GoBegin2(func() (*cobra.Command, error) { + defer api.Close() + rootCmd.SetArgs([]string{"release", "delete", "--project", fireProject.Name, "--version", "2.0", "--version", "2.1", "--no-prompt", "--dry-run"}) + return rootCmd.ExecuteC() + }) + + // note the absence of any DELETE requests; an unexpected request would leave the + // mock server with nothing to respond to it + standardDeleteTestBody(api) + + _, err := testutil.ReceivePair(cmdReceiver) + assert.Nil(t, err) + assert.Equal(t, 0, api.GetPendingMessageCount()) + + assert.Equal(t, heredoc.Doc(` + DRY RUN: no changes will be made in Octopus. + + Would delete 2 release(s) from project Fire Project: + 2.1 + 2.0 + + DRY RUN: no releases were deleted. + `), stdOut.String()) + assert.Equal(t, "", stdErr.String()) + }}, + + {"interactive: dry run doesn't ask for confirmation and doesn't delete anything", func(t *testing.T, api *testutil.MockHttpServer, qa *testutil.AskMocker, rootCmd *cobra.Command, stdOut *bytes.Buffer, stdErr *bytes.Buffer) { + cmdReceiver := testutil.GoBegin2(func() (*cobra.Command, error) { + defer api.Close() + rootCmd.SetArgs([]string{"release", "delete", fireProject.Name, "2.1", "--dry-run"}) + return rootCmd.ExecuteC() + }) + + standardDeleteTestBody(api) + + _, err := testutil.ReceivePair(cmdReceiver) + assert.Nil(t, err) + assert.Equal(t, 0, api.GetPendingMessageCount()) + + assert.Equal(t, heredoc.Doc(` + Project Fire Project + DRY RUN: no changes will be made in Octopus. + + Would delete 1 release(s) from project Fire Project: + 2.1 + + DRY RUN: no releases were deleted. + `), stdOut.String()) + assert.Equal(t, "", stdErr.String()) + }}, + // ----- failure modes ------ {"noprompt: error when deleting 1 release and it fails", func(t *testing.T, api *testutil.MockHttpServer, qa *testutil.AskMocker, rootCmd *cobra.Command, stdOut *bytes.Buffer, stdErr *bytes.Buffer) { diff --git a/pkg/cmd/root/root.go b/pkg/cmd/root/root.go index 05106062..b859e0ef 100644 --- a/pkg/cmd/root/root.go +++ b/pkg/cmd/root/root.go @@ -25,6 +25,7 @@ import ( workerCmd "github.com/OctopusDeploy/cli/pkg/cmd/worker" workerPoolCmd "github.com/OctopusDeploy/cli/pkg/cmd/workerpool" "github.com/OctopusDeploy/cli/pkg/constants" + "github.com/OctopusDeploy/cli/pkg/dryrun" "github.com/OctopusDeploy/cli/pkg/factory" "github.com/OctopusDeploy/cli/pkg/question" "github.com/spf13/cobra" @@ -116,7 +117,7 @@ func NewCmdRoot(f factory.Factory, clientFactory apiclient.ClientFactory, askPro // if we attempt to check the flags before Execute is called, cobra hasn't parsed anything yet, // so we'll get bad values. PersistentPreRun is a convenient callback for setting up our // environment after parsing but before execution. - cmd.PersistentPreRun = func(_ *cobra.Command, _ []string) { + cmd.PersistentPreRun = func(executedCmd *cobra.Command, _ []string) { // map flag alias values for k, v := range flagAliases { for _, aliasName := range v { @@ -138,6 +139,13 @@ func NewCmdRoot(f factory.Factory, clientFactory apiclient.ClientFactory, askPro if spaceNameOrId := viper.GetString(constants.ConfigSpace); spaceNameOrId != "" { clientFactory.SetSpaceNameOrId(spaceNameOrId) } + + // --dry-run is declared by the individual commands that support it, not here. + // Arming the client guard means that if such a command still reaches a mutating + // endpoint, the request is refused rather than quietly going through. + if clientFactory != nil && dryrun.IsEnabled(executedCmd) { + clientFactory.SetDryRun(true) + } } cmd.RunE = func(cmd *cobra.Command, args []string) error { diff --git a/pkg/constants/constants.go b/pkg/constants/constants.go index 39b2ccf0..a2ab4f39 100644 --- a/pkg/constants/constants.go +++ b/pkg/constants/constants.go @@ -12,6 +12,7 @@ const ( FlagOutputFormatLegacy = "outputFormat" FlagNoPrompt = "no-prompt" FlagEnableServiceMessages = "enable-service-messages" + FlagDryRun = "dry-run" ) // flags for storing things in the go context diff --git a/pkg/dryrun/dryrun.go b/pkg/dryrun/dryrun.go new file mode 100644 index 00000000..4552e8f4 --- /dev/null +++ b/pkg/dryrun/dryrun.go @@ -0,0 +1,95 @@ +// Package dryrun provides the shared pieces of the --dry-run flag: declaring it, +// detecting it, the banners a dry run prints, and the guard that keeps it honest. +// +// The flag is declared per command rather than persistently on the root command. +// A persistent flag would be accepted everywhere, including by the commands which +// have not implemented it, and silently mutating Octopus while the caller believes +// the run was a rehearsal is worse than having no flag at all. Declaring it locally +// means `--dry-run` on an unsupported command fails with "unknown flag". +// +// GuardRoundTripper is the second half of that guarantee. Once a dry run is under +// way, any request that would change server state is refused before it is sent, so +// a partially implemented dry run fails loudly instead of quietly mutating. +package dryrun + +import ( + "fmt" + "net/http" + "strings" + + "github.com/OctopusDeploy/cli/pkg/constants" + "github.com/OctopusDeploy/cli/pkg/output" + "github.com/spf13/cobra" + "github.com/spf13/pflag" +) + +const FlagDescription = "Show what would happen, without making any changes in Octopus" + +// AddFlag declares --dry-run on a command that genuinely supports it. +func AddFlag(flags *pflag.FlagSet, value *bool) { + flags.BoolVar(value, constants.FlagDryRun, false, FlagDescription) +} + +// IsEnabled reports whether the command being executed declared --dry-run and it was set. +func IsEnabled(cmd *cobra.Command) bool { + if cmd == nil { + return false + } + enabled, err := cmd.Flags().GetBool(constants.FlagDryRun) + if err != nil { // the command doesn't declare the flag + return false + } + return enabled +} + +// Header opens a dry run's output. +func Header(cmd *cobra.Command) { + cmd.Printf("%s no changes will be made in Octopus.\n\n", output.Yellow("DRY RUN:")) +} + +// Footer closes a dry run's output, restating that nothing happened. +func Footer(cmd *cobra.Command, summary string) { + cmd.Printf("\n%s %s\n", output.Yellow("DRY RUN:"), summary) +} + +// BlockedError is returned when a dry run attempts a request that would change server state. +type BlockedError struct { + Method string + URL string +} + +func (e *BlockedError) Error() string { + return fmt.Sprintf("dry run blocked a %s request to %s; this command does not fully support --dry-run, please raise an issue", e.Method, e.URL) +} + +// GuardRoundTripper refuses to send anything other than a read-only request. +type GuardRoundTripper struct { + Next http.RoundTripper +} + +func NewGuardRoundTripper(next http.RoundTripper) *GuardRoundTripper { + if next == nil { + next = http.DefaultTransport + } + return &GuardRoundTripper{Next: next} +} + +func (g *GuardRoundTripper) RoundTrip(r *http.Request) (*http.Response, error) { + if !isReadOnly(r.Method) { + url := "" + if r.URL != nil { + url = r.URL.Path + } + return nil, &BlockedError{Method: strings.ToUpper(r.Method), URL: url} + } + return g.Next.RoundTrip(r) +} + +func isReadOnly(method string) bool { + switch strings.ToUpper(method) { + case http.MethodGet, http.MethodHead, http.MethodOptions: + return true + default: + return false + } +} diff --git a/pkg/dryrun/dryrun_test.go b/pkg/dryrun/dryrun_test.go new file mode 100644 index 00000000..578d829c --- /dev/null +++ b/pkg/dryrun/dryrun_test.go @@ -0,0 +1,101 @@ +package dryrun_test + +import ( + "bytes" + "io" + "net/http" + "testing" + + cmdRoot "github.com/OctopusDeploy/cli/pkg/cmd/root" + "github.com/OctopusDeploy/cli/pkg/constants" + "github.com/OctopusDeploy/cli/pkg/dryrun" + "github.com/OctopusDeploy/cli/test/fixtures" + "github.com/OctopusDeploy/cli/test/testutil" + "github.com/spf13/cobra" + "github.com/stretchr/testify/assert" +) + +type recordingRoundTripper struct { + Requests []*http.Request +} + +func (r *recordingRoundTripper) RoundTrip(req *http.Request) (*http.Response, error) { + r.Requests = append(r.Requests, req) + return &http.Response{StatusCode: http.StatusOK, Body: io.NopCloser(bytes.NewReader(nil))}, nil +} + +func TestGuardRoundTripper(t *testing.T) { + tests := []struct { + method string + blocked bool + }{ + {http.MethodGet, false}, + {http.MethodHead, false}, + {http.MethodOptions, false}, + {http.MethodPost, true}, + {http.MethodPut, true}, + {http.MethodPatch, true}, + {http.MethodDelete, true}, + } + + for _, test := range tests { + t.Run(test.method, func(t *testing.T) { + next := &recordingRoundTripper{} + guard := dryrun.NewGuardRoundTripper(next) + + request, err := http.NewRequest(test.method, "http://server/api/Spaces-1/releases/Releases-1", nil) + assert.Nil(t, err) + + response, err := guard.RoundTrip(request) + + if test.blocked { + assert.Nil(t, response) + assert.EqualError(t, err, "dry run blocked a "+test.method+" request to /api/Spaces-1/releases/Releases-1; this command does not fully support --dry-run, please raise an issue") + assert.Empty(t, next.Requests, "the request must not reach the server") + } else { + assert.Nil(t, err) + assert.NotNil(t, response) + assert.Len(t, next.Requests, 1) + } + }) + } +} + +func TestIsEnabled(t *testing.T) { + t.Run("false when the command doesn't declare the flag", func(t *testing.T) { + cmd := &cobra.Command{Use: "thing"} + assert.False(t, dryrun.IsEnabled(cmd)) + }) + + t.Run("false when the flag is declared but not set", func(t *testing.T) { + cmd := &cobra.Command{Use: "thing"} + value := false + dryrun.AddFlag(cmd.Flags(), &value) + assert.False(t, dryrun.IsEnabled(cmd)) + }) + + t.Run("true when the flag is set", func(t *testing.T) { + cmd := &cobra.Command{Use: "thing"} + value := false + dryrun.AddFlag(cmd.Flags(), &value) + assert.Nil(t, cmd.Flags().Set(constants.FlagDryRun, "true")) + assert.True(t, dryrun.IsEnabled(cmd)) + assert.True(t, value) + }) +} + +// --dry-run must never be silently accepted by a command that hasn't implemented it, +// or the caller would believe a mutation was skipped when it wasn't. +func TestUnsupportedCommandRejectsDryRun(t *testing.T) { + stdout, stderr := &bytes.Buffer{}, &bytes.Buffer{} + api := testutil.NewMockHttpServer() + space1 := fixtures.NewSpace("Spaces-1", "Default Space") + + rootCmd := cmdRoot.NewCmdRoot(testutil.NewMockFactoryWithSpace(api, space1), nil, nil) + rootCmd.SetOut(stdout) + rootCmd.SetErr(stderr) + rootCmd.SetArgs([]string{"release", "list", "--project", "Fire Project", "--dry-run"}) + + _, err := rootCmd.ExecuteC() + assert.EqualError(t, err, "unknown flag: --dry-run") +} diff --git a/pkg/packages/packages.go b/pkg/packages/packages.go index 3eff889a..36037129 100644 --- a/pkg/packages/packages.go +++ b/pkg/packages/packages.go @@ -533,32 +533,40 @@ func printPackageVersions(ioWriter io.Writer, packages []*StepPackageVersion) er return t.Print() } -func AskPackageOverrideLoop( - packageVersionBaseline []*StepPackageVersion, - defaultPackageVersion string, // the --package-version command line flag - initialPackageOverrideFlags []string, // the --package command line flag (multiple occurrences) - asker question.Asker, - stdout io.Writer) ([]*StepPackageVersion, []*PackageVersionOverride, error) { +// BuildPackageVersionOverrides resolves the package specifications that arrived on the +// command line (--package-version and --package) against a baseline. Anything that can't +// be parsed or resolved is silently ignored. +func BuildPackageVersionOverrides(packageVersionBaseline []*StepPackageVersion, defaultPackageVersion string, packageOverrideFlags []string) []*PackageVersionOverride { 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 { + for _, s := range packageOverrideFlags { ambOverride, err := ParsePackageOverrideString(s) if err != nil { - continue // silently ignore anything that wasn't parseable (should we emit a warning?) + continue } resolvedOverride, err := ResolvePackageOverride(ambOverride, packageVersionBaseline) if err != nil { - continue // silently ignore anything that wasn't parseable (should we emit a warning?) + continue } packageVersionOverrides = append(packageVersionOverrides, resolvedOverride) } + return packageVersionOverrides +} + +func AskPackageOverrideLoop( + packageVersionBaseline []*StepPackageVersion, + defaultPackageVersion string, // the --package-version command line flag + initialPackageOverrideFlags []string, // the --package command line flag (multiple occurrences) + asker question.Asker, + stdout io.Writer) ([]*StepPackageVersion, []*PackageVersionOverride, error) { + packageVersionOverrides := BuildPackageVersionOverrides(packageVersionBaseline, defaultPackageVersion, initialPackageOverrideFlags) + overriddenPackageVersions := ApplyPackageOverrides(packageVersionBaseline, packageVersionOverrides) outerLoop: