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

Allow credential file to not set subscription_id and use the param sp… #1380

Open
wants to merge 1 commit into
base: dev
Choose a base branch
from

Conversation

is-consulting
Copy link

SUMMARY

This Pull request allows a user to not set the subscription_id value when using a credential file. This enables use of the "subscription_id" param specified in the dynamic inventory if unset.

ISSUE TYPE
  • Feature Pull Request
COMPONENT NAME

Authentication / Credentials

ADDITIONAL INFORMATION
plugin: azure.azcollection.azure_rm
# When using a credential File, this parameter was ignored and is now used:
subscription_id: 00000000-0000-0000-0000-0000  
auth_source: auto
include_vm_resource_groups: ['*']
include_vmss_resource_groups: ['*']
default_host_filters: []
conditional_groups:
  linux: "'linux' in os_profile.system"
  windows: "'windows' in os_profile.system"

…ecified in the dynamic inventory if unset instead
@Fred-sun Fred-sun added ready_for_review The PR has been modified and can be reviewed and merged high_priority High priority medium_priority Medium priority inventory plugin/Inventory/azure_rm.py related issues labels Dec 18, 2023
@Fred-sun Fred-sun removed ready_for_review The PR has been modified and can be reviewed and merged high_priority High priority labels Dec 18, 2023
@Fred-sun Fred-sun added the ready_for_review The PR has been modified and can be reviewed and merged label Jan 3, 2024
@@ -1529,8 +1529,10 @@ def _get_profile(self, profile="default"):
except Exception:
pass

if credentials.get('subscription_id'):
return credentials
if credentials.get('subscription_id') is None:
Copy link
Collaborator

Choose a reason for hiding this comment

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

The profile will be None if credentials.get('subscription_id') not None. credentials.get('subscription_id') and subscription_id which one is the first priority?

Copy link
Author

Choose a reason for hiding this comment

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

In order to not change the behavior I give credentials.get('subscription_id') the first priority

Copy link
Collaborator

Choose a reason for hiding this comment

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

credentials.get('subscription_id') not None not handled, profile will returns as None

Copy link
Collaborator

@xuzhang3 xuzhang3 Mar 22, 2024

Choose a reason for hiding this comment

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

@is-consulting ping credentials.get('subscription_id') not None not handled.

Suggested change
if credentials.get('subscription_id') is None:
if credentials.get('subscription_id') is None:
...
else:
return credentials

@Fred-sun
Copy link
Collaborator

@is-consulting kindly ping!

@xuzhang3 xuzhang3 removed the ready_for_review The PR has been modified and can be reviewed and merged label Jul 1, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
inventory plugin/Inventory/azure_rm.py related issues medium_priority Medium priority
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants