Handle Secrets in DeleteSnapshot

This PR handles secrets at DeleteSnapshot time.

pkg/connection/connection.go

@@ -50,7 +50,7 @@ type CSIConnection interface {
 	CreateSnapshot(ctx context.Context, snapshotName string, snapshot *crdv1.VolumeSnapshot, volume *v1.PersistentVolume, parameters map[string]string, snapshotterCredentials map[string]string) (driverName string, snapshotId string, timestamp int64, status *csi.SnapshotStatus, err error)
 
 	// DeleteSnapshot deletes a snapshot from a volume
-	DeleteSnapshot(ctx context.Context, snapshotID string) (err error)
+	DeleteSnapshot(ctx context.Context, snapshotID string, snapshotterCredentials map[string]string) (err error)
 
 	// GetSnapshotStatus lists snapshot from a volume
 	GetSnapshotStatus(ctx context.Context, snapshotID string) (*csi.SnapshotStatus, int64, error)
@@ -218,12 +218,12 @@ func (c *csiConnection) CreateSnapshot(ctx context.Context, snapshotName string,
 	return driverName, rsp.Snapshot.Id, rsp.Snapshot.CreatedAt, rsp.Snapshot.Status, nil
 }
 
-func (c *csiConnection) DeleteSnapshot(ctx context.Context, snapshotID string) (err error) {
+func (c *csiConnection) DeleteSnapshot(ctx context.Context, snapshotID string, snapshotterCredentials map[string]string) (err error) {
 	client := csi.NewControllerClient(c.conn)
 
 	req := csi.DeleteSnapshotRequest{
 		SnapshotId:            snapshotID,
-		DeleteSnapshotSecrets: nil,
+		DeleteSnapshotSecrets: snapshotterCredentials,
 	}
 
 	if _, err := client.DeleteSnapshot(ctx, &req); err != nil {

pkg/controller/csi_handler.go

@@ -31,7 +31,7 @@ import (
 // Handler is responsible for handling VolumeSnapshot events from informer.
 type Handler interface {
 	CreateSnapshot(snapshot *crdv1.VolumeSnapshot, volume *v1.PersistentVolume, parameters map[string]string, snapshotterCredentials map[string]string) (string, string, int64, *csi.SnapshotStatus, error)
-	DeleteSnapshot(content *crdv1.VolumeSnapshotContent) error
+	DeleteSnapshot(content *crdv1.VolumeSnapshotContent, snapshotterCredentials map[string]string) error
 	GetSnapshotStatus(content *crdv1.VolumeSnapshotContent) (*csi.SnapshotStatus, int64, error)
 }
 
@@ -69,14 +69,14 @@ func (handler *csiHandler) CreateSnapshot(snapshot *crdv1.VolumeSnapshot, volume
 	return handler.csiConnection.CreateSnapshot(ctx, snapshotName, snapshot, volume, parameters, snapshotterCredentials)
 }
 
-func (handler *csiHandler) DeleteSnapshot(content *crdv1.VolumeSnapshotContent) error {
+func (handler *csiHandler) DeleteSnapshot(content *crdv1.VolumeSnapshotContent, snapshotterCredentials map[string]string) error {
 	if content.Spec.CSI == nil {
 		return fmt.Errorf("CSISnapshot not defined in spec")
 	}
 	ctx, cancel := context.WithTimeout(context.Background(), handler.timeout)
 	defer cancel()
 
-	err := handler.csiConnection.DeleteSnapshot(ctx, content.Spec.CSI.SnapshotHandle)
+	err := handler.csiConnection.DeleteSnapshot(ctx, content.Spec.CSI.SnapshotHandle, snapshotterCredentials)
 	if err != nil {
 		return fmt.Errorf("failed to delete snapshot data %s: %q", content.Name, err)
 	}

pkg/controller/csi_snapshot_controller.go

@@ -483,7 +483,7 @@ func (ctrl *CSISnapshotController) createSnapshotOperation(snapshot *crdv1.Volum
 	contentName := GetSnapshotContentNameForSnapshot(snapshot)
 
 	// Resolve snapshotting secret credentials.
-	snapshotterSecretRef, err := GetSecretReference(snapshotterSecretNameKey, snapshotterSecretNamespaceKey, class.Parameters, contentName, nil)
+	snapshotterSecretRef, err := GetSecretReference(snapshotterSecretNameKey, snapshotterSecretNamespaceKey, class.Parameters, contentName, snapshot)
 	if err != nil {
 		return nil, err
 	}
@@ -577,7 +577,25 @@ func (ctrl *CSISnapshotController) createSnapshotOperation(snapshot *crdv1.Volum
 func (ctrl *CSISnapshotController) DeleteSnapshotContentOperation(content *crdv1.VolumeSnapshotContent) error {
 	glog.V(4).Infof("deleteSnapshotOperation [%s] started", content.Name)
 
-	err := ctrl.handler.DeleteSnapshot(content)
+	// get secrets if VolumeSnapshotClass specifies it
+	var snapshotterCredentials map[string]string
+	snapshotClassName := content.Spec.VolumeSnapshotClassName
+	if len(snapshotClassName) != 0 {
+		if snapshotClass, err := ctrl.clientset.VolumesnapshotV1alpha1().VolumeSnapshotClasses().Get(snapshotClassName, metav1.GetOptions{}); err == nil {
+			// Resolve snapshotting secret credentials.
+			// No VolumeSnapshot is provided when resolving delete secret names, since the VolumeSnapshot may or may not exist at delete time.
+			snapshotterSecretRef, err := GetSecretReference(snapshotterSecretNameKey, snapshotterSecretNamespaceKey, snapshotClass.Parameters, content.Name, nil)
+			if err != nil {
+				return err
+			}
+			snapshotterCredentials, err = GetCredentials(ctrl.client, snapshotterSecretRef)
+			if err != nil {
+				return err
+			}
+		}
+	}
+
+	err := ctrl.handler.DeleteSnapshot(content, snapshotterCredentials)
 	if err != nil {
 		return fmt.Errorf("failed to delete snapshot %#v, err: %v", content.Name, err)
 	}

pkg/controller/util.go

@@ -159,56 +159,57 @@ func GetSecretReference(nameKey, namespaceKey string, snapshotClassParams map[st
 
 	ref := &v1.SecretReference{}
 
-	{
+	// Secret namespace template can make use of the VolumeSnapshotContent name or the VolumeSnapshot namespace.
-		// Secret namespace template can make use of the VolumeSnapshotContent name or the VolumeSnapshot namespace.
+	// Note that neither of those things are under the control of the VolumeSnapshot user.
-		// Note that neither of those things are under the control of the VolumeSnapshot user.
+	namespaceParams := map[string]string{"volumesnapshotcontent.name": snapContentName}
-		namespaceParams := map[string]string{"volumesnapshotcontent.name": snapContentName}
+	// snapshot may be nil when resolving create/delete snapshot secret names because the
-		if snapshot != nil {
+	// snapshot may or may not exist at delete time
-			namespaceParams["volumesnapshot.namespace"] = snapshot.Namespace
+	if snapshot != nil {
-		}
+		namespaceParams["volumesnapshot.namespace"] = snapshot.Namespace
-
-		resolvedNamespace, err := resolveTemplate(namespaceTemplate, namespaceParams)
-		if err != nil {
-			return nil, fmt.Errorf("error resolving %s value %q: %v", namespaceKey, namespaceTemplate, err)
-		}
-		if len(validation.IsDNS1123Label(resolvedNamespace)) > 0 {
-			if namespaceTemplate != resolvedNamespace {
-				return nil, fmt.Errorf("%s parameter %q resolved to %q which is not a valid namespace name", namespaceKey, namespaceTemplate, resolvedNamespace)
-			}
-			return nil, fmt.Errorf("%s parameter %q is not a valid namespace name", namespaceKey, namespaceTemplate)
-		}
-		ref.Namespace = resolvedNamespace
 	}
 
-	{
+	resolvedNamespace, err := resolveTemplate(namespaceTemplate, namespaceParams)
-		// Secret name template can make use of the VolumeSnapshotContent name, VolumeSnapshot name or namespace,
+	if err != nil {
-		// or a VolumeSnapshot annotation.
+		return nil, fmt.Errorf("error resolving %s value %q: %v", namespaceKey, namespaceTemplate, err)
-		// Note that VolumeSnapshot name and annotations are under the VolumeSnapshot user's control.
-		nameParams := map[string]string{"volumesnapshotcontent.name": snapContentName}
-		if snapshot != nil {
-			nameParams["volumesnapshot.name"] = snapshot.Name
-			nameParams["volumesnapshot.namespace"] = snapshot.Namespace
-			for k, v := range snapshot.Annotations {
-				nameParams["volumesnapshot.annotations['"+k+"']"] = v
-			}
-		}
-		resolvedName, err := resolveTemplate(nameTemplate, nameParams)
-		if err != nil {
-			return nil, fmt.Errorf("error resolving %s value %q: %v", nameKey, nameTemplate, err)
-		}
-		if len(validation.IsDNS1123Subdomain(resolvedName)) > 0 {
-			if nameTemplate != resolvedName {
-				return nil, fmt.Errorf("%s parameter %q resolved to %q which is not a valid secret name", nameKey, nameTemplate, resolvedName)
-			}
-			return nil, fmt.Errorf("%s parameter %q is not a valid secret name", nameKey, nameTemplate)
-		}
-		ref.Name = resolvedName
 	}
+	glog.V(4).Infof("GetSecretReference namespaceTemplate %s, namespaceParams: %+v, resolved %s", namespaceTemplate, namespaceParams, resolvedNamespace)
+
+	if len(validation.IsDNS1123Label(resolvedNamespace)) > 0 {
+		if namespaceTemplate != resolvedNamespace {
+			return nil, fmt.Errorf("%s parameter %q resolved to %q which is not a valid namespace name", namespaceKey, namespaceTemplate, resolvedNamespace)
+		}
+		return nil, fmt.Errorf("%s parameter %q is not a valid namespace name", namespaceKey, namespaceTemplate)
+	}
+	ref.Namespace = resolvedNamespace
+
+	// Secret name template can make use of the VolumeSnapshotContent name, VolumeSnapshot name or namespace,
+	// or a VolumeSnapshot annotation.
+	// Note that VolumeSnapshot name and annotations are under the VolumeSnapshot user's control.
+	nameParams := map[string]string{"volumesnapshotcontent.name": snapContentName}
+	if snapshot != nil {
+		nameParams["volumesnapshot.name"] = snapshot.Name
+		nameParams["volumesnapshot.namespace"] = snapshot.Namespace
+		for k, v := range snapshot.Annotations {
+			nameParams["volumesnapshot.annotations['"+k+"']"] = v
+		}
+	}
+	resolvedName, err := resolveTemplate(nameTemplate, nameParams)
+	if err != nil {
+		return nil, fmt.Errorf("error resolving %s value %q: %v", nameKey, nameTemplate, err)
+	}
+	if len(validation.IsDNS1123Subdomain(resolvedName)) > 0 {
+		if nameTemplate != resolvedName {
+			return nil, fmt.Errorf("%s parameter %q resolved to %q which is not a valid secret name", nameKey, nameTemplate, resolvedName)
+		}
+		return nil, fmt.Errorf("%s parameter %q is not a valid secret name", nameKey, nameTemplate)
+	}
+	ref.Name = resolvedName
 
 	glog.V(4).Infof("GetSecretReference validated Secret: %+v", ref)
 	return ref, nil
 }
 
+// resolveTemplate resolves the template by checking if the value is missing for a key
 func resolveTemplate(template string, params map[string]string) (string, error) {
 	missingParams := sets.NewString()
 	resolved := os.Expand(template, func(k string) string {
@@ -224,6 +225,7 @@ func resolveTemplate(template string, params map[string]string) (string, error)
 	return resolved, nil
 }
 
+// GetCredentials retrieves credentials stored in v1.SecretReference
 func GetCredentials(k8s kubernetes.Interface, ref *v1.SecretReference) (map[string]string, error) {
 	if ref == nil {
 		return nil, nil