feedc0de/my-rook-config
1
0 Fork 0
forked from rook/rook
Code Pull Requests Activity

Merge pull request #16460 from rook/mergify/bp/release-1.18/pr-16452

Browse Source 
csi: fix prior cephx key deletion logic (backport #16452)
This commit is contained in:
Blaine Gardner authored and GitHub committed 2025-09-10 10:48:51 -06:00
2 parents 599d2766dd 9b71f192ed
commit c729d02dc7
2 files changed
+88 -27
Download Patch File Download Diff File Expand all files Collapse all files

No files matched your search

pkg/operator/ceph/csi/secrets.go
+18 -13
View File
@@ -110,15 +110,17 @@ func createCSIKeyring(
return "", "", 0, shouldRotate, errors.Wrapf(err, "failed to check if keys should to be rotated for CSI key %s", csiKeyrigName)
}
// add the new key to the list of all keys
// do this before deleting old keys so that delete logic is the same when keys are first rotated
// and afterwards when they are not rotated
allKeyWithBaseName = append(allKeyWithBaseName, latestClientId)
allKeyWithBaseName = deduplicate(allKeyWithBaseName)
keysDeleted, err := deleteOldKeyGen(context, clusterInfo, allKeyWithBaseName, cephCluster.Spec.Security.CephX.CSI.KeepPriorKeyCountMax)
if err != nil {
logger.Errorf("failed to delete keys during CSI key rotation. %v. Continuing with key rotation", err)
}
// add the new key to the list of all keys
allKeyWithBaseName = append(allKeyWithBaseName, latestClientId)
allKeyWithBaseName = deduplicate(allKeyWithBaseName)
currentKeyCount := len(allKeyWithBaseName) - len(keysDeleted)
if currentKeyCount <= 0 {
logger.Warningf("currentKeyCount is %d is less than or equal to 0; this could be due to an error in key generation", currentKeyCount)
@@ -435,16 +437,8 @@ func deleteOldKeyGen(context *clusterd.Context, clusterInfo *client.ClusterInfo,
return []string{}, nil
}
keyDeleteCount := max(0, len(sortedClientList)-int(count)) // can't delete negative count
if len(sortedClientList) == keyDeleteCount && keyDeleteCount > 0 {
// Above condition will delete all the keys related to specific CSI driver keys, this will break CSI and Ceph connection.
// To avoid above we'll delete `count-1`
logger.Warningf("current `keepPriorCountMax` value %d will delete all the related CSI driver key count %d. Deleting `keepPriorCountMax +1` keys", count, keyDeleteCount)
keyDeleteCount -= 1
}
listOfKeysDeleted := []string{}
for i := range keyDeleteCount {
for i := range deleteCount(len(sortedClientList), int(count)) {
err := client.AuthDelete(context, clusterInfo, sortedClientList[i])
if err != nil {
// in case of partial success, return successfully deleted list on error
@@ -455,6 +449,17 @@ func deleteOldKeyGen(context *clusterd.Context, clusterInfo *client.ClusterInfo,
return listOfKeysDeleted, nil
}
// calculate the number of keys to delete, given current num keys and desired (max) prior count
func deleteCount(numKeys, keepPriorCount int) int {
// assumption: not called with numKeys < 2
// current key is counted in numKeys, so ignore it in delete count
c := numKeys - keepPriorCount - 1
if c < 0 {
return 0 // can't delete negative number of keys
}
return c
}
func deleteOwnedCSISecretsByCephCluster(context *clusterd.Context, clusterInfo *client.ClusterInfo) error {
ownerRef := metav1.OwnerReference{
APIVersion: "ceph.rook.io/v1",
pkg/operator/ceph/csi/secrets_test.go
+70 -14
View File
@@ -37,11 +37,22 @@ import (
const (
mockAuthLs = `{"auth_dump":[
{"entity":"osd.0"},{"entity":"client.admin"},{"entity":"client.bootstrap-mds"},{"entity":"client.bootstrap-rbd-mirror"},{"entity":"client.csi-rbd-node.10"},
{"entity":"client.csi-rbd-node.4"},{"entity":"client.csi-rbd-node.2"},{"entity":"client.csi-rbd-node.1"},{"entity":"client.csi-cepfs-node"},
{"entity":"client.csi-rbd-node"},{"entity":"client.csi-rbd-provisioner"},{"entity":"client.rbd-mirror-peer"},{"entity":"mgr.a"},
{"entity":"client.csi-cepfs-node"},{"entity":"client.csi-cephfs-node.2"}
]}`
{"entity":"osd.0"},
{"entity":"client.admin"},
{"entity":"client.bootstrap-mds"},
{"entity":"client.bootstrap-rbd-mirror"},
{"entity":"client.csi-rbd-node.10"},
{"entity":"client.csi-rbd-node.4"},
{"entity":"client.csi-rbd-node.2"},
{"entity":"client.csi-rbd-node.1"},
{"entity":"client.csi-cepfs-node"},
{"entity":"client.csi-rbd-node"},
{"entity":"client.csi-rbd-provisioner"},
{"entity":"client.rbd-mirror-peer"},
{"entity":"mgr.a"},
{"entity":"client.csi-cepfs-node"},
{"entity":"client.csi-cephfs-node.2"}
]}`
)
func TestCephCSIKeyringRBDNodeCaps(t *testing.T) {
@@ -183,16 +194,31 @@ func TestDeleteOldKeyGen(t *testing.T) {
"client.csi-rbd-node.10", "client.csi-rbd-node.1", "client.csi-rbd-node.4", "client.csi-rbd-node.2", "client.csi-rbd-node",
}
// Only the oldest keys ("client.csi-rbd-node" and "client.csi-rbd-node.1") are deleted to retain the latest 3 generations.
keyDeleted, err := deleteOldKeyGen(ctx, clusterInfo, keys, 3)
var keyDeleted []string
var err error
// deleting nothing when keep prior count more than current number of existing prior keys
keyDeleted, err = deleteOldKeyGen(ctx, clusterInfo, keys, 5)
assert.NoError(t, err)
assert.Equal(t, []string{}, keyDeleted)
// deleting nothing when keep prior count is same as current number of existing prior keys
keyDeleted, err = deleteOldKeyGen(ctx, clusterInfo, keys, 4)
assert.NoError(t, err)
assert.Equal(t, []string{}, keyDeleted)
// Delete 1 oldest key ("client.csi-rbd-node") is deleted to retain the prior 3 generations.
keyDeleted, err = deleteOldKeyGen(ctx, clusterInfo, keys, 3)
assert.NoError(t, err)
assert.Equal(t, []string{"client.csi-rbd-node"}, keyDeleted)
keyDeleted, err = deleteOldKeyGen(ctx, clusterInfo, keys, 2)
assert.NoError(t, err)
assert.Equal(t, []string{"client.csi-rbd-node", "client.csi-rbd-node.1"}, keyDeleted)
// When keepCount is 1, only the most recent key ("client.csi-rbd-node.10") is kept; older keys ("client.csi-rbd-node.1", "client.csi-rbd-node.4") are deleted.
keys = []string{"client.csi-rbd-node.10", "client.csi-rbd-node.1", "client.csi-rbd-node.4"}
keyDeleted, err = deleteOldKeyGen(ctx, clusterInfo, keys, 1)
assert.NoError(t, err)
assert.Equal(t, []string{"client.csi-rbd-node.1", "client.csi-rbd-node.4"}, keyDeleted)
assert.Equal(t, []string{"client.csi-rbd-node", "client.csi-rbd-node.1", "client.csi-rbd-node.2"}, keyDeleted)
// No keys should be deleted when the input key list is empty.
keys = []string{}
@@ -370,11 +396,16 @@ func TestGetCSIKeyInfoAndDeleteOldKey(t *testing.T) {
keepCount uint8
wantDeleted []string
}{
{"test deletion", keyWithBaseName, 3, []string{"client.csi-rbd-node", "client.csi-rbd-node.1"}},
{"keep count = to total key count", keyWithBaseName, 10, []string{}},
{"keep count > to total key count", keyWithBaseName, 11, []string{}},
{"test deletion", keyWithBaseName, 2, []string{"client.csi-rbd-node", "client.csi-rbd-node.1"}},
{"keep count = current gen", keyWithBaseName, 10, []string{}},
{"keep count > current gen", keyWithBaseName, 11, []string{}},
{"keep count 0", keyWithBaseName, 0, []string{"client.csi-rbd-node", "client.csi-rbd-node.1", "client.csi-rbd-node.2", "client.csi-rbd-node.4"}},
{"keep count 1", keyWithBaseName, 1, []string{"client.csi-rbd-node", "client.csi-rbd-node.1", "client.csi-rbd-node.2", "client.csi-rbd-node.4"}},
{"keep count 1", keyWithBaseName, 1, []string{"client.csi-rbd-node", "client.csi-rbd-node.1", "client.csi-rbd-node.2"}},
{"keep count 2", keyWithBaseName, 2, []string{"client.csi-rbd-node", "client.csi-rbd-node.1"}},
{"keep count 3", keyWithBaseName, 3, []string{"client.csi-rbd-node"}},
{"keep count = current count - 1 = 4", keyWithBaseName, 4, []string{}},
{"keep count = current count = 5", keyWithBaseName, 5, []string{}},
{"keep count > current count", keyWithBaseName, 6, []string{}},
{"key list empty, keep count 0", []string{}, 0, []string{}},
{"key list empty, keep count 1", []string{}, 1, []string{}},
{"key list empty, keep count 3", []string{}, 3, []string{}},
@@ -541,3 +572,28 @@ func TestGetPriorKeyCount(t *testing.T) {
})
}
}
func Test_deleteCount(t *testing.T) {
tests := []struct {
numKeys int
keepPriorCount int
want int
}{
{2, 0, 1},
{3, 0, 2},
{4, 0, 3},
{2, 1, 0},
{3, 1, 1},
{4, 1, 2},
{2, 2, 0},
{3, 2, 0},
{4, 2, 1},
}
for _, tt := range tests {
t.Run(fmt.Sprintf("%d,%d", tt.numKeys, tt.keepPriorCount), func(t *testing.T) {
if got := deleteCount(tt.numKeys, tt.keepPriorCount); got != tt.want {
t.Errorf("deleteCount() = %v, want %v", got, tt.want)
}
})
}
}