From 856398075589952ee1240b27a283f235d3bdc033 Mon Sep 17 00:00:00 2001 From: vitaliy-leschenko Date: Fri, 29 Jul 2022 19:39:59 +0300 Subject: [PATCH 01/10] Fix path for a smb global mapping to allow mount the share more that once --- .gitignore | 1 + pkg/server/smb/server.go | 41 +++++++++++++++++++++++++++++++--------- 2 files changed, 33 insertions(+), 9 deletions(-) diff --git a/.gitignore b/.gitignore index 3dafce90..301a677c 100644 --- a/.gitignore +++ b/.gitignore @@ -1,3 +1,4 @@ /bin/ settings.json integrationtests/integrationtests.test.exe +.vs/ \ No newline at end of file diff --git a/pkg/server/smb/server.go b/pkg/server/smb/server.go index 6340270c..188b48de 100644 --- a/pkg/server/smb/server.go +++ b/pkg/server/smb/server.go @@ -25,6 +25,20 @@ func normalizeWindowsPath(path string) string { return normalizedPath } +func normalizeMappingPath(path string) string { + items := strings.Split(path, "\\") + parts := []string{} + for _, s := range items { + if len(s) > 0 { + parts = append(parts, s) + if len(parts) == 2 { + break + } + } + } + return strings.ToLower("\\\\" + parts[0] + "\\" + parts[1]) +} + func NewServer(hostAPI smb.API, fsServer *fsserver.Server) (*Server, error) { return &Server{ hostAPI: hostAPI, @@ -43,31 +57,37 @@ func (s *Server) NewSmbGlobalMapping(context context.Context, request *internal. return response, fmt.Errorf("remote path is empty") } - isMapped, err := s.hostAPI.IsSmbMapped(remotePath) + mappingPath := normalizeMappingPath(remotePath) + + isMapped, err := s.hostAPI.IsSmbMapped(mappingPath) if err != nil { isMapped = false } if isMapped { - valid, err := s.fsServer.PathValid(context, remotePath) + klog.V(4).Infof("Remote %s already mapped. Validating...", mappingPath) + + valid, err := s.fsServer.PathValid(context, mappingPath) if err != nil { - klog.Warningf("PathValid(%s) failed with %v, ignore error", remotePath, err) + klog.Warningf("PathValid(%s) failed with %v, ignore error", mappingPath, err) } if !valid { - klog.V(4).Infof("RemotePath %s is not valid, removing now", remotePath) - err := s.hostAPI.RemoveSmbGlobalMapping(remotePath) + klog.V(4).Infof("RemotePath %s is not valid, removing now", mappingPath) + err := s.hostAPI.RemoveSmbGlobalMapping(mappingPath) if err != nil { - klog.Errorf("RemoveSmbGlobalMapping(%s) failed with %v", remotePath, err) + klog.Errorf("RemoveSmbGlobalMapping(%s) failed with %v", mappingPath, err) return response, err } isMapped = false + } else { + klog.V(4).Infof("RemotePath %s is valid", mappingPath) } } if !isMapped { - klog.V(4).Infof("Remote %s not mapped. Mapping now!", remotePath) - err := s.hostAPI.NewSmbGlobalMapping(remotePath, request.Username, request.Password) + klog.V(4).Infof("Remote %s not mapped. Mapping now!", mappingPath) + err := s.hostAPI.NewSmbGlobalMapping(mappingPath, request.Username, request.Password) if err != nil { klog.Errorf("failed NewSmbGlobalMapping %v", err) return response, err @@ -75,6 +95,7 @@ func (s *Server) NewSmbGlobalMapping(context context.Context, request *internal. } if len(localPath) != 0 { + klog.V(4).Infof("ValidatePluginPath: '%s'", localPath) err = s.fsServer.ValidatePluginPath(localPath) if err != nil { klog.Errorf("failed validate plugin path %v", err) @@ -101,11 +122,13 @@ func (s *Server) RemoveSmbGlobalMapping(context context.Context, request *intern return response, fmt.Errorf("remote path is empty") } - err := s.hostAPI.RemoveSmbGlobalMapping(remotePath) + mappingPath := normalizeMappingPath(remotePath) + err := s.hostAPI.RemoveSmbGlobalMapping(mappingPath) if err != nil { klog.Errorf("failed RemoveSmbGlobalMapping %v", err) return response, err } + klog.V(2).Infof("RemoveSmbGlobalMapping on remote path %q is completed", request.RemotePath) return response, nil } From 5c1def8e06b6f70885a158bcfd8d63566a08af66 Mon Sep 17 00:00:00 2001 From: vitaliy-leschenko Date: Mon, 1 Aug 2022 17:31:03 +0300 Subject: [PATCH 02/10] add unit tests --- pkg/server/smb/server.go | 19 ++++++++++--- pkg/server/smb/server_test.go | 50 ++++++++++++++++++++++++++++++++++- 2 files changed, 64 insertions(+), 5 deletions(-) diff --git a/pkg/server/smb/server.go b/pkg/server/smb/server.go index 188b48de..48a8bc4f 100644 --- a/pkg/server/smb/server.go +++ b/pkg/server/smb/server.go @@ -25,7 +25,7 @@ func normalizeWindowsPath(path string) string { return normalizedPath } -func normalizeMappingPath(path string) string { +func getRootMappingPath(path string) (string, error) { items := strings.Split(path, "\\") parts := []string{} for _, s := range items { @@ -36,7 +36,13 @@ func normalizeMappingPath(path string) string { } } } - return strings.ToLower("\\\\" + parts[0] + "\\" + parts[1]) + if len(parts) != 2 { + klog.Errorf("remote path (%s) is invalid", path) + return nil, fmt.Errorf("remote path (%s) is invalid", path) + } + // parts[0] is a smb host name + // parts[1] is a smb share name + return strings.ToLower("\\\\" + parts[0] + "\\" + parts[1]), nil } func NewServer(hostAPI smb.API, fsServer *fsserver.Server) (*Server, error) { @@ -57,7 +63,9 @@ func (s *Server) NewSmbGlobalMapping(context context.Context, request *internal. return response, fmt.Errorf("remote path is empty") } - mappingPath := normalizeMappingPath(remotePath) + if mappingPath, err := getRootMappingPath(remotePath); err != nil { + return response, err; + } isMapped, err := s.hostAPI.IsSmbMapped(mappingPath) if err != nil { @@ -122,7 +130,10 @@ func (s *Server) RemoveSmbGlobalMapping(context context.Context, request *intern return response, fmt.Errorf("remote path is empty") } - mappingPath := normalizeMappingPath(remotePath) + if mappingPath, err := getRootMappingPath(remotePath); err != nil { + return response, err; + } + err := s.hostAPI.RemoveSmbGlobalMapping(mappingPath) if err != nil { klog.Errorf("failed RemoveSmbGlobalMapping %v", err) diff --git a/pkg/server/smb/server_test.go b/pkg/server/smb/server_test.go index e2b2e649..84bb6516 100644 --- a/pkg/server/smb/server_test.go +++ b/pkg/server/smb/server_test.go @@ -79,7 +79,7 @@ func TestNewSmbGlobalMapping(t *testing.T) { expectError: true, }, { - remote: "\\test\\path", + remote: "\\\\hostname\\path", username: "", password: "", version: v1, @@ -111,3 +111,51 @@ func TestNewSmbGlobalMapping(t *testing.T) { } } } + +func TestGetRootMappingPath(t *testing.T) { + testCases := []struct { + remote string + expectResult string + expectError bool + }{ + { + remote: "", + expectResult: nil, + expectError: true, + }, + { + remote: "hostname", + expectResult: nil, + expectError: true, + }, + { + remote: "\\\\hostname\\path", + expectResult: "\\\\hostname\\path", + expectError: false, + }, + { + remote: "\\\\hostname\\path\\", + expectResult: "\\\\hostname\\path", + expectError: false, + }, + { + remote: "\\\\hostname\\path\\subpath", + expectResult: "\\\\hostname\\path", + expectError: false, + }, + } + for _, tc := range testCases { + result, err := getRootMappingPath(tc.remote) + if tc.expectError && err == nil { + t.Errorf("Expected error but getRootMappingPath returned a nil error") + } + if !tc.expectError { + if err != nil { + t.Errorf("Expected no errors but getRootMappingPath returned error: %v", err) + } + if expectResult != result { + t.Errorf("Expected (%s) but getRootMappingPath returned (%s)", expectResult, result) + } + } + } +} \ No newline at end of file From e20a78f0ac0ca9ac4db5792a4ab17b9339e7b2d7 Mon Sep 17 00:00:00 2001 From: vitaliy-leschenko Date: Mon, 1 Aug 2022 17:34:51 +0300 Subject: [PATCH 03/10] fix build --- pkg/server/smb/server.go | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/pkg/server/smb/server.go b/pkg/server/smb/server.go index 48a8bc4f..b83c7d2c 100644 --- a/pkg/server/smb/server.go +++ b/pkg/server/smb/server.go @@ -63,7 +63,8 @@ func (s *Server) NewSmbGlobalMapping(context context.Context, request *internal. return response, fmt.Errorf("remote path is empty") } - if mappingPath, err := getRootMappingPath(remotePath); err != nil { + mappingPath, err := getRootMappingPath(remotePath) + if err != nil { return response, err; } @@ -130,7 +131,8 @@ func (s *Server) RemoveSmbGlobalMapping(context context.Context, request *intern return response, fmt.Errorf("remote path is empty") } - if mappingPath, err := getRootMappingPath(remotePath); err != nil { + mappingPath, err := getRootMappingPath(remotePath) + if err != nil { return response, err; } From 82aaeb3b2c520f37203e124a2d4e3fe8f4d871b6 Mon Sep 17 00:00:00 2001 From: vitaliy-leschenko Date: Mon, 1 Aug 2022 17:38:48 +0300 Subject: [PATCH 04/10] fix tests --- pkg/server/smb/server_test.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/pkg/server/smb/server_test.go b/pkg/server/smb/server_test.go index 84bb6516..3fcff15e 100644 --- a/pkg/server/smb/server_test.go +++ b/pkg/server/smb/server_test.go @@ -120,12 +120,12 @@ func TestGetRootMappingPath(t *testing.T) { }{ { remote: "", - expectResult: nil, + expectResult: "", expectError: true, }, { remote: "hostname", - expectResult: nil, + expectResult: "", expectError: true, }, { From db174604ab47e5a3597b8be5d60b39f6b0cecc90 Mon Sep 17 00:00:00 2001 From: vitaliy-leschenko Date: Mon, 1 Aug 2022 17:41:34 +0300 Subject: [PATCH 05/10] fix tests --- pkg/server/smb/server.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/server/smb/server.go b/pkg/server/smb/server.go index b83c7d2c..2e05e071 100644 --- a/pkg/server/smb/server.go +++ b/pkg/server/smb/server.go @@ -38,7 +38,7 @@ func getRootMappingPath(path string) (string, error) { } if len(parts) != 2 { klog.Errorf("remote path (%s) is invalid", path) - return nil, fmt.Errorf("remote path (%s) is invalid", path) + return "", fmt.Errorf("remote path (%s) is invalid", path) } // parts[0] is a smb host name // parts[1] is a smb share name From c2b22a7528187c2eb755bc7e6535fa1f0f34e345 Mon Sep 17 00:00:00 2001 From: vitaliy-leschenko Date: Mon, 1 Aug 2022 17:44:54 +0300 Subject: [PATCH 06/10] fix build --- pkg/server/smb/server.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/server/smb/server.go b/pkg/server/smb/server.go index 2e05e071..173c357a 100644 --- a/pkg/server/smb/server.go +++ b/pkg/server/smb/server.go @@ -136,7 +136,7 @@ func (s *Server) RemoveSmbGlobalMapping(context context.Context, request *intern return response, err; } - err := s.hostAPI.RemoveSmbGlobalMapping(mappingPath) + err = s.hostAPI.RemoveSmbGlobalMapping(mappingPath) if err != nil { klog.Errorf("failed RemoveSmbGlobalMapping %v", err) return response, err From 2476fe985504369624dc1478f233c5366cdfc9e8 Mon Sep 17 00:00:00 2001 From: vitaliy-leschenko Date: Mon, 1 Aug 2022 17:50:04 +0300 Subject: [PATCH 07/10] fix build --- pkg/server/smb/server.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/pkg/server/smb/server.go b/pkg/server/smb/server.go index 173c357a..76a74598 100644 --- a/pkg/server/smb/server.go +++ b/pkg/server/smb/server.go @@ -65,7 +65,7 @@ func (s *Server) NewSmbGlobalMapping(context context.Context, request *internal. mappingPath, err := getRootMappingPath(remotePath) if err != nil { - return response, err; + return response, err } isMapped, err := s.hostAPI.IsSmbMapped(mappingPath) @@ -133,7 +133,7 @@ func (s *Server) RemoveSmbGlobalMapping(context context.Context, request *intern mappingPath, err := getRootMappingPath(remotePath) if err != nil { - return response, err; + return response, err } err = s.hostAPI.RemoveSmbGlobalMapping(mappingPath) From 0cbab41fe8990ec288e456f92e1d2babea6993ef Mon Sep 17 00:00:00 2001 From: vitaliy-leschenko Date: Mon, 1 Aug 2022 17:51:11 +0300 Subject: [PATCH 08/10] fix build --- pkg/server/smb/server_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/server/smb/server_test.go b/pkg/server/smb/server_test.go index 3fcff15e..67c6d78f 100644 --- a/pkg/server/smb/server_test.go +++ b/pkg/server/smb/server_test.go @@ -150,7 +150,7 @@ func TestGetRootMappingPath(t *testing.T) { t.Errorf("Expected error but getRootMappingPath returned a nil error") } if !tc.expectError { - if err != nil { + if err != nil { t.Errorf("Expected no errors but getRootMappingPath returned error: %v", err) } if expectResult != result { From ce88358a4bd94d26b76adc477717d97ea3bbdab0 Mon Sep 17 00:00:00 2001 From: vitaliy-leschenko Date: Mon, 1 Aug 2022 17:54:29 +0300 Subject: [PATCH 09/10] fix formatting --- pkg/server/smb/server_test.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/pkg/server/smb/server_test.go b/pkg/server/smb/server_test.go index 67c6d78f..f274f808 100644 --- a/pkg/server/smb/server_test.go +++ b/pkg/server/smb/server_test.go @@ -155,7 +155,7 @@ func TestGetRootMappingPath(t *testing.T) { } if expectResult != result { t.Errorf("Expected (%s) but getRootMappingPath returned (%s)", expectResult, result) - } + } } } -} \ No newline at end of file +} From 0b7cc3d63f50fdeb8e4a8bc2a3a9ec9977026cf2 Mon Sep 17 00:00:00 2001 From: vitaliy-leschenko Date: Tue, 2 Aug 2022 21:19:15 +0300 Subject: [PATCH 10/10] fix tests+build --- pkg/server/smb/server_test.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/pkg/server/smb/server_test.go b/pkg/server/smb/server_test.go index f274f808..5706d0ca 100644 --- a/pkg/server/smb/server_test.go +++ b/pkg/server/smb/server_test.go @@ -153,8 +153,8 @@ func TestGetRootMappingPath(t *testing.T) { if err != nil { t.Errorf("Expected no errors but getRootMappingPath returned error: %v", err) } - if expectResult != result { - t.Errorf("Expected (%s) but getRootMappingPath returned (%s)", expectResult, result) + if tc.expectResult != result { + t.Errorf("Expected (%s) but getRootMappingPath returned (%s)", tc.expectResult, result) } } }