CNTRLPLANE-3237: kms preflight: add AlwaysSucceedKMSPreflightDeployer

[p0lyn0mial]

Summary by CodeRabbit

• New Features

  • Added a KMS preflight deployment option that records configuration, reports successful readiness conditions, and supports cleanup.
  • Status now clearly indicates when a preflight deployment has not yet been created or has been removed.
  • Preflight status reports success even when configuration details are empty.

• Tests

  • Added coverage for deployment, status reporting, required KMS conditions, empty configuration, and cleanup behavior.

[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: 1de4c05a-19c2-4fce-86a0-dd7fec4c7d58

📥 Commits

Reviewing files that changed from the base of the PR and between a620ed5 and cecc12a.

📒 Files selected for processing (2)

• pkg/operator/encryption/kms/preflight/always_succeed_deployer.go
• pkg/operator/encryption/kms/preflight/always_succeed_deployer_test.go

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

• pkg/operator/encryption/kms/preflight/always_succeed_deployer_test.go
• pkg/operator/encryption/kms/preflight/always_succeed_deployer.go

---

Walkthrough

Adds an always-success KMS preflight deployer. It records the configuration hash, reports successful KMS conditions after deployment, returns NotFound before deployment and after cleanup, and includes lifecycle tests.

Changes

KMS preflight deployment

Layer / File(s)	Summary
Deployer lifecycle behavior pkg/operator/encryption/kms/preflight/always_succeed_deployer.go	Adds construction, deployment, status reporting, and cleanup. The deployer records the configuration hash and reports successful KMS preflight conditions.
Lifecycle validation pkg/operator/encryption/kms/preflight/always_succeed_deployer_test.go	Tests initial and post-cleanup NotFound states, successful pod status, configuration-hash reporting, successful-result reporting, remote key ID reporting, and empty-hash deployment.

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

Suggested reviewers: ardaguclu, dgrisonnet

🚥 Pre-merge checks | ✅ 14 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Test Structure And Quality	⚠️ Warning	The added test combines pre-deploy Status, successful Deploy conditions, Cleanup reset, and empty-hash behavior in one test function (lines 17–65).	Split the lifecycle and empty-hash checks into focused test functions or subtests with independent setup; retain the existing diagnostic assertion messages.

✅ 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 identifies the KMS preflight component added by this pull request and includes the related issue identifier.
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 added test uses standard testing, not Ginkgo. Its only test name is the static TestAlwaysSucceedKMSPreflightDeployer; dynamic values appear only in the test body.
Microshift Test Compatibility	✅ Passed	The added test is a standard Go testing.T unit test, not a Ginkgo e2e test, and it references no MicroShift-unavailable APIs, namespaces, or unsupported assumptions.
Single Node Openshift (Sno) Test Compatibility	✅ Passed	The patch adds only a standard Go unit test (func Test...) in the preflight package; it adds no Ginkgo e2e test or multi-node assumption.
Topology-Aware Scheduling Compatibility	✅ Passed	The PR adds only an in-memory preflight deployer; Deploy records state and Status returns PodStatus, with no workload manifest, scheduling field, topology logic, or node constraint changed.
Ote Binary Stdout Contract	✅ Passed	The two changed Go files add no stdout, logging, init, main, TestMain, or suite-setup code; they only implement and test the deployer.
Ipv6 And Disconnected Network Test Compatibility	✅ Passed	The added test is a standard Go Test function, not a Ginkgo e2e test, and the changed files contain no IPv4 assumptions or external network operations.
No-Weak-Crypto	✅ Passed	The PR adds no crypto imports, weak algorithms, cipher modes, custom crypto, or secret/token comparisons; it only stores and reports a configuration hash.
Container-Privileges	✅ Passed	Both changed files are Go source/test files, not manifests; added-line and structural scans found no privileged settings, host namespaces, SYS_ADMIN, root execution, or privilege escalation.
No-Sensitive-Data-In-Logs	✅ Passed	The added deployer and test contain no logging calls and do not emit passwords, tokens, keys, 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.}

[coderabbitai]

Actionable comments posted: 1

🤖 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.

Inline comments:
In `@pkg/operator/encryption/kms/preflight/always_succeed_deployer.go`:
- Around line 22-24: Use a separate boolean deployment-state field in
AlwaysSucceedKMSPreflightDeployer rather than inferring deployment from
configHash, so Deploy with an empty hash still causes Status to report success.
Update Deploy and Status to maintain and consult this state, and add a lifecycle
test covering deployment with an empty hash.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

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

Review profile: CHILL

Plan: Enterprise

Run ID: 94325e6c-641d-4c0b-93fa-f929372226df

📥 Commits

Reviewing files that changed from the base of the PR and between 26e4f37 and a620ed5.

📒 Files selected for processing (2)

• pkg/operator/encryption/kms/preflight/always_succeed_deployer.go
• pkg/operator/encryption/kms/preflight/always_succeed_deployer_test.go

[ardaguclu]

/lgtm

[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: #2392

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