-
Notifications
You must be signed in to change notification settings - Fork 68
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
patch for managed identity storage authentication #777
base: main
Are you sure you want to change the base?
patch for managed identity storage authentication #777
Conversation
15fd0a0
to
0307e67
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is very helpful! There is more to be done for this update (fix the unit tests, configs, etc.), but that's a great start. I'll split this PR into several smaller updates and work on them together with @DelphianCalamity
We'll close that PR once all its functionality is in master. For now I'll mark it as draft
@DelphianCalamity please read the following Contributor License Agreement(CLA). If you agree with the CLA, please reply with the following information.
Contributor License AgreementContribution License AgreementThis Contribution License Agreement (“Agreement”) is agreed to by the party signing below (“You”),
|
1 similar comment
@DelphianCalamity please read the following Contributor License Agreement(CLA). If you agree with the CLA, please reply with the following information.
Contributor License AgreementContribution License AgreementThis Contribution License Agreement (“Agreement”) is agreed to by the party signing below (“You”),
|
…FileShareService (#779) 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 --------- Co-authored-by: Brian Kroth <[email protected]>
This is a temporary patch for switching to managed identity storage authentication.
Users must create a managed identity and assign to it proper "contributor" roles that allow it to manage the file share.
Then they must pass the managed identity's info
managedIdentityClientId
andmanagedIdentityName
to MLOS as global parameters.The managed identity must be assigned to the MLOS host in order for the
local file share service
to work.Implementation Details:
- The managed identity must be assigned to all provisioned VMs.Currently, I assign the identity using the REST API right after the VM has been provisioned. A better approach would be to assign the identity directly using the ARM template. The reason I did not use the better/easier approach is that I was somehow tricked after reading this into thinking that it was not supported! I intend to switch soon.
[EDIT]: now assigning ID using ARM instead
azure-file-share-service
instead which I updated to use the Managed Identity Credential. To make that script available to the provisioned VMs I packaged it and uploaded it to PyPI . Then, scripts booting/setting up the VM must install the package (e.g. in boot-ubuntu.jsonc) in order to be able to interact with the file share. The proper, long-term solution would be to enable mounting again after adding to it support for managed identities.I updated the config files for the
RedisBench
experiment and verified that the flow works.