Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
37 changes: 22 additions & 15 deletions pkg/cmd/release/deploy/deploy.go
Original file line number Diff line number Diff line change
Expand Up @@ -317,6 +317,23 @@ func deployRun(cmd *cobra.Command, f factory.Factory, flags *DeployFlags) error
return err
}
options.ProjectName = project.GetName()

if options.ReleaseVersion != "" {
// resolve the release up front; the executions API reports an unknown version as an
// unhelpful null reference error, and having the ID saves looking it up again later.
// Only a "no such release" answer is fatal: this lookup is new to the deploy path, so
// anything else (no ReleaseView permission, a transient 5xx) must not fail a deploy
// that would previously have succeeded. In those cases the server stays the authority
// and we simply go without the release ID.
release, err := selectors.FindRelease(octopus, f.GetCurrentSpace().ID, project, options.ReleaseVersion)

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Behavioral note: this pre-flight makes every automation-mode deploy depend on being able to GET the release (ReleaseView), which the old flow never required — the executions API only ever saw the version string. A CI service account scoped to deploy but not to read releases (or a transient 5xx on this GET) now aborts a deploy that previously succeeded, since non-404 errors are returned untouched. Probably an acceptable trade, but worth a conscious decision — an alternative is to treat only a definitive 404 as fatal and fall through to the POST on any other lookup failure.

@NickJosevski NickJosevski Sep 4, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actioned in 14ff2ee. The pre-flight now only aborts on selectors.ReleaseNotFoundError (via errors.As); any other error leaves options.ReleaseID empty and falls through to the POST, letting the server stay the authority. New automation-mode test covers a 403 from the release GET still reaching the deploy.

One residual worth naming, because it is narrower than it looks. The fatal/non-fatal split is not really "was it a 404" — it is "did the response carry a body". DoRawJsonRequest short-circuits on resp.ContentLength == 0 before it ever reads the status, so the status code is only available for responses that had a decodable JSON body:

  • 404 with an APIError body → Confirmed: true → fatal (correct)
  • 404 with an empty body → Confirmed: false → fatal (correct, and the reason this case cannot be made non-fatal — it would put the NRE back)
  • 403 / 502 with a body → returned untouched → non-fatal, deploy proceeds (what the new test covers)
  • 403 / 502 with Content-Length: 0 → indistinguishable from the row above it → still fatal, reported as "could not resolve …"

So the mitigation covers failures that come back with a body, and the bodyless non-404 is the leftover. Note a genuine proxy 502 usually has an HTML body, which fails to decode as APIError and returns a decode error — also non-fatal. That leaves the gap at explicitly bodyless non-404 responses, which is a small set.

The question that would close it properly: does Octopus GET /api/{space}/projects/{id}/releases/{version} return a 404 with an APIError body for a version that does not exist? If it always does, the empty-body branch is only ever reached by non-404s and could safely be non-fatal, collapsing the residual entirely. This PR handles both shapes defensively but does not establish which one the server actually sends — worth confirming against a real instance before deciding.

@NickJosevski NickJosevski Sep 4, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Checked against a live instance (md.octopus.app, Octopus Server 2026.3.14820, Octopus Cloud). The residual is not reachable there — Confirmed: true is the real path.

Raw HTTP for GET /api/Spaces-1/projects/{id}/releases/{missing-version}:

HTTP/2 404
content-type: application/json; charset=UTF-8

{ "ErrorMessage": "Release '9.9.9-does-not-exist' for project 'Cycle' was not found." }

The 404 carries an APIError body, so it decodes on the error path and arrives as Confirmed: true.

The framing detail matters more than the body, though. This endpoint never sends Content-Length — HTTP/2 omits it, and forcing --http1.1 gives transfer-encoding: chunked. Go sets resp.ContentLength = -1 in both cases, so resp.ContentLength == 0 in DoRawJsonRequest is false and the short-circuit is never taken at all, regardless of status code. Verified through the real SDK rather than inferred:

RAW: proto=HTTP/2.0 status=404 ContentLength=-1  -> short-circuit (==0)? false

--- FindRelease("9.9.9-nope") ---
  ReleaseNotFoundError, Confirmed=true
--- FindRelease("latest") ---
  ReleaseNotFoundError, Confirmed=true
--- FindRelease("0.0.23") ---
  OK, release ID=Releases-1094

So on this server the Confirmed: false branch is unreachable, which also means the bodyless-403/502 case I worried about cannot arise from the server itself. It could still arise from something in front of it (a customer reverse proxy emitting Content-Length: 0), so the branch is worth keeping as a guard — but as a guard, not as an expected path. I would leave 14ff2ee as-is; it costs nothing and the errors.As split is the right shape either way.

var releaseNotFound *selectors.ReleaseNotFoundError
if errors.As(err, &releaseNotFound) {
return err
}
if err == nil {
options.ReleaseID = release.ID
}
}
}

}
Expand Down Expand Up @@ -350,20 +367,10 @@ func deployRun(cmd *cobra.Command, f factory.Factory, flags *DeployFlags) error

// output web URL all the time, so long as output format is not JSON or basic
if err == nil && !constants.IsProgrammaticOutputFormat(outputFormat) {
releaseID := options.ReleaseID
if releaseID == "" {
// we may already have the release ID from AskQuestions. If not, we need to go and look up the release ID to link to it
// which needs the project ID. Errors here are ignorable; it's not the end of the world if we can't print the web link
prj, err := selectors.FindProject(octopus, options.ProjectName)
if err == nil {
rel, err := releases.GetReleaseInProject(octopus, f.GetCurrentSpace().ID, prj.ID, options.ReleaseVersion)
if err == nil {
releaseID = rel.ID
}
}
}

if releaseID != "" {
// both paths that reach here have already resolved the release: AskQuestions in interactive
// mode, the pre-flight lookup in automation mode. It stays empty only when that lookup failed
// for a reason we deliberately ignored, in which case repeating it here would fail too.
if releaseID := options.ReleaseID; releaseID != "" {
link := output.Bluef("%s/app#/%s/releases/%s", f.GetCurrentHost(), f.GetCurrentSpace().ID, releaseID)
cmd.Printf("\nView this release on Octopus Deploy: %s\n", link)
}
Expand Down Expand Up @@ -426,7 +433,7 @@ func AskQuestions(octopus *octopusApiClient.Client, stdout io.Writer, asker ques
return err
}
} else {
selectedRelease, err = releases.GetReleaseInProject(octopus, space.ID, selectedProject.ID, options.ReleaseVersion)
selectedRelease, err = selectors.FindRelease(octopus, space.ID, selectedProject, options.ReleaseVersion)
if err != nil {
return err
}
Expand Down
118 changes: 94 additions & 24 deletions pkg/cmd/release/deploy/deploy_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1594,6 +1594,7 @@ func TestDeployCreate_AutomationMode(t *testing.T) {
api.ExpectRequest(t, "GET", "/api/").RespondWith(rootResource)
api.ExpectRequest(t, "GET", "/api/Spaces-1").RespondWith(rootResource)
api.ExpectRequest(t, "GET", "/api/Spaces-1/projects/"+fireProject.GetName()).RespondWith(fireProject)
api.ExpectRequest(t, "GET", "/api/Spaces-1/projects/"+fireProjectID+"/releases/1.9").RespondWith(release10)

_, err := testutil.ReceivePair(cmdReceiver)
assert.EqualError(t, err, "environment(s) must be specified")
Expand All @@ -1602,6 +1603,87 @@ func TestDeployCreate_AutomationMode(t *testing.T) {
assert.Equal(t, "", stdErr.String())
}},

{"release deploy reports a release version that doesn't exist", func(t *testing.T, api *testutil.MockHttpServer, rootCmd *cobra.Command, stdOut *bytes.Buffer, stdErr *bytes.Buffer) {
cmdReceiver := testutil.GoBegin2(func() (*cobra.Command, error) {
defer api.Close()
rootCmd.SetArgs([]string{"release", "deploy", "--project", fireProject.Name, "--version", "9.9", "--environment", "dev"})
return rootCmd.ExecuteC()
})

api.ExpectRequest(t, "GET", "/api/").RespondWith(rootResource)
api.ExpectRequest(t, "GET", "/api/Spaces-1").RespondWith(rootResource)
api.ExpectRequest(t, "GET", "/api/Spaces-1/projects/"+fireProject.GetName()).RespondWith(fireProject)
api.ExpectRequest(t, "GET", "/api/Spaces-1/projects/"+fireProjectID+"/releases/9.9").
RespondWithStatus(404, "404 Not Found", &core.APIError{ErrorMessage: "The resource you requested was not found."})

_, err := testutil.ReceivePair(cmdReceiver)
assert.EqualError(t, err, "cannot find a release with version '9.9' in project 'Fire Project'")

assert.Equal(t, "", stdOut.String())
assert.Equal(t, "", stdErr.String())
}},

{"release deploy explains that 'latest' is not a supported release version", func(t *testing.T, api *testutil.MockHttpServer, rootCmd *cobra.Command, stdOut *bytes.Buffer, stdErr *bytes.Buffer) {
cmdReceiver := testutil.GoBegin2(func() (*cobra.Command, error) {
defer api.Close()
rootCmd.SetArgs([]string{"release", "deploy", "--project", fireProject.Name, "--version", "latest", "--environment", "dev"})
return rootCmd.ExecuteC()
})

api.ExpectRequest(t, "GET", "/api/").RespondWith(rootResource)
api.ExpectRequest(t, "GET", "/api/Spaces-1").RespondWith(rootResource)
api.ExpectRequest(t, "GET", "/api/Spaces-1/projects/"+fireProject.GetName()).RespondWith(fireProject)
api.ExpectRequest(t, "GET", "/api/Spaces-1/projects/"+fireProjectID+"/releases/latest").RespondWithStatus(404, "NotFound", nil)

_, err := testutil.ReceivePair(cmdReceiver)
assert.EqualError(t, err, "could not resolve a release with version 'latest' in project 'Fire Project'; the server returned an empty response, which usually means there is no such release, but can also mean the lookup itself failed. 'latest' is not a supported alias, specify an exact version. Run 'octopus release list --project \"Fire Project\"' to see the available versions")

assert.Equal(t, "", stdOut.String())
assert.Equal(t, "", stdErr.String())
}},

{"release deploy proceeds when the release lookup fails for a reason other than not-found", func(t *testing.T, api *testutil.MockHttpServer, rootCmd *cobra.Command, stdOut *bytes.Buffer, stdErr *bytes.Buffer) {
cmdReceiver := testutil.GoBegin2(func() (*cobra.Command, error) {
defer api.Close()
rootCmd.SetArgs([]string{"release", "deploy", "--project", fireProject.Name, "--version", "1.0", "--environment", "dev"})
return rootCmd.ExecuteC()
})

api.ExpectRequest(t, "GET", "/api/").RespondWith(rootResource)
api.ExpectRequest(t, "GET", "/api/Spaces-1").RespondWith(rootResource)
api.ExpectRequest(t, "GET", "/api/Spaces-1/projects/"+fireProject.GetName()).RespondWith(fireProject)

// an account allowed to deploy but not to read releases must not be blocked by the pre-flight lookup
api.ExpectRequest(t, "GET", "/api/Spaces-1/projects/"+fireProjectID+"/releases/1.0").
RespondWithStatus(403, "403 Forbidden", &core.APIError{ErrorMessage: "You do not have permission to perform this action."})

req := api.ExpectRequest(t, "POST", "/api/Spaces-1/deployments/create/untenanted/v1")
requestBody, err := testutil.ReadJson[deployments.CreateDeploymentUntenantedCommandV1](req.Request.Body)
assert.Nil(t, err)

assert.Equal(t, deployments.CreateDeploymentUntenantedCommandV1{
ReleaseVersion: "1.0",
EnvironmentNames: []string{"dev"},
CreateExecutionAbstractCommandV1: deployments.CreateExecutionAbstractCommandV1{
SpaceID: "Spaces-1",
ProjectIDOrName: fireProject.Name,
},
}, requestBody)

req.RespondWith(&deployments.CreateDeploymentResponseV1{
DeploymentServerTasks: []*deployments.DeploymentServerTask{
{DeploymentID: "Deployments-203", ServerTaskID: "ServerTasks-29394"},
},
})

_, err = testutil.ReceivePair(cmdReceiver)
assert.Nil(t, err)

// no release ID, so no web link; the deployment itself still went ahead
assert.Equal(t, "Successfully started 1 deployment(s)\n", stdOut.String())
assert.Equal(t, "", stdErr.String())
}},

{"release deploy specifying project, version, env only (bare minimum) assuming untenanted", func(t *testing.T, api *testutil.MockHttpServer, rootCmd *cobra.Command, stdOut *bytes.Buffer, stdErr *bytes.Buffer) {
cmdReceiver := testutil.GoBegin2(func() (*cobra.Command, error) {
defer api.Close()
Expand All @@ -1612,6 +1694,7 @@ func TestDeployCreate_AutomationMode(t *testing.T) {
api.ExpectRequest(t, "GET", "/api/").RespondWith(rootResource)
api.ExpectRequest(t, "GET", "/api/Spaces-1").RespondWith(rootResource)
api.ExpectRequest(t, "GET", "/api/Spaces-1/projects/"+fireProject.GetName()).RespondWith(fireProject)
api.ExpectRequest(t, "GET", "/api/Spaces-1/projects/"+fireProjectID+"/releases/1.0").RespondWith(release10)

// Note: because we didn't specify --tenant or --tenant-tag, automation-mode code is going to assume untenanted
req := api.ExpectRequest(t, "POST", "/api/Spaces-1/deployments/create/untenanted/v1")
Expand All @@ -1634,12 +1717,7 @@ func TestDeployCreate_AutomationMode(t *testing.T) {
},
})

// now it's going to try and look up the project/version to generate the web URL
api.ExpectRequest(t, "GET", "/api/Spaces-1/projects/Fire Project").RespondWithStatus(404, "NotFound", nil)
api.ExpectRequest(t, "GET", "/api/Spaces-1/projects?partialName=Fire+Project").RespondWith(resources.Resources[*projects.Project]{
Items: []*projects.Project{fireProject},
})
api.ExpectRequest(t, "GET", "/api/Spaces-1/projects/"+fireProjectID+"/releases/1.0").RespondWith(release10)
// no lookup to generate the web URL; the release was already resolved before deploying

_, err = testutil.ReceivePair(cmdReceiver)
assert.Nil(t, err)
Expand All @@ -1662,6 +1740,7 @@ func TestDeployCreate_AutomationMode(t *testing.T) {
api.ExpectRequest(t, "GET", "/api/").RespondWith(rootResource)
api.ExpectRequest(t, "GET", "/api/Spaces-1").RespondWith(rootResource)
api.ExpectRequest(t, "GET", "/api/Spaces-1/projects/"+fireProject.GetName()).RespondWith(fireProject)
api.ExpectRequest(t, "GET", "/api/Spaces-1/projects/"+fireProjectID+"/releases/2.1").RespondWith(release10)

// Note: because we didn't specify --tenant or --tenant-tag, automation-mode code is going to assume untenanted
req := api.ExpectRequest(t, "POST", "/api/Spaces-1/deployments/create/untenanted/v1")
Expand All @@ -1684,12 +1763,7 @@ func TestDeployCreate_AutomationMode(t *testing.T) {
},
})

// now it's going to try and look up the project/version to generate the web URL
api.ExpectRequest(t, "GET", "/api/Spaces-1/projects/Fire Project").RespondWithStatus(404, "NotFound", nil)
api.ExpectRequest(t, "GET", "/api/Spaces-1/projects?partialName=Fire+Project").RespondWith(resources.Resources[*projects.Project]{
Items: []*projects.Project{fireProject},
})
api.ExpectRequest(t, "GET", "/api/Spaces-1/projects/"+fireProjectID+"/releases/2.1").RespondWith(release10)
// no lookup to generate the web URL; the release was already resolved before deploying

_, err = testutil.ReceivePair(cmdReceiver)
assert.Nil(t, err)
Expand All @@ -1712,6 +1786,7 @@ func TestDeployCreate_AutomationMode(t *testing.T) {
api.ExpectRequest(t, "GET", "/api/").RespondWith(rootResource)
api.ExpectRequest(t, "GET", "/api/Spaces-1").RespondWith(rootResource)
api.ExpectRequest(t, "GET", "/api/Spaces-1/projects/"+fireProject.GetName()).RespondWith(fireProject)
api.ExpectRequest(t, "GET", "/api/Spaces-1/projects/"+fireProjectID+"/releases/1.0").RespondWith(release10)

// Note: because we didn't specify --tenant or --tenant-tag, automation-mode code is going to assume untenanted
api.ExpectRequest(t, "POST", "/api/Spaces-1/deployments/create/untenanted/v1").RespondWith(&deployments.CreateDeploymentResponseV1{
Expand Down Expand Up @@ -1742,6 +1817,7 @@ func TestDeployCreate_AutomationMode(t *testing.T) {
api.ExpectRequest(t, "GET", "/api/").RespondWith(rootResource)
api.ExpectRequest(t, "GET", "/api/Spaces-1").RespondWith(rootResource)
api.ExpectRequest(t, "GET", "/api/Spaces-1/projects/"+fireProject.GetName()).RespondWith(fireProject)
api.ExpectRequest(t, "GET", "/api/Spaces-1/projects/"+fireProjectID+"/releases/1.0").RespondWith(release10)

// Note: because we didn't specify --tenant or --tenant-tag, automation-mode code is going to assume untenanted
serverTasks := []*deployments.DeploymentServerTask{
Expand Down Expand Up @@ -1773,6 +1849,7 @@ func TestDeployCreate_AutomationMode(t *testing.T) {
api.ExpectRequest(t, "GET", "/api/").RespondWith(rootResource)
api.ExpectRequest(t, "GET", "/api/Spaces-1").RespondWith(rootResource)
api.ExpectRequest(t, "GET", "/api/Spaces-1/projects/"+fireProject.GetName()).RespondWith(fireProject)
api.ExpectRequest(t, "GET", "/api/Spaces-1/projects/"+fireProjectID+"/releases/1.0").RespondWith(release10)

req := api.ExpectRequest(t, "POST", "/api/Spaces-1/deployments/create/tenanted/v1")
requestBody, err := testutil.ReadJson[deployments.CreateDeploymentTenantedCommandV1](req.Request.Body)
Expand All @@ -1794,12 +1871,7 @@ func TestDeployCreate_AutomationMode(t *testing.T) {
},
})

// now it's going to try and look up the project/version to generate the web URL
api.ExpectRequest(t, "GET", "/api/Spaces-1/projects/Fire Project").RespondWithStatus(404, "NotFound", nil)
api.ExpectRequest(t, "GET", "/api/Spaces-1/projects?partialName=Fire+Project").RespondWith(resources.Resources[*projects.Project]{
Items: []*projects.Project{fireProject},
})
api.ExpectRequest(t, "GET", "/api/Spaces-1/projects/"+fireProjectID+"/releases/1.0").RespondWith(release10)
// no lookup to generate the web URL; the release was already resolved before deploying

_, err = testutil.ReceivePair(cmdReceiver)
assert.Nil(t, err)
Expand All @@ -1822,6 +1894,7 @@ func TestDeployCreate_AutomationMode(t *testing.T) {
api.ExpectRequest(t, "GET", "/api/").RespondWith(rootResource)
api.ExpectRequest(t, "GET", "/api/Spaces-1").RespondWith(rootResource)
api.ExpectRequest(t, "GET", "/api/Spaces-1/projects/"+fireProject.GetName()).RespondWith(fireProject)
api.ExpectRequest(t, "GET", "/api/Spaces-1/projects/"+fireProjectID+"/releases/1.0").RespondWith(release10)

req := api.ExpectRequest(t, "POST", "/api/Spaces-1/deployments/create/tenanted/v1")
requestBody, err := testutil.ReadJson[deployments.CreateDeploymentTenantedCommandV1](req.Request.Body)
Expand All @@ -1843,12 +1916,7 @@ func TestDeployCreate_AutomationMode(t *testing.T) {
},
})

// now it's going to try and look up the project/version to generate the web URL
api.ExpectRequest(t, "GET", "/api/Spaces-1/projects/Fire Project").RespondWithStatus(404, "NotFound", nil)
api.ExpectRequest(t, "GET", "/api/Spaces-1/projects?partialName=Fire+Project").RespondWith(resources.Resources[*projects.Project]{
Items: []*projects.Project{fireProject},
})
api.ExpectRequest(t, "GET", "/api/Spaces-1/projects/"+fireProjectID+"/releases/1.0").RespondWith(release10)
// no lookup to generate the web URL; the release was already resolved before deploying

_, err = testutil.ReceivePair(cmdReceiver)
assert.Nil(t, err)
Expand Down Expand Up @@ -1888,6 +1956,7 @@ func TestDeployCreate_AutomationMode(t *testing.T) {
api.ExpectRequest(t, "GET", "/api/").RespondWith(rootResource)
api.ExpectRequest(t, "GET", "/api/Spaces-1").RespondWith(rootResource)
api.ExpectRequest(t, "GET", "/api/Spaces-1/projects/"+fireProject.GetName()).RespondWith(fireProject)
api.ExpectRequest(t, "GET", "/api/Spaces-1/projects/"+fireProjectID+"/releases/1.0").RespondWith(release10)

// Note: because we didn't specify --tenant or --tenant-tag, automation-mode code is going to assume untenanted
req := api.ExpectRequest(t, "POST", "/api/Spaces-1/deployments/create/untenanted/v1")
Expand Down Expand Up @@ -1962,6 +2031,7 @@ func TestDeployCreate_AutomationMode(t *testing.T) {
api.ExpectRequest(t, "GET", "/api/").RespondWith(rootResource)
api.ExpectRequest(t, "GET", "/api/Spaces-1").RespondWith(rootResource)
api.ExpectRequest(t, "GET", "/api/Spaces-1/projects/"+fireProject.GetName()).RespondWith(fireProject)
api.ExpectRequest(t, "GET", "/api/Spaces-1/projects/"+fireProjectID+"/releases/1.0").RespondWith(release10)

req := api.ExpectRequest(t, "POST", "/api/Spaces-1/deployments/create/tenanted/v1")
requestBody, err := testutil.ReadJson[deployments.CreateDeploymentTenantedCommandV1](req.Request.Body)
Expand Down
15 changes: 1 addition & 14 deletions pkg/cmd/release/progression/shared/shared.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ func GetReleaseID(octopus *client.Client, spaceID string, projectIdentifier stri
return "", err
}

selectedRelease, err := FindRelease(octopus, selectedProject, version)
selectedRelease, err := selectors.FindRelease(octopus, spaceID, selectedProject, version)
if err != nil {
return "", err
}
Expand All @@ -38,16 +38,3 @@ func SelectRelease(octopus *client.Client, project *projects.Project, ask questi

return selectedRelease, nil
}

func FindRelease(octopus *client.Client, project *projects.Project, version string) (*releases.Release, error) {
existingRelease, err := releases.GetReleaseInProject(octopus, octopus.GetSpaceID(), project.GetID(), version)
if err != nil {
return nil, err
}

if existingRelease == nil {
return nil, fmt.Errorf("unable to locate a release with version/release number '%s'", version)
}

return existingRelease, nil
}
Loading