Add support for azure auth method #293

lapwingcloud · 2022-08-14T06:48:04Z

SUMMARY

Fixes #78

Supports 3 types of azure auth

use a azure access token (jwt) directly
use azure service principal
use azure managed identity

ISSUE TYPE

Feature Pull Request

COMPONENT NAME

module_utils/_auth_method_azure.py

ADDITIONAL INFORMATION

Currently I'm not sure how to run unit tests locally, but I verify this on my azure vm with

jwt

jchen@vm-vault:~$ curl -s 'http://169.254.169.254/metadata/identity/oauth2/token?api-version=2018-02-01&resource=https%3A%2F%2Fmanagement.azure.com%2F&client_id=a57bbbed-d82a-4321-9d6f-abc89542730e' -H Metadata:true | jq
{
  "access_token": "[jwt]",
  "client_id": "a57bbbed-d82a-4321-9d6f-abc89542730e",
  "expires_in": "85943",
  "expires_on": "1660545929",
  "ext_expires_in": "86399",
  "not_before": "1660459229",
  "resource": "https://management.azure.com/",
  "token_type": "Bearer"
}

- hosts: localhost
  connection: local
  gather_facts: false
  tasks:
    - name: Get vault secret
      community.hashi_vault.vault_kv2_get:
        url: https://vault.jchen.au:8200
        engine_mount_point: kv
        path: test
        auth_method: azure
        role_id: msi-vault
        jwt: "[jwt]"
      register: response

    - name: Debug vault response
      ansible.builtin.debug:
        msg:
          - "Secret: {{ response.secret }}"
          - "Data: {{ response.data }} (contains secret data & metadata in kv2)"
          - "Metadata: {{ response.metadata }}"
          - "Full response: {{ response.raw }}"
          - "Value of key 'FOO' in the secret: {{ response.secret.FOO }}"

jchen@vm-vault:~$ ansible-playbook ansible/kv_get.yaml
[WARNING]: No inventory was parsed, only implicit localhost is available
[WARNING]: provided hosts list is empty, only localhost is available. Note that the implicit localhost does not match 'all'

PLAY [localhost] ***********************************************************************************************************************************************************************************************************************************************************************

TASK [Get vault secret] ****************************************************************************************************************************************************************************************************************************************************************
ok: [localhost]

TASK [Debug vault response] ************************************************************************************************************************************************************************************************************************************************************
ok: [localhost] => {
    "msg": [
        "Secret: {'FOO': 'bar'}",
        "Data: {'data': {'FOO': 'bar'}, 'metadata': {'created_time': '2022-08-14T00:46:30.180160604Z', 'custom_metadata': None, 'deletion_time': '', 'destroyed': False, 'version': 1}} (contains secret data & metadata in kv2)",
        "Metadata: {'created_time': '2022-08-14T00:46:30.180160604Z', 'custom_metadata': None, 'deletion_time': '', 'destroyed': False, 'version': 1}",
        "Full response: {'request_id': 'd1555c0a-67e7-36e9-5353-bc638f867ff5', 'lease_id': '', 'renewable': False, 'lease_duration': 0, 'data': {'data': {'FOO': 'bar'}, 'metadata': {'created_time': '2022-08-14T00:46:30.180160604Z', 'custom_metadata': None, 'deletion_time': '', 'destroyed': False, 'version': 1}}, 'wrap_info': None, 'warnings': None, 'auth': None}",
        "Value of key 'FOO' in the secret: bar"
    ]
}

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

service principal

- hosts: localhost
  connection: local
  gather_facts: false
  tasks:
    - name: Get vault secret
      community.hashi_vault.vault_kv2_get:
        url: https://vault.jchen.au:8200
        engine_mount_point: kv
        path: test
        auth_method: azure
        role_id: msi-vault
        azure_tenant_id: "702a96ad-ac90-48e9-bbcb-759edc3ad591"
        azure_client_id: "8d74f877-fc8e-43bf-af74-481027e1a084"
        azure_client_secret: "[secret]"
      register: response

    - name: Debug vault response
      ansible.builtin.debug:
        msg:
          - "Secret: {{ response.secret }}"
          - "Data: {{ response.data }} (contains secret data & metadata in kv2)"
          - "Metadata: {{ response.metadata }}"
          - "Full response: {{ response.raw }}"
          - "Value of key 'FOO' in the secret: {{ response.secret.FOO }}"

jchen@vm-vault:~$ ansible-playbook ansible/kv_get.yaml
[WARNING]: No inventory was parsed, only implicit localhost is available
[WARNING]: provided hosts list is empty, only localhost is available. Note that the implicit localhost does not match
'all'

PLAY [localhost] *******************************************************************************************************
TASK [Get vault secret] ************************************************************************************************ok: [localhost]

TASK [Debug vault response] ********************************************************************************************ok: [localhost] => {
    "msg": [
        "Secret: {'FOO': 'bar'}",
        "Data: {'data': {'FOO': 'bar'}, 'metadata': {'created_time': '2022-08-14T00:46:30.180160604Z', 'custom_metadata': None, 'deletion_time': '', 'destroyed': False, 'version': 1}} (contains secret data & metadata in kv2)",
        "Metadata: {'created_time': '2022-08-14T00:46:30.180160604Z', 'custom_metadata': None, 'deletion_time': '', 'destroyed': False, 'version': 1}",
        "Full response: {'request_id': '42dc6f76-7d2e-7316-3d7f-d71ae8f3f043', 'lease_id': '', 'renewable': False, 'lease_duration': 0, 'data': {'data': {'FOO': 'bar'}, 'metadata': {'created_time': '2022-08-14T00:46:30.180160604Z', 'custom_metadata': None, 'deletion_time': '', 'destroyed': False, 'version': 1}}, 'wrap_info': None, 'warnings': None, 'auth': None}",
        "Value of key 'FOO' in the secret: bar"
    ]
}

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

managed identity

- hosts: localhost
  connection: local
  gather_facts: false
  tasks:
    - name: Get vault secret
      community.hashi_vault.vault_kv2_get:
        url: https://vault.jchen.au:8200
        engine_mount_point: kv
        path: test
        auth_method: azure
        role_id: msi-vault
        azure_client_id: "a57bbbed-d82a-4321-9d6f-abc89542730e"
      register: response

    - name: Debug vault response
      ansible.builtin.debug:
        msg:
          - "Secret: {{ response.secret }}"
          - "Data: {{ response.data }} (contains secret data & metadata in kv2)"
          - "Metadata: {{ response.metadata }}"
          - "Full response: {{ response.raw }}"
          - "Value of key 'FOO' in the secret: {{ response.secret.FOO }}"

result

jchen@vm-vault:~$ ansible-playbook ansible/kv_get.yaml
[WARNING]: No inventory was parsed, only implicit localhost is available
[WARNING]: provided hosts list is empty, only localhost is available. Note that the implicit localhost does not match 'all'

PLAY [localhost] ***********************************************************************************************************************************************************************************************************************************************************************

TASK [Get vault secret] ****************************************************************************************************************************************************************************************************************************************************************
ok: [localhost]

TASK [Debug vault response] ************************************************************************************************************************************************************************************************************************************************************
ok: [localhost] => {
    "msg": [
        "Secret: {'FOO': 'bar'}",
        "Data: {'data': {'FOO': 'bar'}, 'metadata': {'created_time': '2022-08-14T00:46:30.180160604Z', 'custom_metadata': None, 'deletion_time': '', 'destroyed': False, 'version': 1}} (contains secret data & metadata in kv2)",
        "Metadata: {'created_time': '2022-08-14T00:46:30.180160604Z', 'custom_metadata': None, 'deletion_time': '', 'destroyed': False, 'version': 1}",
        "Full response: {'request_id': '85d6c8dd-cc7d-e7b5-817c-e69a5d21e383', 'lease_id': '', 'renewable': False, 'lease_duration': 0, 'data': {'data': {'FOO': 'bar'}, 'metadata': {'created_time': '2022-08-14T00:46:30.180160604Z', 'custom_metadata': None, 'deletion_time': '', 'destroyed': False, 'version': 1}}, 'wrap_info': None, 'warnings': None, 'auth': None}",
        "Value of key 'FOO' in the secret: bar"
    ]
}

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

github-actions · 2022-08-14T06:57:27Z

Docs Build 📝

Thank you for contribution!✨

This PR has been merged and the docs are now incorporated into main:
https://ansible-collections.github.io/community.hashi_vault/branch/main

briantist · 2022-08-15T22:10:48Z

Hi @jchenship ! Thanks for opening this MR!

Currently I'm not sure how to run unit tests locally, but I verify this on my azure vm with

Check out the Contributor guide for how to check out the collection to the proper directory structure.

From there, you can run:

ansible-test units --docker default

If you have trouble running the container, you can try to run the same with ansible-core>=2.11,<2.12 because the newer containers sometimes have issues running locally.

As an alternative, you can try:

ansible-test units --venv --requirements

I've kicked off a CI run, and here's the result of the unit tests:
https://github.com/ansible-collections/community.hashi_vault/runs/7846893677?check_suite_focus=true

It seems at a minimum you'll need to add your requirements to tests/unit/requirements.txt so that they get installed properly when the tests are run.

Unfortunately since you're a new contributor, I have to click the button to allow CI to run for every change you push, so it will be much faster for you to get the tests running locally.

I haven't looked very deeply at the changes yet, but on the surface they look good.

In addition to unit tests, you'll also need integration tests. Since we don't have access to Azure in the CI, we can do this via MMock to mock the responses from Vault. This is how we test AWS and LDAP right now, so you can take a look at how that works.

This is described a little bit in this comment #78 (comment) and also in the Contributor guide.

Since you've already got a fixture in place for the units, adding the appropriate mocks in MMock should be relatively easy as well.

Minor nit: update the copyright name and date in the files you're adding.

The sanity tests in the collection have been failing in the collection recently with these errors:
https://github.com/ansible-collections/community.hashi_vault/runs/7839632355?check_suite_focus=true#step:8:124

I have not had the time to investigate and fix those; they will fail for you too, you may ignore those for now, however do not ignore any other sanity test failures you see.

Sanity tests can be run locally too:

ansible-test sanity --docker default

It's ok if you can't get the hang of all the pieces, I can help get that stuff over the line, but it may take a little while. You've caught me at a time where I have a lot going on and will be out of town until next week, so I apologize for slow responses.

lapwingcloud · 2022-08-16T05:01:05Z

@briantist thanks a lot for the tips and feedback, I'm quite busy at work in the weekdays as well, I will continue to work on this during the weekend because

lapwingcloud · 2022-08-20T13:21:19Z

@briantist I fixed unit tests and added integration tests, they passed in CI. So requesting a review, thank you!

The only thing is there're failed sanity tests but don't seem to be related to by changes so I'm not sure what to do with it.

briantist · 2022-08-20T15:51:39Z

@jchenship thank you! This is looking great, I think nearly complete. I just landed #297 which will fix the sanity tests.

Please rebase so we can get the full CI run and see coverage.

We'll still need at least a few minor changes, but let's push up the rebase first to get the CI complete.

Small changes needed after that:

changelog fragment
fix copyright year and name in your new files
add version_added: to the new options

I would like to release the collection version 3.2.0 today or tomorrow, because it contains a bugfix that has already affected some users (#294). If we can finish this up quickly, we can include this in 3.2.0 also.

If not, I will look to release that first, and this can go into 3.3.0, which is not a problem at all, I have no issue with making a new release very quickly after the other one.

codecov · 2022-08-21T00:35:48Z

Codecov Report

Merging #293 (7bb2964) into main (06daf8c) will increase coverage by 0.05%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##             main     #293      +/-   ##
==========================================
+ Coverage   98.46%   98.52%   +0.05%     
==========================================
  Files          71       73       +2     
  Lines        3464     3602     +138     
  Branches      301      321      +20     
==========================================
+ Hits         3411     3549     +138     
  Misses         44       44              
  Partials        9        9

Flag	Coverage Δ
env_docker-default	`98.52% <100.00%> (+0.05%)`	⬆️
integration	`80.93% <55.00%> (-0.77%)`	⬇️
sanity	`38.77% <27.50%> (-0.34%)`	⬇️
target_ansible-doc	`100.00% <ø> (ø)`
target_auth_approle	`89.47% <ø> (ø)`
target_auth_aws_iam	`50.00% <ø> (ø)`
target_auth_azure	`53.84% <53.84%> (?)`
target_auth_cert	`86.36% <ø> (ø)`
target_auth_jwt	`91.30% <ø> (ø)`
target_auth_ldap	`89.47% <ø> (ø)`
target_auth_none	`100.00% <ø> (ø)`
target_auth_token	`73.07% <ø> (ø)`
target_auth_userpass	`85.71% <ø> (ø)`
target_connection_options	`74.76% <ø> (ø)`
target_controller	`82.51% <30.00%> (-1.16%)`	⬇️
target_filter_vault_login_token	`77.77% <ø> (ø)`
target_import	`38.77% <27.50%> (-0.34%)`	⬇️
target_lookup_hashi_vault	`81.33% <ø> (ø)`
target_lookup_vault_ansible_settings	`55.87% <30.00%> (-1.35%)`	⬇️
target_lookup_vault_kv1_get	`91.30% <ø> (ø)`
target_lookup_vault_kv2_get	`91.66% <ø> (ø)`
target_lookup_vault_login	`88.57% <ø> (ø)`
target_lookup_vault_read	`90.00% <ø> (ø)`
target_lookup_vault_token_create	`78.18% <ø> (ø)`
target_lookup_vault_write	`57.57% <30.00%> (-1.69%)`	⬇️
target_module_utils	`97.02% <100.00%> (+0.30%)`	⬆️
target_module_vault_kv1_get	`87.50% <ø> (ø)`
target_module_vault_kv2_get	`87.23% <ø> (ø)`
target_module_vault_login	`83.72% <ø> (ø)`
target_module_vault_pki_generate_certificate	`78.72% <ø> (ø)`
target_module_vault_read	`85.71% <ø> (ø)`
target_module_vault_token_create	`90.00% <ø> (ø)`
target_module_vault_write	`56.46% <30.00%> (-1.86%)`	⬇️
target_modules	`77.43% <30.00%> (-1.32%)`	⬇️
units	`95.78% <100.00%> (+0.16%)`	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files	Coverage Δ
plugins/doc_fragments/auth.py	`100.00% <ø> (ø)`
plugins/module_utils/_auth_method_azure.py	`100.00% <100.00%> (ø)`
plugins/module_utils/_authenticator.py	`100.00% <100.00%> (ø)`
...ins/module_utils/authentication/test_auth_azure.py	`100.00% <100.00%> (ø)`

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

lapwingcloud · 2022-08-21T05:17:30Z

@briantist Hey I have fixed all the things you mentioned, please have another review, and release it in 3.2.0 if it's ok, thank you! Otherwise I will continue to fix it and hopefully we can release it in 3.3.0

briantist

I'm making some suggestions here that are mostly about wording/text changes.
I will accept commit these changes right away, but if you disagree with any of them we can discuss and change them to something else.

I have some other code changes but they are hard to do via review comments, so I will check out your PR and push some changes directly to your branch.

I'll let you know when ready so you can pull down the commits, and look over the changes I've made.

Thanks again, this is really great!

plugins/doc_fragments/auth.py

plugins/module_utils/_auth_method_azure.py

forgot to commit this one before

briantist

@jchenship I think this is ready to go, please have a look at the commits I added and let me know if you think any other changes are needed, or if you have questions.

I tried to make smaller commits so it is easier to see what changes I made, I hope it's helpful.

lapwingcloud · 2022-08-21T23:24:04Z

@briantist Yeah that looks good to me, thanks for your help to refine the PR, look forward for it to be released!

briantist · 2022-08-22T00:58:09Z

@jchenship thank you so much for this contribution! I hope you'll consider more in the future :)

This is now released as part of version 3.2.0! 🎉

lapwingcloud mentioned this pull request Aug 14, 2022

Feature Request: Add support for azure auth method #78

Closed

briantist self-assigned this Aug 20, 2022

briantist added the enhancement New feature or request label Aug 20, 2022

lapwingcloud added 4 commits August 21, 2022 09:47

Add support for azure auth method

b4f3f9e

fix unit tests

ba691a1

add integration tests

c29bb29

add changelog fragment, fix copyright, add version_added

68fb225

lapwingcloud force-pushed the jchen-auth-azure branch from 5860d90 to 68fb225 Compare August 21, 2022 00:02

fix sanity tests

9d80629

briantist reviewed Aug 21, 2022

View reviewed changes

briantist added 10 commits August 21, 2022 11:13

Apply suggestions from code review

440b4e3

add env, ini, and vars plugins entries for azure auth options

96fb078

make role_id explicitly required

f4e60ab

validate method cleanup, unit test additions

3f0eaaf

changelog nits

53d87d7

docs and requirements

2cb302c

add more copyright/license headers

103857e

fix sanity line length

6a0d1c1

Update plugins/doc_fragments/auth.py

c20b449

forgot to commit this one before

remove unused fixture

7bb2964

briantist approved these changes Aug 21, 2022

View reviewed changes

briantist added this to the v3.2.0 milestone Aug 21, 2022

briantist merged commit 2939aad into ansible-collections:main Aug 22, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add support for azure auth method #293

Add support for azure auth method #293

lapwingcloud commented Aug 14, 2022 •

edited

Loading

github-actions bot commented Aug 14, 2022 •

edited

Loading

briantist commented Aug 15, 2022

lapwingcloud commented Aug 16, 2022

lapwingcloud commented Aug 20, 2022

briantist commented Aug 20, 2022

codecov bot commented Aug 21, 2022 •

edited

Loading

lapwingcloud commented Aug 21, 2022

briantist left a comment

briantist left a comment •

edited

Loading

lapwingcloud commented Aug 21, 2022

briantist commented Aug 22, 2022

Add support for azure auth method #293

Add support for azure auth method #293

Conversation

lapwingcloud commented Aug 14, 2022 • edited Loading

SUMMARY

ISSUE TYPE

COMPONENT NAME

ADDITIONAL INFORMATION

jwt

service principal

managed identity

github-actions bot commented Aug 14, 2022 • edited Loading

Docs Build 📝

briantist commented Aug 15, 2022

lapwingcloud commented Aug 16, 2022

lapwingcloud commented Aug 20, 2022

briantist commented Aug 20, 2022

codecov bot commented Aug 21, 2022 • edited Loading

Codecov Report

lapwingcloud commented Aug 21, 2022

briantist left a comment

Choose a reason for hiding this comment

briantist left a comment • edited Loading

Choose a reason for hiding this comment

lapwingcloud commented Aug 21, 2022

briantist commented Aug 22, 2022

lapwingcloud commented Aug 14, 2022 •

edited

Loading

github-actions bot commented Aug 14, 2022 •

edited

Loading

codecov bot commented Aug 21, 2022 •

edited

Loading

briantist left a comment •

edited

Loading