Skip to content

Commit 88971e0

Browse files
sd2kinjectedfusion
andauthored
fix: handle text/* content-type responses from Grafana API (#694)
Co-authored-by: injectedfusion <6646111+injectedfusion@users.noreply.github.com>
1 parent 383182c commit 88971e0

2 files changed

Lines changed: 46 additions & 5 deletions

File tree

mcpgrafana.go

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,8 @@ import (
1919
"sync"
2020
"time"
2121

22+
"github.com/go-openapi/runtime"
23+
openapiclient "github.com/go-openapi/runtime/client"
2224
"github.com/go-openapi/strfmt"
2325
"github.com/grafana/grafana-openapi-client-go/client"
2426
"github.com/grafana/incident-go"
@@ -710,6 +712,17 @@ func NewGrafanaClient(ctx context.Context, grafanaURL, apiKey string, auth *url.
710712
slog.Debug("Creating Grafana client", "url", parsedURL.Redacted(), "api_key_set", apiKey != "", "basic_auth_set", config.BasicAuth != nil, "org_id", cfg.OrgID, "timeout", timeout, "extra_headers_count", len(config.ExtraHeaders))
711713
grafanaClient := client.NewHTTPClientWithConfig(strfmt.Default, cfg)
712714

715+
// Some Grafana versions (v12+) and reverse proxies return JSON responses
716+
// with text/plain or text/html content-type headers. The default
717+
// TextConsumer cannot deserialize these into typed Go structs. Override
718+
// with JSONConsumer so the client can parse the response body correctly.
719+
// See: https://github.com/grafana/mcp-grafana/issues/635
720+
if rt, ok := grafanaClient.Transport.(*openapiclient.Runtime); ok {
721+
jsonConsumer := runtime.JSONConsumer()
722+
rt.Consumers[runtime.TextMime] = jsonConsumer
723+
rt.Consumers[runtime.HTMLMime] = jsonConsumer
724+
}
725+
713726
// Always enable HTTP tracing for context propagation (no-op when no exporter configured)
714727
// Use reflection to wrap the transport without importing the runtime client package
715728
v := reflect.ValueOf(grafanaClient.Transport)

mcpgrafana_test.go

Lines changed: 33 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,13 +1,11 @@
1-
//go:build unit
2-
// +build unit
3-
41
package mcpgrafana
52

63
import (
74
"context"
85
"net/http"
96
"net/http/httptest"
107
"net/url"
8+
"strings"
119
"testing"
1210

1311
"github.com/go-openapi/runtime/client"
@@ -645,6 +643,37 @@ func TestToolTracingInstrumentation(t *testing.T) {
645643
})
646644
}
647645

646+
func TestTextMimeConsumerOverride(t *testing.T) {
647+
// Verify that NewGrafanaClient overrides text/plain and text/html consumers
648+
// with JSON consumers so that responses with incorrect content-type headers
649+
// (e.g. from Grafana v12 or reverse proxies) are still parsed correctly.
650+
// See: https://github.com/grafana/mcp-grafana/issues/635
651+
ctx := WithGrafanaConfig(context.Background(), GrafanaConfig{})
652+
c := NewGrafanaClient(ctx, "http://localhost:3000", "test-api-key", nil)
653+
require.NotNil(t, c)
654+
655+
rt, ok := c.Transport.(*client.Runtime)
656+
require.True(t, ok, "expected Transport to be *client.Runtime")
657+
658+
// The text/plain and text/html consumers should no longer be the default
659+
// TextConsumer. Verify by checking they can consume a JSON object into a
660+
// map (TextConsumer would fail on this).
661+
for _, mime := range []string{"text/plain", "text/html"} {
662+
consumer, exists := rt.Consumers[mime]
663+
require.True(t, exists, "consumer for %s should exist", mime)
664+
require.NotNil(t, consumer, "consumer for %s should not be nil", mime)
665+
666+
// JSONConsumer can unmarshal into a map; TextConsumer cannot.
667+
var result map[string]interface{}
668+
err := consumer.Consume(
669+
strings.NewReader(`{"status":"ok"}`),
670+
&result,
671+
)
672+
assert.NoError(t, err, "consumer for %s should parse JSON", mime)
673+
assert.Equal(t, "ok", result["status"], "consumer for %s should return parsed value", mime)
674+
}
675+
}
676+
648677
func TestHTTPTracingConfiguration(t *testing.T) {
649678
t.Run("HTTP tracing always enabled for context propagation", func(t *testing.T) {
650679
// Create context (HTTP tracing always enabled)
@@ -732,7 +761,6 @@ func assertHasAttribute(t *testing.T, attributes []attribute.KeyValue, key strin
732761
t.Errorf("Expected attribute %s with value %s not found", key, expectedValue)
733762
}
734763

735-
736764
func TestExtraHeadersFromEnv(t *testing.T) {
737765
t.Run("empty env returns nil", func(t *testing.T) {
738766
t.Setenv("GRAFANA_EXTRA_HEADERS", "")
@@ -1042,7 +1070,7 @@ func TestFetchPublicURL(t *testing.T) {
10421070
})
10431071

10441072
extraHeaders := map[string]string{
1045-
"X-Custom-Auth": "proxy-token-123",
1073+
"X-Custom-Auth": "proxy-token-123",
10461074
"X-Forwarded-For": "10.0.0.1",
10471075
}
10481076
fetchPublicURL(context.Background(), ts.URL, "", nil, nil, extraHeaders)

0 commit comments

Comments
 (0)