Merge pull request #37909 from thusoy/invalid-timezone

Throw error when setting invalid timezone

salt/modules/timezone.py

@@ -5,6 +5,7 @@ Module for managing timezone on POSIX-like systems.
 from __future__ import absolute_import
 
 # Import python libs
+import filecmp
 import os
 import errno
 import logging
@@ -280,34 +281,28 @@ def zone_compare(timezone):
     if 'Solaris' in __grains__['os_family']:
         return timezone == get_zone()
 
-    curtzstring = get_zone()
-    if curtzstring != timezone:
-        return False
-
-    tzfile = '/etc/localtime'
-    zonepath = '/usr/share/zoneinfo/{0}'.format(timezone)
-
-    if not os.path.exists(tzfile):
-        return 'Error: {0} does not exist.'.format(tzfile)
-
-    hash_type = __opts__.get('hash_type', 'md5')
-
+    tzfile = _get_etc_localtime_path()
+    zonepath = _get_zone_file(timezone)
     try:
-        usrzone = salt.utils.get_hash(zonepath, hash_type)
-    except IOError as exc:
-        raise SaltInvocationError('Invalid timezone \'{0}\''.format(timezone))
+        return filecmp.cmp(tzfile, zonepath, shallow=False)
+    except OSError as exc:
+        problematic_file = exc.filename
+        if problematic_file == zonepath:
+            raise SaltInvocationError(
+                'Can\'t find a local timezone "{0}"'.format(timezone))
+        elif problematic_file == tzfile:
+            raise CommandExecutionError(
+                'Failed to read {0} to determine current timezone: {1}'
+                .format(tzfile, exc.strerror))
+        raise
 
-    try:
-        etczone = salt.utils.get_hash(tzfile, hash_type)
-    except IOError as exc:
-        raise CommandExecutionError(
-            'Problem reading timezone file {0}: {1}'
-            .format(tzfile, exc.strerror)
-        )
 
-    if usrzone == etczone:
-        return True
-    return False
+def _get_etc_localtime_path():
+    return '/etc/localtime'
+
+
+def _get_zone_file(timezone):
+    return '/usr/share/zoneinfo/{0}'.format(timezone)
 
 
 def get_hwclock():

tests/unit/modules/timezone_test.py

@@ -0,0 +1,87 @@
+# -*- coding: utf-8 -*-
+
+# Import Python Libs
+from __future__ import absolute_import
+from tempfile import NamedTemporaryFile
+import os
+
+# Import Salt Testing Libs
+from salt.exceptions import CommandExecutionError, SaltInvocationError
+from salttesting import TestCase, skipIf
+from salttesting.helpers import ensure_in_syspath
+from salttesting.mock import (
+    patch,
+    NO_MOCK,
+    NO_MOCK_REASON
+)
+
+ensure_in_syspath('../../')
+
+# Import Salt Libs
+from salt.modules import timezone
+
+# Globals
+timezone.__salt__ = {}
+timezone.__opts__ = {}
+timezone.__grains__ = {}
+
+GET_ZONE_FILE = 'salt.modules.timezone._get_zone_file'
+GET_ETC_LOCALTIME_PATH = 'salt.modules.timezone._get_etc_localtime_path'
+
+
+@skipIf(NO_MOCK, NO_MOCK_REASON)
+@patch.dict(timezone.__grains__, {'os_family': 'Ubuntu'})
+class TimezoneTestCase(TestCase):
+
+    def setUp(self):
+        self.tempfiles = []
+
+    def tearDown(self):
+        for tempfile in self.tempfiles:
+            try:
+                os.remove(tempfile.name)
+            except OSError:
+                pass
+
+    def test_zone_compare_equal(self):
+        etc_localtime = self.create_tempfile_with_contents('a')
+        zone_path = self.create_tempfile_with_contents('a')
+
+        with patch(GET_ZONE_FILE, lambda p: zone_path.name):
+            with patch(GET_ETC_LOCALTIME_PATH, lambda: etc_localtime.name):
+
+                self.assertTrue(timezone.zone_compare('foo'))
+
+    def test_zone_compare_nonexistent(self):
+        etc_localtime = self.create_tempfile_with_contents('a')
+
+        with patch(GET_ZONE_FILE, lambda p: '/foopath/nonexistent'):
+            with patch(GET_ETC_LOCALTIME_PATH, lambda: etc_localtime.name):
+
+                self.assertRaises(SaltInvocationError, timezone.zone_compare, 'foo')
+
+    def test_zone_compare_unequal(self):
+        etc_localtime = self.create_tempfile_with_contents('a')
+        zone_path = self.create_tempfile_with_contents('b')
+
+        with patch(GET_ZONE_FILE, lambda p: zone_path.name):
+            with patch(GET_ETC_LOCALTIME_PATH, lambda: etc_localtime.name):
+
+                self.assertFalse(timezone.zone_compare('foo'))
+
+    def test_missing_localtime(self):
+        with patch(GET_ZONE_FILE, lambda p: '/nonexisting'):
+            with patch(GET_ETC_LOCALTIME_PATH, lambda: '/also-missing'):
+                self.assertRaises(CommandExecutionError, timezone.zone_compare, 'foo')
+
+    def create_tempfile_with_contents(self, contents):
+        temp = NamedTemporaryFile(delete=False)
+        temp.write(contents)
+        temp.close()
+        self.tempfiles.append(temp)
+        return temp
+
+
+if __name__ == '__main__':
+    from integration import run_tests
+    run_tests(TimezoneTestCase, needs_daemon=False)