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

Keyvault Lookup Plugin Added #109

Merged
merged 37 commits into from
Apr 14, 2022

Conversation

taasest8
Copy link
Contributor

SUMMARY

Moving azure_keyvault_secret lookup plugin from old role repo to this collection.
Using code suggested from taarpa6 that also does a check on the http status code before further processing. This fixes a (time-out) issue when not using an Azure managed identity and not running from an Azure VM.

Old PR (closed but never merged): Azure/azure_preview_modules#349

ISSUE TYPE
  • New Module Pull Request
COMPONENT NAME

azure_keyvault_secret lookup plugin

ADDITIONAL INFORMATION

To use a lookup plugin the full namespace needs to be referenced.

- name: lookup secrets
          set_fact:
            secret_var: "{{ lookup('azure.azcollection.azure_keyvault_secret','secret-name', vault_url=az_vault_uri, client_id=az_vault_client_id, secret=az_vault_secret, tenant_id=az_vault_tenant) }}"

@Fred-sun
Copy link
Collaborator

@haiyuazhang Could you please help take a review this issue? Thank you very much!

@haiyuazhang haiyuazhang force-pushed the dev branch 2 times, most recently from 57c6057 to 3e1c870 Compare July 3, 2020 18:05
@Fred-sun Fred-sun added medium_priority Medium priority plugin new plugin and removed medium_priority Medium priority labels Aug 24, 2020
@uda
Copy link

uda commented Sep 3, 2020

Hi @taasest8, this is great and solves many issues in automation.

Can this be modified to leverage generic Azure CLI auth as well? not just MSI or in-code SP credentials.

@weemel
Copy link

weemel commented Oct 15, 2020

Hello, I guess this is no longer valid as the azure module is now deprecated - are there any plans to update the plugin with azure-identity and merge it? I'd be happy to help as this would be useful for templating :)

@Ompragash
Copy link
Member

Hello, I guess this is no longer valid as the azure module is now deprecated - are there any plans to update the plugin with azure-identity and merge it? I'd be happy to help as this would be useful for templating :)

@Fred-sun Could you confirm?

@Fred-sun
Copy link
Collaborator

Fred-sun commented Apr 6, 2021

@Ompragash Yes, It will be useful. I will review with developer and push for merge! thank you very much!

@taasest8 taasest8 requested a review from Fred-sun April 4, 2022 09:35
@taasest8
Copy link
Contributor Author

taasest8 commented Apr 4, 2022

Hello @Fred-sun, did the minor changes, what rights do you need on the pr ? afaik you should have enough rights as you are the owner of the base repository.

@guillaumewatteeux
Copy link
Contributor

@taasest8 could you add an option to bypass MSI token ?

Copy link
Collaborator

@Fred-sun Fred-sun left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

trailing whitespace

plugins/lookup/azure_keyvault_secret.py Outdated Show resolved Hide resolved
plugins/lookup/azure_keyvault_secret.py Outdated Show resolved Hide resolved
taasest8 and others added 2 commits April 6, 2022 12:50
Co-authored-by: Fred-sun <37327967+Fred-sun@users.noreply.github.com>
Co-authored-by: Fred-sun <37327967+Fred-sun@users.noreply.github.com>
Copy link
Collaborator

@Fred-sun Fred-sun left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Complete all test exceptions to avoid security test failures due to requests not being imported!

plugins/lookup/azure_keyvault_secret.py Outdated Show resolved Hide resolved
plugins/lookup/azure_keyvault_secret.py Outdated Show resolved Hide resolved
taasest8 and others added 2 commits April 7, 2022 09:28
Co-authored-by: Fred-sun <37327967+Fred-sun@users.noreply.github.com>
Co-authored-by: Fred-sun <37327967+Fred-sun@users.noreply.github.com>
plugins/lookup/azure_keyvault_secret.py Outdated Show resolved Hide resolved
plugins/lookup/azure_keyvault_secret.py Outdated Show resolved Hide resolved
@Fred-sun
Copy link
Collaborator

Fred-sun commented Apr 7, 2022

delete duplicate line!

taasest8 and others added 4 commits April 7, 2022 09:35
Co-authored-by: Fred-sun <37327967+Fred-sun@users.noreply.github.com>
Co-authored-by: Fred-sun <37327967+Fred-sun@users.noreply.github.com>
Co-authored-by: Fred-sun <37327967+Fred-sun@users.noreply.github.com>
@Fred-sun
Copy link
Collaborator

Fred-sun commented Apr 7, 2022

@taasest8 Thank you for your update. I will complete the inspection and promote the merger as soon as possible. Thank you very much!

@Fred-sun
Copy link
Collaborator

Fred-sun commented Apr 7, 2022

@taasest8 One last change!

Co-authored-by: Fred-sun <37327967+Fred-sun@users.noreply.github.com>
plugins/lookup/azure_keyvault_secret.py Outdated Show resolved Hide resolved
plugins/lookup/azure_keyvault_secret.py Outdated Show resolved Hide resolved
taasest8 and others added 2 commits April 7, 2022 21:10
Co-authored-by: Fred-sun <37327967+Fred-sun@users.noreply.github.com>
Co-authored-by: Fred-sun <37327967+Fred-sun@users.noreply.github.com>
@Fred-sun
Copy link
Collaborator

Fred-sun commented Apr 8, 2022

ready_for_review

@Fred-sun Fred-sun added the ready_for_review The PR has been modified and can be reviewed and merged label Apr 8, 2022
@xuzhang3
Copy link
Collaborator

LGTM

@xuzhang3 xuzhang3 merged commit 7f8e745 into ansible-collections:dev Apr 14, 2022
@taasest8 taasest8 deleted the feature/keyvault-lookup branch April 14, 2022 10:17
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 ready_for_review The PR has been modified and can be reviewed and merged
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants