Merge pull request #102 from leakingtapan/subpath

Update to read subpath from volume handle

.travis.yml

@@ -6,7 +6,7 @@ env:
     - GO111MODULE=on
 
 go:
-  - "1.12.7"
+  - "1.13.4"
 
 before_install:
   - go get github.com/mattn/goveralls

Dockerfile

@@ -12,7 +12,7 @@
 # See the License for the specific language governing permissions and
 # limitations under the License.
 
-FROM golang:1.12.7-stretch as builder
+FROM golang:1.13.4-stretch as builder
 WORKDIR /go/src/github.com/kubernetes-sigs/aws-efs-csi-driver
 ADD . .
 RUN make

docs/README.md

@@ -80,7 +80,7 @@ Before the example, you need to:
 Please go through [CSI Spec](https://github.com/container-storage-interface/spec/blob/master/spec.md) and [General CSI driver development guideline](https://kubernetes-csi.github.io/docs/Development.html) to get some basic understanding of CSI driver before you start.
 
 ### Requirements
-* Golang 1.12.7+
+* Golang 1.13.4+
 
 ### Dependency
 Dependencies are managed through go module. To build the project, first turn on go mod using `export GO111MODULE=on`, to build the project run: `make`

pkg/driver/node.go

@@ -20,6 +20,7 @@ import (
 	"context"
 	"fmt"
 	"os"
+	"path"
 	"path/filepath"
 	"strings"
 
@@ -63,22 +64,37 @@ func (d *Driver) NodePublishVolume(ctx context.Context, req *csi.NodePublishVolu
 	}
 
 	// TODO when CreateVolume is implemented, it must use the same key names
-	path := "/"
+	subpath := "/"
 	volContext := req.GetVolumeContext()
 	for k, v := range volContext {
 		switch strings.ToLower(k) {
+		//Deprecated
 		case "path":
+			klog.Warning("Use of path under volumeAttributes is depracated. This field will be removed in future release")
 			if !filepath.IsAbs(v) {
 				return nil, status.Error(codes.InvalidArgument, fmt.Sprintf("Volume context property %q must be an absolute path", k))
 			}
-			path = filepath.Join(path, v)
+			subpath = filepath.Join(subpath, v)
 		default:
 			return nil, status.Error(codes.InvalidArgument, fmt.Sprintf("Volume context property %s not supported", k))
 		}
 	}
 
 	volumeId := req.GetVolumeId()
-	source := fmt.Sprintf("%s:%s", volumeId, path)
+	if !isValidFileSystemId(volumeId) {
+		return nil, status.Error(codes.InvalidArgument, fmt.Sprintf("volume ID %s is invalid", volumeId))
+	}
+
+	var source string
+	tokens := strings.Split(volumeId, ":")
+	if len(tokens) == 1 {
+		// fs-xxxxxx
+		source = fmt.Sprintf("%s:%s", volumeId, subpath)
+	} else if len(tokens) == 2 {
+		// fs-xxxxxx:/a/b/c
+		cleanPath := path.Clean(tokens[1])
+		source = fmt.Sprintf("%s:%s", tokens[0], cleanPath)
+	}
 
 	mountOptions := []string{}
 	if req.GetReadonly() {
@@ -181,3 +197,7 @@ func (d *Driver) isValidVolumeCapabilities(volCaps []*csi.VolumeCapability) bool
 	}
 	return foundAll
 }
+
+func isValidFileSystemId(filesystemId string) bool {
+	return strings.HasPrefix(filesystemId, "fs-")
+}

pkg/driver/node_test.go

@@ -31,7 +31,7 @@ func TestNodePublishVolume(t *testing.T) {
 	var (
 		endpoint   = "endpoint"
 		nodeID     = "nodeID"
-		volumeId   = "volumeId"
+		volumeId   = "fs-volumeId"
 		targetPath = "/target/path"
 		stdVolCap  = &csi.VolumeCapability{
 			AccessType: &csi.VolumeCapability_Mount{
@@ -174,6 +174,35 @@ func TestNodePublishVolume(t *testing.T) {
 				mockCtrl.Finish()
 			},
 		},
+		{
+			name: "success: normal with path in volume handle",
+			testFunc: func(t *testing.T) {
+				mockCtrl := gomock.NewController(t)
+				mockMounter := mocks.NewMockMounter(mockCtrl)
+				driver := &Driver{
+					endpoint: endpoint,
+					nodeID:   nodeID,
+					mounter:  mockMounter,
+				}
+				source := volumeId + ":/a/b"
+
+				ctx := context.Background()
+				req := &csi.NodePublishVolumeRequest{
+					VolumeId:         volumeId + ":/a/b/",
+					VolumeCapability: stdVolCap,
+					TargetPath:       targetPath,
+				}
+
+				mockMounter.EXPECT().MakeDir(gomock.Eq(targetPath)).Return(nil)
+				mockMounter.EXPECT().Mount(gomock.Eq(source), gomock.Eq(targetPath), gomock.Eq("efs"), gomock.Any()).Return(nil)
+				_, err := driver.NodePublishVolume(ctx, req)
+				if err != nil {
+					t.Fatalf("NodePublishVolume is failed: %v", err)
+				}
+
+				mockCtrl.Finish()
+			},
+		},
 		{
 			name: "fail: missing target path",
 			testFunc: func(t *testing.T) {
@@ -193,7 +222,7 @@ func TestNodePublishVolume(t *testing.T) {
 
 				_, err := driver.NodePublishVolume(ctx, req)
 				if err == nil {
-					t.Fatalf("NodePublishVolume is not failed: %v", err)
+					t.Fatalf("NodePublishVolume is not failed")
 				}
 
 				mockCtrl.Finish()
@@ -218,7 +247,7 @@ func TestNodePublishVolume(t *testing.T) {
 
 				_, err := driver.NodePublishVolume(ctx, req)
 				if err == nil {
-					t.Fatalf("NodePublishVolume is not failed: %v", err)
+					t.Fatalf("NodePublishVolume is not failed")
 				}
 
 				mockCtrl.Finish()
@@ -251,7 +280,7 @@ func TestNodePublishVolume(t *testing.T) {
 
 				_, err := driver.NodePublishVolume(ctx, req)
 				if err == nil {
-					t.Fatalf("NodePublishVolume is not failed: %v", err)
+					t.Fatalf("NodePublishVolume is not failed")
 				}
 
 				mockCtrl.Finish()
@@ -280,7 +309,7 @@ func TestNodePublishVolume(t *testing.T) {
 
 				_, err = driver.NodePublishVolume(ctx, req)
 				if err == nil {
-					t.Fatalf("NodePublishVolume is not failed: %v", err)
+					t.Fatalf("NodePublishVolume is not failed")
 				}
 
 				mockCtrl.Finish()
@@ -311,7 +340,7 @@ func TestNodePublishVolume(t *testing.T) {
 
 				_, err = driver.NodePublishVolume(ctx, req)
 				if err == nil {
-					t.Fatalf("NodePublishVolume is not failed: %v", err)
+					t.Fatalf("NodePublishVolume is not failed")
 				}
 
 				mockCtrl.Finish()
@@ -338,7 +367,7 @@ func TestNodePublishVolume(t *testing.T) {
 
 				_, err := driver.NodePublishVolume(ctx, req)
 				if err == nil {
-					t.Fatalf("NodePublishVolume is not failed: %v", err)
+					t.Fatalf("NodePublishVolume is not failed")
 				}
 
 				mockCtrl.Finish()
@@ -365,7 +394,33 @@ func TestNodePublishVolume(t *testing.T) {
 
 				_, err := driver.NodePublishVolume(ctx, req)
 				if err == nil {
-					t.Fatalf("NodePublishVolume is not failed: %v", err)
+					t.Fatalf("NodePublishVolume is not failed")
+				}
+
+				mockCtrl.Finish()
+			},
+		},
+		{
+			name: "fail: invalid filesystem ID",
+			testFunc: func(t *testing.T) {
+				mockCtrl := gomock.NewController(t)
+				mockMounter := mocks.NewMockMounter(mockCtrl)
+				driver := &Driver{
+					endpoint: endpoint,
+					nodeID:   nodeID,
+					mounter:  mockMounter,
+				}
+
+				ctx := context.Background()
+				req := &csi.NodePublishVolumeRequest{
+					VolumeId:         "invalid-id",
+					VolumeCapability: stdVolCap,
+					TargetPath:       targetPath,
+				}
+
+				_, err := driver.NodePublishVolume(ctx, req)
+				if err == nil {
+					t.Fatalf("NodePublishVolume is not failed")
 				}
 
 				mockCtrl.Finish()

test/e2e/e2e_test.go

@@ -47,6 +47,7 @@ var (
 )
 
 func init() {
+	testing.Init()
 	// k8s.io/kubernetes/test/e2e/framework requires env KUBECONFIG to be set
 	// it does not fall back to defaults
 	if os.Getenv(kubeconfigEnvVar) == "" {