diff --git a/cmd/install/assets/hypershift_operator.go b/cmd/install/assets/hypershift_operator.go index cf3897679b3f..458e4ce3637e 100644 --- a/cmd/install/assets/hypershift_operator.go +++ b/cmd/install/assets/hypershift_operator.go @@ -1065,9 +1065,6 @@ func (o HyperShiftOperatorService) Build() *corev1.Service { Labels: map[string]string{ "name": HypershiftOperatorName, }, - Annotations: map[string]string{ - "service.beta.openshift.io/serving-cert-secret-name": "manager-serving-cert", - }, }, Spec: corev1.ServiceSpec{ Type: corev1.ServiceTypeClusterIP, @@ -2075,6 +2072,7 @@ func (o HyperShiftReaderClusterRoleBinding) Build() *rbacv1.ClusterRoleBinding { type HyperShiftMutatingWebhookConfiguration struct { Namespace *corev1.Namespace EnableAuditLogPersistence bool + CABundle []byte } const ( @@ -2095,9 +2093,6 @@ func (o HyperShiftMutatingWebhookConfiguration) Build() *admissionregistrationv1 ObjectMeta: metav1.ObjectMeta{ Namespace: o.Namespace.Name, Name: hyperv1.GroupVersion.Group, - Annotations: map[string]string{ - "service.beta.openshift.io/inject-cabundle": "true", - }, }, Webhooks: []admissionregistrationv1.MutatingWebhook{ { @@ -2116,6 +2111,7 @@ func (o HyperShiftMutatingWebhookConfiguration) Build() *admissionregistrationv1 }, }, ClientConfig: admissionregistrationv1.WebhookClientConfig{ + CABundle: o.CABundle, Service: &admissionregistrationv1.ServiceReference{ Namespace: "hypershift", Name: "operator", @@ -2142,6 +2138,7 @@ func (o HyperShiftMutatingWebhookConfiguration) Build() *admissionregistrationv1 }, }, ClientConfig: admissionregistrationv1.WebhookClientConfig{ + CABundle: o.CABundle, Service: &admissionregistrationv1.ServiceReference{ Namespace: "hypershift", Name: "operator", @@ -2182,6 +2179,7 @@ func (o HyperShiftMutatingWebhookConfiguration) Build() *admissionregistrationv1 }, }, ClientConfig: admissionregistrationv1.WebhookClientConfig{ + CABundle: o.CABundle, Service: &admissionregistrationv1.ServiceReference{ Namespace: "hypershift", Name: "operator", @@ -2211,6 +2209,7 @@ func (o HyperShiftMutatingWebhookConfiguration) Build() *admissionregistrationv1 }, }, ClientConfig: admissionregistrationv1.WebhookClientConfig{ + CABundle: o.CABundle, Service: &admissionregistrationv1.ServiceReference{ Namespace: "hypershift", Name: "operator", @@ -2231,6 +2230,7 @@ func (o HyperShiftMutatingWebhookConfiguration) Build() *admissionregistrationv1 type HyperShiftValidatingWebhookConfiguration struct { Namespace string + CABundle []byte } func (o HyperShiftValidatingWebhookConfiguration) Build() *admissionregistrationv1.ValidatingWebhookConfiguration { @@ -2248,9 +2248,6 @@ func (o HyperShiftValidatingWebhookConfiguration) Build() *admissionregistration ObjectMeta: metav1.ObjectMeta{ Namespace: o.Namespace, Name: hyperv1.GroupVersion.Group, - Annotations: map[string]string{ - "service.beta.openshift.io/inject-cabundle": "true", - }, }, Webhooks: []admissionregistrationv1.ValidatingWebhook{ { @@ -2270,6 +2267,7 @@ func (o HyperShiftValidatingWebhookConfiguration) Build() *admissionregistration }, }, ClientConfig: admissionregistrationv1.WebhookClientConfig{ + CABundle: o.CABundle, Service: &admissionregistrationv1.ServiceReference{ Namespace: "hypershift", Name: "operator", @@ -2298,6 +2296,7 @@ func (o HyperShiftValidatingWebhookConfiguration) Build() *admissionregistration }, }, ClientConfig: admissionregistrationv1.WebhookClientConfig{ + CABundle: o.CABundle, Service: &admissionregistrationv1.ServiceReference{ Namespace: "hypershift", Name: "operator", diff --git a/cmd/install/install.go b/cmd/install/install.go index c1079477b640..2b15a91c6407 100644 --- a/cmd/install/install.go +++ b/cmd/install/install.go @@ -28,6 +28,7 @@ import ( "github.com/openshift/hypershift/cmd/install/assets" "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" @@ -678,10 +679,22 @@ 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) } @@ -689,6 +702,7 @@ 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) } @@ -740,7 +754,7 @@ func hyperShiftOperatorManifests(ctx context.Context, client crclient.Client, op objects = append(objects, sharedIngressObjs...) } - crds, err = setupCRDs(ctx, client, opts, operatorNamespace, operatorService) + crds, err = setupCRDs(ctx, client, opts, operatorNamespace, operatorService, webhookCABundle) if err != nil { return nil, nil, err } @@ -783,7 +797,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) ([]crclient.Object, error) { +func setupCRDs(ctx context.Context, client crclient.Client, opts Options, operatorNamespace *corev1.Namespace, operatorService *corev1.Service, webhookCABundle []byte) ([]crclient.Object, error) { // Build a set of existing IPAM CRDs if a client is available existingIPAMCRDs := set.New[string]() if client != nil { @@ -866,7 +880,6 @@ func setupCRDs(ctx context.Context, client crclient.Client, opts Options, operat if crd.Annotations != nil { crd.Annotations = map[string]string{} } - crd.Annotations["service.beta.openshift.io/inject-cabundle"] = "true" crd.Spec.Conversion = &apiextensionsv1.CustomResourceConversion{ Strategy: apiextensionsv1.WebhookConverter, Webhook: &apiextensionsv1.WebhookConversion{ @@ -877,6 +890,7 @@ func setupCRDs(ctx context.Context, client crclient.Client, opts Options, operat Port: ptr.To[int32](443), Path: ptr.To("/convert"), }, + CABundle: webhookCABundle, }, ConversionReviewVersions: []string{"v1beta1", "v1alpha1"}, }, diff --git a/cmd/install/install_test.go b/cmd/install/install_test.go index 347552d3c420..373b6948ba49 100644 --- a/cmd/install/install_test.go +++ b/cmd/install/install_test.go @@ -394,7 +394,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) + crds, err := setupCRDs(t.Context(), nil, tc.inputOptions, &corev1.Namespace{}, nil, nil) g.Expect(err).ToNot(HaveOccurred()) nodePoolCRDS := make([]crclient.Object, 0) var machineDeploymentCRD crclient.Object diff --git a/hypershift-operator/controllers/webhookcerts/webhookcerts_controller.go b/hypershift-operator/controllers/webhookcerts/webhookcerts_controller.go new file mode 100644 index 000000000000..34c8d693c88b --- /dev/null +++ b/hypershift-operator/controllers/webhookcerts/webhookcerts_controller.go @@ -0,0 +1,254 @@ +/* +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 webhookcerts + +import ( + "bytes" + "context" + "crypto/x509" + "fmt" + "time" + + "github.com/openshift/hypershift/support/certs" + "github.com/openshift/hypershift/support/upsert" + + admissionregistrationv1 "k8s.io/api/admissionregistration/v1" + corev1 "k8s.io/api/core/v1" + apiextensionsv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1" + apierrors "k8s.io/apimachinery/pkg/api/errors" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + + 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/predicate" +) + +const ( + // CASecretName is the name of the Secret containing the self-signed CA used to sign webhook serving certs. + CASecretName = "webhook-serving-ca" + // ServingCertSecretName is the name of the Secret containing the serving cert for the webhook server. + ServingCertSecretName = "manager-serving-cert" + + requeueInterval = 12 * time.Hour +) + +// 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. +type WebhookCertReconciler struct { + Client client.Client + Namespace string + ServiceName string + createOrUpdate upsert.CreateOrUpdateFN +} + +func (r *WebhookCertReconciler) SetupWithManager(mgr ctrl.Manager, createOrUpdate upsert.CreateOrUpdateProvider) error { + r.Client = mgr.GetClient() + r.createOrUpdate = createOrUpdate.CreateOrUpdate + + 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) + }))). + Complete(r) +} + +func (r *WebhookCertReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Result, error) { + log := ctrl.LoggerFrom(ctx) + + // 1. Reconcile the self-signed CA. + caSecret := &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: CASecretName, + Namespace: r.Namespace, + }, + } + if _, err := r.createOrUpdate(ctx, r.Client, caSecret, func() error { + 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) + } + + // 2. Reconcile the serving cert signed by the CA. + dnsNames := webhookDNSNames(r.ServiceName, r.Namespace) + servingSecret := &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: ServingCertSecretName, + Namespace: r.Namespace, + }, + } + if _, err := r.createOrUpdate(ctx, r.Client, servingSecret, func() error { + servingSecret.Type = corev1.SecretTypeTLS + return certs.ReconcileSignedCert( + servingSecret, + caSecret, + "hypershift-operator", + []string{"openshift"}, + []x509.ExtKeyUsage{x509.ExtKeyUsageServerAuth}, + corev1.TLSCertKey, + corev1.TLSPrivateKeyKey, + "", + dnsNames, + nil, + ) + }); err != nil { + return ctrl.Result{}, 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) + } + + // 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) + } + + log.Info("Webhook certs reconciled", "requeueAfter", requeueInterval) + return ctrl.Result{RequeueAfter: requeueInterval}, nil +} + +// patchCRDsCABundle patches the caBundle on all CRDs whose conversion webhook points to our service. +func (r *WebhookCertReconciler) patchCRDsCABundle(ctx context.Context, caBundle []byte) error { + crdList := &apiextensionsv1.CustomResourceDefinitionList{} + if err := r.Client.List(ctx, crdList); err != nil { + return fmt.Errorf("failed to list CRDs: %w", err) + } + + for i := range crdList.Items { + crd := &crdList.Items[i] + if crd.Spec.Conversion == nil || crd.Spec.Conversion.Webhook == nil || crd.Spec.Conversion.Webhook.ClientConfig == nil { + continue + } + svc := crd.Spec.Conversion.Webhook.ClientConfig.Service + if svc == nil || svc.Name != r.ServiceName || svc.Namespace != r.Namespace { + continue + } + if bytes.Equal(crd.Spec.Conversion.Webhook.ClientConfig.CABundle, caBundle) { + continue + } + patch := client.MergeFrom(crd.DeepCopy()) + crd.Spec.Conversion.Webhook.ClientConfig.CABundle = caBundle + if err := r.Client.Patch(ctx, crd, patch); err != nil { + return fmt.Errorf("failed to patch CRD %s caBundle: %w", crd.Name, err) + } + } + return nil +} + +// patchWebhookConfigsCABundle patches the caBundle on MutatingWebhookConfiguration and ValidatingWebhookConfiguration +// resources named hypershift.openshift.io. +func (r *WebhookCertReconciler) patchWebhookConfigsCABundle(ctx context.Context, caBundle []byte) error { + webhookName := "hypershift.openshift.io" + + // Patch MutatingWebhookConfiguration + mwc := &admissionregistrationv1.MutatingWebhookConfiguration{} + if err := r.Client.Get(ctx, client.ObjectKey{Name: webhookName}, mwc); err != nil { + if !apierrors.IsNotFound(err) { + return fmt.Errorf("failed to get MutatingWebhookConfiguration: %w", err) + } + } else { + needsPatch := false + for i := range mwc.Webhooks { + if !bytes.Equal(mwc.Webhooks[i].ClientConfig.CABundle, caBundle) { + needsPatch = true + mwc.Webhooks[i].ClientConfig.CABundle = caBundle + } + } + if needsPatch { + if err := r.Client.Update(ctx, mwc); err != nil { + return fmt.Errorf("failed to update MutatingWebhookConfiguration: %w", err) + } + } + } + + // Patch ValidatingWebhookConfiguration + vwc := &admissionregistrationv1.ValidatingWebhookConfiguration{} + if err := r.Client.Get(ctx, client.ObjectKey{Name: webhookName}, vwc); err != nil { + if !apierrors.IsNotFound(err) { + return fmt.Errorf("failed to get ValidatingWebhookConfiguration: %w", err) + } + } else { + needsPatch := false + for i := range vwc.Webhooks { + if !bytes.Equal(vwc.Webhooks[i].ClientConfig.CABundle, caBundle) { + needsPatch = true + vwc.Webhooks[i].ClientConfig.CABundle = caBundle + } + } + if needsPatch { + if err := r.Client.Update(ctx, vwc); err != nil { + return fmt.Errorf("failed to update ValidatingWebhookConfiguration: %w", err) + } + } + } + + return nil +} + +// webhookDNSNames returns the DNS names for the webhook serving cert. +func webhookDNSNames(serviceName, namespace string) []string { + return []string{ + fmt.Sprintf("%s.%s.svc", serviceName, namespace), + fmt.Sprintf("%s.%s.svc.cluster.local", serviceName, namespace), + } +} + +// GenerateInitialWebhookCerts generates the CA and serving cert secrets for use at install time. +// It also returns the CA bundle bytes for injection into CRDs and webhook configs. +func GenerateInitialWebhookCerts(namespace, serviceName string) (*corev1.Secret, *corev1.Secret, []byte, error) { + caSecret := &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: CASecretName, + Namespace: namespace, + }, + Data: map[string][]byte{}, + } + if err := certs.ReconcileSelfSignedCA(caSecret, "hypershift-webhook-ca", "openshift"); err != nil { + return nil, nil, nil, fmt.Errorf("failed to generate webhook CA: %w", err) + } + + dnsNames := webhookDNSNames(serviceName, namespace) + servingSecret := &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: ServingCertSecretName, + Namespace: namespace, + }, + Type: corev1.SecretTypeTLS, + Data: map[string][]byte{}, + } + if err := certs.ReconcileSignedCert( + servingSecret, + caSecret, + "hypershift-operator", + []string{"openshift"}, + []x509.ExtKeyUsage{x509.ExtKeyUsageServerAuth}, + corev1.TLSCertKey, + corev1.TLSPrivateKeyKey, + "", + dnsNames, + nil, + ); err != nil { + return nil, nil, nil, fmt.Errorf("failed to generate webhook serving cert: %w", err) + } + + caBundle := caSecret.Data[certs.CASignerCertMapKey] + return caSecret, servingSecret, caBundle, nil +} diff --git a/hypershift-operator/controllers/webhookcerts/webhookcerts_controller_test.go b/hypershift-operator/controllers/webhookcerts/webhookcerts_controller_test.go new file mode 100644 index 000000000000..ddd9ec6c5b29 --- /dev/null +++ b/hypershift-operator/controllers/webhookcerts/webhookcerts_controller_test.go @@ -0,0 +1,273 @@ +package webhookcerts + +import ( + "context" + "testing" + "time" + + . "github.com/onsi/gomega" + + "github.com/openshift/hypershift/support/certs" + + admissionregistrationv1 "k8s.io/api/admissionregistration/v1" + corev1 "k8s.io/api/core/v1" + apiextensionsv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/runtime" + + ctrl "sigs.k8s.io/controller-runtime" + "sigs.k8s.io/controller-runtime/pkg/client" + "sigs.k8s.io/controller-runtime/pkg/client/fake" + "sigs.k8s.io/controller-runtime/pkg/controller/controllerutil" +) + +func newScheme(t *testing.T) *runtime.Scheme { + t.Helper() + g := NewWithT(t) + s := runtime.NewScheme() + g.Expect(corev1.AddToScheme(s)).To(Succeed()) + g.Expect(admissionregistrationv1.AddToScheme(s)).To(Succeed()) + g.Expect(apiextensionsv1.AddToScheme(s)).To(Succeed()) + return s +} + +func newReconciler(cl client.Client) *WebhookCertReconciler { + return &WebhookCertReconciler{ + Client: cl, + Namespace: "hypershift", + ServiceName: "operator", + createOrUpdate: func(ctx context.Context, c client.Client, obj client.Object, f controllerutil.MutateFn) (controllerutil.OperationResult, error) { + return controllerutil.CreateOrUpdate(ctx, c, obj, f) + }, + } +} + +func caRequest() ctrl.Request { + return ctrl.Request{NamespacedName: client.ObjectKey{Name: CASecretName, Namespace: "hypershift"}} +} + +func TestReconcile(t *testing.T) { + t.Run("When no secrets exist it should create the CA and serving cert secrets", func(t *testing.T) { + g := NewWithT(t) + + cl := fake.NewClientBuilder().WithScheme(newScheme(t)).Build() + r := newReconciler(cl) + + result, err := r.Reconcile(t.Context(), caRequest()) + g.Expect(err).ToNot(HaveOccurred()) + g.Expect(result.RequeueAfter).To(Equal(12 * time.Hour)) + + // CA secret should exist with CA cert data. + 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)) + g.Expect(caSecret.Type).To(Equal(corev1.SecretTypeOpaque)) + + // Serving cert secret should exist with TLS cert data. + 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 it should not error and should requeue", func(t *testing.T) { + g := NewWithT(t) + + // Pre-create valid secrets via GenerateInitialWebhookCerts. + caSecret, servingSecret, _, err := GenerateInitialWebhookCerts("hypershift", "operator") + g.Expect(err).ToNot(HaveOccurred()) + + cl := fake.NewClientBuilder().WithScheme(newScheme(t)).WithObjects(caSecret, servingSecret).Build() + r := newReconciler(cl) + + result, err := r.Reconcile(t.Context(), caRequest()) + g.Expect(err).ToNot(HaveOccurred()) + g.Expect(result.RequeueAfter).To(Equal(12 * time.Hour)) + }) + + t.Run("When a CRD has a conversion webhook pointing to our service it should patch its caBundle", func(t *testing.T) { + g := NewWithT(t) + + crd := &apiextensionsv1.CustomResourceDefinition{ + ObjectMeta: metav1.ObjectMeta{Name: "hostedclusters.hypershift.openshift.io"}, + Spec: apiextensionsv1.CustomResourceDefinitionSpec{ + Group: "hypershift.openshift.io", + Names: apiextensionsv1.CustomResourceDefinitionNames{ + Plural: "hostedclusters", + Singular: "hostedcluster", + Kind: "HostedCluster", + }, + Scope: apiextensionsv1.NamespaceScoped, + Versions: []apiextensionsv1.CustomResourceDefinitionVersion{ + {Name: "v1beta1", Served: true, Storage: true, Schema: &apiextensionsv1.CustomResourceValidation{ + OpenAPIV3Schema: &apiextensionsv1.JSONSchemaProps{Type: "object"}, + }}, + }, + Conversion: &apiextensionsv1.CustomResourceConversion{ + Strategy: apiextensionsv1.WebhookConverter, + Webhook: &apiextensionsv1.WebhookConversion{ + ClientConfig: &apiextensionsv1.WebhookClientConfig{ + CABundle: []byte("old-ca"), + Service: &apiextensionsv1.ServiceReference{ + Namespace: "hypershift", + Name: "operator", + }, + }, + ConversionReviewVersions: []string{"v1beta1"}, + }, + }, + }, + } + + cl := fake.NewClientBuilder().WithScheme(newScheme(t)).WithObjects(crd).Build() + r := newReconciler(cl) + + _, err := r.Reconcile(t.Context(), caRequest()) + g.Expect(err).ToNot(HaveOccurred()) + + updatedCRD := &apiextensionsv1.CustomResourceDefinition{} + g.Expect(cl.Get(t.Context(), client.ObjectKey{Name: crd.Name}, updatedCRD)).To(Succeed()) + g.Expect(updatedCRD.Spec.Conversion.Webhook.ClientConfig.CABundle).ToNot(Equal([]byte("old-ca"))) + g.Expect(updatedCRD.Spec.Conversion.Webhook.ClientConfig.CABundle).ToNot(BeEmpty()) + }) + + t.Run("When a CRD conversion webhook points to a different service it should not patch its caBundle", func(t *testing.T) { + g := NewWithT(t) + + crd := &apiextensionsv1.CustomResourceDefinition{ + ObjectMeta: metav1.ObjectMeta{Name: "others.example.io"}, + Spec: apiextensionsv1.CustomResourceDefinitionSpec{ + Group: "example.io", + Names: apiextensionsv1.CustomResourceDefinitionNames{ + Plural: "others", + Singular: "other", + Kind: "Other", + }, + Scope: apiextensionsv1.NamespaceScoped, + Versions: []apiextensionsv1.CustomResourceDefinitionVersion{ + {Name: "v1", Served: true, Storage: true, Schema: &apiextensionsv1.CustomResourceValidation{ + OpenAPIV3Schema: &apiextensionsv1.JSONSchemaProps{Type: "object"}, + }}, + }, + Conversion: &apiextensionsv1.CustomResourceConversion{ + Strategy: apiextensionsv1.WebhookConverter, + Webhook: &apiextensionsv1.WebhookConversion{ + ClientConfig: &apiextensionsv1.WebhookClientConfig{ + CABundle: []byte("unchanged"), + Service: &apiextensionsv1.ServiceReference{ + Namespace: "other-ns", + Name: "other-service", + }, + }, + ConversionReviewVersions: []string{"v1"}, + }, + }, + }, + } + + cl := fake.NewClientBuilder().WithScheme(newScheme(t)).WithObjects(crd).Build() + r := newReconciler(cl) + + _, err := r.Reconcile(t.Context(), caRequest()) + g.Expect(err).ToNot(HaveOccurred()) + + updatedCRD := &apiextensionsv1.CustomResourceDefinition{} + g.Expect(cl.Get(t.Context(), client.ObjectKey{Name: crd.Name}, updatedCRD)).To(Succeed()) + g.Expect(updatedCRD.Spec.Conversion.Webhook.ClientConfig.CABundle).To(Equal([]byte("unchanged"))) + }) + + t.Run("When webhook configurations exist it should patch their caBundle", func(t *testing.T) { + g := NewWithT(t) + + mwc := &admissionregistrationv1.MutatingWebhookConfiguration{ + ObjectMeta: metav1.ObjectMeta{Name: "hypershift.openshift.io"}, + Webhooks: []admissionregistrationv1.MutatingWebhook{ + { + Name: "defaulting.hypershift.openshift.io", + ClientConfig: admissionregistrationv1.WebhookClientConfig{CABundle: []byte("old")}, + SideEffects: sideEffectNone(), + AdmissionReviewVersions: []string{"v1"}, + }, + }, + } + vwc := &admissionregistrationv1.ValidatingWebhookConfiguration{ + ObjectMeta: metav1.ObjectMeta{Name: "hypershift.openshift.io"}, + Webhooks: []admissionregistrationv1.ValidatingWebhook{ + { + Name: "validating.hypershift.openshift.io", + ClientConfig: admissionregistrationv1.WebhookClientConfig{CABundle: []byte("old")}, + SideEffects: sideEffectNone(), + AdmissionReviewVersions: []string{"v1"}, + }, + }, + } + + cl := fake.NewClientBuilder().WithScheme(newScheme(t)).WithObjects(mwc, vwc).Build() + r := newReconciler(cl) + + _, err := r.Reconcile(t.Context(), caRequest()) + g.Expect(err).ToNot(HaveOccurred()) + + updatedMWC := &admissionregistrationv1.MutatingWebhookConfiguration{} + g.Expect(cl.Get(t.Context(), client.ObjectKey{Name: "hypershift.openshift.io"}, updatedMWC)).To(Succeed()) + g.Expect(updatedMWC.Webhooks[0].ClientConfig.CABundle).ToNot(Equal([]byte("old"))) + g.Expect(updatedMWC.Webhooks[0].ClientConfig.CABundle).ToNot(BeEmpty()) + + updatedVWC := &admissionregistrationv1.ValidatingWebhookConfiguration{} + g.Expect(cl.Get(t.Context(), client.ObjectKey{Name: "hypershift.openshift.io"}, updatedVWC)).To(Succeed()) + g.Expect(updatedVWC.Webhooks[0].ClientConfig.CABundle).ToNot(Equal([]byte("old"))) + g.Expect(updatedVWC.Webhooks[0].ClientConfig.CABundle).ToNot(BeEmpty()) + }) + + t.Run("When webhook configurations do not exist it should not error", func(t *testing.T) { + g := NewWithT(t) + + cl := fake.NewClientBuilder().WithScheme(newScheme(t)).Build() + r := newReconciler(cl) + + _, err := r.Reconcile(t.Context(), caRequest()) + g.Expect(err).ToNot(HaveOccurred()) + }) +} + +func TestGenerateInitialWebhookCerts(t *testing.T) { + t.Run("When generating certs it should return valid CA and serving cert secrets", func(t *testing.T) { + g := NewWithT(t) + + caSecret, servingSecret, caBundle, err := GenerateInitialWebhookCerts("hypershift", "operator") + g.Expect(err).ToNot(HaveOccurred()) + + // CA secret + g.Expect(caSecret.Name).To(Equal(CASecretName)) + g.Expect(caSecret.Namespace).To(Equal("hypershift")) + g.Expect(caSecret.Data).To(HaveKey(certs.CASignerCertMapKey)) + g.Expect(caBundle).ToNot(BeEmpty()) + g.Expect(caBundle).To(Equal(caSecret.Data[certs.CASignerCertMapKey])) + + // Serving cert secret + g.Expect(servingSecret.Name).To(Equal(ServingCertSecretName)) + g.Expect(servingSecret.Namespace).To(Equal("hypershift")) + 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 TestWebhookDNSNames(t *testing.T) { + t.Run("When given service name and namespace it should return correct DNS names", func(t *testing.T) { + g := NewWithT(t) + + names := webhookDNSNames("operator", "hypershift") + g.Expect(names).To(ConsistOf( + "operator.hypershift.svc", + "operator.hypershift.svc.cluster.local", + )) + }) +} + +func sideEffectNone() *admissionregistrationv1.SideEffectClass { + se := admissionregistrationv1.SideEffectClassNone + return &se +} diff --git a/hypershift-operator/main.go b/hypershift-operator/main.go index c1a85a3040a0..d5276240438d 100644 --- a/hypershift-operator/main.go +++ b/hypershift-operator/main.go @@ -44,6 +44,7 @@ import ( sharedingress "github.com/openshift/hypershift/hypershift-operator/controllers/sharedingress" hosupportedversion "github.com/openshift/hypershift/hypershift-operator/controllers/supportedversion" "github.com/openshift/hypershift/hypershift-operator/controllers/uwmtelemetry" + "github.com/openshift/hypershift/hypershift-operator/controllers/webhookcerts" "github.com/openshift/hypershift/hypershift-operator/featuregate" kvinfra "github.com/openshift/hypershift/kubevirtexternalinfra" sharedingressconfiggenerator "github.com/openshift/hypershift/sharedingress-config-generator" @@ -643,6 +644,16 @@ func run(ctx context.Context, opts *StartOptions, log logr.Logger) error { } } + if opts.CertDir != "" { + webhookCertReconciler := &webhookcerts.WebhookCertReconciler{ + Namespace: opts.Namespace, + ServiceName: "operator", + } + if err := webhookCertReconciler.SetupWithManager(mgr, createOrUpdate); err != nil { + return fmt.Errorf("unable to create webhook cert controller: %w", err) + } + } + // Start controllers to manage dedicated request serving isolation if opts.EnableDedicatedRequestServingIsolation && !azureutil.IsAroHCP() { // Use the new scheduler if we support size tagging on hosted clusters