CNTRLPLANE-3237: operator/encryption: add FlatEntry and FormatKMSSecretDataKey helpers for secret data keys

[p0lyn0mial]

Summary by CodeRabbit

• Refactor

  • Improved KMS plugin secret data key parsing and formatting to enhance internal code structure and maintainability.
  • Introduced new helper utilities for consistent key formatting and flat entry generation in encryption data handling.

• Tests

  • Updated encryption key parsing test coverage to reflect internal improvements.

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

Walkthrough

This PR refactors KMS secret data key handling by extracting string formatting logic into dedicated helpers. The changes introduce FormatKMSSecretDataKey to standardize KMS plugin secret key formatting in the encryptiondata package, and add a FlatEntry helper in the state package to build flat map entry keys consistently.

Changes

KMS Secret Data Key Helpers Refactoring

Layer / File(s)	Summary
KMS Secret Data Key Format and Parser pkg/operator/encryption/encryptiondata/secret.go, pkg/operator/encryption/encryptiondata/secret_test.go	New exported FormatKMSSecretDataKey(rawKey, keyID) helper builds the "kms-plugin-secret-{rawKey}-{keyID}" format. Internal parser is renamed to parseKMSSecretDataKey. FromSecret and ToSecret are updated to use these helpers instead of direct string concatenation, and the test is updated to verify the renamed parser.
KMSSecretData Flat Entry Helper pkg/operator/encryption/state/types.go	New FlatEntry(secretName, dataKey) method on KMSSecretData builds the flat key format "secretName_dataKey". FlatEntries() is updated to use this helper for constructing map keys instead of inline concatenation.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~8 minutes

Possibly related PRs

• openshift/library-go#2262: Related refactor of KMSSecretData.FlatEntries behavior and flat key construction patterns.

Suggested reviewers

• dgrisonnet

🚥 Pre-merge checks | ✅ 14 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 33.33% 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 describes the main changes: adding FlatEntry and FormatKMSSecretDataKey helper functions for secret data key handling in the operator/encryption package.
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	PR modifies only standard Go testing files (secret_test.go, types_test.go), not Ginkgo tests. All test names are static and descriptive with no dynamic content.
Test Structure And Quality	✅ Passed	The repository does not use Ginkgo testing framework. No Ginkgo test code (It blocks, BeforeEach, etc.) exists in this PR or codebase. The check is not applicable.
Microshift Test Compatibility	✅ Passed	PR adds only unit tests in standard Go testing package, not Ginkgo e2e tests. Check is not applicable.
Single Node Openshift (Sno) Test Compatibility	✅ Passed	This PR adds unit tests and helper functions only; no Ginkgo e2e tests are present. The check for SNO compatibility is not applicable.
Topology-Aware Scheduling Compatibility	✅ Passed	PR adds only helper functions for encryption secret data key handling with no scheduling constraints, pod affinity rules, node selectors, or topology-dependent workload configurations.
Ote Binary Stdout Contract	✅ Passed	PR modifies library-go (shared operator library), not an OTE test binary. New helper functions contain no stdout writes and are not process-level entry points.
Ipv6 And Disconnected Network Test Compatibility	✅ Passed	No Ginkgo e2e tests found in this PR. Changes are limited to unit tests and helper functions in openshift/library-go's encryption operator package.
No-Weak-Crypto	✅ Passed	Helper functions added for KMS secret data key formatting/parsing. No weak crypto algorithms, custom implementations, or insecure secret comparisons detected.
Container-Privileges	✅ Passed	PR modifies only Go source files for encryption helpers; no Kubernetes manifests or container configs with privileged settings were changed.
No-Sensitive-Data-In-Logs	✅ Passed	PR adds helper functions for secret data keys with no logging of sensitive data. Error messages only log non-sensitive metadata like keyID integers and key names, never secret values.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

---

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

[ardaguclu]

Is there a particular reason to locate it under KMSSecretData? or just to follow the convention of FlatEntries?

[p0lyn0mial]

It belongs to KMSSecretData semantically.

It's the encoding convention that Set, SetFromRawKey, and FlatEntries all revolve around.
Keeping it on the type makes it discoverable alongside those methods.

Does it make sense ?

[coderabbitai]

🧹 Nitpick comments (1)

pkg/operator/encryption/encryptiondata/secret.go (1)

213-214: 💤 Low value

Consider using + concatenation for consistency and performance.

Using fmt.Sprintf for simple string concatenation is slightly less efficient than the + operator. The FlatEntry helper in types.go (line 136) uses + for similar concatenation, so using + here would maintain consistency.

♻️ Proposed refactor

 func FormatKMSSecretDataKey(rawKey, keyID string) string {
-	return fmt.Sprintf("%s%s-%s", encryptionConfigSecretDataPrefix, rawKey, keyID)
+	return encryptionConfigSecretDataPrefix + rawKey + "-" + keyID
 }

🤖 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/operator/encryption/encryptiondata/secret.go` around lines 213 - 214, The
function FormatKMSSecretDataKey currently builds the key string with fmt.Sprintf
which is unnecessary for simple concatenation; replace the fmt.Sprintf usage
with direct string concatenation using the + operator (use
encryptionConfigSecretDataPrefix + rawKey + "-" + keyID) in
FormatKMSSecretDataKey so it matches the FlatEntry helper's style and avoids fmt
import/overhead.

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

Nitpick comments:
In `@pkg/operator/encryption/encryptiondata/secret.go`:
- Around line 213-214: The function FormatKMSSecretDataKey currently builds the
key string with fmt.Sprintf which is unnecessary for simple concatenation;
replace the fmt.Sprintf usage with direct string concatenation using the +
operator (use encryptionConfigSecretDataPrefix + rawKey + "-" + keyID) in
FormatKMSSecretDataKey so it matches the FlatEntry helper's style and avoids fmt
import/overhead.

---

ℹ️ Review info

⚙️ Run configuration

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

Review profile: CHILL

Plan: Enterprise

Run ID: bd8e4c72-9b03-429f-8592-ed7843278ae6

📥 Commits

Reviewing files that changed from the base of the PR and between 11735f6 and 26942ec.

📒 Files selected for processing (3)

• pkg/operator/encryption/encryptiondata/secret.go
• pkg/operator/encryption/encryptiondata/secret_test.go
• pkg/operator/encryption/state/types.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

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