Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Update to read subpath from volume handle #102

Merged
merged 2 commits into from
Nov 27, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion .travis.yml
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ env:
- GO111MODULE=on

go:
- "1.12.7"
- "1.13.4"

before_install:
- go get github.com/mattn/goveralls
Expand Down
2 changes: 1 addition & 1 deletion Dockerfile
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion docs/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -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`
Expand Down
26 changes: 23 additions & 3 deletions pkg/driver/node.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ import (
"context"
"fmt"
"os"
"path"
"path/filepath"
"strings"

Expand Down Expand Up @@ -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() {
Expand Down Expand Up @@ -181,3 +197,7 @@ func (d *Driver) isValidVolumeCapabilities(volCaps []*csi.VolumeCapability) bool
}
return foundAll
}

func isValidFileSystemId(filesystemId string) bool {
return strings.HasPrefix(filesystemId, "fs-")
}
71 changes: 63 additions & 8 deletions pkg/driver/node_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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{
Expand Down Expand Up @@ -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) {
Expand All @@ -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()
Expand All @@ -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()
Expand Down Expand Up @@ -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()
Expand Down Expand Up @@ -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()
Expand Down Expand Up @@ -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()
Expand All @@ -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()
Expand All @@ -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()
Expand Down
1 change: 1 addition & 0 deletions test/e2e/e2e_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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) == "" {
Expand Down