Skip to content

OCPBUGS-49691: Include named cert CAs in root-ca ConfigMap #9064

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
hypershift-jira-solve-ci wants to merge 2 commits into
base: main
Choose a base branch
from
+192 −169
Open

OCPBUGS-49691: Include named cert CAs in root-ca ConfigMap #9064

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
ee48baa
fix(OCPBUGS-49691): include named cert CAs in root-ca ConfigMap
Jul 22, 2026
991af7c
fix: address review findings for OCPBUGS-49691
Jul 22, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
76 changes: 37 additions & 39 deletions control-plane-operator/controllers/hostedcontrolplane/hostedcontrolplane_controller.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Comment on lines +1904 to +1918

@coderabbitai coderabbitai Bot Jul 22, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🩺 Stability & Availability | 🟠 Major | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail
rg -n -C 4 'SetupWithManager|Watches\(|Owns\(|EnqueueRequestsFromMapFunc|NamedCertificates|ServingCertificate' control-plane-operator/controllers/hostedcontrolplane

Repository: openshift/hypershift

Length of output: 33300

Watch the named certificate Secrets here. This code only reads them during HCP reconcile, and the Secret watch in this controller only enqueues owner refs. If these are user-managed Secrets, create/update/delete events won’t trigger a reconcile, so root-ca can stay stale until some unrelated HCP change.

🤖 Prompt for AI Agents 
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In
`@control-plane-operator/controllers/hostedcontrolplane/hostedcontrolplane_controller.go`
around lines 1904 - 1918, The named certificate Secret lookup in the
HostedControlPlane reconciliation flow does not establish watches for
user-managed Secrets. Update the controller setup and the named-certificate
handling around GetNamedCertificates to watch referenced Secrets and enqueue the
owning HostedControlPlane when those Secrets are created, updated, or deleted,
while preserving the existing not-found and error behavior.

}

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)

Expand All @@ -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)
}
Expand Down Expand Up @@ -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
}
128 changes: 0 additions & 128 deletions control-plane-operator/controllers/hostedcontrolplane/hostedcontrolplane_controller_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down
14 changes: 12 additions & 2 deletions control-plane-operator/controllers/hostedcontrolplane/pki/ca.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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{
Expand All @@ -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 {
Expand Down
Loading