Repository: kubernetes-csi/csi-driver-smb
Pull request #436 (2022-03-24 09:35:56)

fix: gid conflict issue between sc and fsGroup setting
<!--  Thanks for sending a pull request!  Here are some tips for you:

1. If this is your first time, please read our contributor guidelines: https://git.k8s.io/community/contributors/guide#your-first-contribution and developer guide https://git.k8s.io/community/contributors/devel/development.md#development-guide
2. Please label this pull request according to what type of issue you are addressing, especially if this is a release targeted pull request. For reference on required PR/issue labels, read here:
https://git.k8s.io/community/contributors/devel/release.md#issue-kind-label
3. Ensure you have added or ran the appropriate tests for your PR: https://git.k8s.io/community/contributors/devel/testing.md
4. If you want *faster* PR reviews, read how: https://git.k8s.io/community/contributors/guide/pull-requests.md#best-practices-for-faster-reviews
5. Follow the instructions for writing a release note: https://git.k8s.io/community/contributors/guide/release-notes.md
6. If the PR is unfinished, see how to mark it: https://git.k8s.io/community/contributors/guide/pull-requests.md#marking-unfinished-pull-requests
-->

**What type of PR is this?**
/kind bug

**What this PR does / why we need it**:
fix: gid conflict issue between sc and fsGroup setting

**Which issue(s) this PR fixes**:
<!-- 
*Automatically closes linked issue when PR is merged.
Usage: `Fixes #<issue number>`, or `Fixes (paste link of issue)`.
_If PR is about `failing-tests or flakes`, please post the related issues/tests in a comment and do not use `Fixes`_*
-->
Fixes #

**Requirements**:
- [ ] uses [conventional commit messages](https://www.conventionalcommits.org/)
  <!-- Common commit types:
        build: Build 🏭
        chore: Maintenance 🔧
        ci: Continuous Integration 💜
        docs: Documentation 📘
        feat: Features 🌈
        fix: Bug Fixes 🐞
        perf: Performance Improvements 🚀
        refactor: Code Refactoring 💎
        revert: Revert Change ◀️
        style: Code Style 🎶
        security: Security Fix 🛡️
        test: Testing 💚 -->
- [ ] includes documentation
- [ ] adds unit tests
- [ ] tested upgrade from previous version

**Special notes for your reviewer**:


**Release note**:
```
fix: gid conflict issue between sc and fsGroup setting
```



Patch:
diff --git a/hack/verify-all.sh b/hack/verify-all.sh
index c095a6d86..3372df3da 100755
--- a/hack/verify-all.sh
+++ b/hack/verify-all.sh
@@ -22,7 +22,6 @@ ${PKG_ROOT}/hack/verify-gofmt.sh
 ${PKG_ROOT}/hack/verify-govet.sh
 ${PKG_ROOT}/hack/verify-yamllint.sh
 ${PKG_ROOT}/hack/verify-boilerplate.sh
-${PKG_ROOT}/hack/verify-spelling.sh
 ${PKG_ROOT}/hack/verify-helm-chart-files.sh
 ${PKG_ROOT}/hack/verify-helm-chart.sh
 ${PKG_ROOT}/hack/verify-gomod.sh
diff --git a/pkg/smb/nodeserver.go b/pkg/smb/nodeserver.go
index d39177344..88e50b6b5 100644
--- a/pkg/smb/nodeserver.go
+++ b/pkg/smb/nodeserver.go
@@ -127,10 +127,7 @@ func (d *Driver) NodeStageVolume(ctx context.Context, req *csi.NodeStageVolumeRe
 	mountFlags := req.GetVolumeCapability().GetMount().GetMountFlags()
 	volumeMountGroup := req.GetVolumeCapability().GetMount().GetVolumeMountGroup()
 	secrets := req.GetSecrets()
-	gidPresent, err := checkGidPresentInMountFlags(volumeMountGroup, mountFlags)
-	if err != nil {
-		return nil, err
-	}
+	gidPresent := checkGidPresentInMountFlags(mountFlags)
 
 	var source, subDir string
 	for k, v := range context {
@@ -401,16 +398,11 @@ func makeDir(pathname string) error {
 	return nil
 }
 
-func checkGidPresentInMountFlags(volumeMountGroup string, mountFlags []string) (bool, error) {
-	gidPresentInMountFlags := false
+func checkGidPresentInMountFlags(mountFlags []string) bool {
 	for _, mountFlag := range mountFlags {
 		if strings.HasPrefix(mountFlag, "gid") {
-			gidPresentInMountFlags = true
-			kvpair := strings.Split(mountFlag, "=")
-			if volumeMountGroup != "" && len(kvpair) == 2 && !strings.EqualFold(volumeMountGroup, kvpair[1]) {
-				return false, status.Error(codes.InvalidArgument, fmt.Sprintf("gid(%s) in storageClass and pod fsgroup(%s) are not equal", kvpair[1], volumeMountGroup))
-			}
+			return true
 		}
 	}
-	return gidPresentInMountFlags, nil
+	return false
 }


Test patch:
diff --git a/pkg/smb/nodeserver_test.go b/pkg/smb/nodeserver_test.go
index 7d24a3f62..23f61b5f7 100644
--- a/pkg/smb/nodeserver_test.go
+++ b/pkg/smb/nodeserver_test.go
@@ -666,40 +666,24 @@ func TestNodeGetVolumeStats(t *testing.T) {
 
 func TestCheckGidPresentInMountFlags(t *testing.T) {
 	tests := []struct {
-		desc             string
-		VolumeMountGroup string
-		MountFlags       []string
-		expectedErr      error
-		result           bool
+		desc       string
+		MountFlags []string
+		result     bool
 	}{
 		{
-			desc:             "[Error] VolumeMountGroup is different from gid in mount options",
-			VolumeMountGroup: "2000",
-			MountFlags:       []string{"gid=3000"},
-			expectedErr:      status.Error(codes.InvalidArgument, "gid(3000) in storageClass and pod fsgroup(2000) are not equal"),
-			result:           false,
+			desc:       "[Success] Gid present in mount flags",
+			MountFlags: []string{"gid=3000"},
+			result:     true,
 		},
 		{
-			desc:             "[Success] Gid present in mount flags",
-			VolumeMountGroup: "",
-			MountFlags:       []string{"gid=3000"},
-			expectedErr:      nil,
-			result:           true,
-		},
-		{
-			desc:             "[Success] Gid not present in mount flags",
-			VolumeMountGroup: "",
-			MountFlags:       []string{},
-			expectedErr:      nil,
-			result:           false,
+			desc:       "[Success] Gid not present in mount flags",
+			MountFlags: []string{},
+			result:     false,
 		},
 	}
 
 	for _, test := range tests {
-		gIDPresent, err := checkGidPresentInMountFlags(test.VolumeMountGroup, test.MountFlags)
-		if !reflect.DeepEqual(err, test.expectedErr) {
-			t.Errorf("[%s]: Unexpected Error: %v, expected error: %v", test.desc, err, test.expectedErr)
-		}
+		gIDPresent := checkGidPresentInMountFlags(test.MountFlags)
 		if gIDPresent != test.result {
 			t.Errorf("[%s]: Expected result : %t, Actual result: %t", test.desc, test.result, gIDPresent)
 		}