WIP by sanchezl · Pull Request #10462 · openshift/installer

sanchezl · 2026-04-04T13:27:14Z

No description provided.

Add configurable PKI support to InstallConfig behind the ConfigurablePKI feature gate, allowing users to specify cryptographic parameters for installer-generated signer certificates. Key changes: - Define custom PKIConfig type with only signerCertificates field - Add PKI *PKIConfig field to InstallConfig - Add ConfigurablePKI feature gate entry in GatedFeatures() - Create pkg/types/pki/ with validation functions - Wire PKI validation into ValidateInstallConfig() - Require signerCertificates.key when pki is present (pki: {} is invalid) Assisted-by: Claude Code (Opus 4.6)

Update pkg/asset/tls/ to generate signer certificates with either RSA or ECDSA keys based on the PKI config from InstallConfig. Leaf certificates continue to use RSA 2048. Key changes: - Add DefaultRSAKeySize/DefaultKeyAlgorithm constants - Add RSA/ECDSA key generation functions and PEM encode/decode support - Change SelfSignedCertificate to accept crypto.Signer - Change SignedCertificate/GenerateSignedCertificate caKey to crypto.PrivateKey - Add pkiConfig parameter to SelfSignedCertKey.Generate() - Update PKIConfigToKeyParams to accept *types.PKIConfig - Set algorithm-appropriate KeyUsage (ECDSA omits KeyEncipherment) - All signer assets now depend on InstallConfig and pass PKI config - Add type assertion in BoundSASigningKey.Load() for RSA requirement Assisted-by: Claude Code (Opus 4.6)

Add tests covering PKI validation, feature gate enforcement, certificate generation with PKI configs, and cross-algorithm certificate signing. Key additions: - Test ValidatePKIConfig catches invalid configs and empty PKI - Test ConfigurablePKI feature gate with TechPreview and CustomNoUpgrade - Test ValidateInstallConfig catches invalid PKI with field paths - Test SelfSignedCertKey.Generate() with non-nil PKI configs - Test ECDSA CA signing RSA leaf certificate with chain verification - Test RSA/ECDSA key generation, KeyUsage flags, signature algorithm detection - Test PEM encode/decode roundtrip for RSA and ECDSA keys Assisted-by: Claude Code (Opus 4.6)

Document the configurable PKI feature in docs/user/customization.md, following the existing inline documentation pattern for install-config properties. Key additions: - Add pki field entry with nested signerCertificates structure - Add RSA 4096 and ECDSA P-384 install-config example fragments - Document ConfigurablePKI feature gate requirement - Note scope: signer certificates only, leaf certs unaffected Assisted-by: Claude Code (Opus 4.6)

…abled When the ConfigurablePKI feature gate is active, the installer now generates a config.openshift.io/v1alpha1 PKI custom resource manifest (manifests/cluster-pki-02-config.yaml) that is applied to the cluster during bootstrap. This CR provides day-2 operators with the certificate parameters to use when rotating certificates. The PKI CR uses mode: Custom with DefaultPKIProfile as the base (defaults: ECDSA P-256, signerCertificates: ECDSA P-384). User overrides from install-config pki.signerCertificates are layered on top. When the feature gate is enabled but pki is not specified in install-config, the installer also aligns its own signer cert generation to ECDSA P-384 (matching DefaultPKIProfile) instead of the legacy RSA-2048 default. Assisted-by: Claude Code (Opus 4.6)

openshift-ci · 2026-04-04T13:27: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

coderabbitai · 2026-04-04T13:27:22Z

Important

Review skipped

Auto reviews are limited based on label configuration.

🚫 Review skipped — only excluded labels are configured. (1)

do-not-merge/work-in-progress

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 7bbf2213-cbdc-4fe9-a2cd-901aa5500941

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

🔍 Trigger review

✨ Finishing Touches

🧪 Generate unit tests (beta)

Create PR with unit tests

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

openshift-ci · 2026-04-04T13:27:42Z

[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 pawanpinjarkar 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-04-04T13:35:32Z

/test

sanchezl · 2026-04-04T13:35:59Z

/test all

Replace configv1alpha1.CertificateConfig embedding in types.PKIConfig with installer-local types (CertificateConfig, KeyConfig, RSAKeyConfig, ECDSAKeyConfig, KeyAlgorithm, ECDSACurve). This prevents accidentally picking up new fields from the upstream API type. The JSON serialization shape is unchanged, so install-config YAML parsing and the CRD remain compatible. Conversion to upstream API types happens at the boundary when building the PKI CR manifest.

Add SignerPKIConfig asset to the dependency graph that resolves the effective PKI profile for certificate generation. The profile is never nil: feature gate off returns explicit RSA 2048, feature gate on returns library-go's DefaultPKIProfile with user overrides. EffectiveSignerPKIProfile now returns configv1alpha1.PKIProfile (never nil) instead of *types.PKIConfig (nullable). The local DefaultPKIProfile copy is replaced by an import from library-go/pkg/pki. The deprecated EffectiveSignerPKIConfig shim is retained temporarily for callers not yet migrated.

SelfSignedCertKey.Generate() now accepts a libcrypto.KeyPairGenerator and delegates to library-go's NewSigningCertificate for CA certs. Non-CA self-signed certs (IronicTLSCert) fall back to the legacy path. All 11 signer assets now depend on SignerPKIConfig and resolve their KeyPairGenerator via libpki.ResolveCertificateConfig with the <component>.<purpose> certificate naming convention.

sanchezl · 2026-04-04T14:16:42Z

/retest required

sanchezl · 2026-04-04T14:17:08Z

/test all

SignedCertKey.Generate() now uses library-go's GetCAFromBytes to reconstruct the CA, then dispatches to NewServerCertificate, NewClientCertificate, or NewPeerCertificate based on an explicit CertificateType field on CertCfg. Each leaf cert asset now depends on SignerPKIConfig and resolves its KeyPairGenerator via libpki.ResolveCertificateConfig. When ConfigurablePKI is enabled, leaves follow the profile defaults (ECDSA P-256). When disabled, they use RSA 2048. AppendParent is handled by selectively encoding only the leaf cert (DoNotAppendParent) or the full chain (AppendParent) from the library-go TLSCertificateConfig.

PrivateKeyToPem now delegates to libcrypto.EncodeKey and CertToPem delegates to libcrypto.EncodeCertificates. Both are marked as deprecated in favor of using library-go directly. PemToPrivateKey and PemToCertificate retain their implementations since library-go doesn't expose standalone PEM parsing functions, but are also marked deprecated.

PKIConfiguration now depends on tls.SignerPKIConfig instead of InstallConfig directly. It uses the pre-resolved Profile to build the PKI CR, and gates manifest emission on ConfigurablePKIEnabled. This eliminates duplicate config resolution logic and the local toAPICertificateConfig conversion function.

Replace the forked selfSignedCertificate, generateSelfSignedCertificate, generateSubjectKeyID, and rsaPublicKey with a single call to libcrypto.NewSigningCertificate. The forked code existed to allow empty OU in the Subject, which library-go handles natively. This removes ~80 lines of duplicated crypto code including a SHA-1 SubjectKeyId implementation.

…review # Conflicts: # go.mod # go.sum # vendor/modules.txt

sanchezl · 2026-04-05T18:21:33Z

/test all

Files like .idea/, .envrc, dev-retest*.sh, CLAUDE-luis.md, and docs/design/ drafts were untracked local files that got swept up by git add -A during the merge conflict resolution.

sanchezl · 2026-04-05T18:39:05Z

/retest-required

sanchezl · 2026-04-05T18:51:07Z

/retest-required

sanchezl · 2026-04-05T19:34:40Z

/test all

The tls_assets testscript test runs "openshift-install agent create certificates" which now requires InstallConfig in the dependency chain (via SignerPKIConfig). Previously signer assets like KubeAPIServerLBSignerCertKey had no dependencies, but now they depend on SignerPKIConfig which depends on InstallConfig. Add a minimal install-config.yaml and agent-config.yaml to the test, matching the pattern used by other agent image asset tests.

sanchezl · 2026-04-05T21:11:52Z

/retest-required

The "agent create certificates" command generates signer certs at runtime on the bootstrap node (for the agent UI flow) without an install-config.yaml present. Before the library-go migration, these signers had zero dependencies and always generated RSA 2048 keys. The SignerPKIConfig asset now implements WritableAsset with a Load() method that checks for install-config on disk. When install-config is absent, Load() returns the default RSA 2048 profile, bypassing the Generate() dependency chain entirely. When install-config is present, Load() returns false and the normal Generate() path resolves the profile from InstallConfig. This also reverts the install-config addition to the tls_assets integration test, since the test should work without it.

sanchezl · 2026-04-05T21:27:42Z

/retest-required

sanchezl · 2026-04-05T21:48:42Z

/retest-required

sanchezl · 2026-04-06T01:15:40Z

/retest-required

sanchezl · 2026-04-06T01:16:21Z

/retest

sanchezl · 2026-04-06T03:29:29Z

/test all

sanchezl · 2026-04-06T03:31:18Z

/retest

sanchezl · 2026-04-06T14:14:50Z

/label do-not-merge/work-in-progress

openshift-ci · 2026-04-06T14:14:53Z

@sanchezl: The label(s) /label do-not-merge/work-in-progress cannot be applied. These labels are supported: acknowledge-critical-fixes-only, platform/aws, platform/azure, platform/baremetal, platform/google, platform/libvirt, platform/openstack, ga, tide/merge-method-merge, tide/merge-method-rebase, tide/merge-method-squash, px-approved, docs-approved, qe-approved, ux-approved, no-qe, rebase/manual, cluster-config-api-changed, run-integration-tests, verified, approved, backport-risk-assessed, bugzilla/valid-bug, cherry-pick-approved, jira/skip-dependent-bug-check, jira/valid-bug, ok-to-test, stability-fix-approved, staff-eng-approved. Is this label configured under labels -> additional_labels or labels -> restricted_labels in plugin.yaml?

Details

In response to this:

/label do-not-merge/work-in-progress

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.

sanchezl · 2026-04-06T14:15:30Z

/hold work-in-progress

openshift-ci · 2026-04-06T19:12:37Z

@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-single-node-live-iso	`88daf8f`	link	false	`/test e2e-metal-single-node-live-iso`
ci/prow/integration-tests	`88daf8f`	link	true	`/test integration-tests`
ci/prow/e2e-metal-ipi-ovn-dualstack	`88daf8f`	link	false	`/test e2e-metal-ipi-ovn-dualstack`
ci/prow/e2e-metal-ipi-ovn-virtualmedia	`88daf8f`	link	false	`/test e2e-metal-ipi-ovn-virtualmedia`
ci/prow/e2e-metal-assisted	`88daf8f`	link	false	`/test e2e-metal-assisted`
ci/prow/e2e-aws-ovn	`88daf8f`	link	true	`/test e2e-aws-ovn`
ci/prow/e2e-metal-ovn-two-node-fencing	`88daf8f`	link	false	`/test e2e-metal-ovn-two-node-fencing`

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.

hasbro17 and others added 6 commits March 18, 2026 11:46

vendor: bump library-go

0779824

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 Apr 4, 2026

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

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

sanchezl force-pushed the install-config-pki-review branch from e9f8ba0 to a41ad19 Compare April 4, 2026 14:06

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

sanchezl added 3 commits April 4, 2026 10:13

sanchezl force-pushed the install-config-pki-review branch from a41ad19 to f5e4533 Compare April 4, 2026 14:14

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

sanchezl added 4 commits April 5, 2026 14:14

sanchezl force-pushed the install-config-pki-review branch from fb0bf42 to b4808ea Compare April 5, 2026 18:14

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

Merge remote-tracking branch 'upstream/main' into install-config-pki-…

4918ce4

…review # Conflicts: # go.mod # go.sum # vendor/modules.txt

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

remove accidentally committed local files

3d7f48a

Files like .idea/, .envrc, dev-retest*.sh, CLAUDE-luis.md, and docs/design/ drafts were untracked local files that got swept up by git add -A during the merge conflict resolution.

pki: fix deprecated comment formatting in defaults.go

a349a48

tls: fix gofmt import grouping in signerpkiconfig.go

88daf8f

sanchezl force-pushed the install-config-pki-review branch from eba946f to 88daf8f Compare April 6, 2026 01:15

sanchezl marked this pull request as ready for review April 6, 2026 14:12

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

openshift-ci bot requested review from sadasu and tsorya April 6, 2026 14:14

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

Conversation

sanchezl commented Apr 4, 2026

Uh oh!

openshift-ci bot commented Apr 4, 2026

Uh oh!

coderabbitai bot commented Apr 4, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Review skipped

Uh oh!

openshift-ci bot commented Apr 4, 2026

Uh oh!

sanchezl commented Apr 4, 2026

Uh oh!

sanchezl commented Apr 4, 2026

Uh oh!

sanchezl commented Apr 4, 2026

Uh oh!

sanchezl commented Apr 4, 2026

Uh oh!

sanchezl commented Apr 5, 2026

Uh oh!

sanchezl commented Apr 5, 2026

Uh oh!

sanchezl commented Apr 5, 2026

Uh oh!

sanchezl commented Apr 5, 2026

Uh oh!

sanchezl commented Apr 5, 2026

Uh oh!

sanchezl commented Apr 5, 2026

Uh oh!

sanchezl commented Apr 5, 2026

Uh oh!

sanchezl commented Apr 6, 2026

Uh oh!

sanchezl commented Apr 6, 2026

Uh oh!

sanchezl commented Apr 6, 2026

Uh oh!

sanchezl commented Apr 6, 2026

Uh oh!

sanchezl commented Apr 6, 2026

Uh oh!

openshift-ci bot commented Apr 6, 2026

Uh oh!

sanchezl commented Apr 6, 2026

Uh oh!

openshift-ci bot commented Apr 6, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

coderabbitai bot commented Apr 4, 2026 •

edited

Loading