WIP: Fake bump to get credentials in KMS plugin#2169
Open
bertinatto wants to merge 4 commits into
Open
Conversation
openshift-ci
Bot
added
the
do-not-merge/work-in-progress
Contributor
|
Skipping CI for Draft Pull Request. |
WalkthroughAdds go.mod replace directives for ginkgo and library-go, changes managePods to use kmspluginlifecycle.AddKMSPluginSidecarToStaticPodSpec, and updates e2e KMS tests to use librarykms.DefaultVaultEncryptionProvider. ChangesKMS sidecar + deps + tests
Possibly related PRs
Suggested reviewers
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes 🚥 Pre-merge checks | ✅ 11 | ❌ 4❌ Failed checks (3 warnings, 1 inconclusive)
✅ Passed checks (11 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
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 `@go.mod`:
- Line 140: The PR introduces a direct module replacement "replace
github.com/openshift/library-go => github.com/bertinatto/library-go
v0.0.0-20260528185215-82202de1e3b1" in go.mod (also mirrored in
vendor/modules.txt) without documented provenance or supply-chain controls;
either revert that replace to restore the upstream module or, if the fork is
required, add a short documented justification and mitigation: record the exact
fork/version and why it's needed, verify license compatibility, run and record
CVE/supply-chain scans for that specific version, ensure the release pipeline
produces provenance/SBOM and artifact signing (e.g., Sigstore/cosign) for builds
using the fork, and add an explicit removal plan and timeline; update repository
docs (e.g., a SECURITY or DEPENDENCIES note) to reflect these steps before
merging.
🪄 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: 731381c7-46ff-46da-b155-61fef9fe56fa
⛔ Files ignored due to path filters (6)
go.sumis excluded by!**/*.sumvendor/github.com/openshift/library-go/pkg/operator/certrotation/target.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/library-go/pkg/operator/encryption/encryptiondata/secret.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/library-go/pkg/operator/encryption/kms/pluginlifecycle/sidecar.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/library-go/pkg/operator/encryption/kms/pluginlifecycle/vault.gois excluded by!**/vendor/**,!vendor/**vendor/modules.txtis excluded by!**/vendor/**,!vendor/**
📒 Files selected for processing (2)
go.modpkg/operator/targetconfigcontroller/targetconfigcontroller.go
|
|
||
| replace github.com/onsi/ginkgo/v2 => github.com/openshift/onsi-ginkgo/v2 v2.6.1-0.20251001123353-fd5b1fb35db1 | ||
|
|
||
| replace github.com/openshift/library-go => github.com/bertinatto/library-go v0.0.0-20260528185215-82202de1e3b1 |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "[1] Confirm replace directive in go.mod"
rg -n '^replace github.com/openshift/library-go => ' go.mod
echo "[2] Extract replacement target/version and flag non-openshift namespace"
python - <<'PY'
import re, pathlib
text = pathlib.Path("go.mod").read_text()
m = re.search(r'^replace github\.com/openshift/library-go => (\S+) (\S+)$', text, re.M)
if not m:
raise SystemExit("replace directive not found")
target, version = m.groups()
print(f"target={target}")
print(f"version={version}")
print(f"non_official_namespace={not target.startswith('github.com/openshift/')}")
PY
echo "[3] Confirm vendored module metadata carries the same replacement"
rg -n 'github.com/openshift/library-go .*=> github.com/bertinatto/library-go' vendor/modules.txtRepository: openshift/cluster-kube-apiserver-operator
Length of output: 743
Block merging personal-fork replace for github.com/openshift/library-go without provenance controls
go.mod(line 140) replacesgithub.com/openshift/library-gowithgithub.com/bertinatto/library-goatv0.0.0-20260528185215-82202de1e3b1.vendor/modules.txtincludes the same replacement mapping, so vendored builds will use the fork.- Require a documented justification + license compatibility and CVE/supply-chain scanning for that exact version, and ensure the release pipeline meets provenance/SBOM and artifact-signing requirements (e.g., Sigstore/cosign) with an explicit removal plan; otherwise revert the
replace.
replace github.com/openshift/library-go => github.com/bertinatto/library-go v0.0.0-20260528185215-82202de1e3b1🤖 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 `@go.mod` at line 140, The PR introduces a direct module replacement "replace
github.com/openshift/library-go => github.com/bertinatto/library-go
v0.0.0-20260528185215-82202de1e3b1" in go.mod (also mirrored in
vendor/modules.txt) without documented provenance or supply-chain controls;
either revert that replace to restore the upstream module or, if the fork is
required, add a short documented justification and mitigation: record the exact
fork/version and why it's needed, verify license compatibility, run and record
CVE/supply-chain scans for that specific version, ensure the release pipeline
produces provenance/SBOM and artifact signing (e.g., Sigstore/cosign) for builds
using the fork, and add an explicit removal plan and timeline; update repository
docs (e.g., a SECURITY or DEPENDENCIES note) to reflect these steps before
merging.
Member
Author
|
/test all |
Contributor
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
bertinatto
force-pushed
the
kms-plugins-lifecycle-vault-credentials
branch
from
07a5fec to
d985e36
Compare
bertinatto
force-pushed
the
kms-plugins-lifecycle-vault-credentials
branch
from
d985e36 to
b0b6df4
Compare
openshift-ci
Bot
removed
the
do-not-merge/work-in-progress
There was a problem hiding this comment.
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 `@test/e2e-encryption-kms/encryption_kms.go`:
- Line 55: The scenario comments are outdated: after setting EncryptionProvider
to librarykms.DefaultVaultEncryptionProvider the comments that read "Deploys the
mock KMS plugin" are misleading. Update the scenario step comments near the
EncryptionProvider assignment and the other mentions to describe the real Vault
provider flow (e.g., "Deploys the Vault KMS provider / configures Vault KMS"),
ensuring references to librarykms.DefaultVaultEncryptionProvider and any related
setup steps reflect the real Vault deployment instead of "mock".
🪄 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: f812f600-5966-4222-80f1-6ae2230df190
⛔ Files ignored due to path filters (7)
go.sumis excluded by!**/*.sumvendor/github.com/openshift/library-go/pkg/operator/certrotation/target.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/library-go/pkg/operator/encryption/encryptiondata/secret.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/library-go/pkg/operator/encryption/kms/pluginlifecycle/sidecar.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/library-go/pkg/operator/encryption/kms/pluginlifecycle/vault.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/library-go/test/library/encryption/kms/vault.gois excluded by!**/vendor/**,!vendor/**vendor/modules.txtis excluded by!**/vendor/**,!vendor/**
📒 Files selected for processing (3)
go.modpkg/operator/targetconfigcontroller/targetconfigcontroller.gotest/e2e-encryption-kms/encryption_kms.go
🚧 Files skipped from review as they are similar to previous changes (1)
- pkg/operator/targetconfigcontroller/targetconfigcontroller.go
| ResourceFunc: operatorencryption.SecretOfLife, | ||
| ResourceName: "SecretOfLife", | ||
| EncryptionProvider: librarykms.DefaultFakeVaultEncryptionProvider, | ||
| EncryptionProvider: librarykms.DefaultVaultEncryptionProvider, |
There was a problem hiding this comment.
Update scenario comments to match the real Vault provider flow.
After switching to librarykms.DefaultVaultEncryptionProvider, the step lists still say “Deploys the mock KMS plugin” (Line 29, Line 61), which is now misleading.
Suggested comment update
// testKMSEncryptionOnOff tests KMS encryption on/off cycle.
// This test:
-// 1. Deploys the mock KMS plugin
+// 1. Configures the Vault-backed KMS provider
// 2. Creates a test secret (SecretOfLife)
// 3. Enables KMS encryption
// 4. Verifies secret is encrypted
// 5. Disables encryption (Identity)
// 6. Verifies secret is NOT encrypted
// 7. Re-enables KMS encryption
// 8. Verifies secret is encrypted again
// 9. Disables encryption (Identity) again
// 10. Verifies secret is NOT encrypted again
// testKMSEncryptionProvidersMigration tests migration between KMS and AES encryption providers.
// This test:
-// 1. Deploys the mock KMS plugin
+// 1. Configures the Vault-backed KMS provider
// 2. Creates a test secret (SecretOfLife)
// 3. Randomly picks one AES encryption provider (AESGCM or AESCBC)
// 4. Shuffles the selected AES provider with KMS to create a randomized migration order
// 5. Migrates between the providers in the shuffled order
// 6. Verifies secret is correctly encrypted after each migrationAlso applies to: 84-84
🤖 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 `@test/e2e-encryption-kms/encryption_kms.go` at line 55, The scenario comments
are outdated: after setting EncryptionProvider to
librarykms.DefaultVaultEncryptionProvider the comments that read "Deploys the
mock KMS plugin" are misleading. Update the scenario step comments near the
EncryptionProvider assignment and the other mentions to describe the real Vault
provider flow (e.g., "Deploys the Vault KMS provider / configures Vault KMS"),
ensuring references to librarykms.DefaultVaultEncryptionProvider and any related
setup steps reflect the real Vault deployment instead of "mock".
Contributor
|
@bertinatto: The following test failed, say
Full PR test history. Your PR dashboard. DetailsInstructions 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. |
Member
Author
|
/testwith openshift/cluster-kube-apiserver-operator/main/e2e-gcp-operator-encryption-kms openshift/cluster-openshift-apiserver-operator#699 openshift/cluster-authentication-operator#903 |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary by CodeRabbit
Chores
Bug Fixes
Tests