Skip to content

Commit 80af10c

Browse files
author
Kubernetes Submit Queue
authored
Merge pull request #46966 from ilackarms/compression-gating
Automatic merge from submit-queue (batch tested with PRs 47883, 47179, 46966, 47982, 47945) Add feature gating to REST Compression **What this PR does / why we need it**: Adds feature gating to opt out of REST API compression **Which issue this PR fixes** *(optional, in `fixes #<issue number>(, fixes #<issue_number>, ...)` format, will close that issue when PR gets merged)*: fixes #46963 **Special notes for your reviewer**: This PR is a fix / addendum to #45666 **Release note**: ```release-note ```
2 parents 171f48a + c305f72 commit 80af10c

File tree

10 files changed

+534
-22
lines changed

10 files changed

+534
-22
lines changed

staging/src/k8s.io/apiserver/pkg/endpoints/BUILD

+2
Original file line numberDiff line numberDiff line change
@@ -55,6 +55,7 @@ go_test(
5555
"//vendor/k8s.io/apiserver/pkg/endpoints/request:go_default_library",
5656
"//vendor/k8s.io/apiserver/pkg/endpoints/testing:go_default_library",
5757
"//vendor/k8s.io/apiserver/pkg/registry/rest:go_default_library",
58+
"//vendor/k8s.io/apiserver/pkg/server/filters:go_default_library",
5859
],
5960
)
6061

@@ -84,5 +85,6 @@ go_library(
8485
"//vendor/k8s.io/apiserver/pkg/endpoints/metrics:go_default_library",
8586
"//vendor/k8s.io/apiserver/pkg/endpoints/request:go_default_library",
8687
"//vendor/k8s.io/apiserver/pkg/registry/rest:go_default_library",
88+
"//vendor/k8s.io/apiserver/pkg/server/filters:go_default_library",
8789
],
8890
)

staging/src/k8s.io/apiserver/pkg/endpoints/apiserver_test.go

+182
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@ package endpoints
1818

1919
import (
2020
"bytes"
21+
"compress/gzip"
2122
"encoding/json"
2223
"errors"
2324
"fmt"
@@ -66,6 +67,7 @@ import (
6667
"k8s.io/apiserver/pkg/endpoints/request"
6768
genericapitesting "k8s.io/apiserver/pkg/endpoints/testing"
6869
"k8s.io/apiserver/pkg/registry/rest"
70+
"k8s.io/apiserver/pkg/server/filters"
6971
)
7072

7173
// alwaysAdmit is an implementation of admission.Interface which always says yes to an admit request.
@@ -1207,6 +1209,110 @@ func TestRequestsWithInvalidQuery(t *testing.T) {
12071209
}
12081210
}
12091211

1212+
func TestListCompression(t *testing.T) {
1213+
testCases := []struct {
1214+
url string
1215+
namespace string
1216+
selfLink string
1217+
legacy bool
1218+
label string
1219+
field string
1220+
acceptEncoding string
1221+
}{
1222+
// list items in a namespace in the path
1223+
{
1224+
url: "/" + grouplessPrefix + "/" + grouplessGroupVersion.Version + "/namespaces/default/simple",
1225+
namespace: "default",
1226+
selfLink: "/" + grouplessPrefix + "/" + grouplessGroupVersion.Version + "/namespaces/default/simple",
1227+
acceptEncoding: "",
1228+
},
1229+
{
1230+
url: "/" + grouplessPrefix + "/" + grouplessGroupVersion.Version + "/namespaces/default/simple",
1231+
namespace: "default",
1232+
selfLink: "/" + grouplessPrefix + "/" + grouplessGroupVersion.Version + "/namespaces/default/simple",
1233+
acceptEncoding: "gzip",
1234+
},
1235+
}
1236+
for i, testCase := range testCases {
1237+
storage := map[string]rest.Storage{}
1238+
simpleStorage := SimpleRESTStorage{expectedResourceNamespace: testCase.namespace}
1239+
storage["simple"] = &simpleStorage
1240+
selfLinker := &setTestSelfLinker{
1241+
t: t,
1242+
namespace: testCase.namespace,
1243+
expectedSet: testCase.selfLink,
1244+
}
1245+
var handler = handleInternal(storage, admissionControl, selfLinker, nil)
1246+
1247+
requestContextMapper = request.NewRequestContextMapper()
1248+
1249+
handler = filters.WithCompression(handler, requestContextMapper)
1250+
handler = genericapifilters.WithRequestInfo(handler, newTestRequestInfoResolver(), requestContextMapper)
1251+
handler = request.WithRequestContext(handler, requestContextMapper)
1252+
1253+
server := httptest.NewServer(handler)
1254+
1255+
defer server.Close()
1256+
1257+
req, err := http.NewRequest("GET", server.URL+testCase.url, nil)
1258+
if err != nil {
1259+
t.Errorf("%d: unexpected error: %v", i, err)
1260+
continue
1261+
}
1262+
// It's necessary to manually set Accept-Encoding here
1263+
// to prevent http.DefaultClient from automatically
1264+
// decoding responses
1265+
req.Header.Set("Accept-Encoding", testCase.acceptEncoding)
1266+
resp, err := http.DefaultClient.Do(req)
1267+
if err != nil {
1268+
t.Errorf("%d: unexpected error: %v", i, err)
1269+
continue
1270+
}
1271+
defer resp.Body.Close()
1272+
if resp.StatusCode != http.StatusOK {
1273+
t.Errorf("%d: unexpected status: %d from url %s, Expected: %d, %#v", i, resp.StatusCode, testCase.url, http.StatusOK, resp)
1274+
body, err := ioutil.ReadAll(resp.Body)
1275+
if err != nil {
1276+
t.Errorf("%d: unexpected error: %v", i, err)
1277+
continue
1278+
}
1279+
t.Logf("%d: body: %s", i, string(body))
1280+
continue
1281+
}
1282+
// TODO: future, restore get links
1283+
if !selfLinker.called {
1284+
t.Errorf("%d: never set self link", i)
1285+
}
1286+
if !simpleStorage.namespacePresent {
1287+
t.Errorf("%d: namespace not set", i)
1288+
} else if simpleStorage.actualNamespace != testCase.namespace {
1289+
t.Errorf("%d: %q unexpected resource namespace: %s", i, testCase.url, simpleStorage.actualNamespace)
1290+
}
1291+
if simpleStorage.requestedLabelSelector == nil || simpleStorage.requestedLabelSelector.String() != testCase.label {
1292+
t.Errorf("%d: unexpected label selector: %v", i, simpleStorage.requestedLabelSelector)
1293+
}
1294+
if simpleStorage.requestedFieldSelector == nil || simpleStorage.requestedFieldSelector.String() != testCase.field {
1295+
t.Errorf("%d: unexpected field selector: %v", i, simpleStorage.requestedFieldSelector)
1296+
}
1297+
1298+
var decoder *json.Decoder
1299+
if testCase.acceptEncoding == "gzip" {
1300+
gzipReader, err := gzip.NewReader(resp.Body)
1301+
if err != nil {
1302+
t.Fatalf("unexpected error creating gzip reader: %v", err)
1303+
}
1304+
decoder = json.NewDecoder(gzipReader)
1305+
} else {
1306+
decoder = json.NewDecoder(resp.Body)
1307+
}
1308+
var itemOut genericapitesting.SimpleList
1309+
err = decoder.Decode(&itemOut)
1310+
if err != nil {
1311+
t.Errorf("failed to read response body as SimpleList: %v", err)
1312+
}
1313+
}
1314+
}
1315+
12101316
func TestLogs(t *testing.T) {
12111317
handler := handle(map[string]rest.Storage{})
12121318
server := httptest.NewServer(handler)
@@ -1522,6 +1628,82 @@ func TestGet(t *testing.T) {
15221628
}
15231629
}
15241630

1631+
func TestGetCompression(t *testing.T) {
1632+
storage := map[string]rest.Storage{}
1633+
simpleStorage := SimpleRESTStorage{
1634+
item: genericapitesting.Simple{
1635+
Other: "foo",
1636+
},
1637+
}
1638+
selfLinker := &setTestSelfLinker{
1639+
t: t,
1640+
expectedSet: "/" + prefix + "/" + testGroupVersion.Group + "/" + testGroupVersion.Version + "/namespaces/default/simple/id",
1641+
name: "id",
1642+
namespace: "default",
1643+
}
1644+
1645+
requestContextMapper = request.NewRequestContextMapper()
1646+
1647+
storage["simple"] = &simpleStorage
1648+
handler := handleLinker(storage, selfLinker)
1649+
handler = filters.WithCompression(handler, requestContextMapper)
1650+
handler = genericapifilters.WithRequestInfo(handler, newTestRequestInfoResolver(), requestContextMapper)
1651+
handler = request.WithRequestContext(handler, requestContextMapper)
1652+
server := httptest.NewServer(handler)
1653+
defer server.Close()
1654+
1655+
tests := []struct {
1656+
acceptEncoding string
1657+
}{
1658+
{acceptEncoding: ""},
1659+
{acceptEncoding: "gzip"},
1660+
}
1661+
1662+
for _, test := range tests {
1663+
req, err := http.NewRequest("GET", server.URL+"/"+prefix+"/"+testGroupVersion.Group+"/"+testGroupVersion.Version+"/namespaces/default/simple/id", nil)
1664+
if err != nil {
1665+
t.Fatalf("unexpected error cretaing request: %v", err)
1666+
}
1667+
// It's necessary to manually set Accept-Encoding here
1668+
// to prevent http.DefaultClient from automatically
1669+
// decoding responses
1670+
req.Header.Set("Accept-Encoding", test.acceptEncoding)
1671+
resp, err := http.DefaultClient.Do(req)
1672+
if err != nil {
1673+
t.Fatalf("unexpected error: %v", err)
1674+
}
1675+
if resp.StatusCode != http.StatusOK {
1676+
t.Fatalf("unexpected response: %#v", resp)
1677+
}
1678+
var decoder *json.Decoder
1679+
if test.acceptEncoding == "gzip" {
1680+
gzipReader, err := gzip.NewReader(resp.Body)
1681+
if err != nil {
1682+
t.Fatalf("unexpected error creating gzip reader: %v", err)
1683+
}
1684+
decoder = json.NewDecoder(gzipReader)
1685+
} else {
1686+
decoder = json.NewDecoder(resp.Body)
1687+
}
1688+
var itemOut genericapitesting.Simple
1689+
err = decoder.Decode(&itemOut)
1690+
if err != nil {
1691+
t.Errorf("unexpected error: %v", err)
1692+
}
1693+
body, err := ioutil.ReadAll(resp.Body)
1694+
if err != nil {
1695+
t.Errorf("unexpected error reading body: %v", err)
1696+
}
1697+
1698+
if itemOut.Name != simpleStorage.item.Name {
1699+
t.Errorf("Unexpected data: %#v, expected %#v (%s)", itemOut, simpleStorage.item, string(body))
1700+
}
1701+
if !selfLinker.called {
1702+
t.Errorf("Never set self link")
1703+
}
1704+
}
1705+
}
1706+
15251707
func TestGetUninitialized(t *testing.T) {
15261708
storage := map[string]rest.Storage{}
15271709
simpleStorage := SimpleRESTStorage{

staging/src/k8s.io/apiserver/pkg/endpoints/groupversion.go

+8-3
Original file line numberDiff line numberDiff line change
@@ -87,6 +87,10 @@ type APIGroupVersion struct {
8787
// ResourceLister is an interface that knows how to list resources
8888
// for this API Group.
8989
ResourceLister discovery.APIResourceLister
90+
91+
// EnableAPIResponseCompression indicates whether API Responses should support compression
92+
// if the client requests it via Accept-Encoding
93+
EnableAPIResponseCompression bool
9094
}
9195

9296
// InstallREST registers the REST handlers (storage, watch, proxy and redirect) into a restful Container.
@@ -138,9 +142,10 @@ func (g *APIGroupVersion) UpdateREST(container *restful.Container) error {
138142
func (g *APIGroupVersion) newInstaller() *APIInstaller {
139143
prefix := path.Join(g.Root, g.GroupVersion.Group, g.GroupVersion.Version)
140144
installer := &APIInstaller{
141-
group: g,
142-
prefix: prefix,
143-
minRequestTimeout: g.MinRequestTimeout,
145+
group: g,
146+
prefix: prefix,
147+
minRequestTimeout: g.MinRequestTimeout,
148+
enableAPIResponseCompression: g.EnableAPIResponseCompression,
144149
}
145150
return installer
146151
}

staging/src/k8s.io/apiserver/pkg/endpoints/installer.go

+11-3
Original file line numberDiff line numberDiff line change
@@ -40,6 +40,7 @@ import (
4040
"k8s.io/apiserver/pkg/endpoints/metrics"
4141
"k8s.io/apiserver/pkg/endpoints/request"
4242
"k8s.io/apiserver/pkg/registry/rest"
43+
genericfilters "k8s.io/apiserver/pkg/server/filters"
4344
)
4445

4546
const (
@@ -48,9 +49,10 @@ const (
4849
)
4950

5051
type APIInstaller struct {
51-
group *APIGroupVersion
52-
prefix string // Path prefix where API resources are to be registered.
53-
minRequestTimeout time.Duration
52+
group *APIGroupVersion
53+
prefix string // Path prefix where API resources are to be registered.
54+
minRequestTimeout time.Duration
55+
enableAPIResponseCompression bool
5456
}
5557

5658
// Struct capturing information about an action ("GET", "POST", "WATCH", "PROXY", etc).
@@ -584,6 +586,9 @@ func (a *APIInstaller) registerResourceHandlers(path string, storage rest.Storag
584586
handler = restfulGetResource(getter, exporter, reqScope)
585587
}
586588
handler = metrics.InstrumentRouteFunc(action.Verb, resource, subresource, handler)
589+
if a.enableAPIResponseCompression {
590+
handler = genericfilters.RestfulWithCompression(handler, a.group.Context)
591+
}
587592
doc := "read the specified " + kind
588593
if hasSubresource {
589594
doc = "read " + subresource + " of the specified " + kind
@@ -613,6 +618,9 @@ func (a *APIInstaller) registerResourceHandlers(path string, storage rest.Storag
613618
doc = "list " + subresource + " of objects of kind " + kind
614619
}
615620
handler := metrics.InstrumentRouteFunc(action.Verb, resource, subresource, restfulListResource(lister, watcher, reqScope, false, a.minRequestTimeout))
621+
if a.enableAPIResponseCompression {
622+
handler = genericfilters.RestfulWithCompression(handler, a.group.Context)
623+
}
616624
route := ws.GET(action.Path).To(handler).
617625
Doc(doc).
618626
Param(ws.QueryParameter("pretty", "If 'true', then the output is pretty printed.")).

staging/src/k8s.io/apiserver/pkg/features/kube_features.go

+7
Original file line numberDiff line numberDiff line change
@@ -41,6 +41,12 @@ const (
4141
// pluggable output backends and an audit policy specifying how different requests should be
4242
// audited.
4343
AdvancedAuditing utilfeature.Feature = "AdvancedAuditing"
44+
45+
// owner: @ilackams
46+
// alpha: v1.7
47+
//
48+
// Enables compression of REST responses (GET and LIST only)
49+
APIResponseCompression utilfeature.Feature = "APIResponseCompression"
4450
)
4551

4652
func init() {
@@ -53,4 +59,5 @@ func init() {
5359
var defaultKubernetesFeatureGates = map[utilfeature.Feature]utilfeature.FeatureSpec{
5460
StreamingProxyRedirects: {Default: true, PreRelease: utilfeature.Beta},
5561
AdvancedAuditing: {Default: false, PreRelease: utilfeature.Alpha},
62+
APIResponseCompression: {Default: false, PreRelease: utilfeature.Alpha},
5663
}

staging/src/k8s.io/apiserver/pkg/server/config.go

+20-13
Original file line numberDiff line numberDiff line change
@@ -160,6 +160,10 @@ type Config struct {
160160
// Predicate which is true for paths of long-running http requests
161161
LongRunningFunc apirequest.LongRunningRequestCheck
162162

163+
// EnableAPIResponseCompression indicates whether API Responses should support compression
164+
// if the client requests it via Accept-Encoding
165+
EnableAPIResponseCompression bool
166+
163167
//===========================================================================
164168
// values below here are targets for removal
165169
//===========================================================================
@@ -206,19 +210,20 @@ type SecureServingInfo struct {
206210
// NewConfig returns a Config struct with the default values
207211
func NewConfig(codecs serializer.CodecFactory) *Config {
208212
return &Config{
209-
Serializer: codecs,
210-
ReadWritePort: 443,
211-
RequestContextMapper: apirequest.NewRequestContextMapper(),
212-
BuildHandlerChainFunc: DefaultBuildHandlerChain,
213-
LegacyAPIGroupPrefixes: sets.NewString(DefaultLegacyAPIPrefix),
214-
DisabledPostStartHooks: sets.NewString(),
215-
HealthzChecks: []healthz.HealthzChecker{healthz.PingHealthz},
216-
EnableIndex: true,
217-
EnableDiscovery: true,
218-
EnableProfiling: true,
219-
MaxRequestsInFlight: 400,
220-
MaxMutatingRequestsInFlight: 200,
221-
MinRequestTimeout: 1800,
213+
Serializer: codecs,
214+
ReadWritePort: 443,
215+
RequestContextMapper: apirequest.NewRequestContextMapper(),
216+
BuildHandlerChainFunc: DefaultBuildHandlerChain,
217+
LegacyAPIGroupPrefixes: sets.NewString(DefaultLegacyAPIPrefix),
218+
DisabledPostStartHooks: sets.NewString(),
219+
HealthzChecks: []healthz.HealthzChecker{healthz.PingHealthz},
220+
EnableIndex: true,
221+
EnableDiscovery: true,
222+
EnableProfiling: true,
223+
MaxRequestsInFlight: 400,
224+
MaxMutatingRequestsInFlight: 200,
225+
MinRequestTimeout: 1800,
226+
EnableAPIResponseCompression: utilfeature.DefaultFeatureGate.Enabled(features.APIResponseCompression),
222227

223228
// Default to treating watch as a long-running operation
224229
// Generic API servers have no inherent long-running subresources
@@ -412,6 +417,8 @@ func (c completedConfig) New(name string, delegationTarget DelegationTarget) (*G
412417
healthzChecks: c.HealthzChecks,
413418

414419
DiscoveryGroupManager: discovery.NewRootAPIsHandler(c.DiscoveryAddresses, c.Serializer, c.RequestContextMapper),
420+
421+
enableAPIResponseCompression: c.EnableAPIResponseCompression,
415422
}
416423

417424
for k, v := range delegationTarget.PostStartHooks() {

staging/src/k8s.io/apiserver/pkg/server/filters/BUILD

+3
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@ load(
1111
go_test(
1212
name = "go_default_test",
1313
srcs = [
14+
"compression_test.go",
1415
"cors_test.go",
1516
"maxinflight_test.go",
1617
"timeout_test.go",
@@ -31,6 +32,7 @@ go_test(
3132
go_library(
3233
name = "go_default_library",
3334
srcs = [
35+
"compression.go",
3436
"cors.go",
3537
"doc.go",
3638
"longrunning.go",
@@ -40,6 +42,7 @@ go_library(
4042
],
4143
tags = ["automanaged"],
4244
deps = [
45+
"//vendor/github.com/emicklei/go-restful:go_default_library",
4346
"//vendor/github.com/golang/glog:go_default_library",
4447
"//vendor/k8s.io/apimachinery/pkg/api/errors:go_default_library",
4548
"//vendor/k8s.io/apimachinery/pkg/runtime/schema:go_default_library",

0 commit comments

Comments
 (0)