Skip to content

fix: report unknown release versions instead of a server null reference #696

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
NickJosevski wants to merge 5 commits into
base: main
Choose a base branch
from
Draft

fix: report unknown release versions instead of a server null reference #696

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
af509e5
fix: report unknown release versions instead of a server null reference
NickJosevski Aug 19, 2026
2541136
fix: don't assert "not found" when the release lookup is ambiguous
NickJosevski Aug 31, 2026
cf2dc4c
refactor: drop the now-unreachable web-URL release lookup
NickJosevski Aug 31, 2026
14ff2ee
fix: only a missing release aborts the deploy pre-flight
NickJosevski Aug 31, 2026
4e9ec94
refactor: call selectors.FindRelease directly from GetReleaseID
NickJosevski Aug 31, 2026
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
View file
Open in desktop
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)

@NickJosevski NickJosevski Aug 30, 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.

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.

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
View file
Open in desktop
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
View file
Open in desktop
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