feat: support workload identity setting in static PV mount #1566
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
k8s-ci-robot
added
kind/feature
cvvz
changed the title
k8s-ci-robot
added
the
do-not-merge/work-in-progress
andyzhangx
reviewed
Nov 14, 2023
|
|
||
| ## prerequisite | ||
|
|
||
| ### 1. Create a cluster with oidc-issuer enabled |
There was a problem hiding this comment.
there is already doc here: https://github.com/kubernetes-sigs/azurefile-csi-driver/blob/master/docs/workload-identity.md, it's better removing redundant doc steps otherwise we need to maintain two doc pages.
There was a problem hiding this comment.
Basically, these are two very different features for different purpose although they both use federated workload identity and actually there are very few common steps between these two docs and maintain two docs should be better.
|
btw, |
k8s-ci-robot
added
the
needs-rebase
cvvz
changed the title
k8s-ci-robot
added
size/L
andyzhangx
reviewed
Dec 14, 2023
| az aks get-credentials -n $CLUSTER_NAME -g $RESOURCE_GROUP --overwrite-existing | ||
| ``` | ||
|
|
||
| ### 2. Create a storage account and fileshare (or use your own storage account and fileshare) |
There was a problem hiding this comment.
remove the account creation steps
|
|
||
| az group create --name $RESOURCE_GROUP --location $REGION | ||
|
|
||
| az aks create -n $CLUSTER_NAME -g $RESOURCE_GROUP --enable-oidc-issuer |
There was a problem hiding this comment.
let's only mention --enable-oidc-issuer should be specified for cluster creation.
k8s-ci-robot
added
needs-rebase
andyzhangx
reviewed
Dec 18, 2023
cvvz
changed the title
k8s-ci-robot
removed
the
do-not-merge/work-in-progress
andyzhangx
reviewed
Dec 26, 2023
pkg/azurefile/nodeserver.go
Outdated
| @@ -59,6 +59,19 @@ func (d *Driver) NodePublishVolume(ctx context.Context, req *csi.NodePublishVolu | |||
| mountPermissions := d.mountPermissions | |||
| context := req.GetVolumeContext() | |||
| if context != nil { | |||
| // token request | |||
| if context[serviceAccountTokenField] != "" && context[clientIDField] != "" { | |||
There was a problem hiding this comment.
clientID is a user input parameter, since all user input parameters are not case sensitive, suggest using a func to check whether it's a token request
There was a problem hiding this comment.
I use a func to check the clientIDField for both upper and lower case.
pkg/azurefile/nodeserver.go
Outdated
| @@ -155,6 +168,12 @@ func (d *Driver) NodeStageVolume(ctx context.Context, req *csi.NodeStageVolumeRe | |||
|
|
|||
| volumeID := req.GetVolumeId() | |||
| context := req.GetVolumeContext() | |||
|
|
|||
| if context[serviceAccountTokenField] == "" && context[clientIDField] != "" { | |||
There was a problem hiding this comment.
clientID is a user input parameter, since all user input parameters are not case sensitive, suggest using a func to check whether it's a token request
andyzhangx
reviewed
Dec 27, 2023
|
/retest |
andyzhangx
reviewed
Dec 27, 2023
pkg/azurefile/azurefile.go
Outdated
| @@ -765,8 +778,16 @@ func (d *Driver) GetAccountInfo(ctx context.Context, volumeID string, secrets, r | |||
| } | |||
| } | |||
|
|
|||
| // if client id is specified, we only use service account token to get account key | |||
| if clientID != "" { | |||
| klog.Info("clientID is specified, use service account token to get account key") | |||
There was a problem hiding this comment.
klog.V(2).Infof("clientID(%s) is specified, use service account token to get account key", clientID)
commit 15c7f99 Merge: 8388a01 7fe3ebc Author: weizhichen <weizhichen@microsoft.com> Date: Wed Dec 27 09:37:50 2023 +0000 Merge branch 'master' of https://github.com/kubernetes-sigs/azurefile-csi-driver into workload-identity commit 8388a01 Author: weizhichen <weizhichen@microsoft.com> Date: Wed Dec 27 09:33:51 2023 +0000 fix commit 44c4812 Author: weizhichen <weizhichen@microsoft.com> Date: Wed Dec 27 06:15:20 2023 +0000 fix commit 09e5b67 Author: weizhichen <weizhichen@microsoft.com> Date: Wed Dec 27 04:00:47 2023 +0000 fix commit 354f4d7 Author: weizhichen <weizhichen@microsoft.com> Date: Tue Dec 26 08:29:07 2023 +0000 helm commit 960912d Merge: 0c58154 5dd86f5 Author: weizhichen <weizhichen@microsoft.com> Date: Tue Dec 26 08:00:51 2023 +0000 Merge branch 'master' of https://github.com/kubernetes-sigs/azurefile-csi-driver into workload-identity commit 0c58154 Merge: e3adfa4 d519073 Author: weizhichen <weizhichen@microsoft.com> Date: Mon Dec 18 02:18:53 2023 +0000 Merge branch 'master' of https://github.com/kubernetes-sigs/azurefile-csi-driver into workload-identity commit e3adfa4 Author: weizhichen <weizhichen@microsoft.com> Date: Thu Dec 14 03:17:25 2023 +0000 spell commit 78c82b5 Author: weizhichen <weizhichen@microsoft.com> Date: Thu Dec 14 03:09:16 2023 +0000 doc commit 7bb959f Author: weizhichen <weizhichen@microsoft.com> Date: Thu Dec 14 03:05:08 2023 +0000 doc commit 2dde59d Author: weizhichen <weizhichen@microsoft.com> Date: Wed Dec 13 14:45:37 2023 +0000 doc commit e774ff5 Author: weizhichen <weizhichen@microsoft.com> Date: Wed Dec 13 13:46:37 2023 +0000 update go mod commit 2e79ca3 Merge: 7846026 6cfe218 Author: weizhichen <weizhichen@microsoft.com> Date: Wed Dec 13 13:19:41 2023 +0000 Merge branch 'master' of https://github.com/kubernetes-sigs/azurefile-csi-driver into workload-identity commit 7846026 Author: weizhichen <weizhichen@microsoft.com> Date: Wed Dec 13 13:07:48 2023 +0000 update go mod commit 0446a46 Author: weizhichen <weizhichen@microsoft.com> Date: Fri Nov 24 08:52:03 2023 +0000 update cloud-provider-azure commit ff2aeb4 Author: weizhichen <weizhichen@microsoft.com> Date: Tue Nov 14 12:29:14 2023 +0000 doc commit 633641b Author: weizhichen <weizhichen@microsoft.com> Date: Tue Nov 14 12:01:02 2023 +0000 add docs commit afcb818 Author: weizhichen <weizhichen@microsoft.com> Date: Tue Nov 14 11:20:40 2023 +0000 feat: support workload identity setting in static PV mount on AKS
cvvz
force-pushed
the
workload-identity
branch
from
15c7f99 to
64c67e3
Compare
andyzhangx
reviewed
Dec 27, 2023
|
/retest |
1 similar comment
|
/retest |
k8s-ci-robot
added
the
lgtm
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: andyzhangx, cvvz The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
k8s-ci-robot
added
the
approved
|
/cherrypick release-1.29 |
|
@andyzhangx: #1566 failed to apply on top of branch "release-1.29": In response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. |
|
@cvvz can you cherrypick to release-1.29 and also make similar change in blob csi driver? thanks. |
|
@cvvz: The following tests failed, say
Full PR test history. Your PR dashboard. Please help us cut down on flakes by linking to an open issue when you hit one in your PR. Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. I understand the commands that are listed here. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
What type of PR is this?
/kind feature
What this PR does / why we need it:
Which issue(s) this PR fixes:
Fixes
Azure/AKS#3432
Azure/azure-storage-fuse#1049 (comment)
Requirements:
Special notes for your reviewer:
documentation: https://github.com/kubernetes-sigs/azurefile-csi-driver/pull/1566/files#diff-ba6bdf5d643d138b4a41b51808bf8041b48d4d519ebc3c511b4464d14a6ba639
Here are the csi driver's logs:
Release note: