UPSTREAM: <carry>: return 429 instead of 404 when the server hasn't b…

…een ready

WithNotFoundProtectorHandler will return 429 instead of 404 iff:
 - server hasn't been ready (/readyz=false)
 - the user is GC or the namespace lifecycle controller
 - the path is for an aggregated API or CR

This handler ensures that the system stays consistent even when requests are received before the server is ready.
In particular it prevents child deletion in case of GC or/and orphaned content in case of the namespaces controller.

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

@@ -0,0 +1,50 @@
+package server
+
+import (
+	"context"
+	"net/http"
+	"strings"
+
+	"k8s.io/apiserver/pkg/authorization/authorizer"
+)
+
+// WithNotFoundProtectorHandler will return 429 instead of 404 iff:
+//  - server hasn't been ready (/readyz=false)
+//  - the user is GC or the namespace lifecycle controller
+//  - the path is for an aggregated API or CR
+//
+// This handler ensures that the system stays consistent even when requests are received before the server is ready.
+// In particular it prevents child deletion in case of GC or/and orphaned content in case of the namespaces controller.
+func WithNotFoundProtectorHandler(delegate http.Handler, hasBeenReadyCh <-chan struct{}, authorizerAttributesFunc func(ctx context.Context) (authorizer.Attributes, error)) http.Handler {
+	return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		select {
+		case <-hasBeenReadyCh:
+			delegate.ServeHTTP(w, r)
+			return
+		default:
+		}
+
+		ctx := r.Context()
+		attribs, err := authorizerAttributesFunc(ctx)
+		if err != nil {
+			delegate.ServeHTTP(w, r)
+			return
+		}
+
+		if patchMatches(r.URL.Path) && userMatches(attribs.GetUser().GetName()) {
+			w.Header().Set("Retry-After", "3")
+			http.Error(w, "The server hasn't been ready yet, please try again later.", http.StatusTooManyRequests)
+			return
+		}
+		delegate.ServeHTTP(w, r)
+	})
+}
+
+func patchMatches(path string) bool {
+	// since discovery contains all groups, we have to block the discovery paths until CRDs and APIServices are synced
+	return strings.HasPrefix(path, "/apis") || strings.HasPrefix(path, "/apis/")
+}
+
+func userMatches(user string) bool {
+	return user == "system:serviceaccount:kube-system:generic-garbage-collector" || user == "system:serviceaccount:kube-system:namespace-controller"
+}

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

@@ -0,0 +1,74 @@
+package server
+
+import (
+	"context"
+	"net/http"
+	"net/http/httptest"
+	"testing"
+
+	"k8s.io/apiserver/pkg/authentication/user"
+	"k8s.io/apiserver/pkg/authorization/authorizer"
+)
+
+func TestWithNotFoundProtectorHandler(t *testing.T) {
+	// test data
+	var currentUser, currentPath string
+	hasBeenReadyCh := make(chan struct{})
+	delegate := http.HandlerFunc(func(w http.ResponseWriter, req *http.Request) {
+		w.WriteHeader(http.StatusOK)
+	})
+	authorizerAttributesTestFunc := func(ctx context.Context) (authorizer.Attributes, error) {
+		return authorizer.AttributesRecord{
+			User: &user.DefaultInfo{
+				Name: currentUser,
+			},
+			Path: currentPath,
+		}, nil
+	}
+	testServer := httptest.NewServer(WithNotFoundProtectorHandler(delegate, hasBeenReadyCh, authorizerAttributesTestFunc))
+	defer testServer.Close()
+
+	// scenario 1: server hasn't been ready and the user doesn't match
+	currentUser = "bob"
+	currentPath = "/apis/operators.coreos.com/v1alpha1/namespaces/abc/clusterserviceversions"
+	expect(t, testServer, currentPath, http.StatusOK)
+
+	// scenario 2: hasn't been ready and the path doesn't match
+	currentUser = "system:serviceaccount:kube-system:generic-garbage-collector"
+	currentPath = "/api/v1/namespaces/abc/endpoints"
+	expect(t, testServer, currentPath, http.StatusOK)
+
+	// scenario 3: hasn't been ready for GC
+	currentUser = "system:serviceaccount:kube-system:generic-garbage-collector"
+	currentPath = "/apis/operators.coreos.com/v1alpha1/namespaces/abc/clusterserviceversions"
+	expect(t, testServer, currentPath, http.StatusTooManyRequests)
+
+	// scenario 4: hasn't been ready for ns controller
+	currentUser = "system:serviceaccount:kube-system:namespace-controller"
+	currentPath = "/apis/operators.coreos.com/v1alpha1/namespaces/abc/clusterserviceversions"
+	expect(t, testServer, currentPath, http.StatusTooManyRequests)
+
+	close(hasBeenReadyCh)
+
+	// scenario 5: has been ready for GC
+	currentUser = "system:serviceaccount:kube-system:generic-garbage-collector"
+	currentPath = "/apis/operators.coreos.com/v1alpha1/namespaces/abc/clusterserviceversions"
+	expect(t, testServer, currentPath, http.StatusOK)
+
+	// scenario 6: has been ready for ns controller
+	currentUser = "system:serviceaccount:kube-system:namespace-controller"
+	currentPath = "/apis/operators.coreos.com/v1alpha1/namespaces/abc/clusterserviceversions"
+	expect(t, testServer, currentPath, http.StatusOK)
+}
+
+func expect(t *testing.T, testServer *httptest.Server, currentPath string, expectedStatusCode int) {
+	t.Helper()
+
+	response, err := testServer.Client().Get(testServer.URL + currentPath)
+	if err != nil {
+		t.Fatal(err)
+	}
+	if response.StatusCode != expectedStatusCode {
+		t.Fatalf("expected %d, got %d", expectedStatusCode, response.StatusCode)
+	}
+}