Skip to content

Commit

Permalink
Merge pull request #258 from alexander-ding/enh/rename-api-groups
Browse files Browse the repository at this point in the history 
Rename Smb to SMB
  • Loading branch information
@k8s-ci-robot
k8s-ci-robot authored Oct 18, 2022
2 parents 5c1a742 + d7d054f commit 9a01a14
Show file tree
Hide file tree
Showing 5 changed files with 76 additions and 76 deletions.
38 changes: 19 additions & 19 deletions integrationtests/smb_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ import (
smbapi "github.com/kubernetes-csi/csi-proxy/pkg/smb/api"
)

func TestSmb(t *testing.T) {
func TestSMB(t *testing.T) {
fsClient, err := fs.New(fsapi.New())
require.Nil(t, err)
client, err := smb.New(smbapi.New(), fsClient)
Expand All @@ -34,45 +34,45 @@ func TestSmb(t *testing.T) {
localPath := fmt.Sprintf("C:\\localpath%s", randomString(5))

if err = setupUser(username, password); err != nil {
t.Fatalf("TestSmbAPIGroup %v", err)
t.Fatalf("TestSMBAPIGroup %v", err)
}
defer removeUser(t, username)

if err = setupSmbShare(smbShare, sharePath, username); err != nil {
t.Fatalf("TestSmbAPIGroup %v", err)
if err = setupSMBShare(smbShare, sharePath, username); err != nil {
t.Fatalf("TestSMBAPIGroup %v", err)
}
defer removeSmbShare(t, smbShare)
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{
mountSMBShareReq := &smb.NewSMBGlobalMappingRequest{
RemotePath: remotePath,
Username: username,
Password: password,
}
_, err = client.NewSmbGlobalMapping(context.Background(), mountSmbShareReq)
_, err = client.NewSMBGlobalMapping(context.Background(), mountSMBShareReq)
if err != nil {
t.Fatalf("TestSmbAPIGroup %v", err)
t.Fatalf("TestSMBAPIGroup %v", err)
}

err = getSmbGlobalMapping(remotePath)
err = getSMBGlobalMapping(remotePath)
assert.Nil(t, err)

err = writeReadFile(remotePath)
assert.Nil(t, err)

unmountSmbShareReq := &smb.RemoveSmbGlobalMappingRequest{
unmountSMBShareReq := &smb.RemoveSMBGlobalMappingRequest{
RemotePath: remotePath,
}
_, err = client.RemoveSmbGlobalMapping(context.Background(), unmountSmbShareReq)
_, err = client.RemoveSMBGlobalMapping(context.Background(), unmountSMBShareReq)
if err != nil {
t.Fatalf("TestSmbAPIGroup %v", err)
t.Fatalf("TestSMBAPIGroup %v", err)
}
err = getSmbGlobalMapping(remotePath)
err = getSMBGlobalMapping(remotePath)
assert.NotNil(t, err)
err = writeReadFile(localPath)
assert.NotNil(t, err)
Expand Down Expand Up @@ -118,9 +118,9 @@ func removeUser(t *testing.T, username string) {
}
}

func setupSmbShare(shareName, localPath, username string) error {
func setupSMBShare(shareName, localPath, username string) error {
if err := os.MkdirAll(localPath, 0755); err != nil {
return fmt.Errorf("setupSmbShare failed to create local path %q: %v", localPath, err)
return fmt.Errorf("setupSMBShare failed to create local path %q: %v", localPath, err)
}
cmdLine := fmt.Sprintf(`New-SMBShare -Name $Env:sharename -Path $Env:path -fullaccess $Env:username`)
cmd := exec.Command("powershell", "/c", cmdLine)
Expand All @@ -129,24 +129,24 @@ func setupSmbShare(shareName, localPath, username string) error {
fmt.Sprintf("path=%s", localPath),
fmt.Sprintf("username=%s", username))
if output, err := cmd.CombinedOutput(); err != nil {
return fmt.Errorf("setupSmbShare failed: %v, output: %q", err, string(output))
return fmt.Errorf("setupSMBShare failed: %v, output: %q", err, string(output))
}

return nil
}

func removeSmbShare(t *testing.T, shareName string) {
func removeSMBShare(t *testing.T, shareName string) {
cmdLine := fmt.Sprintf(`Remove-SMBShare -Name $Env:sharename -Force`)
cmd := exec.Command("powershell", "/c", cmdLine)
cmd.Env = append(os.Environ(),
fmt.Sprintf("sharename=%s", shareName))
if output, err := cmd.CombinedOutput(); err != nil {
t.Fatalf("setupSmbShare failed: %v, output: %q", err, string(output))
t.Fatalf("setupSMBShare failed: %v, output: %q", err, string(output))
}
return
}

func getSmbGlobalMapping(remotePath string) error {
func getSMBGlobalMapping(remotePath 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(`(Get-SmbGlobalMapping -RemotePath $Env:smbremotepath).Status`)
Expand Down
24 changes: 12 additions & 12 deletions pkg/smb/api/api.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -8,10 +8,10 @@ import (
)

type API interface {
IsSmbMapped(remotePath string) (bool, error)
NewSmbLink(remotePath, localPath string) error
NewSmbGlobalMapping(remotePath, username, password string) error
RemoveSmbGlobalMapping(remotePath string) error
IsSMBMapped(remotePath string) (bool, error)
NewSMBLink(remotePath, localPath string) error
NewSMBGlobalMapping(remotePath, username, password string) error
RemoveSMBGlobalMapping(remotePath string) error
}

type smbAPI struct{}
Expand All @@ -22,12 +22,12 @@ func New() API {
return smbAPI{}
}

func (smbAPI) IsSmbMapped(remotePath string) (bool, error) {
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)
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") {
Expand All @@ -36,14 +36,14 @@ func (smbAPI) IsSmbMapped(remotePath string) (bool, error) {
return true, nil
}

// NewSmbLink - creates a directory symbolic link to the remote share.
// 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 {
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.
Expand All @@ -59,7 +59,7 @@ func (smbAPI) NewSmbLink(remotePath, localPath string) error {
return nil
}

func (smbAPI) NewSmbGlobalMapping(remotePath, username, password string) error {
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` +
Expand All @@ -69,15 +69,15 @@ func (smbAPI) NewSmbGlobalMapping(remotePath, username, password string) error {
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 fmt.Errorf("NewSMBGlobalMapping failed. output: %q, err: %v", string(output), err)
}
return nil
}

func (smbAPI) RemoveSmbGlobalMapping(remotePath string) error {
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 fmt.Errorf("UnmountSMBShare failed. output: %q, err: %v", string(output), err)
}
return nil
}
56 changes: 28 additions & 28 deletions pkg/smb/smb.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -10,21 +10,21 @@ import (
"k8s.io/klog/v2"
)

type Smb struct {
type SMB struct {
hostAPI smbapi.API
fs fs.Interface
}

type Interface interface {
// NewSmbGlobalMapping creates an SMB mapping on the SMB client to an SMB share.
NewSmbGlobalMapping(context.Context, *NewSmbGlobalMappingRequest) (*NewSmbGlobalMappingResponse, error)
// NewSMBGlobalMapping creates an SMB mapping on the SMB client to an SMB share.
NewSMBGlobalMapping(context.Context, *NewSMBGlobalMappingRequest) (*NewSMBGlobalMappingResponse, error)

// RemoveSmbGlobalMapping removes the SMB mapping to an SMB share.
RemoveSmbGlobalMapping(context.Context, *RemoveSmbGlobalMappingRequest) (*RemoveSmbGlobalMappingResponse, error)
// RemoveSMBGlobalMapping removes the SMB mapping to an SMB share.
RemoveSMBGlobalMapping(context.Context, *RemoveSMBGlobalMappingRequest) (*RemoveSMBGlobalMappingResponse, error)
}

// check that Smb implements the Interface
var _ Interface = &Smb{}
// check that SMB implements the Interface
var _ Interface = &SMB{}

func normalizeWindowsPath(path string) string {
normalizedPath := strings.Replace(path, "/", "\\", -1)
Expand All @@ -46,21 +46,21 @@ func getRootMappingPath(path string) (string, error) {
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
// 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{
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{}
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

Expand All @@ -74,7 +74,7 @@ func (s *Smb) NewSmbGlobalMapping(context context.Context, request *NewSmbGlobal
return response, err
}

isMapped, err := s.hostAPI.IsSmbMapped(mappingPath)
isMapped, err := s.hostAPI.IsSMBMapped(mappingPath)
if err != nil {
isMapped = false
}
Expand All @@ -89,9 +89,9 @@ func (s *Smb) NewSmbGlobalMapping(context context.Context, request *NewSmbGlobal

if !validResp.Valid {
klog.V(4).Infof("RemotePath %s is not valid, removing now", mappingPath)
err := s.hostAPI.RemoveSmbGlobalMapping(mappingPath)
err := s.hostAPI.RemoveSMBGlobalMapping(mappingPath)
if err != nil {
klog.Errorf("RemoveSmbGlobalMapping(%s) failed with %v", mappingPath, err)
klog.Errorf("RemoveSMBGlobalMapping(%s) failed with %v", mappingPath, err)
return response, err
}
isMapped = false
Expand All @@ -102,9 +102,9 @@ func (s *Smb) NewSmbGlobalMapping(context context.Context, request *NewSmbGlobal

if !isMapped {
klog.V(4).Infof("Remote %s not mapped. Mapping now!", mappingPath)
err = s.hostAPI.NewSmbGlobalMapping(mappingPath, request.Username, request.Password)
err = s.hostAPI.NewSMBGlobalMapping(mappingPath, request.Username, request.Password)
if err != nil {
klog.Errorf("failed NewSmbGlobalMapping %v", err)
klog.Errorf("failed NewSMBGlobalMapping %v", err)
return response, err
}
}
Expand All @@ -116,20 +116,20 @@ func (s *Smb) NewSmbGlobalMapping(context context.Context, request *NewSmbGlobal
klog.Errorf("failed validate plugin path %v", err)
return response, err
}
err = s.hostAPI.NewSmbLink(remotePath, localPath)
err = s.hostAPI.NewSMBLink(remotePath, localPath)
if err != nil {
klog.Errorf("failed NewSmbLink %v", err)
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)
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{}
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 == "" {
Expand All @@ -142,12 +142,12 @@ func (s *Smb) RemoveSmbGlobalMapping(context context.Context, request *RemoveSmb
return response, err
}

err = s.hostAPI.RemoveSmbGlobalMapping(mappingPath)
err = s.hostAPI.RemoveSMBGlobalMapping(mappingPath)
if err != nil {
klog.Errorf("failed RemoveSmbGlobalMapping %v", err)
klog.Errorf("failed RemoveSMBGlobalMapping %v", err)
return response, err
}

klog.V(2).Infof("RemoveSmbGlobalMapping on remote path %q is completed", request.RemotePath)
klog.V(2).Infof("RemoveSMBGlobalMapping on remote path %q is completed", request.RemotePath)
return response, nil
}
26 changes: 13 additions & 13 deletions pkg/smb/smb_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -9,23 +9,23 @@ import (
smbapi "github.com/kubernetes-csi/csi-proxy/pkg/smb/api"
)

type fakeSmbAPI struct{}
type fakeSMBAPI struct{}

var _ smbapi.API = &fakeSmbAPI{}
var _ smbapi.API = &fakeSMBAPI{}

func (fakeSmbAPI) NewSmbGlobalMapping(remotePath, username, password string) error {
func (fakeSMBAPI) NewSMBGlobalMapping(remotePath, username, password string) error {
return nil
}

func (fakeSmbAPI) RemoveSmbGlobalMapping(remotePath string) error {
func (fakeSMBAPI) RemoveSMBGlobalMapping(remotePath string) error {
return nil
}

func (fakeSmbAPI) IsSmbMapped(remotePath string) (bool, error) {
func (fakeSMBAPI) IsSMBMapped(remotePath string) (bool, error) {
return false, nil
}

func (fakeSmbAPI) NewSmbLink(remotePath, localPath string) error {
func (fakeSMBAPI) NewSMBLink(remotePath, localPath string) error {
return nil
}

Expand Down Expand Up @@ -56,7 +56,7 @@ func (fakeFileSystemAPI) IsSymlink(path string) (bool, error) {
return true, nil
}

func TestNewSmbGlobalMapping(t *testing.T) {
func TestNewSMBGlobalMapping(t *testing.T) {
testCases := []struct {
remote string
local string
Expand All @@ -82,23 +82,23 @@ func TestNewSmbGlobalMapping(t *testing.T) {
t.Fatalf("FileSystem client could not be initialized for testing: %v", err)
}

client, err := New(&fakeSmbAPI{}, fsClient)
client, err := New(&fakeSMBAPI{}, fsClient)
if err != nil {
t.Fatalf("Smb client could not be initialized for testing: %v", err)
t.Fatalf("SMB client could not be initialized for testing: %v", err)
}
for _, tc := range testCases {
req := &NewSmbGlobalMappingRequest{
req := &NewSMBGlobalMappingRequest{
LocalPath: tc.local,
RemotePath: tc.remote,
Username: tc.username,
Password: tc.password,
}
_, err := client.NewSmbGlobalMapping(context.TODO(), req)
_, err := client.NewSMBGlobalMapping(context.TODO(), req)
if tc.expectError && err == nil {
t.Errorf("Expected error but NewSmbGlobalMapping returned a nil error")
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)
t.Errorf("Expected no errors but NewSMBGlobalMapping returned error: %v", err)
}
}
}
Expand Down
Loading

0 comments on commit 9a01a14

Please sign in to comment.