Skip to content

Commit 90f0947

Browse files
committed
Issue #2973 Return structured JSON errors from config API
Detect MultiError at the HTTP response layer and return a JSON array of structured ValidationError objects with HTTP 422. Add nil guard in writeMultiError and handle io.ReadAll errors on error response paths.
1 parent f0c2a3d commit 90f0947

File tree

4 files changed

+71
-2
lines changed

4 files changed

+71
-2
lines changed

pkg/crc/api/api_http_test.go

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
package api
22

33
import (
4+
"encoding/json"
45
"fmt"
56
"io"
67
"net/http"
@@ -10,6 +11,7 @@ import (
1011
"strings"
1112
"testing"
1213

14+
"github.com/crc-org/crc/v2/pkg/crc/api/client"
1315
crcConfig "github.com/crc-org/crc/v2/pkg/crc/config"
1416
"github.com/crc-org/crc/v2/pkg/crc/constants"
1517
"github.com/crc-org/crc/v2/pkg/crc/machine/fakemachine"
@@ -301,6 +303,10 @@ var testCases = []testCase{
301303
request: get("config?cpus").withBody("xx"),
302304
response: jSon(`{"Configs":{"cpus":4}}`),
303305
},
306+
{
307+
request: post("config").withBody(`{"properties":{"cpus":0}}`),
308+
response: httpError(422).withBody("[{\"message\":\"Value '0' for configuration property 'cpus' is invalid, reason: requires CPUs \\u003e= 4\"}]\n"),
309+
},
304310

305311
// logs
306312
{
@@ -504,3 +510,30 @@ func TestRoutes(t *testing.T) {
504510
}
505511
}
506512
}
513+
514+
func TestSetConfigMultipleErrors(t *testing.T) {
515+
pullSecretPath := createDummyPullSecret(t)
516+
defer os.Remove(pullSecretPath)
517+
server := newMockServer(pullSecretPath)
518+
519+
// Send a request with multiple invalid config values
520+
req := post("config").withBody(`{"properties":{"cpus":0,"memory":1}}`)
521+
resp := sendRequest(server.Handler(), &req)
522+
523+
require.Equal(t, http.StatusUnprocessableEntity, resp.StatusCode)
524+
525+
body, err := io.ReadAll(resp.Body)
526+
require.NoError(t, err)
527+
528+
var validationErrors []client.ValidationError
529+
require.NoError(t, json.Unmarshal(body, &validationErrors))
530+
require.Len(t, validationErrors, 2)
531+
532+
// Map iteration order is non-deterministic, so check membership rather than order
533+
messages := make([]string, len(validationErrors))
534+
for i, ve := range validationErrors {
535+
messages[i] = ve.Message
536+
}
537+
assert.Contains(t, messages, "Value '0' for configuration property 'cpus' is invalid, reason: requires CPUs >= 4")
538+
assert.Contains(t, messages, "Value '1' for configuration property 'memory' is invalid, reason: requires memory in MiB >= 10752")
539+
}

pkg/crc/api/client/client.go

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -258,11 +258,19 @@ func (c *client) sendRequest(url string, method string, data io.Reader) ([]byte,
258258
switch method {
259259
case http.MethodPost:
260260
if res.StatusCode != http.StatusOK && res.StatusCode != http.StatusCreated {
261-
return nil, fmt.Errorf("Error occurred sending POST request to : %s : %d", url, res.StatusCode)
261+
body, readErr := io.ReadAll(res.Body)
262+
if readErr != nil {
263+
return nil, fmt.Errorf("failed to read error response body: %w", readErr)
264+
}
265+
return nil, &HTTPError{URL: url, Method: method, StatusCode: res.StatusCode, Body: string(body)}
262266
}
263267
case http.MethodDelete, http.MethodGet:
264268
if res.StatusCode != http.StatusOK {
265-
return nil, fmt.Errorf("Error occurred sending %s request to : %s : %d", method, url, res.StatusCode)
269+
body, readErr := io.ReadAll(res.Body)
270+
if readErr != nil {
271+
return nil, fmt.Errorf("failed to read error response body: %w", readErr)
272+
}
273+
return nil, &HTTPError{URL: url, Method: method, StatusCode: res.StatusCode, Body: string(body)}
266274
}
267275
}
268276

pkg/crc/api/client/types.go

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -66,3 +66,8 @@ type TelemetryRequest struct {
6666
Source string `json:"source"`
6767
Status string `json:"status"`
6868
}
69+
70+
// ValidationError represents a single validation error in a structured JSON error response.
71+
type ValidationError struct {
72+
Message string `json:"message"`
73+
}

pkg/crc/api/helpers.go

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,8 @@ import (
77
"net/url"
88
"sync"
99

10+
"github.com/crc-org/crc/v2/pkg/crc/api/client"
11+
"github.com/crc-org/crc/v2/pkg/crc/errors"
1012
"github.com/crc-org/crc/v2/pkg/crc/logging"
1113
)
1214

@@ -111,6 +113,10 @@ func (s *server) Handler() http.Handler {
111113
url: r.URL,
112114
}
113115
if err := handler(c); err != nil {
116+
if multiErr, ok := err.(errors.MultiError); ok {
117+
writeMultiError(w, multiErr)
118+
return
119+
}
114120
http.Error(w, err.Error(), http.StatusInternalServerError)
115121
return
116122
}
@@ -125,3 +131,20 @@ func (s *server) Handler() http.Handler {
125131
}
126132
})
127133
}
134+
135+
func writeMultiError(w http.ResponseWriter, multiErr errors.MultiError) {
136+
validationErrors := make([]client.ValidationError, 0, len(multiErr.Errors))
137+
for _, e := range multiErr.Errors {
138+
if e == nil {
139+
continue
140+
}
141+
validationErrors = append(validationErrors, client.ValidationError{
142+
Message: e.Error(),
143+
})
144+
}
145+
w.Header().Set("Content-Type", "application/json")
146+
w.WriteHeader(http.StatusUnprocessableEntity)
147+
if err := json.NewEncoder(w).Encode(validationErrors); err != nil {
148+
logging.Error("Failed to write error response: ", err)
149+
}
150+
}

0 commit comments

Comments
 (0)