diff --git a/staging/src/k8s.io/apiextensions-apiserver/pkg/controller/openapi/builder.go b/staging/src/k8s.io/apiextensions-apiserver/pkg/controller/openapi/builder.go index a239d726f5e..ba7722a73ec 100644 --- a/staging/src/k8s.io/apiextensions-apiserver/pkg/controller/openapi/builder.go +++ b/staging/src/k8s.io/apiextensions-apiserver/pkg/controller/openapi/builder.go @@ -297,7 +297,7 @@ func (b *builder) buildKubeNative(schema *structuralschema.Structural, v2 bool) // 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 { + if schema == nil || (v2 && schema.XPreserveUnknownFields) { ret = &spec.Schema{ SchemaProps: spec.SchemaProps{Type: []string{"object"}}, } @@ -311,7 +311,7 @@ func (b *builder) buildKubeNative(schema *structuralschema.Structural, v2 bool) ret.SetProperty("metadata", *spec.RefSchema(objectMetaSchemaRef). WithDescription(swaggerPartialObjectMetadataDescriptions["metadata"])) addTypeMetaProperties(ret) - addEmbeddedProperties(ret) + addEmbeddedProperties(ret, v2) } ret.AddExtension(endpoints.ROUTE_META_GVK, []interface{}{ map[string]interface{}{ @@ -324,23 +324,28 @@ func (b *builder) buildKubeNative(schema *structuralschema.Structural, v2 bool) return ret } -func addEmbeddedProperties(s *spec.Schema) { +func addEmbeddedProperties(s *spec.Schema, v2 bool) { if s == nil { return } for k := range s.Properties { v := s.Properties[k] - addEmbeddedProperties(&v) + addEmbeddedProperties(&v, v2) s.Properties[k] = v } if s.Items != nil { - addEmbeddedProperties(s.Items.Schema) + addEmbeddedProperties(s.Items.Schema, v2) } if s.AdditionalProperties != nil { - addEmbeddedProperties(s.AdditionalProperties.Schema) + addEmbeddedProperties(s.AdditionalProperties.Schema, v2) } + if isTrue, ok := s.VendorExtensible.Extensions.GetBool("x-kubernetes-preserve-unknown-fields"); ok && isTrue && 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 + } if isTrue, ok := s.VendorExtensible.Extensions.GetBool("x-kubernetes-embedded-resource"); ok && isTrue { s.SetProperty("apiVersion", withDescription(getDefinition(typeMetaType).SchemaProps.Properties["apiVersion"], "apiVersion defines the versioned schema of this representation of an object. More info: https://git.k8s.io/community/contributors/devel/api-conventions.md#resources", diff --git a/staging/src/k8s.io/apiextensions-apiserver/pkg/controller/openapi/builder_test.go b/staging/src/k8s.io/apiextensions-apiserver/pkg/controller/openapi/builder_test.go index 5c8da1bfde3..89e919cf283 100644 --- a/staging/src/k8s.io/apiextensions-apiserver/pkg/controller/openapi/builder_test.go +++ b/staging/src/k8s.io/apiextensions-apiserver/pkg/controller/openapi/builder_test.go @@ -59,6 +59,18 @@ func TestNewBuilder(t *testing.T) { `{"$ref":"#/definitions/io.k8s.bar.v1.Foo"}`, true, }, + {"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, + }, + {"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"}`, + false, + }, {"with extensions", ` { @@ -155,22 +167,7 @@ func TestNewBuilder(t *testing.T) { "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/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/api-conventions.md#types-kinds", - "type":"string" - }, - "metadata":{ - "description":"Standard object's metadata. More info: https://git.k8s.io/community/contributors/devel/api-conventions.md#metadata", - "$ref":"#/definitions/io.k8s.apimachinery.pkg.apis.meta.v1.ObjectMeta" - } - } + "type":"object" } }, "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/conversion.go b/staging/src/k8s.io/apiextensions-apiserver/pkg/controller/openapi/conversion.go index 55360ea5303..5b998e65bd7 100644 --- a/staging/src/k8s.io/apiextensions-apiserver/pkg/controller/openapi/conversion.go +++ b/staging/src/k8s.io/apiextensions-apiserver/pkg/controller/openapi/conversion.go @@ -65,6 +65,14 @@ func ToStructuralOpenAPIV2(in *structuralschema.Structural) *structuralschema.St changed = true } + if s.XPreserveUnknownFields { + // unknown fields break if items or properties are set in kubectl + s.Items = nil + s.Properties = nil + + changed = true + } + return changed }, // we drop all junctors above, and hence, never reach nested value validations diff --git a/test/e2e/apimachinery/crd_publish_openapi.go b/test/e2e/apimachinery/crd_publish_openapi.go index d08bc901772..caa28026e62 100644 --- a/test/e2e/apimachinery/crd_publish_openapi.go +++ b/test/e2e/apimachinery/crd_publish_openapi.go @@ -159,6 +159,74 @@ var _ = SIGDescribe("CustomResourcePublishOpenAPI", func() { } }) + ginkgo.It("works for CRD preserving unknown fields at the schema root", func() { + crd, err := setupCRDAndVerifySchema(f, schemaPreserveRoot, nil, "unknown-at-root", "v1") + if err != nil { + e2elog.Failf("%v", err) + } + + meta := fmt.Sprintf(metaPattern, crd.Crd.Spec.Names.Kind, crd.Crd.Spec.Group, crd.Crd.Spec.Versions[0].Name, "test-cr") + 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,"a":{"b":[{"c":"d"}]}}`, meta) + if _, err := framework.RunKubectlInput(randomCR, ns, "create", "-f", "-"); err != nil { + e2elog.Failf("failed to create random CR %s for CRD that allows unknown properties at the root: %v", randomCR, err) + } + if _, err := framework.RunKubectl(ns, "delete", crd.Crd.Spec.Names.Plural, "test-cr"); err != nil { + e2elog.Failf("failed to delete random CR: %v", err) + } + if _, err := framework.RunKubectlInput(randomCR, ns, "apply", "-f", "-"); err != nil { + e2elog.Failf("failed to apply random CR %s for CRD without schema: %v", randomCR, err) + } + if _, err := framework.RunKubectl(ns, "delete", crd.Crd.Spec.Names.Plural, "test-cr"); err != nil { + e2elog.Failf("failed to delete random CR: %v", err) + } + + ginkgo.By("kubectl explain works to explain CR") + if err := verifyKubectlExplain(crd.Crd.Spec.Names.Plural, fmt.Sprintf(`(?s)KIND:.*%s`, crd.Crd.Spec.Names.Kind)); err != nil { + e2elog.Failf("%v", err) + } + + if err := cleanupCRD(f, crd); err != nil { + e2elog.Failf("%v", err) + } + }) + + ginkgo.It("works for CRD preserving unknown fields in an embedded object", func() { + crd, err := setupCRDAndVerifySchema(f, schemaPreserveNested, nil, "unknown-in-nested", "v1") + if err != nil { + e2elog.Failf("%v", err) + } + + meta := fmt.Sprintf(metaPattern, crd.Crd.Spec.Names.Kind, crd.Crd.Spec.Group, crd.Crd.Spec.Versions[0].Name, "test-cr") + 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) + if _, err := framework.RunKubectlInput(randomCR, ns, "create", "-f", "-"); err != nil { + e2elog.Failf("failed to create random CR %s for CRD that allows unknown properties in a nested object: %v", randomCR, err) + } + if _, err := framework.RunKubectl(ns, "delete", crd.Crd.Spec.Names.Plural, "test-cr"); err != nil { + e2elog.Failf("failed to delete random CR: %v", err) + } + if _, err := framework.RunKubectlInput(randomCR, ns, "apply", "-f", "-"); err != nil { + e2elog.Failf("failed to apply random CR %s for CRD without schema: %v", randomCR, err) + } + if _, err := framework.RunKubectl(ns, "delete", crd.Crd.Spec.Names.Plural, "test-cr"); err != nil { + e2elog.Failf("failed to delete random CR: %v", err) + } + + ginkgo.By("kubectl explain works to explain CR") + if err := verifyKubectlExplain(crd.Crd.Spec.Names.Plural, `(?s)DESCRIPTION:.*preserve-unknown-properties in nested field for Testing`); err != nil { + e2elog.Failf("%v", err) + } + + if err := cleanupCRD(f, crd); err != nil { + e2elog.Failf("%v", err) + } + }) + ginkgo.It("works for multiple CRDs of different groups", func() { ginkgo.By("CRs in different groups (two CRDs) show up in OpenAPI documentation") crdFoo, err := setupCRD(f, schemaFoo, "foo", "v1") @@ -337,18 +405,23 @@ var _ = SIGDescribe("CustomResourcePublishOpenAPI", func() { }) func setupCRD(f *framework.Framework, schema []byte, groupSuffix string, versions ...string) (*crd.TestCrd, error) { + expect := schema + if schema == nil { + // to be backwards compatible, we expect CRD controller to treat + // CRD with nil schema specially and publish an empty schema + expect = []byte(`type: object`) + } + return setupCRDAndVerifySchema(f, schema, expect, groupSuffix, versions...) +} + +func setupCRDAndVerifySchema(f *framework.Framework, schema, expect []byte, groupSuffix string, versions ...string) (*crd.TestCrd, error) { group := fmt.Sprintf("%s-test-%s.k8s.io", f.BaseName, groupSuffix) if len(versions) == 0 { return nil, fmt.Errorf("require at least one version for CRD") } - expect := schema props := &v1beta1.JSONSchemaProps{} - if schema == nil { - // to be backwards compatible, we expect CRD controller to treat - // CRD with nil schema specially and publish an empty schema - expect = []byte(`type: object`) - } else { + if schema != nil { if err := yaml.Unmarshal(schema, props); err != nil { return nil, err } @@ -412,7 +485,8 @@ func mustSucceedMultipleTimes(n int, f func() (bool, error)) func() (bool, error } } -// waitForDefinition waits for given definition showing up in swagger with given schema +// waitForDefinition waits for given definition showing up in swagger with given schema. +// If schema is nil, only the existence of the given name is checked. func waitForDefinition(c k8sclientset.Interface, name string, schema []byte) error { expect := spec.Schema{} if err := convertJSONSchemaProps(schema, &expect); err != nil { @@ -424,10 +498,12 @@ func waitForDefinition(c k8sclientset.Interface, name string, schema []byte) err if !ok { return false, fmt.Sprintf("spec.SwaggerProps.Definitions[\"%s\"] not found", name) } - // drop properties and extension that we added - dropDefaults(&d) - if !apiequality.Semantic.DeepEqual(expect, d) { - return false, fmt.Sprintf("spec.SwaggerProps.Definitions[\"%s\"] not match; expect: %v, actual: %v", name, expect, d) + if schema != nil { + // drop properties and extension that we added + dropDefaults(&d) + if !apiequality.Semantic.DeepEqual(expect, d) { + return false, fmt.Sprintf("spec.SwaggerProps.Definitions[\"%s\"] not match; expect: %v, actual: %v", name, expect, d) + } } return true, "" }) @@ -602,3 +678,45 @@ properties: type: array items: type: object`) + +var schemaPreserveRoot = []byte(`description: preserve-unknown-properties at root for Testing +x-kubernetes-preserve-unknown-fields: true +type: object +properties: + spec: + description: Specification of Waldo + type: object + properties: + dummy: + description: Dummy property. + type: object + status: + description: Status of Waldo + type: object + properties: + bars: + description: List of Bars and their statuses. + type: array + items: + type: object`) + +var schemaPreserveNested = []byte(`description: preserve-unknown-properties in nested field for Testing +type: object +properties: + spec: + description: Specification of Waldo + type: object + x-kubernetes-preserve-unknown-fields: true + properties: + dummy: + description: Dummy property. + type: object + status: + description: Status of Waldo + type: object + properties: + bars: + description: List of Bars and their statuses. + type: array + items: + type: object`)