-
Notifications
You must be signed in to change notification settings - Fork 475
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.
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
fix: Validation of resource request for app type both helm and devtron #5348
base: main
Are you sure you want to change the base?
Conversation
Quality Gate failedFailed conditions |
handler.logger.Errorw("error in validating resource request", "err", err) | ||
common.WriteJsonResp(w, err, nil, http.StatusBadRequest) | ||
return | ||
} | ||
if !valid { | ||
apiError := utils.ApiError{ | ||
InternalMessage: fmt.Sprintf("resource %s: \"%s\" doesn't exist", request.K8sRequest.ResourceIdentifier.GroupVersionKind.Kind, request.K8sRequest.ResourceIdentifier.Name), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
extract the error message into variable adn use that here
} | ||
if !valid { | ||
apiError := utils.ApiError{ | ||
InternalMessage: fmt.Sprintf("resource %s: \"%s\" doesn't exist", request.K8sRequest.ResourceIdentifier.GroupVersionKind.Kind, request.K8sRequest.ResourceIdentifier.Name), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
same here
@@ -204,10 +240,52 @@ func (handler *K8sApplicationRestHandlerImpl) GetResource(w http.ResponseWriter, | |||
//setting devtronAppIdentifier value in request | |||
request.DevtronAppIdentifier = devtronAppIdentifier | |||
request.ClusterId = request.DevtronAppIdentifier.ClusterId | |||
appId := request.DevtronAppIdentifier.AppId | |||
envId := request.DevtronAppIdentifier.EnvId | |||
cdPipeline, err := handler.k8sApplicationService.GetPipelineByAppIdEnvId(appId, envId) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why is this method in k8's application service. this logic should be in pipeline service or pipeline config service.
} | ||
common.WriteJsonResp(w, &apiError, nil, http.StatusBadRequest) | ||
return | ||
} | ||
} else if request.DeploymentType == bean2.ArgoInstalledType { | ||
//TODO Implement ResourceRequest Validation for ArgoCD Installed APPs From ResourceTree |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
the below logic seems duplicate, please move it to seperate method which will handle this validation
Description
Fixes #
Checklist:
Does this PR introduce a user-facing change?