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

Allow module-level subscription id to be used for cross-subscription resource management #694

Merged
merged 1 commit into from
Nov 22, 2021

Conversation

l3ender
Copy link
Contributor

@l3ender l3ender commented Nov 20, 2021

SUMMARY

This change allows the module defined parameter of subscription_id to be used for management clients (e.g. when retrieving resources). It does NOT change how authentication to Azure operates.

This is helpful when using a credentials file for authentication. The credentials file uses subscription A (authentication), but a playbook needs to retrieve resources which are in subscription B. Adding support for using the module-provided subscription_id parameter allows for playbooks to operate under a single set of credentials but supports cross-subscription resource loading (providing the credentials have access to other subscription).

Example:

Credentials file:

[default]
subscription_id=aaaaaaaa-aaaa-aaaa-aaaa-aaaaaaaaaaaa
client_id=xxx
tenant=xxx
secret=xxx

At runtime, a playbook wants to use resources in a different subscription:

- name: "Get container registry detail in different subscription"
  azure.azcollection.azure_rm_containerregistry_info:
    name: "my-container-registry"
    resource_group: "my-resource-group"
    retrieve_credentials: true
    subscription_id: "bbbbbbbb-bbbb-bbbb-bbbb-bbbbbbbbbbbb"

Before this change, registries for subscription A would be retrieved instead of those in subscription B.

ISSUE TYPE
  • Feature Pull Request
COMPONENT NAME

Any/all.

ADDITIONAL INFORMATION

I am unsure if it's possible to add test coverage for this, but I am using in our environment successfully. The use case is we have a container registry in one subscription which holds all our Docker images. We deploy webapps using the Docker images to multiple subscriptions and need Ansible to support cross-subscription resource loading.

@l3ender
Copy link
Contributor Author

l3ender commented Nov 20, 2021

@Fred-sun Please take a look at this and offer your thoughts. Let me know if I can provide any other information or detail. Thank you!!

@Fred-sun
Copy link
Collaborator

@Fred-sun Please take a look at this and offer your thoughts. Let me know if I can provide any other information or detail. Thank you!!

@l3ender Okay, we'll move forward with the merger as soon as possible. Thank you for your contribution!

@Fred-sun Fred-sun added medium_priority Medium priority plugin new plugin work in In trying to solve, or in working with contributors labels Nov 22, 2021
@Fred-sun
Copy link
Collaborator

@l3ender Thank you for your contribution and this change is not recommended. First you specify subscription_id here, but how do you get its client_id and tenant? Second, the order in which credentials are obtained is defined in the function _get_credentials. If you want to update the order, you are advised to contribute _get_credentials. Thank you very much!

@l3ender
Copy link
Contributor Author

l3ender commented Nov 22, 2021

Hi @Fred-sun, I think you misunderstand usage of the change. The use of module-level subscription_id parameter is only for usage when dealing with management clients (e.g. interacting with resources). The change does not affect authentication or usage of credentials at all, so client_id and tenant are not applicable (because those are used during authentication which this change does not affect).

Hope it makes sense! Let me know if I can clarify any further. Thank you!

@Fred-sun
Copy link
Collaborator

@l3ender I'm sorry. I'll review it. Thank you very much!

@xuzhang3
Copy link
Collaborator

LGTM

@xuzhang3 xuzhang3 merged commit 486274f into ansible-collections:dev Nov 22, 2021
@xuzhang3 xuzhang3 deleted the module-subscription-id branch November 22, 2021 09:32
@l3ender
Copy link
Contributor Author

l3ender commented Apr 22, 2022

Resolves #446.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
medium_priority Medium priority plugin new plugin work in In trying to solve, or in working with contributors
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants