feat(webhookcerts): add --enable-webhook-cert-reconciler flag

cmd/install/assets/hypershift_operator.go

@@ -489,6 +489,7 @@ type HyperShiftOperatorDeployment struct {
 	EnableCIDebugOutput                     bool
 	EnableWebhook                           bool
 	EnableValidatingWebhook                 bool
+	EnableWebhookCertReconciler             bool
 	PrivatePlatform                         string
 	AWSPrivateSecret                        *corev1.Secret
 	AWSPrivateSecretKey                     string
@@ -865,6 +866,9 @@ func (o HyperShiftOperatorDeployment) addWebhookResources(args *[]string, volume
 	if o.EnableValidatingWebhook {
 		*args = append(*args, "--enable-validating-webhook=true")
 	}
+	if !o.EnableWebhookCertReconciler {
+		*args = append(*args, "--enable-webhook-cert-reconciler=false")
+	}
 }
 
 func (o HyperShiftOperatorDeployment) addOIDCResources(args *[]string, volumeMounts *[]corev1.VolumeMount, volumes *[]corev1.Volume) {

cmd/install/assets/hypershift_operator_test.go

@@ -1004,12 +1004,14 @@ func TestBuildEnvVars(t *testing.T) {
 
 func TestAddWebhookResources(t *testing.T) {
 	tests := []struct {
-		name                    string
-		enableWebhook           bool
-		enableValidatingWebhook bool
-		expectArgs              []string
-		expectVolumeMountCount  int
-		expectVolumeCount       int
+		name                        string
+		enableWebhook               bool
+		enableValidatingWebhook     bool
+		enableWebhookCertReconciler bool
+		expectArgs                  []string
+		notExpectArgs               []string
+		expectVolumeMountCount      int
+		expectVolumeCount           int
 	}{
 		{
 			name:                   "When webhook is disabled, it should not add any resources",
@@ -1018,28 +1020,41 @@ func TestAddWebhookResources(t *testing.T) {
 			expectVolumeCount:      0,
 		},
 		{
-			name:                   "When webhook is enabled without validating webhook, it should add serving-cert resources and cert-dir arg",
-			enableWebhook:          true,
-			expectArgs:             []string{"--cert-dir=/var/run/secrets/serving-cert"},
-			expectVolumeMountCount: 1,
-			expectVolumeCount:      1,
+			name:                        "When webhook is enabled without validating webhook, it should add serving-cert resources and cert-dir arg",
+			enableWebhook:               true,
+			enableWebhookCertReconciler: true,
+			expectArgs:                  []string{"--cert-dir=/var/run/secrets/serving-cert"},
+			notExpectArgs:               []string{"--enable-webhook-cert-reconciler=false"},
+			expectVolumeMountCount:      1,
+			expectVolumeCount:           1,
 		},
 		{
-			name:                    "When webhook and validating webhook are both enabled, it should add cert-dir and enable-validating-webhook args",
-			enableWebhook:           true,
-			enableValidatingWebhook: true,
-			expectArgs:              []string{"--cert-dir=/var/run/secrets/serving-cert", "--enable-validating-webhook=true"},
-			expectVolumeMountCount:  1,
-			expectVolumeCount:       1,
+			name:                        "When webhook and validating webhook are both enabled, it should add cert-dir and enable-validating-webhook args",
+			enableWebhook:               true,
+			enableValidatingWebhook:     true,
+			enableWebhookCertReconciler: true,
+			expectArgs:                  []string{"--cert-dir=/var/run/secrets/serving-cert", "--enable-validating-webhook=true"},
+			notExpectArgs:               []string{"--enable-webhook-cert-reconciler=false"},
+			expectVolumeMountCount:      1,
+			expectVolumeCount:           1,
+		},
+		{
+			name:                        "When webhook cert reconciler is disabled, it should add the flag to args",
+			enableWebhook:               true,
+			enableWebhookCertReconciler: false,
+			expectArgs:                  []string{"--cert-dir=/var/run/secrets/serving-cert", "--enable-webhook-cert-reconciler=false"},
+			expectVolumeMountCount:      1,
+			expectVolumeCount:           1,
 		},
 	}
 
 	for _, tc := range tests {
 		t.Run(tc.name, func(t *testing.T) {
 			g := NewGomegaWithT(t)
 			d := HyperShiftOperatorDeployment{
-				EnableWebhook:           tc.enableWebhook,
-				EnableValidatingWebhook: tc.enableValidatingWebhook,
+				EnableWebhook:               tc.enableWebhook,
+				EnableValidatingWebhook:     tc.enableValidatingWebhook,
+				EnableWebhookCertReconciler: tc.enableWebhookCertReconciler,
 			}
 			var args []string
 			var volumeMounts []corev1.VolumeMount
@@ -1052,6 +1067,9 @@ func TestAddWebhookResources(t *testing.T) {
 			for _, expected := range tc.expectArgs {
 				g.Expect(args).To(ContainElement(expected))
 			}
+			for _, notExpected := range tc.notExpectArgs {
+				g.Expect(args).NotTo(ContainElement(notExpected))
+			}
 		})
 	}
 }

cmd/install/install.go

@@ -155,6 +155,7 @@ type Options struct {
 	ScaleFromZeroCreds                        string
 	ScaleFromZeroCredentialsSecret            string
 	ScaleFromZeroCredentialsSecretKey         string
+	EnableWebhookCertReconciler               bool
 	RenderSensitive                           bool
 }
 
@@ -377,6 +378,7 @@ func NewCommand() *cobra.Command {
 	cmd.PersistentFlags().BoolVar(&opts.EnableDefaultingWebhook, "enable-defaulting-webhook", opts.EnableDefaultingWebhook, "Enable webhook for defaulting hypershift API types")
 	cmd.PersistentFlags().BoolVar(&opts.EnableValidatingWebhook, "enable-validating-webhook", opts.EnableValidatingWebhook, "Enable webhook for validating hypershift API types")
 	cmd.PersistentFlags().BoolVar(&opts.EnableConversionWebhook, "enable-conversion-webhook", opts.EnableConversionWebhook, "Enable webhook for converting hypershift API types")
+	cmd.PersistentFlags().BoolVar(&opts.EnableWebhookCertReconciler, "enable-webhook-cert-reconciler", opts.EnableWebhookCertReconciler, "If true, the operator manages webhook TLS certificates. Set to false when an external certificate manager (e.g. cert-manager) manages the serving cert")
 	cmd.PersistentFlags().BoolVar(&opts.ExcludeEtcdManifests, "exclude-etcd", opts.ExcludeEtcdManifests, "Leave out etcd manifests")
 	cmd.PersistentFlags().Var(&opts.PlatformMonitoring, "platform-monitoring", "Select an option for enabling platform cluster monitoring. Valid values are: None, OperatorOnly, All")
 	cmd.PersistentFlags().BoolVar(&opts.EnableCIDebugOutput, "enable-ci-debug-output", opts.EnableCIDebugOutput, "If extra CI debug output should be enabled")
@@ -536,6 +538,7 @@ func NewInstallOptionsWithDefaults() Options {
 	opts.EnableConversionWebhook = true
 	opts.EnableDedicatedRequestServingIsolation = true
 	opts.EnableDefaultingWebhook = false
+	opts.EnableWebhookCertReconciler = true
 	opts.EnableEtcdRecovery = true
 	opts.EnableSizeTagging = false
 	opts.EnableValidatingWebhook = false
@@ -1259,6 +1262,7 @@ func setupOperatorResources(opts Options, userCABundleCM *corev1.ConfigMap, trus
 		EnableCIDebugOutput:                     opts.EnableCIDebugOutput,
 		EnableWebhook:                           opts.EnableDefaultingWebhook || opts.EnableConversionWebhook || opts.EnableValidatingWebhook || opts.EnableAuditLogPersistence,
 		EnableValidatingWebhook:                 opts.EnableValidatingWebhook,
+		EnableWebhookCertReconciler:             opts.EnableWebhookCertReconciler,
 		PrivatePlatform:                         opts.PrivatePlatform,
 		AWSPrivateRegion:                        opts.AWSPrivateRegion,
 		GCPProject:                              opts.GCPProject,

hypershift-operator/controllers/webhookcerts/webhookcerts_controller.go

@@ -62,39 +62,73 @@ const (
 
 // WebhookCertReconciler reconciles the self-managed webhook CA and serving cert.
 // It is used on non-OpenShift clusters where the service-ca operator is not available.
+//
+// When ManageCerts is false, the reconciler runs in caBundle-only mode: it reads the
+// CA from the serving cert secret's ca.crt key and patches CRDs and webhook configurations,
+// but does not create or manage the CA or serving cert secrets.
 type WebhookCertReconciler struct {
 	Client         client.Client
 	Namespace      string
 	ServiceName    string
+	ManageCerts    bool
 	createOrUpdate upsert.CreateOrUpdateFN
 }
 
 func (r *WebhookCertReconciler) SetupWithManager(mgr ctrl.Manager, createOrUpdate upsert.CreateOrUpdateProvider) error {
 	r.Client = mgr.GetClient()
 	r.createOrUpdate = createOrUpdate.CreateOrUpdate
 
+	secretFilter := func(o client.Object) bool {
+		if o.GetNamespace() != r.Namespace {
+			return false
+		}
+		if r.ManageCerts {
+			return o.GetName() == CASecretName || o.GetName() == ServingCertSecretName
+		}
+		return o.GetName() == ServingCertSecretName
+	}
+
 	return ctrl.NewControllerManagedBy(mgr).
 		Named("webhookcerts").
-		For(&corev1.Secret{}, builder.WithPredicates(predicate.NewPredicateFuncs(func(o client.Object) bool {
-			return o.GetNamespace() == r.Namespace &&
-				(o.GetName() == CASecretName || o.GetName() == ServingCertSecretName)
-		}))).
+		For(&corev1.Secret{}, builder.WithPredicates(predicate.NewPredicateFuncs(secretFilter))).
 		Complete(r)
 }
 
 func (r *WebhookCertReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Result, error) {
 	log := ctrl.LoggerFrom(ctx)
 
-	// 0. Handle upgrade from service-ca managed certs.
-	// On existing OpenShift clusters, the service-ca operator may have created the
-	// serving cert secret and annotated the Service. We must remove these before
-	// reconciling our own certs, otherwise service-ca will keep overwriting the secret
-	// with a cert signed by a different CA than the one we inject into webhook configs.
-	if err := r.removeServiceCAResources(ctx, log); err != nil {
+	var (
+		caBundle []byte
+		err      error
+	)
+	if r.ManageCerts {
+		caBundle, err = r.reconcileCerts(ctx, log)
+	} else {
+		caBundle, err = r.readCABundleFromServingCert(ctx)
+	}
+	if err != nil {
 		return ctrl.Result{}, err
 	}
 
-	// 1. Reconcile the self-signed CA.
+	if err := r.patchCRDsCABundle(ctx, caBundle); err != nil {
+		return ctrl.Result{}, fmt.Errorf("failed to patch CRD caBundle: %w", err)
+	}
+
+	if err := r.patchWebhookConfigsCABundle(ctx, caBundle); err != nil {
+		return ctrl.Result{}, fmt.Errorf("failed to patch webhook config caBundle: %w", err)
+	}
+
+	log.Info("Webhook certs reconciled", "manageCerts", r.ManageCerts, "requeueAfter", requeueInterval)
+	return ctrl.Result{RequeueAfter: requeueInterval}, nil
+}
+
+func (r *WebhookCertReconciler) reconcileCerts(ctx context.Context, log logr.Logger) ([]byte, error) {
+	// Handle upgrade from service-ca managed certs.
+	if err := r.removeServiceCAResources(ctx, log); err != nil {
+		return nil, err
+	}
+
+	// Reconcile the self-signed CA.
 	caSecret := &corev1.Secret{
 		ObjectMeta: metav1.ObjectMeta{
 			Name:      CASecretName,
@@ -105,10 +139,10 @@ func (r *WebhookCertReconciler) Reconcile(ctx context.Context, req ctrl.Request)
 		caSecret.Type = corev1.SecretTypeOpaque
 		return certs.ReconcileSelfSignedCA(caSecret, "hypershift-webhook-ca", "openshift")
 	}); err != nil {
-		return ctrl.Result{}, fmt.Errorf("failed to reconcile webhook CA secret: %w", err)
+		return nil, fmt.Errorf("failed to reconcile webhook CA secret: %w", err)
 	}
 
-	// 2. Reconcile the serving cert signed by the CA.
+	// Reconcile the serving cert signed by the CA.
 	dnsNames := webhookDNSNames(r.ServiceName, r.Namespace)
 	servingSecret := &corev1.Secret{
 		ObjectMeta: metav1.ObjectMeta{
@@ -131,22 +165,24 @@ func (r *WebhookCertReconciler) Reconcile(ctx context.Context, req ctrl.Request)
 			nil,
 		)
 	}); err != nil {
-		return ctrl.Result{}, fmt.Errorf("failed to reconcile webhook serving cert: %w", err)
+		return nil, fmt.Errorf("failed to reconcile webhook serving cert: %w", err)
 	}
 
-	// 3. Patch caBundle on CRDs with conversion webhooks.
-	caBundle := caSecret.Data[certs.CASignerCertMapKey]
-	if err := r.patchCRDsCABundle(ctx, caBundle); err != nil {
-		return ctrl.Result{}, fmt.Errorf("failed to patch CRD caBundle: %w", err)
-	}
+	return caSecret.Data[certs.CASignerCertMapKey], nil
+}
 
-	// 4. Patch caBundle on webhook configurations.
-	if err := r.patchWebhookConfigsCABundle(ctx, caBundle); err != nil {
-		return ctrl.Result{}, fmt.Errorf("failed to patch webhook config caBundle: %w", err)
+// readCABundleFromServingCert reads the CA bundle from the serving cert secret's ca.crt key,
+// as populated by an external certificate manager (e.g. cert-manager).
+func (r *WebhookCertReconciler) readCABundleFromServingCert(ctx context.Context) ([]byte, error) {
+	servingSecret := &corev1.Secret{}
+	if err := r.Client.Get(ctx, client.ObjectKey{Namespace: r.Namespace, Name: ServingCertSecretName}, servingSecret); err != nil {
+		return nil, fmt.Errorf("failed to get serving cert secret: %w", err)
 	}
-
-	log.Info("Webhook certs reconciled", "requeueAfter", requeueInterval)
-	return ctrl.Result{RequeueAfter: requeueInterval}, nil
+	caBundle, ok := servingSecret.Data[certs.CASignerCertMapKey]
+	if !ok || len(caBundle) == 0 {
+		return nil, fmt.Errorf("serving cert secret %s/%s does not contain a %s key", r.Namespace, ServingCertSecretName, certs.CASignerCertMapKey)
+	}
+	return caBundle, nil
 }
 
 // patchCRDsCABundle patches the caBundle on all CRDs whose conversion webhook points to our service.