Modify RBD plugin to use a single ID and move the id and key into the secret

RBD plugin needs only a single ID to manage images and operations against a pool, mentioned in the storage class. The current scheme of 2 IDs is hence not needed and removed in this commit. Further, unlike CephFS plugin, the RBD plugin splits the user id and the key into the storage class and the secret respectively. Also the parameter name for the key in the secret is noted in the storageclass making it a variant and hampers usability/comprehension. This is also fixed by moving the id and the key to the secret and not retaining the same in the storage class, like CephFS. Fixes #270 Testing done: - Basic PVC creation and mounting Signed-off-by: ShyamsundarR <srangana@redhat.com>
2025-06-13 02:33:34 +00:00 · 2019-06-01 17:26:42 -04:00
parent 22ff5c0911
commit c5762b6b5c
25 changed files with 284 additions and 402 deletions
--- a/pkg/util/cephcmds.go
+++ b/pkg/util/cephcmds.go
@ -55,15 +55,15 @@ type cephStoragePoolSummary struct {

 // GetPoolID searches a list of pools in a cluster and returns the ID of the pool that matches
 // the passed in poolName parameter
-func GetPoolID(monitors, adminID, key, poolName string) (int64, error) {
+func GetPoolID(monitors string, cr *Credentials, poolName string) (int64, error) {
 	// ceph <options> -f json osd lspools
 	// JSON out: [{"poolnum":<int64>,"poolname":<string>}]

 	stdout, _, err := ExecCommand(
 		"ceph",
 		"-m", monitors,
-		"--id", adminID,
-		"--key="+key,
+		"--id", cr.ID,
+		"--key="+cr.Key,
 		"-c", CephConfigPath,
 		"-f", "json",
 		"osd", "lspools")
@ -90,15 +90,15 @@ func GetPoolID(monitors, adminID, key, poolName string) (int64, error) {

 // GetPoolName lists all pools in a ceph cluster, and matches the pool whose pool ID is equal to
 // the requested poolID parameter
-func GetPoolName(monitors, adminID, key string, poolID int64) (string, error) {
+func GetPoolName(monitors string, cr *Credentials, poolID int64) (string, error) {
 	// ceph <options> -f json osd lspools
 	// [{"poolnum":1,"poolname":"replicapool"}]

 	stdout, _, err := ExecCommand(
 		"ceph",
 		"-m", monitors,
-		"--id", adminID,
-		"--key="+key,
+		"--id", cr.ID,
+		"--key="+cr.Key,
 		"-c", CephConfigPath,
 		"-f", "json",
 		"osd", "lspools")
@ -124,12 +124,12 @@ func GetPoolName(monitors, adminID, key string, poolID int64) (string, error) {
 }

 // SetOMapKeyValue sets the given key and value into the provided Ceph omap name
-func SetOMapKeyValue(monitors, adminID, key, poolName, namespace, oMapName, oMapKey, keyValue string) error {
+func SetOMapKeyValue(monitors string, cr *Credentials, poolName, namespace, oMapName, oMapKey, keyValue string) error {
 	// Command: "rados <options> setomapval oMapName oMapKey keyValue"
 	args := []string{
 		"-m", monitors,
-		"--id", adminID,
-		"--key=" + key,
+		"--id", cr.ID,
+		"--key=" + cr.Key,
 		"-c", CephConfigPath,
 		"-p", poolName,
 		"setomapval", oMapName, oMapKey, keyValue,
@ -150,7 +150,7 @@ func SetOMapKeyValue(monitors, adminID, key, poolName, namespace, oMapName, oMap
 }

 // GetOMapValue gets the value for the given key from the named omap
-func GetOMapValue(monitors, adminID, key, poolName, namespace, oMapName, oMapKey string) (string, error) {
+func GetOMapValue(monitors string, cr *Credentials, poolName, namespace, oMapName, oMapKey string) (string, error) {
 	// Command: "rados <options> getomapval oMapName oMapKey <outfile>"
 	// No such key: replicapool/csi.volumes.directory.default/csi.volname
 	tmpFile, err := ioutil.TempFile("", "omap-get-")
@ -163,8 +163,8 @@ func GetOMapValue(monitors, adminID, key, poolName, namespace, oMapName, oMapKey

 	args := []string{
 		"-m", monitors,
-		"--id", adminID,
-		"--key=" + key,
+		"--id", cr.ID,
+		"--key=" + cr.Key,
 		"-c", CephConfigPath,
 		"-p", poolName,
 		"getomapval", oMapName, oMapKey, tmpFile.Name(),
@ -201,12 +201,12 @@ func GetOMapValue(monitors, adminID, key, poolName, namespace, oMapName, oMapKey
 }

 // RemoveOMapKey removes the omap key from the given omap name
-func RemoveOMapKey(monitors, adminID, key, poolName, namespace, oMapName, oMapKey string) error {
+func RemoveOMapKey(monitors string, cr *Credentials, poolName, namespace, oMapName, oMapKey string) error {
 	// Command: "rados <options> rmomapkey oMapName oMapKey"
 	args := []string{
 		"-m", monitors,
-		"--id", adminID,
-		"--key=" + key,
+		"--id", cr.ID,
+		"--key=" + cr.Key,
 		"-c", CephConfigPath,
 		"-p", poolName,
 		"rmomapkey", oMapName, oMapKey,
@ -229,12 +229,12 @@ func RemoveOMapKey(monitors, adminID, key, poolName, namespace, oMapName, oMapKe

 // CreateObject creates the object name passed in and returns ErrObjectExists if the provided object
 // is already present in rados
-func CreateObject(monitors, adminID, key, poolName, namespace, objectName string) error {
+func CreateObject(monitors string, cr *Credentials, poolName, namespace, objectName string) error {
 	// Command: "rados <options> create objectName"
 	args := []string{
 		"-m", monitors,
-		"--id", adminID,
-		"--key=" + key,
+		"--id", cr.ID,
+		"--key=" + cr.Key,
 		"-c", CephConfigPath,
 		"-p", poolName,
 		"create", objectName,
@ -259,12 +259,12 @@ func CreateObject(monitors, adminID, key, poolName, namespace, objectName string

 // RemoveObject removes the entire omap name passed in and returns ErrObjectNotFound is provided omap
 // is not found in rados
-func RemoveObject(monitors, adminID, key, poolName, namespace, oMapName string) error {
+func RemoveObject(monitors string, cr *Credentials, poolName, namespace, oMapName string) error {
 	// Command: "rados <options> rm oMapName"
 	args := []string{
 		"-m", monitors,
-		"--id", adminID,
-		"--key=" + key,
+		"--id", cr.ID,
+		"--key=" + cr.Key,
 		"-c", CephConfigPath,
 		"-p", poolName,
 		"rm", oMapName,
--- a/pkg/util/credentials.go
+++ b/pkg/util/credentials.go
@ -0,0 +1,66 @@
+/*
+Copyright 2018 The Ceph-CSI Authors.
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+package util
+
+import (
+	"fmt"
+)
+
+const (
+	credUserID   = "userID"
+	credUserKey  = "userKey"
+	credAdminID  = "adminID"
+	credAdminKey = "adminKey"
+	credMonitors = "monitors"
+)
+
+type Credentials struct {
+	ID  string
+	Key string
+}
+
+func getCredentials(idField, keyField string, secrets map[string]string) (*Credentials, error) {
+	var (
+		c  = &Credentials{}
+		ok bool
+	)
+
+	if c.ID, ok = secrets[idField]; !ok {
+		return nil, fmt.Errorf("missing ID field '%s' in secrets", idField)
+	}
+
+	if c.Key, ok = secrets[keyField]; !ok {
+		return nil, fmt.Errorf("missing key field '%s' in secrets", keyField)
+	}
+
+	return c, nil
+}
+
+func GetUserCredentials(secrets map[string]string) (*Credentials, error) {
+	return getCredentials(credUserID, credUserKey, secrets)
+}
+
+func GetAdminCredentials(secrets map[string]string) (*Credentials, error) {
+	return getCredentials(credAdminID, credAdminKey, secrets)
+}
+
+func GetMonValFromSecret(secrets map[string]string) (string, error) {
+	if mons, ok := secrets[credMonitors]; ok {
+		return mons, nil
+	}
+	return "", fmt.Errorf("missing %q", credMonitors)
+}
--- a/pkg/util/util.go
+++ b/pkg/util/util.go
@ -94,8 +94,8 @@ func ValidateDriverName(driverName string) error {

 // GenerateVolID generates a volume ID based on passed in parameters and version, to be returned
 // to the CO system
-func GenerateVolID(monitors, id, key, pool, clusterID, objUUID string, volIDVersion uint16) (string, error) {
-	poolID, err := GetPoolID(monitors, id, key, pool)
+func GenerateVolID(monitors string, cr *Credentials, pool, clusterID, objUUID string, volIDVersion uint16) (string, error) {
+	poolID, err := GetPoolID(monitors, cr, pool)
 	if err != nil {
 		return "", err
 	}
--- a/pkg/util/voljournal.go
+++ b/pkg/util/voljournal.go
@ -175,7 +175,7 @@ Return values:
 	there was no reservation found
 	- error: non-nil in case of any errors
 */
-func (cj *CSIJournal) CheckReservation(monitors, id, key, pool, reqName, parentName string) (string, error) {
+func (cj *CSIJournal) CheckReservation(monitors string, cr *Credentials, pool, reqName, parentName string) (string, error) {
 	var snapSource bool

 	if parentName != "" {
@ -187,7 +187,7 @@ func (cj *CSIJournal) CheckReservation(monitors, id, key, pool, reqName, parentN
 	}

 	// check if request name is already part of the directory omap
-	objUUID, err := GetOMapValue(monitors, id, key, pool, cj.namespace, cj.csiDirectory,
+	objUUID, err := GetOMapValue(monitors, cr, pool, cj.namespace, cj.csiDirectory,
 		cj.csiNameKeyPrefix+reqName)
 	if err != nil {
 		// error should specifically be not found, for volume to be absent, any other error
@ -198,13 +198,13 @@ func (cj *CSIJournal) CheckReservation(monitors, id, key, pool, reqName, parentN
 		return "", err
 	}

-	savedReqName, savedReqParentName, err := cj.GetObjectUUIDData(monitors, id, key, pool,
+	savedReqName, savedReqParentName, err := cj.GetObjectUUIDData(monitors, cr, pool,
 		objUUID, snapSource)
 	if err != nil {
 		// error should specifically be not found, for image to be absent, any other error
 		// is not conclusive, and we should not proceed
 		if _, ok := err.(ErrKeyNotFound); ok {
-			err = cj.UndoReservation(monitors, id, key, pool, cj.namingPrefix+objUUID, reqName)
+			err = cj.UndoReservation(monitors, cr, pool, cj.namingPrefix+objUUID, reqName)
 		}
 		return "", err
 	}
@ -243,11 +243,11 @@ prior to cleaning up the reservation
 NOTE: As the function manipulates omaps, it should be called with a lock against the request name
 held, to prevent parallel operations from modifying the state of the omaps for this request name.
 */
-func (cj *CSIJournal) UndoReservation(monitors, id, key, pool, volName, reqName string) error {
+func (cj *CSIJournal) UndoReservation(monitors string, cr *Credentials, pool, volName, reqName string) error {
 	// delete volume UUID omap (first, inverse of create order)
 	// TODO: Check cases where volName can be empty, and we need to just cleanup the reqName
 	imageUUID := strings.TrimPrefix(volName, cj.namingPrefix)
-	err := RemoveObject(monitors, id, key, pool, cj.namespace, cj.cephUUIDDirectoryPrefix+imageUUID)
+	err := RemoveObject(monitors, cr, pool, cj.namespace, cj.cephUUIDDirectoryPrefix+imageUUID)
 	if err != nil {
 		if _, ok := err.(ErrObjectNotFound); !ok {
 			klog.Errorf("failed removing oMap %s (%s)", cj.cephUUIDDirectoryPrefix+imageUUID, err)
@ -256,7 +256,7 @@ func (cj *CSIJournal) UndoReservation(monitors, id, key, pool, volName, reqName
 	}

 	// delete the request name key (last, inverse of create order)
-	err = RemoveOMapKey(monitors, id, key, pool, cj.namespace, cj.csiDirectory,
+	err = RemoveOMapKey(monitors, cr, pool, cj.namespace, cj.csiDirectory,
 		cj.csiNameKeyPrefix+reqName)
 	if err != nil {
 		klog.Errorf("failed removing oMap key %s (%s)", cj.csiNameKeyPrefix+reqName, err)
@ -269,7 +269,7 @@ func (cj *CSIJournal) UndoReservation(monitors, id, key, pool, volName, reqName
 // reserveOMapName creates an omap with passed in oMapNamePrefix and a generated <uuid>.
 // It ensures generated omap name does not already exist and if conflicts are detected, a set
 // number of retires with newer uuids are attempted before returning an error
-func reserveOMapName(monitors, id, key, pool, namespace, oMapNamePrefix string) (string, error) {
+func reserveOMapName(monitors string, cr *Credentials, pool, namespace, oMapNamePrefix string) (string, error) {
 	var iterUUID string

 	maxAttempts := 5
@ -278,7 +278,7 @@ func reserveOMapName(monitors, id, key, pool, namespace, oMapNamePrefix string)
 		// generate a uuid for the image name
 		iterUUID = uuid.NewUUID().String()

-		err := CreateObject(monitors, id, key, pool, namespace, oMapNamePrefix+iterUUID)
+		err := CreateObject(monitors, cr, pool, namespace, oMapNamePrefix+iterUUID)
 		if err != nil {
 			if _, ok := err.(ErrObjectExists); ok {
 				attempt++
@ -310,7 +310,7 @@ Return values:
 	- string: Contains the UUID that was reserved for the passed in reqName
 	- error: non-nil in case of any errors
 */
-func (cj *CSIJournal) ReserveName(monitors, id, key, pool, reqName, parentName string) (string, error) {
+func (cj *CSIJournal) ReserveName(monitors string, cr *Credentials, pool, reqName, parentName string) (string, error) {
 	var snapSource bool

 	if parentName != "" {
@ -325,14 +325,14 @@ func (cj *CSIJournal) ReserveName(monitors, id, key, pool, reqName, parentName s
 	// NOTE: If any service loss occurs post creation of the UUID directory, and before
 	// setting the request name key (csiNameKey) to point back to the UUID directory, the
 	// UUID directory key will be leaked
-	volUUID, err := reserveOMapName(monitors, id, key, pool, cj.namespace, cj.cephUUIDDirectoryPrefix)
+	volUUID, err := reserveOMapName(monitors, cr, pool, cj.namespace, cj.cephUUIDDirectoryPrefix)
 	if err != nil {
 		return "", err
 	}

 	// Create request name (csiNameKey) key in csiDirectory and store the UUId based
 	// volume name into it
-	err = SetOMapKeyValue(monitors, id, key, pool, cj.namespace, cj.csiDirectory,
+	err = SetOMapKeyValue(monitors, cr, pool, cj.namespace, cj.csiDirectory,
 		cj.csiNameKeyPrefix+reqName, volUUID)
 	if err != nil {
 		return "", err
@ -340,7 +340,7 @@ func (cj *CSIJournal) ReserveName(monitors, id, key, pool, reqName, parentName s
 	defer func() {
 		if err != nil {
 			klog.Warningf("reservation failed for volume: %s", reqName)
-			errDefer := cj.UndoReservation(monitors, id, key, pool, cj.namingPrefix+volUUID,
+			errDefer := cj.UndoReservation(monitors, cr, pool, cj.namingPrefix+volUUID,
 				reqName)
 			if errDefer != nil {
 				klog.Warningf("failed undoing reservation of volume: %s (%v)", reqName, errDefer)
@ -349,7 +349,7 @@ func (cj *CSIJournal) ReserveName(monitors, id, key, pool, reqName, parentName s
 	}()

 	// Update UUID directory to store CSI request name
-	err = SetOMapKeyValue(monitors, id, key, pool, cj.namespace, cj.cephUUIDDirectoryPrefix+volUUID,
+	err = SetOMapKeyValue(monitors, cr, pool, cj.namespace, cj.cephUUIDDirectoryPrefix+volUUID,
 		cj.csiNameKey, reqName)
 	if err != nil {
 		return "", err
@ -357,7 +357,7 @@ func (cj *CSIJournal) ReserveName(monitors, id, key, pool, reqName, parentName s

 	if snapSource {
 		// Update UUID directory to store source volume UUID in case of snapshots
-		err = SetOMapKeyValue(monitors, id, key, pool, cj.namespace, cj.cephUUIDDirectoryPrefix+volUUID,
+		err = SetOMapKeyValue(monitors, cr, pool, cj.namespace, cj.cephUUIDDirectoryPrefix+volUUID,
 			cj.cephSnapSourceKey, parentName)
 		if err != nil {
 			return "", err
@ -374,7 +374,7 @@ Return values:
 	- string: Contains the parent image name for the passed in UUID, if it is a snapshot
 	- error: non-nil in case of any errors
 */
-func (cj *CSIJournal) GetObjectUUIDData(monitors, id, key, pool, objectUUID string, snapSource bool) (string, string, error) {
+func (cj *CSIJournal) GetObjectUUIDData(monitors string, cr *Credentials, pool, objectUUID string, snapSource bool) (string, string, error) {
 	var sourceName string

 	if snapSource && cj.cephSnapSourceKey == "" {
@ -383,14 +383,14 @@ func (cj *CSIJournal) GetObjectUUIDData(monitors, id, key, pool, objectUUID stri
 	}

 	// TODO: fetch all omap vals in one call, than make multiple listomapvals
-	requestName, err := GetOMapValue(monitors, id, key, pool, cj.namespace,
+	requestName, err := GetOMapValue(monitors, cr, pool, cj.namespace,
 		cj.cephUUIDDirectoryPrefix+objectUUID, cj.csiNameKey)
 	if err != nil {
 		return "", "", err
 	}

 	if snapSource {
-		sourceName, err = GetOMapValue(monitors, id, key, pool, cj.namespace,
+		sourceName, err = GetOMapValue(monitors, cr, pool, cj.namespace,
 			cj.cephUUIDDirectoryPrefix+objectUUID, cj.cephSnapSourceKey)
 		if err != nil {
 			return "", "", err