Skip to content

Commit

Permalink
Allow PVC name/namespace as template for provisioner-secret
Browse files Browse the repository at this point in the history 
Backport kubernetes-csi#274 from the master
  • Loading branch information
@ggriffiths
ggriffiths authored and oleksiys committed Jun 3, 2019
1 parent cfb8518 commit 65328d1
Show file tree
Hide file tree
Showing 2 changed files with 260 additions and 6 deletions.
20 changes: 15 additions & 5 deletions pkg/controller/controller.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -445,7 +445,12 @@ func (p *csiProvisioner) Provision(options controller.VolumeOptions) (*v1.Persis

// Resolve provision secret credentials.
// No PVC is provided when resolving provision/delete secret names, since the PVC may or may not exist at delete time.
provisionerSecretRef, err := getSecretReference(provisionerSecretParams, createVolumeRequestParameters, pvName, nil)
provisionerSecretRef, err := getSecretReference(provisionerSecretParams, createVolumeRequestParameters, pvName, &v1.PersistentVolumeClaim{
ObjectMeta: metav1.ObjectMeta{
Name: options.PVC.Name,
Namespace: options.PVC.Namespace,
},
})
if err != nil {
return nil, err
}
Expand Down Expand Up @@ -681,18 +686,23 @@ func (p *csiProvisioner) Delete(volume *v1.PersistentVolume) error {
if len(storageClassName) != 0 {
if storageClass, err := p.client.StorageV1().StorageClasses().Get(storageClassName, metav1.GetOptions{}); err == nil {
// Resolve provision secret credentials.
// No PVC is provided when resolving provision/delete secret names, since the PVC may or may not exist at delete time.
provisionerSecretRef, err := getSecretReference(provisionerSecretParams, storageClass.Parameters, volume.Name, nil)
provisionerSecretRef, err := getSecretReference(provisionerSecretParams, storageClass.Parameters, volume.Name, &v1.PersistentVolumeClaim{
ObjectMeta: metav1.ObjectMeta{
Name: volume.Spec.ClaimRef.Name,
Namespace: volume.Spec.ClaimRef.Namespace,
},
})
if err != nil {
return err
}

credentials, err := getCredentials(p.client, provisionerSecretRef)
if err != nil {
return err
// Continue with deletion, as the secret may have already been deleted.
klog.Errorf("Failed to get credentials for volume %s: %s", volume.Name, err.Error())
}
req.Secrets = credentials
}

}
ctx, cancel := context.WithTimeout(context.Background(), p.timeout)
defer cancel()
Expand Down
246 changes: 245 additions & 1 deletion pkg/controller/controller_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@ import (
"google.golang.org/grpc"
corev1 "k8s.io/api/core/v1"
v1 "k8s.io/api/core/v1"
storagev1 "k8s.io/api/storage/v1"
storage "k8s.io/api/storage/v1beta1"
"k8s.io/apimachinery/pkg/api/resource"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
Expand Down Expand Up @@ -555,6 +556,20 @@ func TestGetSecretReference(t *testing.T) {
pvc: nil,
expectRef: &v1.SecretReference{Name: "name", Namespace: "ns"},
},
"simple - valid, pvc name and namespace": {
secretParams: provisionerSecretParams,
params: map[string]string{
provisionerSecretNameKey: "param-name",
provisionerSecretNamespaceKey: "param-ns",
},
pvc: &v1.PersistentVolumeClaim{
ObjectMeta: metav1.ObjectMeta{
Name: "name",
Namespace: "ns",
},
},
expectRef: &v1.SecretReference{Name: "param-name", Namespace: "param-ns"},
},
"simple - invalid name": {
secretParams: nodePublishSecretParams,
params: map[string]string{nodePublishSecretNameKey: "bad name", nodePublishSecretNamespaceKey: "ns"},
Expand All @@ -569,7 +584,20 @@ func TestGetSecretReference(t *testing.T) {
expectRef: nil,
expectErr: true,
},
"template - valid": {
"template - PVC name annotations not supported for Provision and Delete": {
secretParams: provisionerSecretParams,
params: map[string]string{
prefixedProvisionerSecretNameKey: "static-${pv.name}-${pvc.namespace}-${pvc.name}-${pvc.annotations['akey']}",
},
pvc: &v1.PersistentVolumeClaim{
ObjectMeta: metav1.ObjectMeta{
Name: "name",
Namespace: "ns",
},
},
expectErr: true,
},
"template - valid nodepublish secret ref": {
secretParams: nodePublishSecretParams,
params: map[string]string{
nodePublishSecretNameKey: "static-${pv.name}-${pvc.namespace}-${pvc.name}-${pvc.annotations['akey']}",
Expand All @@ -585,6 +613,63 @@ func TestGetSecretReference(t *testing.T) {
},
expectRef: &v1.SecretReference{Name: "static-pvname-pvcnamespace-pvcname-avalue", Namespace: "static-pvname-pvcnamespace"},
},
"template - valid provisioner secret ref": {
secretParams: provisionerSecretParams,
params: map[string]string{
provisionerSecretNameKey: "static-provisioner-${pv.name}-${pvc.namespace}-${pvc.name}",
provisionerSecretNamespaceKey: "static-provisioner-${pv.name}-${pvc.namespace}",
},
pvName: "pvname",
pvc: &v1.PersistentVolumeClaim{
ObjectMeta: metav1.ObjectMeta{
Name: "pvcname",
Namespace: "pvcnamespace",
},
},
expectRef: &v1.SecretReference{Name: "static-provisioner-pvname-pvcnamespace-pvcname", Namespace: "static-provisioner-pvname-pvcnamespace"},
},
"template - valid, with pvc.name": {
secretParams: provisionerSecretParams,
params: map[string]string{
provisionerSecretNameKey: "${pvc.name}",
provisionerSecretNamespaceKey: "ns",
},
pvc: &v1.PersistentVolumeClaim{
ObjectMeta: metav1.ObjectMeta{
Name: "pvcname",
Namespace: "pvcns",
},
},
expectRef: &v1.SecretReference{Name: "pvcname", Namespace: "ns"},
},
"template - valid, provisioner with pvc name and namepsace": {
secretParams: provisionerSecretParams,
params: map[string]string{
provisionerSecretNameKey: "${pvc.name}",
provisionerSecretNamespaceKey: "${pvc.namespace}",
},
pvc: &v1.PersistentVolumeClaim{
ObjectMeta: metav1.ObjectMeta{
Name: "pvcname",
Namespace: "pvcns",
},
},
expectRef: &v1.SecretReference{Name: "pvcname", Namespace: "pvcns"},
},
"template - valid, static pvc name and templated namespace": {
secretParams: provisionerSecretParams,
params: map[string]string{
provisionerSecretNameKey: "static-name-1",
provisionerSecretNamespaceKey: "${pvc.namespace}",
},
pvc: &v1.PersistentVolumeClaim{
ObjectMeta: metav1.ObjectMeta{
Name: "name",
Namespace: "ns",
},
},
expectRef: &v1.SecretReference{Name: "static-name-1", Namespace: "ns"},
},
"template - invalid namespace tokens": {
secretParams: nodePublishSecretParams,
params: map[string]string{
Expand Down Expand Up @@ -1787,3 +1872,162 @@ func TestProvisionWithMountOptions(t *testing.T) {
t.Errorf("expected mount options %v; got: %v", expectedOptions, pv.Spec.MountOptions)
}
}

type deleteTestcase struct {
persistentVolume *v1.PersistentVolume
storageClass *storagev1.StorageClass
mockDelete bool
expectErr bool
}

// TestDelete is a test of the delete operation
func TestDelete(t *testing.T) {
tt := map[string]deleteTestcase{
"fail - nil PV": deleteTestcase{
persistentVolume: nil,
expectErr: true,
},
"fail - nil volume.Spec.CSI": deleteTestcase{
persistentVolume: &v1.PersistentVolume{
ObjectMeta: metav1.ObjectMeta{
Name: "pv",
Namespace: "ns",
Annotations: map[string]string{
prefixedProvisionerSecretNameKey: "static-${pv.name}-${pvc.namespace}-${pvc.name}-${pvc.annotations['akey']}",
},
},
Spec: v1.PersistentVolumeSpec{
PersistentVolumeSource: v1.PersistentVolumeSource{},
},
},
expectErr: true,
},
"fail - pvc.annotations not supported": deleteTestcase{
persistentVolume: &v1.PersistentVolume{
ObjectMeta: metav1.ObjectMeta{
Name: "pv",
Namespace: "ns",
},
Spec: v1.PersistentVolumeSpec{
PersistentVolumeSource: v1.PersistentVolumeSource{
CSI: &v1.CSIPersistentVolumeSource{
VolumeHandle: "vol-id-1",
},
},
ClaimRef: &v1.ObjectReference{
Name: "sc-name",
Namespace: "ns",
},
StorageClassName: "sc-name",
},
},
storageClass: &storagev1.StorageClass{
ObjectMeta: metav1.ObjectMeta{
Name: "sc-name",
Namespace: "ns",
},
Parameters: map[string]string{
prefixedProvisionerSecretNameKey: "static-${pv.name}-${pvc.namespace}-${pvc.name}-${pvc.annotations['akey']}",
},
},
expectErr: true,
},
"simple - valid case": deleteTestcase{
persistentVolume: &v1.PersistentVolume{
ObjectMeta: metav1.ObjectMeta{
Name: "pv",
Namespace: "ns",
},
Spec: v1.PersistentVolumeSpec{
PersistentVolumeSource: v1.PersistentVolumeSource{
CSI: &v1.CSIPersistentVolumeSource{
VolumeHandle: "vol-id-1",
},
},
},
},
storageClass: &storagev1.StorageClass{
ObjectMeta: metav1.ObjectMeta{
Name: "sc-name",
Namespace: "ns",
},
Parameters: map[string]string{
prefixedProvisionerSecretNameKey: "static-${pv.name}-${pvc.namespace}-${pvc.name}",
},
},
expectErr: false,
mockDelete: true,
},
"simple - valid case with ClaimRef set": deleteTestcase{
persistentVolume: &v1.PersistentVolume{
ObjectMeta: metav1.ObjectMeta{
Name: "pv",
Namespace: "ns",
},
Spec: v1.PersistentVolumeSpec{
PersistentVolumeSource: v1.PersistentVolumeSource{
CSI: &v1.CSIPersistentVolumeSource{
VolumeHandle: "vol-id-1",
},
},
ClaimRef: &v1.ObjectReference{
Name: "pvc-name",
Namespace: "ns",
},
},
},
storageClass: &storagev1.StorageClass{
ObjectMeta: metav1.ObjectMeta{
Name: "sc-name",
Namespace: "ns",
},
Parameters: map[string]string{
prefixedProvisionerSecretNameKey: "static-${pv.name}-${pvc.namespace}-${pvc.name}",
},
},
expectErr: false,
mockDelete: true,
},
}

for k, tc := range tt {
runDeleteTest(t, k, tc)
}
}

func runDeleteTest(t *testing.T, k string, tc deleteTestcase) {
t.Logf("Running test: %v", k)

tmpdir := tempDir(t)

defer os.RemoveAll(tmpdir)
mockController, driver, _, controllerServer, csiConn, err := createMockServer(t, tmpdir)
if err != nil {
t.Fatal(err)
}
defer mockController.Finish()
defer driver.Stop()

var clientSet *fakeclientset.Clientset

if tc.storageClass != nil {
clientSet = fakeclientset.NewSimpleClientset(tc.storageClass)
} else {
clientSet = fakeclientset.NewSimpleClientset()
}

if tc.mockDelete {
controllerServer.EXPECT().DeleteVolume(gomock.Any(), gomock.Any()).Return(&csi.DeleteVolumeResponse{}, nil).Times(1)
}

pluginCaps, controllerCaps := provisionCapabilities()
csiProvisioner := NewCSIProvisioner(clientSet, 5*time.Second, "test-provisioner", "test", 5, csiConn.conn, nil, driverName, pluginCaps, controllerCaps, "")

err = csiProvisioner.Delete(tc.persistentVolume)
if tc.expectErr && err == nil {
t.Errorf("test %q: Expected error, got none", k)
}
if !tc.expectErr && err != nil {
t.Errorf("test %q: got error: %v", k, err)
}
}

0 comments on commit 65328d1

Please sign in to comment.