Skip to content

Commit 73e5818

Browse files
Sneha-atSneha-at
committed
add unit tests to validate mount options
1 parent 9e0ddcb commit 73e5818

File tree

2 files changed

+156
-22
lines changed

2 files changed

+156
-22
lines changed

‎pkg/csi_driver/node.go‎

Lines changed: 34 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -140,35 +140,23 @@ func (s *nodeServer) NodePublishVolume(ctx context.Context, req *csi.NodePublish
140140
gcsFuseSidecarImage := gcsFuseSidecarContainerImage(pod)
141141
enableSidecarBucketAccessCheckForSidecarVersion := s.driver.config.EnableSidecarBucketAccessCheck && gcsFuseSidecarImage != "" && isSidecarVersionSupportedForGivenFeature(gcsFuseSidecarImage, SidecarBucketAccessCheckMinVersion)
142142
identityProvider := ""
143-
if s.shouldPopulateIdentityProvider(pod, optInHostnetworkKSA, userSpecifiedIdentityProvider != "") {
143+
shouldPopulateIdentityProviderForHnwKSA := s.shouldPopulateIdentityProvider(pod, optInHostnetworkKSA, userSpecifiedIdentityProvider != "")
144+
if shouldPopulateIdentityProviderForHnwKSA {
144145
if userSpecifiedIdentityProvider != "" {
145146
identityProvider = userSpecifiedIdentityProvider
146147
} else {
147148
identityProvider = s.driver.config.TokenManager.GetIdentityProvider()
148149
}
149-
klog.V(6).Infof("NodePublishVolume populating identity provider %q in mount options", identityProvider)
150-
fuseMountOptions = joinMountOptions(fuseMountOptions, []string{util.OptInHnw + "=true", util.TokenServerIdentityProviderConst + "=" + identityProvider})
151150
} else if enableSidecarBucketAccessCheckForSidecarVersion {
152-
//Enable sidecar bucket access check only for Workload Identity workloads. This feature consumes additional quota for Host Network pods as we do not have token caching.
153-
fuseMountOptions = joinMountOptions(fuseMountOptions, []string{util.EnableSidecarBucketAccessCheckConst + "=" + strconv.FormatBool(s.driver.config.EnableSidecarBucketAccessCheck)})
154-
}
155-
156-
if enableSidecarBucketAccessCheckForSidecarVersion {
157-
if identityProvider == "" {
158-
identityProvider = s.driver.config.TokenManager.GetIdentityProvider()
159-
fuseMountOptions = joinMountOptions(fuseMountOptions, []string{util.TokenServerIdentityProviderConst + "=" + identityProvider})
160-
}
161-
klog.Infof("Got identity provider %s", identityProvider)
162-
163-
identityPool := s.driver.config.TokenManager.GetIdentityPool()
164-
fuseMountOptions = joinMountOptions(fuseMountOptions, []string{
165-
util.PodNamespaceConst + "=" + vc[VolumeContextKeyPodNamespace],
166-
util.ServiceAccountNameConst + "=" + vc[VolumeContextKeyServiceAccountName],
167-
util.TokenServerIdentityPoolConst + "=" + identityPool})
151+
// As host network and sidecar bucket access check are exclusive the below assignment doesn't overwrite any of host network settings.
152+
identityProvider = s.driver.config.TokenManager.GetIdentityProvider()
168153
}
169154

170-
if enableCloudProfilerForSidecar && gcsFuseSidecarImage != "" && isSidecarVersionSupportedForGivenFeature(gcsFuseSidecarImage, SidecarCloudProfilerMinVersion) {
171-
fuseMountOptions = joinMountOptions(fuseMountOptions, []string{util.EnableCloudProfilerForSidecarConst + "=" + strconv.FormatBool(enableCloudProfilerForSidecar)})
155+
enableCloudProfilerForVersion := enableCloudProfilerForSidecar && gcsFuseSidecarImage != "" && isSidecarVersionSupportedForGivenFeature(gcsFuseSidecarImage, SidecarCloudProfilerMinVersion)
156+
identityPool := s.driver.config.TokenManager.GetIdentityPool()
157+
fuseMountOptions, err = addFuseMountOptions(identityProvider, identityPool, fuseMountOptions, vc, shouldPopulateIdentityProviderForHnwKSA, enableSidecarBucketAccessCheckForSidecarVersion, enableCloudProfilerForVersion)
158+
if err != nil {
159+
return nil, status.Errorf(codes.InvalidArgument, "failed to prepare sidecar fuse mount options: %v", err)
172160
}
173161

174162
node, err := s.k8sClients.GetNode(s.driver.config.NodeID)
@@ -213,7 +201,7 @@ func (s *nodeServer) NodePublishVolume(ctx context.Context, req *csi.NodePublish
213201
}
214202
}
215203

216-
// Only pass mountOptions flags for defaulting if sidecar container is managed and satisifies min version requirement
204+
// Only pass mountOptions flags for defaulting if sidecar container is managed and satisfies min version requirement
217205
if gcsFuseSidecarImage != "" && isSidecarVersionSupportedForGivenFeature(gcsFuseSidecarImage, MachineTypeAutoConfigSidecarMinVersion) {
218206
shouldDisableAutoConfig := s.driver.config.DisableAutoconfig
219207
machineType, ok := node.Labels[clientset.MachineTypeKey]
@@ -396,3 +384,27 @@ func gcsFuseSidecarContainerImage(pod *corev1.Pod) string {
396384
}
397385
return ""
398386
}
387+
388+
func addFuseMountOptions(identityProvider string, identityPool string, fuseMountOptions []string, vc map[string]string, hostNetworkEnabled bool, enableSidecarBucketAccessCheckForSidecarVersion bool, enableCloudProfilerForVersion bool) ([]string, error) {
389+
if hostNetworkEnabled {
390+
// Identity Provider is populated separately for host network enabled workloads.
391+
fuseMountOptions = joinMountOptions(fuseMountOptions, []string{util.OptInHnw + "=true", util.TokenServerIdentityProviderConst + "=" + identityProvider})
392+
} else if enableSidecarBucketAccessCheckForSidecarVersion {
393+
if identityPool == "" || identityProvider == "" {
394+
// Identity pool and provider details are used in sidecar mounter to create the metadata service
395+
return nil, fmt.Errorf("failed to get either of identity pool %q or identity provider %q", identityPool, identityProvider)
396+
}
397+
// Enable sidecar bucket access check only for Workload Identity workloads. This feature consumes additional quota for Host Network pods as we do not have token caching.
398+
fuseMountOptions = joinMountOptions(fuseMountOptions, []string{
399+
util.PodNamespaceConst + "=" + vc[VolumeContextKeyPodNamespace],
400+
util.ServiceAccountNameConst + "=" + vc[VolumeContextKeyServiceAccountName],
401+
util.TokenServerIdentityPoolConst + "=" + identityPool,
402+
util.TokenServerIdentityProviderConst + "=" + identityProvider,
403+
util.EnableSidecarBucketAccessCheckConst + "=" + strconv.FormatBool(enableSidecarBucketAccessCheckForSidecarVersion)})
404+
}
405+
klog.V(6).Infof("NodePublishVolume populating identity provider %q in mount options", identityProvider)
406+
if enableCloudProfilerForVersion {
407+
fuseMountOptions = joinMountOptions(fuseMountOptions, []string{util.EnableCloudProfilerForSidecarConst + "=" + strconv.FormatBool(enableCloudProfilerForVersion)})
408+
}
409+
return fuseMountOptions, nil
410+
}

‎pkg/csi_driver/node_test.go‎

Lines changed: 122 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -458,3 +458,125 @@ func TestNodePublishVolumeWILabelCheck(t *testing.T) {
458458
}
459459
}
460460
}
461+
462+
func TestAddFuseMountOptions(t *testing.T) {
463+
t.Parallel()
464+
465+
vc := map[string]string{
466+
VolumeContextKeyPodNamespace: "sample-namespace",
467+
VolumeContextKeyServiceAccountName: "sample-service-account",
468+
}
469+
sampleIdentityPool := "sample-identity-pool"
470+
sampleIdentityProvider := "sample-identity-provider"
471+
cases := []struct {
472+
name string
473+
inputFuseMountOptions []string
474+
expectedFuseMountOptions []string
475+
identityProvider string
476+
volumeContextParams map[string]string
477+
enableSidecarBucketAccessCheckForVersion bool
478+
enableCloudProfilerForVersion bool
479+
hostNetworkEnabled bool
480+
identityPool string
481+
expectErr bool
482+
}{
483+
{
484+
name: "validate fuse mount options for host network workloads",
485+
enableSidecarBucketAccessCheckForVersion: false,
486+
hostNetworkEnabled: true,
487+
identityProvider: sampleIdentityProvider,
488+
identityPool: sampleIdentityPool,
489+
inputFuseMountOptions: []string{},
490+
expectedFuseMountOptions: []string{
491+
util.OptInHnw + "=true",
492+
util.TokenServerIdentityProviderConst + "=" + sampleIdentityProvider,
493+
},
494+
},
495+
{
496+
name: "validate sidecar bucket access check is disabled when host network is enabled",
497+
enableSidecarBucketAccessCheckForVersion: true,
498+
hostNetworkEnabled: true,
499+
identityProvider: sampleIdentityProvider,
500+
identityPool: sampleIdentityPool,
501+
inputFuseMountOptions: []string{},
502+
expectedFuseMountOptions: []string{
503+
util.OptInHnw + "=true",
504+
util.TokenServerIdentityProviderConst + "=" + sampleIdentityProvider,
505+
},
506+
},
507+
{
508+
name: "enable sidecar bucket access check on non-host network and validate fuse mount options are correctly set",
509+
volumeContextParams: vc,
510+
enableSidecarBucketAccessCheckForVersion: true,
511+
identityProvider: sampleIdentityProvider,
512+
identityPool: sampleIdentityPool,
513+
inputFuseMountOptions: []string{},
514+
expectedFuseMountOptions: []string{
515+
util.PodNamespaceConst + "=" + vc[VolumeContextKeyPodNamespace],
516+
util.ServiceAccountNameConst + "=" + vc[VolumeContextKeyServiceAccountName],
517+
util.TokenServerIdentityPoolConst + "=" + sampleIdentityPool,
518+
util.TokenServerIdentityProviderConst + "=" + sampleIdentityProvider,
519+
util.EnableSidecarBucketAccessCheckConst + "=true",
520+
},
521+
},
522+
{
523+
name: "verify sidecar bucket access disabled does not set fuse mount options",
524+
volumeContextParams: vc,
525+
enableSidecarBucketAccessCheckForVersion: false,
526+
identityProvider: sampleIdentityProvider,
527+
identityPool: sampleIdentityPool,
528+
inputFuseMountOptions: []string{},
529+
expectedFuseMountOptions: []string{},
530+
},
531+
{
532+
name: "validate cloud profiler flag is correctly set",
533+
enableCloudProfilerForVersion: true,
534+
inputFuseMountOptions: []string{},
535+
expectedFuseMountOptions: []string{
536+
util.EnableCloudProfilerForSidecarConst + "=true",
537+
},
538+
},
539+
{
540+
name: "validate sidecar bucket access check when identity provider is not set",
541+
volumeContextParams: vc,
542+
enableSidecarBucketAccessCheckForVersion: true,
543+
identityProvider: "",
544+
identityPool: sampleIdentityPool,
545+
expectedFuseMountOptions: nil,
546+
expectErr: true,
547+
},
548+
{
549+
name: "validate sidecar bucket access check when identity pool is not set",
550+
volumeContextParams: vc,
551+
enableSidecarBucketAccessCheckForVersion: true,
552+
identityProvider: sampleIdentityProvider,
553+
identityPool: "",
554+
expectedFuseMountOptions: nil,
555+
expectErr: true,
556+
},
557+
{
558+
name: "validate cloud profiler with host network",
559+
hostNetworkEnabled: true,
560+
enableCloudProfilerForVersion: true,
561+
identityProvider: sampleIdentityProvider,
562+
expectedFuseMountOptions: []string{
563+
util.EnableCloudProfilerForSidecarConst + "=true",
564+
util.OptInHnw + "=true",
565+
util.TokenServerIdentityProviderConst + "=" + sampleIdentityProvider,
566+
},
567+
},
568+
}
569+
570+
for _, tc := range cases {
571+
t.Run(tc.name, func(t *testing.T) {
572+
gotFuseMountOption, err := addFuseMountOptions(tc.identityProvider, tc.identityPool, tc.inputFuseMountOptions, tc.volumeContextParams, tc.hostNetworkEnabled, tc.enableSidecarBucketAccessCheckForVersion, tc.enableCloudProfilerForVersion)
573+
if (err != nil) != tc.expectErr {
574+
t.Errorf("for test case %q, got error: %v, but expectErr: %v", tc.name, err, tc.expectErr)
575+
}
576+
less := func(a, b string) bool { return a < b }
577+
if diff := cmp.Diff(tc.expectedFuseMountOptions, gotFuseMountOption, cmpopts.SortSlices(less)); diff != "" {
578+
t.Errorf("got unexpected options args for testcase %s (-got, +want)\n%s", tc.name, diff)
579+
}
580+
})
581+
}
582+
}

0 commit comments

Comments
 (0)