From ed279ba65bc310bd23a3773eda73b7c75be4ef94 Mon Sep 17 00:00:00 2001 From: Oscar Zhou <100548325+oscarzhou-portainer@users.noreply.github.com> Date: Thu, 4 May 2023 10:01:33 +1200 Subject: [PATCH] fix(edgestack): incorrect response code (#8873) --- .../handler/edgestacks/edgestack_create.go | 52 +++++++++---------- api/internal/edge/edgestack.go | 4 +- api/internal/edge/edgestacks/error.go | 15 ++++++ api/internal/edge/edgestacks/service.go | 3 ++ 4 files changed, 46 insertions(+), 28 deletions(-) create mode 100644 api/internal/edge/edgestacks/error.go diff --git a/api/http/handler/edgestacks/edgestack_create.go b/api/http/handler/edgestacks/edgestack_create.go index 70ef27554..5b515d3d4 100644 --- a/api/http/handler/edgestacks/edgestack_create.go +++ b/api/http/handler/edgestacks/edgestack_create.go @@ -13,16 +13,9 @@ import ( "github.com/portainer/portainer/api/filesystem" gittypes "github.com/portainer/portainer/api/git/types" "github.com/portainer/portainer/api/http/security" + edgestackservice "github.com/portainer/portainer/api/internal/edge/edgestacks" ) -type InvalidPayloadError struct { - msg string -} - -func (e *InvalidPayloadError) Error() string { - return e.msg -} - func (handler *Handler) edgeStackCreate(w http.ResponseWriter, r *http.Request) *httperror.HandlerError { method, err := request.RetrieveRouteVariableValue(r, "method") if err != nil { @@ -37,7 +30,7 @@ func (handler *Handler) edgeStackCreate(w http.ResponseWriter, r *http.Request) edgeStack, err := handler.createSwarmStack(method, dryrun, tokenData.ID, r) if err != nil { - var payloadError *InvalidPayloadError + var payloadError *edgestackservice.InvalidPayloadError switch { case errors.As(err, &payloadError): return httperror.BadRequest("Invalid payload", err) @@ -59,7 +52,7 @@ func (handler *Handler) createSwarmStack(method string, dryrun bool, userID port case "file": return handler.createSwarmStackFromFileUpload(r, dryrun) } - return nil, &InvalidPayloadError{"Invalid value for query parameter: method. Value must be one of: string, repository or file"} + return nil, edgestackservice.NewInvalidPayloadError("Invalid value for query parameter: method. Value must be one of: string, repository or file") } type swarmStackFromFileContentPayload struct { @@ -83,13 +76,13 @@ type swarmStackFromFileContentPayload struct { func (payload *swarmStackFromFileContentPayload) Validate(r *http.Request) error { if govalidator.IsNull(payload.Name) { - return &InvalidPayloadError{msg: "Invalid stack name"} + return edgestackservice.NewInvalidPayloadError("Invalid stack name") } if govalidator.IsNull(payload.StackFileContent) { - return &InvalidPayloadError{msg: "Invalid stack file content"} + return edgestackservice.NewInvalidPayloadError("Invalid stack file content") } if len(payload.EdgeGroups) == 0 { - return &InvalidPayloadError{msg: "Edge Groups are mandatory for an Edge stack"} + return edgestackservice.NewInvalidPayloadError("Edge Groups are mandatory for an Edge stack") } return nil } @@ -104,7 +97,8 @@ func (payload *swarmStackFromFileContentPayload) Validate(r *http.Request) error // @param body body swarmStackFromFileContentPayload true "stack config" // @param dryrun query string false "if true, will not create an edge stack, but just will check the settings and return a non-persisted edge stack object" // @success 200 {object} portainer.EdgeStack -// @failure 500 +// @failure 400 "Bad request" +// @failure 500 "Internal server error" // @failure 503 "Edge compute features are disabled" // @router /edge_stacks/create/string [post] func (handler *Handler) createSwarmStackFromFileContent(r *http.Request, dryrun bool) (*portainer.EdgeStack, error) { @@ -153,7 +147,7 @@ func (handler *Handler) storeFileContent(stackFolder string, deploymentType port } if hasDockerEndpoint { - return "", "", "", fmt.Errorf("edge stack with docker environment cannot be deployed with kubernetes or nomad config") + return "", "", "", errors.New("edge stack with docker environment cannot be deployed with kubernetes or nomad config") } if deploymentType == portainer.EdgeStackDeploymentKubernetes { @@ -169,7 +163,8 @@ func (handler *Handler) storeFileContent(stackFolder string, deploymentType port } - return "", "", "", fmt.Errorf("invalid deployment type: %d", deploymentType) + errMessage := fmt.Sprintf("invalid deployment type: %d", deploymentType) + return "", "", "", edgestackservice.NewInvalidPayloadError(errMessage) } type swarmStackFromGitRepositoryPayload struct { @@ -205,13 +200,13 @@ type swarmStackFromGitRepositoryPayload struct { func (payload *swarmStackFromGitRepositoryPayload) Validate(r *http.Request) error { if govalidator.IsNull(payload.Name) { - return &InvalidPayloadError{msg: "Invalid stack name"} + return edgestackservice.NewInvalidPayloadError("Invalid stack name") } if govalidator.IsNull(payload.RepositoryURL) || !govalidator.IsURL(payload.RepositoryURL) { - return &InvalidPayloadError{msg: "Invalid repository URL. Must correspond to a valid URL format"} + return edgestackservice.NewInvalidPayloadError("Invalid repository URL. Must correspond to a valid URL format") } if payload.RepositoryAuthentication && (govalidator.IsNull(payload.RepositoryUsername) || govalidator.IsNull(payload.RepositoryPassword)) { - return &InvalidPayloadError{msg: "Invalid repository credentials. Password must be specified when authentication is enabled"} + return edgestackservice.NewInvalidPayloadError("Invalid repository credentials. Username and password must be specified when authentication is enabled") } if govalidator.IsNull(payload.FilePathInRepository) { switch payload.DeploymentType { @@ -222,7 +217,7 @@ func (payload *swarmStackFromGitRepositoryPayload) Validate(r *http.Request) err } } if len(payload.EdgeGroups) == 0 { - return &InvalidPayloadError{msg: "Edge Groups are mandatory for an Edge stack"} + return edgestackservice.NewInvalidPayloadError("Edge Groups are mandatory for an Edge stack") } return nil } @@ -238,7 +233,8 @@ func (payload *swarmStackFromGitRepositoryPayload) Validate(r *http.Request) err // @param body body swarmStackFromGitRepositoryPayload true "stack config" // @param dryrun query string false "if true, will not create an edge stack, but just will check the settings and return a non-persisted edge stack object" // @success 200 {object} portainer.EdgeStack -// @failure 500 +// @failure 400 "Bad request" +// @failure 500 "Internal server error" // @failure 503 "Edge compute features are disabled" // @router /edge_stacks/create/repository [post] func (handler *Handler) createSwarmStackFromGitRepository(r *http.Request, dryrun bool, userID portainer.UserID) (*portainer.EdgeStack, error) { @@ -294,26 +290,26 @@ type swarmStackFromFileUploadPayload struct { func (payload *swarmStackFromFileUploadPayload) Validate(r *http.Request) error { name, err := request.RetrieveMultiPartFormValue(r, "Name", false) if err != nil { - return &InvalidPayloadError{msg: "Invalid stack name"} + return edgestackservice.NewInvalidPayloadError("Invalid stack name") } payload.Name = name composeFileContent, _, err := request.RetrieveMultiPartFormFile(r, "file") if err != nil { - return &InvalidPayloadError{msg: "Invalid Compose file. Ensure that the Compose file is uploaded correctly"} + return edgestackservice.NewInvalidPayloadError("Invalid Compose file. Ensure that the Compose file is uploaded correctly") } payload.StackFileContent = composeFileContent var edgeGroups []portainer.EdgeGroupID err = request.RetrieveMultiPartFormJSONValue(r, "EdgeGroups", &edgeGroups, false) if err != nil || len(edgeGroups) == 0 { - return &InvalidPayloadError{msg: "Edge Groups are mandatory for an Edge stack"} + return edgestackservice.NewInvalidPayloadError("Edge Groups are mandatory for an Edge stack") } payload.EdgeGroups = edgeGroups deploymentType, err := request.RetrieveNumericMultiPartFormValue(r, "DeploymentType", false) if err != nil { - return &InvalidPayloadError{msg: "Invalid deployment type"} + return edgestackservice.NewInvalidPayloadError("Invalid deployment type") } payload.DeploymentType = portainer.EdgeStackDeploymentType(deploymentType) @@ -348,7 +344,8 @@ func (payload *swarmStackFromFileUploadPayload) Validate(r *http.Request) error // @param RetryDeploy formData bool false "Retry deploy" // @param dryrun query string false "if true, will not create an edge stack, but just will check the settings and return a non-persisted edge stack object" // @success 200 {object} portainer.EdgeStack -// @failure 500 +// @failure 400 "Bad request" +// @failure 500 "Internal server error" // @failure 503 "Edge compute features are disabled" // @router /edge_stacks/create/file [post] func (handler *Handler) createSwarmStackFromFileUpload(r *http.Request, dryrun bool) (*portainer.EdgeStack, error) { @@ -401,5 +398,6 @@ func (handler *Handler) storeManifestFromGitRepository(stackFolder string, relat return "", repositoryConfig.ConfigFilePath, projectPath, nil } - return "", "", "", fmt.Errorf("unknown deployment type: %d", deploymentType) + errMessage := fmt.Sprintf("unknown deployment type: %d", deploymentType) + return "", "", "", edgestackservice.NewInvalidPayloadError(errMessage) } diff --git a/api/internal/edge/edgestack.go b/api/internal/edge/edgestack.go index cb9f363dc..78789cdc3 100644 --- a/api/internal/edge/edgestack.go +++ b/api/internal/edge/edgestack.go @@ -8,6 +8,8 @@ import ( "github.com/portainer/portainer/api/dataservices" ) +var ErrEdgeGroupNotFound = errors.New("Edge group was not found") + // EdgeStackRelatedEndpoints returns a list of environments(endpoints) related to this Edge stack func EdgeStackRelatedEndpoints(edgeGroupIDs []portainer.EdgeGroupID, endpoints []portainer.Endpoint, endpointGroups []portainer.EndpointGroup, edgeGroups []portainer.EdgeGroup) ([]portainer.EndpointID, error) { edgeStackEndpoints := []portainer.EndpointID{} @@ -23,7 +25,7 @@ func EdgeStackRelatedEndpoints(edgeGroupIDs []portainer.EdgeGroupID, endpoints [ } if edgeGroup == nil { - return nil, errors.New("Edge group was not found") + return nil, ErrEdgeGroupNotFound } edgeStackEndpoints = append(edgeStackEndpoints, EdgeGroupRelatedEndpoints(edgeGroup, endpoints, endpointGroups)...) diff --git a/api/internal/edge/edgestacks/error.go b/api/internal/edge/edgestacks/error.go new file mode 100644 index 000000000..3844c9d6f --- /dev/null +++ b/api/internal/edge/edgestacks/error.go @@ -0,0 +1,15 @@ +package edgestacks + +type InvalidPayloadError struct { + msg string +} + +func (e *InvalidPayloadError) Error() string { + return e.msg +} + +func NewInvalidPayloadError(errMsg string) *InvalidPayloadError { + return &InvalidPayloadError{ + msg: errMsg, + } +} diff --git a/api/internal/edge/edgestacks/service.go b/api/internal/edge/edgestacks/service.go index a9a90235b..8d0498f8d 100644 --- a/api/internal/edge/edgestacks/service.go +++ b/api/internal/edge/edgestacks/service.go @@ -80,6 +80,9 @@ func (service *Service) PersistEdgeStack( relatedEndpointIds, err := edge.EdgeStackRelatedEndpoints(stack.EdgeGroups, relationConfig.Endpoints, relationConfig.EndpointGroups, relationConfig.EdgeGroups) if err != nil { + if err == edge.ErrEdgeGroupNotFound { + return nil, NewInvalidPayloadError(err.Error()) + } return nil, fmt.Errorf("unable to persist environment relation in database: %w", err) }