CNTRLPLANE-3237: kms: wire preflight deployer

[bertinatto]

Summary by CodeRabbit

• Bug Fixes
  • Improved encryption configuration handling for the OAuth API server.
  • Added periodic KMS preflight validation, with checks every 10 seconds.
  • Improved reliability when preparing encryption settings and validating key management service readiness.
• Maintenance
  • Updated supporting components to use a newer maintained library version for improved compatibility and stability.

[openshift-merge-bot]

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]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Repository: openshift/coderabbit/.coderabbit.yaml

Review profile: CHILL

Plan: Enterprise

Run ID: 00e6c250-f207-4bf2-9045-791651038e3b

📥 Commits

Reviewing files that changed from the base of the PR and between f332826 and 21942ff.

⛔ Files ignored due to path filters (2)

• go.sum is excluded by !**/*.sum
• vendor/modules.txt is excluded by !**/vendor/**, !vendor/**

📒 Files selected for processing (1)

• go.mod

Included review availability: Your plan provides up to 12 included reviews per hour; 9 remain after this review.

---

Walkthrough

The operator updates the library-go dependency. The OAuth API server now uses a pod-based KMS preflight deployer and the default EncryptionPlanner-based encryption configuration computation.

Changes

OAuth encryption setup

Layer / File(s)	Summary
Library dependency update go.mod	The module updates github.com/openshift/library-go to a newer pseudo-version.
Encryption preflight wiring pkg/operator/starter.go	The operator configures the pod-based KMS preflight deployer with Kubernetes clients, preflight settings, the operator image, and a 10-second interval. It selects the default EncryptionPlanner-based computation and removes the unused import.

Estimated code review effort: 2 (Simple) | ~10 minutes

Merge Risk: ⚪ Minimal · up to 21942

This change introduces no evidenced merge-blocking or material user or production risk and is merge-ready after normal checks and review.

Suggested reviewers: atiratree, flavianmissi

---

Important

Pre-merge checks failed

Please resolve all errors before merging. Addressing warnings is optional.

❌ Failed checks (1 error, 2 warnings)

Check name	Status	Explanation	Resolution
No-Sensitive-Data-In-Logs	❌ Error	The pull request activates the real KMS preflight pod through NewPodPreflightDeployer in pkg/operator/starter.go. The preflight checker logs every KMS EncryptResponse.Annotations value at `vendo…	Do not log KMS annotation values. Log only non-sensitive metadata such as the annotation count, or remove the annotation field from the message. Review other KMS error and status paths for credential-bearing values before enabling the prefl…
Docstring Coverage	⚠️ Warning	Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 1 functions across 1 files. (1 skipped: 1 …	Write docstrings for the functions missing them to satisfy the coverage threshold.
Topology-Aware Scheduling Compatibility	⚠️ Warning	The change activates a preflight Pod that has a hard-coded nodeSelector for node-role.kubernetes.io/master. The PR changes starter.go from NewAlwaysSucceedKMSPreflightDeployer() to `NewPodPref…	Make the preflight workload topology-aware. Do not require node-role.kubernetes.io/master on External clusters. Select suitable hosted-cluster worker nodes for HyperShift, while preserving valid placement for SNO, TNF, and TNA. Keep arb…

✅ Passed checks (12 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly and concisely describes the main change: wiring the KMS preflight deployer.
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	No changed test titles were found. The pull-request diff contains no *_test.go files, and the changed Go files contain no It(), Describe(), Context(), or When() declarations. The remaining changes are…
Test Structure And Quality	✅ Passed	PASS: The pull request changes no Ginkgo test files. The target-to-HEAD diff contains only dependency/vendor updates and production code in pkg/operator/starter.go; changed Go files contain no It,…
Microshift Test Compatibility	✅ Passed	The pull request adds no Ginkgo e2e tests and modifies no test files. The diff contains dependency/vendor updates and production encryption wiring only. Therefore, the MicroShift test compatibility ch…
Single Node Openshift (Sno) Test Compatibility	✅ Passed	No new Ginkgo e2e tests were added. The PR changes dependency/vendor files and OAuth API server encryption wiring only. Therefore, no test introduces a multi-node or HA assumption under this check.
Ote Binary Stdout Contract	✅ Passed	PASS: The PR does not change the OTE binary entrypoint or suite setup. cmd/cluster-authentication-operator-tests-ext/main.go remains unchanged and calls klog.LogToStderr(true) before command execu…
Ipv6 And Disconnected Network Test Compatibility	✅ Passed	No new Ginkgo e2e tests were added. The pull request changes go.mod, go.sum, pkg/operator/starter.go, and vendored encryption library Go files. The diff contains no added *_test.go files and n…
No-Weak-Crypto	✅ Passed	PASS: The pull request introduces no MD5, SHA1, DES, 3DES, RC4, Blowfish, or ECB usage. The changed controller code uses existing AES-256 key generation through crypto/rand. The new FNV-32 code hashes…
Container-Privileges	✅ Passed	No custom-check failure condition is introduced. The pull request changes starter.go to use NewPodPreflightDeployer, whose constructor leaves virtualStaticPod false. Therefore the conditional `h…

Full details: Docstring Coverage

Explanation

Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 1 functions across 1 files. (1 skipped: 1 unsupported.)

Full details: Stable And Deterministic Test Names

Explanation

No changed test titles were found. The pull-request diff contains no *_test.go files, and the changed Go files contain no It(), Describe(), Context(), or When() declarations. The remaining changes are production encryption code and dependency metadata, so the check's failure conditions are not introduced.

Full details: Test Structure And Quality

Explanation

PASS: The pull request changes no Ginkgo test files. The target-to-HEAD diff contains only dependency/vendor updates and production code in pkg/operator/starter.go; changed Go files contain no It, Eventually, Consistently, or Ginkgo assertions. The existing KMS tests are unchanged, and their Ginkgo blocks include explicit 120-minute timeouts.

Full details: Microshift Test Compatibility

Explanation

The pull request adds no Ginkgo e2e tests and modifies no test files. The diff contains dependency/vendor updates and production encryption wiring only. Therefore, the MicroShift test compatibility check is not applicable.

Full details: Topology-Aware Scheduling Compatibility

Explanation

The change activates a preflight Pod that has a hard-coded nodeSelector for node-role.kubernetes.io/master. The PR changes starter.go from NewAlwaysSucceedKMSPreflightDeployer() to NewPodPreflightDeployer(...). The KMS controller calls this deployer when KMS encryption requires preflight validation. The Pod template has no ControlPlaneTopology check. On HyperShift (External), the hosted cluster has no master or control-plane nodes, so the preflight Pod remains Pending. This matches the explicit control-plane nodeSelector failure condition.

Resolution

Make the preflight workload topology-aware. Do not require node-role.kubernetes.io/master on External clusters. Select suitable hosted-cluster worker nodes for HyperShift, while preserving valid placement for SNO, TNF, and TNA. Keep arbiter nodes excluded on TNA and avoid broad taint tolerations. Pass the topology-aware placement into the preflight Pod template or add the required topology check before creating the Pod.

Full details: Ote Binary Stdout Contract

Explanation

PASS: The PR does not change the OTE binary entrypoint or suite setup. cmd/cluster-authentication-operator-tests-ext/main.go remains unchanged and calls klog.LogToStderr(true) before command execution. The added code only wires the KMS preflight deployer and encryption planner. The changed Go lines add no stdout writes, init, TestMain, suite setup, or RunSpecs code. The changed vendored encryption code has no direct stdout writes or initialization code; its klog calls are inside controller methods, not process-level code.

Full details: Ipv6 And Disconnected Network Test Compatibility

Explanation

No new Ginkgo e2e tests were added. The pull request changes go.mod, go.sum, pkg/operator/starter.go, and vendored encryption library Go files. The diff contains no added *_test.go files and no added It(), Describe(), Context(), or When() declarations. Therefore, this custom test-compatibility check is not applicable.

Full details: No-Weak-Crypto

Explanation

PASS: The pull request introduces no MD5, SHA1, DES, 3DES, RC4, Blowfish, or ECB usage. The changed controller code uses existing AES-256 key generation through crypto/rand. The new FNV-32 code hashes KMS configuration data for change detection, not encryption or authentication. Added comparisons check modes, resource names, state, and configuration hashes. They do not compare secret or token values. The existing vendor/modules.txt Blowfish entry is unchanged.

Full details: Container-Privileges

Explanation

No custom-check failure condition is introduced. The pull request changes starter.go to use NewPodPreflightDeployer, whose constructor leaves virtualStaticPod false. Therefore the conditional hostNetwork: true in the unchanged preflight template is not rendered for this path. The changed files add no privileged: true, hostPID, hostIPC, SYS_ADMIN, allowPrivilegeEscalation: true, or runAsUser: 0 settings. The existing operator runAsUser: 0 setting is unchanged and has an inline justification.

Full details: No-Sensitive-Data-In-Logs

Explanation

The pull request activates the real KMS preflight pod through NewPodPreflightDeployer in pkg/operator/starter.go. The preflight checker logs every KMS EncryptResponse.Annotations value at vendor/.../kms/preflight/checker.go:97. The KMS API defines these annotations as arbitrary additional metadata, and its protocol states that sensitive data may be present unless the plugin pre-encrypts it. Therefore, the newly activated pod can write sensitive metadata to logs.

Resolution

Do not log KMS annotation values. Log only non-sensitive metadata such as the annotation count, or remove the annotation field from the message. Review other KMS error and status paths for credential-bearing values before enabling the preflight checker.

• Fix all pre-merge checks with AI

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

---

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

[openshift-ci]

[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 liouk 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

[bertinatto]

/pipeline required

[openshift-merge-bot]

Scheduling required tests:
/test e2e-agnostic
/test e2e-agnostic-upgrade
/test e2e-console-login
/test e2e-gcp-operator-disruptive
/test e2e-oidc
/test e2e-operator

Scheduling tests matching the pipeline_run_if_changed or not excluded by pipeline_skip_if_only_changed parameters:
/test e2e-aws-operator-encryption-kms
/test e2e-aws-operator-encryption-kms-2
/test e2e-gcp-operator-encryption-perf
/test e2e-gcp-operator-encryption-rotation
/test e2e-operator-encryption

[bertinatto]

/pipeline required

[openshift-merge-bot]

Scheduling required tests:
/test e2e-agnostic
/test e2e-agnostic-upgrade
/test e2e-console-login
/test e2e-gcp-operator-disruptive
/test e2e-oidc
/test e2e-operator

Scheduling tests matching the pipeline_run_if_changed or not excluded by pipeline_skip_if_only_changed parameters:
/test e2e-aws-operator-encryption-kms
/test e2e-aws-operator-encryption-kms-2
/test e2e-gcp-operator-encryption-perf
/test e2e-gcp-operator-encryption-rotation
/test e2e-operator-encryption

[openshift-ci-robot]

@bertinatto: This pull request references CNTRLPLANE-3237 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 "5.1.0" version, but no target version was set.

Details

In response to this:

Summary by CodeRabbit

• Bug Fixes
• Improved encryption configuration handling for the OAuth API server.
• Added real preflight validation for the key management service (KMS), including periodic checks every 10 seconds.
• Updated supporting components to use the maintained library implementation for improved compatibility.

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]

@bertinatto: 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-agnostic	f332826	link	true	/test e2e-agnostic
ci/prow/e2e-console-login	f332826	link	true	/test e2e-console-login
ci/prow/unit	21942ff	link	true	/test 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.