Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Added the feature to use secrets of ListSnapshots #252

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ module github.com/kubernetes-csi/external-snapshotter/v2
go 1.12

require (
github.com/container-storage-interface/spec v1.1.0
github.com/container-storage-interface/spec v1.2.0
github.com/golang/mock v1.2.0
github.com/golang/protobuf v1.3.2
github.com/google/go-cmp v0.3.1 // indirect
Expand Down
2 changes: 2 additions & 0 deletions go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,8 @@ github.com/census-instrumentation/opencensus-proto v0.2.1/go.mod h1:f6KPmirojxKA
github.com/client9/misspell v0.3.4/go.mod h1:qj6jICC3Q7zFZvVWo7KLAzC3yx5G7kyvSDkc90ppPyw=
github.com/container-storage-interface/spec v1.1.0 h1:qPsTqtR1VUPvMPeK0UnCZMtXaKGyyLPG8gj/wG6VqMs=
github.com/container-storage-interface/spec v1.1.0/go.mod h1:6URME8mwIBbpVyZV93Ce5St17xBiQJQY67NDsuohiy4=
github.com/container-storage-interface/spec v1.2.0 h1:bD9KIVgaVKKkQ/UbVUY9kCaH/CJbhNxe0eeB4JeJV2s=
github.com/container-storage-interface/spec v1.2.0/go.mod h1:6URME8mwIBbpVyZV93Ce5St17xBiQJQY67NDsuohiy4=
github.com/davecgh/go-spew v0.0.0-20151105211317-5215b55f46b2/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38=
github.com/davecgh/go-spew v1.1.0/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38=
github.com/davecgh/go-spew v1.1.1 h1:vj9j/u1bqnvCEfJOwUhtlOARqs3+rkHYY13jYWTU97c=
Expand Down
2 changes: 1 addition & 1 deletion pkg/common-controller/snapshot_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -573,7 +573,7 @@ func (ctrl *csiSnapshotCommonController) getCreateSnapshotInput(snapshot *crdv1.
contentName := utils.GetSnapshotContentNameForSnapshot(snapshot)

// Resolve snapshotting secret credentials.
snapshotterSecretRef, err := utils.GetSecretReference(class.Parameters, contentName, snapshot)
snapshotterSecretRef, err := utils.GetSecretReference(utils.SnapshotterSecretParams, class.Parameters, contentName, snapshot)
if err != nil {
return nil, nil, "", nil, err
}
Expand Down
2 changes: 1 addition & 1 deletion pkg/sidecar-controller/content_create_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ func TestSyncContent(t *testing.T) {
readyToUse: true,
},
},
expectedListCalls: []listCall{{"sid1-1", true, time.Now(), 1, nil}},
expectedListCalls: []listCall{{"sid1-1", map[string]string{}, true, time.Now(), 1, nil}},
errors: noerrors,
test: testSyncContent,
})
Expand Down
6 changes: 3 additions & 3 deletions pkg/sidecar-controller/csi_handler.go
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ import (
type Handler interface {
CreateSnapshot(content *crdv1.VolumeSnapshotContent, parameters map[string]string, snapshotterCredentials map[string]string) (string, string, time.Time, int64, bool, error)
DeleteSnapshot(content *crdv1.VolumeSnapshotContent, snapshotterCredentials map[string]string) error
GetSnapshotStatus(content *crdv1.VolumeSnapshotContent) (bool, time.Time, int64, error)
GetSnapshotStatus(content *crdv1.VolumeSnapshotContent, snapshotterListCredentials map[string]string) (bool, time.Time, int64, error)
}

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

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

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

csiSnapshotStatus, timestamp, size, err := handler.snapshotter.GetSnapshotStatus(ctx, snapshotHandle)
csiSnapshotStatus, timestamp, size, err := handler.snapshotter.GetSnapshotStatus(ctx, snapshotHandle, snapshotterListCredentials)
if err != nil {
return false, time.Time{}, 0, fmt.Errorf("failed to list snapshot for content %s: %q", content.Name, err)
}
Expand Down
8 changes: 7 additions & 1 deletion pkg/sidecar-controller/framework_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -810,6 +810,7 @@ func emptyDataSecretAnnotations() map[string]string {

type listCall struct {
snapshotID string
secrets map[string]string
// information to return
readyToUse bool
createTime time.Time
Expand Down Expand Up @@ -911,7 +912,7 @@ func (f *fakeSnapshotter) DeleteSnapshot(ctx context.Context, snapshotID string,
return call.err
}

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

if !reflect.DeepEqual(call.secrets, snapshotterListCredentials) {
f.t.Errorf("Wrong CSI List Snapshot call: snapshotID=%s, expected secrets %+v, got %+v", snapshotID, call.secrets, snapshotterListCredentials)
err = fmt.Errorf("unexpected List Snapshot call")
}

if err != nil {
return false, time.Time{}, 0, fmt.Errorf("unexpected call")
}
Expand Down
25 changes: 24 additions & 1 deletion pkg/sidecar-controller/snapshot_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -252,10 +252,33 @@ func (ctrl *csiSnapshotSideCarController) checkandUpdateContentStatusOperation(c
var readyToUse = false
var driverName string
var snapshotID string
var snapshotterListCredentials map[string]string

if content.Spec.Source.SnapshotHandle != nil {
klog.V(5).Infof("checkandUpdateContentStatusOperation: call GetSnapshotStatus for snapshot which is pre-bound to content [%s]", content.Name)
readyToUse, creationTime, size, err = ctrl.handler.GetSnapshotStatus(content)

if content.Spec.VolumeSnapshotClassName != nil {
class, err := ctrl.getSnapshotClass(*content.Spec.VolumeSnapshotClassName)
if err != nil {
klog.Errorf("Failed to get snapshot class %s for snapshot content %s: %v", *content.Spec.VolumeSnapshotClassName, content.Name, err)
return nil, fmt.Errorf("failed to get snapshot class %s for snapshot content %s: %v", *content.Spec.VolumeSnapshotClassName, content.Name, err)
}

snapshotterListSecretRef, err := utils.GetSecretReference(utils.SnapshotterListSecretParams, class.Parameters, content.GetObjectMeta().GetName(), nil)
if err != nil {
klog.Errorf("Failed to get secret reference for snapshot content %s: %v", content.Name, err)
return nil, fmt.Errorf("failed to get secret reference for snapshot content %s: %v", content.Name, err)
}

snapshotterListCredentials, err = utils.GetCredentials(ctrl.client, snapshotterListSecretRef)
if err != nil {
// Continue with deletion, as the secret may have already been deleted.
klog.Errorf("Failed to get credentials for snapshot content %s: %v", content.Name, err)
return nil, fmt.Errorf("failed to get credentials for snapshot content %s: %v", content.Name, err)
}
}

readyToUse, creationTime, size, err = ctrl.handler.GetSnapshotStatus(content, snapshotterListCredentials)
if err != nil {
klog.Errorf("checkandUpdateContentStatusOperation: failed to call get snapshot status to check whether snapshot is ready to use %q", err)
return nil, err
Expand Down
28 changes: 18 additions & 10 deletions pkg/sidecar-controller/snapshot_delete_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,13 @@ var class5Parameters = map[string]string{
utils.AnnDeletionSecretRefNamespace: "default",
}

var class6Parameters = map[string]string{
utils.PrefixedSnapshotterSecretNameKey: "secret",
utils.PrefixedSnapshotterSecretNamespaceKey: "default",
utils.PrefixedSnapshotterListSecretNameKey: "secret",
utils.PrefixedSnapshotterListSecretNamespaceKey: "default",
}

var snapshotClasses = []*crdv1.VolumeSnapshotClass{
{
TypeMeta: metav1.TypeMeta{
Expand Down Expand Up @@ -126,6 +133,7 @@ var snapshotClasses = []*crdv1.VolumeSnapshotClass{
Annotations: map[string]string{utils.IsDefaultSnapshotClassAnnotation: "true"},
},
Driver: mockDriverName,
Parameters: class6Parameters,
DeletionPolicy: crdv1.VolumeSnapshotContentDelete,
},
}
Expand Down Expand Up @@ -156,7 +164,7 @@ func TestDeleteSync(t *testing.T) {
readyToUse: true,
},
},
expectedListCalls: []listCall{{"sid1-1", true, time.Now(), 1, nil}},
expectedListCalls: []listCall{{"sid1-1", map[string]string{}, true, time.Now(), 1, nil}},
expectedDeleteCalls: []deleteCall{{"sid1-1", nil, nil}},
test: testSyncContent,
},
Expand All @@ -178,7 +186,7 @@ func TestDeleteSync(t *testing.T) {
readyToUse: true,
},
},
expectedListCalls: []listCall{{"sid1-2", true, time.Now(), 1, nil}},
expectedListCalls: []listCall{{"sid1-2", map[string]string{}, true, time.Now(), 1, nil}},
expectedDeleteCalls: []deleteCall{{"sid1-2", nil, nil}},
test: testSyncContent,
},
Expand All @@ -201,7 +209,7 @@ func TestDeleteSync(t *testing.T) {
},
expectedDeleteCalls: []deleteCall{{"sid1-3", nil, fmt.Errorf("mock csi driver delete error")}},
expectedEvents: []string{"Warning SnapshotDeleteError"},
expectedListCalls: []listCall{{"sid1-3", true, time.Now(), 1, nil}},
expectedListCalls: []listCall{{"sid1-3", map[string]string{}, true, time.Now(), 1, nil}},
test: testSyncContent,
},
{
Expand All @@ -216,7 +224,7 @@ func TestDeleteSync(t *testing.T) {
name: "1-5 - csi driver delete snapshot returns error, bound finalizer should remain",
initialContents: newContentArrayWithDeletionTimestamp("content1-5", "sid1-5", "snap1-5", "sid1-5", validSecretClass, "", "snap1-5-volumehandle", deletionPolicy, nil, &defaultSize, true, &timeNowMetav1),
expectedContents: newContentArrayWithDeletionTimestamp("content1-5", "sid1-5", "snap1-5", "sid1-5", validSecretClass, "", "snap1-5-volumehandle", deletionPolicy, nil, &defaultSize, true, &timeNowMetav1),
expectedListCalls: []listCall{{"sid1-5", true, time.Now(), 1000, nil}},
expectedListCalls: []listCall{{"sid1-5", map[string]string{}, true, time.Now(), 1000, nil}},
expectedDeleteCalls: []deleteCall{{"sid1-5", nil, errors.New("mock csi driver delete error")}},
expectedEvents: []string{"Warning SnapshotDeleteError"},
errors: noerrors,
Expand All @@ -227,7 +235,7 @@ func TestDeleteSync(t *testing.T) {
name: "1-6 - content is deleted before deleting",
initialContents: newContentArray("content1-6", "sid1-6", "snap1-6", "sid1-6", classGold, "sid1-6", "", deletionPolicy, nil, nil, true),
expectedContents: nocontents,
expectedListCalls: []listCall{{"sid1-6", false, time.Now(), 0, nil}},
expectedListCalls: []listCall{{"sid1-6", nil, false, time.Now(), 0, nil}},
expectedDeleteCalls: []deleteCall{{"sid1-6", map[string]string{"foo": "bar"}, nil}},
expectedEvents: noevents,
errors: noerrors,
Expand All @@ -243,7 +251,7 @@ func TestDeleteSync(t *testing.T) {
initialContents: newContentArrayWithReadyToUse("content1-7", "", "snap1-7", "sid1-7", validSecretClass, "sid1-7", "", deletePolicy, nil, &defaultSize, &True, true),
expectedContents: newContentArrayWithReadyToUse("content1-7", "", "snap1-7", "sid1-7", validSecretClass, "sid1-7", "", deletePolicy, nil, &defaultSize, &True, true),
expectedEvents: noevents,
expectedListCalls: []listCall{{"sid1-7", true, time.Now(), 1000, nil}},
expectedListCalls: []listCall{{"sid1-7", map[string]string{}, true, time.Now(), 1000, nil}},
initialSecrets: []*v1.Secret{secret()},
errors: noerrors,
test: testSyncContent,
Expand All @@ -253,7 +261,7 @@ func TestDeleteSync(t *testing.T) {
initialContents: newContentArrayWithReadyToUse("content1-8", "sid1-8", "none-existed-snapshot", "sid1-8", validSecretClass, "sid1-8", "", retainPolicy, nil, &defaultSize, &True, true),
expectedContents: newContentArrayWithReadyToUse("content1-8", "sid1-8", "none-existed-snapshot", "sid1-8", validSecretClass, "sid1-8", "", retainPolicy, nil, &defaultSize, &True, true),
expectedEvents: noevents,
expectedListCalls: []listCall{{"sid1-8", true, time.Now(), 0, nil}},
expectedListCalls: []listCall{{"sid1-8", map[string]string{}, true, time.Now(), 0, nil}},
errors: noerrors,
test: testSyncContent,
},
Expand All @@ -262,7 +270,7 @@ func TestDeleteSync(t *testing.T) {
initialContents: newContentArrayWithDeletionTimestamp("content1-9", "sid1-9", "snap1-9", "sid1-9", emptySecretClass, "", "snap1-9-volumehandle", deletePolicy, nil, &defaultSize, true, &timeNowMetav1),
expectedContents: newContentArrayWithDeletionTimestamp("content1-9", "sid1-9", "snap1-9", "", emptySecretClass, "", "snap1-9-volumehandle", deletePolicy, nil, &defaultSize, false, &timeNowMetav1),
expectedEvents: noevents,
expectedListCalls: []listCall{{"sid1-9", true, time.Now(), 0, nil}},
expectedListCalls: []listCall{{"sid1-9", map[string]string{}, true, time.Now(), 0, nil}},
errors: noerrors,
initialSecrets: []*v1.Secret{}, // secret does not exist
expectedDeleteCalls: []deleteCall{{"sid1-9", nil, nil}},
Expand All @@ -273,7 +281,7 @@ func TestDeleteSync(t *testing.T) {
initialContents: newContentArrayWithDeletionTimestamp("content1-10", "sid1-10", "snap1-10", "sid1-10", emptySecretClass, "", "snap1-10-volumehandle", retainPolicy, nil, &defaultSize, true, &timeNowMetav1),
expectedContents: newContentArrayWithDeletionTimestamp("content1-10", "sid1-10", "snap1-10", "sid1-10", emptySecretClass, "", "snap1-10-volumehandle", retainPolicy, nil, &defaultSize, false, &timeNowMetav1),
expectedEvents: noevents,
expectedListCalls: []listCall{{"sid1-10", true, time.Now(), 0, nil}},
expectedListCalls: []listCall{{"sid1-10", map[string]string{}, true, time.Now(), 0, nil}},
errors: noerrors,
initialSecrets: []*v1.Secret{},
test: testSyncContent,
Expand All @@ -292,7 +300,7 @@ func TestDeleteSync(t *testing.T) {
initialContents: newContentArrayWithDeletionTimestamp("content1-12", "sid1-12", "snap1-12", "sid1-12", emptySecretClass, "sid1-12", "", retainPolicy, nil, &defaultSize, true, &timeNowMetav1),
expectedContents: newContentArrayWithDeletionTimestamp("content1-12", "sid1-12", "snap1-12", "sid1-12", emptySecretClass, "sid1-12", "", retainPolicy, nil, &defaultSize, false, &timeNowMetav1),
expectedEvents: noevents,
expectedListCalls: []listCall{{"sid1-12", true, time.Now(), 0, nil}},
expectedListCalls: []listCall{{"sid1-12", map[string]string{}, true, time.Now(), 0, nil}},
errors: noerrors,
initialSecrets: []*v1.Secret{},
test: testSyncContent,
Expand Down
5 changes: 3 additions & 2 deletions pkg/snapshotter/snapshotter.go
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ type Snapshotter interface {
DeleteSnapshot(ctx context.Context, snapshotID string, snapshotterCredentials map[string]string) (err error)

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

type snapshot struct {
Expand Down Expand Up @@ -112,7 +112,7 @@ func (s *snapshot) isListSnapshotsSupported(ctx context.Context) (bool, error) {
return false, nil
}

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

client := csi.NewControllerClient(s.conn)
Expand All @@ -127,6 +127,7 @@ func (s *snapshot) GetSnapshotStatus(ctx context.Context, snapshotID string) (bo
}
req := csi.ListSnapshotsRequest{
SnapshotId: snapshotID,
Secrets: snapshotterListCredentials,
}
rsp, err := client.ListSnapshots(ctx, &req)
if err != nil {
Expand Down
41 changes: 30 additions & 11 deletions pkg/snapshotter/snapshotter_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -362,17 +362,24 @@ func TestGetSnapshotStatus(t *testing.T) {
},
}

secret := map[string]string{"foo": "bar"}
secretRequest := &csi.ListSnapshotsRequest{
SnapshotId: defaultID,
Secrets: secret,
}

tests := []struct {
name string
snapshotID string
listSnapshotsSupported bool
input *csi.ListSnapshotsRequest
output *csi.ListSnapshotsResponse
injectError codes.Code
expectError bool
expectReady bool
expectCreateAt time.Time
expectSize int64
name string
snapshotID string
snapshotterListCredentials map[string]string
listSnapshotsSupported bool
input *csi.ListSnapshotsRequest
output *csi.ListSnapshotsResponse
injectError codes.Code
expectError bool
expectReady bool
expectCreateAt time.Time
expectSize int64
}{
{
name: "success",
Expand All @@ -385,6 +392,18 @@ func TestGetSnapshotStatus(t *testing.T) {
expectCreateAt: createTime,
expectSize: size,
},
{
name: "secret",
snapshotID: defaultID,
snapshotterListCredentials: secret,
listSnapshotsSupported: true,
input: secretRequest,
output: defaultResponse,
expectError: false,
expectReady: true,
expectCreateAt: createTime,
expectSize: size,
},
{
name: "ListSnapshots not supported",
snapshotID: defaultID,
Expand Down Expand Up @@ -455,7 +474,7 @@ func TestGetSnapshotStatus(t *testing.T) {
}

s := NewSnapshotter(csiConn)
ready, createTime, size, err := s.GetSnapshotStatus(context.Background(), test.snapshotID)
ready, createTime, size, err := s.GetSnapshotStatus(context.Background(), test.snapshotID, test.snapshotterListCredentials)
if test.expectError && err == nil {
t.Errorf("test %q: Expected error, got none", test.name)
}
Expand Down
Loading