Merge pull request #1250 from andyzhangx/fix-cloud-panic

fix: panic when controller does not have cloud config

Author: andyzhangx

pkg/blob/azure.go

@@ -25,6 +25,7 @@ import (
 	"github.com/Azure/azure-sdk-for-go/services/network/mgmt/2022-07-01/network"
 	"github.com/Azure/azure-sdk-for-go/storage"
 	"github.com/Azure/go-autorest/autorest"
+	azure2 "github.com/Azure/go-autorest/autorest/azure"
 	"golang.org/x/net/context"
 	"k8s.io/client-go/kubernetes"
 	"k8s.io/klog/v2"
@@ -159,7 +160,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.AzureAuthConfig, &env, kvEndPoint)
 	if err != nil {
@@ -236,3 +237,17 @@ func (d *Driver) updateSubnetServiceEndpoints(ctx context.Context, vnetResourceG
 
 	return nil
 }
+
+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

@@ -375,3 +375,88 @@ func TestUpdateSubnetServiceEndpoints(t *testing.T) {
 		t.Run(tc.name, tc.testFunc)
 	}
 }
+
+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

@@ -309,11 +309,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{
@@ -558,7 +554,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())
 		}
@@ -677,7 +673,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
 			}
@@ -714,7 +710,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

@@ -30,7 +30,6 @@ import (
 	azcache "sigs.k8s.io/cloud-provider-azure/pkg/cache"
 	"sigs.k8s.io/cloud-provider-azure/pkg/metrics"
 
-	"github.com/Azure/azure-sdk-for-go/storage"
 	"github.com/container-storage-interface/spec/lib/go/csi"
 
 	"k8s.io/apimachinery/pkg/util/wait"
@@ -334,11 +333,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) == "" {