Skip to content

Commit 735b41e

Browse files
pedjakclaude
pedjak
and
claude
authored
refactor(e2e): rename ClusterObjectSet secret cucumber steps for clarity (#2622)
Rename "ref Secrets/refs" step definitions to "referred secrets" for better readability. Update the labels step to accept a data table and extract a shared matchLabels helper with deterministic key ordering. Add diagnostic logging when label matching fails during polling. Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
1 parent fa32b36 commit 735b41e

File tree

2 files changed

+69
-63
lines changed

2 files changed

+69
-63
lines changed

‎test/e2e/features/install.feature‎

Lines changed: 9 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -524,12 +524,15 @@ Feature: Install ClusterExtension
524524
"""
525525
Then ClusterExtension is rolled out
526526
And ClusterExtension is available
527-
And ClusterObjectSet "${NAME}-1" phase objects use refs
528-
And ClusterObjectSet "${NAME}-1" ref Secrets exist in "olmv1-system" namespace
529-
And ClusterObjectSet "${NAME}-1" ref Secrets are immutable
530-
And ClusterObjectSet "${NAME}-1" ref Secrets are labeled with revision and owner
531-
And ClusterObjectSet "${NAME}-1" ref Secrets have ownerReference to the revision
532-
And ClusterObjectSet "${NAME}-1" ref Secrets have type "olm.operatorframework.io/object-data"
527+
And ClusterObjectSet "${NAME}-1" phase objects are managed in Kubernetes secrets
528+
And ClusterObjectSet "${NAME}-1" referred secrets exist in "olmv1-system" namespace
529+
And ClusterObjectSet "${NAME}-1" referred secrets are immutable
530+
And ClusterObjectSet "${NAME}-1" referred secrets contain labels
531+
| key | value |
532+
| olm.operatorframework.io/revision-name | ${NAME}-1 |
533+
| olm.operatorframework.io/owner-name | ${NAME} |
534+
And ClusterObjectSet "${NAME}-1" referred secrets are owned by the object set
535+
And ClusterObjectSet "${NAME}-1" referred secrets have type "olm.operatorframework.io/object-data"
533536

534537
@DeploymentConfig
535538
Scenario: deploymentConfig nodeSelector is applied to the operator deployment

‎test/e2e/steps/steps.go‎

Lines changed: 60 additions & 57 deletions
Original file line numberDiff line numberDiff line change
@@ -96,12 +96,12 @@ func RegisterSteps(sc *godog.ScenarioContext) {
9696
sc.Step(`^(?i)ClusterObjectSet "([^"]+)" contains annotation "([^"]+)" with value$`, ClusterObjectSetHasAnnotationWithValue)
9797
sc.Step(`^(?i)ClusterObjectSet "([^"]+)" has label "([^"]+)" with value "([^"]+)"$`, ClusterObjectSetHasLabelWithValue)
9898
sc.Step(`^(?i)ClusterObjectSet "([^"]+)" phase objects are not found or not owned by the revision$`, ClusterObjectSetObjectsNotFoundOrNotOwned)
99-
sc.Step(`^(?i)ClusterObjectSet "([^"]+)" phase objects use refs$`, ClusterObjectSetPhaseObjectsUseRefs)
100-
sc.Step(`^(?i)ClusterObjectSet "([^"]+)" ref Secrets exist in "([^"]+)" namespace$`, ClusterObjectSetRefSecretsExist)
101-
sc.Step(`^(?i)ClusterObjectSet "([^"]+)" ref Secrets are immutable$`, ClusterObjectSetRefSecretsAreImmutable)
102-
sc.Step(`^(?i)ClusterObjectSet "([^"]+)" ref Secrets are labeled with revision and owner$`, ClusterObjectSetRefSecretsLabeled)
103-
sc.Step(`^(?i)ClusterObjectSet "([^"]+)" ref Secrets have ownerReference to the revision$`, ClusterObjectSetRefSecretsHaveOwnerRef)
104-
sc.Step(`^(?i)ClusterObjectSet "([^"]+)" ref Secrets have type "([^"]+)"$`, ClusterObjectSetReferredSecretsHaveType)
99+
sc.Step(`^(?i)ClusterObjectSet "([^"]+)" phase objects are managed in Kubernetes secrets$`, ClusterObjectSetPhaseObjectsManagedInSecrets)
100+
sc.Step(`^(?i)ClusterObjectSet "([^"]+)" referred secrets exist in "([^"]+)" namespace$`, ClusterObjectSetReferredSecretsExist)
101+
sc.Step(`^(?i)ClusterObjectSet "([^"]+)" referred secrets are immutable$`, ClusterObjectSetReferredSecretsAreImmutable)
102+
sc.Step(`^(?i)ClusterObjectSet "([^"]+)" referred secrets contain labels$`, ClusterObjectSetReferredSecretsContainLabels)
103+
sc.Step(`^(?i)ClusterObjectSet "([^"]+)" referred secrets are owned by the object set$`, ClusterObjectSetReferredSecretsOwnedByObjectSet)
104+
sc.Step(`^(?i)ClusterObjectSet "([^"]+)" referred secrets have type "([^"]+)"$`, ClusterObjectSetReferredSecretsHaveType)
105105

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

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

@@ -761,14 +761,14 @@ func ClusterObjectSetPhaseObjectsUseRefs(ctx context.Context, revisionName strin
761761
return nil
762762
}
763763

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

771-
secretNames, err := collectRefSecretNames(ctx, revisionName)
771+
secretNames, err := collectReferredSecretNames(ctx, revisionName)
772772
if err != nil {
773773
return err
774774
}
@@ -782,64 +782,59 @@ func ClusterObjectSetRefSecretsExist(ctx context.Context, revisionName, namespac
782782
return nil
783783
}
784784

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

791-
secrets, err := listRefSecrets(ctx, revisionName)
791+
secrets, err := listReferredSecrets(ctx, revisionName)
792792
if err != nil {
793793
return err
794794
}
795795
if len(secrets) == 0 {
796-
return fmt.Errorf("no ref Secrets found for revision %q", revisionName)
796+
return fmt.Errorf("no referred secrets found for revision %q", revisionName)
797797
}
798798
for _, s := range secrets {
799799
if s.Immutable == nil || !*s.Immutable {
800-
return fmt.Errorf("ref Secret %s/%s is not immutable", s.Namespace, s.Name)
800+
return fmt.Errorf("referred secret %s/%s is not immutable", s.Namespace, s.Name)
801801
}
802802
}
803803
return nil
804804
}
805805

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

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

820-
// Get the owner name from the ClusterObjectSet's own labels.
821-
cosObj, err := getResource("clusterobjectset", revisionName, "")
822-
if err != nil {
823-
return fmt.Errorf("getting ClusterObjectSet %q: %w", revisionName, err)
824-
}
825-
expectedOwner := cosObj.GetLabels()["olm.operatorframework.io/owner-name"]
826-
827-
for _, s := range secrets {
828-
revLabel := s.Labels["olm.operatorframework.io/revision-name"]
829-
if revLabel != revisionName {
830-
return fmt.Errorf("secret %s/%s has revision-name label %q, expected %q", s.Namespace, s.Name, revLabel, revisionName)
820+
waitFor(ctx, func() bool {
821+
secrets, err := listReferredSecrets(ctx, revisionName)
822+
if err != nil || len(secrets) == 0 {
823+
return false
831824
}
832-
ownerLabel := s.Labels["olm.operatorframework.io/owner-name"]
833-
if expectedOwner != "" && ownerLabel != expectedOwner {
834-
return fmt.Errorf("secret %s/%s has owner-name label %q, expected %q", s.Namespace, s.Name, ownerLabel, expectedOwner)
825+
for _, s := range secrets {
826+
if _, _, ok := matchLabels(s.Labels, expected); !ok {
827+
return false
828+
}
835829
}
836-
}
830+
return true
831+
})
837832
return nil
838833
}
839834

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

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

852-
secrets, err := listRefSecrets(ctx, revisionName)
847+
secrets, err := listReferredSecrets(ctx, revisionName)
853848
if err != nil {
854849
return err
855850
}
856851
if len(secrets) == 0 {
857-
return fmt.Errorf("no ref Secrets found for revision %q", revisionName)
852+
return fmt.Errorf("no referred secrets found for revision %q", revisionName)
858853
}
859854

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

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

884-
secrets, err := listRefSecrets(ctx, revisionName)
879+
secrets, err := listReferredSecrets(ctx, revisionName)
885880
if err != nil {
886881
return err
887882
}
888883
if len(secrets) == 0 {
889-
return fmt.Errorf("no ref Secrets found for revision %q", revisionName)
884+
return fmt.Errorf("no referred secrets found for revision %q", revisionName)
890885
}
891886

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

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

@@ -929,18 +924,18 @@ func collectRefSecretNames(ctx context.Context, revisionName string) ([]string,
929924
}
930925
}
931926
if len(names) == 0 {
932-
return nil, fmt.Errorf("no ref Secret names found in ClusterObjectSet %q", revisionName)
927+
return nil, fmt.Errorf("no referred secret names found in ClusterObjectSet %q", revisionName)
933928
}
934929
return names, nil
935930
}
936931

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

1795+
// matchLabels checks that actual labels contain all expected key-value pairs.
1796+
// Returns the first mismatched key, the actual value, and false if a mismatch is found.
1797+
func matchLabels(actual, expected map[string]string) (string, string, bool) {
1798+
for k, v := range expected {
1799+
if a, found := actual[k]; !found || a != v {
1800+
return k, a, false
1801+
}
1802+
}
1803+
return "", "", true
1804+
}
1805+
18001806
// ResourceHasLabels waits for a resource to have all labels specified in the data table.
18011807
// The table must have "key" and "value" columns.
18021808
// Only supports namespaced resources (always uses sc.namespace).
@@ -1823,12 +1829,9 @@ func ResourceHasLabels(ctx context.Context, resourceName string, table *godog.Ta
18231829
if err := json.Unmarshal([]byte(out), &obj); err != nil {
18241830
return false
18251831
}
1826-
labels := obj.GetLabels()
1827-
for k, v := range expected {
1828-
if actual, found := labels[k]; !found || actual != v {
1829-
logger.V(1).Info("Label not yet present or value mismatch", "resource", resourceName, "key", k, "expected", v, "actual", actual)
1830-
return false
1831-
}
1832+
if key, got, ok := matchLabels(obj.GetLabels(), expected); !ok {
1833+
logger.V(1).Info("Label not yet present or value mismatch", "resource", resourceName, "key", key, "expected", expected[key], "actual", got)
1834+
return false
18321835
}
18331836
return true
18341837
})

0 commit comments

Comments
 (0)