CNTRLPLANE-3237: kms preflight: don't degrade when encryption is disabled

[p0lyn0mial]

Summary by CodeRabbit

• Bug Fixes
  • Improved encryption preflight handling when the cluster is no longer configured for KMS.
  • Prevented stale KMS configuration data from triggering incorrect preflight behavior.
  • Ensured cleanup completes correctly without incorrectly reporting degraded or progressing status.

[openshift-ci-robot]

@p0lyn0mial: 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.0.0" version, but no target version was set.

Details

In response to this:

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.

[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: b7610c80-79e5-49fd-ab95-5588bff01efb

📥 Commits

Reviewing files that changed from the base of the PR and between aa9e810 and ba8bc75.

📒 Files selected for processing (2)

• pkg/operator/encryption/controllers/kms_preflight_controller.go
• pkg/operator/encryption/controllers/kms_preflight_controller_test.go

---

Walkthrough

The controller now checks the APIServer encryption type before KMS preflight processing. It skips preflight for identity encryption, even with a stale KMS hash. Tests verify pod cleanup and unchanged status conditions.

Changes

KMS preflight handling

Layer / File(s)	Summary
Gate preflight on encryption type pkg/operator/encryption/controllers/kms_preflight_controller.go	preflightRequired retrieves the APIServer configuration first and skips preflight when encryption is not KMS.
Validate identity-encryption cleanup pkg/operator/encryption/controllers/kms_preflight_controller_test.go	The fixture sets KMS encryption explicitly. A test verifies cleanup after reverting to identity encryption with a stale observed hash.

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

Possibly related PRs

• openshift/library-go#2392: Also changes preflightRequired to skip preflight for non-KMS encryption and stale hashes.
• openshift/library-go#2396: Related to KMS preflight status, hash, and cleanup handling.

🚥 Pre-merge checks | ✅ 15

✅ Passed checks (15 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 change to prevent KMS preflight degradation when encryption is disabled.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
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 changed test uses a static table name, with no Ginkgo titles or dynamic interpolation; the added title contains only fixed descriptive text.
Test Structure And Quality	✅ Passed	The added table-driven test uses isolated fake clients, asserts cleanup and conditions with diagnostic messages, and has no cluster waits requiring timeouts; its pattern matches nearby controller t...
Microshift Test Compatibility	✅ Passed	The PR adds no Ginkgo e2e tests. The changed test uses Go's testing.T and local fake clients, so the MicroShift API compatibility check does not apply.
Single Node Openshift (Sno) Test Compatibility	✅ Passed	The PR adds a standard Go unit-test scenario in pkg/operator/encryption/controllers, not a Ginkgo e2e test, and it makes no multi-node or HA assumptions.
Topology-Aware Scheduling Compatibility	✅ Passed	The commit only changes APIServer encryption-type handling and tests; it adds no deployment manifests or scheduling constraints such as affinity, topology spread, selectors, tolerations, replicas,...
Ote Binary Stdout Contract	✅ Passed	The PR adds no stdout writes, entry-point code, or suite setup. The existing klog call remains inside preflightRequired, outside the specified process-level scopes.
Ipv6 And Disconnected Network Test Compatibility	✅ Passed	The added scenario is a standard Go table test, not a Ginkgo e2e test. It uses fake clients and no IPv4 assumptions or external network connectivity.
No-Weak-Crypto	✅ Passed	The commit only reorders APIServer checks, adds config API imports, and adds tests; it introduces no MD5, SHA1, DES, RC4, Blowfish, ECB, crypto, or secret-comparison code.
Container-Privileges	✅ Passed	The PR changes only two Go files and adds no privileged settings; the existing preflight manifest with hostNetwork: true is unchanged.
No-Sensitive-Data-In-Logs	✅ Passed	The patch adds no logging or event calls. Existing output contains only KMS config hashes and pre-existing pod/API errors, not passwords, tokens, PII, hostnames, or customer data.

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

---

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

[ardaguclu]

There is nothing to cleanup?

[ardaguclu]

ah, I found the caller. Ignore my comment

[p0lyn0mial]

the controller always clean up in this case -

library-go/pkg/operator/encryption/controllers/kms_preflight_controller.go

Line 465 in ba8bc75

if requiredHash == "" {

does it make sense ?

[ardaguclu]

/lgtm
/approve

[openshift-ci]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: ardaguclu, p0lyn0mial

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:

• pkg/operator/encryption/OWNERS [ardaguclu,p0lyn0mial]

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

[p0lyn0mial]

xref: openshift/cluster-kube-apiserver-operator#2252

[openshift-ci]

@p0lyn0mial: all tests passed!

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.