From 59629acf470cee79f2dd91ee5be10712c122b855 Mon Sep 17 00:00:00 2001 From: Alexander Zielenski <351783+alexzielenski@users.noreply.github.com> Date: Tue, 26 Jul 2022 09:43:09 -0700 Subject: [PATCH 1/2] correct OpenAPI extension in error message when attemping to use preserveUnknownFields the error message leads you astray --- .../pkg/apis/apiextensions/validation/validation.go | 2 +- .../apiserver/schema/cel/celcoststability_test.go | 2 +- .../pkg/apiserver/schema/cel/validation_test.go | 4 ++-- .../pkg/apiserver/schema/cel/values.go | 2 +- test/e2e/apimachinery/crd_publish_openapi.go | 12 ++++++------ 5 files changed, 11 insertions(+), 11 deletions(-) diff --git a/staging/src/k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/validation/validation.go b/staging/src/k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/validation/validation.go index b97493d040b..c557d2914ef 100644 --- a/staging/src/k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/validation/validation.go +++ b/staging/src/k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/validation/validation.go @@ -1681,7 +1681,7 @@ func validatePreserveUnknownFields(crd, oldCRD *apiextensions.CustomResourceDefi var errs field.ErrorList if crd != nil && crd.Spec.PreserveUnknownFields != nil && *crd.Spec.PreserveUnknownFields { // disallow changing spec.preserveUnknownFields=false to spec.preserveUnknownFields=true - errs = append(errs, field.Invalid(field.NewPath("spec").Child("preserveUnknownFields"), crd.Spec.PreserveUnknownFields, "cannot set to true, set x-preserve-unknown-fields to true in spec.versions[*].schema instead")) + errs = append(errs, field.Invalid(field.NewPath("spec").Child("preserveUnknownFields"), crd.Spec.PreserveUnknownFields, "cannot set to true, set x-kubernetes-preserve-unknown-fields to true in spec.versions[*].schema instead")) } return errs } diff --git a/staging/src/k8s.io/apiextensions-apiserver/pkg/apiserver/schema/cel/celcoststability_test.go b/staging/src/k8s.io/apiextensions-apiserver/pkg/apiserver/schema/cel/celcoststability_test.go index 20a38d4171b..68ff70de870 100644 --- a/staging/src/k8s.io/apiextensions-apiserver/pkg/apiserver/schema/cel/celcoststability_test.go +++ b/staging/src/k8s.io/apiextensions-apiserver/pkg/apiserver/schema/cel/celcoststability_test.go @@ -608,7 +608,7 @@ func TestCelCostStability(t *testing.T) { }), expectCost: map[string]int64{ // 'kind', 'apiVersion', 'metadata.name' and 'metadata.generateName' are always accessible - // even if not specified in the schema, regardless of if x-preserve-unknown-fields is set. + // even if not specified in the schema, regardless of if x-kubernetes-preserve-unknown-fields is set. "self.embedded.kind == 'Pod'": 4, "self.embedded.apiVersion == 'v1'": 4, "self.embedded.metadata.name == 'foo'": 5, diff --git a/staging/src/k8s.io/apiextensions-apiserver/pkg/apiserver/schema/cel/validation_test.go b/staging/src/k8s.io/apiextensions-apiserver/pkg/apiserver/schema/cel/validation_test.go index cd38bdcf812..07c272e3b6e 100644 --- a/staging/src/k8s.io/apiextensions-apiserver/pkg/apiserver/schema/cel/validation_test.go +++ b/staging/src/k8s.io/apiextensions-apiserver/pkg/apiserver/schema/cel/validation_test.go @@ -772,7 +772,7 @@ func TestValidationExpressions(t *testing.T) { }), valid: []string{ // 'kind', 'apiVersion', 'metadata.name' and 'metadata.generateName' are always accessible - // even if not specified in the schema, regardless of if x-preserve-unknown-fields is set. + // even if not specified in the schema, regardless of if x-kubernetes-preserve-unknown-fields is set. "self.embedded.kind == 'Pod'", "self.embedded.apiVersion == 'v1'", "self.embedded.metadata.name == 'foo'", @@ -782,7 +782,7 @@ func TestValidationExpressions(t *testing.T) { "has(self.embedded)", }, // only field declared in the schema can be field selected in CEL expressions, regardless of if - // x-preserve-unknown-fields is set. + // x-kubernetes-preserve-unknown-fields is set. errors: map[string]string{ "has(self.embedded.spec)": "undefined field 'spec'", }, diff --git a/staging/src/k8s.io/apiextensions-apiserver/pkg/apiserver/schema/cel/values.go b/staging/src/k8s.io/apiextensions-apiserver/pkg/apiserver/schema/cel/values.go index 4fa90ab106f..f012ffdc45a 100644 --- a/staging/src/k8s.io/apiextensions-apiserver/pkg/apiserver/schema/cel/values.go +++ b/staging/src/k8s.io/apiextensions-apiserver/pkg/apiserver/schema/cel/values.go @@ -84,7 +84,7 @@ func UnstructuredToVal(unstructured interface{}, schema *structuralschema.Struct }, } } - // A object with x-preserve-unknown-fields but no properties or additionalProperties is treated + // A object with x-kubernetes-preserve-unknown-fields but no properties or additionalProperties is treated // as an empty object. if schema.XPreserveUnknownFields { return &unstructuredMap{ diff --git a/test/e2e/apimachinery/crd_publish_openapi.go b/test/e2e/apimachinery/crd_publish_openapi.go index 6109523f636..247b0b73966 100644 --- a/test/e2e/apimachinery/crd_publish_openapi.go +++ b/test/e2e/apimachinery/crd_publish_openapi.go @@ -145,8 +145,8 @@ var _ = SIGDescribe("CustomResourcePublishOpenAPI [Privileged:ClusterAdmin]", fu /* Release: v1.16 - Testname: Custom Resource OpenAPI Publish, with x-preserve-unknown-fields in object - Description: Register a custom resource definition with x-preserve-unknown-fields in the top level object. + Testname: Custom Resource OpenAPI Publish, with x-kubernetes-preserve-unknown-fields in object + Description: Register a custom resource definition with x-kubernetes-preserve-unknown-fields in the top level object. Attempt to create and apply a change a custom resource, via kubectl; kubectl validation MUST accept unknown properties. Attempt kubectl explain; the output MUST contain a valid DESCRIPTION stanza. */ @@ -186,8 +186,8 @@ var _ = SIGDescribe("CustomResourcePublishOpenAPI [Privileged:ClusterAdmin]", fu /* Release: v1.16 - Testname: Custom Resource OpenAPI Publish, with x-preserve-unknown-fields at root - Description: Register a custom resource definition with x-preserve-unknown-fields in the schema root. + Testname: Custom Resource OpenAPI Publish, with x-kubernetes-preserve-unknown-fields at root + Description: Register a custom resource definition with x-kubernetes-preserve-unknown-fields in the schema root. Attempt to create and apply a change a custom resource, via kubectl; kubectl validation MUST accept unknown properties. Attempt kubectl explain; the output MUST show the custom resource KIND. */ @@ -227,8 +227,8 @@ var _ = SIGDescribe("CustomResourcePublishOpenAPI [Privileged:ClusterAdmin]", fu /* Release: v1.16 - Testname: Custom Resource OpenAPI Publish, with x-preserve-unknown-fields in embedded object - Description: Register a custom resource definition with x-preserve-unknown-fields in an embedded object. + Testname: Custom Resource OpenAPI Publish, with x-kubernetes-preserve-unknown-fields in embedded object + Description: Register a custom resource definition with x-kubernetes-preserve-unknown-fields in an embedded object. Attempt to create and apply a change a custom resource, via kubectl; kubectl validation MUST accept unknown properties. Attempt kubectl explain; the output MUST show that x-preserve-unknown-properties is used on the nested field. From a51d4c1fee7e802103416b6bb323c785917eca5c Mon Sep 17 00:00:00 2001 From: Alexander Zielenski <351783+alexzielenski@users.noreply.github.com> Date: Tue, 26 Jul 2022 11:05:46 -0700 Subject: [PATCH 2/2] update conformance x-preserve-unknown-fields becomes x-kubernetes-preserve-unknown-fields --- test/conformance/testdata/conformance.yaml | 16 +++++++++------- 1 file changed, 9 insertions(+), 7 deletions(-) diff --git a/test/conformance/testdata/conformance.yaml b/test/conformance/testdata/conformance.yaml index db7e5cf97eb..55970941509 100755 --- a/test/conformance/testdata/conformance.yaml +++ b/test/conformance/testdata/conformance.yaml @@ -246,20 +246,21 @@ update to reflect the rename. release: v1.16 file: test/e2e/apimachinery/crd_publish_openapi.go -- testname: Custom Resource OpenAPI Publish, with x-preserve-unknown-fields at root +- testname: Custom Resource OpenAPI Publish, with x-kubernetes-preserve-unknown-fields + at root codename: '[sig-api-machinery] CustomResourcePublishOpenAPI [Privileged:ClusterAdmin] works for CRD preserving unknown fields at the schema root [Conformance]' - description: Register a custom resource definition with x-preserve-unknown-fields + description: Register a custom resource definition with x-kubernetes-preserve-unknown-fields in the schema root. Attempt to create and apply a change a custom resource, via kubectl; kubectl validation MUST accept unknown properties. Attempt kubectl explain; the output MUST show the custom resource KIND. release: v1.16 file: test/e2e/apimachinery/crd_publish_openapi.go -- testname: Custom Resource OpenAPI Publish, with x-preserve-unknown-fields in embedded - object +- testname: Custom Resource OpenAPI Publish, with x-kubernetes-preserve-unknown-fields + in embedded object codename: '[sig-api-machinery] CustomResourcePublishOpenAPI [Privileged:ClusterAdmin] works for CRD preserving unknown fields in an embedded object [Conformance]' - description: Register a custom resource definition with x-preserve-unknown-fields + description: Register a custom resource definition with x-kubernetes-preserve-unknown-fields in an embedded object. Attempt to create and apply a change a custom resource, via kubectl; kubectl validation MUST accept unknown properties. Attempt kubectl explain; the output MUST show that x-preserve-unknown-properties is used on the @@ -279,10 +280,11 @@ validation should be the same. release: v1.16 file: test/e2e/apimachinery/crd_publish_openapi.go -- testname: Custom Resource OpenAPI Publish, with x-preserve-unknown-fields in object +- testname: Custom Resource OpenAPI Publish, with x-kubernetes-preserve-unknown-fields + in object codename: '[sig-api-machinery] CustomResourcePublishOpenAPI [Privileged:ClusterAdmin] works for CRD without validation schema [Conformance]' - description: Register a custom resource definition with x-preserve-unknown-fields + description: Register a custom resource definition with x-kubernetes-preserve-unknown-fields in the top level object. Attempt to create and apply a change a custom resource, via kubectl; kubectl validation MUST accept unknown properties. Attempt kubectl explain; the output MUST contain a valid DESCRIPTION stanza.