-
Notifications
You must be signed in to change notification settings - Fork 336
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
azure_rm_adapplication_info
- Feature azure rm adapplication info diff
#1560
Changes from 9 commits
6603c4b
2b452a3
46e95b1
bf011af
774e0fb
9143df6
944b9b4
886021f
5bbfe53
4a13a10
f62a2fc
f06e9db
dca54f2
ac79aad
b41c5e4
870d950
5eb8ee7
6b34872
57a6cf2
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change | ||||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
@@ -36,6 +36,11 @@ | |||||||||||||||||||
description: | ||||||||||||||||||||
- The applications' Name. | ||||||||||||||||||||
type: str | ||||||||||||||||||||
app_diff: | ||||||||||||||||||||
description: | ||||||||||||||||||||
- The application Name or the app id. | ||||||||||||||||||||
type: list | ||||||||||||||||||||
elements: dict | ||||||||||||||||||||
|
||||||||||||||||||||
extends_documentation_fragment: | ||||||||||||||||||||
- azure.azcollection.azure | ||||||||||||||||||||
|
@@ -63,6 +68,12 @@ | |||||||||||||||||||
- name: get ad app info ---- by display name | ||||||||||||||||||||
azure_rm_adapplication_info: | ||||||||||||||||||||
app_display_name: "{{ display_name }}" | ||||||||||||||||||||
|
||||||||||||||||||||
- name: get ad app diff ---- by display name | ||||||||||||||||||||
azure_rm_adapplication_info: | ||||||||||||||||||||
app_diff: | ||||||||||||||||||||
- app_display_name: "{{ display_name }}" | ||||||||||||||||||||
- app_id: "{{ app_id }}" | ||||||||||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Wrong indentation: expected 6 but found 8 |
||||||||||||||||||||
''' | ||||||||||||||||||||
|
||||||||||||||||||||
RETURN = ''' | ||||||||||||||||||||
|
@@ -129,6 +140,75 @@ | |||||||||||||||||||
returned: always | ||||||||||||||||||||
type: list | ||||||||||||||||||||
sample: [] | ||||||||||||||||||||
app_diff: | ||||||||||||||||||||
description: | ||||||||||||||||||||
- The info of the ad application. | ||||||||||||||||||||
type: complex | ||||||||||||||||||||
returned: aways | ||||||||||||||||||||
contains: | ||||||||||||||||||||
app_display_name: | ||||||||||||||||||||
description: | ||||||||||||||||||||
- Object's display name or its prefix. | ||||||||||||||||||||
type: str | ||||||||||||||||||||
returned: always | ||||||||||||||||||||
sample: app | ||||||||||||||||||||
app_id: | ||||||||||||||||||||
description: | ||||||||||||||||||||
- The application ID. | ||||||||||||||||||||
returned: always | ||||||||||||||||||||
type: str | ||||||||||||||||||||
sample: xxxxxxxx-xxxx-xxxx-xxxx-xxxxxxxxxxxx | ||||||||||||||||||||
identifier_uris: | ||||||||||||||||||||
description: | ||||||||||||||||||||
- The identifiers_uri list of app. | ||||||||||||||||||||
type: list | ||||||||||||||||||||
returned: always | ||||||||||||||||||||
sample: ["http://ansible-atodorov"] | ||||||||||||||||||||
object_id: | ||||||||||||||||||||
description: | ||||||||||||||||||||
- It's application's object ID. | ||||||||||||||||||||
returned: always | ||||||||||||||||||||
type: str | ||||||||||||||||||||
sample: xxxxxxxx-xxxx-xxxx-xxxx-xxxxxxxxxxxx | ||||||||||||||||||||
sign_in_audience: | ||||||||||||||||||||
description: | ||||||||||||||||||||
- The application can be used from any Azure AD tenants | ||||||||||||||||||||
type: str | ||||||||||||||||||||
returned: always | ||||||||||||||||||||
sample: AzureADandPersonalMicrosoftAccount | ||||||||||||||||||||
available_to_other_tenants: | ||||||||||||||||||||
description: | ||||||||||||||||||||
- The application can be used from any Azure AD tenants | ||||||||||||||||||||
type: str | ||||||||||||||||||||
returned: always | ||||||||||||||||||||
sample: AzureADandPersonalMicrosoftAccount | ||||||||||||||||||||
public_client_reply_urls: | ||||||||||||||||||||
description: | ||||||||||||||||||||
- The public client redirect urls. | ||||||||||||||||||||
- Space-separated URIs to which Azure AD will redirect in response to an OAuth 2.0 request. | ||||||||||||||||||||
returned: always | ||||||||||||||||||||
type: list | ||||||||||||||||||||
sample: [] | ||||||||||||||||||||
web_reply_urls: | ||||||||||||||||||||
description: | ||||||||||||||||||||
- The web redirect urls. | ||||||||||||||||||||
- Space-separated URIs to which Azure AD will redirect in response to an OAuth 2.0 request. | ||||||||||||||||||||
returned: always | ||||||||||||||||||||
type: list | ||||||||||||||||||||
sample: [] | ||||||||||||||||||||
spa_reply_urls: | ||||||||||||||||||||
description: | ||||||||||||||||||||
- The spa redirect urls. | ||||||||||||||||||||
- Space-separated URIs to which Azure AD will redirect in response to an OAuth 2.0 request. | ||||||||||||||||||||
returned: always | ||||||||||||||||||||
type: list | ||||||||||||||||||||
sample: [] | ||||||||||||||||||||
state: | ||||||||||||||||||||
description: | ||||||||||||||||||||
- absent --> The app isn't in the app_diff. | ||||||||||||||||||||
returned: always | ||||||||||||||||||||
type: str | ||||||||||||||||||||
sample: absent | ||||||||||||||||||||
''' | ||||||||||||||||||||
|
||||||||||||||||||||
from ansible_collections.azure.azcollection.plugins.module_utils.azure_rm_common_ext import AzureRMModuleBase | ||||||||||||||||||||
|
@@ -149,7 +229,8 @@ def __init__(self): | |||||||||||||||||||
app_id=dict(type='str'), | ||||||||||||||||||||
object_id=dict(type='str'), | ||||||||||||||||||||
identifier_uri=dict(type='str'), | ||||||||||||||||||||
app_display_name=dict(type='str') | ||||||||||||||||||||
app_display_name=dict(type='str'), | ||||||||||||||||||||
app_diff=dict(type='list', elements='dict') | ||||||||||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. If defined this way, it will be easier to understand!
Suggested change
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. If I have the configuration of my adapps in a YAML or JSON file, I don't want to edit it additionally. I want to provide it directly to the plugin, not just the "app_display_name" or the "app_id". However, if I provide this as an option, I have to filter the YAML or JSON for these two options, which means another Ansible task in the playbook. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. But your code only handles these two fields and no other fields? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. My code need one of these fields, the other fields will be ignored. In the future with this way, it's possible to check the diffrence of any field and to update them. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes, that is correct. The code only handles these two fields. However, I can provide a YAML or JSON that contains the entire content of multiple ADAPPs in a list without it being blocked. This way, I can directly compare my stored configuration of the ADAPPs without having to parse it again, create a list, and pass the values into a variable |
||||||||||||||||||||
) | ||||||||||||||||||||
self.app_id = None | ||||||||||||||||||||
self.app_display_name = None | ||||||||||||||||||||
|
@@ -167,7 +248,6 @@ def exec_module(self, **kwargs): | |||||||||||||||||||
setattr(self, key, kwargs[key]) | ||||||||||||||||||||
|
||||||||||||||||||||
applications = [] | ||||||||||||||||||||
|
||||||||||||||||||||
try: | ||||||||||||||||||||
self._client = self.get_msgraph_client() | ||||||||||||||||||||
if self.object_id: | ||||||||||||||||||||
|
@@ -183,12 +263,27 @@ def exec_module(self, **kwargs): | |||||||||||||||||||
apps = asyncio.get_event_loop().run_until_complete(self.get_applications(sub_filters)) | ||||||||||||||||||||
applications = list(apps) | ||||||||||||||||||||
self.results['applications'] = [self.to_dict(app) for app in applications] | ||||||||||||||||||||
current_app = [self.to_dict(app) for app in applications] | ||||||||||||||||||||
except APIError as e: | ||||||||||||||||||||
if e.response_status_code != 404: | ||||||||||||||||||||
self.fail("failed to get application info {0}".format(str(e))) | ||||||||||||||||||||
except Exception as ge: | ||||||||||||||||||||
self.fail("failed to get application info {0}".format(str(ge))) | ||||||||||||||||||||
|
||||||||||||||||||||
except Exception as e: | ||||||||||||||||||||
if "Name or service not known" in str(e): | ||||||||||||||||||||
self.fail("DNS resolution error occurred.") | ||||||||||||||||||||
else: | ||||||||||||||||||||
self.fail("An unexpected error occurred: {0}".format(str(e))) | ||||||||||||||||||||
if self.app_diff: | ||||||||||||||||||||
temp_app = [] | ||||||||||||||||||||
for app in current_app: | ||||||||||||||||||||
found = False | ||||||||||||||||||||
for diff in self.app_diff: | ||||||||||||||||||||
if app.get("app_id") == diff.get("app_id") or app.get("app_display_name") == diff.get("app_display_name"): | ||||||||||||||||||||
found = True | ||||||||||||||||||||
break | ||||||||||||||||||||
if not found: | ||||||||||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Is this a definition error? Add to 'app_diff' if it should be True! Thank you!
Suggested change
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. No, that is correct. It compares the provided list with the production Azure data, and if something is not present in the production environment, the status "absent" is added. This way, you can immediately reuse the return to delete the adapps that do not belong in the production environment using "plugins/modules/azure_rm_adapplication.py". There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Understood, please fix the following error, and make a detailed definition of the parameter definition. Thank you! There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I have adjusted the documentation and re-added the NOT. |
||||||||||||||||||||
app['state'] = 'absent' | ||||||||||||||||||||
temp_app.append(app) | ||||||||||||||||||||
self.results['app_diff'] = temp_app | ||||||||||||||||||||
return self.results | ||||||||||||||||||||
|
||||||||||||||||||||
def to_dict(self, object): | ||||||||||||||||||||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,8 +1,12 @@ | ||
- name: Set variables | ||
- name: Set display name variable | ||
ansible.builtin.set_fact: | ||
display_name: "app{{ resource_group | hash('sha1') | truncate(20, True, '') }}" | ||
display_name: "test_app_{{ 999999999999999999994 | random | to_uuid }}" | ||
run_once: true | ||
|
||
- name: Get full list from ad app info | ||
azure_rm_adapplication_info: | ||
register: app_info_output | ||
|
||
- name: Create application | ||
azure_rm_adapplication: | ||
display_name: "{{ display_name }}" | ||
|
@@ -12,31 +16,24 @@ | |
ansible.builtin.assert: | ||
that: create_output.changed | ||
|
||
- name: Create application again idempotent test | ||
- name: Create application by app_id again idempotent test | ||
azure_rm_adapplication: | ||
app_id: "{{ create_output.app_id }}" | ||
register: output | ||
display_name: "{{ display_name }}" | ||
register: output_app_id | ||
|
||
- name: Assert the idempotent success | ||
- name: Assert application by app_id the idempotent success | ||
ansible.builtin.assert: | ||
that: not output.changed | ||
that: not output_app_id.changed | ||
|
||
- name: Create application with more parameter | ||
azure_rm_adapplication: | ||
display_name: "{{ display_name }}-01" | ||
sign_in_audience: AzureADandPersonalMicrosoftAccount | ||
credential_description: "for test" | ||
end_date: 2021-10-01 | ||
start_date: 2021-05-18 | ||
display_name: "{{ display_name }}" | ||
app_id: "{{ create_output.app_id }}" | ||
homepage: "https://{{ display_name }}.test.com" | ||
sign_in_audience: AzureADMultipleOrgs | ||
identifier_uris: | ||
- "{{ display_name }}.com" | ||
app_roles: | ||
- allowed_member_types: | ||
- User | ||
description: "for app role test" | ||
display_name: "{{ display_name }}_approle" | ||
is_enabled: true | ||
value: Password@0329 | ||
- "api://{{ create_output.app_id }}" | ||
register: second_output | ||
|
||
- name: Assert secondary resource create success | ||
|
@@ -46,45 +43,58 @@ | |
- name: Get ad app info by object id | ||
azure_rm_adapplication_info: | ||
object_id: "{{ create_output.object_id }}" | ||
register: output | ||
register: object_id_output | ||
|
||
- name: Get ad app info by app id | ||
azure_rm_adapplication_info: | ||
app_id: "{{ create_output.app_id }}" | ||
register: output | ||
register: app_id_output | ||
|
||
- name: Get ad app info by display name | ||
azure_rm_adapplication_info: | ||
app_display_name: "{{ create_output.app_display_name }}" | ||
register: display_name_test_output | ||
app_display_name: "{{ display_name }}" | ||
register: display_name_output | ||
|
||
- name: Assert the application facts | ||
ansible.builtin.assert: | ||
that: | ||
- output.applications[0].app_display_name == "{{ display_name }}" | ||
- output.applications | length == 1 | ||
- display_name_test_output.applications[0].app_display_name == "{{ display_name }}" | ||
- display_name_test_output.applications | length == 1 | ||
- object_id_output.applications[0].app_display_name == "{{ display_name }}" | ||
- object_id_output.applications | length == 1 | ||
- app_id_output.applications[0].app_display_name == "{{ display_name }}" | ||
- app_id_output.applications | length == 1 | ||
- display_name_output.applications[0].app_display_name == "{{ display_name }}" | ||
- display_name_output.applications | length == 1 | ||
|
||
- name: Get difference from app_info_output and current | ||
azure_rm_adapplication_info: | ||
app_diff: "{{ app_info_output.applications }}" | ||
register: diff_output | ||
|
||
- name: Assert the difference | ||
ansible.builtin.assert: | ||
that: diff_output.app_diff[0].app_display_name == "{{ display_name }}" | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Wrong indentation: expected 6 but found 8 There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I don't understand this issue.. |
||
|
||
- name: Delete ad app by app id | ||
- name: Delete ad app by app_id | ||
azure_rm_adapplication: | ||
app_id: "{{ create_output.app_id }}" | ||
display_name: "{{ display_name }}" | ||
state: absent | ||
register: output | ||
|
||
- name: Assert the application delete success | ||
ansible.builtin.assert: | ||
that: output.changed | ||
|
||
- name: Delete ad app by app id | ||
- name: Delete ad app by app id again | ||
azure_rm_adapplication: | ||
app_id: "{{ second_output.app_id }}" | ||
display_name: "{{ display_name }}" | ||
state: absent | ||
register: output | ||
|
||
- name: Assert the secondary application delete success | ||
- name: Assert the secondary application delete no changed | ||
ansible.builtin.assert: | ||
that: output.changed | ||
that: not output.changed | ||
|
||
- name: Get ad app info by app id | ||
azure_rm_adapplication_info: | ||
|
@@ -94,4 +104,4 @@ | |
- name: Assert there is no application | ||
ansible.builtin.assert: | ||
that: | ||
- output.applications | length == 0 | ||
- output.applications | length == 0 | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. No new line character at the end of file There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. New commit: Fixed in 1 line. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Wrong indentation: expected 6 but found 8
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fix wrong indentation