From e2fd72ca845d7e19e577ed3b0999b002622d0c67 Mon Sep 17 00:00:00 2001 From: "Dr. Stefan Schimanski" Date: Sun, 9 Jun 2019 10:34:41 +0200 Subject: [PATCH] apiextensions: do not check for pruned defaults under metadata --- .../apiextensions/validation/validation.go | 95 ++++++-- .../validation/validation_test.go | 207 ++++++++++++++++++ .../apiserver/schema/objectmeta/algorithm.go | 2 +- .../pkg/apiserver/schema/validation.go | 1 + .../test/integration/objectmeta_test.go | 109 +++++++-- 5 files changed, 371 insertions(+), 43 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 aa5b519ae52..d9ea00b6f4b 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 @@ -566,6 +566,11 @@ func ValidateCustomResourceColumnDefinition(col *apiextensions.CustomResourceCol // specStandardValidator applies validations for different OpenAPI specification versions. type specStandardValidator interface { validate(spec *apiextensions.JSONSchemaProps, fldPath *field.Path) field.ErrorList + withForbiddenDefaults(reason string) specStandardValidator + + // insideResourceMeta returns true when validating either TypeMeta or ObjectMeta, from an embedded resource or on the top-level. + insideResourceMeta() bool + withInsideResourceMeta() specStandardValidator } // ValidateCustomResourceDefinitionValidation statically validates @@ -612,7 +617,7 @@ func ValidateCustomResourceDefinitionValidation(customResourceValidation *apiext openAPIV3Schema := &specStandardValidatorV3{ allowDefaults: allowDefaults, } - allErrs = append(allErrs, ValidateCustomResourceDefinitionOpenAPISchema(schema, fldPath.Child("openAPIV3Schema"), openAPIV3Schema)...) + allErrs = append(allErrs, ValidateCustomResourceDefinitionOpenAPISchema(schema, fldPath.Child("openAPIV3Schema"), openAPIV3Schema, true)...) if mustBeStructural { if ss, err := structuralschema.NewStructural(schema); err != nil { @@ -636,7 +641,7 @@ func ValidateCustomResourceDefinitionValidation(customResourceValidation *apiext } // ValidateCustomResourceDefinitionOpenAPISchema statically validates -func ValidateCustomResourceDefinitionOpenAPISchema(schema *apiextensions.JSONSchemaProps, fldPath *field.Path, ssv specStandardValidator) field.ErrorList { +func ValidateCustomResourceDefinitionOpenAPISchema(schema *apiextensions.JSONSchemaProps, fldPath *field.Path, ssv specStandardValidator, isRoot bool) field.ErrorList { allErrs := field.ErrorList{} if schema == nil { @@ -664,53 +669,68 @@ func ValidateCustomResourceDefinitionOpenAPISchema(schema *apiextensions.JSONSch allErrs = append(allErrs, field.Forbidden(fldPath.Child("additionalProperties"), "additionalProperties and properties are mutual exclusive")) } } - allErrs = append(allErrs, ValidateCustomResourceDefinitionOpenAPISchema(schema.AdditionalProperties.Schema, fldPath.Child("additionalProperties"), ssv)...) + // Note: we forbid additionalProperties at resource root, both embedded and top-level. + // But further inside, additionalProperites is possible, e.g. for labels or annotations. + subSsv := ssv + if ssv.insideResourceMeta() { + // we have to forbid defaults inside additionalProperties because pruning without actual value is ambiguous + subSsv = ssv.withForbiddenDefaults("inside additionalProperties applying to object metadata") + } + allErrs = append(allErrs, ValidateCustomResourceDefinitionOpenAPISchema(schema.AdditionalProperties.Schema, fldPath.Child("additionalProperties"), subSsv, false)...) } if len(schema.Properties) != 0 { for property, jsonSchema := range schema.Properties { - allErrs = append(allErrs, ValidateCustomResourceDefinitionOpenAPISchema(&jsonSchema, fldPath.Child("properties").Key(property), ssv)...) + subSsv := ssv + if (isRoot || schema.XEmbeddedResource) && property == "metadata" { + // we recurse into the schema that applies to ObjectMeta. + subSsv = ssv.withInsideResourceMeta() + if isRoot { + subSsv = subSsv.withForbiddenDefaults(fmt.Sprintf("in top-level %s", property)) + } + } + allErrs = append(allErrs, ValidateCustomResourceDefinitionOpenAPISchema(&jsonSchema, fldPath.Child("properties").Key(property), subSsv, false)...) } } - allErrs = append(allErrs, ValidateCustomResourceDefinitionOpenAPISchema(schema.Not, fldPath.Child("not"), ssv)...) + allErrs = append(allErrs, ValidateCustomResourceDefinitionOpenAPISchema(schema.Not, fldPath.Child("not"), ssv, false)...) if len(schema.AllOf) != 0 { for i, jsonSchema := range schema.AllOf { - allErrs = append(allErrs, ValidateCustomResourceDefinitionOpenAPISchema(&jsonSchema, fldPath.Child("allOf").Index(i), ssv)...) + allErrs = append(allErrs, ValidateCustomResourceDefinitionOpenAPISchema(&jsonSchema, fldPath.Child("allOf").Index(i), ssv, false)...) } } if len(schema.OneOf) != 0 { for i, jsonSchema := range schema.OneOf { - allErrs = append(allErrs, ValidateCustomResourceDefinitionOpenAPISchema(&jsonSchema, fldPath.Child("oneOf").Index(i), ssv)...) + allErrs = append(allErrs, ValidateCustomResourceDefinitionOpenAPISchema(&jsonSchema, fldPath.Child("oneOf").Index(i), ssv, false)...) } } if len(schema.AnyOf) != 0 { for i, jsonSchema := range schema.AnyOf { - allErrs = append(allErrs, ValidateCustomResourceDefinitionOpenAPISchema(&jsonSchema, fldPath.Child("anyOf").Index(i), ssv)...) + allErrs = append(allErrs, ValidateCustomResourceDefinitionOpenAPISchema(&jsonSchema, fldPath.Child("anyOf").Index(i), ssv, false)...) } } if len(schema.Definitions) != 0 { for definition, jsonSchema := range schema.Definitions { - allErrs = append(allErrs, ValidateCustomResourceDefinitionOpenAPISchema(&jsonSchema, fldPath.Child("definitions").Key(definition), ssv)...) + allErrs = append(allErrs, ValidateCustomResourceDefinitionOpenAPISchema(&jsonSchema, fldPath.Child("definitions").Key(definition), ssv, false)...) } } if schema.Items != nil { - allErrs = append(allErrs, ValidateCustomResourceDefinitionOpenAPISchema(schema.Items.Schema, fldPath.Child("items"), ssv)...) + allErrs = append(allErrs, ValidateCustomResourceDefinitionOpenAPISchema(schema.Items.Schema, fldPath.Child("items"), ssv, false)...) if len(schema.Items.JSONSchemas) != 0 { for i, jsonSchema := range schema.Items.JSONSchemas { - allErrs = append(allErrs, ValidateCustomResourceDefinitionOpenAPISchema(&jsonSchema, fldPath.Child("items").Index(i), ssv)...) + allErrs = append(allErrs, ValidateCustomResourceDefinitionOpenAPISchema(&jsonSchema, fldPath.Child("items").Index(i), ssv, false)...) } } } if schema.Dependencies != nil { for dependency, jsonSchemaPropsOrStringArray := range schema.Dependencies { - allErrs = append(allErrs, ValidateCustomResourceDefinitionOpenAPISchema(jsonSchemaPropsOrStringArray.Schema, fldPath.Child("dependencies").Key(dependency), ssv)...) + allErrs = append(allErrs, ValidateCustomResourceDefinitionOpenAPISchema(jsonSchemaPropsOrStringArray.Schema, fldPath.Child("dependencies").Key(dependency), ssv, false)...) } } @@ -722,7 +742,26 @@ func ValidateCustomResourceDefinitionOpenAPISchema(schema *apiextensions.JSONSch } type specStandardValidatorV3 struct { - allowDefaults bool + allowDefaults bool + disallowDefaultsReason string + isInsideResourceMeta bool +} + +func (v *specStandardValidatorV3) withForbiddenDefaults(reason string) specStandardValidator { + clone := *v + clone.disallowDefaultsReason = reason + clone.allowDefaults = false + return &clone +} + +func (v *specStandardValidatorV3) withInsideResourceMeta() specStandardValidator { + clone := *v + clone.isInsideResourceMeta = true + return &clone +} + +func (v *specStandardValidatorV3) insideResourceMeta() bool { + return v.isInsideResourceMeta } // validate validates against OpenAPI Schema v3. @@ -741,23 +780,37 @@ func (v *specStandardValidatorV3) validate(schema *apiextensions.JSONSchemaProps if v.allowDefaults { if s, err := structuralschema.NewStructural(schema); err == nil { // ignore errors here locally. They will show up for the root of the schema. - pruned := runtime.DeepCopyJSONValue(interface{}(*schema.Default)) - pruning.Prune(pruned, s, false) - if err := schemaobjectmeta.Coerce(fldPath, pruned, s, false, false); err != nil { + + clone := runtime.DeepCopyJSONValue(interface{}(*schema.Default)) + if !v.isInsideResourceMeta || s.XEmbeddedResource { + pruning.Prune(clone, s, s.XEmbeddedResource) + // If we are under metadata, there are implicitly specified fields like kind, apiVersion, metadata, labels. + // We cannot prune as they are pruned as well. This allows more defaults than we would like to. + // TODO: be precise about pruning under metadata + } + // TODO: coerce correctly if we are not at the object root, but somewhere below. + if err := schemaobjectmeta.Coerce(fldPath, clone, s, s.XEmbeddedResource, false); err != nil { allErrs = append(allErrs, err) } - if !reflect.DeepEqual(pruned, *schema.Default) { - allErrs = append(allErrs, field.Invalid(fldPath.Child("default"), schema.Default, "must not have unspecified fields")) + if !reflect.DeepEqual(clone, interface{}(*schema.Default)) { + allErrs = append(allErrs, field.Invalid(fldPath.Child("default"), schema.Default, "must not have unknown fields")) + } else if s.XEmbeddedResource { + // validate an embedded resource + schemaobjectmeta.Validate(fldPath, interface{}(*schema.Default), nil, true) } - // validate the default value. Only validating and pruned defaults are allowed. + // validate the default value with user the provided schema. validator := govalidate.NewSchemaValidator(s.ToGoOpenAPI(), nil, "", strfmt.Default) - if err := apiservervalidation.ValidateCustomResource(pruned, validator); err != nil { + if err := apiservervalidation.ValidateCustomResource(interface{}(*schema.Default), validator); err != nil { allErrs = append(allErrs, field.Invalid(fldPath.Child("default"), schema.Default, fmt.Sprintf("must validate: %v", err))) } } } else { - allErrs = append(allErrs, field.Forbidden(fldPath.Child("default"), "must not be set")) + detail := "must not be set" + if len(v.disallowDefaultsReason) > 0 { + detail += " " + v.disallowDefaultsReason + } + allErrs = append(allErrs, field.Forbidden(fldPath.Child("default"), detail)) } } diff --git a/staging/src/k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/validation/validation_test.go b/staging/src/k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/validation/validation_test.go index 655a377e461..8808b04c1ec 100644 --- a/staging/src/k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/validation/validation_test.go +++ b/staging/src/k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/validation/validation_test.go @@ -1956,6 +1956,213 @@ func TestValidateCustomResourceDefinition(t *testing.T) { }, enabledFeatures: []featuregate.Feature{features.CustomResourceDefaulting}, }, + { + name: "metadata defaults", + resource: &apiextensions.CustomResourceDefinition{ + ObjectMeta: metav1.ObjectMeta{Name: "plural.group.com"}, + Spec: apiextensions.CustomResourceDefinitionSpec{ + Group: "group.com", + Version: "v1", + Versions: []apiextensions.CustomResourceDefinitionVersion{ + { + Name: "v1", + Served: true, + Storage: true, + Schema: &apiextensions.CustomResourceValidation{ + OpenAPIV3Schema: &apiextensions.JSONSchemaProps{ + Type: "object", + Properties: map[string]apiextensions.JSONSchemaProps{ + "metadata": { + Type: "object", + // forbidden: no default for top-level metadata + Default: jsonPtr(map[string]interface{}{ + "name": "foo", + }), + }, + "embedded": { + Type: "object", + XEmbeddedResource: true, + Properties: map[string]apiextensions.JSONSchemaProps{ + "metadata": { + Type: "object", + Default: jsonPtr(map[string]interface{}{ + "name": "foo", + // TODO: forbid unknown field under metadata + "unknown": int64(42), + }), + }, + }, + }, + }, + }, + }, + }, + { + Name: "v2", + Served: true, + Storage: false, + Schema: &apiextensions.CustomResourceValidation{ + OpenAPIV3Schema: &apiextensions.JSONSchemaProps{ + Type: "object", + Properties: map[string]apiextensions.JSONSchemaProps{ + "metadata": { + Type: "object", + Properties: map[string]apiextensions.JSONSchemaProps{ + "name": { + Type: "string", + // forbidden: no default in top-level metadata + Default: jsonPtr("foo"), + }, + }, + }, + "embedded": { + Type: "object", + XEmbeddedResource: true, + Properties: map[string]apiextensions.JSONSchemaProps{ + "apiVersion": { + Type: "string", + Default: jsonPtr("v1"), + }, + "kind": { + Type: "string", + Default: jsonPtr("Pod"), + }, + "metadata": { + Type: "object", + Properties: map[string]apiextensions.JSONSchemaProps{ + "name": { + Type: "string", + Default: jsonPtr("foo"), + }, + }, + }, + }, + }, + }, + }, + }, + }, + { + Name: "v3", + Served: true, + Storage: false, + Schema: &apiextensions.CustomResourceValidation{ + OpenAPIV3Schema: &apiextensions.JSONSchemaProps{ + Type: "object", + Properties: map[string]apiextensions.JSONSchemaProps{ + "embedded": { + Type: "object", + XEmbeddedResource: true, + Properties: map[string]apiextensions.JSONSchemaProps{ + "apiVersion": { + Type: "string", + Default: jsonPtr("v1"), + }, + "kind": { + Type: "string", + // TODO: forbid non-validating nested values in metadata + Default: jsonPtr("%"), + }, + "metadata": { + Type: "object", + Default: jsonPtr(map[string]interface{}{ + "labels": map[string]interface{}{ + // TODO: forbid non-validating nested field in meta + "bar": "x y", + }, + }), + }, + }, + }, + }, + }, + }, + }, + { + Name: "v4", + Served: true, + Storage: false, + Schema: &apiextensions.CustomResourceValidation{ + OpenAPIV3Schema: &apiextensions.JSONSchemaProps{ + Type: "object", + Properties: map[string]apiextensions.JSONSchemaProps{ + "embedded": { + Type: "object", + XEmbeddedResource: true, + Properties: map[string]apiextensions.JSONSchemaProps{ + "metadata": { + Type: "object", + Properties: map[string]apiextensions.JSONSchemaProps{ + "name": { + Type: "string", + // TODO: forbid wrongly typed nested fields in metadata + Default: jsonPtr("%"), + }, + "labels": { + Type: "object", + Properties: map[string]apiextensions.JSONSchemaProps{ + "bar": { + Type: "string", + // TODO: forbid non-validating nested fields in metadata + Default: jsonPtr("x y"), + }, + }, + }, + "annotations": { + Type: "object", + AdditionalProperties: &apiextensions.JSONSchemaPropsOrBool{ + Schema: &apiextensions.JSONSchemaProps{ + Type: "string", + // forbidden: no default under additionalProperties inside of metadata + Default: jsonPtr("abc"), + }, + }, + }, + }, + }, + }, + }, + }, + }, + }, + }, + }, + Scope: apiextensions.NamespaceScoped, + Names: apiextensions.CustomResourceDefinitionNames{ + Plural: "plural", + Singular: "singular", + Kind: "Plural", + ListKind: "PluralList", + }, + PreserveUnknownFields: pointer.BoolPtr(false), + }, + Status: apiextensions.CustomResourceDefinitionStatus{ + StoredVersions: []string{"v1"}, + }, + }, + errors: []validationMatch{ + // Forbidden: must not be set in top-level metadata + forbidden("spec", "versions[0]", "schema", "openAPIV3Schema", "properties[metadata]", "default"), + // Invalid value: map[string]interface {}{"name":"foo", "unknown":42}: must not have unknown fields + // TODO: invalid("spec", "versions[0]", "schema", "openAPIV3Schema", "properties[embedded]", "properties[metadata]", "default"), + + // Forbidden: must not be set in top-level metadata + forbidden("spec", "versions[1]", "schema", "openAPIV3Schema", "properties[metadata]", "properties[name]", "default"), + + // Invalid value: "x y" + // TODO: invalid("spec", "versions[2]", "schema", "openAPIV3Schema", "properties[embedded]", "properties[metadata]", "default"), + // Invalid value: "%": kind: Invalid value: "%" + // TODO: invalid("spec", "versions[2]", "schema", "openAPIV3Schema", "properties[embedded]", "properties[kind]", "default"), + + // Invalid value: "%" + // TODO: invalid("spec", "versions[3]", "schema", "openAPIV3Schema", "properties[embedded]", "properties[metadata]", "properties[labels]", "properties[bar]", "default"), + // Invalid value: "x y" + // TODO: invalid("spec", "versions[3]", "schema", "openAPIV3Schema", "properties[embedded]", "properties[metadata]", "properties[name]", "default"), + // Forbidden: must not be set inside additionalProperties applying to object metadata + forbidden("spec", "versions[3]", "schema", "openAPIV3Schema", "properties[embedded]", "properties[metadata]", "properties[annotations]", "additionalProperties", "default"), + }, + enabledFeatures: []featuregate.Feature{features.CustomResourceDefaulting}, + }, { name: "contradicting meta field types", resource: &apiextensions.CustomResourceDefinition{ diff --git a/staging/src/k8s.io/apiextensions-apiserver/pkg/apiserver/schema/objectmeta/algorithm.go b/staging/src/k8s.io/apiextensions-apiserver/pkg/apiserver/schema/objectmeta/algorithm.go index ab7c73c98ee..fd3ff23d703 100644 --- a/staging/src/k8s.io/apiextensions-apiserver/pkg/apiserver/schema/objectmeta/algorithm.go +++ b/staging/src/k8s.io/apiextensions-apiserver/pkg/apiserver/schema/objectmeta/algorithm.go @@ -55,7 +55,7 @@ func (c *coercer) coerce(pth *field.Path, x interface{}, s *structuralschema.Str if _, ok := v.(string); !ok && c.dropInvalidFields { delete(x, k) } else if !ok { - return field.Invalid(pth, v, "must be a string") + return field.Invalid(pth.Child(k), v, "must be a string") } case "metadata": meta, found, err := GetObjectMeta(x, c.dropInvalidFields) diff --git a/staging/src/k8s.io/apiextensions-apiserver/pkg/apiserver/schema/validation.go b/staging/src/k8s.io/apiextensions-apiserver/pkg/apiserver/schema/validation.go index 39ef89dbf06..54206840148 100644 --- a/staging/src/k8s.io/apiextensions-apiserver/pkg/apiserver/schema/validation.go +++ b/staging/src/k8s.io/apiextensions-apiserver/pkg/apiserver/schema/validation.go @@ -160,6 +160,7 @@ func validateStructuralInvariants(s *Structural, lvl level, fldPath *field.Path) metadata.Properties = nil } metadata.Type = "" + metadata.Default.Object = nil // this is checked in API validation (and also tested) if metadata.ValueValidation == nil { metadata.ValueValidation = &ValueValidation{} } diff --git a/staging/src/k8s.io/apiextensions-apiserver/test/integration/objectmeta_test.go b/staging/src/k8s.io/apiextensions-apiserver/test/integration/objectmeta_test.go index c5e5e27cfc1..27cd47aba86 100644 --- a/staging/src/k8s.io/apiextensions-apiserver/test/integration/objectmeta_test.go +++ b/staging/src/k8s.io/apiextensions-apiserver/test/integration/objectmeta_test.go @@ -29,6 +29,7 @@ import ( apiextensionsv1beta1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1beta1" "k8s.io/apiextensions-apiserver/pkg/client/clientset/clientset" serveroptions "k8s.io/apiextensions-apiserver/pkg/cmd/server/options" + "k8s.io/apiextensions-apiserver/pkg/features" "k8s.io/apiextensions-apiserver/test/integration/fixtures" "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" @@ -37,7 +38,9 @@ import ( "k8s.io/apimachinery/pkg/util/diff" "k8s.io/apimachinery/pkg/util/json" genericapirequest "k8s.io/apiserver/pkg/endpoints/request" + utilfeature "k8s.io/apiserver/pkg/util/feature" "k8s.io/client-go/dynamic" + featuregatetesting "k8s.io/component-base/featuregate/testing" "k8s.io/utils/pointer" ) @@ -262,7 +265,7 @@ var embeddedResourceFixture = &apiextensionsv1beta1.CustomResourceDefinition{ ListKind: "FooList", }, Scope: apiextensionsv1beta1.ClusterScoped, - PreserveUnknownFields: pointer.BoolPtr(true), + PreserveUnknownFields: pointer.BoolPtr(false), Subresources: &apiextensionsv1beta1.CustomResourceSubresources{ Status: &apiextensionsv1beta1.CustomResourceSubresourceStatus{}, }, @@ -276,22 +279,47 @@ properties: embedded: type: object x-kubernetes-embedded-resource: true + x-kubernetes-preserve-unknown-fields: true noEmbeddedObject: type: object + x-kubernetes-preserve-unknown-fields: true embeddedNested: type: object x-kubernetes-embedded-resource: true + x-kubernetes-preserve-unknown-fields: true properties: embedded: type: object x-kubernetes-embedded-resource: true + x-kubernetes-preserve-unknown-fields: true + defaults: + type: object + x-kubernetes-embedded-resource: true + x-kubernetes-preserve-unknown-fields: true + default: + apiVersion: v1 + kind: Pod + labels: + foo: bar + invalidDefaults: + type: object + properties: + embedded: + type: object + x-kubernetes-embedded-resource: true + x-kubernetes-preserve-unknown-fields: true + default: + apiVersion: "foo/v1" + kind: "%" + metadata: + labels: + foo: bar + abc: "x y" ` embeddedResourceInstance = ` kind: Foo apiVersion: tests.apiextensions.k8s.io/v1beta1 -metadata: - name: foo embedded: apiVersion: foo/v1 kind: Foo @@ -342,13 +370,16 @@ embeddedNested: kind: Foo metadata: name: foo +defaults: + apiVersion: v1 + kind: Pod + labels: + foo: bar ` wronglyTypedEmbeddedResourceInstance = ` kind: Foo apiVersion: tests.apiextensions.k8s.io/v1beta1 -metadata: - name: invalid embedded: apiVersion: foo/v1 kind: Foo @@ -360,8 +391,6 @@ embedded: invalidEmbeddedResourceInstance = ` kind: Foo apiVersion: tests.apiextensions.k8s.io/v1beta1 -metadata: - name: invalid embedded: apiVersion: foo/v1 kind: "%" @@ -377,10 +406,13 @@ embeddedNested: kind: "%" metadata: name: .. +invalidDefaults: {} ` ) func TestEmbeddedResources(t *testing.T) { + defer featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.CustomResourceDefaulting, true)() + tearDownFn, apiExtensionClient, dynamicClient, err := fixtures.StartDefaultServerWithClients(t) if err != nil { t.Fatal(err) @@ -404,6 +436,7 @@ func TestEmbeddedResources(t *testing.T) { if err := yaml.Unmarshal([]byte(embeddedResourceInstance), &foo.Object); err != nil { t.Fatal(err) } + unstructured.SetNestedField(foo.Object, "foo", "metadata", "name") foo, err = fooClient.Create(foo, metav1.CreateOptions{}) if err != nil { t.Fatalf("Unable to create CR: %v", err) @@ -421,11 +454,12 @@ func TestEmbeddedResources(t *testing.T) { } t.Logf("Trying to create wrongly typed CR") - invalid := &unstructured.Unstructured{} - if err := yaml.Unmarshal([]byte(wronglyTypedEmbeddedResourceInstance), &invalid.Object); err != nil { + wronglyTyped := &unstructured.Unstructured{} + if err := yaml.Unmarshal([]byte(wronglyTypedEmbeddedResourceInstance), &wronglyTyped.Object); err != nil { t.Fatal(err) } - _, err = fooClient.Create(invalid, metav1.CreateOptions{}) + unstructured.SetNestedField(wronglyTyped.Object, "invalid", "metadata", "name") + _, err = fooClient.Create(wronglyTyped, metav1.CreateOptions{}) if err == nil { t.Fatal("Expected creation to fail, but didn't") } @@ -440,24 +474,57 @@ func TestEmbeddedResources(t *testing.T) { } t.Logf("Trying to create invalid CR") - wronglyTyped := &unstructured.Unstructured{} - if err := yaml.Unmarshal([]byte(invalidEmbeddedResourceInstance), &wronglyTyped.Object); err != nil { + invalid := &unstructured.Unstructured{} + if err := yaml.Unmarshal([]byte(invalidEmbeddedResourceInstance), &invalid.Object); err != nil { t.Fatal(err) } - _, err = fooClient.Create(wronglyTyped, metav1.CreateOptions{}) + unstructured.SetNestedField(invalid.Object, "invalid", "metadata", "name") + unstructured.SetNestedField(invalid.Object, "x y", "metadata", "labels", "foo") + _, err = fooClient.Create(invalid, metav1.CreateOptions{}) if err == nil { t.Fatal("Expected creation to fail, but didn't") } t.Logf("Creation of invalid object failed with: %v", err) - for _, s := range []string{ - `embedded.kind: Invalid value: "%"`, - `embedded.metadata.name: Invalid value: ".."`, - `embeddedNested.kind: Invalid value: "%"`, - `embeddedNested.metadata.name: Invalid value: ".."`, - `embeddedNested.embedded.kind: Invalid value: "%"`, - `embeddedNested.embedded.metadata.name: Invalid value: ".."`, - } { + invalidErrors := []string{ + `[metadata.labels: Invalid value: "x y"`, + ` embedded.kind: Invalid value: "%"`, + ` embedded.metadata.name: Invalid value: ".."`, + ` embeddedNested.kind: Invalid value: "%"`, + ` embeddedNested.metadata.name: Invalid value: ".."`, + ` embeddedNested.embedded.kind: Invalid value: "%"`, + ` embeddedNested.embedded.metadata.name: Invalid value: ".."`, + ` invalidDefaults.embedded.kind: Invalid value: "%"`, + ` invalidDefaults.embedded.metadata.labels: Invalid value: "x y"`, + } + for _, s := range invalidErrors { + if !strings.Contains(err.Error(), s) { + t.Errorf("missing error: %s", s) + } + } + + t.Logf("Creating a valid CR and then updating it with invalid values, expecting the same errors") + valid := &unstructured.Unstructured{} + if err := yaml.Unmarshal([]byte(embeddedResourceInstance), &valid.Object); err != nil { + t.Fatal(err) + } + unstructured.SetNestedField(valid.Object, "valid", "metadata", "name") + valid, err = fooClient.Create(valid, metav1.CreateOptions{}) + if err != nil { + t.Fatalf("Unable to create CR: %v", err) + } + for k, v := range invalid.Object { + if k == "metadata" { + continue + } + valid.Object[k] = v + } + unstructured.SetNestedField(valid.Object, "x y", "metadata", "labels", "foo") + if _, err = fooClient.Update(valid, metav1.UpdateOptions{}); err == nil { + t.Fatal("Expected update error, but got none") + } + t.Logf("Update failed with: %v", err) + for _, s := range invalidErrors { if !strings.Contains(err.Error(), s) { t.Errorf("missing error: %s", s) }