From a92b46730ee6258fb069899f78e05339ce0b1999 Mon Sep 17 00:00:00 2001 From: Darren Shepherd Date: Fri, 5 Oct 2018 11:48:16 -0700 Subject: [PATCH] Remove APIResponseCompression --- pkg/features/kube_features.go | 1 - .../apiserver/pkg/endpoints/groupversion.go | 5 - .../apiserver/pkg/endpoints/installer.go | 14 +- .../apiserver/pkg/features/kube_features.go | 7 - .../src/k8s.io/apiserver/pkg/server/config.go | 13 +- .../pkg/server/filters/compression.go | 181 ------------------ .../pkg/server/filters/compression_test.go | 106 ---------- .../apiserver/pkg/server/genericapiserver.go | 5 - 8 files changed, 5 insertions(+), 327 deletions(-) delete mode 100644 staging/src/k8s.io/apiserver/pkg/server/filters/compression.go delete mode 100644 staging/src/k8s.io/apiserver/pkg/server/filters/compression_test.go diff --git a/pkg/features/kube_features.go b/pkg/features/kube_features.go index de0c3ea846..31f3939c50 100644 --- a/pkg/features/kube_features.go +++ b/pkg/features/kube_features.go @@ -403,7 +403,6 @@ var defaultKubernetesFeatureGates = map[utilfeature.Feature]utilfeature.FeatureS genericfeatures.ValidateProxyRedirects: {Default: true, PreRelease: utilfeature.Beta}, genericfeatures.AdvancedAuditing: {Default: true, PreRelease: utilfeature.GA}, genericfeatures.DynamicAuditing: {Default: false, PreRelease: utilfeature.Alpha}, - genericfeatures.APIResponseCompression: {Default: false, PreRelease: utilfeature.Alpha}, genericfeatures.APIListChunking: {Default: true, PreRelease: utilfeature.Beta}, genericfeatures.DryRun: {Default: true, PreRelease: utilfeature.Beta}, genericfeatures.ServerSideApply: {Default: false, PreRelease: utilfeature.Alpha}, diff --git a/staging/src/k8s.io/apiserver/pkg/endpoints/groupversion.go b/staging/src/k8s.io/apiserver/pkg/endpoints/groupversion.go index 79cfefe466..a65627acf6 100644 --- a/staging/src/k8s.io/apiserver/pkg/endpoints/groupversion.go +++ b/staging/src/k8s.io/apiserver/pkg/endpoints/groupversion.go @@ -81,10 +81,6 @@ type APIGroupVersion struct { MinRequestTimeout time.Duration - // EnableAPIResponseCompression indicates whether API Responses should support compression - // if the client requests it via Accept-Encoding - EnableAPIResponseCompression bool - // OpenAPIModels exposes the OpenAPI models to each individual handler. OpenAPIModels openapiproto.Models @@ -102,7 +98,6 @@ func (g *APIGroupVersion) InstallREST(container *restful.Container) error { group: g, prefix: prefix, minRequestTimeout: g.MinRequestTimeout, - enableAPIResponseCompression: g.EnableAPIResponseCompression, } apiResources, ws, registrationErrors := installer.Install() diff --git a/staging/src/k8s.io/apiserver/pkg/endpoints/installer.go b/staging/src/k8s.io/apiserver/pkg/endpoints/installer.go index c48c2d61f6..848c1c699f 100644 --- a/staging/src/k8s.io/apiserver/pkg/endpoints/installer.go +++ b/staging/src/k8s.io/apiserver/pkg/endpoints/installer.go @@ -40,7 +40,6 @@ import ( "k8s.io/apiserver/pkg/endpoints/metrics" "k8s.io/apiserver/pkg/features" "k8s.io/apiserver/pkg/registry/rest" - genericfilters "k8s.io/apiserver/pkg/server/filters" utilfeature "k8s.io/apiserver/pkg/util/feature" ) @@ -50,10 +49,9 @@ const ( ) type APIInstaller struct { - group *APIGroupVersion - prefix string // Path prefix where API resources are to be registered. - minRequestTimeout time.Duration - enableAPIResponseCompression bool + group *APIGroupVersion + prefix string // Path prefix where API resources are to be registered. + minRequestTimeout time.Duration } // Struct capturing information about an action ("GET", "POST", "WATCH", "PROXY", etc). @@ -623,9 +621,6 @@ func (a *APIInstaller) registerResourceHandlers(path string, storage rest.Storag handler = metrics.InstrumentRouteFunc(action.Verb, group, version, resource, subresource, requestScope, metrics.APIServerComponent, handler) } - if a.enableAPIResponseCompression { - handler = genericfilters.RestfulWithCompression(handler) - } doc := "read the specified " + kind if isSubresource { doc = "read " + subresource + " of the specified " + kind @@ -655,9 +650,6 @@ func (a *APIInstaller) registerResourceHandlers(path string, storage rest.Storag doc = "list " + subresource + " of objects of kind " + kind } handler := metrics.InstrumentRouteFunc(action.Verb, group, version, resource, subresource, requestScope, metrics.APIServerComponent, restfulListResource(lister, watcher, reqScope, false, a.minRequestTimeout)) - if a.enableAPIResponseCompression { - handler = genericfilters.RestfulWithCompression(handler) - } route := ws.GET(action.Path).To(handler). Doc(doc). Param(ws.QueryParameter("pretty", "If 'true', then the output is pretty printed.")). diff --git a/staging/src/k8s.io/apiserver/pkg/features/kube_features.go b/staging/src/k8s.io/apiserver/pkg/features/kube_features.go index 587a6dfe54..724bb644b8 100644 --- a/staging/src/k8s.io/apiserver/pkg/features/kube_features.go +++ b/staging/src/k8s.io/apiserver/pkg/features/kube_features.go @@ -60,12 +60,6 @@ const ( // AuditSink API object. DynamicAuditing utilfeature.Feature = "DynamicAuditing" - // owner: @ilackams - // alpha: v1.7 - // - // Enables compression of REST responses (GET and LIST only) - APIResponseCompression utilfeature.Feature = "APIResponseCompression" - // owner: @smarterclayton // alpha: v1.8 // beta: v1.9 @@ -121,7 +115,6 @@ var defaultKubernetesFeatureGates = map[utilfeature.Feature]utilfeature.FeatureS ValidateProxyRedirects: {Default: true, PreRelease: utilfeature.Beta}, AdvancedAuditing: {Default: true, PreRelease: utilfeature.GA}, DynamicAuditing: {Default: false, PreRelease: utilfeature.Alpha}, - APIResponseCompression: {Default: false, PreRelease: utilfeature.Alpha}, APIListChunking: {Default: true, PreRelease: utilfeature.Beta}, DryRun: {Default: true, PreRelease: utilfeature.Beta}, ServerSideApply: {Default: false, PreRelease: utilfeature.Alpha}, diff --git a/staging/src/k8s.io/apiserver/pkg/server/config.go b/staging/src/k8s.io/apiserver/pkg/server/config.go index 75d1276c47..d979d2f727 100644 --- a/staging/src/k8s.io/apiserver/pkg/server/config.go +++ b/staging/src/k8s.io/apiserver/pkg/server/config.go @@ -53,13 +53,11 @@ import ( genericapifilters "k8s.io/apiserver/pkg/endpoints/filters" apiopenapi "k8s.io/apiserver/pkg/endpoints/openapi" apirequest "k8s.io/apiserver/pkg/endpoints/request" - "k8s.io/apiserver/pkg/features" genericregistry "k8s.io/apiserver/pkg/registry/generic" genericfilters "k8s.io/apiserver/pkg/server/filters" "k8s.io/apiserver/pkg/server/healthz" "k8s.io/apiserver/pkg/server/routes" serverstore "k8s.io/apiserver/pkg/server/storage" - utilfeature "k8s.io/apiserver/pkg/util/feature" "k8s.io/client-go/informers" restclient "k8s.io/client-go/rest" certutil "k8s.io/client-go/util/cert" @@ -171,10 +169,6 @@ type Config struct { // Predicate which is true for paths of long-running http requests LongRunningFunc apirequest.LongRunningRequestCheck - // EnableAPIResponseCompression indicates whether API Responses should support compression - // if the client requests it via Accept-Encoding - EnableAPIResponseCompression bool - // MergedResourceConfig indicates which groupVersion enabled and its resources enabled/disabled. // This is composed of genericapiserver defaultAPIResourceConfig and those parsed from flags. // If not specify any in flags, then genericapiserver will only enable defaultAPIResourceConfig. @@ -281,8 +275,7 @@ func NewConfig(codecs serializer.CodecFactory) *Config { // proto when persisted in etcd. Assuming the upper bound of // the size ratio is 10:1, we set 100MB as the largest request // body size to be accepted and decoded in a write request. - MaxRequestBodyBytes: int64(100 * 1024 * 1024), - EnableAPIResponseCompression: utilfeature.DefaultFeatureGate.Enabled(features.APIResponseCompression), + MaxRequestBodyBytes: int64(100 * 1024 * 1024), // Default to treating watch as a long-running operation // Generic API servers have no inherent long-running subresources @@ -472,9 +465,7 @@ func (c completedConfig) New(name string, delegationTarget DelegationTarget) (*G healthzChecks: c.HealthzChecks, DiscoveryGroupManager: discovery.NewRootAPIsHandler(c.DiscoveryAddresses, c.Serializer), - - enableAPIResponseCompression: c.EnableAPIResponseCompression, - maxRequestBodyBytes: c.MaxRequestBodyBytes, + maxRequestBodyBytes: c.MaxRequestBodyBytes, } for { diff --git a/staging/src/k8s.io/apiserver/pkg/server/filters/compression.go b/staging/src/k8s.io/apiserver/pkg/server/filters/compression.go deleted file mode 100644 index 625cd5c8d3..0000000000 --- a/staging/src/k8s.io/apiserver/pkg/server/filters/compression.go +++ /dev/null @@ -1,181 +0,0 @@ -/* -Copyright 2017 The Kubernetes 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 filters - -import ( - "compress/gzip" - "compress/zlib" - "errors" - "fmt" - "io" - "net/http" - "strings" - - "github.com/emicklei/go-restful" - - "k8s.io/apimachinery/pkg/util/runtime" - "k8s.io/apiserver/pkg/endpoints/request" -) - -// Compressor is an interface to compression writers -type Compressor interface { - io.WriteCloser - Flush() error -} - -const ( - headerAcceptEncoding = "Accept-Encoding" - headerContentEncoding = "Content-Encoding" - - encodingGzip = "gzip" - encodingDeflate = "deflate" -) - -// WithCompression wraps an http.Handler with the Compression Handler -func WithCompression(handler http.Handler) http.Handler { - return http.HandlerFunc(func(w http.ResponseWriter, req *http.Request) { - wantsCompression, encoding := wantsCompressedResponse(req) - w.Header().Set("Vary", "Accept-Encoding") - if wantsCompression { - compressionWriter, err := NewCompressionResponseWriter(w, encoding) - if err != nil { - handleError(w, req, err) - runtime.HandleError(fmt.Errorf("failed to compress HTTP response: %v", err)) - return - } - compressionWriter.Header().Set("Content-Encoding", encoding) - handler.ServeHTTP(compressionWriter, req) - compressionWriter.(*compressionResponseWriter).Close() - } else { - handler.ServeHTTP(w, req) - } - }) -} - -// wantsCompressedResponse reads the Accept-Encoding header to see if and which encoding is requested. -func wantsCompressedResponse(req *http.Request) (bool, string) { - // don't compress watches - ctx := req.Context() - info, ok := request.RequestInfoFrom(ctx) - if !ok { - return false, "" - } - if !info.IsResourceRequest { - return false, "" - } - if info.Verb == "watch" { - return false, "" - } - header := req.Header.Get(headerAcceptEncoding) - gi := strings.Index(header, encodingGzip) - zi := strings.Index(header, encodingDeflate) - // use in order of appearance - switch { - case gi == -1: - return zi != -1, encodingDeflate - case zi == -1: - return gi != -1, encodingGzip - case gi < zi: - return true, encodingGzip - default: - return true, encodingDeflate - } -} - -type compressionResponseWriter struct { - writer http.ResponseWriter - compressor Compressor - encoding string -} - -// NewCompressionResponseWriter returns wraps w with a compression ResponseWriter, using the given encoding -func NewCompressionResponseWriter(w http.ResponseWriter, encoding string) (http.ResponseWriter, error) { - var compressor Compressor - switch encoding { - case encodingGzip: - compressor = gzip.NewWriter(w) - case encodingDeflate: - compressor = zlib.NewWriter(w) - default: - return nil, fmt.Errorf("%s is not a supported encoding type", encoding) - } - return &compressionResponseWriter{ - writer: w, - compressor: compressor, - encoding: encoding, - }, nil -} - -// compressionResponseWriter implements http.Responsewriter Interface -var _ http.ResponseWriter = &compressionResponseWriter{} - -func (c *compressionResponseWriter) Header() http.Header { - return c.writer.Header() -} - -// compress data according to compression method -func (c *compressionResponseWriter) Write(p []byte) (int, error) { - if c.compressorClosed() { - return -1, errors.New("compressing error: tried to write data using closed compressor") - } - c.Header().Set(headerContentEncoding, c.encoding) - defer c.compressor.Flush() - return c.compressor.Write(p) -} - -func (c *compressionResponseWriter) WriteHeader(status int) { - c.writer.WriteHeader(status) -} - -// CloseNotify is part of http.CloseNotifier interface -func (c *compressionResponseWriter) CloseNotify() <-chan bool { - return c.writer.(http.CloseNotifier).CloseNotify() -} - -// Close the underlying compressor -func (c *compressionResponseWriter) Close() error { - if c.compressorClosed() { - return errors.New("Compressing error: tried to close already closed compressor") - } - - c.compressor.Close() - c.compressor = nil - return nil -} - -func (c *compressionResponseWriter) Flush() { - if c.compressorClosed() { - return - } - c.compressor.Flush() -} - -func (c *compressionResponseWriter) compressorClosed() bool { - return nil == c.compressor -} - -// RestfulWithCompression wraps WithCompression to be compatible with go-restful -func RestfulWithCompression(function restful.RouteFunction) restful.RouteFunction { - return restful.RouteFunction(func(request *restful.Request, response *restful.Response) { - handler := WithCompression(http.HandlerFunc(func(w http.ResponseWriter, req *http.Request) { - response.ResponseWriter = w - request.Request = req - function(request, response) - })) - handler.ServeHTTP(response.ResponseWriter, request.Request) - }) -} diff --git a/staging/src/k8s.io/apiserver/pkg/server/filters/compression_test.go b/staging/src/k8s.io/apiserver/pkg/server/filters/compression_test.go deleted file mode 100644 index b179cff8ee..0000000000 --- a/staging/src/k8s.io/apiserver/pkg/server/filters/compression_test.go +++ /dev/null @@ -1,106 +0,0 @@ -/* -Copyright 2017 The Kubernetes 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 filters - -import ( - "bytes" - "compress/gzip" - "io" - "io/ioutil" - "net/http" - "net/http/httptest" - "testing" - - "k8s.io/apimachinery/pkg/util/sets" - "k8s.io/apiserver/pkg/endpoints/filters" - "k8s.io/apiserver/pkg/endpoints/request" -) - -func TestCompression(t *testing.T) { - tests := []struct { - encoding string - watch bool - }{ - {"", false}, - {"gzip", true}, - {"gzip", false}, - } - - responseData := []byte("1234") - - for _, test := range tests { - handler := WithCompression( - http.HandlerFunc(func(w http.ResponseWriter, req *http.Request) { - w.Write(responseData) - }), - ) - handler = filters.WithRequestInfo(handler, newTestRequestInfoResolver()) - server := httptest.NewServer(handler) - defer server.Close() - client := http.Client{ - Transport: &http.Transport{ - DisableCompression: true, - }, - } - - url := server.URL + "/api/v1/pods" - if test.watch { - url = url + "?watch=1" - } - request, err := http.NewRequest("GET", url, nil) - if err != nil { - t.Errorf("unexpected error: %v", err) - } - request.Header.Set("Accept-Encoding", test.encoding) - response, err := client.Do(request) - if err != nil { - t.Errorf("unexpected error: %v", err) - } - var reader io.Reader - if test.encoding == "gzip" && !test.watch { - if response.Header.Get("Content-Encoding") != "gzip" { - t.Fatal("expected response header Content-Encoding to be set to \"gzip\"") - } - if response.Header.Get("Vary") != "Accept-Encoding" { - t.Fatal("expected response header Vary to be set to \"Accept-Encoding\"") - } - reader, err = gzip.NewReader(response.Body) - if err != nil { - t.Fatalf("unexpected error: %v", err) - } - } else { - if response.Header.Get("Content-Encoding") == "gzip" { - t.Fatal("expected response header Content-Encoding not to be set") - } - reader = response.Body - } - body, err := ioutil.ReadAll(reader) - if err != nil { - t.Fatalf("unexpected error: %v", err) - } - if !bytes.Equal(body, responseData) { - t.Fatalf("Expected response body %s to equal %s", body, responseData) - } - } -} - -func newTestRequestInfoResolver() *request.RequestInfoFactory { - return &request.RequestInfoFactory{ - APIPrefixes: sets.NewString("api", "apis"), - GrouplessAPIPrefixes: sets.NewString("api"), - } -} diff --git a/staging/src/k8s.io/apiserver/pkg/server/genericapiserver.go b/staging/src/k8s.io/apiserver/pkg/server/genericapiserver.go index 450488e13d..27e151e409 100644 --- a/staging/src/k8s.io/apiserver/pkg/server/genericapiserver.go +++ b/staging/src/k8s.io/apiserver/pkg/server/genericapiserver.go @@ -157,10 +157,6 @@ type GenericAPIServer struct { // the create-on-update case Authorizer authorizer.Authorizer - // enableAPIResponseCompression indicates whether API Responses should support compression - // if the client requests it via Accept-Encoding - enableAPIResponseCompression bool - // delegationTarget is the next delegate in the chain. This is never nil. delegationTarget DelegationTarget @@ -466,7 +462,6 @@ func (s *GenericAPIServer) newAPIGroupVersion(apiGroupInfo *APIGroupInfo, groupV Admit: s.admissionControl, MinRequestTimeout: s.minRequestTimeout, - EnableAPIResponseCompression: s.enableAPIResponseCompression, Authorizer: s.Authorizer, } }