From 8c5e414d530fd87cb0fb0d0e93749b6028b4178b Mon Sep 17 00:00:00 2001 From: Madhu Rajanna Date: Tue, 15 Mar 2022 18:28:02 +0530 Subject: [PATCH] rbd: do not read pvc namespace from volume attributes Below are the 3 different cases where we need the PVC namespace for encryption * CreateVolume:- Read the namespace from the createVolume parameters and store it in the omap * NodeStage:- Read the namespace from the omap not from the volumeContext * Regenerate:- Read the pvc namespace from the claimRef not from the volumeAttributes. Signed-off-by: Madhu Rajanna --- .../persistentvolume/persistentvolume.go | 8 +++++++- internal/rbd/controllerserver.go | 16 +++++++++++++++- internal/rbd/encryption.go | 14 +++----------- internal/rbd/nodeserver.go | 14 ++++++++++---- internal/rbd/rbd_journal.go | 4 +++- internal/rbd/rbd_util.go | 7 +------ 6 files changed, 39 insertions(+), 24 deletions(-) diff --git a/internal/controller/persistentvolume/persistentvolume.go b/internal/controller/persistentvolume/persistentvolume.go index 8c9fa6de4..e6957d88c 100644 --- a/internal/controller/persistentvolume/persistentvolume.go +++ b/internal/controller/persistentvolume/persistentvolume.go @@ -139,6 +139,12 @@ func (r ReconcilePersistentVolume) reconcilePV(ctx context.Context, obj runtime. if pv.Spec.CSI == nil || pv.Spec.CSI.Driver != r.config.DriverName { return nil } + // PV is not attached to any PVC + if pv.Spec.ClaimRef == nil { + return nil + } + + pvcNamespace := pv.Spec.ClaimRef.Namespace requestName := pv.Name volumeHandler := pv.Spec.CSI.VolumeHandle secretName := "" @@ -171,7 +177,7 @@ func (r ReconcilePersistentVolume) reconcilePV(ctx context.Context, obj runtime. } defer cr.DeleteCredentials() - rbdVolID, err := rbd.RegenerateJournal(pv.Spec.CSI.VolumeAttributes, volumeHandler, requestName, cr) + rbdVolID, err := rbd.RegenerateJournal(pv.Spec.CSI.VolumeAttributes, volumeHandler, requestName, pvcNamespace, cr) if err != nil { log.ErrorLogMsg("failed to regenerate journal %s", err) diff --git a/internal/rbd/controllerserver.go b/internal/rbd/controllerserver.go index 5e96069ec..1dc179e28 100644 --- a/internal/rbd/controllerserver.go +++ b/internal/rbd/controllerserver.go @@ -124,13 +124,27 @@ func (cs *ControllerServer) parseVolCreateRequest( rbdVol, err := genVolFromVolumeOptions( ctx, req.GetParameters(), - req.GetSecrets(), isMultiWriter && isBlock, false) if err != nil { return nil, status.Error(codes.InvalidArgument, err.Error()) } + // if the KMS is of type VaultToken, additional metadata is needed + // depending on the tenant, the KMS can be configured with other + // options + // FIXME: this works only on Kubernetes, how do other CO supply metadata? + // namespace is derived from the `csi.storage.k8s.io/pvc/namespace` + // parameter. + + // get the owner of the PVC which is required for few encryption related operations + rbdVol.Owner = k8s.GetOwner(req.GetParameters()) + + err = rbdVol.initKMS(ctx, req.GetParameters(), req.GetSecrets()) + if err != nil { + return nil, status.Error(codes.InvalidArgument, err.Error()) + } + rbdVol.RequestName = req.GetName() // Volume Size - Default is 1 GiB diff --git a/internal/rbd/encryption.go b/internal/rbd/encryption.go index 217fe4cfd..2255f1020 100644 --- a/internal/rbd/encryption.go +++ b/internal/rbd/encryption.go @@ -265,22 +265,14 @@ func (ri *rbdImage) initKMS(ctx context.Context, volOptions, credentials map[str } // ParseEncryptionOpts returns kmsID and sets Owner attribute. -func (ri *rbdImage) ParseEncryptionOpts(ctx context.Context, volOptions map[string]string) (string, error) { +func (ri *rbdImage) ParseEncryptionOpts( + ctx context.Context, + volOptions map[string]string) (string, error) { var ( err error ok bool encrypted, kmsID string ) - - // if the KMS is of type VaultToken, additional metadata is needed - // depending on the tenant, the KMS can be configured with other - // options - // FIXME: this works only on Kubernetes, how do other CO supply metadata? - ri.Owner, ok = volOptions["csi.storage.k8s.io/pvc/namespace"] - if !ok { - log.DebugLog(ctx, "could not detect owner for %s", ri) - } - encrypted, ok = volOptions["encrypted"] if !ok { return "", nil diff --git a/internal/rbd/nodeserver.go b/internal/rbd/nodeserver.go index affe42fab..143a31ef7 100644 --- a/internal/rbd/nodeserver.go +++ b/internal/rbd/nodeserver.go @@ -147,8 +147,7 @@ func healerStageTransaction(ctx context.Context, cr *util.Credentials, volOps *r func populateRbdVol( ctx context.Context, req *csi.NodeStageVolumeRequest, - cr *util.Credentials, - secrets map[string]string) (*rbdVolume, error) { + cr *util.Credentials) (*rbdVolume, error) { var err error var j *journal.Connection volID := req.GetVolumeId() @@ -173,7 +172,7 @@ func populateRbdVol( disableInUseChecks = true } - rv, err := genVolFromVolumeOptions(ctx, req.GetVolumeContext(), secrets, disableInUseChecks, true) + rv, err := genVolFromVolumeOptions(ctx, req.GetVolumeContext(), disableInUseChecks, true) if err != nil { return nil, status.Error(codes.Internal, err.Error()) } @@ -213,6 +212,8 @@ func populateRbdVol( return nil, status.Error(codes.Internal, err.Error()) } rv.RbdImageName = imageAttributes.ImageName + // set owner after extracting the owner name from the journal + rv.Owner = imageAttributes.Owner } err = rv.Connect(cr) @@ -235,6 +236,11 @@ func populateRbdVol( return nil, status.Error(codes.Internal, err.Error()) } + err = rv.initKMS(ctx, req.GetVolumeContext(), req.GetSecrets()) + if err != nil { + return nil, status.Error(codes.Internal, err.Error()) + } + if req.GetVolumeContext()["mounter"] == rbdDefaultMounter && !isKrbdFeatureSupported(ctx, strings.Join(rv.ImageFeatureSet.Names(), ",")) { if !parseBoolOption(ctx, req.GetVolumeContext(), tryOtherMounters, false) { @@ -320,7 +326,7 @@ func (ns *NodeServer) NodeStageVolume( } isStaticVol := parseBoolOption(ctx, req.GetVolumeContext(), staticVol, false) - rv, err := populateRbdVol(ctx, req, cr, req.GetSecrets()) + rv, err := populateRbdVol(ctx, req, cr) if err != nil { return nil, err } diff --git a/internal/rbd/rbd_journal.go b/internal/rbd/rbd_journal.go index bace5e0ff..e2a50354e 100644 --- a/internal/rbd/rbd_journal.go +++ b/internal/rbd/rbd_journal.go @@ -531,7 +531,7 @@ func undoVolReservation(ctx context.Context, rbdVol *rbdVolume, cr *util.Credent // which are not same across clusters. func RegenerateJournal( volumeAttributes map[string]string, - volumeID, requestName string, + volumeID, requestName, owner string, cr *util.Credentials) (string, error) { ctx := context.Background() var ( @@ -551,6 +551,8 @@ func RegenerateJournal( ErrInvalidVolID, err, rbdVol.VolID) } + rbdVol.Owner = owner + kmsID, err = rbdVol.ParseEncryptionOpts(ctx, volumeAttributes) if err != nil { return "", err diff --git a/internal/rbd/rbd_util.go b/internal/rbd/rbd_util.go index a64c29870..3afe68962 100644 --- a/internal/rbd/rbd_util.go +++ b/internal/rbd/rbd_util.go @@ -1142,7 +1142,7 @@ func generateVolumeFromMapping( func genVolFromVolumeOptions( ctx context.Context, - volOptions, credentials map[string]string, + volOptions map[string]string, disableInUseChecks, checkClusterIDMapping bool) (*rbdVolume, error) { var ( ok bool @@ -1195,11 +1195,6 @@ func genVolFromVolumeOptions( rbdVol.Mounter) rbdVol.DisableInUseChecks = disableInUseChecks - err = rbdVol.initKMS(ctx, volOptions, credentials) - if err != nil { - return nil, err - } - return rbdVol, nil }