add more unit test cases

Author: Sneha-at

pkg/csi_driver/node.go

@@ -138,7 +138,10 @@ func (s *nodeServer) NodePublishVolume(ctx context.Context, req *csi.NodePublish
 		return nil, status.Errorf(codes.NotFound, "failed to get pod: %v", err)
 	}
 	gcsFuseSidecarImage := gcsFuseSidecarContainerImage(pod)
-	enableSidecarBucketAccessCheckForSidecarVersion := s.driver.config.EnableSidecarBucketAccessCheck && gcsFuseSidecarImage != "" && isSidecarVersionSupportedForGivenFeature(gcsFuseSidecarImage, SidecarBucketAccessCheckMinVersion)
+	if gcsFuseSidecarImage == "" {
+		return nil, status.Errorf(codes.Internal, "failed to fetch gcs fuse sidecar container image from pod")
+	}
+	enableSidecarBucketAccessCheckForSidecarVersion := s.driver.config.EnableSidecarBucketAccessCheck && isSidecarVersionSupportedForGivenFeature(gcsFuseSidecarImage, SidecarBucketAccessCheckMinVersion)
 	identityProvider := ""
 	shouldPopulateIdentityProviderForHnwKSA := s.shouldPopulateIdentityProvider(pod, optInHostnetworkKSA, userSpecifiedIdentityProvider != "")
 	if shouldPopulateIdentityProviderForHnwKSA {
@@ -152,7 +155,7 @@ func (s *nodeServer) NodePublishVolume(ctx context.Context, req *csi.NodePublish
 		identityProvider = s.driver.config.TokenManager.GetIdentityProvider()
 	}
 
-	enableCloudProfilerForVersion := enableCloudProfilerForSidecar && gcsFuseSidecarImage != "" && isSidecarVersionSupportedForGivenFeature(gcsFuseSidecarImage, SidecarCloudProfilerMinVersion)
+	enableCloudProfilerForVersion := enableCloudProfilerForSidecar && isSidecarVersionSupportedForGivenFeature(gcsFuseSidecarImage, SidecarCloudProfilerMinVersion)
 	identityPool := s.driver.config.TokenManager.GetIdentityPool()
 	fuseMountOptions, err = addFuseMountOptions(identityProvider, identityPool, fuseMountOptions, vc, shouldPopulateIdentityProviderForHnwKSA, enableSidecarBucketAccessCheckForSidecarVersion, enableCloudProfilerForVersion)
 	if err != nil {
@@ -202,7 +205,7 @@ func (s *nodeServer) NodePublishVolume(ctx context.Context, req *csi.NodePublish
 	}
 
 	// Only pass mountOptions flags for defaulting if sidecar container is managed and satisfies min version requirement
-	if gcsFuseSidecarImage != "" && isSidecarVersionSupportedForGivenFeature(gcsFuseSidecarImage, MachineTypeAutoConfigSidecarMinVersion) {
+	if isSidecarVersionSupportedForGivenFeature(gcsFuseSidecarImage, MachineTypeAutoConfigSidecarMinVersion) {
 		shouldDisableAutoConfig := s.driver.config.DisableAutoconfig
 		machineType, ok := node.Labels[clientset.MachineTypeKey]
 		if ok {
@@ -385,6 +388,7 @@ func gcsFuseSidecarContainerImage(pod *corev1.Pod) string {
 	return ""
 }
 
+// addFuseMountOptions modifies the input array of existing FUSE mount options by appending additional options for respective enabled features.
 func addFuseMountOptions(identityProvider string, identityPool string, fuseMountOptions []string, vc map[string]string, hostNetworkEnabled bool, enableSidecarBucketAccessCheckForSidecarVersion bool, enableCloudProfilerForVersion bool) ([]string, error) {
 	if hostNetworkEnabled {
 		// Identity Provider is populated separately for host network enabled workloads.

pkg/csi_driver/node_test.go

@@ -481,7 +481,7 @@ func TestAddFuseMountOptions(t *testing.T) {
 		expectErr                                bool
 	}{
 		{
-			name:                                     "validate fuse mount options for host network workloads",
+			name:                                     "identity provider is added in fuse mount options for host network workloads",
 			enableSidecarBucketAccessCheckForVersion: false,
 			hostNetworkEnabled:                       true,
 			identityProvider:                         sampleIdentityProvider,
@@ -493,7 +493,7 @@ func TestAddFuseMountOptions(t *testing.T) {
 			},
 		},
 		{
-			name:                                     "validate sidecar bucket access check is disabled when host network is enabled",
+			name:                                     "fuse mount options related to sidecar bucket access check are not set for host network",
 			enableSidecarBucketAccessCheckForVersion: true,
 			hostNetworkEnabled:                       true,
 			identityProvider:                         sampleIdentityProvider,
@@ -505,7 +505,7 @@ func TestAddFuseMountOptions(t *testing.T) {
 			},
 		},
 		{
-			name:                                     "enable sidecar bucket access check on non-host network and validate fuse mount options are correctly set",
+			name:                                     "validate respective fuse mount options for sidecar bucket access check are set when host network is disabled",
 			volumeContextParams:                      vc,
 			enableSidecarBucketAccessCheckForVersion: true,
 			identityProvider:                         sampleIdentityProvider,
@@ -520,7 +520,7 @@ func TestAddFuseMountOptions(t *testing.T) {
 			},
 		},
 		{
-			name:                                     "verify sidecar bucket access disabled does not set fuse mount options",
+			name:                                     "validate no fuse mount options are added when host network and sidecar bucket check is disabled",
 			volumeContextParams:                      vc,
 			enableSidecarBucketAccessCheckForVersion: false,
 			identityProvider:                         sampleIdentityProvider,
@@ -529,15 +529,15 @@ func TestAddFuseMountOptions(t *testing.T) {
 			expectedFuseMountOptions:                 []string{},
 		},
 		{
-			name:                          "validate cloud profiler flag is correctly set",
+			name:                          "validate cloud profiler fuse mount option is set when cloud profiler is enabled",
 			enableCloudProfilerForVersion: true,
 			inputFuseMountOptions:         []string{},
 			expectedFuseMountOptions: []string{
 				util.EnableCloudProfilerForSidecarConst + "=true",
 			},
 		},
 		{
-			name:                                     "validate sidecar bucket access check when identity provider is not set",
+			name:                                     "missing identity provider should fail when sidecar bucket access check is enabled",
 			volumeContextParams:                      vc,
 			enableSidecarBucketAccessCheckForVersion: true,
 			identityProvider:                         "",
@@ -546,7 +546,7 @@ func TestAddFuseMountOptions(t *testing.T) {
 			expectErr:                                true,
 		},
 		{
-			name:                                     "validate sidecar bucket access check when identity pool is not set",
+			name:                                     "missing identity pool should fail when sidecar bucket access check is enabled",
 			volumeContextParams:                      vc,
 			enableSidecarBucketAccessCheckForVersion: true,
 			identityProvider:                         sampleIdentityProvider,
@@ -555,7 +555,7 @@ func TestAddFuseMountOptions(t *testing.T) {
 			expectErr:                                true,
 		},
 		{
-			name:                          "validate cloud profiler with host network",
+			name:                          "verify fuse mount options are not overwritten when multiple features are enabled",
 			hostNetworkEnabled:            true,
 			enableCloudProfilerForVersion: true,
 			identityProvider:              sampleIdentityProvider,
@@ -565,6 +565,18 @@ func TestAddFuseMountOptions(t *testing.T) {
 				util.TokenServerIdentityProviderConst + "=" + sampleIdentityProvider,
 			},
 		},
+		{
+			name:                          "verify new mount options are appended to existing fuse mount options",
+			hostNetworkEnabled:            true,
+			enableCloudProfilerForVersion: true,
+			identityProvider:              sampleIdentityProvider,
+			inputFuseMountOptions:         []string{util.EnableCloudProfilerForSidecarConst + "=true"},
+			expectedFuseMountOptions: []string{
+				util.EnableCloudProfilerForSidecarConst + "=true",
+				util.OptInHnw + "=true",
+				util.TokenServerIdentityProviderConst + "=" + sampleIdentityProvider,
+			},
+		},
 	}
 
 	for _, tc := range cases {