diff --git a/pkg/blob/blob.go b/pkg/blob/blob.go index f72ae8c37..888836506 100644 --- a/pkg/blob/blob.go +++ b/pkg/blob/blob.go @@ -754,20 +754,28 @@ func (d *Driver) GetAuthEnv(ctx context.Context, volumeID, protocol string, attr authEnv = append(authEnv, "AZURE_STORAGE_SPN_TENANT_ID="+storageSPNTenantID) } - if azureStorageAuthType == storageAuthTypeMSI { - // check whether authEnv contains AZURE_STORAGE_IDENTITY_ prefix + if strings.EqualFold(azureStorageAuthType, storageAuthTypeMSI) { + // check whether authEnv contains a non-empty AZURE_STORAGE_IDENTITY_ value containsIdentityEnv := false for _, env := range authEnv { if strings.HasPrefix(env, "AZURE_STORAGE_IDENTITY_") { - klog.V(2).Infof("AZURE_STORAGE_IDENTITY_ is already set in authEnv, skip setting it again") - containsIdentityEnv = true - break + // skip empty values like "AZURE_STORAGE_IDENTITY_CLIENT_ID=" + parts := strings.SplitN(env, "=", 2) + if len(parts) == 2 && parts[1] != "" { + klog.V(2).Infof("AZURE_STORAGE_IDENTITY_ is already set in authEnv, skip setting it again") + containsIdentityEnv = true + break + } } } - if !containsIdentityEnv && d.cloud != nil && d.cloud.Config.AzureAuthConfig.UserAssignedIdentityID != "" { - klog.V(2).Infof("azureStorageAuthType is set to %s, add AZURE_STORAGE_IDENTITY_CLIENT_ID(%s) into authEnv", - azureStorageAuthType, d.cloud.Config.AzureAuthConfig.UserAssignedIdentityID) - authEnv = append(authEnv, "AZURE_STORAGE_IDENTITY_CLIENT_ID="+d.cloud.Config.AzureAuthConfig.UserAssignedIdentityID) + if !containsIdentityEnv { + if d.cloud != nil && d.cloud.Config.AzureAuthConfig.UserAssignedIdentityID != "" { + klog.V(2).Infof("MSI auth: AzureStorageIdentityClientID not specified, default to kubelet identity (%s)", + d.cloud.Config.AzureAuthConfig.UserAssignedIdentityID) + authEnv = append(authEnv, "AZURE_STORAGE_IDENTITY_CLIENT_ID="+d.cloud.Config.AzureAuthConfig.UserAssignedIdentityID) + } else { + klog.Warningf("MSI auth: no identity client ID provided and kubelet identity not available, MSI/IMDS will be used as fallback") + } } } diff --git a/pkg/blob/blob_test.go b/pkg/blob/blob_test.go index 563fd532c..2d0aaaa8b 100644 --- a/pkg/blob/blob_test.go +++ b/pkg/blob/blob_test.go @@ -2663,6 +2663,120 @@ func TestGetAuthEnvMSIAuthTypeSkipsIdentityEnvIfAlreadySet(t *testing.T) { assert.True(t, found, "Explicit AZURE_STORAGE_IDENTITY_CLIENT_ID should be preserved") } +func TestGetAuthEnvMSIDefaultsToKubeletIdentity(t *testing.T) { + d := NewFakeDriver() + d.cloud = &storage.AccountRepo{} + d.cloud.Config.AzureAuthConfig = azclient.AzureAuthConfig{ + UserAssignedIdentityID: "kubelet-identity-client-id", + } + + attrib := map[string]string{ + containerNameField: "containername", + storageAccountField: "accountname", + storageAuthTypeField: storageAuthTypeMSI, + } + secret := map[string]string{ + accountNameField: "accountname", + accountKeyField: "testkey", + } + volumeID := "rg#accountname#containername" + + _, _, _, _, authEnv, err := d.GetAuthEnv(context.TODO(), volumeID, "", attrib, secret) //nolint:dogsled + assert.NoError(t, err) + + found := false + for _, env := range authEnv { + if env == "AZURE_STORAGE_IDENTITY_CLIENT_ID=kubelet-identity-client-id" { + found = true + break + } + } + assert.True(t, found, "Should default to kubelet identity when AzureStorageIdentityClientID is not specified") +} + +func TestGetAuthEnvMSIWarningWhenNoIdentityAvailable(t *testing.T) { + d := NewFakeDriver() + d.cloud = &storage.AccountRepo{} + // UserAssignedIdentityID is empty — no kubelet identity available, falls back to IMDS + + attrib := map[string]string{ + containerNameField: "containername", + storageAccountField: "accountname", + storageAuthTypeField: storageAuthTypeMSI, + } + secret := map[string]string{ + accountNameField: "accountname", + accountKeyField: "testkey", + } + volumeID := "rg#accountname#containername" + + _, _, _, _, _, err := d.GetAuthEnv(context.TODO(), volumeID, "", attrib, secret) //nolint:dogsled + assert.NoError(t, err) // should not error, falls back to IMDS +} + +func TestGetAuthEnvMSIUppercase(t *testing.T) { + d := NewFakeDriver() + d.cloud = &storage.AccountRepo{} + d.cloud.Config.AzureAuthConfig = azclient.AzureAuthConfig{ + UserAssignedIdentityID: "kubelet-identity-client-id", + } + + attrib := map[string]string{ + containerNameField: "containername", + storageAccountField: "accountname", + storageAuthTypeField: "MSI", // uppercase as commonly used in docs/examples + } + secret := map[string]string{ + accountNameField: "accountname", + accountKeyField: "testkey", + } + volumeID := "rg#accountname#containername" + + _, _, _, _, authEnv, err := d.GetAuthEnv(context.TODO(), volumeID, "", attrib, secret) //nolint:dogsled + assert.NoError(t, err) + + found := false + for _, env := range authEnv { + if env == "AZURE_STORAGE_IDENTITY_CLIENT_ID=kubelet-identity-client-id" { + found = true + break + } + } + assert.True(t, found, "Should default to kubelet identity even with uppercase MSI") +} + +func TestGetAuthEnvMSIEmptyClientIDFallsBackToKubelet(t *testing.T) { + d := NewFakeDriver() + d.cloud = &storage.AccountRepo{} + d.cloud.Config.AzureAuthConfig = azclient.AzureAuthConfig{ + UserAssignedIdentityID: "kubelet-identity-client-id", + } + + attrib := map[string]string{ + containerNameField: "containername", + storageAccountField: "accountname", + storageAuthTypeField: storageAuthTypeMSI, + storageIdentityClientIDField: "", // empty value + } + secret := map[string]string{ + accountNameField: "accountname", + accountKeyField: "testkey", + } + volumeID := "rg#accountname#containername" + + _, _, _, _, authEnv, err := d.GetAuthEnv(context.TODO(), volumeID, "", attrib, secret) //nolint:dogsled + assert.NoError(t, err) + + found := false + for _, env := range authEnv { + if env == "AZURE_STORAGE_IDENTITY_CLIENT_ID=kubelet-identity-client-id" { + found = true + break + } + } + assert.True(t, found, "Empty AzureStorageIdentityClientID should fall back to kubelet identity") +} + func TestIsValidSubscriptionID(t *testing.T) { tests := []struct { desc string