From db73fe91303520c232d0d4d1a7b3bddf110dcc62 Mon Sep 17 00:00:00 2001 From: Mateus Rangel Date: Wed, 4 Jan 2023 00:01:00 -0300 Subject: [PATCH 1/9] Improving the documentation on how we generate the default value of the filename parameter --- lib/ansible/modules/apt_repository.py | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/lib/ansible/modules/apt_repository.py b/lib/ansible/modules/apt_repository.py index f9a0cd91958a51..35bca4ef5c02c9 100644 --- a/lib/ansible/modules/apt_repository.py +++ b/lib/ansible/modules/apt_repository.py @@ -72,8 +72,11 @@ filename: description: - Sets the name of the source list file in sources.list.d. - Defaults to a file name based on the repository source url. - The .list extension will be automatically added. + - Defaults to a file name based on the repository source URL (for example, https://user:pass@packagecloud.io/linz/prod/ubuntu/) following those steps + - 1) The URL Scheme is removed -> user:pass@packagecloud.io/linz/prod/ubuntu/ + - 2) The username and password in the URL are removed -> packagecloud.io/linz/prod/ubuntu/ + - 3) All contiguous non-alphanumeric(English) characters are removed and replaced with a single underscore -> packagecloud_io_linz_prod_ubuntu + - 4) Finally, '.list' is appended and we have the filename -> packagecloud_io_linz_prod_ubuntu.list type: str version_added: '2.1' codename: From f5dd4d9820347f0232977057a879b80ca88c62be Mon Sep 17 00:00:00 2001 From: Mateus Rangel Date: Wed, 4 Jan 2023 00:23:23 -0300 Subject: [PATCH 2/9] fix pep8 --- lib/ansible/modules/apt_repository.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/lib/ansible/modules/apt_repository.py b/lib/ansible/modules/apt_repository.py index 35bca4ef5c02c9..9893631a3e25a9 100644 --- a/lib/ansible/modules/apt_repository.py +++ b/lib/ansible/modules/apt_repository.py @@ -72,7 +72,8 @@ filename: description: - Sets the name of the source list file in sources.list.d. - - Defaults to a file name based on the repository source URL (for example, https://user:pass@packagecloud.io/linz/prod/ubuntu/) following those steps + - Defaults to a file name based on the repository source URL (for example, https://user:pass@packagecloud.io/linz/prod/ubuntu/) + following those steps - 1) The URL Scheme is removed -> user:pass@packagecloud.io/linz/prod/ubuntu/ - 2) The username and password in the URL are removed -> packagecloud.io/linz/prod/ubuntu/ - 3) All contiguous non-alphanumeric(English) characters are removed and replaced with a single underscore -> packagecloud_io_linz_prod_ubuntu From f810f3d544fecab6c359ed0266e7484a72069419 Mon Sep 17 00:00:00 2001 From: Mateus Rangel Date: Wed, 4 Jan 2023 22:45:45 -0300 Subject: [PATCH 3/9] removing unnecessary documentation and improving the module's return --- lib/ansible/modules/apt_repository.py | 22 ++++++++++------------ 1 file changed, 10 insertions(+), 12 deletions(-) diff --git a/lib/ansible/modules/apt_repository.py b/lib/ansible/modules/apt_repository.py index 9893631a3e25a9..ae441e31f6d6f5 100644 --- a/lib/ansible/modules/apt_repository.py +++ b/lib/ansible/modules/apt_repository.py @@ -72,12 +72,8 @@ filename: description: - Sets the name of the source list file in sources.list.d. - - Defaults to a file name based on the repository source URL (for example, https://user:pass@packagecloud.io/linz/prod/ubuntu/) - following those steps - - 1) The URL Scheme is removed -> user:pass@packagecloud.io/linz/prod/ubuntu/ - - 2) The username and password in the URL are removed -> packagecloud.io/linz/prod/ubuntu/ - - 3) All contiguous non-alphanumeric(English) characters are removed and replaced with a single underscore -> packagecloud_io_linz_prod_ubuntu - - 4) Finally, '.list' is appended and we have the filename -> packagecloud_io_linz_prod_ubuntu.list + Defaults to a file name based on the repository source url. + The .list extension will be automatically added. type: str version_added: '2.1' codename: @@ -692,15 +688,17 @@ def main(): sources_after = sourceslist.dump() changed = sources_before != sources_after - if changed and module._diff: - diff = [] - for filename in set(sources_before.keys()).union(sources_after.keys()): + diff = [] + sources_added = set() + sources_removed = set() + if changed: + sources_added = set(sources_after.keys()).difference(sources_before.keys()) + sources_removed = set(sources_before.keys()).difference(sources_after.keys()) + for filename in set(sources_added.union(sources_removed)): diff.append({'before': sources_before.get(filename, ''), 'after': sources_after.get(filename, ''), 'before_header': (filename, '/dev/null')[filename not in sources_before], 'after_header': (filename, '/dev/null')[filename not in sources_after]}) - else: - diff = {} if changed and not module.check_mode: try: @@ -732,7 +730,7 @@ def main(): revert_sources_list(sources_before, sources_after, sourceslist_before) module.fail_json(msg=to_native(ex)) - module.exit_json(changed=changed, repo=repo, state=state, diff=diff) + module.exit_json(changed=changed, repo=repo, sources_added=sources_added, sources_removed=sources_removed, state=state, diff=diff) if __name__ == '__main__': From 9e56bcc5ef508e85cc45c4781800af71dedd7c8a Mon Sep 17 00:00:00 2001 From: Mateus Rangel Date: Thu, 5 Jan 2023 00:14:44 -0300 Subject: [PATCH 4/9] making the RETURN docs --- lib/ansible/modules/apt_repository.py | 20 +++++++++++++++++++- 1 file changed, 19 insertions(+), 1 deletion(-) diff --git a/lib/ansible/modules/apt_repository.py b/lib/ansible/modules/apt_repository.py index ae441e31f6d6f5..44801d08f47d8b 100644 --- a/lib/ansible/modules/apt_repository.py +++ b/lib/ansible/modules/apt_repository.py @@ -146,7 +146,25 @@ state: present ''' -RETURN = '''#''' +RETURN = ''' +repo: + description: A source string for the repository + returned: always + type: str + sample: "deb https://artifacts.elastic.co/packages/6.x/apt stable main" + +sources_added: + description: List of sources added + returned: success, sources were added + type: list + sample: ["/etc/apt/sources.list.d/artifacts_elastic_co_packages_6_x_apt.list"] + +sources_removed: + description: List of sources removed + returned: success, sources were removed + type: list + sample: ["/etc/apt/sources.list.d/artifacts_elastic_co_packages_6_x_apt.list"] +''' import copy import glob From 93bb37a810fa24926c9eaa6b0d959fa29f5b8a77 Mon Sep 17 00:00:00 2001 From: Mateus Rangel Date: Thu, 5 Jan 2023 00:21:11 -0300 Subject: [PATCH 5/9] pep8 --- lib/ansible/modules/apt_repository.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/ansible/modules/apt_repository.py b/lib/ansible/modules/apt_repository.py index 44801d08f47d8b..50079f7590b87d 100644 --- a/lib/ansible/modules/apt_repository.py +++ b/lib/ansible/modules/apt_repository.py @@ -152,13 +152,13 @@ returned: always type: str sample: "deb https://artifacts.elastic.co/packages/6.x/apt stable main" - + sources_added: description: List of sources added returned: success, sources were added type: list sample: ["/etc/apt/sources.list.d/artifacts_elastic_co_packages_6_x_apt.list"] - + sources_removed: description: List of sources removed returned: success, sources were removed From baeb819d57ee2297ba6f9fde49d429485cf36721 Mon Sep 17 00:00:00 2001 From: Mateus Rangel Date: Thu, 5 Jan 2023 17:53:05 -0300 Subject: [PATCH 6/9] version_added and changelog --- changelogs/fragments/79658-improving-return-and-docs.yaml | 2 ++ lib/ansible/modules/apt_repository.py | 2 ++ 2 files changed, 4 insertions(+) create mode 100644 changelogs/fragments/79658-improving-return-and-docs.yaml diff --git a/changelogs/fragments/79658-improving-return-and-docs.yaml b/changelogs/fragments/79658-improving-return-and-docs.yaml new file mode 100644 index 00000000000000..bf0cc2a83d0e63 --- /dev/null +++ b/changelogs/fragments/79658-improving-return-and-docs.yaml @@ -0,0 +1,2 @@ +minor_changes: + - apt_repository - adds ``sources_added`` and ``sources_removed`` to the return of the module (https://github.com/ansible/ansible/issues/79306). \ No newline at end of file diff --git a/lib/ansible/modules/apt_repository.py b/lib/ansible/modules/apt_repository.py index 50079f7590b87d..57284f92942c03 100644 --- a/lib/ansible/modules/apt_repository.py +++ b/lib/ansible/modules/apt_repository.py @@ -158,12 +158,14 @@ returned: success, sources were added type: list sample: ["/etc/apt/sources.list.d/artifacts_elastic_co_packages_6_x_apt.list"] + version_added: "2.15" sources_removed: description: List of sources removed returned: success, sources were removed type: list sample: ["/etc/apt/sources.list.d/artifacts_elastic_co_packages_6_x_apt.list"] + version_added: "2.15" ''' import copy From 32b892c577d3e49d9524b257e2e5fa3077151ff6 Mon Sep 17 00:00:00 2001 From: Mateus Rangel Date: Thu, 5 Jan 2023 18:27:53 -0300 Subject: [PATCH 7/9] module._diff --- lib/ansible/modules/apt_repository.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/ansible/modules/apt_repository.py b/lib/ansible/modules/apt_repository.py index 57284f92942c03..a9fa8936c6cb53 100644 --- a/lib/ansible/modules/apt_repository.py +++ b/lib/ansible/modules/apt_repository.py @@ -711,7 +711,7 @@ def main(): diff = [] sources_added = set() sources_removed = set() - if changed: + if changed and module._diff: sources_added = set(sources_after.keys()).difference(sources_before.keys()) sources_removed = set(sources_before.keys()).difference(sources_after.keys()) for filename in set(sources_added.union(sources_removed)): From 60b75414a24693a8fb10230fd7449e0d20c796bb Mon Sep 17 00:00:00 2001 From: Mateus Rangel Date: Thu, 5 Jan 2023 18:48:28 -0300 Subject: [PATCH 8/9] module._diff fix --- lib/ansible/modules/apt_repository.py | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/lib/ansible/modules/apt_repository.py b/lib/ansible/modules/apt_repository.py index a9fa8936c6cb53..e4a957b32d2db1 100644 --- a/lib/ansible/modules/apt_repository.py +++ b/lib/ansible/modules/apt_repository.py @@ -711,14 +711,15 @@ def main(): diff = [] sources_added = set() sources_removed = set() - if changed and module._diff: + if changed: sources_added = set(sources_after.keys()).difference(sources_before.keys()) sources_removed = set(sources_before.keys()).difference(sources_after.keys()) - for filename in set(sources_added.union(sources_removed)): - diff.append({'before': sources_before.get(filename, ''), - 'after': sources_after.get(filename, ''), - 'before_header': (filename, '/dev/null')[filename not in sources_before], - 'after_header': (filename, '/dev/null')[filename not in sources_after]}) + if module._diff: + for filename in set(sources_added.union(sources_removed)): + diff.append({'before': sources_before.get(filename, ''), + 'after': sources_after.get(filename, ''), + 'before_header': (filename, '/dev/null')[filename not in sources_before], + 'after_header': (filename, '/dev/null')[filename not in sources_after]}) if changed and not module.check_mode: try: From 47d8c32a37d0b7645b7053e16a5c7f8ac13f3db1 Mon Sep 17 00:00:00 2001 From: Matt Davis Date: Mon, 6 Feb 2023 10:46:14 -0800 Subject: [PATCH 9/9] add rudimentary tests for new outputs --- .../targets/apt_repository/tasks/apt.yml | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/test/integration/targets/apt_repository/tasks/apt.yml b/test/integration/targets/apt_repository/tasks/apt.yml index 0dc25afd598dc0..cb418c3436b4e0 100644 --- a/test/integration/targets/apt_repository/tasks/apt.yml +++ b/test/integration/targets/apt_repository/tasks/apt.yml @@ -152,6 +152,11 @@ - 'result.changed' - 'result.state == "present"' - 'result.repo == "{{test_ppa_spec}}"' + - '"sources_added" in result' + - 'result.sources_added | length == 1' + - '"git" in result.sources_added[0]' + - '"sources_removed" in result' + - 'result.sources_removed | length == 0' - result_cache is not changed - name: 'examine apt cache mtime' @@ -167,6 +172,17 @@ apt_repository: repo='{{test_ppa_spec}}' state=absent register: result +- assert: + that: + - 'result.changed' + - 'result.state == "absent"' + - 'result.repo == "{{test_ppa_spec}}"' + - '"sources_added" in result' + - 'result.sources_added | length == 0' + - '"sources_removed" in result' + - 'result.sources_removed | length == 1' + - '"git" in result.sources_removed[0]' + # When installing a repo with the spec, the key is *NOT* added - name: 'ensure ppa key is absent (expect: pass)' apt_key: id='{{test_ppa_key}}' state=absent