Fix DeleteCollection API decode DeleteOptions fail

The reason for the issue is that the apiserver uses the Scheme in the global variable pkg/api/legacyscheme/scheme.go, and registers the DeleteOptions corresponding to each APIGroup in the Scheme. But DeleteOptions in meta.k8s.io/v1 is not registered, resulting in a notRegisteredErr. Use metainternalversionscheme.Codecs as Serializer
kubernetes · Oct 19, 2022 · e7d7f4a · e7d7f4a
1 parent 83415e5
commit e7d7f4a
Show file tree

Hide file tree

Showing 3 changed files with 82 additions and 7 deletions.
diff --git a/staging/src/k8s.io/apiserver/pkg/endpoints/handlers/delete.go b/staging/src/k8s.io/apiserver/pkg/endpoints/handlers/delete.go
@@ -71,7 +71,8 @@ func DeleteResource(r rest.GracefulDeleter, allowsOptions bool, scope *RequestSc
 
 		options := &metav1.DeleteOptions{}
 		if allowsOptions {
-			body, err := limitedReadBodyWithRecordMetric(ctx, req, scope.MaxRequestBodyBytes, scope.Resource.GroupResource().String(), requestmetrics.Patch)
+			body, err := limitedReadBodyWithRecordMetric(ctx, req, scope.MaxRequestBodyBytes, scope.Resource.GroupResource().String(), requestmetrics.Delete)
+			trace.Step("limitedReadBody done", utiltrace.Field{"len", len(body)}, utiltrace.Field{"err", err})
 			if err != nil {
 				scope.err(err, w, req)
 				return
@@ -214,13 +215,14 @@ func DeleteCollection(r rest.CollectionDeleter, checkBody bool, scope *RequestSc
 
 		options := &metav1.DeleteOptions{}
 		if checkBody {
-			body, err := limitedReadBody(req, scope.MaxRequestBodyBytes)
+			body, err := limitedReadBodyWithRecordMetric(ctx, req, scope.MaxRequestBodyBytes, scope.Resource.GroupResource().String(), requestmetrics.DeleteCollection)
+			trace.Step("limitedReadBody done", utiltrace.Field{"len", len(body)}, utiltrace.Field{"err", err})
 			if err != nil {
 				scope.err(err, w, req)
 				return
 			}
 			if len(body) > 0 {
-				s, err := negotiation.NegotiateInputSerializer(req, false, scope.Serializer)
+				s, err := negotiation.NegotiateInputSerializer(req, false, metainternalversionscheme.Codecs)
 				if err != nil {
 					scope.err(err, w, req)
 					return

diff --git a/staging/src/k8s.io/apiserver/pkg/endpoints/handlers/delete_test.go b/staging/src/k8s.io/apiserver/pkg/endpoints/handlers/delete_test.go
@@ -19,6 +19,8 @@ package handlers
 import (
 	"context"
 	"io"
+	"net/http"
+	"strings"
 	"testing"
 
 	metainternalversionscheme "k8s.io/apimachinery/pkg/apis/meta/internalversion/scheme"
@@ -28,6 +30,7 @@ import (
 	"k8s.io/apimachinery/pkg/runtime/serializer"
 	auditapis "k8s.io/apiserver/pkg/apis/audit"
 	"k8s.io/apiserver/pkg/audit"
+	"k8s.io/apiserver/pkg/endpoints/handlers/negotiation"
 	"k8s.io/utils/pointer"
 )
 
@@ -132,3 +135,72 @@ func TestDeleteResourceAuditLogRequestObject(t *testing.T) {
 		})
 	}
 }
+
+func TestDeleteCollection(t *testing.T) {
+	req := &http.Request{
+		Header: http.Header{},
+	}
+	req.Header.Set("Content-Type", "application/json")
+
+	fakeCorev1GroupVersion := schema.GroupVersion{
+		Group:   "",
+		Version: "v1",
+	}
+	fakeCorev1Scheme := runtime.NewScheme()
+	fakeCorev1Scheme.AddKnownTypes(fakeCorev1GroupVersion, &metav1.DeleteOptions{})
+	fakeCorev1Codec := serializer.NewCodecFactory(fakeCorev1Scheme)
+
+	tests := []struct {
+		name         string
+		codecFactory serializer.CodecFactory
+		data         []byte
+		expectErr    string
+	}{
+		//  for issue: https://github.com/kubernetes/kubernetes/issues/111985
+		{
+			name:         "decode '{}' to metav1.DeleteOptions with fakeCorev1Codecs",
+			codecFactory: fakeCorev1Codec,
+			data:         []byte("{}"),
+			expectErr:    "no kind \"DeleteOptions\" is registered",
+		},
+		{
+			name:         "decode '{}' to metav1.DeleteOptions with metainternalversionscheme.Codecs",
+			codecFactory: metainternalversionscheme.Codecs,
+			data:         []byte("{}"),
+			expectErr:    "",
+		},
+		{
+			name:         "decode versioned (corev1) DeleteOptions with metainternalversionscheme.Codecs",
+			codecFactory: metainternalversionscheme.Codecs,
+			data:         []byte(`{"apiVersion":"v1","kind":"DeleteOptions","gracePeriodSeconds":123}`),
+			expectErr:    "",
+		},
+		{
+			name:         "decode versioned (foo) DeleteOptions with metainternalversionscheme.Codecs",
+			codecFactory: metainternalversionscheme.Codecs,
+			data:         []byte(`{"apiVersion":"foo/v1","kind":"DeleteOptions","gracePeriodSeconds":123}`),
+			expectErr:    "",
+		},
+	}
+
+	defaultGVK := metav1.SchemeGroupVersion.WithKind("DeleteOptions")
+	for _, test := range tests {
+		t.Run(test.name, func(t *testing.T) {
+			s, err := negotiation.NegotiateInputSerializer(req, false, test.codecFactory)
+			if err != nil {
+				t.Fatal(err)
+			}
+
+			options := &metav1.DeleteOptions{}
+			_, _, err = metainternalversionscheme.Codecs.DecoderToVersion(s.Serializer, defaultGVK.GroupVersion()).Decode(test.data, &defaultGVK, options)
+			if test.expectErr != "" {
+				if err == nil {
+					t.Fatalf("expect %s but got nil", test.expectErr)
+				}
+				if !strings.Contains(err.Error(), test.expectErr) {
+					t.Fatalf("expect %s but got %s", test.expectErr, err.Error())
+				}
+			}
+		})
+	}
+}
diff --git a/staging/src/k8s.io/apiserver/pkg/endpoints/handlers/metrics/metrics.go b/staging/src/k8s.io/apiserver/pkg/endpoints/handlers/metrics/metrics.go
@@ -24,10 +24,11 @@ import (
 type RequestBodyVerb string
 
 const (
-	Patch  RequestBodyVerb = "patch"
-	Delete RequestBodyVerb = "delete"
-	Update RequestBodyVerb = "update"
-	Create RequestBodyVerb = "create"
+	Patch            RequestBodyVerb = "patch"
+	Delete           RequestBodyVerb = "delete"
+	Update           RequestBodyVerb = "update"
+	Create           RequestBodyVerb = "create"
+	DeleteCollection RequestBodyVerb = "delete_collection"
 )
 
 var (