diff --git a/control-plane-operator/controllers/hostedcontrolplane/hostedcontrolplane_controller.go b/control-plane-operator/controllers/hostedcontrolplane/hostedcontrolplane_controller.go index 3df07330720b..2adf92304dfd 100644 --- a/control-plane-operator/controllers/hostedcontrolplane/hostedcontrolplane_controller.go +++ b/control-plane-operator/controllers/hostedcontrolplane/hostedcontrolplane_controller.go @@ -1889,6 +1889,38 @@ func (r *HostedControlPlaneReconciler) reconcileAzurePlatformCerts(ctx context.C return nil } +func (r *HostedControlPlaneReconciler) getRootCATrustSources(ctx context.Context, hcp *hyperv1.HostedControlPlane) (*corev1.ConfigMap, []*corev1.Secret, error) { + var observedDefaultIngressCert *corev1.ConfigMap + if capabilities.IsIngressCapabilityEnabled(hcp.Spec.Capabilities) { + observedDefaultIngressCert = manifests.IngressObservedDefaultIngressCertCA(hcp.Namespace) + if err := r.Get(ctx, client.ObjectKeyFromObject(observedDefaultIngressCert), observedDefaultIngressCert); err != nil { + if !apierrors.IsNotFound(err) { + return nil, nil, fmt.Errorf("failed to get observed default ingress cert: %w", err) + } + observedDefaultIngressCert = nil + } + } + + var namedCertSecrets []*corev1.Secret + for _, namedCert := range hcp.Spec.Configuration.GetNamedCertificates() { + if namedCert.ServingCertificate.Name == "" { + continue + } + secret := &corev1.Secret{} + if err := r.Get(ctx, client.ObjectKey{Namespace: hcp.Namespace, Name: namedCert.ServingCertificate.Name}, secret); err != nil { + if !apierrors.IsNotFound(err) { + return nil, nil, fmt.Errorf("failed to get named certificate secret %s: %w", namedCert.ServingCertificate.Name, err) + } + log := ctrl.LoggerFrom(ctx) + log.Info("Named certificate secret not found, skipping for root CA trust bundle", "secret", namedCert.ServingCertificate.Name) + continue + } + namedCertSecrets = append(namedCertSecrets, secret) + } + + return observedDefaultIngressCert, namedCertSecrets, nil +} + func (r *HostedControlPlaneReconciler) reconcilePKI(ctx context.Context, hcp *hyperv1.HostedControlPlane, infraStatus infra.InfrastructureStatus, createOrUpdate upsert.CreateOrUpdateFN) error { p := pki.NewPKIParams(hcp, infraStatus.APIHost, infraStatus.OAuthHost, infraStatus.KonnectivityHost) @@ -1899,19 +1931,14 @@ func (r *HostedControlPlaneReconciler) reconcilePKI(ctx context.Context, hcp *hy return fmt.Errorf("failed to reconcile root CA: %w", err) } - var observedDefaultIngressCert *corev1.ConfigMap - if capabilities.IsIngressCapabilityEnabled(hcp.Spec.Capabilities) { - observedDefaultIngressCert = manifests.IngressObservedDefaultIngressCertCA(hcp.Namespace) - if err := r.Get(ctx, client.ObjectKeyFromObject(observedDefaultIngressCert), observedDefaultIngressCert); err != nil { - if !apierrors.IsNotFound(err) { - return fmt.Errorf("failed to get observed default ingress cert: %w", err) - } - observedDefaultIngressCert = nil - } + observedDefaultIngressCert, namedCertSecrets, err := r.getRootCATrustSources(ctx, hcp) + if err != nil { + return fmt.Errorf("failed to get root CA trust sources: %w", err) } + rootCAConfigMap := manifests.RootCAConfigMap(hcp.Namespace) if _, err := createOrUpdate(ctx, r, rootCAConfigMap, func() error { - return pki.ReconcileRootCAConfigMap(rootCAConfigMap, p.OwnerRef, rootCASecret, observedDefaultIngressCert) + return pki.ReconcileRootCAConfigMap(rootCAConfigMap, p.OwnerRef, rootCASecret, observedDefaultIngressCert, namedCertSecrets) }); err != nil { return fmt.Errorf("failed to reconcile root CA configmap: %w", err) } @@ -3306,32 +3333,3 @@ func setKASCustomKubeconfigStatus(ctx context.Context, hcp *hyperv1.HostedContro return nil } - -// includeServingCertificates includes additional serving certificates into the provided root CA ConfigMap. -// It retrieves the named certificates specified in the HostedControlPlane's APIServer configuration and appends -// their contents to the "ca.crt" entry in the root CA ConfigMap. -func includeServingCertificates(ctx context.Context, c client.Client, hcp *hyperv1.HostedControlPlane, rootCA *corev1.ConfigMap) (*corev1.ConfigMap, error) { - var tlsCRT string - newRootCA := rootCA.DeepCopy() - - if hcp.Spec.Configuration != nil && hcp.Spec.Configuration.APIServer != nil && len(hcp.Spec.Configuration.APIServer.ServingCerts.NamedCertificates) > 0 { - for _, servingCert := range hcp.Spec.Configuration.APIServer.ServingCerts.NamedCertificates { - newCRT := &corev1.Secret{} - if err := c.Get(ctx, client.ObjectKey{Namespace: hcp.Namespace, Name: servingCert.ServingCertificate.Name}, newCRT); err != nil { - return nil, fmt.Errorf("failed to get serving certificate secret: %w", err) - } - - if len(tlsCRT) <= 0 { - tlsCRT = newRootCA.Data["ca.crt"] - } - - tlsCRT = fmt.Sprintf("%s\n%s", tlsCRT, string(newCRT.Data["tls.crt"])) - } - - if len(tlsCRT) > 0 { - newRootCA.Data["ca.crt"] = tlsCRT - } - } - - return newRootCA, nil -} diff --git a/control-plane-operator/controllers/hostedcontrolplane/hostedcontrolplane_controller_test.go b/control-plane-operator/controllers/hostedcontrolplane/hostedcontrolplane_controller_test.go index dffc60c8d6a3..4fb9a5966dd4 100644 --- a/control-plane-operator/controllers/hostedcontrolplane/hostedcontrolplane_controller_test.go +++ b/control-plane-operator/controllers/hostedcontrolplane/hostedcontrolplane_controller_test.go @@ -906,134 +906,6 @@ func TestSetKASCustomKubeconfigStatus(t *testing.T) { } } -func TestIncludeServingCertificates(t *testing.T) { - ctx := t.Context() - hcp := sampleHCP(t) - rootCA := &corev1.ConfigMap{ - ObjectMeta: metav1.ObjectMeta{ - Name: "root-ca", - Namespace: hcp.Namespace, - }, - Data: map[string]string{ - "ca.crt": "root-ca-cert", - }, - } - - tests := []struct { - name string - servingCerts *configv1.APIServerServingCerts - servingSecrets []*corev1.Secret - expectedCert string - expectError bool - }{ - { - name: "APIServer servingCerts is nil", - servingCerts: &configv1.APIServerServingCerts{}, - expectedCert: "root-ca-cert", - }, - { - name: "APIServer servingCerts configuration with one named certificates", - servingCerts: &configv1.APIServerServingCerts{ - NamedCertificates: []configv1.APIServerNamedServingCert{ - { - ServingCertificate: configv1.SecretNameReference{ - Name: "serving-cert-1", - }, - }, - }, - }, - servingSecrets: []*corev1.Secret{ - { - ObjectMeta: metav1.ObjectMeta{ - Name: "serving-cert-1", - Namespace: hcp.Namespace, - }, - Data: map[string][]byte{ - "tls.crt": []byte("cert-1"), - }, - }, - }, - expectedCert: "root-ca-cert\ncert-1", - }, - { - name: "APIServer servingCerts configuration with multiple named certificates", - servingCerts: &configv1.APIServerServingCerts{ - NamedCertificates: []configv1.APIServerNamedServingCert{ - { - ServingCertificate: configv1.SecretNameReference{ - Name: "serving-cert-1", - }, - }, - { - ServingCertificate: configv1.SecretNameReference{ - Name: "serving-cert-2", - }, - }, - }, - }, - servingSecrets: []*corev1.Secret{ - { - ObjectMeta: metav1.ObjectMeta{ - Name: "serving-cert-1", - Namespace: hcp.Namespace, - }, - Data: map[string][]byte{ - "tls.crt": []byte("cert-1"), - }, - }, - { - ObjectMeta: metav1.ObjectMeta{ - Name: "serving-cert-2", - Namespace: hcp.Namespace, - }, - Data: map[string][]byte{ - "tls.crt": []byte("cert-2"), - }, - }, - }, - expectedCert: "root-ca-cert\ncert-1\ncert-2", - }, - { - name: "APIServer servingCerts configuration with missing named certificate", - servingCerts: &configv1.APIServerServingCerts{ - NamedCertificates: []configv1.APIServerNamedServingCert{ - { - ServingCertificate: configv1.SecretNameReference{ - Name: "missing-cert", - }, - }, - }, - }, - expectError: true, - }, - } - - for _, tc := range tests { - t.Run(tc.name, func(t *testing.T) { - g := NewGomegaWithT(t) - - hcp.Spec.Configuration = &hyperv1.ClusterConfiguration{ - APIServer: &configv1.APIServerSpec{ - ServingCerts: *tc.servingCerts, - }, - } - - fakeClient := fake.NewClientBuilder().WithObjects(rootCA).Build() - for _, secret := range tc.servingSecrets { - _ = fakeClient.Create(ctx, secret) - } - - newRootCA, err := includeServingCertificates(ctx, fakeClient, hcp, rootCA) - if tc.expectError { - g.Expect(err).To(HaveOccurred()) - } else { - g.Expect(err).NotTo(HaveOccurred()) - g.Expect(newRootCA.Data["ca.crt"]).To(Equal(tc.expectedCert)) - } - }) - } -} - // TestControlPlaneComponents is a generic test which generates a fixture for each registered component's deployment/statefulset. // This is helpful to allow to inspect the final manifest yaml result after all the pre/post-processing is applied. func TestControlPlaneComponents(t *testing.T) { diff --git a/control-plane-operator/controllers/hostedcontrolplane/pki/ca.go b/control-plane-operator/controllers/hostedcontrolplane/pki/ca.go index 58a306a01765..3c791356b981 100644 --- a/control-plane-operator/controllers/hostedcontrolplane/pki/ca.go +++ b/control-plane-operator/controllers/hostedcontrolplane/pki/ca.go @@ -106,7 +106,7 @@ func ReconcileEtcdMetricsSignerConfigMap(cm *corev1.ConfigMap, ownerRef config.O return reconcileAggregateCA(cm, ownerRef, etcdMetricsSigner) } -func ReconcileRootCAConfigMap(cm *corev1.ConfigMap, ownerRef config.OwnerRef, rootCA *corev1.Secret, observedDefaultIngressCert *corev1.ConfigMap) error { +func ReconcileRootCAConfigMap(cm *corev1.ConfigMap, ownerRef config.OwnerRef, rootCA *corev1.Secret, observedDefaultIngressCert *corev1.ConfigMap, namedCertSecrets []*corev1.Secret) error { sources := []*corev1.Secret{rootCA} if observedDefaultIngressCert != nil { sources = append(sources, &corev1.Secret{ @@ -115,7 +115,17 @@ func ReconcileRootCAConfigMap(cm *corev1.ConfigMap, ownerRef config.OwnerRef, ro }, }) } - return reconcileAggregateCA(cm, ownerRef, sources...) + if err := reconcileAggregateCA(cm, ownerRef, sources...); err != nil { + return err + } + + for _, s := range namedCertSecrets { + if tlsCrt := s.Data[corev1.TLSCertKey]; len(tlsCrt) > 0 { + cm.Data[certs.CASignerCertMapKey] += "\n" + string(tlsCrt) + } + } + + return nil } func ReconcileKonnectivityConfigMap(cm *corev1.ConfigMap, ownerRef config.OwnerRef, konnectivityCA *corev1.Secret) error { diff --git a/control-plane-operator/controllers/hostedcontrolplane/pki/ca_test.go b/control-plane-operator/controllers/hostedcontrolplane/pki/ca_test.go new file mode 100644 index 000000000000..995935600b43 --- /dev/null +++ b/control-plane-operator/controllers/hostedcontrolplane/pki/ca_test.go @@ -0,0 +1,143 @@ +package pki + +import ( + "testing" + + . "github.com/onsi/gomega" + + "github.com/openshift/hypershift/support/certs" + "github.com/openshift/hypershift/support/config" + + corev1 "k8s.io/api/core/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" +) + +func TestReconcileRootCAConfigMap(t *testing.T) { + t.Parallel() + + ownerRef := config.OwnerRef{} + + rootCASecret := &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: "root-ca", + Namespace: "test-ns", + }, + Data: map[string][]byte{ + certs.CASignerCertMapKey: []byte("root-ca-cert"), + }, + } + + tests := []struct { + name string + observedDefaultIngressCert *corev1.ConfigMap + namedCertSecrets []*corev1.Secret + expectedCACert string + }{ + { + name: "When no named certs are configured, it should contain only the root CA", + expectedCACert: "root-ca-cert", + }, + { + name: "When an observed default ingress cert exists, it should include root CA and ingress cert", + observedDefaultIngressCert: &corev1.ConfigMap{ + Data: map[string]string{ + certs.CASignerCertMapKey: "ingress-cert", + }, + }, + expectedCACert: "root-ca-certingress-cert", + }, + { + name: "When one named cert is configured, it should append its tls.crt to the CA bundle", + namedCertSecrets: []*corev1.Secret{ + { + Data: map[string][]byte{ + corev1.TLSCertKey: []byte("named-cert-1"), + }, + }, + }, + expectedCACert: "root-ca-cert\nnamed-cert-1", + }, + { + name: "When multiple named certs are configured, it should append all tls.crt to the CA bundle", + namedCertSecrets: []*corev1.Secret{ + { + Data: map[string][]byte{ + corev1.TLSCertKey: []byte("named-cert-1"), + }, + }, + { + Data: map[string][]byte{ + corev1.TLSCertKey: []byte("named-cert-2"), + }, + }, + }, + expectedCACert: "root-ca-cert\nnamed-cert-1\nnamed-cert-2", + }, + { + name: "When a named cert has empty tls.crt, it should skip it", + namedCertSecrets: []*corev1.Secret{ + { + Data: map[string][]byte{ + corev1.TLSCertKey: []byte("named-cert-1"), + }, + }, + { + Data: map[string][]byte{ + corev1.TLSCertKey: []byte(""), + }, + }, + }, + expectedCACert: "root-ca-cert\nnamed-cert-1", + }, + { + name: "When a named cert has no tls.crt key, it should skip it", + namedCertSecrets: []*corev1.Secret{ + { + Data: map[string][]byte{ + corev1.TLSCertKey: []byte("named-cert-1"), + }, + }, + { + Data: map[string][]byte{ + "other-key": []byte("some-data"), + }, + }, + }, + expectedCACert: "root-ca-cert\nnamed-cert-1", + }, + { + name: "When ingress cert and named certs are both configured, it should include all", + observedDefaultIngressCert: &corev1.ConfigMap{ + Data: map[string]string{ + certs.CASignerCertMapKey: "ingress-cert", + }, + }, + namedCertSecrets: []*corev1.Secret{ + { + Data: map[string][]byte{ + corev1.TLSCertKey: []byte("named-cert-1"), + }, + }, + }, + expectedCACert: "root-ca-certingress-cert\nnamed-cert-1", + }, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + t.Parallel() + g := NewWithT(t) + + cm := &corev1.ConfigMap{ + ObjectMeta: metav1.ObjectMeta{ + Name: "root-ca", + Namespace: "test-ns", + }, + } + + err := ReconcileRootCAConfigMap(cm, ownerRef, rootCASecret, tc.observedDefaultIngressCert, tc.namedCertSecrets) + g.Expect(err).ToNot(HaveOccurred()) + g.Expect(cm.Data[certs.CASignerCertMapKey]).To(Equal(tc.expectedCACert)) + }) + } +}