CNTRLPLANE-2241: Conditionally add the KMS plugin volume mount to the kube-apiserver container by bertinatto · Pull Request #2015 · openshift/cluster-kube-apiserver-operator

bertinatto · 2026-01-23T18:49:00Z

/assign @ardaguclu @benluddy @p0lyn0mial

coderabbitai · 2026-01-23T18:49:21Z

Walkthrough

Update adds a new featureGateAccessor parameter and field to the target config controller APIs and propagates it through callers for KMS plugin volume/mount integration; also updates dependency versions in go.mod and promotes gomega to a direct require.

Changes

Cohort / File(s)	Summary
Target Config Controller `pkg/operator/starter.go`, `pkg/operator/targetconfigcontroller/targetconfigcontroller.go`	Add `featureGateAccessor featuregates.FeatureGateAccess` field to `TargetConfigController`; update `NewTargetConfigController` signature to accept it; update `managePods` signature and callers to pass `featureGateAccessor` and invoke `encryptionkms.AddKMSPluginVolumeAndMountToPodSpec` with it.
Dependencies `go.mod`	Bump `github.com/openshift/api`, `github.com/openshift/client-go`, `github.com/openshift/library-go`; add `github.com/onsi/gomega v1.35.1` as a direct require.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

✨ Finishing touches

📝 Generate docstrings

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

bertinatto · 2026-01-23T19:21:32Z

/payload-job periodic-ci-openshift-release-master-nightly-4.22-e2e-vsphere-ovn-techpreview
/payload-job periodic-ci-openshift-release-master-nightly-4.22-e2e-vsphere-ovn-techpreview-serial

openshift-ci · 2026-01-23T19:21:38Z

@bertinatto: trigger 2 job(s) for the /payload-(with-prs|job|aggregate|job-with-prs|aggregate-with-prs) command

periodic-ci-openshift-release-master-nightly-4.22-e2e-vsphere-ovn-techpreview
periodic-ci-openshift-release-master-nightly-4.22-e2e-vsphere-ovn-techpreview-serial

See details on https://pr-payload-tests.ci.openshift.org/runs/ci/ba812410-f890-11f0-8937-792694951cf7-0

benluddy · 2026-01-23T20:48:46Z

pkg/operator/targetconfigcontroller/targetconfigcontroller.go

+		return fmt.Errorf("failed to get apiserver/cluster: %w", err)
+	}
+
+	if apiServer.Spec.Encryption.Type != configv1.EncryptionTypeKMS {


This decision needs to be based on the revision we're currently rendering, not the current state. We also need to keep mounting the socket path when we're migrating from a revision where KMS was previously enabled.

Maybe the encryption controllers should be able to return any host path(s) they need for a given revision? This is subtle, and we will want to keep implementations in sync across the different apiserver operators.

This decision needs to be based on the revision we're currently rendering, not the current state. We also need to keep mounting the socket path when we're migrating from a revision where KMS was previously enabled.

I agree with @benluddy.

Let's say, APIServer CR is configured like this;

apiVersion: config.openshift.io/v1 kind: APIServer metadata: name: cluster spec: encryption: type: KMS

Encryption controllers generates;

apiVersion: apiserver.config.k8s.io/v1 kind: EncryptionConfiguration resources: - resources: - secrets providers: - kms: name: myKmsProvider endpoint: unix:///var/run/kms-plugin/socket.sock cachesize: 100 timeout: 3s - identity: {}

After that cluster admin moves to aescbc;

apiVersion: config.openshift.io/v1 kind: APIServer metadata: name: cluster spec: encryption: type: aescbc

EncryptionConfiguration is in this format;

apiVersion: apiserver.config.k8s.io/v1 kind: EncryptionConfiguration resources: - resources: - secrets providers: - aescbc: keys: - name: key1 secret: key - kms: # <---- KMS is still in here name: myKmsProvider endpoint: unix:///var/run/kms-plugin/socket.sock cachesize: 100 timeout: 3s - identity: {}

In order to migrate resources from KMS to aescbc, kms plugin must still be accessible.

In my opinion, if feature gate is enabled, we can always mount this hostPath. In v2, if we adopt side-car approach, kms plugin will be accessible within the pod network and we won't need hostPath.

Maybe the encryption controllers should be able to return any host path(s) they need for a given revision? This is subtle, and we will want to keep implementations in sync across the different apiserver operators.

Encryption controllers are provider agnostic. So they don't know these details. But I believe that we'll need plugin lifecycle controllers which can return this information.

Let’s also add a comment that this solution is considered temporary and is needed for v1.
In the future, we will probably come up with a different solution.

I also think that for v1, if FG is enabled, we could simply use hostPath - let's keep it simple for now.

It looks like the new FG will be KMSEncryption

Switched to use the feature gate instead. Also, added a comment stating this is a temporary solution.

bertinatto · 2026-01-26T17:29:09Z

/payload-job periodic-ci-openshift-release-master-nightly-4.22-e2e-vsphere-ovn-techpreview
/payload-job periodic-ci-openshift-release-master-nightly-4.22-e2e-vsphere-ovn-techpreview-serial

openshift-ci · 2026-01-26T17:29:12Z

@bertinatto: trigger 2 job(s) for the /payload-(with-prs|job|aggregate|job-with-prs|aggregate-with-prs) command

periodic-ci-openshift-release-master-nightly-4.22-e2e-vsphere-ovn-techpreview
periodic-ci-openshift-release-master-nightly-4.22-e2e-vsphere-ovn-techpreview-serial

See details on https://pr-payload-tests.ci.openshift.org/runs/ci/86364ca0-fadc-11f0-8791-89ff27c17445-0

bertinatto · 2026-01-26T17:53:43Z

/hold
For https://github.com/openshift/api/pull/2669/files (I'll need to re-vendor and change the feature gate name).

bertinatto · 2026-01-26T19:53:39Z

/payload-job periodic-ci-openshift-release-master-nightly-4.22-e2e-vsphere-ovn-techpreview
/payload-job periodic-ci-openshift-release-master-nightly-4.22-e2e-vsphere-ovn-techpreview-serial

openshift-ci · 2026-01-26T19:53:44Z

@bertinatto: trigger 2 job(s) for the /payload-(with-prs|job|aggregate|job-with-prs|aggregate-with-prs) command

periodic-ci-openshift-release-master-nightly-4.22-e2e-vsphere-ovn-techpreview
periodic-ci-openshift-release-master-nightly-4.22-e2e-vsphere-ovn-techpreview-serial

See details on https://pr-payload-tests.ci.openshift.org/runs/ci/b6582390-faf0-11f0-9d5b-cd80cedec70e-0

bertinatto · 2026-01-27T14:00:19Z

/payload-job periodic-ci-openshift-release-master-nightly-4.22-e2e-vsphere-ovn-techpreview
/payload-job periodic-ci-openshift-release-master-nightly-4.22-e2e-vsphere-ovn-techpreview-serial

openshift-ci · 2026-01-27T14:00:38Z

@bertinatto: trigger 2 job(s) for the /payload-(with-prs|job|aggregate|job-with-prs|aggregate-with-prs) command

periodic-ci-openshift-release-master-nightly-4.22-e2e-vsphere-ovn-techpreview
periodic-ci-openshift-release-master-nightly-4.22-e2e-vsphere-ovn-techpreview-serial

See details on https://pr-payload-tests.ci.openshift.org/runs/ci/845ac2b0-fb88-11f0-86f7-a63e297741b3-0

bertinatto · 2026-01-28T17:54:41Z

/payload-job periodic-ci-openshift-release-master-nightly-4.22-e2e-vsphere-ovn-techpreview
/payload-job periodic-ci-openshift-release-master-nightly-4.22-e2e-vsphere-ovn-techpreview-serial

openshift-ci · 2026-01-28T17:55:13Z

@bertinatto: trigger 2 job(s) for the /payload-(with-prs|job|aggregate|job-with-prs|aggregate-with-prs) command

periodic-ci-openshift-release-master-nightly-4.22-e2e-vsphere-ovn-techpreview
periodic-ci-openshift-release-master-nightly-4.22-e2e-vsphere-ovn-techpreview-serial

See details on https://pr-payload-tests.ci.openshift.org/runs/ci/6c0f51e0-fc72-11f0-95f5-0f998cdda80a-0

bertinatto · 2026-01-28T20:04:24Z

/retest

bertinatto · 2026-01-29T06:45:07Z

/test k8s-e2e-gcp

bertinatto · 2026-01-29T10:23:54Z

/retest

ardaguclu · 2026-01-29T12:35:22Z

go.mod

 	sigs.k8s.io/kube-storage-version-migrator v0.0.6-0.20230721195810-5c8923c5ff96
 )

+require github.com/onsi/gomega v1.35.1


What caused this change?

also could it be merged with the existing require block ?

no, go.mod isn't supposed to be edited manually, go mod should be used instead. Something in the chain of the library-go bump caused this

coderabbitai

Actionable comments posted: 1

🤖 Fix all issues with AI agents

In `@go.mod`:
- Around line 40-41: The standalone require for github.com/onsi/gomega v1.35.1
// indirect should be moved into the existing indirect require block so all
indirect dependencies are grouped together; edit the go.mod to remove the
separate line and insert that require entry into the existing indirect block
(near the other // indirect entries) preserving alphabetical order among the
package paths.

go.mod

p0lyn0mial · 2026-01-29T13:46:31Z

pkg/operator/targetconfigcontroller/targetconfigcontroller.go

 	}

+	if err := encryptionkms.AddKMSPluginVolumeAndMountToPodSpec(&required.Spec, "kube-apiserver", featureGateAccessor); err != nil {
+		return nil, false, fmt.Errorf("failed to add KMS encryption volumes: %v", err)


wrap the err with %w

…ontainer

p0lyn0mial · 2026-01-29T15:58:05Z

/lgtm

p0lyn0mial · 2026-01-29T15:59:02Z

/retitle CNTRLPLANE-2241: Conditionally add the KMS plugin volume mount to the kube-apiserver container

openshift-ci-robot · 2026-01-29T15:59:32Z

@bertinatto: This pull request references CNTRLPLANE-2241 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 "4.22.0" version, but no target version was set.

Details

In response to this:

/assign @ardaguclu @benluddy @p0lyn0mial

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 · 2026-01-29T16:02:52Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: 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:

~~OWNERS~~ [p0lyn0mial]

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

bertinatto · 2026-01-29T18:24:44Z

/retest-required

openshift-ci · 2026-01-29T21:41:54Z

@bertinatto: The following tests 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-single-node	`f82dace`	link	false	`/test e2e-gcp-operator-encryption-single-node`
ci/prow/e2e-gcp-operator-serial-ote	`f82dace`	link	false	`/test e2e-gcp-operator-serial-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.

bertinatto · 2026-01-29T22:32:59Z

/verified by @bertinatto

openshift-ci-robot · 2026-01-29T22:33:16Z

@bertinatto: This PR has been marked as verified by @bertinatto.

Details

In response to this:

/verified by @bertinatto

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.

bertinatto · 2026-03-05T12:41:46Z

/cherry-pick release-4.21

openshift-cherrypick-robot · 2026-03-05T12:42:33Z

@bertinatto: #2015 failed to apply on top of branch "release-4.21":

Applying: Conditionally add the KMS plugin volume mount to the kube-apiserver container
Applying: Bump library-go to get KMS helper
.git/rebase-apply/patch:380: trailing whitespace.
update-non-codegen: update-protobuf tests-vendor update-prerelease-lifecycle-gen 
.git/rebase-apply/patch:1165: new blank line at EOF.
+
.git/rebase-apply/patch:5737: new blank line at EOF.
+
warning: 3 lines add whitespace errors.
Using index info to reconstruct a base tree...
M	go.mod
M	go.sum
M	vendor/modules.txt
Falling back to patching base and 3-way merge...
Auto-merging vendor/modules.txt
CONFLICT (content): Merge conflict in vendor/modules.txt
Removing vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1/gcpserviceendpoint.go
Auto-merging go.sum
CONFLICT (content): Merge conflict in go.sum
Auto-merging go.mod
CONFLICT (content): Merge conflict in go.mod
error: Failed to merge in the changes.
hint: Use 'git am --show-current-patch=diff' to see the failed patch
hint: When you have resolved this problem, run "git am --continue".
hint: If you prefer to skip this patch, run "git am --skip" instead.
hint: To restore the original branch and stop patching, run "git am --abort".
hint: Disable this message with "git config set advice.mergeConflict false"
Patch failed at 0002 Bump library-go to get KMS helper

Details

In response to this:

/cherry-pick release-4.21

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.

ardaguclu · 2026-03-05T12:56:03Z

/cherry-pick release-4.21

In any case I will have to bump apiservers with the changes in library-go. Maybe it is better to get the changes in that PR.

openshift-ci bot requested review from benluddy and p0lyn0mial January 23, 2026 18:49

benluddy reviewed Jan 23, 2026

View reviewed changes

bertinatto force-pushed the kms branch 2 times, most recently from 05ca276 to 0f9be05 Compare January 26, 2026 17:28

openshift-ci bot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Jan 26, 2026

bertinatto force-pushed the kms branch from 0f9be05 to 225e64a Compare January 26, 2026 19:52

bertinatto changed the title ~~Conditionally add the KMS plugin volume mount to the kube-apiserver container~~ WIP" Conditionally add the KMS plugin volume mount to the kube-apiserver container Jan 26, 2026

bertinatto changed the title ~~WIP" Conditionally add the KMS plugin volume mount to the kube-apiserver container~~ WIP: Conditionally add the KMS plugin volume mount to the kube-apiserver container Jan 26, 2026

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 Jan 26, 2026

bertinatto mentioned this pull request Jan 26, 2026

CNTRLPLANE-2241: kms: add helper for TechPreview V1 openshift/library-go#2090

Merged

bertinatto force-pushed the kms branch from 225e64a to 0909378 Compare January 27, 2026 13:59

bertinatto force-pushed the kms branch from 0909378 to ae5a4d2 Compare January 28, 2026 17:54

bertinatto force-pushed the kms branch from ae5a4d2 to aacd995 Compare January 29, 2026 12:28

bertinatto changed the title ~~WIP: Conditionally add the KMS plugin volume mount to the kube-apiserver container~~ Conditionally add the KMS plugin volume mount to the kube-apiserver container Jan 29, 2026

ardaguclu reviewed Jan 29, 2026

View reviewed changes

bertinatto force-pushed the kms branch 2 times, most recently from a7f5eb0 to f59d118 Compare January 29, 2026 12:52

ardaguclu mentioned this pull request Jan 29, 2026

[WIP]Adding KMS TestKMSEncryptionOnOff test #2018

Closed

bertinatto force-pushed the kms branch 2 times, most recently from e640ca0 to f3e3574 Compare January 29, 2026 13:36

coderabbitai bot reviewed Jan 29, 2026

View reviewed changes

go.mod Show resolved Hide resolved

p0lyn0mial reviewed Jan 29, 2026

View reviewed changes

bertinatto added 2 commits January 29, 2026 12:46

Conditionally add the KMS plugin volume mount to the kube-apiserver c…

0e25cb4

…ontainer

Bump library-go to get KMS helper

f82dace

bertinatto force-pushed the kms branch from f3e3574 to f82dace Compare January 29, 2026 15:47

openshift-ci bot changed the title ~~Conditionally add the KMS plugin volume mount to the kube-apiserver container~~ CNTRLPLANE-2241: Conditionally add the KMS plugin volume mount to the kube-apiserver container Jan 29, 2026

openshift-ci-robot added the jira/valid-reference Indicates that this PR references a valid Jira ticket of any type. label Jan 29, 2026

openshift-ci bot assigned p0lyn0mial Jan 29, 2026

openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label Jan 29, 2026

openshift-ci bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Jan 29, 2026

openshift-ci-robot added the verified Signifies that the PR passed pre-merge verification criteria label Jan 29, 2026

openshift-merge-bot bot merged commit fecf00c into openshift:main Jan 29, 2026
22 of 24 checks passed

bertinatto mentioned this pull request Mar 5, 2026

[release-4.21] OCPBUGS-74418: Conditionally add the KMS plugin volume mount to the kube-apiserver container #2061

Merged

Conversation

bertinatto commented Jan 23, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

coderabbitai bot commented Jan 23, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Walkthrough

Changes

Estimated code review effort

Uh oh!

bertinatto commented Jan 23, 2026

Uh oh!

openshift-ci bot commented Jan 23, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ardaguclu Jan 26, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

bertinatto commented Jan 26, 2026

Uh oh!

openshift-ci bot commented Jan 26, 2026

Uh oh!

bertinatto commented Jan 26, 2026

Uh oh!

bertinatto commented Jan 26, 2026

Uh oh!

openshift-ci bot commented Jan 26, 2026

Uh oh!

bertinatto commented Jan 27, 2026

Uh oh!

openshift-ci bot commented Jan 27, 2026

Uh oh!

bertinatto commented Jan 28, 2026

Uh oh!

openshift-ci bot commented Jan 28, 2026

Uh oh!

bertinatto commented Jan 28, 2026

Uh oh!

bertinatto commented Jan 29, 2026

Uh oh!

bertinatto commented Jan 29, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

coderabbitai bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

p0lyn0mial commented Jan 29, 2026

Uh oh!

p0lyn0mial commented Jan 29, 2026 • edited by openshift-ci bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

openshift-ci-robot commented Jan 29, 2026 • edited by openshift-ci bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

openshift-ci bot commented Jan 29, 2026

Uh oh!

bertinatto commented Jan 29, 2026

Uh oh!

openshift-ci bot commented Jan 29, 2026

Uh oh!

bertinatto commented Jan 29, 2026

Uh oh!

bertinatto commented Jan 23, 2026 •

edited

Loading

coderabbitai bot commented Jan 23, 2026 •

edited

Loading

ardaguclu Jan 26, 2026 •

edited

Loading

p0lyn0mial commented Jan 29, 2026 •

edited by openshift-ci bot

Loading

openshift-ci-robot commented Jan 29, 2026 •

edited by openshift-ci bot

Loading