Cron logic unification

saltstack · Mar 1, 2019 · 305f5cf · 305f5cf
1 parent ef5ae41
commit 305f5cf
Show file tree

Hide file tree

Showing 2 changed files with 59 additions and 41 deletions.
diff --git a/salt/modules/cron.py b/salt/modules/cron.py
@@ -208,13 +208,24 @@ def write_cron_file(user, path):
 
         Some OS' do not support specifying user via the `crontab` command i.e. (Solaris, AIX)
     '''
-    if not _check_instance_uid_match(user) or __grains__.get('os_family') in ('Solaris', 'AIX'):
+    # Some OS' do not support specifying user via the `crontab` command
+    if __grains__.get('os_family') in ('Solaris', 'AIX'):
         return __salt__['cmd.retcode'](_get_cron_cmdstr(path),
                                        runas=user,
                                        python_shell=False) == 0
-    else:
+    # If Salt is running from same user as requested in cron module we don't need any user switch
+    elif _check_instance_uid_match(user):
+        return __salt__['cmd.retcode'](_get_cron_cmdstr(path),
+                                       python_shell=False) == 0
+    # If Salt is running from root user it could modify any user's crontab
+    elif _check_instance_uid_match('root'):
         return __salt__['cmd.retcode'](_get_cron_cmdstr(path, user),
                                        python_shell=False) == 0
+    # Edge cases here, let's try do a runas
+    else:
+        return __salt__['cmd.retcode'](_get_cron_cmdstr(path),
+                                       runas=user,
+                                       python_shell=False) == 0
 
 
 def write_cron_file_verbose(user, path):
@@ -233,13 +244,24 @@ def write_cron_file_verbose(user, path):
 
         Some OS' do not support specifying user via the `crontab` command i.e. (Solaris, AIX)
     '''
-    if not _check_instance_uid_match(user) or __grains__.get('os_family') in ('Solaris', 'AIX'):
+    # Some OS' do not support specifying user via the `crontab` command
+    if __grains__.get('os_family') in ('Solaris', 'AIX'):
         return __salt__['cmd.run_all'](_get_cron_cmdstr(path),
                                        runas=user,
                                        python_shell=False)
-    else:
+    # If Salt is running from same user as requested in cron module we don't need any user switch
+    elif _check_instance_uid_match(user):
+        return __salt__['cmd.run_all'](_get_cron_cmdstr(path),
+                                       python_shell=False)
+    # If Salt is running from root user it could modify any user's crontab
+    elif _check_instance_uid_match('root'):
         return __salt__['cmd.run_all'](_get_cron_cmdstr(path, user),
                                        python_shell=False)
+    # Edge cases here, let's try do a runas
+    else:
+        return __salt__['cmd.run_all'](_get_cron_cmdstr(path),
+                                       runas=user,
+                                       python_shell=False)
 
 
 def _write_cron_lines(user, lines):
@@ -248,7 +270,7 @@ def _write_cron_lines(user, lines):
     '''
     lines = [salt.utils.stringutils.to_str(_l) for _l in lines]
     path = salt.utils.files.mkstemp()
-    if not _check_instance_uid_match(user) or __grains__.get('os_family') in ('Solaris', 'AIX'):
+    if _check_instance_uid_match('root') or __grains__.get('os_family') in ('Solaris', 'AIX'):
         # In some cases crontab command should be executed as user rather than root
         with salt.utils.files.fpopen(path, 'w+', uid=__salt__['file.user_to_uid'](user), mode=0o600) as fp_:
             fp_.writelines(lines)
@@ -284,6 +306,7 @@ def raw_cron(user):
 
         salt '*' cron.raw_cron root
     '''
+    # Some OS' do not support specifying user via the `crontab` command
     if __grains__.get('os_family') in ('Solaris', 'AIX'):
         cmd = 'crontab -l'
         # Preserve line endings
@@ -292,20 +315,31 @@ def raw_cron(user):
                                            ignore_retcode=True,
                                            rstrip=False,
                                            python_shell=False)).splitlines(True)
+    # If Salt is running from same user as requested in cron module we don't need any user switch
     elif _check_instance_uid_match(user):
         cmd = 'crontab -l'
         # Preserve line endings
         lines = sdecode(__salt__['cmd.run_stdout'](cmd,
                                            ignore_retcode=True,
                                            rstrip=False,
                                            python_shell=False)).splitlines(True)
-    else:
+    # If Salt is running from root user it could modify any user's crontab
+    elif _check_instance_uid_match('root'):
         cmd = 'crontab -u {0} -l'.format(user)
         # Preserve line endings
         lines = sdecode(__salt__['cmd.run_stdout'](cmd,
                                            ignore_retcode=True,
                                            rstrip=False,
                                            python_shell=False)).splitlines(True)
+    # Edge cases here, let's try do a runas
+    else:
+        cmd = 'crontab -l'
+        # Preserve line endings
+        lines = sdecode(__salt__['cmd.run_stdout'](cmd,
+                                           runas=user,
+                                           ignore_retcode=True,
+                                           rstrip=False,
+                                           python_shell=False)).splitlines(True)
 
     if len(lines) != 0 and lines[0].startswith('# DO NOT EDIT THIS FILE - edit the master and reinstall.'):
         del lines[0:3]

diff --git a/tests/unit/modules/test_cron.py b/tests/unit/modules/test_cron.py
@@ -614,39 +614,37 @@ def test__load_tab(self):
     def test_write_cron_file_root_rh(self):
         '''
         Assert that write_cron_file() is called with the correct cron command and user: RedHat
-          - If instance running uid matches crontab user uid, runas STUB_USER without -u flag.
+          - If instance running uid matches crontab user uid, run without -u flag.
         '''
         with patch.dict(cron.__grains__, {'os_family': 'RedHat'}), \
                 patch.dict(cron.__salt__, {'cmd.retcode': MagicMock()}), \
                     patch('salt.modules.cron._check_instance_uid_match',
                           new=MagicMock(return_value=True)):
             cron.write_cron_file(STUB_USER, STUB_PATH)
             cron.__salt__['cmd.retcode'].assert_called_with("crontab /tmp",
-                                                            runas=STUB_USER,
                                                             python_shell=False)
 
     def test_write_cron_file_foo_rh(self):
         '''
         Assert that write_cron_file() is called with the correct cron command and user: RedHat
-          - If instance running with uid that doesn't match crontab user uid, run with -u flag
+          - If instance running with uid that doesn't match crontab user uid, runas foo
         '''
         with patch.dict(cron.__grains__, {'os_family': 'RedHat'}), \
                 patch.dict(cron.__salt__, {'cmd.retcode': MagicMock()}), \
                     patch('salt.modules.cron._check_instance_uid_match',
                           MagicMock(return_value=False)):
             cron.write_cron_file('foo', STUB_PATH)
-            cron.__salt__['cmd.retcode'].assert_called_with("crontab -u foo /tmp",
+            cron.__salt__['cmd.retcode'].assert_called_with("crontab /tmp",
+                                                            runas='foo',
                                                             python_shell=False)
 
     def test_write_cron_file_root_sol(self):
         '''
         Assert that write_cron_file() is called with the correct cron command and user: Solaris
           - Solaris should always run without a -u flag
         '''
-        with patch.dict(cron.__grains__, {'os_family': 'RedHat'}), \
-                patch.dict(cron.__salt__, {'cmd.retcode': MagicMock()}), \
-                    patch('salt.modules.cron._check_instance_uid_match',
-                          MagicMock(return_value=True)):
+        with patch.dict(cron.__grains__, {'os_family': 'Solaris'}), \
+                patch.dict(cron.__salt__, {'cmd.retcode': MagicMock()}):
             cron.write_cron_file(STUB_USER, STUB_PATH)
             cron.__salt__['cmd.retcode'].assert_called_with("crontab /tmp",
                                                             runas=STUB_USER,
@@ -658,9 +656,7 @@ def test_write_cron_file_foo_sol(self):
           - Solaris should always run without a -u flag
         '''
         with patch.dict(cron.__grains__, {'os_family': 'Solaris'}), \
-                patch.dict(cron.__salt__, {'cmd.retcode': MagicMock()}), \
-                    patch('salt.modules.cron._check_instance_uid_match',
-                          MagicMock(return_value=False)):
+                patch.dict(cron.__salt__, {'cmd.retcode': MagicMock()}):
             cron.write_cron_file('foo', STUB_PATH)
             cron.__salt__['cmd.retcode'].assert_called_with("crontab /tmp",
                                                             runas='foo',
@@ -672,9 +668,7 @@ def test_write_cron_file_root_aix(self):
           - AIX should always run without a -u flag
         '''
         with patch.dict(cron.__grains__, {'os_family': 'AIX'}), \
-                patch.dict(cron.__salt__, {'cmd.retcode': MagicMock()}), \
-                    patch('salt.modules.cron._check_instance_uid_match',
-                          MagicMock(return_value=True)):
+                patch.dict(cron.__salt__, {'cmd.retcode': MagicMock()}):
             cron.write_cron_file(STUB_USER, STUB_PATH)
             cron.__salt__['cmd.retcode'].assert_called_with("crontab /tmp",
                                                             runas=STUB_USER,
@@ -686,9 +680,7 @@ def test_write_cron_file_foo_aix(self):
           - AIX should always run without a -u flag
         '''
         with patch.dict(cron.__grains__, {'os_family': 'AIX'}), \
-                patch.dict(cron.__salt__, {'cmd.retcode': MagicMock()}), \
-                    patch('salt.modules.cron._check_instance_uid_match',
-                          MagicMock(return_value=False)):
+                patch.dict(cron.__salt__, {'cmd.retcode': MagicMock()}):
             cron.write_cron_file('foo', STUB_PATH)
             cron.__salt__['cmd.retcode'].assert_called_with("crontab /tmp",
                                                             runas='foo',
@@ -697,28 +689,28 @@ def test_write_cron_file_foo_aix(self):
     def test_write_cr_file_v_root_rh(self):
         '''
         Assert that write_cron_file_verbose() is called with the correct cron command and user: RedHat
-          - If instance running uid matches crontab user uid, runas STUB_USER without -u flag.
+          - If instance running uid matches crontab user uid, run without -u flag.
         '''
         with patch.dict(cron.__grains__, {'os_family': 'Redhat'}), \
                 patch.dict(cron.__salt__, {'cmd.run_all': MagicMock()}), \
                     patch('salt.modules.cron._check_instance_uid_match',
                           MagicMock(return_value=True)):
             cron.write_cron_file_verbose(STUB_USER, STUB_PATH)
             cron.__salt__['cmd.run_all'].assert_called_with("crontab /tmp",
-                                                            runas=STUB_USER,
                                                             python_shell=False)
 
     def test_write_cr_file_v_foo_rh(self):
         '''
         Assert that write_cron_file_verbose() is called with the correct cron command and user: RedHat
-          - If instance running with uid that doesn't match crontab user uid, run with -u flag
+          - If instance running with uid that doesn't match crontab user uid, runas 'foo'
         '''
         with patch.dict(cron.__grains__, {'os_family': 'Redhat'}), \
                 patch.dict(cron.__salt__, {'cmd.run_all': MagicMock()}), \
                     patch('salt.modules.cron._check_instance_uid_match',
                             MagicMock(return_value=False)):
             cron.write_cron_file_verbose('foo', STUB_PATH)
-            cron.__salt__['cmd.run_all'].assert_called_with("crontab -u foo /tmp",
+            cron.__salt__['cmd.run_all'].assert_called_with("crontab /tmp",
+                                                            runas='foo',
                                                             python_shell=False)
 
     def test_write_cr_file_v_root_sol(self):
@@ -727,9 +719,7 @@ def test_write_cr_file_v_root_sol(self):
           - Solaris should always run without a -u flag
         '''
         with patch.dict(cron.__grains__, {'os_family': 'Solaris'}), \
-                patch.dict(cron.__salt__, {'cmd.run_all': MagicMock()}), \
-                    patch('salt.modules.cron._check_instance_uid_match',
-                          MagicMock(return_value=True)):
+                patch.dict(cron.__salt__, {'cmd.run_all': MagicMock()}):
             cron.write_cron_file_verbose(STUB_USER, STUB_PATH)
             cron.__salt__['cmd.run_all'].assert_called_with("crontab /tmp",
                                                             runas=STUB_USER,
@@ -741,9 +731,7 @@ def test_write_cr_file_v_foo_sol(self):
           - Solaris should always run without a -u flag
         '''
         with patch.dict(cron.__grains__, {'os_family': 'Solaris'}), \
-                patch.dict(cron.__salt__, {'cmd.run_all': MagicMock()}), \
-                    patch('salt.modules.cron._check_instance_uid_match',
-                          MagicMock(return_value=False)):
+                patch.dict(cron.__salt__, {'cmd.run_all': MagicMock()}):
             cron.write_cron_file_verbose('foo', STUB_PATH)
             cron.__salt__['cmd.run_all'].assert_called_with("crontab /tmp",
                                                             runas='foo',
@@ -755,9 +743,7 @@ def test_write_cr_file_v_root_aix(self):
           - AIX should always run without a -u flag
         '''
         with patch.dict(cron.__grains__, {'os_family': 'AIX'}), \
-                patch.dict(cron.__salt__, {'cmd.run_all': MagicMock()}), \
-                    patch('salt.modules.cron._check_instance_uid_match',
-                          MagicMock(return_value=True)):
+                patch.dict(cron.__salt__, {'cmd.run_all': MagicMock()}):
             cron.write_cron_file_verbose(STUB_USER, STUB_PATH)
             cron.__salt__['cmd.run_all'].assert_called_with("crontab /tmp",
                                                             runas=STUB_USER,
@@ -769,9 +755,7 @@ def test_write_cr_file_v_foo_aix(self):
           - AIX should always run without a -u flag
         '''
         with patch.dict(cron.__grains__, {'os_family': 'AIX'}), \
-                patch.dict(cron.__salt__, {'cmd.run_all': MagicMock()}), \
-                    patch('salt.modules.cron._check_instance_uid_match',
-                          MagicMock(return_value=False)):
+                patch.dict(cron.__salt__, {'cmd.run_all': MagicMock()}):
             cron.write_cron_file_verbose('foo', STUB_PATH)
             cron.__salt__['cmd.run_all'].assert_called_with("crontab /tmp",
                                                             runas='foo',
@@ -788,7 +772,6 @@ def test_raw_cron_root_redhat(self):
                           MagicMock(return_value=True)):
             cron.raw_cron(STUB_USER)
             cron.__salt__['cmd.run_stdout'].assert_called_with("crontab -l",
-                                                               runas=STUB_USER,
                                                                ignore_retcode=True,
                                                                rstrip=False,
                                                                python_shell=False)
@@ -803,7 +786,8 @@ def test_raw_cron_foo_redhat(self):
                     patch('salt.modules.cron._check_instance_uid_match',
                           MagicMock(return_value=False)):
             cron.raw_cron(STUB_USER)
-            cron.__salt__['cmd.run_stdout'].assert_called_with("crontab -u root -l",
+            cron.__salt__['cmd.run_stdout'].assert_called_with("crontab -l",
+                                                               runas=STUB_USER,
                                                                ignore_retcode=True,
                                                                rstrip=False,
                                                                python_shell=False)