Skip to content

Commit 13aa381

Browse files
committed
Issue #2973 Return structured JSON errors from config API
1 parent 7e2be6f commit 13aa381

File tree

4 files changed

+73
-2
lines changed

4 files changed

+73
-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: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,11 +2,14 @@ package api
22

33
import (
44
"encoding/json"
5+
stderrors "errors"
56
"io"
67
"net/http"
78
"net/url"
89
"sync"
910

11+
"github.com/crc-org/crc/v2/pkg/crc/api/client"
12+
"github.com/crc-org/crc/v2/pkg/crc/errors"
1013
"github.com/crc-org/crc/v2/pkg/crc/logging"
1114
)
1215

@@ -111,6 +114,11 @@ func (s *server) Handler() http.Handler {
111114
url: r.URL,
112115
}
113116
if err := handler(c); err != nil {
117+
var multiErr errors.MultiError
118+
if stderrors.As(err, &multiErr) {
119+
writeMultiError(w, multiErr)
120+
return
121+
}
114122
http.Error(w, err.Error(), http.StatusInternalServerError)
115123
return
116124
}
@@ -125,3 +133,20 @@ func (s *server) Handler() http.Handler {
125133
}
126134
})
127135
}
136+
137+
func writeMultiError(w http.ResponseWriter, multiErr errors.MultiError) {
138+
validationErrors := make([]client.ValidationError, 0, len(multiErr.Errors))
139+
for _, e := range multiErr.Errors {
140+
if e == nil {
141+
continue
142+
}
143+
validationErrors = append(validationErrors, client.ValidationError{
144+
Message: e.Error(),
145+
})
146+
}
147+
w.Header().Set("Content-Type", "application/json")
148+
w.WriteHeader(http.StatusUnprocessableEntity)
149+
if err := json.NewEncoder(w).Encode(validationErrors); err != nil {
150+
logging.Error("Failed to write error response: ", err)
151+
}
152+
}

0 commit comments

Comments
 (0)