Skip to content

CNTRLPLANE-3236: support deploying Vault mock KMS plugin#693

Open
bertinatto wants to merge 5 commits into
openshift:mainfrom
bertinatto:kms-plugins-lifecycle-vault-mock
Open

CNTRLPLANE-3236: support deploying Vault mock KMS plugin#693
bertinatto wants to merge 5 commits into
openshift:mainfrom
bertinatto:kms-plugins-lifecycle-vault-mock

Conversation

@bertinatto
Copy link
Copy Markdown
Member

@bertinatto bertinatto commented May 20, 2026

Summary by CodeRabbit

  • Improvements
    • KMS encryption for OpenShift APIServer deployments now uses a KMS plugin sidecar instead of injecting volumes/mounts, deriving encryption configuration from the operator revision and respecting feature gates for smoother rollouts.
  • Tests
    • Encryption e2e tests updated to use context-aware shared helpers for more robust execution.

@openshift-ci-robot openshift-ci-robot added the jira/valid-reference Indicates that this PR references a valid Jira ticket of any type. label May 20, 2026
@openshift-ci openshift-ci Bot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label May 20, 2026
@openshift-ci-robot
Copy link
Copy Markdown

openshift-ci-robot commented May 20, 2026

@bertinatto: This pull request references CNTRLPLANE-3236 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:

  • test changes
  • go mod update
  • kms: support deploying the vault mock plugin
  • bump(openshift/library-go): to get KMS plugin lifecycle changes

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 openshift-ci Bot requested review from kaleemsiddiqu and sanchezl May 20, 2026 15:48
@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented May 20, 2026

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

  • @coderabbitai resume to resume automatic reviews.
  • @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

  • ▶️ Resume reviews
  • 🔍 Trigger review

Walkthrough

Migrates OpenShift APIServer KMS integration from legacy volume/mounts to a kmspluginlifecycle KMS plugin sidecar; updates import and replaces pod-spec volume/mount logic with sidecar injection using operatorConfig.Status.LatestAvailableRevision.

Changes

KMS Plugin Sidecar Migration

Layer / File(s) Summary
KMS plugin sidecar integration
pkg/operator/workload/workload_openshiftapiserver_v311_00_sync.go, go.mod
Import changed from encryptionkms to kmspluginlifecycle. Deployment reconciliation now injects a KMS plugin sidecar (encryption-config name derived from operatorConfig.Status.LatestAvailableRevision, passing core client and featureGateAccessor) instead of adding encryption volumes/mounts; failure message updated. github.com/openshift/library-go dependency version bumped in go.mod.
E2E test context & KMS config updates
test/e2e-encryption-kms/encryption_kms.go, test/e2e-encryption-rotation/e2e-encryption-rotation_test.go, test/e2e-encryption/encryption_test.go, test/e2e/operator_test.go
Tests updated to pass contexts (e.g., t.Context() or ctx) into shared library helpers; KMS provider config now uses librarykms.DefaultFakeKMSPluginConfig directly and unused configv1 import removed.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related PRs

Suggested reviewers

  • gangwgr

Suggested labels

lgtm

🚥 Pre-merge checks | ✅ 10 | ❌ 2

❌ Failed checks (2 warnings)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
Test Structure And Quality ⚠️ Warning Tests lack assertion messages per requirement 4. All require.NoError(t, err) calls are bare without diagnostic messages to help identify failures. Add meaningful failure messages to assertions: require.NoError(t, err, "failed to create/delete test namespace")
✅ Passed checks (10 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title 'CNTRLPLANE-3236: support deploying Vault mock KMS plugin' directly matches the main objective of the PR—enabling deployment of a Vault mock KMS plugin—as evidenced by the workload sync file changes that switch from legacy encryptionkms to kmspluginlifecycle KMS plugin sidecar approach.
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 All test names are stable and deterministic. Ginkgo test titles use static strings; Go test function names are static. Dynamic values (namespace names) appear only in test bodies, not titles.
Microshift Test Compatibility ✅ Passed Tests with [OCPFeatureGate:KMSEncryption] are automatically skipped on MicroShift. NetworkPolicy tests use standard Kubernetes APIs available on MicroShift.
Single Node Openshift (Sno) Test Compatibility ✅ Passed This PR does not add new Ginkgo e2e tests; it only modifies existing test calls to pass context arguments. The check applies only when new tests are added.
Topology-Aware Scheduling Compatibility ✅ Passed PR changes KMS sidecar injection only, not scheduling constraints. Existing anti-affinity and master nodeSelector predate this change.
Ote Binary Stdout Contract ✅ Passed PR has no stdout violations: no fmt.Print/os.Stdout writes or klog misconfig in process-level code. Changes are KMS integration and test parameter adjustments only.
Ipv6 And Disconnected Network Test Compatibility ✅ Passed PR modifies existing tests only, adding no new Ginkgo test definitions. No IPv4 assumptions or external connectivity requirements found in test code.

✏️ 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.

@bertinatto bertinatto changed the title [WIP] CNTRLPLANE-3236: support deploying Vault mock KMS plugin CNTRLPLANE-3236: support deploying Vault mock KMS plugin May 20, 2026
@openshift-ci openshift-ci Bot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label May 20, 2026
@bertinatto bertinatto force-pushed the kms-plugins-lifecycle-vault-mock branch from 8d648f4 to ee53bca Compare May 20, 2026 21:10
@gangwgr
Copy link
Copy Markdown
Contributor

gangwgr commented May 21, 2026

/retest

@gangwgr
Copy link
Copy Markdown
Contributor

gangwgr commented May 21, 2026

/test e2e-gcp-operator-encryption-kms-ote

@gangwgr
Copy link
Copy Markdown
Contributor

gangwgr commented May 21, 2026

/retest

@bertinatto
Copy link
Copy Markdown
Member Author

/retest-required

Copy link
Copy Markdown
Member

@ardaguclu ardaguclu left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

/lgtm
/approve
/hold
for confirmation

&required.Spec.Template.Spec,
"openshift-apiserver",
operatorclient.TargetNamespace,
fmt.Sprintf("encryption-config-%d", operatorConfig.Status.LatestAvailableRevision),
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

confirmed that this is how we're currently setting the revision in the deployment manifest:

"${REVISION}", strconv.Itoa(int(operatorConfig.Status.LatestAvailableRevision)),

@openshift-ci openshift-ci Bot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label May 21, 2026
@openshift-ci openshift-ci Bot added the lgtm Indicates that a PR is ready to be merged. label May 21, 2026
@openshift-ci
Copy link
Copy Markdown
Contributor

openshift-ci Bot commented May 21, 2026

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: ardaguclu

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:

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

@openshift-ci openshift-ci Bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label May 21, 2026
@openshift-ci openshift-ci Bot removed the lgtm Indicates that a PR is ready to be merged. label May 21, 2026
@openshift-ci
Copy link
Copy Markdown
Contributor

openshift-ci Bot commented May 21, 2026

New changes are detected. LGTM label has been removed.

@bertinatto bertinatto force-pushed the kms-plugins-lifecycle-vault-mock branch from 8f796bd to 5dc5009 Compare May 21, 2026 17:47
@openshift-ci
Copy link
Copy Markdown
Contributor

openshift-ci Bot commented May 21, 2026

@bertinatto: The following test 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-gcp-operator-encryption-kms-ote 5dc5009 link false /test e2e-gcp-operator-encryption-kms-ote

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

approved Indicates a PR has been approved by an approver from all required OWNERS files. do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. jira/valid-reference Indicates that this PR references a valid Jira ticket of any type.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants