Skip to content

NO-JIRA: add more rotation tests scenarios #2149

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
tjungblu wants to merge 1 commit into from
Closed

NO-JIRA: add more rotation tests scenarios #2149

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions go.mod
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -136,3 +136,5 @@ require (
)

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/tjungblu/library-go v0.0.0-20260520063220-fea2dbe4012f
4 changes: 2 additions & 2 deletions go.sum
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -165,8 +165,6 @@ github.com/openshift/build-machinery-go v0.0.0-20250530140348-dc5b2804eeee h1:+S
github.com/openshift/build-machinery-go v0.0.0-20250530140348-dc5b2804eeee/go.mod h1:8jcm8UPtg2mCAsxfqKil1xrmRMI3a+XU2TZ9fF8A7TE=
github.com/openshift/client-go v0.0.0-20260512113608-deb4dc54551a h1:EKx2XhOKehd1C5ptY7IrLl4WV35E8kP0pRPnG5BUZXk=
github.com/openshift/client-go v0.0.0-20260512113608-deb4dc54551a/go.mod h1:V933kvY/cb/Un7UCEOhXHUySNX327u7Epe8g9KNqg2Q=
github.com/openshift/library-go v0.0.0-20260513130945-942eeae4479f h1:fQ22ssXdHHmSj2nxz9ccJr7zx80oZa3HiL+Or7zS8VI=
github.com/openshift/library-go v0.0.0-20260513130945-942eeae4479f/go.mod h1:gKG9lctU0yEftSoT3DUyeIWz1oAgF0EHUpwI4pnCo4o=
github.com/openshift/onsi-ginkgo/v2 v2.6.1-0.20251001123353-fd5b1fb35db1 h1:PMTgifBcBRLJJiM+LgSzPDTk9/Rx4qS09OUrfpY6GBQ=
github.com/openshift/onsi-ginkgo/v2 v2.6.1-0.20251001123353-fd5b1fb35db1/go.mod h1:7Du3c42kxCUegi0IImZ1wUQzMBVecgIHjR1C+NkhLQo=
github.com/orisano/pixelmatch v0.0.0-20220722002657-fb0b55479cde/go.mod h1:nZgzbfBr3hhjoZnS66nKrHmduYNpc34ny7RK4z5/HM0=
Expand Down Expand Up @@ -215,6 +213,8 @@ github.com/stretchr/testify v1.8.0/go.mod h1:yNjHg4UonilssWZ8iaSj1OCr/vHnekPRkoO
github.com/stretchr/testify v1.8.1/go.mod h1:w2LPCIKwWwSfY2zedu0+kehJoqGctiVI29o6fzry7u4=
github.com/stretchr/testify v1.11.1 h1:7s2iGBzp5EwR7/aIZr8ao5+dra3wiQyKjjFuvgVKu7U=
github.com/stretchr/testify v1.11.1/go.mod h1:wZwfW3scLgRK+23gO65QZefKpKQRnfz6sD981Nm4B6U=
github.com/tjungblu/library-go v0.0.0-20260520063220-fea2dbe4012f h1:Uj59W+q2NAKcjP3iv9K3jsHc9zMF5fMb3Pg0TRFte2c=
github.com/tjungblu/library-go v0.0.0-20260520063220-fea2dbe4012f/go.mod h1:gKG9lctU0yEftSoT3DUyeIWz1oAgF0EHUpwI4pnCo4o=
github.com/tmc/grpc-websocket-proxy v0.0.0-20220101234140-673ab2c3ae75 h1:6fotK7otjonDflCTK0BCfls4SPy3NcCVb5dqqmbRknE=
github.com/tmc/grpc-websocket-proxy v0.0.0-20220101234140-673ab2c3ae75/go.mod h1:KO6IkyS8Y3j8OdNO85qEYBsRPuteD+YciPomcXdrMnk=
github.com/x448/float16 v0.8.4 h1:qLwI1I70+NjRFUR3zs1JPUCgaCXSh3SW62uAKT1mSBM=
Expand Down
59 changes: 48 additions & 11 deletions test/e2e-encryption-rotation/encryption_rotation_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
package e2e_encryption_rotation

import (
"context"
"flag"
"fmt"
"testing"
Expand All @@ -10,7 +9,6 @@ import (
"github.com/openshift/cluster-kube-apiserver-operator/pkg/operator/operatorclient"
operatorencryption "github.com/openshift/cluster-kube-apiserver-operator/test/library/encryption"
library "github.com/openshift/library-go/test/library/encryption"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
)

var provider = flag.String("provider", "aescbc", "encryption provider used by the tests")
Expand All @@ -32,15 +30,54 @@ func TestEncryptionRotation(t *testing.T) {
CreateResourceFunc: operatorencryption.CreateAndStoreSecretOfLife,
GetRawResourceFunc: operatorencryption.GetRawSecretOfLife,
UnsupportedConfigFunc: func(raw []byte) error {
operatorClient := operatorencryption.GetOperator(t)
apiServerOperator, err := operatorClient.Get(context.TODO(), "cluster", metav1.GetOptions{})
if err != nil {
return err
}
apiServerOperator.Spec.UnsupportedConfigOverrides.Raw = raw
_, err = operatorClient.Update(context.TODO(), apiServerOperator, metav1.UpdateOptions{})
return err
return operatorencryption.UpdateUnsupportedConfig(t, raw)
},
EncryptionProvider: configv1.APIServerEncryption{Type: configv1.EncryptionType(*provider)},
EncryptionProvider: library.EncryptionProvider{APIServerEncryption: configv1.APIServerEncryption{Type: configv1.EncryptionType(*provider)}},
})
}

// TestEncryptionRotationDuringFirstMigration applies encryption (initial storage migration) and forces a key
// rotation while that first migration is still running. The cluster must converge to the last requested write key.
func TestEncryptionRotationDuringFirstMigration(t *testing.T) {
library.TestEncryptionRotationDuringFirstMigration(t, library.RotationScenario{
BasicScenario: library.BasicScenario{
Namespace: operatorclient.GlobalMachineSpecifiedConfigNamespace,
LabelSelector: "encryption.apiserver.operator.openshift.io/component" + "=" + operatorclient.TargetNamespace,
EncryptionConfigSecretName: fmt.Sprintf("encryption-config-%s", operatorclient.TargetNamespace),
EncryptionConfigSecretNamespace: operatorclient.GlobalMachineSpecifiedConfigNamespace,
OperatorNamespace: operatorclient.OperatorNamespace,
TargetGRs: operatorencryption.DefaultTargetGRs,
AssertFunc: operatorencryption.AssertSecretsAndConfigMaps,
},
CreateResourceFunc: operatorencryption.CreateAndStoreSecretOfLife,
GetRawResourceFunc: operatorencryption.GetRawSecretOfLife,
GetOperatorConditionsFunc: operatorencryption.GetClusterOperatorConditions,
UnsupportedConfigFunc: func(raw []byte) error {
return operatorencryption.UpdateUnsupportedConfig(t, raw)
},
EncryptionProvider: library.EncryptionProvider{APIServerEncryption: configv1.APIServerEncryption{Type: configv1.EncryptionType(*provider)}},
})
}
Comment on lines +41 to +60

@coderabbitai coderabbitai Bot May 20, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Guard new rotation tests for MicroShift incompatibility.

These new tests depend on ClusterOperator conditions (GetClusterOperatorConditions), which are not available on MicroShift. Add the usual MicroShift protection (label/tag or runtime skip) so they don’t run in unsupported environments.

As per coding guidelines, “Flag tests referencing unavailable APIs: … ClusterOperator … Use [apigroup:...] tags for API-specific tests, add [Skipped:MicroShift] label, or use exutil.IsMicroShiftCluster() runtime checks.”

Also applies to: 64-83

🧰 Tools
🪛 golangci-lint (2.12.2)

[error] 56-56: : # github.com/openshift/cluster-kube-apiserver-operator/test/e2e-encryption [github.com/openshift/cluster-kube-apiserver-operator/test/e2e-encryption.test]
test/e2e-encryption/encryption_test.go:56:35: cannot use configv1.APIServerEncryption{…} (value of struct type "github.com/openshift/api/config/v1".APIServerEncryption) as "github.com/openshift/library-go/test/library/encryption".EncryptionProvider value in struct literal

(typecheck)

🤖 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-rotation/encryption_rotation_test.go` around lines 41 -
60, This test (TestEncryptionRotationDuringFirstMigration) calls
GetClusterOperatorConditions via
library.TestEncryptionRotationDuringFirstMigration and must be guarded from
running on MicroShift; add the standard MicroShift protection by either tagging
the test with the API-specific tag/[Skipped:MicroShift] or performing a runtime
skip at the top of TestEncryptionRotationDuringFirstMigration (e.g., call
exutil.IsMicroShiftCluster() and t.Skipf when true) so ClusterOperator-dependent
logic is not executed on MicroShift, and apply the same change to the other
related test block (the similar test around lines 64-83) that also uses
GetClusterOperatorConditions.


// TestEncryptionRotationDuringOngoingRotation forces a second key rotation while the migration triggered by the
// first forced rotation is still running. The cluster must converge to the last requested write key.
func TestEncryptionRotationDuringOngoingRotation(t *testing.T) {
library.TestEncryptionRotationDuringOngoingRotation(t, library.RotationScenario{
BasicScenario: library.BasicScenario{
Namespace: operatorclient.GlobalMachineSpecifiedConfigNamespace,
LabelSelector: "encryption.apiserver.operator.openshift.io/component" + "=" + operatorclient.TargetNamespace,
EncryptionConfigSecretName: fmt.Sprintf("encryption-config-%s", operatorclient.TargetNamespace),
EncryptionConfigSecretNamespace: operatorclient.GlobalMachineSpecifiedConfigNamespace,
OperatorNamespace: operatorclient.OperatorNamespace,
TargetGRs: operatorencryption.DefaultTargetGRs,
AssertFunc: operatorencryption.AssertSecretsAndConfigMaps,
},
CreateResourceFunc: operatorencryption.CreateAndStoreSecretOfLife,
GetRawResourceFunc: operatorencryption.GetRawSecretOfLife,
GetOperatorConditionsFunc: operatorencryption.GetClusterOperatorConditions,
UnsupportedConfigFunc: func(raw []byte) error {
return operatorencryption.UpdateUnsupportedConfig(t, raw)
},
EncryptionProvider: library.EncryptionProvider{APIServerEncryption: configv1.APIServerEncryption{Type: configv1.EncryptionType(*provider)}},
})
}
21 changes: 21 additions & 0 deletions test/library/encryption/helpers.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import (
"testing"
"time"

operatorv1 "github.com/openshift/api/operator/v1"
corev1 "k8s.io/api/core/v1"
"k8s.io/apimachinery/pkg/api/errors"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
Expand Down Expand Up @@ -81,3 +82,23 @@ func SecretOfLife(t testing.TB, namespace string) runtime.Object {
},
}
}

func UpdateUnsupportedConfig(t *testing.T, raw []byte) error {
operatorClient := GetOperator(t)
apiServerOperator, err := operatorClient.Get(context.TODO(), "cluster", metav1.GetOptions{})
if err != nil {
return err
}
apiServerOperator.Spec.UnsupportedConfigOverrides.Raw = raw
_, err = operatorClient.Update(context.TODO(), apiServerOperator, metav1.UpdateOptions{})
return err
Comment on lines +93 to +94

@coderabbitai coderabbitai Bot May 20, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail
# Verify whether update helpers in this file already use retry-on-conflict.
rg -n -C2 'func UpdateUnsupportedConfig|RetryOnConflict|operatorClient\.Update\(' test/library/encryption/helpers.go

Repository: openshift/cluster-kube-apiserver-operator

Length of output: 481

🏁 Script executed:

# Check the full file content to understand context and imports
head -30 test/library/encryption/helpers.go

Repository: openshift/cluster-kube-apiserver-operator

Length of output: 939

🏁 Script executed:

# Look for all Update calls in the file
rg -n '\.Update\(' test/library/encryption/helpers.go

Repository: openshift/cluster-kube-apiserver-operator

Length of output: 179

🏁 Script executed:

# Check if retry pattern is used elsewhere in the file or related test utilities
rg -n 'RetryOnConflict|retry\.Retry' test/library/encryption/helpers.go

Repository: openshift/cluster-kube-apiserver-operator

Length of output: 67

🏁 Script executed:

# Search for similar update patterns in other test helper files
fd -e go -path "*/test/library/*" | head -10

Repository: openshift/cluster-kube-apiserver-operator

Length of output: 254

🏁 Script executed:

# Check if k8s.io/client-go/util/retry is used anywhere in the codebase
rg -n 'client-go/util/retry|RetryOnConflict' --type go

Repository: openshift/cluster-kube-apiserver-operator

Length of output: 3188

🏁 Script executed:

# Look for similar Update patterns in other test files
rg -n -B2 'operatorClient.*Update' --type go test/

Repository: openshift/cluster-kube-apiserver-operator

Length of output: 813

🏁 Script executed:

# Check if there are any retry patterns in the broader codebase
rg -n 'retry\.Retry|retry\.DefaultRetry' --type go | head -20

Repository: openshift/cluster-kube-apiserver-operator

Length of output: 2696

🏁 Script executed:

# Check what exists in the vendor code - DefaultRetry vs DefaultBackoff
rg -n 'var DefaultRetry|var DefaultBackoff|DefaultRetry =|DefaultBackoff =' vendor/k8s.io/client-go/util/retry/ --type go

Repository: openshift/cluster-kube-apiserver-operator

Length of output: 245

🏁 Script executed:

# Check the library-go encryption helpers that are imported
rg -n -A10 'func UpdateUnsupportedConfig' vendor/github.com/openshift/library-go/test/library/encryption/helpers.go

Repository: openshift/cluster-kube-apiserver-operator

Length of output: 67

🏁 Script executed:

# Check the exact signature of RetryOnConflict in vendor
rg -n -A5 'func RetryOnConflict' vendor/k8s.io/client-go/util/retry/util.go

Repository: openshift/cluster-kube-apiserver-operator

Length of output: 215

🏁 Script executed:

# Check the difference between DefaultRetry and DefaultBackoff
rg -n -A10 'var DefaultRetry = wait.Backoff|var DefaultBackoff = wait.Backoff' vendor/k8s.io/client-go/util/retry/util.go

Repository: openshift/cluster-kube-apiserver-operator

Length of output: 877

🏁 Script executed:

# Check if the library-go helpers are actually being used by this file
rg -n 'library\.|library-go' test/library/encryption/helpers.go

Repository: openshift/cluster-kube-apiserver-operator

Length of output: 429

🏁 Script executed:

# Look more broadly for any UpdateUnsupportedConfig patterns in library-go test helpers
rg -n 'UpdateUnsupportedConfig\|UnsupportedConfigOverrides' vendor/github.com/openshift/library-go/test/library/encryption/

Repository: openshift/cluster-kube-apiserver-operator

Length of output: 67

🏁 Script executed:

# Check what the recommended pattern is in similar test helpers
rg -n 'RetryOnConflict.*DefaultRetry|RetryOnConflict.*DefaultBackoff' --type go | head -15

Repository: openshift/cluster-kube-apiserver-operator

Length of output: 1410

Use retry-on-conflict when updating operator resources.

Line 93 performs a single Update on the operator resource without handling conflicts. Wrap this in a retry loop using the standard k8s.io/client-go/util/retry pattern to handle concurrent modifications.

Proposed fix 
 import (
 	"context"
 	"fmt"
 	"testing"
 	"time"
@@
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/runtime"
+	"k8s.io/client-go/util/retry"
@@
 func UpdateUnsupportedConfig(t *testing.T, raw []byte) error {
 	operatorClient := GetOperator(t)
-	apiServerOperator, err := operatorClient.Get(context.TODO(), "cluster", metav1.GetOptions{})
-	if err != nil {
-		return err
-	}
-	apiServerOperator.Spec.UnsupportedConfigOverrides.Raw = raw
-	_, err = operatorClient.Update(context.TODO(), apiServerOperator, metav1.UpdateOptions{})
-	return err
+	return retry.RetryOnConflict(retry.DefaultBackoff, func() error {
+		apiServerOperator, err := operatorClient.Get(context.TODO(), "cluster", metav1.GetOptions{})
+		if err != nil {
+			return err
+		}
+		apiServerOperator.Spec.UnsupportedConfigOverrides.Raw = raw
+		_, err = operatorClient.Update(context.TODO(), apiServerOperator, metav1.UpdateOptions{})
+		return err
+	})
 }

Note: Use retry.DefaultBackoff (not DefaultRetry) to align with project convention and the documented recommendation for conflicts on actively-managed resources.

🤖 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/library/encryption/helpers.go` around lines 93 - 94, Wrap the single
operatorClient.Update call in a retry.RetryOnConflict loop using
retry.DefaultBackoff from k8s.io/client-go/util/retry to handle update
conflicts; inside the closure call operatorClient.Update(ctx, apiServerOperator,
metav1.UpdateOptions{}), and on conflict fetch the latest apiServerOperator (via
operatorClient.Get) and reapply your changes before retrying so the update
eventually succeeds. Ensure you import k8s.io/client-go/util/retry and keep
using metav1.UpdateOptions{} and the same context (context.TODO or passed ctx).

}

func GetClusterOperatorConditions(t testing.TB) ([]operatorv1.OperatorCondition, error) {
operatorClient := GetOperator(t)
apiServerOperator, err := operatorClient.Get(context.TODO(), "cluster", metav1.GetOptions{})
if err != nil {
return nil, err
}
return apiServerOperator.Status.Conditions, nil
}
166 changes: 154 additions & 12 deletions vendor/github.com/openshift/library-go/test/library/encryption/helpers.go
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion ...thub.com/openshift/library-go/test/library/encryption/kms/k8s_mock_kms_plugin_deployer.go
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Loading