diff --git a/internal/vmware/client.go b/internal/vmware/client.go index a51b4feb23..53fe006cf0 100644 --- a/internal/vmware/client.go +++ b/internal/vmware/client.go @@ -413,12 +413,9 @@ func (c *Client) getSessionScopedJSON( switch resp.StatusCode { case http.StatusOK: default: - return classifyReadStatusCode(label, resp.StatusCode) + return classifyReadStatusCodeWithBody(label, resp.StatusCode, body) } - if err := json.Unmarshal(body, target); err != nil { - return &ConnectionError{Category: "endpoint", Message: fmt.Sprintf("VMware %s response was not valid JSON", label)} - } - return nil + return decodeVIJSONBody(body, label, target) } // postSessionScopedJSON posts a JSON body to a session-authenticated vCenter @@ -454,12 +451,9 @@ func (c *Client) postSessionScopedJSON( return fmt.Errorf("read %s response: %w", label, readErr) } if resp.StatusCode != http.StatusOK { - return classifyReadStatusCode(label, resp.StatusCode) + return classifyReadStatusCodeWithBody(label, resp.StatusCode, responseBody) } - if err := json.Unmarshal(responseBody, target); err != nil { - return &ConnectionError{Category: "endpoint", Message: fmt.Sprintf("VMware %s response was not valid JSON", label)} - } - return nil + return decodeVIJSONBody(responseBody, label, target) } func (c *Client) getAutomationJSON( @@ -607,6 +601,16 @@ func (c *Client) loginVIJSON(ctx context.Context, release string, sessionManager } func classifyReadStatusCode(label string, statusCode int) error { + return classifyReadStatusCodeWithBody(label, statusCode, nil) +} + +// classifyReadStatusCodeWithBody classifies a non-2xx read and, when the body is +// present, appends the vSphere JSON API fault detail. The JSON API answers +// client-side argument problems (an unknown argument name or shape) with an +// HTTP 5xx plus a JSON fault body, so the faultstring/invalidProperty is the only +// actionable part of the response; discarding it leaves the operator with an +// opaque "HTTP 500". +func classifyReadStatusCodeWithBody(label string, statusCode int, body []byte) error { switch statusCode { case http.StatusUnauthorized: return &ConnectionError{Category: "auth", Message: fmt.Sprintf("VMware authentication failed while reading %s", label)} @@ -617,10 +621,77 @@ func classifyReadStatusCode(label string, statusCode int) error { case http.StatusServiceUnavailable: return &ConnectionError{Category: "unavailable", Message: fmt.Sprintf("VMware %s is temporarily unavailable", label)} default: - return &ConnectionError{Category: "endpoint", Message: fmt.Sprintf("VMware %s request failed with HTTP %d", label, statusCode)} + message := fmt.Sprintf("VMware %s request failed with HTTP %d", label, statusCode) + if detail := viJSONFaultDetail(body); detail != "" { + message = fmt.Sprintf("%s (%s)", message, detail) + } + return &ConnectionError{Category: "endpoint", Message: message} } } +// viJSONFault models the fault envelope the vSphere JSON API returns in the body +// of a rejected request. +type viJSONFault struct { + TypeName string `json:"_typeName"` + FaultString string `json:"faultstring"` + InvalidProperty string `json:"invalidProperty"` +} + +const viJSONFaultDetailLimit = 200 + +// viJSONFaultDetail extracts a bounded, single-line summary of a vSphere JSON +// API fault body. It returns "" when the body is not a recognisable fault so the +// caller falls back to the plain status line. The text is server-controlled, so +// it is collapsed to one line and length-bounded before it reaches logs or the +// connection status surface. +func viJSONFaultDetail(body []byte) string { + trimmed := bytes.TrimSpace(body) + if len(trimmed) == 0 || trimmed[0] != '{' { + return "" + } + var fault viJSONFault + if err := json.Unmarshal(trimmed, &fault); err != nil { + return "" + } + parts := make([]string, 0, 3) + if value := viJSONFaultFieldText(fault.FaultString); value != "" { + parts = append(parts, value) + } + if value := viJSONFaultFieldText(fault.InvalidProperty); value != "" { + parts = append(parts, "invalidProperty="+value) + } + if value := viJSONFaultFieldText(fault.TypeName); value != "" { + parts = append(parts, "type="+value) + } + if len(parts) == 0 { + return "" + } + return strings.Join(parts, ", ") +} + +func viJSONFaultFieldText(raw string) string { + raw = strings.Join(strings.Fields(raw), " ") + if len(raw) > viJSONFaultDetailLimit { + raw = raw[:viJSONFaultDetailLimit] + } + return raw +} + +// decodeVIJSONBody decodes a vSphere JSON API response body into target. A +// successful response with an empty body represents an absent/null value (for +// example the parent of a top-level folder), not a malformed response, so it +// leaves target at its zero value instead of raising a spurious "not valid JSON" +// enrichment error. +func decodeVIJSONBody(body []byte, label string, target any) error { + if len(bytes.TrimSpace(body)) == 0 { + return nil + } + if err := json.Unmarshal(body, target); err != nil { + return &ConnectionError{Category: "endpoint", Message: fmt.Sprintf("VMware %s response was not valid JSON", label)} + } + return nil +} + func classifyTransportError(stage string, err error) error { if err == nil { return nil diff --git a/internal/vmware/client_json_robustness_test.go b/internal/vmware/client_json_robustness_test.go new file mode 100644 index 0000000000..6fee486e52 --- /dev/null +++ b/internal/vmware/client_json_robustness_test.go @@ -0,0 +1,133 @@ +package vmware + +import ( + "context" + "net/http" + "net/http/httptest" + "strings" + "testing" + "time" +) + +func TestDecodeVIJSONBodyTreatsEmptySuccessAsAbsent(t *testing.T) { + var ref *viJSONReference + if err := decodeVIJSONBody([]byte(" \n\t "), "folder parent", &ref); err != nil { + t.Fatalf("empty success body should be tolerated, got %v", err) + } + if ref != nil { + t.Fatalf("empty success body should leave target nil, got %+v", ref) + } +} + +func TestDecodeVIJSONBodyRejectsMalformedBody(t *testing.T) { + var ref *viJSONReference + err := decodeVIJSONBody([]byte("nope"), "folder parent", &ref) + if err == nil { + t.Fatal("expected malformed body to fail") + } + connectionErr, ok := err.(*ConnectionError) + if !ok { + t.Fatalf("expected ConnectionError, got %T", err) + } + if connectionErr.Category != "endpoint" || !strings.Contains(connectionErr.Message, "not valid JSON") { + t.Fatalf("unexpected error: %+v", connectionErr) + } +} + +func TestVIJSONFaultDetailExtractsBoundedSummary(t *testing.T) { + body := []byte(`{"_typeName":"InvalidArgument","invalidProperty":"perfSpec","faultstring":"Unknown argument."}`) + got := viJSONFaultDetail(body) + for _, want := range []string{"Unknown argument.", "invalidProperty=perfSpec", "type=InvalidArgument"} { + if !strings.Contains(got, want) { + t.Fatalf("fault detail %q missing %q", got, want) + } + } + if strings.ContainsAny(got, "\n\r") { + t.Fatalf("fault detail must be single-line, got %q", got) + } +} + +func TestVIJSONFaultDetailIgnoresNonFaultBody(t *testing.T) { + for _, body := range [][]byte{nil, []byte(""), []byte("internal error"), []byte(`{"foo":"bar"}`), []byte("not json")} { + if got := viJSONFaultDetail(body); got != "" { + t.Fatalf("body %q produced fault detail %q, want empty", body, got) + } + } +} + +func TestClassifyReadStatusCodeWithBodySurfacesFault(t *testing.T) { + body := []byte(`{"faultstring":"Invalid URI format"}`) + err := classifyReadStatusCodeWithBody("vmware performance metrics", http.StatusInternalServerError, body) + connectionErr, ok := err.(*ConnectionError) + if !ok { + t.Fatalf("expected ConnectionError, got %T", err) + } + if connectionErr.Category != "endpoint" { + t.Fatalf("category = %q, want endpoint", connectionErr.Category) + } + if !strings.Contains(connectionErr.Message, "HTTP 500") || !strings.Contains(connectionErr.Message, "Invalid URI format") { + t.Fatalf("message %q should carry status and fault detail", connectionErr.Message) + } +} + +func TestCollectEntityReferenceToleratesEmptyParentBody(t *testing.T) { + mux := http.NewServeMux() + mux.HandleFunc("/sdk/vim25/9.0.0.0/Folder/group-h5/parent", func(w http.ResponseWriter, _ *http.Request) { + w.WriteHeader(http.StatusOK) + }) + server := httptest.NewTLSServer(mux) + defer server.Close() + + client, err := NewClient(ClientConfig{ + Host: server.URL, + Username: "admin", + Password: "secret", + InsecureSkipVerify: true, + Timeout: 5 * time.Second, + }) + if err != nil { + t.Fatalf("NewClient: %v", err) + } + + ref, err := client.collectEntityReference(context.Background(), "9.0.0.0", "vi-session", "Folder", "group-h5", "parent", "folder parent") + if err != nil { + t.Fatalf("empty parent body should not degrade the connection: %v", err) + } + if ref != nil { + t.Fatalf("expected nil parent reference, got %+v", ref) + } +} + +func TestCollectEntityReferenceSurfacesFaultDetail(t *testing.T) { + mux := http.NewServeMux() + mux.HandleFunc("/sdk/vim25/9.0.0.0/Folder/group-h5/parent", func(w http.ResponseWriter, _ *http.Request) { + w.Header().Set("Content-Type", "application/json") + w.WriteHeader(http.StatusInternalServerError) + _, _ = w.Write([]byte(`{"_typeName":"InvalidArgument","invalidProperty":"entity","faultstring":"Invalid MoRef field: entity"}`)) + }) + server := httptest.NewTLSServer(mux) + defer server.Close() + + client, err := NewClient(ClientConfig{ + Host: server.URL, + Username: "admin", + Password: "secret", + InsecureSkipVerify: true, + Timeout: 5 * time.Second, + }) + if err != nil { + t.Fatalf("NewClient: %v", err) + } + + _, err = client.collectEntityReference(context.Background(), "9.0.0.0", "vi-session", "Folder", "group-h5", "parent", "folder parent") + if err == nil { + t.Fatal("expected fault response to fail") + } + connectionErr, ok := err.(*ConnectionError) + if !ok { + t.Fatalf("expected ConnectionError, got %T", err) + } + if !strings.Contains(connectionErr.Message, "Invalid MoRef field: entity") { + t.Fatalf("message %q should carry the fault detail", connectionErr.Message) + } +} diff --git a/internal/vmware/client_metrics.go b/internal/vmware/client_metrics.go index 1d72189427..9ccb4f8230 100644 --- a/internal/vmware/client_metrics.go +++ b/internal/vmware/client_metrics.go @@ -320,12 +320,9 @@ func (c *Client) postVIJSONJSON(ctx context.Context, sessionID, path, label stri switch resp.StatusCode { case http.StatusOK: default: - return classifyReadStatusCode(label, resp.StatusCode) + return classifyReadStatusCodeWithBody(label, resp.StatusCode, responseBody) } - if err := json.Unmarshal(responseBody, target); err != nil { - return &ConnectionError{Category: "endpoint", Message: fmt.Sprintf("VMware %s response was not valid JSON", label)} - } - return nil + return decodeVIJSONBody(responseBody, label, target) } func perfMetricIDsForEntity(