From 2a28df4495bc177b1a51f76228f98f786035e6a1 Mon Sep 17 00:00:00 2001 From: Jordan Liggitt Date: Tue, 25 Jul 2017 12:10:06 -0400 Subject: [PATCH] Typedef visitor to document parameters --- pkg/api/persistentvolume/util.go | 5 +- pkg/api/persistentvolume/util_test.go | 96 ++++++++++++++++----------- 2 files changed, 63 insertions(+), 38 deletions(-) diff --git a/pkg/api/persistentvolume/util.go b/pkg/api/persistentvolume/util.go index 264e3f93b1..d4edd807a7 100644 --- a/pkg/api/persistentvolume/util.go +++ b/pkg/api/persistentvolume/util.go @@ -27,10 +27,13 @@ func getClaimRefNamespace(pv *api.PersistentVolume) string { return "" } +// Visitor is called with each object's namespace and name, and returns true if visiting should continue +type Visitor func(namespace, name string) (shouldContinue bool) + // VisitPVSecretNames invokes the visitor function with the name of every secret // referenced by the PV spec. If visitor returns false, visiting is short-circuited. // Returns true if visiting completed, false if visiting was short-circuited. -func VisitPVSecretNames(pv *api.PersistentVolume, visitor func(string, string) bool) bool { +func VisitPVSecretNames(pv *api.PersistentVolume, visitor Visitor) bool { source := &pv.Spec.PersistentVolumeSource switch { case source.AzureFile != nil: diff --git a/pkg/api/persistentvolume/util_test.go b/pkg/api/persistentvolume/util_test.go index f35c5e7eb2..87564d3372 100644 --- a/pkg/api/persistentvolume/util_test.go +++ b/pkg/api/persistentvolume/util_test.go @@ -31,44 +31,56 @@ func TestPVSecrets(t *testing.T) { // Stub containing all possible secret references in a PV. // The names of the referenced secrets match struct paths detected by reflection. pvs := []*api.PersistentVolume{ - {Spec: api.PersistentVolumeSpec{PersistentVolumeSource: api.PersistentVolumeSource{ - AzureFile: &api.AzureFileVolumeSource{ - SecretName: "Spec.PersistentVolumeSource.AzureFile.SecretName"}}}}, - {Spec: api.PersistentVolumeSpec{PersistentVolumeSource: api.PersistentVolumeSource{ - CephFS: &api.CephFSVolumeSource{ - SecretRef: &api.LocalObjectReference{ - Name: "Spec.PersistentVolumeSource.CephFS.SecretRef"}}}}}, - {Spec: api.PersistentVolumeSpec{PersistentVolumeSource: api.PersistentVolumeSource{ - FlexVolume: &api.FlexVolumeSource{ - SecretRef: &api.LocalObjectReference{ - Name: "Spec.PersistentVolumeSource.FlexVolume.SecretRef"}}}}}, - {Spec: api.PersistentVolumeSpec{PersistentVolumeSource: api.PersistentVolumeSource{ - RBD: &api.RBDVolumeSource{ - SecretRef: &api.LocalObjectReference{ - Name: "Spec.PersistentVolumeSource.RBD.SecretRef"}}}}}, - {Spec: api.PersistentVolumeSpec{PersistentVolumeSource: api.PersistentVolumeSource{ - ScaleIO: &api.ScaleIOVolumeSource{ - SecretRef: &api.LocalObjectReference{ - Name: "Spec.PersistentVolumeSource.ScaleIO.SecretRef"}}}}}, - {Spec: api.PersistentVolumeSpec{PersistentVolumeSource: api.PersistentVolumeSource{ - ISCSI: &api.ISCSIVolumeSource{ - SecretRef: &api.LocalObjectReference{ - Name: "Spec.PersistentVolumeSource.ISCSI.SecretRef"}}}}}, - {Spec: api.PersistentVolumeSpec{PersistentVolumeSource: api.PersistentVolumeSource{ - StorageOS: &api.StorageOSPersistentVolumeSource{ - SecretRef: &api.ObjectReference{ - Name: "Spec.PersistentVolumeSource.StorageOS.SecretRef", - Namespace: "Spec.PersistentVolumeSource.StorageOS.SecretRef"}}}}}, + {Spec: api.PersistentVolumeSpec{ + ClaimRef: &api.ObjectReference{Namespace: "claimrefns", Name: "claimrefname"}, + PersistentVolumeSource: api.PersistentVolumeSource{ + AzureFile: &api.AzureFileVolumeSource{ + SecretName: "Spec.PersistentVolumeSource.AzureFile.SecretName"}}}}, + {Spec: api.PersistentVolumeSpec{ + ClaimRef: &api.ObjectReference{Namespace: "claimrefns", Name: "claimrefname"}, + PersistentVolumeSource: api.PersistentVolumeSource{ + CephFS: &api.CephFSVolumeSource{ + SecretRef: &api.LocalObjectReference{ + Name: "Spec.PersistentVolumeSource.CephFS.SecretRef"}}}}}, + {Spec: api.PersistentVolumeSpec{ + ClaimRef: &api.ObjectReference{Namespace: "claimrefns", Name: "claimrefname"}, + PersistentVolumeSource: api.PersistentVolumeSource{ + FlexVolume: &api.FlexVolumeSource{ + SecretRef: &api.LocalObjectReference{ + Name: "Spec.PersistentVolumeSource.FlexVolume.SecretRef"}}}}}, + {Spec: api.PersistentVolumeSpec{ + ClaimRef: &api.ObjectReference{Namespace: "claimrefns", Name: "claimrefname"}, + PersistentVolumeSource: api.PersistentVolumeSource{ + RBD: &api.RBDVolumeSource{ + SecretRef: &api.LocalObjectReference{ + Name: "Spec.PersistentVolumeSource.RBD.SecretRef"}}}}}, + {Spec: api.PersistentVolumeSpec{ + ClaimRef: &api.ObjectReference{Namespace: "claimrefns", Name: "claimrefname"}, + PersistentVolumeSource: api.PersistentVolumeSource{ + ScaleIO: &api.ScaleIOVolumeSource{ + SecretRef: &api.LocalObjectReference{ + Name: "Spec.PersistentVolumeSource.ScaleIO.SecretRef"}}}}}, + {Spec: api.PersistentVolumeSpec{ + ClaimRef: &api.ObjectReference{Namespace: "claimrefns", Name: "claimrefname"}, + PersistentVolumeSource: api.PersistentVolumeSource{ + ISCSI: &api.ISCSIVolumeSource{ + SecretRef: &api.LocalObjectReference{ + Name: "Spec.PersistentVolumeSource.ISCSI.SecretRef"}}}}}, + {Spec: api.PersistentVolumeSpec{ + ClaimRef: &api.ObjectReference{Namespace: "claimrefns", Name: "claimrefname"}, + PersistentVolumeSource: api.PersistentVolumeSource{ + StorageOS: &api.StorageOSPersistentVolumeSource{ + SecretRef: &api.ObjectReference{ + Name: "Spec.PersistentVolumeSource.StorageOS.SecretRef", + Namespace: "storageosns"}}}}}, } extractedNames := sets.NewString() - extractedNamespaces := sets.NewString() + extractedNamesWithNamespace := sets.NewString() for _, pv := range pvs { VisitPVSecretNames(pv, func(namespace, name string) bool { extractedNames.Insert(name) - if namespace != "" { - extractedNamespaces.Insert(namespace) - } + extractedNamesWithNamespace.Insert(namespace + "/" + name) return true }) } @@ -108,12 +120,22 @@ func TestPVSecrets(t *testing.T) { t.Error("Extra secret names extracted. Verify VisitPVSecretNames() is correctly extracting secret names") } - expectedSecretNamespaces := sets.NewString( - "Spec.PersistentVolumeSource.StorageOS.SecretRef", + expectedNamespacedNames := sets.NewString( + "claimrefns/Spec.PersistentVolumeSource.AzureFile.SecretName", + "claimrefns/Spec.PersistentVolumeSource.CephFS.SecretRef", + "claimrefns/Spec.PersistentVolumeSource.FlexVolume.SecretRef", + "claimrefns/Spec.PersistentVolumeSource.RBD.SecretRef", + "claimrefns/Spec.PersistentVolumeSource.ScaleIO.SecretRef", + "claimrefns/Spec.PersistentVolumeSource.ISCSI.SecretRef", + "storageosns/Spec.PersistentVolumeSource.StorageOS.SecretRef", ) - - if len(expectedSecretNamespaces.Difference(extractedNamespaces)) > 0 { - t.Errorf("Missing expected secret namespace") + if missingNames := expectedNamespacedNames.Difference(extractedNamesWithNamespace); len(missingNames) > 0 { + t.Logf("Missing expected namespaced names:\n%s", strings.Join(missingNames.List(), "\n")) + t.Error("Missing expected namespaced names. Verify the PV stub above includes these references, then verify VisitPVSecretNames() is correctly finding the missing names") + } + if extraNames := extractedNamesWithNamespace.Difference(expectedNamespacedNames); len(extraNames) > 0 { + t.Logf("Extra namespaced names:\n%s", strings.Join(extraNames.List(), "\n")) + t.Error("Extra namespaced names extracted. Verify VisitPVSecretNames() is correctly extracting secret names") } }