Skip to content

Commit 898a953

Browse files
NickJosevskiclaude
andcommitted
fix: stop deployment target commands crashing for AWS ECS targets
Fixes #604 'deployment-target view' and 'deployment-target list' panicked with a nil pointer dereference for AWS ECS targets. An ECS target's endpoint has the communication style "AwsEcsCluster", which the SDK does not model: machine.UnmarshalJSON deserialises only the endpoints it knows by name and leaves DeploymentTarget.Endpoint nil for the rest, so every read of the endpoint panicked. This is not ECS-specific — any target type Server gains after the SDK was last updated arrives the same way — and because 'list' reads every target's endpoint, one ECS target took down the listing for the whole space. Endpoint reads now go through shared.GetCommunicationStyle, and the type reports as "Unknown" when there is no endpoint to name it from. Everything the target itself carries still reports. Two adjacent crashes in the same paths are guarded too: TentacleVersionDetails, which the API omits for a Tentacle it has never contacted, and the unchecked endpoint type assertions in the type-specific views. Naming the type and showing ECS endpoint details needs the SDK to deserialise the endpoint first; the CLI can only degrade cleanly. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
1 parent bf27551 commit 898a953

15 files changed

Lines changed: 460 additions & 21 deletions

File tree

‎pkg/cmd/target/azure-web-app/view/view.go‎

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -45,7 +45,11 @@ func ViewRun(opts *shared.ViewOptions) error {
4545

4646
func contributeEndpoint(opts *shared.ViewOptions, targetEndpoint machines.IEndpoint) ([]*output.DataRow, error) {
4747
data := []*output.DataRow{}
48-
endpoint := targetEndpoint.(*machines.AzureWebAppEndpoint)
48+
endpoint, err := shared.EndpointAs[*machines.AzureWebAppEndpoint](targetEndpoint, "Azure Web App")
49+
if err != nil {
50+
return nil, err
51+
}
52+
4953
accountRows, err := shared.ContributeAccount(opts, endpoint.AccountID)
5054
if err != nil {
5155
return nil, err

‎pkg/cmd/target/kubernetes/view/view.go‎

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -40,7 +40,10 @@ func ViewRun(opts *shared.ViewOptions) error {
4040

4141
func contributeEndpoint(_ *shared.ViewOptions, targetEndpoint machines.IEndpoint) ([]*output.DataRow, error) {
4242
data := []*output.DataRow{}
43-
endpoint := targetEndpoint.(*machines.KubernetesEndpoint)
43+
endpoint, err := shared.EndpointAs[*machines.KubernetesEndpoint](targetEndpoint, "Kubernetes")
44+
if err != nil {
45+
return nil, err
46+
}
4447

4548
data = append(data, output.NewDataRow("Authentication Type", endpoint.Authentication.GetAuthenticationType()))
4649

‎pkg/cmd/target/list/list.go‎

Lines changed: 14 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -80,7 +80,7 @@ func ListRun(opts *ListOptions) error {
8080
environmentNames := resolveValues(item.EnvironmentIDs, environmentMap)
8181
tenantNames := resolveValues(item.TenantIDs, tenantMap)
8282
workerPool := shared.ResolveDefaultWorkerPool(item, workerPoolMap, "None")
83-
return []string{output.Bold(item.Name), machinescommon.CommunicationStyleToDescriptionMap[item.Endpoint.GetCommunicationStyle()], output.FormatAsList(item.Roles), output.FormatAsList(environmentNames), output.FormatAsList(tenantNames), output.FormatAsList(item.TenantTags), workerPool}
83+
return []string{output.Bold(item.Name), describeTargetType(item), output.FormatAsList(item.Roles), output.FormatAsList(environmentNames), output.FormatAsList(tenantNames), output.FormatAsList(item.TenantTags), workerPool}
8484
},
8585
},
8686
Basic: func(item *machines.DeploymentTarget) string {
@@ -89,6 +89,19 @@ func ListRun(opts *ListOptions) error {
8989
})
9090
}
9191

92+
func describeTargetType(target *machines.DeploymentTarget) string {
93+
communicationStyle := shared.GetCommunicationStyle(target)
94+
if communicationStyle == "" {
95+
return shared.UnknownValue
96+
}
97+
98+
if description, ok := machinescommon.CommunicationStyleToDescriptionMap[communicationStyle]; ok {
99+
return description
100+
}
101+
102+
return communicationStyle
103+
}
104+
92105
func resolveValues(keys []string, lookup map[string]string) []string {
93106
var values []string
94107
for _, key := range keys {

‎pkg/cmd/target/list/list_test.go‎

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,30 @@
1+
package list
2+
3+
import (
4+
"encoding/json"
5+
"net/url"
6+
"testing"
7+
8+
"github.com/OctopusDeploy/go-octopusdeploy/v2/pkg/machines"
9+
"github.com/stretchr/testify/assert"
10+
"github.com/stretchr/testify/require"
11+
)
12+
13+
func TestDescribeTargetType_KnownStyle(t *testing.T) {
14+
target := machines.NewDeploymentTarget("web-01", machines.NewListeningTentacleEndpoint(&url.URL{Scheme: "https", Host: "tentacle:10933"}, "thumbprint"), nil, nil)
15+
16+
assert.Equal(t, "Listening Tentacle", describeTargetType(target))
17+
}
18+
19+
func TestDescribeTargetType_EndpointMissing(t *testing.T) {
20+
target := &machines.DeploymentTarget{}
21+
require.NoError(t, json.Unmarshal([]byte(`{
22+
"Id": "Machines-1041",
23+
"Name": "aws ecs",
24+
"Endpoint": { "CommunicationStyle": "AwsEcsCluster", "ClusterName": "repro-604-cluster" }
25+
}`), target))
26+
27+
assert.NotPanics(t, func() {
28+
assert.Equal(t, "Unknown", describeTargetType(target))
29+
})
30+
}

‎pkg/cmd/target/listening-tentacle/view/view.go‎

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -42,9 +42,13 @@ func ViewRun(opts *shared.ViewOptions) error {
4242
func contributeEndpoint(opts *shared.ViewOptions, targetEndpoint machines.IEndpoint) ([]*output.DataRow, error) {
4343
data := []*output.DataRow{}
4444

45-
endpoint := targetEndpoint.(*machines.ListeningTentacleEndpoint)
46-
data = append(data, output.NewDataRow("URI", endpoint.URI.String()))
47-
data = append(data, output.NewDataRow("Tentacle version", endpoint.TentacleVersionDetails.Version))
45+
endpoint, err := shared.EndpointAs[*machines.ListeningTentacleEndpoint](targetEndpoint, "Listening Tentacle")
46+
if err != nil {
47+
return nil, err
48+
}
49+
50+
data = append(data, output.NewDataRow("URI", shared.FormatUri(endpoint.URI)))
51+
data = append(data, output.NewDataRow("Tentacle version", shared.FormatTentacleVersion(endpoint.TentacleVersionDetails)))
4852

4953
proxyData, err := shared.ContributeProxy(opts, endpoint.ProxyID)
5054
if err != nil {

‎pkg/cmd/target/polling-tentacle/view/view.go‎

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -42,9 +42,13 @@ func ViewRun(opts *shared.ViewOptions) error {
4242
func contributeEndpoint(opts *shared.ViewOptions, targetEndpoint machines.IEndpoint) ([]*output.DataRow, error) {
4343
data := []*output.DataRow{}
4444

45-
endpoint := targetEndpoint.(*machines.PollingTentacleEndpoint)
46-
data = append(data, output.NewDataRow("URI", endpoint.URI.String()))
47-
data = append(data, output.NewDataRow("Tentacle version", endpoint.TentacleVersionDetails.Version))
45+
endpoint, err := shared.EndpointAs[*machines.PollingTentacleEndpoint](targetEndpoint, "Polling Tentacle")
46+
if err != nil {
47+
return nil, err
48+
}
49+
50+
data = append(data, output.NewDataRow("URI", shared.FormatUri(endpoint.URI)))
51+
data = append(data, output.NewDataRow("Tentacle version", shared.FormatTentacleVersion(endpoint.TentacleVersionDetails)))
4852

4953
return data, nil
5054
}

‎pkg/cmd/target/shared/json.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -41,7 +41,7 @@ func GetDeploymentTargetAsJson(deps *cmd.Dependencies, target *machines.Deployme
4141
Name: target.Name,
4242
HealthStatus: target.HealthStatus,
4343
StatusSummary: target.StatusSummary,
44-
CommunicationStyle: target.Endpoint.GetCommunicationStyle(),
44+
CommunicationStyle: GetCommunicationStyle(target),
4545
Environments: environments,
4646
Roles: target.Roles,
4747
Tenants: tenants,

‎pkg/cmd/target/shared/target.go‎

Lines changed: 36 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ package shared
33
import (
44
"fmt"
55
"math"
6+
"net/url"
67

78
"github.com/OctopusDeploy/cli/pkg/cmd"
89
"github.com/OctopusDeploy/go-octopusdeploy/v2/pkg/client"
@@ -41,10 +42,39 @@ func GetAllTargets(client client.Client, query machines.MachinesQuery) ([]*machi
4142
return res.Items, nil
4243
}
4344

45+
// Displayed wherever the API or the SDK left us nothing to report.
46+
const UnknownValue = "Unknown"
47+
48+
// Returns an empty string when the SDK left us no endpoint to read the style from.
49+
func GetCommunicationStyle(target *machines.DeploymentTarget) string {
50+
if target == nil || machines.IsNil(target.Endpoint) {
51+
return ""
52+
}
53+
54+
return target.Endpoint.GetCommunicationStyle()
55+
}
56+
57+
func FormatUri(uri *url.URL) string {
58+
if uri == nil {
59+
return UnknownValue
60+
}
61+
62+
return uri.String()
63+
}
64+
65+
// The API omits the version for a Tentacle it has never contacted.
66+
func FormatTentacleVersion(details *machines.TentacleVersionDetails) string {
67+
if details == nil {
68+
return UnknownValue
69+
}
70+
71+
return details.Version
72+
}
73+
4474
func GetEndpointDetails(target *machines.DeploymentTarget) map[string]string {
4575
details := make(map[string]string)
4676

47-
switch target.Endpoint.GetCommunicationStyle() {
77+
switch GetCommunicationStyle(target) {
4878
case "AzureWebApp":
4979
if endpoint, ok := target.Endpoint.(*machines.AzureWebAppEndpoint); ok {
5080
webApp := endpoint.WebAppName
@@ -59,7 +89,7 @@ func GetEndpointDetails(target *machines.DeploymentTarget) map[string]string {
5989
}
6090
case "Ssh":
6191
if endpoint, ok := target.Endpoint.(*machines.SSHEndpoint); ok {
62-
details["URI"] = endpoint.URI.String()
92+
details["URI"] = FormatUri(endpoint.URI)
6393
runtime := "Mono"
6494
if endpoint.DotNetCorePlatform != "" {
6595
runtime = endpoint.DotNetCorePlatform
@@ -68,13 +98,13 @@ func GetEndpointDetails(target *machines.DeploymentTarget) map[string]string {
6898
}
6999
case "TentaclePassive":
70100
if endpoint, ok := target.Endpoint.(*machines.ListeningTentacleEndpoint); ok {
71-
details["URI"] = endpoint.URI.String()
72-
details["Tentacle version"] = endpoint.TentacleVersionDetails.Version
101+
details["URI"] = FormatUri(endpoint.URI)
102+
details["Tentacle version"] = FormatTentacleVersion(endpoint.TentacleVersionDetails)
73103
}
74104
case "TentacleActive":
75105
if endpoint, ok := target.Endpoint.(*machines.PollingTentacleEndpoint); ok {
76-
details["URI"] = endpoint.URI.String()
77-
details["Tentacle version"] = endpoint.TentacleVersionDetails.Version
106+
details["URI"] = FormatUri(endpoint.URI)
107+
details["Tentacle version"] = FormatTentacleVersion(endpoint.TentacleVersionDetails)
78108
}
79109
case "None":
80110
// Cloud regions typically don't have additional endpoint details
Lines changed: 128 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,128 @@
1+
package shared_test
2+
3+
import (
4+
"encoding/json"
5+
"net/url"
6+
"testing"
7+
8+
"github.com/OctopusDeploy/cli/pkg/cmd/target/shared"
9+
"github.com/OctopusDeploy/go-octopusdeploy/v2/pkg/machines"
10+
"github.com/stretchr/testify/assert"
11+
"github.com/stretchr/testify/require"
12+
)
13+
14+
// An AWS ECS target as the API returns it, trimmed to the fields the CLI reads.
15+
// Captured from /api/Spaces-1/machines on a server with an ECS target configured.
16+
const ecsTargetJson = `{
17+
"Id": "Machines-1041",
18+
"Name": "aws ecs",
19+
"SpaceId": "Spaces-1",
20+
"EnvironmentIds": ["Environments-1"],
21+
"Roles": ["ecs"],
22+
"TenantIds": [],
23+
"TenantTags": [],
24+
"HealthStatus": "Healthy",
25+
"StatusSummary": "This machine was successfully health checked.",
26+
"IsDisabled": false,
27+
"Endpoint": {
28+
"CommunicationStyle": "AwsEcsCluster",
29+
"DefaultWorkerPoolId": "WorkerPools-1",
30+
"ClusterName": "repro-604-cluster",
31+
"Region": "ap-southeast-2",
32+
"AccountId": "",
33+
"UseInstanceRole": true,
34+
"AssumeRole": false,
35+
"AssumedRoleArn": null,
36+
"AssumedRoleSession": null,
37+
"AssumeRoleSessionDurationSeconds": null,
38+
"AssumeRoleExternalId": null,
39+
"Id": null,
40+
"LastModifiedOn": null,
41+
"LastModifiedBy": null,
42+
"Links": {}
43+
}
44+
}`
45+
46+
// If this fails because the SDK learned to deserialise "AwsEcsCluster", the
47+
// unknown-type fallbacks below can start reporting real detail.
48+
func TestEcsTargetDeserialisesWithoutAnEndpoint(t *testing.T) {
49+
target := parseTarget(t, ecsTargetJson)
50+
51+
assert.True(t, machines.IsNil(target.Endpoint), "expected the SDK to leave the endpoint nil for an AwsEcsCluster target")
52+
}
53+
54+
func TestGetCommunicationStyle_EndpointMissing(t *testing.T) {
55+
target := parseTarget(t, ecsTargetJson)
56+
57+
assert.NotPanics(t, func() {
58+
assert.Equal(t, "", shared.GetCommunicationStyle(target))
59+
})
60+
}
61+
62+
func TestGetCommunicationStyle_NilTarget(t *testing.T) {
63+
assert.NotPanics(t, func() {
64+
assert.Equal(t, "", shared.GetCommunicationStyle(nil))
65+
})
66+
}
67+
68+
func TestGetCommunicationStyle_KnownEndpoint(t *testing.T) {
69+
target := machines.NewDeploymentTarget("web-01", machines.NewListeningTentacleEndpoint(&url.URL{Scheme: "https", Host: "tentacle:10933"}, "thumbprint"), []string{"Environments-1"}, []string{"web"})
70+
71+
assert.Equal(t, "TentaclePassive", shared.GetCommunicationStyle(target))
72+
}
73+
74+
func TestGetEndpointDetails_EndpointMissing(t *testing.T) {
75+
target := parseTarget(t, ecsTargetJson)
76+
77+
var details map[string]string
78+
assert.NotPanics(t, func() {
79+
details = shared.GetEndpointDetails(target)
80+
})
81+
assert.Empty(t, details)
82+
}
83+
84+
func TestGetEndpointDetails_KnownEndpoint(t *testing.T) {
85+
endpoint := machines.NewListeningTentacleEndpoint(&url.URL{Scheme: "https", Host: "tentacle:10933"}, "thumbprint")
86+
endpoint.TentacleVersionDetails = &machines.TentacleVersionDetails{Version: "6.3.1"}
87+
target := machines.NewDeploymentTarget("web-01", endpoint, []string{"Environments-1"}, []string{"web"})
88+
89+
details := shared.GetEndpointDetails(target)
90+
91+
assert.Equal(t, "https://tentacle:10933", details["URI"])
92+
assert.Equal(t, "6.3.1", details["Tentacle version"])
93+
}
94+
95+
func TestGetEndpointDetails_TentacleNeverHealthChecked(t *testing.T) {
96+
endpoint := machines.NewListeningTentacleEndpoint(&url.URL{Scheme: "https", Host: "tentacle:10933"}, "thumbprint")
97+
target := machines.NewDeploymentTarget("web-01", endpoint, []string{"Environments-1"}, []string{"web"})
98+
99+
var details map[string]string
100+
assert.NotPanics(t, func() {
101+
details = shared.GetEndpointDetails(target)
102+
})
103+
assert.Equal(t, "Unknown", details["Tentacle version"])
104+
assert.Equal(t, "https://tentacle:10933", details["URI"])
105+
}
106+
107+
func TestFormatUri_Missing(t *testing.T) {
108+
assert.NotPanics(t, func() {
109+
assert.Equal(t, "Unknown", shared.FormatUri(nil))
110+
})
111+
}
112+
113+
func TestResolveDefaultWorkerPool_EndpointMissing(t *testing.T) {
114+
target := parseTarget(t, ecsTargetJson)
115+
116+
assert.NotPanics(t, func() {
117+
assert.Equal(t, "N/A", shared.ResolveDefaultWorkerPool(target, map[string]string{}, "None"))
118+
})
119+
}
120+
121+
func parseTarget(t *testing.T, payload string) *machines.DeploymentTarget {
122+
t.Helper()
123+
124+
target := &machines.DeploymentTarget{}
125+
require.NoError(t, json.Unmarshal([]byte(payload), target))
126+
127+
return target
128+
}

‎pkg/cmd/target/shared/view.go‎

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -40,6 +40,16 @@ func NewViewOptions(viewFlags *ViewFlags, dependencies *cmd.Dependencies, args [
4040
}
4141
}
4242

43+
func EndpointAs[T machines.IEndpoint](endpoint machines.IEndpoint, description string) (T, error) {
44+
typedEndpoint, ok := endpoint.(T)
45+
if !ok {
46+
var zero T
47+
return zero, fmt.Errorf("this deployment target is not a %s deployment target", description)
48+
}
49+
50+
return typedEndpoint, nil
51+
}
52+
4353
func ViewRun(opts *ViewOptions, contributeEndpoint ContributeEndpointCallback, description string) error {
4454
var target, err = opts.Client.Machines.GetByIdentifier(opts.IdOrName)
4555
if err != nil {
@@ -53,6 +63,10 @@ func ViewRun(opts *ViewOptions, contributeEndpoint ContributeEndpointCallback, d
5363
data = append(data, output.NewDataRow("Current status", target.StatusSummary))
5464

5565
if contributeEndpoint != nil {
66+
if machines.IsNil(target.Endpoint) {
67+
return fmt.Errorf("cannot view '%s' as a %s deployment target: its target type is not supported by this version of the CLI", target.Name, description)
68+
}
69+
5670
newRows, err := contributeEndpoint(opts, target.Endpoint)
5771
if err != nil {
5872
return err

0 commit comments

Comments
 (0)