Skip to content

Commit 085945f

Browse files
andyzhangxk8s-infra-cherrypick-robot
authored and
k8s-infra-cherrypick-robot
committed
fix: panic when controller does not have cloud config
1 parent 9fce328 commit 085945f

File tree

4 files changed

+106
-15
lines changed

4 files changed

+106
-15
lines changed

pkg/blob/azure.go

+16-1
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@ import (
2525
"github.com/Azure/azure-sdk-for-go/services/network/mgmt/2022-07-01/network"
2626
"github.com/Azure/azure-sdk-for-go/storage"
2727
"github.com/Azure/go-autorest/autorest"
28+
azure2 "github.com/Azure/go-autorest/autorest/azure"
2829
"golang.org/x/net/context"
2930
"k8s.io/client-go/kubernetes"
3031
"k8s.io/klog/v2"
@@ -159,7 +160,7 @@ func (d *Driver) initializeKvClient() (*kv.BaseClient, error) {
159160

160161
// getKeyvaultToken retrieves a new service principal token to access keyvault
161162
func (d *Driver) getKeyvaultToken() (authorizer autorest.Authorizer, err error) {
162-
env := d.cloud.Environment
163+
env := d.getCloudEnvironment()
163164
kvEndPoint := strings.TrimSuffix(env.KeyVaultEndpoint, "/")
164165
servicePrincipalToken, err := providerconfig.GetServicePrincipalToken(&d.cloud.AzureAuthConfig, &env, kvEndPoint)
165166
if err != nil {
@@ -235,3 +236,17 @@ func (d *Driver) updateSubnetServiceEndpoints(ctx context.Context, vnetResourceG
235236

236237
return nil
237238
}
239+
240+
func (d *Driver) getStorageEndPointSuffix() string {
241+
if d.cloud == nil || d.cloud.Environment.StorageEndpointSuffix == "" {
242+
return defaultStorageEndPointSuffix
243+
}
244+
return d.cloud.Environment.StorageEndpointSuffix
245+
}
246+
247+
func (d *Driver) getCloudEnvironment() azure2.Environment {
248+
if d.cloud == nil {
249+
return azure2.PublicCloud
250+
}
251+
return d.cloud.Environment
252+
}

pkg/blob/azure_test.go

+85
Original file line numberDiff line numberDiff line change
@@ -382,3 +382,88 @@ func TestUpdateSubnetServiceEndpoints(t *testing.T) {
382382
t.Run(tc.name, tc.testFunc)
383383
}
384384
}
385+
386+
func TestGetStorageEndPointSuffix(t *testing.T) {
387+
d := NewFakeDriver()
388+
389+
ctrl := gomock.NewController(t)
390+
defer ctrl.Finish()
391+
392+
tests := []struct {
393+
name string
394+
cloud *azureprovider.Cloud
395+
expectedSuffix string
396+
}{
397+
{
398+
name: "nil cloud",
399+
cloud: nil,
400+
expectedSuffix: "core.windows.net",
401+
},
402+
{
403+
name: "empty cloud",
404+
cloud: &azureprovider.Cloud{},
405+
expectedSuffix: "core.windows.net",
406+
},
407+
{
408+
name: "cloud with storage endpoint suffix",
409+
cloud: &azureprovider.Cloud{
410+
Environment: azure.Environment{
411+
StorageEndpointSuffix: "suffix",
412+
},
413+
},
414+
expectedSuffix: "suffix",
415+
},
416+
{
417+
name: "public cloud",
418+
cloud: &azureprovider.Cloud{
419+
Environment: azure.PublicCloud,
420+
},
421+
expectedSuffix: "core.windows.net",
422+
},
423+
{
424+
name: "china cloud",
425+
cloud: &azureprovider.Cloud{
426+
Environment: azure.ChinaCloud,
427+
},
428+
expectedSuffix: "core.chinacloudapi.cn",
429+
},
430+
}
431+
432+
for _, test := range tests {
433+
d.cloud = test.cloud
434+
suffix := d.getStorageEndPointSuffix()
435+
assert.Equal(t, test.expectedSuffix, suffix, test.name)
436+
}
437+
}
438+
439+
func TestGetCloudEnvironment(t *testing.T) {
440+
d := NewFakeDriver()
441+
442+
ctrl := gomock.NewController(t)
443+
defer ctrl.Finish()
444+
445+
tests := []struct {
446+
name string
447+
cloud *azureprovider.Cloud
448+
expectedEnv azure.Environment
449+
}{
450+
{
451+
name: "nil cloud",
452+
cloud: nil,
453+
expectedEnv: azure.PublicCloud,
454+
},
455+
{
456+
name: "cloud with environment",
457+
cloud: &azureprovider.Cloud{
458+
Environment: azure.ChinaCloud,
459+
},
460+
expectedEnv: azure.ChinaCloud,
461+
},
462+
}
463+
464+
for _, test := range tests {
465+
d.cloud = test.cloud
466+
env := d.getCloudEnvironment()
467+
assert.Equal(t, test.expectedEnv, env, test.name)
468+
}
469+
}

pkg/blob/controllerserver.go

+4-8
Original file line numberDiff line numberDiff line change
@@ -301,11 +301,7 @@ func (d *Driver) CreateVolume(ctx context.Context, req *csi.CreateVolumeRequest)
301301
}
302302

303303
if strings.TrimSpace(storageEndpointSuffix) == "" {
304-
if d.cloud.Environment.StorageEndpointSuffix != "" {
305-
storageEndpointSuffix = d.cloud.Environment.StorageEndpointSuffix
306-
} else {
307-
storageEndpointSuffix = defaultStorageEndPointSuffix
308-
}
304+
storageEndpointSuffix = d.getStorageEndPointSuffix()
309305
}
310306

311307
accountOptions := &azure.AccountOptions{
@@ -550,7 +546,7 @@ func (d *Driver) ValidateVolumeCapabilities(ctx context.Context, req *csi.Valida
550546
var exist bool
551547
secrets := req.GetSecrets()
552548
if len(secrets) > 0 {
553-
container, err := getContainerReference(containerName, secrets, d.cloud.Environment)
549+
container, err := getContainerReference(containerName, secrets, d.getCloudEnvironment())
554550
if err != nil {
555551
return nil, status.Error(codes.Internal, err.Error())
556552
}
@@ -665,7 +661,7 @@ func (d *Driver) CreateBlobContainer(ctx context.Context, subsID, resourceGroupN
665661
return wait.ExponentialBackoff(d.cloud.RequestBackoff(), func() (bool, error) {
666662
var err error
667663
if len(secrets) > 0 {
668-
container, getErr := getContainerReference(containerName, secrets, d.cloud.Environment)
664+
container, getErr := getContainerReference(containerName, secrets, d.getCloudEnvironment())
669665
if getErr != nil {
670666
return true, getErr
671667
}
@@ -697,7 +693,7 @@ func (d *Driver) DeleteBlobContainer(ctx context.Context, subsID, resourceGroupN
697693
return wait.ExponentialBackoff(d.cloud.RequestBackoff(), func() (bool, error) {
698694
var err error
699695
if len(secrets) > 0 {
700-
container, getErr := getContainerReference(containerName, secrets, d.cloud.Environment)
696+
container, getErr := getContainerReference(containerName, secrets, d.getCloudEnvironment())
701697
if getErr != nil {
702698
return true, getErr
703699
}

pkg/blob/nodeserver.go

+1-6
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,6 @@ import (
2929
volumehelper "sigs.k8s.io/blob-csi-driver/pkg/util"
3030
azcache "sigs.k8s.io/cloud-provider-azure/pkg/cache"
3131

32-
"github.com/Azure/azure-sdk-for-go/storage"
3332
"github.com/container-storage-interface/spec/lib/go/csi"
3433

3534
"k8s.io/apimachinery/pkg/util/wait"
@@ -318,11 +317,7 @@ func (d *Driver) NodeStageVolume(ctx context.Context, req *csi.NodeStageVolumeRe
318317
containerName = replaceWithMap(containerName, containerNameReplaceMap)
319318

320319
if strings.TrimSpace(storageEndpointSuffix) == "" {
321-
if d.cloud.Environment.StorageEndpointSuffix != "" {
322-
storageEndpointSuffix = d.cloud.Environment.StorageEndpointSuffix
323-
} else {
324-
storageEndpointSuffix = storage.DefaultBaseURL
325-
}
320+
storageEndpointSuffix = d.getStorageEndPointSuffix()
326321
}
327322

328323
if strings.TrimSpace(serverAddress) == "" {

0 commit comments

Comments
 (0)