Merge pull request #1252 from andyzhangx/fix-cloud-panic-1.22

[release-1.22] fix: panic when controller does not have cloud config

Author: andyzhangx

pkg/blob/azure.go

@@ -29,6 +29,7 @@ import (
 	"github.com/Azure/azure-sdk-for-go/storage"
 
 	"github.com/Azure/go-autorest/autorest"
+	azure2 "github.com/Azure/go-autorest/autorest/azure"
 
 	clientset "k8s.io/client-go/kubernetes"
 	"k8s.io/client-go/rest"
@@ -184,7 +185,7 @@ func (d *Driver) initializeKvClient() (*kv.BaseClient, error) {
 
 // getKeyvaultToken retrieves a new service principal token to access keyvault
 func (d *Driver) getKeyvaultToken() (authorizer autorest.Authorizer, err error) {
-	env := d.cloud.Environment
+	env := d.getCloudEnvironment()
 	kvEndPoint := strings.TrimSuffix(env.KeyVaultEndpoint, "/")
 	servicePrincipalToken, err := providerconfig.GetServicePrincipalToken(&d.cloud.Config.AzureAuthConfig, &env, kvEndPoint)
 	if err != nil {
@@ -273,3 +274,17 @@ func getKubeConfig(kubeconfig string) (config *rest.Config, err error) {
 	}
 	return config, err
 }
+
+func (d *Driver) getStorageEndPointSuffix() string {
+	if d.cloud == nil || d.cloud.Environment.StorageEndpointSuffix == "" {
+		return defaultStorageEndPointSuffix
+	}
+	return d.cloud.Environment.StorageEndpointSuffix
+}
+
+func (d *Driver) getCloudEnvironment() azure2.Environment {
+	if d.cloud == nil {
+		return azure2.PublicCloud
+	}
+	return d.cloud.Environment
+}

pkg/blob/azure_test.go

@@ -457,3 +457,88 @@ users:
 		}
 	}
 }
+
+func TestGetStorageEndPointSuffix(t *testing.T) {
+	d := NewFakeDriver()
+
+	ctrl := gomock.NewController(t)
+	defer ctrl.Finish()
+
+	tests := []struct {
+		name           string
+		cloud          *azureprovider.Cloud
+		expectedSuffix string
+	}{
+		{
+			name:           "nil cloud",
+			cloud:          nil,
+			expectedSuffix: "core.windows.net",
+		},
+		{
+			name:           "empty cloud",
+			cloud:          &azureprovider.Cloud{},
+			expectedSuffix: "core.windows.net",
+		},
+		{
+			name: "cloud with storage endpoint suffix",
+			cloud: &azureprovider.Cloud{
+				Environment: azure.Environment{
+					StorageEndpointSuffix: "suffix",
+				},
+			},
+			expectedSuffix: "suffix",
+		},
+		{
+			name: "public cloud",
+			cloud: &azureprovider.Cloud{
+				Environment: azure.PublicCloud,
+			},
+			expectedSuffix: "core.windows.net",
+		},
+		{
+			name: "china cloud",
+			cloud: &azureprovider.Cloud{
+				Environment: azure.ChinaCloud,
+			},
+			expectedSuffix: "core.chinacloudapi.cn",
+		},
+	}
+
+	for _, test := range tests {
+		d.cloud = test.cloud
+		suffix := d.getStorageEndPointSuffix()
+		assert.Equal(t, test.expectedSuffix, suffix, test.name)
+	}
+}
+
+func TestGetCloudEnvironment(t *testing.T) {
+	d := NewFakeDriver()
+
+	ctrl := gomock.NewController(t)
+	defer ctrl.Finish()
+
+	tests := []struct {
+		name        string
+		cloud       *azureprovider.Cloud
+		expectedEnv azure.Environment
+	}{
+		{
+			name:        "nil cloud",
+			cloud:       nil,
+			expectedEnv: azure.PublicCloud,
+		},
+		{
+			name: "cloud with environment",
+			cloud: &azureprovider.Cloud{
+				Environment: azure.ChinaCloud,
+			},
+			expectedEnv: azure.ChinaCloud,
+		},
+	}
+
+	for _, test := range tests {
+		d.cloud = test.cloud
+		env := d.getCloudEnvironment()
+		assert.Equal(t, test.expectedEnv, env, test.name)
+	}
+}

pkg/blob/controllerserver.go

@@ -281,11 +281,7 @@ func (d *Driver) CreateVolume(ctx context.Context, req *csi.CreateVolumeRequest)
 	}
 
 	if strings.TrimSpace(storageEndpointSuffix) == "" {
-		if d.cloud.Environment.StorageEndpointSuffix != "" {
-			storageEndpointSuffix = d.cloud.Environment.StorageEndpointSuffix
-		} else {
-			storageEndpointSuffix = defaultStorageEndPointSuffix
-		}
+		storageEndpointSuffix = d.getStorageEndPointSuffix()
 	}
 
 	accountOptions := &azure.AccountOptions{
@@ -510,7 +506,7 @@ func (d *Driver) ValidateVolumeCapabilities(ctx context.Context, req *csi.Valida
 	var exist bool
 	secrets := req.GetSecrets()
 	if len(secrets) > 0 {
-		container, err := getContainerReference(containerName, secrets, d.cloud.Environment)
+		container, err := getContainerReference(containerName, secrets, d.getCloudEnvironment())
 		if err != nil {
 			return nil, status.Error(codes.Internal, err.Error())
 		}
@@ -625,7 +621,7 @@ func (d *Driver) CreateBlobContainer(ctx context.Context, subsID, resourceGroupN
 	return wait.ExponentialBackoff(d.cloud.RequestBackoff(), func() (bool, error) {
 		var err error
 		if len(secrets) > 0 {
-			container, getErr := getContainerReference(containerName, secrets, d.cloud.Environment)
+			container, getErr := getContainerReference(containerName, secrets, d.getCloudEnvironment())
 			if getErr != nil {
 				return true, getErr
 			}
@@ -657,7 +653,7 @@ func (d *Driver) DeleteBlobContainer(ctx context.Context, subsID, resourceGroupN
 	return wait.ExponentialBackoff(d.cloud.RequestBackoff(), func() (bool, error) {
 		var err error
 		if len(secrets) > 0 {
-			container, getErr := getContainerReference(containerName, secrets, d.cloud.Environment)
+			container, getErr := getContainerReference(containerName, secrets, d.getCloudEnvironment())
 			if getErr != nil {
 				return true, getErr
 			}

pkg/blob/nodeserver.go

@@ -29,7 +29,6 @@ import (
 	volumehelper "sigs.k8s.io/blob-csi-driver/pkg/util"
 	azcache "sigs.k8s.io/cloud-provider-azure/pkg/cache"
 
-	"github.com/Azure/azure-sdk-for-go/storage"
 	"github.com/container-storage-interface/spec/lib/go/csi"
 
 	"k8s.io/apimachinery/pkg/util/wait"
@@ -299,11 +298,7 @@ func (d *Driver) NodeStageVolume(ctx context.Context, req *csi.NodeStageVolumeRe
 	containerName = replaceWithMap(containerName, containerNameReplaceMap)
 
 	if strings.TrimSpace(storageEndpointSuffix) == "" {
-		if d.cloud.Environment.StorageEndpointSuffix != "" {
-			storageEndpointSuffix = d.cloud.Environment.StorageEndpointSuffix
-		} else {
-			storageEndpointSuffix = storage.DefaultBaseURL
-		}
+		storageEndpointSuffix = d.getStorageEndPointSuffix()
 	}
 
 	if strings.TrimSpace(serverAddress) == "" {