Skip to content

🌱 Rename ClusterObjectSet secret cucumber steps for clarity #2622

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

Merged
openshift-merge-bot merged 1 commit into from
Apr 2, 2026
+69 −63
Merged

🌱 Rename ClusterObjectSet secret cucumber steps for clarity #2622

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
15 changes: 9 additions & 6 deletions test/e2e/features/install.feature
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -524,12 +524,15 @@ Feature: Install ClusterExtension
"""
Then ClusterExtension is rolled out
And ClusterExtension is available
And ClusterObjectSet "${NAME}-1" phase objects use refs
And ClusterObjectSet "${NAME}-1" ref Secrets exist in "olmv1-system" namespace
And ClusterObjectSet "${NAME}-1" ref Secrets are immutable
And ClusterObjectSet "${NAME}-1" ref Secrets are labeled with revision and owner
And ClusterObjectSet "${NAME}-1" ref Secrets have ownerReference to the revision
And ClusterObjectSet "${NAME}-1" ref Secrets have type "olm.operatorframework.io/object-data"
And ClusterObjectSet "${NAME}-1" phase objects are managed in Kubernetes secrets
And ClusterObjectSet "${NAME}-1" referred secrets exist in "olmv1-system" namespace
And ClusterObjectSet "${NAME}-1" referred secrets are immutable
And ClusterObjectSet "${NAME}-1" referred secrets contain labels
| key | value |
| olm.operatorframework.io/revision-name | ${NAME}-1 |
| olm.operatorframework.io/owner-name | ${NAME} |
And ClusterObjectSet "${NAME}-1" referred secrets are owned by the object set
And ClusterObjectSet "${NAME}-1" referred secrets have type "olm.operatorframework.io/object-data"

@DeploymentConfig
Scenario: deploymentConfig nodeSelector is applied to the operator deployment
Expand Down
117 changes: 60 additions & 57 deletions test/e2e/steps/steps.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -96,12 +96,12 @@ func RegisterSteps(sc *godog.ScenarioContext) {
sc.Step(`^(?i)ClusterObjectSet "([^"]+)" contains annotation "([^"]+)" with value$`, ClusterObjectSetHasAnnotationWithValue)
sc.Step(`^(?i)ClusterObjectSet "([^"]+)" has label "([^"]+)" with value "([^"]+)"$`, ClusterObjectSetHasLabelWithValue)
sc.Step(`^(?i)ClusterObjectSet "([^"]+)" phase objects are not found or not owned by the revision$`, ClusterObjectSetObjectsNotFoundOrNotOwned)
sc.Step(`^(?i)ClusterObjectSet "([^"]+)" phase objects use refs$`, ClusterObjectSetPhaseObjectsUseRefs)
sc.Step(`^(?i)ClusterObjectSet "([^"]+)" ref Secrets exist in "([^"]+)" namespace$`, ClusterObjectSetRefSecretsExist)
sc.Step(`^(?i)ClusterObjectSet "([^"]+)" ref Secrets are immutable$`, ClusterObjectSetRefSecretsAreImmutable)
sc.Step(`^(?i)ClusterObjectSet "([^"]+)" ref Secrets are labeled with revision and owner$`, ClusterObjectSetRefSecretsLabeled)
sc.Step(`^(?i)ClusterObjectSet "([^"]+)" ref Secrets have ownerReference to the revision$`, ClusterObjectSetRefSecretsHaveOwnerRef)
sc.Step(`^(?i)ClusterObjectSet "([^"]+)" ref Secrets have type "([^"]+)"$`, ClusterObjectSetReferredSecretsHaveType)
sc.Step(`^(?i)ClusterObjectSet "([^"]+)" phase objects are managed in Kubernetes secrets$`, ClusterObjectSetPhaseObjectsManagedInSecrets)
sc.Step(`^(?i)ClusterObjectSet "([^"]+)" referred secrets exist in "([^"]+)" namespace$`, ClusterObjectSetReferredSecretsExist)
sc.Step(`^(?i)ClusterObjectSet "([^"]+)" referred secrets are immutable$`, ClusterObjectSetReferredSecretsAreImmutable)
sc.Step(`^(?i)ClusterObjectSet "([^"]+)" referred secrets contain labels$`, ClusterObjectSetReferredSecretsContainLabels)
sc.Step(`^(?i)ClusterObjectSet "([^"]+)" referred secrets are owned by the object set$`, ClusterObjectSetReferredSecretsOwnedByObjectSet)
sc.Step(`^(?i)ClusterObjectSet "([^"]+)" referred secrets have type "([^"]+)"$`, ClusterObjectSetReferredSecretsHaveType)

sc.Step(`^(?i)resource "([^"]+)" is installed$`, ResourceAvailable)
sc.Step(`^(?i)resource "([^"]+)" is available$`, ResourceAvailable)
Expand Down Expand Up @@ -715,9 +715,9 @@ func ClusterObjectSetObjectsNotFoundOrNotOwned(ctx context.Context, revisionName
return nil
}

// ClusterObjectSetPhaseObjectsUseRefs verifies that every object in every phase of the named
// ClusterObjectSetPhaseObjectsManagedInSecrets verifies that every object in every phase of the named
// ClusterObjectSet uses a ref (not an inline object). Polls with timeout.
func ClusterObjectSetPhaseObjectsUseRefs(ctx context.Context, revisionName string) error {
func ClusterObjectSetPhaseObjectsManagedInSecrets(ctx context.Context, revisionName string) error {
sc := scenarioCtx(ctx)
revisionName = substituteScenarioVars(strings.TrimSpace(revisionName), sc)

Expand Down Expand Up @@ -761,14 +761,14 @@ func ClusterObjectSetPhaseObjectsUseRefs(ctx context.Context, revisionName strin
return nil
}

// ClusterObjectSetRefSecretsExist verifies that all Secrets referenced by the named
// ClusterObjectSetReferredSecretsExist verifies that all Secrets referenced by the named
// ClusterObjectSet's phase objects exist in the given namespace. Polls with timeout.
func ClusterObjectSetRefSecretsExist(ctx context.Context, revisionName, namespace string) error {
func ClusterObjectSetReferredSecretsExist(ctx context.Context, revisionName, namespace string) error {
sc := scenarioCtx(ctx)
revisionName = substituteScenarioVars(strings.TrimSpace(revisionName), sc)
namespace = substituteScenarioVars(strings.TrimSpace(namespace), sc)

secretNames, err := collectRefSecretNames(ctx, revisionName)
secretNames, err := collectReferredSecretNames(ctx, revisionName)
if err != nil {
return err
}
Expand All @@ -782,64 +782,59 @@ func ClusterObjectSetRefSecretsExist(ctx context.Context, revisionName, namespac
return nil
}

// ClusterObjectSetRefSecretsAreImmutable verifies that all ref Secrets for the named
// ClusterObjectSetReferredSecretsAreImmutable verifies that all referred Secrets for the named
// ClusterObjectSet are immutable. Polls with timeout.
func ClusterObjectSetRefSecretsAreImmutable(ctx context.Context, revisionName string) error {
func ClusterObjectSetReferredSecretsAreImmutable(ctx context.Context, revisionName string) error {
sc := scenarioCtx(ctx)
revisionName = substituteScenarioVars(strings.TrimSpace(revisionName), sc)

secrets, err := listRefSecrets(ctx, revisionName)
secrets, err := listReferredSecrets(ctx, revisionName)
if err != nil {
return err
}
if len(secrets) == 0 {
return fmt.Errorf("no ref Secrets found for revision %q", revisionName)
return fmt.Errorf("no referred secrets found for revision %q", revisionName)
}
for _, s := range secrets {
if s.Immutable == nil || !*s.Immutable {
return fmt.Errorf("ref Secret %s/%s is not immutable", s.Namespace, s.Name)
return fmt.Errorf("referred secret %s/%s is not immutable", s.Namespace, s.Name)
}
}
return nil
}

// ClusterObjectSetRefSecretsLabeled verifies that all ref Secrets for the named
// ClusterObjectSet have the expected revision-name and owner-name labels.
func ClusterObjectSetRefSecretsLabeled(ctx context.Context, revisionName string) error {
// ClusterObjectSetReferredSecretsContainLabels verifies that all referred Secrets for the named
// ClusterObjectSet have the expected labels specified in the data table. Polls with timeout.
func ClusterObjectSetReferredSecretsContainLabels(ctx context.Context, revisionName string, table *godog.Table) error {
sc := scenarioCtx(ctx)
revisionName = substituteScenarioVars(strings.TrimSpace(revisionName), sc)

secrets, err := listRefSecrets(ctx, revisionName)
expected, err := parseKeyValueTable(table)
if err != nil {
return err
return fmt.Errorf("invalid labels table: %w", err)
}
if len(secrets) == 0 {
return fmt.Errorf("no ref Secrets found for revision %q", revisionName)
for k, v := range expected {
expected[k] = substituteScenarioVars(v, sc)
}

// Get the owner name from the ClusterObjectSet's own labels.
cosObj, err := getResource("clusterobjectset", revisionName, "")
if err != nil {
return fmt.Errorf("getting ClusterObjectSet %q: %w", revisionName, err)
}
expectedOwner := cosObj.GetLabels()["olm.operatorframework.io/owner-name"]

for _, s := range secrets {
revLabel := s.Labels["olm.operatorframework.io/revision-name"]
if revLabel != revisionName {
return fmt.Errorf("secret %s/%s has revision-name label %q, expected %q", s.Namespace, s.Name, revLabel, revisionName)
waitFor(ctx, func() bool {
secrets, err := listReferredSecrets(ctx, revisionName)
if err != nil || len(secrets) == 0 {
return false
}
ownerLabel := s.Labels["olm.operatorframework.io/owner-name"]
if expectedOwner != "" && ownerLabel != expectedOwner {
return fmt.Errorf("secret %s/%s has owner-name label %q, expected %q", s.Namespace, s.Name, ownerLabel, expectedOwner)
for _, s := range secrets {
if _, _, ok := matchLabels(s.Labels, expected); !ok {
return false
}
}
}
return true
})
Comment on lines +820 to +831
Copy link

Copilot AI Apr 1, 2026

Choose a reason for hiding this comment

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

When label matching fails, the retry loop returns false without emitting any diagnostic signal about which secret failed or which key/value mismatched. Since waitFor will eventually fail with a generic timeout, debugging can be unnecessarily difficult; consider logging the secret name/namespace and the mismatched label key (and expected vs got) on mismatch (similar to the ResourceHasLabels logging).

Copilot uses AI. Check for mistakes.
return nil
}

// ClusterObjectSetRefSecretsHaveOwnerRef verifies that all ref Secrets for the named
// ClusterObjectSetReferredSecretsOwnedByObjectSet verifies that all referred Secrets for the named
// ClusterObjectSet have an ownerReference pointing to the ClusterObjectSet with controller=true.
func ClusterObjectSetRefSecretsHaveOwnerRef(ctx context.Context, revisionName string) error {
func ClusterObjectSetReferredSecretsOwnedByObjectSet(ctx context.Context, revisionName string) error {
sc := scenarioCtx(ctx)
revisionName = substituteScenarioVars(strings.TrimSpace(revisionName), sc)

Expand All @@ -849,12 +844,12 @@ func ClusterObjectSetRefSecretsHaveOwnerRef(ctx context.Context, revisionName st
}
cosUID := cosObj.GetUID()

secrets, err := listRefSecrets(ctx, revisionName)
secrets, err := listReferredSecrets(ctx, revisionName)
if err != nil {
return err
}
if len(secrets) == 0 {
return fmt.Errorf("no ref Secrets found for revision %q", revisionName)
return fmt.Errorf("no referred secrets found for revision %q", revisionName)
}

for _, s := range secrets {
Expand All @@ -875,18 +870,18 @@ func ClusterObjectSetRefSecretsHaveOwnerRef(ctx context.Context, revisionName st
return nil
}

// ClusterObjectSetReferredSecretsHaveType verifies that all ref Secrets for the named
// ClusterObjectSetReferredSecretsHaveType verifies that all referred Secrets for the named
// ClusterObjectSet have the specified Secret type.
func ClusterObjectSetReferredSecretsHaveType(ctx context.Context, revisionName, expectedType string) error {
sc := scenarioCtx(ctx)
revisionName = substituteScenarioVars(strings.TrimSpace(revisionName), sc)

secrets, err := listRefSecrets(ctx, revisionName)
secrets, err := listReferredSecrets(ctx, revisionName)
if err != nil {
return err
}
if len(secrets) == 0 {
return fmt.Errorf("no ref Secrets found for revision %q", revisionName)
return fmt.Errorf("no referred secrets found for revision %q", revisionName)
}

for _, s := range secrets {
Expand All @@ -897,8 +892,8 @@ func ClusterObjectSetReferredSecretsHaveType(ctx context.Context, revisionName,
return nil
}

// collectRefSecretNames returns the unique set of Secret names referenced by the ClusterObjectSet's phase objects.
func collectRefSecretNames(ctx context.Context, revisionName string) ([]string, error) {
// collectReferredSecretNames returns the unique set of Secret names referenced by the ClusterObjectSet's phase objects.
func collectReferredSecretNames(ctx context.Context, revisionName string) ([]string, error) {
var names []string
seen := sets.New[string]()

Expand Down Expand Up @@ -929,18 +924,18 @@ func collectRefSecretNames(ctx context.Context, revisionName string) ([]string,
}
}
if len(names) == 0 {
return nil, fmt.Errorf("no ref Secret names found in ClusterObjectSet %q", revisionName)
return nil, fmt.Errorf("no referred secret names found in ClusterObjectSet %q", revisionName)
}
return names, nil
}

// listRefSecrets lists all Secrets in the OLM namespace that have the revision-name label
// listReferredSecrets lists all Secrets in the OLM namespace that have the revision-name label
// matching the given revision name.
func listRefSecrets(_ context.Context, revisionName string) ([]corev1.Secret, error) {
func listReferredSecrets(_ context.Context, revisionName string) ([]corev1.Secret, error) {
out, err := k8sClient("get", "secrets", "-n", olmNamespace,
"-l", "olm.operatorframework.io/revision-name="+revisionName, "-o", "json")
if err != nil {
return nil, fmt.Errorf("listing ref Secrets for revision %q: %w", revisionName, err)
return nil, fmt.Errorf("listing referred secrets for revision %q: %w", revisionName, err)
}
var secretList corev1.SecretList
if err := json.Unmarshal([]byte(out), &secretList); err != nil {
Expand Down Expand Up @@ -1797,6 +1792,17 @@ func ResourceHasAnnotations(ctx context.Context, resourceName string, table *god
return nil
}

// matchLabels checks that actual labels contain all expected key-value pairs.
// Returns the first mismatched key, the actual value, and false if a mismatch is found.
func matchLabels(actual, expected map[string]string) (string, string, bool) {
for k, v := range expected {
if a, found := actual[k]; !found || a != v {
return k, a, false
}
}
return "", "", true
}
Comment on lines +1795 to +1804
Copy link

Copilot AI Apr 1, 2026

Choose a reason for hiding this comment

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

The helper’s “first mismatched key” is nondeterministic because Go map iteration order is random, which can make logs/flakes harder to reproduce. Consider iterating over a stable ordering of keys (e.g., sort the expected keys before checking) so failures are deterministic and easier to debug.

Copilot uses AI. Check for mistakes.

// ResourceHasLabels waits for a resource to have all labels specified in the data table.
// The table must have "key" and "value" columns.
// Only supports namespaced resources (always uses sc.namespace).
Expand All @@ -1823,12 +1829,9 @@ func ResourceHasLabels(ctx context.Context, resourceName string, table *godog.Ta
if err := json.Unmarshal([]byte(out), &obj); err != nil {
return false
}
labels := obj.GetLabels()
for k, v := range expected {
if actual, found := labels[k]; !found || actual != v {
logger.V(1).Info("Label not yet present or value mismatch", "resource", resourceName, "key", k, "expected", v, "actual", actual)
return false
}
if key, got, ok := matchLabels(obj.GetLabels(), expected); !ok {
logger.V(1).Info("Label not yet present or value mismatch", "resource", resourceName, "key", key, "expected", expected[key], "actual", got)
return false
}
return true
})
Expand Down
Loading