search

Azure / azure_preview_modules

Azure preview modules for Ansible
https://galaxy.ansible.com/azure/azure_preview_modules
43 stars 49 forks source link

Support azure cli credentials with multiple `subscription_id`s #358

Closed internetionals closed 4 years ago

internetionals commented 4 years ago

This is the same change I have a pull request for at the Ansible main repository, but it isn't entirely clear what the correct path for new changes is for the Azure Ansible modules.

See original pull request: https://github.com/ansible/ansible/pull/65331

I also couldn't find the doc_fragments part from the original change, so this change doesn't update the documentation, which the original does.

Also relates to these issues/pull requests in the main Ansible repository:

Below is the text from the pull request against the main Ansible repository:

SUMMARY

When using credentials obtained from the azure cli only the default subscription can be used, even though the azure cli is authenticated for multiple subscriptions.

This change passes any optionally specified subscription_id (similar to how it's done for auth_source: msi) along when requesting the azure cli credentials. If none is specified it falls back to the current behaviour of selecting the default subscription.

The only other change that had to be made was in the auth_source: auto case where we would always assume that all credential information is passed using arguments when subscription_id is set. I changed this to check the fields that actually refer to specific credentials (namely client_id (for service principals) and ad_user (for user names)). This way we still fall through to the azure cli method, eventhough only subscription_id is explicitly set.

Fixes #321 Fixes ansible#63182

This effectively fixes the same problem as ansible#48089 is trying to fix, only in a way that is more consistent with how credentials and subscription id's are determined for other auth_sources. This was one of the main criticisms that's probably holding that pull request up.

ISSUE TYPE
COMPONENT NAME

azure_rm_common

ADDITIONAL INFORMATION

We have different Azure subscriptions for our prod and dev environments. By default we only work on the dev environment, but we have a number of playbooks that we want to work with all virtual machines at once.

Demonstration playbook:

- name: Get azure vm info from different subscriptions
  hosts: localhost
  tasks:
  - azure_rm_virtualmachine_info:
      name: prod-db
      resource_group: prod
      subscription_id: XXXXXXXX-XXXX-XXXX-XXXX-XXXXXXXXXXXX
      auth_source: cli
  - azure_rm_virtualmachine_info:
      name: dev-db
      resource_group: dev
      subscription_id: YYYYYYYY-YYYY-YYYY-YYYY-YYYYYYYYYYYY
      auth_source: cli

Before:

$ ansible-playbook playbooks/az_info.yml

PLAY [Get azure vm info from different subscriptions] *********************************************************************************

TASK [azure_rm_virtualmachine_info] ***************************************************************************************************
fatal: [localhost]: FAILED! => changed=false
  msg: |-
    Error getting virtual machine prod-db - Azure Error: ResourceGroupNotFound
    Message: Resource group 'prod' could not be found.

PLAY RECAP ****************************************************************************************************************************
localhost                  : ok=0    changed=0    unreachable=0    failed=1    skipped=0    rescued=0    ignored=0

After:

$ ansible-playbook playbooks/az_info.yml

PLAY [Get azure vm info from different subscriptions] *********************************************************************************

TASK [azure_rm_virtualmachine_info] ***************************************************************************************************
ok: [localhost]

TASK [azure_rm_virtualmachine_info] ***************************************************************************************************
ok: [localhost]

PLAY RECAP ****************************************************************************************************************************
localhost                  : ok=2    changed=0    unreachable=0    failed=0    skipped=0    rescued=0    ignored=0
msftclas commented 4 years ago

CLA assistant check
All CLA requirements met.

Fred-sun commented 4 years ago

@internetionals Could you pleae help fixes the CI fail, when you're free it will push for PR review and meger! Thank you very much!

internetionals commented 4 years ago

First inspection of those CI failures don't appear to be related to this specific change. There should not be any unicode related issues being introduced that weren't there before.

This change is centered arount the state of the auth_source: cli logic, that gets read from the az tool, but these tests actually use a totally different auth_source: credential_file or auth_source: env.

The only other field we use is the subscription_id which can come from the env, but that should never contain unicode characters (being a UUID). Furthermore, this logic handles it identically to eg. the auth_source: msi code, so that should logically suffer the same issues if that were the case.

Having said that, let's look at the errors (after a rerun of the tests):

internetionals commented 4 years ago

Too bad, my PYTHONIOENCODING=utf-8 didn't appear to help. I'm creating new issues for these problems.

Fred-sun commented 4 years ago

@internetionalsThank you for taking the time to contribute to this PR. We will transfer ansible 's azure module related Issue and PR to azure collection (https://github.com/ansible-collections/azure/pulls), can you transfer the Issue to azure collection repo?

internetionals commented 4 years ago

I already submitted this pull request there, see: ansible-collections/azure#53