From a2b0eddc4435d2feb28d24c7e079d7bd06511baf Mon Sep 17 00:00:00 2001 From: Roman Bednar Date: Tue, 25 Jan 2022 11:40:18 +0100 Subject: [PATCH 1/2] azure_file: add namespace tests for InTree to CSI conversion When translating InTree pv to CSI pv we use default secret namespace when it's not found in the InTree pv. Using the default is not ideal for several reasons: 1) it can result in failed pod creation after users migrate to cluster with CSI enabled because the existing intree pvs might not have the namespace defined. In that case the "default" is used and mount fails because secret could not be found. 2) falling back to "default" namespace can result in referencing a secret from different namespace which is a security risk However, there is another object we can use to determine correct namespace which presence can be safely assumed - ClaimRef. Mounting a volume is done only through a PVC which is bound. Binding adds ClaimRef to PV and finally the volume gets mounted which is where the translation code is used. --- .../plugins/azure_file_test.go | 64 +++++++++++++++++++ 1 file changed, 64 insertions(+) diff --git a/staging/src/k8s.io/csi-translation-lib/plugins/azure_file_test.go b/staging/src/k8s.io/csi-translation-lib/plugins/azure_file_test.go index 19ba1f5225b..80a6519da13 100644 --- a/staging/src/k8s.io/csi-translation-lib/plugins/azure_file_test.go +++ b/staging/src/k8s.io/csi-translation-lib/plugins/azure_file_test.go @@ -223,6 +223,25 @@ func TestTranslateAzureFileInTreePVToCSI(t *testing.T) { volume: &corev1.PersistentVolume{}, expErr: true, }, + { + name: "return error if secret namespace could not be found", + volume: &corev1.PersistentVolume{ + ObjectMeta: metav1.ObjectMeta{ + Name: "uuid", + Annotations: map[string]string{resourceGroupAnnotation: "rg"}, + }, + Spec: corev1.PersistentVolumeSpec{ + PersistentVolumeSource: corev1.PersistentVolumeSource{ + AzureFile: &corev1.AzureFilePersistentVolumeSource{ + ShareName: "sharename", + SecretName: "secretname", + ReadOnly: true, + }, + }, + }, + }, + expErr: true, + }, { name: "azure file volume", volume: &corev1.PersistentVolume{ @@ -299,6 +318,51 @@ func TestTranslateAzureFileInTreePVToCSI(t *testing.T) { }, }, }, + { + name: "get secret namespace from ClaimRef when it's missing in pv spec source", + volume: &corev1.PersistentVolume{ + ObjectMeta: metav1.ObjectMeta{ + Name: "uuid", + Annotations: map[string]string{resourceGroupAnnotation: "rg"}, + }, + Spec: corev1.PersistentVolumeSpec{ + PersistentVolumeSource: corev1.PersistentVolumeSource{ + AzureFile: &corev1.AzureFilePersistentVolumeSource{ + ShareName: "sharename", + SecretName: "secretname", + //SecretNamespace: &secretNamespace, + ReadOnly: true, + }, + }, + ClaimRef: &corev1.ObjectReference{ + Namespace: secretNamespace, + }, + }, + }, + expVol: &corev1.PersistentVolume{ + ObjectMeta: metav1.ObjectMeta{ + Name: "uuid", + Annotations: map[string]string{resourceGroupAnnotation: "rg"}, + }, + Spec: corev1.PersistentVolumeSpec{ + PersistentVolumeSource: corev1.PersistentVolumeSource{ + CSI: &corev1.CSIPersistentVolumeSource{ + Driver: "file.csi.azure.com", + ReadOnly: true, + NodeStageSecretRef: &corev1.SecretReference{ + Name: "secretname", + Namespace: secretNamespace, + }, + VolumeAttributes: map[string]string{shareNameField: "sharename"}, + VolumeHandle: "rg#secretname#sharename#uuid#secretnamespace", + }, + }, + ClaimRef: &corev1.ObjectReference{ + Namespace: secretNamespace, + }, + }, + }, + }, } for _, tc := range cases { From 507c63c610d4d46e0f5a24e566653118cc4f1810 Mon Sep 17 00:00:00 2001 From: Roman Bednar Date: Mon, 7 Feb 2022 11:13:11 +0100 Subject: [PATCH 2/2] azure_file: try to get secret namespace from ClaimRef This is the actual fix - attempt to obtain a namespace from ClaimRef. Or fail if namespace could not be found instead of using "default". --- .../csi-translation-lib/plugins/azure_file.go | 14 +++++++++++++- 1 file changed, 13 insertions(+), 1 deletion(-) diff --git a/staging/src/k8s.io/csi-translation-lib/plugins/azure_file.go b/staging/src/k8s.io/csi-translation-lib/plugins/azure_file.go index 8588419187c..6a688d7098f 100644 --- a/staging/src/k8s.io/csi-translation-lib/plugins/azure_file.go +++ b/staging/src/k8s.io/csi-translation-lib/plugins/azure_file.go @@ -129,9 +129,21 @@ func (t *azureFileCSITranslator) TranslateInTreePVToCSI(pv *v1.PersistentVolume) resourceGroup = v } } - namespace := defaultSecretNamespace + + // Secret is required when mounting a volume but pod presence cannot be assumed - we should not try to read pod now. + namespace := "" + // Try to read SecretNamespace from source pv. if azureSource.SecretNamespace != nil { namespace = *azureSource.SecretNamespace + } else { + // Try to read namespace from ClaimRef which should be always present. + if pv.Spec.ClaimRef != nil { + namespace = pv.Spec.ClaimRef.Namespace + } + } + + if len(namespace) == 0 { + return nil, fmt.Errorf("could not find a secret namespace in PersistentVolumeSource or ClaimRef") } volumeID := fmt.Sprintf(volumeIDTemplate, resourceGroup, accountName, azureSource.ShareName, pv.ObjectMeta.Name, namespace)