From dfbdec4b6ab2667df2512140270b110115d954b2 Mon Sep 17 00:00:00 2001 From: Madhu Rajanna Date: Thu, 25 Jul 2019 14:31:10 +0530 Subject: [PATCH] add validation to check if stagingPath exists It's CO responsibility to create the stagingPath as per the CSI spec. The CO SHALL ensure // that the path is directory and that the process serving the // request has `read` and `write` permission to that directory. The // CO SHALL be responsible for creating the directory if it does not // exist. Signed-off-by: Madhu Rajanna --- pkg/cephfs/nodeserver.go | 9 ------- pkg/rbd/nodeserver.go | 53 ++++++++++++++++++++++++++++++---------- pkg/util/util.go | 8 ++++++ pkg/util/validate.go | 5 ++++ 4 files changed, 53 insertions(+), 22 deletions(-) diff --git a/pkg/cephfs/nodeserver.go b/pkg/cephfs/nodeserver.go index 0c6dc95b9..8c75f5257 100644 --- a/pkg/cephfs/nodeserver.go +++ b/pkg/cephfs/nodeserver.go @@ -104,11 +104,6 @@ func (ns *NodeServer) NodeStageVolume(ctx context.Context, req *csi.NodeStageVol } } - if err = util.CreateMountPoint(stagingTargetPath); err != nil { - klog.Errorf("failed to create staging mount point at %s for volume %s: %v", stagingTargetPath, volID, err) - return nil, status.Error(codes.Internal, err.Error()) - } - idLk := nodeVolumeIDLocker.Lock(string(volID)) defer nodeVolumeIDLocker.Unlock(idLk, string(volID)) @@ -289,10 +284,6 @@ func (ns *NodeServer) NodeUnstageVolume(ctx context.Context, req *csi.NodeUnstag return nil, status.Error(codes.Internal, err.Error()) } - if err = os.Remove(stagingTargetPath); err != nil { - return nil, status.Error(codes.Internal, err.Error()) - } - klog.Infof("cephfs: successfully unmounted volume %s from %s", req.GetVolumeId(), stagingTargetPath) return &csi.NodeUnstageVolumeResponse{}, nil diff --git a/pkg/rbd/nodeserver.go b/pkg/rbd/nodeserver.go index 5f5e53ddc..a0b9ced33 100644 --- a/pkg/rbd/nodeserver.go +++ b/pkg/rbd/nodeserver.go @@ -91,10 +91,10 @@ func (ns *NodeServer) NodeStageVolume(ctx context.Context, req *csi.NodeStageVol idLk := nodeVolumeIDLocker.Lock(volID) defer nodeVolumeIDLocker.Unlock(idLk, volID) - // Check if that target path exists properly - isNotMnt, err := ns.createMountPath(stagingTargetPath, isBlock) - if err != nil { - klog.Errorf("stat failed: %v", err) + var isNotMnt bool + // check if stagingPath is already mounted + isNotMnt, err = mount.IsNotMountPoint(ns.mounter, stagingTargetPath) + if err != nil && !os.IsNotExist(err) { return nil, status.Error(codes.Internal, err.Error()) } @@ -117,19 +117,27 @@ func (ns *NodeServer) NodeStageVolume(ctx context.Context, req *csi.NodeStageVol klog.V(4).Infof("rbd image: %s/%s was successfully mapped at %s\n", req.GetVolumeId(), volOptions.Pool, devicePath) isMounted := false + isStagePathCreated := false // if mounting to stagingpath fails unmap the rbd device. this wont leave any // stale rbd device if unstage is not called defer func() { if err != nil { - ns.cleanupStagingPath(stagingTargetPath, devicePath, volID, isBlock, isMounted) + ns.cleanupStagingPath(stagingTargetPath, devicePath, volID, isStagePathCreated, isBlock, isMounted) } }() + err = ns.createStageMountPoint(stagingTargetPath, isBlock) + if err != nil { + return nil, status.Error(codes.Internal, err.Error()) + } + isStagePathCreated = true + // nodeStage Path err = ns.mountVolumeToStagePath(req, stagingTargetPath, devicePath) if err != nil { return nil, status.Error(codes.Internal, err.Error()) } isMounted = true + err = os.Chmod(stagingTargetPath, 0777) if err != nil { return nil, status.Error(codes.Internal, err.Error()) @@ -140,7 +148,7 @@ func (ns *NodeServer) NodeStageVolume(ctx context.Context, req *csi.NodeStageVol return &csi.NodeStageVolumeResponse{}, nil } -func (ns *NodeServer) cleanupStagingPath(stagingTargetPath, devicePath, volID string, isBlock, isMounted bool) { +func (ns *NodeServer) cleanupStagingPath(stagingTargetPath, devicePath, volID string, isStagePathCreated, isBlock, isMounted bool) { var err error if isMounted { err = ns.mounter.Unmount(stagingTargetPath) @@ -149,7 +157,7 @@ func (ns *NodeServer) cleanupStagingPath(stagingTargetPath, devicePath, volID st } } // remove the block file created on staging path - if isBlock { + if isBlock && isStagePathCreated { err = os.Remove(stagingTargetPath) if err != nil { klog.Errorf("failed to remove stagingtargetPath: %s with error: %v", stagingTargetPath, err) @@ -161,6 +169,21 @@ func (ns *NodeServer) cleanupStagingPath(stagingTargetPath, devicePath, volID st } } +func (ns *NodeServer) createStageMountPoint(mountPath string, isBlock bool) error { + if isBlock { + pathFile, err := os.OpenFile(mountPath, os.O_CREATE|os.O_RDWR, 0750) + if err != nil { + klog.Errorf("failed to create mountPath:%s with error: %v", mountPath, err) + return status.Error(codes.Internal, err.Error()) + } + if err = pathFile.Close(); err != nil { + klog.Errorf("failed to close mountPath:%s with error: %v", mountPath, err) + return status.Error(codes.Internal, err.Error()) + } + } + return nil +} + // NodePublishVolume mounts the volume mounted to the device path to the target // path func (ns *NodeServer) NodePublishVolume(ctx context.Context, req *csi.NodePublishVolumeRequest) (*csi.NodePublishVolumeResponse, error) { @@ -180,7 +203,7 @@ func (ns *NodeServer) NodePublishVolume(ctx context.Context, req *csi.NodePublis defer targetPathLocker.Unlock(idLk, targetPath) // Check if that target path exists properly - notMnt, err := ns.createMountPath(targetPath, isBlock) + notMnt, err := ns.createTargetMountPath(targetPath, isBlock) if err != nil { return nil, err } @@ -280,7 +303,7 @@ func (ns *NodeServer) mountVolume(stagingPath string, req *csi.NodePublishVolume return nil } -func (ns *NodeServer) createMountPath(mountPath string, isBlock bool) (bool, error) { +func (ns *NodeServer) createTargetMountPath(mountPath string, isBlock bool) (bool, error) { // Check if that mount path exists properly notMnt, err := mount.IsNotMountPoint(ns.mounter, mountPath) if err != nil { @@ -376,8 +399,10 @@ unmount: stagingTargetPath = blockStagingPath goto unmount } - if err = os.RemoveAll(stagingTargetPath); err != nil { - return nil, status.Error(codes.Internal, err.Error()) + if stagingTargetPath == blockStagingPath { + if err = os.Remove(stagingTargetPath); err != nil { + return nil, status.Error(codes.Internal, err.Error()) + } } return &csi.NodeUnstageVolumeResponse{}, nil } @@ -391,8 +416,10 @@ unmount: return nil, err } - if err = os.RemoveAll(stagingTargetPath); err != nil { - return nil, status.Error(codes.Internal, err.Error()) + if stagingTargetPath == blockStagingPath { + if err = os.Remove(stagingTargetPath); err != nil { + return nil, status.Error(codes.Internal, err.Error()) + } } klog.Infof("rbd: successfully unmounted volume %s from %s", req.GetVolumeId(), stagingTargetPath) diff --git a/pkg/util/util.go b/pkg/util/util.go index 2a403c205..187dcf472 100644 --- a/pkg/util/util.go +++ b/pkg/util/util.go @@ -125,6 +125,14 @@ func CreateMountPoint(mountPath string) error { return os.MkdirAll(mountPath, 0750) } +// checkDirExists checks directory exists or not +func checkDirExists(p string) bool { + if _, err := os.Stat(p); os.IsNotExist(err) { + return false + } + return true +} + // IsMountPoint checks if the given path is mountpoint or not func IsMountPoint(p string) (bool, error) { dummyMount := mount.New("") diff --git a/pkg/util/validate.go b/pkg/util/validate.go index 9db09fa13..685e099ad 100644 --- a/pkg/util/validate.go +++ b/pkg/util/validate.go @@ -24,6 +24,11 @@ func ValidateNodeStageVolumeRequest(req *csi.NodeStageVolumeRequest) error { return status.Error(codes.InvalidArgument, "stage secrets cannot be nil or empty") } + // validate stagingpath exists + ok := checkDirExists(req.GetStagingTargetPath()) + if !ok { + return status.Error(codes.InvalidArgument, "staging path doesnot exists on node") + } return nil }