Merge pull request #120368 from Jefftree/openapi-remove-openapiv2-skip-filter

remove SkipFilterSchemaForKubectlOpenAPIV2Validation
2025-11-04 04:08:16 +00:00 · 2023-09-05 16:21:11 -07:00
parent 10d257889d 8255858a7c
commit ce19650212
2 changed files with 5 additions and 198 deletions
--- a/staging/src/k8s.io/apiextensions-apiserver/pkg/controller/openapi/builder/builder.go
+++ b/staging/src/k8s.io/apiextensions-apiserver/pkg/controller/openapi/builder/builder.go
@@ -89,12 +89,6 @@ type Options struct {
 	// Convert to OpenAPI v2.
 	V2 bool

-	// Only takes effect if the flag and V2 and both set to true. If the condition is reached,
-	// publish OpenAPI V2 but skip running the spec through ToStructuralOpenAPIV2
-	// This prevents XPreserveUnknownFields:true fields from being cleared
-	// Used only by server side apply
-	SkipFilterSchemaForKubectlOpenAPIV2Validation bool
-
 	// Strip value validation.
 	StripValueValidation bool

@@ -385,14 +379,14 @@ func (b *builder) buildKubeNative(schema *structuralschema.Structural, opts Opti
 	// and forbid anything outside of apiVersion, kind and metadata. We have to fix kubectl to stop doing this, e.g. by
 	// adding additionalProperties=true support to explicitly allow additional fields.
 	// TODO: fix kubectl to understand additionalProperties=true
-	if schema == nil || ((opts.V2 && !opts.SkipFilterSchemaForKubectlOpenAPIV2Validation) && (schema.XPreserveUnknownFields || crdPreserveUnknownFields)) {
+	if schema == nil || (opts.V2 && (schema.XPreserveUnknownFields || crdPreserveUnknownFields)) {
 		ret = &spec.Schema{
 			SchemaProps: spec.SchemaProps{Type: []string{"object"}},
 		}
 		// no, we cannot add more properties here, not even TypeMeta/ObjectMeta because kubectl will complain about
 		// unknown fields for anything else.
 	} else {
-		if opts.V2 && !opts.SkipFilterSchemaForKubectlOpenAPIV2Validation {
+		if opts.V2 {
 			schema = openapiv2.ToStructuralOpenAPIV2(schema)
 		}

@@ -429,7 +423,7 @@ func addEmbeddedProperties(s *spec.Schema, opts Options) {
 		addEmbeddedProperties(s.AdditionalProperties.Schema, opts)
 	}

-	if isTrue, ok := s.VendorExtensible.Extensions.GetBool("x-kubernetes-preserve-unknown-fields"); ok && isTrue && opts.V2 && !opts.SkipFilterSchemaForKubectlOpenAPIV2Validation {
+	if isTrue, ok := s.VendorExtensible.Extensions.GetBool("x-kubernetes-preserve-unknown-fields"); ok && isTrue && opts.V2 {
 		// don't add metadata properties if we're publishing to openapi v2 and are allowing unknown fields.
 		// adding these metadata properties makes kubectl refuse to validate unknown fields.
 		return
--- 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
@@ -44,43 +44,31 @@ func TestNewBuilder(t *testing.T) {
 		wantedSchema      string
 		wantedItemsSchema string

-		v2                                            bool // produce OpenAPIv2
-		skipFilterSchemaForKubectlOpenAPIV2Validation bool // produce OpenAPIv2 without going through the ToStructuralOpenAPIV2 path
+		v2 bool // produce OpenAPIv2
 	}{
 		{
 			"nil",
 			"",
 			`{"type":"object","x-kubernetes-group-version-kind":[{"group":"bar.k8s.io","kind":"Foo","version":"v1"}]}`, `{"$ref":"#/definitions/io.k8s.bar.v1.Foo"}`,
 			true,
-			false,
 		},
 		{"with properties",
 			`{"type":"object","properties":{"spec":{"type":"object"},"status":{"type":"object"}}}`,
 			`{"type":"object","properties":{"apiVersion":{"type":"string"},"kind":{"type":"string"},"metadata":{"$ref":"#/definitions/io.k8s.apimachinery.pkg.apis.meta.v1.ObjectMeta"},"spec":{"type":"object"},"status":{"type":"object"}},"x-kubernetes-group-version-kind":[{"group":"bar.k8s.io","kind":"Foo","version":"v1"}]}`,
 			`{"$ref":"#/definitions/io.k8s.bar.v1.Foo"}`,
 			true,
-			false,
 		},
 		{"type only",
 			`{"type":"object"}`,
 			`{"type":"object","properties":{"apiVersion":{"type":"string"},"kind":{"type":"string"},"metadata":{"$ref":"#/definitions/io.k8s.apimachinery.pkg.apis.meta.v1.ObjectMeta"}},"x-kubernetes-group-version-kind":[{"group":"bar.k8s.io","kind":"Foo","version":"v1"}]}`,
 			`{"$ref":"#/definitions/io.k8s.bar.v1.Foo"}`,
 			true,
-			false,
 		},
 		{"preserve unknown at root v2",
 			`{"type":"object","x-kubernetes-preserve-unknown-fields":true}`,
 			`{"type":"object","x-kubernetes-group-version-kind":[{"group":"bar.k8s.io","kind":"Foo","version":"v1"}]}`,
 			`{"$ref":"#/definitions/io.k8s.bar.v1.Foo"}`,
 			true,
-			false,
-		},
-		{"preserve unknown at root v3",
-			`{"type":"object","x-kubernetes-preserve-unknown-fields":true}`,
-			`{"type":"object","x-kubernetes-preserve-unknown-fields":true,"properties":{"apiVersion":{"type":"string"},"kind":{"type":"string"},"metadata":{"$ref":"#/definitions/io.k8s.apimachinery.pkg.apis.meta.v1.ObjectMeta"}},"x-kubernetes-group-version-kind":[{"group":"bar.k8s.io","kind":"Foo","version":"v1"}]}`,
-			`{"$ref":"#/definitions/io.k8s.bar.v1.Foo"}`,
-			true,
-			true,
 		},
 		{"with extensions",
 			`
@@ -184,174 +172,6 @@ func TestNewBuilder(t *testing.T) {
 }`,
 			`{"$ref":"#/definitions/io.k8s.bar.v1.Foo"}`,
 			true,
-			false,
-		},
-		{"with extensions as v3 schema",
-			`
-{
-  "type":"object",
-  "properties": {
-    "int-or-string-1": {
-      "x-kubernetes-int-or-string": true,
-      "anyOf": [
-        {"type":"integer"},
-        {"type":"string"}
-      ]
-    },
-    "int-or-string-2": {
-      "x-kubernetes-int-or-string": true,
-      "allOf": [{
-        "anyOf": [
-          {"type":"integer"},
-          {"type":"string"}
-        ]
-      }, {
-        "anyOf": [
-          {"minimum": 42.0}
-        ]
-      }]
-    },
-    "int-or-string-3": {
-      "x-kubernetes-int-or-string": true,
-      "anyOf": [
-        {"type":"integer"},
-        {"type":"string"}
-      ],
-      "allOf": [{
-        "anyOf": [
-          {"minimum": 42.0}
-        ]
-      }]
-    },
-    "int-or-string-4": {
-      "x-kubernetes-int-or-string": true,
-      "anyOf": [
-        {"minimum": 42.0}
-      ]
-    },
-    "int-or-string-5": {
-      "x-kubernetes-int-or-string": true,
-      "anyOf": [
-        {"minimum": 42.0}
-      ],
-      "allOf": [
-        {"minimum": 42.0}
-      ]
-    },
-    "int-or-string-6": {
-      "x-kubernetes-int-or-string": true
-    },
-    "preserve-unknown-fields": {
-      "x-kubernetes-preserve-unknown-fields": true
-    },
-    "embedded-object": {
-      "x-kubernetes-embedded-resource": true,
-      "x-kubernetes-preserve-unknown-fields": true,
-      "type": "object"
-    }
-  }
-}`,
-			`
-{
-  "type":"object",
-  "properties": {
-    "apiVersion": {"type":"string"},
-    "kind": {"type":"string"},
-    "metadata": {"$ref":"#/definitions/io.k8s.apimachinery.pkg.apis.meta.v1.ObjectMeta"},
-    "int-or-string-1": {
-      "x-kubernetes-int-or-string": true,
-      "anyOf": [
-        {"type":"integer"},
-        {"type":"string"}
-      ]
-    },
-    "int-or-string-2": {
-      "x-kubernetes-int-or-string": true,
-      "allOf": [{
-        "anyOf": [
-          {"type":"integer"},
-          {"type":"string"}
-        ]
-      }, {
-        "anyOf": [
-          {"minimum": 42.0}
-        ]
-      }]
-    },
-    "int-or-string-3": {
-      "x-kubernetes-int-or-string": true,
-      "anyOf": [
-        {"type":"integer"},
-        {"type":"string"}
-      ],
-      "allOf": [{
-        "anyOf": [
-          {"minimum": 42.0}
-        ]
-      }]
-    },
-    "int-or-string-4": {
-      "x-kubernetes-int-or-string": true,
-      "allOf": [{
-        "anyOf": [
-          {"type":"integer"},
-          {"type":"string"}
-        ]
-      }],
-      "anyOf": [
-        {"minimum": 42.0}
-      ]
-    },
-    "int-or-string-5": {
-      "x-kubernetes-int-or-string": true,
-      "anyOf": [
-        {"minimum": 42.0}
-      ],
-      "allOf": [{
-        "anyOf": [
-          {"type":"integer"},
-          {"type":"string"}
-        ]
-      }, {
-        "minimum": 42.0
-      }]
-    },
-    "int-or-string-6": {
-      "x-kubernetes-int-or-string": true,
-      "anyOf": [
-        {"type":"integer"},
-        {"type":"string"}
-      ]
-    },
-    "preserve-unknown-fields": {
-      "x-kubernetes-preserve-unknown-fields": true
-    },
-    "embedded-object": {
-      "x-kubernetes-embedded-resource": true,
-      "x-kubernetes-preserve-unknown-fields": true,
-      "type": "object",
-      "required":["kind","apiVersion"],
-      "properties":{
-        "apiVersion":{
-          "description":"apiVersion defines the versioned schema of this representation of an object. More info: https://git.k8s.io/community/contributors/devel/sig-architecture/api-conventions.md#resources",
-          "type":"string"
-        },
-        "kind":{
-          "description":"kind is a string value representing the type of this object. In CamelCase. More info: https://git.k8s.io/community/contributors/devel/sig-architecture/api-conventions.md#types-kinds",
-          "type":"string"
-        },
-        "metadata":{
-          "description":"Standard object's metadata. More info: https://git.k8s.io/community/contributors/devel/sig-architecture/api-conventions.md#metadata",
-          "$ref":"#/definitions/io.k8s.apimachinery.pkg.apis.meta.v1.ObjectMeta"
-        }
-      }
-    }
-  },
-  "x-kubernetes-group-version-kind":[{"group":"bar.k8s.io","kind":"Foo","version":"v1"}]
-}`,
-			`{"$ref":"#/definitions/io.k8s.bar.v1.Foo"}`,
-			true,
-			true,
 		},
 	}
 	for _, tt := range tests {
@@ -391,7 +211,7 @@ func TestNewBuilder(t *testing.T) {
 					},
 					Scope: apiextensionsv1.NamespaceScoped,
 				},
-			}, "v1", schema, Options{V2: tt.v2, SkipFilterSchemaForKubectlOpenAPIV2Validation: tt.skipFilterSchemaForKubectlOpenAPIV2Validation})
+			}, "v1", schema, Options{V2: tt.v2})

 			var wantedSchema, wantedItemsSchema spec.Schema
 			if err := json.Unmarshal([]byte(tt.wantedSchema), &wantedSchema); err != nil {
@@ -614,13 +434,6 @@ func TestBuildOpenAPIV2(t *testing.T) {
 			`{"type":"object","properties":{"apiVersion":{"type":"string"},"kind":{"type":"string"},"metadata":{"$ref":"#/definitions/io.k8s.apimachinery.pkg.apis.meta.v1.ObjectMeta"},"foo":{"type":"string"}},"x-kubernetes-group-version-kind":[{"group":"bar.k8s.io","kind":"Foo","version":"v1"}]}`,
 			Options{V2: true},
 		},
-		{
-			"v3",
-			`{"type":"object","properties":{"foo":{"type":"string","oneOf":[{"pattern":"a"},{"pattern":"b"}]}}}`,
-			nil,
-			`{"type":"object","properties":{"apiVersion":{"type":"string"},"kind":{"type":"string"},"metadata":{"$ref":"#/definitions/io.k8s.apimachinery.pkg.apis.meta.v1.ObjectMeta"},"foo":{"type":"string","oneOf":[{"pattern":"a"},{"pattern":"b"}]}},"x-kubernetes-group-version-kind":[{"group":"bar.k8s.io","kind":"Foo","version":"v1"}]}`,
-			Options{V2: true, SkipFilterSchemaForKubectlOpenAPIV2Validation: true},
-		},
 	}
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {