diff --git a/internal/rbd/controllerserver.go b/internal/rbd/controllerserver.go index 05a8f4bab..62ed50197 100644 --- a/internal/rbd/controllerserver.go +++ b/internal/rbd/controllerserver.go @@ -957,12 +957,30 @@ func (cs *ControllerServer) DeleteVolume( return &csi.DeleteVolumeResponse{}, nil } - rbdVol, err := GenVolFromVolID(ctx, volumeID, cr, req.GetSecrets()) - defer func() { - if rbdVol != nil { - rbdVol.Destroy(ctx) + // secrets can be different for volumes created with in-tree drivers + secrets := req.GetSecrets() + if util.IsMigrationSecret(secrets) { + secrets, err = util.ParseAndSetSecretMapFromMigSecret(secrets) + if err != nil { + return nil, status.Error(codes.Internal, err.Error()) } - }() + } + + mgr := NewManager(cs.Driver.GetInstanceID(), nil, secrets) + defer mgr.Destroy(ctx) + + var rbdVol *rbdVolume + vol, err := mgr.GetVolumeByID(ctx, volumeID) + if vol != nil { + defer vol.Destroy(ctx) + + var ok bool + rbdVol, ok = vol.(*rbdVolume) // FIXME: temporary cast until rbdVolume is cleaned up + if !ok { + // this can never happen, mgr.GetVolumeByID() returns a *rbdVolume on success + log.ErrorLog(ctx, "failed to cast %q of type %T to %T", vol, vol, rbdVol) + } + } if err != nil { return cs.checkErrAndUndoReserve(ctx, err, volumeID, rbdVol, cr) } diff --git a/internal/util/credentials.go b/internal/util/credentials.go index 091efe3e0..90b4ed4b5 100644 --- a/internal/util/credentials.go +++ b/internal/util/credentials.go @@ -135,7 +135,7 @@ func GetMonValFromSecret(secrets map[string]string) (string, error) { func ParseAndSetSecretMapFromMigSecret(secretmap map[string]string) (map[string]string, error) { newSecretMap := make(map[string]string) // parse and set userKey - if !isMigrationSecret(secretmap) { + if !IsMigrationSecret(secretmap) { return nil, errors.New("passed secret map does not contain user key or it is nil") } newSecretMap[credUserKey] = secretmap[migUserKey] @@ -148,11 +148,11 @@ func ParseAndSetSecretMapFromMigSecret(secretmap map[string]string) (map[string] return newSecretMap, nil } -// isMigrationSecret validates if the passed in secretmap is a secret +// IsMigrationSecret validates if the passed in secretmap is a secret // of a migration volume request. The migration secret carry a field // called `key` which is the equivalent of `userKey` which is what we // check here for identifying the secret. -func isMigrationSecret(secrets map[string]string) bool { +func IsMigrationSecret(secrets map[string]string) bool { // the below 'nil' check is an extra measure as the request validators like // ValidateNodeStageVolumeRequest() already does the nil check, however considering // this function can be called independently with a map of secret values @@ -166,7 +166,7 @@ func isMigrationSecret(secrets map[string]string) bool { // secret. If it is not a migration it will continue the attempt to create credentials from it // without parsing the secret. This function returns credentials and error. func NewUserCredentialsWithMigration(secrets map[string]string) (*Credentials, error) { - if isMigrationSecret(secrets) { + if IsMigrationSecret(secrets) { migSecret, err := ParseAndSetSecretMapFromMigSecret(secrets) if err != nil { return nil, err diff --git a/internal/util/credentials_test.go b/internal/util/credentials_test.go index 7054bf700..e15bcfa02 100644 --- a/internal/util/credentials_test.go +++ b/internal/util/credentials_test.go @@ -41,8 +41,8 @@ func TestIsMigrationSecret(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { t.Parallel() - if got := isMigrationSecret(tt.vc); got != tt.want { - t.Errorf("isMigrationSecret() = %v, want %v", got, tt.want) + if got := IsMigrationSecret(tt.vc); got != tt.want { + t.Errorf("IsMigrationSecret() = %v, want %v", got, tt.want) } }) }