Skip to content

Commit

Permalink
Add unit tests and remove pvc.Annotation secret support for create
Browse files Browse the repository at this point in the history 
Signed-off-by: Grant Griffiths <[email protected]>
  • Loading branch information
@ggriffiths
ggriffiths committed May 14, 2019
1 parent 9625615 commit e0ab76e
Show file tree
Hide file tree
Showing 2 changed files with 181 additions and 15 deletions.
30 changes: 20 additions & 10 deletions pkg/controller/controller.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -445,7 +445,7 @@ func (p *csiProvisioner) Provision(options controller.VolumeOptions) (*v1.Persis
rep := &csi.CreateVolumeResponse{}

// Resolve provision secret credentials.
provisionerSecretRef, err := getSecretReference(provisionerSecretParams, createVolumeRequestParameters, pvName, options.PVC)
provisionerSecretRef, err := getProvisionSecretReference(provisionerSecretParams, createVolumeRequestParameters, pvName, options.PVC)
if err != nil {
return nil, err
}
Expand Down Expand Up @@ -663,19 +663,17 @@ func (p *csiProvisioner) Delete(volume *v1.PersistentVolume) error {
storageClassName := volume.Spec.StorageClassName
if len(storageClassName) != 0 {
if storageClass, err := p.client.StorageV1().StorageClasses().Get(storageClassName, metav1.GetOptions{}); err == nil {

// Get PVC for secret reference if the volume is bound.
var pvc *v1.PersistentVolumeClaim
if volume.Spec.ClaimRef != nil {
pvc, _ = p.client.CoreV1().PersistentVolumeClaims(volume.Spec.ClaimRef.Namespace).Get(volume.Spec.ClaimRef.Name, metav1.GetOptions{})
// If we do not find the PVC, continue with deletion.
}

// Resolve provision secret credentials.
provisionerSecretRef, err := getSecretReference(provisionerSecretParams, storageClass.Parameters, volume.Name, pvc)
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 {
// Continue with deletion, as the secret may have already been deleted.
Expand Down Expand Up @@ -756,6 +754,18 @@ func verifyAndGetSecretNameAndNamespaceTemplate(secret secretParamsMap, storageC
}
}

func getProvisionSecretReference(secretParams secretParamsMap, storageClassParams map[string]string, pvName string, pvc *v1.PersistentVolumeClaim) (*v1.SecretReference, error) {
// validate that no annotations are being passed in as secrets
for k, v := range pvc.Annotations {
if strings.Contains(k, "csi.storage.k8s.io/provisioner-secret") &&
strings.Contains(v, "pvc.annotations") {
return nil, fmt.Errorf("Secrets stored in PVC annotations is not supported for Provision and Delete")
}
}

return getSecretReference(secretParams, storageClassParams, pvName, pvc)
}

// getSecretReference returns a reference to the secret specified in the given nameTemplate
// and namespaceTemplate, or an error if the templates are not specified correctly.
// no lookup of the referenced secret is performed, and the secret may or may not exist.
Expand Down
166 changes: 161 additions & 5 deletions pkg/controller/controller_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -583,18 +583,17 @@ func TestGetSecretReference(t *testing.T) {
"template - valid": {
secretParams: nodePublishSecretParams,
params: map[string]string{
nodePublishSecretNameKey: "static-${pv.name}-${pvc.namespace}-${pvc.name}-${pvc.annotations['akey']}",
nodePublishSecretNameKey: "static-${pv.name}-${pvc.namespace}-${pvc.name}",
nodePublishSecretNamespaceKey: "static-${pv.name}-${pvc.namespace}",
},
pvName: "pvname",
pvc: &v1.PersistentVolumeClaim{
ObjectMeta: metav1.ObjectMeta{
Name: "pvcname",
Namespace: "pvcnamespace",
Annotations: map[string]string{"akey": "avalue"},
Name: "pvcname",
Namespace: "pvcnamespace",
},
},
expectRef: &v1.SecretReference{Name: "static-pvname-pvcnamespace-pvcname-avalue", Namespace: "static-pvname-pvcnamespace"},
expectRef: &v1.SecretReference{Name: "static-pvname-pvcnamespace-pvcname", Namespace: "static-pvname-pvcnamespace"},
},
"template - valid, with pvc name": {
secretParams: provisionerSecretParams,
Expand Down Expand Up @@ -680,6 +679,78 @@ func TestGetSecretReference(t *testing.T) {
}
}

func TestGetProvisionSecretReference(t *testing.T) {
testcases := map[string]struct {
secretParams secretParamsMap
params map[string]string
pvName string
pvc *v1.PersistentVolumeClaim

expectRef *v1.SecretReference
expectErr bool
}{
"simple - valid name and namespace": {
secretParams: provisionerSecretParams,
params: map[string]string{
provisionerSecretNameKey: "${pvc.name}",
provisionerSecretNamespaceKey: "${pvc.namespace}",
},
pvc: &v1.PersistentVolumeClaim{
ObjectMeta: metav1.ObjectMeta{
Name: "name",
Namespace: "ns",
},
},
expectRef: &v1.SecretReference{Name: "name", Namespace: "ns"},
},
"simple - PVC name annotations not supported for Provision and Delete": {
secretParams: provisionerSecretParams,
pvc: &v1.PersistentVolumeClaim{
ObjectMeta: metav1.ObjectMeta{
Name: "name",
Namespace: "ns",
Annotations: map[string]string{
prefixedProvisionerSecretNameKey: "static-${pv.name}-${pvc.namespace}-${pvc.name}-${pvc.annotations['akey']}",
},
},
},
expectErr: true,
},
"simple - PVC namespace annotations not supported for Provision and Delete": {
secretParams: provisionerSecretParams,
pvc: &v1.PersistentVolumeClaim{
ObjectMeta: metav1.ObjectMeta{
Name: "name",
Namespace: "ns",
Annotations: map[string]string{
prefixedProvisionerSecretNamespaceKey: "static-${pv.name}-${pvc.namespace}-${pvc.name}-${pvc.annotations['akey']}",
},
},
},
expectErr: true,
},
}

for k, tc := range testcases {
t.Run(k, func(t *testing.T) {
ref, err := getProvisionSecretReference(tc.secretParams, tc.params, tc.pvName, tc.pvc)
if err != nil {
if tc.expectErr {
return
}
t.Fatalf("Did not expect error but got: %v", err)
} else {
if tc.expectErr {
t.Fatalf("Expected error but got none")
}
}
if !reflect.DeepEqual(ref, tc.expectRef) {
t.Errorf("Expected %v, got %v", tc.expectRef, ref)
}
})
}
}

type provisioningTestcase struct {
volOpts controller.VolumeOptions
notNilSelector bool
Expand Down Expand Up @@ -1840,3 +1911,88 @@ func TestProvisionWithMountOptions(t *testing.T) {
t.Errorf("expected mount options %v; got: %v", expectedOptions, pv.Spec.MountOptions)
}
}

type deleteTestcase struct {
persistentVolume *v1.PersistentVolume

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,
},
"simple - valid": 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{
CSI: &v1.CSIPersistentVolumeSource{
VolumeHandle: "vol-id-1",
},
},
},
},
expectErr: false,
},
}

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()

clientSet := fakeclientset.NewSimpleClientset()

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

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

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 e0ab76e

Please sign in to comment.