Implement fully-featured content negotiation for API requests, and allow overriding the default API codec.

Signed-off-by: Charles Korn <charles.korn@grafana.com>

go.mod

@@ -160,7 +160,7 @@ require (
 	github.com/modern-go/concurrent v0.0.0-20180306012644-bacd9c7ef1dd // indirect
 	github.com/modern-go/reflect2 v1.0.2 // indirect
 	github.com/morikuni/aec v1.0.0 // indirect
-	github.com/munnerz/goautoneg v0.0.0-20191010083416-a7dc8b61c822 // indirect
+	github.com/munnerz/goautoneg v0.0.0-20191010083416-a7dc8b61c822
 	github.com/opencontainers/go-digest v1.0.0 // indirect
 	github.com/opencontainers/image-spec v1.0.2 // indirect
 	github.com/pmezard/go-difflib v1.0.0 // indirect

web/api/v1/api.go

@@ -32,6 +32,7 @@ import (
 	"github.com/go-kit/log/level"
 	"github.com/grafana/regexp"
 	jsoniter "github.com/json-iterator/go"
+	"github.com/munnerz/goautoneg"
 	"github.com/pkg/errors"
 	"github.com/prometheus/client_golang/prometheus"
 	"github.com/prometheus/common/model"
@@ -68,20 +69,19 @@ const (
 type errorType string
 
 const (
-	errorNone        errorType = ""
-	errorTimeout     errorType = "timeout"
-	errorCanceled    errorType = "canceled"
-	errorExec        errorType = "execution"
-	errorBadData     errorType = "bad_data"
-	errorInternal    errorType = "internal"
-	errorUnavailable errorType = "unavailable"
-	errorNotFound    errorType = "not_found"
+	errorNone          errorType = ""
+	errorTimeout       errorType = "timeout"
+	errorCanceled      errorType = "canceled"
+	errorExec          errorType = "execution"
+	errorBadData       errorType = "bad_data"
+	errorInternal      errorType = "internal"
+	errorUnavailable   errorType = "unavailable"
+	errorNotFound      errorType = "not_found"
+	errorNotAcceptable errorType = "not_acceptable"
 )
 
 var LocalhostRepresentations = []string{"127.0.0.1", "localhost", "::1"}
 
-var defaultCodec = JSONCodec{}
-
 type apiError struct {
 	typ errorType
 	err error
@@ -212,7 +212,7 @@ type API struct {
 	remoteWriteHandler http.Handler
 	remoteReadHandler  http.Handler
 
-	codecs map[string]Codec
+	codecs []Codec
 }
 
 // NewAPI returns an initialized API type.
@@ -271,11 +271,9 @@ func NewAPI(
 		statsRenderer:    defaultStatsRenderer,
 
 		remoteReadHandler: remote.NewReadHandler(logger, registerer, q, configFunc, remoteReadSampleLimit, remoteReadConcurrencyLimit, remoteReadMaxBytesInFrame),
-
-		codecs: map[string]Codec{},
 	}
 
-	a.InstallCodec(defaultCodec)
+	a.InstallCodec(JSONCodec{})
 
 	if statsRenderer != nil {
 		a.statsRenderer = statsRenderer
@@ -289,13 +287,15 @@ func NewAPI(
 }
 
 // InstallCodec adds codec to this API's available codecs.
-// If codec handles a content type handled by a codec already installed in this API, codec replaces the previous codec.
+// Codecs installed first take precedence over codecs installed later when evaluating wildcards in Accept headers.
+// The first installed codec is used as a fallback when the Accept header cannot be satisfied or if there is no Accept header.
 func (api *API) InstallCodec(codec Codec) {
-	if api.codecs == nil {
-		api.codecs = map[string]Codec{}
-	}
+	api.codecs = append(api.codecs, codec)
+}
 
-	api.codecs[codec.ContentType()] = codec
+// ClearCodecs removes all available codecs from this API, including the default codec installed by NewAPI.
+func (api *API) ClearCodecs() {
+	api.codecs = nil
 }
 
 func setUnavailStatusOnTSDBNotReady(r apiFuncResult) apiFuncResult {
@@ -1583,7 +1583,12 @@ func (api *API) respond(w http.ResponseWriter, req *http.Request, data interface
 		Warnings: warningStrings,
 	}
 
-	codec := api.negotiateCodec(req, resp)
+	codec, err := api.negotiateCodec(req, resp)
+	if err != nil {
+		api.respondError(w, &apiError{errorNotAcceptable, err}, nil)
+		return
+	}
+
 	b, err := codec.Encode(resp)
 	if err != nil {
 		level.Error(api.logger).Log("msg", "error marshaling response", "err", err)
@@ -1591,33 +1596,28 @@ func (api *API) respond(w http.ResponseWriter, req *http.Request, data interface
 		return
 	}
 
-	w.Header().Set("Content-Type", codec.ContentType())
+	w.Header().Set("Content-Type", codec.ContentType().String())
 	w.WriteHeader(http.StatusOK)
 	if n, err := w.Write(b); err != nil {
 		level.Error(api.logger).Log("msg", "error writing response", "bytesWritten", n, "err", err)
 	}
 }
 
-// HTTP content negotiation is hard (see https://developer.mozilla.org/en-US/docs/Web/HTTP/Content_negotiation).
-// Ideally, we shouldn't be implementing this ourselves - https://github.com/golang/go/issues/19307 is an open proposal to add
-// this to the Go stdlib and has links to a number of other implementations.
-//
-// This is an MVP, and doesn't support features like wildcards or weighting.
-func (api *API) negotiateCodec(req *http.Request, resp *Response) Codec {
-	acceptHeader := req.Header.Get("Accept")
-	if acceptHeader == "" {
-		return defaultCodec
+func (api *API) negotiateCodec(req *http.Request, resp *Response) (Codec, error) {
+	for _, clause := range goautoneg.ParseAccept(req.Header.Get("Accept")) {
+		for _, codec := range api.codecs {
+			if codec.ContentType().Satisfies(clause) && codec.CanEncode(resp) {
+				return codec, nil
+			}
+		}
 	}
 
-	for _, contentType := range strings.Split(acceptHeader, ",") {
-		codec, ok := api.codecs[strings.TrimSpace(contentType)]
-		if ok && codec.CanEncode(resp) {
-			return codec
-		}
+	defaultCodec := api.codecs[0]
+	if !defaultCodec.CanEncode(resp) {
+		return nil, fmt.Errorf("cannot encode response as %s", defaultCodec.ContentType())
 	}
 
-	level.Debug(api.logger).Log("msg", "could not find suitable codec for response, falling back to default codec", "accept_header", acceptHeader)
-	return defaultCodec
+	return defaultCodec, nil
 }
 
 func (api *API) respondError(w http.ResponseWriter, apiErr *apiError, data interface{}) {
@@ -1648,6 +1648,8 @@ func (api *API) respondError(w http.ResponseWriter, apiErr *apiError, data inter
 		code = http.StatusInternalServerError
 	case errorNotFound:
 		code = http.StatusNotFound
+	case errorNotAcceptable:
+		code = http.StatusNotAcceptable
 	default:
 		code = http.StatusInternalServerError
 	}

web/api/v1/api_test.go

@@ -2769,9 +2769,11 @@ func TestRespondSuccess(t *testing.T) {
 		logger: log.NewNopLogger(),
 	}
 
-	api.InstallCodec(&testCodec{contentType: "test/cannot-encode", canEncode: false})
-	api.InstallCodec(&testCodec{contentType: "test/can-encode", canEncode: true})
-	api.InstallCodec(&testCodec{contentType: "test/can-encode-2", canEncode: true})
+	api.ClearCodecs()
+	api.InstallCodec(JSONCodec{})
+	api.InstallCodec(&testCodec{contentType: MIMEType{"test", "cannot-encode"}, canEncode: false})
+	api.InstallCodec(&testCodec{contentType: MIMEType{"test", "can-encode"}, canEncode: true})
+	api.InstallCodec(&testCodec{contentType: MIMEType{"test", "can-encode-2"}, canEncode: true})
 
 	s := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
 		api.respond(w, r, "test", nil)
@@ -2854,6 +2856,34 @@ func TestRespondSuccess(t *testing.T) {
 	}
 }
 
+func TestRespondSuccess_DefaultCodecCannotEncodeResponse(t *testing.T) {
+	api := API{
+		logger: log.NewNopLogger(),
+	}
+
+	api.ClearCodecs()
+	api.InstallCodec(&testCodec{contentType: MIMEType{"application", "default-format"}, canEncode: false})
+
+	s := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		api.respond(w, r, "test", nil)
+	}))
+	defer s.Close()
+
+	req, err := http.NewRequest(http.MethodGet, s.URL, nil)
+	require.NoError(t, err)
+
+	resp, err := http.DefaultClient.Do(req)
+	require.NoError(t, err)
+
+	body, err := io.ReadAll(resp.Body)
+	defer resp.Body.Close()
+	require.NoError(t, err)
+
+	require.Equal(t, http.StatusNotAcceptable, resp.StatusCode)
+	require.Equal(t, "application/json", resp.Header.Get("Content-Type"))
+	require.Equal(t, `{"status":"error","errorType":"not_acceptable","error":"cannot encode response as application/default-format"}`, string(body))
+}
+
 func TestRespondError(t *testing.T) {
 	s := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
 		api := API{}
@@ -3193,6 +3223,7 @@ func BenchmarkRespond(b *testing.B) {
 	}
 	b.ResetTimer()
 	api := API{}
+	api.InstallCodec(JSONCodec{})
 	for n := 0; n < b.N; n++ {
 		api.respond(&testResponseWriter, request, response, nil)
 	}
@@ -3307,11 +3338,11 @@ func TestGetGlobalURL(t *testing.T) {
 }
 
 type testCodec struct {
-	contentType string
+	contentType MIMEType
 	canEncode   bool
 }
 
-func (t *testCodec) ContentType() string {
+func (t *testCodec) ContentType() MIMEType {
 	return t.contentType
 }
 

web/api/v1/codec.go

@@ -13,10 +13,12 @@
 
 package v1
 
+import "github.com/munnerz/goautoneg"
+
 // A Codec performs encoding of API responses.
 type Codec interface {
 	// ContentType returns the MIME time that this Codec emits.
-	ContentType() string
+	ContentType() MIMEType
 
 	// CanEncode determines if this Codec can encode resp.
 	CanEncode(resp *Response) bool
@@ -24,3 +26,28 @@ type Codec interface {
 	// Encode encodes resp, ready for transmission to an API consumer.
 	Encode(resp *Response) ([]byte, error)
 }
+
+type MIMEType struct {
+	Type    string
+	SubType string
+}
+
+func (m MIMEType) String() string {
+	return m.Type + "/" + m.SubType
+}
+
+func (m MIMEType) Satisfies(accept goautoneg.Accept) bool {
+	if accept.Type == "*" && accept.SubType == "*" {
+		return true
+	}
+
+	if accept.Type == m.Type && accept.SubType == "*" {
+		return true
+	}
+
+	if accept.Type == m.Type && accept.SubType == m.SubType {
+		return true
+	}
+
+	return false
+}

web/api/v1/codec_test.go

@@ -0,0 +1,68 @@
+// Copyright 2016 The Prometheus Authors
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//     http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package v1
+
+import (
+	"testing"
+
+	"github.com/munnerz/goautoneg"
+	"github.com/stretchr/testify/require"
+)
+
+func TestMIMEType_String(t *testing.T) {
+	m := MIMEType{Type: "application", SubType: "json"}
+
+	require.Equal(t, "application/json", m.String())
+}
+
+func TestMIMEType_Satisfies(t *testing.T) {
+	m := MIMEType{Type: "application", SubType: "json"}
+
+	scenarios := map[string]struct {
+		accept   goautoneg.Accept
+		expected bool
+	}{
+		"exact match": {
+			accept:   goautoneg.Accept{Type: "application", SubType: "json"},
+			expected: true,
+		},
+		"sub-type wildcard match": {
+			accept:   goautoneg.Accept{Type: "application", SubType: "*"},
+			expected: true,
+		},
+		"full wildcard match": {
+			accept:   goautoneg.Accept{Type: "*", SubType: "*"},
+			expected: true,
+		},
+		"inverted": {
+			accept:   goautoneg.Accept{Type: "json", SubType: "application"},
+			expected: false,
+		},
+		"inverted sub-type wildcard": {
+			accept:   goautoneg.Accept{Type: "json", SubType: "*"},
+			expected: false,
+		},
+		"complete mismatch": {
+			accept:   goautoneg.Accept{Type: "text", SubType: "plain"},
+			expected: false,
+		},
+	}
+
+	for name, scenario := range scenarios {
+		t.Run(name, func(t *testing.T) {
+			actual := m.Satisfies(scenario.accept)
+			require.Equal(t, scenario.expected, actual)
+		})
+	}
+}

web/api/v1/json_codec.go

@@ -34,8 +34,8 @@ func init() {
 // JSONCodec is a Codec that encodes API responses as JSON.
 type JSONCodec struct{}
 
-func (j JSONCodec) ContentType() string {
-	return "application/json"
+func (j JSONCodec) ContentType() MIMEType {
+	return MIMEType{Type: "application", SubType: "json"}
 }
 
 func (j JSONCodec) CanEncode(_ *Response) bool {