From aac11005b5f0a15b9e176c65df4e6098970b0915 Mon Sep 17 00:00:00 2001 From: Daniel Lipovetsky Date: Fri, 10 May 2024 17:12:21 -0700 Subject: [PATCH] test: Ensure ownerRef assertions for all Kinds are evaluated The assertFuncs are passed as a list. Previously, if two assertFuncs were defined for the same Kind, the assertFunc that appeared later in the list would overwrite the one that appeared earlier. If someone introduced a second assertFunc for a given Kind, they would receive no signal that the first assertFunc was overwritten. That would result in missed assertions. With this change, if two or more assertFuncs are defined for the same Kind, both are evaluated. --- test/framework/ownerreference_helpers.go | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/test/framework/ownerreference_helpers.go b/test/framework/ownerreference_helpers.go index 9fb0c277b443..8f68f2d2d28f 100644 --- a/test/framework/ownerreference_helpers.go +++ b/test/framework/ownerreference_helpers.go @@ -102,10 +102,10 @@ func ValidateOwnerReferencesResilience(ctx context.Context, proxy ClusterProxy, } func AssertOwnerReferences(namespace, kubeconfigPath string, ownerGraphFilterFunction clusterctlcluster.GetOwnerGraphFilterFunction, assertFuncs ...map[string]func(reference []metav1.OwnerReference) error) { - allAssertFuncs := map[string]func(reference []metav1.OwnerReference) error{} + allAssertFuncs := map[string][]func(reference []metav1.OwnerReference) error{} for _, m := range assertFuncs { for k, v := range m { - allAssertFuncs[k] = v + allAssertFuncs[k] = append(allAssertFuncs[k], v) } } Eventually(func() error { @@ -125,8 +125,10 @@ func AssertOwnerReferences(namespace, kubeconfigPath string, ownerGraphFilterFun allErrs = append(allErrs, fmt.Errorf("kind %s does not have an associated ownerRef assertion function", v.Object.Kind)) continue } - if err := allAssertFuncs[v.Object.Kind](v.Owners); err != nil { - allErrs = append(allErrs, errors.Wrapf(err, "Unexpected ownerReferences for %s/%s", v.Object.Kind, v.Object.Name)) + for _, f := range allAssertFuncs[v.Object.Kind] { + if err := f(v.Owners); err != nil { + allErrs = append(allErrs, errors.Wrapf(err, "Unexpected ownerReferences for %s/%s", v.Object.Kind, v.Object.Name)) + } } } return kerrors.NewAggregate(allErrs) @@ -308,7 +310,6 @@ var DockerInfraOwnerReferenceAssertions = map[string]func([]metav1.OwnerReferenc dockerMachineKind: func(owners []metav1.OwnerReference) error { // The DockerMachine must be owned and controlled by a Machine or a DockerMachinePool. return HasOneOfExactOwners(owners, []metav1.OwnerReference{machineController}, []metav1.OwnerReference{machineController, dockerMachinePoolController}) - }, dockerMachineTemplateKind: func(owners []metav1.OwnerReference) error { // Base DockerMachineTemplates referenced in a ClusterClass must be owned by the ClusterClass.