[WIP] feat: default to kubelet identity when AzureStorageIdentityClientID is empty for MSI auth by andyzhangx · Pull Request #2430 · kubernetes-sigs/blob-csi-driver

andyzhangx · 2026-04-27T13:52:19Z

What type of PR is this?
/kind feature

What this PR does / why we need it:
When AzureStorageAuthType is set to MSI but AzureStorageIdentityClientID is not specified, the driver now:

Defaults to the kubelet identity (UserAssignedIdentityID from cloud config) — this is the built-in managed identity bound to the AKS agent node pool (named {cluster-name}-agentpool)
Returns a clear error if the kubelet identity is also not available, instead of silently proceeding and failing at mount time with a cryptic error

This improves the user experience for the common case where users set AzureStorageAuthType: MSI in their PV/StorageClass but forget to specify the identity client ID — the driver will automatically use the kubelet identity if available.

How does this PR make you feel?
Less cryptic mount failures 🎉

Does this PR introduce a user-facing change?

When AzureStorageAuthType is set to MSI and AzureStorageIdentityClientID is not specified, the driver defaults to the kubelet identity. If no identity is available, a clear error is returned.

…s empty for MSI auth When AzureStorageAuthType is set to MSI but AzureStorageIdentityClientID is not specified, the driver now defaults to the kubelet identity (UserAssignedIdentityID from cloud config). If the kubelet identity is also not available, the driver returns a clear error instead of silently proceeding and failing at mount time.

k8s-ci-robot · 2026-04-27T13:52:26Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: andyzhangx

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Details

Needs approval from an approver in each of these files:

~~OWNERS~~ [andyzhangx]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

Copilot

Pull request overview

Adds stricter MSI-auth identity selection in GetAuthEnv to improve failure clarity when an identity isn’t explicitly provided, aligning behavior with a “default to kubelet identity” expectation in AKS-style setups.

Changes:

Default MSI mounts to UserAssignedIdentityID from cloud config when no AZURE_STORAGE_IDENTITY_* env is already set.
Return an early, clearer error when MSI is selected but neither an explicit identity nor a kubelet identity is available.
Add unit tests covering the new defaulting and error behavior.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 4 comments.

File	Description
pkg/blob/blob.go	Changes MSI auth env construction: default to kubelet identity or return a clearer error when missing.
pkg/blob/blob_test.go	Adds tests validating kubelet-identity defaulting and the new “no identity available” error.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

…ead of error for IMDS fallback

Copilot

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-04-27T14:24:07Z

+	if strings.EqualFold(azureStorageAuthType, storageAuthTypeMSI) {
+		// check whether authEnv contains a non-empty AZURE_STORAGE_IDENTITY_ value
 		containsIdentityEnv := false


azureStorageAuthType is compared using == storageAuthTypeMSI, but docs/examples commonly set AzureStorageAuthType: MSI (uppercase). This means the MSI-specific identity defaulting/error path won’t run for those values. Use strings.EqualFold(azureStorageAuthType, storageAuthTypeMSI) here (consistent with the earlier secret-handling check) and add/adjust a unit test to cover uppercase MSI.

Copilot · 2026-04-27T14:24:07Z

+		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)


This change makes MSI auth fail fast when neither AzureStorageIdentityClientID nor cloud.Config.AzureAuthConfig.UserAssignedIdentityID is set. That can be a breaking behavior change for clusters that rely on system-assigned MSI/IMDS (where omitting the client ID can still be valid). If the intention is to preserve IMDS fallback, consider logging a warning and continuing instead of returning an error; otherwise, please update the public docs/parameters to reflect that an identity client ID (explicit or kubelet) is now required for AzureStorageAuthType=MSI.

Copilot · 2026-04-27T14:24:08Z

+	_, _, _, _, _, err := d.GetAuthEnv(context.TODO(), volumeID, "", attrib, secret) //nolint:dogsled
+	assert.NoError(t, err) // should not error, falls back to IMDS
+}
+


The new MSI defaulting behavior isn’t covered for two important inputs that occur in docs/config: (1) AzureStorageAuthType: MSI (uppercase), and (2) AzureStorageIdentityClientID present but empty/whitespace. Adding targeted unit tests for these cases will prevent regressions and will catch the current case-sensitivity/empty-value pitfalls.

Suggested change

func TestGetAuthEnvMSIDefaultsToKubeletIdentityWhenClientIDWhitespace(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: " ",

}

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 empty or whitespace")

}

andyzhangx · 2026-04-27T14:45:44Z

/retest

andyzhangx · 2026-04-28T02:39:35Z

/retest

k8s-ci-robot added the kind/feature Categorizes issue or PR as related to a new feature. label Apr 27, 2026

k8s-ci-robot added the cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. label Apr 27, 2026

k8s-ci-robot requested review from cvvz and feiskyer April 27, 2026 13:52

k8s-ci-robot added approved Indicates a PR has been approved by an approver from all required OWNERS files. size/M Denotes a PR that changes 30-99 lines, ignoring generated files. labels Apr 27, 2026

fix: add nolint:dogsled for test with expected error return

cc21f55

andyzhangx requested a review from Copilot April 27, 2026 14:01

Copilot started reviewing on behalf of andyzhangx April 27, 2026 14:01 View session

Copilot AI reviewed Apr 27, 2026

View reviewed changes

Comment thread pkg/blob/blob.go Outdated

Comment thread pkg/blob/blob.go Outdated

Comment thread pkg/blob/blob_test.go

Comment thread pkg/blob/blob.go Outdated

andyzhangx requested a review from Copilot April 27, 2026 14:20

fix: use case-insensitive MSI check, handle empty clientID, warn inst…

ab09ab0

…ead of error for IMDS fallback

k8s-ci-robot added size/L Denotes a PR that changes 100-499 lines, ignoring generated files. and removed size/M Denotes a PR that changes 30-99 lines, ignoring generated files. labels Apr 27, 2026

Copilot started reviewing on behalf of andyzhangx April 27, 2026 14:20 View session

Copilot AI reviewed Apr 27, 2026

View reviewed changes

fix: gofmt alignment

85b77c4

andyzhangx changed the title ~~feat: default to kubelet identity when AzureStorageIdentityClientID is empty for MSI auth~~ [WIP] feat: default to kubelet identity when AzureStorageIdentityClientID is empty for MSI auth Apr 28, 2026

k8s-ci-robot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Apr 28, 2026

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[WIP] feat: default to kubelet identity when AzureStorageIdentityClientID is empty for MSI auth#2430

[WIP] feat: default to kubelet identity when AzureStorageIdentityClientID is empty for MSI auth#2430
andyzhangx wants to merge 4 commits into
kubernetes-sigs:masterfrom
andyzhangx:default-kubelet-identity-msi

andyzhangx commented Apr 27, 2026

Uh oh!

k8s-ci-robot commented Apr 27, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Apr 27, 2026

Uh oh!

Copilot AI Apr 27, 2026

Uh oh!

Copilot AI Apr 27, 2026

Uh oh!

andyzhangx commented Apr 27, 2026

Uh oh!

andyzhangx commented Apr 28, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

+func TestGetAuthEnvMSIDefaultsToKubeletIdentityWhenClientIDWhitespace(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:    "   ",
+	}
+	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 empty or whitespace")
+}

Conversation

andyzhangx commented Apr 27, 2026

Uh oh!

k8s-ci-robot commented Apr 27, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI Apr 27, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 27, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 27, 2026

Choose a reason for hiding this comment

Uh oh!

andyzhangx commented Apr 27, 2026

Uh oh!

andyzhangx commented Apr 28, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants