From 3eab636b3f544251bcbde4f89986212c2423fb46 Mon Sep 17 00:00:00 2001 From: Adrian Likins Date: Mon, 25 Sep 2017 15:00:31 -0400 Subject: [PATCH] Fix 'distribution' fact for ArchLinux (#30723) * Fix 'distribution' fact for ArchLinux Allow empty wasn't breaking out of the process_dist_files loop, so a empty /etc/arch-release would continue searching and eventually try /etc/os-release. The os-release parsing works, but the distro name there is 'Arch Linux' which does not match the 2.3 behavior of 'Archlinux' Add a OS_RELEASE_ALIAS map for the cases where we need to get the distro name from os-release but use an alias. We can't include 'Archlinux' in SEARCH_STRING because a name match on its keys but without a match on the content causes a fallback to using the first whitespace seperated item from the file content as the name. For os-release, that is in form 'NAME=Arch Linux' With os-release returning the right name, this also supports the case where there is no /etc/arch-release, but there is a /etc/os-release Fixes #30600 * pep8 and comment cleanup --- .../module_utils/facts/system/distribution.py | 23 ++++++++-- .../module_utils/test_distribution_version.py | 42 +++++++++++++++++++ 2 files changed, 62 insertions(+), 3 deletions(-) diff --git a/lib/ansible/module_utils/facts/system/distribution.py b/lib/ansible/module_utils/facts/system/distribution.py index f4e06de3f39..c0b86014a7d 100644 --- a/lib/ansible/module_utils/facts/system/distribution.py +++ b/lib/ansible/module_utils/facts/system/distribution.py @@ -64,6 +64,7 @@ class DistributionFiles: {'path': '/etc/system-release', 'name': 'Amazon'}, {'path': '/etc/alpine-release', 'name': 'Alpine'}, {'path': '/etc/arch-release', 'name': 'Archlinux', 'allowempty': True}, + {'path': '/etc/os-release', 'name': 'Archlinux'}, {'path': '/etc/os-release', 'name': 'SUSE'}, {'path': '/etc/SuSE-release', 'name': 'SUSE'}, {'path': '/etc/gentoo-release', 'name': 'Gentoo'}, @@ -84,6 +85,13 @@ class DistributionFiles: 'SMGL': 'Source Mage GNU/Linux', } + # We can't include this in SEARCH_STRING because a name match on its keys + # causes a fallback to using the first whitespace seperated item from the file content + # as the name. For os-release, that is in form 'NAME=Arch' + OS_RELEASE_ALIAS = { + 'Archlinux': 'Arch Linux' + } + def __init__(self, module): self.module = module @@ -114,16 +122,19 @@ class DistributionFiles: return True, dist_file_dict + if name in self.OS_RELEASE_ALIAS: + if self.OS_RELEASE_ALIAS[name] in dist_file_content: + dist_file_dict['distribution'] = name + return True, dist_file_dict + # call a dedicated function for parsing the file content # TODO: replace with a map or a class try: # FIXME: most of these dont actually look at the dist file contents, but random other stuff distfunc_name = 'parse_distribution_file_' + name # print('distfunc_name: %s' % distfunc_name) - distfunc = getattr(self, distfunc_name) # print('distfunc: %s' % distfunc) - parsed, dist_file_dict = distfunc(name, dist_file_content, path, collected_facts) return parsed, dist_file_dict except AttributeError as exc: @@ -165,6 +176,12 @@ class DistributionFiles: has_dist_file, dist_file_content = self._get_dist_file_content(path, allow_empty=allow_empty) + # but we allow_empty. For example, ArchLinux with an empty /etc/arch-release and a + # /etc/os-release with a different name + if has_dist_file and allow_empty: + dist_file_facts['distribution'] = name + break + if not has_dist_file: # keep looking continue @@ -180,7 +197,7 @@ class DistributionFiles: dist_file_facts['distribution_file_parsed'] = parsed_dist_file - # finally found the right os dist file and were able to parse it + # finally found the right os dit file and were able to parse it if parsed_dist_file: dist_file_facts.update(parsed_dist_file_facts) break diff --git a/test/units/module_utils/test_distribution_version.py b/test/units/module_utils/test_distribution_version.py index 148f1a9747e..b1e79fa19a8 100644 --- a/test/units/module_utils/test_distribution_version.py +++ b/test/units/module_utils/test_distribution_version.py @@ -815,6 +815,48 @@ DISTRIB_DESCRIPTION="CoreOS 976.0.0 (Coeur Rouge)" "distribution_version": "NA" } }, + + # ArchLinux with an empty /etc/arch-release and a /etc/os-release with "NAME=Arch Linux" + { + "platform.dist": [ + "", + "", + "" + ], + "input": { + "/etc/os-release": "NAME=\"Arch Linux\"\nPRETTY_NAME=\"Arch Linux\"\nID=arch\nID_LIKE=archlinux\nANSI_COLOR=\"0;36\"\nHOME_URL=\"https://www.archlinux.org/\"\nSUPPORT_URL=\"https://bbs.archlinux.org/\"\nBUG_REPORT_URL=\"https://bugs.archlinux.org/\"\n\n", # noqa + "/etc/arch-release": "", + }, + "name": "Arch Linux NA", + "result": { + "distribution_release": "NA", + "distribution": "Archlinux", + "distribution_major_version": "NA", + "os_family": "Archlinux", + "distribution_version": "NA" + } + }, + + # ArchLinux with no /etc/arch-release but with a /etc/os-release with NAME=Arch Linux + # The fact needs to map 'Arch Linux' to 'Archlinux' for compat with 2.3 and earlier facts + { + "platform.dist": [ + "", + "", + "" + ], + "input": { + "/etc/os-release": "NAME=\"Arch Linux\"\nPRETTY_NAME=\"Arch Linux\"\nID=arch\nID_LIKE=archlinux\nANSI_COLOR=\"0;36\"\nHOME_URL=\"https://www.archlinux.org/\"\nSUPPORT_URL=\"https://bbs.archlinux.org/\"\nBUG_REPORT_URL=\"https://bugs.archlinux.org/\"\n\n", # noqa + }, + "name": "Arch Linux no arch-release NA", + "result": { + "distribution_release": "NA", + "distribution": "Archlinux", + "distribution_major_version": "NA", + "os_family": "Archlinux", + "distribution_version": "NA" + } + } ]