diff --git a/api/appStore/deployment/CommonDeploymentRestHandler.go b/api/appStore/deployment/CommonDeploymentRestHandler.go index 019fcaa51d..f831eb9aa2 100644 --- a/api/appStore/deployment/CommonDeploymentRestHandler.go +++ b/api/appStore/deployment/CommonDeploymentRestHandler.go @@ -87,12 +87,12 @@ func (handler *CommonDeploymentRestHandlerImpl) getAppOfferingMode(installedAppI if len(appId) > 0 { appIdentifier, err := handler.helmAppService.DecodeAppId(appId) if err != nil { - err = &util.ApiError{HttpStatusCode: http.StatusBadRequest, UserMessage: "invalid app id"} + // err = &util.ApiError{HttpStatusCode: http.StatusBadRequest, UserMessage: "invalid app id"} return appOfferingMode, installedAppDto, err } installedAppDto, err = handler.installedAppService.GetInstalledAppByClusterNamespaceAndName(appIdentifier) if err != nil { - err = &util.ApiError{HttpStatusCode: http.StatusBadRequest, UserMessage: "unable to find app in database"} + // err = &util.ApiError{HttpStatusCode: http.StatusBadRequest, UserMessage: "unable to find app in database"} return appOfferingMode, installedAppDto, err } // this is the case when hyperion apps does not linked yet @@ -103,7 +103,7 @@ func (handler *CommonDeploymentRestHandlerImpl) getAppOfferingMode(installedAppI installedAppDto.AppOfferingMode = appOfferingMode appIdentifier, err := handler.helmAppService.DecodeAppId(appId) if err != nil { - err = &util.ApiError{HttpStatusCode: http.StatusBadRequest, UserMessage: "invalid app id"} + // err = &util.ApiError{HttpStatusCode: http.StatusBadRequest, UserMessage: "invalid app id, expected format clusterId|namespace|releaseName"} return appOfferingMode, installedAppDto, err } installedAppDto.ClusterId = appIdentifier.ClusterId @@ -113,16 +113,17 @@ func (handler *CommonDeploymentRestHandlerImpl) getAppOfferingMode(installedAppI } else if len(installedAppId) > 0 { installedAppId, err := strconv.Atoi(installedAppId) if err != nil { - err = &util.ApiError{HttpStatusCode: http.StatusBadRequest, UserMessage: "invalid installed app id"} + handler.Logger.Errorw("Invalid installedAppId expected int value", "installedAppId", installedAppId, "err", err) return appOfferingMode, installedAppDto, err } installedAppDto, err = handler.installedAppService.GetInstalledAppByInstalledAppId(installedAppId) if err != nil { - err = &util.ApiError{HttpStatusCode: http.StatusBadRequest, UserMessage: "unable to find app in database"} + // err = &util.ApiError{HttpStatusCode: http.StatusBadRequest, UserMessage: "unable to find app in database"} return appOfferingMode, installedAppDto, err } } else { err := &util.ApiError{HttpStatusCode: http.StatusBadRequest, UserMessage: "app id missing in request"} + handler.Logger.Errorw("appId is missing and is a required field", "appId", appId, "err", err) return appOfferingMode, installedAppDto, err } if installedAppDto != nil && installedAppDto.InstalledAppId > 0 { @@ -188,6 +189,7 @@ func (handler *CommonDeploymentRestHandlerImpl) GetDeploymentHistoryValues(w htt v := r.URL.Query() installedAppId := v.Get("installedAppId") appId := v.Get("appId") + appOfferingMode, installedAppDto, err := handler.getAppOfferingMode(installedAppId, appId) if err != nil { common.WriteJsonResp(w, err, "bad request", http.StatusBadRequest) diff --git a/api/cluster/EnvironmentRestHandler.go b/api/cluster/EnvironmentRestHandler.go index a4866feee4..c7c81e9c0a 100644 --- a/api/cluster/EnvironmentRestHandler.go +++ b/api/cluster/EnvironmentRestHandler.go @@ -447,11 +447,15 @@ func (impl EnvironmentRestHandlerImpl) GetCombinedEnvironmentListForDropDownByCl id, err := strconv.Atoi(clusterId) if err != nil { impl.logger.Errorw("request err, GetCombinedEnvironmentListForDropDownByClusterIds", "err", err, "clusterIdString", clusterIdString) - common.WriteJsonResp(w, err, "please send valid cluster Ids", http.StatusBadRequest) + common.HandleParameterError(w, r, "ids", clusterIdString) return } clusterIds = append(clusterIds, id) } + } else { + impl.logger.Errorw("request err empty query param, GetCombinedEnvironmentListForDropDownByClusterIds", "err", err, "clusterIdString", clusterIdString) + common.HandleParameterError(w, r, "ids", clusterIdString) + return } token := r.Header.Get("token") clusters, err := impl.environmentClusterMappingsService.GetCombinedEnvironmentListForDropDownByClusterIds(token, clusterIds, impl.rbacEnforcementUtil.CheckAuthorizationForGlobalEnvironment) diff --git a/api/devtronResource/DevtronResourceHistoryRestHandler.go b/api/devtronResource/DevtronResourceHistoryRestHandler.go index bee643e93b..c7a0f14f45 100644 --- a/api/devtronResource/DevtronResourceHistoryRestHandler.go +++ b/api/devtronResource/DevtronResourceHistoryRestHandler.go @@ -54,9 +54,16 @@ func (handler *HistoryRestHandlerImpl) GetDeploymentHistory(w http.ResponseWrite queryParams := apiBean.GetHistoryQueryParams{} err := decoder.Decode(&queryParams, v) if err != nil { + handler.logger.Errorw("error in decoding query parameters", "queryParams", v, "err", err) common.WriteJsonResp(w, err, nil, http.StatusBadRequest) return } + // Validating that filterCriteria is provided + if len(queryParams.FilterCriteria) == 0 { + handler.logger.Errorw("missing required filterCriteria parameter") + common.HandleParameterError(w, r, "filterCriteria", "") + return + } decodedReqBean, err := handler.apiReqDecoderService.GetFilterCriteriaParamsForDeploymentHistory(queryParams.FilterCriteria) if err != nil { handler.logger.Errorw("error in getting filter criteria params", "err", err, "filterCriteria", queryParams.FilterCriteria) diff --git a/api/helm-app/service/helper.go b/api/helm-app/service/helper.go index 50a6798cc7..6bf78ad287 100644 --- a/api/helm-app/service/helper.go +++ b/api/helm-app/service/helper.go @@ -29,7 +29,7 @@ import ( func DecodeExternalAppAppId(appId string) (*bean.AppIdentifier, error) { component := strings.Split(appId, "|") if len(component) != 3 { - return nil, fmt.Errorf("malformed app id %s", appId) + return nil, fmt.Errorf("malformed appId, appId: %s, expected format clusterId|namespace|releaseName", appId) } clusterId, err := strconv.Atoi(component[0]) if err != nil { diff --git a/api/restHandler/app/pipeline/AutoCompleteRestHandler.go b/api/restHandler/app/pipeline/AutoCompleteRestHandler.go index ee4895af9a..7c6e8f7f52 100644 --- a/api/restHandler/app/pipeline/AutoCompleteRestHandler.go +++ b/api/restHandler/app/pipeline/AutoCompleteRestHandler.go @@ -136,7 +136,11 @@ func (handler DevtronAppAutoCompleteRestHandlerImpl) GetAppListForAutocomplete(w return } } else { - teamIdInt, err = strconv.Atoi(teamId) + teamIdInt, err = common.ExtractIntPathParamWithContext(w, r, "teamId", teamId+" team") + if err != nil { + // Error already written by ExtractIntPathParamWithContext + return + } if err != nil { common.WriteJsonResp(w, err, nil, http.StatusBadRequest) return diff --git a/api/restHandler/app/pipeline/configure/BuildPipelineRestHandler.go b/api/restHandler/app/pipeline/configure/BuildPipelineRestHandler.go index 800c010916..a1b634d5ab 100644 --- a/api/restHandler/app/pipeline/configure/BuildPipelineRestHandler.go +++ b/api/restHandler/app/pipeline/configure/BuildPipelineRestHandler.go @@ -1209,15 +1209,21 @@ func (handler *PipelineConfigRestHandlerImpl) FetchMaterialInfo(w http.ResponseW func (handler *PipelineConfigRestHandlerImpl) GetCIPipelineById(w http.ResponseWriter, r *http.Request) { token := r.Header.Get("token") - vars := mux.Vars(r) - appId, err := strconv.Atoi(vars["appId"]) + // vars := mux.Vars(r) + //appId, err := strconv.Atoi(vars["appId"]) + appId, err := common.ExtractIntPathParamWithContext(w, r, "appId", "app") if err != nil { - common.WriteJsonResp(w, err, nil, http.StatusBadRequest) + // response already written by ExtractIntPathParamWithContext + //common.WriteJsonResp(w, err, nil, http.StatusBadRequest) + handler.Logger.Error("request err getting , GetCIPipelineById", "appId", appId, "err", err) return } - pipelineId, err := strconv.Atoi(vars["pipelineId"]) + //pipelineId, err := strconv.Atoi(vars["pipelineId"]) + pipelineId, err := common.ExtractIntPathParamWithContext(w, r, "pipelineId", "pipeline") if err != nil { - common.WriteJsonResp(w, err, nil, http.StatusBadRequest) + // response already written by ExtractIntPathParamWithContext + // common.WriteJsonResp(w, err, nil, http.StatusBadRequest) + handler.Logger.Error("request err getting , GetCIPipelineById", "pipelineId", pipelineId, "err", err) return } @@ -1711,20 +1717,29 @@ func (handler *PipelineConfigRestHandlerImpl) FetchWorkflowDetails(w http.Respon common.HandleUnauthorized(w, r) return } - vars := mux.Vars(r) - appId, err := strconv.Atoi(vars["appId"]) + // vars := mux.Vars(r) + // appId, err = strconv.Atoi(vars["appId"]) + appId, err := common.ExtractIntPathParamWithContext(w, r, "appId", "app") if err != nil { - common.WriteJsonResp(w, err, nil, http.StatusBadRequest) + // response already written by ExtractIntPathParamWithContext + // common.WriteJsonResp(w, err, nil, http.StatusBadRequest) + handler.Logger.Error("request err getting , FetchWorkflowDetails", "appId", appId, "err", err) return } - pipelineId, err := strconv.Atoi(vars["pipelineId"]) + // pipelineId, err := strconv.Atoi(vars["pipelineId"]) + pipelineId, err := common.ExtractIntPathParamWithContext(w, r, "pipelineId", "pipeline") if err != nil { - common.WriteJsonResp(w, err, nil, http.StatusBadRequest) + // response already written by ExtractIntPathParamWithContext + // common.WriteJsonResp(w, err, nil, http.StatusBadRequest) + handler.Logger.Error("request err getting , FetchWorkflowDetails", "pipelineId", pipelineId, "err", err) return } - buildId, err := strconv.Atoi(vars["workflowId"]) + // buildId, err := strconv.Atoi(vars["workflowId"]) + buildId, err := common.ExtractIntPathParamWithContext(w, r, "workflowId", "workflow") if err != nil || buildId == 0 { - common.WriteJsonResp(w, err, nil, http.StatusBadRequest) + // response already written by ExtractIntPathParamWithContext + // common.WriteJsonResp(w, err, nil, http.StatusBadRequest) + handler.Logger.Error("request err getting , FetchWorkflowDetails", "buildId", buildId, "err", err) return } handler.Logger.Infow("request payload, FetchWorkflowDetails", "appId", appId, "pipelineId", pipelineId, "buildId", buildId, "buildId", buildId) diff --git a/pkg/devtronResource/APIReqDecoderService.go b/pkg/devtronResource/APIReqDecoderService.go index 2c4c620a93..c708047524 100644 --- a/pkg/devtronResource/APIReqDecoderService.go +++ b/pkg/devtronResource/APIReqDecoderService.go @@ -8,6 +8,7 @@ import ( "go.uber.org/zap" "net/http" "strconv" + "strings" ) // APIReqDecoderService is common service used for getting decoded devtronResource and history related api params. @@ -52,11 +53,26 @@ func (impl *APIReqDecoderServiceImpl) GetFilterCriteriaParamsForDeploymentHistor return nil, &util2.ApiError{ HttpStatusCode: http.StatusBadRequest, Code: "400", - InternalMessage: "invalid format filter criteria!", - UserMessage: "invalid format filter criteria!", + InternalMessage: "missing required identifiers: either (appId and envId) or pipelineId must be provided to fetch history", + UserMessage: "Please provide either both App ID and Env ID, or a Pipeline ID to view history.", } } if resp.PipelineId == 0 { + missingResources := []string{} + if resp.AppId == 0 { + missingResources = append(missingResources, "appId") + } + if resp.EnvId == 0 { + missingResources = append(missingResources, "EnvId") + } + if len(missingResources) > 0 { + return nil, &util2.ApiError{ + HttpStatusCode: http.StatusBadRequest, + Code: "400", + InternalMessage: "missing required resources: " + strings.Join(missingResources, ", "), + UserMessage: "missing required resources: " + strings.Join(missingResources, ", "), + } + } pipelineObj, err := impl.pipelineRepository.FindActiveByAppIdAndEnvId(resp.AppId, resp.EnvId) if err != nil { impl.logger.Errorw("error in getting pipeline", "appId", resp.AppId, "envId", resp.EnvId, "err", err)