From e9fb936418e2d4a47769a55931a9a9315ad7296e Mon Sep 17 00:00:00 2001 From: Jing Xu Date: Wed, 9 Jun 2021 15:49:58 -0700 Subject: [PATCH] Combine plugin and pod paths into one KubeletPath Instead of using pod and plugin paths, comebine them into one kubeletPath option --- cmd/csi-proxy/main.go | 9 +++--- go.mod | 2 ++ internal/server/filesystem/server.go | 35 ++++++++------------- internal/server/filesystem/server_test.go | 38 +++-------------------- vendor/modules.txt | 2 +- 5 files changed, 25 insertions(+), 61 deletions(-) diff --git a/cmd/csi-proxy/main.go b/cmd/csi-proxy/main.go index dddde9f8..0da9f728 100644 --- a/cmd/csi-proxy/main.go +++ b/cmd/csi-proxy/main.go @@ -23,10 +23,9 @@ import ( ) var ( - kubeletCSIPluginsPath = flag.String("kubelet-csi-plugins-path", `C:\var\lib\kubelet`, "Prefix path of the Kubelet plugin directory in the host file system") - kubeletPodPath = flag.String("kubelet-pod-path", `C:\var\lib\kubelet`, "Prefix path of the kubelet pod directory in the host file system") - windowsSvc = flag.Bool("windows-service", false, "Configure as a Windows Service") - service *handler + kubeletPath = flag.String("kubelet-path", `C:\var\lib\kubelet`, "Prefix path of the kubelet directory in the host file system") + windowsSvc = flag.Bool("windows-service", false, "Configure as a Windows Service") + service *handler ) type handler struct { @@ -61,7 +60,7 @@ func main() { // apiGroups returns the list of enabled API groups. func apiGroups() ([]srvtypes.APIGroup, error) { - fssrv, err := filesystemsrv.NewServer(*kubeletCSIPluginsPath, *kubeletPodPath, filesystemapi.New()) + fssrv, err := filesystemsrv.NewServer(*kubeletPath, filesystemapi.New()) if err != nil { return []srvtypes.APIGroup{}, err } diff --git a/go.mod b/go.mod index a4725fca..abfabfbe 100644 --- a/go.mod +++ b/go.mod @@ -17,6 +17,8 @@ replace ( require ( github.com/Microsoft/go-winio v0.4.16 + github.com/golang/protobuf v1.4.1 + // github.com/golang/protobuf v1.4.1 github.com/google/go-cmp v0.5.0 github.com/iancoleman/strcase v0.0.0-20190422225806-e506e3ef7365 github.com/kubernetes-csi/csi-proxy/client v0.0.0-00010101000000-000000000000 diff --git a/internal/server/filesystem/server.go b/internal/server/filesystem/server.go index b9ee115b..acf04473 100644 --- a/internal/server/filesystem/server.go +++ b/internal/server/filesystem/server.go @@ -14,9 +14,8 @@ import ( ) type Server struct { - kubeletCSIPluginsPath string - kubeletPodPath string - hostAPI filesystem.API + kubeletPath string + hostAPI filesystem.API } // check that Server fulfills internal.ServerInterface @@ -25,11 +24,10 @@ var _ internal.ServerInterface = &Server{} var invalidPathCharsRegexWindows = regexp.MustCompile(`["/\:\?\*|]`) var absPathRegexWindows = regexp.MustCompile(`^[a-zA-Z]:\\`) -func NewServer(kubeletCSIPluginsPath string, kubeletPodPath string, hostAPI filesystem.API) (*Server, error) { +func NewServer(kubeletPath string, hostAPI filesystem.API) (*Server, error) { return &Server{ - kubeletCSIPluginsPath: kubeletCSIPluginsPath, - kubeletPodPath: kubeletPodPath, - hostAPI: hostAPI, + kubeletPath: kubeletPath, + hostAPI: hostAPI, }, nil } @@ -70,18 +68,11 @@ func isAbsWindows(path string) bool { // from other parts of the library is that it seems internal.PLUGIN was not // usable from outside the internal path tree. func (s *Server) ValidatePluginPath(path string) error { - return s.validatePathWindows(internal.PLUGIN, path) + return s.validatePathWindows(path) } -func (s *Server) validatePathWindows(pathCtx internal.PathContext, path string) error { - prefix := "" - if pathCtx == internal.PLUGIN { - prefix = s.kubeletCSIPluginsPath - } else if pathCtx == internal.POD { - prefix = s.kubeletPodPath - } else { - return fmt.Errorf("invalid PathContext: %v", pathCtx) - } +func (s *Server) validatePathWindows(path string) error { + prefix := s.kubeletPath pathlen := len(path) @@ -115,7 +106,7 @@ func (s *Server) validatePathWindows(pathCtx internal.PathContext, path string) // 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) { klog.V(2).Infof("Request: PathExists with path=%q", request.Path) - err := s.validatePathWindows(request.Context, request.Path) + err := s.validatePathWindows(request.Path) if err != nil { klog.Errorf("failed validatePathWindows %v", err) return nil, err @@ -138,7 +129,7 @@ func (s *Server) PathValid(ctx context.Context, path string) (bool, error) { func (s *Server) Mkdir(ctx context.Context, request *internal.MkdirRequest, version apiversion.Version) (*internal.MkdirResponse, error) { klog.V(2).Infof("Request: Mkdir with path=%q", request.Path) - err := s.validatePathWindows(request.Context, request.Path) + err := s.validatePathWindows(request.Path) if err != nil { klog.Errorf("failed validatePathWindows %v", err) return nil, err @@ -154,7 +145,7 @@ func (s *Server) Mkdir(ctx context.Context, request *internal.MkdirRequest, vers func (s *Server) Rmdir(ctx context.Context, request *internal.RmdirRequest, version apiversion.Version) (*internal.RmdirResponse, error) { klog.V(2).Infof("Request: Rmdir with path=%q", request.Path) - err := s.validatePathWindows(request.Context, request.Path) + err := s.validatePathWindows(request.Path) if err != nil { klog.Errorf("failed validatePathWindows %v", err) return nil, err @@ -181,12 +172,12 @@ func (s *Server) LinkPath(ctx context.Context, request *internal.LinkPathRequest func (s *Server) CreateSymlink(ctx context.Context, request *internal.CreateSymlinkRequest, version apiversion.Version) (*internal.CreateSymlinkResponse, error) { klog.V(2).Infof("Request: CreateSymlink with targetPath=%q sourcePath=%q", request.TargetPath, request.SourcePath) - err := s.validatePathWindows(internal.POD, request.TargetPath) + err := s.validatePathWindows(request.TargetPath) if err != nil { klog.Errorf("failed validatePathWindows for target path %v", err) return nil, err } - err = s.validatePathWindows(internal.PLUGIN, request.SourcePath) + err = s.validatePathWindows(request.SourcePath) if err != nil { klog.Errorf("failed validatePathWindows for source path %v", err) return nil, err diff --git a/internal/server/filesystem/server_test.go b/internal/server/filesystem/server_test.go index 815cb011..f19640d5 100644 --- a/internal/server/filesystem/server_test.go +++ b/internal/server/filesystem/server_test.go @@ -41,104 +41,90 @@ func TestMkdirWindows(t *testing.T) { testCases := []struct { name string path string - pathCtx internal.PathContext version apiversion.Version expectError bool }{ { name: "path outside of pod context with pod context set", path: `C:\foo\bar`, - pathCtx: internal.POD, version: v1, expectError: true, }, { name: "path inside pod context with pod context set", path: `C:\var\lib\kubelet\pods\pv1`, - pathCtx: internal.POD, version: v1, expectError: false, }, { name: "path outside of plugin context with plugin context set", path: `C:\foo\bar`, - pathCtx: internal.PLUGIN, version: v1, expectError: true, }, { name: "path inside plugin context with plugin context set", path: `C:\var\lib\kubelet\plugins\pv1`, - pathCtx: internal.PLUGIN, version: v1, expectError: false, }, { name: "path with invalid character `:` beyond drive letter prefix", path: `C:\var\lib\kubelet\plugins\csi-plugin\pv1:foo`, - pathCtx: internal.PLUGIN, version: v1, expectError: true, }, { name: "path with invalid character `/`", path: `C:\var\lib\kubelet\pods\pv1/foo`, - pathCtx: internal.POD, version: v1, expectError: true, }, { name: "path with invalid character `*`", path: `C:\var\lib\kubelet\plugins\csi-plugin\pv1*foo`, - pathCtx: internal.PLUGIN, version: v1, expectError: true, }, { name: "path with invalid character `?`", path: `C:\var\lib\kubelet\pods\pv1?foo`, - pathCtx: internal.POD, version: v1, expectError: true, }, { name: "path with invalid character `|`", path: `C:\var\lib\kubelet\plugins\csi-plugin|pv1\foo`, - pathCtx: internal.PLUGIN, version: v1, expectError: true, }, { name: "path with invalid characters `..`", path: `C:\var\lib\kubelet\pods\pv1\..\..\..\system32`, - pathCtx: internal.POD, version: v1, expectError: true, }, { name: "path with invalid prefix `\\`", path: `\\csi-plugin\..\..\..\system32`, - pathCtx: internal.POD, version: v1, expectError: true, }, { name: "relative path", path: `pv1\foo`, - pathCtx: internal.POD, version: v1, expectError: true, }, } - srv, err := NewServer(`C:\var\lib\kubelet\plugins`, `C:\var\lib\kubelet\pods`, &fakeFileSystemAPI{}) + srv, err := NewServer(`C:\var\lib\kubelet`, &fakeFileSystemAPI{}) if err != nil { t.Fatalf("FileSystem Server could not be initialized for testing: %v", err) } for _, tc := range testCases { t.Logf("test case: %s", tc.name) req := &internal.MkdirRequest{ - Path: tc.path, - Context: tc.pathCtx, + Path: tc.path, } _, err := srv.Mkdir(context.TODO(), req, tc.version) if tc.expectError && err == nil { @@ -158,7 +144,6 @@ func TestRmdirWindows(t *testing.T) { testCases := []struct { name string path string - pathCtx internal.PathContext version apiversion.Version expectError bool force bool @@ -166,98 +151,85 @@ func TestRmdirWindows(t *testing.T) { { name: "path outside of pod context with pod context set", path: `C:\foo\bar`, - pathCtx: internal.POD, version: v1, expectError: true, }, { name: "path inside pod context with pod context set", path: `C:\var\lib\kubelet\pods\pv1`, - pathCtx: internal.POD, version: v1, expectError: false, }, { name: "path outside of plugin context with plugin context set", path: `C:\foo\bar`, - pathCtx: internal.PLUGIN, version: v1, expectError: true, }, { name: "path inside plugin context with plugin context set", path: `C:\var\lib\kubelet\plugins\pv1`, - pathCtx: internal.PLUGIN, version: v1, expectError: false, }, { name: "path with invalid character `:` beyond drive letter prefix", path: `C:\var\lib\kubelet\plugins\csi-plugin\pv1:foo`, - pathCtx: internal.PLUGIN, version: v1, expectError: true, }, { name: "path with invalid character `/`", path: `C:\var\lib\kubelet\pods\pv1/foo`, - pathCtx: internal.POD, version: v1, expectError: true, }, { name: "path with invalid character `*`", path: `C:\var\lib\kubelet\plugins\csi-plugin\pv1*foo`, - pathCtx: internal.PLUGIN, version: v1, expectError: true, }, { name: "path with invalid character `?`", path: `C:\var\lib\kubelet\pods\pv1?foo`, - pathCtx: internal.POD, version: v1, expectError: true, }, { name: "path with invalid character `|`", path: `C:\var\lib\kubelet\plugins\csi-plugin|pv1\foo`, - pathCtx: internal.PLUGIN, version: v1, expectError: true, }, { name: "path with invalid characters `..`", path: `C:\var\lib\kubelet\pods\pv1\..\..\..\system32`, - pathCtx: internal.POD, version: v1, expectError: true, }, { name: "path with invalid prefix `\\`", path: `\\csi-plugin\..\..\..\system32`, - pathCtx: internal.POD, version: v1, expectError: true, }, { name: "relative path", path: `pv1\foo`, - pathCtx: internal.POD, version: v1, expectError: true, }, } - srv, err := NewServer(`C:\var\lib\kubelet\plugins`, `C:\var\lib\kubelet\pods`, &fakeFileSystemAPI{}) + srv, err := NewServer(`C:\var\lib\kubelet`, &fakeFileSystemAPI{}) if err != nil { t.Fatalf("FileSystem Server could not be initialized for testing: %v", err) } for _, tc := range testCases { t.Logf("test case: %s", tc.name) req := &internal.RmdirRequest{ - Path: tc.path, - Context: tc.pathCtx, - Force: tc.force, + Path: tc.path, + Force: tc.force, } _, err := srv.Rmdir(context.TODO(), req, tc.version) if tc.expectError && err == nil { diff --git a/vendor/modules.txt b/vendor/modules.txt index 8292679d..b3e649fa 100644 --- a/vendor/modules.txt +++ b/vendor/modules.txt @@ -6,7 +6,7 @@ github.com/Microsoft/go-winio/pkg/guid github.com/davecgh/go-spew/spew # github.com/go-logr/logr v0.2.0 github.com/go-logr/logr -# github.com/golang/protobuf v1.4.1 +github.com/golang/protobuf v1.4.1 github.com/golang/protobuf/proto github.com/golang/protobuf/ptypes github.com/golang/protobuf/ptypes/any