From 2163b414ef7a20a8bf0e067a9696b8da7dffcfc2 Mon Sep 17 00:00:00 2001 From: Borja Clemente Date: Tue, 19 May 2026 13:19:21 +0200 Subject: [PATCH 1/3] feat(webhook): bootstrap serving certs at operator startup The webhook server requires TLS certs on disk before it can start listening. Previously, certs were only created via `hypershift install` manifests, meaning the operator would fail to start if the serving cert secret was not yet present like it is the case when running `hyperhsift install render` without the `--render-sensitive` flag. This is used in a scenario where the rendered manifests are pushed to a gitops workflow and, therefore, secrets can not be rendered. By adding bootstrap cert generation at startup, the operator is self-sufficient: if the secret exists, the volume mount delivers certs normally; if it is missing or empty, certs are generated, persisted, and written to disk. The secret volume is now marked optional so the pod can start without it. Signed-off-by: Borja Clemente --- cmd/install/assets/hypershift_operator.go | 10 +-- .../webhookcerts/webhookcerts_controller.go | 70 +++++++++++++++++++ hypershift-operator/main.go | 7 ++ 3 files changed, 83 insertions(+), 4 deletions(-) diff --git a/cmd/install/assets/hypershift_operator.go b/cmd/install/assets/hypershift_operator.go index d2af9802030..20a64e11d95 100644 --- a/cmd/install/assets/hypershift_operator.go +++ b/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", + }, }, }, } diff --git a/hypershift-operator/controllers/webhookcerts/webhookcerts_controller.go b/hypershift-operator/controllers/webhookcerts/webhookcerts_controller.go index b2d3c7652b4..06a1e4e3659 100644 --- a/hypershift-operator/controllers/webhookcerts/webhookcerts_controller.go +++ b/hypershift-operator/controllers/webhookcerts/webhookcerts_controller.go @@ -364,3 +364,73 @@ 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") + + existingSecret := &corev1.Secret{} + err := c.Get(ctx, client.ObjectKey{Namespace: namespace, Name: ServingCertSecretName}, existingSecret) + if err != nil && !apierrors.IsNotFound(err) { + return fmt.Errorf("failed to check for existing serving cert secret: %w", err) + } + + // if secret exists check it is not empty + if err == nil { + if len(existingSecret.Data[corev1.TLSCertKey]) > 0 && len(existingSecret.Data[corev1.TLSPrivateKeyKey]) > 0 { + log.Info("Serving cert secret already exists, volume mount will provide certs") + return nil + } + log.Info("Serving cert secret exists but has empty data, regenerating") + } + + log.Info("Generating webhook certificates") + caSecret, servingSecret, _, err := GenerateInitialWebhookCerts(namespace, serviceName) + if err != nil { + return fmt.Errorf("failed to generate webhook certs: %w", err) + } + + if createErr := c.Create(ctx, caSecret); createErr != nil { + if !apierrors.IsAlreadyExists(createErr) { + return fmt.Errorf("failed to create CA secret: %w", createErr) + } + // CA already exists — fetch it and regenerate the serving cert so it + // is signed by the persisted CA, not the transient one we just created. + persistedCA := &corev1.Secret{} + if err := c.Get(ctx, client.ObjectKey{Namespace: namespace, Name: CASecretName}, persistedCA); err != nil { + return fmt.Errorf("failed to get existing CA secret: %w", err) + } + servingSecret.Data = map[string][]byte{} + dnsNames := webhookDNSNames(serviceName, namespace) + if err := certs.ReconcileSignedCert( + servingSecret, + persistedCA, + "hypershift-operator", + []string{"openshift"}, + []x509.ExtKeyUsage{x509.ExtKeyUsageServerAuth}, + corev1.TLSCertKey, + corev1.TLSPrivateKeyKey, + "", + dnsNames, + nil, + ); err != nil { + return fmt.Errorf("failed to regenerate serving cert from persisted CA: %w", err) + } + } + + if createErr := c.Create(ctx, servingSecret); createErr != nil { + if !apierrors.IsAlreadyExists(createErr) { + return fmt.Errorf("failed to create serving cert secret: %w", createErr) + } + existingSecret.Data = servingSecret.Data + if err := c.Update(ctx, existingSecret); err != nil { + return fmt.Errorf("failed to update serving cert secret: %w", err) + } + } + + log.Info("Webhook certificates bootstrapped") + return nil +} diff --git a/hypershift-operator/main.go b/hypershift-operator/main.go index 11e552f0a27..a663242bf0f 100644 --- a/hypershift-operator/main.go +++ b/hypershift-operator/main.go @@ -24,6 +24,7 @@ import ( hyperv1 "github.com/openshift/hypershift/api/hypershift/v1beta1" awsutil "github.com/openshift/hypershift/cmd/infra/aws/util" + "github.com/openshift/hypershift/cmd/install/assets" pkiconfig "github.com/openshift/hypershift/control-plane-pki-operator/config" etcdrecovery "github.com/openshift/hypershift/etcd-recovery" "github.com/openshift/hypershift/hypershift-operator/controllers/auditlogpersistence" @@ -274,6 +275,12 @@ func run(ctx context.Context, opts *StartOptions, log logr.Logger) error { return fmt.Errorf("failed to construct api reading client: %w", err) } + if opts.CertDir != "" { + if err := webhookcerts.EnsureWebhookCerts(ctx, apiReadingClient, opts.Namespace, assets.HypershiftOperatorName); err != nil { + return fmt.Errorf("failed to bootstrap webhook certs: %w", err) + } + } + if err := reconcileDeprecationValidatingAdmissionPolicy(ctx, apiReadingClient, mgmtClusterCaps, log); err != nil { return fmt.Errorf("failed to reconcile deprecation ValidatingAdmissionPolicy: %w", err) } From 1b868d86d9fcd1264b7a9eabfea5e1beaa506af4 Mon Sep 17 00:00:00 2001 From: Borja Clemente Date: Wed, 20 May 2026 12:04:40 +0200 Subject: [PATCH 2/3] test(unit): Add unit test for cert secret bootstrapping Test the new certs secret bootstrapping logic in the hypershift-operator. Signed-off-by: Borja Clemente --- .../webhookcerts/webhookcerts_controller.go | 82 ++++++------- .../webhookcerts_controller_test.go | 108 ++++++++++++++++++ 2 files changed, 143 insertions(+), 47 deletions(-) diff --git a/hypershift-operator/controllers/webhookcerts/webhookcerts_controller.go b/hypershift-operator/controllers/webhookcerts/webhookcerts_controller.go index 06a1e4e3659..79852656eec 100644 --- a/hypershift-operator/controllers/webhookcerts/webhookcerts_controller.go +++ b/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" @@ -372,19 +373,9 @@ func GenerateInitialWebhookCerts(namespace, serviceName string) (*corev1.Secret, func EnsureWebhookCerts(ctx context.Context, c client.Client, namespace, serviceName string) error { log := ctrl.LoggerFrom(ctx).WithName("webhook-cert-bootstrap") - existingSecret := &corev1.Secret{} - err := c.Get(ctx, client.ObjectKey{Namespace: namespace, Name: ServingCertSecretName}, existingSecret) - if err != nil && !apierrors.IsNotFound(err) { - return fmt.Errorf("failed to check for existing serving cert secret: %w", err) - } - - // if secret exists check it is not empty - if err == nil { - if len(existingSecret.Data[corev1.TLSCertKey]) > 0 && len(existingSecret.Data[corev1.TLSPrivateKeyKey]) > 0 { - log.Info("Serving cert secret already exists, volume mount will provide certs") - return nil - } - log.Info("Serving cert secret exists but has empty data, regenerating") + if certsExist(ctx, c, namespace) { + log.Info("Webhook cert secrets already exist with valid data, skipping bootstrap") + return nil } log.Info("Generating webhook certificates") @@ -393,44 +384,41 @@ func EnsureWebhookCerts(ctx context.Context, c client.Client, namespace, service return fmt.Errorf("failed to generate webhook certs: %w", err) } - if createErr := c.Create(ctx, caSecret); createErr != nil { - if !apierrors.IsAlreadyExists(createErr) { - return fmt.Errorf("failed to create CA secret: %w", createErr) - } - // CA already exists — fetch it and regenerate the serving cert so it - // is signed by the persisted CA, not the transient one we just created. - persistedCA := &corev1.Secret{} - if err := c.Get(ctx, client.ObjectKey{Namespace: namespace, Name: CASecretName}, persistedCA); err != nil { - return fmt.Errorf("failed to get existing CA secret: %w", err) - } - servingSecret.Data = map[string][]byte{} - dnsNames := webhookDNSNames(serviceName, namespace) - if err := certs.ReconcileSignedCert( - servingSecret, - persistedCA, - "hypershift-operator", - []string{"openshift"}, - []x509.ExtKeyUsage{x509.ExtKeyUsageServerAuth}, - corev1.TLSCertKey, - corev1.TLSPrivateKeyKey, - "", - dnsNames, - nil, - ); err != nil { - return fmt.Errorf("failed to regenerate serving cert from persisted CA: %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) } - if createErr := c.Create(ctx, servingSecret); createErr != nil { - if !apierrors.IsAlreadyExists(createErr) { - return fmt.Errorf("failed to create serving cert secret: %w", createErr) - } - existingSecret.Data = servingSecret.Data - if err := c.Update(ctx, existingSecret); err != nil { - return fmt.Errorf("failed to update serving cert 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 +} diff --git a/hypershift-operator/controllers/webhookcerts/webhookcerts_controller_test.go b/hypershift-operator/controllers/webhookcerts/webhookcerts_controller_test.go index ec96cf3dd5b..4dadf3ab127 100644 --- a/hypershift-operator/controllers/webhookcerts/webhookcerts_controller_test.go +++ b/hypershift-operator/controllers/webhookcerts/webhookcerts_controller_test.go @@ -2,6 +2,8 @@ package webhookcerts import ( "context" + "crypto/x509" + "encoding/pem" "testing" "time" @@ -406,7 +408,113 @@ func TestGenerateInitialWebhookCerts(t *testing.T) { g.Expect(servingSecret.Type).To(Equal(corev1.SecretTypeTLS)) g.Expect(servingSecret.Data).To(HaveKey(corev1.TLSCertKey)) g.Expect(servingSecret.Data).To(HaveKey(corev1.TLSPrivateKeyKey)) + }) +} + +func TestEnsureWebhookCerts(t *testing.T) { + t.Run("When no secrets exist it should create secrets", func(t *testing.T) { + g := NewWithT(t) + + cl := fake.NewClientBuilder().WithScheme(newScheme(t)).Build() + + err := EnsureWebhookCerts(t.Context(), cl, "hypershift", "operator") + g.Expect(err).ToNot(HaveOccurred()) + + caSecret := &corev1.Secret{} + g.Expect(cl.Get(t.Context(), client.ObjectKey{Name: CASecretName, Namespace: "hypershift"}, caSecret)).To(Succeed()) + g.Expect(caSecret.Data).To(HaveKey(certs.CASignerCertMapKey)) + + servingSecret := &corev1.Secret{} + g.Expect(cl.Get(t.Context(), client.ObjectKey{Name: ServingCertSecretName, Namespace: "hypershift"}, servingSecret)).To(Succeed()) + g.Expect(servingSecret.Data).To(HaveKey(corev1.TLSCertKey)) + g.Expect(servingSecret.Data).To(HaveKey(corev1.TLSPrivateKeyKey)) + g.Expect(servingSecret.Type).To(Equal(corev1.SecretTypeTLS)) + }) + t.Run("When secrets already exist with valid data it should not modify them", func(t *testing.T) { + g := NewWithT(t) + + caSecret, servingSecret, _, err := GenerateInitialWebhookCerts("hypershift", "operator") + g.Expect(err).ToNot(HaveOccurred()) + + cl := fake.NewClientBuilder().WithScheme(newScheme(t)).WithObjects(caSecret, servingSecret).Build() + + err = EnsureWebhookCerts(t.Context(), cl, "hypershift", "operator") + g.Expect(err).ToNot(HaveOccurred()) + + updatedServingSecret := &corev1.Secret{} + g.Expect(cl.Get(t.Context(), client.ObjectKey{Name: ServingCertSecretName, Namespace: "hypershift"}, updatedServingSecret)).To(Succeed()) + g.Expect(updatedServingSecret.Data[corev1.TLSCertKey]).To(Equal(servingSecret.Data[corev1.TLSCertKey])) + g.Expect(updatedServingSecret.Data[corev1.TLSPrivateKeyKey]).To(Equal(servingSecret.Data[corev1.TLSPrivateKeyKey])) + + updatedCASecret := &corev1.Secret{} + g.Expect(cl.Get(t.Context(), client.ObjectKey{Name: CASecretName, Namespace: "hypershift"}, updatedCASecret)).To(Succeed()) + g.Expect(updatedCASecret.Data[certs.CASignerCertMapKey]).To(Equal(caSecret.Data[certs.CASignerCertMapKey])) + g.Expect(updatedCASecret.Data[certs.CASignerKeyMapKey]).To(Equal(caSecret.Data[certs.CASignerKeyMapKey])) + }) + + t.Run("When serving cert secret exists with empty data it should regenerate and update", func(t *testing.T) { + g := NewWithT(t) + + emptySecret := &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: ServingCertSecretName, + Namespace: "hypershift", + }, + Type: corev1.SecretTypeTLS, + Data: map[string][]byte{}, + } + + cl := fake.NewClientBuilder().WithScheme(newScheme(t)).WithObjects(emptySecret).Build() + + err := EnsureWebhookCerts(t.Context(), cl, "hypershift", "operator") + g.Expect(err).ToNot(HaveOccurred()) + + updatedServingSecret := &corev1.Secret{} + g.Expect(cl.Get(t.Context(), client.ObjectKey{Name: ServingCertSecretName, Namespace: "hypershift"}, updatedServingSecret)).To(Succeed()) + g.Expect(updatedServingSecret.Data[corev1.TLSCertKey]).ToNot(BeEmpty()) + g.Expect(updatedServingSecret.Data[corev1.TLSPrivateKeyKey]).ToNot(BeEmpty()) + + updatedCASecret := &corev1.Secret{} + g.Expect(cl.Get(t.Context(), client.ObjectKey{Name: CASecretName, Namespace: "hypershift"}, updatedCASecret)).To(Succeed()) + g.Expect(updatedCASecret.Data[certs.CASignerCertMapKey]).ToNot(BeEmpty()) + g.Expect(updatedCASecret.Data[certs.CASignerKeyMapKey]).ToNot(BeEmpty()) + }) + + t.Run("When CA exists but serving cert is missing it should regenerate serving cert signed by persisted CA", func(t *testing.T) { + g := NewWithT(t) + + caSecret, _, _, err := GenerateInitialWebhookCerts("hypershift", "operator") + g.Expect(err).ToNot(HaveOccurred()) + originalCACert := caSecret.Data[certs.CASignerCertMapKey] + originalCAKey := caSecret.Data[certs.CASignerKeyMapKey] + + cl := fake.NewClientBuilder().WithScheme(newScheme(t)).WithObjects(caSecret).Build() + + err = EnsureWebhookCerts(t.Context(), cl, "hypershift", "operator") + g.Expect(err).ToNot(HaveOccurred()) + + // CA secret should not be overwritten. + updatedCASecret := &corev1.Secret{} + g.Expect(cl.Get(t.Context(), client.ObjectKey{Name: CASecretName, Namespace: "hypershift"}, updatedCASecret)).To(Succeed()) + g.Expect(updatedCASecret.Data[certs.CASignerCertMapKey]).To(Equal(originalCACert)) + g.Expect(updatedCASecret.Data[certs.CASignerKeyMapKey]).To(Equal(originalCAKey)) + + // Serving cert should exist with valid TLS data. + servingSecret := &corev1.Secret{} + g.Expect(cl.Get(t.Context(), client.ObjectKey{Name: ServingCertSecretName, Namespace: "hypershift"}, servingSecret)).To(Succeed()) + g.Expect(servingSecret.Data[corev1.TLSCertKey]).ToNot(BeEmpty()) + g.Expect(servingSecret.Data[corev1.TLSPrivateKeyKey]).ToNot(BeEmpty()) + + // Verify the serving cert was signed by the persisted CA. + caPool := x509.NewCertPool() + g.Expect(caPool.AppendCertsFromPEM(originalCACert)).To(BeTrue()) + block, _ := pem.Decode(servingSecret.Data[corev1.TLSCertKey]) + g.Expect(block).ToNot(BeNil()) + leaf, err := x509.ParseCertificate(block.Bytes) + g.Expect(err).ToNot(HaveOccurred()) + _, err = leaf.Verify(x509.VerifyOptions{Roots: caPool}) + g.Expect(err).ToNot(HaveOccurred()) }) } From e1993ba8576e87c01ac4ecd49da0ff0bc451bae4 Mon Sep 17 00:00:00 2001 From: Borja Clemente Date: Wed, 20 May 2026 12:05:35 +0200 Subject: [PATCH 3/3] refactor(cli): Remove the cert secret bootstrapping from the CLI Cert secret bootstrapping now lives in the hypershift_operator and the CLI no longer needs to create the secrets. Signed-off-by: Borja Clemente --- cmd/install/assets/hypershift_operator.go | 14 +++--- cmd/install/install.go | 20 ++------- cmd/install/install_test.go | 43 ++++++++++++++++++- .../webhookcerts_controller_test.go | 15 +++---- 4 files changed, 57 insertions(+), 35 deletions(-) diff --git a/cmd/install/assets/hypershift_operator.go b/cmd/install/assets/hypershift_operator.go index 20a64e11d95..3163fdcbaed 100644 --- a/cmd/install/assets/hypershift_operator.go +++ b/cmd/install/assets/hypershift_operator.go @@ -2052,7 +2052,6 @@ func (o HyperShiftReaderClusterRoleBinding) Build() *rbacv1.ClusterRoleBinding { type HyperShiftMutatingWebhookConfiguration struct { Namespace *corev1.Namespace EnableAuditLogPersistence bool - CABundle []byte } const ( @@ -2091,7 +2090,7 @@ func (o HyperShiftMutatingWebhookConfiguration) Build() *admissionregistrationv1 }, }, ClientConfig: admissionregistrationv1.WebhookClientConfig{ - CABundle: o.CABundle, + CABundle: nil, Service: &admissionregistrationv1.ServiceReference{ Namespace: "hypershift", Name: "operator", @@ -2118,7 +2117,7 @@ func (o HyperShiftMutatingWebhookConfiguration) Build() *admissionregistrationv1 }, }, ClientConfig: admissionregistrationv1.WebhookClientConfig{ - CABundle: o.CABundle, + CABundle: nil, Service: &admissionregistrationv1.ServiceReference{ Namespace: "hypershift", Name: "operator", @@ -2159,7 +2158,7 @@ func (o HyperShiftMutatingWebhookConfiguration) Build() *admissionregistrationv1 }, }, ClientConfig: admissionregistrationv1.WebhookClientConfig{ - CABundle: o.CABundle, + CABundle: nil, Service: &admissionregistrationv1.ServiceReference{ Namespace: "hypershift", Name: "operator", @@ -2189,7 +2188,7 @@ func (o HyperShiftMutatingWebhookConfiguration) Build() *admissionregistrationv1 }, }, ClientConfig: admissionregistrationv1.WebhookClientConfig{ - CABundle: o.CABundle, + CABundle: nil, Service: &admissionregistrationv1.ServiceReference{ Namespace: "hypershift", Name: "operator", @@ -2210,7 +2209,6 @@ func (o HyperShiftMutatingWebhookConfiguration) Build() *admissionregistrationv1 type HyperShiftValidatingWebhookConfiguration struct { Namespace string - CABundle []byte } func (o HyperShiftValidatingWebhookConfiguration) Build() *admissionregistrationv1.ValidatingWebhookConfiguration { @@ -2247,7 +2245,7 @@ func (o HyperShiftValidatingWebhookConfiguration) Build() *admissionregistration }, }, ClientConfig: admissionregistrationv1.WebhookClientConfig{ - CABundle: o.CABundle, + CABundle: nil, Service: &admissionregistrationv1.ServiceReference{ Namespace: "hypershift", Name: "operator", @@ -2276,7 +2274,7 @@ func (o HyperShiftValidatingWebhookConfiguration) Build() *admissionregistration }, }, ClientConfig: admissionregistrationv1.WebhookClientConfig{ - CABundle: o.CABundle, + CABundle: nil, Service: &admissionregistrationv1.ServiceReference{ Namespace: "hypershift", Name: "operator", diff --git a/cmd/install/install.go b/cmd/install/install.go index 2afbf660900..6d9589a2aef 100644 --- a/cmd/install/install.go +++ b/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,22 +755,10 @@ 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) } @@ -779,7 +766,6 @@ func hyperShiftOperatorManifests(ctx context.Context, client crclient.Client, op 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"}, }, diff --git a/cmd/install/install_test.go b/cmd/install/install_test.go index 33375544104..61052faf3a4 100644 --- a/cmd/install/install_test.go +++ b/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 diff --git a/hypershift-operator/controllers/webhookcerts/webhookcerts_controller_test.go b/hypershift-operator/controllers/webhookcerts/webhookcerts_controller_test.go index 4dadf3ab127..c125b5887a7 100644 --- a/hypershift-operator/controllers/webhookcerts/webhookcerts_controller_test.go +++ b/hypershift-operator/controllers/webhookcerts/webhookcerts_controller_test.go @@ -481,34 +481,31 @@ func TestEnsureWebhookCerts(t *testing.T) { g.Expect(updatedCASecret.Data[certs.CASignerKeyMapKey]).ToNot(BeEmpty()) }) - t.Run("When CA exists but serving cert is missing it should regenerate serving cert signed by persisted CA", func(t *testing.T) { + t.Run("When CA exists but serving cert is missing it should regenerate both secrets", func(t *testing.T) { g := NewWithT(t) caSecret, _, _, err := GenerateInitialWebhookCerts("hypershift", "operator") g.Expect(err).ToNot(HaveOccurred()) - originalCACert := caSecret.Data[certs.CASignerCertMapKey] - originalCAKey := caSecret.Data[certs.CASignerKeyMapKey] cl := fake.NewClientBuilder().WithScheme(newScheme(t)).WithObjects(caSecret).Build() err = EnsureWebhookCerts(t.Context(), cl, "hypershift", "operator") g.Expect(err).ToNot(HaveOccurred()) - // CA secret should not be overwritten. + // Both secrets should exist with valid data. updatedCASecret := &corev1.Secret{} g.Expect(cl.Get(t.Context(), client.ObjectKey{Name: CASecretName, Namespace: "hypershift"}, updatedCASecret)).To(Succeed()) - g.Expect(updatedCASecret.Data[certs.CASignerCertMapKey]).To(Equal(originalCACert)) - g.Expect(updatedCASecret.Data[certs.CASignerKeyMapKey]).To(Equal(originalCAKey)) + g.Expect(updatedCASecret.Data[certs.CASignerCertMapKey]).ToNot(BeEmpty()) + g.Expect(updatedCASecret.Data[certs.CASignerKeyMapKey]).ToNot(BeEmpty()) - // Serving cert should exist with valid TLS data. servingSecret := &corev1.Secret{} g.Expect(cl.Get(t.Context(), client.ObjectKey{Name: ServingCertSecretName, Namespace: "hypershift"}, servingSecret)).To(Succeed()) g.Expect(servingSecret.Data[corev1.TLSCertKey]).ToNot(BeEmpty()) g.Expect(servingSecret.Data[corev1.TLSPrivateKeyKey]).ToNot(BeEmpty()) - // Verify the serving cert was signed by the persisted CA. + // Verify the serving cert was signed by the (possibly replaced) CA. caPool := x509.NewCertPool() - g.Expect(caPool.AppendCertsFromPEM(originalCACert)).To(BeTrue()) + g.Expect(caPool.AppendCertsFromPEM(updatedCASecret.Data[certs.CASignerCertMapKey])).To(BeTrue()) block, _ := pem.Decode(servingSecret.Data[corev1.TLSCertKey]) g.Expect(block).ToNot(BeNil()) leaf, err := x509.ParseCertificate(block.Bytes)