AGENT-1443: IRI Add certificate regeneration to MCS cert rotation controller by rwsu · Pull Request #5721 · openshift/machine-config-operator

rwsu · 2026-02-27T23:17:09Z

- What I did

Updated certrotation_controller to generate new IRI TLS certificates when the MCS CA rotates
Created reconcileIRICertificate function to update the IRI TLS certificates
The function only runs if the NoRegistryClusterInstall feature gate is enabled
Add localhost, 127.0.0.1, and ::1 to the IRI certificate SANs to match the installer-generated certificate.
Add idempotency check that verifies the existing IRI cert against the current MCS CA before regenerating, preventing unnecessary Secret updates and node rollouts on controller restarts.

- How to verify it

Force the MCS CA to rotate by deleting the secret
oc delete secret machine-config-server-ca -n openshift-machine-config-operator
Verify the IRI TLS certificates have been updated at /etc/iri-registry/certs/tls.crt
Verify the IRI registry continues to work with the new certificates
curl --cacert /etc/iri-registry/certs/tls.crt https://api-int.:22625/v2/_catalog

- Description for the changelog

When the MCS CA rotates, the IRI TLS certificate (internal-release-image-tls) was not being regenerated, leaving it signed by the old CA. This adds a reconcileIRICertificate() method that generates a new IRI server cert signed by the current MCS CA and creates/updates the secret, triggered alongside user-data secret reconciliation on CA configmap add/update events.

…troller When the MCS CA rotates, the IRI TLS certificate (internal-release-image-tls) was not being regenerated, leaving it signed by the old CA. This adds a reconcileIRICertificate() method that generates a new IRI server cert signed by the current MCS CA and creates/updates the secret. The rotation is triggered on MCS CA rotation via CA bundle ConfigMap events. When the machine-config-server-ca Secret is deleted or expires, library-go's CertRotationController regenerates the CA key pair and updates the machine-config-server-ca ConfigMap (CA bundle). Our addConfigMap and updateConfigMap handlers detect this ConfigMap change and call reconcileIRICertificate() alongside reconcileUserDataSecrets(). Assisted-by: Claude Opus 4.6 <noreply@anthropic.com>

…cert rotation Gate IRI certificate reconciliation on the NoRegistryClusterInstall feature flag so it only runs on clusters that use the IRI registry. Add localhost, 127.0.0.1, and ::1 to the IRI certificate SANs to match the installer-generated certificate. Add idempotency check that verifies the existing IRI cert against the current MCS CA before regenerating, preventing unnecessary Secret updates and node rollouts on controller restarts. Assisted-by: Claude Opus 4.6 <noreply@anthropic.com>

openshift-ci-robot · 2026-02-27T23:17:13Z

@rwsu: This pull request references AGENT-1443 which is a valid jira issue.

Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the story to target the "4.22.0" version, but no target version was set.

Details

In response to this:

- What I did

Updated certrotation_controller to generate new IRI TLS certificates when the MCS CA rotates

Created reconcileIRICertificate function to update the IRI TLS certificates

The function only runs if the NoRegistryClusterInstall feature gate is enabled

Add localhost, 127.0.0.1, and ::1 to the IRI certificate SANs to match the installer-generated certificate.

Add idempotency check that verifies the existing IRI cert against the current MCS CA before regenerating, preventing unnecessary Secret updates and node rollouts on controller restarts.

- How to verify it

Force the MCS CA to rotate by deleting the secret
oc delete secret machine-config-server-ca -n openshift-machine-config-operator

Verify the IRI TLS certificates have been updated at /etc/iri-registry/certs/tls.crt

Verify the IRI registry continues to work with the new certificates
curl --cacert /etc/iri-registry/certs/tls.crt https://api-int.:22625/v2/_catalog

- Description for the changelog

When the MCS CA rotates, the IRI TLS certificate (internal-release-image-tls) was not being regenerated, leaving it signed by the old CA. This adds a reconcileIRICertificate() method that generates a new IRI server cert signed by the current MCS CA and creates/updates the secret, triggered alongside user-data secret reconciliation on CA configmap add/update events.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository.

openshift-ci · 2026-02-27T23:17:43Z

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: rwsu
Once this PR has been reviewed and has the lgtm label, please assign isabella-janssen for approval. For more information see the Code Review Process.

The full list of commands accepted by this bot can be found here.

Details

Needs approval from an approver in each of these files:

OWNERS

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

rwsu · 2026-03-02T15:29:31Z

/retest-required

rwsu · 2026-03-02T15:30:11Z

/cc @djoshy @andfasano

openshift-ci · 2026-03-02T18:34:10Z

@rwsu: all tests passed!

Full PR test history. Your PR dashboard.

Details

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. I understand the commands that are listed here.

rwsu · 2026-03-02T22:53:19Z

/verified by TestIRICertificateRotation unit test and @rwsu

openshift-ci-robot · 2026-03-02T22:53:29Z

@rwsu: This PR has been marked as verified by TestIRICertificateRotation unit test and @rwsu.

Details

In response to this:

/verified by TestIRICertificateRotation unit test and @rwsu

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository.

djoshy

Overall makes sense, left a few comments.

djoshy · 2026-03-03T15:33:49Z

pkg/controller/certrotation/certrotation_controller.go

+		return
+	}
+
+	// Get hostnames from the dynamic serving rotation (includes api-int hostname and platform VIPs)


Are the hostnames from the infra object required for the IRI cert? I was under the impression it wasn't.

It's possible the infra hostnames can get updated outside of the configmap being updated(very unlikely, but plauisble), that could result in the hostnames in the IRI cert would be stale. The MCS TLS cert handles this as it feeds of the dynamic serving rotation so I thought it would be worth mentioning:

machine-config-operator/pkg/controller/certrotation/certrotation_controller.go

Lines 155 to 158 in caa75db

CertCreator: &certrotation.ServingRotation{

Hostnames: c.hostnamesRotation.GetHostnames,

HostnamesChanged: c.hostnamesRotation.hostnamesChanged,

},

If we do want infra hostnames changes to be accounted for, we can call reconcileIRICertificate when the hostname queue gets updated, and I would also recommend adding a check for the IPs in isIRICertValid(). If not, we can just take out this bit that uses the hostnames from the dynamic serving rotation.

During the installation, when the initial IRI TLS certificate is generated (see here), we don't use hostnames, just localhost and the apiInt url.
The entry point for the IRI (logical) service is - and must remain - the apiInt. The localhost must be supported as a special case to allow masters to consume their own local registries in case of reboot / disconnect (ie when the apiInt is not reachable for any reason). So to summarize I don't think hostname are really required, but just apiInt and localHost (same behavior of the installer asset)

djoshy · 2026-03-03T15:50:58Z

pkg/controller/certrotation/certrotation_controller.go

 		c.reconcileUserDataSecrets()
 	}()
+	go func() {
+		c.reconcileIRICertificate()
+	}()


I forgot about these being individual go routines in my original implementation. There is a very small chance of a race here on create or updates. Given that the CA rotates once every 8 years or so, the risk should be minimal. Even if there is a race, the fresh GETs on the CA should result in valid creates/updates.

If we want to be careful we can combine these into one thread and set up a mutex, but IMO that's not a blocker for merging this. The safest way to do this would be via a workqueue like our other controllers, but again definitely not a blocker, just me lamenting my original decisions 😄

What about serializing them? Ie:

go func() { c.reconcileUserDataSecrets() c.reconcileIRICertificate() }()

djoshy · 2026-03-03T16:30:37Z

pkg/controller/certrotation/certrotation_controller_test.go

+	}
+
+	for _, test := range tests {
+		test := test


Not a blocker, but this pattern is no longer needed in go 1.22+ ref: https://go.dev/doc/go1.22#language

andfasano · 2026-03-04T10:22:09Z

pkg/controller/certrotation/certrotation_controller.go

 		c.reconcileUserDataSecrets()
 	}()
+	go func() {
+		c.reconcileIRICertificate()
+	}()


What about serializing them? Ie:

go func() { c.reconcileUserDataSecrets() c.reconcileIRICertificate() }()

andfasano · 2026-03-04T10:25:17Z

pkg/controller/certrotation/certrotation_controller.go

+		return
+	}
+	klog.Infof("Reconciling IRI certificate")
+


It is also required to check for the presence of the IRI cluster resource. If not present, it means that the feature is not enabled

andfasano · 2026-03-04T10:31:39Z

pkg/controller/certrotation/certrotation_controller.go

 	return nil
 }
+
+func (c *CertRotationController) reconcileIRICertificate() {


this internal method is very long, please evaluate to refactor it in smaller methods

andfasano · 2026-03-04T10:38:45Z

pkg/controller/certrotation/certrotation_controller.go

+		klog.Errorf("Cannot get IRI TLS secret: %v", err)
+		return
+	}
+


It looks like this whole block could be simplified with the happy path, and secretExists removed. Ie:

iriSecret, err := c.kubeClient.CoreV1().Secrets(ctrlcommon.MCONamespace).Get(context.TODO(), ctrlcommon.InternalReleaseImageTLSSecretName, metav1.GetOptions{}) if err != nil && !errors.IsNotFound(err) { klog.Errorf("Cannot get IRI TLS secret: %v", err) return } if iriSecret != nil && c.isIRICertValid(iriSecret, ca) { klog.Infof("IRI TLS certificate is still valid under the current MCS CA, skipping rotation") return }

Later you could simply check for iriSecret != nil

andfasano · 2026-03-04T10:49:14Z

pkg/controller/certrotation/certrotation_controller.go

+		return
+	}
+
+	// Get hostnames from the dynamic serving rotation (includes api-int hostname and platform VIPs)


During the installation, when the initial IRI TLS certificate is generated (see here), we don't use hostnames, just localhost and the apiInt url.
The entry point for the IRI (logical) service is - and must remain - the apiInt. The localhost must be supported as a special case to allow masters to consume their own local registries in case of reboot / disconnect (ie when the apiInt is not reachable for any reason). So to summarize I don't think hostname are really required, but just apiInt and localHost (same behavior of the installer asset)

andfasano · 2026-03-04T10:52:59Z

pkg/controller/certrotation/certrotation_controller.go

+		return
+	}
+
+	if !secretExists {


Why do we need to take into account this case? It doesn't seem a valid one (at least here): if the secret does not exist for any reason, the IRI controller will be broken and won't work as well

andfasano · 2026-03-04T10:55:13Z

pkg/controller/certrotation/certrotation_controller_test.go

I think it would be really useful to have at least one e2e test for the rotation, if not too complex, here https://github.com/openshift/machine-config-operator/tree/main/test/e2e-iri. As soon as openshift/release#73866 will land, it will be possible to verify directly the e2e tests for the IRI controller in the MCO presubmit jobs

rwsu added 2 commits February 27, 2026 17:14

openshift-ci-robot added the jira/valid-reference Indicates that this PR references a valid Jira ticket of any type. label Feb 27, 2026

openshift-ci bot requested review from cheesesashimi and dkhater-redhat February 27, 2026 23:17

openshift-ci bot requested review from andfasano and djoshy March 2, 2026 15:30

openshift-ci-robot added the verified Signifies that the PR passed pre-merge verification criteria label Mar 2, 2026

djoshy reviewed Mar 3, 2026

View reviewed changes

andfasano reviewed Mar 4, 2026

View reviewed changes

	CertCreator: &certrotation.ServingRotation{
	Hostnames: c.hostnamesRotation.GetHostnames,
	HostnamesChanged: c.hostnamesRotation.hostnamesChanged,
	},

Conversation

rwsu commented Feb 27, 2026

Uh oh!

openshift-ci-robot commented Feb 27, 2026 • edited by openshift-ci bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

openshift-ci bot commented Feb 27, 2026

Uh oh!

rwsu commented Mar 2, 2026

Uh oh!

rwsu commented Mar 2, 2026

Uh oh!

openshift-ci bot commented Mar 2, 2026

Uh oh!

rwsu commented Mar 2, 2026

Uh oh!

openshift-ci-robot commented Mar 2, 2026

Uh oh!

djoshy left a comment

Choose a reason for hiding this comment

Uh oh!

djoshy Mar 3, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

openshift-ci-robot commented Feb 27, 2026 •

edited by openshift-ci bot

Loading

djoshy Mar 3, 2026 •

edited

Loading