From 931ff97b5513de51fe4f50df97a73a6206d5a1b1 Mon Sep 17 00:00:00 2001 From: Rafael Benevides Date: Wed, 14 Jan 2026 16:43:19 -0300 Subject: [PATCH 1/6] HYPERFLEET-452 - feat: implement RFC 9457 Problem Details error model Refactor error handling to use RFC 9457 Problem Details format with HyperFleet extension fields for consistent, standards-compliant API errors. Changes: - Update OpenAPI spec with ProblemDetails schema and application/problem+json - Add error codes in HYPERFLEET-CAT-NUM format (VAL, AUT, AUZ, NTF, CNF, etc.) - Add AsProblemDetails() method with type, title, status, detail, instance - Include extension fields: code, timestamp, trace_id - Add errors array for field-level validation details - Update all handlers and middleware to use new error format - Update tests for new error structure --- openapi/openapi.yaml | 102 +++++-- pkg/api/error.go | 99 ++++--- pkg/api/error_types.go | 13 - pkg/api/presenters/error.go | 5 +- pkg/auth/auth_middleware.go | 2 +- pkg/auth/helpers.go | 16 +- pkg/db/transaction_middleware.go | 10 +- pkg/errors/errors.go | 331 +++++++++++++++-------- pkg/handlers/framework.go | 13 +- pkg/handlers/helpers.go | 28 ++ pkg/middleware/schema_validation.go | 14 +- pkg/middleware/schema_validation_test.go | 4 +- pkg/services/generic_test.go | 4 +- pkg/validators/schema_validator.go | 8 +- pkg/validators/schema_validator_test.go | 2 +- test/integration/clusters_test.go | 46 ++-- test/integration/node_pools_test.go | 12 +- 17 files changed, 444 insertions(+), 265 deletions(-) delete mode 100755 pkg/api/error_types.go diff --git a/openapi/openapi.yaml b/openapi/openapi.yaml index d19b926..8028955 100644 --- a/openapi/openapi.yaml +++ b/openapi/openapi.yaml @@ -1,7 +1,7 @@ openapi: 3.0.0 info: title: HyperFleet API - version: 1.0.2 + version: 1.0.3 contact: name: HyperFleet Team license: @@ -37,7 +37,7 @@ paths: default: description: An unexpected error response. content: - application/json: + application/problem+json: schema: $ref: '#/components/schemas/Error' security: @@ -64,7 +64,7 @@ paths: default: description: An unexpected error response. content: - application/json: + application/problem+json: schema: $ref: '#/components/schemas/Error' requestBody: @@ -98,7 +98,7 @@ paths: default: description: An unexpected error response. content: - application/json: + application/problem+json: schema: $ref: '#/components/schemas/Error' security: @@ -132,7 +132,7 @@ paths: default: description: An unexpected error response. content: - application/json: + application/problem+json: schema: $ref: '#/components/schemas/Error' security: @@ -160,7 +160,7 @@ paths: default: description: An unexpected error response. content: - application/json: + application/problem+json: schema: $ref: '#/components/schemas/Error' requestBody: @@ -201,7 +201,7 @@ paths: default: description: An unexpected error response. content: - application/json: + application/problem+json: schema: $ref: '#/components/schemas/Error' security: @@ -280,7 +280,7 @@ paths: default: description: An unexpected error response. content: - application/json: + application/problem+json: schema: $ref: '#/components/schemas/Error' /api/hyperfleet/v1/clusters/{cluster_id}/statuses: @@ -373,7 +373,7 @@ paths: default: description: An unexpected error response. content: - application/json: + application/problem+json: schema: $ref: '#/components/schemas/Error' security: @@ -916,33 +916,83 @@ components: description: Status value for conditions Error: type: object + description: RFC 9457 Problem Details error format with HyperFleet extensions + required: + - type + - title + - status properties: - id: + type: type: string - kind: + format: uri + description: URI reference identifying the problem type + example: https://api.hyperfleet.io/errors/validation-error + title: type: string - description: Resource kind - href: + description: Short human-readable summary of the problem + example: Validation Failed + status: + type: integer + description: HTTP status code + example: 400 + detail: type: string - description: Resource URI + description: Human-readable explanation specific to this occurrence + example: The cluster name field is required + instance: + type: string + format: uri + description: URI reference for this specific occurrence + example: /api/hyperfleet/v1/clusters code: type: string - reason: + description: Machine-readable error code in HYPERFLEET-CAT-NUM format + example: HYPERFLEET-VAL-001 + timestamp: type: string - operation_id: + format: date-time + description: RFC3339 timestamp of when the error occurred + example: '2024-01-15T10:30:00Z' + trace_id: type: string - details: + description: Distributed trace ID for correlation + example: abc123def456 + errors: type: array + description: Field-level validation errors (for validation failures) items: - type: object - properties: - field: - type: string - description: Field path that failed validation - error: - type: string - description: Validation error message for this field - description: Field-level validation errors (optional) + $ref: '#/components/schemas/ValidationError' + ValidationError: + type: object + description: Field-level validation error detail + required: + - field + - message + properties: + field: + type: string + description: JSON path to the field that failed validation + example: spec.name + value: + description: The invalid value that was provided (if safe to include) + constraint: + type: string + description: The validation constraint that was violated + enum: + - required + - min + - max + - min_length + - max_length + - pattern + - enum + - format + - unique + example: required + message: + type: string + description: Human-readable error message for this field + example: Cluster name is required NodePool: type: object required: diff --git a/pkg/api/error.go b/pkg/api/error.go index b687e3a..8ccf4d0 100755 --- a/pkg/api/error.go +++ b/pkg/api/error.go @@ -6,69 +6,84 @@ import ( "fmt" "net/http" "os" + "time" + "github.com/openshift-hyperfleet/hyperfleet-api/pkg/api/openapi" "github.com/openshift-hyperfleet/hyperfleet-api/pkg/errors" "github.com/openshift-hyperfleet/hyperfleet-api/pkg/logger" ) -// SendNotFound sends a 404 response with some details about the non existing resource. +// SendNotFound sends a 404 response in RFC 9457 Problem Details format. func SendNotFound(w http.ResponseWriter, r *http.Request) { - // Set the content type: - w.Header().Set("Content-Type", "application/json") - - // Prepare the body: - id := "404" - reason := fmt.Sprintf( - "The requested resource '%s' doesn't exist", - r.URL.Path, - ) - body := Error{ - Type: ErrorType, - ID: id, - HREF: "/api/hyperfleet/v1/errors/" + id, - Code: "hyperfleet-" + id, - Reason: reason, + w.Header().Set("Content-Type", "application/problem+json") + + traceID, _ := logger.GetRequestID(r.Context()) + now := time.Now().UTC() + detail := fmt.Sprintf("The requested resource '%s' doesn't exist", r.URL.Path) + + body := openapi.Error{ + Type: errors.ErrorTypeNotFound, + Title: "Resource Not Found", + Status: http.StatusNotFound, + Detail: &detail, + Instance: &r.URL.Path, + Code: ptrString(errors.CodeNotFoundGeneric), + Timestamp: &now, + TraceId: &traceID, } + data, err := json.Marshal(body) if err != nil { + logger.WithError(r.Context(), err).Error("Failed to marshal not found response") SendPanic(w, r) return } - // Send the response: w.WriteHeader(http.StatusNotFound) _, err = w.Write(data) if err != nil { err = fmt.Errorf("can't send response body for request '%s'", r.URL.Path) logger.WithError(r.Context(), err).Error("Failed to send response") - return } } +// SendUnauthorized sends a 401 response in RFC 9457 Problem Details format. func SendUnauthorized(w http.ResponseWriter, r *http.Request, message string) { - w.Header().Set("Content-Type", "application/json") + w.Header().Set("Content-Type", "application/problem+json") - // Prepare the body: - apiError := errors.Unauthorized("%s", message) - data, err := json.Marshal(apiError) + traceID, _ := logger.GetRequestID(r.Context()) + now := time.Now().UTC() + + body := openapi.Error{ + Type: errors.ErrorTypeAuth, + Title: "Authentication Required", + Status: http.StatusUnauthorized, + Detail: &message, + Instance: &r.URL.Path, + Code: ptrString(errors.CodeAuthNoCredentials), + Timestamp: &now, + TraceId: &traceID, + } + + data, err := json.Marshal(body) if err != nil { + logger.WithError(r.Context(), err).Error("Failed to marshal unauthorized response") SendPanic(w, r) return } - // Send the response: w.WriteHeader(http.StatusUnauthorized) _, err = w.Write(data) if err != nil { err = fmt.Errorf("can't send response body for request '%s'", r.URL.Path) logger.WithError(r.Context(), err).Error("Failed to send response") - return } } -// SendPanic sends a panic error response to the client, but it doesn't end the process. +// SendPanic sends a panic error response in RFC 9457 Problem Details format. func SendPanic(w http.ResponseWriter, r *http.Request) { - w.Header().Set("Content-Type", "application/json") + w.Header().Set("Content-Type", "application/problem+json") + w.WriteHeader(http.StatusInternalServerError) _, err := w.Write(panicBody) if err != nil { err = fmt.Errorf( @@ -81,33 +96,33 @@ func SendPanic(w http.ResponseWriter, r *http.Request) { } // panicBody is the error body that will be sent when something unexpected happens while trying to -// send another error response. For example, if sending an error response fails because the error -// description can't be converted to JSON. +// send another error response. var panicBody []byte func init() { ctx := context.Background() var err error - // Create the panic error body: - panicID := "1000" - panicError := Error{ - Type: ErrorType, - ID: panicID, - HREF: "/api/hyperfleet/v1/" + panicID, - Code: "hyperfleet-" + panicID, - Reason: "An unexpected error happened, please check the log of the service " + - "for details", + detail := "An unexpected error happened, please check the log of the service for details" + instance := "/api/hyperfleet/v1" + + panicError := openapi.Error{ + Type: errors.ErrorTypeInternal, + Title: "Internal Server Error", + Status: http.StatusInternalServerError, + Detail: &detail, + Instance: &instance, + Code: ptrString(errors.CodeInternalGeneral), } - // Convert it to JSON: panicBody, err = json.Marshal(panicError) if err != nil { - err = fmt.Errorf( - "can't create the panic error body: %s", - err.Error(), - ) + err = fmt.Errorf("can't create the panic error body: %s", err.Error()) logger.WithError(ctx, err).Error("Failed to create panic error body") os.Exit(1) } } + +func ptrString(s string) *string { + return &s +} diff --git a/pkg/api/error_types.go b/pkg/api/error_types.go deleted file mode 100755 index 57a0bac..0000000 --- a/pkg/api/error_types.go +++ /dev/null @@ -1,13 +0,0 @@ -package api - -// ErrorType is the name of the type used to report errors. -const ErrorType = "Error" - -// Error represents an error reported by the API. -type Error struct { - Type string `json:"type,omitempty"` - ID string `json:"id,omitempty"` - HREF string `json:"href,omitempty"` - Code string `json:"code,omitempty"` - Reason string `json:"reason,omitempty"` -} diff --git a/pkg/api/presenters/error.go b/pkg/api/presenters/error.go index 4c3ee23..772adb3 100755 --- a/pkg/api/presenters/error.go +++ b/pkg/api/presenters/error.go @@ -5,6 +5,7 @@ import ( "github.com/openshift-hyperfleet/hyperfleet-api/pkg/errors" ) -func PresentError(err *errors.ServiceError) openapi.Error { - return err.AsOpenapiError("") +// PresentError converts a ServiceError to RFC 9457 Problem Details format +func PresentError(err *errors.ServiceError, instance string, traceID string) openapi.Error { + return err.AsProblemDetails(instance, traceID) } diff --git a/pkg/auth/auth_middleware.go b/pkg/auth/auth_middleware.go index 59750d1..2b4eb24 100755 --- a/pkg/auth/auth_middleware.go +++ b/pkg/auth/auth_middleware.go @@ -26,7 +26,7 @@ func (a *Middleware) AuthenticateAccountJWT(next http.Handler) http.Handler { ctx := r.Context() payload, err := GetAuthPayload(r) if err != nil { - handleError(ctx, w, errors.ErrorUnauthorized, fmt.Sprintf("Unable to get payload details from JWT token: %s", err)) + handleError(ctx, w, r, errors.ErrorUnauthenticated, fmt.Sprintf("Unable to get payload details from JWT token: %s", err)) return } diff --git a/pkg/auth/helpers.go b/pkg/auth/helpers.go index 7ef4318..169459e 100755 --- a/pkg/auth/helpers.go +++ b/pkg/auth/helpers.go @@ -9,23 +9,27 @@ import ( "github.com/openshift-hyperfleet/hyperfleet-api/pkg/logger" ) -func handleError(ctx context.Context, w http.ResponseWriter, code errors.ServiceErrorCode, reason string) { - requestID, ok := logger.GetRequestID(ctx) +func handleError(ctx context.Context, w http.ResponseWriter, r *http.Request, code errors.ServiceErrorCode, reason string) { + traceID, ok := logger.GetRequestID(ctx) if !ok { - requestID = "unknown" + traceID = "unknown" } err := errors.New(code, "%s", reason) + instance := "" + if r != nil { + instance = r.URL.Path + } if err.HttpCode >= 400 && err.HttpCode <= 499 { logger.WithError(ctx, err).Warn("Client error occurred") } else { logger.WithError(ctx, err).Error("Server error occurred") } - writeJSONResponse(w, err.HttpCode, err.AsOpenapiError(requestID)) + writeProblemDetailsResponse(w, err.HttpCode, err.AsProblemDetails(instance, traceID)) } -func writeJSONResponse(w http.ResponseWriter, code int, payload interface{}) { - w.Header().Set("Content-Type", "application/json") +func writeProblemDetailsResponse(w http.ResponseWriter, code int, payload interface{}) { + w.Header().Set("Content-Type", "application/problem+json") w.WriteHeader(code) if payload != nil { diff --git a/pkg/db/transaction_middleware.go b/pkg/db/transaction_middleware.go index 2e46a6c..8b06c83 100755 --- a/pkg/db/transaction_middleware.go +++ b/pkg/db/transaction_middleware.go @@ -17,9 +17,9 @@ func TransactionMiddleware(next http.Handler, connection SessionFactory) http.Ha if err != nil { logger.WithError(r.Context(), err).Error("Could not create transaction") // use default error to avoid exposing internals to users - err := errors.GeneralError("") - requestID, _ := logger.GetRequestID(r.Context()) - writeJSONResponse(w, err.HttpCode, err.AsOpenapiError(requestID)) + serviceErr := errors.GeneralError("") + traceID, _ := logger.GetRequestID(r.Context()) + writeProblemDetailsResponse(w, serviceErr.HttpCode, serviceErr.AsProblemDetails(r.URL.Path, traceID)) return } @@ -35,8 +35,8 @@ func TransactionMiddleware(next http.Handler, connection SessionFactory) http.Ha }) } -func writeJSONResponse(w http.ResponseWriter, code int, payload interface{}) { - w.Header().Set("Content-Type", "application/json") +func writeProblemDetailsResponse(w http.ResponseWriter, code int, payload interface{}) { + w.Header().Set("Content-Type", "application/problem+json") w.WriteHeader(code) if payload != nil { diff --git a/pkg/errors/errors.go b/pkg/errors/errors.go index dab9357..0dec39c 100755 --- a/pkg/errors/errors.go +++ b/pkg/errors/errors.go @@ -4,127 +4,193 @@ import ( "context" "fmt" "net/http" - "strconv" + "time" "github.com/openshift-hyperfleet/hyperfleet-api/pkg/api/openapi" "github.com/openshift-hyperfleet/hyperfleet-api/pkg/logger" - "github.com/openshift-hyperfleet/hyperfleet-api/pkg/util" ) +// Error type URIs for RFC 9457 const ( - // Prefix used for error code strings - // Example: - // ErrorCodePrefix = "rh-text" - // results in: hyperfleet-1 - ErrorCodePrefix = "hyperfleet" - - // HREF for API errors - ErrorHref = "/api/hyperfleet/v1/errors/" - - // InvalidToken occurs when a token is invalid (generally, not found in the database) - ErrorInvalidToken ServiceErrorCode = 1 - - // Forbidden occurs when a user has been blacklisted - ErrorForbidden ServiceErrorCode = 4 - - // Conflict occurs when a database constraint is violated - ErrorConflict ServiceErrorCode = 6 - - // NotFound occurs when a record is not found in the database - ErrorNotFound ServiceErrorCode = 7 - - // Validation occurs when an object fails validation - ErrorValidation ServiceErrorCode = 8 - - // General occurs when an error fails to match any other error code - ErrorGeneral ServiceErrorCode = 9 - - // NotImplemented occurs when an API REST method is not implemented in a handler - ErrorNotImplemented ServiceErrorCode = 10 - - // Unauthorized occurs when the requester is not authorized to perform the specified action - ErrorUnauthorized ServiceErrorCode = 11 - - // Unauthenticated occurs when the provided credentials cannot be validated - ErrorUnauthenticated ServiceErrorCode = 15 - - // MalformedRequest occurs when the request body cannot be read - ErrorMalformedRequest ServiceErrorCode = 17 + ErrorTypeBase = "https://api.hyperfleet.io/errors/" + ErrorTypeValidation = ErrorTypeBase + "validation-error" + ErrorTypeAuth = ErrorTypeBase + "authentication-error" + ErrorTypeAuthz = ErrorTypeBase + "authorization-error" + ErrorTypeNotFound = ErrorTypeBase + "not-found" + ErrorTypeConflict = ErrorTypeBase + "conflict" + ErrorTypeRateLimit = ErrorTypeBase + "rate-limit" + ErrorTypeInternal = ErrorTypeBase + "internal-error" + ErrorTypeService = ErrorTypeBase + "service-unavailable" + ErrorTypeBadRequest = ErrorTypeBase + "bad-request" + ErrorTypeMalformed = ErrorTypeBase + "malformed-request" + ErrorTypeNotImpl = ErrorTypeBase + "not-implemented" +) - // Bad Request - ErrorBadRequest ServiceErrorCode = 21 +// Error codes in HYPERFLEET-CAT-NUM format +const ( + // Validation errors (VAL) - 400/422 + CodeValidationMultiple = "HYPERFLEET-VAL-000" + CodeValidationRequired = "HYPERFLEET-VAL-001" + CodeValidationInvalid = "HYPERFLEET-VAL-002" + CodeValidationFormat = "HYPERFLEET-VAL-003" + CodeValidationRange = "HYPERFLEET-VAL-004" + + // Authentication errors (AUT) - 401 + CodeAuthNoCredentials = "HYPERFLEET-AUT-001" + CodeAuthInvalidCredentials = "HYPERFLEET-AUT-002" + CodeAuthExpiredToken = "HYPERFLEET-AUT-003" + + // Authorization errors (AUZ) - 403 + CodeAuthzInsufficient = "HYPERFLEET-AUZ-001" + CodeAuthzForbidden = "HYPERFLEET-AUZ-002" + + // Not Found errors (NTF) - 404 + CodeNotFoundGeneric = "HYPERFLEET-NTF-001" + CodeNotFoundCluster = "HYPERFLEET-NTF-002" + CodeNotFoundNodePool = "HYPERFLEET-NTF-003" + + // Conflict errors (CNF) - 409 + CodeConflictExists = "HYPERFLEET-CNF-001" + CodeConflictVersion = "HYPERFLEET-CNF-002" + + // Rate Limit errors (LMT) - 429 + CodeRateLimitExceeded = "HYPERFLEET-LMT-001" + + // Internal errors (INT) - 500 + CodeInternalGeneral = "HYPERFLEET-INT-001" + CodeInternalDatabase = "HYPERFLEET-INT-002" + + // Service errors (SVC) - 502/503/504 + CodeServiceUnavailable = "HYPERFLEET-SVC-001" + CodeServiceTimeout = "HYPERFLEET-SVC-002" + + // Bad Request errors + CodeBadRequest = "HYPERFLEET-VAL-005" + CodeMalformedBody = "HYPERFLEET-VAL-006" + CodeSearchParseFail = "HYPERFLEET-VAL-007" + CodeNotImplemented = "HYPERFLEET-INT-003" +) - // Invalid Search Query - ErrorFailedToParseSearch ServiceErrorCode = 23 +// ServiceErrorCode is kept for backwards compatibility +type ServiceErrorCode int - // DatabaseAdvisoryLock occurs whe the advisory lock is failed to get +// Legacy error codes mapped to new format +const ( + ErrorInvalidToken ServiceErrorCode = 1 + ErrorForbidden ServiceErrorCode = 4 + ErrorConflict ServiceErrorCode = 6 + ErrorNotFound ServiceErrorCode = 7 + ErrorValidation ServiceErrorCode = 8 + ErrorGeneral ServiceErrorCode = 9 + ErrorNotImplemented ServiceErrorCode = 10 + ErrorUnauthorized ServiceErrorCode = 11 + ErrorUnauthenticated ServiceErrorCode = 15 + ErrorMalformedRequest ServiceErrorCode = 17 + ErrorBadRequest ServiceErrorCode = 21 + ErrorFailedToParseSearch ServiceErrorCode = 23 ErrorDatabaseAdvisoryLock ServiceErrorCode = 26 ) -type ServiceErrorCode int - type ServiceErrors []ServiceError -// ValidationDetail represents a single field validation error +// ValidationDetail represents a single field validation error (RFC 9457 format) type ValidationDetail struct { - Field string `json:"field"` - Error string `json:"error"` + Field string `json:"field"` + Value interface{} `json:"value,omitempty"` + Constraint string `json:"constraint,omitempty"` + Message string `json:"message"` +} + +// ServiceError represents an API error with RFC 9457 Problem Details support +type ServiceError struct { + // Code is the legacy numeric error code + Code ServiceErrorCode + // RFC9457Code is the new HYPERFLEET-CAT-NUM format code + RFC9457Code string + // Type is the RFC 9457 type URI + Type string + // Title is a short human-readable summary + Title string + // Reason is the context-specific reason (maps to detail in RFC 9457) + Reason string + // HttpCode is the HTTP status code + HttpCode int + // Details contains field-level validation errors + Details []ValidationDetail +} + +// errorDefinition holds the default values for each error type +type errorDefinition struct { + Code ServiceErrorCode + RFC9457Code string + Type string + Title string + Reason string + HttpCode int +} + +var errorDefinitions = map[ServiceErrorCode]errorDefinition{ + ErrorInvalidToken: {ErrorInvalidToken, CodeAuthExpiredToken, ErrorTypeAuth, "Invalid Token", "Invalid token provided", http.StatusForbidden}, + ErrorForbidden: {ErrorForbidden, CodeAuthzForbidden, ErrorTypeAuthz, "Forbidden", "Forbidden to perform this action", http.StatusForbidden}, + ErrorConflict: {ErrorConflict, CodeConflictExists, ErrorTypeConflict, "Resource Conflict", "An entity with the specified unique values already exists", http.StatusConflict}, + ErrorNotFound: {ErrorNotFound, CodeNotFoundGeneric, ErrorTypeNotFound, "Resource Not Found", "Resource not found", http.StatusNotFound}, + ErrorValidation: {ErrorValidation, CodeValidationMultiple, ErrorTypeValidation, "Validation Failed", "General validation failure", http.StatusBadRequest}, + ErrorGeneral: {ErrorGeneral, CodeInternalGeneral, ErrorTypeInternal, "Internal Server Error", "Unspecified error", http.StatusInternalServerError}, + ErrorNotImplemented: {ErrorNotImplemented, CodeNotImplemented, ErrorTypeNotImpl, "Not Implemented", "Functionality not implemented", http.StatusNotImplemented}, + ErrorUnauthorized: {ErrorUnauthorized, CodeAuthzInsufficient, ErrorTypeAuthz, "Unauthorized", "Account is unauthorized to perform this action", http.StatusForbidden}, + ErrorUnauthenticated: {ErrorUnauthenticated, CodeAuthNoCredentials, ErrorTypeAuth, "Authentication Required", "Account authentication could not be verified", http.StatusUnauthorized}, + ErrorMalformedRequest: {ErrorMalformedRequest, CodeMalformedBody, ErrorTypeMalformed, "Malformed Request", "Unable to read request body", http.StatusBadRequest}, + ErrorBadRequest: {ErrorBadRequest, CodeBadRequest, ErrorTypeBadRequest, "Bad Request", "Bad request", http.StatusBadRequest}, + ErrorFailedToParseSearch: {ErrorFailedToParseSearch, CodeSearchParseFail, ErrorTypeValidation, "Invalid Search Query", "Failed to parse search query", http.StatusBadRequest}, + ErrorDatabaseAdvisoryLock: {ErrorDatabaseAdvisoryLock, CodeInternalDatabase, ErrorTypeInternal, "Database Error", "Database advisory lock error", http.StatusInternalServerError}, } func Find(code ServiceErrorCode) (bool, *ServiceError) { - for _, err := range Errors() { - if err.Code == code { - return true, &err - } + def, exists := errorDefinitions[code] + if !exists { + return false, nil + } + return true, &ServiceError{ + Code: def.Code, + RFC9457Code: def.RFC9457Code, + Type: def.Type, + Title: def.Title, + Reason: def.Reason, + HttpCode: def.HttpCode, } - return false, nil } func Errors() ServiceErrors { - return ServiceErrors{ - {Code: ErrorInvalidToken, Reason: "Invalid token provided", HttpCode: http.StatusForbidden}, - {Code: ErrorForbidden, Reason: "Forbidden to perform this action", HttpCode: http.StatusForbidden}, - {Code: ErrorConflict, Reason: "An entity with the specified unique values already exists", HttpCode: http.StatusConflict}, - {Code: ErrorNotFound, Reason: "Resource not found", HttpCode: http.StatusNotFound}, - {Code: ErrorValidation, Reason: "General validation failure", HttpCode: http.StatusBadRequest}, - {Code: ErrorGeneral, Reason: "Unspecified error", HttpCode: http.StatusInternalServerError}, - {Code: ErrorNotImplemented, Reason: "HTTP Method not implemented for this endpoint", HttpCode: http.StatusMethodNotAllowed}, - {Code: ErrorUnauthorized, Reason: "Account is unauthorized to perform this action", HttpCode: http.StatusForbidden}, - {Code: ErrorUnauthenticated, Reason: "Account authentication could not be verified", HttpCode: http.StatusUnauthorized}, - {Code: ErrorMalformedRequest, Reason: "Unable to read request body", HttpCode: http.StatusBadRequest}, - {Code: ErrorBadRequest, Reason: "Bad request", HttpCode: http.StatusBadRequest}, - {Code: ErrorFailedToParseSearch, Reason: "Failed to parse search query", HttpCode: http.StatusBadRequest}, - {Code: ErrorDatabaseAdvisoryLock, Reason: "Database advisory lock error", HttpCode: http.StatusInternalServerError}, + errors := make(ServiceErrors, 0, len(errorDefinitions)) + for _, def := range errorDefinitions { + errors = append(errors, ServiceError{ + Code: def.Code, + RFC9457Code: def.RFC9457Code, + Type: def.Type, + Title: def.Title, + Reason: def.Reason, + HttpCode: def.HttpCode, + }) } + return errors } -type ServiceError struct { - // Code is the numeric and distinct ID for the error - Code ServiceErrorCode - // Reason is the context-specific reason the error was generated - Reason string - // HttopCode is the HttpCode associated with the error when the error is returned as an API response - HttpCode int - // Details contains field-level validation errors (optional) - Details []ValidationDetail -} - -// New Reason can be a string with format verbs, which will be replace by the specified values +// New creates a new ServiceError with optional custom reason func New(code ServiceErrorCode, reason string, values ...interface{}) *ServiceError { - // If the code isn't defined, use the general error code - var err *ServiceError exists, err := Find(code) if !exists { ctx := context.Background() logger.With(ctx, logger.FieldErrorCode, code).Error("Undefined error code used") err = &ServiceError{ - Code: ErrorGeneral, - Reason: "Unspecified error", - HttpCode: 500, + Code: ErrorGeneral, + RFC9457Code: CodeInternalGeneral, + Type: ErrorTypeInternal, + Title: "Internal Server Error", + Reason: "Unspecified error", + HttpCode: http.StatusInternalServerError, } } - // If the reason is unspecified, use the default if reason != "" { err.Reason = fmt.Sprintf(reason, values...) } @@ -133,7 +199,7 @@ func New(code ServiceErrorCode, reason string, values ...interface{}) *ServiceEr } func (e *ServiceError) Error() string { - return fmt.Sprintf("%s: %s", *CodeStr(e.Code), e.Reason) + return fmt.Sprintf("%s: %s", e.RFC9457Code, e.Reason) } func (e *ServiceError) AsError() error { @@ -141,55 +207,75 @@ func (e *ServiceError) AsError() error { } func (e *ServiceError) Is404() bool { - return e.Code == NotFound("").Code + return e.Code == ErrorNotFound } func (e *ServiceError) IsConflict() bool { - return e.Code == Conflict("").Code + return e.Code == ErrorConflict } func (e *ServiceError) IsForbidden() bool { - return e.Code == Forbidden("").Code + return e.Code == ErrorForbidden } -func (e *ServiceError) AsOpenapiError(operationID string) openapi.Error { - openapiErr := openapi.Error{ - Kind: util.PtrString("Error"), - Id: util.PtrString(strconv.Itoa(int(e.Code))), - Href: Href(e.Code), - Code: CodeStr(e.Code), - Reason: util.PtrString(e.Reason), - OperationId: util.PtrString(operationID), +// validConstraints maps string values to their corresponding ValidationErrorConstraint enum values +var validConstraints = map[string]openapi.ValidationErrorConstraint{ + "required": openapi.Required, + "min": openapi.Min, + "max": openapi.Max, + "min_length": openapi.MinLength, + "max_length": openapi.MaxLength, + "pattern": openapi.Pattern, + "enum": openapi.Enum, + "format": openapi.Format, + "unique": openapi.Unique, +} + +// isValidConstraint checks if the given constraint string is a valid ValidationErrorConstraint +// and returns the corresponding enum value if valid +func isValidConstraint(constraint string) (openapi.ValidationErrorConstraint, bool) { + if c, ok := validConstraints[constraint]; ok { + return c, true + } + return "", false +} + +// AsProblemDetails converts the ServiceError to RFC 9457 Problem Details format +func (e *ServiceError) AsProblemDetails(instance string, traceID string) openapi.Error { + now := time.Now().UTC() + problemDetails := openapi.Error{ + Type: e.Type, + Title: e.Title, + Status: e.HttpCode, + Detail: &e.Reason, + Instance: &instance, + Code: &e.RFC9457Code, + Timestamp: &now, + TraceId: &traceID, } - // Add validation details if present + // Add validation errors if present if len(e.Details) > 0 { - details := make([]struct { - Error *string `json:"error,omitempty"` - Field *string `json:"field,omitempty"` - }, len(e.Details)) + validationErrors := make([]openapi.ValidationError, len(e.Details)) for i, detail := range e.Details { - details[i] = struct { - Error *string `json:"error,omitempty"` - Field *string `json:"field,omitempty"` - }{ - Field: util.PtrString(detail.Field), - Error: util.PtrString(detail.Error), + validationErrors[i] = openapi.ValidationError{ + Field: detail.Field, + Message: detail.Message, + Value: detail.Value, + } + if detail.Constraint != "" { + if constraint, ok := isValidConstraint(detail.Constraint); ok { + validationErrors[i].Constraint = &constraint + } } } - openapiErr.Details = &details + problemDetails.Errors = &validationErrors } - return openapiErr + return problemDetails } -func CodeStr(code ServiceErrorCode) *string { - return util.PtrString(fmt.Sprintf("%s-%d", ErrorCodePrefix, code)) -} - -func Href(code ServiceErrorCode) *string { - return util.PtrString(fmt.Sprintf("%s%d", ErrorHref, code)) -} +// Constructor functions func NotFound(reason string, values ...interface{}) *ServiceError { return New(ErrorNotFound, reason, values...) @@ -211,6 +297,11 @@ func Forbidden(reason string, values ...interface{}) *ServiceError { return New(ErrorForbidden, reason, values...) } +func NotImplementedError(reason string, values ...interface{}) *ServiceError { + return New(ErrorNotImplemented, reason, values...) +} + +// NotImplemented is an alias for NotImplementedError for backwards compatibility func NotImplemented(reason string, values ...interface{}) *ServiceError { return New(ErrorNotImplemented, reason, values...) } @@ -244,5 +335,5 @@ func FailedToParseSearch(reason string, values ...interface{}) *ServiceError { } func DatabaseAdvisoryLock(err error) *ServiceError { - return New(ErrorDatabaseAdvisoryLock, err.Error(), []string{}) + return New(ErrorDatabaseAdvisoryLock, "%s", err.Error()) } diff --git a/pkg/handlers/framework.go b/pkg/handlers/framework.go index 87a0403..b19cc27 100755 --- a/pkg/handlers/framework.go +++ b/pkg/handlers/framework.go @@ -29,20 +29,23 @@ type errorHandlerFunc func(r *http.Request, w http.ResponseWriter, err *errors.S type httpAction func() (interface{}, *errors.ServiceError) func handleError(r *http.Request, w http.ResponseWriter, err *errors.ServiceError) { - requestID, _ := logger.GetRequestID(r.Context()) - // If this is a 400 error, its the user's issue, log as info rather than error + traceID, _ := logger.GetRequestID(r.Context()) + instance := r.URL.Path + + // Log with RFC 9457 code format if err.HttpCode >= 400 && err.HttpCode <= 499 { logger.With(r.Context(), - "code", err.Code, + "code", err.RFC9457Code, "http_code", err.HttpCode, "reason", err.Reason).Info("Client error response") } else { logger.With(r.Context(), - "code", err.Code, + "code", err.RFC9457Code, "http_code", err.HttpCode, "reason", err.Reason).Error("Server error response") } - writeJSONResponse(w, r, err.HttpCode, err.AsOpenapiError(requestID)) + + writeProblemDetailsResponse(w, r, err.HttpCode, err.AsProblemDetails(instance, traceID)) } func handle(w http.ResponseWriter, r *http.Request, cfg *handlerConfig, httpStatus int) { diff --git a/pkg/handlers/helpers.go b/pkg/handlers/helpers.go index acecac8..79bef30 100755 --- a/pkg/handlers/helpers.go +++ b/pkg/handlers/helpers.go @@ -38,4 +38,32 @@ func writeJSONResponse(w http.ResponseWriter, r *http.Request, code int, payload } } +// writeProblemDetailsResponse writes an RFC 9457 Problem Details response +func writeProblemDetailsResponse(w http.ResponseWriter, r *http.Request, code int, payload interface{}) { + w.Header().Set("Content-Type", "application/problem+json") + w.Header().Set("Vary", "Authorization") + + w.WriteHeader(code) + + if payload != nil { + response, err := json.Marshal(payload) + if err != nil { + logger.With(r.Context(), + logger.HTTPPath(r.URL.Path), + logger.HTTPMethod(r.Method), + logger.HTTPStatusCode(code), + ).WithError(err).Error("Failed to marshal Problem Details response payload") + return + } + if _, err := w.Write(response); err != nil { + logger.With(r.Context(), + logger.HTTPPath(r.URL.Path), + logger.HTTPMethod(r.Method), + logger.HTTPStatusCode(code), + ).WithError(err).Error("Failed to write Problem Details response body") + return + } + } +} + // Prepare a 'list' of non-db-backed resources diff --git a/pkg/middleware/schema_validation.go b/pkg/middleware/schema_validation.go index 217e2ec..6dab5d9 100644 --- a/pkg/middleware/schema_validation.go +++ b/pkg/middleware/schema_validation.go @@ -12,22 +12,22 @@ import ( "github.com/openshift-hyperfleet/hyperfleet-api/pkg/validators" ) -// handleValidationError writes validation error response +// handleValidationError writes validation error response in RFC 9457 Problem Details format func handleValidationError(w http.ResponseWriter, r *http.Request, err *errors.ServiceError) { - requestID, ok := logger.GetRequestID(r.Context()) + traceID, ok := logger.GetRequestID(r.Context()) if !ok { - requestID = "unknown" + traceID = "unknown" } // Log validation errors as warn (client error, not server error) logger.With(r.Context(), - "operation_id", requestID, + "trace_id", traceID, ).WithError(err).Warn("Validation error") - // Write JSON error response - w.Header().Set("Content-Type", "application/json") + // Write RFC 9457 Problem Details error response + w.Header().Set("Content-Type", "application/problem+json") w.WriteHeader(err.HttpCode) - if encodeErr := json.NewEncoder(w).Encode(err.AsOpenapiError(requestID)); encodeErr != nil { + if encodeErr := json.NewEncoder(w).Encode(err.AsProblemDetails(r.URL.Path, traceID)); encodeErr != nil { logger.With(r.Context(), logger.HTTPPath(r.URL.Path), logger.HTTPMethod(r.Method), diff --git a/pkg/middleware/schema_validation_test.go b/pkg/middleware/schema_validation_test.go index 7a94202..27e3908 100644 --- a/pkg/middleware/schema_validation_test.go +++ b/pkg/middleware/schema_validation_test.go @@ -109,7 +109,7 @@ func TestSchemaValidationMiddleware_PostRequestInvalidSpec(t *testing.T) { err := json.Unmarshal(rr.Body.Bytes(), &errorResponse) Expect(err).To(BeNil()) Expect(errorResponse.Code).ToNot(BeNil()) - Expect(errorResponse.Reason).ToNot(BeNil()) + Expect(errorResponse.Detail).ToNot(BeNil()) } func TestSchemaValidationMiddleware_PatchRequestValidation(t *testing.T) { @@ -348,7 +348,7 @@ func TestSchemaValidationMiddleware_InvalidSpecType(t *testing.T) { var errorResponse openapi.Error err := json.Unmarshal(rr.Body.Bytes(), &errorResponse) Expect(err).To(BeNil()) - Expect(*errorResponse.Reason).To(ContainSubstring("spec field must be an object")) + Expect(*errorResponse.Detail).To(ContainSubstring("spec field must be an object")) } func TestSchemaValidationMiddleware_MalformedJSON(t *testing.T) { diff --git a/pkg/services/generic_test.go b/pkg/services/generic_test.go index f21c9df..83b831a 100755 --- a/pkg/services/generic_test.go +++ b/pkg/services/generic_test.go @@ -29,11 +29,11 @@ func TestSQLTranslation(t *testing.T) { tests := []map[string]interface{}{ { "search": "garbage", - "error": "hyperfleet-21: Failed to parse search query: garbage", + "error": "HYPERFLEET-VAL-005: Failed to parse search query: garbage", }, { "search": "spec = '{}'", - "error": "hyperfleet-21: spec is not a valid field name", + "error": "HYPERFLEET-VAL-005: spec is not a valid field name", }, } for _, test := range tests { diff --git a/pkg/validators/schema_validator.go b/pkg/validators/schema_validator.go index d3886cc..1180030 100644 --- a/pkg/validators/schema_validator.go +++ b/pkg/validators/schema_validator.go @@ -137,15 +137,15 @@ func convertValidationError(err error, prefix string) []errors.ValidationDetail // - "property 'unknownField' is unsupported" // - "number must be at least 10" details = append(details, errors.ValidationDetail{ - Field: field, - Error: e.Reason, + Field: field, + Message: e.Reason, }) default: // Fallback for unknown error types // Error message already contains the full description details = append(details, errors.ValidationDetail{ - Field: prefix, - Error: err.Error(), + Field: prefix, + Message: err.Error(), }) } diff --git a/pkg/validators/schema_validator_test.go b/pkg/validators/schema_validator_test.go index 34ca2de..7f48624 100644 --- a/pkg/validators/schema_validator_test.go +++ b/pkg/validators/schema_validator_test.go @@ -184,7 +184,7 @@ func TestValidateClusterSpec_InvalidEnum(t *testing.T) { Expect(serviceErr.Details).ToNot(BeEmpty()) // Verify we get validation details (field path extraction is tested separately) - Expect(serviceErr.Details[0].Error).ToNot(BeEmpty()) + Expect(serviceErr.Details[0].Message).ToNot(BeEmpty()) } func TestValidateClusterSpec_InvalidNestedField(t *testing.T) { diff --git a/test/integration/clusters_test.go b/test/integration/clusters_test.go index 1ad896a..1263371 100644 --- a/test/integration/clusters_test.go +++ b/test/integration/clusters_test.go @@ -365,7 +365,7 @@ func TestClusterSchemaValidation(t *testing.T) { var errorResponse openapi.Error _ = json.Unmarshal(resp2.Body(), &errorResponse) Expect(errorResponse.Code).ToNot(BeNil()) - Expect(errorResponse.Reason).ToNot(BeNil()) + Expect(errorResponse.Detail).ToNot(BeNil()) } else { t.Logf("Base schema may accept any spec type, status: %d", resp2.StatusCode()) } @@ -435,14 +435,14 @@ func TestClusterSchemaValidationWithProviderSchema(t *testing.T) { } Expect(errorResponse.Code).ToNot(BeNil()) - Expect(*errorResponse.Code).To(Equal("hyperfleet-8")) // Validation error code - Expect(errorResponse.Details).ToNot(BeEmpty(), "Should include field-level error details") + Expect(*errorResponse.Code).To(Equal("HYPERFLEET-VAL-000")) // Validation error code (RFC 9457 format) + Expect(errorResponse.Errors).ToNot(BeEmpty(), "Should include field-level error details") - // Verify details contain field path + // Verify errors contain field path foundRegionError := false - if errorResponse.Details != nil { - for _, detail := range *errorResponse.Details { - if detail.Field != nil && strings.Contains(*detail.Field, "region") { + if errorResponse.Errors != nil { + for _, detail := range *errorResponse.Errors { + if strings.Contains(detail.Field, "region") { foundRegionError = true break } @@ -488,23 +488,23 @@ func TestClusterSchemaValidationErrorDetails(t *testing.T) { t.Fatalf("failed to unmarshal error response: %v, response body: %s", err, string(resp.Body())) } - // Verify error structure - Expect(errorResponse.Kind).ToNot(BeNil()) - Expect(*errorResponse.Kind).To(Equal("Error")) + // Verify error structure (RFC 9457 Problem Details format) + Expect(errorResponse.Type).ToNot(BeEmpty()) + Expect(errorResponse.Title).ToNot(BeEmpty()) Expect(errorResponse.Code).ToNot(BeNil()) - // Both hyperfleet-8 (validation error) and hyperfleet-17 (invalid request format) are acceptable + // Both HYPERFLEET-VAL-000 (validation error) and HYPERFLEET-VAL-006 (malformed request) are acceptable // as they both indicate the spec field is invalid - validCodes := []string{"hyperfleet-8", "hyperfleet-17"} + validCodes := []string{"HYPERFLEET-VAL-000", "HYPERFLEET-VAL-006"} Expect(validCodes).To(ContainElement(*errorResponse.Code), "Expected validation or format error code") - Expect(errorResponse.Reason).ToNot(BeNil()) - Expect(*errorResponse.Reason).To(ContainSubstring("spec")) + Expect(errorResponse.Detail).ToNot(BeNil()) + Expect(*errorResponse.Detail).To(ContainSubstring("spec")) - Expect(errorResponse.Href).ToNot(BeNil()) - Expect(errorResponse.OperationId).ToNot(BeNil()) + Expect(errorResponse.Instance).ToNot(BeNil()) + Expect(errorResponse.TraceId).ToNot(BeNil()) - t.Logf("Error response: code=%s, reason=%s", *errorResponse.Code, *errorResponse.Reason) + t.Logf("Error response: code=%s, detail=%s", *errorResponse.Code, *errorResponse.Detail) } // TestClusterList_DefaultSorting tests that clusters are sorted by created_time desc by default @@ -699,10 +699,10 @@ func TestClusterPost_EmptyKind(t *testing.T) { err = json.Unmarshal(restyResp.Body(), &errorResponse) Expect(err).ToNot(HaveOccurred()) - // Verify error message contains "kind is required" - reason, ok := errorResponse["reason"].(string) + // Verify error message contains "kind is required" (RFC 9457 uses "detail" field) + detail, ok := errorResponse["detail"].(string) Expect(ok).To(BeTrue()) - Expect(reason).To(ContainSubstring("kind is required")) + Expect(detail).To(ContainSubstring("kind is required")) } // TestClusterPost_WrongKind tests that wrong kind field returns 400 @@ -734,8 +734,8 @@ func TestClusterPost_WrongKind(t *testing.T) { err = json.Unmarshal(restyResp.Body(), &errorResponse) Expect(err).ToNot(HaveOccurred()) - // Verify error message contains "kind must be 'Cluster'" - reason, ok := errorResponse["reason"].(string) + // Verify error message contains "kind must be 'Cluster'" (RFC 9457 uses "detail" field) + detail, ok := errorResponse["detail"].(string) Expect(ok).To(BeTrue()) - Expect(reason).To(ContainSubstring("kind must be 'Cluster'")) + Expect(detail).To(ContainSubstring("kind must be 'Cluster'")) } diff --git a/test/integration/node_pools_test.go b/test/integration/node_pools_test.go index 3680a71..d90601f 100644 --- a/test/integration/node_pools_test.go +++ b/test/integration/node_pools_test.go @@ -263,10 +263,10 @@ func TestNodePoolPost_EmptyKind(t *testing.T) { err = json.Unmarshal(restyResp.Body(), &errorResponse) Expect(err).ToNot(HaveOccurred()) - // Verify error message contains "kind is required" - reason, ok := errorResponse["reason"].(string) + // Verify error message contains "kind is required" (RFC 9457 uses "detail" field) + detail, ok := errorResponse["detail"].(string) Expect(ok).To(BeTrue()) - Expect(reason).To(ContainSubstring("kind is required")) + Expect(detail).To(ContainSubstring("kind is required")) } // TestNodePoolPost_WrongKind tests that wrong kind field returns 400 @@ -302,8 +302,8 @@ func TestNodePoolPost_WrongKind(t *testing.T) { err = json.Unmarshal(restyResp.Body(), &errorResponse) Expect(err).ToNot(HaveOccurred()) - // Verify error message contains "kind must be 'NodePool'" - reason, ok := errorResponse["reason"].(string) + // Verify error message contains "kind must be 'NodePool'" (RFC 9457 uses "detail" field) + detail, ok := errorResponse["detail"].(string) Expect(ok).To(BeTrue()) - Expect(reason).To(ContainSubstring("kind must be 'NodePool'")) + Expect(detail).To(ContainSubstring("kind must be 'NodePool'")) } From fa9484a53a877e4bb19ea7bdea70326a891d34bc Mon Sep 17 00:00:00 2001 From: Rafael Benevides Date: Thu, 15 Jan 2026 18:04:18 -0300 Subject: [PATCH 2/6] HYPERFLEET-452 - refactor: address PR review feedback - Remove legacy ServiceErrorCode type and numeric error constants - Use RFC 9457 codes (HYPERFLEET-XXX-NNN) as map keys directly - Consolidate WriteProblemDetailsResponse into pkg/api/response - Fix SendPanic to include trace_id and timestamp dynamically - Fix CodeAuthExpiredToken to return 401 instead of 403 - Remove unused NotImplementedError function - Use error code constants in tests instead of hardcoded strings --- pkg/api/error.go | 28 ++++++- pkg/api/response/problem_details.go | 37 +++++++++ pkg/auth/auth_middleware.go | 2 +- pkg/auth/helpers.go | 23 +----- pkg/db/transaction_middleware.go | 21 +---- pkg/errors/errors.go | 119 +++++++++++----------------- pkg/errors/errors_test.go | 11 +-- pkg/handlers/framework.go | 3 +- pkg/handlers/helpers.go | 28 ------- pkg/services/generic_test.go | 6 +- test/factories/node_pools.go | 4 +- 11 files changed, 130 insertions(+), 152 deletions(-) create mode 100644 pkg/api/response/problem_details.go diff --git a/pkg/api/error.go b/pkg/api/error.go index 8ccf4d0..e53995b 100755 --- a/pkg/api/error.go +++ b/pkg/api/error.go @@ -81,10 +81,36 @@ func SendUnauthorized(w http.ResponseWriter, r *http.Request, message string) { } // SendPanic sends a panic error response in RFC 9457 Problem Details format. +// It attempts to include trace_id and timestamp dynamically, falling back to +// a pre-computed body if marshaling fails. func SendPanic(w http.ResponseWriter, r *http.Request) { w.Header().Set("Content-Type", "application/problem+json") w.WriteHeader(http.StatusInternalServerError) - _, err := w.Write(panicBody) + + // Try to generate a complete response with trace_id and timestamp + traceID, _ := logger.GetRequestID(r.Context()) + now := time.Now().UTC() + detail := "An unexpected error happened, please check the log of the service for details" + instance := r.URL.Path + + panicError := openapi.Error{ + Type: errors.ErrorTypeInternal, + Title: "Internal Server Error", + Status: http.StatusInternalServerError, + Detail: &detail, + Instance: &instance, + Code: ptrString(errors.CodeInternalGeneral), + Timestamp: &now, + TraceId: &traceID, + } + + data, err := json.Marshal(panicError) + if err != nil { + // Fallback to pre-computed body without trace_id/timestamp + data = panicBody + } + + _, err = w.Write(data) if err != nil { err = fmt.Errorf( "can't send panic response for request '%s': %s", diff --git a/pkg/api/response/problem_details.go b/pkg/api/response/problem_details.go new file mode 100644 index 0000000..a812950 --- /dev/null +++ b/pkg/api/response/problem_details.go @@ -0,0 +1,37 @@ +package response + +import ( + "context" + "encoding/json" + "net/http" + + "github.com/openshift-hyperfleet/hyperfleet-api/pkg/logger" +) + +// WriteProblemDetailsResponse writes an RFC 9457 Problem Details response +func WriteProblemDetailsResponse(w http.ResponseWriter, r *http.Request, code int, payload interface{}) { + w.Header().Set("Content-Type", "application/problem+json") + w.Header().Set("Vary", "Authorization") + + w.WriteHeader(code) + + if payload != nil { + response, err := json.Marshal(payload) + if err != nil { + logResponseError(r.Context(), r, code, "Failed to marshal Problem Details response payload", err) + return + } + if _, err := w.Write(response); err != nil { + logResponseError(r.Context(), r, code, "Failed to write Problem Details response body", err) + return + } + } +} + +func logResponseError(ctx context.Context, r *http.Request, code int, message string, err error) { + logger.With(ctx, + logger.HTTPPath(r.URL.Path), + logger.HTTPMethod(r.Method), + logger.HTTPStatusCode(code), + ).WithError(err).Error(message) +} diff --git a/pkg/auth/auth_middleware.go b/pkg/auth/auth_middleware.go index 2b4eb24..c38804e 100755 --- a/pkg/auth/auth_middleware.go +++ b/pkg/auth/auth_middleware.go @@ -26,7 +26,7 @@ func (a *Middleware) AuthenticateAccountJWT(next http.Handler) http.Handler { ctx := r.Context() payload, err := GetAuthPayload(r) if err != nil { - handleError(ctx, w, r, errors.ErrorUnauthenticated, fmt.Sprintf("Unable to get payload details from JWT token: %s", err)) + handleError(ctx, w, r, errors.CodeAuthNoCredentials, fmt.Sprintf("Unable to get payload details from JWT token: %s", err)) return } diff --git a/pkg/auth/helpers.go b/pkg/auth/helpers.go index 169459e..1ee6a4f 100755 --- a/pkg/auth/helpers.go +++ b/pkg/auth/helpers.go @@ -2,14 +2,14 @@ package auth import ( "context" - "encoding/json" "net/http" + "github.com/openshift-hyperfleet/hyperfleet-api/pkg/api/response" "github.com/openshift-hyperfleet/hyperfleet-api/pkg/errors" "github.com/openshift-hyperfleet/hyperfleet-api/pkg/logger" ) -func handleError(ctx context.Context, w http.ResponseWriter, r *http.Request, code errors.ServiceErrorCode, reason string) { +func handleError(ctx context.Context, w http.ResponseWriter, r *http.Request, code string, reason string) { traceID, ok := logger.GetRequestID(ctx) if !ok { traceID = "unknown" @@ -25,22 +25,5 @@ func handleError(ctx context.Context, w http.ResponseWriter, r *http.Request, co logger.WithError(ctx, err).Error("Server error occurred") } - writeProblemDetailsResponse(w, err.HttpCode, err.AsProblemDetails(instance, traceID)) -} - -func writeProblemDetailsResponse(w http.ResponseWriter, code int, payload interface{}) { - w.Header().Set("Content-Type", "application/problem+json") - w.WriteHeader(code) - - if payload != nil { - response, err := json.Marshal(payload) - if err != nil { - // Headers already sent, can't change status code - return - } - if _, err := w.Write(response); err != nil { - // Writing failed, nothing we can do at this point - return - } - } + response.WriteProblemDetailsResponse(w, r, err.HttpCode, err.AsProblemDetails(instance, traceID)) } diff --git a/pkg/db/transaction_middleware.go b/pkg/db/transaction_middleware.go index 8b06c83..a481a44 100755 --- a/pkg/db/transaction_middleware.go +++ b/pkg/db/transaction_middleware.go @@ -1,9 +1,9 @@ package db import ( - "encoding/json" "net/http" + "github.com/openshift-hyperfleet/hyperfleet-api/pkg/api/response" "github.com/openshift-hyperfleet/hyperfleet-api/pkg/db/db_context" "github.com/openshift-hyperfleet/hyperfleet-api/pkg/errors" "github.com/openshift-hyperfleet/hyperfleet-api/pkg/logger" @@ -19,7 +19,7 @@ func TransactionMiddleware(next http.Handler, connection SessionFactory) http.Ha // use default error to avoid exposing internals to users serviceErr := errors.GeneralError("") traceID, _ := logger.GetRequestID(r.Context()) - writeProblemDetailsResponse(w, serviceErr.HttpCode, serviceErr.AsProblemDetails(r.URL.Path, traceID)) + response.WriteProblemDetailsResponse(w, r, serviceErr.HttpCode, serviceErr.AsProblemDetails(r.URL.Path, traceID)) return } @@ -34,20 +34,3 @@ func TransactionMiddleware(next http.Handler, connection SessionFactory) http.Ha next.ServeHTTP(w, r) }) } - -func writeProblemDetailsResponse(w http.ResponseWriter, code int, payload interface{}) { - w.Header().Set("Content-Type", "application/problem+json") - w.WriteHeader(code) - - if payload != nil { - response, err := json.Marshal(payload) - if err != nil { - // Log error but don't expose to client since headers already sent - return - } - if _, err := w.Write(response); err != nil { - // Response writing failed, nothing we can do at this point - return - } - } -} diff --git a/pkg/errors/errors.go b/pkg/errors/errors.go index 0dec39c..e9e1c7e 100755 --- a/pkg/errors/errors.go +++ b/pkg/errors/errors.go @@ -71,26 +71,6 @@ const ( CodeNotImplemented = "HYPERFLEET-INT-003" ) -// ServiceErrorCode is kept for backwards compatibility -type ServiceErrorCode int - -// Legacy error codes mapped to new format -const ( - ErrorInvalidToken ServiceErrorCode = 1 - ErrorForbidden ServiceErrorCode = 4 - ErrorConflict ServiceErrorCode = 6 - ErrorNotFound ServiceErrorCode = 7 - ErrorValidation ServiceErrorCode = 8 - ErrorGeneral ServiceErrorCode = 9 - ErrorNotImplemented ServiceErrorCode = 10 - ErrorUnauthorized ServiceErrorCode = 11 - ErrorUnauthenticated ServiceErrorCode = 15 - ErrorMalformedRequest ServiceErrorCode = 17 - ErrorBadRequest ServiceErrorCode = 21 - ErrorFailedToParseSearch ServiceErrorCode = 23 - ErrorDatabaseAdvisoryLock ServiceErrorCode = 26 -) - type ServiceErrors []ServiceError // ValidationDetail represents a single field validation error (RFC 9457 format) @@ -103,9 +83,7 @@ type ValidationDetail struct { // ServiceError represents an API error with RFC 9457 Problem Details support type ServiceError struct { - // Code is the legacy numeric error code - Code ServiceErrorCode - // RFC9457Code is the new HYPERFLEET-CAT-NUM format code + // RFC9457Code is the HYPERFLEET-CAT-NUM format code RFC9457Code string // Type is the RFC 9457 type URI Type string @@ -121,38 +99,37 @@ type ServiceError struct { // errorDefinition holds the default values for each error type type errorDefinition struct { - Code ServiceErrorCode - RFC9457Code string - Type string - Title string - Reason string - HttpCode int + Type string + Title string + Reason string + HttpCode int } -var errorDefinitions = map[ServiceErrorCode]errorDefinition{ - ErrorInvalidToken: {ErrorInvalidToken, CodeAuthExpiredToken, ErrorTypeAuth, "Invalid Token", "Invalid token provided", http.StatusForbidden}, - ErrorForbidden: {ErrorForbidden, CodeAuthzForbidden, ErrorTypeAuthz, "Forbidden", "Forbidden to perform this action", http.StatusForbidden}, - ErrorConflict: {ErrorConflict, CodeConflictExists, ErrorTypeConflict, "Resource Conflict", "An entity with the specified unique values already exists", http.StatusConflict}, - ErrorNotFound: {ErrorNotFound, CodeNotFoundGeneric, ErrorTypeNotFound, "Resource Not Found", "Resource not found", http.StatusNotFound}, - ErrorValidation: {ErrorValidation, CodeValidationMultiple, ErrorTypeValidation, "Validation Failed", "General validation failure", http.StatusBadRequest}, - ErrorGeneral: {ErrorGeneral, CodeInternalGeneral, ErrorTypeInternal, "Internal Server Error", "Unspecified error", http.StatusInternalServerError}, - ErrorNotImplemented: {ErrorNotImplemented, CodeNotImplemented, ErrorTypeNotImpl, "Not Implemented", "Functionality not implemented", http.StatusNotImplemented}, - ErrorUnauthorized: {ErrorUnauthorized, CodeAuthzInsufficient, ErrorTypeAuthz, "Unauthorized", "Account is unauthorized to perform this action", http.StatusForbidden}, - ErrorUnauthenticated: {ErrorUnauthenticated, CodeAuthNoCredentials, ErrorTypeAuth, "Authentication Required", "Account authentication could not be verified", http.StatusUnauthorized}, - ErrorMalformedRequest: {ErrorMalformedRequest, CodeMalformedBody, ErrorTypeMalformed, "Malformed Request", "Unable to read request body", http.StatusBadRequest}, - ErrorBadRequest: {ErrorBadRequest, CodeBadRequest, ErrorTypeBadRequest, "Bad Request", "Bad request", http.StatusBadRequest}, - ErrorFailedToParseSearch: {ErrorFailedToParseSearch, CodeSearchParseFail, ErrorTypeValidation, "Invalid Search Query", "Failed to parse search query", http.StatusBadRequest}, - ErrorDatabaseAdvisoryLock: {ErrorDatabaseAdvisoryLock, CodeInternalDatabase, ErrorTypeInternal, "Database Error", "Database advisory lock error", http.StatusInternalServerError}, +var errorDefinitions = map[string]errorDefinition{ + CodeAuthExpiredToken: {ErrorTypeAuth, "Invalid Token", "Invalid token provided", http.StatusUnauthorized}, + CodeAuthzForbidden: {ErrorTypeAuthz, "Forbidden", "Forbidden to perform this action", http.StatusForbidden}, + CodeConflictExists: {ErrorTypeConflict, "Resource Conflict", "An entity with the specified unique values already exists", http.StatusConflict}, + CodeNotFoundGeneric: {ErrorTypeNotFound, "Resource Not Found", "Resource not found", http.StatusNotFound}, + CodeValidationMultiple: {ErrorTypeValidation, "Validation Failed", "General validation failure", http.StatusBadRequest}, + CodeInternalGeneral: {ErrorTypeInternal, "Internal Server Error", "Unspecified error", http.StatusInternalServerError}, + CodeNotImplemented: {ErrorTypeNotImpl, "Not Implemented", "Functionality not implemented", http.StatusNotImplemented}, + CodeAuthzInsufficient: {ErrorTypeAuthz, "Unauthorized", "Account is unauthorized to perform this action", http.StatusForbidden}, + CodeAuthNoCredentials: {ErrorTypeAuth, "Authentication Required", "Account authentication could not be verified", http.StatusUnauthorized}, + CodeMalformedBody: {ErrorTypeMalformed, "Malformed Request", "Unable to read request body", http.StatusBadRequest}, + CodeBadRequest: {ErrorTypeBadRequest, "Bad Request", "Bad request", http.StatusBadRequest}, + CodeSearchParseFail: {ErrorTypeValidation, "Invalid Search Query", "Failed to parse search query", http.StatusBadRequest}, + CodeInternalDatabase: {ErrorTypeInternal, "Database Error", "Database advisory lock error", http.StatusInternalServerError}, + CodeAuthInvalidCredentials: {ErrorTypeAuth, "Invalid Credentials", "The provided credentials are invalid", http.StatusUnauthorized}, } -func Find(code ServiceErrorCode) (bool, *ServiceError) { +// Find looks up an error definition by its RFC 9457 code +func Find(code string) (bool, *ServiceError) { def, exists := errorDefinitions[code] if !exists { return false, nil } return true, &ServiceError{ - Code: def.Code, - RFC9457Code: def.RFC9457Code, + RFC9457Code: code, Type: def.Type, Title: def.Title, Reason: def.Reason, @@ -160,12 +137,12 @@ func Find(code ServiceErrorCode) (bool, *ServiceError) { } } +// Errors returns all defined errors func Errors() ServiceErrors { errors := make(ServiceErrors, 0, len(errorDefinitions)) - for _, def := range errorDefinitions { + for code, def := range errorDefinitions { errors = append(errors, ServiceError{ - Code: def.Code, - RFC9457Code: def.RFC9457Code, + RFC9457Code: code, Type: def.Type, Title: def.Title, Reason: def.Reason, @@ -176,13 +153,12 @@ func Errors() ServiceErrors { } // New creates a new ServiceError with optional custom reason -func New(code ServiceErrorCode, reason string, values ...interface{}) *ServiceError { +func New(code string, reason string, values ...interface{}) *ServiceError { exists, err := Find(code) if !exists { ctx := context.Background() logger.With(ctx, logger.FieldErrorCode, code).Error("Undefined error code used") err = &ServiceError{ - Code: ErrorGeneral, RFC9457Code: CodeInternalGeneral, Type: ErrorTypeInternal, Title: "Internal Server Error", @@ -207,15 +183,15 @@ func (e *ServiceError) AsError() error { } func (e *ServiceError) Is404() bool { - return e.Code == ErrorNotFound + return e.Type == ErrorTypeNotFound } func (e *ServiceError) IsConflict() bool { - return e.Code == ErrorConflict + return e.Type == ErrorTypeConflict } func (e *ServiceError) IsForbidden() bool { - return e.Code == ErrorForbidden + return e.Type == ErrorTypeAuthz } // validConstraints maps string values to their corresponding ValidationErrorConstraint enum values @@ -278,62 +254,61 @@ func (e *ServiceError) AsProblemDetails(instance string, traceID string) openapi // Constructor functions func NotFound(reason string, values ...interface{}) *ServiceError { - return New(ErrorNotFound, reason, values...) + return New(CodeNotFoundGeneric, reason, values...) } func GeneralError(reason string, values ...interface{}) *ServiceError { - return New(ErrorGeneral, reason, values...) + return New(CodeInternalGeneral, reason, values...) } func Unauthorized(reason string, values ...interface{}) *ServiceError { - return New(ErrorUnauthorized, reason, values...) + return New(CodeAuthzInsufficient, reason, values...) } func Unauthenticated(reason string, values ...interface{}) *ServiceError { - return New(ErrorUnauthenticated, reason, values...) + return New(CodeAuthNoCredentials, reason, values...) } func Forbidden(reason string, values ...interface{}) *ServiceError { - return New(ErrorForbidden, reason, values...) -} - -func NotImplementedError(reason string, values ...interface{}) *ServiceError { - return New(ErrorNotImplemented, reason, values...) + return New(CodeAuthzForbidden, reason, values...) } -// NotImplemented is an alias for NotImplementedError for backwards compatibility func NotImplemented(reason string, values ...interface{}) *ServiceError { - return New(ErrorNotImplemented, reason, values...) + return New(CodeNotImplemented, reason, values...) } func Conflict(reason string, values ...interface{}) *ServiceError { - return New(ErrorConflict, reason, values...) + return New(CodeConflictExists, reason, values...) } func Validation(reason string, values ...interface{}) *ServiceError { - return New(ErrorValidation, reason, values...) + return New(CodeValidationMultiple, reason, values...) } // ValidationWithDetails creates a validation error with field-level details func ValidationWithDetails(reason string, details []ValidationDetail) *ServiceError { - err := New(ErrorValidation, "%s", reason) + err := New(CodeValidationMultiple, "%s", reason) err.Details = details return err } func MalformedRequest(reason string, values ...interface{}) *ServiceError { - return New(ErrorMalformedRequest, reason, values...) + return New(CodeMalformedBody, reason, values...) } func BadRequest(reason string, values ...interface{}) *ServiceError { - return New(ErrorBadRequest, reason, values...) + return New(CodeBadRequest, reason, values...) } func FailedToParseSearch(reason string, values ...interface{}) *ServiceError { message := fmt.Sprintf("Failed to parse search query: %s", reason) - return New(ErrorFailedToParseSearch, message, values...) + return New(CodeSearchParseFail, message, values...) } func DatabaseAdvisoryLock(err error) *ServiceError { - return New(ErrorDatabaseAdvisoryLock, "%s", err.Error()) + return New(CodeInternalDatabase, "%s", err.Error()) +} + +func InvalidToken(reason string, values ...interface{}) *ServiceError { + return New(CodeAuthExpiredToken, reason, values...) } diff --git a/pkg/errors/errors_test.go b/pkg/errors/errors_test.go index a6fe11d..c37b7da 100755 --- a/pkg/errors/errors_test.go +++ b/pkg/errors/errors_test.go @@ -8,18 +8,19 @@ import ( func TestErrorFormatting(t *testing.T) { RegisterTestingT(t) - err := New(ErrorGeneral, "test %s, %d", "errors", 1) + err := New(CodeInternalGeneral, "test %s, %d", "errors", 1) Expect(err.Reason).To(Equal("test errors, 1")) } func TestErrorFind(t *testing.T) { RegisterTestingT(t) - exists, err := Find(ErrorNotFound) + exists, err := Find(CodeNotFoundGeneric) Expect(exists).To(Equal(true)) - Expect(err.Code).To(Equal(ErrorNotFound)) + Expect(err.Type).To(Equal(ErrorTypeNotFound)) + Expect(err.RFC9457Code).To(Equal(CodeNotFoundGeneric)) - // Hopefully we never reach 91,823,719 error codes or this test will fail - exists, err = Find(ServiceErrorCode(91823719)) + // Test with invalid code + exists, err = Find("INVALID-CODE-999") Expect(exists).To(Equal(false)) Expect(err).To(BeNil()) } diff --git a/pkg/handlers/framework.go b/pkg/handlers/framework.go index b19cc27..8e2fd75 100755 --- a/pkg/handlers/framework.go +++ b/pkg/handlers/framework.go @@ -5,6 +5,7 @@ import ( "io" "net/http" + "github.com/openshift-hyperfleet/hyperfleet-api/pkg/api/response" "github.com/openshift-hyperfleet/hyperfleet-api/pkg/errors" "github.com/openshift-hyperfleet/hyperfleet-api/pkg/logger" ) @@ -45,7 +46,7 @@ func handleError(r *http.Request, w http.ResponseWriter, err *errors.ServiceErro "reason", err.Reason).Error("Server error response") } - writeProblemDetailsResponse(w, r, err.HttpCode, err.AsProblemDetails(instance, traceID)) + response.WriteProblemDetailsResponse(w, r, err.HttpCode, err.AsProblemDetails(instance, traceID)) } func handle(w http.ResponseWriter, r *http.Request, cfg *handlerConfig, httpStatus int) { diff --git a/pkg/handlers/helpers.go b/pkg/handlers/helpers.go index 79bef30..acecac8 100755 --- a/pkg/handlers/helpers.go +++ b/pkg/handlers/helpers.go @@ -38,32 +38,4 @@ func writeJSONResponse(w http.ResponseWriter, r *http.Request, code int, payload } } -// writeProblemDetailsResponse writes an RFC 9457 Problem Details response -func writeProblemDetailsResponse(w http.ResponseWriter, r *http.Request, code int, payload interface{}) { - w.Header().Set("Content-Type", "application/problem+json") - w.Header().Set("Vary", "Authorization") - - w.WriteHeader(code) - - if payload != nil { - response, err := json.Marshal(payload) - if err != nil { - logger.With(r.Context(), - logger.HTTPPath(r.URL.Path), - logger.HTTPMethod(r.Method), - logger.HTTPStatusCode(code), - ).WithError(err).Error("Failed to marshal Problem Details response payload") - return - } - if _, err := w.Write(response); err != nil { - logger.With(r.Context(), - logger.HTTPPath(r.URL.Path), - logger.HTTPMethod(r.Method), - logger.HTTPStatusCode(code), - ).WithError(err).Error("Failed to write Problem Details response body") - return - } - } -} - // Prepare a 'list' of non-db-backed resources diff --git a/pkg/services/generic_test.go b/pkg/services/generic_test.go index 83b831a..f66a309 100755 --- a/pkg/services/generic_test.go +++ b/pkg/services/generic_test.go @@ -29,11 +29,11 @@ func TestSQLTranslation(t *testing.T) { tests := []map[string]interface{}{ { "search": "garbage", - "error": "HYPERFLEET-VAL-005: Failed to parse search query: garbage", + "error": errors.CodeBadRequest + ": Failed to parse search query: garbage", }, { "search": "spec = '{}'", - "error": "HYPERFLEET-VAL-005: spec is not a valid field name", + "error": errors.CodeBadRequest + ": spec is not a valid field name", }, } for _, test := range tests { @@ -45,7 +45,7 @@ func TestSQLTranslation(t *testing.T) { d := g.GetInstanceDao(context.Background(), model) _, serviceErr = genericService.buildSearch(listCtx, &d) Expect(serviceErr).To(HaveOccurred()) - Expect(serviceErr.Code).To(Equal(errors.ErrorBadRequest)) + Expect(serviceErr.Type).To(Equal(errors.ErrorTypeBadRequest)) Expect(serviceErr.Error()).To(Equal(errorMsg)) } diff --git a/test/factories/node_pools.go b/test/factories/node_pools.go index 3b4f7dd..c47e614 100644 --- a/test/factories/node_pools.go +++ b/test/factories/node_pools.go @@ -42,8 +42,8 @@ func (f *Factories) NewNodePool(id string) (*api.NodePool, error) { sub, serviceErr := nodePoolService.Create(context.Background(), nodePool) // Check for real errors (not typed nil) - if serviceErr != nil && serviceErr.Code != 0 { - return nil, fmt.Errorf("failed to create nodepool: %s (code: %d)", serviceErr.Reason, serviceErr.Code) + if serviceErr != nil && serviceErr.RFC9457Code != "" { + return nil, fmt.Errorf("failed to create nodepool: %s (code: %s)", serviceErr.Reason, serviceErr.RFC9457Code) } if sub == nil { From 9ced81c5fe6ed8fdbcf0622288dfa7880c8e637f Mon Sep 17 00:00:00 2001 From: Rafael Benevides Date: Thu, 15 Jan 2026 19:38:58 -0300 Subject: [PATCH 3/6] HYPERFLEET-452 - fix: add missing error codes to errorDefinitions map Add all declared error code constants to errorDefinitions map to prevent undefined/500 fallbacks when using New/Find: - CodeValidationRequired, CodeValidationInvalid, CodeValidationFormat, CodeValidationRange - CodeNotFoundCluster, CodeNotFoundNodePool - CodeConflictVersion - CodeRateLimitExceeded - CodeServiceUnavailable, CodeServiceTimeout Also reorganized the map with comments grouping codes by category. --- pkg/errors/errors.go | 51 +++++++++++++++++++++++++++++++++----------- 1 file changed, 38 insertions(+), 13 deletions(-) diff --git a/pkg/errors/errors.go b/pkg/errors/errors.go index e9e1c7e..cfe6f4f 100755 --- a/pkg/errors/errors.go +++ b/pkg/errors/errors.go @@ -106,20 +106,45 @@ type errorDefinition struct { } var errorDefinitions = map[string]errorDefinition{ - CodeAuthExpiredToken: {ErrorTypeAuth, "Invalid Token", "Invalid token provided", http.StatusUnauthorized}, - CodeAuthzForbidden: {ErrorTypeAuthz, "Forbidden", "Forbidden to perform this action", http.StatusForbidden}, - CodeConflictExists: {ErrorTypeConflict, "Resource Conflict", "An entity with the specified unique values already exists", http.StatusConflict}, - CodeNotFoundGeneric: {ErrorTypeNotFound, "Resource Not Found", "Resource not found", http.StatusNotFound}, - CodeValidationMultiple: {ErrorTypeValidation, "Validation Failed", "General validation failure", http.StatusBadRequest}, - CodeInternalGeneral: {ErrorTypeInternal, "Internal Server Error", "Unspecified error", http.StatusInternalServerError}, - CodeNotImplemented: {ErrorTypeNotImpl, "Not Implemented", "Functionality not implemented", http.StatusNotImplemented}, - CodeAuthzInsufficient: {ErrorTypeAuthz, "Unauthorized", "Account is unauthorized to perform this action", http.StatusForbidden}, - CodeAuthNoCredentials: {ErrorTypeAuth, "Authentication Required", "Account authentication could not be verified", http.StatusUnauthorized}, - CodeMalformedBody: {ErrorTypeMalformed, "Malformed Request", "Unable to read request body", http.StatusBadRequest}, - CodeBadRequest: {ErrorTypeBadRequest, "Bad Request", "Bad request", http.StatusBadRequest}, - CodeSearchParseFail: {ErrorTypeValidation, "Invalid Search Query", "Failed to parse search query", http.StatusBadRequest}, - CodeInternalDatabase: {ErrorTypeInternal, "Database Error", "Database advisory lock error", http.StatusInternalServerError}, + // Authentication errors (AUT) - 401 + CodeAuthNoCredentials: {ErrorTypeAuth, "Authentication Required", "Account authentication could not be verified", http.StatusUnauthorized}, CodeAuthInvalidCredentials: {ErrorTypeAuth, "Invalid Credentials", "The provided credentials are invalid", http.StatusUnauthorized}, + CodeAuthExpiredToken: {ErrorTypeAuth, "Invalid Token", "Invalid token provided", http.StatusUnauthorized}, + + // Authorization errors (AUZ) - 403 + CodeAuthzInsufficient: {ErrorTypeAuthz, "Unauthorized", "Account is unauthorized to perform this action", http.StatusForbidden}, + CodeAuthzForbidden: {ErrorTypeAuthz, "Forbidden", "Forbidden to perform this action", http.StatusForbidden}, + + // Validation errors (VAL) - 400 + CodeValidationMultiple: {ErrorTypeValidation, "Validation Failed", "Multiple validation errors occurred", http.StatusBadRequest}, + CodeValidationRequired: {ErrorTypeValidation, "Required Field Missing", "A required field is missing", http.StatusBadRequest}, + CodeValidationInvalid: {ErrorTypeValidation, "Invalid Value", "The provided value is invalid", http.StatusBadRequest}, + CodeValidationFormat: {ErrorTypeValidation, "Invalid Format", "The provided value has an invalid format", http.StatusBadRequest}, + CodeValidationRange: {ErrorTypeValidation, "Value Out of Range", "The provided value is out of the allowed range", http.StatusBadRequest}, + CodeBadRequest: {ErrorTypeBadRequest, "Bad Request", "Bad request", http.StatusBadRequest}, + CodeMalformedBody: {ErrorTypeMalformed, "Malformed Request", "Unable to read request body", http.StatusBadRequest}, + CodeSearchParseFail: {ErrorTypeValidation, "Invalid Search Query", "Failed to parse search query", http.StatusBadRequest}, + + // Not Found errors (NTF) - 404 + CodeNotFoundGeneric: {ErrorTypeNotFound, "Resource Not Found", "Resource not found", http.StatusNotFound}, + CodeNotFoundCluster: {ErrorTypeNotFound, "Cluster Not Found", "The specified cluster was not found", http.StatusNotFound}, + CodeNotFoundNodePool: {ErrorTypeNotFound, "NodePool Not Found", "The specified node pool was not found", http.StatusNotFound}, + + // Conflict errors (CNF) - 409 + CodeConflictExists: {ErrorTypeConflict, "Resource Conflict", "An entity with the specified unique values already exists", http.StatusConflict}, + CodeConflictVersion: {ErrorTypeConflict, "Version Conflict", "The resource version does not match", http.StatusConflict}, + + // Rate Limit errors (LMT) - 429 + CodeRateLimitExceeded: {ErrorTypeRateLimit, "Rate Limit Exceeded", "Too many requests, please try again later", http.StatusTooManyRequests}, + + // Internal errors (INT) - 500/501 + CodeInternalGeneral: {ErrorTypeInternal, "Internal Server Error", "Unspecified error", http.StatusInternalServerError}, + CodeInternalDatabase: {ErrorTypeInternal, "Database Error", "A database error occurred", http.StatusInternalServerError}, + CodeNotImplemented: {ErrorTypeNotImpl, "Not Implemented", "Functionality not implemented", http.StatusNotImplemented}, + + // Service errors (SVC) - 503/504 + CodeServiceUnavailable: {ErrorTypeService, "Service Unavailable", "The service is temporarily unavailable", http.StatusServiceUnavailable}, + CodeServiceTimeout: {ErrorTypeService, "Service Timeout", "The service request timed out", http.StatusGatewayTimeout}, } // Find looks up an error definition by its RFC 9457 code From ce045e334361436a45edf59847343107a1357063 Mon Sep 17 00:00:00 2001 From: Rafael Benevides Date: Thu, 15 Jan 2026 19:50:29 -0300 Subject: [PATCH 4/6] HYPERFLEET-452 - fix: omit empty Instance and TraceId from Problem Details JSON Only set Instance and TraceId pointers in AsProblemDetails when the input values are non-empty, preserving nil to omit fields from JSON output. --- pkg/errors/errors.go | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/pkg/errors/errors.go b/pkg/errors/errors.go index cfe6f4f..d50db53 100755 --- a/pkg/errors/errors.go +++ b/pkg/errors/errors.go @@ -249,10 +249,16 @@ func (e *ServiceError) AsProblemDetails(instance string, traceID string) openapi Title: e.Title, Status: e.HttpCode, Detail: &e.Reason, - Instance: &instance, Code: &e.RFC9457Code, Timestamp: &now, - TraceId: &traceID, + } + + // Only set Instance and TraceId if non-empty to omit from JSON when nil + if instance != "" { + problemDetails.Instance = &instance + } + if traceID != "" { + problemDetails.TraceId = &traceID } // Add validation errors if present From b0738eef2c4cd3b08c171b9d9e2ff87e2e7c970c Mon Sep 17 00:00:00 2001 From: Rafael Benevides Date: Thu, 15 Jan 2026 19:55:09 -0300 Subject: [PATCH 5/6] HYPERFLEET-452 - fix: avoid leaking database error details to clients DatabaseAdvisoryLock now logs the full error server-side for debugging and returns a generic "internal database error" message to clients, preventing sensitive database information from being exposed. --- pkg/errors/errors.go | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/pkg/errors/errors.go b/pkg/errors/errors.go index d50db53..2f99caf 100755 --- a/pkg/errors/errors.go +++ b/pkg/errors/errors.go @@ -337,7 +337,11 @@ func FailedToParseSearch(reason string, values ...interface{}) *ServiceError { } func DatabaseAdvisoryLock(err error) *ServiceError { - return New(CodeInternalDatabase, "%s", err.Error()) + // Log the full error server-side for debugging + ctx := context.Background() + logger.WithError(ctx, err).Error("Database advisory lock error") + // Return a generic message to avoid leaking sensitive database info + return New(CodeInternalDatabase, "internal database error") } func InvalidToken(reason string, values ...interface{}) *ServiceError { From d0c2bfbbc71ab62696049f389e7731035b84ade7 Mon Sep 17 00:00:00 2001 From: Rafael Benevides Date: Thu, 15 Jan 2026 20:04:42 -0300 Subject: [PATCH 6/6] HYPERFLEET-452 - fix: prevent fmt.Sprintf panic when reason contains % without values New function now only calls fmt.Sprintf when len(values) > 0, otherwise assigns reason directly to avoid panic when reason contains '%' characters but no format values are provided. --- pkg/errors/errors.go | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/pkg/errors/errors.go b/pkg/errors/errors.go index 2f99caf..64b5025 100755 --- a/pkg/errors/errors.go +++ b/pkg/errors/errors.go @@ -193,7 +193,11 @@ func New(code string, reason string, values ...interface{}) *ServiceError { } if reason != "" { - err.Reason = fmt.Sprintf(reason, values...) + if len(values) > 0 { + err.Reason = fmt.Sprintf(reason, values...) + } else { + err.Reason = reason + } } return err