Skip to content

Commit

Permalink
simplification of errors (notfound and conflicts)
Browse files Browse the repository at this point in the history
  • Loading branch information
enrichman committed Jul 5, 2022
1 parent 1ba782a commit c2f2256
Show file tree
Hide file tree
Showing 5 changed files with 36 additions and 54 deletions.
3 changes: 2 additions & 1 deletion internal/api/v1/service/catalog.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import (
"github.com/epinio/epinio/internal/api/v1/response"
"github.com/epinio/epinio/internal/services"
"github.com/gin-gonic/gin"
"github.com/pkg/errors"

apierror "github.com/epinio/epinio/pkg/api/core/v1/errors"
k8sapierrors "k8s.io/apimachinery/pkg/api/errors"
Expand Down Expand Up @@ -49,7 +50,7 @@ func (ctr Controller) CatalogShow(c *gin.Context) apierror.APIErrors {
service, err := kubeServiceClient.GetCatalogService(ctx, serviceName)
if err != nil {
if k8sapierrors.IsNotFound(err) {
return apierror.NewNotFoundError("service instance doesn't exist")
return apierror.NewNotFoundError(errors.Wrap(err, "service instance doesn't exist"))
}

return apierror.InternalError(err)
Expand Down
3 changes: 2 additions & 1 deletion internal/api/v1/service/delete.go
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ import (
apierror "github.com/epinio/epinio/pkg/api/core/v1/errors"
"github.com/epinio/epinio/pkg/api/core/v1/models"
"github.com/gin-gonic/gin"
"github.com/pkg/errors"

k8serrors "k8s.io/apimachinery/pkg/api/errors"
)
Expand Down Expand Up @@ -99,7 +100,7 @@ func (ctr Controller) Delete(c *gin.Context) apierror.APIErrors {
err = kubeServiceClient.Delete(ctx, namespace, serviceName)
if err != nil {
if k8serrors.IsNotFound(err) {
return apierror.NewNotFoundError("service not found")
return apierror.NewNotFoundError(errors.Wrap(err, "service not found"))
}

return apierror.InternalError(err)
Expand Down
9 changes: 4 additions & 5 deletions internal/api/v1/service/validate.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,17 +2,16 @@ package service

import (
"context"
"errors"
"fmt"

"github.com/epinio/epinio/helpers/kubernetes"
"github.com/epinio/epinio/internal/helm"
"github.com/epinio/epinio/internal/names"
"github.com/epinio/epinio/internal/services"
"github.com/go-logr/logr"
"github.com/pkg/errors"

apierror "github.com/epinio/epinio/pkg/api/core/v1/errors"

k8serrors "k8s.io/apimachinery/pkg/api/errors"

helmrelease "helm.sh/helm/v3/pkg/release"
Expand All @@ -38,14 +37,14 @@ func ValidateService(
theService, err := kubeServiceClient.Get(ctx, namespace, service)
if err != nil {
if k8serrors.IsNotFound(err) {
return apierror.NewNotFoundError("service not found")
return apierror.NewNotFoundError(errors.Wrap(err, "service not found"))
}

return apierror.InternalError(err)
}
// See internal/services/instances.go - not found => no error, nil structure
if theService == nil {
return apierror.NewNotFoundError("service not found")
return apierror.NewNotFoundError(errors.New("service not found"))
}

logger.Info("getting helm client")
Expand All @@ -61,7 +60,7 @@ func ValidateService(
srv, err := client.GetRelease(releaseName)
if err != nil {
if errors.Is(err, helmdriver.ErrReleaseNotFound) {
return apierror.NewNotFoundError(fmt.Sprintf("%s - %s", err.Error(), releaseName))
return apierror.NewNotFoundError(errors.Wrapf(err, "release %s not found", releaseName))
}
return apierror.InternalError(err)
}
Expand Down
2 changes: 1 addition & 1 deletion internal/cli/server/server.go
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ func NewHandler(logger logr.Logger) (*gin.Engine, error) {
response.Error(ctx, apierrors.NewAPIError("Method not allowed", "", http.StatusMethodNotAllowed))
})
router.NoRoute(func(ctx *gin.Context) {
response.Error(ctx, apierrors.NewNotFoundError("Route not found"))
response.Error(ctx, apierrors.NewNotFoundError(errors.New("Route not found")))
})
router.Use(gin.Recovery())

Expand Down
73 changes: 27 additions & 46 deletions pkg/api/core/v1/errors/errors.go
Original file line number Diff line number Diff line change
Expand Up @@ -104,80 +104,67 @@ func NewBadRequest(msg string, details ...string) APIError {
}

// NewNotFoundError constructs a general API error for when something desired does not exist
func NewNotFoundError(msg string, details ...string) APIError {
return NewAPIError(msg, strings.Join(details, ", "), http.StatusNotFound)
func NewNotFoundError(err error, details ...string) APIError {
return NewAPIError(err.Error(), strings.Join(details, ", "), http.StatusNotFound)
}

// UserNotFound constructs an API error for when the user name is not found in the header
func UserNotFound() APIError {
return NewAPIError(
"User not found in the request header",
"",
http.StatusBadRequest)
// NewConflictError constructs a general API error for when something conflicts
func NewConflictError(err error, details ...string) APIError {
return NewAPIError(err.Error(), strings.Join(details, ", "), http.StatusConflict)
}

////////////////////////////////////////////
//
// Helpers to build common "domain" errors
//
////////////////////////////////////////////

// NamespaceIsNotKnown constructs an API error for when the desired namespace does not exist
func NamespaceIsNotKnown(namespace string) APIError {
return NewNotFoundError(fmt.Errorf("Targeted namespace '%s' does not exist", namespace))
}

// UserNotFound constructs an API error for when the user name is not found in the header
func UserNotFound() APIError {
return NewAPIError(
fmt.Sprintf("Targeted namespace '%s' does not exist", namespace),
"User not found in the request header",
"",
http.StatusNotFound)
http.StatusBadRequest)
}

// AppAlreadyKnown constructs an API error for when we have a conflict with an existing app
func AppAlreadyKnown(app string) APIError {
return NewAPIError(
fmt.Sprintf("Application '%s' already exists", app),
"",
http.StatusConflict)
return NewConflictError(fmt.Errorf("Application '%s' already exists", app))
}

// AppIsNotKnown constructs an API error for when the desired app does not exist
func AppIsNotKnown(app string) APIError {
return NewAPIError(
fmt.Sprintf("Application '%s' does not exist", app),
"",
http.StatusNotFound)
return NewNotFoundError(fmt.Errorf("Application '%s' does not exist", app))
}

// ServiceIsNotKnown constructs an API error for when the desired service does not exist
func ServiceIsNotKnown(service string) APIError {
return NewAPIError(
fmt.Sprintf("Service '%s' does not exist", service),
"",
http.StatusNotFound)
return NewNotFoundError(fmt.Errorf("Service '%s' does not exist", service))
}

// ConfigurationIsNotKnown constructs an API error for when the desired configuration instance does not exist
func ConfigurationIsNotKnown(configuration string) APIError {
return NewAPIError(
fmt.Sprintf("Configuration '%s' does not exist", configuration),
"",
http.StatusNotFound)
return NewNotFoundError(fmt.Errorf("Configuration '%s' does not exist", configuration))
}

// NamespaceAlreadyKnown constructs an API error for when we have a conflict with an existing namespace
func NamespaceAlreadyKnown(namespace string) APIError {
return NewAPIError(
fmt.Sprintf("Namespace '%s' already exists", namespace),
"",
http.StatusConflict)
return NewConflictError(fmt.Errorf("Namespace '%s' already exists", namespace))
}

// ConfigurationAlreadyKnown constructs an API error for when we have a conflict with an existing configuration instance
func ConfigurationAlreadyKnown(configuration string) APIError {
return NewAPIError(
fmt.Sprintf("Configuration '%s' already exists", configuration),
"",
http.StatusConflict)
return NewConflictError(fmt.Errorf("Configuration '%s' already exists", configuration))
}

// ConfigurationAlreadyBound constructs an API error for when the configuration to bind is already bound to the app
func ConfigurationAlreadyBound(configuration string) APIError {
return NewAPIError(
fmt.Sprintf("Configuration '%s' already bound", configuration),
"",
http.StatusConflict)
return NewConflictError(fmt.Errorf("Configuration '%s' already bound", configuration))
}

// ConfigurationIsNotBound constructs an API error for when the configuration to unbind is actually not bound to the app
Expand All @@ -190,16 +177,10 @@ func ConfigurationIsNotBound(configuration string) APIError {

// AppChartAlreadyKnown constructs an API error for when we have a conflict with an existing app chart
func AppChartAlreadyKnown(app string) APIError {
return NewAPIError(
fmt.Sprintf("Application Chart '%s' already exists", app),
"",
http.StatusConflict)
return NewConflictError(fmt.Errorf("Application Chart '%s' already exists", app))
}

// AppChartIsNotKnown constructs an API error for when the desired app chart does not exist
func AppChartIsNotKnown(app string) APIError {
return NewAPIError(
fmt.Sprintf("Application Chart '%s' does not exist", app),
"",
http.StatusNotFound)
return NewNotFoundError(fmt.Errorf("Application Chart '%s' does not exist", app))
}

0 comments on commit c2f2256

Please sign in to comment.