diff --git a/integrationtests/smb_test.go b/integrationtests/smb_test.go index 65a359d4..885dc3de 100644 --- a/integrationtests/smb_test.go +++ b/integrationtests/smb_test.go @@ -1,17 +1,98 @@ package integrationtests import ( + "context" "fmt" "io/ioutil" "math/rand" "os" "os/exec" "strings" + "testing" "time" - "testing" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + + fs "github.com/kubernetes-csi/csi-proxy/pkg/filesystem" + fsapi "github.com/kubernetes-csi/csi-proxy/pkg/filesystem/api" + "github.com/kubernetes-csi/csi-proxy/pkg/smb" + smbapi "github.com/kubernetes-csi/csi-proxy/pkg/smb/api" ) +func TestSmbAPIGroup(t *testing.T) { + t.Run("v1alpha1SmbTests", func(t *testing.T) { + v1alpha1SmbTests(t) + }) + t.Run("v1beta1SmbTests", func(t *testing.T) { + v1beta1SmbTests(t) + }) + t.Run("v1beta2SmbTests", func(t *testing.T) { + v1beta2SmbTests(t) + }) + t.Run("v1SmbTests", func(t *testing.T) { + v1SmbTests(t) + }) +} + +func TestSmb(t *testing.T) { + fsClient, err := fs.New(fsapi.New()) + require.Nil(t, err) + client, err := smb.New(smbapi.New(), fsClient) + require.Nil(t, err) + + username := randomString(5) + password := randomString(10) + "!" + sharePath := fmt.Sprintf("C:\\smbshare%s", randomString(5)) + smbShare := randomString(6) + + localPath := fmt.Sprintf("C:\\localpath%s", randomString(5)) + + if err = setupUser(username, password); err != nil { + t.Fatalf("TestSmbAPIGroup %v", err) + } + defer removeUser(t, username) + + if err = setupSmbShare(smbShare, sharePath, username); err != nil { + t.Fatalf("TestSmbAPIGroup %v", err) + } + defer removeSmbShare(t, smbShare) + + hostname, err := os.Hostname() + assert.Nil(t, err) + + username = "domain\\" + username + remotePath := "\\\\" + hostname + "\\" + smbShare + // simulate Mount SMB operations around staging a volume on a node + mountSmbShareReq := &smb.NewSmbGlobalMappingRequest{ + RemotePath: remotePath, + Username: username, + Password: password, + } + _, err = client.NewSmbGlobalMapping(context.Background(), mountSmbShareReq) + if err != nil { + t.Fatalf("TestSmbAPIGroup %v", err) + } + + err = getSmbGlobalMapping(remotePath) + assert.Nil(t, err) + + err = writeReadFile(remotePath) + assert.Nil(t, err) + + unmountSmbShareReq := &smb.RemoveSmbGlobalMappingRequest{ + RemotePath: remotePath, + } + _, err = client.RemoveSmbGlobalMapping(context.Background(), unmountSmbShareReq) + if err != nil { + t.Fatalf("TestSmbAPIGroup %v", err) + } + err = getSmbGlobalMapping(remotePath) + assert.NotNil(t, err) + err = writeReadFile(localPath) + assert.NotNil(t, err) +} + const letterset = "abcdefghijklmnopqrstuvwxyz" + "ABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789" var seededRand = rand.New(rand.NewSource(time.Now().UnixNano())) @@ -121,18 +202,3 @@ func writeReadFile(path string) error { } return nil } - -func TestSmbAPIGroup(t *testing.T) { - t.Run("v1alpha1SmbTests", func(t *testing.T) { - v1alpha1SmbTests(t) - }) - t.Run("v1beta1SmbTests", func(t *testing.T) { - v1beta1SmbTests(t) - }) - t.Run("v1beta2SmbTests", func(t *testing.T) { - v1beta2SmbTests(t) - }) - t.Run("v1SmbTests", func(t *testing.T) { - v1SmbTests(t) - }) -} diff --git a/pkg/smb/api/api.go b/pkg/smb/api/api.go new file mode 100644 index 00000000..ce21c40f --- /dev/null +++ b/pkg/smb/api/api.go @@ -0,0 +1,83 @@ +package api + +import ( + "fmt" + "strings" + + "github.com/kubernetes-csi/csi-proxy/pkg/utils" +) + +type API interface { + IsSmbMapped(remotePath string) (bool, error) + NewSmbLink(remotePath, localPath string) error + NewSmbGlobalMapping(remotePath, username, password string) error + RemoveSmbGlobalMapping(remotePath string) error +} + +type smbAPI struct{} + +var _ API = &smbAPI{} + +func New() API { + return smbAPI{} +} + +func (smbAPI) IsSmbMapped(remotePath string) (bool, error) { + cmdLine := `$(Get-SmbGlobalMapping -RemotePath $Env:smbremotepath -ErrorAction Stop).Status ` + cmdEnv := fmt.Sprintf("smbremotepath=%s", remotePath) + out, err := utils.RunPowershellCmd(cmdLine, cmdEnv) + if err != nil { + return false, fmt.Errorf("error checking smb mapping. cmd %s, output: %s, err: %v", remotePath, string(out), err) + } + + if len(out) == 0 || !strings.EqualFold(strings.TrimSpace(string(out)), "OK") { + return false, nil + } + return true, nil +} + +// NewSmbLink - creates a directory symbolic link to the remote share. +// The os.Symlink was having issue for cases where the destination was an SMB share - the container +// runtime would complain stating "Access Denied". Because of this, we had to perform +// this operation with powershell commandlet creating an directory softlink. +// Since os.Symlink is currently being used in working code paths, no attempt is made in +// alpha to merge the paths. +// TODO (for beta release): Merge the link paths - os.Symlink and Powershell link path. +func (smbAPI) NewSmbLink(remotePath, localPath string) error { + if !strings.HasSuffix(remotePath, "\\") { + // Golang has issues resolving paths mapped to file shares if they do not end in a trailing \ + // so add one if needed. + remotePath = remotePath + "\\" + } + + cmdLine := `New-Item -ItemType SymbolicLink $Env:smblocalPath -Target $Env:smbremotepath` + output, err := utils.RunPowershellCmd(cmdLine, fmt.Sprintf("smbremotepath=%s", remotePath), fmt.Sprintf("smblocalpath=%s", localPath)) + if err != nil { + return fmt.Errorf("error linking %s to %s. output: %s, err: %v", remotePath, localPath, string(output), err) + } + + return nil +} + +func (smbAPI) NewSmbGlobalMapping(remotePath, username, password string) error { + // use PowerShell Environment Variables to store user input string to prevent command line injection + // https://docs.microsoft.com/en-us/powershell/module/microsoft.powershell.core/about/about_environment_variables?view=powershell-5.1 + cmdLine := fmt.Sprintf(`$PWord = ConvertTo-SecureString -String $Env:smbpassword -AsPlainText -Force` + + `;$Credential = New-Object -TypeName System.Management.Automation.PSCredential -ArgumentList $Env:smbuser, $PWord` + + `;New-SmbGlobalMapping -RemotePath $Env:smbremotepath -Credential $Credential -RequirePrivacy $true`) + + if output, err := utils.RunPowershellCmd(cmdLine, fmt.Sprintf("smbuser=%s", username), + fmt.Sprintf("smbpassword=%s", password), + fmt.Sprintf("smbremotepath=%s", remotePath)); err != nil { + return fmt.Errorf("NewSmbGlobalMapping failed. output: %q, err: %v", string(output), err) + } + return nil +} + +func (smbAPI) RemoveSmbGlobalMapping(remotePath string) error { + cmd := `Remove-SmbGlobalMapping -RemotePath $Env:smbremotepath -Force` + if output, err := utils.RunPowershellCmd(cmd, fmt.Sprintf("smbremotepath=%s", remotePath)); err != nil { + return fmt.Errorf("UnmountSmbShare failed. output: %q, err: %v", string(output), err) + } + return nil +} diff --git a/pkg/smb/smb.go b/pkg/smb/smb.go new file mode 100644 index 00000000..4ac2ff5d --- /dev/null +++ b/pkg/smb/smb.go @@ -0,0 +1,150 @@ +package smb + +import ( + "context" + "fmt" + "strings" + + fs "github.com/kubernetes-csi/csi-proxy/pkg/filesystem" + smbapi "github.com/kubernetes-csi/csi-proxy/pkg/smb/api" + "k8s.io/klog/v2" +) + +type Smb struct { + hostAPI smbapi.API + fs fs.Interface +} + +type Interface interface { + NewSmbGlobalMapping(context.Context, *NewSmbGlobalMappingRequest) (*NewSmbGlobalMappingResponse, error) + RemoveSmbGlobalMapping(context.Context, *RemoveSmbGlobalMappingRequest) (*RemoveSmbGlobalMappingResponse, error) +} + +// check that Smb implements the Interface +var _ Interface = &Smb{} + +func normalizeWindowsPath(path string) string { + normalizedPath := strings.Replace(path, "/", "\\", -1) + return normalizedPath +} + +func getRootMappingPath(path string) (string, error) { + items := strings.Split(path, "\\") + parts := []string{} + for _, s := range items { + if len(s) > 0 { + parts = append(parts, s) + if len(parts) == 2 { + break + } + } + } + if len(parts) != 2 { + klog.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 + return strings.ToLower("\\\\" + parts[0] + "\\" + parts[1]), nil +} + +func New(hostAPI smbapi.API, fsClient fs.Interface) (*Smb, error) { + return &Smb{ + hostAPI: hostAPI, + fs: fsClient, + }, nil +} + +func (s *Smb) NewSmbGlobalMapping(context context.Context, request *NewSmbGlobalMappingRequest) (*NewSmbGlobalMappingResponse, error) { + klog.V(2).Infof("calling NewSmbGlobalMapping with remote path %q", request.RemotePath) + response := &NewSmbGlobalMappingResponse{} + remotePath := normalizeWindowsPath(request.RemotePath) + localPath := request.LocalPath + + if remotePath == "" { + klog.Errorf("remote path is empty") + return response, fmt.Errorf("remote path is empty") + } + + mappingPath, err := getRootMappingPath(remotePath) + if err != nil { + return response, err + } + + isMapped, err := s.hostAPI.IsSmbMapped(mappingPath) + if err != nil { + isMapped = false + } + + if isMapped { + klog.V(4).Infof("Remote %s already mapped. Validating...", mappingPath) + + validResp, err := s.fs.PathValid(context, &fs.PathValidRequest{Path: mappingPath}) + if err != nil { + klog.Warningf("PathValid(%s) failed with %v, ignore error", mappingPath, err) + } + + if !validResp.Valid { + 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", 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!", mappingPath) + err = s.hostAPI.NewSmbGlobalMapping(mappingPath, request.Username, request.Password) + if err != nil { + klog.Errorf("failed NewSmbGlobalMapping %v", err) + return response, err + } + } + + if len(localPath) != 0 { + klog.V(4).Infof("ValidatePathWindows: '%s'", localPath) + err = fs.ValidatePathWindows(localPath) + if err != nil { + klog.Errorf("failed validate plugin path %v", err) + return response, err + } + err = s.hostAPI.NewSmbLink(remotePath, localPath) + if err != nil { + klog.Errorf("failed NewSmbLink %v", err) + return response, fmt.Errorf("creating link %s to %s failed with error: %v", localPath, remotePath, err) + } + } + + klog.V(2).Infof("NewSmbGlobalMapping on remote path %q is completed", request.RemotePath) + return response, nil +} + +func (s *Smb) RemoveSmbGlobalMapping(context context.Context, request *RemoveSmbGlobalMappingRequest) (*RemoveSmbGlobalMappingResponse, error) { + klog.V(2).Infof("calling RemoveSmbGlobalMapping with remote path %q", request.RemotePath) + response := &RemoveSmbGlobalMappingResponse{} + remotePath := normalizeWindowsPath(request.RemotePath) + + if remotePath == "" { + klog.Errorf("remote path is empty") + return response, fmt.Errorf("remote path is empty") + } + + mappingPath, err := getRootMappingPath(remotePath) + if err != nil { + return response, err + } + + 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 +} diff --git a/pkg/smb/smb_test.go b/pkg/smb/smb_test.go new file mode 100644 index 00000000..03fe635f --- /dev/null +++ b/pkg/smb/smb_test.go @@ -0,0 +1,152 @@ +package smb + +import ( + "context" + "testing" + + fs "github.com/kubernetes-csi/csi-proxy/pkg/filesystem" + fsapi "github.com/kubernetes-csi/csi-proxy/pkg/filesystem/api" + smbapi "github.com/kubernetes-csi/csi-proxy/pkg/smb/api" +) + +type fakeSmbAPI struct{} + +var _ smbapi.API = &fakeSmbAPI{} + +func (fakeSmbAPI) NewSmbGlobalMapping(remotePath, username, password string) error { + return nil +} + +func (fakeSmbAPI) RemoveSmbGlobalMapping(remotePath string) error { + return nil +} + +func (fakeSmbAPI) IsSmbMapped(remotePath string) (bool, error) { + return false, nil +} + +func (fakeSmbAPI) NewSmbLink(remotePath, localPath string) error { + return nil +} + +type fakeFileSystemAPI struct{} + +var _ fsapi.API = &fakeFileSystemAPI{} + +func (fakeFileSystemAPI) PathExists(path string) (bool, error) { + return true, nil +} +func (fakeFileSystemAPI) PathValid(path string) (bool, error) { + return true, nil +} +func (fakeFileSystemAPI) Mkdir(path string) error { + return nil +} +func (fakeFileSystemAPI) Rmdir(path string, force bool) error { + return nil +} +func (fakeFileSystemAPI) RmdirContents(path string) error { + return nil +} +func (fakeFileSystemAPI) CreateSymlink(tgt string, src string) error { + return nil +} + +func (fakeFileSystemAPI) IsSymlink(path string) (bool, error) { + return true, nil +} + +func TestNewSmbGlobalMapping(t *testing.T) { + testCases := []struct { + remote string + local string + username string + password string + expectError bool + }{ + { + remote: "", + username: "", + password: "", + expectError: true, + }, + { + remote: "\\\\hostname\\path", + username: "", + password: "", + expectError: false, + }, + } + fsClient, err := fs.New(&fakeFileSystemAPI{}) + if err != nil { + t.Fatalf("FileSystem client could not be initialized for testing: %v", err) + } + + client, err := New(&fakeSmbAPI{}, fsClient) + if err != nil { + t.Fatalf("Smb client could not be initialized for testing: %v", err) + } + for _, tc := range testCases { + req := &NewSmbGlobalMappingRequest{ + LocalPath: tc.local, + RemotePath: tc.remote, + Username: tc.username, + Password: tc.password, + } + _, err := client.NewSmbGlobalMapping(context.TODO(), req) + if tc.expectError && err == nil { + t.Errorf("Expected error but NewSmbGlobalMapping returned a nil error") + } + if !tc.expectError && err != nil { + t.Errorf("Expected no errors but NewSmbGlobalMapping returned error: %v", err) + } + } +} + +func TestGetRootMappingPath(t *testing.T) { + testCases := []struct { + remote string + expectResult string + expectError bool + }{ + { + remote: "", + expectResult: "", + expectError: true, + }, + { + remote: "hostname", + expectResult: "", + 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 tc.expectResult != result { + t.Errorf("Expected (%s) but getRootMappingPath returned (%s)", tc.expectResult, result) + } + } + } +} diff --git a/pkg/smb/types.go b/pkg/smb/types.go new file mode 100644 index 00000000..d38ab64a --- /dev/null +++ b/pkg/smb/types.go @@ -0,0 +1,20 @@ +package smb + +type NewSmbGlobalMappingRequest struct { + RemotePath string + LocalPath string + Username string + Password string +} + +type NewSmbGlobalMappingResponse struct { + // Intentionally empty. +} + +type RemoveSmbGlobalMappingRequest struct { + RemotePath string +} + +type RemoveSmbGlobalMappingResponse struct { + // Intentionally empty. +}