CNTRLPLANE-3361: kms: accept VaultKMSPluginConfig directly in newVaultSidecarProvider

[p0lyn0mial]

Summary by CodeRabbit

• Refactor

  • Enhanced internal structure of KMS encryption plugin configuration handling by simplifying how Vault-specific configuration is passed through the plugin lifecycle, reducing unnecessary object nesting and improving overall code clarity, maintainability, and efficiency.

• Tests

  • Comprehensive test updates to validate the refactored configuration handling patterns and behavior across multiple scenarios and use cases.

[openshift-ci-robot]

@p0lyn0mial: This pull request references CNTRLPLANE-3361 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

The PR refactors Vault KMS plugin sidecar provider initialization to eliminate configuration indirection. newVaultSidecarProvider now accepts configv1.VaultKMSPluginConfig directly and stores it as a value, while the caller passes pluginConfig.Vault directly. Tests are updated to match the simplified interface.

Changes

Vault KMS Config Parameter Refactoring

Layer / File(s)	Summary
Config struct field and constructor signature pkg/operator/encryption/kms/pluginlifecycle/vault.go	The vault struct's config field changes from pointer *configv1.VaultKMSPluginConfig to value type configv1.VaultKMSPluginConfig; newVaultSidecarProvider signature updates to accept and assign the Vault config directly.
Caller site and test updates pkg/operator/encryption/kms/pluginlifecycle/sidecar.go, pkg/operator/encryption/kms/pluginlifecycle/vault_test.go	newSidecarProvider extracts and passes pluginConfig.Vault directly; test fixtures and three test cases are refactored to use vaultConfig as a top-level field, and provider instantiation is updated to pass the simplified config.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Suggested labels

approved, lgtm

Suggested reviewers

• dgrisonnet

🚥 Pre-merge checks | ✅ 14 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 66.67% 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.
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	Tests use standard Go testing package, not Ginkgo. The custom check for Ginkgo test name stability does not apply to this PR.
Test Structure And Quality	✅ Passed	Custom check for Ginkgo test code structure is not applicable - vault_test.go uses Go's standard testing package with t.Run() subtests, not Ginkgo (no Describe/It/BeforeEach/AfterEach blocks).
Microshift Test Compatibility	✅ Passed	No Ginkgo e2e tests are added in this PR. Only standard Go unit tests (testing.T) are modified in vault_test.go. Check only applies to Ginkgo tests.
Single Node Openshift (Sno) Test Compatibility	✅ Passed	No new Ginkgo e2e tests added. The vault_test.go changes update an existing Go unit test, not e2e tests. Check does not apply.
Topology-Aware Scheduling Compatibility	✅ Passed	PR is a pure refactoring of KMS sidecar configuration passing. No new scheduling constraints (affinity, tolerations, nodeSelector, topology spread) are introduced by these changes.
Ote Binary Stdout Contract	✅ Passed	PR adds new KMS plugin sidecar code without any process-level main, init, TestMain, or suite functions. No stdout writes detected; klog calls are within normal function scope, not process-level setup.
Ipv6 And Disconnected Network Test Compatibility	✅ Passed	PR does not add Ginkgo e2e tests. Modified files contain only standard Go unit tests (vault_test.go uses testing.T, not Ginkgo patterns), so custom check is not applicable.
No-Weak-Crypto	✅ Passed	No weak crypto patterns detected. PR refactors KMS config handling without introducing MD5, SHA1, DES, RC4, 3DES, Blowfish, ECB, custom crypto, or insecure secret comparisons.
Container-Privileges	✅ Passed	No privileged container settings found. PR refactors KMS plugin config passing without modifying container security configurations.
No-Sensitive-Data-In-Logs	✅ Passed	No logging statements expose sensitive data. Vault configuration values are only placed in container arguments, never logged. Error messages and logs only expose namespace/secret names and counts.
Title check	✅ Passed	The title clearly and specifically describes the main change: refactoring newVaultSidecarProvider to accept VaultKMSPluginConfig directly instead of the wrapper KMSPluginConfig, which aligns with all three modified files.

_{✏️ 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.}

[p0lyn0mial]

previously the function received pluginConfig by value but was storing an address of a tmp var allocated on the heap.

[ardaguclu]

/lgtm
/hold
until @bertinatto checks it, as the owner of these bits

[ardaguclu]

I think this is the right direction. Side cars are specifically generated per each provider, so that it is better to pass their own dedicated configuration.

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

[bertinatto]

/hold cancel
/lgtm

[openshift-ci]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: ardaguclu, bertinatto, 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,bertinatto,p0lyn0mial]

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