From 2c6cf2f890c93d964761296c7bca25b78c293ce6 Mon Sep 17 00:00:00 2001 From: Nick Josevski Date: Wed, 19 Aug 2026 11:43:46 +1000 Subject: [PATCH 1/8] feat: toggle the enabled state of deployment targets Adds `octopus deployment-target enable|disable [ | ]`, which flips `IsDisabled` on the machine and reports when the target is already in the requested state. The target is prompted for when no name or ID is supplied, matching the existing `tenant enable|disable` commands. Also adds a shared `--disabled` flag to every deployment-target create command so a target can be created in a disabled state, and includes it in the generated automation command. Fixes #311 Co-Authored-By: Claude Opus 5 (1M context) --- pkg/cmd/target/azure-web-app/create/create.go | 7 +- pkg/cmd/target/cloud-region/create/create.go | 7 +- pkg/cmd/target/disable/disable.go | 28 +++++ pkg/cmd/target/disable/disable_test.go | 119 ++++++++++++++++++ pkg/cmd/target/enable/enable.go | 28 +++++ pkg/cmd/target/enable/enable_test.go | 119 ++++++++++++++++++ pkg/cmd/target/kubernetes/create/create.go | 6 + .../listening-tentacle/create/create.go | 7 +- pkg/cmd/target/shared/disabledstate.go | 95 ++++++++++++++ pkg/cmd/target/shared/disabledstate_test.go | 73 +++++++++++ pkg/cmd/target/ssh/create/create.go | 7 +- pkg/cmd/target/target.go | 4 + pkg/cmd/target/target_test.go | 47 +++++++ pkg/machinescommon/disabled.go | 22 ++++ test/testutil/fakeoctopusserver.go | 1 + 15 files changed, 566 insertions(+), 4 deletions(-) create mode 100644 pkg/cmd/target/disable/disable.go create mode 100644 pkg/cmd/target/disable/disable_test.go create mode 100644 pkg/cmd/target/enable/enable.go create mode 100644 pkg/cmd/target/enable/enable_test.go create mode 100644 pkg/cmd/target/shared/disabledstate.go create mode 100644 pkg/cmd/target/shared/disabledstate_test.go create mode 100644 pkg/cmd/target/target_test.go create mode 100644 pkg/machinescommon/disabled.go diff --git a/pkg/cmd/target/azure-web-app/create/create.go b/pkg/cmd/target/azure-web-app/create/create.go index e41c0bf2..21d72f8e 100644 --- a/pkg/cmd/target/azure-web-app/create/create.go +++ b/pkg/cmd/target/azure-web-app/create/create.go @@ -46,6 +46,7 @@ type CreateFlags struct { *shared.CreateTargetRoleFlags *shared.CreateTargetTenantFlags *shared.WorkerPoolFlags + *machinescommon.CreateTargetDisabledFlags *machinescommon.WebFlags } @@ -73,6 +74,7 @@ func NewCreateFlags() *CreateFlags { CreateTargetEnvironmentFlags: shared.NewCreateTargetEnvironmentFlags(), CreateTargetTenantFlags: shared.NewCreateTargetTenantFlags(), WorkerPoolFlags: shared.NewWorkerPoolFlags(), + CreateTargetDisabledFlags: machinescommon.NewCreateTargetDisabledFlags(), WebFlags: machinescommon.NewWebFlags(), } } @@ -123,6 +125,7 @@ func NewCmdCreate(f factory.Factory) *cobra.Command { shared.RegisterCreateTargetRoleFlags(cmd, createFlags.CreateTargetRoleFlags) shared.RegisterCreateTargetTenantFlags(cmd, createFlags.CreateTargetTenantFlags) shared.RegisterCreateTargetWorkerPoolFlags(cmd, createFlags.WorkerPoolFlags) + machinescommon.RegisterCreateTargetDisabledFlags(cmd, createFlags.CreateTargetDisabledFlags) machinescommon.RegisterWebFlag(cmd, createFlags.WebFlags) return cmd } @@ -174,6 +177,8 @@ func createRun(opts *CreateOptions) error { return err } + deploymentTarget.IsDisabled = opts.Disabled.Value + createdTarget, err := opts.Client.Machines.Add(deploymentTarget) if err != nil { return err @@ -181,7 +186,7 @@ func createRun(opts *CreateOptions) error { fmt.Fprintf(opts.Out, "Successfully created Azure web app '%s'.\n", deploymentTarget.Name) if !opts.NoPrompt { - autoCmd := flag.GenerateAutomationCmd(opts.CmdPath, opts.GetSpaceNameOrEmpty(), opts.Name, opts.Account, opts.WebApp, opts.ResourceGroup, opts.Slot, opts.Environments, opts.Roles, opts.Tags, opts.TenantedDeploymentMode, opts.Tenants, opts.TenantTags) + autoCmd := flag.GenerateAutomationCmd(opts.CmdPath, opts.GetSpaceNameOrEmpty(), opts.Name, opts.Account, opts.WebApp, opts.ResourceGroup, opts.Slot, opts.Environments, opts.Roles, opts.Tags, opts.TenantedDeploymentMode, opts.Tenants, opts.TenantTags, opts.Disabled) fmt.Fprintf(opts.Out, "\nAutomation Command: %s\n", autoCmd) } diff --git a/pkg/cmd/target/cloud-region/create/create.go b/pkg/cmd/target/cloud-region/create/create.go index ca73e7ef..30b987bb 100644 --- a/pkg/cmd/target/cloud-region/create/create.go +++ b/pkg/cmd/target/cloud-region/create/create.go @@ -28,6 +28,7 @@ type CreateFlags struct { *shared.CreateTargetRoleFlags *shared.WorkerPoolFlags *shared.CreateTargetTenantFlags + *machinescommon.CreateTargetDisabledFlags *machinescommon.WebFlags } @@ -47,6 +48,7 @@ func NewCreateFlags() *CreateFlags { CreateTargetEnvironmentFlags: shared.NewCreateTargetEnvironmentFlags(), CreateTargetRoleFlags: shared.NewCreateTargetRoleFlags(), CreateTargetTenantFlags: shared.NewCreateTargetTenantFlags(), + CreateTargetDisabledFlags: machinescommon.NewCreateTargetDisabledFlags(), WebFlags: machinescommon.NewWebFlags(), } } @@ -84,6 +86,7 @@ func NewCmdCreate(f factory.Factory) *cobra.Command { shared.RegisterCreateTargetRoleFlags(cmd, createFlags.CreateTargetRoleFlags) shared.RegisterCreateTargetWorkerPoolFlags(cmd, createFlags.WorkerPoolFlags) shared.RegisterCreateTargetTenantFlags(cmd, createFlags.CreateTargetTenantFlags) + machinescommon.RegisterCreateTargetDisabledFlags(cmd, createFlags.CreateTargetDisabledFlags) machinescommon.RegisterWebFlag(cmd, createFlags.WebFlags) return cmd @@ -122,13 +125,15 @@ func createRun(opts *CreateOptions) error { return err } + target.IsDisabled = opts.Disabled.Value + createdTarget, err := opts.Client.Machines.Add(target) if err != nil { return err } fmt.Fprintf(opts.Out, "Successfully created cloud region '%s'.\n", target.Name) if !opts.NoPrompt { - autoCmd := flag.GenerateAutomationCmd(opts.CmdPath, opts.GetSpaceNameOrEmpty(), opts.Name, opts.WorkerPool, opts.Environments, opts.Roles, opts.Tags, opts.TenantedDeploymentMode, opts.Tenants, opts.TenantTags) + autoCmd := flag.GenerateAutomationCmd(opts.CmdPath, opts.GetSpaceNameOrEmpty(), opts.Name, opts.WorkerPool, opts.Environments, opts.Roles, opts.Tags, opts.TenantedDeploymentMode, opts.Tenants, opts.TenantTags, opts.Disabled) fmt.Fprintf(opts.Out, "\nAutomation Command: %s\n", autoCmd) } diff --git a/pkg/cmd/target/disable/disable.go b/pkg/cmd/target/disable/disable.go new file mode 100644 index 00000000..6be5ea20 --- /dev/null +++ b/pkg/cmd/target/disable/disable.go @@ -0,0 +1,28 @@ +package disable + +import ( + "github.com/MakeNowJust/heredoc/v2" + "github.com/OctopusDeploy/cli/pkg/cmd" + "github.com/OctopusDeploy/cli/pkg/cmd/target/shared" + "github.com/OctopusDeploy/cli/pkg/constants" + "github.com/OctopusDeploy/cli/pkg/factory" + "github.com/OctopusDeploy/cli/pkg/usage" + "github.com/spf13/cobra" +) + +func NewCmdDisable(f factory.Factory) *cobra.Command { + return &cobra.Command{ + Args: usage.MaximumNArgs(1), + Use: "disable [ | ]", + Short: "Disable a deployment target", + Long: "Disable a deployment target in Octopus Deploy", + Example: heredoc.Docf(` + %[1]s deployment-target disable Machines-100 + %[1]s deployment-target disable 'web-server' + `, constants.ExecutableName), + RunE: func(c *cobra.Command, args []string) error { + opts := shared.NewSetDisabledStateOptions(args, cmd.NewDependencies(f, c)) + return shared.SetDisabledState(opts, true) + }, + } +} diff --git a/pkg/cmd/target/disable/disable_test.go b/pkg/cmd/target/disable/disable_test.go new file mode 100644 index 00000000..b622856b --- /dev/null +++ b/pkg/cmd/target/disable/disable_test.go @@ -0,0 +1,119 @@ +package disable_test + +import ( + "bytes" + "testing" + + "github.com/AlecAivazis/survey/v2" + cmdRoot "github.com/OctopusDeploy/cli/pkg/cmd/root" + "github.com/OctopusDeploy/cli/pkg/question" + "github.com/OctopusDeploy/cli/test/fixtures" + "github.com/OctopusDeploy/cli/test/testutil" + "github.com/OctopusDeploy/go-octopusdeploy/v2/pkg/machines" + "github.com/OctopusDeploy/go-octopusdeploy/v2/pkg/resources" + "github.com/spf13/cobra" + "github.com/stretchr/testify/assert" +) + +var rootResource = testutil.NewRootResource() + +const spaceID = "Spaces-1" + +func newTarget(id string, name string, isDisabled bool) *machines.DeploymentTarget { + target := machines.NewDeploymentTarget(name, machines.NewCloudRegionEndpoint(), []string{"Environments-1"}, []string{"web"}) + target.ID = id + target.SpaceID = spaceID + target.IsDisabled = isDisabled + return target +} + +func TestDeploymentTargetDisable(t *testing.T) { + space1 := fixtures.NewSpace(spaceID, "Default Space") + + tests := []struct { + name string + run func(t *testing.T, api *testutil.MockHttpServer, qa *testutil.AskMocker, rootCmd *cobra.Command, stdOut *bytes.Buffer, stdErr *bytes.Buffer) + }{ + {"disables a target identified on the command line", 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{"deployment-target", "disable", "Machines-100", "--no-prompt"}) + 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/machines/Machines-100").RespondWith(newTarget("Machines-100", "web-server", false)) + + updateRequest := api.ExpectRequest(t, "PUT", "/api/Spaces-1/machines/Machines-100") + updated, err := testutil.ReadJson[machines.DeploymentTarget](updateRequest.Request.Body) + assert.Nil(t, err) + assert.True(t, updated.IsDisabled) + updateRequest.RespondWith(newTarget("Machines-100", "web-server", true)) + + _, err = testutil.ReceivePair(cmdReceiver) + assert.Nil(t, err) + assert.Contains(t, stdOut.String(), "Successfully disabled deployment target 'web-server'") + assert.Equal(t, "", stdErr.String()) + }}, + + {"does not update a target which is already disabled", 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{"deployment-target", "disable", "Machines-100", "--no-prompt"}) + 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/machines/Machines-100").RespondWith(newTarget("Machines-100", "web-server", true)) + + _, err := testutil.ReceivePair(cmdReceiver) + assert.Nil(t, err) + assert.Contains(t, stdOut.String(), "is already disabled") + assert.Equal(t, "", stdErr.String()) + }}, + + {"prompts for the target when none was supplied", 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{"deployment-target", "disable"}) + 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/machines?take=2147483647"). + RespondWith(resources.Resources[*machines.DeploymentTarget]{Items: []*machines.DeploymentTarget{ + newTarget("Machines-100", "web-server", false), + newTarget("Machines-200", "db-server", false), + }}) + + _ = qa.ExpectQuestion(t, &survey.Select{ + Message: "Select the deployment target you wish to disable:", + Options: []string{"web-server", "db-server"}, + }).AnswerWith("db-server") + + api.ExpectRequest(t, "GET", "/api/Spaces-1/machines/Machines-200").RespondWith(newTarget("Machines-200", "db-server", false)) + api.ExpectRequest(t, "PUT", "/api/Spaces-1/machines/Machines-200").RespondWith(newTarget("Machines-200", "db-server", true)) + + _, err := testutil.ReceivePair(cmdReceiver) + assert.Nil(t, err) + assert.Contains(t, stdOut.String(), "Successfully disabled deployment target 'db-server'") + assert.Equal(t, "", stdErr.String()) + }}, + } + + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + stdout, stderr := &bytes.Buffer{}, &bytes.Buffer{} + api, qa := testutil.NewMockServerAndAsker() + askProvider := question.NewAskProvider(qa.AsAsker()) + fac := testutil.NewMockFactoryWithSpaceAndPrompt(api, space1, askProvider) + rootCmd := cmdRoot.NewCmdRoot(fac, nil, askProvider) + rootCmd.SetOut(stdout) + rootCmd.SetErr(stderr) + test.run(t, api, qa, rootCmd, stdout, stderr) + }) + } +} diff --git a/pkg/cmd/target/enable/enable.go b/pkg/cmd/target/enable/enable.go new file mode 100644 index 00000000..cd850ae3 --- /dev/null +++ b/pkg/cmd/target/enable/enable.go @@ -0,0 +1,28 @@ +package enable + +import ( + "github.com/MakeNowJust/heredoc/v2" + "github.com/OctopusDeploy/cli/pkg/cmd" + "github.com/OctopusDeploy/cli/pkg/cmd/target/shared" + "github.com/OctopusDeploy/cli/pkg/constants" + "github.com/OctopusDeploy/cli/pkg/factory" + "github.com/OctopusDeploy/cli/pkg/usage" + "github.com/spf13/cobra" +) + +func NewCmdEnable(f factory.Factory) *cobra.Command { + return &cobra.Command{ + Args: usage.MaximumNArgs(1), + Use: "enable [ | ]", + Short: "Enable a deployment target", + Long: "Enable a deployment target in Octopus Deploy", + Example: heredoc.Docf(` + %[1]s deployment-target enable Machines-100 + %[1]s deployment-target enable 'web-server' + `, constants.ExecutableName), + RunE: func(c *cobra.Command, args []string) error { + opts := shared.NewSetDisabledStateOptions(args, cmd.NewDependencies(f, c)) + return shared.SetDisabledState(opts, false) + }, + } +} diff --git a/pkg/cmd/target/enable/enable_test.go b/pkg/cmd/target/enable/enable_test.go new file mode 100644 index 00000000..91dfcbad --- /dev/null +++ b/pkg/cmd/target/enable/enable_test.go @@ -0,0 +1,119 @@ +package enable_test + +import ( + "bytes" + "testing" + + "github.com/AlecAivazis/survey/v2" + cmdRoot "github.com/OctopusDeploy/cli/pkg/cmd/root" + "github.com/OctopusDeploy/cli/pkg/question" + "github.com/OctopusDeploy/cli/test/fixtures" + "github.com/OctopusDeploy/cli/test/testutil" + "github.com/OctopusDeploy/go-octopusdeploy/v2/pkg/machines" + "github.com/OctopusDeploy/go-octopusdeploy/v2/pkg/resources" + "github.com/spf13/cobra" + "github.com/stretchr/testify/assert" +) + +var rootResource = testutil.NewRootResource() + +const spaceID = "Spaces-1" + +func newTarget(id string, name string, isDisabled bool) *machines.DeploymentTarget { + target := machines.NewDeploymentTarget(name, machines.NewCloudRegionEndpoint(), []string{"Environments-1"}, []string{"web"}) + target.ID = id + target.SpaceID = spaceID + target.IsDisabled = isDisabled + return target +} + +func TestDeploymentTargetEnable(t *testing.T) { + space1 := fixtures.NewSpace(spaceID, "Default Space") + + tests := []struct { + name string + run func(t *testing.T, api *testutil.MockHttpServer, qa *testutil.AskMocker, rootCmd *cobra.Command, stdOut *bytes.Buffer, stdErr *bytes.Buffer) + }{ + {"enables a target identified on the command line", 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{"deployment-target", "enable", "Machines-100", "--no-prompt"}) + 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/machines/Machines-100").RespondWith(newTarget("Machines-100", "web-server", true)) + + updateRequest := api.ExpectRequest(t, "PUT", "/api/Spaces-1/machines/Machines-100") + updated, err := testutil.ReadJson[machines.DeploymentTarget](updateRequest.Request.Body) + assert.Nil(t, err) + assert.False(t, updated.IsDisabled) + updateRequest.RespondWith(newTarget("Machines-100", "web-server", false)) + + _, err = testutil.ReceivePair(cmdReceiver) + assert.Nil(t, err) + assert.Contains(t, stdOut.String(), "Successfully enabled deployment target 'web-server'") + assert.Equal(t, "", stdErr.String()) + }}, + + {"does not update a target which is already enabled", 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{"deployment-target", "enable", "Machines-100", "--no-prompt"}) + 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/machines/Machines-100").RespondWith(newTarget("Machines-100", "web-server", false)) + + _, err := testutil.ReceivePair(cmdReceiver) + assert.Nil(t, err) + assert.Contains(t, stdOut.String(), "is already enabled") + assert.Equal(t, "", stdErr.String()) + }}, + + {"prompts for the target when none was supplied", 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{"deployment-target", "enable"}) + 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/machines?take=2147483647"). + RespondWith(resources.Resources[*machines.DeploymentTarget]{Items: []*machines.DeploymentTarget{ + newTarget("Machines-100", "web-server", true), + newTarget("Machines-200", "db-server", true), + }}) + + _ = qa.ExpectQuestion(t, &survey.Select{ + Message: "Select the deployment target you wish to enable:", + Options: []string{"web-server", "db-server"}, + }).AnswerWith("web-server") + + api.ExpectRequest(t, "GET", "/api/Spaces-1/machines/Machines-100").RespondWith(newTarget("Machines-100", "web-server", true)) + api.ExpectRequest(t, "PUT", "/api/Spaces-1/machines/Machines-100").RespondWith(newTarget("Machines-100", "web-server", false)) + + _, err := testutil.ReceivePair(cmdReceiver) + assert.Nil(t, err) + assert.Contains(t, stdOut.String(), "Successfully enabled deployment target 'web-server'") + assert.Equal(t, "", stdErr.String()) + }}, + } + + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + stdout, stderr := &bytes.Buffer{}, &bytes.Buffer{} + api, qa := testutil.NewMockServerAndAsker() + askProvider := question.NewAskProvider(qa.AsAsker()) + fac := testutil.NewMockFactoryWithSpaceAndPrompt(api, space1, askProvider) + rootCmd := cmdRoot.NewCmdRoot(fac, nil, askProvider) + rootCmd.SetOut(stdout) + rootCmd.SetErr(stderr) + test.run(t, api, qa, rootCmd, stdout, stderr) + }) + } +} diff --git a/pkg/cmd/target/kubernetes/create/create.go b/pkg/cmd/target/kubernetes/create/create.go index 08a86416..f4d506b3 100644 --- a/pkg/cmd/target/kubernetes/create/create.go +++ b/pkg/cmd/target/kubernetes/create/create.go @@ -151,6 +151,7 @@ type CreateFlags struct { *machinescommon.CreateTargetMachinePolicyFlags *shared.WorkerPoolFlags *shared.CreateTargetTenantFlags + *machinescommon.CreateTargetDisabledFlags *machinescommon.WebFlags } @@ -216,6 +217,7 @@ func NewCreateFlags() *CreateFlags { CreateTargetRoleFlags: shared.NewCreateTargetRoleFlags(), CreateTargetEnvironmentFlags: shared.NewCreateTargetEnvironmentFlags(), + CreateTargetDisabledFlags: machinescommon.NewCreateTargetDisabledFlags(), WebFlags: machinescommon.NewWebFlags(), WorkerPoolFlags: shared.NewWorkerPoolFlags(), CreateTargetTenantFlags: shared.NewCreateTargetTenantFlags(), @@ -307,6 +309,7 @@ func NewCmdCreate(f factory.Factory) *cobra.Command { shared.RegisterCreateTargetWorkerPoolFlags(cmd, createFlags.WorkerPoolFlags) shared.RegisterCreateTargetTenantFlags(cmd, createFlags.CreateTargetTenantFlags) shared.RegisterCreateTargetRoleFlags(cmd, createFlags.CreateTargetRoleFlags) + machinescommon.RegisterCreateTargetDisabledFlags(cmd, createFlags.CreateTargetDisabledFlags) machinescommon.RegisterWebFlag(cmd, createFlags.WebFlags) return cmd @@ -464,6 +467,8 @@ func (opts *CreateOptions) Commit() error { return err } + deploymentTarget.IsDisabled = opts.Disabled.Value + createdTarget, err := opts.Client.Machines.Add(deploymentTarget) if err != nil { return err @@ -522,6 +527,7 @@ func (opts *CreateOptions) Commit() error { opts.Tenants, opts.TenantTags, opts.WorkerPool, + opts.Disabled, ) fmt.Fprintf(opts.Out, "\nAutomation Command: %s\n", autoCmd) } diff --git a/pkg/cmd/target/listening-tentacle/create/create.go b/pkg/cmd/target/listening-tentacle/create/create.go index 30f35ec9..5fb5f2e1 100644 --- a/pkg/cmd/target/listening-tentacle/create/create.go +++ b/pkg/cmd/target/listening-tentacle/create/create.go @@ -35,6 +35,7 @@ type CreateFlags struct { *shared.CreateTargetRoleFlags *machinescommon.CreateTargetMachinePolicyFlags *shared.CreateTargetTenantFlags + *machinescommon.CreateTargetDisabledFlags *machinescommon.WebFlags } @@ -58,6 +59,7 @@ func NewCreateFlags() *CreateFlags { CreateTargetMachinePolicyFlags: machinescommon.NewCreateTargetMachinePolicyFlags(), CreateTargetEnvironmentFlags: shared.NewCreateTargetEnvironmentFlags(), CreateTargetTenantFlags: shared.NewCreateTargetTenantFlags(), + CreateTargetDisabledFlags: machinescommon.NewCreateTargetDisabledFlags(), WebFlags: machinescommon.NewWebFlags(), } } @@ -99,6 +101,7 @@ func NewCmdCreate(f factory.Factory) *cobra.Command { machinescommon.RegisterCreateTargetProxyFlags(cmd, createFlags.CreateTargetProxyFlags, "Listening Tentacle") machinescommon.RegisterCreateTargetMachinePolicyFlags(cmd, createFlags.CreateTargetMachinePolicyFlags) shared.RegisterCreateTargetTenantFlags(cmd, createFlags.CreateTargetTenantFlags) + machinescommon.RegisterCreateTargetDisabledFlags(cmd, createFlags.CreateTargetDisabledFlags) machinescommon.RegisterWebFlag(cmd, createFlags.WebFlags) return cmd @@ -147,6 +150,8 @@ func createRun(opts *CreateOptions) error { return err } + deploymentTarget.IsDisabled = opts.Disabled.Value + createdTarget, err := opts.Client.Machines.Add(deploymentTarget) if err != nil { return err @@ -154,7 +159,7 @@ func createRun(opts *CreateOptions) error { fmt.Fprintf(opts.Out, "Successfully created listening tenatcle '%s'.\n", deploymentTarget.Name) if !opts.NoPrompt { - autoCmd := flag.GenerateAutomationCmd(opts.CmdPath, opts.GetSpaceNameOrEmpty(), opts.Name, opts.URL, opts.Thumbprint, opts.Environments, opts.Roles, opts.Tags, opts.Proxy, opts.MachinePolicy, opts.TenantedDeploymentMode, opts.Tenants, opts.TenantTags) + autoCmd := flag.GenerateAutomationCmd(opts.CmdPath, opts.GetSpaceNameOrEmpty(), opts.Name, opts.URL, opts.Thumbprint, opts.Environments, opts.Roles, opts.Tags, opts.Proxy, opts.MachinePolicy, opts.TenantedDeploymentMode, opts.Tenants, opts.TenantTags, opts.Disabled) fmt.Fprintf(opts.Out, "\nAutomation Command: %s\n", autoCmd) } diff --git a/pkg/cmd/target/shared/disabledstate.go b/pkg/cmd/target/shared/disabledstate.go new file mode 100644 index 00000000..81028ffa --- /dev/null +++ b/pkg/cmd/target/shared/disabledstate.go @@ -0,0 +1,95 @@ +package shared + +import ( + "errors" + "fmt" + + "github.com/OctopusDeploy/cli/pkg/cmd" + "github.com/OctopusDeploy/cli/pkg/output" + "github.com/OctopusDeploy/cli/pkg/question/selectors" + "github.com/OctopusDeploy/go-octopusdeploy/v2/pkg/machines" +) + +type SetDisabledStateOptions struct { + *cmd.Dependencies + *GetTargetsOptions + IdOrName string +} + +func NewSetDisabledStateOptions(args []string, dependencies *cmd.Dependencies) *SetDisabledStateOptions { + idOrName := "" + if len(args) > 0 { + idOrName = args[0] + } + + return &SetDisabledStateOptions{ + Dependencies: dependencies, + GetTargetsOptions: NewGetTargetsOptionsForAllTargets(dependencies), + IdOrName: idOrName, + } +} + +// SetDisabledState enables or disables a deployment target, prompting for the target when no +// name or ID was supplied. +func SetDisabledState(opts *SetDisabledStateOptions, isDisabled bool) error { + if !opts.NoPrompt { + if err := PromptMissingTarget(opts, isDisabled); err != nil { + return err + } + } + + if opts.IdOrName == "" { + return errors.New("deployment target identifier is required but was not provided") + } + + target, err := opts.Client.Machines.GetByIdentifier(opts.IdOrName) + if err != nil { + return err + } + + state := disabledStateDescription(isDisabled) + if target.IsDisabled == isDisabled { + _, _ = fmt.Fprintf(opts.Out, "Deployment target '%s' %s is already %s.\n", target.Name, output.Dimf("(%s)", target.GetID()), state) + return nil + } + + target.IsDisabled = isDisabled + if _, err = machines.Update(opts.Client, target); err != nil { + return err + } + + _, _ = fmt.Fprintf(opts.Out, "Successfully %s deployment target '%s' %s.\n", state, target.Name, output.Dimf("(%s)", target.GetID())) + return nil +} + +func PromptMissingTarget(opts *SetDisabledStateOptions, isDisabled bool) error { + if opts.IdOrName != "" { + return nil + } + + selectedTarget, err := selectors.Select( + opts.Ask, + fmt.Sprintf("Select the deployment target you wish to %s:", actionDescription(isDisabled)), + opts.GetTargetsCallback, + func(target *machines.DeploymentTarget) string { return target.Name }) + if err != nil { + return err + } + + opts.IdOrName = selectedTarget.GetID() + return nil +} + +func actionDescription(isDisabled bool) string { + if isDisabled { + return "disable" + } + return "enable" +} + +func disabledStateDescription(isDisabled bool) string { + if isDisabled { + return "disabled" + } + return "enabled" +} diff --git a/pkg/cmd/target/shared/disabledstate_test.go b/pkg/cmd/target/shared/disabledstate_test.go new file mode 100644 index 00000000..919c1efc --- /dev/null +++ b/pkg/cmd/target/shared/disabledstate_test.go @@ -0,0 +1,73 @@ +package shared_test + +import ( + "testing" + + "github.com/OctopusDeploy/cli/pkg/cmd" + "github.com/OctopusDeploy/cli/pkg/cmd/target/shared" + "github.com/OctopusDeploy/cli/test/testutil" + "github.com/OctopusDeploy/go-octopusdeploy/v2/pkg/machines" + "github.com/stretchr/testify/assert" +) + +func TestPromptMissingTarget_IdentifierSupplied(t *testing.T) { + pa := []*testutil.PA{} + + asker, checkRemainingPrompts := testutil.NewMockAsker(t, pa) + opts := shared.NewSetDisabledStateOptions([]string{"Machines-1"}, &cmd.Dependencies{Ask: asker}) + + err := shared.PromptMissingTarget(opts, true) + checkRemainingPrompts() + + assert.NoError(t, err) + assert.Equal(t, "Machines-1", opts.IdOrName) +} + +func TestPromptMissingTarget_NoIdentifierSupplied(t *testing.T) { + pa := []*testutil.PA{ + testutil.NewSelectPrompt("Select the deployment target you wish to disable:", "", []string{"web-server", "db-server"}, "db-server"), + } + + asker, checkRemainingPrompts := testutil.NewMockAsker(t, pa) + opts := shared.NewSetDisabledStateOptions([]string{}, &cmd.Dependencies{Ask: asker}) + opts.GetTargetsCallback = func() ([]*machines.DeploymentTarget, error) { + return []*machines.DeploymentTarget{ + newTestTarget("Machines-1", "web-server"), + newTestTarget("Machines-2", "db-server"), + }, nil + } + + err := shared.PromptMissingTarget(opts, true) + checkRemainingPrompts() + + assert.NoError(t, err) + assert.Equal(t, "Machines-2", opts.IdOrName) +} + +func TestPromptMissingTarget_EnableUsesEnableWording(t *testing.T) { + pa := []*testutil.PA{ + testutil.NewSelectPrompt("Select the deployment target you wish to enable:", "", []string{"web-server", "db-server"}, "web-server"), + } + + asker, checkRemainingPrompts := testutil.NewMockAsker(t, pa) + opts := shared.NewSetDisabledStateOptions([]string{}, &cmd.Dependencies{Ask: asker}) + opts.GetTargetsCallback = func() ([]*machines.DeploymentTarget, error) { + return []*machines.DeploymentTarget{ + newTestTarget("Machines-1", "web-server"), + newTestTarget("Machines-2", "db-server"), + }, nil + } + + err := shared.PromptMissingTarget(opts, false) + checkRemainingPrompts() + + assert.NoError(t, err) + assert.Equal(t, "Machines-1", opts.IdOrName) +} + +func newTestTarget(id string, name string) *machines.DeploymentTarget { + target := machines.NewDeploymentTarget(name, machines.NewCloudRegionEndpoint(), []string{"Environments-1"}, []string{"web"}) + target.ID = id + target.SpaceID = "Spaces-1" + return target +} diff --git a/pkg/cmd/target/ssh/create/create.go b/pkg/cmd/target/ssh/create/create.go index 504378fd..c1fe2954 100644 --- a/pkg/cmd/target/ssh/create/create.go +++ b/pkg/cmd/target/ssh/create/create.go @@ -33,6 +33,7 @@ type CreateFlags struct { *shared.CreateTargetRoleFlags *machinescommon.CreateTargetMachinePolicyFlags *shared.CreateTargetTenantFlags + *machinescommon.CreateTargetDisabledFlags *machinescommon.WebFlags *machinescommon.SshCommonFlags } @@ -58,6 +59,7 @@ func NewCreateFlags() *CreateFlags { CreateTargetMachinePolicyFlags: machinescommon.NewCreateTargetMachinePolicyFlags(), CreateTargetEnvironmentFlags: shared.NewCreateTargetEnvironmentFlags(), CreateTargetTenantFlags: shared.NewCreateTargetTenantFlags(), + CreateTargetDisabledFlags: machinescommon.NewCreateTargetDisabledFlags(), WebFlags: machinescommon.NewWebFlags(), } } @@ -100,6 +102,7 @@ func NewCmdCreate(f factory.Factory) *cobra.Command { machinescommon.RegisterCreateTargetProxyFlags(cmd, createFlags.CreateTargetProxyFlags, "SSH target") machinescommon.RegisterCreateTargetMachinePolicyFlags(cmd, createFlags.CreateTargetMachinePolicyFlags) shared.RegisterCreateTargetTenantFlags(cmd, createFlags.CreateTargetTenantFlags) + machinescommon.RegisterCreateTargetDisabledFlags(cmd, createFlags.CreateTargetDisabledFlags) machinescommon.RegisterWebFlag(cmd, createFlags.WebFlags) return cmd @@ -159,6 +162,8 @@ func createRun(opts *CreateOptions) error { return err } + deploymentTarget.IsDisabled = opts.Disabled.Value + createdTarget, err := opts.Client.Machines.Add(deploymentTarget) if err != nil { return err @@ -166,7 +171,7 @@ func createRun(opts *CreateOptions) error { fmt.Fprintf(opts.Out, "Successfully created SSH deployment target '%s'.\n", deploymentTarget.Name) if !opts.NoPrompt { - autoCmd := flag.GenerateAutomationCmd(opts.CmdPath, opts.GetSpaceNameOrEmpty(), opts.Name, opts.HostName, opts.Port, opts.Fingerprint, opts.Runtime, opts.Platform, opts.Environments, opts.Roles, opts.Tags, opts.Account, opts.Proxy, opts.MachinePolicy, opts.TenantedDeploymentMode, opts.Tenants, opts.TenantTags) + autoCmd := flag.GenerateAutomationCmd(opts.CmdPath, opts.GetSpaceNameOrEmpty(), opts.Name, opts.HostName, opts.Port, opts.Fingerprint, opts.Runtime, opts.Platform, opts.Environments, opts.Roles, opts.Tags, opts.Account, opts.Proxy, opts.MachinePolicy, opts.TenantedDeploymentMode, opts.Tenants, opts.TenantTags, opts.Disabled) fmt.Fprintf(opts.Out, "\nAutomation Command: %s\n", autoCmd) } diff --git a/pkg/cmd/target/target.go b/pkg/cmd/target/target.go index aa6129a2..76fbc581 100644 --- a/pkg/cmd/target/target.go +++ b/pkg/cmd/target/target.go @@ -5,6 +5,8 @@ import ( cmdAzureWebApp "github.com/OctopusDeploy/cli/pkg/cmd/target/azure-web-app" cmdCloudRegion "github.com/OctopusDeploy/cli/pkg/cmd/target/cloud-region" cmdDelete "github.com/OctopusDeploy/cli/pkg/cmd/target/delete" + cmdDisable "github.com/OctopusDeploy/cli/pkg/cmd/target/disable" + cmdEnable "github.com/OctopusDeploy/cli/pkg/cmd/target/enable" cmdKubernetes "github.com/OctopusDeploy/cli/pkg/cmd/target/kubernetes" cmdList "github.com/OctopusDeploy/cli/pkg/cmd/target/list" cmdListeningTentacle "github.com/OctopusDeploy/cli/pkg/cmd/target/listening-tentacle" @@ -35,6 +37,8 @@ func NewCmdDeploymentTarget(f factory.Factory) *cobra.Command { cmd.AddCommand(cmdAzureWebApp.NewCmdAzureWebApp(f)) cmd.AddCommand(cmdKubernetes.NewCmdKubernetes(f)) cmd.AddCommand(cmdDelete.NewCmdDelete(f)) + cmd.AddCommand(cmdEnable.NewCmdEnable(f)) + cmd.AddCommand(cmdDisable.NewCmdDisable(f)) cmd.AddCommand(cmdList.NewCmdList(f)) cmd.AddCommand(cmdView.NewCmdView(f)) diff --git a/pkg/cmd/target/target_test.go b/pkg/cmd/target/target_test.go new file mode 100644 index 00000000..58aaab43 --- /dev/null +++ b/pkg/cmd/target/target_test.go @@ -0,0 +1,47 @@ +package target_test + +import ( + "testing" + + "github.com/OctopusDeploy/cli/pkg/cmd/target" + "github.com/OctopusDeploy/cli/pkg/machinescommon" + "github.com/OctopusDeploy/cli/test/testutil" + "github.com/spf13/cobra" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func TestDeploymentTargetHasEnableAndDisableCommands(t *testing.T) { + cmd := target.NewCmdDeploymentTarget(testutil.NewMockFactory(testutil.NewMockHttpServer())) + + assert.NotNil(t, findCommand(cmd, "enable")) + assert.NotNil(t, findCommand(cmd, "disable")) +} + +func TestEveryTargetCreateCommandSupportsDisabled(t *testing.T) { + root := target.NewCmdDeploymentTarget(testutil.NewMockFactory(testutil.NewMockHttpServer())) + + targetTypes := []string{"azure-web-app", "cloud-region", "kubernetes", "listening-tentacle", "ssh"} + for _, targetType := range targetTypes { + t.Run(targetType, func(t *testing.T) { + typeCmd := findCommand(root, targetType) + require.NotNil(t, typeCmd) + + createCmd := findCommand(typeCmd, "create") + require.NotNil(t, createCmd) + + disabled := createCmd.Flags().Lookup(machinescommon.FlagDisabled) + require.NotNil(t, disabled) + assert.Equal(t, "false", disabled.DefValue) + }) + } +} + +func findCommand(parent *cobra.Command, name string) *cobra.Command { + for _, c := range parent.Commands() { + if c.Name() == name { + return c + } + } + return nil +} diff --git a/pkg/machinescommon/disabled.go b/pkg/machinescommon/disabled.go new file mode 100644 index 00000000..e9fc77ae --- /dev/null +++ b/pkg/machinescommon/disabled.go @@ -0,0 +1,22 @@ +package machinescommon + +import ( + "github.com/OctopusDeploy/cli/pkg/util/flag" + "github.com/spf13/cobra" +) + +const FlagDisabled = "disabled" + +type CreateTargetDisabledFlags struct { + Disabled *flag.Flag[bool] +} + +func NewCreateTargetDisabledFlags() *CreateTargetDisabledFlags { + return &CreateTargetDisabledFlags{ + Disabled: flag.New[bool](FlagDisabled, false), + } +} + +func RegisterCreateTargetDisabledFlags(cmd *cobra.Command, disabledFlags *CreateTargetDisabledFlags) { + cmd.Flags().BoolVar(&disabledFlags.Disabled.Value, disabledFlags.Disabled.Name, false, "Create the deployment target in a disabled state.") +} diff --git a/test/testutil/fakeoctopusserver.go b/test/testutil/fakeoctopusserver.go index d417eee3..ac337484 100644 --- a/test/testutil/fakeoctopusserver.go +++ b/test/testutil/fakeoctopusserver.go @@ -227,6 +227,7 @@ func NewRootResource() *octopusApiClient.RootResource { root.Links[constants.LinkAccounts] = "/api/Spaces-1/accounts{/id}{?skip,take,ids,partialName,accountType}" root.Links[constants.LinkPackages] = "/api/Spaces-1/packages{/id}{?nuGetPackageId,filter,latest,skip,take,includeNotes}" root.Links[constants.LinkLifecycles] = "/api/Spaces-1/lifecycles{/id}{?skip,take,ids,partialName}" + root.Links[constants.LinkMachines] = "/api/Spaces-1/machines{/id}{?skip,take,name,ids,partialName,roles,isDisabled,healthStatuses,commStyles,tenantIds,tenantTags,environmentIds,thumbprint,deploymentId,shellNames}" root.Links[constants.LinkProjectGroups] = "/api/Spaces-1/projectgroups{/id}{?skip,take,ids,partialName}" root.Links[constants.LinkUsers] = "/api/users" root.Links[constants.LinkCurrentUser] = "/api/users/me" From f5be1bbea2f5c84beeeab1459c51297198a5c038 Mon Sep 17 00:00:00 2001 From: Nick Josevski Date: Sat, 22 Aug 2026 12:55:11 +1000 Subject: [PATCH 2/8] test: integration tests for deployment target enable and disable Covers what MockHttpServer can't: the server honouring IsDisabled on create, and the read-modify-write PUT leaving the rest of the target's settings intact. Also pins that a worker ID is not accepted. Co-Authored-By: Claude Opus 5 (1M context) --- test/integration/target_test.go | 165 ++++++++++++++++++++++++++++++++ 1 file changed, 165 insertions(+) create mode 100644 test/integration/target_test.go diff --git a/test/integration/target_test.go b/test/integration/target_test.go new file mode 100644 index 00000000..caf494f9 --- /dev/null +++ b/test/integration/target_test.go @@ -0,0 +1,165 @@ +package integration_test + +import ( + "fmt" + "testing" + + "github.com/OctopusDeploy/cli/test/integration" + "github.com/OctopusDeploy/cli/test/testutil" + octopusApiClient "github.com/OctopusDeploy/go-octopusdeploy/v2/pkg/client" + "github.com/OctopusDeploy/go-octopusdeploy/v2/pkg/environments" + "github.com/OctopusDeploy/go-octopusdeploy/v2/pkg/machines" + "github.com/google/uuid" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +// cloud regions are the cheapest target to create for real: no endpoint +// credentials and no connectivity for the server to check +func createCloudRegion(t *testing.T, apiClient *octopusApiClient.Client, envName string, name string, extraArgs ...string) *machines.DeploymentTarget { + args := append([]string{ + "deployment-target", "cloud-region", "create", + "--name", name, "--environment", envName, "--role", "target-tests", + }, extraArgs...) + + stdOut, stdErr, err := integration.RunCli("Default", args...) + if !testutil.AssertSuccess(t, err, stdOut, stdErr) { + return nil + } + + target, err := apiClient.Machines.GetByIdentifier(name) + testutil.RequireSuccess(t, err) + t.Cleanup(func() { assert.Nil(t, apiClient.Machines.DeleteByID(target.GetID())) }) + return target +} + +// settings a toggle must not disturb. HealthStatus, Status and StatusSummary are +// excluded on purpose: the server derives them from IsDisabled. +type targetSettings struct { + Name string + EnvironmentIDs []string + Roles []string + TenantedDeploymentMode string + TenantIDs []string + TenantTags []string + MachinePolicyID string + Thumbprint string + URI string + EndpointType string +} + +func settingsOf(target *machines.DeploymentTarget) targetSettings { + return targetSettings{ + Name: target.Name, + EnvironmentIDs: target.EnvironmentIDs, + Roles: target.Roles, + TenantedDeploymentMode: string(target.TenantedDeploymentMode), + TenantIDs: target.TenantIDs, + TenantTags: target.TenantTags, + MachinePolicyID: target.MachinePolicyID, + Thumbprint: target.Thumbprint, + URI: target.URI, + EndpointType: fmt.Sprintf("%T", target.Endpoint), + } +} + +func TestDeploymentTargetEnableDisable(t *testing.T) { + runId := uuid.New() + apiClient, err := integration.GetApiClient(space1ID) + testutil.RequireSuccess(t, err) + + env, err := apiClient.Environments.Add(environments.NewEnvironment(fmt.Sprintf("tgtenv-%s", runId))) + testutil.RequireSuccess(t, err) + t.Cleanup(func() { assert.Nil(t, apiClient.Environments.DeleteByID(env.GetID())) }) + + t.Run("create --disabled", func(t *testing.T) { + target := createCloudRegion(t, apiClient, env.Name, fmt.Sprintf("tgt-disabled-%s", runId), "--disabled") + require.NotNil(t, target) + assert.True(t, target.IsDisabled) + }) + + t.Run("create without --disabled", func(t *testing.T) { + target := createCloudRegion(t, apiClient, env.Name, fmt.Sprintf("tgt-enabled-%s", runId)) + require.NotNil(t, target) + assert.False(t, target.IsDisabled) + }) + + t.Run("enable and disable change nothing else", func(t *testing.T) { + target := createCloudRegion(t, apiClient, env.Name, fmt.Sprintf("tgt-toggle-%s", runId), "--disabled") + require.NotNil(t, target) + before := settingsOf(target) + + stdOut, stdErr, err := integration.RunCli("Default", "deployment-target", "enable", target.Name) + if !testutil.AssertSuccess(t, err, stdOut, stdErr) { + return + } + assert.Contains(t, stdOut, fmt.Sprintf("Successfully enabled deployment target '%s'", target.Name)) + + enabled, err := apiClient.Machines.GetByIdentifier(target.GetID()) + testutil.RequireSuccess(t, err) + assert.False(t, enabled.IsDisabled) + + // the update is a read-modify-write of the whole target, so the rest of + // its settings have to survive the round trip + assert.Equal(t, before, settingsOf(enabled)) + + // and back again, by ID this time + stdOut, stdErr, err = integration.RunCli("Default", "deployment-target", "disable", target.GetID()) + if !testutil.AssertSuccess(t, err, stdOut, stdErr) { + return + } + assert.Contains(t, stdOut, fmt.Sprintf("Successfully disabled deployment target '%s'", target.Name)) + + disabled, err := apiClient.Machines.GetByIdentifier(target.GetID()) + testutil.RequireSuccess(t, err) + assert.True(t, disabled.IsDisabled) + assert.Equal(t, before, settingsOf(disabled)) + }) + + t.Run("already in the requested state", func(t *testing.T) { + target := createCloudRegion(t, apiClient, env.Name, fmt.Sprintf("tgt-noop-%s", runId)) + require.NotNil(t, target) + + stdOut, stdErr, err := integration.RunCli("Default", "deployment-target", "enable", target.Name) + if !testutil.AssertSuccess(t, err, stdOut, stdErr) { + return + } + assert.Contains(t, stdOut, fmt.Sprintf("Deployment target '%s' (%s) is already enabled.", target.Name, target.GetID())) + + unchanged, err := apiClient.Machines.GetByIdentifier(target.GetID()) + testutil.RequireSuccess(t, err) + assert.False(t, unchanged.IsDisabled) + assert.Equal(t, target.ModifiedOn, unchanged.ModifiedOn, "no update should have been sent") + }) + + t.Run("errors", func(t *testing.T) { + for _, tc := range []struct { + name string + args []string + expected string + }{ + {"unknown name", []string{"enable", "no-such-target"}, "cannot find machine with the name or ID of 'no-such-target'"}, + {"no identifier without prompting", []string{"disable"}, "deployment target identifier is required but was not provided"}, + } { + t.Run(tc.name, func(t *testing.T) { + args := append([]string{"deployment-target"}, tc.args...) + stdOut, stdErr, err := integration.RunCli("Default", args...) + assert.Error(t, err, stdOut) + assert.Contains(t, stdOut+stdErr, tc.expected) + }) + } + + t.Run("a worker is not a deployment target", func(t *testing.T) { + workers, err := apiClient.Workers.Get(machines.WorkersQuery{Take: 1}) + testutil.RequireSuccess(t, err) + if len(workers.Items) == 0 { + t.Skip("no workers in this space") + } + workerID := workers.Items[0].GetID() + + stdOut, stdErr, err := integration.RunCli("Default", "deployment-target", "enable", workerID) + assert.Error(t, err, stdOut) + assert.Contains(t, stdOut+stdErr, fmt.Sprintf("cannot find machine with the name or ID of '%s'", workerID)) + }) + }) +} From ec38540459d1219d118c49a8dd1860635715d184 Mon Sep 17 00:00:00 2001 From: Nick Josevski Date: Mon, 24 Aug 2026 19:14:16 +1000 Subject: [PATCH 3/8] test: cover --disabled on listening tentacles too Second endpoint type, so the assertion is about the server honouring IsDisabled rather than about cloud regions specifically. Co-Authored-By: Claude Opus 5 (1M context) --- test/integration/target_test.go | 29 +++++++++++++++++++++++++++++ 1 file changed, 29 insertions(+) diff --git a/test/integration/target_test.go b/test/integration/target_test.go index caf494f9..0871c573 100644 --- a/test/integration/target_test.go +++ b/test/integration/target_test.go @@ -33,6 +33,28 @@ func createCloudRegion(t *testing.T, apiClient *octopusApiClient.Client, envName return target } +// a second endpoint type, to check the server honours IsDisabled on more than +// just cloud regions. Needs an explicit machine policy under --no-prompt. +func createListeningTentacle(t *testing.T, apiClient *octopusApiClient.Client, envName string, name string, thumbprint string, extraArgs ...string) *machines.DeploymentTarget { + args := append([]string{ + "deployment-target", "listening-tentacle", "create", + "--name", name, "--environment", envName, "--role", "target-tests", + "--machine-policy", "Default Machine Policy", + "--thumbprint", thumbprint, + "--url", fmt.Sprintf("https://%s.invalid:10933", name), + }, extraArgs...) + + stdOut, stdErr, err := integration.RunCli("Default", args...) + if !testutil.AssertSuccess(t, err, stdOut, stdErr) { + return nil + } + + target, err := apiClient.Machines.GetByIdentifier(name) + testutil.RequireSuccess(t, err) + t.Cleanup(func() { assert.Nil(t, apiClient.Machines.DeleteByID(target.GetID())) }) + return target +} + // settings a toggle must not disturb. HealthStatus, Status and StatusSummary are // excluded on purpose: the server derives them from IsDisabled. type targetSettings struct { @@ -84,6 +106,13 @@ func TestDeploymentTargetEnableDisable(t *testing.T) { assert.False(t, target.IsDisabled) }) + t.Run("create --disabled on a listening tentacle", func(t *testing.T) { + target := createListeningTentacle(t, apiClient, env.Name, + fmt.Sprintf("tgt-lt-disabled-%s", runId), "0123456789ABCDEF0123456789ABCDEF01234567", "--disabled") + require.NotNil(t, target) + assert.True(t, target.IsDisabled) + }) + t.Run("enable and disable change nothing else", func(t *testing.T) { target := createCloudRegion(t, apiClient, env.Name, fmt.Sprintf("tgt-toggle-%s", runId), "--disabled") require.NotNil(t, target) From 02ae4228e66f4fdd004b18769b414dd173435ac7 Mon Sep 17 00:00:00 2001 From: Nick Josevski Date: Mon, 31 Aug 2026 12:05:16 +1000 Subject: [PATCH 4/8] fix: always prompt before toggling a deployment target selectors.Select returns the single item without asking when the list has exactly one entry, so in a space with one deployment target a bare `octopus deployment-target disable` mutated that target with no prompt at all. Ask through question.SelectMap instead, which always asks - matching `tenant enable|disable`, which reaches SelectMap via selectors.ByName. Co-Authored-By: Claude Opus 5 (1M context) --- pkg/cmd/target/shared/disabledstate.go | 13 ++++++++++--- pkg/cmd/target/shared/disabledstate_test.go | 20 ++++++++++++++++++++ 2 files changed, 30 insertions(+), 3 deletions(-) diff --git a/pkg/cmd/target/shared/disabledstate.go b/pkg/cmd/target/shared/disabledstate.go index 81028ffa..ef78a090 100644 --- a/pkg/cmd/target/shared/disabledstate.go +++ b/pkg/cmd/target/shared/disabledstate.go @@ -6,7 +6,7 @@ import ( "github.com/OctopusDeploy/cli/pkg/cmd" "github.com/OctopusDeploy/cli/pkg/output" - "github.com/OctopusDeploy/cli/pkg/question/selectors" + "github.com/OctopusDeploy/cli/pkg/question" "github.com/OctopusDeploy/go-octopusdeploy/v2/pkg/machines" ) @@ -67,10 +67,17 @@ func PromptMissingTarget(opts *SetDisabledStateOptions, isDisabled bool) error { return nil } - selectedTarget, err := selectors.Select( + targets, err := opts.GetTargetsCallback() + if err != nil { + return err + } + + // deliberately not selectors.Select: that auto-selects when there is exactly one target, which + // would mutate the target without the user ever being asked. Enable/disable always asks. + selectedTarget, err := question.SelectMap( opts.Ask, fmt.Sprintf("Select the deployment target you wish to %s:", actionDescription(isDisabled)), - opts.GetTargetsCallback, + targets, func(target *machines.DeploymentTarget) string { return target.Name }) if err != nil { return err diff --git a/pkg/cmd/target/shared/disabledstate_test.go b/pkg/cmd/target/shared/disabledstate_test.go index 919c1efc..5f9c26ab 100644 --- a/pkg/cmd/target/shared/disabledstate_test.go +++ b/pkg/cmd/target/shared/disabledstate_test.go @@ -65,6 +65,26 @@ func TestPromptMissingTarget_EnableUsesEnableWording(t *testing.T) { assert.Equal(t, "Machines-1", opts.IdOrName) } +// selectors.Select auto-selects when there is exactly one item; enable/disable must not do that +// because it would mutate the only target in the space without asking. +func TestPromptMissingTarget_AsksEvenWhenThereIsOnlyOneTarget(t *testing.T) { + pa := []*testutil.PA{ + testutil.NewSelectPrompt("Select the deployment target you wish to disable:", "", []string{"web-server"}, "web-server"), + } + + asker, checkRemainingPrompts := testutil.NewMockAsker(t, pa) + opts := shared.NewSetDisabledStateOptions([]string{}, &cmd.Dependencies{Ask: asker}) + opts.GetTargetsCallback = func() ([]*machines.DeploymentTarget, error) { + return []*machines.DeploymentTarget{newTestTarget("Machines-1", "web-server")}, nil + } + + err := shared.PromptMissingTarget(opts, true) + checkRemainingPrompts() + + assert.NoError(t, err) + assert.Equal(t, "Machines-1", opts.IdOrName) +} + func newTestTarget(id string, name string) *machines.DeploymentTarget { target := machines.NewDeploymentTarget(name, machines.NewCloudRegionEndpoint(), []string{"Environments-1"}, []string{"web"}) target.ID = id From 76276724fa59692a658ee2a830f59689173317ae Mon Sep 17 00:00:00 2001 From: Nick Josevski Date: Mon, 31 Aug 2026 12:05:48 +1000 Subject: [PATCH 5/8] perf: reuse the prompted deployment target instead of re-fetching it The prompt already loads every machine in the space and the user picks a whole *machines.DeploymentTarget, but only the ID was kept, so GetByIdentifier immediately fetched the same object again (and a second time when the ID lookup falls through to the name lookup). Carry the selected target on the options and only call GetByIdentifier for an identifier supplied on the command line. Co-Authored-By: Claude Opus 5 (1M context) --- pkg/cmd/target/disable/disable_test.go | 1 - pkg/cmd/target/enable/enable_test.go | 1 - pkg/cmd/target/shared/disabledstate.go | 21 ++++++++++++++------- 3 files changed, 14 insertions(+), 9 deletions(-) diff --git a/pkg/cmd/target/disable/disable_test.go b/pkg/cmd/target/disable/disable_test.go index b622856b..16568c87 100644 --- a/pkg/cmd/target/disable/disable_test.go +++ b/pkg/cmd/target/disable/disable_test.go @@ -94,7 +94,6 @@ func TestDeploymentTargetDisable(t *testing.T) { Options: []string{"web-server", "db-server"}, }).AnswerWith("db-server") - api.ExpectRequest(t, "GET", "/api/Spaces-1/machines/Machines-200").RespondWith(newTarget("Machines-200", "db-server", false)) api.ExpectRequest(t, "PUT", "/api/Spaces-1/machines/Machines-200").RespondWith(newTarget("Machines-200", "db-server", true)) _, err := testutil.ReceivePair(cmdReceiver) diff --git a/pkg/cmd/target/enable/enable_test.go b/pkg/cmd/target/enable/enable_test.go index 91dfcbad..ca5bc078 100644 --- a/pkg/cmd/target/enable/enable_test.go +++ b/pkg/cmd/target/enable/enable_test.go @@ -94,7 +94,6 @@ func TestDeploymentTargetEnable(t *testing.T) { Options: []string{"web-server", "db-server"}, }).AnswerWith("web-server") - api.ExpectRequest(t, "GET", "/api/Spaces-1/machines/Machines-100").RespondWith(newTarget("Machines-100", "web-server", true)) api.ExpectRequest(t, "PUT", "/api/Spaces-1/machines/Machines-100").RespondWith(newTarget("Machines-100", "web-server", false)) _, err := testutil.ReceivePair(cmdReceiver) diff --git a/pkg/cmd/target/shared/disabledstate.go b/pkg/cmd/target/shared/disabledstate.go index ef78a090..b5f93eec 100644 --- a/pkg/cmd/target/shared/disabledstate.go +++ b/pkg/cmd/target/shared/disabledstate.go @@ -14,6 +14,9 @@ type SetDisabledStateOptions struct { *cmd.Dependencies *GetTargetsOptions IdOrName string + // Target is the deployment target chosen at the prompt. When set it is used directly, saving a + // round trip back to the server for something we already have. + Target *machines.DeploymentTarget } func NewSetDisabledStateOptions(args []string, dependencies *cmd.Dependencies) *SetDisabledStateOptions { @@ -38,13 +41,16 @@ func SetDisabledState(opts *SetDisabledStateOptions, isDisabled bool) error { } } - if opts.IdOrName == "" { - return errors.New("deployment target identifier is required but was not provided") - } + target := opts.Target + if target == nil { + if opts.IdOrName == "" { + return errors.New("deployment target identifier is required but was not provided") + } - target, err := opts.Client.Machines.GetByIdentifier(opts.IdOrName) - if err != nil { - return err + var err error + if target, err = opts.Client.Machines.GetByIdentifier(opts.IdOrName); err != nil { + return err + } } state := disabledStateDescription(isDisabled) @@ -54,7 +60,7 @@ func SetDisabledState(opts *SetDisabledStateOptions, isDisabled bool) error { } target.IsDisabled = isDisabled - if _, err = machines.Update(opts.Client, target); err != nil { + if _, err := machines.Update(opts.Client, target); err != nil { return err } @@ -83,6 +89,7 @@ func PromptMissingTarget(opts *SetDisabledStateOptions, isDisabled bool) error { return err } + opts.Target = selectedTarget opts.IdOrName = selectedTarget.GetID() return nil } From 62be6123517c8e4669682b806211b49ef4a508f2 Mon Sep 17 00:00:00 2001 From: Nick Josevski Date: Mon, 31 Aug 2026 12:07:44 +1000 Subject: [PATCH 6/8] fix: only offer targets that aren't already in the requested state `disable` used to list already-disabled targets and `enable` already-enabled ones, so picking one ended in the "is already disabled/enabled" no-op after the whole interactive flow. Enable asks the machines endpoint for isDisabled=true (the query field is omitempty, so only the true case can be expressed) and both paths filter client-side, which also covers servers that ignore the query parameter. When nothing is eligible the command now says so instead of prompting. The desired state moves onto SetDisabledStateOptions so the target query can be built with it. Co-Authored-By: Claude Opus 5 (1M context) --- pkg/cmd/target/disable/disable.go | 4 +- pkg/cmd/target/disable/disable_test.go | 28 ++++++++++++++ pkg/cmd/target/enable/enable.go | 4 +- pkg/cmd/target/enable/enable_test.go | 2 +- pkg/cmd/target/shared/disabledstate.go | 41 +++++++++++++++----- pkg/cmd/target/shared/disabledstate_test.go | 42 ++++++++++++++------- 6 files changed, 92 insertions(+), 29 deletions(-) diff --git a/pkg/cmd/target/disable/disable.go b/pkg/cmd/target/disable/disable.go index 6be5ea20..bcccc737 100644 --- a/pkg/cmd/target/disable/disable.go +++ b/pkg/cmd/target/disable/disable.go @@ -21,8 +21,8 @@ func NewCmdDisable(f factory.Factory) *cobra.Command { %[1]s deployment-target disable 'web-server' `, constants.ExecutableName), RunE: func(c *cobra.Command, args []string) error { - opts := shared.NewSetDisabledStateOptions(args, cmd.NewDependencies(f, c)) - return shared.SetDisabledState(opts, true) + opts := shared.NewSetDisabledStateOptions(args, cmd.NewDependencies(f, c), true) + return shared.SetDisabledState(opts) }, } } diff --git a/pkg/cmd/target/disable/disable_test.go b/pkg/cmd/target/disable/disable_test.go index 16568c87..315409dc 100644 --- a/pkg/cmd/target/disable/disable_test.go +++ b/pkg/cmd/target/disable/disable_test.go @@ -74,6 +74,34 @@ func TestDeploymentTargetDisable(t *testing.T) { assert.Equal(t, "", stdErr.String()) }}, + {"does not offer targets which are already disabled", 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{"deployment-target", "disable"}) + 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/machines?take=2147483647"). + RespondWith(resources.Resources[*machines.DeploymentTarget]{Items: []*machines.DeploymentTarget{ + newTarget("Machines-100", "web-server", false), + newTarget("Machines-200", "db-server", true), + }}) + + _ = qa.ExpectQuestion(t, &survey.Select{ + Message: "Select the deployment target you wish to disable:", + Options: []string{"web-server"}, + }).AnswerWith("web-server") + + api.ExpectRequest(t, "PUT", "/api/Spaces-1/machines/Machines-100").RespondWith(newTarget("Machines-100", "web-server", true)) + + _, err := testutil.ReceivePair(cmdReceiver) + assert.Nil(t, err) + assert.Contains(t, stdOut.String(), "Successfully disabled deployment target 'web-server'") + assert.Equal(t, "", stdErr.String()) + }}, + {"prompts for the target when none was supplied", 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() diff --git a/pkg/cmd/target/enable/enable.go b/pkg/cmd/target/enable/enable.go index cd850ae3..afa42f9e 100644 --- a/pkg/cmd/target/enable/enable.go +++ b/pkg/cmd/target/enable/enable.go @@ -21,8 +21,8 @@ func NewCmdEnable(f factory.Factory) *cobra.Command { %[1]s deployment-target enable 'web-server' `, constants.ExecutableName), RunE: func(c *cobra.Command, args []string) error { - opts := shared.NewSetDisabledStateOptions(args, cmd.NewDependencies(f, c)) - return shared.SetDisabledState(opts, false) + opts := shared.NewSetDisabledStateOptions(args, cmd.NewDependencies(f, c), false) + return shared.SetDisabledState(opts) }, } } diff --git a/pkg/cmd/target/enable/enable_test.go b/pkg/cmd/target/enable/enable_test.go index ca5bc078..eccf8bdb 100644 --- a/pkg/cmd/target/enable/enable_test.go +++ b/pkg/cmd/target/enable/enable_test.go @@ -83,7 +83,7 @@ func TestDeploymentTargetEnable(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/machines?take=2147483647"). + api.ExpectRequest(t, "GET", "/api/Spaces-1/machines?isDisabled=true&take=2147483647"). RespondWith(resources.Resources[*machines.DeploymentTarget]{Items: []*machines.DeploymentTarget{ newTarget("Machines-100", "web-server", true), newTarget("Machines-200", "db-server", true), diff --git a/pkg/cmd/target/shared/disabledstate.go b/pkg/cmd/target/shared/disabledstate.go index b5f93eec..513afd9a 100644 --- a/pkg/cmd/target/shared/disabledstate.go +++ b/pkg/cmd/target/shared/disabledstate.go @@ -14,29 +14,37 @@ type SetDisabledStateOptions struct { *cmd.Dependencies *GetTargetsOptions IdOrName string + // Disabled is the state the deployment target should end up in. + Disabled bool // Target is the deployment target chosen at the prompt. When set it is used directly, saving a // round trip back to the server for something we already have. Target *machines.DeploymentTarget } -func NewSetDisabledStateOptions(args []string, dependencies *cmd.Dependencies) *SetDisabledStateOptions { +func NewSetDisabledStateOptions(args []string, dependencies *cmd.Dependencies, disabled bool) *SetDisabledStateOptions { idOrName := "" if len(args) > 0 { idOrName = args[0] } + // Only targets that aren't already in the requested state are worth offering. The machines + // endpoint can filter server-side for the enable case (the query field is omitempty, so only + // isDisabled=true can be expressed); the disable case is filtered client-side below. + query := machines.MachinesQuery{IsDisabled: !disabled} + return &SetDisabledStateOptions{ Dependencies: dependencies, - GetTargetsOptions: NewGetTargetsOptionsForAllTargets(dependencies), + GetTargetsOptions: NewGetTargetsOptions(dependencies, query), IdOrName: idOrName, + Disabled: disabled, } } // SetDisabledState enables or disables a deployment target, prompting for the target when no // name or ID was supplied. -func SetDisabledState(opts *SetDisabledStateOptions, isDisabled bool) error { +func SetDisabledState(opts *SetDisabledStateOptions) error { if !opts.NoPrompt { - if err := PromptMissingTarget(opts, isDisabled); err != nil { + if err := PromptMissingTarget(opts); err != nil { return err } } @@ -53,13 +61,13 @@ func SetDisabledState(opts *SetDisabledStateOptions, isDisabled bool) error { } } - state := disabledStateDescription(isDisabled) - if target.IsDisabled == isDisabled { + state := disabledStateDescription(opts.Disabled) + if target.IsDisabled == opts.Disabled { _, _ = fmt.Fprintf(opts.Out, "Deployment target '%s' %s is already %s.\n", target.Name, output.Dimf("(%s)", target.GetID()), state) return nil } - target.IsDisabled = isDisabled + target.IsDisabled = opts.Disabled if _, err := machines.Update(opts.Client, target); err != nil { return err } @@ -68,7 +76,7 @@ func SetDisabledState(opts *SetDisabledStateOptions, isDisabled bool) error { return nil } -func PromptMissingTarget(opts *SetDisabledStateOptions, isDisabled bool) error { +func PromptMissingTarget(opts *SetDisabledStateOptions) error { if opts.IdOrName != "" { return nil } @@ -78,12 +86,25 @@ func PromptMissingTarget(opts *SetDisabledStateOptions, isDisabled bool) error { return err } + // The server-side filter isn't guaranteed (older servers, and the disable case can't express + // it), so drop anything already in the requested state here as well. + candidates := make([]*machines.DeploymentTarget, 0, len(targets)) + for _, target := range targets { + if target.IsDisabled != opts.Disabled { + candidates = append(candidates, target) + } + } + + if len(candidates) == 0 { + return fmt.Errorf("no deployment targets to %s were found", actionDescription(opts.Disabled)) + } + // deliberately not selectors.Select: that auto-selects when there is exactly one target, which // would mutate the target without the user ever being asked. Enable/disable always asks. selectedTarget, err := question.SelectMap( opts.Ask, - fmt.Sprintf("Select the deployment target you wish to %s:", actionDescription(isDisabled)), - targets, + fmt.Sprintf("Select the deployment target you wish to %s:", actionDescription(opts.Disabled)), + candidates, func(target *machines.DeploymentTarget) string { return target.Name }) if err != nil { return err diff --git a/pkg/cmd/target/shared/disabledstate_test.go b/pkg/cmd/target/shared/disabledstate_test.go index 5f9c26ab..50858de3 100644 --- a/pkg/cmd/target/shared/disabledstate_test.go +++ b/pkg/cmd/target/shared/disabledstate_test.go @@ -14,9 +14,9 @@ func TestPromptMissingTarget_IdentifierSupplied(t *testing.T) { pa := []*testutil.PA{} asker, checkRemainingPrompts := testutil.NewMockAsker(t, pa) - opts := shared.NewSetDisabledStateOptions([]string{"Machines-1"}, &cmd.Dependencies{Ask: asker}) + opts := shared.NewSetDisabledStateOptions([]string{"Machines-1"}, &cmd.Dependencies{Ask: asker}, true) - err := shared.PromptMissingTarget(opts, true) + err := shared.PromptMissingTarget(opts) checkRemainingPrompts() assert.NoError(t, err) @@ -29,15 +29,15 @@ func TestPromptMissingTarget_NoIdentifierSupplied(t *testing.T) { } asker, checkRemainingPrompts := testutil.NewMockAsker(t, pa) - opts := shared.NewSetDisabledStateOptions([]string{}, &cmd.Dependencies{Ask: asker}) + opts := shared.NewSetDisabledStateOptions([]string{}, &cmd.Dependencies{Ask: asker}, true) opts.GetTargetsCallback = func() ([]*machines.DeploymentTarget, error) { return []*machines.DeploymentTarget{ - newTestTarget("Machines-1", "web-server"), - newTestTarget("Machines-2", "db-server"), + newTestTarget("Machines-1", "web-server", false), + newTestTarget("Machines-2", "db-server", false), }, nil } - err := shared.PromptMissingTarget(opts, true) + err := shared.PromptMissingTarget(opts) checkRemainingPrompts() assert.NoError(t, err) @@ -50,15 +50,15 @@ func TestPromptMissingTarget_EnableUsesEnableWording(t *testing.T) { } asker, checkRemainingPrompts := testutil.NewMockAsker(t, pa) - opts := shared.NewSetDisabledStateOptions([]string{}, &cmd.Dependencies{Ask: asker}) + opts := shared.NewSetDisabledStateOptions([]string{}, &cmd.Dependencies{Ask: asker}, false) opts.GetTargetsCallback = func() ([]*machines.DeploymentTarget, error) { return []*machines.DeploymentTarget{ - newTestTarget("Machines-1", "web-server"), - newTestTarget("Machines-2", "db-server"), + newTestTarget("Machines-1", "web-server", true), + newTestTarget("Machines-2", "db-server", true), }, nil } - err := shared.PromptMissingTarget(opts, false) + err := shared.PromptMissingTarget(opts) checkRemainingPrompts() assert.NoError(t, err) @@ -73,21 +73,35 @@ func TestPromptMissingTarget_AsksEvenWhenThereIsOnlyOneTarget(t *testing.T) { } asker, checkRemainingPrompts := testutil.NewMockAsker(t, pa) - opts := shared.NewSetDisabledStateOptions([]string{}, &cmd.Dependencies{Ask: asker}) + opts := shared.NewSetDisabledStateOptions([]string{}, &cmd.Dependencies{Ask: asker}, true) opts.GetTargetsCallback = func() ([]*machines.DeploymentTarget, error) { - return []*machines.DeploymentTarget{newTestTarget("Machines-1", "web-server")}, nil + return []*machines.DeploymentTarget{newTestTarget("Machines-1", "web-server", false)}, nil } - err := shared.PromptMissingTarget(opts, true) + err := shared.PromptMissingTarget(opts) checkRemainingPrompts() assert.NoError(t, err) assert.Equal(t, "Machines-1", opts.IdOrName) } -func newTestTarget(id string, name string) *machines.DeploymentTarget { +func TestPromptMissingTarget_ErrorsWhenNoTargetIsInTheOppositeState(t *testing.T) { + asker, checkRemainingPrompts := testutil.NewMockAsker(t, []*testutil.PA{}) + opts := shared.NewSetDisabledStateOptions([]string{}, &cmd.Dependencies{Ask: asker}, true) + opts.GetTargetsCallback = func() ([]*machines.DeploymentTarget, error) { + return []*machines.DeploymentTarget{newTestTarget("Machines-1", "web-server", true)}, nil + } + + err := shared.PromptMissingTarget(opts) + checkRemainingPrompts() + + assert.EqualError(t, err, "no deployment targets to disable were found") +} + +func newTestTarget(id string, name string, isDisabled bool) *machines.DeploymentTarget { target := machines.NewDeploymentTarget(name, machines.NewCloudRegionEndpoint(), []string{"Environments-1"}, []string{"web"}) target.ID = id target.SpaceID = "Spaces-1" + target.IsDisabled = isDisabled return target } From a3d40fb41c5264ea2e62d9aeef2d0d02e4eae454 Mon Sep 17 00:00:00 2001 From: Nick Josevski Date: Mon, 31 Aug 2026 12:07:52 +1000 Subject: [PATCH 7/8] fix: correct "tenatcle" typo in the listening tentacle create output Pre-existing, but in a line this PR already touches. Co-Authored-By: Claude Opus 5 (1M context) --- pkg/cmd/target/listening-tentacle/create/create.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/cmd/target/listening-tentacle/create/create.go b/pkg/cmd/target/listening-tentacle/create/create.go index 5fb5f2e1..f76acd78 100644 --- a/pkg/cmd/target/listening-tentacle/create/create.go +++ b/pkg/cmd/target/listening-tentacle/create/create.go @@ -157,7 +157,7 @@ func createRun(opts *CreateOptions) error { return err } - fmt.Fprintf(opts.Out, "Successfully created listening tenatcle '%s'.\n", deploymentTarget.Name) + fmt.Fprintf(opts.Out, "Successfully created listening tentacle '%s'.\n", deploymentTarget.Name) if !opts.NoPrompt { autoCmd := flag.GenerateAutomationCmd(opts.CmdPath, opts.GetSpaceNameOrEmpty(), opts.Name, opts.URL, opts.Thumbprint, opts.Environments, opts.Roles, opts.Tags, opts.Proxy, opts.MachinePolicy, opts.TenantedDeploymentMode, opts.Tenants, opts.TenantTags, opts.Disabled) fmt.Fprintf(opts.Out, "\nAutomation Command: %s\n", autoCmd) From 314eafc766c1544932434c6eec585a2a528bd321 Mon Sep 17 00:00:00 2001 From: Nick Josevski Date: Mon, 31 Aug 2026 12:08:37 +1000 Subject: [PATCH 8/8] test: share a deployment target fixture instead of duplicating it `newTarget` was byte-for-byte identical in the enable and disable command tests, with a third near-copy in the shared package's test. Move it to test/fixtures next to NewSpace/NewTenant so the copies can't drift. Co-Authored-By: Claude Opus 5 (1M context) --- pkg/cmd/target/disable/disable_test.go | 26 +++++++-------------- pkg/cmd/target/enable/enable_test.go | 20 +++++----------- pkg/cmd/target/shared/disabledstate_test.go | 21 ++++++----------- test/fixtures/projects.go | 9 +++++++ 4 files changed, 31 insertions(+), 45 deletions(-) diff --git a/pkg/cmd/target/disable/disable_test.go b/pkg/cmd/target/disable/disable_test.go index 315409dc..5b96931e 100644 --- a/pkg/cmd/target/disable/disable_test.go +++ b/pkg/cmd/target/disable/disable_test.go @@ -19,14 +19,6 @@ var rootResource = testutil.NewRootResource() const spaceID = "Spaces-1" -func newTarget(id string, name string, isDisabled bool) *machines.DeploymentTarget { - target := machines.NewDeploymentTarget(name, machines.NewCloudRegionEndpoint(), []string{"Environments-1"}, []string{"web"}) - target.ID = id - target.SpaceID = spaceID - target.IsDisabled = isDisabled - return target -} - func TestDeploymentTargetDisable(t *testing.T) { space1 := fixtures.NewSpace(spaceID, "Default Space") @@ -43,13 +35,13 @@ func TestDeploymentTargetDisable(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/machines/Machines-100").RespondWith(newTarget("Machines-100", "web-server", false)) + api.ExpectRequest(t, "GET", "/api/Spaces-1/machines/Machines-100").RespondWith(fixtures.NewDeploymentTarget(spaceID, "Machines-100", "web-server", false)) updateRequest := api.ExpectRequest(t, "PUT", "/api/Spaces-1/machines/Machines-100") updated, err := testutil.ReadJson[machines.DeploymentTarget](updateRequest.Request.Body) assert.Nil(t, err) assert.True(t, updated.IsDisabled) - updateRequest.RespondWith(newTarget("Machines-100", "web-server", true)) + updateRequest.RespondWith(fixtures.NewDeploymentTarget(spaceID, "Machines-100", "web-server", true)) _, err = testutil.ReceivePair(cmdReceiver) assert.Nil(t, err) @@ -66,7 +58,7 @@ func TestDeploymentTargetDisable(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/machines/Machines-100").RespondWith(newTarget("Machines-100", "web-server", true)) + api.ExpectRequest(t, "GET", "/api/Spaces-1/machines/Machines-100").RespondWith(fixtures.NewDeploymentTarget(spaceID, "Machines-100", "web-server", true)) _, err := testutil.ReceivePair(cmdReceiver) assert.Nil(t, err) @@ -85,8 +77,8 @@ func TestDeploymentTargetDisable(t *testing.T) { api.ExpectRequest(t, "GET", "/api/Spaces-1").RespondWith(rootResource) api.ExpectRequest(t, "GET", "/api/Spaces-1/machines?take=2147483647"). RespondWith(resources.Resources[*machines.DeploymentTarget]{Items: []*machines.DeploymentTarget{ - newTarget("Machines-100", "web-server", false), - newTarget("Machines-200", "db-server", true), + fixtures.NewDeploymentTarget(spaceID, "Machines-100", "web-server", false), + fixtures.NewDeploymentTarget(spaceID, "Machines-200", "db-server", true), }}) _ = qa.ExpectQuestion(t, &survey.Select{ @@ -94,7 +86,7 @@ func TestDeploymentTargetDisable(t *testing.T) { Options: []string{"web-server"}, }).AnswerWith("web-server") - api.ExpectRequest(t, "PUT", "/api/Spaces-1/machines/Machines-100").RespondWith(newTarget("Machines-100", "web-server", true)) + api.ExpectRequest(t, "PUT", "/api/Spaces-1/machines/Machines-100").RespondWith(fixtures.NewDeploymentTarget(spaceID, "Machines-100", "web-server", true)) _, err := testutil.ReceivePair(cmdReceiver) assert.Nil(t, err) @@ -113,8 +105,8 @@ func TestDeploymentTargetDisable(t *testing.T) { api.ExpectRequest(t, "GET", "/api/Spaces-1").RespondWith(rootResource) api.ExpectRequest(t, "GET", "/api/Spaces-1/machines?take=2147483647"). RespondWith(resources.Resources[*machines.DeploymentTarget]{Items: []*machines.DeploymentTarget{ - newTarget("Machines-100", "web-server", false), - newTarget("Machines-200", "db-server", false), + fixtures.NewDeploymentTarget(spaceID, "Machines-100", "web-server", false), + fixtures.NewDeploymentTarget(spaceID, "Machines-200", "db-server", false), }}) _ = qa.ExpectQuestion(t, &survey.Select{ @@ -122,7 +114,7 @@ func TestDeploymentTargetDisable(t *testing.T) { Options: []string{"web-server", "db-server"}, }).AnswerWith("db-server") - api.ExpectRequest(t, "PUT", "/api/Spaces-1/machines/Machines-200").RespondWith(newTarget("Machines-200", "db-server", true)) + api.ExpectRequest(t, "PUT", "/api/Spaces-1/machines/Machines-200").RespondWith(fixtures.NewDeploymentTarget(spaceID, "Machines-200", "db-server", true)) _, err := testutil.ReceivePair(cmdReceiver) assert.Nil(t, err) diff --git a/pkg/cmd/target/enable/enable_test.go b/pkg/cmd/target/enable/enable_test.go index eccf8bdb..84b2c169 100644 --- a/pkg/cmd/target/enable/enable_test.go +++ b/pkg/cmd/target/enable/enable_test.go @@ -19,14 +19,6 @@ var rootResource = testutil.NewRootResource() const spaceID = "Spaces-1" -func newTarget(id string, name string, isDisabled bool) *machines.DeploymentTarget { - target := machines.NewDeploymentTarget(name, machines.NewCloudRegionEndpoint(), []string{"Environments-1"}, []string{"web"}) - target.ID = id - target.SpaceID = spaceID - target.IsDisabled = isDisabled - return target -} - func TestDeploymentTargetEnable(t *testing.T) { space1 := fixtures.NewSpace(spaceID, "Default Space") @@ -43,13 +35,13 @@ func TestDeploymentTargetEnable(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/machines/Machines-100").RespondWith(newTarget("Machines-100", "web-server", true)) + api.ExpectRequest(t, "GET", "/api/Spaces-1/machines/Machines-100").RespondWith(fixtures.NewDeploymentTarget(spaceID, "Machines-100", "web-server", true)) updateRequest := api.ExpectRequest(t, "PUT", "/api/Spaces-1/machines/Machines-100") updated, err := testutil.ReadJson[machines.DeploymentTarget](updateRequest.Request.Body) assert.Nil(t, err) assert.False(t, updated.IsDisabled) - updateRequest.RespondWith(newTarget("Machines-100", "web-server", false)) + updateRequest.RespondWith(fixtures.NewDeploymentTarget(spaceID, "Machines-100", "web-server", false)) _, err = testutil.ReceivePair(cmdReceiver) assert.Nil(t, err) @@ -66,7 +58,7 @@ func TestDeploymentTargetEnable(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/machines/Machines-100").RespondWith(newTarget("Machines-100", "web-server", false)) + api.ExpectRequest(t, "GET", "/api/Spaces-1/machines/Machines-100").RespondWith(fixtures.NewDeploymentTarget(spaceID, "Machines-100", "web-server", false)) _, err := testutil.ReceivePair(cmdReceiver) assert.Nil(t, err) @@ -85,8 +77,8 @@ func TestDeploymentTargetEnable(t *testing.T) { api.ExpectRequest(t, "GET", "/api/Spaces-1").RespondWith(rootResource) api.ExpectRequest(t, "GET", "/api/Spaces-1/machines?isDisabled=true&take=2147483647"). RespondWith(resources.Resources[*machines.DeploymentTarget]{Items: []*machines.DeploymentTarget{ - newTarget("Machines-100", "web-server", true), - newTarget("Machines-200", "db-server", true), + fixtures.NewDeploymentTarget(spaceID, "Machines-100", "web-server", true), + fixtures.NewDeploymentTarget(spaceID, "Machines-200", "db-server", true), }}) _ = qa.ExpectQuestion(t, &survey.Select{ @@ -94,7 +86,7 @@ func TestDeploymentTargetEnable(t *testing.T) { Options: []string{"web-server", "db-server"}, }).AnswerWith("web-server") - api.ExpectRequest(t, "PUT", "/api/Spaces-1/machines/Machines-100").RespondWith(newTarget("Machines-100", "web-server", false)) + api.ExpectRequest(t, "PUT", "/api/Spaces-1/machines/Machines-100").RespondWith(fixtures.NewDeploymentTarget(spaceID, "Machines-100", "web-server", false)) _, err := testutil.ReceivePair(cmdReceiver) assert.Nil(t, err) diff --git a/pkg/cmd/target/shared/disabledstate_test.go b/pkg/cmd/target/shared/disabledstate_test.go index 50858de3..091f8f02 100644 --- a/pkg/cmd/target/shared/disabledstate_test.go +++ b/pkg/cmd/target/shared/disabledstate_test.go @@ -5,6 +5,7 @@ import ( "github.com/OctopusDeploy/cli/pkg/cmd" "github.com/OctopusDeploy/cli/pkg/cmd/target/shared" + "github.com/OctopusDeploy/cli/test/fixtures" "github.com/OctopusDeploy/cli/test/testutil" "github.com/OctopusDeploy/go-octopusdeploy/v2/pkg/machines" "github.com/stretchr/testify/assert" @@ -32,8 +33,8 @@ func TestPromptMissingTarget_NoIdentifierSupplied(t *testing.T) { opts := shared.NewSetDisabledStateOptions([]string{}, &cmd.Dependencies{Ask: asker}, true) opts.GetTargetsCallback = func() ([]*machines.DeploymentTarget, error) { return []*machines.DeploymentTarget{ - newTestTarget("Machines-1", "web-server", false), - newTestTarget("Machines-2", "db-server", false), + fixtures.NewDeploymentTarget("Spaces-1", "Machines-1", "web-server", false), + fixtures.NewDeploymentTarget("Spaces-1", "Machines-2", "db-server", false), }, nil } @@ -53,8 +54,8 @@ func TestPromptMissingTarget_EnableUsesEnableWording(t *testing.T) { opts := shared.NewSetDisabledStateOptions([]string{}, &cmd.Dependencies{Ask: asker}, false) opts.GetTargetsCallback = func() ([]*machines.DeploymentTarget, error) { return []*machines.DeploymentTarget{ - newTestTarget("Machines-1", "web-server", true), - newTestTarget("Machines-2", "db-server", true), + fixtures.NewDeploymentTarget("Spaces-1", "Machines-1", "web-server", true), + fixtures.NewDeploymentTarget("Spaces-1", "Machines-2", "db-server", true), }, nil } @@ -75,7 +76,7 @@ func TestPromptMissingTarget_AsksEvenWhenThereIsOnlyOneTarget(t *testing.T) { asker, checkRemainingPrompts := testutil.NewMockAsker(t, pa) opts := shared.NewSetDisabledStateOptions([]string{}, &cmd.Dependencies{Ask: asker}, true) opts.GetTargetsCallback = func() ([]*machines.DeploymentTarget, error) { - return []*machines.DeploymentTarget{newTestTarget("Machines-1", "web-server", false)}, nil + return []*machines.DeploymentTarget{fixtures.NewDeploymentTarget("Spaces-1", "Machines-1", "web-server", false)}, nil } err := shared.PromptMissingTarget(opts) @@ -89,7 +90,7 @@ func TestPromptMissingTarget_ErrorsWhenNoTargetIsInTheOppositeState(t *testing.T asker, checkRemainingPrompts := testutil.NewMockAsker(t, []*testutil.PA{}) opts := shared.NewSetDisabledStateOptions([]string{}, &cmd.Dependencies{Ask: asker}, true) opts.GetTargetsCallback = func() ([]*machines.DeploymentTarget, error) { - return []*machines.DeploymentTarget{newTestTarget("Machines-1", "web-server", true)}, nil + return []*machines.DeploymentTarget{fixtures.NewDeploymentTarget("Spaces-1", "Machines-1", "web-server", true)}, nil } err := shared.PromptMissingTarget(opts) @@ -97,11 +98,3 @@ func TestPromptMissingTarget_ErrorsWhenNoTargetIsInTheOppositeState(t *testing.T assert.EqualError(t, err, "no deployment targets to disable were found") } - -func newTestTarget(id string, name string, isDisabled bool) *machines.DeploymentTarget { - target := machines.NewDeploymentTarget(name, machines.NewCloudRegionEndpoint(), []string{"Environments-1"}, []string{"web"}) - target.ID = id - target.SpaceID = "Spaces-1" - target.IsDisabled = isDisabled - return target -} diff --git a/test/fixtures/projects.go b/test/fixtures/projects.go index 25525fce..4172ea0d 100644 --- a/test/fixtures/projects.go +++ b/test/fixtures/projects.go @@ -12,6 +12,7 @@ import ( "github.com/OctopusDeploy/go-octopusdeploy/v2/pkg/deployments" "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/machines" "github.com/OctopusDeploy/go-octopusdeploy/v2/pkg/projects" "github.com/OctopusDeploy/go-octopusdeploy/v2/pkg/releases" "github.com/OctopusDeploy/go-octopusdeploy/v2/pkg/resources" @@ -241,6 +242,14 @@ func NewTenant(spaceID string, tenantID string, name string, tenantTags ...strin return result } +func NewDeploymentTarget(spaceID string, targetID string, name string, isDisabled bool) *machines.DeploymentTarget { + result := machines.NewDeploymentTarget(name, machines.NewCloudRegionEndpoint(), []string{"Environments-1"}, []string{"web"}) + result.ID = targetID + result.SpaceID = spaceID + result.IsDisabled = isDisabled + return result +} + func NewRunbook(spaceID string, projectID string, runbookID string, name string) *runbooks.Runbook { result := runbooks.NewRunbook(name, projectID) result.ID = runbookID