Merge pull request #49574 from liggitt/visitor-type
Automatic merge from submit-queue Typedef visitor to document parameters adds a typedef to clarify the parameters of the visitor updates the unit test to verify each namespace/name pair
This commit is contained in:
		| @@ -27,10 +27,13 @@ func getClaimRefNamespace(pv *api.PersistentVolume) string { | |||||||
| 	return "" | 	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 | // VisitPVSecretNames invokes the visitor function with the name of every secret | ||||||
| // referenced by the PV spec. If visitor returns false, visiting is short-circuited. | // referenced by the PV spec. If visitor returns false, visiting is short-circuited. | ||||||
| // Returns true if visiting completed, false if visiting was 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 | 	source := &pv.Spec.PersistentVolumeSource | ||||||
| 	switch { | 	switch { | ||||||
| 	case source.AzureFile != nil: | 	case source.AzureFile != nil: | ||||||
|   | |||||||
| @@ -31,44 +31,56 @@ func TestPVSecrets(t *testing.T) { | |||||||
| 	// Stub containing all possible secret references in a PV. | 	// Stub containing all possible secret references in a PV. | ||||||
| 	// The names of the referenced secrets match struct paths detected by reflection. | 	// The names of the referenced secrets match struct paths detected by reflection. | ||||||
| 	pvs := []*api.PersistentVolume{ | 	pvs := []*api.PersistentVolume{ | ||||||
| 		{Spec: api.PersistentVolumeSpec{PersistentVolumeSource: api.PersistentVolumeSource{ | 		{Spec: api.PersistentVolumeSpec{ | ||||||
| 			AzureFile: &api.AzureFileVolumeSource{ | 			ClaimRef: &api.ObjectReference{Namespace: "claimrefns", Name: "claimrefname"}, | ||||||
| 				SecretName: "Spec.PersistentVolumeSource.AzureFile.SecretName"}}}}, | 			PersistentVolumeSource: api.PersistentVolumeSource{ | ||||||
| 		{Spec: api.PersistentVolumeSpec{PersistentVolumeSource: api.PersistentVolumeSource{ | 				AzureFile: &api.AzureFileVolumeSource{ | ||||||
| 			CephFS: &api.CephFSVolumeSource{ | 					SecretName: "Spec.PersistentVolumeSource.AzureFile.SecretName"}}}}, | ||||||
| 				SecretRef: &api.LocalObjectReference{ | 		{Spec: api.PersistentVolumeSpec{ | ||||||
| 					Name: "Spec.PersistentVolumeSource.CephFS.SecretRef"}}}}}, | 			ClaimRef: &api.ObjectReference{Namespace: "claimrefns", Name: "claimrefname"}, | ||||||
| 		{Spec: api.PersistentVolumeSpec{PersistentVolumeSource: api.PersistentVolumeSource{ | 			PersistentVolumeSource: api.PersistentVolumeSource{ | ||||||
| 			FlexVolume: &api.FlexVolumeSource{ | 				CephFS: &api.CephFSVolumeSource{ | ||||||
| 				SecretRef: &api.LocalObjectReference{ | 					SecretRef: &api.LocalObjectReference{ | ||||||
| 					Name: "Spec.PersistentVolumeSource.FlexVolume.SecretRef"}}}}}, | 						Name: "Spec.PersistentVolumeSource.CephFS.SecretRef"}}}}}, | ||||||
| 		{Spec: api.PersistentVolumeSpec{PersistentVolumeSource: api.PersistentVolumeSource{ | 		{Spec: api.PersistentVolumeSpec{ | ||||||
| 			RBD: &api.RBDVolumeSource{ | 			ClaimRef: &api.ObjectReference{Namespace: "claimrefns", Name: "claimrefname"}, | ||||||
| 				SecretRef: &api.LocalObjectReference{ | 			PersistentVolumeSource: api.PersistentVolumeSource{ | ||||||
| 					Name: "Spec.PersistentVolumeSource.RBD.SecretRef"}}}}}, | 				FlexVolume: &api.FlexVolumeSource{ | ||||||
| 		{Spec: api.PersistentVolumeSpec{PersistentVolumeSource: api.PersistentVolumeSource{ | 					SecretRef: &api.LocalObjectReference{ | ||||||
| 			ScaleIO: &api.ScaleIOVolumeSource{ | 						Name: "Spec.PersistentVolumeSource.FlexVolume.SecretRef"}}}}}, | ||||||
| 				SecretRef: &api.LocalObjectReference{ | 		{Spec: api.PersistentVolumeSpec{ | ||||||
| 					Name: "Spec.PersistentVolumeSource.ScaleIO.SecretRef"}}}}}, | 			ClaimRef: &api.ObjectReference{Namespace: "claimrefns", Name: "claimrefname"}, | ||||||
| 		{Spec: api.PersistentVolumeSpec{PersistentVolumeSource: api.PersistentVolumeSource{ | 			PersistentVolumeSource: api.PersistentVolumeSource{ | ||||||
| 			ISCSI: &api.ISCSIVolumeSource{ | 				RBD: &api.RBDVolumeSource{ | ||||||
| 				SecretRef: &api.LocalObjectReference{ | 					SecretRef: &api.LocalObjectReference{ | ||||||
| 					Name: "Spec.PersistentVolumeSource.ISCSI.SecretRef"}}}}}, | 						Name: "Spec.PersistentVolumeSource.RBD.SecretRef"}}}}}, | ||||||
| 		{Spec: api.PersistentVolumeSpec{PersistentVolumeSource: api.PersistentVolumeSource{ | 		{Spec: api.PersistentVolumeSpec{ | ||||||
| 			StorageOS: &api.StorageOSPersistentVolumeSource{ | 			ClaimRef: &api.ObjectReference{Namespace: "claimrefns", Name: "claimrefname"}, | ||||||
| 				SecretRef: &api.ObjectReference{ | 			PersistentVolumeSource: api.PersistentVolumeSource{ | ||||||
| 					Name:      "Spec.PersistentVolumeSource.StorageOS.SecretRef", | 				ScaleIO: &api.ScaleIOVolumeSource{ | ||||||
| 					Namespace: "Spec.PersistentVolumeSource.StorageOS.SecretRef"}}}}}, | 					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() | 	extractedNames := sets.NewString() | ||||||
| 	extractedNamespaces := sets.NewString() | 	extractedNamesWithNamespace := sets.NewString() | ||||||
| 	for _, pv := range pvs { | 	for _, pv := range pvs { | ||||||
| 		VisitPVSecretNames(pv, func(namespace, name string) bool { | 		VisitPVSecretNames(pv, func(namespace, name string) bool { | ||||||
| 			extractedNames.Insert(name) | 			extractedNames.Insert(name) | ||||||
| 			if namespace != "" { | 			extractedNamesWithNamespace.Insert(namespace + "/" + name) | ||||||
| 				extractedNamespaces.Insert(namespace) |  | ||||||
| 			} |  | ||||||
| 			return true | 			return true | ||||||
| 		}) | 		}) | ||||||
| 	} | 	} | ||||||
| @@ -108,12 +120,22 @@ func TestPVSecrets(t *testing.T) { | |||||||
| 		t.Error("Extra secret names extracted. Verify VisitPVSecretNames() is correctly extracting secret names") | 		t.Error("Extra secret names extracted. Verify VisitPVSecretNames() is correctly extracting secret names") | ||||||
| 	} | 	} | ||||||
|  |  | ||||||
| 	expectedSecretNamespaces := sets.NewString( | 	expectedNamespacedNames := sets.NewString( | ||||||
| 		"Spec.PersistentVolumeSource.StorageOS.SecretRef", | 		"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 missingNames := expectedNamespacedNames.Difference(extractedNamesWithNamespace); len(missingNames) > 0 { | ||||||
| 	if len(expectedSecretNamespaces.Difference(extractedNamespaces)) > 0 { | 		t.Logf("Missing expected namespaced names:\n%s", strings.Join(missingNames.List(), "\n")) | ||||||
| 		t.Errorf("Missing expected secret namespace") | 		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") | ||||||
| 	} | 	} | ||||||
| } | } | ||||||
|  |  | ||||||
|   | |||||||
		Reference in New Issue
	
	Block a user
	 Kubernetes Submit Queue
					Kubernetes Submit Queue