diff --git a/staging/src/k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/fuzzer/fuzzer.go b/staging/src/k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/fuzzer/fuzzer.go index a599e61a3a3..3344ce2be25 100644 --- a/staging/src/k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/fuzzer/fuzzer.go +++ b/staging/src/k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/fuzzer/fuzzer.go @@ -66,6 +66,9 @@ func Funcs(codecs runtimeserializer.CodecFactory) []interface{} { Strategy: apiextensions.NoneConverter, } } + if obj.Conversion.Strategy == apiextensions.WebhookConverter && len(obj.Conversion.ConversionReviewVersions) == 0 { + obj.Conversion.ConversionReviewVersions = []string{"v1beta1"} + } }, func(obj *apiextensions.CustomResourceDefinition, c fuzz.Continue) { c.FuzzNoCustom(obj) diff --git a/staging/src/k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/types.go b/staging/src/k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/types.go index fcbd8bd197a..21873d46020 100644 --- a/staging/src/k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/types.go +++ b/staging/src/k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/types.go @@ -84,6 +84,15 @@ type CustomResourceConversion struct { // `webhookClientConfig` is the instructions for how to call the webhook if strategy is `Webhook`. WebhookClientConfig *WebhookClientConfig + + // ConversionReviewVersions is an ordered list of preferred `ConversionReview` + // versions the Webhook expects. API server will try to use first version in + // the list which it supports. If none of the versions specified in this list + // supported by API server, conversion will fail for this object. + // If a persisted Webhook configuration specifies allowed versions and does not + // include any versions known to the API Server, calls to the webhook will fail. + // +optional + ConversionReviewVersions []string } // WebhookClientConfig contains the information to make a TLS diff --git a/staging/src/k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1beta1/defaults.go b/staging/src/k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1beta1/defaults.go index 5aae97cf1a2..7bea2d69887 100644 --- a/staging/src/k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1beta1/defaults.go +++ b/staging/src/k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1beta1/defaults.go @@ -68,6 +68,9 @@ func SetDefaults_CustomResourceDefinitionSpec(obj *CustomResourceDefinitionSpec) Strategy: NoneConverter, } } + if obj.Conversion.Strategy == WebhookConverter && len(obj.Conversion.ConversionReviewVersions) == 0 { + obj.Conversion.ConversionReviewVersions = []string{SchemeGroupVersion.Version} + } } // hasPerVersionColumns returns true if a CRD uses per-version columns. diff --git a/staging/src/k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1beta1/types.go b/staging/src/k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1beta1/types.go index e99d9e49b52..973ac0eedd5 100644 --- a/staging/src/k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1beta1/types.go +++ b/staging/src/k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1beta1/types.go @@ -91,6 +91,16 @@ type CustomResourceConversion struct { // alpha-level and is only honored by servers that enable the CustomResourceWebhookConversion feature. // +optional WebhookClientConfig *WebhookClientConfig `json:"webhookClientConfig,omitempty" protobuf:"bytes,2,name=webhookClientConfig"` + + // ConversionReviewVersions is an ordered list of preferred `ConversionReview` + // versions the Webhook expects. API server will try to use first version in + // the list which it supports. If none of the versions specified in this list + // supported by API server, conversion will fail for this object. + // If a persisted Webhook configuration specifies allowed versions and does not + // include any versions known to the API Server, calls to the webhook will fail. + // Default to `['v1beta1']`. + // +optional + ConversionReviewVersions []string `json:"conversionReviewVersions,omitempty" protobuf:"bytes,3,rep,name=conversionReviewVersions"` } // WebhookClientConfig contains the information to make a TLS 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 79191281eab..7e74b38ce79 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 @@ -24,12 +24,14 @@ import ( apiequality "k8s.io/apimachinery/pkg/api/equality" genericvalidation "k8s.io/apimachinery/pkg/api/validation" "k8s.io/apimachinery/pkg/util/sets" + utilvalidation "k8s.io/apimachinery/pkg/util/validation" validationutil "k8s.io/apimachinery/pkg/util/validation" "k8s.io/apimachinery/pkg/util/validation/field" utilfeature "k8s.io/apiserver/pkg/util/feature" "k8s.io/apiserver/pkg/util/webhook" "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions" + "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1beta1" apiservervalidation "k8s.io/apiextensions-apiserver/pkg/apiserver/validation" apiextensionsfeatures "k8s.io/apiextensions-apiserver/pkg/features" ) @@ -113,6 +115,10 @@ func ValidateCustomResourceDefinitionVersion(version *apiextensions.CustomResour // ValidateCustomResourceDefinitionSpec statically validates func ValidateCustomResourceDefinitionSpec(spec *apiextensions.CustomResourceDefinitionSpec, fldPath *field.Path) field.ErrorList { + return validateCustomResourceDefinitionSpec(spec, true, fldPath) +} + +func validateCustomResourceDefinitionSpec(spec *apiextensions.CustomResourceDefinitionSpec, requireRecognizedVersion bool, fldPath *field.Path) field.ErrorList { allErrs := field.ErrorList{} if len(spec.Group) == 0 { @@ -205,7 +211,7 @@ func ValidateCustomResourceDefinitionSpec(spec *apiextensions.CustomResourceDefi } } - allErrs = append(allErrs, ValidateCustomResourceConversion(spec.Conversion, fldPath.Child("conversion"))...) + allErrs = append(allErrs, validateCustomResourceConversion(spec.Conversion, requireRecognizedVersion, fldPath.Child("conversion"))...) return allErrs } @@ -225,8 +231,66 @@ func validateEnumStrings(fldPath *field.Path, value string, accepted []string, r return field.ErrorList{field.NotSupported(fldPath, value, accepted)} } +var acceptedConversionReviewVersion = []string{v1beta1.SchemeGroupVersion.Version} + +func isAcceptedConversionReviewVersion(v string) bool { + for _, version := range acceptedConversionReviewVersion { + if v == version { + return true + } + } + return false +} + +func validateConversionReviewVersions(versions []string, requireRecognizedVersion bool, fldPath *field.Path) field.ErrorList { + allErrs := field.ErrorList{} + if len(versions) < 1 { + allErrs = append(allErrs, field.Required(fldPath, "")) + } else { + seen := map[string]bool{} + hasAcceptedVersion := false + for i, v := range versions { + if seen[v] { + allErrs = append(allErrs, field.Invalid(fldPath.Index(i), v, "duplicate version")) + continue + } + seen[v] = true + for _, errString := range utilvalidation.IsDNS1035Label(v) { + allErrs = append(allErrs, field.Invalid(fldPath.Index(i), v, errString)) + } + if isAcceptedConversionReviewVersion(v) { + hasAcceptedVersion = true + } + } + if requireRecognizedVersion && !hasAcceptedVersion { + allErrs = append(allErrs, field.Invalid( + fldPath, versions, + fmt.Sprintf("none of the versions accepted by this server. accepted version(s) are %v", + strings.Join(acceptedConversionReviewVersion, ", ")))) + } + } + return allErrs +} + +// hasValidConversionReviewVersion return true if there is a valid version or if the list is empty. +func hasValidConversionReviewVersionOrEmpty(versions []string) bool { + if len(versions) < 1 { + return true + } + for _, v := range versions { + if isAcceptedConversionReviewVersion(v) { + return true + } + } + return false +} + // ValidateCustomResourceConversion statically validates func ValidateCustomResourceConversion(conversion *apiextensions.CustomResourceConversion, fldPath *field.Path) field.ErrorList { + return validateCustomResourceConversion(conversion, true, fldPath) +} + +func validateCustomResourceConversion(conversion *apiextensions.CustomResourceConversion, requireRecognizedVersion bool, fldPath *field.Path) field.ErrorList { allErrs := field.ErrorList{} if conversion == nil { return allErrs @@ -250,15 +314,22 @@ func ValidateCustomResourceConversion(conversion *apiextensions.CustomResourceCo allErrs = append(allErrs, webhook.ValidateWebhookService(fldPath.Child("webhookClientConfig").Child("service"), cc.Service.Name, cc.Service.Namespace, cc.Service.Path)...) } } - } else if conversion.WebhookClientConfig != nil { - allErrs = append(allErrs, field.Forbidden(fldPath.Child("webhookClientConfig"), "should not be set when strategy is not set to Webhook")) + allErrs = append(allErrs, validateConversionReviewVersions(conversion.ConversionReviewVersions, requireRecognizedVersion, fldPath.Child("conversionReviewVersions"))...) + } else { + if conversion.WebhookClientConfig != nil { + allErrs = append(allErrs, field.Forbidden(fldPath.Child("webhookClientConfig"), "should not be set when strategy is not set to Webhook")) + } + if len(conversion.ConversionReviewVersions) > 0 { + allErrs = append(allErrs, field.Forbidden(fldPath.Child("conversionReviewVersions"), "should not be set when strategy is not set to Webhook")) + } } return allErrs } // ValidateCustomResourceDefinitionSpecUpdate statically validates func ValidateCustomResourceDefinitionSpecUpdate(spec, oldSpec *apiextensions.CustomResourceDefinitionSpec, established bool, fldPath *field.Path) field.ErrorList { - allErrs := ValidateCustomResourceDefinitionSpec(spec, fldPath) + requireRecognizedVersion := oldSpec.Conversion == nil || hasValidConversionReviewVersionOrEmpty(oldSpec.Conversion.ConversionReviewVersions) + allErrs := validateCustomResourceDefinitionSpec(spec, requireRecognizedVersion, 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 4a2f9f19375..031cbee9425 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 @@ -35,6 +35,9 @@ func required(path ...string) validationMatch { func invalid(path ...string) validationMatch { return validationMatch{path: field.NewPath(path[0], path[1:]...), errorType: field.ErrorTypeInvalid} } +func invalidIndex(index int, path ...string) validationMatch { + return validationMatch{path: field.NewPath(path[0], path[1:]...).Index(index), errorType: field.ErrorTypeInvalid} +} func unsupported(path ...string) validationMatch { return validationMatch{path: field.NewPath(path[0], path[1:]...), errorType: field.ErrorTypeNotSupported} } @@ -65,7 +68,7 @@ func TestValidateCustomResourceDefinition(t *testing.T) { errors []validationMatch }{ { - name: "webhookconfig: blank URL", + name: "webhookconfig: both service and URL provided", resource: &apiextensions.CustomResourceDefinition{ ObjectMeta: metav1.ObjectMeta{Name: "plural.group.com"}, Spec: apiextensions.CustomResourceDefinitionSpec{ @@ -109,7 +112,7 @@ func TestValidateCustomResourceDefinition(t *testing.T) { }, }, { - name: "webhookconfig: both service and URL provided", + name: "webhookconfig: blank URL", resource: &apiextensions.CustomResourceDefinition{ ObjectMeta: metav1.ObjectMeta{Name: "plural.group.com"}, Spec: apiextensions.CustomResourceDefinitionSpec{ @@ -189,6 +192,207 @@ func TestValidateCustomResourceDefinition(t *testing.T) { forbidden("spec", "conversion", "webhookClientConfig"), }, }, + { + name: "ConversionReviewVersions_should_not_be_set", + 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: "version", + Served: true, + Storage: true, + }, + { + Name: "version2", + Served: true, + Storage: false, + }, + }, + Conversion: &apiextensions.CustomResourceConversion{ + Strategy: apiextensions.ConversionStrategyType("None"), + ConversionReviewVersions: []string{"v1beta1"}, + }, + }, + Status: apiextensions.CustomResourceDefinitionStatus{ + StoredVersions: []string{"version"}, + }, + }, + errors: []validationMatch{ + forbidden("spec", "conversion", "conversionReviewVersions"), + }, + }, + { + name: "webhookconfig: invalid ConversionReviewVersion", + 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: "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{"invalid-version"}, + }, + }, + Status: apiextensions.CustomResourceDefinitionStatus{ + StoredVersions: []string{"version"}, + }, + }, + errors: []validationMatch{ + invalid("spec", "conversion", "conversionReviewVersions"), + }, + }, + { + name: "webhookconfig: invalid ConversionReviewVersion version string", + 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: "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{"0v"}, + }, + }, + Status: apiextensions.CustomResourceDefinitionStatus{ + StoredVersions: []string{"version"}, + }, + }, + errors: []validationMatch{ + invalidIndex(0, "spec", "conversion", "conversionReviewVersions"), + invalid("spec", "conversion", "conversionReviewVersions"), + }, + }, + { + name: "webhookconfig: at least one valid ConversionReviewVersion", + 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: "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{"invalid-version", "v1beta1"}, + }, + }, + Status: apiextensions.CustomResourceDefinitionStatus{ + StoredVersions: []string{"version"}, + }, + }, + errors: []validationMatch{}, + }, + { + name: "webhookconfig: duplicate ConversionReviewVersion", + 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: "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", "v1beta1"}, + }, + }, + Status: apiextensions.CustomResourceDefinitionStatus{ + StoredVersions: []string{"version"}, + }, + }, + errors: []validationMatch{ + invalidIndex(1, "spec", "conversion", "conversionReviewVersions"), + }, + }, { name: "missing_webhookconfig", resource: &apiextensions.CustomResourceDefinition{ @@ -646,6 +850,10 @@ func TestValidateCustomResourceDefinition(t *testing.T) { } for _, tc := range tests { + // duplicate defaulting behaviour + if tc.resource.Spec.Conversion != nil && tc.resource.Spec.Conversion.Strategy == apiextensions.WebhookConverter && len(tc.resource.Spec.Conversion.ConversionReviewVersions) == 0 { + tc.resource.Spec.Conversion.ConversionReviewVersions = []string{"v1beta1"} + } errs := ValidateCustomResourceDefinition(tc.resource) seenErrs := make([]bool, len(errs)) @@ -679,6 +887,231 @@ func TestValidateCustomResourceDefinitionUpdate(t *testing.T) { resource *apiextensions.CustomResourceDefinition errors []validationMatch }{ + { + name: "webhookconfig: should pass on invalid ConversionReviewVersion with old invalid versions", + 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{"invalid-version"}, + }, + }, + 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{"invalid-version_0, invalid-version"}, + }, + }, + Status: apiextensions.CustomResourceDefinitionStatus{ + StoredVersions: []string{"version"}, + }, + }, + errors: []validationMatch{}, + }, + { + name: "webhookconfig: should fail on invalid ConversionReviewVersion with old valid versions", + 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{"invalid-version"}, + }, + }, + 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", "invalid-version"}, + }, + }, + Status: apiextensions.CustomResourceDefinitionStatus{ + StoredVersions: []string{"version"}, + }, + }, + errors: []validationMatch{ + invalid("spec", "conversion", "conversionReviewVersions"), + }, + }, + { + name: "webhookconfig: should fail on invalid ConversionReviewVersion with missing old versions", + 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{"invalid-version"}, + }, + }, + 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"), + }, + }, + }, + Status: apiextensions.CustomResourceDefinitionStatus{ + StoredVersions: []string{"version"}, + }, + }, + errors: []validationMatch{ + invalid("spec", "conversion", "conversionReviewVersions"), + }, + }, { name: "unchanged", old: &apiextensions.CustomResourceDefinition{ diff --git a/staging/src/k8s.io/apiextensions-apiserver/pkg/apiserver/conversion/webhook_converter.go b/staging/src/k8s.io/apiextensions-apiserver/pkg/apiserver/conversion/webhook_converter.go index 3cc295ea187..0957ae9f8a3 100644 --- a/staging/src/k8s.io/apiextensions-apiserver/pkg/apiserver/conversion/webhook_converter.go +++ b/staging/src/k8s.io/apiextensions-apiserver/pkg/apiserver/conversion/webhook_converter.go @@ -60,6 +60,8 @@ type webhookConverter struct { restClient *rest.RESTClient name string nopConverter nopConverter + + conversionReviewVersions []string } func webhookClientConfigForCRD(crd *internal.CustomResourceDefinition) *webhook.ClientConfig { @@ -96,6 +98,8 @@ func (f *webhookConverterFactory) NewWebhookConverter(validVersions map[schema.G restClient: restClient, name: crd.Name, nopConverter: nopConverter{validVersions: validVersions}, + + conversionReviewVersions: crd.Spec.Conversion.ConversionReviewVersions, }, nil } @@ -136,6 +140,16 @@ func (c *webhookConverter) Convert(in, out, context interface{}) error { return nil } +// hasConversionReviewVersion check whether a version is accepted by a given webhook. +func (c *webhookConverter) hasConversionReviewVersion(v string) bool { + for _, b := range c.conversionReviewVersions { + if b == v { + return true + } + } + return false +} + func createConversionReview(obj runtime.Object, apiVersion string) *v1beta1.ConversionReview { listObj, isList := obj.(*unstructured.UnstructuredList) var objects []runtime.RawExtension @@ -216,6 +230,12 @@ func (c *webhookConverter) ConvertToVersion(in runtime.Object, target runtime.Gr } } + // Currently converter only supports `v1beta1` ConversionReview + // TODO: Make CRD webhooks caller capable of sending/receiving multiple ConversionReview versions + if !c.hasConversionReviewVersion(v1beta1.SchemeGroupVersion.Version) { + return nil, fmt.Errorf("webhook does not accept v1beta1 ConversionReview") + } + request := createConversionReview(in, toGV.String()) if len(request.Request.Objects) == 0 { if !isList {