fix: copilot comments

Author: andyzhangx

pkg/azurefile/controllerserver.go

@@ -120,9 +120,9 @@ func (d *Driver) CreateVolume(ctx context.Context, req *csi.CreateVolumeRequest)
 	}
 	var sku, subsID, resourceGroup, location, account, fileShareName, diskName, fsType, secretName string
 	var secretNamespace, pvcNamespace, protocol, customTags, storageEndpointSuffix, networkEndpointType, shareAccessTier, accountAccessTier, rootSquashType, tagValueDelimiter string
-	var createAccount, useSeretCache, matchTags, selectRandomMatchingAccount, getLatestAccountKey, encryptInTransit bool
+	var createAccount, useSeretCache, matchTags, selectRandomMatchingAccount, getLatestAccountKey, encryptInTransit, mountWithManagedIdentity, mountWithWIToken bool
 	var vnetResourceGroup, vnetName, vnetLinkName, publicNetworkAccess, subnetName, shareNamePrefix, fsGroupChangePolicy, useDataPlaneAPI string
-	var requireInfraEncryption, disableDeleteRetentionPolicy, enableLFS, isMultichannelEnabled, allowSharedKeyAccess, requiresSmbOAuth *bool
+	var requireInfraEncryption, disableDeleteRetentionPolicy, enableLFS, isMultichannelEnabled, allowSharedKeyAccess *bool
 	var provisionedBandwidthMibps, provisionedIops *int32
 	// set allowBlobPublicAccess as false by default
 	allowBlobPublicAccess := ptr.To(false)
@@ -131,6 +131,7 @@ func (d *Driver) CreateVolume(ctx context.Context, req *csi.CreateVolumeRequest)
 	// store account key to k8s secret by default
 	storeAccountKey := true
 
+	var err error
 	var accountQuota int32
 	// Apply ProvisionerParameters (case-insensitive). We leave validation of
 	// the values to the cloud provider.
@@ -298,26 +299,24 @@ func (d *Driver) CreateVolume(ctx context.Context, req *csi.CreateVolumeRequest)
 			}
 			provisionedIops = to.Ptr(int32(value))
 		case mountWithManagedIdentityField:
-			value, err := strconv.ParseBool(v)
+			mountWithManagedIdentity, err = strconv.ParseBool(v)
 			if err != nil {
 				return nil, status.Errorf(codes.InvalidArgument, "invalid %s: %s in storage class", mountWithManagedIdentityField, v)
 			}
-			if value {
-				requiresSmbOAuth = &value
-			}
 		case mountWithWITokenField:
-			value, err := strconv.ParseBool(v)
+			mountWithWIToken, err = strconv.ParseBool(v)
 			if err != nil {
 				return nil, status.Errorf(codes.InvalidArgument, "invalid %s: %s in storage class", mountWithWITokenField, v)
 			}
-			if value {
-				requiresSmbOAuth = &value
-			}
 		default:
 			return nil, status.Errorf(codes.InvalidArgument, "invalid parameter %q in storage class", k)
 		}
 	}
 
+	if mountWithManagedIdentity && mountWithWIToken {
+		return nil, status.Error(codes.InvalidArgument, "mountwithmanagedidentity and mountwithworkloadidentitytoken cannot be both true in storage class")
+	}
+
 	if matchTags && account != "" {
 		return nil, status.Errorf(codes.InvalidArgument, "matchTags must set as false when storageAccount(%s) is provided", account)
 	}
@@ -535,6 +534,12 @@ func (d *Driver) CreateVolume(ctx context.Context, req *csi.CreateVolumeRequest)
 		}
 	}
 
+	var requiresSmbOAuth *bool
+	if mountWithManagedIdentity || mountWithWIToken {
+		klog.V(2).Info("enabling smb oauth for managed identity or work identity token based mount")
+		requiresSmbOAuth = to.Ptr(true)
+	}
+
 	accountOptions := &storage.AccountOptions{
 		Name:                                    account,
 		Type:                                    sku,

pkg/azurefile/controllerserver_test.go

@@ -1095,6 +1095,24 @@ var _ = ginkgo.Describe("TestCreateVolume", func() {
 			})
 		})
 
+		ginkgo.When("mountWithManagedIdentity and mountWithWIToken cannot be both true", func() {
+			ginkgo.It("should fail", func(ctx context.Context) {
+				req := &csi.CreateVolumeRequest{
+					Name:               "random-vol-name-valid-request",
+					VolumeCapabilities: stdVolCap,
+					CapacityRange:      lessThanPremCapRange,
+					Parameters: map[string]string{
+						mountWithManagedIdentityField: "true",
+						mountWithWITokenField:         "true",
+					},
+				}
+
+				expectedErr := status.Errorf(codes.InvalidArgument, "%s and %s cannot be both true in storage class", mountWithManagedIdentityField, mountWithWITokenField)
+				_, err := d.CreateVolume(ctx, req)
+				gomega.Expect(err).To(gomega.Equal(expectedErr))
+			})
+		})
+
 		ginkgo.When("invalid parameter", func() {
 			ginkgo.It("should fail", func(ctx context.Context) {
 				name := "baz"

pkg/azurefile/nodeserver.go

@@ -359,6 +359,10 @@ func (d *Driver) NodeStageVolume(ctx context.Context, req *csi.NodeStageVolumeRe
 		return nil, status.Errorf(codes.InvalidArgument, "fsGroupChangePolicy(%s) is not supported, supported fsGroupChangePolicy list: %v", fsGroupChangePolicy, supportedFSGroupChangePolicyList)
 	}
 
+	if mountWithManagedIdentity && mountWithWIToken {
+		return nil, status.Error(codes.InvalidArgument, "mountWithManagedIdentity and mountWithWIToken cannot be both true")
+	}
+
 	lockKey := fmt.Sprintf("%s-%s", volumeID, targetPath)
 	if acquired := d.volumeLocks.TryAcquire(lockKey); !acquired {
 		return nil, status.Errorf(codes.Aborted, volumeOperationAlreadyExistsFmt, volumeID)

pkg/azurefile/nodeserver_test.go

@@ -761,6 +761,22 @@ func TestNodeStageVolume(t *testing.T) {
 				DefaultError: status.Error(codes.InvalidArgument, fmt.Sprintf("invalid mountPermissions %s", "07ab")),
 			},
 		},
+		{
+			desc: "[Error] mountWithManagedIdentity and mountWithWIToken cannot be both true",
+			req: &csi.NodeStageVolumeRequest{VolumeId: "vol_1##", StagingTargetPath: sourceTest,
+				VolumeCapability: &stdVolCap,
+				VolumeContext: map[string]string{
+					shareNameField:                "test_sharename",
+					storageAccountField:           "test_accountname",
+					serviceAccountTokenField:      "token",
+					mountWithManagedIdentityField: "true",
+					mountWithWITokenField:         "true",
+				},
+				Secrets: secrets},
+			expectedErr: testutil.TestError{
+				DefaultError: status.Error(codes.InvalidArgument, "mountWithManagedIdentity and mountWithWIToken cannot be both true"),
+			},
+		},
 		{
 			desc: "[Success] Valid request with Kata CC Mount enabled",
 			setup: func() {