Skip to content

Commit f236f02

Browse files
fix: Handling all cases for 5xx (#5100)
* make ConvertToApiError a generic to handle to handle grpc err as well and return apierror * ConvertToApiError applied to service layer for parsing err from kubelink * code review fix * fix
1 parent 84b689c commit f236f02

File tree

13 files changed

+123
-9
lines changed

13 files changed

+123
-9
lines changed

api/helm-app/HelmAppRestHandler.go

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ import (
88
service2 "github.com/devtron-labs/devtron/api/helm-app/service"
99
"github.com/devtron-labs/devtron/pkg/appStore/installedApp/service"
1010
"github.com/devtron-labs/devtron/pkg/appStore/installedApp/service/EAMode"
11+
clientErrors "github.com/devtron-labs/devtron/pkg/errors"
1112
"net/http"
1213
"strconv"
1314
"strings"
@@ -119,6 +120,10 @@ func (handler *HelmAppRestHandlerImpl) GetApplicationDetail(w http.ResponseWrite
119120
//RBAC enforcer Ends
120121
appdetail, err := handler.helmAppService.GetApplicationDetail(context.Background(), appIdentifier)
121122
if err != nil {
123+
apiError := clientErrors.ConvertToApiError(err)
124+
if apiError != nil {
125+
err = apiError
126+
}
122127
common.WriteJsonResp(w, err, nil, http.StatusInternalServerError)
123128
return
124129
}
@@ -221,6 +226,10 @@ func (handler *HelmAppRestHandlerImpl) GetReleaseInfo(w http.ResponseWriter, r *
221226
//RBAC enforcer Ends
222227
releaseInfo, err := handler.helmAppService.GetValuesYaml(r.Context(), appIdentifier)
223228
if err != nil {
229+
apiError := clientErrors.ConvertToApiError(err)
230+
if apiError != nil {
231+
err = apiError
232+
}
224233
common.WriteJsonResp(w, err, nil, http.StatusInternalServerError)
225234
return
226235
}
@@ -264,6 +273,10 @@ func (handler *HelmAppRestHandlerImpl) GetDesiredManifest(w http.ResponseWriter,
264273
//RBAC enforcer Ends
265274
res, err := handler.helmAppService.GetDesiredManifest(r.Context(), appIdentifier, desiredManifestRequest.Resource)
266275
if err != nil {
276+
apiError := clientErrors.ConvertToApiError(err)
277+
if apiError != nil {
278+
err = apiError
279+
}
267280
common.WriteJsonResp(w, err, nil, http.StatusInternalServerError)
268281
return
269282
}
@@ -321,6 +334,10 @@ func (handler *HelmAppRestHandlerImpl) DeleteApplication(w http.ResponseWriter,
321334
res, err = handler.helmAppService.DeleteApplication(r.Context(), appIdentifier)
322335
}
323336
if err != nil {
337+
apiError := clientErrors.ConvertToApiError(err)
338+
if apiError != nil {
339+
err = apiError
340+
}
324341
common.WriteJsonResp(w, err, nil, http.StatusInternalServerError)
325342
return
326343
}

api/k8s/application/k8sApplicationRestHandler.go

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@ import (
1919
"github.com/devtron-labs/devtron/pkg/auth/authorisation/casbin"
2020
"github.com/devtron-labs/devtron/pkg/auth/user"
2121
"github.com/devtron-labs/devtron/pkg/cluster"
22+
clientErrors "github.com/devtron-labs/devtron/pkg/errors"
2223
"github.com/devtron-labs/devtron/pkg/k8s"
2324
application2 "github.com/devtron-labs/devtron/pkg/k8s/application"
2425
bean2 "github.com/devtron-labs/devtron/pkg/k8s/application/bean"
@@ -275,6 +276,10 @@ func (handler *K8sApplicationRestHandlerImpl) GetHostUrlsByBatch(w http.Response
275276
//RBAC enforcer Ends
276277
appDetail, err := handler.helmAppService.GetApplicationDetail(r.Context(), appIdentifier)
277278
if err != nil {
279+
apiError := clientErrors.ConvertToApiError(err)
280+
if apiError != nil {
281+
err = apiError
282+
}
278283
common.WriteJsonResp(w, err, nil, http.StatusInternalServerError)
279284
return
280285
}

internal/errors/bean.go

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -42,6 +42,8 @@ func (r *ClientStatusCode) GetHttpStatusCodeForGivenGrpcCode() int {
4242
return http.StatusRequestTimeout
4343
case codes.Canceled:
4444
return constants.HttpClientSideTimeout
45+
case codes.PermissionDenied:
46+
return http.StatusUnprocessableEntity
4547
default:
4648
return http.StatusInternalServerError
4749
}

pkg/appStore/installedApp/service/EAMode/EAModeDeploymentService.go

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@ import (
1111
util2 "github.com/devtron-labs/devtron/pkg/appStore/util"
1212
commonBean "github.com/devtron-labs/devtron/pkg/deployment/gitOps/common/bean"
1313
validationBean "github.com/devtron-labs/devtron/pkg/deployment/gitOps/validation/bean"
14+
clientErrors "github.com/devtron-labs/devtron/pkg/errors"
1415
"net/http"
1516
"time"
1617

@@ -128,6 +129,10 @@ func (impl *EAModeDeploymentServiceImpl) InstallApp(installAppVersionRequest *ap
128129

129130
_, err = impl.helmAppService.InstallRelease(ctx, installAppVersionRequest.ClusterId, installReleaseRequest)
130131
if err != nil {
132+
apiError := clientErrors.ConvertToApiError(err)
133+
if apiError != nil {
134+
err = apiError
135+
}
131136
return installAppVersionRequest, err
132137
}
133138
return installAppVersionRequest, nil
@@ -157,6 +162,10 @@ func (impl *EAModeDeploymentServiceImpl) DeleteInstalledApp(ctx context.Context,
157162
deleteResponse, err := impl.helmAppService.DeleteApplication(ctx, appIdentifier)
158163
if err != nil {
159164
impl.Logger.Errorw("error in deleting helm application", "error", err, "appIdentifier", appIdentifier)
165+
apiError := clientErrors.ConvertToApiError(err)
166+
if apiError != nil {
167+
err = apiError
168+
}
160169
return err
161170
}
162171
if deleteResponse == nil || !deleteResponse.GetSuccess() {
@@ -182,6 +191,10 @@ func (impl *EAModeDeploymentServiceImpl) RollbackRelease(ctx context.Context, in
182191
helmAppDeploymentDetail, err := impl.helmAppService.GetDeploymentDetail(ctx, helmAppIdeltifier, deploymentVersion)
183192
if err != nil {
184193
impl.Logger.Errorw("Error in getting helm application deployment detail", "err", err)
194+
apiError := clientErrors.ConvertToApiError(err)
195+
if apiError != nil {
196+
err = apiError
197+
}
185198
return installedApp, false, err
186199
}
187200
valuesYamlJson := helmAppDeploymentDetail.GetValuesYaml()
@@ -208,6 +221,12 @@ func (impl *EAModeDeploymentServiceImpl) GetDeploymentHistory(ctx context.Contex
208221
ReleaseName: installedApp.AppName,
209222
}
210223
history, err := impl.helmAppService.GetDeploymentHistory(ctx, helmAppIdentifier)
224+
if err != nil {
225+
apiError := clientErrors.ConvertToApiError(err)
226+
if apiError != nil {
227+
err = apiError
228+
}
229+
}
211230
return history, err
212231
}
213232

@@ -236,6 +255,10 @@ func (impl *EAModeDeploymentServiceImpl) GetDeploymentHistoryInfo(ctx context.Co
236255
span.End()
237256
if err != nil {
238257
impl.Logger.Errorw("error in getting deployment detail", "err", err)
258+
apiError := clientErrors.ConvertToApiError(err)
259+
if apiError != nil {
260+
err = apiError
261+
}
239262
return nil, err
240263
}
241264

pkg/appStore/installedApp/service/FullMode/deployment/InstalledAppGitOpsService.go

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@ import (
1616
"github.com/google/go-github/github"
1717
"github.com/microsoft/azure-devops-go-api/azuredevops"
1818
"github.com/xanzy/go-gitlab"
19+
"strconv"
1920

2021
//"github.com/xanzy/go-gitlab"
2122
"net/http"
@@ -297,7 +298,8 @@ func (impl *FullModeDeploymentServiceImpl) getGitCommitConfig(installAppVersionR
297298
return nil, err
298299
}
299300
if util.IsErrNoRows(err) {
300-
return nil, fmt.Errorf("Invalid request! No InstalledApp found.")
301+
apiErr := &util.ApiError{HttpStatusCode: http.StatusNotFound, Code: strconv.Itoa(http.StatusNotFound), InternalMessage: "Invalid request! No InstalledApp found.", UserMessage: "Invalid request! No InstalledApp found."}
302+
return nil, apiErr
301303
}
302304
installAppVersionRequest.GitOpsRepoURL = InstalledApp.GitOpsRepoUrl
303305
}

pkg/appStore/installedApp/service/FullMode/resource/NotesService.go

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ import (
55
"fmt"
66
"github.com/devtron-labs/devtron/api/helm-app/gRPC"
77
"github.com/devtron-labs/devtron/internal/util"
8+
clientErrors "github.com/devtron-labs/devtron/pkg/errors"
89
"github.com/go-pg/pg"
910
"net/http"
1011
"regexp"
@@ -106,6 +107,10 @@ func (impl *InstalledAppResourceServiceImpl) findNotesForArgoApplication(install
106107
notes, err = impl.helmAppService.GetNotes(context.Background(), installReleaseRequest)
107108
if err != nil {
108109
impl.logger.Errorw("error in fetching notes", "err", err)
110+
apiError := clientErrors.ConvertToApiError(err)
111+
if apiError != nil {
112+
err = apiError
113+
}
109114
return notes, err
110115
}
111116
_, err = impl.updateNotesForInstalledApp(installedAppId, notes)

pkg/errors/utils.go

Lines changed: 26 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ package errors
22

33
import (
44
util2 "github.com/devtron-labs/devtron/internal/util"
5+
"google.golang.org/grpc/status"
56
"net/http"
67
"strconv"
78
"strings"
@@ -15,6 +16,7 @@ const (
1516
NamespaceNotFoundErrorMsg = "namespace not found"
1617
InvalidValueErrorMsg = "invalid value in manifest"
1718
OperationInProgressErrorMsg = "another operation (install/upgrade/rollback) is in progress"
19+
ForbiddenErrMsg = "forbidden"
1820
)
1921

2022
var errorHttpStatusCodeMap = map[string]int{
@@ -24,18 +26,34 @@ var errorHttpStatusCodeMap = map[string]int{
2426
ArrayStringMismatchErrorMsg: http.StatusFailedDependency,
2527
InvalidValueErrorMsg: http.StatusFailedDependency,
2628
OperationInProgressErrorMsg: http.StatusConflict,
29+
//forbidden error from kubernetes would not be a parameter for us to mark a user forbidden to that resource or not,
30+
//since this is not rbac from devtron, hence map it to StatusUnprocessableEntity
31+
ForbiddenErrMsg: http.StatusUnprocessableEntity,
2732
}
2833

2934
func ConvertToApiError(err error) *util2.ApiError {
30-
for errMsg, statusCode := range errorHttpStatusCodeMap {
31-
if strings.Contains(err.Error(), errMsg) {
32-
return &util2.ApiError{
33-
InternalMessage: err.Error(),
34-
UserMessage: err.Error(),
35-
HttpStatusCode: statusCode,
36-
Code: strconv.Itoa(statusCode),
35+
var apiError *util2.ApiError
36+
if _, ok := status.FromError(err); ok {
37+
clientCode, _ := util2.GetClientDetailedError(err)
38+
httpStatusCode := clientCode.GetHttpStatusCodeForGivenGrpcCode()
39+
apiError = &util2.ApiError{
40+
HttpStatusCode: httpStatusCode,
41+
Code: strconv.Itoa(httpStatusCode),
42+
InternalMessage: err.Error(),
43+
UserMessage: err.Error(),
44+
}
45+
} else {
46+
for errMsg, statusCode := range errorHttpStatusCodeMap {
47+
if strings.Contains(err.Error(), errMsg) {
48+
apiError = &util2.ApiError{
49+
InternalMessage: err.Error(),
50+
UserMessage: err.Error(),
51+
HttpStatusCode: statusCode,
52+
Code: strconv.Itoa(statusCode),
53+
}
3754
}
3855
}
3956
}
40-
return nil
57+
58+
return apiError
4159
}

pkg/k8s/application/k8sApplicationService.go

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ import (
99
"github.com/devtron-labs/devtron/api/helm-app/gRPC"
1010
client "github.com/devtron-labs/devtron/api/helm-app/service"
1111
"github.com/devtron-labs/devtron/pkg/auth/authorisation/casbin"
12+
clientErrors "github.com/devtron-labs/devtron/pkg/errors"
1213
"io"
1314
v1 "k8s.io/client-go/kubernetes/typed/core/v1"
1415
"net/http"
@@ -428,6 +429,10 @@ func (impl *K8sApplicationServiceImpl) ValidateResourceRequest(ctx context.Conte
428429
app, err := impl.helmAppService.GetApplicationDetail(ctx, appIdentifier)
429430
if err != nil {
430431
impl.logger.Errorw("error in getting app detail", "err", err, "appDetails", appIdentifier)
432+
apiError := clientErrors.ConvertToApiError(err)
433+
if apiError != nil {
434+
err = apiError
435+
}
431436
return false, err
432437
}
433438
valid := false
@@ -1019,6 +1024,10 @@ func (impl *K8sApplicationServiceImpl) RecreateResource(ctx context.Context, req
10191024
manifestRes, err := impl.helmAppService.GetDesiredManifest(ctx, request.AppIdentifier, resourceIdentifier)
10201025
if err != nil {
10211026
impl.logger.Errorw("error in getting desired manifest for validation", "err", err)
1027+
apiError := clientErrors.ConvertToApiError(err)
1028+
if apiError != nil {
1029+
err = apiError
1030+
}
10221031
return nil, err
10231032
}
10241033
manifest, manifestOk := manifestRes.GetManifestOk()

pkg/module/ModuleService.go

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@ import (
2424
"github.com/devtron-labs/devtron/api/helm-app/gRPC"
2525
client "github.com/devtron-labs/devtron/api/helm-app/service"
2626
"github.com/devtron-labs/devtron/internal/sql/repository/security"
27+
clientErrors "github.com/devtron-labs/devtron/pkg/errors"
2728
moduleRepo "github.com/devtron-labs/devtron/pkg/module/repo"
2829
moduleUtil "github.com/devtron-labs/devtron/pkg/module/util"
2930
"github.com/devtron-labs/devtron/pkg/server"
@@ -225,6 +226,10 @@ func (impl ModuleServiceImpl) handleModuleNotFoundStatus(moduleName string) (Mod
225226
releaseInfo, err := impl.helmAppService.GetValuesYaml(context.Background(), devtronHelmAppIdentifier)
226227
if err != nil {
227228
impl.logger.Errorw("Error in getting values yaml for devtron operator helm release", "moduleName", moduleName, "err", err)
229+
apiError := clientErrors.ConvertToApiError(err)
230+
if apiError != nil {
231+
err = apiError
232+
}
228233
return ModuleStatusNotInstalled, moduleType, false, err
229234
}
230235
releaseValues := releaseInfo.MergedValues
@@ -426,6 +431,10 @@ func (impl ModuleServiceImpl) HandleModuleAction(userId int32, moduleName string
426431
updateResponse, err := impl.helmAppService.UpdateApplicationWithChartInfoWithExtraValues(context.Background(), devtronHelmAppIdentifier, chartRepository, extraValues, extraValuesYamlUrl, true)
427432
if err != nil {
428433
impl.logger.Errorw("error in updating helm release ", "err", err)
434+
apiError := clientErrors.ConvertToApiError(err)
435+
if apiError != nil {
436+
err = apiError
437+
}
429438
module.Status = ModuleStatusInstallFailed
430439
impl.moduleRepository.Update(module)
431440
return nil, err

pkg/pipeline/AppDeploymentTypeChangeManager.go

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,7 @@ import (
3434
commonBean "github.com/devtron-labs/devtron/pkg/deployment/gitOps/common/bean"
3535
"github.com/devtron-labs/devtron/pkg/deployment/gitOps/config"
3636
bean3 "github.com/devtron-labs/devtron/pkg/deployment/trigger/devtronApps/bean"
37+
clientErrors "github.com/devtron-labs/devtron/pkg/errors"
3738
"github.com/devtron-labs/devtron/pkg/eventProcessor/out"
3839
bean2 "github.com/devtron-labs/devtron/pkg/eventProcessor/out/bean"
3940
"github.com/juju/errors"
@@ -762,6 +763,10 @@ func (impl *AppDeploymentTypeChangeManagerImpl) deleteHelmApp(ctx context.Contex
762763

763764
if err != nil {
764765
impl.logger.Errorw("error in deleting helm application", "error", err, "appIdentifier", appIdentifier)
766+
apiError := clientErrors.ConvertToApiError(err)
767+
if apiError != nil {
768+
err = apiError
769+
}
765770
return err
766771
}
767772

pkg/pipeline/DeploymentPipelineConfigService.go

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -48,6 +48,7 @@ import (
4848
"github.com/devtron-labs/devtron/pkg/deployment/gitOps/git"
4949
"github.com/devtron-labs/devtron/pkg/deployment/manifest/deployedAppMetrics"
5050
config2 "github.com/devtron-labs/devtron/pkg/deployment/providerConfig"
51+
clientErrors "github.com/devtron-labs/devtron/pkg/errors"
5152
"github.com/devtron-labs/devtron/pkg/eventProcessor/out"
5253
"github.com/devtron-labs/devtron/pkg/imageDigestPolicy"
5354
bean3 "github.com/devtron-labs/devtron/pkg/pipeline/bean"
@@ -890,6 +891,10 @@ func (impl *CdPipelineConfigServiceImpl) DeleteHelmTypePipelineDeploymentApp(ctx
890891
} else {
891892
if err != nil {
892893
impl.logger.Errorw("error in deleting helm application", "error", err, "appIdentifier", appIdentifier)
894+
apiError := clientErrors.ConvertToApiError(err)
895+
if apiError != nil {
896+
err = apiError
897+
}
893898
return err
894899
}
895900
if deleteResourceResponse == nil || !deleteResourceResponse.GetSuccess() {

pkg/server/ServerService.go

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@ import (
2222
"errors"
2323
"github.com/devtron-labs/devtron/api/helm-app/gRPC"
2424
client "github.com/devtron-labs/devtron/api/helm-app/service"
25+
clientErrors "github.com/devtron-labs/devtron/pkg/errors"
2526
moduleRepo "github.com/devtron-labs/devtron/pkg/module/repo"
2627
moduleUtil "github.com/devtron-labs/devtron/pkg/module/util"
2728
serverBean "github.com/devtron-labs/devtron/pkg/server/bean"
@@ -78,6 +79,10 @@ func (impl ServerServiceImpl) GetServerInfo(showServerStatus bool) (*serverBean.
7879
devtronAppDetail, err := impl.helmAppService.GetApplicationDetail(context.Background(), devtronHelmAppIdentifier)
7980
if err != nil {
8081
impl.logger.Errorw("error in getting devtron helm app release status ", "err", err)
82+
apiError := clientErrors.ConvertToApiError(err)
83+
if apiError != nil {
84+
err = apiError
85+
}
8186
return nil, err
8287
}
8388

@@ -153,6 +158,10 @@ func (impl ServerServiceImpl) HandleServerAction(userId int32, serverActionReque
153158
updateResponse, err := impl.helmAppService.UpdateApplicationWithChartInfoWithExtraValues(context.Background(), devtronHelmAppIdentifier, chartRepository, extraValues, extraValuesYamlUrl, true)
154159
if err != nil {
155160
impl.logger.Errorw("error in updating helm release ", "err", err)
161+
apiError := clientErrors.ConvertToApiError(err)
162+
if apiError != nil {
163+
err = apiError
164+
}
156165
return nil, err
157166
}
158167
if !updateResponse.GetSuccess() {

pkg/webhook/helm/WebhookHelmService.go

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,7 @@ import (
2828
bean3 "github.com/devtron-labs/devtron/pkg/attributes/bean"
2929
"github.com/devtron-labs/devtron/pkg/chartRepo"
3030
"github.com/devtron-labs/devtron/pkg/cluster"
31+
clientErrors "github.com/devtron-labs/devtron/pkg/errors"
3132
"github.com/go-pg/pg"
3233
"go.uber.org/zap"
3334
"net/http"
@@ -154,6 +155,10 @@ func (impl WebhookHelmServiceImpl) CreateOrUpdateHelmApplication(ctx context.Con
154155
res, err := impl.helmAppService.InstallRelease(ctx, clusterId, installReleaseRequest)
155156
if err != nil {
156157
impl.logger.Errorw("Error in installing helm release", "appIdentifier", appIdentifier, "err", err)
158+
apiError := clientErrors.ConvertToApiError(err)
159+
if apiError != nil {
160+
err = apiError
161+
}
157162
return nil, common.InternalServerError, err.Error(), http.StatusInternalServerError
158163
}
159164
if !res.GetSuccess() {

0 commit comments

Comments
 (0)