diff --git a/hyperfleet-operator/internal/render/nodepool_test.go b/hyperfleet-operator/internal/render/nodepool_test.go index e615bbc8..af53cc56 100644 --- a/hyperfleet-operator/internal/render/nodepool_test.go +++ b/hyperfleet-operator/internal/render/nodepool_test.go @@ -187,9 +187,9 @@ func TestNodePoolResourceLabels(t *testing.T) { func TestNodePoolResourceAutoRepair(t *testing.T) { tests := []struct { - name string + name string autoRepair *bool - want bool + want bool }{ {"nil defaults to true", nil, true}, {"explicit true", ptr.To(true), true}, diff --git a/platform-api/pkg/api/error.go b/platform-api/pkg/api/error.go new file mode 100644 index 00000000..a65b88be --- /dev/null +++ b/platform-api/pkg/api/error.go @@ -0,0 +1,85 @@ +package api + +import ( + "encoding/json" + "fmt" + "net/http" +) + +// APIError is a typed error response. HTTPStatus drives the HTTP status code; +// Code, Message, and optional Errors are serialized to JSON under "kind":"Error". +// Reason, when set, is the fmt template used by WithReason() to build dynamic Errors. +type APIError struct { + Code string `json:"code"` + HTTPStatus int `json:"-"` + Message string `json:"reason"` + Errors any `json:"errors,omitempty"` + Reason string `json:"-"` +} + +// WithErrors returns a copy of e with Errors set to v for structured payloads +// (e.g. a slice of field-level validation errors). +func (e APIError) WithErrors(v any) APIError { + e.Errors = v + return e +} + +// WithReason returns a copy of e with Errors set by applying e.Reason to args +// via fmt.Errorf. Panics if e.Reason is empty so misconfiguration is caught at +// test time. +func (e APIError) WithReason(args ...any) APIError { + if e.Reason == "" { + panic(fmt.Sprintf("api: WithReason() called on %q which has no Reason template", e.Code)) + } + e.Errors = fmt.Errorf(e.Reason, args...) + return e +} + +// WriteError serializes def as a JSON error response. The return value follows +// the same contract as Write: a marshal failure is returned before headers are +// committed; a write failure after WriteHeader is unrecoverable but still +// returned so the caller can log it. +// +// When Errors implements error and marshals to "{}" or "null" (plain errors), +// reason is derived from Errors.Error() and the errors field is suppressed. +// For structured Errors (exported fields), the static Message is kept and +// Errors serializes as-is. +func WriteError(w http.ResponseWriter, def APIError) error { + if err, ok := def.Errors.(error); ok { + b, merr := json.Marshal(def.Errors) + if merr != nil { + writeFallback(w) + return merr + } + if len(b) == 0 || string(b) == "{}" || string(b) == "null" { + // Plain error: derive reason from message, suppress empty errors field. + def.Message = err.Error() + def.Errors = nil + } + // Structured error: keep the static Message and let Errors serialize as-is. + } + b, err := json.Marshal(struct { + Kind string `json:"kind"` + APIError + }{Kind: "Error", APIError: def}) + if err != nil { + writeFallback(w) + return err + } + w.Header().Set("Content-Type", "application/json") + w.WriteHeader(def.HTTPStatus) + _, err = w.Write(b) + return err +} + +// fallbackBody is the pre-marshaled form of ErrInternalMarshal, populated by +// errorcodes.go's init() after ErrInternalMarshal is set. +var fallbackBody []byte + +// writeFallback writes a 500 JSON body when normal serialization has failed. +// It must not call Write or WriteError to avoid circular/recursive calls. +func writeFallback(w http.ResponseWriter) { + w.Header().Set("Content-Type", "application/json") + w.WriteHeader(http.StatusInternalServerError) + _, _ = w.Write(fallbackBody) +} diff --git a/platform-api/pkg/api/error_test.go b/platform-api/pkg/api/error_test.go new file mode 100644 index 00000000..a0deb26e --- /dev/null +++ b/platform-api/pkg/api/error_test.go @@ -0,0 +1,269 @@ +package api_test + +import ( + "encoding/json" + "errors" + "net/http" + "net/http/httptest" + "testing" + + "github.com/openshift-online/rosa-hyperfleet-api/platform-api/pkg/api" +) + +var base = api.APIError{ + Code: "TEST-001", + HTTPStatus: http.StatusBadRequest, + Message: "something went wrong", +} + +// structuredError has exported fields so it marshals to non-empty JSON. +type structuredError struct { + Field string `json:"field"` + Detail string `json:"detail"` +} + +func (e *structuredError) Error() string { return e.Detail } + +func decode(t *testing.T, w *httptest.ResponseRecorder) map[string]any { + t.Helper() + var out map[string]any + if err := json.NewDecoder(w.Body).Decode(&out); err != nil { + t.Fatalf("decode response: %v", err) + } + return out +} + +func write(def api.APIError) *httptest.ResponseRecorder { + w := httptest.NewRecorder() + if err := api.WriteError(w, def); err != nil { + panic("WriteError: " + err.Error()) + } + return w +} + +// --- WithErrors --- + +func TestWithErrors_SetsErrors(t *testing.T) { + payload := []string{"a", "b"} + got := base.WithErrors(payload) + if got.Errors == nil { + t.Fatal("expected Errors to be set") + } +} + +func TestWithErrors_DoesNotMutateBase(t *testing.T) { + _ = base.WithErrors("x") + if base.Errors != nil { + t.Fatal("WithErrors must not mutate the receiver") + } +} + +// --- WithReason --- + +func TestWithReason_AppliesTemplate(t *testing.T) { + e := api.APIError{Code: "X", HTTPStatus: 400, Message: "m", Reason: "hello %s"} + got := e.WithReason("world") + if got.Errors == nil { + t.Fatal("expected Errors to be set") + } + if got.Errors.(error).Error() != "hello world" { + t.Fatalf("unexpected reason: %v", got.Errors) + } +} + +func TestWithReason_WrapsErrorWithW(t *testing.T) { + sentinel := errors.New("sentinel") + e := api.APIError{Code: "X", HTTPStatus: 500, Message: "m", Reason: "%w"} + got := e.WithReason(sentinel) + if !errors.Is(got.Errors.(error), sentinel) { + t.Fatal("expected error chain to be preserved via %w") + } +} + +func TestWithReason_PanicsWithoutTemplate(t *testing.T) { + defer func() { + if r := recover(); r == nil { + t.Fatal("expected panic when Reason is empty") + } + }() + base.WithReason("arg") +} + +func TestWithReason_DoesNotMutateBase(t *testing.T) { + e := api.APIError{Code: "X", HTTPStatus: 400, Message: "m", Reason: "%s"} + _ = e.WithReason("x") + if e.Errors != nil { + t.Fatal("WithReason must not mutate the receiver") + } +} + +// --- Write: HTTP envelope --- + +func TestWrite_StatusCode(t *testing.T) { + w := write(api.APIError{Code: "X", HTTPStatus: http.StatusNotFound, Message: "m"}) + if w.Code != http.StatusNotFound { + t.Fatalf("expected 404, got %d", w.Code) + } +} + +func TestWrite_ContentType(t *testing.T) { + w := write(base) + if ct := w.Header().Get("Content-Type"); ct != "application/json" { + t.Fatalf("expected application/json, got %q", ct) + } +} + +func TestWrite_KindIsError(t *testing.T) { + w := write(base) + resp := decode(t, w) + if resp["kind"] != "Error" { + t.Fatalf("expected kind=Error, got %v", resp["kind"]) + } +} + +func TestWrite_CodeAndReason(t *testing.T) { + w := write(base) + resp := decode(t, w) + if resp["code"] != "TEST-001" { + t.Fatalf("unexpected code: %v", resp["code"]) + } + if resp["reason"] != "something went wrong" { + t.Fatalf("unexpected reason: %v", resp["reason"]) + } +} + +// --- Write: plain error (no exported fields) --- + +func TestWrite_PlainError_ReasonFromError(t *testing.T) { + e := api.APIError{Code: "TEST-001", HTTPStatus: http.StatusNotFound, Message: "not found", Reason: "cluster %q not found"} + w := write(e.WithReason("abc")) + resp := decode(t, w) + if resp["reason"] != `cluster "abc" not found` { + t.Fatalf("unexpected reason: %v", resp["reason"]) + } +} + +func TestWrite_PlainError_ErrorsFieldSuppressed(t *testing.T) { + e := api.APIError{Code: "TEST-001", HTTPStatus: http.StatusBadRequest, Message: "bad", Reason: "%w"} + w := write(e.WithReason(errors.New("oops"))) + resp := decode(t, w) + if _, ok := resp["errors"]; ok { + t.Fatal("errors field must be suppressed for plain errors") + } +} + +// --- Write: structured error (exported fields) --- + +func TestWrite_StructuredError_ReasonIsStatic(t *testing.T) { + def := base.WithErrors(&structuredError{Field: "foo", Detail: "too long"}) + w := write(def) + resp := decode(t, w) + if resp["reason"] != "something went wrong" { + t.Fatalf("expected static reason, got %v", resp["reason"]) + } +} + +func TestWrite_StructuredError_ErrorsFieldPresent(t *testing.T) { + def := base.WithErrors(&structuredError{Field: "foo", Detail: "too long"}) + w := write(def) + resp := decode(t, w) + if resp["errors"] == nil { + t.Fatal("expected errors field to be present for structured errors") + } + errs := resp["errors"].(map[string]any) + if errs["field"] != "foo" { + t.Fatalf("unexpected errors.field: %v", errs["field"]) + } +} + +// --- Write: no errors --- + +func TestWrite_NoErrors_NoErrorsField(t *testing.T) { + w := write(base) + resp := decode(t, w) + if _, ok := resp["errors"]; ok { + t.Fatal("errors field must be absent when not set") + } +} + +// --- Write: full response format --- + +func TestWrite_ResponseFormat(t *testing.T) { + cases := []struct { + name string + def api.APIError + wantStatus int + wantKind string + wantCode string + wantReason string + wantErrors any // nil means field must be absent + forbidden []string // keys that must not appear in the response + }{ + { + name: "static message no errors", + def: api.APIError{Code: "A-001", HTTPStatus: http.StatusBadRequest, Message: "bad request"}, + wantStatus: http.StatusBadRequest, + wantKind: "Error", + wantCode: "A-001", + wantReason: "bad request", + forbidden: []string{"HTTPStatus", "http_status", "Reason", "Format"}, + }, + { + name: "plain error derives reason and suppresses errors field", + def: api.APIError{Code: "A-002", HTTPStatus: http.StatusNotFound, Message: "default", Reason: "item %q not found"}.WithReason("xyz"), + wantStatus: http.StatusNotFound, + wantKind: "Error", + wantCode: "A-002", + wantReason: `item "xyz" not found`, + forbidden: []string{"HTTPStatus", "http_status", "Reason", "Format"}, + }, + { + name: "structured error keeps static reason and exposes errors", + def: api.APIError{Code: "A-003", HTTPStatus: http.StatusUnprocessableEntity, Message: "validation failed"}.WithErrors(&structuredError{Field: "name", Detail: "required"}), + wantStatus: http.StatusUnprocessableEntity, + wantKind: "Error", + wantCode: "A-003", + wantReason: "validation failed", + wantErrors: map[string]any{"field": "name", "detail": "required"}, + forbidden: []string{"HTTPStatus", "http_status", "Reason", "Format"}, + }, + } + + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + w := write(tc.def) + + if w.Code != tc.wantStatus { + t.Errorf("status: got %d, want %d", w.Code, tc.wantStatus) + } + + resp := decode(t, w) + + if resp["kind"] != tc.wantKind { + t.Errorf("kind: got %v, want %q", resp["kind"], tc.wantKind) + } + if resp["code"] != tc.wantCode { + t.Errorf("code: got %v, want %q", resp["code"], tc.wantCode) + } + if resp["reason"] != tc.wantReason { + t.Errorf("reason: got %v, want %q", resp["reason"], tc.wantReason) + } + + if tc.wantErrors == nil { + if _, ok := resp["errors"]; ok { + t.Errorf("errors: expected absent, got %v", resp["errors"]) + } + } else { + if resp["errors"] == nil { + t.Error("errors: expected present, got absent") + } + } + + for _, key := range tc.forbidden { + if _, ok := resp[key]; ok { + t.Errorf("internal field %q must not appear in response", key) + } + } + }) + } +} diff --git a/platform-api/pkg/api/errorcodes.go b/platform-api/pkg/api/errorcodes.go new file mode 100644 index 00000000..9b40b8a0 --- /dev/null +++ b/platform-api/pkg/api/errorcodes.go @@ -0,0 +1,27 @@ +package api + +import ( + "encoding/json" + "net/http" +) + +// ErrInternalMarshal is written when response serialization fails before any +// headers have been committed, ensuring the client receives a proper 500 instead +// of an empty default 200. +var ErrInternalMarshal APIError + +func init() { + ErrInternalMarshal = APIError{ + Code: "INTERNAL-001", + HTTPStatus: http.StatusInternalServerError, + Message: "internal server error", + } + var err error + fallbackBody, err = json.Marshal(struct { + Kind string `json:"kind"` + APIError + }{Kind: "Error", APIError: ErrInternalMarshal}) + if err != nil { + panic("api: failed to marshal fallback error body: " + err.Error()) + } +} diff --git a/platform-api/pkg/api/response.go b/platform-api/pkg/api/response.go new file mode 100644 index 00000000..89829cef --- /dev/null +++ b/platform-api/pkg/api/response.go @@ -0,0 +1,31 @@ +package api + +import ( + "encoding/json" + "fmt" + "net/http" +) + +// Write serializes data as a JSON response with the given HTTP status code. +// If data is nil, only the status code is written (suitable for 204 No Content). +// If marshaling fails, a 500 error response is written before the error is +// returned so the client never receives an empty default 200. If the write +// itself fails (headers already committed), the error is returned for the +// caller to log — the connection is already broken. +func Write(w http.ResponseWriter, status int, data any) error { + if data == nil { + w.WriteHeader(status) + return nil + } + b, err := json.Marshal(data) + if err != nil { + if werr := WriteError(w, ErrInternalMarshal); werr != nil { + return fmt.Errorf("marshal: %w; write error response: %v", err, werr) + } + return err + } + w.Header().Set("Content-Type", "application/json") + w.WriteHeader(status) + _, err = w.Write(b) + return err +} diff --git a/platform-api/pkg/clients/hyperfleetdb/convert.go b/platform-api/pkg/clients/hyperfleetdb/convert.go index 429528e4..7685777d 100644 --- a/platform-api/pkg/clients/hyperfleetdb/convert.go +++ b/platform-api/pkg/clients/hyperfleetdb/convert.go @@ -193,6 +193,14 @@ func metaTime(obj metav1.Object) time.Time { const clusterNSPrefix = "cluster-" +// clusterUUIDLen is the fixed length of a RFC 4122 UUID string (e.g. "4610b27e-8f77-4f4c-9661-c11b42e04dec"). +const clusterUUIDLen = 36 + +// MaxClusterNameLen is the maximum allowed cluster name length. +// HyperShift creates a control plane namespace as "-", +// which expands to "cluster--" and must fit within 63 characters (k8s namespace limit). +const MaxClusterNameLen = 63 - len(clusterNSPrefix) - clusterUUIDLen - len("-") + func clusterNamespace(clusterID string) string { return clusterNSPrefix + clusterID } diff --git a/platform-api/pkg/handlers/accounts.go b/platform-api/pkg/handlers/accounts.go index ac444433..cd1e52da 100644 --- a/platform-api/pkg/handlers/accounts.go +++ b/platform-api/pkg/handlers/accounts.go @@ -7,6 +7,7 @@ import ( "github.com/gorilla/mux" + "github.com/openshift-online/rosa-hyperfleet-api/platform-api/pkg/api" "github.com/openshift-online/rosa-hyperfleet-api/platform-api/pkg/authz" "github.com/openshift-online/rosa-hyperfleet-api/platform-api/pkg/middleware" ) @@ -57,12 +58,12 @@ func (h *AccountsHandler) Create(w http.ResponseWriter, r *http.Request) { var req EnableAccountRequest if err := json.NewDecoder(r.Body).Decode(&req); err != nil { - h.writeError(w, http.StatusBadRequest, "invalid-request", "Invalid request body") + writeAPIError(w, ErrAccountCreateInvalidBody, h.logger) return } if req.AccountID == "" { - h.writeError(w, http.StatusBadRequest, "missing-account-id", "accountId is required") + writeAPIError(w, ErrAccountCreateMissingID, h.logger) return } @@ -70,33 +71,33 @@ func (h *AccountsHandler) Create(w http.ResponseWriter, r *http.Request) { existing, err := h.authorizer.GetAccount(ctx, req.AccountID) if err != nil { h.logger.Error("failed to check existing account", "error", err, "account_id", req.AccountID) - h.writeError(w, http.StatusInternalServerError, "internal-error", "Failed to check account status") + writeAPIError(w, ErrAccountCreateCheckFailed, h.logger) return } if existing != nil { - h.writeError(w, http.StatusConflict, "account-exists", "Account is already enabled") + writeAPIError(w, ErrAccountCreateExists, h.logger) return } account, err := h.authorizer.EnableAccount(ctx, req.AccountID, callerARN, req.Privileged) if err != nil { h.logger.Error("failed to enable account", "error", err, "account_id", req.AccountID) - h.writeError(w, http.StatusInternalServerError, "internal-error", "Failed to enable account") + writeAPIError(w, ErrAccountCreateFailed, h.logger) return } - h.logger.Info("account enabled", "account_id", req.AccountID, "privileged", req.Privileged) + h.logger.Info("account enabled", "account_id", redact(req.AccountID), "privileged", req.Privileged) - w.Header().Set("Content-Type", "application/json") - w.WriteHeader(http.StatusCreated) - _ = json.NewEncoder(w).Encode(AccountResponse{ + if err := api.Write(w, http.StatusCreated, AccountResponse{ Kind: "Account", AccountID: account.AccountID, PolicyStoreID: account.PolicyStoreID, Privileged: account.Privileged, CreatedAt: account.CreatedAt, CreatedBy: account.CreatedBy, - }) + }); err != nil { + h.logger.Error("failed to write response", "error", err) + } } // List handles GET /api/v0/accounts @@ -106,7 +107,7 @@ func (h *AccountsHandler) List(w http.ResponseWriter, r *http.Request) { accounts, err := h.authorizer.ListAccounts(ctx) if err != nil { h.logger.Error("failed to list accounts", "error", err) - h.writeError(w, http.StatusInternalServerError, "internal-error", "Failed to list accounts") + writeAPIError(w, ErrAccountListFailed, h.logger) return } @@ -122,12 +123,13 @@ func (h *AccountsHandler) List(w http.ResponseWriter, r *http.Request) { } } - w.Header().Set("Content-Type", "application/json") - _ = json.NewEncoder(w).Encode(AccountListResponse{ + if err := api.Write(w, http.StatusOK, AccountListResponse{ Kind: "AccountList", Items: items, Total: len(items), - }) + }); err != nil { + h.logger.Error("failed to write response", "error", err) + } } // Get handles GET /api/v0/accounts/{id} @@ -139,24 +141,25 @@ func (h *AccountsHandler) Get(w http.ResponseWriter, r *http.Request) { account, err := h.authorizer.GetAccount(ctx, accountID) if err != nil { h.logger.Error("failed to get account", "error", err, "account_id", accountID) - h.writeError(w, http.StatusInternalServerError, "internal-error", "Failed to get account") + writeAPIError(w, ErrAccountGetFailed, h.logger) return } if account == nil { - h.writeError(w, http.StatusNotFound, "not-found", "Account not found") + writeAPIError(w, ErrAccountGetNotFound, h.logger) return } - w.Header().Set("Content-Type", "application/json") - _ = json.NewEncoder(w).Encode(AccountResponse{ + if err := api.Write(w, http.StatusOK, AccountResponse{ Kind: "Account", AccountID: account.AccountID, PolicyStoreID: account.PolicyStoreID, Privileged: account.Privileged, CreatedAt: account.CreatedAt, CreatedBy: account.CreatedBy, - }) + }); err != nil { + h.logger.Error("failed to write response", "error", err) + } } // Delete handles DELETE /api/v0/accounts/{id} @@ -171,24 +174,13 @@ func (h *AccountsHandler) Delete(w http.ResponseWriter, r *http.Request) { err := h.authorizer.DisableAccount(ctx, accountID) if err != nil { h.logger.Error("failed to disable account", "error", err, "account_id", accountID) - h.writeError(w, http.StatusInternalServerError, "internal-error", "Failed to disable account") + writeAPIError(w, ErrAccountDeleteFailed, h.logger) return } h.logger.Info("account disabled", "account_id", accountID) - w.WriteHeader(http.StatusNoContent) -} - -func (h *AccountsHandler) writeError(w http.ResponseWriter, status int, code, reason string) { - w.Header().Set("Content-Type", "application/json") - w.WriteHeader(status) - - resp := map[string]any{ - "kind": "Error", - "code": code, - "reason": reason, + if err := api.Write(w, http.StatusNoContent, nil); err != nil { + h.logger.Error("failed to write response", "error", err) } - - _ = json.NewEncoder(w).Encode(resp) } diff --git a/platform-api/pkg/handlers/authz.go b/platform-api/pkg/handlers/authz.go index 169d5134..6516d07f 100644 --- a/platform-api/pkg/handlers/authz.go +++ b/platform-api/pkg/handlers/authz.go @@ -7,6 +7,7 @@ import ( "github.com/gorilla/mux" + "github.com/openshift-online/rosa-hyperfleet-api/platform-api/pkg/api" "github.com/openshift-online/rosa-hyperfleet-api/platform-api/pkg/authz" "github.com/openshift-online/rosa-hyperfleet-api/platform-api/pkg/middleware" ) @@ -140,36 +141,36 @@ func (h *AuthzHandler) CreatePolicy(w http.ResponseWriter, r *http.Request) { var req CreatePolicyRequest if err := json.NewDecoder(r.Body).Decode(&req); err != nil { - h.writeError(w, http.StatusBadRequest, "invalid-request", "Invalid request body") + writeAPIError(w, ErrAuthzPolicyCreateInvalidBody, h.logger) return } if req.Name == "" { - h.writeError(w, http.StatusBadRequest, "missing-name", "name is required") + writeAPIError(w, ErrAuthzPolicyCreateMissingName, h.logger) return } if req.Policy == "" { - h.writeError(w, http.StatusBadRequest, "missing-policy", "policy (Cedar text) is required") + writeAPIError(w, ErrAuthzPolicyCreateMissingText, h.logger) return } p, err := h.service.CreatePolicy(ctx, accountID, req.Name, req.Description, req.Policy) if err != nil { h.logger.Error("failed to create policy", "error", err, "account_id", accountID) - h.writeError(w, http.StatusBadRequest, "invalid-policy", err.Error()) + writeAPIError(w, ErrAuthzPolicyCreateInvalid.WithReason(err), h.logger) return } - w.Header().Set("Content-Type", "application/json") - w.WriteHeader(http.StatusCreated) - _ = json.NewEncoder(w).Encode(PolicyResponse{ + if err := api.Write(w, http.StatusCreated, PolicyResponse{ Kind: "Policy", PolicyID: p.PolicyID, Name: p.Name, Description: p.Description, CreatedAt: p.CreatedAt, - }) + }); err != nil { + h.logger.Error("failed to write response", "error", err) + } } //nolint:dupl // ListPolicies and ListGroups are structurally similar but operate on different types @@ -180,7 +181,7 @@ func (h *AuthzHandler) ListPolicies(w http.ResponseWriter, r *http.Request) { policies, err := h.service.ListPolicies(ctx, accountID) if err != nil { h.logger.Error("failed to list policies", "error", err, "account_id", accountID) - h.writeError(w, http.StatusInternalServerError, "internal-error", "Failed to list policies") + writeAPIError(w, ErrAuthzPolicyListFailed, h.logger) return } @@ -195,12 +196,13 @@ func (h *AuthzHandler) ListPolicies(w http.ResponseWriter, r *http.Request) { } } - w.Header().Set("Content-Type", "application/json") - _ = json.NewEncoder(w).Encode(PolicyListResponse{ + if err := api.Write(w, http.StatusOK, PolicyListResponse{ Kind: "PolicyList", Items: items, Total: len(items), - }) + }); err != nil { + h.logger.Error("failed to write response", "error", err) + } } func (h *AuthzHandler) GetPolicy(w http.ResponseWriter, r *http.Request) { @@ -212,23 +214,24 @@ func (h *AuthzHandler) GetPolicy(w http.ResponseWriter, r *http.Request) { p, err := h.service.GetPolicy(ctx, accountID, policyID) if err != nil { h.logger.Error("failed to get policy", "error", err, "account_id", accountID, "policy_id", policyID) - h.writeError(w, http.StatusInternalServerError, "internal-error", "Failed to get policy") + writeAPIError(w, ErrAuthzPolicyGetFailed, h.logger) return } if p == nil { - h.writeError(w, http.StatusNotFound, "not-found", "Policy not found") + writeAPIError(w, ErrAuthzPolicyGetNotFound, h.logger) return } - w.Header().Set("Content-Type", "application/json") - _ = json.NewEncoder(w).Encode(PolicyResponse{ + if err := api.Write(w, http.StatusOK, PolicyResponse{ Kind: "Policy", PolicyID: p.PolicyID, Name: p.Name, Description: p.Description, CreatedAt: p.CreatedAt, - }) + }); err != nil { + h.logger.Error("failed to write response", "error", err) + } } func (h *AuthzHandler) UpdatePolicy(w http.ResponseWriter, r *http.Request) { @@ -239,25 +242,26 @@ func (h *AuthzHandler) UpdatePolicy(w http.ResponseWriter, r *http.Request) { var req CreatePolicyRequest if err := json.NewDecoder(r.Body).Decode(&req); err != nil { - h.writeError(w, http.StatusBadRequest, "invalid-request", "Invalid request body") + writeAPIError(w, ErrAuthzPolicyUpdateInvalidBody, h.logger) return } p, err := h.service.UpdatePolicy(ctx, accountID, policyID, req.Name, req.Description, req.Policy) if err != nil { h.logger.Error("failed to update policy", "error", err, "account_id", accountID, "policy_id", policyID) - h.writeError(w, http.StatusBadRequest, "invalid-policy", err.Error()) + writeAPIError(w, ErrAuthzPolicyUpdateInvalid.WithReason(err), h.logger) return } - w.Header().Set("Content-Type", "application/json") - _ = json.NewEncoder(w).Encode(PolicyResponse{ + if err := api.Write(w, http.StatusOK, PolicyResponse{ Kind: "Policy", PolicyID: p.PolicyID, Name: p.Name, Description: p.Description, CreatedAt: p.CreatedAt, - }) + }); err != nil { + h.logger.Error("failed to write response", "error", err) + } } func (h *AuthzHandler) DeletePolicy(w http.ResponseWriter, r *http.Request) { @@ -270,14 +274,16 @@ func (h *AuthzHandler) DeletePolicy(w http.ResponseWriter, r *http.Request) { if err != nil { h.logger.Error("failed to delete policy", "error", err, "account_id", accountID, "policy_id", policyID) if err.Error() == "cannot delete policy with existing attachments" { - h.writeError(w, http.StatusConflict, "policy-in-use", err.Error()) + writeAPIError(w, ErrAuthzPolicyDeleteInUse.WithReason(err), h.logger) return } - h.writeError(w, http.StatusInternalServerError, "internal-error", "Failed to delete policy") + writeAPIError(w, ErrAuthzPolicyDeleteFailed, h.logger) return } - w.WriteHeader(http.StatusNoContent) + if err := api.Write(w, http.StatusNoContent, nil); err != nil { + h.logger.Error("failed to write response", "error", err) + } } // Group Handlers @@ -288,31 +294,31 @@ func (h *AuthzHandler) CreateGroup(w http.ResponseWriter, r *http.Request) { var req CreateGroupRequest if err := json.NewDecoder(r.Body).Decode(&req); err != nil { - h.writeError(w, http.StatusBadRequest, "invalid-request", "Invalid request body") + writeAPIError(w, ErrAuthzGroupCreateInvalidBody, h.logger) return } if req.Name == "" { - h.writeError(w, http.StatusBadRequest, "missing-name", "name is required") + writeAPIError(w, ErrAuthzGroupCreateMissingName, h.logger) return } g, err := h.service.CreateGroup(ctx, accountID, req.Name, req.Description) if err != nil { h.logger.Error("failed to create group", "error", err, "account_id", accountID) - h.writeError(w, http.StatusInternalServerError, "internal-error", "Failed to create group") + writeAPIError(w, ErrAuthzGroupCreateFailed, h.logger) return } - w.Header().Set("Content-Type", "application/json") - w.WriteHeader(http.StatusCreated) - _ = json.NewEncoder(w).Encode(GroupResponse{ + if err := api.Write(w, http.StatusCreated, GroupResponse{ Kind: "Group", GroupID: g.GroupID, Name: g.Name, Description: g.Description, CreatedAt: g.CreatedAt, - }) + }); err != nil { + h.logger.Error("failed to write response", "error", err) + } } //nolint:dupl // ListGroups and ListPolicies are structurally similar but operate on different types @@ -323,7 +329,7 @@ func (h *AuthzHandler) ListGroups(w http.ResponseWriter, r *http.Request) { groups, err := h.service.ListGroups(ctx, accountID) if err != nil { h.logger.Error("failed to list groups", "error", err, "account_id", accountID) - h.writeError(w, http.StatusInternalServerError, "internal-error", "Failed to list groups") + writeAPIError(w, ErrAuthzGroupListFailed, h.logger) return } @@ -338,12 +344,13 @@ func (h *AuthzHandler) ListGroups(w http.ResponseWriter, r *http.Request) { } } - w.Header().Set("Content-Type", "application/json") - _ = json.NewEncoder(w).Encode(GroupListResponse{ + if err := api.Write(w, http.StatusOK, GroupListResponse{ Kind: "GroupList", Items: items, Total: len(items), - }) + }); err != nil { + h.logger.Error("failed to write response", "error", err) + } } func (h *AuthzHandler) GetGroup(w http.ResponseWriter, r *http.Request) { @@ -355,23 +362,24 @@ func (h *AuthzHandler) GetGroup(w http.ResponseWriter, r *http.Request) { g, err := h.service.GetGroup(ctx, accountID, groupID) if err != nil { h.logger.Error("failed to get group", "error", err, "account_id", accountID, "group_id", groupID) - h.writeError(w, http.StatusInternalServerError, "internal-error", "Failed to get group") + writeAPIError(w, ErrAuthzGroupGetFailed, h.logger) return } if g == nil { - h.writeError(w, http.StatusNotFound, "not-found", "Group not found") + writeAPIError(w, ErrAuthzGroupGetNotFound, h.logger) return } - w.Header().Set("Content-Type", "application/json") - _ = json.NewEncoder(w).Encode(GroupResponse{ + if err := api.Write(w, http.StatusOK, GroupResponse{ Kind: "Group", GroupID: g.GroupID, Name: g.Name, Description: g.Description, CreatedAt: g.CreatedAt, - }) + }); err != nil { + h.logger.Error("failed to write response", "error", err) + } } func (h *AuthzHandler) DeleteGroup(w http.ResponseWriter, r *http.Request) { @@ -383,11 +391,13 @@ func (h *AuthzHandler) DeleteGroup(w http.ResponseWriter, r *http.Request) { err := h.service.DeleteGroup(ctx, accountID, groupID) if err != nil { h.logger.Error("failed to delete group", "error", err, "account_id", accountID, "group_id", groupID) - h.writeError(w, http.StatusInternalServerError, "internal-error", "Failed to delete group") + writeAPIError(w, ErrAuthzGroupDeleteFailed, h.logger) return } - w.WriteHeader(http.StatusNoContent) + if err := api.Write(w, http.StatusNoContent, nil); err != nil { + h.logger.Error("failed to write response", "error", err) + } } func (h *AuthzHandler) UpdateGroupMembers(w http.ResponseWriter, r *http.Request) { @@ -398,7 +408,7 @@ func (h *AuthzHandler) UpdateGroupMembers(w http.ResponseWriter, r *http.Request var req UpdateMembersRequest if err := json.NewDecoder(r.Body).Decode(&req); err != nil { - h.writeError(w, http.StatusBadRequest, "invalid-request", "Invalid request body") + writeAPIError(w, ErrAuthzGroupMembersUpdateInvalidBody, h.logger) return } @@ -406,7 +416,7 @@ func (h *AuthzHandler) UpdateGroupMembers(w http.ResponseWriter, r *http.Request for _, memberARN := range req.Add { if err := h.service.AddGroupMember(ctx, accountID, groupID, memberARN); err != nil { h.logger.Error("failed to add group member", "error", err, "account_id", accountID, "group_id", groupID, "member", memberARN) - h.writeError(w, http.StatusInternalServerError, "internal-error", "Failed to add group member") + writeAPIError(w, ErrAuthzGroupMembersUpdateAddFailed, h.logger) return } } @@ -415,7 +425,7 @@ func (h *AuthzHandler) UpdateGroupMembers(w http.ResponseWriter, r *http.Request for _, memberARN := range req.Remove { if err := h.service.RemoveGroupMember(ctx, accountID, groupID, memberARN); err != nil { h.logger.Error("failed to remove group member", "error", err, "account_id", accountID, "group_id", groupID, "member", memberARN) - h.writeError(w, http.StatusInternalServerError, "internal-error", "Failed to remove group member") + writeAPIError(w, ErrAuthzGroupMembersUpdateRemFailed, h.logger) return } } @@ -424,16 +434,17 @@ func (h *AuthzHandler) UpdateGroupMembers(w http.ResponseWriter, r *http.Request members, err := h.service.ListGroupMembers(ctx, accountID, groupID) if err != nil { h.logger.Error("failed to list group members", "error", err, "account_id", accountID, "group_id", groupID) - h.writeError(w, http.StatusInternalServerError, "internal-error", "Failed to list group members") + writeAPIError(w, ErrAuthzGroupMembersUpdateListFailed, h.logger) return } - w.Header().Set("Content-Type", "application/json") - _ = json.NewEncoder(w).Encode(MemberListResponse{ + if err := api.Write(w, http.StatusOK, MemberListResponse{ Kind: "MemberList", Items: members, Total: len(members), - }) + }); err != nil { + h.logger.Error("failed to write response", "error", err) + } } func (h *AuthzHandler) ListGroupMembers(w http.ResponseWriter, r *http.Request) { @@ -445,16 +456,17 @@ func (h *AuthzHandler) ListGroupMembers(w http.ResponseWriter, r *http.Request) members, err := h.service.ListGroupMembers(ctx, accountID, groupID) if err != nil { h.logger.Error("failed to list group members", "error", err, "account_id", accountID, "group_id", groupID) - h.writeError(w, http.StatusInternalServerError, "internal-error", "Failed to list group members") + writeAPIError(w, ErrAuthzGroupMembersListFailed, h.logger) return } - w.Header().Set("Content-Type", "application/json") - _ = json.NewEncoder(w).Encode(MemberListResponse{ + if err := api.Write(w, http.StatusOK, MemberListResponse{ Kind: "MemberList", Items: members, Total: len(members), - }) + }); err != nil { + h.logger.Error("failed to write response", "error", err) + } } // Attachment Handlers @@ -465,37 +477,37 @@ func (h *AuthzHandler) CreateAttachment(w http.ResponseWriter, r *http.Request) var req CreateAttachmentRequest if err := json.NewDecoder(r.Body).Decode(&req); err != nil { - h.writeError(w, http.StatusBadRequest, "invalid-request", "Invalid request body") + writeAPIError(w, ErrAuthzAttachCreateInvalidBody, h.logger) return } if req.PolicyID == "" || req.TargetType == "" || req.TargetID == "" { - h.writeError(w, http.StatusBadRequest, "missing-fields", "policyId, targetType, and targetId are required") + writeAPIError(w, ErrAuthzAttachCreateMissingFields, h.logger) return } if req.TargetType != "user" && req.TargetType != "group" { - h.writeError(w, http.StatusBadRequest, "invalid-target-type", "targetType must be 'user' or 'group'") + writeAPIError(w, ErrAuthzAttachCreateInvalidTarget, h.logger) return } a, err := h.service.AttachPolicy(ctx, accountID, req.PolicyID, authz.TargetType(req.TargetType), req.TargetID) if err != nil { h.logger.Error("failed to attach policy", "error", err, "account_id", accountID, "policy_id", req.PolicyID) - h.writeError(w, http.StatusBadRequest, "attachment-failed", err.Error()) + writeAPIError(w, ErrAuthzAttachCreateFailed.WithReason(err), h.logger) return } - w.Header().Set("Content-Type", "application/json") - w.WriteHeader(http.StatusCreated) - _ = json.NewEncoder(w).Encode(AttachmentResponse{ + if err := api.Write(w, http.StatusCreated, AttachmentResponse{ Kind: "Attachment", AttachmentID: a.AttachmentID, PolicyID: a.PolicyID, TargetType: string(a.TargetType), TargetID: a.TargetID, CreatedAt: a.CreatedAt, - }) + }); err != nil { + h.logger.Error("failed to write response", "error", err) + } } func (h *AuthzHandler) ListAttachments(w http.ResponseWriter, r *http.Request) { @@ -512,7 +524,7 @@ func (h *AuthzHandler) ListAttachments(w http.ResponseWriter, r *http.Request) { attachments, err := h.service.ListAttachments(ctx, accountID, filter) if err != nil { h.logger.Error("failed to list attachments", "error", err, "account_id", accountID) - h.writeError(w, http.StatusInternalServerError, "internal-error", "Failed to list attachments") + writeAPIError(w, ErrAuthzAttachListFailed, h.logger) return } @@ -528,12 +540,13 @@ func (h *AuthzHandler) ListAttachments(w http.ResponseWriter, r *http.Request) { } } - w.Header().Set("Content-Type", "application/json") - _ = json.NewEncoder(w).Encode(AttachmentListResponse{ + if err := api.Write(w, http.StatusOK, AttachmentListResponse{ Kind: "AttachmentList", Items: items, Total: len(items), - }) + }); err != nil { + h.logger.Error("failed to write response", "error", err) + } } func (h *AuthzHandler) DeleteAttachment(w http.ResponseWriter, r *http.Request) { @@ -545,11 +558,13 @@ func (h *AuthzHandler) DeleteAttachment(w http.ResponseWriter, r *http.Request) err := h.service.DetachPolicy(ctx, accountID, attachmentID) if err != nil { h.logger.Error("failed to detach policy", "error", err, "account_id", accountID, "attachment_id", attachmentID) - h.writeError(w, http.StatusInternalServerError, "internal-error", "Failed to detach policy") + writeAPIError(w, ErrAuthzAttachDeleteFailed, h.logger) return } - w.WriteHeader(http.StatusNoContent) + if err := api.Write(w, http.StatusNoContent, nil); err != nil { + h.logger.Error("failed to write response", "error", err) + } } // Admin Handlers @@ -561,28 +576,28 @@ func (h *AuthzHandler) AddAdmin(w http.ResponseWriter, r *http.Request) { var req AddAdminRequest if err := json.NewDecoder(r.Body).Decode(&req); err != nil { - h.writeError(w, http.StatusBadRequest, "invalid-request", "Invalid request body") + writeAPIError(w, ErrAuthzAdminAddInvalidBody, h.logger) return } if req.PrincipalARN == "" { - h.writeError(w, http.StatusBadRequest, "missing-principal-arn", "principalArn is required") + writeAPIError(w, ErrAuthzAdminAddMissingPrinc, h.logger) return } err := h.service.AddAdmin(ctx, accountID, req.PrincipalARN, callerARN) if err != nil { h.logger.Error("failed to add admin", "error", err, "account_id", accountID, "principal_arn", req.PrincipalARN) - h.writeError(w, http.StatusInternalServerError, "internal-error", "Failed to add admin") + writeAPIError(w, ErrAuthzAdminAddFailed, h.logger) return } - w.WriteHeader(http.StatusCreated) - w.Header().Set("Content-Type", "application/json") - _ = json.NewEncoder(w).Encode(map[string]any{ + if err := api.Write(w, http.StatusCreated, map[string]any{ "kind": "Admin", "principalArn": req.PrincipalARN, - }) + }); err != nil { + h.logger.Error("failed to write response", "error", err) + } } func (h *AuthzHandler) ListAdmins(w http.ResponseWriter, r *http.Request) { @@ -592,16 +607,17 @@ func (h *AuthzHandler) ListAdmins(w http.ResponseWriter, r *http.Request) { admins, err := h.service.ListAdmins(ctx, accountID) if err != nil { h.logger.Error("failed to list admins", "error", err, "account_id", accountID) - h.writeError(w, http.StatusInternalServerError, "internal-error", "Failed to list admins") + writeAPIError(w, ErrAuthzAdminListFailed, h.logger) return } - w.Header().Set("Content-Type", "application/json") - _ = json.NewEncoder(w).Encode(AdminListResponse{ + if err := api.Write(w, http.StatusOK, AdminListResponse{ Kind: "AdminList", Items: admins, Total: len(admins), - }) + }); err != nil { + h.logger.Error("failed to write response", "error", err) + } } func (h *AuthzHandler) RemoveAdmin(w http.ResponseWriter, r *http.Request) { @@ -614,11 +630,13 @@ func (h *AuthzHandler) RemoveAdmin(w http.ResponseWriter, r *http.Request) { err := h.service.RemoveAdmin(ctx, accountID, principalARN) if err != nil { h.logger.Error("failed to remove admin", "error", err, "account_id", accountID, "principal_arn", principalARN) - h.writeError(w, http.StatusInternalServerError, "internal-error", "Failed to remove admin") + writeAPIError(w, ErrAuthzAdminDeleteFailed, h.logger) return } - w.WriteHeader(http.StatusNoContent) + if err := api.Write(w, http.StatusNoContent, nil); err != nil { + h.logger.Error("failed to write response", "error", err) + } } // CheckAuthorization evaluates an authorization request and returns the decision. @@ -628,22 +646,22 @@ func (h *AuthzHandler) CheckAuthorization(w http.ResponseWriter, r *http.Request var req CheckAuthorizationRequest if err := json.NewDecoder(r.Body).Decode(&req); err != nil { - h.writeError(w, http.StatusBadRequest, "invalid-request", "Invalid request body") + writeAPIError(w, ErrAuthzCheckInvalidBody, h.logger) return } if req.Principal == "" { - h.writeError(w, http.StatusBadRequest, "missing-principal", "principal is required") + writeAPIError(w, ErrAuthzCheckMissingPrinc, h.logger) return } if req.Action == "" { - h.writeError(w, http.StatusBadRequest, "missing-action", "action is required") + writeAPIError(w, ErrAuthzCheckMissingAction, h.logger) return } if req.Resource == "" { - h.writeError(w, http.StatusBadRequest, "missing-resource", "resource is required") + writeAPIError(w, ErrAuthzCheckMissingRes, h.logger) return } @@ -661,7 +679,7 @@ func (h *AuthzHandler) CheckAuthorization(w http.ResponseWriter, r *http.Request allowed, err := h.checker.Authorize(ctx, authzReq) if err != nil { h.logger.Error("authorization check failed", "error", err, "account_id", accountID, "principal", req.Principal, "action", req.Action) - h.writeError(w, http.StatusInternalServerError, "authorization-error", err.Error()) + writeAPIError(w, ErrAuthzCheckFailed.WithReason(err), h.logger) return } @@ -670,22 +688,10 @@ func (h *AuthzHandler) CheckAuthorization(w http.ResponseWriter, r *http.Request decision = "ALLOW" } - w.Header().Set("Content-Type", "application/json") - _ = json.NewEncoder(w).Encode(CheckAuthorizationResponse{ + if err := api.Write(w, http.StatusOK, CheckAuthorizationResponse{ Kind: "AuthorizationDecision", Decision: decision, - }) -} - -func (h *AuthzHandler) writeError(w http.ResponseWriter, status int, code, reason string) { - w.Header().Set("Content-Type", "application/json") - w.WriteHeader(status) - - resp := map[string]any{ - "kind": "Error", - "code": code, - "reason": reason, + }); err != nil { + h.logger.Error("failed to write response", "error", err) } - - _ = json.NewEncoder(w).Encode(resp) } diff --git a/platform-api/pkg/handlers/cluster.go b/platform-api/pkg/handlers/cluster.go index 7aa9e2dd..c2b10611 100644 --- a/platform-api/pkg/handlers/cluster.go +++ b/platform-api/pkg/handlers/cluster.go @@ -2,7 +2,6 @@ package handlers import ( "encoding/json" - "fmt" "io" "log/slog" "net/http" @@ -14,6 +13,7 @@ import ( metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "github.com/openshift-online/rosa-hyperfleet-api/platform-api/internal/codegen/featuregate" + "github.com/openshift-online/rosa-hyperfleet-api/platform-api/pkg/api" "github.com/openshift-online/rosa-hyperfleet-api/platform-api/pkg/clients/hyperfleetdb" "github.com/openshift-online/rosa-hyperfleet-api/platform-api/pkg/middleware" "github.com/openshift-online/rosa-hyperfleet-api/platform-api/pkg/types" @@ -70,7 +70,7 @@ func (h *ClusterHandler) List(w http.ResponseWriter, r *http.Request) { list, err := h.db.ListClusters(ctx, accountID) if err != nil { h.logger.Error("failed to list clusters", "error", err, "account_id", accountID) - h.writeError(w, http.StatusInternalServerError, "CLUSTERS-MGMT-LIST-001", "Failed to list clusters") + writeAPIError(w, ErrClusterList, h.logger) return } @@ -96,7 +96,9 @@ func (h *ClusterHandler) List(w http.ResponseWriter, r *http.Request) { "offset": offset, } - h.writeJSON(w, http.StatusOK, response) + if err := api.Write(w, http.StatusOK, response); err != nil { + h.logger.Error("failed to write response", "error", err) + } } // Create handles POST /api/v0/clusters @@ -106,30 +108,34 @@ func (h *ClusterHandler) Create(w http.ResponseWriter, r *http.Request) { var req types.ClusterCreateRequest if err := json.NewDecoder(r.Body).Decode(&req); err != nil { - h.writeError(w, http.StatusBadRequest, "CLUSTERS-MGMT-CREATE-001", "Invalid request body") + writeAPIError(w, ErrClusterCreateInvalidBody, h.logger) return } if req.Name == "" || req.Spec == nil { - h.writeError(w, http.StatusBadRequest, "CLUSTERS-MGMT-CREATE-002", "Missing required fields: name and spec") + writeAPIError(w, ErrClusterCreateMissingFields, h.logger) + return + } + + if len(req.Name) > hyperfleetdb.MaxClusterNameLen { + writeAPIError(w, ErrClusterCreateNameTooLong, h.logger) return } if errs := h.validator.ValidateCreate(req.Spec, featuregate.Default); errs != nil { - h.writeValidationErrors(w, errs) + writeAPIError(w, ErrClusterValidation.WithErrors(errs), h.logger) return } existing, err := h.db.ListClusters(ctx, accountID) if err != nil { h.logger.Error("failed to check cluster name uniqueness", "error", err, "account_id", accountID) - h.writeError(w, http.StatusInternalServerError, "CLUSTERS-MGMT-CREATE-004", "Failed to validate cluster name") + writeAPIError(w, ErrClusterCreateNameCheck, h.logger) return } for i := range existing.Items { if existing.Items[i].Name == req.Name { - h.writeError(w, http.StatusConflict, "CLUSTERS-MGMT-CREATE-005", - fmt.Sprintf("A cluster named %q already exists in this account", req.Name)) + writeAPIError(w, ErrClusterCreateNameConflict.WithReason(req.Name), h.logger) return } } @@ -147,7 +153,7 @@ func (h *ClusterHandler) Create(w http.ResponseWriter, r *http.Request) { cr, err := hyperfleetdb.PlatformCreateToClusterCR(clusterID, accountID, &req) if err != nil { h.logger.Error("failed to convert cluster spec", "error", err, "account_id", accountID) - h.writeError(w, http.StatusBadRequest, "CLUSTERS-MGMT-CREATE-002", "Invalid cluster spec") + writeAPIError(w, ErrClusterCreateInvalidSpec, h.logger) return } @@ -167,15 +173,17 @@ func (h *ClusterHandler) Create(w http.ResponseWriter, r *http.Request) { } h.logger.Error("failed to create cluster", "error", err, "account_id", accountID) if hyperfleetdb.IsAlreadyExists(err) { - h.writeError(w, http.StatusInternalServerError, "CLUSTERS-MGMT-CREATE-007", "Unable to generate unique DNS identifier") + writeAPIError(w, ErrClusterCreateIDExhausted, h.logger) return } - h.writeError(w, http.StatusInternalServerError, "CLUSTERS-MGMT-CREATE-003", "Failed to create cluster") + writeAPIError(w, ErrClusterCreateFailed, h.logger) return } cluster := hyperfleetdb.ClusterCRToPlatform(cr) - h.writeJSON(w, http.StatusCreated, cluster) + if err := api.Write(w, http.StatusCreated, cluster); err != nil { + h.logger.Error("failed to write response", "error", err) + } return } } @@ -192,15 +200,17 @@ func (h *ClusterHandler) Get(w http.ResponseWriter, r *http.Request) { cr, err := h.db.GetCluster(ctx, accountID, clusterID) if err != nil { if hyperfleetdb.IsNotFound(err) { - h.writeError(w, http.StatusNotFound, "CLUSTERS-MGMT-GET-001", "Cluster not found") + writeAPIError(w, ErrClusterGetNotFound, h.logger) return } h.logger.Error("failed to get cluster", "error", err, "account_id", accountID, "cluster_id", clusterID) - h.writeError(w, http.StatusInternalServerError, "CLUSTERS-MGMT-GET-002", "Failed to get cluster") + writeAPIError(w, ErrClusterGetFailed, h.logger) return } - h.writeJSON(w, http.StatusOK, hyperfleetdb.ClusterCRToPlatform(cr)) + if err := api.Write(w, http.StatusOK, hyperfleetdb.ClusterCRToPlatform(cr)); err != nil { + h.logger.Error("failed to write response", "error", err) + } } // Update handles PUT /api/v0/clusters/{id} @@ -212,18 +222,18 @@ func (h *ClusterHandler) Update(w http.ResponseWriter, r *http.Request) { body, err := io.ReadAll(r.Body) if err != nil { - h.writeError(w, http.StatusBadRequest, "CLUSTERS-MGMT-UPDATE-001", "Failed to read request body") + writeAPIError(w, ErrClusterUpdateInvalidBody, h.logger) return } var req types.ClusterUpdateRequest if err := json.Unmarshal(body, &req); err != nil { - h.writeError(w, http.StatusBadRequest, "CLUSTERS-MGMT-UPDATE-001", "Invalid request body") + writeAPIError(w, ErrClusterUpdateInvalidBody, h.logger) return } if req.Spec == nil { - h.writeError(w, http.StatusBadRequest, "CLUSTERS-MGMT-UPDATE-002", "Missing required field: spec") + writeAPIError(w, ErrClusterUpdateMissingFields, h.logger) return } @@ -232,16 +242,16 @@ func (h *ClusterHandler) Update(w http.ResponseWriter, r *http.Request) { cr, err := h.db.GetCluster(ctx, accountID, clusterID) if err != nil { if hyperfleetdb.IsNotFound(err) { - h.writeError(w, http.StatusNotFound, "CLUSTERS-MGMT-UPDATE-003", "Cluster not found") + writeAPIError(w, ErrClusterUpdateNotFound, h.logger) return } h.logger.Error("failed to get cluster for update", "error", err, "account_id", accountID, "cluster_id", clusterID) - h.writeError(w, http.StatusInternalServerError, "CLUSTERS-MGMT-UPDATE-004", "Failed to update cluster") + writeAPIError(w, ErrClusterUpdateFailed, h.logger) return } if errs := h.validator.ValidateUpdate(req.Spec, &cr.Spec, featuregate.Default); errs != nil { - h.writeValidationErrors(w, errs) + writeAPIError(w, ErrClusterValidation.WithErrors(errs), h.logger) return } @@ -252,23 +262,25 @@ func (h *ClusterHandler) Update(w http.ResponseWriter, r *http.Request) { Spec json.RawMessage `json:"spec"` } if err := json.Unmarshal(body, &envelope); err != nil { - h.writeError(w, http.StatusBadRequest, "CLUSTERS-MGMT-UPDATE-001", "Invalid request body") + writeAPIError(w, ErrClusterUpdateInvalidBody, h.logger) return } if err := hyperfleetdb.MergeSpecJSON(&cr.Spec, envelope.Spec); err != nil { h.logger.Error("failed to merge cluster spec", "error", err) - h.writeError(w, http.StatusBadRequest, "CLUSTERS-MGMT-UPDATE-002", "Invalid cluster spec") + writeAPIError(w, ErrClusterUpdateInvalidSpec, h.logger) return } if err := h.db.UpdateCluster(ctx, cr); err != nil { h.logger.Error("failed to update cluster", "error", err, "account_id", accountID, "cluster_id", clusterID) - h.writeError(w, http.StatusInternalServerError, "CLUSTERS-MGMT-UPDATE-004", "Failed to update cluster") + writeAPIError(w, ErrClusterUpdateFailed, h.logger) return } - h.writeJSON(w, http.StatusOK, hyperfleetdb.ClusterCRToPlatform(cr)) + if err := api.Write(w, http.StatusOK, hyperfleetdb.ClusterCRToPlatform(cr)); err != nil { + h.logger.Error("failed to write response", "error", err) + } } // Delete handles DELETE /api/v0/clusters/{id} @@ -283,11 +295,11 @@ func (h *ClusterHandler) Delete(w http.ResponseWriter, r *http.Request) { err := h.db.DeleteCluster(ctx, accountID, clusterID) if err != nil { if hyperfleetdb.IsNotFound(err) { - h.writeError(w, http.StatusNotFound, "CLUSTERS-MGMT-DELETE-001", "Cluster not found") + writeAPIError(w, ErrClusterDeleteNotFound, h.logger) return } h.logger.Error("failed to delete cluster", "error", err, "account_id", accountID, "cluster_id", clusterID) - h.writeError(w, http.StatusInternalServerError, "CLUSTERS-MGMT-DELETE-002", "Failed to delete cluster") + writeAPIError(w, ErrClusterDeleteFailed, h.logger) return } @@ -296,7 +308,9 @@ func (h *ClusterHandler) Delete(w http.ResponseWriter, r *http.Request) { "cluster_id": clusterID, } - h.writeJSON(w, http.StatusAccepted, response) + if err := api.Write(w, http.StatusAccepted, response); err != nil { + h.logger.Error("failed to write response", "error", err) + } } // GetStatus handles GET /api/v0/clusters/{id}/statuses @@ -311,43 +325,15 @@ func (h *ClusterHandler) GetStatus(w http.ResponseWriter, r *http.Request) { cr, err := h.db.GetCluster(ctx, accountID, clusterID) if err != nil { if hyperfleetdb.IsNotFound(err) { - h.writeError(w, http.StatusNotFound, "CLUSTERS-MGMT-STATUS-001", "Cluster not found") + writeAPIError(w, ErrClusterStatusNotFound, h.logger) return } h.logger.Error("failed to get cluster status", "error", err, "account_id", accountID, "cluster_id", clusterID) - h.writeError(w, http.StatusInternalServerError, "CLUSTERS-MGMT-STATUS-002", "Failed to get cluster status") + writeAPIError(w, ErrClusterStatusFailed, h.logger) return } - h.writeJSON(w, http.StatusOK, hyperfleetdb.ClusterStatusFromCR(cr)) -} - -// Helper methods -func (h *ClusterHandler) writeJSON(w http.ResponseWriter, status int, data any) { - w.Header().Set("Content-Type", "application/json") - w.WriteHeader(status) - _ = json.NewEncoder(w).Encode(data) -} - -func (h *ClusterHandler) writeValidationErrors(w http.ResponseWriter, errs validation.ValidationErrors) { - w.Header().Set("Content-Type", "application/json") - w.WriteHeader(http.StatusUnprocessableEntity) - resp := map[string]any{ - "kind": "Error", - "code": "CLUSTERS-MGMT-VALIDATION-001", - "reason": "Request validation failed", - "errors": errs, - } - _ = json.NewEncoder(w).Encode(resp) -} - -func (h *ClusterHandler) writeError(w http.ResponseWriter, status int, code, reason string) { - w.Header().Set("Content-Type", "application/json") - w.WriteHeader(status) - resp := map[string]any{ - "kind": "Error", - "code": code, - "reason": reason, + if err := api.Write(w, http.StatusOK, hyperfleetdb.ClusterStatusFromCR(cr)); err != nil { + h.logger.Error("failed to write response", "error", err) } - _ = json.NewEncoder(w).Encode(resp) } diff --git a/platform-api/pkg/handlers/cluster_test.go b/platform-api/pkg/handlers/cluster_test.go index 6b9eb06c..73bc8016 100644 --- a/platform-api/pkg/handlers/cluster_test.go +++ b/platform-api/pkg/handlers/cluster_test.go @@ -11,6 +11,7 @@ import ( "net/http" "net/http/httptest" "os" + "strings" "sync" "sync/atomic" "testing" @@ -271,6 +272,32 @@ func TestClusterHandler_Create_MissingFields(t *testing.T) { } } +func TestClusterHandler_Create_NameTooLong(t *testing.T) { + scheme := newTestScheme() + fc := fake.NewClientBuilder().WithScheme(scheme).Build() + logger := slog.New(slog.NewTextHandler(os.Stdout, nil)) + handler := NewClusterHandler(hyperfleetdb.NewClientFrom(fc, logger), "https://oidc.example.com", 0, logger) + + longName := strings.Repeat("a", hyperfleetdb.MaxClusterNameLen+1) + body, _ := json.Marshal(map[string]any{"name": longName, "spec": map[string]any{}}) + + req := httptest.NewRequest(http.MethodPost, "/api/v0/clusters", bytes.NewReader(body)) + req = req.WithContext(testContext(testAccountID)) + + w := httptest.NewRecorder() + handler.Create(w, req) + + if w.Code != http.StatusBadRequest { + t.Fatalf("expected 400, got %d: %s", w.Code, w.Body.String()) + } + + var errResp map[string]any + _ = json.NewDecoder(w.Body).Decode(&errResp) + if errResp["code"] != ErrClusterCreateNameTooLong.Code { + t.Errorf("expected code %s, got %v", ErrClusterCreateNameTooLong.Code, errResp["code"]) + } +} + func TestClusterHandler_Get_Success(t *testing.T) { scheme := newTestScheme() fc := fake.NewClientBuilder().WithScheme(scheme).WithObjects( @@ -320,8 +347,8 @@ func TestClusterHandler_Get_NotFound(t *testing.T) { var errResp map[string]any _ = json.NewDecoder(w.Body).Decode(&errResp) - if errResp["code"] != "CLUSTERS-MGMT-GET-001" { - t.Errorf("expected code CLUSTERS-MGMT-GET-001, got %v", errResp["code"]) + if errResp["code"] != ErrClusterGetNotFound.Code { + t.Errorf("expected code %s, got %v", ErrClusterGetNotFound.Code, errResp["code"]) } } @@ -526,8 +553,8 @@ func TestClusterHandler_Create_DuplicateName(t *testing.T) { var errResp map[string]any _ = json.NewDecoder(w.Body).Decode(&errResp) - if errResp["code"] != "CLUSTERS-MGMT-CREATE-005" { - t.Errorf("expected code CLUSTERS-MGMT-CREATE-005, got %v", errResp["code"]) + if errResp["code"] != ErrClusterCreateNameConflict.Code { + t.Errorf("expected code %s, got %v", ErrClusterCreateNameConflict.Code, errResp["code"]) } } @@ -634,8 +661,8 @@ func TestClusterHandler_Create_Hash4ExhaustedRetries(t *testing.T) { var errResp map[string]any _ = json.NewDecoder(w.Body).Decode(&errResp) - if errResp["code"] != "CLUSTERS-MGMT-CREATE-007" { - t.Errorf("expected code CLUSTERS-MGMT-CREATE-007, got %v", errResp["code"]) + if errResp["code"] != ErrClusterCreateIDExhausted.Code { + t.Errorf("expected code %s, got %v", ErrClusterCreateIDExhausted.Code, errResp["code"]) } } diff --git a/platform-api/pkg/handlers/errorcodes.go b/platform-api/pkg/handlers/errorcodes.go new file mode 100644 index 00000000..05dc8647 --- /dev/null +++ b/platform-api/pkg/handlers/errorcodes.go @@ -0,0 +1,410 @@ +package handlers + +import ( + "fmt" + "log/slog" + "net/http" + + "github.com/openshift-online/rosa-hyperfleet-api/platform-api/pkg/api" + "github.com/openshift-online/rosa-hyperfleet-api/platform-api/pkg/clients/hyperfleetdb" +) + +// APIError is an alias for api.APIError so handler code uses the short form. +type APIError = api.APIError + +func writeAPIError(w http.ResponseWriter, def APIError, logger *slog.Logger) { + if err := api.WriteError(w, def); err != nil { + logger.Error("failed to write error response", "error", err) + } +} + +// Cluster error codes +var ( + ErrClusterList APIError + + ErrClusterCreateInvalidBody APIError + ErrClusterCreateMissingFields APIError + ErrClusterCreateFailed APIError + ErrClusterCreateNameCheck APIError + ErrClusterCreateNameConflict APIError + ErrClusterCreateNameTooLong APIError + ErrClusterCreateIDExhausted APIError + ErrClusterCreateInvalidSpec APIError + + ErrClusterGetNotFound APIError + ErrClusterGetFailed APIError + + ErrClusterUpdateInvalidBody APIError + ErrClusterUpdateMissingFields APIError + ErrClusterUpdateNotFound APIError + ErrClusterUpdateFailed APIError + ErrClusterUpdateInvalidSpec APIError + + ErrClusterDeleteNotFound APIError + ErrClusterDeleteFailed APIError + + ErrClusterStatusNotFound APIError + ErrClusterStatusFailed APIError + + ErrClusterValidation APIError +) + +// NodePool error codes +var ( + ErrNodePoolList APIError + + ErrNodePoolCreateInvalidBody APIError + ErrNodePoolCreateMissingFields APIError + ErrNodePoolCreateNameConflict APIError + ErrNodePoolCreateClusterNotFound APIError + ErrNodePoolCreateClusterCheck APIError + ErrNodePoolCreateInvalidSpec APIError + ErrNodePoolCreateFailed APIError + + ErrNodePoolGetNotFound APIError + ErrNodePoolGetFailed APIError + + ErrNodePoolUpdateInvalidBody APIError + ErrNodePoolUpdateMissingFields APIError + ErrNodePoolUpdateNotFound APIError + ErrNodePoolUpdateFailed APIError + ErrNodePoolUpdateInvalidSpec APIError + + ErrNodePoolDeleteNotFound APIError + ErrNodePoolDeleteFailed APIError + + ErrNodePoolStatusNotFound APIError + ErrNodePoolStatusFailed APIError + + ErrNodePoolValidation APIError +) + +// Accounts error codes +var ( + ErrAccountCreateInvalidBody APIError + ErrAccountCreateMissingID APIError + ErrAccountCreateCheckFailed APIError + ErrAccountCreateExists APIError + ErrAccountCreateFailed APIError + + ErrAccountListFailed APIError + + ErrAccountGetFailed APIError + ErrAccountGetNotFound APIError + + ErrAccountDeleteFailed APIError +) + +// Management cluster error codes +var ( + ErrMCCreateInvalidBody APIError + ErrMCCreateMissingID APIError + ErrMCCreateMissingReg APIError + ErrMCCreateMissingAcct APIError + ErrMCCreateExists APIError + ErrMCCreateFailed APIError + + ErrMCListFailed APIError + + ErrMCGetNotFound APIError + ErrMCGetFailed APIError +) + +// Authz policy error codes +var ( + ErrAuthzPolicyCreateInvalidBody APIError + ErrAuthzPolicyCreateMissingName APIError + ErrAuthzPolicyCreateMissingText APIError + ErrAuthzPolicyCreateInvalid APIError + + ErrAuthzPolicyListFailed APIError + + ErrAuthzPolicyGetFailed APIError + ErrAuthzPolicyGetNotFound APIError + + ErrAuthzPolicyUpdateInvalidBody APIError + ErrAuthzPolicyUpdateInvalid APIError + + ErrAuthzPolicyDeleteFailed APIError + ErrAuthzPolicyDeleteInUse APIError +) + +// Authz group error codes +var ( + ErrAuthzGroupCreateInvalidBody APIError + ErrAuthzGroupCreateMissingName APIError + ErrAuthzGroupCreateFailed APIError + + ErrAuthzGroupListFailed APIError + + ErrAuthzGroupGetFailed APIError + ErrAuthzGroupGetNotFound APIError + + ErrAuthzGroupDeleteFailed APIError + + ErrAuthzGroupMembersUpdateInvalidBody APIError + ErrAuthzGroupMembersUpdateAddFailed APIError + ErrAuthzGroupMembersUpdateRemFailed APIError + ErrAuthzGroupMembersUpdateListFailed APIError + + ErrAuthzGroupMembersListFailed APIError +) + +// Authz attachment error codes +var ( + ErrAuthzAttachCreateInvalidBody APIError + ErrAuthzAttachCreateMissingFields APIError + ErrAuthzAttachCreateInvalidTarget APIError + ErrAuthzAttachCreateFailed APIError + + ErrAuthzAttachListFailed APIError + ErrAuthzAttachDeleteFailed APIError +) + +// Authz admin error codes +var ( + ErrAuthzAdminAddInvalidBody APIError + ErrAuthzAdminAddMissingPrinc APIError + ErrAuthzAdminAddFailed APIError + + ErrAuthzAdminListFailed APIError + ErrAuthzAdminDeleteFailed APIError +) + +// Authz check error codes +var ( + ErrAuthzCheckInvalidBody APIError + ErrAuthzCheckMissingPrinc APIError + ErrAuthzCheckMissingAction APIError + ErrAuthzCheckMissingRes APIError + ErrAuthzCheckFailed APIError +) + +// ZOA error codes +var ( + ErrZoaCreateUnknownAction APIError + ErrZoaCreateInvalidBody APIError + ErrZoaCreateMissingCluster APIError + ErrZoaCreateMissingJira APIError + ErrZoaCreateInvalidJira APIError + ErrZoaCreateInvalidParams APIError + ErrZoaCreateCooldown APIError + ErrZoaCreateMaxConcurrent APIError + ErrZoaCreateDryRunError APIError + ErrZoaCreateStoreFailed APIError + ErrZoaCreateRenderFailed APIError + ErrZoaCreateDispatchFailed APIError + ErrZoaCreateStoreSaveFailed APIError + + ErrZoaGetStoreFailed APIError + ErrZoaGetNotFound APIError + + ErrZoaListStoreFailed APIError + + ErrZoaAuditDisabled APIError + ErrZoaAuditListFailed APIError +) + +// Info error codes +var ErrInfoRegionalAccountUnavailable APIError + +func init() { + // Cluster — List + ErrClusterList = APIError{Code: "CLUSTERS-MGMT-LIST-001", HTTPStatus: http.StatusInternalServerError, Message: "Failed to list clusters"} + + // Cluster — Create + ErrClusterCreateInvalidBody = APIError{Code: "CLUSTERS-MGMT-CREATE-001", HTTPStatus: http.StatusBadRequest, Message: "Invalid request body"} + ErrClusterCreateMissingFields = APIError{Code: "CLUSTERS-MGMT-CREATE-002", HTTPStatus: http.StatusBadRequest, Message: "Missing required fields: name and spec"} + ErrClusterCreateFailed = APIError{Code: "CLUSTERS-MGMT-CREATE-003", HTTPStatus: http.StatusInternalServerError, Message: "Failed to create cluster"} + ErrClusterCreateNameCheck = APIError{Code: "CLUSTERS-MGMT-CREATE-004", HTTPStatus: http.StatusInternalServerError, Message: "Failed to validate cluster name"} + ErrClusterCreateNameConflict = APIError{Code: "CLUSTERS-MGMT-CREATE-005", HTTPStatus: http.StatusConflict, Message: "Cluster name already exists in this account", Reason: "a cluster named %q already exists in this account"} + ErrClusterCreateNameTooLong = APIError{Code: "CLUSTERS-MGMT-CREATE-006", HTTPStatus: http.StatusBadRequest, Message: fmt.Sprintf("Cluster name must be no more than %d characters", hyperfleetdb.MaxClusterNameLen)} + ErrClusterCreateIDExhausted = APIError{Code: "CLUSTERS-MGMT-CREATE-007", HTTPStatus: http.StatusInternalServerError, Message: "Unable to generate unique DNS identifier"} + ErrClusterCreateInvalidSpec = APIError{Code: "CLUSTERS-MGMT-CREATE-008", HTTPStatus: http.StatusBadRequest, Message: "Invalid cluster spec"} + + // Cluster — Get + ErrClusterGetNotFound = APIError{Code: "CLUSTERS-MGMT-GET-001", HTTPStatus: http.StatusNotFound, Message: "Cluster not found"} + ErrClusterGetFailed = APIError{Code: "CLUSTERS-MGMT-GET-002", HTTPStatus: http.StatusInternalServerError, Message: "Failed to get cluster"} + + // Cluster — Update + ErrClusterUpdateInvalidBody = APIError{Code: "CLUSTERS-MGMT-UPDATE-001", HTTPStatus: http.StatusBadRequest, Message: "Invalid request body"} + ErrClusterUpdateMissingFields = APIError{Code: "CLUSTERS-MGMT-UPDATE-002", HTTPStatus: http.StatusBadRequest, Message: "Missing required field: spec"} + ErrClusterUpdateNotFound = APIError{Code: "CLUSTERS-MGMT-UPDATE-003", HTTPStatus: http.StatusNotFound, Message: "Cluster not found"} + ErrClusterUpdateFailed = APIError{Code: "CLUSTERS-MGMT-UPDATE-004", HTTPStatus: http.StatusInternalServerError, Message: "Failed to update cluster"} + ErrClusterUpdateInvalidSpec = APIError{Code: "CLUSTERS-MGMT-UPDATE-005", HTTPStatus: http.StatusBadRequest, Message: "Invalid cluster spec"} + + // Cluster — Delete + ErrClusterDeleteNotFound = APIError{Code: "CLUSTERS-MGMT-DELETE-001", HTTPStatus: http.StatusNotFound, Message: "Cluster not found"} + ErrClusterDeleteFailed = APIError{Code: "CLUSTERS-MGMT-DELETE-002", HTTPStatus: http.StatusInternalServerError, Message: "Failed to delete cluster"} + + // Cluster — Status + ErrClusterStatusNotFound = APIError{Code: "CLUSTERS-MGMT-STATUS-001", HTTPStatus: http.StatusNotFound, Message: "Cluster not found"} + ErrClusterStatusFailed = APIError{Code: "CLUSTERS-MGMT-STATUS-002", HTTPStatus: http.StatusInternalServerError, Message: "Failed to get cluster status"} + + // Cluster — Validation + ErrClusterValidation = APIError{Code: "CLUSTERS-MGMT-VALIDATION-001", HTTPStatus: http.StatusUnprocessableEntity, Message: "A validation error has occurred, check the errors field for more information"} + + // NodePool — List + ErrNodePoolList = APIError{Code: "NODEPOOLS-MGMT-LIST-001", HTTPStatus: http.StatusInternalServerError, Message: "Failed to list nodepools"} + + // NodePool — Create + ErrNodePoolCreateInvalidBody = APIError{Code: "NODEPOOLS-MGMT-CREATE-001", HTTPStatus: http.StatusBadRequest, Message: "Invalid request body"} + ErrNodePoolCreateMissingFields = APIError{Code: "NODEPOOLS-MGMT-CREATE-002", HTTPStatus: http.StatusBadRequest, Message: "Missing required fields: name, cluster_id, and spec"} + ErrNodePoolCreateNameConflict = APIError{Code: "NODEPOOLS-MGMT-CREATE-003", HTTPStatus: http.StatusConflict, Message: "NodePool already exists"} + ErrNodePoolCreateClusterNotFound = APIError{Code: "NODEPOOLS-MGMT-CREATE-004", HTTPStatus: http.StatusNotFound, Message: "Referenced cluster not found"} + ErrNodePoolCreateClusterCheck = APIError{Code: "NODEPOOLS-MGMT-CREATE-005", HTTPStatus: http.StatusInternalServerError, Message: "Failed to validate cluster reference"} + ErrNodePoolCreateInvalidSpec = APIError{Code: "NODEPOOLS-MGMT-CREATE-006", HTTPStatus: http.StatusBadRequest, Message: "Invalid nodepool spec"} + ErrNodePoolCreateFailed = APIError{Code: "NODEPOOLS-MGMT-CREATE-007", HTTPStatus: http.StatusInternalServerError, Message: "Failed to create nodepool"} + + // NodePool — Get + ErrNodePoolGetNotFound = APIError{Code: "NODEPOOLS-MGMT-GET-001", HTTPStatus: http.StatusNotFound, Message: "NodePool not found"} + ErrNodePoolGetFailed = APIError{Code: "NODEPOOLS-MGMT-GET-002", HTTPStatus: http.StatusInternalServerError, Message: "Failed to get nodepool"} + + // NodePool — Update + ErrNodePoolUpdateInvalidBody = APIError{Code: "NODEPOOLS-MGMT-UPDATE-001", HTTPStatus: http.StatusBadRequest, Message: "Invalid request body"} + ErrNodePoolUpdateMissingFields = APIError{Code: "NODEPOOLS-MGMT-UPDATE-002", HTTPStatus: http.StatusBadRequest, Message: "Missing required field: spec"} + ErrNodePoolUpdateNotFound = APIError{Code: "NODEPOOLS-MGMT-UPDATE-003", HTTPStatus: http.StatusNotFound, Message: "NodePool not found"} + ErrNodePoolUpdateFailed = APIError{Code: "NODEPOOLS-MGMT-UPDATE-004", HTTPStatus: http.StatusInternalServerError, Message: "Failed to update nodepool"} + ErrNodePoolUpdateInvalidSpec = APIError{Code: "NODEPOOLS-MGMT-UPDATE-005", HTTPStatus: http.StatusBadRequest, Message: "Invalid nodepool spec"} + + // NodePool — Delete + ErrNodePoolDeleteNotFound = APIError{Code: "NODEPOOLS-MGMT-DELETE-001", HTTPStatus: http.StatusNotFound, Message: "NodePool not found"} + ErrNodePoolDeleteFailed = APIError{Code: "NODEPOOLS-MGMT-DELETE-002", HTTPStatus: http.StatusInternalServerError, Message: "Failed to delete nodepool"} + + // NodePool — Status + ErrNodePoolStatusNotFound = APIError{Code: "NODEPOOLS-MGMT-STATUS-001", HTTPStatus: http.StatusNotFound, Message: "NodePool not found"} + ErrNodePoolStatusFailed = APIError{Code: "NODEPOOLS-MGMT-STATUS-002", HTTPStatus: http.StatusInternalServerError, Message: "Failed to get nodepool status"} + + // NodePool — Validation + ErrNodePoolValidation = APIError{Code: "NODEPOOLS-MGMT-VALIDATION-001", HTTPStatus: http.StatusUnprocessableEntity, Message: "A validation error has occurred, check the errors field for more information"} + + // Accounts — Create + ErrAccountCreateInvalidBody = APIError{Code: "ACCOUNTS-MGMT-CREATE-001", HTTPStatus: http.StatusBadRequest, Message: "Invalid request body"} + ErrAccountCreateMissingID = APIError{Code: "ACCOUNTS-MGMT-CREATE-002", HTTPStatus: http.StatusBadRequest, Message: "accountId is required"} + ErrAccountCreateCheckFailed = APIError{Code: "ACCOUNTS-MGMT-CREATE-003", HTTPStatus: http.StatusInternalServerError, Message: "Failed to check account status"} + ErrAccountCreateExists = APIError{Code: "ACCOUNTS-MGMT-CREATE-004", HTTPStatus: http.StatusConflict, Message: "Account is already enabled"} + ErrAccountCreateFailed = APIError{Code: "ACCOUNTS-MGMT-CREATE-005", HTTPStatus: http.StatusInternalServerError, Message: "Failed to enable account"} + + // Accounts — List + ErrAccountListFailed = APIError{Code: "ACCOUNTS-MGMT-LIST-001", HTTPStatus: http.StatusInternalServerError, Message: "Failed to list accounts"} + + // Accounts — Get + ErrAccountGetFailed = APIError{Code: "ACCOUNTS-MGMT-GET-001", HTTPStatus: http.StatusInternalServerError, Message: "Failed to get account"} + ErrAccountGetNotFound = APIError{Code: "ACCOUNTS-MGMT-GET-002", HTTPStatus: http.StatusNotFound, Message: "Account not found"} + + // Accounts — Delete + ErrAccountDeleteFailed = APIError{Code: "ACCOUNTS-MGMT-DELETE-001", HTTPStatus: http.StatusInternalServerError, Message: "Failed to disable account"} + + // Management clusters — Create + ErrMCCreateInvalidBody = APIError{Code: "MC-MGMT-CREATE-001", HTTPStatus: http.StatusBadRequest, Message: "Invalid request body"} + ErrMCCreateMissingID = APIError{Code: "MC-MGMT-CREATE-002", HTTPStatus: http.StatusBadRequest, Message: "id is required"} + ErrMCCreateMissingReg = APIError{Code: "MC-MGMT-CREATE-003", HTTPStatus: http.StatusBadRequest, Message: "region is required"} + ErrMCCreateMissingAcct = APIError{Code: "MC-MGMT-CREATE-004", HTTPStatus: http.StatusBadRequest, Message: "accountId is required"} + ErrMCCreateExists = APIError{Code: "MC-MGMT-CREATE-005", HTTPStatus: http.StatusConflict, Message: "Management cluster already registered", Reason: "management cluster already registered: %s"} + ErrMCCreateFailed = APIError{Code: "MC-MGMT-CREATE-006", HTTPStatus: http.StatusInternalServerError, Message: "Failed to save management cluster config"} + + // Management clusters — List + ErrMCListFailed = APIError{Code: "MC-MGMT-LIST-001", HTTPStatus: http.StatusInternalServerError, Message: "Failed to load management cluster config"} + + // Management clusters — Get + ErrMCGetNotFound = APIError{Code: "MC-MGMT-GET-001", HTTPStatus: http.StatusNotFound, Message: "Management cluster not found"} + ErrMCGetFailed = APIError{Code: "MC-MGMT-GET-002", HTTPStatus: http.StatusInternalServerError, Message: "Failed to load management cluster config"} + + // Authz — Policy — Create + ErrAuthzPolicyCreateInvalidBody = APIError{Code: "AUTHZ-POLICY-CREATE-001", HTTPStatus: http.StatusBadRequest, Message: "Invalid request body"} + ErrAuthzPolicyCreateMissingName = APIError{Code: "AUTHZ-POLICY-CREATE-002", HTTPStatus: http.StatusBadRequest, Message: "name is required"} + ErrAuthzPolicyCreateMissingText = APIError{Code: "AUTHZ-POLICY-CREATE-003", HTTPStatus: http.StatusBadRequest, Message: "policy (Cedar text) is required"} + ErrAuthzPolicyCreateInvalid = APIError{Code: "AUTHZ-POLICY-CREATE-004", HTTPStatus: http.StatusBadRequest, Message: "Invalid policy", Reason: "%w"} + + // Authz — Policy — List + ErrAuthzPolicyListFailed = APIError{Code: "AUTHZ-POLICY-LIST-001", HTTPStatus: http.StatusInternalServerError, Message: "Failed to list policies"} + + // Authz — Policy — Get + ErrAuthzPolicyGetFailed = APIError{Code: "AUTHZ-POLICY-GET-001", HTTPStatus: http.StatusInternalServerError, Message: "Failed to get policy"} + ErrAuthzPolicyGetNotFound = APIError{Code: "AUTHZ-POLICY-GET-002", HTTPStatus: http.StatusNotFound, Message: "Policy not found"} + + // Authz — Policy — Update + ErrAuthzPolicyUpdateInvalidBody = APIError{Code: "AUTHZ-POLICY-UPDATE-001", HTTPStatus: http.StatusBadRequest, Message: "Invalid request body"} + ErrAuthzPolicyUpdateInvalid = APIError{Code: "AUTHZ-POLICY-UPDATE-002", HTTPStatus: http.StatusBadRequest, Message: "Invalid policy", Reason: "%w"} + + // Authz — Policy — Delete + ErrAuthzPolicyDeleteFailed = APIError{Code: "AUTHZ-POLICY-DELETE-001", HTTPStatus: http.StatusInternalServerError, Message: "Failed to delete policy"} + ErrAuthzPolicyDeleteInUse = APIError{Code: "AUTHZ-POLICY-DELETE-002", HTTPStatus: http.StatusConflict, Message: "Cannot delete policy with existing attachments", Reason: "%w"} + + // Authz — Group — Create + ErrAuthzGroupCreateInvalidBody = APIError{Code: "AUTHZ-GROUP-CREATE-001", HTTPStatus: http.StatusBadRequest, Message: "Invalid request body"} + ErrAuthzGroupCreateMissingName = APIError{Code: "AUTHZ-GROUP-CREATE-002", HTTPStatus: http.StatusBadRequest, Message: "name is required"} + ErrAuthzGroupCreateFailed = APIError{Code: "AUTHZ-GROUP-CREATE-003", HTTPStatus: http.StatusInternalServerError, Message: "Failed to create group"} + + // Authz — Group — List + ErrAuthzGroupListFailed = APIError{Code: "AUTHZ-GROUP-LIST-001", HTTPStatus: http.StatusInternalServerError, Message: "Failed to list groups"} + + // Authz — Group — Get + ErrAuthzGroupGetFailed = APIError{Code: "AUTHZ-GROUP-GET-001", HTTPStatus: http.StatusInternalServerError, Message: "Failed to get group"} + ErrAuthzGroupGetNotFound = APIError{Code: "AUTHZ-GROUP-GET-002", HTTPStatus: http.StatusNotFound, Message: "Group not found"} + + // Authz — Group — Delete + ErrAuthzGroupDeleteFailed = APIError{Code: "AUTHZ-GROUP-DELETE-001", HTTPStatus: http.StatusInternalServerError, Message: "Failed to delete group"} + + // Authz — Group — Members + ErrAuthzGroupMembersUpdateInvalidBody = APIError{Code: "AUTHZ-GROUP-MEMBERS-UPDATE-001", HTTPStatus: http.StatusBadRequest, Message: "Invalid request body"} + ErrAuthzGroupMembersUpdateAddFailed = APIError{Code: "AUTHZ-GROUP-MEMBERS-UPDATE-002", HTTPStatus: http.StatusInternalServerError, Message: "Failed to add group member"} + ErrAuthzGroupMembersUpdateRemFailed = APIError{Code: "AUTHZ-GROUP-MEMBERS-UPDATE-003", HTTPStatus: http.StatusInternalServerError, Message: "Failed to remove group member"} + ErrAuthzGroupMembersUpdateListFailed = APIError{Code: "AUTHZ-GROUP-MEMBERS-UPDATE-004", HTTPStatus: http.StatusInternalServerError, Message: "Failed to list group members"} + ErrAuthzGroupMembersListFailed = APIError{Code: "AUTHZ-GROUP-MEMBERS-LIST-001", HTTPStatus: http.StatusInternalServerError, Message: "Failed to list group members"} + + // Authz — Attachment — Create + ErrAuthzAttachCreateInvalidBody = APIError{Code: "AUTHZ-ATTACH-CREATE-001", HTTPStatus: http.StatusBadRequest, Message: "Invalid request body"} + ErrAuthzAttachCreateMissingFields = APIError{Code: "AUTHZ-ATTACH-CREATE-002", HTTPStatus: http.StatusBadRequest, Message: "policyId, targetType, and targetId are required"} + ErrAuthzAttachCreateInvalidTarget = APIError{Code: "AUTHZ-ATTACH-CREATE-003", HTTPStatus: http.StatusBadRequest, Message: "targetType must be 'user' or 'group'"} + ErrAuthzAttachCreateFailed = APIError{Code: "AUTHZ-ATTACH-CREATE-004", HTTPStatus: http.StatusBadRequest, Message: "Failed to attach policy", Reason: "%w"} + + // Authz — Attachment — List / Delete + ErrAuthzAttachListFailed = APIError{Code: "AUTHZ-ATTACH-LIST-001", HTTPStatus: http.StatusInternalServerError, Message: "Failed to list attachments"} + ErrAuthzAttachDeleteFailed = APIError{Code: "AUTHZ-ATTACH-DELETE-001", HTTPStatus: http.StatusInternalServerError, Message: "Failed to detach policy"} + + // Authz — Admin — Add + ErrAuthzAdminAddInvalidBody = APIError{Code: "AUTHZ-ADMIN-ADD-001", HTTPStatus: http.StatusBadRequest, Message: "Invalid request body"} + ErrAuthzAdminAddMissingPrinc = APIError{Code: "AUTHZ-ADMIN-ADD-002", HTTPStatus: http.StatusBadRequest, Message: "principalArn is required"} + ErrAuthzAdminAddFailed = APIError{Code: "AUTHZ-ADMIN-ADD-003", HTTPStatus: http.StatusInternalServerError, Message: "Failed to add admin"} + + // Authz — Admin — List / Delete + ErrAuthzAdminListFailed = APIError{Code: "AUTHZ-ADMIN-LIST-001", HTTPStatus: http.StatusInternalServerError, Message: "Failed to list admins"} + ErrAuthzAdminDeleteFailed = APIError{Code: "AUTHZ-ADMIN-DELETE-001", HTTPStatus: http.StatusInternalServerError, Message: "Failed to remove admin"} + + // Authz — Check + ErrAuthzCheckInvalidBody = APIError{Code: "AUTHZ-CHECK-001", HTTPStatus: http.StatusBadRequest, Message: "Invalid request body"} + ErrAuthzCheckMissingPrinc = APIError{Code: "AUTHZ-CHECK-002", HTTPStatus: http.StatusBadRequest, Message: "principal is required"} + ErrAuthzCheckMissingAction = APIError{Code: "AUTHZ-CHECK-003", HTTPStatus: http.StatusBadRequest, Message: "action is required"} + ErrAuthzCheckMissingRes = APIError{Code: "AUTHZ-CHECK-004", HTTPStatus: http.StatusBadRequest, Message: "resource is required"} + ErrAuthzCheckFailed = APIError{Code: "AUTHZ-CHECK-005", HTTPStatus: http.StatusInternalServerError, Message: "Authorization check failed", Reason: "%w"} + + // ZOA — Create + ErrZoaCreateUnknownAction = APIError{Code: "ZOA-CREATE-001", HTTPStatus: http.StatusNotFound, Message: "Trusted action not found", Reason: "trusted action not found: %s"} + ErrZoaCreateInvalidBody = APIError{Code: "ZOA-CREATE-002", HTTPStatus: http.StatusBadRequest, Message: "Invalid request body"} + ErrZoaCreateMissingCluster = APIError{Code: "ZOA-CREATE-003", HTTPStatus: http.StatusBadRequest, Message: "target_cluster is required"} + ErrZoaCreateMissingJira = APIError{Code: "ZOA-CREATE-004", HTTPStatus: http.StatusBadRequest, Message: "jira is required for all trusted actions (e.g. ROSAENG-1234)"} + ErrZoaCreateInvalidJira = APIError{Code: "ZOA-CREATE-005", HTTPStatus: http.StatusBadRequest, Message: "jira does not have correct format; expected PROJECT-NUMBER (e.g. ROSAENG-1234)"} + ErrZoaCreateInvalidParams = APIError{Code: "ZOA-CREATE-006", HTTPStatus: http.StatusBadRequest, Message: "Invalid parameters", Reason: "%w"} + ErrZoaCreateCooldown = APIError{Code: "ZOA-CREATE-007", HTTPStatus: http.StatusTooManyRequests, Message: "Write cooldown in effect", Reason: "%w"} + ErrZoaCreateMaxConcurrent = APIError{Code: "ZOA-CREATE-008", HTTPStatus: http.StatusTooManyRequests, Message: "Too many concurrent executions on target", Reason: "%w"} + ErrZoaCreateDryRunError = APIError{Code: "ZOA-CREATE-009", HTTPStatus: http.StatusInternalServerError, Message: "Dry run action not found", Reason: "dry_run_action '%s' not found in registry"} + ErrZoaCreateStoreFailed = APIError{Code: "ZOA-CREATE-010", HTTPStatus: http.StatusInternalServerError, Message: "Failed to create execution"} + ErrZoaCreateRenderFailed = APIError{Code: "ZOA-CREATE-011", HTTPStatus: http.StatusInternalServerError, Message: "Failed to build trusted action manifest"} + ErrZoaCreateDispatchFailed = APIError{Code: "ZOA-CREATE-012", HTTPStatus: http.StatusBadGateway, Message: "Failed to dispatch trusted action"} + ErrZoaCreateStoreSaveFailed = APIError{Code: "ZOA-CREATE-013", HTTPStatus: http.StatusInternalServerError, Message: "Failed to persist execution state"} + + // ZOA — Get + ErrZoaGetStoreFailed = APIError{Code: "ZOA-GET-001", HTTPStatus: http.StatusInternalServerError, Message: "Failed to retrieve execution"} + ErrZoaGetNotFound = APIError{Code: "ZOA-GET-002", HTTPStatus: http.StatusNotFound, Message: "Execution not found"} + + // ZOA — List + ErrZoaListStoreFailed = APIError{Code: "ZOA-LIST-001", HTTPStatus: http.StatusInternalServerError, Message: "Failed to list executions"} + + // ZOA — Audit + ErrZoaAuditDisabled = APIError{Code: "ZOA-AUDIT-001", HTTPStatus: http.StatusNotFound, Message: "Audit logging is not enabled"} + ErrZoaAuditListFailed = APIError{Code: "ZOA-AUDIT-002", HTTPStatus: http.StatusInternalServerError, Message: "Failed to list audit log"} + + // Info + ErrInfoRegionalAccountUnavailable = APIError{Code: "INFO-001", HTTPStatus: http.StatusServiceUnavailable, Message: "regional account ID is not configured"} +} diff --git a/platform-api/pkg/handlers/health.go b/platform-api/pkg/handlers/health.go index 2e1f31b2..1c5a9f7d 100644 --- a/platform-api/pkg/handlers/health.go +++ b/platform-api/pkg/handlers/health.go @@ -1,22 +1,26 @@ package handlers import ( - "encoding/json" + "log/slog" "net/http" "sync/atomic" + + "github.com/openshift-online/rosa-hyperfleet-api/platform-api/pkg/api" ) // HealthHandler handles health check endpoints type HealthHandler struct { - ready *atomic.Bool + ready *atomic.Bool + logger *slog.Logger } // NewHealthHandler creates a new HealthHandler -func NewHealthHandler() *HealthHandler { +func NewHealthHandler(logger *slog.Logger) *HealthHandler { ready := &atomic.Bool{} ready.Store(true) return &HealthHandler{ - ready: ready, + ready: ready, + logger: logger, } } @@ -27,19 +31,21 @@ func (h *HealthHandler) SetReady(ready bool) { // Liveness handles GET /live func (h *HealthHandler) Liveness(w http.ResponseWriter, r *http.Request) { - w.Header().Set("Content-Type", "application/json") - _ = json.NewEncoder(w).Encode(map[string]string{"status": "ok"}) + if err := api.Write(w, http.StatusOK, map[string]string{"status": "ok"}); err != nil { + h.logger.Error("failed to write response", "error", err) + } } // Readiness handles GET /ready func (h *HealthHandler) Readiness(w http.ResponseWriter, r *http.Request) { - w.Header().Set("Content-Type", "application/json") - if !h.ready.Load() { - w.WriteHeader(http.StatusServiceUnavailable) - _ = json.NewEncoder(w).Encode(map[string]string{"status": "unavailable"}) + if err := api.Write(w, http.StatusServiceUnavailable, map[string]string{"status": "unavailable"}); err != nil { + h.logger.Error("failed to write response", "error", err) + } return } - _ = json.NewEncoder(w).Encode(map[string]string{"status": "ok"}) + if err := api.Write(w, http.StatusOK, map[string]string{"status": "ok"}); err != nil { + h.logger.Error("failed to write response", "error", err) + } } diff --git a/platform-api/pkg/handlers/info.go b/platform-api/pkg/handlers/info.go index 013fb963..8a091aff 100644 --- a/platform-api/pkg/handlers/info.go +++ b/platform-api/pkg/handlers/info.go @@ -1,42 +1,41 @@ package handlers import ( - "encoding/json" "fmt" + "log/slog" "net/http" "os" "strings" + + "github.com/openshift-online/rosa-hyperfleet-api/platform-api/pkg/api" ) // InfoHandler handles the info endpoint -type InfoHandler struct{} +type InfoHandler struct { + logger *slog.Logger +} // NewInfoHandler creates a new InfoHandler -func NewInfoHandler() *InfoHandler { - return &InfoHandler{} +func NewInfoHandler(logger *slog.Logger) *InfoHandler { + return &InfoHandler{logger: logger} } // Info handles GET /api/v0/info // Returns the ARN of the IAM role used to invoke Lambda functions in this regional account. // The account ID is parsed from the TARGET_GROUP_ARN environment variable. func (h *InfoHandler) Info(w http.ResponseWriter, r *http.Request) { - w.Header().Set("Content-Type", "application/json") - tgARN := os.Getenv("TARGET_GROUP_ARN") // Target Group ARN format: arn:aws:elasticloadbalancing:{region}:{account_id}:targetgroup/{name}/{id} parts := strings.SplitN(tgARN, ":", 6) if len(parts) < 6 || parts[4] == "" { - w.WriteHeader(http.StatusServiceUnavailable) - _ = json.NewEncoder(w).Encode(map[string]string{ - "kind": "Error", - "code": "regional-account-unavailable", - "reason": "regional account ID is not configured", - }) + writeAPIError(w, ErrInfoRegionalAccountUnavailable, h.logger) return } accountID := parts[4] arn := fmt.Sprintf("arn:aws:iam::%s:role/LambdaExecutor", accountID) - _ = json.NewEncoder(w).Encode(map[string]string{"arn": arn}) + if err := api.Write(w, http.StatusOK, map[string]string{"arn": arn}); err != nil { + h.logger.Error("failed to write response", "error", err) + } } diff --git a/platform-api/pkg/handlers/info_test.go b/platform-api/pkg/handlers/info_test.go index d0a2606e..30093f30 100644 --- a/platform-api/pkg/handlers/info_test.go +++ b/platform-api/pkg/handlers/info_test.go @@ -2,6 +2,7 @@ package handlers import ( "encoding/json" + "log/slog" "net/http" "net/http/httptest" "testing" @@ -10,7 +11,7 @@ import ( func TestInfoHandler_Success(t *testing.T) { t.Setenv("TARGET_GROUP_ARN", "arn:aws:elasticloadbalancing:us-east-1:123456789012:targetgroup/rosa-api/abc123") - handler := NewInfoHandler() + handler := NewInfoHandler(slog.Default()) req := httptest.NewRequest(http.MethodGet, "/api/v0/info", nil) w := httptest.NewRecorder() handler.Info(w, req) @@ -37,7 +38,7 @@ func TestInfoHandler_Success(t *testing.T) { func TestInfoHandler_MissingEnvVar(t *testing.T) { t.Setenv("TARGET_GROUP_ARN", "") - handler := NewInfoHandler() + handler := NewInfoHandler(slog.Default()) req := httptest.NewRequest(http.MethodGet, "/api/v0/info", nil) w := httptest.NewRecorder() handler.Info(w, req) @@ -51,7 +52,7 @@ func TestInfoHandler_MissingEnvVar(t *testing.T) { t.Fatalf("failed to decode response: %v", err) } - if result["code"] != "regional-account-unavailable" { + if result["code"] != ErrInfoRegionalAccountUnavailable.Code { t.Errorf("expected code=regional-account-unavailable, got %s", result["code"]) } } @@ -59,7 +60,7 @@ func TestInfoHandler_MissingEnvVar(t *testing.T) { func TestInfoHandler_MalformedARN(t *testing.T) { t.Setenv("TARGET_GROUP_ARN", "not-a-valid-arn") - handler := NewInfoHandler() + handler := NewInfoHandler(slog.Default()) req := httptest.NewRequest(http.MethodGet, "/api/v0/info", nil) w := httptest.NewRecorder() handler.Info(w, req) @@ -73,7 +74,7 @@ func TestInfoHandler_MalformedARN(t *testing.T) { t.Fatalf("failed to decode response: %v", err) } - if result["code"] != "regional-account-unavailable" { + if result["code"] != ErrInfoRegionalAccountUnavailable.Code { t.Errorf("expected code=regional-account-unavailable, got %s", result["code"]) } } diff --git a/platform-api/pkg/handlers/management_cluster.go b/platform-api/pkg/handlers/management_cluster.go index 5edc4551..35850f38 100644 --- a/platform-api/pkg/handlers/management_cluster.go +++ b/platform-api/pkg/handlers/management_cluster.go @@ -10,6 +10,7 @@ import ( hyperfleetv1alpha1 "github.com/openshift-online/rosa-hyperfleet-api/api/v1alpha1" + "github.com/openshift-online/rosa-hyperfleet-api/platform-api/pkg/api" "github.com/openshift-online/rosa-hyperfleet-api/platform-api/pkg/clients/hyperfleetdb" "github.com/openshift-online/rosa-hyperfleet-api/platform-api/pkg/middleware" ) @@ -52,21 +53,21 @@ func (h *ManagementClusterHandler) Create(w http.ResponseWriter, r *http.Request var req ManagementClusterCreateRequest if r.Body != nil && r.ContentLength > 0 { if err := json.NewDecoder(r.Body).Decode(&req); err != nil { - h.writeError(w, http.StatusBadRequest, "invalid-request", "Invalid request body") + writeAPIError(w, ErrMCCreateInvalidBody, h.logger) return } } if req.ID == "" { - h.writeError(w, http.StatusBadRequest, "missing-id", "id is required") + writeAPIError(w, ErrMCCreateMissingID, h.logger) return } if req.Region == "" { - h.writeError(w, http.StatusBadRequest, "missing-region", "region is required") + writeAPIError(w, ErrMCCreateMissingReg, h.logger) return } if req.AccountID == "" { - h.writeError(w, http.StatusBadRequest, "missing-account-id", "accountId is required") + writeAPIError(w, ErrMCCreateMissingAcct, h.logger) return } @@ -82,19 +83,19 @@ func (h *ManagementClusterHandler) Create(w http.ResponseWriter, r *http.Request if err := h.db.CreateManagementCluster(ctx, mc); err != nil { if hyperfleetdb.IsAlreadyExists(err) { - h.writeError(w, http.StatusConflict, "already-exists", "Management cluster already registered: "+req.ID) + writeAPIError(w, ErrMCCreateExists.WithReason(req.ID), h.logger) return } h.logger.Error("failed to create management cluster", "error", err) - h.writeError(w, http.StatusInternalServerError, "config-error", "Failed to save management cluster config") + writeAPIError(w, ErrMCCreateFailed, h.logger) return } - h.logger.Info("management cluster created", "id", mc.Name, "account_id", accountID) + h.logger.Info("management cluster created", "id", redact(mc.Name), "account_id", redact(accountID)) - w.Header().Set("Content-Type", "application/json") - w.WriteHeader(http.StatusCreated) - _ = json.NewEncoder(w).Encode(mcToResponse(mc)) + if err := api.Write(w, http.StatusCreated, mcToResponse(mc)); err != nil { + h.logger.Error("failed to write response", "error", err) + } } // List handles GET /api/v0/management_clusters @@ -107,7 +108,7 @@ func (h *ManagementClusterHandler) List(w http.ResponseWriter, r *http.Request) list, err := h.db.ListManagementClusters(ctx) if err != nil { h.logger.Error("failed to list management clusters", "error", err) - h.writeError(w, http.StatusInternalServerError, "config-error", "Failed to load management cluster config") + writeAPIError(w, ErrMCListFailed, h.logger) return } @@ -118,12 +119,13 @@ func (h *ManagementClusterHandler) List(w http.ResponseWriter, r *http.Request) h.logger.Debug("management clusters listed", "total", len(clusters), "account_id", accountID) - w.Header().Set("Content-Type", "application/json") - _ = json.NewEncoder(w).Encode(map[string]any{ + if err := api.Write(w, http.StatusOK, map[string]any{ "kind": "ManagementClusterList", "items": clusters, "total": len(clusters), - }) + }); err != nil { + h.logger.Error("failed to write response", "error", err) + } } // Get handles GET /api/v0/management_clusters/{id} @@ -138,18 +140,19 @@ func (h *ManagementClusterHandler) Get(w http.ResponseWriter, r *http.Request) { mc, err := h.db.GetManagementCluster(ctx, id) if err != nil { if hyperfleetdb.IsNotFound(err) { - h.writeError(w, http.StatusNotFound, "not-found", "Management cluster not found") + writeAPIError(w, ErrMCGetNotFound, h.logger) return } h.logger.Error("failed to get management cluster", "error", err, "id", id) - h.writeError(w, http.StatusInternalServerError, "config-error", "Failed to load management cluster config") + writeAPIError(w, ErrMCGetFailed, h.logger) return } h.logger.Debug("management cluster retrieved", "id", mc.Name, "account_id", accountID) - w.Header().Set("Content-Type", "application/json") - _ = json.NewEncoder(w).Encode(mcToResponse(mc)) + if err := api.Write(w, http.StatusOK, mcToResponse(mc)); err != nil { + h.logger.Error("failed to write response", "error", err) + } } func mcToResponse(mc *hyperfleetv1alpha1.ManagementCluster) ManagementClusterResponse { @@ -159,16 +162,3 @@ func mcToResponse(mc *hyperfleetv1alpha1.ManagementCluster) ManagementClusterRes AccountID: mc.Spec.AccountID, } } - -func (h *ManagementClusterHandler) writeError(w http.ResponseWriter, status int, code, reason string) { - w.Header().Set("Content-Type", "application/json") - w.WriteHeader(status) - - resp := map[string]any{ - "kind": "Error", - "code": code, - "reason": reason, - } - - _ = json.NewEncoder(w).Encode(resp) -} diff --git a/platform-api/pkg/handlers/nodepool.go b/platform-api/pkg/handlers/nodepool.go index 1d900d0c..e81274ff 100644 --- a/platform-api/pkg/handlers/nodepool.go +++ b/platform-api/pkg/handlers/nodepool.go @@ -9,7 +9,9 @@ import ( "github.com/google/uuid" "github.com/gorilla/mux" + "github.com/openshift-online/rosa-hyperfleet-api/platform-api/internal/codegen/featuregate" + "github.com/openshift-online/rosa-hyperfleet-api/platform-api/pkg/api" "github.com/openshift-online/rosa-hyperfleet-api/platform-api/pkg/clients/hyperfleetdb" "github.com/openshift-online/rosa-hyperfleet-api/platform-api/pkg/middleware" "github.com/openshift-online/rosa-hyperfleet-api/platform-api/pkg/types" @@ -58,7 +60,7 @@ func (h *NodePoolHandler) List(w http.ResponseWriter, r *http.Request) { list, err := h.db.ListNodePools(ctx, accountID, clusterID) if err != nil { h.logger.Error("failed to list nodepools", "error", err, "account_id", accountID) - h.writeError(w, http.StatusInternalServerError, "NODEPOOLS-MGMT-LIST-001", "Failed to list nodepools") + writeAPIError(w, ErrNodePoolList, h.logger) return } @@ -83,7 +85,9 @@ func (h *NodePoolHandler) List(w http.ResponseWriter, r *http.Request) { "offset": offset, } - h.writeJSON(w, http.StatusOK, response) + if err := api.Write(w, http.StatusOK, response); err != nil { + h.logger.Error("failed to write response", "error", err) + } } func (h *NodePoolHandler) Create(w http.ResponseWriter, r *http.Request) { @@ -92,27 +96,27 @@ func (h *NodePoolHandler) Create(w http.ResponseWriter, r *http.Request) { var req types.NodePoolCreateRequest if err := json.NewDecoder(r.Body).Decode(&req); err != nil { - h.writeError(w, http.StatusBadRequest, "NODEPOOLS-MGMT-CREATE-001", "Invalid request body") + writeAPIError(w, ErrNodePoolCreateInvalidBody, h.logger) return } if req.Name == "" || req.ClusterID == "" || req.Spec == nil { - h.writeError(w, http.StatusBadRequest, "NODEPOOLS-MGMT-CREATE-002", "Missing required fields: name, cluster_id, and spec") + writeAPIError(w, ErrNodePoolCreateMissingFields, h.logger) return } if errs := h.validator.ValidateCreate(req.Spec, featuregate.Default); errs != nil { - h.writeValidationErrors(w, errs) + writeAPIError(w, ErrNodePoolValidation.WithErrors(errs), h.logger) return } if _, err := h.db.GetCluster(ctx, accountID, req.ClusterID); err != nil { if hyperfleetdb.IsNotFound(err) { - h.writeError(w, http.StatusNotFound, "NODEPOOLS-MGMT-CREATE-004", "Referenced cluster not found") + writeAPIError(w, ErrNodePoolCreateClusterNotFound, h.logger) return } h.logger.Error("failed to verify cluster exists", "error", err, "account_id", accountID, "cluster_id", req.ClusterID) - h.writeError(w, http.StatusInternalServerError, "NODEPOOLS-MGMT-CREATE-005", "Failed to validate cluster reference") + writeAPIError(w, ErrNodePoolCreateClusterCheck, h.logger) return } @@ -122,21 +126,23 @@ func (h *NodePoolHandler) Create(w http.ResponseWriter, r *http.Request) { cr, err := hyperfleetdb.PlatformCreateToNodePoolCR(accountID, internalPoolID, &req) if err != nil { h.logger.Error("failed to convert nodepool spec", "error", err, "account_id", accountID) - h.writeError(w, http.StatusBadRequest, "NODEPOOLS-MGMT-CREATE-002", "Invalid nodepool spec") + writeAPIError(w, ErrNodePoolCreateInvalidSpec, h.logger) return } if err := h.db.CreateNodePool(ctx, accountID, cr); err != nil { h.logger.Error("failed to create nodepool", "error", err, "account_id", accountID) if hyperfleetdb.IsAlreadyExists(err) { - h.writeError(w, http.StatusConflict, "NODEPOOLS-MGMT-CREATE-003", "NodePool already exists") + writeAPIError(w, ErrNodePoolCreateNameConflict, h.logger) return } - h.writeError(w, http.StatusInternalServerError, "NODEPOOLS-MGMT-CREATE-003", "Failed to create nodepool") + writeAPIError(w, ErrNodePoolCreateFailed, h.logger) return } - h.writeJSON(w, http.StatusCreated, hyperfleetdb.NodePoolCRToPlatform(cr)) + if err := api.Write(w, http.StatusCreated, hyperfleetdb.NodePoolCRToPlatform(cr)); err != nil { + h.logger.Error("failed to write response", "error", err) + } } func (h *NodePoolHandler) Get(w http.ResponseWriter, r *http.Request) { @@ -150,15 +156,17 @@ func (h *NodePoolHandler) Get(w http.ResponseWriter, r *http.Request) { cr, err := h.db.GetNodePool(ctx, accountID, nodepoolID) if err != nil { if hyperfleetdb.IsNotFound(err) { - h.writeError(w, http.StatusNotFound, "NODEPOOLS-MGMT-GET-001", "NodePool not found") + writeAPIError(w, ErrNodePoolGetNotFound, h.logger) return } h.logger.Error("failed to get nodepool", "error", err, "account_id", accountID, "nodepool_id", nodepoolID) - h.writeError(w, http.StatusInternalServerError, "NODEPOOLS-MGMT-GET-002", "Failed to get nodepool") + writeAPIError(w, ErrNodePoolGetFailed, h.logger) return } - h.writeJSON(w, http.StatusOK, hyperfleetdb.NodePoolCRToPlatform(cr)) + if err := api.Write(w, http.StatusOK, hyperfleetdb.NodePoolCRToPlatform(cr)); err != nil { + h.logger.Error("failed to write response", "error", err) + } } func (h *NodePoolHandler) Update(w http.ResponseWriter, r *http.Request) { @@ -169,18 +177,18 @@ func (h *NodePoolHandler) Update(w http.ResponseWriter, r *http.Request) { body, err := io.ReadAll(r.Body) if err != nil { - h.writeError(w, http.StatusBadRequest, "NODEPOOLS-MGMT-UPDATE-001", "Failed to read request body") + writeAPIError(w, ErrNodePoolUpdateInvalidBody, h.logger) return } var req types.NodePoolUpdateRequest if err := json.Unmarshal(body, &req); err != nil { - h.writeError(w, http.StatusBadRequest, "NODEPOOLS-MGMT-UPDATE-001", "Invalid request body") + writeAPIError(w, ErrNodePoolUpdateInvalidBody, h.logger) return } if req.Spec == nil { - h.writeError(w, http.StatusBadRequest, "NODEPOOLS-MGMT-UPDATE-002", "Missing required field: spec") + writeAPIError(w, ErrNodePoolUpdateMissingFields, h.logger) return } @@ -189,16 +197,16 @@ func (h *NodePoolHandler) Update(w http.ResponseWriter, r *http.Request) { cr, err := h.db.GetNodePool(ctx, accountID, nodepoolID) if err != nil { if hyperfleetdb.IsNotFound(err) { - h.writeError(w, http.StatusNotFound, "NODEPOOLS-MGMT-UPDATE-003", "NodePool not found") + writeAPIError(w, ErrNodePoolUpdateNotFound, h.logger) return } h.logger.Error("failed to get nodepool for update", "error", err, "account_id", accountID, "nodepool_id", nodepoolID) - h.writeError(w, http.StatusInternalServerError, "NODEPOOLS-MGMT-UPDATE-004", "Failed to update nodepool") + writeAPIError(w, ErrNodePoolUpdateFailed, h.logger) return } if errs := h.validator.ValidateUpdate(req.Spec, &cr.Spec, featuregate.Default); errs != nil { - h.writeValidationErrors(w, errs) + writeAPIError(w, ErrNodePoolValidation.WithErrors(errs), h.logger) return } @@ -206,23 +214,25 @@ func (h *NodePoolHandler) Update(w http.ResponseWriter, r *http.Request) { Spec json.RawMessage `json:"spec"` } if err := json.Unmarshal(body, &envelope); err != nil { - h.writeError(w, http.StatusBadRequest, "NODEPOOLS-MGMT-UPDATE-001", "Invalid request body") + writeAPIError(w, ErrNodePoolUpdateInvalidBody, h.logger) return } if err := hyperfleetdb.MergeSpecJSON(&cr.Spec, envelope.Spec); err != nil { h.logger.Error("failed to merge nodepool spec", "error", err) - h.writeError(w, http.StatusBadRequest, "NODEPOOLS-MGMT-UPDATE-002", "Invalid nodepool spec") + writeAPIError(w, ErrNodePoolUpdateInvalidSpec, h.logger) return } if err := h.db.UpdateNodePool(ctx, cr); err != nil { h.logger.Error("failed to update nodepool", "error", err, "account_id", accountID, "nodepool_id", nodepoolID) - h.writeError(w, http.StatusInternalServerError, "NODEPOOLS-MGMT-UPDATE-004", "Failed to update nodepool") + writeAPIError(w, ErrNodePoolUpdateFailed, h.logger) return } - h.writeJSON(w, http.StatusOK, hyperfleetdb.NodePoolCRToPlatform(cr)) + if err := api.Write(w, http.StatusOK, hyperfleetdb.NodePoolCRToPlatform(cr)); err != nil { + h.logger.Error("failed to write response", "error", err) + } } func (h *NodePoolHandler) Delete(w http.ResponseWriter, r *http.Request) { @@ -236,11 +246,11 @@ func (h *NodePoolHandler) Delete(w http.ResponseWriter, r *http.Request) { err := h.db.DeleteNodePool(ctx, accountID, nodepoolID) if err != nil { if hyperfleetdb.IsNotFound(err) { - h.writeError(w, http.StatusNotFound, "NODEPOOLS-MGMT-DELETE-001", "NodePool not found") + writeAPIError(w, ErrNodePoolDeleteNotFound, h.logger) return } h.logger.Error("failed to delete nodepool", "error", err, "account_id", accountID, "nodepool_id", nodepoolID) - h.writeError(w, http.StatusInternalServerError, "NODEPOOLS-MGMT-DELETE-002", "Failed to delete nodepool") + writeAPIError(w, ErrNodePoolDeleteFailed, h.logger) return } @@ -249,7 +259,9 @@ func (h *NodePoolHandler) Delete(w http.ResponseWriter, r *http.Request) { "nodepool_id": nodepoolID, } - h.writeJSON(w, http.StatusAccepted, response) + if err := api.Write(w, http.StatusAccepted, response); err != nil { + h.logger.Error("failed to write response", "error", err) + } } func (h *NodePoolHandler) GetStatus(w http.ResponseWriter, r *http.Request) { @@ -263,42 +275,15 @@ func (h *NodePoolHandler) GetStatus(w http.ResponseWriter, r *http.Request) { cr, err := h.db.GetNodePool(ctx, accountID, nodepoolID) if err != nil { if hyperfleetdb.IsNotFound(err) { - h.writeError(w, http.StatusNotFound, "NODEPOOLS-MGMT-STATUS-001", "NodePool not found") + writeAPIError(w, ErrNodePoolStatusNotFound, h.logger) return } h.logger.Error("failed to get nodepool status", "error", err, "account_id", accountID, "nodepool_id", nodepoolID) - h.writeError(w, http.StatusInternalServerError, "NODEPOOLS-MGMT-STATUS-002", "Failed to get nodepool status") + writeAPIError(w, ErrNodePoolStatusFailed, h.logger) return } - h.writeJSON(w, http.StatusOK, hyperfleetdb.NodePoolStatusFromCR(cr)) -} - -func (h *NodePoolHandler) writeJSON(w http.ResponseWriter, status int, data any) { - w.Header().Set("Content-Type", "application/json") - w.WriteHeader(status) - _ = json.NewEncoder(w).Encode(data) -} - -func (h *NodePoolHandler) writeValidationErrors(w http.ResponseWriter, errs validation.ValidationErrors) { - w.Header().Set("Content-Type", "application/json") - w.WriteHeader(http.StatusUnprocessableEntity) - resp := map[string]any{ - "kind": "Error", - "code": "NODEPOOLS-MGMT-VALIDATION-001", - "reason": "Request validation failed", - "errors": errs, - } - _ = json.NewEncoder(w).Encode(resp) -} - -func (h *NodePoolHandler) writeError(w http.ResponseWriter, status int, code, reason string) { - w.Header().Set("Content-Type", "application/json") - w.WriteHeader(status) - resp := map[string]any{ - "kind": "Error", - "code": code, - "reason": reason, + if err := api.Write(w, http.StatusOK, hyperfleetdb.NodePoolStatusFromCR(cr)); err != nil { + h.logger.Error("failed to write response", "error", err) } - _ = json.NewEncoder(w).Encode(resp) } diff --git a/platform-api/pkg/handlers/redact.go b/platform-api/pkg/handlers/redact.go new file mode 100644 index 00000000..a5c5a165 --- /dev/null +++ b/platform-api/pkg/handlers/redact.go @@ -0,0 +1,17 @@ +package handlers + +import ( + "math" + "strings" +) + +// redact masks the first half of s with asterisks for safe logging of +// customer identifiers. +func redact(s string) string { + if len(s) == 0 { + return s + } + runes := []rune(s) + half := int(math.Ceil(float64(len(runes)) / 2)) + return strings.Repeat("*", half) + string(runes[half:]) +} diff --git a/platform-api/pkg/handlers/redact_test.go b/platform-api/pkg/handlers/redact_test.go new file mode 100644 index 00000000..703bbe0d --- /dev/null +++ b/platform-api/pkg/handlers/redact_test.go @@ -0,0 +1,24 @@ +package handlers + +import "testing" + +func TestRedact(t *testing.T) { + tests := []struct { + input string + want string + }{ + {"", ""}, + {"a", "*"}, + {"ab", "*b"}, + {"abcd", "**cd"}, + {"123456789012", "******789012"}, + {"odd", "**d"}, + {"aé", "*é"}, + } + + for _, tt := range tests { + if got := redact(tt.input); got != tt.want { + t.Errorf("redact(%q) = %q, want %q", tt.input, got, tt.want) + } + } +} diff --git a/platform-api/pkg/handlers/zoa.go b/platform-api/pkg/handlers/zoa.go index 9624c6d5..c2f821a6 100644 --- a/platform-api/pkg/handlers/zoa.go +++ b/platform-api/pkg/handlers/zoa.go @@ -16,6 +16,7 @@ import ( "github.com/google/uuid" "github.com/gorilla/mux" + "github.com/openshift-online/rosa-hyperfleet-api/platform-api/pkg/api" "github.com/openshift-online/rosa-hyperfleet-api/platform-api/pkg/clients/hyperfleetdb" "github.com/openshift-online/rosa-hyperfleet-api/platform-api/pkg/middleware" "github.com/openshift-online/rosa-hyperfleet-api/platform-api/pkg/zoa" @@ -77,30 +78,30 @@ func (h *ZoaHandler) Create(w http.ResponseWriter, r *http.Request) { tmpl, ok := h.registry.Get(action) if !ok { - h.writeError(w, http.StatusNotFound, "unknown-action", "Trusted action not found: "+action) + writeAPIError(w, ErrZoaCreateUnknownAction.WithReason(action), h.logger) return } var req zoa.CreateRequest if err := json.NewDecoder(r.Body).Decode(&req); err != nil { - h.writeError(w, http.StatusBadRequest, "invalid-request", "Invalid request body") + writeAPIError(w, ErrZoaCreateInvalidBody, h.logger) return } if req.TargetCluster == "" { h.recordAudit(ctx, r, accountID, callerARN, extractOperator(callerARN), http.StatusBadRequest, action, "", "", "", "") - h.writeError(w, http.StatusBadRequest, "missing-target-cluster", "target_cluster is required") + writeAPIError(w, ErrZoaCreateMissingCluster, h.logger) return } if req.Jira == "" { h.recordAudit(ctx, r, accountID, callerARN, extractOperator(callerARN), http.StatusBadRequest, action, req.TargetCluster, "", "", "") - h.writeError(w, http.StatusBadRequest, "missing-jira", "jira is required for all trusted actions (e.g. ROSAENG-1234)") + writeAPIError(w, ErrZoaCreateMissingJira, h.logger) return } if !isValidJiraFormat(req.Jira) { h.recordAudit(ctx, r, accountID, callerARN, extractOperator(callerARN), http.StatusBadRequest, action, req.TargetCluster, "", req.Jira, "") - h.writeError(w, http.StatusBadRequest, "invalid-jira", "jira does not have correct format; expected PROJECT-NUMBER (e.g. ROSAENG-1234)") + writeAPIError(w, ErrZoaCreateInvalidJira, h.logger) return } @@ -113,7 +114,7 @@ func (h *ZoaHandler) Create(w http.ResponseWriter, r *http.Request) { if err := validateParams(tmpl, cleanParams); err != nil { h.recordAudit(ctx, r, accountID, callerARN, extractOperator(callerARN), http.StatusBadRequest, action, req.TargetCluster, "", req.Jira, "") - h.writeError(w, http.StatusBadRequest, "invalid-params", err.Error()) + writeAPIError(w, ErrZoaCreateInvalidParams.WithReason(err), h.logger) return } @@ -125,7 +126,7 @@ func (h *ZoaHandler) Create(w http.ResponseWriter, r *http.Request) { if cooldown > 0 { if err := h.checkWriteCooldown(ctx, accountID, action, req.TargetCluster, cooldown); err != nil { h.recordAudit(ctx, r, accountID, callerARN, extractOperator(callerARN), http.StatusTooManyRequests, action, req.TargetCluster, "", req.Jira, "") - h.writeError(w, http.StatusTooManyRequests, "write-cooldown", err.Error()) + writeAPIError(w, ErrZoaCreateCooldown.WithReason(err), h.logger) return } } @@ -138,7 +139,7 @@ func (h *ZoaHandler) Create(w http.ResponseWriter, r *http.Request) { } if err := h.checkMaxConcurrent(ctx, accountID, req.TargetCluster, maxConcurrent); err != nil { h.recordAudit(ctx, r, accountID, callerARN, extractOperator(callerARN), http.StatusTooManyRequests, action, req.TargetCluster, "", req.Jira, "") - h.writeError(w, http.StatusTooManyRequests, "max-concurrent", err.Error()) + writeAPIError(w, ErrZoaCreateMaxConcurrent.WithReason(err), h.logger) return } } @@ -151,7 +152,7 @@ func (h *ZoaHandler) Create(w http.ResponseWriter, r *http.Request) { executedAction = tmpl.DryRunAction dryTmpl, ok := h.registry.Get(executedAction) if !ok { - h.writeError(w, http.StatusInternalServerError, "dry-run-error", "dry_run_action '"+tmpl.DryRunAction+"' not found in registry") + writeAPIError(w, ErrZoaCreateDryRunError.WithReason(tmpl.DryRunAction), h.logger) return } tmpl = dryTmpl @@ -184,7 +185,7 @@ func (h *ZoaHandler) Create(w http.ResponseWriter, r *http.Request) { if err := h.store.Create(ctx, exec); err != nil { h.logger.Error("failed to create execution record", "error", err, "execution_id", execID) - h.writeError(w, http.StatusInternalServerError, "store-error", "Failed to create execution") + writeAPIError(w, ErrZoaCreateStoreFailed, h.logger) return } @@ -206,14 +207,14 @@ func (h *ZoaHandler) Create(w http.ResponseWriter, r *http.Request) { if err != nil { h.logger.Error("failed to build manifest", "error", err, "execution_id", execID) _ = h.store.UpdateStatus(ctx, execID, zoa.StatusFailed, time.Now().UTC().Format(time.RFC3339), 0) - h.writeError(w, http.StatusInternalServerError, "render-error", "Failed to build trusted action manifest") + writeAPIError(w, ErrZoaCreateRenderFailed, h.logger) return } if err := h.db.CreateManifest(ctx, zoa.JobNamespace, hfm); err != nil { h.logger.Error("failed to create manifest on hyperfleet-db", "error", err, "execution_id", execID) _ = h.store.UpdateStatus(ctx, execID, zoa.StatusFailed, time.Now().UTC().Format(time.RFC3339), 0) - h.writeError(w, http.StatusBadGateway, "dispatch-error", "Failed to dispatch trusted action") + writeAPIError(w, ErrZoaCreateDispatchFailed, h.logger) return } @@ -222,7 +223,7 @@ func (h *ZoaHandler) Create(w http.ResponseWriter, r *http.Request) { h.logger.Error("failed to update manifest name, cleaning up", "error", err, "execution_id", execID) _ = h.db.DeleteManifest(ctx, zoa.JobNamespace, hfm.Name) _ = h.store.UpdateStatus(ctx, execID, zoa.StatusFailed, time.Now().UTC().Format(time.RFC3339), 0) - h.writeError(w, http.StatusInternalServerError, "store-error", "Failed to persist execution state") + writeAPIError(w, ErrZoaCreateStoreSaveFailed, h.logger) return } @@ -238,9 +239,9 @@ func (h *ZoaHandler) Create(w http.ResponseWriter, r *http.Request) { h.recordAudit(ctx, r, accountID, callerARN, operator, http.StatusAccepted, originalAction, req.TargetCluster, execID, req.Jira, string(exec.ApprovalState)) - w.Header().Set("Content-Type", "application/json") - w.WriteHeader(http.StatusAccepted) - _ = json.NewEncoder(w).Encode(exec) + if err := api.Write(w, http.StatusAccepted, exec); err != nil { + h.logger.Error("failed to write response", "error", err) + } } // Get handles GET /api/v0/trusted-actions/runs/{id} @@ -254,12 +255,12 @@ func (h *ZoaHandler) Get(w http.ResponseWriter, r *http.Request) { exec, err := h.store.Get(ctx, execID) if err != nil { h.logger.Error("failed to get execution", "error", err, "execution_id", execID) - h.writeError(w, http.StatusInternalServerError, "store-error", "Failed to retrieve execution") + writeAPIError(w, ErrZoaGetStoreFailed, h.logger) return } if exec == nil { - h.writeError(w, http.StatusNotFound, "not-found", "Execution not found") + writeAPIError(w, ErrZoaGetNotFound, h.logger) return } @@ -305,9 +306,9 @@ func (h *ZoaHandler) Get(w http.ResponseWriter, r *http.Request) { } } - w.Header().Set("Content-Type", "application/json") - w.WriteHeader(http.StatusOK) - _ = json.NewEncoder(w).Encode(response) + if err := api.Write(w, http.StatusOK, response); err != nil { + h.logger.Error("failed to write response", "error", err) + } } // List handles GET /api/v0/trusted-actions/runs @@ -357,7 +358,7 @@ func (h *ZoaHandler) List(w http.ResponseWriter, r *http.Request) { executions, err := h.store.List(ctx, accountID, limit, filter) if err != nil { h.logger.Error("failed to list executions", "error", err, "account_id", accountID) - h.writeError(w, http.StatusInternalServerError, "store-error", "Failed to list executions") + writeAPIError(w, ErrZoaListStoreFailed, h.logger) return } @@ -373,9 +374,9 @@ func (h *ZoaHandler) List(w http.ResponseWriter, r *http.Request) { operator := extractOperator(callerARN) h.recordAudit(ctx, r, accountID, callerARN, operator, http.StatusOK, "", "", "", "", "") - w.Header().Set("Content-Type", "application/json") - w.WriteHeader(http.StatusOK) - _ = json.NewEncoder(w).Encode(response) + if err := api.Write(w, http.StatusOK, response); err != nil { + h.logger.Error("failed to write response", "error", err) + } } // parseSince converts a duration shorthand (e.g. "1h", "24h", "7d") or RFC3339 timestamp @@ -431,12 +432,12 @@ func (h *ZoaHandler) Catalog(w http.ResponseWriter, r *http.Request) { }) } - w.Header().Set("Content-Type", "application/json") - w.WriteHeader(http.StatusOK) - _ = json.NewEncoder(w).Encode(map[string]any{ + if err := api.Write(w, http.StatusOK, map[string]any{ "items": items, "total": len(items), - }) + }); err != nil { + h.logger.Error("failed to write response", "error", err) + } } // Describe handles GET /api/v0/trusted-actions/{action} @@ -445,7 +446,7 @@ func (h *ZoaHandler) Describe(w http.ResponseWriter, r *http.Request) { tmpl, ok := h.registry.Get(action) if !ok { - h.writeError(w, http.StatusNotFound, "unknown-action", "Trusted action not found: "+action) + writeAPIError(w, ErrZoaCreateUnknownAction.WithReason(action), h.logger) return } @@ -461,9 +462,9 @@ func (h *ZoaHandler) Describe(w http.ResponseWriter, r *http.Request) { RequiredFields: []string{"target_cluster", "jira"}, } - w.Header().Set("Content-Type", "application/json") - w.WriteHeader(http.StatusOK) - _ = json.NewEncoder(w).Encode(response) + if err := api.Write(w, http.StatusOK, response); err != nil { + h.logger.Error("failed to write response", "error", err) + } } func (h *ZoaHandler) fetchS3Content(ctx context.Context, s3URI string) ([]byte, error) { @@ -582,16 +583,6 @@ func extractOperator(callerARN string) string { return callerARN } -func (h *ZoaHandler) writeError(w http.ResponseWriter, status int, code, reason string) { - w.Header().Set("Content-Type", "application/json") - w.WriteHeader(status) - _ = json.NewEncoder(w).Encode(map[string]any{ - "kind": "Error", - "code": code, - "reason": reason, - }) -} - func (h *ZoaHandler) checkWriteCooldown(ctx context.Context, accountID, action, targetCluster string, cooldownSeconds int) error { since := time.Now().UTC().Add(-time.Duration(cooldownSeconds) * time.Second).Format(time.RFC3339) notDryRun := false @@ -663,7 +654,7 @@ func (h *ZoaHandler) recordAudit(ctx context.Context, r *http.Request, accountID // AuditList handles GET /api/v0/trusted-actions/audit func (h *ZoaHandler) AuditList(w http.ResponseWriter, r *http.Request) { if h.auditStore == nil { - h.writeError(w, http.StatusNotFound, "audit-disabled", "Audit logging is not enabled") + writeAPIError(w, ErrZoaAuditDisabled, h.logger) return } @@ -697,7 +688,7 @@ func (h *ZoaHandler) AuditList(w http.ResponseWriter, r *http.Request) { entries, err := h.auditStore.List(ctx, accountID, limit, filter) if err != nil { h.logger.Error("failed to list audit entries", "error", err) - h.writeError(w, http.StatusInternalServerError, "store-error", "Failed to list audit log") + writeAPIError(w, ErrZoaAuditListFailed, h.logger) return } @@ -705,11 +696,11 @@ func (h *ZoaHandler) AuditList(w http.ResponseWriter, r *http.Request) { operator := extractOperator(callerARN) h.recordAudit(ctx, r, accountID, callerARN, operator, http.StatusOK, "", "", "", "", "") - w.Header().Set("Content-Type", "application/json") - w.WriteHeader(http.StatusOK) - _ = json.NewEncoder(w).Encode(map[string]any{ + if err := api.Write(w, http.StatusOK, map[string]any{ "kind": "AuditList", "items": entries, "total": len(entries), - }) + }); err != nil { + h.logger.Error("failed to write response", "error", err) + } } diff --git a/platform-api/pkg/handlers/zoa_test.go b/platform-api/pkg/handlers/zoa_test.go index ab57b532..10c3a739 100644 --- a/platform-api/pkg/handlers/zoa_test.go +++ b/platform-api/pkg/handlers/zoa_test.go @@ -360,7 +360,7 @@ func TestZoaHandler_Create_UnknownParams(t *testing.T) { var errResp map[string]any err := json.NewDecoder(rr.Body).Decode(&errResp) require.NoError(t, err) - assert.Equal(t, "invalid-params", errResp["code"]) + assert.Equal(t, ErrZoaCreateInvalidParams.Code, errResp["code"]) assert.Contains(t, errResp["reason"], "unknown parameter 'namespace'") assert.Contains(t, errResp["reason"], "node_selector") } @@ -412,7 +412,7 @@ script: | assert.Equal(t, http.StatusTooManyRequests, rr.Code) var errResp map[string]any require.NoError(t, json.NewDecoder(rr.Body).Decode(&errResp)) - assert.Equal(t, "write-cooldown", errResp["code"]) + assert.Equal(t, ErrZoaCreateCooldown.Code, errResp["code"]) } func TestZoaHandler_Create_WriteCooldown_ForceBypass(t *testing.T) { @@ -499,7 +499,7 @@ func TestZoaHandler_Create_MaxConcurrent(t *testing.T) { assert.Equal(t, http.StatusTooManyRequests, rr.Code) var errResp map[string]any require.NoError(t, json.NewDecoder(rr.Body).Decode(&errResp)) - assert.Equal(t, "max-concurrent", errResp["code"]) + assert.Equal(t, ErrZoaCreateMaxConcurrent.Code, errResp["code"]) assert.Contains(t, errResp["reason"].(string), "10 active executions") } diff --git a/platform-api/pkg/middleware/account_check.go b/platform-api/pkg/middleware/account_check.go index b784a149..4e4fdc0f 100644 --- a/platform-api/pkg/middleware/account_check.go +++ b/platform-api/pkg/middleware/account_check.go @@ -1,7 +1,6 @@ package middleware import ( - "encoding/json" "log/slog" "net/http" @@ -30,7 +29,7 @@ func (a *AccountCheck) RequireProvisioned(next http.Handler) http.Handler { accountID := GetAccountID(ctx) if accountID == "" { - a.writeError(w, http.StatusForbidden, "missing-account-id", "Account ID header is required") + writeError(w, ErrMissingAccountID, a.logger) return } @@ -44,30 +43,16 @@ func (a *AccountCheck) RequireProvisioned(next http.Handler) http.Handler { provisioned, err := a.authorizer.IsAccountProvisioned(ctx, accountID) if err != nil { a.logger.Error("failed to check account provisioning status", "error", err, "account_id", accountID) - a.writeError(w, http.StatusInternalServerError, "internal-error", "Failed to check account status") + writeError(w, ErrProvisionedCheckFailed, a.logger) return } if !provisioned { a.logger.Warn("account not provisioned", "account_id", accountID) - a.writeError(w, http.StatusForbidden, "account-not-provisioned", - "Account is not provisioned for ROSA authorization. Contact your administrator.") + writeError(w, ErrAccountNotProvisioned, a.logger) return } next.ServeHTTP(w, r) }) } - -func (a *AccountCheck) writeError(w http.ResponseWriter, status int, code, reason string) { - w.Header().Set("Content-Type", "application/json") - w.WriteHeader(status) - - resp := map[string]any{ - "kind": "Error", - "code": code, - "reason": reason, - } - - _ = json.NewEncoder(w).Encode(resp) -} diff --git a/platform-api/pkg/middleware/admin_check.go b/platform-api/pkg/middleware/admin_check.go index 4e284ea5..7c9005a8 100644 --- a/platform-api/pkg/middleware/admin_check.go +++ b/platform-api/pkg/middleware/admin_check.go @@ -1,7 +1,6 @@ package middleware import ( - "encoding/json" "log/slog" "net/http" @@ -30,7 +29,7 @@ func (a *AdminCheck) RequireAdmin(next http.Handler) http.Handler { accountID := GetAccountID(ctx) if accountID == "" { - a.writeError(w, http.StatusForbidden, "missing-account-id", "Account ID header is required") + writeError(w, ErrMissingAccountID, a.logger) return } @@ -42,36 +41,23 @@ func (a *AdminCheck) RequireAdmin(next http.Handler) http.Handler { callerARN := GetCallerARN(ctx) if callerARN == "" { - a.writeError(w, http.StatusForbidden, "missing-caller-arn", "Caller ARN header is required") + writeError(w, ErrMissingCallerARN, a.logger) return } isAdmin, err := a.authorizer.IsAdmin(ctx, accountID, callerARN) if err != nil { a.logger.Error("failed to check admin status", "error", err, "account_id", accountID, "caller_arn", callerARN) - a.writeError(w, http.StatusInternalServerError, "internal-error", "Failed to check admin status") + writeError(w, ErrAdminCheckFailed, a.logger) return } if !isAdmin { a.logger.Warn("admin access denied", "account_id", accountID, "caller_arn", callerARN) - a.writeError(w, http.StatusForbidden, "not-admin", "This operation requires admin privileges") + writeError(w, ErrNotAdmin, a.logger) return } next.ServeHTTP(w, r) }) } - -func (a *AdminCheck) writeError(w http.ResponseWriter, status int, code, reason string) { - w.Header().Set("Content-Type", "application/json") - w.WriteHeader(status) - - resp := map[string]any{ - "kind": "Error", - "code": code, - "reason": reason, - } - - _ = json.NewEncoder(w).Encode(resp) -} diff --git a/platform-api/pkg/middleware/admin_check_test.go b/platform-api/pkg/middleware/admin_check_test.go index ace7ccc8..96c7e6d9 100644 --- a/platform-api/pkg/middleware/admin_check_test.go +++ b/platform-api/pkg/middleware/admin_check_test.go @@ -117,8 +117,8 @@ func TestAdminCheck_RequireAdmin_NonAdminCaller(t *testing.T) { if errorResp["kind"] != "Error" { t.Errorf("expected kind=Error, got %v", errorResp["kind"]) } - if errorResp["code"] != "not-admin" { - t.Errorf("expected code=not-admin, got %v", errorResp["code"]) + if errorResp["code"] != ErrNotAdmin.Code { + t.Errorf("expected code=%s, got %v", ErrNotAdmin.Code, errorResp["code"]) } if errorResp["reason"] != "This operation requires admin privileges" { t.Errorf("expected reason='This operation requires admin privileges', got %v", errorResp["reason"]) @@ -189,8 +189,8 @@ func TestAdminCheck_RequireAdmin_MissingCallerARN(t *testing.T) { if err := json.NewDecoder(w.Body).Decode(&errorResp); err != nil { t.Fatalf("failed to decode error response: %v", err) } - if errorResp["code"] != "missing-caller-arn" { - t.Errorf("expected code=missing-caller-arn, got %v", errorResp["code"]) + if errorResp["code"] != ErrMissingCallerARN.Code { + t.Errorf("expected code=%s, got %v", ErrMissingCallerARN.Code, errorResp["code"]) } } @@ -228,8 +228,8 @@ func TestAdminCheck_RequireAdmin_IsAdminError(t *testing.T) { if err := json.NewDecoder(w.Body).Decode(&errorResp); err != nil { t.Fatalf("failed to decode error response: %v", err) } - if errorResp["code"] != "internal-error" { - t.Errorf("expected code=internal-error, got %v", errorResp["code"]) + if errorResp["code"] != ErrAdminCheckFailed.Code { + t.Errorf("expected code=%s, got %v", ErrAdminCheckFailed.Code, errorResp["code"]) } } @@ -259,7 +259,7 @@ func TestAdminCheck_RequireAdmin_MissingAccountID(t *testing.T) { if err := json.NewDecoder(w.Body).Decode(&errorResp); err != nil { t.Fatalf("failed to decode error response: %v", err) } - if errorResp["code"] != "missing-account-id" { - t.Errorf("expected code=missing-account-id, got %v", errorResp["code"]) + if errorResp["code"] != ErrMissingAccountID.Code { + t.Errorf("expected code=%s, got %v", ErrMissingAccountID.Code, errorResp["code"]) } } diff --git a/platform-api/pkg/middleware/authorization.go b/platform-api/pkg/middleware/authorization.go index 2a303764..266dcfd6 100644 --- a/platform-api/pkg/middleware/authorization.go +++ b/platform-api/pkg/middleware/authorization.go @@ -1,7 +1,6 @@ package middleware import ( - "encoding/json" "log/slog" "net/http" ) @@ -32,29 +31,16 @@ func (a *Authorization) RequireAllowedAccount(next http.Handler) http.Handler { if accountID == "" { a.logger.Warn("missing account ID in request") - a.writeError(w, http.StatusForbidden, "missing-account-id", "Account ID header is required") + writeError(w, ErrMissingAccountID, a.logger) return } if _, allowed := a.allowedAccounts[accountID]; !allowed { a.logger.Warn("account not allowed", "account_id", accountID) - a.writeError(w, http.StatusForbidden, "account-not-allowed", "account not allowed") + writeError(w, ErrAccountNotAllowed, a.logger) return } next.ServeHTTP(w, r) }) } - -func (a *Authorization) writeError(w http.ResponseWriter, status int, code, reason string) { - w.Header().Set("Content-Type", "application/json") - w.WriteHeader(status) - - resp := map[string]any{ - "kind": "Error", - "code": code, - "reason": reason, - } - - _ = json.NewEncoder(w).Encode(resp) -} diff --git a/platform-api/pkg/middleware/authorization_test.go b/platform-api/pkg/middleware/authorization_test.go index d6a88fc3..47b25162 100644 --- a/platform-api/pkg/middleware/authorization_test.go +++ b/platform-api/pkg/middleware/authorization_test.go @@ -70,8 +70,8 @@ func TestAuthorization_RequireAllowedAccount_NotAllowed(t *testing.T) { t.Errorf("expected kind=Error, got %v", errorResp["kind"]) } - if errorResp["code"] != "account-not-allowed" { - t.Errorf("expected code=account-not-allowed, got %v", errorResp["code"]) + if errorResp["code"] != ErrAccountNotAllowed.Code { + t.Errorf("expected code=%s, got %v", ErrAccountNotAllowed.Code, errorResp["code"]) } if errorResp["reason"] != "account not allowed" { @@ -114,8 +114,8 @@ func TestAuthorization_RequireAllowedAccount_MissingAccountID(t *testing.T) { t.Errorf("expected kind=Error, got %v", errorResp["kind"]) } - if errorResp["code"] != "missing-account-id" { - t.Errorf("expected code=missing-account-id, got %v", errorResp["code"]) + if errorResp["code"] != ErrMissingAccountID.Code { + t.Errorf("expected code=%s, got %v", ErrMissingAccountID.Code, errorResp["code"]) } if errorResp["reason"] != "Account ID header is required" { @@ -153,8 +153,8 @@ func TestAuthorization_RequireAllowedAccount_EmptyAccountID(t *testing.T) { t.Fatalf("failed to decode error response: %v", err) } - if errorResp["code"] != "missing-account-id" { - t.Errorf("expected code=missing-account-id, got %v", errorResp["code"]) + if errorResp["code"] != ErrMissingAccountID.Code { + t.Errorf("expected code=%s, got %v", ErrMissingAccountID.Code, errorResp["code"]) } } @@ -387,49 +387,40 @@ func TestAuthorization_RequireAllowedAccount_TwentyAccounts(t *testing.T) { t.Fatalf("failed to decode error response: %v", err) } - if errorResp["code"] != "account-not-allowed" { - t.Errorf("expected code=account-not-allowed, got %v", errorResp["code"]) + if errorResp["code"] != ErrAccountNotAllowed.Code { + t.Errorf("expected code=%s, got %v", ErrAccountNotAllowed.Code, errorResp["code"]) } }) } func TestAuthorization_WriteError(t *testing.T) { - logger := slog.New(slog.NewTextHandler(os.Stdout, nil)) - auth := NewAuthorization([]string{}, logger) - tests := []struct { name string - status int - code string - reason string + def APIError expectedStatus int expectedCode string expectedReason string }{ { - name: "forbidden error", - status: http.StatusForbidden, - code: "account-not-allowed", - reason: "account not allowed", + name: "account not allowed", + def: ErrAccountNotAllowed, expectedStatus: http.StatusForbidden, - expectedCode: "account-not-allowed", - expectedReason: "account not allowed", + expectedCode: ErrAccountNotAllowed.Code, + expectedReason: ErrAccountNotAllowed.Message, }, { - name: "missing account ID error", - status: http.StatusForbidden, - code: "missing-account-id", - reason: "Account ID header is required", + name: "missing account ID", + def: ErrMissingAccountID, expectedStatus: http.StatusForbidden, - expectedCode: "missing-account-id", - expectedReason: "Account ID header is required", + expectedCode: ErrMissingAccountID.Code, + expectedReason: ErrMissingAccountID.Message, }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { w := httptest.NewRecorder() - auth.writeError(w, tt.status, tt.code, tt.reason) + writeError(w, tt.def, slog.Default()) if w.Code != tt.expectedStatus { t.Errorf("expected status %d, got %d", tt.expectedStatus, w.Code) diff --git a/platform-api/pkg/middleware/authz.go b/platform-api/pkg/middleware/authz.go index a2a5586a..d45aa47a 100644 --- a/platform-api/pkg/middleware/authz.go +++ b/platform-api/pkg/middleware/authz.go @@ -2,7 +2,6 @@ package middleware import ( "context" - "encoding/json" "log/slog" "net/http" "strings" @@ -47,12 +46,12 @@ func (a *Authz) Authorize(next http.Handler) http.Handler { callerARN := GetCallerARN(ctx) if accountID == "" { - a.writeError(w, http.StatusForbidden, "missing-account-id", "Account ID header is required") + writeError(w, ErrMissingAccountID, a.logger) return } if callerARN == "" { - a.writeError(w, http.StatusForbidden, "missing-caller-arn", "Caller ARN header is required") + writeError(w, ErrMissingCallerARN, a.logger) return } @@ -71,11 +70,10 @@ func (a *Authz) Authorize(next http.Handler) http.Handler { a.logger.Error("authorization check failed", "error", err, "account_id", accountID, "action", req.Action) // Check if it's a "not provisioned" error if strings.Contains(err.Error(), "not provisioned") { - a.writeError(w, http.StatusForbidden, "account-not-provisioned", - "Account is not provisioned for ROSA authorization") + writeError(w, ErrAccountNotProvisioned, a.logger) return } - a.writeError(w, http.StatusInternalServerError, "authorization-error", "Authorization check failed") + writeError(w, ErrAuthorizationFailed, a.logger) return } @@ -86,8 +84,7 @@ func (a *Authz) Authorize(next http.Handler) http.Handler { "action", req.Action, "resource", req.Resource, ) - a.writeError(w, http.StatusForbidden, "access-denied", - "You do not have permission to perform this action") + writeError(w, ErrAccessDenied, a.logger) return } @@ -217,16 +214,3 @@ const ( contextKeyResourceTags contextKey = "resource_tags" contextKeyRequestTags contextKey = "request_tags" ) - -func (a *Authz) writeError(w http.ResponseWriter, status int, code, reason string) { - w.Header().Set("Content-Type", "application/json") - w.WriteHeader(status) - - resp := map[string]any{ - "kind": "Error", - "code": code, - "reason": reason, - } - - _ = json.NewEncoder(w).Encode(resp) -} diff --git a/platform-api/pkg/middleware/errorcodes.go b/platform-api/pkg/middleware/errorcodes.go new file mode 100644 index 00000000..da3a6aae --- /dev/null +++ b/platform-api/pkg/middleware/errorcodes.go @@ -0,0 +1,50 @@ +package middleware + +import ( + "log/slog" + "net/http" + + "github.com/openshift-online/rosa-hyperfleet-api/platform-api/pkg/api" +) + +// APIError is an alias for api.APIError so middleware code uses the short form. +type APIError = api.APIError + +func writeError(w http.ResponseWriter, def APIError, logger *slog.Logger) { + if err := api.WriteError(w, def); err != nil { + logger.Error("failed to write error response", "error", err) + } +} + +// Auth middleware error codes +var ( + ErrMissingAccountID APIError + ErrMissingCallerARN APIError + ErrInternalError APIError + ErrAccountNotProvisioned APIError + ErrNotAdmin APIError + ErrNotPrivileged APIError + ErrAccountNotAllowed APIError + ErrAuthorizationFailed APIError + ErrAccessDenied APIError + + ErrAdminCheckFailed APIError + ErrPrivilegedCheckFailed APIError + ErrProvisionedCheckFailed APIError +) + +func init() { + ErrMissingAccountID = APIError{Code: "AUTH-001", HTTPStatus: http.StatusForbidden, Message: "Account ID header is required"} + ErrMissingCallerARN = APIError{Code: "AUTH-002", HTTPStatus: http.StatusForbidden, Message: "Caller ARN header is required"} + ErrInternalError = APIError{Code: "AUTH-003", HTTPStatus: http.StatusInternalServerError, Message: "Internal server error"} + ErrAccountNotProvisioned = APIError{Code: "AUTH-004", HTTPStatus: http.StatusForbidden, Message: "Account is not provisioned for ROSA authorization. Contact your administrator."} + ErrNotAdmin = APIError{Code: "AUTH-005", HTTPStatus: http.StatusForbidden, Message: "This operation requires admin privileges"} + ErrNotPrivileged = APIError{Code: "AUTH-006", HTTPStatus: http.StatusForbidden, Message: "This operation requires a privileged account"} + ErrAccountNotAllowed = APIError{Code: "AUTH-007", HTTPStatus: http.StatusForbidden, Message: "account not allowed"} + ErrAuthorizationFailed = APIError{Code: "AUTH-008", HTTPStatus: http.StatusInternalServerError, Message: "Authorization check failed"} + ErrAccessDenied = APIError{Code: "AUTH-009", HTTPStatus: http.StatusForbidden, Message: "You do not have permission to perform this action"} + + ErrAdminCheckFailed = APIError{Code: "AUTH-010", HTTPStatus: http.StatusInternalServerError, Message: "Failed to check admin status"} + ErrPrivilegedCheckFailed = APIError{Code: "AUTH-011", HTTPStatus: http.StatusInternalServerError, Message: "Failed to check privileged status"} + ErrProvisionedCheckFailed = APIError{Code: "AUTH-012", HTTPStatus: http.StatusInternalServerError, Message: "Failed to check account provisioning status"} +} diff --git a/platform-api/pkg/middleware/privileged.go b/platform-api/pkg/middleware/privileged.go index a1b4710c..30adb65e 100644 --- a/platform-api/pkg/middleware/privileged.go +++ b/platform-api/pkg/middleware/privileged.go @@ -2,7 +2,6 @@ package middleware import ( "context" - "encoding/json" "log/slog" "net/http" @@ -58,7 +57,7 @@ func (p *Privileged) RequirePrivileged(next http.Handler) http.Handler { accountID := GetAccountID(ctx) if accountID == "" { - p.writeError(w, http.StatusForbidden, "missing-account-id", "Account ID header is required") + writeError(w, ErrMissingAccountID, p.logger) return } @@ -69,14 +68,14 @@ func (p *Privileged) RequirePrivileged(next http.Handler) http.Handler { isPrivileged, err = p.authorizer.IsPrivileged(ctx, accountID) if err != nil { p.logger.Error("failed to check privileged status", "error", err, "account_id", accountID) - p.writeError(w, http.StatusInternalServerError, "internal-error", "Failed to check account status") + writeError(w, ErrPrivilegedCheckFailed, p.logger) return } } if !isPrivileged { p.logger.Warn("privileged access denied", "account_id", accountID) - p.writeError(w, http.StatusForbidden, "not-privileged", "This operation requires a privileged account") + writeError(w, ErrNotPrivileged, p.logger) return } @@ -84,19 +83,6 @@ func (p *Privileged) RequirePrivileged(next http.Handler) http.Handler { }) } -func (p *Privileged) writeError(w http.ResponseWriter, status int, code, reason string) { - w.Header().Set("Content-Type", "application/json") - w.WriteHeader(status) - - resp := map[string]any{ - "kind": "Error", - "code": code, - "reason": reason, - } - - _ = json.NewEncoder(w).Encode(resp) -} - // GetPrivileged retrieves the privileged status from context func GetPrivileged(ctx context.Context) bool { if v := ctx.Value(ContextKeyPrivileged); v != nil { diff --git a/platform-api/pkg/ratelimit/middleware.go b/platform-api/pkg/ratelimit/middleware.go index 8e253899..becf35d0 100644 --- a/platform-api/pkg/ratelimit/middleware.go +++ b/platform-api/pkg/ratelimit/middleware.go @@ -2,7 +2,6 @@ package ratelimit import ( "context" - "encoding/json" "fmt" "log/slog" "math" @@ -14,6 +13,7 @@ import ( "github.com/prometheus/client_golang/prometheus" "github.com/prometheus/client_golang/prometheus/promauto" + "github.com/openshift-online/rosa-hyperfleet-api/platform-api/pkg/api" "github.com/openshift-online/rosa-hyperfleet-api/platform-api/pkg/middleware" ) @@ -120,14 +120,15 @@ func (l *Limiter) findLimit(method, path string) RouteLimit { } } +var errRateLimit = api.APIError{ + Code: "RATE-LIMIT-001", + HTTPStatus: http.StatusTooManyRequests, + Message: "Too Many Requests", + Reason: "429 Too Many Requests — %s %s, %d req/%ds, retry after %ds", +} + func (l *Limiter) writeRateLimitError(w http.ResponseWriter, method, path string, limit RouteLimit, retryAfter int) { - w.Header().Set("Content-Type", "application/json") - w.WriteHeader(http.StatusTooManyRequests) - if err := json.NewEncoder(w).Encode(map[string]any{ - "kind": "Error", - "code": "429", - "reason": fmt.Sprintf("429 Too Many Requests — %s %s, %d req/%ds, retry after %ds", method, path, limit.Rate, limit.Window, retryAfter), - }); err != nil { - l.logger.Warn("failed to write rate limit error response", "error_type", fmt.Sprintf("%T", err)) + if err := api.WriteError(w, errRateLimit.WithReason(method, path, limit.Rate, limit.Window, retryAfter)); err != nil { + l.logger.Error("failed to write rate limit error response", "error", err) } } diff --git a/platform-api/pkg/ratelimit/middleware_test.go b/platform-api/pkg/ratelimit/middleware_test.go index 5cb035b2..f33158dd 100644 --- a/platform-api/pkg/ratelimit/middleware_test.go +++ b/platform-api/pkg/ratelimit/middleware_test.go @@ -321,8 +321,8 @@ func TestMiddleware_429ResponseFormat(t *testing.T) { if body["kind"] != "Error" { t.Errorf("expected kind=Error, got %v", body["kind"]) } - if body["code"] != "429" { - t.Errorf("expected code=429, got %v", body["code"]) + if body["code"] != errRateLimit.Code { + t.Errorf("expected code=%s, got %v", errRateLimit.Code, body["code"]) } reason, _ := body["reason"].(string) if reason == "" { diff --git a/platform-api/pkg/server/server.go b/platform-api/pkg/server/server.go index 8ced26c7..ebb28a9d 100644 --- a/platform-api/pkg/server/server.go +++ b/platform-api/pkg/server/server.go @@ -42,8 +42,8 @@ func New(cfg *config.Config, dbClient *hyperfleetdb.Client, logger *slog.Logger) ctx := context.Background() // Create handlers - healthHandler := apphandlers.NewHealthHandler() - infoHandler := apphandlers.NewInfoHandler() + healthHandler := apphandlers.NewHealthHandler(logger) + infoHandler := apphandlers.NewInfoHandler(logger) mgmtClusterHandler := apphandlers.NewManagementClusterHandler(dbClient, logger) clusterHandler := apphandlers.NewClusterHandler(dbClient, cfg.Regional.OIDCIssuerBaseURL, cfg.Regional.DefaultClusterExpiration, logger) nodePoolHandler := apphandlers.NewNodePoolHandler(dbClient, logger) diff --git a/platform-api/pkg/validation/field_validator.go b/platform-api/pkg/validation/field_validator.go index eae425c4..ff5025fb 100644 --- a/platform-api/pkg/validation/field_validator.go +++ b/platform-api/pkg/validation/field_validator.go @@ -32,14 +32,11 @@ func (e ValidationErrors) Error() string { if len(e) == 0 { return "no validation errors" } - var sb strings.Builder - sb.WriteString("validation failed:\n") - for _, err := range e { - sb.WriteString(" ") - sb.WriteString(err.Error()) - sb.WriteString("\n") + msgs := make([]string, len(e)) + for i, err := range e { + msgs[i] = err.Error() } - return sb.String() + return strings.Join(msgs, "; ") } type FieldValidator struct { diff --git a/test/e2e-api/ratelimit_e2e_test.go b/test/e2e-api/ratelimit_e2e_test.go index 0cf1386f..a467cc7f 100644 --- a/test/e2e-api/ratelimit_e2e_test.go +++ b/test/e2e-api/ratelimit_e2e_test.go @@ -156,11 +156,13 @@ var _ = Describe("Rate Limiting", Ordered, Label("ratelimit"), func() { Expect(err).NotTo(HaveOccurred()) Expect(retryAfter).To(BeNumerically(">=", 1)) + Expect(rateLimitedResp.StatusCode).To(Equal(http.StatusTooManyRequests)) + var body map[string]interface{} err = json.Unmarshal(rateLimitedResp.Body, &body) Expect(err).NotTo(HaveOccurred()) Expect(body["kind"]).To(Equal("Error")) - Expect(body["code"]).To(Equal("429")) + Expect(body["code"]).To(Equal("RATE-LIMIT-001")) Expect(body["reason"]).To(ContainSubstring("Too Many Requests")) }) diff --git a/test/e2e-cli/cluster_test.go b/test/e2e-cli/cluster_test.go index f35c93ea..b46087cc 100644 --- a/test/e2e-cli/cluster_test.go +++ b/test/e2e-cli/cluster_test.go @@ -360,8 +360,8 @@ var _ = Describe("ROSACTL CLI E2E Tests", Ordered, func() { case http.StatusConflict: var errBody map[string]interface{} Expect(json.Unmarshal(response.Body, &errBody)).To(Succeed()) - Expect(errBody["code"]).To(Equal("account-exists"), "unexpected 409 body: %s", string(response.Body)) - GinkgoWriter.Printf("Customer account %s already enabled (409 account-exists)\n", customerAccountID) + Expect(errBody["code"]).To(Equal("ACCOUNTS-MGMT-CREATE-004"), "unexpected 409 body: %s", string(response.Body)) + GinkgoWriter.Printf("Customer account %s already enabled (409 ACCOUNTS-MGMT-CREATE-004)\n", customerAccountID) default: Fail(fmt.Sprintf("failed to enable customer account: status %d body: %s", response.StatusCode, string(response.Body))) } diff --git a/test/e2e-sdk/sdk_sanity_test.go b/test/e2e-sdk/sdk_sanity_test.go index 83b720d6..0759f357 100644 --- a/test/e2e-sdk/sdk_sanity_test.go +++ b/test/e2e-sdk/sdk_sanity_test.go @@ -284,7 +284,7 @@ var _ = Describe("SDK E2E: cluster and nodepool lifecycle", Ordered, func() { case http.StatusConflict: var body map[string]interface{} Expect(json.Unmarshal(resp.Body, &body)).To(Succeed()) - Expect(body["code"]).To(Equal("account-exists"), + Expect(body["code"]).To(Equal("ACCOUNTS-MGMT-CREATE-004"), "unexpected 409 body: %s", string(resp.Body)) GinkgoWriter.Printf("Customer account %s already registered\n", customerAccountID) default: diff --git a/test/e2e-zoa/zoa_test.go b/test/e2e-zoa/zoa_test.go index 30881122..322d65ec 100644 --- a/test/e2e-zoa/zoa_test.go +++ b/test/e2e-zoa/zoa_test.go @@ -127,7 +127,7 @@ var _ = Describe("ZOA Trusted Actions", Ordered, func() { Reason string `json:"reason"` } Expect(json.Unmarshal(resp.Body, &errResp)).To(Succeed()) - Expect(errResp.Code).To(Equal("missing-target-cluster")) + Expect(errResp.Code).To(Equal("ZOA-CREATE-003")) }) It("should reject request without jira ticket", func() { @@ -142,7 +142,7 @@ var _ = Describe("ZOA Trusted Actions", Ordered, func() { Code string `json:"code"` } Expect(json.Unmarshal(resp.Body, &errResp)).To(Succeed()) - Expect(errResp.Code).To(Equal("missing-jira")) + Expect(errResp.Code).To(Equal("ZOA-CREATE-004")) }) It("should reject request with invalid jira format", func() { @@ -158,7 +158,7 @@ var _ = Describe("ZOA Trusted Actions", Ordered, func() { Code string `json:"code"` } Expect(json.Unmarshal(resp.Body, &errResp)).To(Succeed()) - Expect(errResp.Code).To(Equal("invalid-jira")) + Expect(errResp.Code).To(Equal("ZOA-CREATE-005")) }) It("should reject request with unknown parameters", func() { @@ -177,7 +177,7 @@ var _ = Describe("ZOA Trusted Actions", Ordered, func() { Code string `json:"code"` } Expect(json.Unmarshal(resp.Body, &errResp)).To(Succeed()) - Expect(errResp.Code).To(Equal("invalid-params")) + Expect(errResp.Code).To(Equal("ZOA-CREATE-006")) }) It("should dispatch get_nodes and complete successfully (full wait)", func() { @@ -330,7 +330,7 @@ var _ = Describe("ZOA Trusted Actions", Ordered, func() { Code string `json:"code"` } Expect(json.Unmarshal(resp.Body, &errResp)).To(Succeed()) - Expect(errResp.Code).To(Equal("write-cooldown")) + Expect(errResp.Code).To(Equal("ZOA-CREATE-007")) GinkgoWriter.Printf("Second call correctly rejected: %s\n", errResp.Code) By("Dispatching with force=true (should bypass cooldown)")