OCPBUGS-84955: bootstrap serving certs at hypershift operator startup

cmd/install/assets/hypershift_operator.go

@@ -858,6 +858,7 @@ func (o HyperShiftOperatorDeployment) addWebhookResources(args *[]string, volume
 		VolumeSource: corev1.VolumeSource{
 			Secret: &corev1.SecretVolumeSource{
 				SecretName: "manager-serving-cert",
+				Optional:   ptr.To(true),
 			},
 		},
 	})
@@ -1163,10 +1164,11 @@ func (o ExternalDNSPodMonitor) Build() *prometheusoperatorv1.PodMonitor {
 					"name": ExternalDNSDeploymentName,
 				},
 			},
-			PodMetricsEndpoints: []prometheusoperatorv1.PodMetricsEndpoint{{
-				Port:     ptr.To("metrics"),
-				Interval: "30s",
-			},
+			PodMetricsEndpoints: []prometheusoperatorv1.PodMetricsEndpoint{
+				{
+					Port:     ptr.To("metrics"),
+					Interval: "30s",
+				},
 			},
 		},
 	}
@@ -2050,7 +2052,6 @@ func (o HyperShiftReaderClusterRoleBinding) Build() *rbacv1.ClusterRoleBinding {
 type HyperShiftMutatingWebhookConfiguration struct {
 	Namespace                 *corev1.Namespace
 	EnableAuditLogPersistence bool
-	CABundle                  []byte
 }
 
 const (
@@ -2089,7 +2090,7 @@ func (o HyperShiftMutatingWebhookConfiguration) Build() *admissionregistrationv1
 					},
 				},
 				ClientConfig: admissionregistrationv1.WebhookClientConfig{
-					CABundle: o.CABundle,
+					CABundle: nil,
 					Service: &admissionregistrationv1.ServiceReference{
 						Namespace: "hypershift",
 						Name:      "operator",
@@ -2116,7 +2117,7 @@ func (o HyperShiftMutatingWebhookConfiguration) Build() *admissionregistrationv1
 					},
 				},
 				ClientConfig: admissionregistrationv1.WebhookClientConfig{
-					CABundle: o.CABundle,
+					CABundle: nil,
 					Service: &admissionregistrationv1.ServiceReference{
 						Namespace: "hypershift",
 						Name:      "operator",
@@ -2157,7 +2158,7 @@ func (o HyperShiftMutatingWebhookConfiguration) Build() *admissionregistrationv1
 					},
 				},
 				ClientConfig: admissionregistrationv1.WebhookClientConfig{
-					CABundle: o.CABundle,
+					CABundle: nil,
 					Service: &admissionregistrationv1.ServiceReference{
 						Namespace: "hypershift",
 						Name:      "operator",
@@ -2187,7 +2188,7 @@ func (o HyperShiftMutatingWebhookConfiguration) Build() *admissionregistrationv1
 					},
 				},
 				ClientConfig: admissionregistrationv1.WebhookClientConfig{
-					CABundle: o.CABundle,
+					CABundle: nil,
 					Service: &admissionregistrationv1.ServiceReference{
 						Namespace: "hypershift",
 						Name:      "operator",
@@ -2208,7 +2209,6 @@ func (o HyperShiftMutatingWebhookConfiguration) Build() *admissionregistrationv1
 
 type HyperShiftValidatingWebhookConfiguration struct {
 	Namespace string
-	CABundle  []byte
 }
 
 func (o HyperShiftValidatingWebhookConfiguration) Build() *admissionregistrationv1.ValidatingWebhookConfiguration {
@@ -2245,7 +2245,7 @@ func (o HyperShiftValidatingWebhookConfiguration) Build() *admissionregistration
 					},
 				},
 				ClientConfig: admissionregistrationv1.WebhookClientConfig{
-					CABundle: o.CABundle,
+					CABundle: nil,
 					Service: &admissionregistrationv1.ServiceReference{
 						Namespace: "hypershift",
 						Name:      "operator",
@@ -2274,7 +2274,7 @@ func (o HyperShiftValidatingWebhookConfiguration) Build() *admissionregistration
 					},
 				},
 				ClientConfig: admissionregistrationv1.WebhookClientConfig{
-					CABundle: o.CABundle,
+					CABundle: nil,
 					Service: &admissionregistrationv1.ServiceReference{
 						Namespace: "hypershift",
 						Name:      "operator",

cmd/install/install.go

@@ -30,7 +30,6 @@ import (
 	crdassets "github.com/openshift/hypershift/cmd/install/assets/crds"
 	"github.com/openshift/hypershift/cmd/util"
 	"github.com/openshift/hypershift/hypershift-operator/controllers/sharedingress"
-	"github.com/openshift/hypershift/hypershift-operator/controllers/webhookcerts"
 	hyperapi "github.com/openshift/hypershift/support/api"
 	"github.com/openshift/hypershift/support/config"
 	"github.com/openshift/hypershift/support/metrics"
@@ -756,30 +755,17 @@ func hyperShiftOperatorManifests(ctx context.Context, client crclient.Client, op
 	operatorServiceAccount, rbacObjs := setupRBAC(opts, operatorNamespace)
 	objects = append(objects, rbacObjs...)
 
-	// Generate self-managed webhook CA and serving cert when any webhook is enabled.
-	var webhookCABundle []byte
-	if opts.EnableDefaultingWebhook || opts.EnableConversionWebhook || opts.EnableValidatingWebhook || opts.EnableAuditLogPersistence {
-		caSecret, servingSecret, caBundle, err := webhookcerts.GenerateInitialWebhookCerts(operatorNamespace.Name, assets.HypershiftOperatorName)
-		if err != nil {
-			return nil, nil, fmt.Errorf("failed to generate webhook certs: %w", err)
-		}
-		objects = append(objects, caSecret, servingSecret)
-		webhookCABundle = caBundle
-	}
-
 	if opts.EnableDefaultingWebhook || opts.EnableAuditLogPersistence {
 		mutatingWebhookConfiguration := assets.HyperShiftMutatingWebhookConfiguration{
 			Namespace:                 operatorNamespace,
 			EnableAuditLogPersistence: opts.EnableAuditLogPersistence,
-			CABundle:                  webhookCABundle,
 		}.Build()
 		objects = append(objects, mutatingWebhookConfiguration)
 	}
 
 	if opts.EnableValidatingWebhook {
 		validatingWebhookConfiguration := assets.HyperShiftValidatingWebhookConfiguration{
 			Namespace: operatorNamespace.Name,
-			CABundle:  webhookCABundle,
 		}.Build()
 		objects = append(objects, validatingWebhookConfiguration)
 	}
@@ -831,7 +817,7 @@ func hyperShiftOperatorManifests(ctx context.Context, client crclient.Client, op
 		objects = append(objects, sharedIngressObjs...)
 	}
 
-	crds, err = setupCRDs(ctx, client, opts, operatorNamespace, operatorService, webhookCABundle)
+	crds, err = setupCRDs(ctx, client, opts, operatorNamespace, operatorService)
 	if err != nil {
 		return nil, nil, err
 	}
@@ -874,7 +860,7 @@ var ipamCRDNames = set.New(
 // related to etcd are excluded from the list. If the option EnableConversionWebhook is set to true, the CRDs related
 // to hypershift.openshift.io group are annotated with the necessary annotations to enable the conversion webhook.
 // If a client is provided, IPAM CRDs that already exist in the cluster are skipped to avoid conflicts.
-func setupCRDs(ctx context.Context, client crclient.Client, opts Options, operatorNamespace *corev1.Namespace, operatorService *corev1.Service, webhookCABundle []byte) ([]crclient.Object, error) {
+func setupCRDs(ctx context.Context, client crclient.Client, opts Options, operatorNamespace *corev1.Namespace, operatorService *corev1.Service) ([]crclient.Object, error) {
 	// Build a set of existing IPAM CRDs if a client is available
 	existingIPAMCRDs := set.New[string]()
 	if client != nil {
@@ -967,7 +953,7 @@ func setupCRDs(ctx context.Context, client crclient.Client, opts Options, operat
 									Port:      ptr.To[int32](443),
 									Path:      ptr.To("/convert"),
 								},
-								CABundle: webhookCABundle,
+								CABundle: nil,
 							},
 							ConversionReviewVersions: []string{"v1beta1", "v1alpha1"},
 						},

cmd/install/install_test.go

@@ -5,6 +5,7 @@ import (
 	"context"
 	"io"
 	"io/fs"
+	"os"
 	"path/filepath"
 	"strings"
 	"testing"
@@ -412,7 +413,7 @@ func TestSetupCRDs(t *testing.T) {
 	for _, tc := range tests {
 		t.Run(tc.name, func(t *testing.T) {
 			g := NewGomegaWithT(t)
-			crds, err := setupCRDs(t.Context(), nil, tc.inputOptions, &corev1.Namespace{}, nil, nil)
+			crds, err := setupCRDs(t.Context(), nil, tc.inputOptions, &corev1.Namespace{}, nil)
 			g.Expect(err).ToNot(HaveOccurred())
 			nodePoolCRDS := make([]crclient.Object, 0)
 			var machineDeploymentCRD crclient.Object
@@ -497,6 +498,10 @@ func TestSetupCRDs(t *testing.T) {
 }
 
 func TestRenderHyperShiftOperator_RenderSensitive(t *testing.T) {
+	g := NewGomegaWithT(t)
+	pullSecretFile := filepath.Join(t.TempDir(), "pull-secret.json")
+	g.Expect(os.WriteFile(pullSecretFile, []byte(`{"auths":{}}`), 0o600)).To(Succeed())
+
 	tests := []struct {
 		name            string
 		renderSensitive bool
@@ -524,10 +529,12 @@ func TestRenderHyperShiftOperator_RenderSensitive(t *testing.T) {
 				EnableDefaultingWebhook: true,
 				EnableValidatingWebhook: true,
 				EnableConversionWebhook: true,
+				PullSecretFile:          pullSecretFile,
 				RenderSensitive:         tc.renderSensitive,
 				Format:                  RenderFormatYaml,
 				OutputTypes:             string(OutputAll),
 			}
+
 			err := RenderHyperShiftOperator(t.Context(), &buf, opts)
 			g.Expect(err).ToNot(HaveOccurred())
 
@@ -552,6 +559,39 @@ func TestRenderHyperShiftOperator_RenderSensitive(t *testing.T) {
 			}
 		})
 	}
+
+	t.Run("When webhooks are enabled it should not render webhook cert secrets", func(t *testing.T) {
+		g := NewGomegaWithT(t)
+
+		var buf bytes.Buffer
+		opts := &Options{
+			PrivatePlatform:         string(hyperv1.NonePlatform),
+			PullSecretFile:          pullSecretFile,
+			EnableDefaultingWebhook: true,
+			EnableValidatingWebhook: true,
+			EnableConversionWebhook: true,
+			RenderSensitive:         true,
+			Format:                  RenderFormatYaml,
+			OutputTypes:             string(OutputAll),
+		}
+		err := RenderHyperShiftOperator(t.Context(), &buf, opts)
+		g.Expect(err).ToNot(HaveOccurred())
+
+		var nonWebhookSecretCount int
+		for doc := range strings.SplitSeq(buf.String(), "\n---\n") {
+			if strings.TrimSpace(doc) == "" {
+				continue
+			}
+			obj, _, err := hyperapi.YamlSerializer.Decode([]byte(doc), nil, nil)
+			g.Expect(err).ToNot(HaveOccurred(), "failed to decode rendered manifest")
+			if secret, ok := obj.(*corev1.Secret); ok {
+				g.Expect(secret.Name).ToNot(Equal("webhook-serving-ca"), "webhook CA secret should not be rendered")
+				g.Expect(secret.Name).ToNot(Equal("manager-serving-cert"), "webhook serving cert should not be rendered")
+				nonWebhookSecretCount++
+			}
+		}
+		g.Expect(nonWebhookSecretCount).To(BeNumerically(">", 0), "expected at least one non-webhook secret to be rendered")
+	})
 }
 
 func TestHyperShiftOperatorManifests_SharedIngress(t *testing.T) {
@@ -973,6 +1013,7 @@ func TestWaitForCAPIOperatorSync(t *testing.T) {
 		})
 	}
 }
+
 func TestApplyDefaults(t *testing.T) {
 	tests := []struct {
 		name             string

hypershift-operator/controllers/webhookcerts/webhookcerts_controller.go

@@ -33,6 +33,7 @@ import (
 	ctrl "sigs.k8s.io/controller-runtime"
 	"sigs.k8s.io/controller-runtime/pkg/builder"
 	"sigs.k8s.io/controller-runtime/pkg/client"
+	"sigs.k8s.io/controller-runtime/pkg/controller/controllerutil"
 	"sigs.k8s.io/controller-runtime/pkg/predicate"
 
 	"github.com/go-logr/logr"
@@ -364,3 +365,60 @@ func GenerateInitialWebhookCerts(namespace, serviceName string) (*corev1.Secret,
 	caBundle := caSecret.Data[certs.CASignerCertMapKey]
 	return caSecret, servingSecret, caBundle, nil
 }
+
+// EnsureWebhookCerts ensures that webhook cert secrets exist so the webhook
+// server can start. If the serving cert secret already exists with valid data,
+// this is a no-op (the volume mount handles file delivery). If the secret is
+// missing or has empty data, new certs are generated and persisted as secrets.
+func EnsureWebhookCerts(ctx context.Context, c client.Client, namespace, serviceName string) error {
+	log := ctrl.LoggerFrom(ctx).WithName("webhook-cert-bootstrap")
+
+	if certsExist(ctx, c, namespace) {
+		log.Info("Webhook cert secrets already exist with valid data, skipping bootstrap")
+		return nil
+	}
+
+	log.Info("Generating webhook certificates")
+	caSecret, servingSecret, _, err := GenerateInitialWebhookCerts(namespace, serviceName)
+	if err != nil {
+		return fmt.Errorf("failed to generate webhook certs: %w", err)
+	}
+
+	caObj := &corev1.Secret{ObjectMeta: metav1.ObjectMeta{Name: CASecretName, Namespace: namespace}}
+	if _, err := controllerutil.CreateOrUpdate(ctx, c, caObj, func() error {
+		caObj.Type = corev1.SecretTypeOpaque
+		caObj.Data = caSecret.Data
+		return nil
+	}); err != nil {
+		return fmt.Errorf("failed to create or update CA secret: %w", err)
+	}
+
+	servingObj := &corev1.Secret{ObjectMeta: metav1.ObjectMeta{Name: ServingCertSecretName, Namespace: namespace}}
+	if _, err := controllerutil.CreateOrUpdate(ctx, c, servingObj, func() error {
+		servingObj.Type = corev1.SecretTypeTLS
+		servingObj.Data = servingSecret.Data
+		return nil
+	}); err != nil {
+		return fmt.Errorf("failed to create or update serving cert secret: %w", err)
+	}
+
+	log.Info("Webhook certificates bootstrapped")
+	return nil
+}
+
+// certsExist returns true when both the CA and serving cert secrets exist with non-empty data.
+func certsExist(ctx context.Context, c client.Client, namespace string) bool {
+	ca := &corev1.Secret{}
+	if err := c.Get(ctx, client.ObjectKey{Namespace: namespace, Name: CASecretName}, ca); err != nil {
+		return false
+	}
+	if len(ca.Data[certs.CASignerCertMapKey]) == 0 || len(ca.Data[certs.CASignerKeyMapKey]) == 0 {
+		return false
+	}
+
+	serving := &corev1.Secret{}
+	if err := c.Get(ctx, client.ObjectKey{Namespace: namespace, Name: ServingCertSecretName}, serving); err != nil {
+		return false
+	}
+	return len(serving.Data[corev1.TLSCertKey]) > 0 && len(serving.Data[corev1.TLSPrivateKeyKey]) > 0
+}