Skip to content

Use token-based authentication instead of the access key in the AzureFileShareService - #779

Merged
Sergiy Matusevych (motus) merged 24 commits into
microsoft:mainfrom
motus:sergiym/svc/fs_token_auth
Jul 24, 2024
Merged

Sergiy Matusevych (motus) merged 24 commits into
microsoft:mainfrom
motus:sergiym/svc/fs_token_auth

Conversation

@motus

@motus Sergiy Matusevych (motus) commented Jul 9, 2024 •

Copy link
Copy Markdown
Member

Make AzureFileShareService class use token-based credential instead of the access key. This PR is part of #777

NOTE:

  • We have to use late initialization of the _share_client to avoid issues with JSON schema validation tests.
  • More PRs will follow:
    • Use to azcopy on the remote VMs instead of mounting the file share using the access key
    • Remove references to "storageAccountKey" from configurations and tests and document the new mechanism

@bpkroth

Copy link
Copy Markdown
Contributor

Part of why we went with the mount smb method was reduced dependencies on the target VM.

Use of azcopy instead might be a little more challenging on that front, but let's see.

@bpkroth

Brian Kroth (bpkroth) commented Jul 22, 2024 •

Copy link
Copy Markdown
Contributor

As long as we're removing account storage key, does removing fileshare also make sense? We could use blobs instead I guess. Maybe less nice though and too many changes.

@bpkroth Brian Kroth (bpkroth) changed the title Use token-based authentication instead of the access key in the AzureFlieShareService Use token-based authentication instead of the access key in the AzureFileShareService Jul 23, 2024
Comment thread mlos_bench/mlos_bench/services/remote/azure/azure_fileshare.py

@bpkroth Brian Kroth (bpkroth) left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Mostly looks good. Only feedback was to move the SupportsAuth check to __init__ so we can check for proper configs at startup time rather than at file transfer need time which might be after some experiment trial has already run.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants