Skip to content

Commit b811b3f

Browse files
sebrandon1claude
andcommitted
Issue #2973 Return structured JSON errors from config API
When setting config via the API with invalid values, errors were returned as concatenated plain text, making them unparseable by API clients. Detect MultiError at the HTTP response layer and return a JSON array of validation errors instead. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
1 parent ae41f68 commit b811b3f

File tree

4 files changed

+62
-2
lines changed

4 files changed

+62
-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(500).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.StatusInternalServerError, 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: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -258,11 +258,13 @@ 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, _ := io.ReadAll(res.Body)
262+
return nil, &HTTPError{URL: url, Method: method, StatusCode: res.StatusCode, Body: string(body)}
262263
}
263264
case http.MethodDelete, http.MethodGet:
264265
if res.StatusCode != http.StatusOK {
265-
return nil, fmt.Errorf("Error occurred sending %s request to : %s : %d", method, url, res.StatusCode)
266+
body, _ := io.ReadAll(res.Body)
267+
return nil, &HTTPError{URL: url, Method: method, StatusCode: res.StatusCode, Body: string(body)}
266268
}
267269
}
268270

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: 20 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
}
@@ -124,3 +130,17 @@ func (s *server) Handler() http.Handler {
124130
}
125131
})
126132
}
133+
134+
func writeMultiError(w http.ResponseWriter, multiErr errors.MultiError) {
135+
validationErrors := make([]client.ValidationError, 0, len(multiErr.Errors))
136+
for _, e := range multiErr.Errors {
137+
validationErrors = append(validationErrors, client.ValidationError{
138+
Message: e.Error(),
139+
})
140+
}
141+
w.Header().Set("Content-Type", "application/json")
142+
w.WriteHeader(http.StatusInternalServerError)
143+
if err := json.NewEncoder(w).Encode(validationErrors); err != nil {
144+
logging.Error("Failed to write error response: ", err)
145+
}
146+
}

0 commit comments

Comments
 (0)