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

Fixes required for use with multiple Cloud Environments #854

Merged
merged 2 commits into from
Jul 18, 2022

Conversation

dougalII
Copy link
Contributor

SUMMARY

Fixes credential_scopes for track2 authentication when connecting to non "Azure Public" cloud environments
Fixes for AKV to correct resource url for non "Azure Public" cloud environments

fixes: #331 #836 #702

ISSUE TYPE
  • Bugfix Pull Request
COMPONENT NAME
  • azure_rm_common
  • azure_rm_keyvaultkey
  • azure_rm_keyvaultsecret
  • azure_rm_keyvaultkey_info
  • azure_rm_keyvaultsecret_info
ADDITIONAL INFORMATION

These changes allow you to use other cloud platforms in azure.

The track2 fixes are related to an azure-sdk issue i created: here it uses poorly documented keyword credential_scopes to tell the SDK which environment the token request is for.

The AKV Changes are to remove the hard coded resource="https://vault.azure.net" as each cloud environment has its own resource url for AKV.

… is a requirement when used with alternative azure cloud environments.

fix(akv_url): Fixed AKV Resource URL, this is required as different azure cloud environments.
@Fred-sun Fred-sun added medium_priority Medium priority work in In trying to solve, or in working with contributors labels May 23, 2022
plugins/module_utils/azure_rm_common.py Outdated Show resolved Hide resolved
plugins/modules/azure_rm_keyvaultkey.py Outdated Show resolved Hide resolved
plugins/modules/azure_rm_keyvaultkey_info.py Outdated Show resolved Hide resolved
fix(line_length): split track2 client_kwargs to 2 lines to keep below the 160 character limit
@dougalII dougalII requested a review from Fred-sun June 16, 2022 15:09
@rmawhinnie
Copy link

Hoping to get this in soon, as this is useful to my team as well

@perrymckenzie
Copy link

Agree with @rmawhinnie - our team needs this soon too. Looking forward to this merge!

@danfinn
Copy link
Contributor

danfinn commented Jun 17, 2022

Hoping to get this merged soon as well.

@ramdaspotale
Copy link

it will be very much helpful if get merged sooner.

@@ -891,12 +891,13 @@ def get_mgmt_svc_client(self, client_type, base_url=None, api_version=None, supp
# Some management clients do not take a subscription ID as parameters.
if suppress_subscription_id:
if is_track2:
client_kwargs = dict(credential=self.azure_auth.azure_credential_track2, base_url=base_url)
client_kwargs = dict(credential=self.azure_auth.azure_credential_track2, base_url=base_url, credential_scopes=[base_url + ".default"])
Copy link
Collaborator

Choose a reason for hiding this comment

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

I'm sorry for replying to you so late. Could you please tell me the function of this parameter(credential_scopes) ? Because I didn't apply this parameter in the Track2 SDK. Thank you very much!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hey @Fred-sun,
It essentially sets the management endpoints the generated auth token is for, if this is not set, it defaults to the AzurePublicCloud url, and you get the bellow error when talking to any of the other azure clouds.
Message: The access token has been obtained for wrong audience or resource 'https://management.azure.com'. It should exactly match with one of the allowed audiences 'https://management.core.usgovcloudapi.net/','https://management.core.usgovcloudapi.net','https://management.usgovcloudapi.net/','https://management.usgovcloudapi.net'.

this error was discussed here:
Azure/azure-sdk-for-python#24440

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@Fred-sun, do you need any more information on this? We are finding we are jumping threw a lot of hoops to get these fixes inside our AWX, EE and Operator-SDK environments.

@dougalII dougalII requested a review from Fred-sun June 28, 2022 14:40
@Fred-sun Fred-sun added ready_for_review The PR has been modified and can be reviewed and merged and removed work in In trying to solve, or in working with contributors labels Jul 6, 2022
@Fred-sun
Copy link
Collaborator

Fred-sun commented Jul 6, 2022

@dougalII I will move forward with the merger as soon as possible. Thank you very much!

@xuzhang3
Copy link
Collaborator

@dougalII LGTM

@xuzhang3 xuzhang3 merged commit 9ad8a0c into ansible-collections:dev Jul 18, 2022
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 Government azure_rm_keyvaultsecret_info not able to fetch secrets
7 participants