AGENT-1391: Use TLS cert for the InternalReleaseImage registry #5483

andfasano · 2025-12-10T15:28:34Z

- What I did
This patch uses the TLS cert generated by the installer (see openshift/installer#10147) for the InternalReleaseImage registry.
Also, the root CA, used to sign the IRI cert, is added to the trust store of all the nodes.

This patch also adds the OWNERS file for the InternalReleaseImage controller folder, managed by the agent team

- How to verify it
Add the InternalReleaseImage resource in the bootstrap manifests dir (/etc/mcc/bootstrap) and enable the NoRegistryClusterInstall feature gate.

- Description for the changelog
Add support for InternalReleaseImage resource

This patch injects the specific tls certificate generated by the installer into the IRI registry. It also adds the root CA certificate to all the master nodes, to allow pulling the images from other nodes registry

openshift-ci-robot · 2025-12-10T15:28:39Z

@andfasano: This pull request references AGENT-1391 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.21.0" version, but no target version was set.

Details

In response to this:

- What I did
This patch uses the TLS cert generated by the installer (see openshift/installer#10147) for the InternalReleaseImage registry.
Also, the root CA, used to sign the IRI cert, is added to the trust store of all the nodes

- How to verify it
Add the InternalReleaseImage resource in the bootstrap manifests dir (/etc/mcc/bootstrap) and enable the NoRegistryClusterInstall feature gate.

- Description for the changelog
Add support for InternalReleaseImage resource

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.

pkg/controller/common/constants.go

pkg/controller/internalreleaseimage/internalreleaseimage_renderer.go

reviewed bootstrap/controller logic to allow generation and management of multiple machine configs

members of the agent team

openshift-ci-robot · 2025-12-11T12:25:03Z

@andfasano: This pull request references AGENT-1391 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.21.0" version, but no target version was set.

Details

In response to this:

- What I did
This patch uses the TLS cert generated by the installer (see openshift/installer#10147) for the InternalReleaseImage registry.
Also, the root CA, used to sign the IRI cert, is added to the trust store of all the nodes.

This patch also adds the OWNERS file for the InternalReleaseImage controller folder, managed by the agent team

- How to verify it
Add the InternalReleaseImage resource in the bootstrap manifests dir (/etc/mcc/bootstrap) and enable the NoRegistryClusterInstall feature gate.

- Description for the changelog
Add support for InternalReleaseImage resource

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.

pablintino · 2025-12-11T12:24:58Z

pkg/controller/internalreleaseimage/templates/worker/files/root-ca.yaml

Oh, so, the CA wasn't distributed to workers? Strange... but makes sense, as I see the regular, non-bootstrap, MCS doesn't append the certicates as part of the ignition config. If so, maybe that's because we are already distributing them as part of the other MCs for worker pools?

Neither to masters AFAICS.

Ah yes, we removed this being distributed to nodes since it is essentially non functional once the node joins the cluster; and we did not want future rotations to cause an MC rollout. The rootCA is distributed via user-data-secret and not actually written/managed via MachineConfigs. I think adding it back makes sense here; although perhaps you could throw root-ca in templates/common? Perhaps we should move it to cert_writer as a follow-up, I can see this causing unexpected reboots during upgrades to releases with this PR?(or add a node disruption policy for these files too?) Thoughts, @yuqi-zhang?

ref: #4600 and https://issues.redhat.com/browse/MCO-1323

Note: the 4.21 release for this feature (NoRegistryClusterInstall) will be a TP, so no upgrades will be involved

Out of curiosity, how often does this get rotated? I do agree that this should probably live in the cert rotation path, but since this is only used for IRI enabled clusters (which should be a very small percentage) maybe we're ok with that being a nodedisruptionpolicy

pkg/controller/internalreleaseimage/internalreleaseimage_bootstrap.go

pablintino · 2025-12-11T12:26:23Z

pkg/controller/internalreleaseimage/internalreleaseimage_controller.go

-		client: mcfgClient,
-
+		client:        mcfgClient,
+		kubeClient:    kubeClient,


We try to avoid using the client and use listers.

Any specific reason (ie caching) - I suppose you're referring mainly to resource fetching? Consider that the activity for this controller is pretty limited, looking for very few resources. IIRC also the testing was easier with the regular fake clients, though.

There are some API limits in the openshift tests that catch controllers making too many client resource requests. I don't think you'd hit those, but maybe we can make a followup improvement for this

yuqi-zhang · 2025-12-11T19:08:31Z

pkg/controller/internalreleaseimage/templates/master/files/root-ca.yaml

@@ -0,0 +1,5 @@
+mode: 0644
+path: "/etc/pki/ca-trust/source/anchors/root-ca.crt"


would you consider prefixing this with iri as well to keep it consistent, and for anyone looking at the file to see what it's coming from? Or is the name a hard requirement

yuqi-zhang · 2025-12-11T19:10:57Z

pkg/controller/internalreleaseimage/templates/worker/files/root-ca.yaml

Out of curiosity, how often does this get rotated? I do agree that this should probably live in the cert rotation path, but since this is only used for IRI enabled clusters (which should be a very small percentage) maybe we're ok with that being a nodedisruptionpolicy

yuqi-zhang · 2025-12-11T19:11:33Z

pkg/controller/internalreleaseimage/templates/worker/files/root-ca.yaml

@@ -0,0 +1,5 @@
+mode: 0644
+path: "/etc/pki/ca-trust/source/anchors/root-ca.crt"


Same comment as above applies, preferrably the naming on-disk is indicative of what it's used for, since root-ca is a bit of an overloaded term

yuqi-zhang · 2025-12-11T19:12:40Z

pkg/controller/internalreleaseimage/internalreleaseimage_controller.go

-		client: mcfgClient,
-
+		client:        mcfgClient,
+		kubeClient:    kubeClient,


There are some API limits in the openshift tests that catch controllers making too many client resource requests. I don't think you'd hit those, but maybe we can make a followup improvement for this

djoshy · 2025-12-11T20:17:42Z

/payload-job periodic-ci-openshift-machine-config-operator-release-4.21-periodics-e2e-aws-mco-disruptive-techpreview-1of2 periodic-ci-openshift-machine-config-operator-release-4.21-periodics-e2e-aws-mco-disruptive-techpreview-2of2 periodic-ci-openshift-release-master-ci-4.21-e2e-aws-ovn-techpreview-serial-1of3 periodic-ci-openshift-release-master-ci-4.21-e2e-aws-ovn-techpreview-serial-2of3 periodic-ci-openshift-release-master-ci-4.21-e2e-aws-ovn-techpreview-serial-3of3

openshift-ci · 2025-12-11T20:17:59Z

@djoshy: trigger 5 job(s) for the /payload-(with-prs|job|aggregate|job-with-prs|aggregate-with-prs) command

periodic-ci-openshift-machine-config-operator-release-4.21-periodics-e2e-aws-mco-disruptive-techpreview-1of2
periodic-ci-openshift-machine-config-operator-release-4.21-periodics-e2e-aws-mco-disruptive-techpreview-2of2
periodic-ci-openshift-release-master-ci-4.21-e2e-aws-ovn-techpreview-serial-1of3
periodic-ci-openshift-release-master-ci-4.21-e2e-aws-ovn-techpreview-serial-2of3
periodic-ci-openshift-release-master-ci-4.21-e2e-aws-ovn-techpreview-serial-3of3

See details on https://pr-payload-tests.ci.openshift.org/runs/ci/730b8ff0-d6ce-11f0-9c07-a59bf5b671ce-0

yuqi-zhang

/lgtm
/acknowledge-critical-fixes-only
/hold

For tech preview and install support, I'm fine with merging this. I would like to see followup plans around:

cert rotation and MCD-side rotation upgrade handling
renaming the certificates and filepaths to be more indicative of what they're used for
consider using less direct API calls for object fetching, etc.

Holding until we are certain that payloads will continue to work, as we would not like to break this right before branching, even if it is TP

openshift-ci · 2025-12-11T20:20:50Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: andfasano, yuqi-zhang

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

The pull request process is described here

Details

Needs approval from an approver in each of these files:

~~OWNERS~~ [yuqi-zhang]

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

yuqi-zhang · 2025-12-12T01:45:28Z

/acknowledge-critical-fixes-only

yuqi-zhang · 2025-12-12T01:46:44Z

/payload-job periodic-ci-openshift-release-master-ci-4.21-e2e-aws-ovn-techpreview-serial-2of3 periodic-ci-openshift-release-master-ci-4.21-e2e-aws-ovn-techpreview-serial-3of3

The first run seems promising, I will let the failed ones try again, but the original failures seem unrelated to this.

@andfasano in your AM, if you're comfortable with the state of the PR, feel free to unhold and mark verified if you are doing the verification. All the other labels should be applied so hopefully it'll merge

openshift-ci · 2025-12-12T01:46:47Z

@yuqi-zhang: trigger 2 job(s) for the /payload-(with-prs|job|aggregate|job-with-prs|aggregate-with-prs) command

periodic-ci-openshift-release-master-ci-4.21-e2e-aws-ovn-techpreview-serial-2of3
periodic-ci-openshift-release-master-ci-4.21-e2e-aws-ovn-techpreview-serial-3of3

See details on https://pr-payload-tests.ci.openshift.org/runs/ci/6a9c82b0-d6fc-11f0-8fc2-ccb5502fd2c6-0

andfasano · 2025-12-12T09:01:11Z

/lgtm /acknowledge-critical-fixes-only /hold

For tech preview and install support, I'm fine with merging this. I would like to see followup plans around:
1. cert rotation and MCD-side rotation upgrade handling

2. renaming the certificates and filepaths to be more indicative of what they're used for

3. consider using less direct API calls for object fetching, etc.
Holding until we are certain that payloads will continue to work, as we would not like to break this right before branching, even if it is TP

Thank you @yuqi-zhang, see https://issues.redhat.com/browse/AGENT-1392 for the cert rotation. I will followup asap on the other two points (https://issues.redhat.com/browse/AGENT-1395)

andfasano · 2025-12-12T09:02:13Z

/verified later @andfasano
/label acknowledge-critical-fixes-only
/hold cancel

openshift-ci-robot · 2025-12-12T09:02:26Z

@andfasano: This PR has been marked to be verified later by @andfasano.

Details

In response to this:

/verified later @andfasano
/label acknowledge-critical-fixes-only
/hold cancel

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.

andfasano · 2025-12-12T10:54:05Z

/test e2e-aws-ovn
/test e2e-gcp-op-1of2

openshift-ci · 2025-12-12T17:14:16Z

@andfasano: The following test failed, say /retest to rerun all failed tests or /retest-required to rerun all mandatory failed tests:

Test name	Commit	Details	Required	Rerun command
ci/prow/bootstrap-unit	`c2b3174`	link	false	`/test bootstrap-unit`

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.

andfasano · 2025-12-12T17:39:44Z

/test e2e-gcp-op-single-node

andfasano · 2025-12-15T10:21:59Z

/cherry-pick release-4.21

openshift-cherrypick-robot · 2025-12-15T10:22:42Z

@andfasano: new pull request created: #5495

Details

In response to this:

/cherry-pick release-4.21

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.

add tls certificate to InternalReleaseImage registry

0632e3c

This patch injects the specific tls certificate generated by the installer into the IRI registry. It also adds the root CA certificate to all the master nodes, to allow pulling the images from other nodes registry

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

openshift-ci bot requested review from dkhater-redhat and yuqi-zhang December 10, 2025 15:28

andfasano force-pushed the iri-tls-cert branch 2 times, most recently from 6e1c618 to eab86cb Compare December 10, 2025 18:57

pawanpinjarkar reviewed Dec 10, 2025

View reviewed changes

pkg/controller/common/constants.go Outdated Show resolved Hide resolved

rwsu reviewed Dec 10, 2025

View reviewed changes

andfasano force-pushed the iri-tls-cert branch from eab86cb to ab9024b Compare December 11, 2025 08:51

added tls cert to workers trust store

9f74756

reviewed bootstrap/controller logic to allow generation and management of multiple machine configs

andfasano force-pushed the iri-tls-cert branch from ab9024b to 9f74756 Compare December 11, 2025 08:53

added OWNERS file

c2b3174

members of the agent team

pablintino reviewed Dec 11, 2025

View reviewed changes

yuqi-zhang reviewed Dec 11, 2025

View reviewed changes

yuqi-zhang approved these changes Dec 11, 2025

View reviewed changes

openshift-ci bot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Dec 11, 2025

openshift-ci bot assigned yuqi-zhang Dec 11, 2025

openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label Dec 11, 2025

openshift-ci bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Dec 11, 2025

openshift-ci-robot added the verified-later label Dec 12, 2025

openshift-ci-robot added the verified Signifies that the PR passed pre-merge verification criteria label Dec 12, 2025

openshift-ci bot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Dec 12, 2025

openshift-ci bot added the acknowledge-critical-fixes-only Indicates if the issuer of the label is OK with the policy. label Dec 12, 2025

openshift-merge-bot bot merged commit 0bc0e71 into openshift:main Dec 13, 2025
13 of 14 checks passed

openshift-cherrypick-robot mentioned this pull request Dec 15, 2025

[release-4.21] OCPBUGS-68364: InternalReleaseImage should use TLS certs #5495

Open

		@@ -0,0 +1,5 @@
		mode: 0644
		path: "/etc/pki/ca-trust/source/anchors/root-ca.crt"

AGENT-1391: Use TLS cert for the InternalReleaseImage registry #5483

AGENT-1391: Use TLS cert for the InternalReleaseImage registry #5483

Uh oh!

Conversation

andfasano commented Dec 10, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

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

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

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

Uh oh!

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!

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!

djoshy commented Dec 11, 2025

Uh oh!

openshift-ci bot commented Dec 11, 2025

Uh oh!

yuqi-zhang left a comment

Choose a reason for hiding this comment

Uh oh!

openshift-ci bot commented Dec 11, 2025

Uh oh!

yuqi-zhang commented Dec 12, 2025

Uh oh!

yuqi-zhang commented Dec 12, 2025

Uh oh!

openshift-ci bot commented Dec 12, 2025

Uh oh!

andfasano commented Dec 12, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

andfasano commented Dec 12, 2025

Uh oh!

openshift-ci-robot commented Dec 12, 2025

Uh oh!

andfasano commented Dec 12, 2025

Uh oh!

openshift-ci bot commented Dec 12, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

andfasano commented Dec 12, 2025

Uh oh!

Uh oh!

andfasano commented Dec 15, 2025

Uh oh!

openshift-cherrypick-robot commented Dec 15, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

andfasano commented Dec 10, 2025 •

edited

Loading

openshift-ci-robot commented Dec 10, 2025 •

edited by openshift-ci bot

Loading

openshift-ci-robot commented Dec 11, 2025 •

edited by openshift-ci bot

Loading

andfasano commented Dec 12, 2025 •

edited

Loading

openshift-ci bot commented Dec 12, 2025 •

edited

Loading