From eac04ac049f551a3bfe4cd7b07befa5d18e78c41 Mon Sep 17 00:00:00 2001 From: Prakash Kumar Date: Mon, 13 May 2024 12:29:25 +0530 Subject: [PATCH 1/4] make ConvertToApiError a generic to handle to handle grpc err as well and return apierror --- api/helm-app/service/HelmAppService.go | 5 ++++ internal/errors/bean.go | 2 ++ pkg/errors/utils.go | 32 +++++++++++++++++++------- 3 files changed, 31 insertions(+), 8 deletions(-) diff --git a/api/helm-app/service/HelmAppService.go b/api/helm-app/service/HelmAppService.go index 887736f98e..640df8da3a 100644 --- a/api/helm-app/service/HelmAppService.go +++ b/api/helm-app/service/HelmAppService.go @@ -10,6 +10,7 @@ import ( "github.com/devtron-labs/devtron/api/helm-app/models" "github.com/devtron-labs/devtron/internal/constants" repository2 "github.com/devtron-labs/devtron/internal/sql/repository/dockerRegistry" + clientErrors "github.com/devtron-labs/devtron/pkg/errors" "github.com/go-pg/pg" "net/http" "reflect" @@ -319,6 +320,10 @@ func (impl *HelmAppServiceImpl) getApplicationDetail(ctx context.Context, app *A appdetail, err := impl.helmAppClient.GetAppDetail(ctx, req) if err != nil { impl.logger.Errorw("error in fetching app detail", "err", err) + apiError := clientErrors.ConvertToApiError(err) + if apiError != nil { + return nil, apiError + } return nil, err } diff --git a/internal/errors/bean.go b/internal/errors/bean.go index 55d3726b54..3e30b072a8 100644 --- a/internal/errors/bean.go +++ b/internal/errors/bean.go @@ -42,6 +42,8 @@ func (r *ClientStatusCode) GetHttpStatusCodeForGivenGrpcCode() int { return http.StatusRequestTimeout case codes.Canceled: return constants.HttpClientSideTimeout + case codes.PermissionDenied: + return http.StatusUnprocessableEntity default: return http.StatusInternalServerError } diff --git a/pkg/errors/utils.go b/pkg/errors/utils.go index 9c403f1325..ad41efe92d 100644 --- a/pkg/errors/utils.go +++ b/pkg/errors/utils.go @@ -2,6 +2,7 @@ package errors import ( util2 "github.com/devtron-labs/devtron/internal/util" + "google.golang.org/grpc/status" "net/http" "strconv" "strings" @@ -15,6 +16,7 @@ const ( NamespaceNotFoundErrorMsg = "namespace not found" InvalidValueErrorMsg = "invalid value in manifest" OperationInProgressErrorMsg = "another operation (install/upgrade/rollback) is in progress" + ForbiddenErrMsg = "forbidden" ) var errorHttpStatusCodeMap = map[string]int{ @@ -24,18 +26,32 @@ var errorHttpStatusCodeMap = map[string]int{ ArrayStringMismatchErrorMsg: http.StatusFailedDependency, InvalidValueErrorMsg: http.StatusFailedDependency, OperationInProgressErrorMsg: http.StatusConflict, + ForbiddenErrMsg: http.StatusForbidden, } func ConvertToApiError(err error) *util2.ApiError { - for errMsg, statusCode := range errorHttpStatusCodeMap { - if strings.Contains(err.Error(), errMsg) { - return &util2.ApiError{ - InternalMessage: err.Error(), - UserMessage: err.Error(), - HttpStatusCode: statusCode, - Code: strconv.Itoa(statusCode), + var apiError *util2.ApiError + if _, ok := status.FromError(err); ok { + clientCode, errMsg := util2.GetClientDetailedError(err) + httpStatusCode := clientCode.GetHttpStatusCodeForGivenGrpcCode() + apiError = &util2.ApiError{ + HttpStatusCode: httpStatusCode, + Code: strconv.Itoa(httpStatusCode), + InternalMessage: errMsg, + UserMessage: errMsg, + } + } else { + for errMsg, statusCode := range errorHttpStatusCodeMap { + if strings.Contains(err.Error(), errMsg) { + apiError = &util2.ApiError{ + InternalMessage: err.Error(), + UserMessage: err.Error(), + HttpStatusCode: statusCode, + Code: strconv.Itoa(statusCode), + } } } } - return nil + + return apiError } From 09b29abe69cf2d8331dc619ca5ee620ea21bedb7 Mon Sep 17 00:00:00 2001 From: Prakash Kumar Date: Mon, 13 May 2024 13:57:10 +0530 Subject: [PATCH 2/4] ConvertToApiError applied to service layer for parsing err from kubelink --- api/helm-app/HelmAppRestHandler.go | 17 ++++++++++++++ api/helm-app/service/HelmAppService.go | 5 ---- .../application/k8sApplicationRestHandler.go | 5 ++++ .../service/EAMode/EAModeDeploymentService.go | 23 +++++++++++++++++++ .../deployment/InstalledAppGitOpsService.go | 4 +++- .../service/FullMode/resource/NotesService.go | 5 ++++ pkg/errors/utils.go | 4 +++- pkg/k8s/application/k8sApplicationService.go | 9 ++++++++ pkg/module/ModuleService.go | 9 ++++++++ .../AppDeploymentTypeChangeManager.go | 5 ++++ .../DeploymentPipelineConfigService.go | 5 ++++ pkg/server/ServerService.go | 9 ++++++++ pkg/webhook/helm/WebhookHelmService.go | 5 ++++ 13 files changed, 98 insertions(+), 7 deletions(-) diff --git a/api/helm-app/HelmAppRestHandler.go b/api/helm-app/HelmAppRestHandler.go index 6990ad279e..14a7940717 100644 --- a/api/helm-app/HelmAppRestHandler.go +++ b/api/helm-app/HelmAppRestHandler.go @@ -8,6 +8,7 @@ import ( service2 "github.com/devtron-labs/devtron/api/helm-app/service" "github.com/devtron-labs/devtron/pkg/appStore/installedApp/service" "github.com/devtron-labs/devtron/pkg/appStore/installedApp/service/EAMode" + clientErrors "github.com/devtron-labs/devtron/pkg/errors" "net/http" "strconv" "strings" @@ -119,6 +120,10 @@ func (handler *HelmAppRestHandlerImpl) GetApplicationDetail(w http.ResponseWrite //RBAC enforcer Ends appdetail, err := handler.helmAppService.GetApplicationDetail(context.Background(), appIdentifier) if err != nil { + apiError := clientErrors.ConvertToApiError(err) + if apiError != nil { + err = apiError + } common.WriteJsonResp(w, err, nil, http.StatusInternalServerError) return } @@ -221,6 +226,10 @@ func (handler *HelmAppRestHandlerImpl) GetReleaseInfo(w http.ResponseWriter, r * //RBAC enforcer Ends releaseInfo, err := handler.helmAppService.GetValuesYaml(r.Context(), appIdentifier) if err != nil { + apiError := clientErrors.ConvertToApiError(err) + if apiError != nil { + err = apiError + } common.WriteJsonResp(w, err, nil, http.StatusInternalServerError) return } @@ -264,6 +273,10 @@ func (handler *HelmAppRestHandlerImpl) GetDesiredManifest(w http.ResponseWriter, //RBAC enforcer Ends res, err := handler.helmAppService.GetDesiredManifest(r.Context(), appIdentifier, desiredManifestRequest.Resource) if err != nil { + apiError := clientErrors.ConvertToApiError(err) + if apiError != nil { + err = apiError + } common.WriteJsonResp(w, err, nil, http.StatusInternalServerError) return } @@ -321,6 +334,10 @@ func (handler *HelmAppRestHandlerImpl) DeleteApplication(w http.ResponseWriter, res, err = handler.helmAppService.DeleteApplication(r.Context(), appIdentifier) } if err != nil { + apiError := clientErrors.ConvertToApiError(err) + if apiError != nil { + err = apiError + } common.WriteJsonResp(w, err, nil, http.StatusInternalServerError) return } diff --git a/api/helm-app/service/HelmAppService.go b/api/helm-app/service/HelmAppService.go index 640df8da3a..887736f98e 100644 --- a/api/helm-app/service/HelmAppService.go +++ b/api/helm-app/service/HelmAppService.go @@ -10,7 +10,6 @@ import ( "github.com/devtron-labs/devtron/api/helm-app/models" "github.com/devtron-labs/devtron/internal/constants" repository2 "github.com/devtron-labs/devtron/internal/sql/repository/dockerRegistry" - clientErrors "github.com/devtron-labs/devtron/pkg/errors" "github.com/go-pg/pg" "net/http" "reflect" @@ -320,10 +319,6 @@ func (impl *HelmAppServiceImpl) getApplicationDetail(ctx context.Context, app *A appdetail, err := impl.helmAppClient.GetAppDetail(ctx, req) if err != nil { impl.logger.Errorw("error in fetching app detail", "err", err) - apiError := clientErrors.ConvertToApiError(err) - if apiError != nil { - return nil, apiError - } return nil, err } diff --git a/api/k8s/application/k8sApplicationRestHandler.go b/api/k8s/application/k8sApplicationRestHandler.go index 019c372b9d..46af4b3d1c 100644 --- a/api/k8s/application/k8sApplicationRestHandler.go +++ b/api/k8s/application/k8sApplicationRestHandler.go @@ -19,6 +19,7 @@ import ( "github.com/devtron-labs/devtron/pkg/auth/authorisation/casbin" "github.com/devtron-labs/devtron/pkg/auth/user" "github.com/devtron-labs/devtron/pkg/cluster" + clientErrors "github.com/devtron-labs/devtron/pkg/errors" "github.com/devtron-labs/devtron/pkg/k8s" application2 "github.com/devtron-labs/devtron/pkg/k8s/application" bean2 "github.com/devtron-labs/devtron/pkg/k8s/application/bean" @@ -275,6 +276,10 @@ func (handler *K8sApplicationRestHandlerImpl) GetHostUrlsByBatch(w http.Response //RBAC enforcer Ends appDetail, err := handler.helmAppService.GetApplicationDetail(r.Context(), appIdentifier) if err != nil { + apiError := clientErrors.ConvertToApiError(err) + if apiError != nil { + err = apiError + } common.WriteJsonResp(w, err, nil, http.StatusInternalServerError) return } diff --git a/pkg/appStore/installedApp/service/EAMode/EAModeDeploymentService.go b/pkg/appStore/installedApp/service/EAMode/EAModeDeploymentService.go index 932f267e78..5eb77a17e9 100644 --- a/pkg/appStore/installedApp/service/EAMode/EAModeDeploymentService.go +++ b/pkg/appStore/installedApp/service/EAMode/EAModeDeploymentService.go @@ -10,6 +10,7 @@ import ( "github.com/devtron-labs/devtron/pkg/appStore/installedApp/service/bean" commonBean "github.com/devtron-labs/devtron/pkg/deployment/gitOps/common/bean" validationBean "github.com/devtron-labs/devtron/pkg/deployment/gitOps/validation/bean" + clientErrors "github.com/devtron-labs/devtron/pkg/errors" "net/http" "time" @@ -127,6 +128,10 @@ func (impl *EAModeDeploymentServiceImpl) InstallApp(installAppVersionRequest *ap _, err = impl.helmAppService.InstallRelease(ctx, installAppVersionRequest.ClusterId, installReleaseRequest) if err != nil { + apiError := clientErrors.ConvertToApiError(err) + if apiError != nil { + err = apiError + } return installAppVersionRequest, err } return installAppVersionRequest, nil @@ -156,6 +161,10 @@ func (impl *EAModeDeploymentServiceImpl) DeleteInstalledApp(ctx context.Context, deleteResponse, err := impl.helmAppService.DeleteApplication(ctx, appIdentifier) if err != nil { impl.Logger.Errorw("error in deleting helm application", "error", err, "appIdentifier", appIdentifier) + apiError := clientErrors.ConvertToApiError(err) + if apiError != nil { + err = apiError + } return err } if deleteResponse == nil || !deleteResponse.GetSuccess() { @@ -181,6 +190,10 @@ func (impl *EAModeDeploymentServiceImpl) RollbackRelease(ctx context.Context, in helmAppDeploymentDetail, err := impl.helmAppService.GetDeploymentDetail(ctx, helmAppIdeltifier, deploymentVersion) if err != nil { impl.Logger.Errorw("Error in getting helm application deployment detail", "err", err) + apiError := clientErrors.ConvertToApiError(err) + if apiError != nil { + err = apiError + } return installedApp, false, err } valuesYamlJson := helmAppDeploymentDetail.GetValuesYaml() @@ -207,6 +220,12 @@ func (impl *EAModeDeploymentServiceImpl) GetDeploymentHistory(ctx context.Contex ReleaseName: installedApp.AppName, } history, err := impl.helmAppService.GetDeploymentHistory(ctx, helmAppIdentifier) + if err != nil { + apiError := clientErrors.ConvertToApiError(err) + if apiError != nil { + err = apiError + } + } return history, err } @@ -235,6 +254,10 @@ func (impl *EAModeDeploymentServiceImpl) GetDeploymentHistoryInfo(ctx context.Co span.End() if err != nil { impl.Logger.Errorw("error in getting deployment detail", "err", err) + apiError := clientErrors.ConvertToApiError(err) + if apiError != nil { + err = apiError + } return nil, err } diff --git a/pkg/appStore/installedApp/service/FullMode/deployment/InstalledAppGitOpsService.go b/pkg/appStore/installedApp/service/FullMode/deployment/InstalledAppGitOpsService.go index 34b5660554..f80f9f9deb 100644 --- a/pkg/appStore/installedApp/service/FullMode/deployment/InstalledAppGitOpsService.go +++ b/pkg/appStore/installedApp/service/FullMode/deployment/InstalledAppGitOpsService.go @@ -16,6 +16,7 @@ import ( "github.com/google/go-github/github" "github.com/microsoft/azure-devops-go-api/azuredevops" "github.com/xanzy/go-gitlab" + "strconv" //"github.com/xanzy/go-gitlab" "net/http" @@ -297,7 +298,8 @@ func (impl *FullModeDeploymentServiceImpl) getGitCommitConfig(installAppVersionR return nil, err } if util.IsErrNoRows(err) { - return nil, fmt.Errorf("Invalid request! No InstalledApp found.") + apiErr := &util.ApiError{HttpStatusCode: http.StatusNotFound, Code: strconv.Itoa(http.StatusNotFound), InternalMessage: "Invalid request! No InstalledApp found.", UserMessage: "Invalid request! No InstalledApp found."} + return nil, apiErr } installAppVersionRequest.GitOpsRepoURL = InstalledApp.GitOpsRepoUrl } diff --git a/pkg/appStore/installedApp/service/FullMode/resource/NotesService.go b/pkg/appStore/installedApp/service/FullMode/resource/NotesService.go index df03d62471..4feb888fa5 100644 --- a/pkg/appStore/installedApp/service/FullMode/resource/NotesService.go +++ b/pkg/appStore/installedApp/service/FullMode/resource/NotesService.go @@ -5,6 +5,7 @@ import ( "fmt" "github.com/devtron-labs/devtron/api/helm-app/gRPC" "github.com/devtron-labs/devtron/internal/util" + clientErrors "github.com/devtron-labs/devtron/pkg/errors" "github.com/go-pg/pg" "net/http" "regexp" @@ -107,6 +108,10 @@ func (impl *InstalledAppResourceServiceImpl) findNotesForArgoApplication(install notes, err = impl.helmAppService.GetNotes(context.Background(), installReleaseRequest) if err != nil { impl.logger.Errorw("error in fetching notes", "err", err) + apiError := clientErrors.ConvertToApiError(err) + if apiError != nil { + err = apiError + } return notes, appName, err } _, err = impl.updateNotesForInstalledApp(installedAppId, notes) diff --git a/pkg/errors/utils.go b/pkg/errors/utils.go index ad41efe92d..1df7db4332 100644 --- a/pkg/errors/utils.go +++ b/pkg/errors/utils.go @@ -26,7 +26,9 @@ var errorHttpStatusCodeMap = map[string]int{ ArrayStringMismatchErrorMsg: http.StatusFailedDependency, InvalidValueErrorMsg: http.StatusFailedDependency, OperationInProgressErrorMsg: http.StatusConflict, - ForbiddenErrMsg: http.StatusForbidden, + //forbidden error from kubernetes would not be a parameter for us to mark a user forbidden to that resource or not, + //since this is not rbac from devtron, hence mark map them to StatusUnprocessableEntity + ForbiddenErrMsg: http.StatusUnprocessableEntity, } func ConvertToApiError(err error) *util2.ApiError { diff --git a/pkg/k8s/application/k8sApplicationService.go b/pkg/k8s/application/k8sApplicationService.go index 8b9d092223..e0454fee8d 100644 --- a/pkg/k8s/application/k8sApplicationService.go +++ b/pkg/k8s/application/k8sApplicationService.go @@ -9,6 +9,7 @@ import ( "github.com/devtron-labs/devtron/api/helm-app/gRPC" client "github.com/devtron-labs/devtron/api/helm-app/service" "github.com/devtron-labs/devtron/pkg/auth/authorisation/casbin" + clientErrors "github.com/devtron-labs/devtron/pkg/errors" "io" v1 "k8s.io/client-go/kubernetes/typed/core/v1" "net/http" @@ -428,6 +429,10 @@ func (impl *K8sApplicationServiceImpl) ValidateResourceRequest(ctx context.Conte app, err := impl.helmAppService.GetApplicationDetail(ctx, appIdentifier) if err != nil { impl.logger.Errorw("error in getting app detail", "err", err, "appDetails", appIdentifier) + apiError := clientErrors.ConvertToApiError(err) + if apiError != nil { + err = apiError + } return false, err } valid := false @@ -1019,6 +1024,10 @@ func (impl *K8sApplicationServiceImpl) RecreateResource(ctx context.Context, req manifestRes, err := impl.helmAppService.GetDesiredManifest(ctx, request.AppIdentifier, resourceIdentifier) if err != nil { impl.logger.Errorw("error in getting desired manifest for validation", "err", err) + apiError := clientErrors.ConvertToApiError(err) + if apiError != nil { + err = apiError + } return nil, err } manifest, manifestOk := manifestRes.GetManifestOk() diff --git a/pkg/module/ModuleService.go b/pkg/module/ModuleService.go index 297888cfea..f949f72ff4 100644 --- a/pkg/module/ModuleService.go +++ b/pkg/module/ModuleService.go @@ -24,6 +24,7 @@ import ( "github.com/devtron-labs/devtron/api/helm-app/gRPC" client "github.com/devtron-labs/devtron/api/helm-app/service" "github.com/devtron-labs/devtron/internal/sql/repository/security" + clientErrors "github.com/devtron-labs/devtron/pkg/errors" moduleRepo "github.com/devtron-labs/devtron/pkg/module/repo" moduleUtil "github.com/devtron-labs/devtron/pkg/module/util" "github.com/devtron-labs/devtron/pkg/server" @@ -225,6 +226,10 @@ func (impl ModuleServiceImpl) handleModuleNotFoundStatus(moduleName string) (Mod releaseInfo, err := impl.helmAppService.GetValuesYaml(context.Background(), devtronHelmAppIdentifier) if err != nil { impl.logger.Errorw("Error in getting values yaml for devtron operator helm release", "moduleName", moduleName, "err", err) + apiError := clientErrors.ConvertToApiError(err) + if apiError != nil { + err = apiError + } return ModuleStatusNotInstalled, moduleType, false, err } releaseValues := releaseInfo.MergedValues @@ -426,6 +431,10 @@ func (impl ModuleServiceImpl) HandleModuleAction(userId int32, moduleName string updateResponse, err := impl.helmAppService.UpdateApplicationWithChartInfoWithExtraValues(context.Background(), devtronHelmAppIdentifier, chartRepository, extraValues, extraValuesYamlUrl, true) if err != nil { impl.logger.Errorw("error in updating helm release ", "err", err) + apiError := clientErrors.ConvertToApiError(err) + if apiError != nil { + err = apiError + } module.Status = ModuleStatusInstallFailed impl.moduleRepository.Update(module) return nil, err diff --git a/pkg/pipeline/AppDeploymentTypeChangeManager.go b/pkg/pipeline/AppDeploymentTypeChangeManager.go index 2f4e6e0a54..6ee14cc1b2 100644 --- a/pkg/pipeline/AppDeploymentTypeChangeManager.go +++ b/pkg/pipeline/AppDeploymentTypeChangeManager.go @@ -34,6 +34,7 @@ import ( commonBean "github.com/devtron-labs/devtron/pkg/deployment/gitOps/common/bean" "github.com/devtron-labs/devtron/pkg/deployment/gitOps/config" bean3 "github.com/devtron-labs/devtron/pkg/deployment/trigger/devtronApps/bean" + clientErrors "github.com/devtron-labs/devtron/pkg/errors" "github.com/devtron-labs/devtron/pkg/eventProcessor/out" bean2 "github.com/devtron-labs/devtron/pkg/eventProcessor/out/bean" "github.com/juju/errors" @@ -762,6 +763,10 @@ func (impl *AppDeploymentTypeChangeManagerImpl) deleteHelmApp(ctx context.Contex if err != nil { impl.logger.Errorw("error in deleting helm application", "error", err, "appIdentifier", appIdentifier) + apiError := clientErrors.ConvertToApiError(err) + if apiError != nil { + err = apiError + } return err } diff --git a/pkg/pipeline/DeploymentPipelineConfigService.go b/pkg/pipeline/DeploymentPipelineConfigService.go index b75bc8b42a..9c292a730c 100644 --- a/pkg/pipeline/DeploymentPipelineConfigService.go +++ b/pkg/pipeline/DeploymentPipelineConfigService.go @@ -48,6 +48,7 @@ import ( "github.com/devtron-labs/devtron/pkg/deployment/gitOps/git" "github.com/devtron-labs/devtron/pkg/deployment/manifest/deployedAppMetrics" config2 "github.com/devtron-labs/devtron/pkg/deployment/providerConfig" + clientErrors "github.com/devtron-labs/devtron/pkg/errors" "github.com/devtron-labs/devtron/pkg/eventProcessor/out" "github.com/devtron-labs/devtron/pkg/imageDigestPolicy" bean3 "github.com/devtron-labs/devtron/pkg/pipeline/bean" @@ -890,6 +891,10 @@ func (impl *CdPipelineConfigServiceImpl) DeleteHelmTypePipelineDeploymentApp(ctx } else { if err != nil { impl.logger.Errorw("error in deleting helm application", "error", err, "appIdentifier", appIdentifier) + apiError := clientErrors.ConvertToApiError(err) + if apiError != nil { + err = apiError + } return err } if deleteResourceResponse == nil || !deleteResourceResponse.GetSuccess() { diff --git a/pkg/server/ServerService.go b/pkg/server/ServerService.go index 75a19c99f6..a90dd62b69 100644 --- a/pkg/server/ServerService.go +++ b/pkg/server/ServerService.go @@ -22,6 +22,7 @@ import ( "errors" "github.com/devtron-labs/devtron/api/helm-app/gRPC" client "github.com/devtron-labs/devtron/api/helm-app/service" + clientErrors "github.com/devtron-labs/devtron/pkg/errors" moduleRepo "github.com/devtron-labs/devtron/pkg/module/repo" moduleUtil "github.com/devtron-labs/devtron/pkg/module/util" serverBean "github.com/devtron-labs/devtron/pkg/server/bean" @@ -78,6 +79,10 @@ func (impl ServerServiceImpl) GetServerInfo(showServerStatus bool) (*serverBean. devtronAppDetail, err := impl.helmAppService.GetApplicationDetail(context.Background(), devtronHelmAppIdentifier) if err != nil { impl.logger.Errorw("error in getting devtron helm app release status ", "err", err) + apiError := clientErrors.ConvertToApiError(err) + if apiError != nil { + err = apiError + } return nil, err } @@ -153,6 +158,10 @@ func (impl ServerServiceImpl) HandleServerAction(userId int32, serverActionReque updateResponse, err := impl.helmAppService.UpdateApplicationWithChartInfoWithExtraValues(context.Background(), devtronHelmAppIdentifier, chartRepository, extraValues, extraValuesYamlUrl, true) if err != nil { impl.logger.Errorw("error in updating helm release ", "err", err) + apiError := clientErrors.ConvertToApiError(err) + if apiError != nil { + err = apiError + } return nil, err } if !updateResponse.GetSuccess() { diff --git a/pkg/webhook/helm/WebhookHelmService.go b/pkg/webhook/helm/WebhookHelmService.go index e79d5b6996..1437af6b50 100644 --- a/pkg/webhook/helm/WebhookHelmService.go +++ b/pkg/webhook/helm/WebhookHelmService.go @@ -28,6 +28,7 @@ import ( bean3 "github.com/devtron-labs/devtron/pkg/attributes/bean" "github.com/devtron-labs/devtron/pkg/chartRepo" "github.com/devtron-labs/devtron/pkg/cluster" + clientErrors "github.com/devtron-labs/devtron/pkg/errors" "github.com/go-pg/pg" "go.uber.org/zap" "net/http" @@ -154,6 +155,10 @@ func (impl WebhookHelmServiceImpl) CreateOrUpdateHelmApplication(ctx context.Con res, err := impl.helmAppService.InstallRelease(ctx, clusterId, installReleaseRequest) if err != nil { impl.logger.Errorw("Error in installing helm release", "appIdentifier", appIdentifier, "err", err) + apiError := clientErrors.ConvertToApiError(err) + if apiError != nil { + err = apiError + } return nil, common.InternalServerError, err.Error(), http.StatusInternalServerError } if !res.GetSuccess() { From 0a29f8c3cfd3e2f2ff1b6061d2ac710f9b8c8bcf Mon Sep 17 00:00:00 2001 From: Prakash Kumar Date: Mon, 13 May 2024 19:26:05 +0530 Subject: [PATCH 3/4] code review fix --- pkg/errors/utils.go | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/pkg/errors/utils.go b/pkg/errors/utils.go index 1df7db4332..d39703b323 100644 --- a/pkg/errors/utils.go +++ b/pkg/errors/utils.go @@ -27,22 +27,23 @@ var errorHttpStatusCodeMap = map[string]int{ InvalidValueErrorMsg: http.StatusFailedDependency, OperationInProgressErrorMsg: http.StatusConflict, //forbidden error from kubernetes would not be a parameter for us to mark a user forbidden to that resource or not, - //since this is not rbac from devtron, hence mark map them to StatusUnprocessableEntity + //since this is not rbac from devtron, hence map it to StatusUnprocessableEntity ForbiddenErrMsg: http.StatusUnprocessableEntity, } func ConvertToApiError(err error) *util2.ApiError { var apiError *util2.ApiError if _, ok := status.FromError(err); ok { - clientCode, errMsg := util2.GetClientDetailedError(err) + clientCode, _ := util2.GetClientDetailedError(err) httpStatusCode := clientCode.GetHttpStatusCodeForGivenGrpcCode() apiError = &util2.ApiError{ HttpStatusCode: httpStatusCode, Code: strconv.Itoa(httpStatusCode), - InternalMessage: errMsg, - UserMessage: errMsg, + InternalMessage: err.Error(), + UserMessage: err.Error(), } } else { + // instead of iterating or making a map, think of a better implementations for errMsg, statusCode := range errorHttpStatusCodeMap { if strings.Contains(err.Error(), errMsg) { apiError = &util2.ApiError{ From d737c425f66fab07b7a6e5113496e88a8c659486 Mon Sep 17 00:00:00 2001 From: Prakash Kumar Date: Wed, 22 May 2024 17:29:24 +0530 Subject: [PATCH 4/4] fix --- pkg/errors/utils.go | 1 - 1 file changed, 1 deletion(-) diff --git a/pkg/errors/utils.go b/pkg/errors/utils.go index d39703b323..d13c4293cc 100644 --- a/pkg/errors/utils.go +++ b/pkg/errors/utils.go @@ -43,7 +43,6 @@ func ConvertToApiError(err error) *util2.ApiError { UserMessage: err.Error(), } } else { - // instead of iterating or making a map, think of a better implementations for errMsg, statusCode := range errorHttpStatusCodeMap { if strings.Contains(err.Error(), errMsg) { apiError = &util2.ApiError{