diff --git a/changelogs/fragments/platform-dist-to-nir0s-distro.yaml b/changelogs/fragments/platform-dist-to-nir0s-distro.yaml new file mode 100644 index 00000000000..56132991773 --- /dev/null +++ b/changelogs/fragments/platform-dist-to-nir0s-distro.yaml @@ -0,0 +1,5 @@ +--- +minor_changes: +- Python-3.8 removes platform.dist() from the standard library. To maintain + compatibility we've switched to an alternative library, nir0s/distro to + detect the distribution for fact gathering. diff --git a/hacking/tests/gen_distribution_version_testcase.py b/hacking/tests/gen_distribution_version_testcase.py index 6bc52c4e41c..6d09c77af06 100755 --- a/hacking/tests/gen_distribution_version_testcase.py +++ b/hacking/tests/gen_distribution_version_testcase.py @@ -3,17 +3,21 @@ """ This script generated test_cases for test_distribution_version.py. -To do so it outputs the relevant files from /etc/*release, the output of platform.dist() and the current ansible_facts regarding the distribution version. +To do so it outputs the relevant files from /etc/*release, the output of distro.linux_distribution() +and the current ansible_facts regarding the distribution version. This assumes a working ansible version in the path. """ -import platform + import os.path import subprocess import json import sys +from ansible.module_utils import distro + + filelist = [ '/etc/oracle-release', '/etc/slackware-version', @@ -44,8 +48,7 @@ for f in filelist: with open(f) as fh: fcont[f] = fh.read() -dist = platform.dist() - +dist = distro.linux_distribution(full_distribution_name=False) facts = ['distribution', 'distribution_version', 'distribution_release', 'distribution_major_version', 'os_family'] diff --git a/lib/ansible/module_utils/common/sys_info.py b/lib/ansible/module_utils/common/sys_info.py index 8b62de4f97d..d6782959a8c 100644 --- a/lib/ansible/module_utils/common/sys_info.py +++ b/lib/ansible/module_utils/common/sys_info.py @@ -1,7 +1,17 @@ +# Copyright (c), Michael DeHaan , 2012-2013 +# Copyright (c), Toshio Kuratomi 2016 +# Simplified BSD License (see licenses/simplified_bsd.txt or https://opensource.org/licenses/BSD-2-Clause) + +from __future__ import absolute_import, division, print_function +__metaclass__ = type + import os import platform +from ansible.module_utils import distro + +# Backwards compat. New code should just use platform.system() def get_platform(): ''' :rtype: NativeString @@ -16,39 +26,60 @@ def get_distribution(): :returns: Name of the distribution the module is running on ''' distribution = None - additional_linux = ('alpine', 'arch', 'devuan') - supported_dists = platform._supported_dists + additional_linux if platform.system() == 'Linux': - try: - distribution = platform.linux_distribution(supported_dists=supported_dists)[0].capitalize() - if not distribution and os.path.isfile('/etc/system-release'): - distribution = platform.linux_distribution(supported_dists=['system'])[0].capitalize() - if 'Amazon' in distribution: - distribution = 'Amazon' - else: - distribution = 'OtherLinux' - except Exception: - # FIXME: MethodMissing, I assume? - distribution = platform.dist()[0].capitalize() + distribution = distro.name().capitalize() + + # FIXME: Would we need to normalize these if we used: id() instead of name()? + distribution_words = distribution.split() + if 'Amazon' in distribution_words: + distribution = 'Amazon' + elif not distribution: + distribution = 'OtherLinux' + return distribution def get_distribution_version(): ''' :rtype: NativeString or None - :returns: A string representation of the version of the distribution + :returns: A string representation of the version of the distribution. None if this is not run + on a Linux machine ''' - distribution_version = None + version = None if platform.system() == 'Linux': - try: - distribution_version = platform.linux_distribution()[1] - if not distribution_version and os.path.isfile('/etc/system-release'): - distribution_version = platform.linux_distribution(supported_dists=['system'])[1] - except Exception: - # FIXME: MethodMissing, I assume? - distribution_version = platform.dist()[1] - return distribution_version + version = distro.version() + return version + + +def get_distribution_codename(): + ''' + Return the code name for this Linux Distribution + + :rtype: NativeString or None + :returns: A string representation of the distribution's codename or None if not a Linux distro + ''' + codename = None + if platform.system() == 'Linux': + # Until this gets merged and we update our bundled copy of distro: + # https://github.com/nir0s/distro/pull/230 + # Fixes Fedora 28+ not having a code name and Ubuntu Xenial Xerus needing to be "xenial" + os_release_info = distro.os_release_info() + codename = os_release_info.get('version_codename') + + if codename is None: + codename = os_release_info.get('ubuntu_codename') + + if codename is None and distro.id() == 'ubuntu': + lsb_release_info = distro.lsb_release_info() + codename = lsb_release_info.get('codename') + + if codename is None: + codename = distro.codename() + if codename == u'': + codename = None + + return codename def get_all_subclasses(cls): diff --git a/lib/ansible/module_utils/facts/system/distribution.py b/lib/ansible/module_utils/facts/system/distribution.py index 211d907556d..49e57686c7f 100644 --- a/lib/ansible/module_utils/facts/system/distribution.py +++ b/lib/ansible/module_utils/facts/system/distribution.py @@ -20,8 +20,9 @@ import os import platform import re +from ansible.module_utils.common.sys_info import get_distribution, get_distribution_version, \ + get_distribution_codename from ansible.module_utils.facts.utils import get_file_content - from ansible.module_utils.facts.collector import BaseFactCollector @@ -111,7 +112,7 @@ class DistributionFiles: dist_file_dict = {} if name in self.SEARCH_STRING: # look for the distribution string in the data and replace according to RELEASE_NAME_MAP - # only the distribution name is set, the version is assumed to be correct from platform.dist() + # only the distribution name is set, the version is assumed to be correct from distro.linux_distribution() if self.SEARCH_STRING[name] in dist_file_content: # this sets distribution=RedHat if 'Red Hat' shows up in data dist_file_dict['distribution'] = name @@ -152,12 +153,15 @@ class DistributionFiles: def _guess_distribution(self): # try to find out which linux distribution this is - dist = platform.dist() + dist = (get_distribution(), get_distribution_version(), get_distribution_codename()) distribution_guess = {} - distribution_guess['distribution'] = dist[0].capitalize() or 'NA' + distribution_guess['distribution'] = dist[0] or 'NA' distribution_guess['distribution_version'] = dist[1] or 'NA' - distribution_guess['distribution_major_version'] = dist[1].split('.')[0] or 'NA' - distribution_guess['distribution_release'] = dist[2] or 'NA' + distribution_guess['distribution_major_version'] = \ + distribution_guess['distribution_version'].split('.')[0] or 'NA' + # distribution_release can be the empty string + distribution_guess['distribution_release'] = 'NA' if dist[2] is None else dist[2] + return distribution_guess def process_dist_files(self): @@ -362,8 +366,7 @@ class DistributionFiles: def parse_distribution_file_Coreos(self, name, data, path, collected_facts): coreos_facts = {} # FIXME: pass in ro copy of facts for this kind of thing - dist = platform.dist() - distro = dist[0] + distro = get_distribution() if distro.lower() == 'coreos': if not data: diff --git a/lib/ansible/modules/cloud/misc/xenserver_facts.py b/lib/ansible/modules/cloud/misc/xenserver_facts.py index 47183b6da26..0612308c2fb 100644 --- a/lib/ansible/modules/cloud/misc/xenserver_facts.py +++ b/lib/ansible/modules/cloud/misc/xenserver_facts.py @@ -46,7 +46,6 @@ EXAMPLES = ''' # } ''' -import platform HAVE_XENAPI = False try: @@ -55,6 +54,7 @@ try: except ImportError: pass +from ansible.module_utils import distro from ansible.module_utils.basic import AnsibleModule @@ -70,8 +70,7 @@ class XenServerFacts: @property def version(self): - # Be aware! Deprecated in Python 2.6! - result = platform.dist()[1] + result = distro.linux_distribution()[1] return result @property diff --git a/lib/ansible/modules/system/user.py b/lib/ansible/modules/system/user.py index cb9521abf9a..5d6aaad1213 100644 --- a/lib/ansible/modules/system/user.py +++ b/lib/ansible/modules/system/user.py @@ -379,7 +379,6 @@ import errno import grp import os import re -import platform import pty import pwd import select @@ -388,6 +387,7 @@ import socket import subprocess import time +from ansible.module_utils import distro from ansible.module_utils._text import to_native, to_bytes, to_text from ansible.module_utils.basic import load_platform_subclass, AnsibleModule @@ -569,7 +569,7 @@ class User(object): # errors from useradd trying to create a group when # USERGROUPS_ENAB is set in /etc/login.defs. if os.path.exists('/etc/redhat-release'): - dist = platform.dist() + dist = distro.linux_distribution(full_distribution_name=False) major_release = int(dist[1].split('.')[0]) if major_release <= 5: cmd.append('-n') @@ -578,7 +578,7 @@ class User(object): elif os.path.exists('/etc/SuSE-release'): # -N did not exist in useradd before SLE 11 and did not # automatically create a group - dist = platform.dist() + dist = distro.linux_distribution(full_distribution_name=False) major_release = int(dist[1].split('.')[0]) if major_release >= 12: cmd.append('-N') diff --git a/test/units/executor/module_common/test_recursive_finder.py b/test/units/executor/module_common/test_recursive_finder.py index fb99a3dfe16..f50c821cba3 100644 --- a/test/units/executor/module_common/test_recursive_finder.py +++ b/test/units/executor/module_common/test_recursive_finder.py @@ -47,6 +47,8 @@ MODULE_UTILS_BASIC_IMPORTS = frozenset((('_text',), ('common', 'file'), ('common', 'process'), ('common', 'sys_info'), + ('distro', '__init__'), + ('distro', '_distro'), ('parsing', '__init__'), ('parsing', 'convert_bool'), ('pycompat24',), @@ -63,6 +65,8 @@ MODULE_UTILS_BASIC_FILES = frozenset(('ansible/module_utils/parsing/__init__.py' 'ansible/module_utils/common/__init__.py', 'ansible/module_utils/common/file.py', 'ansible/module_utils/common/sys_info.py', + 'ansible/module_utils/distro/__init__.py', + 'ansible/module_utils/distro/_distro.py', 'ansible/module_utils/pycompat24.py', )) diff --git a/test/units/module_utils/basic/test_platform_distribution.py b/test/units/module_utils/basic/test_platform_distribution.py index 9880f5771d6..e7067b1a762 100644 --- a/test/units/module_utils/basic/test_platform_distribution.py +++ b/test/units/module_utils/basic/test_platform_distribution.py @@ -1,108 +1,159 @@ # -*- coding: utf-8 -*- # (c) 2012-2014, Michael DeHaan # (c) 2016 Toshio Kuratomi -# (c) 2017 Ansible Project +# (c) 2017-2018 Ansible Project # GNU General Public License v3.0+ (see COPYING or https://www.gnu.org/licenses/gpl-3.0.txt) from __future__ import absolute_import, division, print_function __metaclass__ = type +import pytest + from units.mock.procenv import ModuleTestCase from units.compat.mock import patch + from ansible.module_utils.six.moves import builtins +# Functions being tested +from ansible.module_utils.common.sys_info import get_all_subclasses +from ansible.module_utils.common.sys_info import get_distribution +from ansible.module_utils.common.sys_info import get_distribution_version +from ansible.module_utils.common.sys_info import get_platform +from ansible.module_utils.common.sys_info import load_platform_subclass + + realimport = builtins.__import__ -class TestPlatform(ModuleTestCase): - def test_module_utils_basic_get_platform(self): - with patch('platform.system', return_value='foo'): - from ansible.module_utils.common.sys_info import get_platform - self.assertEqual(get_platform(), 'foo') +@pytest.fixture +def platform_linux(mocker): + mocker.patch('platform.system', return_value='Linux') - def test_module_utils_basic_get_distribution(self): - from ansible.module_utils.common.sys_info import get_distribution - with patch('platform.system', return_value='Foo'): - self.assertEqual(get_distribution(), None) +# +# get_platform tests +# - with patch('platform.system', return_value='Linux'): - with patch('platform.linux_distribution', return_value=["foo"]): - self.assertEqual(get_distribution(), "Foo") +def test_get_platform(): + with patch('platform.system', return_value='foo'): + assert get_platform() == 'foo' - with patch('os.path.isfile', return_value=True): - with patch('platform.linux_distribution', side_effect=[("AmazonFooBar", )]): - self.assertEqual(get_distribution(), "Amazonfoobar") - with patch('platform.linux_distribution', side_effect=(("", ), ("AmazonFooBam",))): - self.assertEqual(get_distribution(), "Amazon") +# +# get_distribution tests +# - with patch('platform.linux_distribution', side_effect=[("", ), ("", )]): - self.assertEqual(get_distribution(), "OtherLinux") +def test_get_distribution_not_linux(): + """If it's not Linux, then it has no distribution""" + with patch('platform.system', return_value='Foo'): + assert get_distribution() is None - def _dist(distname='', version='', id='', supported_dists=(), full_distribution_name=1): - if supported_dists != (): - return ("Bar", "2", "Two") - else: - return ("", "", "") - with patch('platform.linux_distribution', side_effect=_dist): - self.assertEqual(get_distribution(), "Bar") +@pytest.mark.usefixtures("platform_linux") +class TestGetDistribution: + """ Tests for get_distribution that have to find somethine""" + def test_distro_known(self): + with patch('ansible.module_utils.distro.name', return_value="foo"): + assert get_distribution() == "Foo" - with patch('platform.linux_distribution', side_effect=Exception("boo")): - with patch('platform.dist', return_value=("bar", "2", "Two")): - self.assertEqual(get_distribution(), "Bar") + def test_distro_unknown(self): + with patch('ansible.module_utils.distro.name', return_value=""): + assert get_distribution() == "OtherLinux" - def test_module_utils_basic_get_distribution_version(self): - from ansible.module_utils.common.sys_info import get_distribution_version + def test_distro_amazon_part_of_another_name(self): + with patch('ansible.module_utils.distro.name', return_value="AmazonFooBar"): + assert get_distribution() == "Amazonfoobar" - with patch('platform.system', return_value='Foo'): - self.assertEqual(get_distribution_version(), None) + def test_distro_amazon_linux(self): + with patch('ansible.module_utils.distro.name', return_value="Amazon Linux AMI"): + assert get_distribution() == "Amazon" - with patch('platform.system', return_value='Linux'): - with patch('platform.linux_distribution', return_value=("foo", "1", "One")): - self.assertEqual(get_distribution_version(), "1") - with patch('os.path.isfile', return_value=True): - def _dist(distname='', version='', id='', supported_dists=(), full_distribution_name=1): - if supported_dists != (): - return ("AmazonFooBar", "2", "") - else: - return ("", "", "") +# +# get_distribution_version tests +# - with patch('platform.linux_distribution', side_effect=_dist): - self.assertEqual(get_distribution_version(), "2") +def test_get_distribution_version_not_linux(): + """If it's not Linux, then it has no distribution""" + with patch('platform.system', return_value='Foo'): + assert get_distribution_version() is None - with patch('platform.linux_distribution', side_effect=Exception("boo")): - with patch('platform.dist', return_value=("bar", "3", "Three")): - self.assertEqual(get_distribution_version(), "3") - def test_module_utils_basic_load_platform_subclass(self): - class LinuxTest: - pass +@pytest.mark.usefixtures("platform_linux") +def test_distro_found(): + with patch('ansible.module_utils.distro.version', return_value="1"): + assert get_distribution_version() == "1" - class Foo(LinuxTest): - platform = "Linux" - distribution = None - class Bar(LinuxTest): - platform = "Linux" - distribution = "Bar" +# +# Tests for LoadPlatformSubclass +# - from ansible.module_utils.common.sys_info import load_platform_subclass +class TestLoadPlatformSubclass: + class LinuxTest: + pass - # match just the platform class, not a specific distribution - with patch('ansible.module_utils.common.sys_info.get_platform', return_value="Linux"): - with patch('ansible.module_utils.common.sys_info.get_distribution', return_value=None): - self.assertIs(type(load_platform_subclass(LinuxTest)), Foo) + class Foo(LinuxTest): + platform = "Linux" + distribution = None - # match both the distribution and platform class - with patch('ansible.module_utils.common.sys_info.get_platform', return_value="Linux"): - with patch('ansible.module_utils.common.sys_info.get_distribution', return_value="Bar"): - self.assertIs(type(load_platform_subclass(LinuxTest)), Bar) + class Bar(LinuxTest): + platform = "Linux" + distribution = "Bar" + def test_not_linux(self): # if neither match, the fallback should be the top-level class with patch('ansible.module_utils.common.sys_info.get_platform', return_value="Foo"): with patch('ansible.module_utils.common.sys_info.get_distribution', return_value=None): - self.assertIs(type(load_platform_subclass(LinuxTest)), LinuxTest) + assert isinstance(load_platform_subclass(self.LinuxTest), self.LinuxTest) + + @pytest.mark.usefixtures("platform_linux") + def test_get_distribution_none(self): + # match just the platform class, not a specific distribution + with patch('ansible.module_utils.common.sys_info.get_distribution', return_value=None): + assert isinstance(load_platform_subclass(self.LinuxTest), self.Foo) + + @pytest.mark.usefixtures("platform_linux") + def test_get_distribution_found(self): + # match both the distribution and platform class + with patch('ansible.module_utils.common.sys_info.get_distribution', return_value="Bar"): + assert isinstance(load_platform_subclass(self.LinuxTest), self.Bar) + + +# +# Tests for get_all_subclasses +# + +class TestGetAllSubclasses: + class Base: + pass + + class BranchI(Base): + pass + + class BranchII(Base): + pass + + class BranchIA(BranchI): + pass + + class BranchIB(BranchI): + pass + + class BranchIIA(BranchII): + pass + + class BranchIIB(BranchII): + pass + + def test_bottom_level(self): + assert get_all_subclasses(self.BranchIIB) == [] + + def test_one_inheritance(self): + assert set(get_all_subclasses(self.BranchII)) == set([self.BranchIIA, self.BranchIIB]) + + def test_toplevel(self): + assert set(get_all_subclasses(self.Base)) == set([self.BranchI, self.BranchII, + self.BranchIA, self.BranchIB, + self.BranchIIA, self.BranchIIB]) diff --git a/test/units/module_utils/test_distribution_version.py b/test/units/module_utils/test_distribution_version.py index ee70c2ffd32..7df9ee4c054 100644 --- a/test/units/module_utils/test_distribution_version.py +++ b/test/units/module_utils/test_distribution_version.py @@ -7,8 +7,11 @@ __metaclass__ = type from itertools import product +import mock import pytest +from ansible.module_utils.six.moves import builtins + # the module we are actually testing (sort of) from ansible.module_utils.facts.system.distribution import DistributionFactCollector @@ -1044,7 +1047,7 @@ def test_distribution_version(am, mocker, testcase): * input files that are faked * those should be complete and also include "irrelevant" files that might be mistaken as coming from other distributions * all files that are not listed here are assumed to not exist at all - * the output of pythons platform.dist() + * the output of ansible.module_utils.distro.linux_distribution() [called platform.dist() for historical reasons] * results for the ansible variables distribution* and os_family """ @@ -1081,14 +1084,43 @@ def test_distribution_version(am, mocker, testcase): def mock_platform_version(): return testcase.get('platform.version', '') + def mock_distro_name(): + return testcase['platform.dist'][0] + + def mock_distro_version(): + return testcase['platform.dist'][1] + + def mock_distro_codename(): + return testcase['platform.dist'][2] + + def mock_open(filename, mode='r'): + if filename in testcase['input']: + file_object = mocker.mock_open(read_data=testcase['input'][filename]).return_value + file_object.__iter__.return_value = testcase['input'][filename].splitlines(True) + else: + file_object = real_open(filename, mode) + return file_object + + def mock_os_path_is_file(filename): + if filename in testcase['input']: + return True + return False + mocker.patch('ansible.module_utils.facts.system.distribution.get_file_content', mock_get_file_content) mocker.patch('ansible.module_utils.facts.system.distribution.get_uname_version', mock_get_uname_version) mocker.patch('ansible.module_utils.facts.system.distribution._file_exists', mock_file_exists) - mocker.patch('platform.dist', lambda: testcase['platform.dist']) + mocker.patch('ansible.module_utils.distro.name', mock_distro_name) + mocker.patch('ansible.module_utils.distro.id', mock_distro_name) + mocker.patch('ansible.module_utils.distro.version', mock_distro_version) + mocker.patch('ansible.module_utils.distro.codename', mock_distro_codename) + mocker.patch('os.path.isfile', mock_os_path_is_file) mocker.patch('platform.system', mock_platform_system) mocker.patch('platform.release', mock_platform_release) mocker.patch('platform.version', mock_platform_version) + real_open = builtins.open + mocker.patch.object(builtins, 'open', new=mock_open) + # run Facts() distro_collector = DistributionFactCollector() generated_facts = distro_collector.collect(am) diff --git a/test/units/module_utils/test_distro.py b/test/units/module_utils/test_distro.py index ffbc5dd6cfe..37e647e2ae8 100644 --- a/test/units/module_utils/test_distro.py +++ b/test/units/module_utils/test_distro.py @@ -16,6 +16,8 @@ import platform import pytest from ansible.module_utils import distro +from ansible.module_utils.common.sys_info import (get_distribution, get_distribution_version, + get_distribution_codename) # Generic test case with minimal assertions about specific returned values. @@ -42,11 +44,12 @@ class TestDistroCompat(): _platform_supported_dists = platform._supported_dists def test_linux_distribution(self): + distro_linux_dist = (get_distribution(), get_distribution_version(), get_distribution_codename()) + platform_linux_dist = platform.linux_distribution(supported_dists=self._platform_supported_dists) - distro_linux_dist = distro.linux_distribution() assert isinstance(distro_linux_dist, type(platform_linux_dist)), \ - 'linux_distrution() returned type (%s) which is different from platform.linux_distribution type (%s)' % \ + 'linux_distribution() returned type (%s) which is different from platform.linux_distribution type (%s)' % \ (type(distro_linux_dist), type(platform_linux_dist)) # TODO: add the cases where we expect them to differ @@ -55,6 +58,9 @@ class TestDistroCompat(): assert distro_linux_dist[0] == platform_linux_dist[0] assert distro_linux_dist[1] == platform_linux_dist[1] - # FIXME: This fails on at least Fedora - # platform returns the 'id', while distro returns the 'codename' + if platform_linux_dist[0] == 'Fedora' and 20 < int(platform_linux_dist[1]) < 28: + pytest.skip("Fedora versions between 20 and 28 return the variant instead of the code name making this test unreliable") + # Fedora considers the platform_linux behaviour to have been a bug as it's finding the + # variant, not the code name. Fedora wants this to be the empty string. + platform_linux_dist = platform_linux_dist[:2] + ('',) assert distro_linux_dist[2] == platform_linux_dist[2]