fake bump test#917
Conversation
|
Skipping CI for Draft Pull Request. |
openshift-ci
Bot
added
the
do-not-merge/work-in-progress
|
/test e2e-aws-operator-encryption-kms |
|
[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 |
|
/test e2e-aws-operator-encryption-kms |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository: openshift/coderabbit/.coderabbit.yaml Review profile: CHILL Plan: Enterprise Run ID: ⛔ Files ignored due to path filters (6)
📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (2)
WalkthroughThe PR disables the existing ChangesKMS-to-KMS E2E Tests and Dependency Wiring
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 13 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (13 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: 2
🧹 Nitpick comments (1)
test/e2e-encryption-kms/encryption_kms.go (1)
17-23: ⚡ Quick winPrefer explicit pending specs over commented-out
g.Itblocks.Lines [17]-[23] hide skipped coverage in comments. Using explicit pending/disabled Ginkgo nodes keeps intent visible in test reports and avoids stale commented code paths.
🤖 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` around lines 17 - 23, The commented-out g.It blocks for TestKMSEncryptionOnOff and TestKMSEncryptionProvidersMigration should be replaced with explicit Ginkgo pending specs rather than remaining as commented code. Remove the comment syntax and use Ginkgo's pending mechanism (such as g.PIt or adding a Skip() call) to explicitly mark these test specifications as disabled, which will make the intent visible in test reports and prevent stale commented code paths.
🤖 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 137: Remove the replace directive for github.com/openshift/library-go
that points to the personal fork github.com/gangwgr/library-go from go.mod. This
replace directive widened the supply-chain trust boundary by redirecting all
imports (including production code) to a personal fork instead of the official
OpenShift-owned repository. If the changes from this fork are still necessary
for functionality, work to upstream them to the official
github.com/openshift/library-go repository, or if they are only needed for local
development or testing, configure them through local-only mechanisms (such as
local go.mod overrides or test-specific environment setup) that do not affect
production builds.
In `@test/e2e-encryption-kms/encryption_kms.go`:
- Around line 112-113: Replace all instances of context.TODO() with the
available ctx parameter from the enclosing function scope to enable proper
cancellation and timeout propagation in the encryption test closures. In file
test/e2e-encryption-kms/encryption_kms.go, make the following changes: at lines
112-113 in the CreateResourceFunc closure within testKMSToKMSMigration, replace
context.TODO() with ctx; at lines 86-88 in the closure within
testKMSEncryptionProvidersMigration, replace context.TODO() with ctx; and at
lines 136-137 in the closure within testKMSToKMSOnOff, replace context.TODO()
with ctx. Each enclosing function has a ctx parameter available in its scope
that should be used instead of creating a new TODO context.
---
Nitpick comments:
In `@test/e2e-encryption-kms/encryption_kms.go`:
- Around line 17-23: The commented-out g.It blocks for TestKMSEncryptionOnOff
and TestKMSEncryptionProvidersMigration should be replaced with explicit Ginkgo
pending specs rather than remaining as commented code. Remove the comment syntax
and use Ginkgo's pending mechanism (such as g.PIt or adding a Skip() call) to
explicitly mark these test specifications as disabled, which will make the
intent visible in test reports and prevent stale commented code paths.
🪄 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: 381667e5-fdca-472c-bca8-228571841975
⛔ Files ignored due to path filters (5)
go.sumis excluded by!**/*.sumvendor/github.com/openshift/library-go/pkg/operator/encryption/controllers/key_controller.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/library-go/test/library/encryption/helpers.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/library-go/test/library/encryption/scenarios.gois excluded by!**/vendor/**,!vendor/**vendor/modules.txtis excluded by!**/vendor/**,!vendor/**
📒 Files selected for processing (2)
go.modtest/e2e-encryption-kms/encryption_kms.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/gangwgr/library-go v0.0.0-20260616121325-7a109a33686c |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "replace directives:"
rg -n '^replace ' go.mod
echo
echo "Non-test imports affected by the library-go replace:"
rg -n --type=go '"github.com/openshift/library-go' -g '!test/**' -g '!**/*_test.go'
echo
echo "Test imports (expected target scope for this PR):"
rg -n --type=go '"github.com/openshift/library-go' testRepository: openshift/cluster-authentication-operator
Length of output: 50397
Remove the personal fork replace directive before merging.
Line 137 redirects all github.com/openshift/library-go imports (including production code in pkg/) to the personal fork github.com/gangwgr/library-go. This widens the supply-chain trust boundary and conflicts with dependency security requirements.
Before merge, either:
- Upstream the required changes to the official
github.com/openshift/library-gorepository, or - Isolate the override to a local/test-only configuration so production builds use the official OpenShift-owned library
🤖 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 137, Remove the replace directive for
github.com/openshift/library-go that points to the personal fork
github.com/gangwgr/library-go from go.mod. This replace directive widened the
supply-chain trust boundary by redirecting all imports (including production
code) to a personal fork instead of the official OpenShift-owned repository. If
the changes from this fork are still necessary for functionality, work to
upstream them to the official github.com/openshift/library-go repository, or if
they are only needed for local development or testing, configure them through
local-only mechanisms (such as local go.mod overrides or test-specific
environment setup) that do not affect production builds.
Source: Coding guidelines
| CreateResourceFunc: func(t testing.TB, _ library.ClientSet, namespace string) runtime.Object { | ||
| return operatorencryption.CreateAndStoreTokenOfLife(context.TODO(), t, operatorencryption.GetClients(t)) |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# Check if the file exists and examine the relevant lines
head -n 150 test/e2e-encryption-kms/encryption_kms.go | tail -n 50Repository: openshift/cluster-authentication-operator
Length of output: 3184
🏁 Script executed:
# Get the full context around lines 112-113 and 136-137
cat -n test/e2e-encryption-kms/encryption_kms.go | sed -n '100,150p'Repository: openshift/cluster-authentication-operator
Length of output: 3506
🏁 Script executed:
# Check the function signature of CreateAndStoreTokenOfLife
rg -A 5 "func CreateAndStoreTokenOfLife" test/Repository: openshift/cluster-authentication-operator
Length of output: 668
🏁 Script executed:
# Look at the enclosing function to understand the scope and whether ctx is available
cat -n test/e2e-encryption-kms/encryption_kms.go | sed -n '1,120p' | tail -n 50Repository: openshift/cluster-authentication-operator
Length of output: 3842
Use ctx instead of context.TODO() in resource creation closures.
At lines 113, 137, and 87, the closures use context.TODO() instead of the available ctx parameter from their enclosing functions (testKMSToKMSMigration, testKMSToKMSOnOff, and testKMSEncryptionProvidersMigration). For long-running serial encryption tests, this bypasses proper cancellation and timeout propagation.
Suggested fix
- CreateResourceFunc: func(t testing.TB, _ library.ClientSet, namespace string) runtime.Object {
- return operatorencryption.CreateAndStoreTokenOfLife(context.TODO(), t, operatorencryption.GetClients(t))
- },
+ CreateResourceFunc: func(t testing.TB, _ library.ClientSet, namespace string) runtime.Object {
+ return operatorencryption.CreateAndStoreTokenOfLife(ctx, t, operatorencryption.GetClients(t))
+ },Also applies to: 86-88, 136-137
🤖 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` around lines 112 - 113, Replace
all instances of context.TODO() with the available ctx parameter from the
enclosing function scope to enable proper cancellation and timeout propagation
in the encryption test closures. In file
test/e2e-encryption-kms/encryption_kms.go, make the following changes: at lines
112-113 in the CreateResourceFunc closure within testKMSToKMSMigration, replace
context.TODO() with ctx; at lines 86-88 in the closure within
testKMSEncryptionProvidersMigration, replace context.TODO() with ctx; and at
lines 136-137 in the closure within testKMSToKMSOnOff, replace context.TODO()
with ctx. Each enclosing function has a ctx parameter available in its scope
that should be used instead of creating a new TODO context.
Source: Coding guidelines
|
/test e2e-aws-operator-encryption-kms |
openshift-ci
Bot
added
the
needs-rebase
|
@gangwgr: 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. |
openshift-ci
Bot
removed
the
needs-rebase
|
/testwith openshift/cluster-authenication-operator/master/e2e-aws-operator-encryption-kms openshift/cluster-kube-apiserver-operator#2203 openshift/cluster-openshift-apiserver-operator#712 |
|
@gangwgr, |
|
/testwith openshift/cluster-authentication-operator/master/e2e-aws-operator-encryption-kms openshift/cluster-kube-apiserver-operator#2203 openshift/cluster-openshift-apiserver-operator#712 |
|
@gangwgr, |
|
/testwith openshift/cluster-authentication-operator/master/e2e-aws-operator-encryption-kms openshift/cluster-kube-apiserver-operator#2203 openshift/cluster-openshift-apiserver-operator#712 |
|
/testwith openshift/cluster-authentication-operator/master/e2e-aws-operator-encryption-kms openshift/cluster-kube-apiserver-operator#2203 openshift/cluster-openshift-apiserver-operator#712 |
1 participant
Summary by CodeRabbit
Tests
Chores
replacedirective to align with the referenced library version.