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

Conditionally call non MSI auth when interacting with keyvault #770

Merged
merged 1 commit into from
Feb 28, 2022

Conversation

Fred-sun
Copy link
Collaborator

SUMMARY

Conditionally call non MSI auth when interacting with keyvault. fixes #134
The related PR #356

ISSUE TYPE
  • Bugfix Pull Request
COMPONENT NAME

azure_rm_keyvaultkey.py
azure_rm_keyvaultkey_info.py
azure_rm_keyvaultsecret.py
azure_rm_keyvaultsecret_info.py

ADDITIONAL INFORMATION

@Fred-sun Fred-sun added medium_priority Medium priority ready_for_review The PR has been modified and can be reviewed and merged labels Feb 27, 2022
@xuzhang3
Copy link
Collaborator

LGTM

@xuzhang3 xuzhang3 merged commit 199c7ee into ansible-collections:dev Feb 28, 2022
Fred-sun added a commit to Fred-sun/ansible_collections_azure that referenced this pull request Mar 8, 2022
* azure_rm_sqldatabase: parse datetime module arguments (ansible-collections#623)

* rm_sqldatabase: parse datetime arguments

* Remove unused sanity test exception on rm_sqldatabase module schema

* Remove unused sanity test exception on rm_sqldatabase module schema bis

* sqldatabase: import dateutil in try/except

* Add dateutil install to test suite

* sqldatabase_info: Add earliest_restore_date value to returned facts

* sqldatabase: add point in time restore test

* Conditionally call non MSI auth when interacting with keyvault (ansible-collections#770)

* Added the VM status detection mechanism (ansible-collections#772)

* Set the parameter to a random number

* Update storage account name

Update azure_rm_virtualmachine vars

add new change

add new change 02

add new change 03

add new change 05

add new change 06

add new change 08

add new change09

update new

Update new 02

Improve code logic

* fix a typo error. related to ansible-collections#757 (ansible-collections#769)

* fix a typo error. related to ansible-collections#757

* remove unused line

Co-authored-by: Daniele Marcocci <[email protected]>

* Update test region (ansible-collections#776)

Co-authored-by: Max <[email protected]>
Co-authored-by: Daniele Marcocci <[email protected]>
Co-authored-by: Daniele Marcocci <[email protected]>
xuzhang3 pushed a commit that referenced this pull request Mar 8, 2022
* Ugrade azure-mgmt-compute SDK to track2

* fix small

* Modify version from v2021-07-01 to v2020-04-01, no disk encryptions operation

* Update small

* fix azure_rm_diskencryption test fail

* fix azure_rm_diskencryption test fail02

* fix sanity error

* fix azure_rm_diskcryptionset test fail

* fix azure_rm_virtualmachinescalesetinstance_info bug

* fix azure_rm_virtualmachinescalesetinstance_info bug 02

* fix azure_rm_virtualmachien*extension test fail

* Update azure_rm_virtualmachinescalesetinstance func paramter to vm_instance_i_ds

* fix azure_rm_virtualmachinescalesetinstance test fail

* fix sanity test fail

* change exception type

* fix azure_rm_hostgroup module

* Update the code that throws the exception

* Merge dev to local branch (#10)

* azure_rm_sqldatabase: parse datetime module arguments (#623)

* rm_sqldatabase: parse datetime arguments

* Remove unused sanity test exception on rm_sqldatabase module schema

* Remove unused sanity test exception on rm_sqldatabase module schema bis

* sqldatabase: import dateutil in try/except

* Add dateutil install to test suite

* sqldatabase_info: Add earliest_restore_date value to returned facts

* sqldatabase: add point in time restore test

* Conditionally call non MSI auth when interacting with keyvault (#770)

* Added the VM status detection mechanism (#772)

* Set the parameter to a random number

* Update storage account name

Update azure_rm_virtualmachine vars

add new change

add new change 02

add new change 03

add new change 05

add new change 06

add new change 08

add new change09

update new

Update new 02

Improve code logic

* fix a typo error. related to #757 (#769)

* fix a typo error. related to #757

* remove unused line

Co-authored-by: Daniele Marcocci <[email protected]>

* Update test region (#776)

Co-authored-by: Max <[email protected]>
Co-authored-by: Daniele Marcocci <[email protected]>
Co-authored-by: Daniele Marcocci <[email protected]>

* Revert "Merge dev to local branch (#10)" (#11)

This reverts commit 1dce8f3.

Co-authored-by: Max <[email protected]>
Co-authored-by: Daniele Marcocci <[email protected]>
Co-authored-by: Daniele Marcocci <[email protected]>
@Fred-sun Fred-sun deleted the fix_keyvault_no_msi branch March 8, 2022 07:07
@uda
Copy link

uda commented Apr 12, 2022

Umm... are you sure this PR does what the title says?

Cause what I see is it conditionally calls MSI auth AGAIN, and the KeyError: 'client_id' exception still happens

This does not fix #134 in any way since the CLI credentials are still ignored

@Fred-sun
Copy link
Collaborator Author

Umm... are you sure this PR does what the title says?

Cause what I see is it conditionally calls MSI auth AGAIN, and the KeyError: 'client_id' exception still happens

This does not fix #134 in any way since the CLI credentials are still ignored

I'm sorry, I have submit new pull request to fixes this bug. Would you give a try? #823

@uda
Copy link

uda commented Apr 18, 2022

I'm sorry, I have submit new pull request to fixes this bug. Would you give a try? #823

Yes, that seems to address the issue, will give it a run in the next few days, Thanks!

@uda
Copy link

uda commented May 9, 2022

Thanks @Fred-sun this change (#823) seems to have fixed the usage of the CLI, w/ and w/o specifically setting ANSIBLE_AZURE_AUTH_SOURCE to cli

When you have a chance please tag it so I can introduce the usage into our pipeline, I have already some deployment simplifications I can't wait to introduce 😃

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
medium_priority Medium priority 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.

azure cli not supported for keyvault?
3 participants