Skip to content

Commit

Permalink
Merge pull request kubernetes-csi#18 from kkmsft/misc_fixes
Browse files Browse the repository at this point in the history
Misc fixes
  • Loading branch information
k8s-ci-robot committed Dec 16, 2019
2 parents 3cd6943 + 5ad1bb9 commit 06d2e21
Show file tree
Hide file tree
Showing 12 changed files with 69 additions and 84 deletions.
1 change: 1 addition & 0 deletions .gitignore
@@ -1 +1,2 @@
/build/
settings.json
81 changes: 38 additions & 43 deletions client/api/filesystem/v1alpha1/api.pb.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

6 changes: 3 additions & 3 deletions cmd/csi-proxy-api-gen/generators/api_group_generated_gen.go
Expand Up @@ -23,7 +23,7 @@ func (g *apiGroupGeneratedGenerator) Filter(*generator.Context, *types.Type) boo
func (g *apiGroupGeneratedGenerator) Imports(*generator.Context) []string {
imports := []string{
"github.com/kubernetes-csi/csi-proxy/client/apiversion",
"github.com/kubernetes-csi/csi-proxy/internal/server",
"srvtypes \"github.com/kubernetes-csi/csi-proxy/internal/server/types\"",
g.groupDefinition.internalServerPkg(),
}

Expand All @@ -44,7 +44,7 @@ func (g *apiGroupGeneratedGenerator) Init(context *generator.Context, writer io.
// ensure the server defines all the required methods
var _ internal.ServerInterface = &Server{}
func (s *Server) VersionedAPIs() []*server.VersionedAPI {
func (s *Server) VersionedAPIs() []*srvtypes.VersionedAPI {
`, nil)

versions := make([]apiversion.Version, len(g.groupDefinition.versions))
Expand All @@ -59,7 +59,7 @@ func (s *Server) VersionedAPIs() []*server.VersionedAPI {
snippetWriter.Do("$.$Server := $.$.NewVersionedServer(s)\n", version)
}

snippetWriter.Do("\n\nreturn []*server.VersionedAPI{\n", nil)
snippetWriter.Do("\n\nreturn []*srvtypes.VersionedAPI{\n", nil)
for _, version := range versions {
snippetWriter.Do(`{
Group: name,
Expand Down
1 change: 1 addition & 0 deletions go.sum
Expand Up @@ -33,6 +33,7 @@ github.com/wk8/gengo v0.0.0-20191001015530-3d2530bfe606ffd99a90d70ef781861042e23
github.com/wk8/gengo v0.0.0-20191001015530-3d2530bfe606ffd99a90d70ef781861042e23a6f/go.mod h1:7+jn4yanRF/Ylvd5qyNPUq0VhvOMDhjXrViFtHMcHxI=
github.com/wk8/gengo v0.0.0-20191007012548-3d2530bfe606 h1:i5JAp+X9ISMaGt0R/9nsiFuNyYN0hXSK9KtphOk/r94=
github.com/wk8/gengo v0.0.0-20191007012548-3d2530bfe606/go.mod h1:7+jn4yanRF/Ylvd5qyNPUq0VhvOMDhjXrViFtHMcHxI=
golang.org/x/crypto v0.0.0-20190308221718-c2843e01d9a2 h1:VklqNMn3ovrHsnt90PveolxSbWFaJdECFbxSq0Mqo2M=
golang.org/x/crypto v0.0.0-20190308221718-c2843e01d9a2/go.mod h1:djNgcEr1/C05ACkg1iLfiJU5Ep61QUkGW8qpdssI0+w=
golang.org/x/lint v0.0.0-20190313153728-d0100b6bd8b3/go.mod h1:6SW0HCj/g11FgYtHlgUYUwCkIfeOF89ocIRzGO/8vkc=
golang.org/x/net v0.0.0-20190311183353-d8887717615a h1:oWX7TPOiFAMXLq8o0ikBYfCJVlRHBcsciT5bXOrH628=
Expand Down

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 0 additions & 1 deletion integrationtests/filesystem_test.go
Expand Up @@ -98,6 +98,5 @@ func TestFilesystemAPIGroup(t *testing.T) {

exists, err = pathExists(stagepath)
assert.False(t, exists, err)

})
}
3 changes: 2 additions & 1 deletion integrationtests/utils.go
Expand Up @@ -18,10 +18,11 @@ import (
"github.com/stretchr/testify/require"

"github.com/kubernetes-csi/csi-proxy/internal/server"
srvtypes "github.com/kubernetes-csi/csi-proxy/internal/server/types"
)

// startServer starts the proxy's GRPC servers, and returns a function to shut them down when done with testing
func startServer(t *testing.T, apiGroups ...server.APIGroup) func() {
func startServer(t *testing.T, apiGroups ...srvtypes.APIGroup) func() {
s := server.NewServer(apiGroups...)

listeningChan := make(chan interface{})
Expand Down
2 changes: 1 addition & 1 deletion internal/server/filesystem/api_group_generated.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

36 changes: 9 additions & 27 deletions internal/server/filesystem/server.go
Expand Up @@ -63,10 +63,7 @@ func isAbsWindows(path string) bool {
// for Windows check for C:\\.. prefix only
// UNC prefixes of the form \\ are not considered
// absolute in the context of CSI proxy
if absPathRegexWindows.MatchString(path) {
return true
}
return false
return absPathRegexWindows.MatchString(path)
}

func (s *Server) validatePathWindows(pathCtx internal.PathContext, path string) error {
Expand All @@ -76,53 +73,38 @@ func (s *Server) validatePathWindows(pathCtx internal.PathContext, path string)
} else if pathCtx == internal.POD {
prefix = s.kubeletPodPath
} else {
return fmt.Errorf("Invalid PathContext: %v", pathCtx)
return fmt.Errorf("invalid PathContext: %v", pathCtx)
}

pathlen := len(path)

if pathlen > utils.MAX_PATH_LENGTH_WINDOWS {
return fmt.Errorf("Path length %d exceeds maximum characters: %d", pathlen, utils.MAX_PATH_LENGTH_WINDOWS)
if pathlen > utils.MaxPathLengthWindows {
return fmt.Errorf("path length %d exceeds maximum characters: %d", pathlen, utils.MaxPathLengthWindows)
}

if pathlen > 0 && (path[0] == '\\') {
return fmt.Errorf("Invalid character \\ at begining of path: %s", path)
return fmt.Errorf("invalid character \\ at beginning of path: %s", path)
}

if isUNCPathWindows(path) {
return fmt.Errorf("Unsupported UNC path prefix: %s", path)
return fmt.Errorf("unsupported UNC path prefix: %s", path)
}

if containsInvalidCharactersWindows(path) {
return fmt.Errorf("Path contains invalid characters: %s", path)
return fmt.Errorf("path contains invalid characters: %s", path)
}

if !isAbsWindows(path) {
return fmt.Errorf("Not an absolute Windows path: %s", path)
return fmt.Errorf("not an absolute Windows path: %s", path)
}

if !strings.HasPrefix(path, prefix) {
return fmt.Errorf("Path: %s is not within context path: %s", path, prefix)
return fmt.Errorf("path: %s is not within context path: %s", path, prefix)
}

return nil
}

func (s *Server) abs(pathCtx internal.PathContext, path string) (string, error) {
if isAbsWindows(path) {
return path, nil
}
prefix := ""
if pathCtx == internal.PLUGIN {
prefix = s.kubeletCSIPluginsPath
} else if pathCtx == internal.POD {
prefix = s.kubeletPodPath
} else {
return "", fmt.Errorf("Invalid PathContext: %v", pathCtx)
}
return prefix + "\\" + path, nil
}

// PathExists checks if the given path exists on the host.
func (s *Server) PathExists(ctx context.Context, request *internal.PathExistsRequest, version apiversion.Version) (*internal.PathExistsResponse, error) {
err := s.validatePathWindows(request.Context, request.Path)
Expand Down
10 changes: 8 additions & 2 deletions internal/server/filesystem/server_test.go
Expand Up @@ -25,6 +25,9 @@ func (fakeFileSystemAPI) LinkPath(tgt string, src string) error {

func TestMkdirWindows(t *testing.T) {
v1alpha1, err := apiversion.NewVersion("v1alpha1")
if err != nil {
t.Fatalf("New version error: %v", err)
}
testCases := []struct {
name string
path string
Expand Down Expand Up @@ -139,13 +142,16 @@ func TestMkdirWindows(t *testing.T) {

func TestRmdirWindows(t *testing.T) {
v1alpha1, err := apiversion.NewVersion("v1alpha1")
if err != nil {
t.Fatalf("New version error: %v", err)
}
testCases := []struct {
name string
path string
pathCtx internal.PathContext
version apiversion.Version
expectError bool
force bool
force bool
}{
{
name: "path outside of pod context with pod context set",
Expand Down Expand Up @@ -241,7 +247,7 @@ func TestRmdirWindows(t *testing.T) {
req := &internal.RmdirRequest{
Path: tc.path,
Context: tc.pathCtx,
Force: tc.force,
Force: tc.force,
}
rmdirResponse, _ := srv.Rmdir(context.TODO(), req, tc.version)
if tc.expectError && rmdirResponse.Error == "" {
Expand Down
2 changes: 1 addition & 1 deletion internal/utils/utils.go
@@ -1,3 +1,3 @@
package utils

const MAX_PATH_LENGTH_WINDOWS = 260
const MaxPathLengthWindows = 260

0 comments on commit 06d2e21

Please sign in to comment.