Skip to content

Commit 3b2c2b8

Browse files
committed
fix: properly handle gzip-encoded responses
1 parent 067637c commit 3b2c2b8

7 files changed

Lines changed: 144 additions & 7 deletions

File tree

e2e/e2e_test.go

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,6 @@ import (
1919
"os"
2020
"path"
2121
"path/filepath"
22-
2322
"testing"
2423
"time"
2524

e2e/embedded_integration_test.go

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,6 @@ import (
1414
"github.com/stretchr/testify/require"
1515
corev1 "k8s.io/api/core/v1"
1616
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
17-
1817
utilfeature "k8s.io/apiserver/pkg/util/feature"
1918
"k8s.io/client-go/kubernetes"
2019
"k8s.io/client-go/rest"

e2e/proxy_test.go

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,6 @@ import (
88
"fmt"
99
"time"
1010

11-
v1 "github.com/authzed/authzed-go/proto/authzed/api/v1"
1211
. "github.com/onsi/ginkgo/v2"
1312
. "github.com/onsi/gomega"
1413
"github.com/samber/lo"
@@ -28,6 +27,8 @@ import (
2827
"k8s.io/client-go/tools/clientcmd"
2928
"k8s.io/utils/pointer"
3029

30+
v1 "github.com/authzed/authzed-go/proto/authzed/api/v1"
31+
3132
"github.com/authzed/spicedb-kubeapi-proxy/pkg/authz/distributedtx"
3233
"github.com/authzed/spicedb-kubeapi-proxy/pkg/config/proxyrule"
3334
"github.com/authzed/spicedb-kubeapi-proxy/pkg/failpoints"

e2e/util_test.go

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -9,8 +9,6 @@ import (
99
"os"
1010
goruntime "runtime"
1111

12-
v1 "github.com/authzed/authzed-go/proto/authzed/api/v1"
13-
"github.com/authzed/spicedb/pkg/tuple"
1412
"github.com/go-logr/logr"
1513
. "github.com/onsi/gomega"
1614
"github.com/samber/lo"
@@ -20,6 +18,9 @@ import (
2018
"sigs.k8s.io/controller-runtime/tools/setup-envtest/store"
2119
"sigs.k8s.io/controller-runtime/tools/setup-envtest/versions"
2220
"sigs.k8s.io/controller-runtime/tools/setup-envtest/workflows"
21+
22+
v1 "github.com/authzed/authzed-go/proto/authzed/api/v1"
23+
"github.com/authzed/spicedb/pkg/tuple"
2324
)
2425

2526
// GetAllTuples collects all tuples matching the filter from SpiceDB

magefiles/util.go

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,6 @@ import (
2424
"github.com/onsi/gomega/gexec"
2525
"golang.org/x/exp/slices"
2626
"sigs.k8s.io/kind/pkg/apis/config/v1alpha4"
27-
2827
kind "sigs.k8s.io/kind/pkg/cluster"
2928
"sigs.k8s.io/kind/pkg/cluster/nodeutils"
3029
"sigs.k8s.io/kind/pkg/cmd"

pkg/authz/responsefilterer.go

Lines changed: 20 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ package authz
22

33
import (
44
"bytes"
5+
"compress/gzip"
56
"context"
67
"encoding/json"
78
"fmt"
@@ -233,7 +234,25 @@ func (rf *StandardResponseFilterer) FilterResp(resp *http.Response) error {
233234
return nil
234235
}
235236

236-
body, err := io.ReadAll(resp.Body)
237+
bodyStream := resp.Body
238+
239+
// NOTE: we need to manually check for gzipped encoding here
240+
// because we're proxying - the request/response context would
241+
// know about the encoding if we were the ones originating the
242+
// request, but since we aren't, we need to manually check this.
243+
// This is needed because the k8s API will automatically gzip responses
244+
// above 128kb by default.
245+
if resp.Header.Get("Content-Encoding") == "gzip" {
246+
bodyStream, err = gzip.NewReader(bodyStream)
247+
if err != nil {
248+
return err
249+
}
250+
defer func() {
251+
_ = bodyStream.Close()
252+
}()
253+
}
254+
255+
body, err := io.ReadAll(bodyStream)
237256
if err != nil {
238257
return err
239258
}

pkg/proxy/embedded_test.go

Lines changed: 119 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
package proxy
22

33
import (
4+
"compress/gzip"
45
"context"
56
"io"
67
"net/http"
@@ -451,6 +452,124 @@ func createKubernetesClient(t *testing.T, embeddedClient *http.Client, username
451452
return clientset
452453
}
453454

455+
// TestGzippedUpstreamResponse tests that the proxy correctly handles gzip-encoded
456+
// responses from the upstream k8s API server. The k8s API server automatically
457+
// gzip-compresses responses that exceed ~128KB. When a k8s client (kubectl,
458+
// client-go) includes Accept-Encoding: gzip on its request, the proxy forwards
459+
// that header upstream. Go's HTTP transport only auto-decompresses a response
460+
// when the transport itself injected Accept-Encoding — because the header was
461+
// already present, the proxy's transport leaves the body compressed. FilterResp
462+
// must therefore decompress Content-Encoding: gzip bodies before attempting to
463+
// decode them as JSON or protobuf.
464+
func TestGzippedUpstreamResponse(t *testing.T) {
465+
defer require.NoError(t, logsv1.ResetForTest(utilfeature.DefaultFeatureGate))
466+
467+
ctx, cancel := context.WithCancel(t.Context())
468+
t.Cleanup(cancel)
469+
470+
responseText := `{"kind":"NamespaceList","apiVersion":"v1","metadata":{"resourceVersion":"1000"},"items":[]}`
471+
472+
// Isolate the REST-mapper discovery cache so test artefacts don't leak.
473+
t.Setenv("KUBECACHEDIR", t.TempDir())
474+
475+
opts := NewOptions(WithEmbeddedProxy, WithEmbeddedSpiceDBEndpoint)
476+
opts.Authentication.Embedded.Enabled = true
477+
478+
opts.RestConfigFunc = func() (*rest.Config, http.RoundTripper, error) {
479+
// A TLS test server is required because the reverse proxy's Director
480+
// always rewrites the upstream scheme to "https://".
481+
mockServer := httptest.NewTLSServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
482+
switch r.URL.Path {
483+
case "/api":
484+
// Core API group discovery.
485+
w.Header().Set("Content-Type", "application/json")
486+
_, _ = w.Write([]byte(`{"kind":"APIVersions","apiVersion":"v1","versions":["v1"],"serverAddressByClientCIDRs":[{"clientCIDR":"0.0.0.0/0","serverAddress":"localhost"}]}`))
487+
case "/apis":
488+
// API group list discovery.
489+
w.Header().Set("Content-Type", "application/json")
490+
_, _ = w.Write([]byte(`{"kind":"APIGroupList","apiVersion":"v1","groups":[]}`))
491+
case "/api/v1":
492+
// Core v1 resource list — tells the REST mapper about Namespace.
493+
w.Header().Set("Content-Type", "application/json")
494+
_, _ = w.Write([]byte(`{"kind":"APIResourceList","apiVersion":"v1","groupVersion":"v1","resources":[{"name":"namespaces","singularName":"namespace","namespaced":false,"kind":"Namespace","verbs":["create","delete","get","list","patch","update","watch"]}]}`))
495+
case "/api/v1/namespaces":
496+
// Simulate the k8s API server's automatic gzip compression for
497+
// responses exceeding ~128KB. The body here is small, but the
498+
// Content-Encoding: gzip header exercises the same FilterResp
499+
// code path as a real large response.
500+
w.Header().Set("Content-Type", "application/json")
501+
w.Header().Set("Content-Encoding", "gzip")
502+
gz := gzip.NewWriter(w)
503+
_, _ = gz.Write([]byte(responseText))
504+
_ = gz.Close()
505+
default:
506+
w.Header().Set("Content-Type", "application/json")
507+
_, _ = w.Write([]byte(`{"kind":"Status","status":"Success"}`))
508+
}
509+
}))
510+
t.Cleanup(mockServer.Close)
511+
512+
// Return the TLS server's own client transport so the reverse proxy
513+
// trusts the self-signed certificate. The transport's
514+
// DisableCompression=false default means it *would* auto-decompress if
515+
// it had injected Accept-Encoding: gzip itself — but because the
516+
// incoming client request already carries that header (see below), the
517+
// transport forwards it unchanged and skips auto-decompression.
518+
return &rest.Config{
519+
Host: mockServer.URL,
520+
// Insecure=true lets the REST mapper's discovery client connect to
521+
// the mock TLS server without certificate errors.
522+
TLSClientConfig: rest.TLSClientConfig{Insecure: true},
523+
}, mockServer.Client().Transport, nil
524+
}
525+
526+
opts.Matcher = rules.MatcherFunc(func(match *request.RequestInfo) []*rules.RunnableRule {
527+
return []*rules.RunnableRule{{
528+
Checks: []rules.RelationshipExpr{},
529+
}}
530+
})
531+
532+
completedConfig, err := opts.Complete(ctx)
533+
require.NoError(t, err)
534+
535+
proxySrv, err := NewServer(ctx, completedConfig)
536+
require.NoError(t, err)
537+
538+
client := proxySrv.GetEmbeddedClient(
539+
WithUser("test-user"),
540+
WithGroups("test-group"),
541+
)
542+
require.NotNil(t, client)
543+
544+
// Include Accept-Encoding: gzip on the request, exactly as kubectl and
545+
// client-go do. The proxy copies this header to its upstream request; the
546+
// upstream transport then sees the header was already present and does NOT
547+
// auto-decompress the gzip response it receives. FilterResp therefore sees
548+
// the raw compressed bytes when it reads resp.Body.
549+
req, err := http.NewRequestWithContext(ctx, "GET", EmbeddedProxyHost+"/api/v1/namespaces", nil)
550+
require.NoError(t, err)
551+
req.Header.Set("Accept-Encoding", "gzip")
552+
553+
resp, err := client.Do(req)
554+
require.NoError(t, err)
555+
t.Cleanup(func() {
556+
_ = resp.Body.Close()
557+
})
558+
559+
body, err := io.ReadAll(resp.Body)
560+
require.NoError(t, err)
561+
require.Equal(t, "gzip", resp.Header.Get("Content-Encoding"), "expecting the response to be gzipped")
562+
require.Equal(t, responseText + "\n", string(body), "unexpected body value")
563+
564+
// The proxy should relay the response successfully.
565+
// A 502 Bad Gateway here means FilterResp attempted to decode the gzip
566+
// bytes as JSON/protobuf without first decompressing them. The fix is to
567+
// detect Content-Encoding: gzip in FilterResp and decompress the body
568+
// before passing it to the codec decoder.
569+
require.Equal(t, http.StatusOK, resp.StatusCode,
570+
"proxy must handle gzip-encoded upstream responses; a 502 indicates the body was not decompressed before decoding")
571+
}
572+
454573
// headerAddingTransport wraps an http.RoundTripper to add authentication headers
455574
type headerAddingTransport struct {
456575
base http.RoundTripper

0 commit comments

Comments
 (0)