Skip to content

Commit

Permalink
Merge pull request #49673 from sheagcraig/fix_plistlib_deprecation_no…
Browse files Browse the repository at this point in the history 
…tice

Use py3-preferred plistlib API to load service plists.
  • Loading branch information
Nicole Thomas authored Sep 30, 2018
2 parents 424062f + 913e720 commit 3dc57a6
Show file tree
Hide file tree
Showing 2 changed files with 71 additions and 31 deletions.
6 changes: 5 additions & 1 deletion salt/utils/mac_utils.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -350,7 +350,11 @@ def _available_services(refresh=False):
try:
# This assumes most of the plist files
# will be already in XML format
plist = plistlib.readPlist(true_path)
if six.PY2:
plist = plistlib.readPlist(true_path)
else:
with salt.utils.files.fopen(true_path, 'rb') as plist_handle:
plist = plistlib.load(plist_handle)

except Exception:
# If plistlib is unable to read the file we'll need to use
Expand Down
96 changes: 66 additions & 30 deletions tests/unit/utils/test_mac_utils.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@

# Import Salt Testing Libs
from tests.support.unit import TestCase, skipIf
from tests.support.mock import MagicMock, patch, NO_MOCK, NO_MOCK_REASON, call
from tests.support.mock import MagicMock, patch, NO_MOCK, NO_MOCK_REASON, call, mock_open
from tests.support.mixins import LoaderModuleMockMixin

# Import Salt libs
Expand Down Expand Up @@ -215,18 +215,22 @@ def test_launchctl_error(self):

@patch('salt.utils.path.os_walk')
@patch('os.path.exists')
@patch('plistlib.readPlist')
@patch('plistlib.readPlist' if six.PY2 else 'plistlib.load')
def test_available_services(self, mock_read_plist, mock_exists, mock_os_walk):
'''
test available_services
'''
mock_os_walk.side_effect = [
[('/Library/LaunchAgents', [], ['com.apple.lla1.plist', 'com.apple.lla2.plist'])],
[('/Library/LaunchDaemons', [], ['com.apple.lld1.plist', 'com.apple.lld2.plist'])],
[('/System/Library/LaunchAgents', [], ['com.apple.slla1.plist', 'com.apple.slla2.plist'])],
[('/System/Library/LaunchDaemons', [], ['com.apple.slld1.plist', 'com.apple.slld2.plist'])],
]

def walk_side_effect(*args, **kwargs):
path = args[0]
results = {
'/Library/LaunchAgents': ['com.apple.lla1.plist', 'com.apple.lla2.plist'],
'/Library/LaunchDaemons': ['com.apple.lld1.plist', 'com.apple.lld2.plist'],
'/System/Library/LaunchAgents': ['com.apple.slla1.plist', 'com.apple.slla2.plist'],
'/System/Library/LaunchDaemons': ['com.apple.slld1.plist', 'com.apple.slld2.plist']}
files = results.get(path, [])
return [(path, [], files)]

mock_os_walk.side_effect = walk_side_effect
mock_read_plist.side_effect = [
MagicMock(Label='com.apple.lla1'),
MagicMock(Label='com.apple.lla2'),
Expand All @@ -239,7 +243,16 @@ def test_available_services(self, mock_read_plist, mock_exists, mock_os_walk):
]

mock_exists.return_value = True
ret = mac_utils._available_services()

if six.PY3:
# Py3's plistlib.load does not handle opening and closing a
# file, unlike py2's plistlib.readPlist. Therefore, we have
# to patch open for py3 since we're using it in addition
# to the plistlib.load call.
with patch('salt.utils.files.fopen', mock_open()):
ret = mac_utils._available_services()
else:
ret = mac_utils._available_services()

# Make sure it's a dict with 8 items
self.assertTrue(isinstance(ret, dict))
Expand All @@ -265,17 +278,22 @@ def test_available_services(self, mock_read_plist, mock_exists, mock_os_walk):

@patch('salt.utils.path.os_walk')
@patch('os.path.exists')
@patch('plistlib.readPlist')
@patch('plistlib.readPlist' if six.PY2 else 'plistlib.load')
def test_available_services_broken_symlink(self, mock_read_plist, mock_exists, mock_os_walk):
'''
test available_services
'''
mock_os_walk.side_effect = [
[('/Library/LaunchAgents', [], ['com.apple.lla1.plist', 'com.apple.lla2.plist'])],
[('/Library/LaunchDaemons', [], ['com.apple.lld1.plist', 'com.apple.lld2.plist'])],
[('/System/Library/LaunchAgents', [], ['com.apple.slla1.plist', 'com.apple.slla2.plist'])],
[('/System/Library/LaunchDaemons', [], ['com.apple.slld1.plist', 'com.apple.slld2.plist'])],
]
def walk_side_effect(*args, **kwargs):
path = args[0]
results = {
'/Library/LaunchAgents': ['com.apple.lla1.plist', 'com.apple.lla2.plist'],
'/Library/LaunchDaemons': ['com.apple.lld1.plist', 'com.apple.lld2.plist'],
'/System/Library/LaunchAgents': ['com.apple.slla1.plist', 'com.apple.slla2.plist'],
'/System/Library/LaunchDaemons': ['com.apple.slld1.plist', 'com.apple.slld2.plist']}
files = results.get(path, [])
return [(path, [], files)]

mock_os_walk.side_effect = walk_side_effect

mock_read_plist.side_effect = [
MagicMock(Label='com.apple.lla1'),
Expand All @@ -287,7 +305,15 @@ def test_available_services_broken_symlink(self, mock_read_plist, mock_exists, m
]

mock_exists.side_effect = [True, True, True, True, False, False, True, True]
ret = mac_utils._available_services()
if six.PY3:
# Py3's plistlib.load does not handle opening and closing a
# file, unlike py2's plistlib.readPlist. Therefore, we have
# to patch open for py3 since we're using it in addition
# to the plistlib.load call.
with patch('salt.utils.files.fopen', mock_open()):
ret = mac_utils._available_services()
else:
ret = mac_utils._available_services()

# Make sure it's a dict with 6 items
self.assertTrue(isinstance(ret, dict))
Expand Down Expand Up @@ -325,12 +351,17 @@ def test_available_services_non_xml(self,
'''
test available_services
'''
mock_os_walk.side_effect = [
[('/Library/LaunchAgents', [], ['com.apple.lla1.plist', 'com.apple.lla2.plist'])],
[('/Library/LaunchDaemons', [], ['com.apple.lld1.plist', 'com.apple.lld2.plist'])],
[('/System/Library/LaunchAgents', [], ['com.apple.slla1.plist', 'com.apple.slla2.plist'])],
[('/System/Library/LaunchDaemons', [], ['com.apple.slld1.plist', 'com.apple.slld2.plist'])],
]
def walk_side_effect(*args, **kwargs):
path = args[0]
results = {
'/Library/LaunchAgents': ['com.apple.lla1.plist', 'com.apple.lla2.plist'],
'/Library/LaunchDaemons': ['com.apple.lld1.plist', 'com.apple.lld2.plist'],
'/System/Library/LaunchAgents': ['com.apple.slla1.plist', 'com.apple.slla2.plist'],
'/System/Library/LaunchDaemons': ['com.apple.slld1.plist', 'com.apple.slld2.plist']}
files = results.get(path, [])
return [(path, [], files)]

mock_os_walk.side_effect = walk_side_effect
attrs = {'cmd.run': MagicMock(return_value='<some xml>')}

def getitem(name):
Expand Down Expand Up @@ -410,12 +441,17 @@ def test_available_services_non_xml_malformed_plist(self,
'''
test available_services
'''
mock_os_walk.side_effect = [
[('/Library/LaunchAgents', [], ['com.apple.lla1.plist', 'com.apple.lla2.plist'])],
[('/Library/LaunchDaemons', [], ['com.apple.lld1.plist', 'com.apple.lld2.plist'])],
[('/System/Library/LaunchAgents', [], ['com.apple.slla1.plist', 'com.apple.slla2.plist'])],
[('/System/Library/LaunchDaemons', [], ['com.apple.slld1.plist', 'com.apple.slld2.plist'])],
]
def walk_side_effect(*args, **kwargs):
path = args[0]
results = {
'/Library/LaunchAgents': ['com.apple.lla1.plist', 'com.apple.lla2.plist'],
'/Library/LaunchDaemons': ['com.apple.lld1.plist', 'com.apple.lld2.plist'],
'/System/Library/LaunchAgents': ['com.apple.slla1.plist', 'com.apple.slla2.plist'],
'/System/Library/LaunchDaemons': ['com.apple.slld1.plist', 'com.apple.slld2.plist']}
files = results.get(path, [])
return [(path, [], files)]

mock_os_walk.side_effect = walk_side_effect
attrs = {'cmd.run': MagicMock(return_value='<some xml>')}

def getitem(name):
Expand Down

0 comments on commit 3dc57a6

Please sign in to comment.