Skip to content

Commit 883842f

Browse files
k8s-ci-robotk8s-ci-robot
authored
Merge pull request #252 from bells17/add-feature-listsnapshots-secrets2
Added the feature to use secrets of ListSnapshots
2 parents af671f0 + 3c2baf4 commit 883842f

File tree

14 files changed

+936
-844
lines changed

14 files changed

+936
-844
lines changed

‎go.mod‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@ module github.com/kubernetes-csi/external-snapshotter/v2
33
go 1.12
44

55
require (
6-
github.com/container-storage-interface/spec v1.1.0
6+
github.com/container-storage-interface/spec v1.2.0
77
github.com/golang/mock v1.2.0
88
github.com/golang/protobuf v1.3.2
99
github.com/google/go-cmp v0.3.1 // indirect

‎go.sum‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,8 @@ github.com/census-instrumentation/opencensus-proto v0.2.1/go.mod h1:f6KPmirojxKA
2828
github.com/client9/misspell v0.3.4/go.mod h1:qj6jICC3Q7zFZvVWo7KLAzC3yx5G7kyvSDkc90ppPyw=
2929
github.com/container-storage-interface/spec v1.1.0 h1:qPsTqtR1VUPvMPeK0UnCZMtXaKGyyLPG8gj/wG6VqMs=
3030
github.com/container-storage-interface/spec v1.1.0/go.mod h1:6URME8mwIBbpVyZV93Ce5St17xBiQJQY67NDsuohiy4=
31+
github.com/container-storage-interface/spec v1.2.0 h1:bD9KIVgaVKKkQ/UbVUY9kCaH/CJbhNxe0eeB4JeJV2s=
32+
github.com/container-storage-interface/spec v1.2.0/go.mod h1:6URME8mwIBbpVyZV93Ce5St17xBiQJQY67NDsuohiy4=
3133
github.com/davecgh/go-spew v0.0.0-20151105211317-5215b55f46b2/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38=
3234
github.com/davecgh/go-spew v1.1.0/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38=
3335
github.com/davecgh/go-spew v1.1.1 h1:vj9j/u1bqnvCEfJOwUhtlOARqs3+rkHYY13jYWTU97c=

‎pkg/common-controller/snapshot_controller.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -573,7 +573,7 @@ func (ctrl *csiSnapshotCommonController) getCreateSnapshotInput(snapshot *crdv1.
573573
contentName := utils.GetSnapshotContentNameForSnapshot(snapshot)
574574

575575
// Resolve snapshotting secret credentials.
576-
snapshotterSecretRef, err := utils.GetSecretReference(class.Parameters, contentName, snapshot)
576+
snapshotterSecretRef, err := utils.GetSecretReference(utils.SnapshotterSecretParams, class.Parameters, contentName, snapshot)
577577
if err != nil {
578578
return nil, nil, "", nil, err
579579
}

‎pkg/sidecar-controller/content_create_test.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -39,7 +39,7 @@ func TestSyncContent(t *testing.T) {
3939
readyToUse: true,
4040
},
4141
},
42-
expectedListCalls: []listCall{{"sid1-1", true, time.Now(), 1, nil}},
42+
expectedListCalls: []listCall{{"sid1-1", map[string]string{}, true, time.Now(), 1, nil}},
4343
errors: noerrors,
4444
test: testSyncContent,
4545
})

‎pkg/sidecar-controller/csi_handler.go‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -31,7 +31,7 @@ import (
3131
type Handler interface {
3232
CreateSnapshot(content *crdv1.VolumeSnapshotContent, parameters map[string]string, snapshotterCredentials map[string]string) (string, string, time.Time, int64, bool, error)
3333
DeleteSnapshot(content *crdv1.VolumeSnapshotContent, snapshotterCredentials map[string]string) error
34-
GetSnapshotStatus(content *crdv1.VolumeSnapshotContent) (bool, time.Time, int64, error)
34+
GetSnapshotStatus(content *crdv1.VolumeSnapshotContent, snapshotterListCredentials map[string]string) (bool, time.Time, int64, error)
3535
}
3636

3737
// csiHandler is a handler that calls CSI to create/delete volume snapshot.
@@ -103,7 +103,7 @@ func (handler *csiHandler) DeleteSnapshot(content *crdv1.VolumeSnapshotContent,
103103
return nil
104104
}
105105

106-
func (handler *csiHandler) GetSnapshotStatus(content *crdv1.VolumeSnapshotContent) (bool, time.Time, int64, error) {
106+
func (handler *csiHandler) GetSnapshotStatus(content *crdv1.VolumeSnapshotContent, snapshotterListCredentials map[string]string) (bool, time.Time, int64, error) {
107107
ctx, cancel := context.WithTimeout(context.Background(), handler.timeout)
108108
defer cancel()
109109

@@ -117,7 +117,7 @@ func (handler *csiHandler) GetSnapshotStatus(content *crdv1.VolumeSnapshotConten
117117
return false, time.Time{}, 0, fmt.Errorf("failed to list snapshot for content %s: snapshotHandle is missing", content.Name)
118118
}
119119

120-
csiSnapshotStatus, timestamp, size, err := handler.snapshotter.GetSnapshotStatus(ctx, snapshotHandle)
120+
csiSnapshotStatus, timestamp, size, err := handler.snapshotter.GetSnapshotStatus(ctx, snapshotHandle, snapshotterListCredentials)
121121
if err != nil {
122122
return false, time.Time{}, 0, fmt.Errorf("failed to list snapshot for content %s: %q", content.Name, err)
123123
}

‎pkg/sidecar-controller/framework_test.go‎

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -810,6 +810,7 @@ func emptyDataSecretAnnotations() map[string]string {
810810

811811
type listCall struct {
812812
snapshotID string
813+
secrets map[string]string
813814
// information to return
814815
readyToUse bool
815816
createTime time.Time
@@ -911,7 +912,7 @@ func (f *fakeSnapshotter) DeleteSnapshot(ctx context.Context, snapshotID string,
911912
return call.err
912913
}
913914

914-
func (f *fakeSnapshotter) GetSnapshotStatus(ctx context.Context, snapshotID string) (bool, time.Time, int64, error) {
915+
func (f *fakeSnapshotter) GetSnapshotStatus(ctx context.Context, snapshotID string, snapshotterListCredentials map[string]string) (bool, time.Time, int64, error) {
915916
if f.listCallCounter >= len(f.listCalls) {
916917
f.t.Errorf("Unexpected CSI list Snapshot call: snapshotID=%s, index: %d, calls: %+v", snapshotID, f.createCallCounter, f.createCalls)
917918
return false, time.Time{}, 0, fmt.Errorf("unexpected call")
@@ -925,6 +926,11 @@ func (f *fakeSnapshotter) GetSnapshotStatus(ctx context.Context, snapshotID stri
925926
err = fmt.Errorf("unexpected List snapshot call")
926927
}
927928

929+
if !reflect.DeepEqual(call.secrets, snapshotterListCredentials) {
930+
f.t.Errorf("Wrong CSI List Snapshot call: snapshotID=%s, expected secrets %+v, got %+v", snapshotID, call.secrets, snapshotterListCredentials)
931+
err = fmt.Errorf("unexpected List Snapshot call")
932+
}
933+
928934
if err != nil {
929935
return false, time.Time{}, 0, fmt.Errorf("unexpected call")
930936
}

‎pkg/sidecar-controller/snapshot_controller.go‎

Lines changed: 24 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -252,10 +252,33 @@ func (ctrl *csiSnapshotSideCarController) checkandUpdateContentStatusOperation(c
252252
var readyToUse = false
253253
var driverName string
254254
var snapshotID string
255+
var snapshotterListCredentials map[string]string
255256

256257
if content.Spec.Source.SnapshotHandle != nil {
257258
klog.V(5).Infof("checkandUpdateContentStatusOperation: call GetSnapshotStatus for snapshot which is pre-bound to content [%s]", content.Name)
258-
readyToUse, creationTime, size, err = ctrl.handler.GetSnapshotStatus(content)
259+
260+
if content.Spec.VolumeSnapshotClassName != nil {
261+
class, err := ctrl.getSnapshotClass(*content.Spec.VolumeSnapshotClassName)
262+
if err != nil {
263+
klog.Errorf("Failed to get snapshot class %s for snapshot content %s: %v", *content.Spec.VolumeSnapshotClassName, content.Name, err)
264+
return nil, fmt.Errorf("failed to get snapshot class %s for snapshot content %s: %v", *content.Spec.VolumeSnapshotClassName, content.Name, err)
265+
}
266+
267+
snapshotterListSecretRef, err := utils.GetSecretReference(utils.SnapshotterListSecretParams, class.Parameters, content.GetObjectMeta().GetName(), nil)
268+
if err != nil {
269+
klog.Errorf("Failed to get secret reference for snapshot content %s: %v", content.Name, err)
270+
return nil, fmt.Errorf("failed to get secret reference for snapshot content %s: %v", content.Name, err)
271+
}
272+
273+
snapshotterListCredentials, err = utils.GetCredentials(ctrl.client, snapshotterListSecretRef)
274+
if err != nil {
275+
// Continue with deletion, as the secret may have already been deleted.
276+
klog.Errorf("Failed to get credentials for snapshot content %s: %v", content.Name, err)
277+
return nil, fmt.Errorf("failed to get credentials for snapshot content %s: %v", content.Name, err)
278+
}
279+
}
280+
281+
readyToUse, creationTime, size, err = ctrl.handler.GetSnapshotStatus(content, snapshotterListCredentials)
259282
if err != nil {
260283
klog.Errorf("checkandUpdateContentStatusOperation: failed to call get snapshot status to check whether snapshot is ready to use %q", err)
261284
return nil, err

‎pkg/sidecar-controller/snapshot_delete_test.go‎

Lines changed: 18 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -61,6 +61,13 @@ var class5Parameters = map[string]string{
6161
utils.AnnDeletionSecretRefNamespace: "default",
6262
}
6363

64+
var class6Parameters = map[string]string{
65+
utils.PrefixedSnapshotterSecretNameKey: "secret",
66+
utils.PrefixedSnapshotterSecretNamespaceKey: "default",
67+
utils.PrefixedSnapshotterListSecretNameKey: "secret",
68+
utils.PrefixedSnapshotterListSecretNamespaceKey: "default",
69+
}
70+
6471
var snapshotClasses = []*crdv1.VolumeSnapshotClass{
6572
{
6673
TypeMeta: metav1.TypeMeta{
@@ -126,6 +133,7 @@ var snapshotClasses = []*crdv1.VolumeSnapshotClass{
126133
Annotations: map[string]string{utils.IsDefaultSnapshotClassAnnotation: "true"},
127134
},
128135
Driver: mockDriverName,
136+
Parameters: class6Parameters,
129137
DeletionPolicy: crdv1.VolumeSnapshotContentDelete,
130138
},
131139
}
@@ -156,7 +164,7 @@ func TestDeleteSync(t *testing.T) {
156164
readyToUse: true,
157165
},
158166
},
159-
expectedListCalls: []listCall{{"sid1-1", true, time.Now(), 1, nil}},
167+
expectedListCalls: []listCall{{"sid1-1", map[string]string{}, true, time.Now(), 1, nil}},
160168
expectedDeleteCalls: []deleteCall{{"sid1-1", nil, nil}},
161169
test: testSyncContent,
162170
},
@@ -178,7 +186,7 @@ func TestDeleteSync(t *testing.T) {
178186
readyToUse: true,
179187
},
180188
},
181-
expectedListCalls: []listCall{{"sid1-2", true, time.Now(), 1, nil}},
189+
expectedListCalls: []listCall{{"sid1-2", map[string]string{}, true, time.Now(), 1, nil}},
182190
expectedDeleteCalls: []deleteCall{{"sid1-2", nil, nil}},
183191
test: testSyncContent,
184192
},
@@ -201,7 +209,7 @@ func TestDeleteSync(t *testing.T) {
201209
},
202210
expectedDeleteCalls: []deleteCall{{"sid1-3", nil, fmt.Errorf("mock csi driver delete error")}},
203211
expectedEvents: []string{"Warning SnapshotDeleteError"},
204-
expectedListCalls: []listCall{{"sid1-3", true, time.Now(), 1, nil}},
212+
expectedListCalls: []listCall{{"sid1-3", map[string]string{}, true, time.Now(), 1, nil}},
205213
test: testSyncContent,
206214
},
207215
{
@@ -216,7 +224,7 @@ func TestDeleteSync(t *testing.T) {
216224
name: "1-5 - csi driver delete snapshot returns error, bound finalizer should remain",
217225
initialContents: newContentArrayWithDeletionTimestamp("content1-5", "sid1-5", "snap1-5", "sid1-5", validSecretClass, "", "snap1-5-volumehandle", deletionPolicy, nil, &defaultSize, true, &timeNowMetav1),
218226
expectedContents: newContentArrayWithDeletionTimestamp("content1-5", "sid1-5", "snap1-5", "sid1-5", validSecretClass, "", "snap1-5-volumehandle", deletionPolicy, nil, &defaultSize, true, &timeNowMetav1),
219-
expectedListCalls: []listCall{{"sid1-5", true, time.Now(), 1000, nil}},
227+
expectedListCalls: []listCall{{"sid1-5", map[string]string{}, true, time.Now(), 1000, nil}},
220228
expectedDeleteCalls: []deleteCall{{"sid1-5", nil, errors.New("mock csi driver delete error")}},
221229
expectedEvents: []string{"Warning SnapshotDeleteError"},
222230
errors: noerrors,
@@ -227,7 +235,7 @@ func TestDeleteSync(t *testing.T) {
227235
name: "1-6 - content is deleted before deleting",
228236
initialContents: newContentArray("content1-6", "sid1-6", "snap1-6", "sid1-6", classGold, "sid1-6", "", deletionPolicy, nil, nil, true),
229237
expectedContents: nocontents,
230-
expectedListCalls: []listCall{{"sid1-6", false, time.Now(), 0, nil}},
238+
expectedListCalls: []listCall{{"sid1-6", nil, false, time.Now(), 0, nil}},
231239
expectedDeleteCalls: []deleteCall{{"sid1-6", map[string]string{"foo": "bar"}, nil}},
232240
expectedEvents: noevents,
233241
errors: noerrors,
@@ -243,7 +251,7 @@ func TestDeleteSync(t *testing.T) {
243251
initialContents: newContentArrayWithReadyToUse("content1-7", "", "snap1-7", "sid1-7", validSecretClass, "sid1-7", "", deletePolicy, nil, &defaultSize, &True, true),
244252
expectedContents: newContentArrayWithReadyToUse("content1-7", "", "snap1-7", "sid1-7", validSecretClass, "sid1-7", "", deletePolicy, nil, &defaultSize, &True, true),
245253
expectedEvents: noevents,
246-
expectedListCalls: []listCall{{"sid1-7", true, time.Now(), 1000, nil}},
254+
expectedListCalls: []listCall{{"sid1-7", map[string]string{}, true, time.Now(), 1000, nil}},
247255
initialSecrets: []*v1.Secret{secret()},
248256
errors: noerrors,
249257
test: testSyncContent,
@@ -253,7 +261,7 @@ func TestDeleteSync(t *testing.T) {
253261
initialContents: newContentArrayWithReadyToUse("content1-8", "sid1-8", "none-existed-snapshot", "sid1-8", validSecretClass, "sid1-8", "", retainPolicy, nil, &defaultSize, &True, true),
254262
expectedContents: newContentArrayWithReadyToUse("content1-8", "sid1-8", "none-existed-snapshot", "sid1-8", validSecretClass, "sid1-8", "", retainPolicy, nil, &defaultSize, &True, true),
255263
expectedEvents: noevents,
256-
expectedListCalls: []listCall{{"sid1-8", true, time.Now(), 0, nil}},
264+
expectedListCalls: []listCall{{"sid1-8", map[string]string{}, true, time.Now(), 0, nil}},
257265
errors: noerrors,
258266
test: testSyncContent,
259267
},
@@ -262,7 +270,7 @@ func TestDeleteSync(t *testing.T) {
262270
initialContents: newContentArrayWithDeletionTimestamp("content1-9", "sid1-9", "snap1-9", "sid1-9", emptySecretClass, "", "snap1-9-volumehandle", deletePolicy, nil, &defaultSize, true, &timeNowMetav1),
263271
expectedContents: newContentArrayWithDeletionTimestamp("content1-9", "sid1-9", "snap1-9", "", emptySecretClass, "", "snap1-9-volumehandle", deletePolicy, nil, &defaultSize, false, &timeNowMetav1),
264272
expectedEvents: noevents,
265-
expectedListCalls: []listCall{{"sid1-9", true, time.Now(), 0, nil}},
273+
expectedListCalls: []listCall{{"sid1-9", map[string]string{}, true, time.Now(), 0, nil}},
266274
errors: noerrors,
267275
initialSecrets: []*v1.Secret{}, // secret does not exist
268276
expectedDeleteCalls: []deleteCall{{"sid1-9", nil, nil}},
@@ -273,7 +281,7 @@ func TestDeleteSync(t *testing.T) {
273281
initialContents: newContentArrayWithDeletionTimestamp("content1-10", "sid1-10", "snap1-10", "sid1-10", emptySecretClass, "", "snap1-10-volumehandle", retainPolicy, nil, &defaultSize, true, &timeNowMetav1),
274282
expectedContents: newContentArrayWithDeletionTimestamp("content1-10", "sid1-10", "snap1-10", "sid1-10", emptySecretClass, "", "snap1-10-volumehandle", retainPolicy, nil, &defaultSize, false, &timeNowMetav1),
275283
expectedEvents: noevents,
276-
expectedListCalls: []listCall{{"sid1-10", true, time.Now(), 0, nil}},
284+
expectedListCalls: []listCall{{"sid1-10", map[string]string{}, true, time.Now(), 0, nil}},
277285
errors: noerrors,
278286
initialSecrets: []*v1.Secret{},
279287
test: testSyncContent,
@@ -292,7 +300,7 @@ func TestDeleteSync(t *testing.T) {
292300
initialContents: newContentArrayWithDeletionTimestamp("content1-12", "sid1-12", "snap1-12", "sid1-12", emptySecretClass, "sid1-12", "", retainPolicy, nil, &defaultSize, true, &timeNowMetav1),
293301
expectedContents: newContentArrayWithDeletionTimestamp("content1-12", "sid1-12", "snap1-12", "sid1-12", emptySecretClass, "sid1-12", "", retainPolicy, nil, &defaultSize, false, &timeNowMetav1),
294302
expectedEvents: noevents,
295-
expectedListCalls: []listCall{{"sid1-12", true, time.Now(), 0, nil}},
303+
expectedListCalls: []listCall{{"sid1-12", map[string]string{}, true, time.Now(), 0, nil}},
296304
errors: noerrors,
297305
initialSecrets: []*v1.Secret{},
298306
test: testSyncContent,

‎pkg/snapshotter/snapshotter.go‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -39,7 +39,7 @@ type Snapshotter interface {
3939
DeleteSnapshot(ctx context.Context, snapshotID string, snapshotterCredentials map[string]string) (err error)
4040

4141
// GetSnapshotStatus returns if a snapshot is ready to use, creation time, and restore size.
42-
GetSnapshotStatus(ctx context.Context, snapshotID string) (bool, time.Time, int64, error)
42+
GetSnapshotStatus(ctx context.Context, snapshotID string, snapshotterListCredentials map[string]string) (bool, time.Time, int64, error)
4343
}
4444

4545
type snapshot struct {
@@ -112,7 +112,7 @@ func (s *snapshot) isListSnapshotsSupported(ctx context.Context) (bool, error) {
112112
return false, nil
113113
}
114114

115-
func (s *snapshot) GetSnapshotStatus(ctx context.Context, snapshotID string) (bool, time.Time, int64, error) {
115+
func (s *snapshot) GetSnapshotStatus(ctx context.Context, snapshotID string, snapshotterListCredentials map[string]string) (bool, time.Time, int64, error) {
116116
klog.V(5).Infof("GetSnapshotStatus: %s", snapshotID)
117117

118118
client := csi.NewControllerClient(s.conn)
@@ -127,6 +127,7 @@ func (s *snapshot) GetSnapshotStatus(ctx context.Context, snapshotID string) (bo
127127
}
128128
req := csi.ListSnapshotsRequest{
129129
SnapshotId: snapshotID,
130+
Secrets: snapshotterListCredentials,
130131
}
131132
rsp, err := client.ListSnapshots(ctx, &req)
132133
if err != nil {

‎pkg/snapshotter/snapshotter_test.go‎

Lines changed: 30 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -362,17 +362,24 @@ func TestGetSnapshotStatus(t *testing.T) {
362362
},
363363
}
364364

365+
secret := map[string]string{"foo": "bar"}
366+
secretRequest := &csi.ListSnapshotsRequest{
367+
SnapshotId: defaultID,
368+
Secrets: secret,
369+
}
370+
365371
tests := []struct {
366-
name string
367-
snapshotID string
368-
listSnapshotsSupported bool
369-
input *csi.ListSnapshotsRequest
370-
output *csi.ListSnapshotsResponse
371-
injectError codes.Code
372-
expectError bool
373-
expectReady bool
374-
expectCreateAt time.Time
375-
expectSize int64
372+
name string
373+
snapshotID string
374+
snapshotterListCredentials map[string]string
375+
listSnapshotsSupported bool
376+
input *csi.ListSnapshotsRequest
377+
output *csi.ListSnapshotsResponse
378+
injectError codes.Code
379+
expectError bool
380+
expectReady bool
381+
expectCreateAt time.Time
382+
expectSize int64
376383
}{
377384
{
378385
name: "success",
@@ -385,6 +392,18 @@ func TestGetSnapshotStatus(t *testing.T) {
385392
expectCreateAt: createTime,
386393
expectSize: size,
387394
},
395+
{
396+
name: "secret",
397+
snapshotID: defaultID,
398+
snapshotterListCredentials: secret,
399+
listSnapshotsSupported: true,
400+
input: secretRequest,
401+
output: defaultResponse,
402+
expectError: false,
403+
expectReady: true,
404+
expectCreateAt: createTime,
405+
expectSize: size,
406+
},
388407
{
389408
name: "ListSnapshots not supported",
390409
snapshotID: defaultID,
@@ -455,7 +474,7 @@ func TestGetSnapshotStatus(t *testing.T) {
455474
}
456475

457476
s := NewSnapshotter(csiConn)
458-
ready, createTime, size, err := s.GetSnapshotStatus(context.Background(), test.snapshotID)
477+
ready, createTime, size, err := s.GetSnapshotStatus(context.Background(), test.snapshotID, test.snapshotterListCredentials)
459478
if test.expectError && err == nil {
460479
t.Errorf("test %q: Expected error, got none", test.name)
461480
}

0 commit comments

Comments
 (0)