Skip to content

HELM-480: Change handler return code #17042

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

Merged
openshift-merge-bot merged 3 commits into from
Sep 2, 2026
Merged

HELM-480: Change handler return code #17042

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
6807fbc
HELM-480: Centralize Helm handler error classification and fix HTTP s…
sowmya-sl Aug 15, 2026
18b4061
chore: Remove trailing newline from handlers.go
sowmya-sl Aug 25, 2026
99b94c6
HELM-480: Use determineErrorStatusCode in HandleHelmInstallAsync
sowmya-sl Aug 26, 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
55 changes: 41 additions & 14 deletions pkg/helm/handlers/handler_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ import (
releasecommon "helm.sh/helm/v4/pkg/release/common"
releasev1 "helm.sh/helm/v4/pkg/release/v1"
repo "helm.sh/helm/v4/pkg/repo/v1"
"helm.sh/helm/v4/pkg/storage/driver"
kv1 "k8s.io/api/core/v1"
v1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/runtime"
Expand Down Expand Up @@ -358,7 +359,7 @@ func TestHelmHandlers_HandleGetRelease(t *testing.T) {
error: errors.New("unknown error occurred"),
httpStatusCode: http.StatusBadGateway,
releaseName: "Test",
expectedResponse: `{"error":"Failed to find helm release: unknown error occurred"}`,
expectedResponse: `{"error":"Failed to get helm release: unknown error occurred"}`,
},
{
name: "Return the requested release serialized in JSON format",
Expand Down Expand Up @@ -454,7 +455,7 @@ func TestHelmHandlers_HandleGetReleaseHistory(t *testing.T) {
name: "chart release history should error out when there is error from helm",
error: errors.New("Chart path is invalid"),
expectedResponse: `{"error":"Failed to list helm release history: Chart path is invalid"}`,
httpStatusCode: http.StatusBadGateway,
httpStatusCode: http.StatusBadRequest,
expectedContentType: "application/json",
releaseName: "test",
},
Expand Down Expand Up @@ -513,7 +514,7 @@ func TestHelmHandlers_HandleHelmUninstallRelease(t *testing.T) {
name: "Invalid chart uninstall release test",
error: errors.New("Chart path is invalid"),
expectedResponse: `{"error":"Failed to uninstall helm release: Chart path is invalid"}`,
httpStatusCode: http.StatusBadGateway,
httpStatusCode: http.StatusBadRequest,
expectedContentType: "application/json",
releaseName: "test",
releaseNamespace: "test-namespace",
Expand Down Expand Up @@ -576,8 +577,8 @@ func TestHelmHandlers_HandleHelmRollbackRelease(t *testing.T) {
name: "Invalid chart rollback release test",
error: errors.New("Chart path is invalid"),
body: `{"name": "test", "namespace":"test", "version":1}`,
expectedResponse: `{"error":"Failed to rollback helm releases: Chart path is invalid"}`,
httpStatusCode: http.StatusBadGateway,
expectedResponse: `{"error":"Failed to rollback helm release: Chart path is invalid"}`,
httpStatusCode: http.StatusBadRequest,
expectedContentType: "application/json",
releaseName: "test",
releaseNamespace: "test",
Expand Down Expand Up @@ -605,15 +606,15 @@ func TestHelmHandlers_HandleHelmRollbackRelease(t *testing.T) {
body: `{"name": "test", "namespace":"test", "version":"abc"}`,
expectedContentType: "application/json",
error: errors.New(`{"error":"Failed to parse request: json: cannot unmarshal string into Go struct field HelmRequest.version of type int"}`),
httpStatusCode: http.StatusBadGateway,
httpStatusCode: http.StatusBadRequest,
releaseName: "test",
releaseNamespace: "test",
},
{
name: "Non exist release rollback should return revision not found error",
error: actions.ErrReleaseRevisionNotFound,
body: `{"name": "test", "namespace":"test", "version":1}`,
expectedResponse: `{"error":"Failed to rollback helm releases: revision not found for provided release"}`,
expectedResponse: `{"error":"Failed to rollback helm release: revision not found for provided release"}`,
httpStatusCode: http.StatusNotFound,
expectedContentType: "application/json",
releaseName: "test",
Expand Down Expand Up @@ -661,7 +662,7 @@ func TestHelmHandlers_HandleHelmUpgradeRelease(t *testing.T) {
name: "Invalid chart path upgrade release test",
error: errors.New("Chart path is invalid"),
expectedResponse: `{"error":"Failed to upgrade helm release: Chart path is invalid"}`,
httpStatusCode: http.StatusBadGateway,
httpStatusCode: http.StatusBadRequest,
expectedContentType: "application/json",
requestBody: `{"name":"test", "namespace": "test-namespace", "version": 1}`,
releaseName: "test",
Expand All @@ -680,7 +681,7 @@ func TestHelmHandlers_HandleHelmUpgradeRelease(t *testing.T) {
},
{
name: "Upgrade of non exist release should return no revision found error",
expectedResponse: `{"error":"Failed to rollback helm releases: revision not found for provided release"}`,
expectedResponse: `{"error":"Failed to upgrade helm release: revision not found for provided release"}`,
release: &fakeRelease,
expectedContentType: "application/json",
error: actions.ErrReleaseRevisionNotFound,
Expand Down Expand Up @@ -915,7 +916,7 @@ func TestHelmHandlers_HandleHelmInstallAsync(t *testing.T) {
name: "Error occurred",
expectedResponse: `{"error":"Failed to install helm chart: Chart path is invalid"}`,
error: errors.New("Chart path is invalid"),
httpStatusCode: http.StatusBadGateway,
httpStatusCode: http.StatusBadRequest,
},
{
name: "Successful install returns release info in JSON format",
Expand Down Expand Up @@ -962,7 +963,7 @@ func TestHelmHandlers_HandleHelmUpgradeReleaseAsync(t *testing.T) {
name: "Invalid chart path upgrade release test",
error: errors.New("Chart path is invalid"),
expectedResponse: `{"error":"Failed to upgrade helm release: Chart path is invalid"}`,
httpStatusCode: http.StatusBadGateway,
httpStatusCode: http.StatusBadRequest,
expectedContentType: "application/json",
requestBody: `{"name":"test", "namespace": "test-namespace", "version": 1}`,
releaseName: "test",
Expand All @@ -981,7 +982,7 @@ func TestHelmHandlers_HandleHelmUpgradeReleaseAsync(t *testing.T) {
},
{
name: "Upgrade of non exist release should return no revision found error",
expectedResponse: `{"error":"Failed to rollback helm releases: revision not found for provided release"}`,
expectedResponse: `{"error":"Failed to upgrade helm release: revision not found for provided release"}`,
secret: &fakeSecret,
expectedContentType: "application/json",
error: actions.ErrReleaseRevisionNotFound,
Expand Down Expand Up @@ -1029,7 +1030,7 @@ func TestHelmHandlers_HandleHelmUnInstallAsync(t *testing.T) {
name: "Error occurred",
expectedResponse: `{"error":"Failed to install helm chart: Chart path is invalid"}`,
error: errors.New("Chart path is invalid"),
httpStatusCode: http.StatusBadGateway,
httpStatusCode: http.StatusBadRequest,
},
{
name: "Successful uninstall returns secret info",
Expand Down Expand Up @@ -1073,7 +1074,7 @@ func TestHelmHandlers_HandleHelmInstallAsyncNoRepo(t *testing.T) {
name: "Invalid JSON request",
requestBody: `{invalid}`,
expectedResponse: `{"error":"Failed to parse request: invalid character 'i' looking for beginning of object key string"}`,
httpStatusCode: http.StatusBadGateway,
httpStatusCode: http.StatusBadRequest,
},
{
name: "Error occurred during chart install from URL",
Expand Down Expand Up @@ -1112,3 +1113,29 @@ func TestHelmHandlers_HandleHelmInstallAsyncNoRepo(t *testing.T) {
})
}
}

func TestDetermineErrorStatusCode(t *testing.T) {
tests := []struct {
name string
err error
expected int
}{
{"actions ErrReleaseNotFound", actions.ErrReleaseNotFound, http.StatusNotFound},
{"driver ErrReleaseNotFound", driver.ErrReleaseNotFound, http.StatusNotFound},
{"actions ErrReleaseRevisionNotFound", actions.ErrReleaseRevisionNotFound, http.StatusNotFound},
{"driver ErrNoDeployedReleases", driver.ErrNoDeployedReleases, http.StatusNotFound},
{"wrapped driver ErrReleaseNotFound", fmt.Errorf("wrap: %w", driver.ErrReleaseNotFound), http.StatusNotFound},
{"invalid chart URL", fmt.Errorf("invalid chart URL: foo"), http.StatusBadRequest},
{"Chart path is invalid", errors.New("Chart path is invalid"), http.StatusBadRequest},
{"invalid revision", errors.New("Revision no. should be more than 0"), http.StatusBadRequest},
{"unknown error", errors.New("something broke"), http.StatusBadGateway},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
got := determineErrorStatusCode(tt.err)
if got != tt.expected {
t.Errorf("determineErrorStatusCode() = %d, want %d", got, tt.expected)
}
})
}
}
65 changes: 31 additions & 34 deletions pkg/helm/handlers/handlers.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,14 +2,17 @@ package handlers

import (
"encoding/json"
"errors"
"fmt"
"net/http"
"strconv"
"strings"

"helm.sh/helm/v4/pkg/action"
chart "helm.sh/helm/v4/pkg/chart/v2"
releasecommon "helm.sh/helm/v4/pkg/release"
releasev1 "helm.sh/helm/v4/pkg/release/v1"
"helm.sh/helm/v4/pkg/storage/driver"
kv1 "k8s.io/api/core/v1"
"k8s.io/client-go/dynamic"
"k8s.io/client-go/rest"
Expand Down Expand Up @@ -79,6 +82,24 @@ type helmHandlers struct {
newProxy func(bearerToken string) (chartproxy.Proxy, error)
}

func determineErrorStatusCode(err error) int {
if errors.Is(err, driver.ErrReleaseNotFound) ||
errors.Is(err, actions.ErrReleaseNotFound) ||
errors.Is(err, actions.ErrReleaseRevisionNotFound) ||
errors.Is(err, driver.ErrNoDeployedReleases) {
return http.StatusNotFound
}

errMsg := err.Error()
if strings.Contains(errMsg, "invalid chart URL") ||
strings.Contains(errMsg, "Chart path is invalid") ||
strings.Contains(errMsg, "Revision no. should be more than 0") {
return http.StatusBadRequest
}

return http.StatusBadGateway
}
Comment thread
coderabbitai[bot] marked this conversation as resolved.

func (h *helmHandlers) restConfig(bearerToken string) *rest.Config {
return &rest.Config{
Host: h.ApiServerHost,
Expand Down Expand Up @@ -143,7 +164,7 @@ func (h *helmHandlers) HandleHelmInstallAsync(user *auth.User, w http.ResponseWr

err := json.NewDecoder(r.Body).Decode(&req)
if err != nil {
serverutils.SendResponse(w, http.StatusBadGateway, serverutils.ApiError{Err: fmt.Sprintf("Failed to parse request: %v", err)})
serverutils.SendResponse(w, http.StatusBadRequest, serverutils.ApiError{Err: fmt.Sprintf("Failed to parse request: %v", err)})
return
}

Expand Down Expand Up @@ -171,7 +192,7 @@ func (h *helmHandlers) HandleHelmInstallAsync(user *auth.User, w http.ResponseWr

resp, err := h.installChartAsync(namespace, req.Name, req.ChartUrl, req.Values, conf, handlerClients.DynamicClient, handlerClients.CoreClient, true, req.IndexEntry)
if err != nil {
serverutils.SendResponse(w, http.StatusBadGateway, serverutils.ApiError{Err: fmt.Sprintf("Failed to install helm chart: %v", err)})
serverutils.SendResponse(w, determineErrorStatusCode(err), serverutils.ApiError{Err: fmt.Sprintf("Failed to install helm chart: %v", err)})
return
}
serverutils.SendResponse(w, http.StatusCreated, resp)
Expand Down Expand Up @@ -212,7 +233,7 @@ func (h *helmHandlers) HandleGetRelease(user *auth.User, w http.ResponseWriter,
conf := h.getActionConfigurations(h.ApiServerHost, ns, user.Token, &h.Transport)
release, err := h.getRelease(chartName, conf)
if err != nil {
serverutils.SendResponse(w, http.StatusBadGateway, serverutils.ApiError{Err: fmt.Sprintf("Failed to find helm release: %v", err)})
serverutils.SendResponse(w, determineErrorStatusCode(err), serverutils.ApiError{Err: fmt.Sprintf("Failed to get helm release: %v", err)})
return
}
w.Header().Set("Content-Type", "application/json")
Expand Down Expand Up @@ -281,11 +302,7 @@ func (h *helmHandlers) HandleUpgradeRelease(user *auth.User, w http.ResponseWrit
}
resp, err := h.upgradeRelease(req.Namespace, req.Name, req.ChartUrl, req.Values, conf, handlerClients.DynamicClient, handlerClients.CoreClient, false, req.IndexEntry)
if err != nil {
if err.Error() == actions.ErrReleaseRevisionNotFound.Error() {
serverutils.SendResponse(w, http.StatusNotFound, serverutils.ApiError{Err: fmt.Sprintf("Failed to rollback helm releases: %v", err)})
return
}
serverutils.SendResponse(w, http.StatusBadGateway, serverutils.ApiError{Err: fmt.Sprintf("Failed to upgrade helm release: %v", err)})
serverutils.SendResponse(w, determineErrorStatusCode(err), serverutils.ApiError{Err: fmt.Sprintf("Failed to upgrade helm release: %v", err)})
return
}

Expand All @@ -311,11 +328,7 @@ func (h *helmHandlers) HandleUpgradeReleaseAsync(user *auth.User, w http.Respons
}
resp, err := h.upgradeReleaseAsync(req.Namespace, req.Name, req.ChartUrl, req.Values, conf, handlerClients.DynamicClient, handlerClients.CoreClient, false, req.IndexEntry, req.BasicAuthSecretName)
if err != nil {
if err.Error() == actions.ErrReleaseRevisionNotFound.Error() {
serverutils.SendResponse(w, http.StatusNotFound, serverutils.ApiError{Err: fmt.Sprintf("Failed to rollback helm releases: %v", err)})
return
}
serverutils.SendResponse(w, http.StatusBadGateway, serverutils.ApiError{Err: fmt.Sprintf("Failed to upgrade helm release: %v", err)})
serverutils.SendResponse(w, determineErrorStatusCode(err), serverutils.ApiError{Err: fmt.Sprintf("Failed to upgrade helm release: %v", err)})
return
}
serverutils.SendResponse(w, http.StatusCreated, resp)
Expand All @@ -329,11 +342,7 @@ func (h *helmHandlers) HandleUninstallRelease(user *auth.User, w http.ResponseWr
conf := h.getActionConfigurations(h.ApiServerHost, ns, user.Token, &h.Transport)
resp, err := h.uninstallRelease(rel, conf)
if err != nil {
if err.Error() == actions.ErrReleaseNotFound.Error() {
serverutils.SendResponse(w, http.StatusNotFound, serverutils.ApiError{Err: fmt.Sprintf("Failed to uninstall helm release: %v", err)})
return
}
serverutils.SendResponse(w, http.StatusBadGateway, serverutils.ApiError{Err: fmt.Sprintf("Failed to uninstall helm release: %v", err)})
serverutils.SendResponse(w, determineErrorStatusCode(err), serverutils.ApiError{Err: fmt.Sprintf("Failed to uninstall helm release: %v", err)})
return
}
w.Header().Set("Content-Type", "application/json")
Expand All @@ -346,18 +355,14 @@ func (h *helmHandlers) HandleRollbackRelease(user *auth.User, w http.ResponseWri

err := json.NewDecoder(r.Body).Decode(&req)
if err != nil {
serverutils.SendResponse(w, http.StatusBadGateway, serverutils.ApiError{Err: fmt.Sprintf("Failed to parse request: %v", err)})
serverutils.SendResponse(w, http.StatusBadRequest, serverutils.ApiError{Err: fmt.Sprintf("Failed to parse request: %v", err)})
return
}

conf := h.getActionConfigurations(h.ApiServerHost, req.Namespace, user.Token, &h.Transport)
rel, err := h.rollbackRelease(req.Name, req.Version, conf)
if err != nil {
if err.Error() == actions.ErrReleaseRevisionNotFound.Error() {
serverutils.SendResponse(w, http.StatusNotFound, serverutils.ApiError{Err: fmt.Sprintf("Failed to rollback helm releases: %v", err)})
return
}
serverutils.SendResponse(w, http.StatusBadGateway, serverutils.ApiError{Err: fmt.Sprintf("Failed to rollback helm releases: %v", err)})
serverutils.SendResponse(w, determineErrorStatusCode(err), serverutils.ApiError{Err: fmt.Sprintf("Failed to rollback helm release: %v", err)})
return
}

Expand All @@ -373,11 +378,7 @@ func (h *helmHandlers) HandleGetReleaseHistory(user *auth.User, w http.ResponseW
conf := h.getActionConfigurations(h.ApiServerHost, ns, user.Token, &h.Transport)
rels, err := h.getReleaseHistory(name, conf)
if err != nil {
if err.Error() == actions.ErrReleaseNotFound.Error() {
serverutils.SendResponse(w, http.StatusNotFound, serverutils.ApiError{Err: fmt.Sprintf("Failed to list helm release history: %v", err)})
return
}
serverutils.SendResponse(w, http.StatusBadGateway, serverutils.ApiError{Err: fmt.Sprintf("Failed to list helm release history: %v", err)})
serverutils.SendResponse(w, determineErrorStatusCode(err), serverutils.ApiError{Err: fmt.Sprintf("Failed to list helm release history: %v", err)})
return
}
res, _ := json.Marshal(rels)
Expand Down Expand Up @@ -440,11 +441,7 @@ func (h *helmHandlers) HandleUninstallReleaseAsync(user *auth.User, w http.Respo
}
err = h.uninstallReleaseAsync(rel, ns, version, conf, handlerClients.CoreClient)
if err != nil {
if err.Error() == actions.ErrReleaseNotFound.Error() {
serverutils.SendResponse(w, http.StatusNotFound, serverutils.ApiError{Err: fmt.Sprintf("Failed to uninstall helm release: %v", err)})
return
}
serverutils.SendResponse(w, http.StatusBadGateway, serverutils.ApiError{Err: fmt.Sprintf("Failed to uninstall helm release: %v", err)})
serverutils.SendResponse(w, determineErrorStatusCode(err), serverutils.ApiError{Err: fmt.Sprintf("Failed to uninstall helm release: %v", err)})
return
}
w.WriteHeader(http.StatusNoContent)
Expand Down