OCPBUGS-105321: pki: switch to ECDSA defaults and thread PKI profile to leaf certs by sanchezl · Pull Request #10743 · openshift/installer

sanchezl · 2026-08-06T00:12:34Z

Why

When ConfigurablePKI is enabled, the installer's DefaultPKIProfile() returns a hardcoded RSA-4096 placeholder instead of library-go's defaults (ECDSA P-256 / P-384). Leaf certs also ignore the PKI profile entirely and always generate RSA-2048. This diverges from what day-2 operators (CKAO, CKMO) expect when they read the PKI CR for certificate rotation.

What

Replace the local DefaultPKIProfile() with library-go's version
Extend SignerKeyParams to carry a full PKIProfile + ConfigurablePKIEnabled
Thread key resolution through to all 25 cert assets (11 signers, 14 leaves) via library-go's ResolveCertificateConfig

Why this is safe

Zero blast radius for default installs. Every cert asset has a clear feature-gate guard:

if !pkiCfg.ConfigurablePKIEnabled {
    // Legacy path — identical to pre-change code, including explicit KeyUsages
    return c.SelfSignedCertKey.Generate(ctx, cfg, "name", nil)
}
// Library-go path — only runs when ConfigurablePKI is enabled

When the feature gate is off (all production installs today), the existing hand-rolled crypto runs unchanged. The new library-go path only executes under TechPreview/CustomNoUpgrade with ConfigurablePKI explicitly enabled.

Agent flow preserved. SignerKeyParams remains zero-dependency — agent create certificates continues to work without install-config on disk, generating RSA-2048 certs via the legacy path. This pattern was established in PR #10595 after review by zaneb, tthvo, and andfasano (see PR #10595 discussion).

Easy to cull. When ConfigurablePKI is promoted to always-on, the legacy branches and the old GenerateSignedCertificate() / PrivateKey() functions can be deleted in one sweep. No behavioral dependencies between the two paths.

Design decisions from prior PRs

This PR builds on the stacked PRs #10594 and #10595 (both merged). Key decisions inherited:

Zero-dependency SignerKeyParams (PR CNTRLPLANE-2012: Wire signer certs to read PKI config via SignerKeyParams #10595, zaneb's review): avoids pulling InstallConfig validation into agent flows
AssetBase.LoadFromFile (PR CNTRLPLANE-2012: Wire signer certs to read PKI config via SignerKeyParams #10595, tthvo's feedback): strict YAML parsing without platform validation
Target list registration (PR CNTRLPLANE-2012: Wire signer certs to read PKI config via SignerKeyParams #10595, zaneb's catch): SignerKeyParams in Manifests and agentManifestsTarget for multi-step state persistence
Not in agentCertificatesTarget (PR CNTRLPLANE-2012: Wire signer certs to read PKI config via SignerKeyParams #10595, zaneb): "install-config is not an input to this command"
Accepted day-0 gap (CNTRLPLANE-3856): agent UI certs start RSA-2048, converge via day-2 rotation

Commit walkthrough

The commits are ordered to build incrementally:

vendor: bump library-go — vendor-only, no functional changes
pki: replace local DefaultPKIProfile with library-go — pkg/types/pki/defaults.go only, small
pki: extend SignerKeyParams — signerkey_params.go + all signer asset dependency updates
tls: add resolveKeyGen helpers — single new file, 24 lines
tls: add library-go code path to SelfSignedCertKey and SignedCertKey — core engine change in certkey.go
tls: wire feature-gate branches — bulk mechanical change, same pattern in every cert asset
tls: fix cert types for library-go path — JournalCertKey → Peer (serves HTTPS and authenticates curl client with same cert), AdminKubeConfigClientCertKey → Client only (drops legacy ServerAuth — verified on live cluster including localhost-recovery), dead code removal

Commit 6 is the largest but entirely mechanical — each asset follows the same pattern from commit 5. Review one asset (e.g., root.go) and spot-check the rest.

Test plan

Unit tests pass (tls, manifests, pki, configimage, ignition/machine)
Build passes (hack/build.sh)
agent create certificates integration test passes without install-config
Feature gate OFF: legacy RSA-2048 path unchanged (unit tests)
Feature gate ON: payload verification (pending)
Reviewer sign-off

Summary by CodeRabbit

New Features
- Added configurable PKI support across cluster, signer, client, and certificate authority certificate generation.
- Supports configurable RSA and ECDSA key profiles, including custom signer settings.
- Preserves existing RSA-2048 certificate generation when configurable PKI is disabled.
Bug Fixes
- Improved certificate profile resolution and error reporting.
Tests
- Expanded coverage for legacy generation and configurable RSA-4096 and ECDSA P-384 certificates.

openshift-merge-bot · 2026-08-06T00:12:37Z

Pipeline controller notification
This repo is configured to use the pipeline controller. Second-stage tests will be triggered either automatically or after lgtm label is added, depending on the repository configuration. The pipeline controller will automatically detect which contexts are required and will utilize /test Prow commands to trigger the second stage.

For optional jobs, comment /test ? to see a list of all defined jobs. To trigger manually all jobs from second stage use /pipeline required command.

This repository is configured in: LGTM mode

coderabbitai · 2026-08-06T00:12:43Z

📝 Walkthrough

Walkthrough

Configurable PKI now flows from effective profiles through SignerKeyParams into library-go certificate generators. TLS assets, PKI manifests, and the ingress signer retain legacy RSA-2048 generation when the feature is disabled. Tests cover disabled PKI, default ECDSA P-384, RSA-4096, and RSA-2048 profiles.

Changes

Configurable PKI integration

Layer / File(s)	Summary
PKI profile and certificate contracts `pkg/types/pki/defaults.go`, `pkg/types/pki/defaults_test.go`, `pkg/asset/tls/signerkey_params.go`, `pkg/asset/tls/tls.go`, `go.mod`	`SignerKeyParams` now stores the effective PKI profile and feature state. `CertCfg` now records the certificate type. Tests cover legacy and configurable defaults.
Generator-based certificate engine `pkg/asset/tls/certkey.go`, `pkg/asset/tls/certnames.go`, `pkg/asset/tls/certkey_test.go`	Certificate generation accepts optional library-go key generators. Nil generators retain legacy behavior. Tests cover RSA-4096, ECDSA P-384, and legacy RSA-2048 generation.
TLS asset migration `pkg/asset/tls/adminkubeconfig.go`, `pkg/asset/tls/aggregator.go`, `pkg/asset/tls/apiserver.go`, `pkg/asset/tls/iricertkey.go`, `pkg/asset/tls/journalcertkey.go`, `pkg/asset/tls/kubecontrolplane.go`, `pkg/asset/tls/kubelet.go`, `pkg/asset/tls/mcscertkey.go`, `pkg/asset/tls/root.go`	TLS assets resolve signer, serving, peer, and client generators from `SignerKeyParams`. Disabled configurable PKI retains explicit legacy certificate settings.
Manifest and ingress integration `pkg/asset/manifests/pki.go`, `pkg/asset/manifests/pki_test.go`, `pkg/asset/imagebased/configimage/ingressoperatorsigner.go`	PKI manifest generation uses effective profiles. The ingress signer resolves configured signer key generators. Tests use signer parameters directly.

Estimated code review effort: 4 (Complex) | ~60 minutes

🚥 Pre-merge checks | ✅ 14 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 58.82% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (14 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly summarizes the main changes: switching PKI defaults to ECDSA and propagating the PKI profile to leaf certificates.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Stable And Deterministic Test Names	✅ Passed	The PR changes only standard Go tests. All Test and t.Run names are static; no Ginkgo It/Describe/Context/When titles or dynamic values were introduced.
Test Structure And Quality	✅ Passed	PASS: The PR changes only standard Go `testing` tests; no Ginkgo DSL, `Eventually`, `Consistently`, `BeforeEach`, or `AfterEach` code is introduced or modified.
Microshift Test Compatibility	✅ Passed	The pull request adds or modifies only standard Go unit tests; no new Ginkgo e2e tests or MicroShift-sensitive test references were introduced.
Single Node Openshift (Sno) Test Compatibility	✅ Passed	The aggregate PR diff adds no Ginkgo e2e tests. Its changed tests use Go testing and t.Run, so no SNO multi-node assumption applies.
Topology-Aware Scheduling Compatibility	✅ Passed	The complete diff changes PKI/TLS certificate generation only; the scheduling-token scan found no affinities, spread constraints, replica settings, selectors, tolerations, PDBs, or topology logic.
Ote Binary Stdout Contract	✅ Passed	The PR changes PKI asset libraries and vendored resolvers only; diff searches found no main, suite setup, stdout writes, klog, or logging-output changes.
Ipv6 And Disconnected Network Test Compatibility	✅ Passed	The PR changes only standard Go unit tests using testing/testify; it adds no Ginkgo e2e tests or external-network calls. The existing 10.0.0.1 literal is unchanged.
No-Weak-Crypto	✅ Passed	The PR diff adds no MD5, SHA-1, DES, RC4, 3DES, Blowfish, ECB, or secret comparisons; new PKI code delegates certificate generation to library-go APIs.
Container-Privileges	✅ Passed	The full PR diff changes Go PKI/TLS code and library vendoring only; no container manifest changes or added privileged, host namespace, SYS_ADMIN, or allowPrivilegeEscalation settings were found.
No-Sensitive-Data-In-Logs	✅ Passed	The diff adds no sensitive-data logging. Existing logrus debug calls retain generic crypto errors, and library-go certificate APIs used here do not write hostnames, credentials, or customer data.

✨ Finishing Touches

🧪 Generate unit tests (beta)

Create PR with unit tests

Warning

There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure.

🔧 golangci-lint (2.12.2)

Error: can't load config: unsupported version of the configuration: "" See https://golangci-lint.run/docs/product/migration-guide for migration instructions
The command is terminated due to an error: can't load config: unsupported version of the configuration: "" See https://golangci-lint.run/docs/product/migration-guide for migration instructions

_{Comment @coderabbitai help to get the list of available commands.}

openshift-ci · 2026-08-06T00:13:18Z

Skipping CI for Draft Pull Request.
If you want CI signal for your change, please convert it to an actual PR.
You can still manually trigger a test run with /test all

sanchezl · 2026-08-06T00:13:39Z

/test all

openshift-ci · 2026-08-06T00:17:55Z

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by:
Once this PR has been reviewed and has the lgtm label, please assign sadasu 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

sanchezl · 2026-08-06T02:19:23Z

/test all

sanchezl · 2026-08-06T02:19:46Z

/pipeline required

openshift-merge-bot · 2026-08-06T02:19:49Z

Scheduling required tests:
/test e2e-azure-nat-gateway-single-zone

Scheduling tests matching the pipeline_run_if_changed or not excluded by pipeline_skip_if_only_changed parameters:
/test e2e-aws-byo-subnet-role-security-groups
/test e2e-aws-default-config
/test e2e-aws-ovn-edge-zones
/test e2e-aws-ovn-edge-zones-manifest-validation
/test e2e-aws-ovn-fips
/test e2e-aws-ovn-heterogeneous
/test e2e-aws-ovn-imdsv2
/test e2e-aws-ovn-shared-vpc-custom-security-groups
/test e2e-aws-ovn-shared-vpc-edge-zones
/test e2e-aws-ovn-single-node
/test e2e-metal-assisted
/test e2e-metal-ipi-ovn
/test e2e-metal-ipi-ovn-dualstack
/test e2e-metal-ipi-ovn-ipv6
/test e2e-metal-ipi-ovn-swapped-hosts
/test e2e-metal-ipi-ovn-virtualmedia
/test e2e-metal-ovn-two-node-arbiter
/test e2e-metal-ovn-two-node-fencing
/test e2e-metal-single-node-live-iso
/test e2e-vsphere-multi-vcenter-ovn
/test e2e-vsphere-ovn
/test e2e-vsphere-ovn-devpreview
/test e2e-vsphere-ovn-disk-setup-techpreview
/test e2e-vsphere-ovn-hybrid-env
/test e2e-vsphere-ovn-techpreview
/test e2e-vsphere-ovn-zones
/test e2e-aws-ovn

sanchezl · 2026-08-06T02:39:22Z

/test all

sanchezl · 2026-08-06T02:39:23Z

/pipeline required

openshift-merge-bot · 2026-08-06T02:39:26Z

Scheduling required tests:
/test e2e-azure-nat-gateway-single-zone

Scheduling tests matching the pipeline_run_if_changed or not excluded by pipeline_skip_if_only_changed parameters:
/test e2e-aws-byo-subnet-role-security-groups
/test e2e-aws-default-config
/test e2e-aws-ovn-edge-zones
/test e2e-aws-ovn-edge-zones-manifest-validation
/test e2e-aws-ovn-fips
/test e2e-aws-ovn-heterogeneous
/test e2e-aws-ovn-imdsv2
/test e2e-aws-ovn-shared-vpc-custom-security-groups
/test e2e-aws-ovn-shared-vpc-edge-zones
/test e2e-aws-ovn-single-node
/test e2e-metal-assisted
/test e2e-metal-ipi-ovn
/test e2e-metal-ipi-ovn-dualstack
/test e2e-metal-ipi-ovn-ipv6
/test e2e-metal-ipi-ovn-swapped-hosts
/test e2e-metal-ipi-ovn-virtualmedia
/test e2e-metal-ovn-two-node-arbiter
/test e2e-metal-ovn-two-node-fencing
/test e2e-metal-single-node-live-iso
/test e2e-vsphere-multi-vcenter-ovn
/test e2e-vsphere-ovn
/test e2e-vsphere-ovn-devpreview
/test e2e-vsphere-ovn-disk-setup-techpreview
/test e2e-vsphere-ovn-hybrid-env
/test e2e-vsphere-ovn-techpreview
/test e2e-vsphere-ovn-zones
/test e2e-aws-ovn

sanchezl · 2026-08-06T04:26:03Z

/payload-job periodic-ci-openshift-release-main-nightly-5.0-e2e-aws-ovn-upgrade-fips-rhcos9-techpreview

sanchezl · 2026-08-06T04:26:06Z

/payload-job periodic-ci-openshift-release-main-nightly-5.0-e2e-aws-ovn-upgrade-fips-rhcos9-10-techpreview

sanchezl · 2026-08-06T04:26:09Z

/payload-job e2e-metal-ipi-ovn-upgrade-rhcos9-techpreview

openshift-ci · 2026-08-06T04:26:10Z

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

periodic-ci-openshift-release-main-nightly-5.0-e2e-aws-ovn-upgrade-fips-rhcos9-techpreview

See details on https://pr-payload-tests.ci.openshift.org/runs/ci/f047f400-914e-11f1-8e35-4bb42ad1a5c4-0

openshift-ci · 2026-08-06T04:26:11Z

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

periodic-ci-openshift-release-main-nightly-5.0-e2e-aws-ovn-upgrade-fips-rhcos9-10-techpreview

See details on https://pr-payload-tests.ci.openshift.org/runs/ci/f1dfab00-914e-11f1-83c2-8d141c46cd05-0

sanchezl · 2026-08-06T04:26:12Z

/payload-job e2e-metal-ipi-ovn-upgrade-rhcos9-10-techpreview

openshift-ci · 2026-08-06T04:26:14Z

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

e2e-metal-ipi-ovn-upgrade-rhcos9-techpreview

See details on https://pr-payload-tests.ci.openshift.org/runs/ci/f3b02540-914e-11f1-86a2-b0b4d2ce1f86-0

sanchezl · 2026-08-06T04:26:15Z

/payload-job periodic-ci-openshift-release-main-nightly-5.0-e2e-aws-ovn-fips

openshift-ci · 2026-08-06T04:26:16Z

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

e2e-metal-ipi-ovn-upgrade-rhcos9-10-techpreview

See details on https://pr-payload-tests.ci.openshift.org/runs/ci/f554fba0-914e-11f1-82cf-c1f8829a801b-0

sanchezl · 2026-08-06T04:26:18Z

/payload-job periodic-ci-openshift-release-main-ci-5.0-e2e-aws-ovn-techpreview

openshift-ci · 2026-08-06T04:26:19Z

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

periodic-ci-openshift-release-main-nightly-5.0-e2e-aws-ovn-fips

See details on https://pr-payload-tests.ci.openshift.org/runs/ci/f745a810-914e-11f1-9a6b-7a95cb0904d2-0

sanchezl · 2026-08-06T04:26:21Z

/payload-job periodic-ci-openshift-release-main-nightly-5.0-e2e-aws-ovn-single-node-techpreview

openshift-ci · 2026-08-06T04:26:22Z

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

periodic-ci-openshift-release-main-ci-5.0-e2e-aws-ovn-techpreview

See details on https://pr-payload-tests.ci.openshift.org/runs/ci/f8b11ef0-914e-11f1-90de-25ab253cf8e2-0

sanchezl · 2026-08-06T04:26:24Z

/payload-job periodic-ci-openshift-openshift-tests-private-release-5.0-amd64-nightly-aws-usgov-ipi-custom-dns-mini-perm-tp-f7

sanchezl · 2026-08-06T04:26:27Z

/payload-job periodic-ci-openshift-openshift-tests-private-release-5.0-amd64-nightly-aws-c2s-ipi-disc-priv-fips-f28-tp-longduration-cloud

sanchezl · 2026-08-06T04:26:30Z

/payload-job periodic-ci-openshift-openshift-tests-private-release-5.0-amd64-nightly-vsphere-short-cert-rotation-f7

openshift-ci · 2026-08-07T18:53:37Z

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

periodic-ci-openshift-openshift-tests-private-release-5.0-amd64-nightly-aws-usgov-ipi-custom-dns-mini-perm-tp-f7

See details on https://pr-payload-tests.ci.openshift.org/runs/ci/c8ef7c00-9290-11f1-94d0-692d5cbb577d-0

openshift-ci · 2026-08-07T18:53:38Z

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

periodic-ci-openshift-openshift-tests-private-release-5.0-amd64-nightly-vsphere-ipi-proxy-fips-regen-cert-f14

See details on https://pr-payload-tests.ci.openshift.org/runs/ci/ce51a880-9290-11f1-8534-9d3be7d1c4e2-0

openshift-ci · 2026-08-07T18:53:38Z

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

e2e-metal-ipi-ovn-upgrade-rhcos9-10-techpreview

See details on https://pr-payload-tests.ci.openshift.org/runs/ci/c1165c10-9290-11f1-925b-81da9ff1cfcf-0

openshift-ci · 2026-08-07T18:53:38Z

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

periodic-ci-openshift-release-main-nightly-5.0-e2e-aws-ovn-fips

See details on https://pr-payload-tests.ci.openshift.org/runs/ci/c365ca50-9290-11f1-9b7e-dc981939baf2-0

openshift-ci · 2026-08-07T18:53:39Z

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

periodic-ci-openshift-release-main-nightly-5.0-e2e-aws-ovn-upgrade-fips-rhcos9-10-techpreview

See details on https://pr-payload-tests.ci.openshift.org/runs/ci/bd6b7c80-9290-11f1-8dd1-93b85b95cdd7-0

openshift-ci · 2026-08-07T18:53:39Z

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

periodic-ci-openshift-release-main-ci-5.0-e2e-gcp-ovn-rhcos10-fips-techpreview-serial-1of2

See details on https://pr-payload-tests.ci.openshift.org/runs/ci/d09f4200-9290-11f1-9196-c620fcae606f-0

openshift-ci · 2026-08-07T18:53:40Z

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

periodic-ci-openshift-release-main-ci-5.0-e2e-aws-ovn-techpreview

See details on https://pr-payload-tests.ci.openshift.org/runs/ci/c4dc16a0-9290-11f1-8f47-a0fd447535ac-0

New imports from the existing library-go module needed for the PKI profile resolution code path.

…eProfile Replace the local DefaultPKIProfile() with library-go's version (ECDSA P-256 defaults, ECDSA P-384 signers). Add EffectiveProfile() which returns a never-nil PKIProfile and a bool indicating whether ConfigurablePKI is enabled. Keep EffectiveSignerPKIConfig() as a deprecated shim, updated to return ECDSA P-384 when the feature gate is on.

…abled Replace the PKIConfig field with a full configv1alpha1.PKIProfile and a ConfigurablePKIEnabled bool. Load() now calls EffectiveProfile(). Generate() remains a no-op for the agent flow (no install-config on disk). All signer assets switch to the new fields. The PKI manifest asset now depends on SignerKeyParams instead of InstallConfig directly.

SelfSignedCertKey.Generate() and SignedCertKey.Generate() now accept a KeyPairGenerator parameter. When nil, the existing legacy path runs unchanged. When non-nil, cert generation delegates to library-go: NewSigningCertificate for CAs, and NewServerCertificate / NewClientCertificate / NewPeerCertificate for leaf certs based on a new CertType field on CertCfg.

Add a ConfigurablePKIEnabled guard to every signer and leaf cert asset. When the feature gate is off, the legacy code path runs unchanged (explicit KeyUsages, RSA-2048 leaves). When on, each asset resolves a KeyPairGenerator from the PKI profile and delegates to library-go.

JournalCertKey needs both ServerAuth and ClientAuth — use CertificateTypePeer in the library-go path. AdminKubeConfigClientCertKey: drop the legacy ServerAuth ExtKeyUsage in the library-go path — it's a pure client cert (verified working on a live TechPreview cluster with ClientAuth only). Remove the unused LegacyPKIConfig() method from SignerKeyParams. Unskip TestSelfSignedCertKeyGenerateWithKeyGen with real test cases.

coderabbitai · 2026-08-12T13:58:19Z

Note

GitHub couldn't provide a complete incremental comparison for this pull request, so CodeRabbit is performing a full review instead. This review may take a little longer.

sanchezl · 2026-08-12T14:00:29Z

/retest

sanchezl · 2026-08-12T14:01:41Z

/pipeline required

openshift-merge-bot · 2026-08-12T14:01:44Z

Scheduling tests matching the pipeline_run_if_changed or not excluded by pipeline_skip_if_only_changed parameters:
/test e2e-aws-ovn

coderabbitai

♻️ Duplicate comments (2)

pkg/asset/tls/certkey.go (2)
214-222: 🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win

Guard tlsCfg.Certs before indexing.

Line 218 still indexes tlsCfg.Certs[0] without a length check. An empty Certs slice panics the installer instead of returning an error.
🤖 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 `@pkg/asset/tls/certkey.go` around lines 214 - 222, Guard tlsCfg.Certs before
the tlsCfg.Certs[0] access in the certificate encoding flow, returning an
appropriate error when the slice is empty instead of allowing a panic. Preserve
the appendParent branch and existing EncodeCertificates error handling for valid
certificate inputs.
377-403: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

Bounds-check kg.Bits, and verify the generator types are value types.

Two points:

Line 382 narrows kg.Bits (an int) to int32. A value above math.MaxInt32 wraps and can yield a negative RSAKeySize that flows into key generation. Reject out-of-range values.

The type switch matches libcrypto.RSAKeyPairGenerator and libcrypto.ECDSAKeyPairGenerator as value types. If the resolver returns pointers, both cases miss and every non-CA self-signed certificate fails with "unsupported KeyPairGenerator type".
🛡️ Proposed bounds check
 	case libcrypto.RSAKeyPairGenerator:
+		if kg.Bits <= 0 || kg.Bits > math.MaxInt32 {
+			return PrivateKeyParams{}, fmt.Errorf("invalid RSA key size: %d", kg.Bits)
+		}
 		return PrivateKeyParams{
 			Algorithm:  types.KeyAlgorithmRSA,
 			RSAKeySize: int32(kg.Bits),
 		}, nil
Add math to the standard library import group.
🔎 Verification script for generator types
#!/bin/bash
# Confirm whether library-go generators are value or pointer types, and how callers construct them.
fd -t f -e go . vendor/github.com/openshift/library-go/pkg/crypto --exec rg -n -B 2 -A 12 'RSAKeyPairGenerator|ECDSAKeyPairGenerator'
rg -n --type=go -C 3 'RSAKeyPairGenerator\{|ECDSAKeyPairGenerator\{' pkg
🤖 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 `@pkg/asset/tls/certkey.go` around lines 377 - 403, Update keyGenToParams to
reject RSA generators whose Bits value is outside the int32 range before
converting it to RSAKeySize, using the math bounds as needed. Verify how callers
construct the generators, then support the pointer forms of RSAKeyPairGenerator
and ECDSAKeyPairGenerator in the type switch if the resolver returns pointers,
while preserving existing value-type handling and unsupported-type errors.
Source: Linters/SAST tools

🤖 Prompt for all review comments with 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.

Duplicate comments:
In `@pkg/asset/tls/certkey.go`:
- Around line 214-222: Guard tlsCfg.Certs before the tlsCfg.Certs[0] access in
the certificate encoding flow, returning an appropriate error when the slice is
empty instead of allowing a panic. Preserve the appendParent branch and existing
EncodeCertificates error handling for valid certificate inputs.
- Around line 377-403: Update keyGenToParams to reject RSA generators whose Bits
value is outside the int32 range before converting it to RSAKeySize, using the
math bounds as needed. Verify how callers construct the generators, then support
the pointer forms of RSAKeyPairGenerator and ECDSAKeyPairGenerator in the type
switch if the resolver returns pointers, while preserving existing value-type
handling and unsupported-type errors.

ℹ️ Review info

⚙️ Run configuration

Configuration used: Repository YAML (base), Central YAML (inherited)

Review profile: CHILL

Plan: Enterprise

Run ID: 55788cae-309e-4271-bb94-1d3c1e51f030

📥 Commits

Reviewing files that changed from the base of the PR and between 3a117ed and 132e614.

⛔ Files ignored due to path filters (5)

vendor/github.com/openshift/library-go/pkg/pki/profile.go is excluded by !vendor/**, !**/vendor/**
vendor/github.com/openshift/library-go/pkg/pki/provider.go is excluded by !vendor/**, !**/vendor/**
vendor/github.com/openshift/library-go/pkg/pki/resolve.go is excluded by !vendor/**, !**/vendor/**
vendor/github.com/openshift/library-go/pkg/pki/types.go is excluded by !vendor/**, !**/vendor/**
vendor/modules.txt is excluded by !vendor/**, !**/vendor/**

📒 Files selected for processing (20)

go.mod
pkg/asset/imagebased/configimage/ingressoperatorsigner.go
pkg/asset/manifests/pki.go
pkg/asset/manifests/pki_test.go
pkg/asset/tls/adminkubeconfig.go
pkg/asset/tls/aggregator.go
pkg/asset/tls/apiserver.go
pkg/asset/tls/certkey.go
pkg/asset/tls/certkey_test.go
pkg/asset/tls/certnames.go
pkg/asset/tls/iricertkey.go
pkg/asset/tls/journalcertkey.go
pkg/asset/tls/kubecontrolplane.go
pkg/asset/tls/kubelet.go
pkg/asset/tls/mcscertkey.go
pkg/asset/tls/root.go
pkg/asset/tls/signerkey_params.go
pkg/asset/tls/tls.go
pkg/types/pki/defaults.go
pkg/types/pki/defaults_test.go

🚧 Files skipped from review as they are similar to previous changes (18)

pkg/asset/tls/tls.go
pkg/asset/tls/certnames.go
pkg/asset/tls/signerkey_params.go
pkg/asset/tls/journalcertkey.go
pkg/asset/tls/iricertkey.go
pkg/asset/manifests/pki_test.go
pkg/asset/tls/aggregator.go
pkg/types/pki/defaults_test.go
pkg/asset/tls/root.go
pkg/asset/tls/kubelet.go
pkg/asset/manifests/pki.go
pkg/asset/tls/apiserver.go
pkg/asset/tls/certkey_test.go
pkg/asset/tls/mcscertkey.go
pkg/asset/tls/kubecontrolplane.go
pkg/asset/imagebased/configimage/ingressoperatorsigner.go
pkg/types/pki/defaults.go
pkg/asset/tls/adminkubeconfig.go

sanchezl · 2026-08-12T19:45:53Z

/pipeline required

openshift-merge-bot · 2026-08-12T19:45:56Z

Scheduling tests matching the pipeline_run_if_changed or not excluded by pipeline_skip_if_only_changed parameters:
/test e2e-aws-ovn

sanchezl · 2026-08-12T20:47:39Z

/test e2e-aws-ovn-pki-default-techpreview
/test e2e-aws-ovn-pki-rsa-techpreview

sanchezl · 2026-08-12T20:49:18Z

/payoad-job periodic-ci-openshift-openshift-tests-private-release-5.0-multi-nightly-aws-eusc-ipi-fips-tp-arm-f7

sanchezl · 2026-08-13T14:05:36Z

/retest required

sanchezl · 2026-08-14T12:49:50Z

/retest-required

sanchezl · 2026-08-15T00:15:07Z

/test e2e-aws-ovn

openshift-ci · 2026-08-15T04:47:28Z

@sanchezl: 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/e2e-metal-ovn-two-node-arbiter	`b7623bc`	link	false	`/test e2e-metal-ovn-two-node-arbiter`
ci/prow/e2e-aws-ovn-edge-zones	`b7623bc`	link	false	`/test e2e-aws-ovn-edge-zones`
ci/prow/e2e-vsphere-ovn-zones	`b7623bc`	link	false	`/test e2e-vsphere-ovn-zones`
ci/prow/e2e-aws-ovn-fips	`b7623bc`	link	false	`/test e2e-aws-ovn-fips`
ci/prow/e2e-metal-ovn-two-node-fencing	`b7623bc`	link	false	`/test e2e-metal-ovn-two-node-fencing`
ci/prow/e2e-vsphere-multi-vcenter-ovn	`b7623bc`	link	false	`/test e2e-vsphere-multi-vcenter-ovn`
ci/prow/e2e-vsphere-ovn-techpreview	`b7623bc`	link	false	`/test e2e-vsphere-ovn-techpreview`
ci/prow/e2e-metal-ipi-ovn-swapped-hosts	`b7623bc`	link	false	`/test e2e-metal-ipi-ovn-swapped-hosts`
ci/prow/e2e-aws-ovn-single-node	`b7623bc`	link	false	`/test e2e-aws-ovn-single-node`
ci/prow/e2e-metal-ipi-ovn-virtualmedia	`b7623bc`	link	false	`/test e2e-metal-ipi-ovn-virtualmedia`
ci/prow/e2e-vsphere-ovn-devpreview	`b7623bc`	link	false	`/test e2e-vsphere-ovn-devpreview`
ci/prow/e2e-metal-single-node-live-iso	`b7623bc`	link	false	`/test e2e-metal-single-node-live-iso`
ci/prow/e2e-metal-ipi-ovn-dualstack	`b7623bc`	link	false	`/test e2e-metal-ipi-ovn-dualstack`
ci/prow/e2e-vsphere-ovn-hybrid-env	`b7623bc`	link	false	`/test e2e-vsphere-ovn-hybrid-env`
ci/prow/e2e-vsphere-ovn-disk-setup-techpreview	`b7623bc`	link	false	`/test e2e-vsphere-ovn-disk-setup-techpreview`
ci/prow/e2e-aws-default-config	`b7623bc`	link	false	`/test e2e-aws-default-config`
ci/prow/e2e-aws-ovn-heterogeneous	`b7623bc`	link	false	`/test e2e-aws-ovn-heterogeneous`
ci/prow/e2e-aws-ovn-shared-vpc-edge-zones	`b7623bc`	link	false	`/test e2e-aws-ovn-shared-vpc-edge-zones`
ci/prow/e2e-vsphere-ovn	`b7623bc`	link	true	`/test e2e-vsphere-ovn`
ci/prow/e2e-metal-assisted	`b7623bc`	link	false	`/test e2e-metal-assisted`
ci/prow/e2e-aws-ovn-imdsv2	`b7623bc`	link	false	`/test e2e-aws-ovn-imdsv2`
ci/prow/e2e-aws-byo-subnet-role-security-groups	`b7623bc`	link	false	`/test e2e-aws-byo-subnet-role-security-groups`
ci/prow/e2e-aws-ovn-shared-vpc-custom-security-groups	`b7623bc`	link	false	`/test e2e-aws-ovn-shared-vpc-custom-security-groups`
ci/prow/e2e-metal-ipi-ovn-ipv6	`b7623bc`	link	true	`/test e2e-metal-ipi-ovn-ipv6`
ci/prow/e2e-metal-ipi-ovn	`b7623bc`	link	false	`/test e2e-metal-ipi-ovn`
ci/prow/e2e-aws-ovn	`132e614`	link	true	`/test e2e-aws-ovn`

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.

openshift-ci Bot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Aug 6, 2026

sanchezl force-pushed the pki-ecdsa-defaults branch from 06ad91c to 9f4aa80 Compare August 6, 2026 02:18

sanchezl force-pushed the pki-ecdsa-defaults branch from 9f4aa80 to 65df21c Compare August 6, 2026 02:36

openshift-ci Bot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Aug 10, 2026

sanchezl added 7 commits August 12, 2026 09:54

vendor: add library-go pkg/pki to vendor

2627753

New imports from the existing library-go module needed for the PKI profile resolution code path.

tls: add resolveKeyGen helpers for library-go PKI profile resolution

4638c2f

sanchezl force-pushed the pki-ecdsa-defaults branch from b7623bc to 132e614 Compare August 12, 2026 13:58

openshift-ci Bot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Aug 12, 2026

coderabbitai Bot reviewed Aug 12, 2026

View reviewed changes

Uh oh!

Conversation

sanchezl commented Aug 6, 2026 • edited by coderabbitai Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Why

What

Why this is safe

Design decisions from prior PRs

Commit walkthrough

Test plan

Summary by CodeRabbit

Uh oh!

openshift-merge-bot Bot commented Aug 6, 2026

Uh oh!

coderabbitai Bot commented Aug 6, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Walkthrough

Changes

❌ Failed checks (1 warning)

Uh oh!

openshift-ci Bot commented Aug 6, 2026

Uh oh!

sanchezl commented Aug 6, 2026

Uh oh!

openshift-ci Bot commented Aug 6, 2026

Uh oh!

sanchezl commented Aug 6, 2026

Uh oh!

sanchezl commented Aug 6, 2026

Uh oh!

openshift-merge-bot Bot commented Aug 6, 2026

Uh oh!

sanchezl commented Aug 6, 2026

Uh oh!

sanchezl commented Aug 6, 2026

Uh oh!

openshift-merge-bot Bot commented Aug 6, 2026

Uh oh!

sanchezl commented Aug 6, 2026

Uh oh!

sanchezl commented Aug 6, 2026

Uh oh!

sanchezl commented Aug 6, 2026

Uh oh!

openshift-ci Bot commented Aug 6, 2026

Uh oh!

openshift-ci Bot commented Aug 6, 2026

Uh oh!

sanchezl commented Aug 6, 2026

Uh oh!

openshift-ci Bot commented Aug 6, 2026

Uh oh!

sanchezl commented Aug 6, 2026

Uh oh!

openshift-ci Bot commented Aug 6, 2026

Uh oh!

sanchezl commented Aug 6, 2026

Uh oh!

openshift-ci Bot commented Aug 6, 2026

Uh oh!

sanchezl commented Aug 6, 2026

Uh oh!

openshift-ci Bot commented Aug 6, 2026

Uh oh!

sanchezl commented Aug 6, 2026

Uh oh!

sanchezl commented Aug 6, 2026

Uh oh!

sanchezl commented Aug 6, 2026

Uh oh!

openshift-ci Bot commented Aug 7, 2026

Uh oh!

openshift-ci Bot commented Aug 7, 2026

Uh oh!

openshift-ci Bot commented Aug 7, 2026

Uh oh!

openshift-ci Bot commented Aug 7, 2026

Uh oh!

openshift-ci Bot commented Aug 7, 2026

Uh oh!

sanchezl commented Aug 6, 2026 •

edited by coderabbitai Bot

Loading

coderabbitai Bot commented Aug 6, 2026 •

edited

Loading