From 9484797a9a39997020ebe70ba1913578267ea113 Mon Sep 17 00:00:00 2001
From: Prankul <84079249+prankulmahajan@users.noreply.github.com>
Date: Mon, 8 Jan 2024 23:53:52 +0530
Subject: [PATCH 1/8] Create CSI socket file using non-root user

Signed-off-by: Prankul <84079249+prankulmahajan@users.noreply.github.com>
---
 .../kubernetes/manifests/config-map.yaml      |   2 +-
 .../manifests/controller-server.yaml          |   1 +
 .../kubernetes/manifests/node-server.yaml     |  14 +-
 go.sum                                        |   2 -
 pkg/ibmcsidriver/fileOps.go                   |  72 +++++++++++
 pkg/ibmcsidriver/fileOps_test.go              | 121 ++++++++++++++++++
 pkg/ibmcsidriver/server.go                    |   9 ++
 7 files changed, 214 insertions(+), 7 deletions(-)
 create mode 100644 pkg/ibmcsidriver/fileOps.go
 create mode 100644 pkg/ibmcsidriver/fileOps_test.go

diff --git a/deploy/kubernetes/driver/kubernetes/manifests/config-map.yaml b/deploy/kubernetes/driver/kubernetes/manifests/config-map.yaml
index 0471f15c..819d0266 100644
--- a/deploy/kubernetes/driver/kubernetes/manifests/config-map.yaml
+++ b/deploy/kubernetes/driver/kubernetes/manifests/config-map.yaml
@@ -16,4 +16,4 @@ data:
   VPC_RETRY_INTERVAL: "60" # This is max retry Gap in seconds even considering for exponential retry
   VPC_API_VERSION: "2021-04-20"
   VPC_API_GENERATION: "1"
-  IKS_ENABLED: "True"
\ No newline at end of file
+  IKS_ENABLED: "True"
diff --git a/deploy/kubernetes/driver/kubernetes/manifests/controller-server.yaml b/deploy/kubernetes/driver/kubernetes/manifests/controller-server.yaml
index aa2a91e5..6756b1ad 100644
--- a/deploy/kubernetes/driver/kubernetes/manifests/controller-server.yaml
+++ b/deploy/kubernetes/driver/kubernetes/manifests/controller-server.yaml
@@ -25,6 +25,7 @@ spec:
       securityContext:
         runAsNonRoot: true
         runAsUser: 2121
+        runAsGroup: 2121
       containers:
         - name: csi-provisioner
           image: MUSTPATCHWITHKUSTOMIZE
diff --git a/deploy/kubernetes/driver/kubernetes/manifests/node-server.yaml b/deploy/kubernetes/driver/kubernetes/manifests/node-server.yaml
index a968a566..26ef81cf 100644
--- a/deploy/kubernetes/driver/kubernetes/manifests/node-server.yaml
+++ b/deploy/kubernetes/driver/kubernetes/manifests/node-server.yaml
@@ -24,6 +24,10 @@ spec:
       serviceAccountName: ibm-vpc-file-node-sa
       tolerations:
       - operator: Exists
+      securityContext:
+        runAsNonRoot: true
+        runAsUser: 2121
+        runAsGroup: 2121
       containers:
         - name: csi-driver-registrar
           image: MUSTPATCHWITHKUSTOMIZE
@@ -31,6 +35,7 @@ spec:
           securityContext:
             runAsNonRoot: false
             runAsUser: 0
+            runAsGroup: 0
             privileged: false
           args:
             - "--v=5"
@@ -64,6 +69,7 @@ spec:
           securityContext:
             runAsNonRoot: false
             runAsUser: 0
+            runAsGroup: 0
             privileged: true
           image: MUSTPATCHWITHKUSTOMIZE
           imagePullPolicy: Always
@@ -91,6 +97,10 @@ spec:
                   fieldPath: spec.nodeName
             - name: SOCKET_PATH
               value: "/var/lib/ibmshare.sock"
+            - name: IS_NODE_SERVER
+              value: "true"
+            - name: SIDECAR_GROUP_ID
+              value: "2121"
           resources:
             limits:
               cpu: 200m
@@ -140,10 +150,6 @@ spec:
               name: mh-logs
         - name: liveness-probe
           image: MUSTPATCHWITHKUSTOMIZE
-          securityContext:
-            runAsNonRoot: false
-            runAsUser: 0
-            privileged: false
           args:
             - "--csi-address=$(CSI_ADDRESS)"
           env:
diff --git a/go.sum b/go.sum
index c07cd0d8..3046531a 100644
--- a/go.sum
+++ b/go.sum
@@ -153,8 +153,6 @@ github.com/hashicorp/go-hclog v0.9.2/go.mod h1:5CU+agLiy3J7N7QjHK5d05KxGsuXiQLrj
 github.com/hashicorp/go-retryablehttp v0.7.5 h1:bJj+Pj19UZMIweq/iie+1u5YCdGrnxCT9yvm0e+Nd5M=
 github.com/hashicorp/go-retryablehttp v0.7.5/go.mod h1:Jy/gPYAdjqffZ/yFGCFV2doI5wjtH1ewM9u8iYVjtX8=
 github.com/ianlancetaylor/demangle v0.0.0-20200824232613-28f6c0f3b639/go.mod h1:aSSvb/t6k1mPoxDqO4vJh6VOCGPwU4O0C2/Eqndh1Sc=
-github.com/imdario/mergo v0.3.7 h1:Y+UAYTZ7gDEuOfhxKWy+dvb5dRQ6rJjFSdX2HZY1/gI=
-github.com/imdario/mergo v0.3.7/go.mod h1:2EnlNZ0deacrJVfApfmtdGgDfMuh/nq6Ok1EcJh5FfA=
 github.com/josharian/intern v1.0.0 h1:vlS4z54oSdjm0bgjRigI+G1HpF+tI+9rE5LLzOg8HmY=
 github.com/josharian/intern v1.0.0/go.mod h1:5DoeVV0s6jJacbCEi61lwdGj/aVlrQvzHFFd8Hwg//Y=
 github.com/json-iterator/go v1.1.12 h1:PV8peI4a0ysnczrg+LtxykD8LfKY9ML6u2jnxaEnrnM=
diff --git a/pkg/ibmcsidriver/fileOps.go b/pkg/ibmcsidriver/fileOps.go
new file mode 100644
index 00000000..68d4085d
--- /dev/null
+++ b/pkg/ibmcsidriver/fileOps.go
@@ -0,0 +1,72 @@
+/**
+ *
+ * Copyright 2024- IBM Inc. All rights reserved
+ * SPDX-License-Identifier: Apache2.0
+ *
+ * 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 ibmcsidriver ...
+package ibmcsidriver
+
+import (
+	"os"
+	"strconv"
+)
+
+const (
+	filePermission = 0660
+)
+
+// socketPermission represents file system operations
+type socketPermission interface {
+	chown(name string, uid, gid int) error
+	chmod(name string, mode os.FileMode) error
+}
+
+// realSocketPermission implements socketPermission
+type opsSocketPermission struct{}
+
+func (f *opsSocketPermission) chown(name string, uid, gid int) error {
+	return os.Chown(name, uid, gid)
+}
+
+func (f *opsSocketPermission) chmod(name string, mode os.FileMode) error {
+	return os.Chmod(name, mode)
+}
+
+func setupSidecar(addr string, ops socketPermission) error {
+	groupSt := os.Getenv("SIDECAR_GROUP_ID")
+	// If env is not set, set default to 0
+	if groupSt == "" {
+		groupSt = "0"
+	}
+
+	group, err := strconv.Atoi(groupSt)
+	if err != nil {
+		return err
+	}
+
+	// Change group of csi socket to non-root user for enabling the csi sidecar
+	if err := ops.chown(addr, -1, group); err != nil {
+		return err
+	}
+
+	// Modify permissions of csi socket
+	// Only the users and the group owners will have read/write access to csi socket
+	if err := ops.chmod(addr, filePermission); err != nil {
+		return err
+	}
+
+	return nil
+}
diff --git a/pkg/ibmcsidriver/fileOps_test.go b/pkg/ibmcsidriver/fileOps_test.go
new file mode 100644
index 00000000..38a266d7
--- /dev/null
+++ b/pkg/ibmcsidriver/fileOps_test.go
@@ -0,0 +1,121 @@
+/**
+ *
+ * Copyright 2024- IBM Inc. All rights reserved
+ * SPDX-License-Identifier: Apache2.0
+ *
+ * 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 ibmcsidriver ...
+package ibmcsidriver
+
+import (
+	"errors"
+	"os"
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/mock"
+)
+
+// Mock implementation of the socketPermission interface
+type mockSocketPermission struct {
+	mock.Mock
+}
+
+func (m *mockSocketPermission) chown(name string, uid, gid int) error {
+	args := m.Called(name, uid, gid)
+	return args.Error(0)
+}
+
+func (m *mockSocketPermission) chmod(name string, mode os.FileMode) error {
+	args := m.Called(name, mode)
+	return args.Error(0)
+}
+
+func TestSetupSidecar(t *testing.T) {
+	tests := []struct {
+		name               string
+		socketPermission   socketPermission
+		groupID            string
+		expectedErr        bool
+		chownErr           error
+		chmodErr           error
+		expectedChownCalls int
+		expectedChmodCalls int
+	}{
+		{
+			name:               "ValidGroupID",
+			socketPermission:   &mockSocketPermission{},
+			groupID:            "2121",
+			expectedErr:        false,
+			chownErr:           nil,
+			chmodErr:           nil,
+			expectedChownCalls: 1,
+			expectedChmodCalls: 1,
+		},
+		{
+			name:               "EmptyGroupID",
+			socketPermission:   &mockSocketPermission{},
+			groupID:            "",
+			expectedErr:        false,
+			chownErr:           nil,
+			chmodErr:           nil,
+			expectedChownCalls: 1,
+			expectedChmodCalls: 1,
+		},
+		{
+			name:               "ChownError",
+			socketPermission:   &mockSocketPermission{},
+			groupID:            "1000",
+			expectedErr:        true,
+			chownErr:           errors.New("chown error"),
+			chmodErr:           nil,
+			expectedChownCalls: 1,
+			expectedChmodCalls: 0, // No chmod expected if chown fails
+		},
+		{
+			name:               "ChmodError",
+			socketPermission:   &mockSocketPermission{},
+			groupID:            "1000",
+			expectedErr:        true,
+			chownErr:           nil,
+			chmodErr:           errors.New("chmod error"),
+			expectedChownCalls: 1,
+			expectedChmodCalls: 1,
+		},
+	}
+
+	for _, tc := range tests {
+		t.Run(tc.name, func(t *testing.T) {
+			// Set the environment variable
+			os.Setenv("SIDECAR_GROUP_ID", tc.groupID)
+			defer os.Unsetenv("SIDECAR_GROUP_ID")
+
+			// Create mock object
+			mockSocketPermission := tc.socketPermission.(*mockSocketPermission)
+
+			// Set expectations for chown and chmod methods
+			mockSocketPermission.On("chown", mock.Anything, -1, mock.AnythingOfType("int")).Return(tc.chownErr).Times(tc.expectedChownCalls)
+			mockSocketPermission.On("chmod", mock.Anything, os.FileMode(filePermission)).Return(tc.chmodErr).Times(tc.expectedChmodCalls)
+
+			err := setupSidecar("/path/to/socket", mockSocketPermission)
+
+			if tc.expectedErr {
+				assert.Error(t, err)
+			} else {
+				assert.NoError(t, err)
+			}
+		})
+	}
+}
diff --git a/pkg/ibmcsidriver/server.go b/pkg/ibmcsidriver/server.go
index df23b0b6..142560c8 100644
--- a/pkg/ibmcsidriver/server.go
+++ b/pkg/ibmcsidriver/server.go
@@ -114,6 +114,7 @@ func (s *nonBlockingGRPCServer) Setup(endpoint string, ids csi.IdentityServer, c
 
 	s.logger.Info("Start listening GRPC Server", zap.Reflect("Scheme", u.Scheme), zap.Reflect("Addr", addr))
 
+	// Create listener
 	listener, err := net.Listen(u.Scheme, addr)
 	if err != nil {
 		msg := "failed to listen GRPC Server"
@@ -121,6 +122,14 @@ func (s *nonBlockingGRPCServer) Setup(endpoint string, ids csi.IdentityServer, c
 		return nil, errors.New(msg)
 	}
 
+	if os.Getenv("IS_NODE_SERVER") == "true" {
+		fileops := &opsSocketPermission{}
+		if err := setupSidecar(addr, fileops); err != nil {
+			s.logger.Error("setupSidecar failed.", zap.Error(err))
+			return nil, err
+		}
+	}
+
 	server := grpc.NewServer(opts...)
 	s.server = server
 

From 95c5551c0acb14a0f6285eeb7ff6421fc25a335f Mon Sep 17 00:00:00 2001
From: Prankul <84079249+prankulmahajan@users.noreply.github.com>
Date: Thu, 22 Aug 2024 16:13:13 +0530
Subject: [PATCH 2/8] fix go mod tidy

---
 go.mod | 1 +
 go.sum | 3 +++
 2 files changed, 4 insertions(+)

diff --git a/go.mod b/go.mod
index a7330010..0314896f 100644
--- a/go.mod
+++ b/go.mod
@@ -82,6 +82,7 @@ require (
 	github.com/prometheus/common v0.44.0 // indirect
 	github.com/prometheus/procfs v0.10.1 // indirect
 	github.com/spf13/pflag v1.0.5 // indirect
+	github.com/stretchr/objx v0.5.0 // indirect
 	go.mongodb.org/mongo-driver v1.13.1 // indirect
 	go.uber.org/atomic v1.11.0 // indirect
 	go.uber.org/multierr v1.11.0 // indirect
diff --git a/go.sum b/go.sum
index 3046531a..11e4ea78 100644
--- a/go.sum
+++ b/go.sum
@@ -153,6 +153,8 @@ github.com/hashicorp/go-hclog v0.9.2/go.mod h1:5CU+agLiy3J7N7QjHK5d05KxGsuXiQLrj
 github.com/hashicorp/go-retryablehttp v0.7.5 h1:bJj+Pj19UZMIweq/iie+1u5YCdGrnxCT9yvm0e+Nd5M=
 github.com/hashicorp/go-retryablehttp v0.7.5/go.mod h1:Jy/gPYAdjqffZ/yFGCFV2doI5wjtH1ewM9u8iYVjtX8=
 github.com/ianlancetaylor/demangle v0.0.0-20200824232613-28f6c0f3b639/go.mod h1:aSSvb/t6k1mPoxDqO4vJh6VOCGPwU4O0C2/Eqndh1Sc=
+github.com/imdario/mergo v0.3.7 h1:Y+UAYTZ7gDEuOfhxKWy+dvb5dRQ6rJjFSdX2HZY1/gI=
+github.com/imdario/mergo v0.3.7/go.mod h1:2EnlNZ0deacrJVfApfmtdGgDfMuh/nq6Ok1EcJh5FfA=
 github.com/josharian/intern v1.0.0 h1:vlS4z54oSdjm0bgjRigI+G1HpF+tI+9rE5LLzOg8HmY=
 github.com/josharian/intern v1.0.0/go.mod h1:5DoeVV0s6jJacbCEi61lwdGj/aVlrQvzHFFd8Hwg//Y=
 github.com/json-iterator/go v1.1.12 h1:PV8peI4a0ysnczrg+LtxykD8LfKY9ML6u2jnxaEnrnM=
@@ -220,6 +222,7 @@ github.com/spf13/pflag v1.0.5 h1:iy+VFUOCP1a+8yFto/drg2CJ5u0yRoB7fZw3DKv/JXA=
 github.com/spf13/pflag v1.0.5/go.mod h1:McXfInJRrz4CZXVZOBLb0bTZqETkiAhM9Iw0y3An2Bg=
 github.com/stretchr/objx v0.1.0/go.mod h1:HFkY916IF+rwdDfMAkV7OtwuqBVzrE8GR6GFx+wExME=
 github.com/stretchr/objx v0.4.0/go.mod h1:YvHI0jy2hoMjB+UWwv71VJQ9isScKT/TqJzVSSt89Yw=
+github.com/stretchr/objx v0.5.0 h1:1zr/of2m5FGMsad5YfcqgdqdWrIhu+EBEJRhR1U7z/c=
 github.com/stretchr/objx v0.5.0/go.mod h1:Yh+to48EsGEfYuaHDzXPcE3xhTkx73EhmCGUpEOglKo=
 github.com/stretchr/testify v1.2.2/go.mod h1:a8OnRcib4nhh0OaRAV+Yts87kKdq0PP7pXfy6kDkUVs=
 github.com/stretchr/testify v1.3.0/go.mod h1:M5WIy9Dh21IEIfnGCwXGc5bZfKNJtfHm1UVUgZn+9EI=

From cba1c708678d4312225b8d923d19423e84d8aaae Mon Sep 17 00:00:00 2001
From: Prankul <84079249+prankulmahajan@users.noreply.github.com>
Date: Thu, 22 Aug 2024 16:26:40 +0530
Subject: [PATCH 3/8] update local yaml

---
 .../kubernetes/manifests/controller-server.yaml |  7 +++++++
 .../kubernetes/manifests/node-server.yaml       | 17 +++++++++++++----
 2 files changed, 20 insertions(+), 4 deletions(-)

diff --git a/deploy/kubernetes/driver/kubernetes/manifests/controller-server.yaml b/deploy/kubernetes/driver/kubernetes/manifests/controller-server.yaml
index 6756b1ad..529faeed 100644
--- a/deploy/kubernetes/driver/kubernetes/manifests/controller-server.yaml
+++ b/deploy/kubernetes/driver/kubernetes/manifests/controller-server.yaml
@@ -188,6 +188,13 @@ spec:
                 configMapKeyRef:
                   name: ibm-vpc-file-csi-configmap
                   key: SIDECAR_ENDPOINT
+          resources:
+            limits:
+              cpu: 12m
+              memory: 20Mi
+            requests:
+              cpu: 3m
+              memory: 5Mi
           volumeMounts:
             - mountPath: /sidecardir
               name: socket-dir
diff --git a/deploy/kubernetes/driver/kubernetes/manifests/node-server.yaml b/deploy/kubernetes/driver/kubernetes/manifests/node-server.yaml
index 26ef81cf..e246ee06 100644
--- a/deploy/kubernetes/driver/kubernetes/manifests/node-server.yaml
+++ b/deploy/kubernetes/driver/kubernetes/manifests/node-server.yaml
@@ -24,10 +24,6 @@ spec:
       serviceAccountName: ibm-vpc-file-node-sa
       tolerations:
       - operator: Exists
-      securityContext:
-        runAsNonRoot: true
-        runAsUser: 2121
-        runAsGroup: 2121
       containers:
         - name: csi-driver-registrar
           image: MUSTPATCHWITHKUSTOMIZE
@@ -149,6 +145,14 @@ spec:
             - mountPath: /tmp/mount-helper
               name: mh-logs
         - name: liveness-probe
+          securityContext:
+            runAsNonRoot: true
+            runAsUser: 2121
+            runAsGroup: 2121
+            privileged: false
+            seLinuxOptions: # seLinux label is set as a precaution for accessing csi socket
+              type: spc_t
+              level: s0
           image: MUSTPATCHWITHKUSTOMIZE
           args:
             - "--csi-address=$(CSI_ADDRESS)"
@@ -169,6 +173,11 @@ spec:
             - name: plugin-dir
               mountPath: /csi
         - name: storage-secret-sidecar
+          securityContext:
+            runAsNonRoot: true
+            runAsUser: 2121
+            runAsGroup: 2121
+            privileged: false
           image: MUSTPATCHWITHKUSTOMIZE
           imagePullPolicy: Always
           args:

From 5bc0642771020d5bd4b715b70d3210889ca63427 Mon Sep 17 00:00:00 2001
From: Prankul <84079249+prankulmahajan@users.noreply.github.com>
Date: Thu, 22 Aug 2024 17:12:18 +0530
Subject: [PATCH 4/8] update comment

---
 pkg/ibmcsidriver/fileOps.go | 1 +
 pkg/ibmcsidriver/server.go  | 2 ++
 2 files changed, 3 insertions(+)

diff --git a/pkg/ibmcsidriver/fileOps.go b/pkg/ibmcsidriver/fileOps.go
index 68d4085d..323e20b6 100644
--- a/pkg/ibmcsidriver/fileOps.go
+++ b/pkg/ibmcsidriver/fileOps.go
@@ -45,6 +45,7 @@ func (f *opsSocketPermission) chmod(name string, mode os.FileMode) error {
 	return os.Chmod(name, mode)
 }
 
+// setupSidecar updates owner/group and permission of the file given(addr)
 func setupSidecar(addr string, ops socketPermission) error {
 	groupSt := os.Getenv("SIDECAR_GROUP_ID")
 	// If env is not set, set default to 0
diff --git a/pkg/ibmcsidriver/server.go b/pkg/ibmcsidriver/server.go
index 142560c8..a1901b8e 100644
--- a/pkg/ibmcsidriver/server.go
+++ b/pkg/ibmcsidriver/server.go
@@ -122,6 +122,8 @@ func (s *nonBlockingGRPCServer) Setup(endpoint string, ids csi.IdentityServer, c
 		return nil, errors.New(msg)
 	}
 
+	// In case of nodeSerer container, setup desired csi socket permissions and user/group.
+	// This is required for running `livenessprobe` container as non-root user/group
 	if os.Getenv("IS_NODE_SERVER") == "true" {
 		fileops := &opsSocketPermission{}
 		if err := setupSidecar(addr, fileops); err != nil {

From d192d447b77058f79d4a092f63b26844bc4783ed Mon Sep 17 00:00:00 2001
From: Prankul <84079249+prankulmahajan@users.noreply.github.com>
Date: Thu, 22 Aug 2024 20:53:45 +0530
Subject: [PATCH 5/8] generate fakes using counterfit

---
 go.mod                                        |   1 -
 go.sum                                        |   1 -
 pkg/ibmcsidriver/fileOps.go                   |  16 +-
 pkg/ibmcsidriver/fileOps_test.go              |  41 ++--
 .../fake_socket_permission.go                 | 188 ++++++++++++++++++
 5 files changed, 211 insertions(+), 36 deletions(-)
 create mode 100644 pkg/ibmcsidriver/ibmcsidriverfakes/fake_socket_permission.go

diff --git a/go.mod b/go.mod
index 0314896f..a7330010 100644
--- a/go.mod
+++ b/go.mod
@@ -82,7 +82,6 @@ require (
 	github.com/prometheus/common v0.44.0 // indirect
 	github.com/prometheus/procfs v0.10.1 // indirect
 	github.com/spf13/pflag v1.0.5 // indirect
-	github.com/stretchr/objx v0.5.0 // indirect
 	go.mongodb.org/mongo-driver v1.13.1 // indirect
 	go.uber.org/atomic v1.11.0 // indirect
 	go.uber.org/multierr v1.11.0 // indirect
diff --git a/go.sum b/go.sum
index 11e4ea78..c07cd0d8 100644
--- a/go.sum
+++ b/go.sum
@@ -222,7 +222,6 @@ github.com/spf13/pflag v1.0.5 h1:iy+VFUOCP1a+8yFto/drg2CJ5u0yRoB7fZw3DKv/JXA=
 github.com/spf13/pflag v1.0.5/go.mod h1:McXfInJRrz4CZXVZOBLb0bTZqETkiAhM9Iw0y3An2Bg=
 github.com/stretchr/objx v0.1.0/go.mod h1:HFkY916IF+rwdDfMAkV7OtwuqBVzrE8GR6GFx+wExME=
 github.com/stretchr/objx v0.4.0/go.mod h1:YvHI0jy2hoMjB+UWwv71VJQ9isScKT/TqJzVSSt89Yw=
-github.com/stretchr/objx v0.5.0 h1:1zr/of2m5FGMsad5YfcqgdqdWrIhu+EBEJRhR1U7z/c=
 github.com/stretchr/objx v0.5.0/go.mod h1:Yh+to48EsGEfYuaHDzXPcE3xhTkx73EhmCGUpEOglKo=
 github.com/stretchr/testify v1.2.2/go.mod h1:a8OnRcib4nhh0OaRAV+Yts87kKdq0PP7pXfy6kDkUVs=
 github.com/stretchr/testify v1.3.0/go.mod h1:M5WIy9Dh21IEIfnGCwXGc5bZfKNJtfHm1UVUgZn+9EI=
diff --git a/pkg/ibmcsidriver/fileOps.go b/pkg/ibmcsidriver/fileOps.go
index 323e20b6..e786e138 100644
--- a/pkg/ibmcsidriver/fileOps.go
+++ b/pkg/ibmcsidriver/fileOps.go
@@ -19,6 +19,8 @@
 // Package ibmcsidriver ...
 package ibmcsidriver
 
+//go:generate go run github.com/maxbrunsfeld/counterfeiter/v6 -generate
+
 import (
 	"os"
 	"strconv"
@@ -28,20 +30,22 @@ const (
 	filePermission = 0660
 )
 
+//counterfeiter:generate . socketPermission
+
 // socketPermission represents file system operations
 type socketPermission interface {
-	chown(name string, uid, gid int) error
-	chmod(name string, mode os.FileMode) error
+	Chown(name string, uid, gid int) error
+	Chmod(name string, mode os.FileMode) error
 }
 
 // realSocketPermission implements socketPermission
 type opsSocketPermission struct{}
 
-func (f *opsSocketPermission) chown(name string, uid, gid int) error {
+func (f *opsSocketPermission) Chown(name string, uid, gid int) error {
 	return os.Chown(name, uid, gid)
 }
 
-func (f *opsSocketPermission) chmod(name string, mode os.FileMode) error {
+func (f *opsSocketPermission) Chmod(name string, mode os.FileMode) error {
 	return os.Chmod(name, mode)
 }
 
@@ -59,13 +63,13 @@ func setupSidecar(addr string, ops socketPermission) error {
 	}
 
 	// Change group of csi socket to non-root user for enabling the csi sidecar
-	if err := ops.chown(addr, -1, group); err != nil {
+	if err := ops.Chown(addr, -1, group); err != nil {
 		return err
 	}
 
 	// Modify permissions of csi socket
 	// Only the users and the group owners will have read/write access to csi socket
-	if err := ops.chmod(addr, filePermission); err != nil {
+	if err := ops.Chmod(addr, filePermission); err != nil {
 		return err
 	}
 
diff --git a/pkg/ibmcsidriver/fileOps_test.go b/pkg/ibmcsidriver/fileOps_test.go
index 38a266d7..fcbfbcf8 100644
--- a/pkg/ibmcsidriver/fileOps_test.go
+++ b/pkg/ibmcsidriver/fileOps_test.go
@@ -16,7 +16,6 @@
  * limitations under the License.
  */
 
-// Package ibmcsidriver ...
 package ibmcsidriver
 
 import (
@@ -24,29 +23,13 @@ import (
 	"os"
 	"testing"
 
+	"github.com/IBM/ibm-vpc-file-csi-driver/pkg/ibmcsidriver/ibmcsidriverfakes"
 	"github.com/stretchr/testify/assert"
-	"github.com/stretchr/testify/mock"
 )
 
-// Mock implementation of the socketPermission interface
-type mockSocketPermission struct {
-	mock.Mock
-}
-
-func (m *mockSocketPermission) chown(name string, uid, gid int) error {
-	args := m.Called(name, uid, gid)
-	return args.Error(0)
-}
-
-func (m *mockSocketPermission) chmod(name string, mode os.FileMode) error {
-	args := m.Called(name, mode)
-	return args.Error(0)
-}
-
 func TestSetupSidecar(t *testing.T) {
 	tests := []struct {
 		name               string
-		socketPermission   socketPermission
 		groupID            string
 		expectedErr        bool
 		chownErr           error
@@ -56,7 +39,6 @@ func TestSetupSidecar(t *testing.T) {
 	}{
 		{
 			name:               "ValidGroupID",
-			socketPermission:   &mockSocketPermission{},
 			groupID:            "2121",
 			expectedErr:        false,
 			chownErr:           nil,
@@ -66,7 +48,6 @@ func TestSetupSidecar(t *testing.T) {
 		},
 		{
 			name:               "EmptyGroupID",
-			socketPermission:   &mockSocketPermission{},
 			groupID:            "",
 			expectedErr:        false,
 			chownErr:           nil,
@@ -76,7 +57,6 @@ func TestSetupSidecar(t *testing.T) {
 		},
 		{
 			name:               "ChownError",
-			socketPermission:   &mockSocketPermission{},
 			groupID:            "1000",
 			expectedErr:        true,
 			chownErr:           errors.New("chown error"),
@@ -86,7 +66,6 @@ func TestSetupSidecar(t *testing.T) {
 		},
 		{
 			name:               "ChmodError",
-			socketPermission:   &mockSocketPermission{},
 			groupID:            "1000",
 			expectedErr:        true,
 			chownErr:           nil,
@@ -102,20 +81,26 @@ func TestSetupSidecar(t *testing.T) {
 			os.Setenv("SIDECAR_GROUP_ID", tc.groupID)
 			defer os.Unsetenv("SIDECAR_GROUP_ID")
 
-			// Create mock object
-			mockSocketPermission := tc.socketPermission.(*mockSocketPermission)
+			// Create the fake object generated by counterfeiter
+			fakeSocketPermission := new(ibmcsidriverfakes.FakeSocketPermission)
 
-			// Set expectations for chown and chmod methods
-			mockSocketPermission.On("chown", mock.Anything, -1, mock.AnythingOfType("int")).Return(tc.chownErr).Times(tc.expectedChownCalls)
-			mockSocketPermission.On("chmod", mock.Anything, os.FileMode(filePermission)).Return(tc.chmodErr).Times(tc.expectedChmodCalls)
+			// Set return values for chown and chmod methods
+			fakeSocketPermission.ChownReturns(tc.chownErr)
+			fakeSocketPermission.ChmodReturns(tc.chmodErr)
 
-			err := setupSidecar("/path/to/socket", mockSocketPermission)
+			// Call the function under test
+			err := setupSidecar("/path/to/socket", fakeSocketPermission)
 
+			// Verify the result
 			if tc.expectedErr {
 				assert.Error(t, err)
 			} else {
 				assert.NoError(t, err)
 			}
+
+			// Verify the number of times Chown and Chmod were called
+			assert.Equal(t, tc.expectedChownCalls, fakeSocketPermission.ChownCallCount())
+			assert.Equal(t, tc.expectedChmodCalls, fakeSocketPermission.ChmodCallCount())
 		})
 	}
 }
diff --git a/pkg/ibmcsidriver/ibmcsidriverfakes/fake_socket_permission.go b/pkg/ibmcsidriver/ibmcsidriverfakes/fake_socket_permission.go
new file mode 100644
index 00000000..35b0aa69
--- /dev/null
+++ b/pkg/ibmcsidriver/ibmcsidriverfakes/fake_socket_permission.go
@@ -0,0 +1,188 @@
+// Code generated by counterfeiter. DO NOT EDIT.
+package ibmcsidriverfakes
+
+import (
+	"io/fs"
+	"sync"
+)
+
+type FakeSocketPermission struct {
+	ChmodStub        func(string, fs.FileMode) error
+	chmodMutex       sync.RWMutex
+	chmodArgsForCall []struct {
+		arg1 string
+		arg2 fs.FileMode
+	}
+	chmodReturns struct {
+		result1 error
+	}
+	chmodReturnsOnCall map[int]struct {
+		result1 error
+	}
+	ChownStub        func(string, int, int) error
+	chownMutex       sync.RWMutex
+	chownArgsForCall []struct {
+		arg1 string
+		arg2 int
+		arg3 int
+	}
+	chownReturns struct {
+		result1 error
+	}
+	chownReturnsOnCall map[int]struct {
+		result1 error
+	}
+	invocations      map[string][][]interface{}
+	invocationsMutex sync.RWMutex
+}
+
+func (fake *FakeSocketPermission) Chmod(arg1 string, arg2 fs.FileMode) error {
+	fake.chmodMutex.Lock()
+	ret, specificReturn := fake.chmodReturnsOnCall[len(fake.chmodArgsForCall)]
+	fake.chmodArgsForCall = append(fake.chmodArgsForCall, struct {
+		arg1 string
+		arg2 fs.FileMode
+	}{arg1, arg2})
+	stub := fake.ChmodStub
+	fakeReturns := fake.chmodReturns
+	fake.recordInvocation("Chmod", []interface{}{arg1, arg2})
+	fake.chmodMutex.Unlock()
+	if stub != nil {
+		return stub(arg1, arg2)
+	}
+	if specificReturn {
+		return ret.result1
+	}
+	return fakeReturns.result1
+}
+
+func (fake *FakeSocketPermission) ChmodCallCount() int {
+	fake.chmodMutex.RLock()
+	defer fake.chmodMutex.RUnlock()
+	return len(fake.chmodArgsForCall)
+}
+
+func (fake *FakeSocketPermission) ChmodCalls(stub func(string, fs.FileMode) error) {
+	fake.chmodMutex.Lock()
+	defer fake.chmodMutex.Unlock()
+	fake.ChmodStub = stub
+}
+
+func (fake *FakeSocketPermission) ChmodArgsForCall(i int) (string, fs.FileMode) {
+	fake.chmodMutex.RLock()
+	defer fake.chmodMutex.RUnlock()
+	argsForCall := fake.chmodArgsForCall[i]
+	return argsForCall.arg1, argsForCall.arg2
+}
+
+func (fake *FakeSocketPermission) ChmodReturns(result1 error) {
+	fake.chmodMutex.Lock()
+	defer fake.chmodMutex.Unlock()
+	fake.ChmodStub = nil
+	fake.chmodReturns = struct {
+		result1 error
+	}{result1}
+}
+
+func (fake *FakeSocketPermission) ChmodReturnsOnCall(i int, result1 error) {
+	fake.chmodMutex.Lock()
+	defer fake.chmodMutex.Unlock()
+	fake.ChmodStub = nil
+	if fake.chmodReturnsOnCall == nil {
+		fake.chmodReturnsOnCall = make(map[int]struct {
+			result1 error
+		})
+	}
+	fake.chmodReturnsOnCall[i] = struct {
+		result1 error
+	}{result1}
+}
+
+func (fake *FakeSocketPermission) Chown(arg1 string, arg2 int, arg3 int) error {
+	fake.chownMutex.Lock()
+	ret, specificReturn := fake.chownReturnsOnCall[len(fake.chownArgsForCall)]
+	fake.chownArgsForCall = append(fake.chownArgsForCall, struct {
+		arg1 string
+		arg2 int
+		arg3 int
+	}{arg1, arg2, arg3})
+	stub := fake.ChownStub
+	fakeReturns := fake.chownReturns
+	fake.recordInvocation("Chown", []interface{}{arg1, arg2, arg3})
+	fake.chownMutex.Unlock()
+	if stub != nil {
+		return stub(arg1, arg2, arg3)
+	}
+	if specificReturn {
+		return ret.result1
+	}
+	return fakeReturns.result1
+}
+
+func (fake *FakeSocketPermission) ChownCallCount() int {
+	fake.chownMutex.RLock()
+	defer fake.chownMutex.RUnlock()
+	return len(fake.chownArgsForCall)
+}
+
+func (fake *FakeSocketPermission) ChownCalls(stub func(string, int, int) error) {
+	fake.chownMutex.Lock()
+	defer fake.chownMutex.Unlock()
+	fake.ChownStub = stub
+}
+
+func (fake *FakeSocketPermission) ChownArgsForCall(i int) (string, int, int) {
+	fake.chownMutex.RLock()
+	defer fake.chownMutex.RUnlock()
+	argsForCall := fake.chownArgsForCall[i]
+	return argsForCall.arg1, argsForCall.arg2, argsForCall.arg3
+}
+
+func (fake *FakeSocketPermission) ChownReturns(result1 error) {
+	fake.chownMutex.Lock()
+	defer fake.chownMutex.Unlock()
+	fake.ChownStub = nil
+	fake.chownReturns = struct {
+		result1 error
+	}{result1}
+}
+
+func (fake *FakeSocketPermission) ChownReturnsOnCall(i int, result1 error) {
+	fake.chownMutex.Lock()
+	defer fake.chownMutex.Unlock()
+	fake.ChownStub = nil
+	if fake.chownReturnsOnCall == nil {
+		fake.chownReturnsOnCall = make(map[int]struct {
+			result1 error
+		})
+	}
+	fake.chownReturnsOnCall[i] = struct {
+		result1 error
+	}{result1}
+}
+
+func (fake *FakeSocketPermission) Invocations() map[string][][]interface{} {
+	fake.invocationsMutex.RLock()
+	defer fake.invocationsMutex.RUnlock()
+	fake.chmodMutex.RLock()
+	defer fake.chmodMutex.RUnlock()
+	fake.chownMutex.RLock()
+	defer fake.chownMutex.RUnlock()
+	copiedInvocations := map[string][][]interface{}{}
+	for key, value := range fake.invocations {
+		copiedInvocations[key] = value
+	}
+	return copiedInvocations
+}
+
+func (fake *FakeSocketPermission) recordInvocation(key string, args []interface{}) {
+	fake.invocationsMutex.Lock()
+	defer fake.invocationsMutex.Unlock()
+	if fake.invocations == nil {
+		fake.invocations = map[string][][]interface{}{}
+	}
+	if fake.invocations[key] == nil {
+		fake.invocations[key] = [][]interface{}{}
+	}
+	fake.invocations[key] = append(fake.invocations[key], args)
+}

From 526b40b426559725a0d84f92ced1ab321901ff00 Mon Sep 17 00:00:00 2001
From: Prankul <84079249+prankulmahajan@users.noreply.github.com>
Date: Fri, 23 Aug 2024 14:14:32 +0530
Subject: [PATCH 6/8] Add check for SIDECAR_GROUP_ID

---
 pkg/ibmcsidriver/fileOps_test.go | 19 +++++++++++++++++--
 1 file changed, 17 insertions(+), 2 deletions(-)

diff --git a/pkg/ibmcsidriver/fileOps_test.go b/pkg/ibmcsidriver/fileOps_test.go
index fcbfbcf8..002fc118 100644
--- a/pkg/ibmcsidriver/fileOps_test.go
+++ b/pkg/ibmcsidriver/fileOps_test.go
@@ -36,6 +36,7 @@ func TestSetupSidecar(t *testing.T) {
 		chmodErr           error
 		expectedChownCalls int
 		expectedChmodCalls int
+		expectedGroupID    int
 	}{
 		{
 			name:               "ValidGroupID",
@@ -45,6 +46,7 @@ func TestSetupSidecar(t *testing.T) {
 			chmodErr:           nil,
 			expectedChownCalls: 1,
 			expectedChmodCalls: 1,
+			expectedGroupID:    2121,
 		},
 		{
 			name:               "EmptyGroupID",
@@ -54,6 +56,7 @@ func TestSetupSidecar(t *testing.T) {
 			chmodErr:           nil,
 			expectedChownCalls: 1,
 			expectedChmodCalls: 1,
+			expectedGroupID:    0, // Default to 0 if SIDECAR_GROUP_ID is empty
 		},
 		{
 			name:               "ChownError",
@@ -63,6 +66,7 @@ func TestSetupSidecar(t *testing.T) {
 			chmodErr:           nil,
 			expectedChownCalls: 1,
 			expectedChmodCalls: 0, // No chmod expected if chown fails
+			expectedGroupID:    1000,
 		},
 		{
 			name:               "ChmodError",
@@ -72,13 +76,18 @@ func TestSetupSidecar(t *testing.T) {
 			chmodErr:           errors.New("chmod error"),
 			expectedChownCalls: 1,
 			expectedChmodCalls: 1,
+			expectedGroupID:    1000,
 		},
 	}
 
 	for _, tc := range tests {
 		t.Run(tc.name, func(t *testing.T) {
-			// Set the environment variable
-			os.Setenv("SIDECAR_GROUP_ID", tc.groupID)
+			// Set SIDECAR_GROUP_ID environment variable
+			if tc.groupID != "" {
+				os.Setenv("SIDECAR_GROUP_ID", tc.groupID)
+			} else {
+				os.Unsetenv("SIDECAR_GROUP_ID")
+			}
 			defer os.Unsetenv("SIDECAR_GROUP_ID")
 
 			// Create the fake object generated by counterfeiter
@@ -101,6 +110,12 @@ func TestSetupSidecar(t *testing.T) {
 			// Verify the number of times Chown and Chmod were called
 			assert.Equal(t, tc.expectedChownCalls, fakeSocketPermission.ChownCallCount())
 			assert.Equal(t, tc.expectedChmodCalls, fakeSocketPermission.ChmodCallCount())
+
+			// Verify the group ID passed to chown
+			if tc.expectedChownCalls > 0 {
+				_, _, actualGroupID := fakeSocketPermission.ChownArgsForCall(0)
+				assert.Equal(t, tc.expectedGroupID, actualGroupID)
+			}
 		})
 	}
 }

From c9d342cc370d56f1b2199f255f3fcd25b37ac9c8 Mon Sep 17 00:00:00 2001
From: Prankul <84079249+prankulmahajan@users.noreply.github.com>
Date: Mon, 26 Aug 2024 18:14:26 +0530
Subject: [PATCH 7/8] add logs to setupSidecar function

---
 pkg/ibmcsidriver/fileOps.go      | 10 +++++++++-
 pkg/ibmcsidriver/fileOps_test.go |  6 +++++-
 pkg/ibmcsidriver/server.go       |  2 +-
 3 files changed, 15 insertions(+), 3 deletions(-)

diff --git a/pkg/ibmcsidriver/fileOps.go b/pkg/ibmcsidriver/fileOps.go
index e786e138..f1ee098c 100644
--- a/pkg/ibmcsidriver/fileOps.go
+++ b/pkg/ibmcsidriver/fileOps.go
@@ -24,6 +24,8 @@ package ibmcsidriver
 import (
 	"os"
 	"strconv"
+
+	"go.uber.org/zap"
 )
 
 const (
@@ -50,10 +52,14 @@ func (f *opsSocketPermission) Chmod(name string, mode os.FileMode) error {
 }
 
 // setupSidecar updates owner/group and permission of the file given(addr)
-func setupSidecar(addr string, ops socketPermission) error {
+func setupSidecar(addr string, ops socketPermission, logger *zap.Logger) error {
 	groupSt := os.Getenv("SIDECAR_GROUP_ID")
+
+	logger.Info("Setting owner and permissions of csi socket file. SIDECAR_GROUP_ID env must match the sidecar container groupID  socket connection.")
+
 	// If env is not set, set default to 0
 	if groupSt == "" {
+		logger.Warn("Unable to fetch SIDECAR_GROUP_ID environment variable. Sidecar containers might fail...")
 		groupSt = "0"
 	}
 
@@ -73,5 +79,7 @@ func setupSidecar(addr string, ops socketPermission) error {
 		return err
 	}
 
+	logger.Info("Successfully set owner and permissions of csi socket file.")
+
 	return nil
 }
diff --git a/pkg/ibmcsidriver/fileOps_test.go b/pkg/ibmcsidriver/fileOps_test.go
index 002fc118..20326a05 100644
--- a/pkg/ibmcsidriver/fileOps_test.go
+++ b/pkg/ibmcsidriver/fileOps_test.go
@@ -97,8 +97,12 @@ func TestSetupSidecar(t *testing.T) {
 			fakeSocketPermission.ChownReturns(tc.chownErr)
 			fakeSocketPermission.ChmodReturns(tc.chmodErr)
 
+			// Creating test logger
+			logger, teardown := GetTestLogger(t)
+			defer teardown()
+
 			// Call the function under test
-			err := setupSidecar("/path/to/socket", fakeSocketPermission)
+			err := setupSidecar("/path/to/socket", fakeSocketPermission, logger)
 
 			// Verify the result
 			if tc.expectedErr {
diff --git a/pkg/ibmcsidriver/server.go b/pkg/ibmcsidriver/server.go
index a1901b8e..aa11aa6e 100644
--- a/pkg/ibmcsidriver/server.go
+++ b/pkg/ibmcsidriver/server.go
@@ -126,7 +126,7 @@ func (s *nonBlockingGRPCServer) Setup(endpoint string, ids csi.IdentityServer, c
 	// This is required for running `livenessprobe` container as non-root user/group
 	if os.Getenv("IS_NODE_SERVER") == "true" {
 		fileops := &opsSocketPermission{}
-		if err := setupSidecar(addr, fileops); err != nil {
+		if err := setupSidecar(addr, fileops, s.logger); err != nil {
 			s.logger.Error("setupSidecar failed.", zap.Error(err))
 			return nil, err
 		}

From a9a01e1351b5272ad17f884d8798205ab8c90c55 Mon Sep 17 00:00:00 2001
From: Prankul <84079249+prankulmahajan@users.noreply.github.com>
Date: Tue, 27 Aug 2024 10:23:48 +0530
Subject: [PATCH 8/8] review comments

---
 pkg/ibmcsidriver/fileOps.go | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/pkg/ibmcsidriver/fileOps.go b/pkg/ibmcsidriver/fileOps.go
index f1ee098c..3d79bf3f 100644
--- a/pkg/ibmcsidriver/fileOps.go
+++ b/pkg/ibmcsidriver/fileOps.go
@@ -55,11 +55,11 @@ func (f *opsSocketPermission) Chmod(name string, mode os.FileMode) error {
 func setupSidecar(addr string, ops socketPermission, logger *zap.Logger) error {
 	groupSt := os.Getenv("SIDECAR_GROUP_ID")
 
-	logger.Info("Setting owner and permissions of csi socket file. SIDECAR_GROUP_ID env must match the sidecar container groupID  socket connection.")
+	logger.Info("Setting owner and permissions of csi socket file. SIDECAR_GROUP_ID env must match the 'livenessprobe' sidecar container groupID for csi socket connection.")
 
 	// If env is not set, set default to 0
 	if groupSt == "" {
-		logger.Warn("Unable to fetch SIDECAR_GROUP_ID environment variable. Sidecar containers might fail...")
+		logger.Warn("Unable to fetch SIDECAR_GROUP_ID environment variable. Sidecar container(s) might fail...")
 		groupSt = "0"
 	}