Merge pull request #124061 from Jefftree/conversion-webhook-invalidca

Validate CABundle when writing CRD

Kubernetes-commit: 04d2f336419b5a824cb96cb88462ef18a90d619d

go.mod

@@ -25,10 +25,10 @@ require (
 	google.golang.org/protobuf v1.34.2
 	gopkg.in/evanphx/json-patch.v4 v4.12.0
 	gopkg.in/yaml.v2 v2.4.0
-	k8s.io/api v0.0.0-20240722223049-b689d905290f
+	k8s.io/api v0.0.0-20240723194852-871340c2e998
 	k8s.io/apimachinery v0.0.0-20240720202316-95b78024e3fe
-	k8s.io/apiserver v0.0.0-20240723030233-2b2a4b0fa8e4
-	k8s.io/client-go v0.0.0-20240723023642-bad8f77ca6ef
+	k8s.io/apiserver v0.0.0-20240723210659-c90207143c20
+	k8s.io/client-go v0.0.0-20240723200359-dcfcc90795cc
 	k8s.io/code-generator v0.0.0-20240720023521-ec3cc888df4c
 	k8s.io/component-base v0.0.0-20240722183709-6cc953a9d440
 	k8s.io/klog/v2 v2.130.1

go.sum

@@ -365,14 +365,14 @@ gopkg.in/yaml.v3 v3.0.1 h1:fxVm/GzAzEWqLHuvctI91KS9hhNmmWOoWu0XTYJS7CA=
 gopkg.in/yaml.v3 v3.0.1/go.mod h1:K4uyk7z7BCEPqu6E+C64Yfv1cQ7kz7rIZviUmN+EgEM=
 honnef.co/go/tools v0.0.0-20190102054323-c2f93a96b099/go.mod h1:rf3lG4BRIbNafJWhAfAdb/ePZxsR/4RtNHQocxwk9r4=
 honnef.co/go/tools v0.0.0-20190523083050-ea95bdfd59fc/go.mod h1:rf3lG4BRIbNafJWhAfAdb/ePZxsR/4RtNHQocxwk9r4=
-k8s.io/api v0.0.0-20240722223049-b689d905290f h1:wtqzslJEcheiQ7hXjw1yGfqUyMCb7G4j72aL64Bzpbo=
-k8s.io/api v0.0.0-20240722223049-b689d905290f/go.mod h1:ytlEzqC2wOTwYET71W7+J+k7O2V7vrDuzmNLBSpgT+k=
+k8s.io/api v0.0.0-20240723194852-871340c2e998 h1:XvMrEqepRsNkn8Bl60PB5TO4ZEOgr70bYrpAedjvTV8=
+k8s.io/api v0.0.0-20240723194852-871340c2e998/go.mod h1:ytlEzqC2wOTwYET71W7+J+k7O2V7vrDuzmNLBSpgT+k=
 k8s.io/apimachinery v0.0.0-20240720202316-95b78024e3fe h1:V9MwpYUwbKlfLKVrhpVuKWiat/LBIhm1pGB9/xdHm5Q=
 k8s.io/apimachinery v0.0.0-20240720202316-95b78024e3fe/go.mod h1:rsPdaZJfTfLsNJSQzNHQvYoTmxhoOEofxtOsF3rtsMo=
-k8s.io/apiserver v0.0.0-20240723030233-2b2a4b0fa8e4 h1:7nrffLiDUbMAXLKzBvyU8rwLHw5WpCw2AjhDO5IZYRs=
-k8s.io/apiserver v0.0.0-20240723030233-2b2a4b0fa8e4/go.mod h1:R1HYbPCD+ClvTmzeLBYaS4aktC3entK1o4hyD+WemtA=
-k8s.io/client-go v0.0.0-20240723023642-bad8f77ca6ef h1:+munBmXPvgGM5AFzdZh7Xe7S2LJ9udXYRfBDfm+0Eac=
-k8s.io/client-go v0.0.0-20240723023642-bad8f77ca6ef/go.mod h1:L1rDFyPUkmS0j6WXGYh5v/iWsfIFYH+LWnFOT1LCsf4=
+k8s.io/apiserver v0.0.0-20240723210659-c90207143c20 h1:32/qwZM293YJ71bcGQD2PhnPOLQBqQoDf6WvzQ/kpxI=
+k8s.io/apiserver v0.0.0-20240723210659-c90207143c20/go.mod h1:NQDM9jV7nUGYBnMcr1Wm1zo17QIrCW1RWoD1kcuH/80=
+k8s.io/client-go v0.0.0-20240723200359-dcfcc90795cc h1:qe0SREEjfE5w3ANvrSURWv00J/ISlqa9Sa3FCBYKRlg=
+k8s.io/client-go v0.0.0-20240723200359-dcfcc90795cc/go.mod h1:XfEsPNNFOR0wNkr3BtkPUN668l7Sx1W4ECSUolQ0mA4=
 k8s.io/code-generator v0.0.0-20240720023521-ec3cc888df4c h1:oiNPH9Y/YrQfxo8eTW/w71aBrSyr9MX/wGBKTwDSZsc=
 k8s.io/code-generator v0.0.0-20240720023521-ec3cc888df4c/go.mod h1:TVAwbna2B36D+IsWJ5oHqKZKSU8ZBtxeiMTb7uKM6Z0=
 k8s.io/component-base v0.0.0-20240722183709-6cc953a9d440 h1:14X+5sRQRsul6tLxIKTP0/DotvWlMd9DFCgMqHP1hZY=

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 {