Skip to content

Commit

Permalink
Handle the exception in salt/utils/platform.py instead
Browse files Browse the repository at this point in the history
Signed-off-by: Pedro Algarvio <[email protected]>
  • Loading branch information
s0undt3ch authored and Megan Wilhite committed Aug 25, 2023
1 parent 717ae80 commit 56ae8aa
Show file tree
Hide file tree
Showing 3 changed files with 61 additions and 11 deletions.
11 changes: 3 additions & 8 deletions salt/grains/core.py
Original file line number Diff line number Diff line change
Expand Up @@ -2320,14 +2320,9 @@ def _legacy_linux_distribution_data(grains, os_release, lsb_has_error):
log.trace(
"Getting OS name, release, and codename from distro id, version, codename"
)
try:
(osname, osrelease, oscodename) = (
x.strip('"').strip("'") for x in _linux_distribution()
)
except subprocess.CalledProcessError:
# When running under the salt user, a call to `lsb_release` might raise
# permissin errors.
return grains
(osname, osrelease, oscodename) = (
x.strip('"').strip("'") for x in _linux_distribution()
)
# Try to assign these three names based on the lsb info, they tend to
# be more accurate than what python gets from /etc/DISTRO-release.
# It's worth noting that Ubuntu has patched their Python distribution
Expand Down
14 changes: 11 additions & 3 deletions salt/utils/platform.py
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
"""
Functions for identifying which platform a machine is
"""

import contextlib
import multiprocessing
import os
import platform
Expand All @@ -18,8 +18,16 @@ def linux_distribution(full_distribution_name=True):
Simple function to return information about the OS distribution (id_name, version, codename).
"""
if full_distribution_name:
return distro.name(), distro.version(best=True), distro.codename()
return distro.id(), distro.version(best=True), distro.codename()
distro_name = distro.name()
else:
distro_name = distro.id()
# Empty string fallbacks
distro_version = distro_codename = ""
with contextlib.suppress(subprocess.CalledProcessError):
distro_version = distro.version(best=True)
with contextlib.suppress(subprocess.CalledProcessError):
distro_codename = distro.codename()
return distro_name, distro_version, distro_codename


@real_memoize
Expand Down
47 changes: 47 additions & 0 deletions tests/pytests/unit/utils/test_platform.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,47 @@
import subprocess

import salt.utils.platform
from tests.support.mock import patch


def test_linux_distribution():
"""
Test that when `distro` fails with a `subprocess.CalledProcessError` salt
returns empty strings as default values.
"""
distro_name = "Salt"
distro_version = "1"
distro_codename = "Awesome"
with patch("distro.name", return_value=distro_name):
with patch("distro.version", return_value=distro_version), patch(
"distro.codename", return_value=distro_codename
):
assert salt.utils.platform.linux_distribution() == (
distro_name,
distro_version,
distro_codename,
)

distro_version = ""
with patch(
"distro.version",
side_effect=subprocess.CalledProcessError(returncode=1, cmd=["foo"]),
), patch("distro.codename", return_value=distro_codename):
assert salt.utils.platform.linux_distribution() == (
distro_name,
distro_version,
distro_codename,
)
distro_codename = ""
with patch(
"distro.version",
side_effect=subprocess.CalledProcessError(returncode=1, cmd=["foo"]),
), patch(
"distro.codename",
side_effect=subprocess.CalledProcessError(returncode=1, cmd=["foo"]),
):
assert salt.utils.platform.linux_distribution() == (
distro_name,
distro_version,
distro_codename,
)

0 comments on commit 56ae8aa

Please sign in to comment.