Merge pull request #1048 from Madhu-1/fix-group-snapshot-secret-bug

Use group specific annotation for the group secrets

pkg/common-controller/groupsnapshot_controller_helper.go

@@ -776,11 +776,11 @@ func (ctrl *csiSnapshotCommonController) createGroupSnapshotContent(groupSnapsho
 		Add secret reference details
 	*/
 	if snapshotterSecretRef != nil {
-		klog.V(5).Infof("createGroupSnapshotContent: set annotation [%s] on volume group snapshot content [%s].", utils.AnnDeletionSecretRefName, groupSnapshotContent.Name)
-		metav1.SetMetaDataAnnotation(&groupSnapshotContent.ObjectMeta, utils.AnnDeletionSecretRefName, snapshotterSecretRef.Name)
+		klog.V(5).Infof("createGroupSnapshotContent: set annotation [%s] on volume group snapshot content [%s].", utils.AnnDeletionGroupSecretRefName, groupSnapshotContent.Name)
+		metav1.SetMetaDataAnnotation(&groupSnapshotContent.ObjectMeta, utils.AnnDeletionGroupSecretRefName, snapshotterSecretRef.Name)
 
-		klog.V(5).Infof("creategroupSnapshotContent: set annotation [%s] on volume group snapshot content [%s].", utils.AnnDeletionSecretRefNamespace, groupSnapshotContent.Name)
-		metav1.SetMetaDataAnnotation(&groupSnapshotContent.ObjectMeta, utils.AnnDeletionSecretRefNamespace, snapshotterSecretRef.Namespace)
+		klog.V(5).Infof("creategroupSnapshotContent: set annotation [%s] on volume group snapshot content [%s].", utils.AnnDeletionGroupSecretRefNamespace, groupSnapshotContent.Name)
+		metav1.SetMetaDataAnnotation(&groupSnapshotContent.ObjectMeta, utils.AnnDeletionGroupSecretRefNamespace, snapshotterSecretRef.Namespace)
 	}
 
 	var updateGroupSnapshotContent *crdv1alpha1.VolumeGroupSnapshotContent
@@ -843,7 +843,7 @@ func (ctrl *csiSnapshotCommonController) getCreateGroupSnapshotInput(groupSnapsh
 	contentName := utils.GetDynamicSnapshotContentNameForGroupSnapshot(groupSnapshot)
 
 	// Get the secret reference
-	snapshotterSecretRef, err := utils.GetGroupSnapshotSecretReference(utils.SnapshotterSecretParams, groupSnapshotClass.Parameters, contentName, groupSnapshot)
+	snapshotterSecretRef, err := utils.GetGroupSnapshotSecretReference(utils.GroupSnapshotterSecretParams, groupSnapshotClass.Parameters, contentName, groupSnapshot)
 	if err != nil {
 		return nil, nil, "", nil, err
 	}

pkg/sidecar-controller/groupsnapshot_helper.go

@@ -297,9 +297,9 @@ func (ctrl *csiSnapshotSideCarController) GetCredentialsFromAnnotationForGroupSn
 	var err error
 
 	// Check if annotation exists
-	if metav1.HasAnnotation(groupSnapshotContent.ObjectMeta, utils.AnnDeletionSecretRefName) && metav1.HasAnnotation(groupSnapshotContent.ObjectMeta, utils.AnnDeletionSecretRefNamespace) {
-		annDeletionSecretName := groupSnapshotContent.Annotations[utils.AnnDeletionSecretRefName]
-		annDeletionSecretNamespace := groupSnapshotContent.Annotations[utils.AnnDeletionSecretRefNamespace]
+	if metav1.HasAnnotation(groupSnapshotContent.ObjectMeta, utils.AnnDeletionGroupSecretRefName) && metav1.HasAnnotation(groupSnapshotContent.ObjectMeta, utils.AnnDeletionGroupSecretRefNamespace) {
+		annDeletionSecretName := groupSnapshotContent.Annotations[utils.AnnDeletionGroupSecretRefName]
+		annDeletionSecretNamespace := groupSnapshotContent.Annotations[utils.AnnDeletionGroupSecretRefNamespace]
 
 		snapshotterSecretRef := &v1.SecretReference{}
 
@@ -423,7 +423,7 @@ func (ctrl *csiSnapshotSideCarController) createGroupSnapshotWrapper(groupSnapsh
 		creationTime = time.Now()
 	}
 
-	groupSnapshotSecret, err := utils.GetSecretReference(utils.GroupSnapshotterSecretParams, class.Parameters, groupSnapshotContent.GetObjectMeta().GetName(), nil)
+	groupSnapshotSecret, err := utils.GetGroupSnapshotSecretReference(utils.GroupSnapshotterSecretParams, class.Parameters, groupSnapshotContent.GetObjectMeta().GetName(), nil)
 	if err != nil {
 		klog.Errorf("Failed to get secret reference for group snapshot content %s: %v", groupSnapshotContent.Name, err)
 		return groupSnapshotContent, fmt.Errorf("failed to get secret reference for group snapshot content %s: %v", groupSnapshotContent.Name, err)
@@ -535,7 +535,7 @@ func (ctrl *csiSnapshotSideCarController) getCSIGroupSnapshotInput(groupSnapshot
 	}
 
 	// Resolve snapshotting secret credentials.
-	snapshotterCredentials, err := ctrl.GetGroupCredentialsFromAnnotation(groupSnapshotContent)
+	snapshotterCredentials, err := ctrl.GetCredentialsFromAnnotationForGroupSnapshot(groupSnapshotContent)
 	if err != nil {
 		return nil, nil, err
 	}
@@ -847,32 +847,3 @@ func (ctrl *csiSnapshotSideCarController) checkandUpdateGroupSnapshotContentStat
 	}
 	return ctrl.createGroupSnapshotWrapper(groupSnapshotContent)
 }
-
-func (ctrl *csiSnapshotSideCarController) GetGroupCredentialsFromAnnotation(content *crdv1alpha1.VolumeGroupSnapshotContent) (map[string]string, error) {
-	var groupSnapshotterCredentials map[string]string
-	var err error
-
-	// Check if annotation exists
-	if metav1.HasAnnotation(content.ObjectMeta, utils.AnnDeletionSecretRefName) && metav1.HasAnnotation(content.ObjectMeta, utils.AnnDeletionSecretRefNamespace) {
-		annDeletionSecretName := content.Annotations[utils.AnnDeletionSecretRefName]
-		annDeletionSecretNamespace := content.Annotations[utils.AnnDeletionSecretRefNamespace]
-
-		groupSnapshotterSecretRef := &v1.SecretReference{}
-
-		if annDeletionSecretName == "" || annDeletionSecretNamespace == "" {
-			return nil, fmt.Errorf("cannot retrieve secrets for volume group snapshot content %#v, err: secret name or namespace not specified", content.Name)
-		}
-
-		groupSnapshotterSecretRef.Name = annDeletionSecretName
-		groupSnapshotterSecretRef.Namespace = annDeletionSecretNamespace
-
-		groupSnapshotterCredentials, err = utils.GetCredentials(ctrl.client, groupSnapshotterSecretRef)
-		if err != nil {
-			// Continue with deletion, as the secret may have already been deleted.
-			klog.Errorf("Failed to get credentials for snapshot %s: %s", content.Name, err.Error())
-			return nil, fmt.Errorf("cannot get credentials for snapshot content %#v", content.Name)
-		}
-	}
-
-	return groupSnapshotterCredentials, nil
-}

pkg/utils/util.go

@@ -137,6 +137,11 @@ const (
 	AnnDeletionSecretRefName      = "snapshot.storage.kubernetes.io/deletion-secret-name"
 	AnnDeletionSecretRefNamespace = "snapshot.storage.kubernetes.io/deletion-secret-namespace"
 
+	// Annotation for secret name and namespace will be added to the group
+	// snapshot content and used at group snapshot content deletion time.
+	AnnDeletionGroupSecretRefName      = "groupsnapshot.storage.kubernetes.io/deletion-secret-name"
+	AnnDeletionGroupSecretRefNamespace = "groupsnapshot.storage.kubernetes.io/deletion-secret-namespace"
+
 	// VolumeSnapshotContentInvalidLabel is applied to invalid content as a label key. The value does not matter.
 	// See https://github.com/kubernetes/enhancements/blob/master/keps/sig-storage/177-volume-snapshot/tighten-validation-webhook-crd.md#automatic-labelling-of-invalid-objects
 	VolumeSnapshotContentInvalidLabel = "snapshot.storage.kubernetes.io/invalid-snapshot-content-resource"
@@ -553,6 +558,8 @@ func RemovePrefixedParameters(param map[string]string) (map[string]string, error
 			case PrefixedSnapshotterSecretNamespaceKey:
 			case PrefixedSnapshotterListSecretNameKey:
 			case PrefixedSnapshotterListSecretNamespaceKey:
+			case PrefixedGroupSnapshotterSecretNameKey:
+			case PrefixedGroupSnapshotterSecretNamespaceKey:
 			default:
 				return map[string]string{}, fmt.Errorf("found unknown parameter key \"%s\" with reserved namespace %s", k, csiParameterPrefix)
 			}