Skip to content

AGENT-1389: create InternalReleaseImage registry TLS certificate#10147

Merged
openshift-merge-bot[bot] merged 1 commit intoopenshift:mainfrom
andfasano:iri-certs
Dec 5, 2025
Merged

AGENT-1389: create InternalReleaseImage registry TLS certificate#10147
openshift-merge-bot[bot] merged 1 commit intoopenshift:mainfrom
andfasano:iri-certs

Conversation

@andfasano
Copy link
Copy Markdown
Contributor

This patch adds two new assets, respectively IRICertKey and InternalReleaseImageTLSSecret, for generating a new TLS certificate (signed by the RootCA asset) to be used for the InternalReleaseImage registry (introduced in #5452).

The code is completely fenced by the NoRegistryClusterInstall feature gate, as it's still in TP.

@openshift-ci-robot openshift-ci-robot added the jira/valid-reference Indicates that this PR references a valid Jira ticket of any type. label Dec 4, 2025
@openshift-ci-robot
Copy link
Copy Markdown
Contributor

openshift-ci-robot commented Dec 4, 2025

@andfasano: This pull request references AGENT-1389 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:

This patch adds two new assets, respectively IRICertKey and InternalReleaseImageTLSSecret, for generating a new TLS certificate (signed by the RootCA asset) to be used for the InternalReleaseImage registry (introduced in #5452).

The code is completely fenced by the NoRegistryClusterInstall feature gate, as it's still in TP.

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
Copy link
Copy Markdown
Contributor Author

cc @pablintino @djoshy

@andfasano andfasano force-pushed the iri-certs branch 2 times, most recently from 7ffe266 to 87c2c7e Compare December 4, 2025 18:45
Copy link
Copy Markdown
Contributor

@patrickdillon patrickdillon left a comment

Choose a reason for hiding this comment

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

/approve

Comment thread data/data/manifests/bootkube/internal-release-image-tls-secret.yaml.template Outdated
Comment thread pkg/asset/manifests/operators.go
}
}

if installConfig.Config.EnabledFeatureGates().Enabled(features.FeatureGateNoRegistryClusterInstall) {
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.

Is this feature only enabled/disabled by the feature gate? It does not require opt in through the install config or anything?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

That's a good point that I've missed it, thanks!
In general to opt in the feature, it is expected that the InternalReleaseImage resource will be provided as an extra manifest (ie, if not present, during the bootstrap the related MCO controller will simply ignore it).
Anyhow currently the only way to consume such feature is via OVE ISO, so the injection point of this extra manifest is done via the Appliance (there's a pending PR for this openshift/appliance#612), the underlying tool used to build the ISO in konflux.

I've added then a support asset to check for the presence of the InternalReleaseImage manifests (note: due the imports rules the best location I was able to find was /pkg/asset/template/content/manifests, please let me know if you think there's a more suitable one)

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

(and included an existence check wherever necessary for the IRI manifest)

@openshift-ci
Copy link
Copy Markdown
Contributor

openshift-ci Bot commented Dec 4, 2025

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: patrickdillon

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:

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

@openshift-ci openshift-ci Bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Dec 4, 2025
@openshift-merge-robot openshift-merge-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Dec 4, 2025
Comment thread pkg/asset/tls/iricertkey.go Outdated
cfg := &CertCfg{
Subject: pkix.Name{CommonName: "system:internal-release-image"},
ExtKeyUsages: []x509.ExtKeyUsage{x509.ExtKeyUsageServerAuth},
Validity: time.Hour * 24 * 182, // 6 months
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.

Just curious, why 6 months? MCSCertKey uses ValidityTenYears().

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Thanks @rwsu , after a discussion with @zaneb we agree to set it back to 10y, not really any real benefit of keeping shorter - even for the TP version

@openshift-merge-robot openshift-merge-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Dec 5, 2025
continue
}

if u.GetKind() == internalReleaseImageKind && u.GetName() == internalReleaseImageInstanceName {
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

note: a future improvement could be to check an exact match for the current release version

@andfasano
Copy link
Copy Markdown
Contributor Author

andfasano commented Dec 5, 2025

/test golint

The latest failures were pretty weird, not related to the current patch o_O
There was a mismatch in the imports, fixing

@openshift-ci
Copy link
Copy Markdown
Contributor

openshift-ci Bot commented Dec 5, 2025

@andfasano: The following tests 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/okd-scos-e2e-vsphere-ovn 4ede1a7 link false /test okd-scos-e2e-vsphere-ovn
ci/prow/e2e-aws-ovn 4ede1a7 link true /test e2e-aws-ovn
ci/prow/govet 4ede1a7 link true /test govet
ci/prow/e2e-aws-ovn-heterogeneous 4ede1a7 link false /test e2e-aws-ovn-heterogeneous
ci/prow/okd-scos-images 4ede1a7 link true /test okd-scos-images
ci/prow/e2e-aws-ovn-single-node 4ede1a7 link false /test e2e-aws-ovn-single-node
ci/prow/artifacts-images 4ede1a7 link true /test artifacts-images
ci/prow/e2e-aws-ovn-edge-zones 4ede1a7 link false /test e2e-aws-ovn-edge-zones
ci/prow/e2e-aws-ovn-imdsv2 4ede1a7 link false /test e2e-aws-ovn-imdsv2
ci/prow/e2e-aws-ovn-edge-zones-manifest-validation 4ede1a7 link true /test e2e-aws-ovn-edge-zones-manifest-validation
ci/prow/e2e-aws-ovn-shared-vpc-edge-zones 4ede1a7 link false /test e2e-aws-ovn-shared-vpc-edge-zones
ci/prow/e2e-aws-byo-subnet-role-security-groups 4ede1a7 link false /test e2e-aws-byo-subnet-role-security-groups
ci/prow/golint 4ede1a7 link true /test golint
ci/prow/e2e-aws-ovn-fips 4ede1a7 link false /test e2e-aws-ovn-fips
ci/prow/e2e-aws-ovn-shared-vpc-custom-security-groups 4ede1a7 link false /test e2e-aws-ovn-shared-vpc-custom-security-groups
ci/prow/images 4ede1a7 link true /test images
ci/prow/aws-private 4ede1a7 link false /test aws-private
ci/prow/unit 4ede1a7 link true /test unit
ci/prow/e2e-aws-default-config 4ede1a7 link false /test e2e-aws-default-config

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.

@patrickdillon
Copy link
Copy Markdown
Contributor

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

@openshift-ci
Copy link
Copy Markdown
Contributor

openshift-ci Bot commented Dec 5, 2025

@patrickdillon: trigger 1 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-1of3

See details on https://pr-payload-tests.ci.openshift.org/runs/ci/db64dd40-d1e2-11f0-9c33-5b02a65e89d7-0

@patrickdillon
Copy link
Copy Markdown
Contributor

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

@patrickdillon
Copy link
Copy Markdown
Contributor

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

@openshift-ci
Copy link
Copy Markdown
Contributor

openshift-ci Bot commented Dec 5, 2025

@patrickdillon: trigger 1 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

See details on https://pr-payload-tests.ci.openshift.org/runs/ci/e2c7bbc0-d1e2-11f0-9bc4-efb6b057c087-0

@openshift-ci
Copy link
Copy Markdown
Contributor

openshift-ci Bot commented Dec 5, 2025

@patrickdillon: trigger 1 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-3of3

See details on https://pr-payload-tests.ci.openshift.org/runs/ci/e52fbb10-d1e2-11f0-9bfd-d3ceb8cbe66a-0

@pawanpinjarkar
Copy link
Copy Markdown
Contributor

/lgtm

@openshift-ci openshift-ci Bot added the lgtm Indicates that a PR is ready to be merged. label Dec 5, 2025
@andfasano
Copy link
Copy Markdown
Contributor Author

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

@openshift-ci openshift-ci Bot added the acknowledge-critical-fixes-only Indicates if the issuer of the label is OK with the policy. label Dec 5, 2025
@openshift-ci-robot openshift-ci-robot added verified-later verified Signifies that the PR passed pre-merge verification criteria labels Dec 5, 2025
@openshift-ci-robot
Copy link
Copy Markdown
Contributor

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

Details

In response to this:

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

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-merge-bot openshift-merge-bot Bot merged commit ca7b596 into openshift:main Dec 5, 2025
14 of 15 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

acknowledge-critical-fixes-only Indicates if the issuer of the label is OK with the policy. approved Indicates a PR has been approved by an approver from all required OWNERS files. jira/valid-reference Indicates that this PR references a valid Jira ticket of any type. lgtm Indicates that a PR is ready to be merged. verified Signifies that the PR passed pre-merge verification criteria verified-later

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants