Skip to content

Commit

Permalink
provider: avoid CSI secret name collisions
Browse files Browse the repository at this point in the history 
provider sends the ceph client secret name to onboarded consumer as gRPC
response, however if both provider & client operators running in same
namespaces the secrets referred will collide. So, we'll send different
secret names with same content to be created by client.

Signed-off-by: Leela Venkaiah G <[email protected]>
  • Loading branch information
@leelavg
leelavg committed Apr 22, 2024
1 parent c50d1a4 commit 149bc59
Show file tree
Hide file tree
Showing 4 changed files with 119 additions and 140 deletions.
105 changes: 47 additions & 58 deletions services/provider/server/server.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -520,6 +520,10 @@ func (s *OCSProviderServer) RevokeStorageClaim(ctx context.Context, req *pb.Revo
return &pb.RevokeStorageClaimResponse{}, nil
}

func storageClaimCephCsiSecretName(secretType, suffix string) string {
return fmt.Sprintf("ceph-client-%s-%s", secretType, suffix)
}

// GetStorageClaim RPC call to get the ceph resources for the StorageClaim.
func (s *OCSProviderServer) GetStorageClaimConfig(ctx context.Context, req *pb.StorageClaimConfigRequest) (*pb.StorageClaimConfigResponse, error) {
storageRequest, err := s.storageRequestManager.Get(ctx, req.StorageConsumerUUID, req.StorageClaimName)
Expand Down Expand Up @@ -550,16 +554,15 @@ func (s *OCSProviderServer) GetStorageClaimConfig(ctx context.Context, req *pb.S
}
var extR []*pb.ExternalResource

var storageClassName string
storageClassData := map[string]string{}

storageRequestHash := getStorageRequestHash(req.StorageConsumerUUID, req.StorageClaimName)
for _, cephRes := range storageRequest.Status.CephResources {
switch cephRes.Kind {
case "CephClient":
clientSecretName, _, err := s.getCephClientInformation(ctx, cephRes.Name)
clientSecretName, clientUserType, err := s.getCephClientInformation(ctx, cephRes.Name)
if err != nil {
return nil, status.Error(codes.Internal, err.Error())
}
extSecretName := storageClaimCephCsiSecretName(clientUserType, storageRequestHash)

cephUserSecret := &v1.Secret{}
err = s.client.Get(ctx, types.NamespacedName{Name: clientSecretName, Namespace: s.namespace}, cephUserSecret)
Expand All @@ -574,10 +577,7 @@ func (s *OCSProviderServer) GetStorageClaimConfig(ctx context.Context, req *pb.S
keyProp = "adminKey"
}
extR = append(extR, &pb.ExternalResource{
// a common suffix '.csi' is being added to distinguish secrets that are created
// by ocs-client-operator vs rook-operator when both these operators are deployed in same namespace
// TODO: need to transform existing secrets during migration manually
Name: clientSecretName + ".csi",
Name: extSecretName,
Kind: "Secret",
Data: mustMarshal(map[string]string{
idProp: cephRes.Name,
Expand All @@ -586,44 +586,43 @@ func (s *OCSProviderServer) GetStorageClaimConfig(ctx context.Context, req *pb.S
})

case "CephBlockPoolRadosNamespace":
storageClassName = "ceph-rbd"
storageClassData["clusterID"] = s.namespace
storageClassData["radosnamespace"] = cephRes.Name

nodeCephClientSecret, _, err := s.getCephClientInformation(ctx, cephRes.CephClients["node"])
if err != nil {
return nil, status.Error(codes.Internal, err.Error())
}

provisionerCephClientSecret, _, err := s.getCephClientInformation(ctx, cephRes.CephClients["provisioner"])
if err != nil {
return nil, status.Error(codes.Internal, err.Error())
}

rns := &rookCephv1.CephBlockPoolRadosNamespace{}
err = s.client.Get(ctx, types.NamespacedName{Name: cephRes.Name, Namespace: s.namespace}, rns)
if err != nil {
return nil, status.Errorf(codes.Internal, "failed to get %s CephBlockPoolRadosNamespace. %v", cephRes.Name, err)
}

storageClassData["pool"] = rns.Spec.BlockPoolName
storageClassData["imageFeatures"] = "layering,deep-flatten,exclusive-lock,object-map,fast-diff"
storageClassData["csi.storage.k8s.io/fstype"] = "ext4"
storageClassData["imageFormat"] = "2"
storageClassData["csi.storage.k8s.io/provisioner-secret-name"] = provisionerCephClientSecret
storageClassData["csi.storage.k8s.io/node-stage-secret-name"] = nodeCephClientSecret
storageClassData["csi.storage.k8s.io/controller-expand-secret-name"] = provisionerCephClientSecret
provisionerSecretName := storageClaimCephCsiSecretName("provisioner", storageRequestHash)
nodeSecretName := storageClaimCephCsiSecretName("node", storageRequestHash)
rbdStorageClassData := map[string]string{
"clusterID": s.namespace,
"radosnamespace": cephRes.Name,
"pool": rns.Spec.BlockPoolName,
"imageFeatures": "layering,deep-flatten,exclusive-lock,object-map,fast-diff",
"csi.storage.k8s.io/fstype": "ext4",
"imageFormat": "2",
"csi.storage.k8s.io/provisioner-secret-name": provisionerSecretName,
"csi.storage.k8s.io/node-stage-secret-name": nodeSecretName,
"csi.storage.k8s.io/controller-expand-secret-name": provisionerSecretName,
}
if storageRequest.Spec.EncryptionMethod != "" {
storageClassData["encrypted"] = "true"
storageClassData["encryptionKMSID"] = storageRequest.Spec.EncryptionMethod
rbdStorageClassData["encrypted"] = "true"
rbdStorageClassData["encryptionKMSID"] = storageRequest.Spec.EncryptionMethod
}

extR = append(extR, &pb.ExternalResource{
Name: "ceph-rbd",
Kind: "StorageClass",
Data: mustMarshal(rbdStorageClassData),
})

extR = append(extR, &pb.ExternalResource{
Name: "ceph-rbd",
Kind: "VolumeSnapshotClass",
Data: mustMarshal(map[string]string{
"clusterID": storageClassData["clusterID"],
"csi.storage.k8s.io/snapshotter-secret-name": provisionerCephClientSecret,
"clusterID": rbdStorageClassData["clusterID"],
"csi.storage.k8s.io/snapshotter-secret-name": provisionerSecretName,
})})

case "CephFilesystemSubVolumeGroup":
Expand All @@ -633,24 +632,23 @@ func (s *OCSProviderServer) GetStorageClaimConfig(ctx context.Context, req *pb.S
return nil, status.Errorf(codes.Internal, "failed to get %s cephFilesystemSubVolumeGroup. %v", cephRes.Name, err)
}

nodeCephClientSecret, _, err := s.getCephClientInformation(ctx, cephRes.CephClients["node"])
if err != nil {
return nil, status.Error(codes.Internal, err.Error())
}

provisionerCephClientSecret, _, err := s.getCephClientInformation(ctx, cephRes.CephClients["provisioner"])
if err != nil {
return nil, status.Error(codes.Internal, err.Error())
provisionerSecretName := storageClaimCephCsiSecretName("provisioner", storageRequestHash)
nodeSecretName := storageClaimCephCsiSecretName("node", storageRequestHash)
cephfsStorageClassData := map[string]string{
"clusterID": getSubVolumeGroupClusterID(subVolumeGroup),
"subvolumegroupname": subVolumeGroup.Name,
"fsName": subVolumeGroup.Spec.FilesystemName,
"pool": subVolumeGroup.GetLabels()[v1alpha1.CephFileSystemDataPoolLabel],
"csi.storage.k8s.io/provisioner-secret-name": provisionerSecretName,
"csi.storage.k8s.io/node-stage-secret-name": nodeSecretName,
"csi.storage.k8s.io/controller-expand-secret-name": provisionerSecretName,
}

storageClassName = "cephfs"
storageClassData["clusterID"] = getSubVolumeGroupClusterID(subVolumeGroup)
storageClassData["subvolumegroupname"] = subVolumeGroup.Name
storageClassData["fsName"] = subVolumeGroup.Spec.FilesystemName
storageClassData["pool"] = subVolumeGroup.GetLabels()[v1alpha1.CephFileSystemDataPoolLabel]
storageClassData["csi.storage.k8s.io/provisioner-secret-name"] = provisionerCephClientSecret
storageClassData["csi.storage.k8s.io/node-stage-secret-name"] = nodeCephClientSecret
storageClassData["csi.storage.k8s.io/controller-expand-secret-name"] = provisionerCephClientSecret
extR = append(extR, &pb.ExternalResource{
Name: "cephfs",
Kind: "StorageClass",
Data: mustMarshal(cephfsStorageClassData),
})

extR = append(extR, &pb.ExternalResource{
Name: cephRes.Name,
Expand All @@ -664,20 +662,11 @@ func (s *OCSProviderServer) GetStorageClaimConfig(ctx context.Context, req *pb.S
Kind: "VolumeSnapshotClass",
Data: mustMarshal(map[string]string{
"clusterID": getSubVolumeGroupClusterID(subVolumeGroup),
"csi.storage.k8s.io/snapshotter-secret-name": provisionerCephClientSecret,
"csi.storage.k8s.io/snapshotter-secret-name": provisionerSecretName,
})})
}
}

if storageClassName == "" {
return nil, status.Error(codes.Internal, "No StorageClass was defined")
}
extR = append(extR, &pb.ExternalResource{
Name: storageClassName,
Kind: "StorageClass",
Data: mustMarshal(storageClassData),
})

klog.Infof("successfully returned the storage class claim %q for %q", req.StorageClaimName, req.StorageConsumerUUID)
return &pb.StorageClaimConfigResponse{ExternalResource: extR}, nil

Expand Down
90 changes: 38 additions & 52 deletions services/provider/server/server_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@ import (
"encoding/json"
"fmt"
"strconv"
"strings"
"testing"

ocsv1alpha1 "github.com/red-hat-storage/ocs-operator/api/v4/v1alpha1"
Expand Down Expand Up @@ -499,29 +498,29 @@ func TestOCSProviderServerGetStorageClaimConfig(t *testing.T) {
"imageFeatures": "layering,deep-flatten,exclusive-lock,object-map,fast-diff",
"csi.storage.k8s.io/fstype": "ext4",
"imageFormat": "2",
"csi.storage.k8s.io/provisioner-secret-name": "rook-ceph-client-3de200d5c23524a4612bde1fdbeb717e",
"csi.storage.k8s.io/node-stage-secret-name": "rook-ceph-client-995e66248ad3e8642de868f461cdd827",
"csi.storage.k8s.io/controller-expand-secret-name": "rook-ceph-client-3de200d5c23524a4612bde1fdbeb717e",
"csi.storage.k8s.io/provisioner-secret-name": "ceph-client-provisioner-8d40b6be71600457b5dec219d2ce2d4c",
"csi.storage.k8s.io/node-stage-secret-name": "ceph-client-node-8d40b6be71600457b5dec219d2ce2d4c",
"csi.storage.k8s.io/controller-expand-secret-name": "ceph-client-provisioner-8d40b6be71600457b5dec219d2ce2d4c",
},
},
"ceph-rbd-volumesnapshotclass": {
Name: "ceph-rbd",
Kind: "VolumeSnapshotClass",
Data: map[string]string{
"clusterID": serverNamespace,
"csi.storage.k8s.io/snapshotter-secret-name": "rook-ceph-client-3de200d5c23524a4612bde1fdbeb717e",
"csi.storage.k8s.io/snapshotter-secret-name": "ceph-client-provisioner-8d40b6be71600457b5dec219d2ce2d4c",
},
},
"rook-ceph-client-3de200d5c23524a4612bde1fdbeb717e": {
Name: "rook-ceph-client-3de200d5c23524a4612bde1fdbeb717e",
"ceph-client-provisioner-8d40b6be71600457b5dec219d2ce2d4c": {
Name: "ceph-client-provisioner-8d40b6be71600457b5dec219d2ce2d4c",
Kind: "Secret",
Data: map[string]string{
"userID": "3de200d5c23524a4612bde1fdbeb717e",
"userKey": "AQADw/hhqBOcORAAJY3fKIvte++L/zYhASjYPQ==",
},
},
"rook-ceph-client-995e66248ad3e8642de868f461cdd827": {
Name: "rook-ceph-client-995e66248ad3e8642de868f461cdd827",
"ceph-client-node-8d40b6be71600457b5dec219d2ce2d4c": {
Name: "ceph-client-node-8d40b6be71600457b5dec219d2ce2d4c",
Kind: "Secret",
Data: map[string]string{
"userID": "995e66248ad3e8642de868f461cdd827",
Expand All @@ -539,29 +538,29 @@ func TestOCSProviderServerGetStorageClaimConfig(t *testing.T) {
"fsName": "myfs",
"subvolumegroupname": "cephFilesystemSubVolumeGroup",
"pool": "",
"csi.storage.k8s.io/provisioner-secret-name": "rook-ceph-client-4ffcb503ff8044c8699dac415f82d604",
"csi.storage.k8s.io/node-stage-secret-name": "rook-ceph-client-1b042fcc8812fe4203689eec38fdfbfa",
"csi.storage.k8s.io/controller-expand-secret-name": "rook-ceph-client-4ffcb503ff8044c8699dac415f82d604",
"csi.storage.k8s.io/provisioner-secret-name": "ceph-client-provisioner-0e8555e6556f70d23a61675af44e880c",
"csi.storage.k8s.io/node-stage-secret-name": "ceph-client-node-0e8555e6556f70d23a61675af44e880c",
"csi.storage.k8s.io/controller-expand-secret-name": "ceph-client-provisioner-0e8555e6556f70d23a61675af44e880c",
},
},
"cephfs-volumesnapshotclass": {
Name: "cephfs",
Kind: "VolumeSnapshotClass",
Data: map[string]string{
"clusterID": "8d26c7378c1b0ec9c2455d1c3601c4cd",
"csi.storage.k8s.io/snapshotter-secret-name": "rook-ceph-client-4ffcb503ff8044c8699dac415f82d604",
"csi.storage.k8s.io/snapshotter-secret-name": "ceph-client-provisioner-0e8555e6556f70d23a61675af44e880c",
},
},
"rook-ceph-client-4ffcb503ff8044c8699dac415f82d604": {
Name: "rook-ceph-client-4ffcb503ff8044c8699dac415f82d604",
"ceph-client-provisioner-0e8555e6556f70d23a61675af44e880c": {
Name: "ceph-client-provisioner-0e8555e6556f70d23a61675af44e880c",
Kind: "Secret",
Data: map[string]string{
"adminID": "4ffcb503ff8044c8699dac415f82d604",
"adminKey": "AQADw/hhqBOcORAAJY3fKIvte++L/zYhASjYPQ==",
},
},
"rook-ceph-client-1b042fcc8812fe4203689eec38fdfbfa": {
Name: "rook-ceph-client-1b042fcc8812fe4203689eec38fdfbfa",
"ceph-client-node-0e8555e6556f70d23a61675af44e880c": {
Name: "ceph-client-node-0e8555e6556f70d23a61675af44e880c",
Kind: "Secret",
Data: map[string]string{
"adminID": "1b042fcc8812fe4203689eec38fdfbfa",
Expand Down Expand Up @@ -708,14 +707,14 @@ func TestOCSProviderServerGetStorageClaimConfig(t *testing.T) {
},
Status: &rookCephv1.CephClientStatus{
Info: map[string]string{
"secretName": "rook-ceph-client-995e66248ad3e8642de868f461cdd827",
"secretName": "ceph-client-node-8d40b6be71600457b5dec219d2ce2d4c",
},
},
}

secret := &v1.Secret{
ObjectMeta: metav1.ObjectMeta{
Name: "rook-ceph-client-995e66248ad3e8642de868f461cdd827",
Name: "ceph-client-node-8d40b6be71600457b5dec219d2ce2d4c",
Namespace: server.namespace,
},
Data: map[string][]byte{
Expand All @@ -738,14 +737,14 @@ func TestOCSProviderServerGetStorageClaimConfig(t *testing.T) {
},
Status: &rookCephv1.CephClientStatus{
Info: map[string]string{
"secretName": "rook-ceph-client-3de200d5c23524a4612bde1fdbeb717e",
"secretName": "ceph-client-provisioner-8d40b6be71600457b5dec219d2ce2d4c",
},
},
}

secret = &v1.Secret{
ObjectMeta: metav1.ObjectMeta{
Name: "rook-ceph-client-3de200d5c23524a4612bde1fdbeb717e",
Name: "ceph-client-provisioner-8d40b6be71600457b5dec219d2ce2d4c",
Namespace: server.namespace,
},
Data: map[string][]byte{
Expand All @@ -768,14 +767,14 @@ func TestOCSProviderServerGetStorageClaimConfig(t *testing.T) {
},
Status: &rookCephv1.CephClientStatus{
Info: map[string]string{
"secretName": "rook-ceph-client-1b042fcc8812fe4203689eec38fdfbfa",
"secretName": "ceph-client-node-0e8555e6556f70d23a61675af44e880c",
},
},
}

secret = &v1.Secret{
ObjectMeta: metav1.ObjectMeta{
Name: "rook-ceph-client-1b042fcc8812fe4203689eec38fdfbfa",
Name: "ceph-client-node-0e8555e6556f70d23a61675af44e880c",
Namespace: server.namespace,
},
Data: map[string][]byte{
Expand All @@ -798,14 +797,14 @@ func TestOCSProviderServerGetStorageClaimConfig(t *testing.T) {
},
Status: &rookCephv1.CephClientStatus{
Info: map[string]string{
"secretName": "rook-ceph-client-4ffcb503ff8044c8699dac415f82d604",
"secretName": "ceph-client-provisioner-0e8555e6556f70d23a61675af44e880c",
},
},
}

secret = &v1.Secret{
ObjectMeta: metav1.ObjectMeta{
Name: "rook-ceph-client-4ffcb503ff8044c8699dac415f82d604",
Name: "ceph-client-provisioner-0e8555e6556f70d23a61675af44e880c",
Namespace: server.namespace,
},
Data: map[string][]byte{
Expand Down Expand Up @@ -854,10 +853,6 @@ func TestOCSProviderServerGetStorageClaimConfig(t *testing.T) {
name = fmt.Sprintf("%s-volumesnapshotclass", name)
} else if extResource.Kind == "StorageClass" {
name = fmt.Sprintf("%s-storageclass", name)
} else if extResource.Kind == "Secret" {
var found bool
name, found = strings.CutSuffix(name, ".csi")
assert.True(t, found)
}
mockResoruce, ok := mockBlockPoolClaimExtR[name]
assert.True(t, ok)
Expand All @@ -866,12 +861,7 @@ func TestOCSProviderServerGetStorageClaimConfig(t *testing.T) {
assert.NoError(t, err)
assert.Equal(t, string(extResource.Data), string(data))
assert.Equal(t, extResource.Kind, mockResoruce.Kind)
if extResource.Kind == "Secret" {
name, _ := strings.CutSuffix(name, ".csi")
assert.Equal(t, name, mockResoruce.Name)
} else {
assert.Equal(t, extResource.Name, mockResoruce.Name)
}
assert.Equal(t, extResource.Name, mockResoruce.Name)
}

// get the storage class request config for share filesystem
Expand All @@ -890,10 +880,6 @@ func TestOCSProviderServerGetStorageClaimConfig(t *testing.T) {
name = fmt.Sprintf("%s-volumesnapshotclass", name)
} else if extResource.Kind == "StorageClass" {
name = fmt.Sprintf("%s-storageclass", name)
} else if extResource.Kind == "Secret" {
var found bool
name, found = strings.CutSuffix(name, ".csi")
assert.True(t, found)
}
mockResoruce, ok := mockShareFilesystemClaimExtR[name]
assert.True(t, ok)
Expand All @@ -902,24 +888,24 @@ func TestOCSProviderServerGetStorageClaimConfig(t *testing.T) {
assert.NoError(t, err)
assert.Equal(t, string(extResource.Data), string(data))
assert.Equal(t, extResource.Kind, mockResoruce.Kind)
if extResource.Kind == "Secret" {
name, _ := strings.CutSuffix(name, ".csi")
assert.Equal(t, name, mockResoruce.Name)
} else {
assert.Equal(t, extResource.Name, mockResoruce.Name)
}
assert.Equal(t, extResource.Name, mockResoruce.Name)
}

// When ceph resources is empty
scrNameHash := getStorageRequestHash(string(consumerResource.UID), shareFilesystemClaimName)
for _, i := range sharedFilesystemClaimResource.Status.CephResources {
if i.Kind == "CephClient" {
cephClient, secret := createCephClientAndSecret(i.Name, server)
cephClient.Status = &rookCephv1.CephClientStatus{
Info: map[string]string{
"secretName": fmt.Sprintf("rook-ceph-client-%s", i.Name),
},
if i.Kind == "CephFilesystemSubVolumeGroup" {
for cephClientUserType, cephClientName := range i.CephClients {
cephClient, secret := createCephClientAndSecret(cephClientName, server)
secret.Name = storageClaimCephCsiSecretName(cephClientUserType, scrNameHash)
cephClient.Status = &rookCephv1.CephClientStatus{
Info: map[string]string{
"secretName": secret.Name,
},
}
assert.NoError(t, client.Delete(ctx, secret))
}
assert.NoError(t, client.Delete(ctx, secret))
break
}
}

Expand Down
Loading

0 comments on commit 149bc59

Please sign in to comment.