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 a9a2bed004e..fa3a1cbe917 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 @@ -28,7 +28,6 @@ import ( "unicode/utf8" celgo "github.com/google/cel-go/cel" - "k8s.io/apiextensions-apiserver/pkg/apihelpers" "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions" apiextensionsv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1" @@ -94,6 +93,8 @@ func ValidateCustomResourceDefinition(ctx context.Context, obj *apiextensions.Cu requireMapListKeysMapSetValidation: true, // strictCost is always true to enforce cost limits. celEnvironmentSet: environment.MustBaseEnvSet(environment.DefaultCompatibilityVersion(), true), + // allowInvalidCABundle is set to true since the CRD is not established yet. + allowInvalidCABundle: true, } allErrs := genericvalidation.ValidateObjectMeta(&obj.ObjectMeta, false, nameValidationFn, field.NewPath("metadata")) @@ -140,6 +141,9 @@ type validationOptions struct { suppressPerExpressionCost bool celEnvironmentSet *environment.EnvSet + // allowInvalidCABundle allows an invalid conversion webhook CABundle on update only if the existing CABundle is invalid. + // An invalid CABundle is also permitted on create and before a CRD is in an Established=True condition. + allowInvalidCABundle bool } type preexistingExpressions struct { @@ -233,7 +237,8 @@ func ValidateCustomResourceDefinitionUpdate(ctx context.Context, obj, oldObj *ap preexistingExpressions: findPreexistingExpressions(&oldObj.Spec), versionsWithUnchangedSchemas: findVersionsWithUnchangedSchemas(obj, oldObj), // strictCost is always true to enforce cost limits. - celEnvironmentSet: environment.MustBaseEnvSet(environment.DefaultCompatibilityVersion(), true), + celEnvironmentSet: environment.MustBaseEnvSet(environment.DefaultCompatibilityVersion(), true), + allowInvalidCABundle: allowInvalidCABundle(oldObj), } return validateCustomResourceDefinitionUpdate(ctx, obj, oldObj, opts) } @@ -485,7 +490,7 @@ func validateCustomResourceDefinitionSpec(ctx context.Context, spec *apiextensio if (spec.Conversion != nil && spec.Conversion.Strategy != apiextensions.NoneConverter) && (spec.PreserveUnknownFields == nil || *spec.PreserveUnknownFields) { 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, opts.requireRecognizedConversionReviewVersion, fldPath.Child("conversion"))...) + allErrs = append(allErrs, validateCustomResourceConversion(spec.Conversion, opts.requireRecognizedConversionReviewVersion, fldPath.Child("conversion"), opts)...) return allErrs } @@ -545,6 +550,20 @@ func validateConversionReviewVersions(versions []string, requireRecognizedVersio return allErrs } +// Allows invalid CA Bundle to be specified only if the existing CABundle is invalid +// or if the CRD is not established yet. +func allowInvalidCABundle(oldCRD *apiextensions.CustomResourceDefinition) bool { + if !apiextensions.IsCRDConditionTrue(oldCRD, apiextensions.Established) { + return true + } + oldConversion := oldCRD.Spec.Conversion + if oldConversion == nil || oldConversion.WebhookClientConfig == nil || + len(oldConversion.WebhookClientConfig.CABundle) == 0 { + return false + } + return len(webhook.ValidateCABundle(field.NewPath("caBundle"), oldConversion.WebhookClientConfig.CABundle)) > 0 +} + // hasValidConversionReviewVersion return true if there is a valid version or if the list is empty. func hasValidConversionReviewVersionOrEmpty(versions []string) bool { if len(versions) < 1 { @@ -558,12 +577,7 @@ func hasValidConversionReviewVersionOrEmpty(versions []string) bool { 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 { +func validateCustomResourceConversion(conversion *apiextensions.CustomResourceConversion, requireRecognizedVersion bool, fldPath *field.Path, opts validationOptions) field.ErrorList { allErrs := field.ErrorList{} if conversion == nil { return allErrs @@ -582,6 +596,9 @@ func validateCustomResourceConversion(conversion *apiextensions.CustomResourceCo case cc.Service != nil: allErrs = append(allErrs, webhook.ValidateWebhookService(fldPath.Child("webhookClientConfig").Child("service"), cc.Service.Name, cc.Service.Namespace, cc.Service.Path, cc.Service.Port)...) } + if len(cc.CABundle) > 0 && !opts.allowInvalidCABundle { + allErrs = append(allErrs, webhook.ValidateCABundle(fldPath.Child("webhookClientConfig").Child("caBundle"), cc.CABundle)...) + } } allErrs = append(allErrs, validateConversionReviewVersions(conversion.ConversionReviewVersions, requireRecognizedVersion, fldPath.Child("conversionReviewVersions"))...) } else { 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 6c11979b314..b0fe74a83d7 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 @@ -92,6 +92,29 @@ func (v validationMatch) contains(s string) validationMatch { func strPtr(s string) *string { return &s } +// exampleCert was generated from crypto/tls/generate_cert.go with the following command: +// +// go run generate_cert.go --rsa-bits 2048 --host example.com --ca --start-date "Jan 1 00:00:00 1970" --duration=1000000h +var exampleCert = []byte(`-----BEGIN CERTIFICATE----- +MIIDADCCAeigAwIBAgIQVHG3Fn9SdWayyLOZKCW1vzANBgkqhkiG9w0BAQsFADAS +MRAwDgYDVQQKEwdBY21lIENvMCAXDTcwMDEwMTAwMDAwMFoYDzIwODQwMTI5MTYw +MDAwWjASMRAwDgYDVQQKEwdBY21lIENvMIIBIjANBgkqhkiG9w0BAQEFAAOCAQ8A +MIIBCgKCAQEArTCu9fiIclNgDdWHphewM+JW55dCb5yYGlJgCBvwbOx547M9p+tn +zm9QOhsdZDHDZsG9tqnWxE2Nc1HpIJyOlfYsOoonpEoG/Ep6nnK91ngj0bn/JlNy ++i/bwU4r97MOukvnOIQez9/D9jAJaOX2+b8/d4lRz9BsqiwJyg+ynZ5tVVYj7aMi +vXnd6HOnJmtqutOtr3beucJnkd6XbwRkLUcAYATT+ZihOWRbTuKqhCg6zGkJOoUG +f8sX61JjoilxiURA//ftGVbdTCU3DrmGmardp5NNOHbumMYU8Vhmqgx1Bqxb+9he +7G42uW5YWYK/GqJzgVPjjlB2dOGj9KrEWQIDAQABo1AwTjAOBgNVHQ8BAf8EBAMC +AqQwEwYDVR0lBAwwCgYIKwYBBQUHAwEwDwYDVR0TAQH/BAUwAwEB/zAWBgNVHREE +DzANggtleGFtcGxlLmNvbTANBgkqhkiG9w0BAQsFAAOCAQEAig4AIi9xWs1+pLES +eeGGdSDoclplFpcbXANnsYYFyLf+8pcWgVi2bOmb2gXMbHFkB07MA82wRJAUTaA+ +2iNXVQMhPCoA7J6ADUbww9doJX2S9HGyArhiV/MhHtE8txzMn2EKNLdhhk3N9rmV +x/qRbWAY1U2z4BpdrAR87Fe81Nlj7h45csW9K+eS+NgXipiNTIfEShKgCFM8EdxL +1WXg7r9AvYV3TNDPWTjLsm1rQzzZQ7Uvcf6deWiNodZd8MOT/BFLclDPTK6cF2Hr +UU4dq6G4kCwMSxWE4cM3HlZ4u1dyIt47VbkP0rtvkBCXx36y+NXYA5lzntchNFZP +uvEQdw== +-----END CERTIFICATE-----`) + func TestValidateCustomResourceDefinition(t *testing.T) { singleVersionList := []apiextensions.CustomResourceDefinitionVersion{ { @@ -235,6 +258,107 @@ func TestValidateCustomResourceDefinition(t *testing.T) { invalid("spec", "conversion", "webhookClientConfig", "service", "port"), }, }, + { + name: "webhookconfig: invalid CABundle should be allowed on Create", + 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{ + Service: &apiextensions.ServiceReference{ + Name: "n", + Namespace: "ns", + Port: 443, + }, + CABundle: []byte("Cg=="), + }, + }, + Validation: &apiextensions.CustomResourceValidation{ + OpenAPIV3Schema: &apiextensions.JSONSchemaProps{ + Type: "object", + }, + }, + PreserveUnknownFields: ptr.To(false), + }, + Status: apiextensions.CustomResourceDefinitionStatus{ + StoredVersions: []string{"version"}, + }, + }, + errors: []validationMatch{}, + }, + { + name: "webhookconfig: valid CABundle", + 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{ + Service: &apiextensions.ServiceReference{ + Name: "n", + Namespace: "ns", + Port: 443, + }, + CABundle: exampleCert, + }, + }, + Validation: &apiextensions.CustomResourceValidation{ + OpenAPIV3Schema: &apiextensions.JSONSchemaProps{ + Type: "object", + }, + }, + PreserveUnknownFields: ptr.To(false), + }, + Status: apiextensions.CustomResourceDefinitionStatus{ + StoredVersions: []string{"version"}, + Conditions: []apiextensions.CustomResourceDefinitionCondition{ + {Type: apiextensions.Established, Status: apiextensions.ConditionTrue}, + }, + }, + }, + errors: []validationMatch{}, + }, { name: "webhookconfig: both service and URL provided", resource: &apiextensions.CustomResourceDefinition{ @@ -5543,6 +5667,362 @@ func TestValidateCustomResourceDefinitionUpdate(t *testing.T) { invalid("spec", "conversion", "conversionReviewVersions"), }, }, + { + name: "webhookconfig: existing invalid CABundle update should pass", + old: &apiextensions.CustomResourceDefinition{ + ObjectMeta: metav1.ObjectMeta{ + Name: "plural.group.com", + ResourceVersion: "42", + }, + Spec: apiextensions.CustomResourceDefinitionSpec{ + Group: "group.com", + Version: "version", + 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{ + Service: &apiextensions.ServiceReference{ + Name: "n", + Namespace: "ns", + Port: 443, + }, + CABundle: []byte("Cg=="), + }, + ConversionReviewVersions: []string{"version2"}, + }, + Validation: &apiextensions.CustomResourceValidation{ + OpenAPIV3Schema: &apiextensions.JSONSchemaProps{ + Type: "object", + }, + }, + Scope: apiextensions.ResourceScope("Cluster"), + Names: apiextensions.CustomResourceDefinitionNames{ + Plural: "plural", + Singular: "singular", + Kind: "kind", + ListKind: "listkind", + }, + }, + Status: apiextensions.CustomResourceDefinitionStatus{ + AcceptedNames: apiextensions.CustomResourceDefinitionNames{ + Plural: "plural", + Singular: "singular", + Kind: "kind", + ListKind: "listkind", + }, + Conditions: []apiextensions.CustomResourceDefinitionCondition{ + {Type: apiextensions.Established, Status: apiextensions.ConditionTrue}, + }, + }, + }, + resource: &apiextensions.CustomResourceDefinition{ + ObjectMeta: metav1.ObjectMeta{ + Name: "plural.group.com", + ResourceVersion: "42", + }, + Spec: apiextensions.CustomResourceDefinitionSpec{ + Group: "group.com", + Version: "version", + 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{ + Service: &apiextensions.ServiceReference{ + Name: "n", + Namespace: "ns", + Port: 443, + }, + CABundle: []byte("Cg=="), + }, + ConversionReviewVersions: []string{"version2"}, + }, + Validation: &apiextensions.CustomResourceValidation{ + OpenAPIV3Schema: &apiextensions.JSONSchemaProps{ + Type: "object", + }, + }, + Scope: apiextensions.ResourceScope("Cluster"), + Names: apiextensions.CustomResourceDefinitionNames{ + Plural: "plural", + Singular: "singular", + Kind: "kind", + ListKind: "listkind", + }, + PreserveUnknownFields: ptr.To(false), + }, + Status: apiextensions.CustomResourceDefinitionStatus{ + AcceptedNames: apiextensions.CustomResourceDefinitionNames{ + Plural: "plural", + Singular: "singular", + Kind: "kind", + ListKind: "listkind", + }, + Conditions: []apiextensions.CustomResourceDefinitionCondition{ + {Type: apiextensions.Established, Status: apiextensions.ConditionTrue}, + }, + StoredVersions: []string{"version"}, + }, + }, + errors: []validationMatch{}, + }, + { + name: "webhookconfig: existing valid CABundle should be able to transition to invalid pre-serving", + old: &apiextensions.CustomResourceDefinition{ + ObjectMeta: metav1.ObjectMeta{ + Name: "plural.group.com", + ResourceVersion: "42", + }, + Spec: apiextensions.CustomResourceDefinitionSpec{ + Group: "group.com", + Version: "version", + 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{ + Service: &apiextensions.ServiceReference{ + Name: "n", + Namespace: "ns", + Port: 443, + }, + CABundle: exampleCert, + }, + ConversionReviewVersions: []string{"version2"}, + }, + Validation: &apiextensions.CustomResourceValidation{ + OpenAPIV3Schema: &apiextensions.JSONSchemaProps{ + Type: "object", + }, + }, + Scope: apiextensions.ResourceScope("Cluster"), + Names: apiextensions.CustomResourceDefinitionNames{ + Plural: "plural", + Singular: "singular", + Kind: "kind", + ListKind: "listkind", + }, + }, + Status: apiextensions.CustomResourceDefinitionStatus{ + AcceptedNames: apiextensions.CustomResourceDefinitionNames{ + Plural: "plural", + Singular: "singular", + Kind: "kind", + ListKind: "listkind", + }, + }, + }, + resource: &apiextensions.CustomResourceDefinition{ + ObjectMeta: metav1.ObjectMeta{ + Name: "plural.group.com", + ResourceVersion: "42", + }, + Spec: apiextensions.CustomResourceDefinitionSpec{ + Group: "group.com", + Version: "version", + 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{ + Service: &apiextensions.ServiceReference{ + Name: "n", + Namespace: "ns", + Port: 443, + }, + CABundle: []byte("Cg=="), + }, + ConversionReviewVersions: []string{"version2"}, + }, + Validation: &apiextensions.CustomResourceValidation{ + OpenAPIV3Schema: &apiextensions.JSONSchemaProps{ + Type: "object", + }, + }, + Scope: apiextensions.ResourceScope("Cluster"), + Names: apiextensions.CustomResourceDefinitionNames{ + Plural: "plural", + Singular: "singular", + Kind: "kind", + ListKind: "listkind", + }, + PreserveUnknownFields: ptr.To(false), + }, + Status: apiextensions.CustomResourceDefinitionStatus{ + AcceptedNames: apiextensions.CustomResourceDefinitionNames{ + Plural: "plural", + Singular: "singular", + Kind: "kind", + ListKind: "listkind", + }, + StoredVersions: []string{"version"}, + }, + }, + errors: []validationMatch{}, + }, + { + name: "webhookconfig: update to invalid CABundle should fail if existing is valid", + old: &apiextensions.CustomResourceDefinition{ + ObjectMeta: metav1.ObjectMeta{ + Name: "plural.group.com", + ResourceVersion: "42", + }, + Spec: apiextensions.CustomResourceDefinitionSpec{ + Group: "group.com", + Version: "version", + 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{ + Service: &apiextensions.ServiceReference{ + Name: "n", + Namespace: "ns", + Port: 443, + }, + CABundle: exampleCert, + }, + ConversionReviewVersions: []string{"version2"}, + }, + Validation: &apiextensions.CustomResourceValidation{ + OpenAPIV3Schema: &apiextensions.JSONSchemaProps{ + Type: "object", + }, + }, + Scope: apiextensions.ResourceScope("Cluster"), + Names: apiextensions.CustomResourceDefinitionNames{ + Plural: "plural", + Singular: "singular", + Kind: "kind", + ListKind: "listkind", + }, + }, + Status: apiextensions.CustomResourceDefinitionStatus{ + AcceptedNames: apiextensions.CustomResourceDefinitionNames{ + Plural: "plural", + Singular: "singular", + Kind: "kind", + ListKind: "listkind", + }, + Conditions: []apiextensions.CustomResourceDefinitionCondition{ + {Type: apiextensions.Established, Status: apiextensions.ConditionTrue}, + }, + }, + }, + resource: &apiextensions.CustomResourceDefinition{ + ObjectMeta: metav1.ObjectMeta{ + Name: "plural.group.com", + ResourceVersion: "42", + }, + Spec: apiextensions.CustomResourceDefinitionSpec{ + Group: "group.com", + Version: "version", + 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{ + Service: &apiextensions.ServiceReference{ + Name: "n", + Namespace: "ns", + Port: 443, + }, + CABundle: []byte("Cg=="), + }, + ConversionReviewVersions: []string{"version2"}, + }, + Validation: &apiextensions.CustomResourceValidation{ + OpenAPIV3Schema: &apiextensions.JSONSchemaProps{ + Type: "object", + }, + }, + Scope: apiextensions.ResourceScope("Cluster"), + Names: apiextensions.CustomResourceDefinitionNames{ + Plural: "plural", + Singular: "singular", + Kind: "kind", + ListKind: "listkind", + }, + PreserveUnknownFields: ptr.To(false), + }, + Status: apiextensions.CustomResourceDefinitionStatus{ + AcceptedNames: apiextensions.CustomResourceDefinitionNames{ + Plural: "plural", + Singular: "singular", + Kind: "kind", + ListKind: "listkind", + }, + Conditions: []apiextensions.CustomResourceDefinitionCondition{ + {Type: apiextensions.Established, Status: apiextensions.ConditionTrue}, + }, + StoredVersions: []string{"version"}, + }, + }, + errors: []validationMatch{ + invalid("spec", "conversion", "webhookClientConfig", "caBundle"), + }, + }, { name: "unchanged", old: &apiextensions.CustomResourceDefinition{ diff --git a/staging/src/k8s.io/apiextensions-apiserver/pkg/controller/establish/establishing_controller.go b/staging/src/k8s.io/apiextensions-apiserver/pkg/controller/establish/establishing_controller.go index 4d5acb4c970..65eb247e560 100644 --- a/staging/src/k8s.io/apiextensions-apiserver/pkg/controller/establish/establishing_controller.go +++ b/staging/src/k8s.io/apiextensions-apiserver/pkg/controller/establish/establishing_controller.go @@ -24,7 +24,9 @@ import ( apierrors "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" utilruntime "k8s.io/apimachinery/pkg/util/runtime" + "k8s.io/apimachinery/pkg/util/validation/field" "k8s.io/apimachinery/pkg/util/wait" + "k8s.io/apiserver/pkg/util/webhook" "k8s.io/client-go/tools/cache" "k8s.io/client-go/util/workqueue" "k8s.io/klog/v2" @@ -131,13 +133,29 @@ func (ec *EstablishingController) sync(key string) error { } crd := cachedCRD.DeepCopy() - establishedCondition := apiextensionsv1.CustomResourceDefinitionCondition{ - Type: apiextensionsv1.Established, - Status: apiextensionsv1.ConditionTrue, - Reason: "InitialNamesAccepted", - Message: "the initial names have been accepted", + + // If the conversion webhook CABundle is invalid, set Established + // condition to false and provide a reason + if cachedCRD.Spec.Conversion != nil && + cachedCRD.Spec.Conversion.Webhook != nil && + cachedCRD.Spec.Conversion.Webhook.ClientConfig != nil && + len(webhook.ValidateCABundle(field.NewPath(""), cachedCRD.Spec.Conversion.Webhook.ClientConfig.CABundle)) > 0 { + errorCondition := apiextensionsv1.CustomResourceDefinitionCondition{ + Type: apiextensionsv1.Established, + Status: apiextensionsv1.ConditionFalse, + Reason: "InvalidCABundle", + Message: "The conversion webhook CABundle is invalid", + } + apiextensionshelpers.SetCRDCondition(crd, errorCondition) + } else { + establishedCondition := apiextensionsv1.CustomResourceDefinitionCondition{ + Type: apiextensionsv1.Established, + Status: apiextensionsv1.ConditionTrue, + Reason: "InitialNamesAccepted", + Message: "the initial names have been accepted", + } + apiextensionshelpers.SetCRDCondition(crd, establishedCondition) } - apiextensionshelpers.SetCRDCondition(crd, establishedCondition) // Update server with new CRD condition. _, err = ec.crdClient.CustomResourceDefinitions().UpdateStatus(context.TODO(), crd, metav1.UpdateOptions{}) diff --git a/staging/src/k8s.io/apiextensions-apiserver/test/integration/cabundle_test.go b/staging/src/k8s.io/apiextensions-apiserver/test/integration/cabundle_test.go new file mode 100644 index 00000000000..1e2e1042ea7 --- /dev/null +++ b/staging/src/k8s.io/apiextensions-apiserver/test/integration/cabundle_test.go @@ -0,0 +1,183 @@ +/* +Copyright 2024 The Kubernetes Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package integration + +import ( + "context" + "testing" + "time" + + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/util/wait" + + apiextensionsv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1" + "k8s.io/apiextensions-apiserver/test/integration/fixtures" +) + +// exampleCert was generated from crypto/tls/generate_cert.go with the following command: +// +// go run generate_cert.go --rsa-bits 2048 --host example.com --ca --start-date "Jan 1 00:00:00 1970" --duration=1000000h +var exampleCert = []byte(`-----BEGIN CERTIFICATE----- +MIIDADCCAeigAwIBAgIQVHG3Fn9SdWayyLOZKCW1vzANBgkqhkiG9w0BAQsFADAS +MRAwDgYDVQQKEwdBY21lIENvMCAXDTcwMDEwMTAwMDAwMFoYDzIwODQwMTI5MTYw +MDAwWjASMRAwDgYDVQQKEwdBY21lIENvMIIBIjANBgkqhkiG9w0BAQEFAAOCAQ8A +MIIBCgKCAQEArTCu9fiIclNgDdWHphewM+JW55dCb5yYGlJgCBvwbOx547M9p+tn +zm9QOhsdZDHDZsG9tqnWxE2Nc1HpIJyOlfYsOoonpEoG/Ep6nnK91ngj0bn/JlNy ++i/bwU4r97MOukvnOIQez9/D9jAJaOX2+b8/d4lRz9BsqiwJyg+ynZ5tVVYj7aMi +vXnd6HOnJmtqutOtr3beucJnkd6XbwRkLUcAYATT+ZihOWRbTuKqhCg6zGkJOoUG +f8sX61JjoilxiURA//ftGVbdTCU3DrmGmardp5NNOHbumMYU8Vhmqgx1Bqxb+9he +7G42uW5YWYK/GqJzgVPjjlB2dOGj9KrEWQIDAQABo1AwTjAOBgNVHQ8BAf8EBAMC +AqQwEwYDVR0lBAwwCgYIKwYBBQUHAwEwDwYDVR0TAQH/BAUwAwEB/zAWBgNVHREE +DzANggtleGFtcGxlLmNvbTANBgkqhkiG9w0BAQsFAAOCAQEAig4AIi9xWs1+pLES +eeGGdSDoclplFpcbXANnsYYFyLf+8pcWgVi2bOmb2gXMbHFkB07MA82wRJAUTaA+ +2iNXVQMhPCoA7J6ADUbww9doJX2S9HGyArhiV/MhHtE8txzMn2EKNLdhhk3N9rmV +x/qRbWAY1U2z4BpdrAR87Fe81Nlj7h45csW9K+eS+NgXipiNTIfEShKgCFM8EdxL +1WXg7r9AvYV3TNDPWTjLsm1rQzzZQ7Uvcf6deWiNodZd8MOT/BFLclDPTK6cF2Hr +UU4dq6G4kCwMSxWE4cM3HlZ4u1dyIt47VbkP0rtvkBCXx36y+NXYA5lzntchNFZP +uvEQdw== +-----END CERTIFICATE-----`) + +var invalidCert = []byte("Cg==") + +// Invalid CABundle should prevent new CRD from being set to Established +func TestInvalidCABundle(t *testing.T) { + ctx := context.Background() + tearDown, apiExtensionClient, _, err := fixtures.StartDefaultServerWithClients(t) + if err != nil { + t.Fatal(err) + } + defer tearDown() + + crd := fixtures.NewRandomNameV1CustomResourceDefinition(apiextensionsv1.NamespaceScoped) + crd.Spec.Conversion = &apiextensionsv1.CustomResourceConversion{ + Strategy: apiextensionsv1.WebhookConverter, + Webhook: &apiextensionsv1.WebhookConversion{ + ClientConfig: &apiextensionsv1.WebhookClientConfig{ + CABundle: invalidCert, + Service: &apiextensionsv1.ServiceReference{ + Namespace: "default", + Name: "example", + }, + }, + ConversionReviewVersions: []string{"v1beta1"}, + }, + } + crd, err = apiExtensionClient.ApiextensionsV1().CustomResourceDefinitions().Create(ctx, crd, metav1.CreateOptions{}) + if err != nil { + t.Fatal(err) + } + // Ensure that Established is false with reason InvalidCABundle + err = wait.PollUntilContextTimeout(ctx, 100*time.Millisecond, 10*time.Second, true, func(ctx context.Context) (bool, error) { + localCrd, err := apiExtensionClient.ApiextensionsV1().CustomResourceDefinitions().Get(ctx, crd.Name, metav1.GetOptions{}) + if err != nil { + return false, err + } + condition := findCRDCondition(localCrd, apiextensionsv1.Established) + if condition == nil { + return false, nil + } + if condition.Status == apiextensionsv1.ConditionFalse && condition.Reason == "InvalidCABundle" { + return true, nil + } + return false, nil + }) + if err != nil { + t.Fatal(err) + } +} + +// Valid CABundle should set CRD to Established. +func TestValidCABundle(t *testing.T) { + ctx := context.Background() + tearDown, apiExtensionClient, _, err := fixtures.StartDefaultServerWithClients(t) + if err != nil { + t.Fatal(err) + } + defer tearDown() + + crd := fixtures.NewRandomNameV1CustomResourceDefinition(apiextensionsv1.NamespaceScoped) + crd.Spec.Conversion = &apiextensionsv1.CustomResourceConversion{ + Strategy: apiextensionsv1.WebhookConverter, + Webhook: &apiextensionsv1.WebhookConversion{ + ClientConfig: &apiextensionsv1.WebhookClientConfig{ + CABundle: exampleCert, + Service: &apiextensionsv1.ServiceReference{ + Namespace: "default", + Name: "example", + }, + }, + ConversionReviewVersions: []string{"v1beta1"}, + }, + } + + crd, err = apiExtensionClient.ApiextensionsV1().CustomResourceDefinitions().Create(ctx, crd, metav1.CreateOptions{}) + if err != nil { + t.Fatal(err) + } + // wait until the CRD is established + err = wait.PollUntilContextTimeout(ctx, 100*time.Millisecond, 10*time.Second, true, func(ctx context.Context) (bool, error) { + localCrd, err := apiExtensionClient.ApiextensionsV1().CustomResourceDefinitions().Get(ctx, crd.Name, metav1.GetOptions{}) + if err != nil { + return false, err + } + condition := findCRDCondition(localCrd, apiextensionsv1.Established) + if condition == nil { + return false, nil + } + if condition.Status == apiextensionsv1.ConditionTrue { + return true, nil + } + return false, nil + }) + if err != nil { + t.Fatal(err) + } + +} + +// No CABundle should set CRD to Established. +func TestMissingCABundle(t *testing.T) { + ctx := context.Background() + tearDown, apiExtensionClient, _, err := fixtures.StartDefaultServerWithClients(t) + if err != nil { + t.Fatal(err) + } + defer tearDown() + + crd := fixtures.NewRandomNameV1CustomResourceDefinition(apiextensionsv1.NamespaceScoped) + crd, err = apiExtensionClient.ApiextensionsV1().CustomResourceDefinitions().Create(ctx, crd, metav1.CreateOptions{}) + if err != nil { + t.Fatal(err) + } + err = wait.PollUntilContextTimeout(ctx, 100*time.Millisecond, 10*time.Second, true, func(ctx context.Context) (bool, error) { + localCrd, err := apiExtensionClient.ApiextensionsV1().CustomResourceDefinitions().Get(ctx, crd.Name, metav1.GetOptions{}) + if err != nil { + return false, err + } + condition := findCRDCondition(localCrd, apiextensionsv1.Established) + if condition == nil { + return false, nil + } + if condition.Status == apiextensionsv1.ConditionTrue { + return true, nil + } + return false, nil + }) + if err != nil { + t.Fatal(err) + } +} diff --git a/staging/src/k8s.io/apiserver/pkg/util/webhook/validation.go b/staging/src/k8s.io/apiserver/pkg/util/webhook/validation.go index 695c00d1763..9cf258b7233 100644 --- a/staging/src/k8s.io/apiserver/pkg/util/webhook/validation.go +++ b/staging/src/k8s.io/apiserver/pkg/util/webhook/validation.go @@ -23,8 +23,18 @@ import ( "k8s.io/apimachinery/pkg/util/validation" "k8s.io/apimachinery/pkg/util/validation/field" + "k8s.io/client-go/transport" ) +func ValidateCABundle(fldPath *field.Path, caBundle []byte) field.ErrorList { + var allErrors field.ErrorList + _, err := transport.TLSConfigFor(&transport.Config{TLS: transport.TLSConfig{CAData: caBundle}}) + if err != nil { + allErrors = append(allErrors, field.Invalid(fldPath, caBundle, err.Error())) + } + return allErrors +} + // ValidateWebhookURL validates webhook's URL. func ValidateWebhookURL(fldPath *field.Path, URL string, forceHttps bool) field.ErrorList { var allErrors field.ErrorList diff --git a/staging/src/k8s.io/apiserver/pkg/util/webhook/validation_test.go b/staging/src/k8s.io/apiserver/pkg/util/webhook/validation_test.go new file mode 100644 index 00000000000..220e6936f9d --- /dev/null +++ b/staging/src/k8s.io/apiserver/pkg/util/webhook/validation_test.go @@ -0,0 +1,85 @@ +/* +Copyright 2024 The Kubernetes Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package webhook + +import ( + "testing" + + "k8s.io/apimachinery/pkg/util/validation/field" +) + +// exampleCert was generated from crypto/tls/generate_cert.go with the following command: +// +// go run generate_cert.go --rsa-bits 2048 --host example.com --ca --start-date "Jan 1 00:00:00 1970" --duration=1000000h +var exampleCert = []byte(`-----BEGIN CERTIFICATE----- +MIIDADCCAeigAwIBAgIQVHG3Fn9SdWayyLOZKCW1vzANBgkqhkiG9w0BAQsFADAS +MRAwDgYDVQQKEwdBY21lIENvMCAXDTcwMDEwMTAwMDAwMFoYDzIwODQwMTI5MTYw +MDAwWjASMRAwDgYDVQQKEwdBY21lIENvMIIBIjANBgkqhkiG9w0BAQEFAAOCAQ8A +MIIBCgKCAQEArTCu9fiIclNgDdWHphewM+JW55dCb5yYGlJgCBvwbOx547M9p+tn +zm9QOhsdZDHDZsG9tqnWxE2Nc1HpIJyOlfYsOoonpEoG/Ep6nnK91ngj0bn/JlNy ++i/bwU4r97MOukvnOIQez9/D9jAJaOX2+b8/d4lRz9BsqiwJyg+ynZ5tVVYj7aMi +vXnd6HOnJmtqutOtr3beucJnkd6XbwRkLUcAYATT+ZihOWRbTuKqhCg6zGkJOoUG +f8sX61JjoilxiURA//ftGVbdTCU3DrmGmardp5NNOHbumMYU8Vhmqgx1Bqxb+9he +7G42uW5YWYK/GqJzgVPjjlB2dOGj9KrEWQIDAQABo1AwTjAOBgNVHQ8BAf8EBAMC +AqQwEwYDVR0lBAwwCgYIKwYBBQUHAwEwDwYDVR0TAQH/BAUwAwEB/zAWBgNVHREE +DzANggtleGFtcGxlLmNvbTANBgkqhkiG9w0BAQsFAAOCAQEAig4AIi9xWs1+pLES +eeGGdSDoclplFpcbXANnsYYFyLf+8pcWgVi2bOmb2gXMbHFkB07MA82wRJAUTaA+ +2iNXVQMhPCoA7J6ADUbww9doJX2S9HGyArhiV/MhHtE8txzMn2EKNLdhhk3N9rmV +x/qRbWAY1U2z4BpdrAR87Fe81Nlj7h45csW9K+eS+NgXipiNTIfEShKgCFM8EdxL +1WXg7r9AvYV3TNDPWTjLsm1rQzzZQ7Uvcf6deWiNodZd8MOT/BFLclDPTK6cF2Hr +UU4dq6G4kCwMSxWE4cM3HlZ4u1dyIt47VbkP0rtvkBCXx36y+NXYA5lzntchNFZP +uvEQdw== +-----END CERTIFICATE-----`) + +func TestValidateCABundle(t *testing.T) { + tests := []struct { + name string + caBundle []byte + expectErr bool + }{ + { + name: "nil caBundle is valid", + caBundle: nil, + expectErr: false, + }, { + name: "empty caBundle is valid", + caBundle: []byte(""), + expectErr: false, + }, { + name: "non empty caBundle with invalid certificate should not validate", + caBundle: []byte("bogus"), + expectErr: true, + }, { + name: "non empty caBundle with no certificate should not validate", + caBundle: []byte("Cg=="), + expectErr: true, + }, { + name: "non empty caBundle with valid certificate should validate", + caBundle: exampleCert, + expectErr: false, + }, + } + + for _, tc := range tests { + errList := ValidateCABundle(field.NewPath(""), tc.caBundle) + if len(errList) > 0 && !tc.expectErr { + t.Errorf("Expected no error for test %s, got %v", tc.name, errList) + } else if len(errList) == 0 && tc.expectErr { + t.Errorf("Expected error for test %s, received no error", tc.name) + } + } +}