Merge pull request #96562 from gautierdelorme/1.19-fix-objects-preser…

…ve-unknown-fields [release-1.19] Fix kubectl CRD validation with preserve-unknown-fields
kubernetes · Nov 17, 2020 · de336d8 · de336d8
2 parents 454965f + 9769357
commit de336d8
Show file tree

Hide file tree

Showing 5 changed files with 48 additions and 3 deletions.
diff --git a/staging/src/k8s.io/apiextensions-apiserver/pkg/controller/openapi/builder/builder_test.go b/staging/src/k8s.io/apiextensions-apiserver/pkg/controller/openapi/builder/builder_test.go
@@ -172,8 +172,7 @@ func TestNewBuilder(t *testing.T) {
     },
     "embedded-object": {
       "x-kubernetes-embedded-resource": true,
-      "x-kubernetes-preserve-unknown-fields": true,
-      "type":"object"
+      "x-kubernetes-preserve-unknown-fields": true
     }
   },
   "x-kubernetes-group-version-kind":[{"group":"bar.k8s.io","kind":"Foo","version":"v1"}]

diff --git a/staging/src/k8s.io/apiextensions-apiserver/pkg/controller/openapi/v2/BUILD b/staging/src/k8s.io/apiextensions-apiserver/pkg/controller/openapi/v2/BUILD
@@ -24,6 +24,7 @@ go_test(
         "//vendor/github.com/googleapis/gnostic/openapiv2:go_default_library",
         "//vendor/gopkg.in/yaml.v2:go_default_library",
         "//vendor/k8s.io/kube-openapi/pkg/util/proto:go_default_library",
+        "//vendor/k8s.io/utils/pointer:go_default_library",
     ],
 )
 

diff --git a/staging/src/k8s.io/apiextensions-apiserver/pkg/controller/openapi/v2/conversion.go b/staging/src/k8s.io/apiextensions-apiserver/pkg/controller/openapi/v2/conversion.go
@@ -73,6 +73,21 @@ func ToStructuralOpenAPIV2(in *structuralschema.Structural) *structuralschema.St
 				changed = true
 			}
 
+			if s.Items == nil && s.Type == "array" {
+				// kubectl cannot cope with array without item schema, e.g. due to XPreserveUnknownFields case above
+				// https://github.com/kubernetes/kube-openapi/blob/64514a1d5d596b96e6f957e2be275ae14d6b0804/pkg/util/proto/document.go#L185
+				s.Type = ""
+
+				changed = true
+			}
+
+			if s.XPreserveUnknownFields && s.Type == "object" {
+				// similar as above, kubectl doesn't properly handle object fields with `x-kubernetes-preserve-unknown-fields: true`
+				s.Type = ""
+
+				changed = true
+			}
+
 			for f, fs := range s.Properties {
 				if fs.Nullable {
 					s.ValueValidation.Required, changed = filterOut(s.ValueValidation.Required, f)

diff --git a/staging/src/k8s.io/apiextensions-apiserver/pkg/controller/openapi/v2/conversion_test.go b/staging/src/k8s.io/apiextensions-apiserver/pkg/controller/openapi/v2/conversion_test.go
@@ -36,6 +36,7 @@ import (
 	apiextensionsv1beta1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1beta1"
 	structuralschema "k8s.io/apiextensions-apiserver/pkg/apiserver/schema"
 	"k8s.io/kube-openapi/pkg/util/proto"
+	"k8s.io/utils/pointer"
 )
 
 func Test_ConvertJSONSchemaPropsToOpenAPIv2Schema(t *testing.T) {
@@ -643,6 +644,30 @@ func Test_ConvertJSONSchemaPropsToOpenAPIv2SchemaByType(t *testing.T) {
 				WithExample(testStr),
 			expectDiff: true,
 		},
+		{
+			name: "preserve-unknown-fields in arrays",
+			in: &apiextensions.JSONSchemaProps{
+				XPreserveUnknownFields: pointer.BoolPtr(true),
+				Type:                   "array",
+				Items: &apiextensions.JSONSchemaPropsOrArray{Schema: &apiextensions.JSONSchemaProps{
+					Type: "string",
+				}},
+			},
+			expected: withVendorExtensions(new(spec.Schema), "x-kubernetes-preserve-unknown-fields", true),
+		},
+		{
+			name: "preserve-unknown-fields in objects",
+			in: &apiextensions.JSONSchemaProps{
+				XPreserveUnknownFields: pointer.BoolPtr(true),
+				Type:                   "object",
+				Properties: map[string]apiextensions.JSONSchemaProps{
+					"foo": {
+						Type: "string",
+					},
+				},
+			},
+			expected: withVendorExtensions(new(spec.Schema), "x-kubernetes-preserve-unknown-fields", true),
+		},
 	}
 
 	for _, test := range tests {
@@ -666,6 +691,11 @@ func Test_ConvertJSONSchemaPropsToOpenAPIv2SchemaByType(t *testing.T) {
 	}
 }
 
+func withVendorExtensions(s *spec.Schema, key string, value interface{}) *spec.Schema {
+	s.VendorExtensible.AddExtension(key, value)
+	return s
+}
+
 func refEqual(x spec.Ref, y spec.Ref) bool {
 	return x.String() == y.String()
 }

diff --git a/test/e2e/apimachinery/crd_publish_openapi.go b/test/e2e/apimachinery/crd_publish_openapi.go
@@ -230,7 +230,7 @@ var _ = SIGDescribe("CustomResourcePublishOpenAPI [Privileged:ClusterAdmin]", fu
 		ns := fmt.Sprintf("--namespace=%v", f.Namespace.Name)
 
 		ginkgo.By("client-side validation (kubectl create and apply) allows request with any unknown properties")
-		randomCR := fmt.Sprintf(`{%s,"spec":{"b":[{"c":"d"}]}}`, meta)
+		randomCR := fmt.Sprintf(`{%s,"spec":{"a":null,"b":[{"c":"d"}]}}`, meta)
 		if _, err := framework.RunKubectlInput(f.Namespace.Name, randomCR, ns, "create", "-f", "-"); err != nil {
 			framework.Failf("failed to create random CR %s for CRD that allows unknown properties in a nested object: %v", randomCR, err)
 		}