fix: move defer up and add unit-tests for mount with "credentials=" option

Author: mpatlasov

pkg/smb/nodeserver_test.go

@@ -92,6 +92,11 @@ func TestNodeStageVolume(t *testing.T) {
 		passwordField: "test_password",
 		domainField:   "test_doamin",
 	}
+	secretsSpecial := map[string]string{
+		usernameField: "test_username",
+		passwordField: "test\"`,password",
+		domainField:   "test_doamin",
+	}
 
 	tests := []struct {
 		desc        string
@@ -182,6 +187,23 @@ func TestNodeStageVolume(t *testing.T) {
 					strings.Replace(testSource, "\\", "\\\\", -1), errorMountSensSource),
 			},
 		},
+		{
+			desc: "[Error] Failed SMB mount mocked by MountSensitive (password with special characters)",
+			req: &csi.NodeStageVolumeRequest{VolumeId: "vol_1##", StagingTargetPath: errorMountSensSource,
+				VolumeCapability: &stdVolCap,
+				VolumeContext:    volContext,
+				Secrets:          secretsSpecial},
+			skipOnWindows: true,
+			flakyWindowsErrorMessage: fmt.Sprintf("rpc error: code = Internal desc = volume(vol_1##) mount \"%s\" on %#v failed "+
+				"with NewSmbGlobalMapping(%s, %s) failed with error: rpc error: code = Unknown desc = NewSmbGlobalMapping failed.",
+				strings.Replace(testSource, "\\", "\\\\", -1), errorMountSensSource, testSource, errorMountSensSource),
+			expectedErr: testutil.TestError{
+				DefaultError: status.Errorf(codes.Internal,
+					"volume(vol_1##) mount \"%s\" on \"%s\" failed with fake "+
+						"MountSensitive: target error",
+					strings.Replace(testSource, "\\", "\\\\", -1), errorMountSensSource),
+			},
+		},
 		{
 			desc: "[Success] Valid request",
 			req: &csi.NodeStageVolumeRequest{VolumeId: "vol_1##", StagingTargetPath: sourceTest,

pkg/smb/smb_common_linux.go

@@ -41,14 +41,19 @@ func Mount(m *mount.SafeFormatAndMount, source, target, fsType string, options,
 		if err != nil {
 			return err
 		}
+		defer func() {
+			file.Close()
+			os.Remove(file.Name())
+		}()
 
 		for _, option := range sensitiveMountOptions {
 			if _, err := file.Write([]byte(fmt.Sprintf("%s\n", option))); err != nil {
 				return err
 			}
 		}
-		file.Close()
-		defer os.Remove(file.Name())
+		if err := file.Close(); err != nil {
+			return err
+		}
 
 		sensitiveMountOptions = []string{fmt.Sprintf("credentials=%s", file.Name())}
 	}

pkg/smb/smb_common_linux_test.go

@@ -0,0 +1,54 @@
+/*
+Copyright 2020 The Kubernetes Authors.
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+package smb
+
+import (
+	"github.com/stretchr/testify/assert"
+	"testing"
+)
+
+func TestNeedsCredentialsOption(t *testing.T) {
+	tests := []struct {
+		options        []string
+		expectedResult bool
+	}{
+		{
+			options:        []string{"username=foo,password=bar"},
+			expectedResult: false,
+		},
+		{
+			options:        []string{"username=foo", "password=bar"},
+			expectedResult: false,
+		},
+		{
+			options:        []string{"username=foo", "password=b\"r"},
+			expectedResult: true,
+		},
+		{
+			options:        []string{"username=foo", "password=b`r"},
+			expectedResult: true,
+		},
+		{
+			options:        []string{"username=foo", "password=b,r"},
+			expectedResult: true,
+		},
+	}
+
+	for _, test := range tests {
+		assert.Equal(t, test.expectedResult, NeedsCredentialsOption(test.options))
+	}
+}