Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: support workload identity token #1556

Open
wants to merge 9 commits into
base: blobfuse/2.4.1
Choose a base branch
from

Conversation

cvvz
Copy link
Contributor

@cvvz cvvz commented Nov 5, 2024

✅ What

Support provide workload identity token when authenticating rather than use the token file.

🤔 Why

Kubelet generate the token when mounting blob volume and we should pass the token to blobfuse2 rather than create a token file.

👩‍🔬 How to validate if applicable

I've already tested with blob-csi-driver

🔖 Related links

@andyzhangx
Copy link
Contributor

@vibhansa-msft this PR exposes WORKLOAD_IDENTITY_TOKEN as env variable, could you take a look? thanks.

EnvAzStorageSpnClientId = "AZURE_STORAGE_SPN_CLIENT_ID"
EnvAzStorageSpnClientSecret = "AZURE_STORAGE_SPN_CLIENT_SECRET"
EnvAzStorageSpnOAuthTokenFilePath = "AZURE_OAUTH_TOKEN_FILE"
EnvAzStorageSpnWorkloadIdentityToken = "WORKLOAD_IDENTITY_TOKEN"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if we use WORKLOAD_IDENTITY_TOKEN env var, who is responsible for token refresh?

Copy link
Contributor Author

@cvvz cvvz Nov 7, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The token was generated by kubelet when it try to mount volume, and it will be passed to csi driver and csi driver set it as WORKLOAD_IDENTITY_TOKEN env. Kubelet is responsible for updating the token ,but I think we should only use the token during mounting.

azspn.config.TenantID,
azspn.config.ClientID,
func(ctx context.Context) (string, error) {
return azspn.config.WorkloadIdentityToken, nil
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this option something like where user is providing the token directly to blobfuse?
If so, what is the process to refresh the token or validation of token ?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why do we need to save the token in a file? that's not safe

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My question was are we expecting a file name here in this env variable or directly the token. If directly token is what we expect then who will be responsible for the refresh. If its a file then re-reding the file (assuming the file now has the latest token) can work but giving directly the token will not work.

Copy link
Contributor Author

@cvvz cvvz Nov 7, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The token was generated by kubelet when try to mount the blob volume and pass to blobfuse for authentication. I think we only need the token for mounting. Kubelet is responsible for updating the token ,but I think we only need the token during mounting.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think here the problem is when token is expired, whether blobfuse mount is still valid or not

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I created a blobfuse pod and the io was not impacted when the token expires.
If the token expired, kubelet will generate a new token when we try to mount again.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If you remount then its fine as its a fresh start. Workflow we are discussing here is when Blobfuse remains mounted for a longer duration and in between the token expires. There will not be any remount in this case and newly generated token will not be forwarded to Blobfuse as well. In such case all storage REST call shall fail with 4xx errors. Can you validate this flow.

Copy link
Contributor Author

@cvvz cvvz Nov 15, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have tested that when the token expires, the blobfuse io won't be impacted. What kind of storage REST call do you mean? CSI driver does not have any storage REST call after volume mounted. Is there any REST call from blobfuse2 side? Can you show me the way to validate that?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Blobfuse interacts with Azure Storage via REST call only. Lets assume you are trying to upload a 10GB file. Job starts and file is divided into block and each block is uploaded via a seperate REST call (few uploads going on in parallel). Now while the upload is in progress the token expires. As Blobfuse is not updated with the new token, it will continue using the expired token to upload the remaining chunks but the storage backend will fail these calls with Auth error as the token is expired. This is the part where you need to somehow convey Blobfuse to start using a new token. Generally MSI/SPN case Blobfuse acquires a token and 5 minute before its expiry it refreshes the token internally. Here as the token is provided as an input, its responsibility of the token provider to take care of the refresh and provide the updated token to Blobfuse while the job is going on. This is what Sourav pointed earlier in his comment that the function you have implemented shall take care of token update as well.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If the token that you are sending it across to Blobfuse is not going to expire then its fine and I can approve the PR

@jainakanksha-msft
Copy link
Collaborator

@cvvz Please update PR description

@vibhansa-msft vibhansa-msft changed the base branch from main to blobfuse/2.4.1 November 29, 2024 08:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants