From 333f6391ed29d9b5f6eecdadcb4c2d80a7ba5608 Mon Sep 17 00:00:00 2001 From: "Dr. Stefan Schimanski" Date: Tue, 28 May 2019 12:15:52 +0200 Subject: [PATCH] apiextensions: add validation of webhook conversion & preserveUnknownFields --- .../apiextensions/validation/validation.go | 13 +- .../validation/validation_test.go | 311 +++++++++++++++++- 2 files changed, 305 insertions(+), 19 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 be134821a4c..2a0f663bdf7 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 @@ -121,10 +121,10 @@ func ValidateCustomResourceDefinitionVersion(version *apiextensions.CustomResour // ValidateCustomResourceDefinitionSpec statically validates func ValidateCustomResourceDefinitionSpec(spec *apiextensions.CustomResourceDefinitionSpec, fldPath *field.Path) field.ErrorList { allowDefaults := utilfeature.DefaultFeatureGate.Enabled(apiextensionsfeatures.CustomResourceDefaulting) - return validateCustomResourceDefinitionSpec(spec, true, allowDefaults, fldPath) + return validateCustomResourceDefinitionSpec(spec, true, allowDefaults, false, fldPath) } -func validateCustomResourceDefinitionSpec(spec *apiextensions.CustomResourceDefinitionSpec, requireRecognizedVersion, allowDefaults bool, fldPath *field.Path) field.ErrorList { +func validateCustomResourceDefinitionSpec(spec *apiextensions.CustomResourceDefinitionSpec, requireRecognizedVersion, allowDefaults, allowConversionWithPreserveUnknownFields bool, fldPath *field.Path) field.ErrorList { allErrs := field.ErrorList{} if len(spec.Group) == 0 { @@ -237,11 +237,18 @@ func validateCustomResourceDefinitionSpec(spec *apiextensions.CustomResourceDefi } } + if !allowConversionWithPreserveUnknownFields && conversionAndPreserveUnknownFields(spec) { + allErrs = append(allErrs, field.Invalid(fldPath.Child("conversion").Child("strategy"), spec.Conversion.Strategy, "must be None if spec.preserveUnknownFields is true")) + } allErrs = append(allErrs, validateCustomResourceConversion(spec.Conversion, requireRecognizedVersion, fldPath.Child("conversion"))...) return allErrs } +func conversionAndPreserveUnknownFields(spec *apiextensions.CustomResourceDefinitionSpec) bool { + return (spec.Conversion != nil && spec.Conversion.Strategy != apiextensions.NoneConverter) && (spec.PreserveUnknownFields == nil || *spec.PreserveUnknownFields) +} + func validateEnumStrings(fldPath *field.Path, value string, accepted []string, required bool) field.ErrorList { if value == "" { if required { @@ -359,7 +366,7 @@ func ValidateCustomResourceDefinitionSpecUpdate(spec, oldSpec *apiextensions.Cus // find out whether any schema had default before. Then we keep allowing it. allowDefaults := utilfeature.DefaultFeatureGate.Enabled(apiextensionsfeatures.CustomResourceDefaulting) || specHasDefaults(oldSpec) - allErrs := validateCustomResourceDefinitionSpec(spec, requireRecognizedVersion, allowDefaults, fldPath) + allErrs := validateCustomResourceDefinitionSpec(spec, requireRecognizedVersion, allowDefaults, conversionAndPreserveUnknownFields(oldSpec), fldPath) if established { // these effect the storage and cannot be changed therefore 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 475f6110950..1d174e34ead 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 @@ -116,7 +116,12 @@ func TestValidateCustomResourceDefinition(t *testing.T) { }, }, }, - PreserveUnknownFields: pointer.BoolPtr(true), + Validation: &apiextensions.CustomResourceValidation{ + OpenAPIV3Schema: &apiextensions.JSONSchemaProps{ + Type: "object", + }, + }, + PreserveUnknownFields: pointer.BoolPtr(false), }, Status: apiextensions.CustomResourceDefinitionStatus{ StoredVersions: []string{"version"}, @@ -161,7 +166,12 @@ func TestValidateCustomResourceDefinition(t *testing.T) { }, }, }, - PreserveUnknownFields: pointer.BoolPtr(true), + Validation: &apiextensions.CustomResourceValidation{ + OpenAPIV3Schema: &apiextensions.JSONSchemaProps{ + Type: "object", + }, + }, + PreserveUnknownFields: pointer.BoolPtr(false), }, Status: apiextensions.CustomResourceDefinitionStatus{ StoredVersions: []string{"version"}, @@ -206,7 +216,12 @@ func TestValidateCustomResourceDefinition(t *testing.T) { }, }, }, - PreserveUnknownFields: pointer.BoolPtr(true), + Validation: &apiextensions.CustomResourceValidation{ + OpenAPIV3Schema: &apiextensions.JSONSchemaProps{ + Type: "object", + }, + }, + PreserveUnknownFields: pointer.BoolPtr(false), }, Status: apiextensions.CustomResourceDefinitionStatus{ StoredVersions: []string{"version"}, @@ -247,7 +262,12 @@ func TestValidateCustomResourceDefinition(t *testing.T) { URL: strPtr(""), }, }, - PreserveUnknownFields: pointer.BoolPtr(true), + Validation: &apiextensions.CustomResourceValidation{ + OpenAPIV3Schema: &apiextensions.JSONSchemaProps{ + Type: "object", + }, + }, + PreserveUnknownFields: pointer.BoolPtr(false), }, Status: apiextensions.CustomResourceDefinitionStatus{ StoredVersions: []string{"version"}, @@ -370,7 +390,12 @@ func TestValidateCustomResourceDefinition(t *testing.T) { }, ConversionReviewVersions: []string{"invalid-version"}, }, - PreserveUnknownFields: pointer.BoolPtr(true), + Validation: &apiextensions.CustomResourceValidation{ + OpenAPIV3Schema: &apiextensions.JSONSchemaProps{ + Type: "object", + }, + }, + PreserveUnknownFields: pointer.BoolPtr(false), }, Status: apiextensions.CustomResourceDefinitionStatus{ StoredVersions: []string{"version"}, @@ -412,7 +437,12 @@ func TestValidateCustomResourceDefinition(t *testing.T) { }, ConversionReviewVersions: []string{"0v"}, }, - PreserveUnknownFields: pointer.BoolPtr(true), + Validation: &apiextensions.CustomResourceValidation{ + OpenAPIV3Schema: &apiextensions.JSONSchemaProps{ + Type: "object", + }, + }, + PreserveUnknownFields: pointer.BoolPtr(false), }, Status: apiextensions.CustomResourceDefinitionStatus{ StoredVersions: []string{"version"}, @@ -455,7 +485,12 @@ func TestValidateCustomResourceDefinition(t *testing.T) { }, ConversionReviewVersions: []string{"invalid-version", "v1beta1"}, }, - PreserveUnknownFields: pointer.BoolPtr(true), + Validation: &apiextensions.CustomResourceValidation{ + OpenAPIV3Schema: &apiextensions.JSONSchemaProps{ + Type: "object", + }, + }, + PreserveUnknownFields: pointer.BoolPtr(false), }, Status: apiextensions.CustomResourceDefinitionStatus{ StoredVersions: []string{"version"}, @@ -495,7 +530,12 @@ func TestValidateCustomResourceDefinition(t *testing.T) { }, ConversionReviewVersions: []string{"v1beta1", "v1beta1"}, }, - PreserveUnknownFields: pointer.BoolPtr(true), + Validation: &apiextensions.CustomResourceValidation{ + OpenAPIV3Schema: &apiextensions.JSONSchemaProps{ + Type: "object", + }, + }, + PreserveUnknownFields: pointer.BoolPtr(false), }, Status: apiextensions.CustomResourceDefinitionStatus{ StoredVersions: []string{"version"}, @@ -533,7 +573,12 @@ func TestValidateCustomResourceDefinition(t *testing.T) { Conversion: &apiextensions.CustomResourceConversion{ Strategy: apiextensions.ConversionStrategyType("Webhook"), }, - PreserveUnknownFields: pointer.BoolPtr(true), + Validation: &apiextensions.CustomResourceValidation{ + OpenAPIV3Schema: &apiextensions.JSONSchemaProps{ + Type: "object", + }, + }, + PreserveUnknownFields: pointer.BoolPtr(false), }, Status: apiextensions.CustomResourceDefinitionStatus{ StoredVersions: []string{"version"}, @@ -571,7 +616,12 @@ func TestValidateCustomResourceDefinition(t *testing.T) { Conversion: &apiextensions.CustomResourceConversion{ Strategy: apiextensions.ConversionStrategyType("non_existing_conversion"), }, - PreserveUnknownFields: pointer.BoolPtr(true), + Validation: &apiextensions.CustomResourceValidation{ + OpenAPIV3Schema: &apiextensions.JSONSchemaProps{ + Type: "object", + }, + }, + PreserveUnknownFields: pointer.BoolPtr(false), }, Status: apiextensions.CustomResourceDefinitionStatus{ StoredVersions: []string{"version"}, @@ -581,6 +631,129 @@ func TestValidateCustomResourceDefinition(t *testing.T) { unsupported("spec", "conversion", "strategy"), }, }, + { + name: "none conversion without preserveUnknownFields=false", + resource: &apiextensions.CustomResourceDefinition{ + ObjectMeta: metav1.ObjectMeta{Name: "plural.group.com"}, + Spec: apiextensions.CustomResourceDefinitionSpec{ + Group: "group.com", + Scope: apiextensions.ResourceScope("Cluster"), + Names: apiextensions.CustomResourceDefinitionNames{ + Plural: "plural", + Singular: "singular", + Kind: "Plural", + ListKind: "PluralList", + }, + Versions: []apiextensions.CustomResourceDefinitionVersion{ + { + Name: "version1", + Served: true, + Storage: true, + }, + { + Name: "version2", + Served: true, + Storage: false, + }, + }, + Conversion: &apiextensions.CustomResourceConversion{ + Strategy: apiextensions.ConversionStrategyType("None"), + }, + PreserveUnknownFields: pointer.BoolPtr(true), + }, + Status: apiextensions.CustomResourceDefinitionStatus{ + StoredVersions: []string{"version1"}, + }, + }, + errors: []validationMatch{}, + }, + { + name: "webhook conversion without preserveUnknownFields=false", + resource: &apiextensions.CustomResourceDefinition{ + ObjectMeta: metav1.ObjectMeta{Name: "plural.group.com"}, + Spec: apiextensions.CustomResourceDefinitionSpec{ + Group: "group.com", + Scope: apiextensions.ResourceScope("Cluster"), + Names: apiextensions.CustomResourceDefinitionNames{ + Plural: "plural", + Singular: "singular", + Kind: "Plural", + ListKind: "PluralList", + }, + Versions: []apiextensions.CustomResourceDefinitionVersion{ + { + Name: "version1", + Served: true, + Storage: true, + }, + { + Name: "version2", + Served: true, + Storage: false, + }, + }, + Conversion: &apiextensions.CustomResourceConversion{ + Strategy: apiextensions.ConversionStrategyType("Webhook"), + WebhookClientConfig: &apiextensions.WebhookClientConfig{ + URL: strPtr("https://example.com/webhook"), + }, + ConversionReviewVersions: []string{"v1beta1"}, + }, + PreserveUnknownFields: pointer.BoolPtr(true), + }, + Status: apiextensions.CustomResourceDefinitionStatus{ + StoredVersions: []string{"version1"}, + }, + }, + errors: []validationMatch{ + invalid("spec", "conversion", "strategy"), + }, + }, + { + name: "webhook conversion with preserveUnknownFields=false", + resource: &apiextensions.CustomResourceDefinition{ + ObjectMeta: metav1.ObjectMeta{Name: "plural.group.com"}, + Spec: apiextensions.CustomResourceDefinitionSpec{ + Group: "group.com", + Scope: apiextensions.ResourceScope("Cluster"), + Names: apiextensions.CustomResourceDefinitionNames{ + Plural: "plural", + Singular: "singular", + Kind: "Plural", + ListKind: "PluralList", + }, + Versions: []apiextensions.CustomResourceDefinitionVersion{ + { + Name: "version1", + Served: true, + Storage: true, + }, + { + Name: "version2", + Served: true, + Storage: false, + }, + }, + Conversion: &apiextensions.CustomResourceConversion{ + Strategy: apiextensions.ConversionStrategyType("Webhook"), + WebhookClientConfig: &apiextensions.WebhookClientConfig{ + URL: strPtr("https://example.com/webhook"), + }, + ConversionReviewVersions: []string{"v1beta1"}, + }, + Validation: &apiextensions.CustomResourceValidation{ + OpenAPIV3Schema: &apiextensions.JSONSchemaProps{ + Type: "object", + }, + }, + PreserveUnknownFields: pointer.BoolPtr(false), + }, + Status: apiextensions.CustomResourceDefinitionStatus{ + StoredVersions: []string{"version1"}, + }, + }, + errors: []validationMatch{}, + }, { name: "no_storage_version", resource: &apiextensions.CustomResourceDefinition{ @@ -1642,7 +1815,12 @@ func TestValidateCustomResourceDefinitionUpdate(t *testing.T) { }, ConversionReviewVersions: []string{"invalid-version"}, }, - PreserveUnknownFields: pointer.BoolPtr(true), + Validation: &apiextensions.CustomResourceValidation{ + OpenAPIV3Schema: &apiextensions.JSONSchemaProps{ + Type: "object", + }, + }, + PreserveUnknownFields: pointer.BoolPtr(false), }, Status: apiextensions.CustomResourceDefinitionStatus{ StoredVersions: []string{"version"}, @@ -1678,7 +1856,12 @@ func TestValidateCustomResourceDefinitionUpdate(t *testing.T) { }, ConversionReviewVersions: []string{"invalid-version_0, invalid-version"}, }, - PreserveUnknownFields: pointer.BoolPtr(true), + Validation: &apiextensions.CustomResourceValidation{ + OpenAPIV3Schema: &apiextensions.JSONSchemaProps{ + Type: "object", + }, + }, + PreserveUnknownFields: pointer.BoolPtr(false), }, Status: apiextensions.CustomResourceDefinitionStatus{ StoredVersions: []string{"version"}, @@ -1718,7 +1901,12 @@ func TestValidateCustomResourceDefinitionUpdate(t *testing.T) { }, ConversionReviewVersions: []string{"invalid-version"}, }, - PreserveUnknownFields: pointer.BoolPtr(true), + Validation: &apiextensions.CustomResourceValidation{ + OpenAPIV3Schema: &apiextensions.JSONSchemaProps{ + Type: "object", + }, + }, + PreserveUnknownFields: pointer.BoolPtr(false), }, Status: apiextensions.CustomResourceDefinitionStatus{ StoredVersions: []string{"version"}, @@ -1754,7 +1942,12 @@ func TestValidateCustomResourceDefinitionUpdate(t *testing.T) { }, ConversionReviewVersions: []string{"v1beta1", "invalid-version"}, }, - PreserveUnknownFields: pointer.BoolPtr(true), + Validation: &apiextensions.CustomResourceValidation{ + OpenAPIV3Schema: &apiextensions.JSONSchemaProps{ + Type: "object", + }, + }, + PreserveUnknownFields: pointer.BoolPtr(false), }, Status: apiextensions.CustomResourceDefinitionStatus{ StoredVersions: []string{"version"}, @@ -1796,7 +1989,12 @@ func TestValidateCustomResourceDefinitionUpdate(t *testing.T) { }, ConversionReviewVersions: []string{"invalid-version"}, }, - PreserveUnknownFields: pointer.BoolPtr(true), + Validation: &apiextensions.CustomResourceValidation{ + OpenAPIV3Schema: &apiextensions.JSONSchemaProps{ + Type: "object", + }, + }, + PreserveUnknownFields: pointer.BoolPtr(false), }, Status: apiextensions.CustomResourceDefinitionStatus{ StoredVersions: []string{"version"}, @@ -1831,7 +2029,12 @@ func TestValidateCustomResourceDefinitionUpdate(t *testing.T) { URL: strPtr("https://example.com/webhook"), }, }, - PreserveUnknownFields: pointer.BoolPtr(true), + Validation: &apiextensions.CustomResourceValidation{ + OpenAPIV3Schema: &apiextensions.JSONSchemaProps{ + Type: "object", + }, + }, + PreserveUnknownFields: pointer.BoolPtr(false), }, Status: apiextensions.CustomResourceDefinitionStatus{ StoredVersions: []string{"version"}, @@ -1841,6 +2044,82 @@ func TestValidateCustomResourceDefinitionUpdate(t *testing.T) { invalid("spec", "conversion", "conversionReviewVersions"), }, }, + { + name: "webhookconfig: should accept preserveUnknownFields=true if set before", + resource: &apiextensions.CustomResourceDefinition{ + ObjectMeta: metav1.ObjectMeta{Name: "plural.group.com", ResourceVersion: "42"}, + Spec: apiextensions.CustomResourceDefinitionSpec{ + Group: "group.com", + Scope: apiextensions.ResourceScope("Cluster"), + Names: apiextensions.CustomResourceDefinitionNames{ + Plural: "plural", + Singular: "singular", + Kind: "Plural", + ListKind: "PluralList", + }, + Versions: []apiextensions.CustomResourceDefinitionVersion{ + { + Name: "version", + Served: true, + Storage: true, + }, + { + Name: "version2", + Served: true, + Storage: false, + }, + }, + Conversion: &apiextensions.CustomResourceConversion{ + Strategy: apiextensions.ConversionStrategyType("Webhook"), + WebhookClientConfig: &apiextensions.WebhookClientConfig{ + URL: strPtr("https://example.com/webhook"), + }, + ConversionReviewVersions: []string{"v1beta1"}, + }, + PreserveUnknownFields: pointer.BoolPtr(true), + }, + Status: apiextensions.CustomResourceDefinitionStatus{ + StoredVersions: []string{"version"}, + }, + }, + old: &apiextensions.CustomResourceDefinition{ + ObjectMeta: metav1.ObjectMeta{Name: "plural.group.com", ResourceVersion: "42"}, + Spec: apiextensions.CustomResourceDefinitionSpec{ + Group: "group.com", + Scope: apiextensions.ResourceScope("Cluster"), + Names: apiextensions.CustomResourceDefinitionNames{ + Plural: "plural", + Singular: "singular", + Kind: "Plural", + ListKind: "PluralList", + }, + Versions: []apiextensions.CustomResourceDefinitionVersion{ + { + Name: "version", + Served: true, + Storage: true, + }, + { + Name: "version2", + Served: true, + Storage: false, + }, + }, + Conversion: &apiextensions.CustomResourceConversion{ + Strategy: apiextensions.ConversionStrategyType("Webhook"), + WebhookClientConfig: &apiextensions.WebhookClientConfig{ + URL: strPtr("https://example.com/webhook"), + }, + ConversionReviewVersions: []string{"v1beta1"}, + }, + PreserveUnknownFields: pointer.BoolPtr(true), + }, + Status: apiextensions.CustomResourceDefinitionStatus{ + StoredVersions: []string{"version"}, + }, + }, + errors: []validationMatch{}, + }, { name: "unchanged", old: &apiextensions.CustomResourceDefinition{