From 53867975e72c7a2d2dd94aac6bd2869411f92094 Mon Sep 17 00:00:00 2001 From: Lukasz Szaszkiewicz Date: Fri, 15 Oct 2021 17:37:16 +0200 Subject: [PATCH] apiserver: indroduces NotFoundHanlder The new handler is meant to be executed at the end of the delegation chain. It simply checks if the request have been made before the server has installed all known HTTP paths. In that case it returns a 503 response otherwise it returns a 404. We don't want to add additional checks to the readyz path as it might prevent fixing bricked clusters. This specific handler is meant to "protect" requests that arrive before the paths and handlers are fully initialized. --- cmd/kube-apiserver/app/server.go | 1 + .../util/notfoundhandler/not_found_handler.go | 66 +++++++++++++++++ .../notfoundhandler/not_found_handler_test.go | 74 +++++++++++++++++++ 3 files changed, 141 insertions(+) create mode 100644 staging/src/k8s.io/apiserver/pkg/util/notfoundhandler/not_found_handler.go create mode 100644 staging/src/k8s.io/apiserver/pkg/util/notfoundhandler/not_found_handler_test.go diff --git a/cmd/kube-apiserver/app/server.go b/cmd/kube-apiserver/app/server.go index ddd00092955..5f3b510a7bf 100644 --- a/cmd/kube-apiserver/app/server.go +++ b/cmd/kube-apiserver/app/server.go @@ -38,6 +38,7 @@ import ( "k8s.io/apimachinery/pkg/util/sets" "k8s.io/apiserver/pkg/admission" "k8s.io/apiserver/pkg/authorization/authorizer" + genericapifilters "k8s.io/apiserver/pkg/endpoints/filters" openapinamer "k8s.io/apiserver/pkg/endpoints/openapi" genericfeatures "k8s.io/apiserver/pkg/features" genericapiserver "k8s.io/apiserver/pkg/server" diff --git a/staging/src/k8s.io/apiserver/pkg/util/notfoundhandler/not_found_handler.go b/staging/src/k8s.io/apiserver/pkg/util/notfoundhandler/not_found_handler.go new file mode 100644 index 00000000000..a828637f572 --- /dev/null +++ b/staging/src/k8s.io/apiserver/pkg/util/notfoundhandler/not_found_handler.go @@ -0,0 +1,66 @@ +/* +Copyright 2021 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 notfoundhandler + +import ( + "context" + "fmt" + "net/http" + + apierrors "k8s.io/apimachinery/pkg/api/errors" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/runtime" + "k8s.io/apimachinery/pkg/runtime/schema" + "k8s.io/apiserver/pkg/endpoints/handlers/responsewriters" + apirequest "k8s.io/apiserver/pkg/endpoints/request" +) + +// New returns an HTTP handler that is meant to be executed at the end of the delegation chain. +// It checks if the request have been made before the server has installed all known HTTP paths. +// In that case it returns a 503 response otherwise it returns a 404. +// +// Note that we don't want to add additional checks to the readyz path as it might prevent fixing bricked clusters. +// This specific handler is meant to "protect" requests that arrive before the paths and handlers are fully initialized. +func New(serializer runtime.NegotiatedSerializer, hasMuxIncompleteKeyFn func(ctx context.Context) bool) *Handler { + return &Handler{serializer: serializer, hasMuxIncompleteKeyFn: hasMuxIncompleteKeyFn} +} + +type Handler struct { + serializer runtime.NegotiatedSerializer + hasMuxIncompleteKeyFn func(ctx context.Context) bool +} + +func (h *Handler) ServeHTTP(rw http.ResponseWriter, req *http.Request) { + if h.hasMuxIncompleteKeyFn(req.Context()) { + errMsg := fmt.Sprintf("the request has been made before all known HTTP paths have been installed, please try again") + err := apierrors.NewServiceUnavailable(errMsg) + if err.ErrStatus.Details == nil { + err.ErrStatus.Details = &metav1.StatusDetails{} + } + err.ErrStatus.Details.RetryAfterSeconds = int32(5) + + gv := schema.GroupVersion{Group: "unknown", Version: "unknown"} + requestInfo, ok := apirequest.RequestInfoFrom(req.Context()) + if ok { + gv.Group = requestInfo.APIGroup + gv.Version = requestInfo.APIVersion + } + responsewriters.ErrorNegotiated(err, h.serializer, gv, rw, req) + return + } + http.NotFound(rw, req) +} diff --git a/staging/src/k8s.io/apiserver/pkg/util/notfoundhandler/not_found_handler_test.go b/staging/src/k8s.io/apiserver/pkg/util/notfoundhandler/not_found_handler_test.go new file mode 100644 index 00000000000..0906e5cb12c --- /dev/null +++ b/staging/src/k8s.io/apiserver/pkg/util/notfoundhandler/not_found_handler_test.go @@ -0,0 +1,74 @@ +/* +Copyright 2021 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 notfoundhandler + +import ( + "context" + "io" + "net/http/httptest" + "strings" + "testing" + + "k8s.io/apimachinery/pkg/runtime" + "k8s.io/apimachinery/pkg/runtime/serializer" +) + +func TestNotFoundHandler(t *testing.T) { + hasMuxIncompleteKeyGlobalValue := false + hasMuxIncompleteKeyTestFn := func(ctx context.Context) bool { return hasMuxIncompleteKeyGlobalValue } + serializer := serializer.NewCodecFactory(runtime.NewScheme()).WithoutConversion() + target := New(serializer, hasMuxIncompleteKeyTestFn) + + // scenario 1: pretend the request has been made after the signal has been ready + req := httptest.NewRequest("GET", "http://apiserver.com/apis/flowcontrol.apiserver.k8s.io/v1beta1", nil) + rw := httptest.NewRecorder() + + target.ServeHTTP(rw, req) + resp := rw.Result() + body, err := io.ReadAll(resp.Body) + if err != nil { + t.Fatal(err) + } + bodyStr := strings.TrimSuffix(string(body), "\n") + + if resp.StatusCode != 404 { + t.Fatalf("unexpected status code %d, expected 503", resp.StatusCode) + } + expectedMsg := "404 page not found" + if bodyStr != expectedMsg { + t.Fatalf("unexpected response: %v, expected: %v", bodyStr, expectedMsg) + } + + // scenario 2: pretend the request has been made before the signal has been ready + hasMuxIncompleteKeyGlobalValue = true + rw = httptest.NewRecorder() + + target.ServeHTTP(rw, req) + resp = rw.Result() + body, err = io.ReadAll(resp.Body) + if err != nil { + t.Fatal(err) + } + bodyStr = strings.TrimSuffix(string(body), "\n") + if resp.StatusCode != 503 { + t.Fatalf("unexpected status code %d, expected 503", resp.StatusCode) + } + expectedMsg = `{"kind":"Status","apiVersion":"v1","metadata":{},"status":"Failure","message":"the request has been made before all known HTTP paths have been installed, please try again","reason":"ServiceUnavailable","details":{"retryAfterSeconds":5},"code":503}` + if bodyStr != expectedMsg { + t.Fatalf("unexpected response: %v, expected: %v", bodyStr, expectedMsg) + } +}