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

Adding basic support for Azure MSI user-assigned identity #54884

Open
wants to merge 1 commit into
base: devel
from

Conversation

Projects
None yet
4 participants
@Moeser
Copy link

Moeser commented Apr 4, 2019

SUMMARY

If more than one user-assigned identity is assigned to a host, then an identifier is required to specify which credentials are retrieved. This change uses the existing support for client_id to retrieve the user-assigned identity by client_id.

ISSUE TYPE
  • Feature Pull Request
COMPONENT NAME

azure_rm

ADDITIONAL INFORMATION

A client_id is only required when two or more user-assigned identities are assigned to a host, and optional if only one user-assigned identity is assigned.

This is only "basic" support, because an object_id or full object identifier can be used instead, but there's no existing code to retrieve those from the yml.

To reproduce:

  • Assign two or more user-assigned identities to a system.
  • Try to use ansible on that system with the azure_rm plugin and MSI as the auth source.
  • Witness a failure to retrieve credentials from MSI.
Adding support for Azure MSI user-assigned identity
If more than one user-assigned identity is assigned to a host, then an identifier is required to specify which credentials are retrieved.  This change uses the existing support for client_id to retrieve the user-assigned identity by client_id.
@ansibot

This comment has been minimized.

@yuwzho

This comment has been minimized.

Copy link
Contributor

yuwzho commented Apr 8, 2019

Would you mind updating the azure_rm_common as well? This is used in all the azure modules. I will test the functionality of the MSI.

def _get_msi_credentials(self, subscription_id_param=None):

@ansibot ansibot removed the needs_triage label Apr 8, 2019

@Fred-sun

This comment has been minimized.

Copy link
Contributor

Fred-sun commented Apr 11, 2019

@Moeser Thanks for your contribution. Could you can give a feedback according by the above comments? Thanks!

@ansibot ansibot added the stale_ci label Apr 19, 2019

@Moeser

This comment has been minimized.

Copy link
Author

Moeser commented Apr 19, 2019

Would you mind updating the azure_rm_common as well? This is used in all the azure modules. I will test the functionality of the MSI.

ansible/lib/ansible/module_utils/azure_rm_common.py

Line 1152 in 3be01cc
def _get_msi_credentials(self, subscription_id_param=None):

I don't think I understand this question. The file I modified is azure_rm_common.py, on the line you quoted.

@yuwzho

This comment has been minimized.

Copy link
Contributor

yuwzho commented Apr 22, 2019

shipit

My bad, I thought you modified the azure_rm.py.
I have already tested it and everything works fine.

@Moeser

This comment has been minimized.

Copy link
Author

Moeser commented Apr 23, 2019

This is my first contribution here. Anything I need to do from my side to merge, or will it be merged automatically on the next build? Also, did I create the PR for the correct branch, or should I have picked stable-2.8?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.