Bug fixes and enhancements.

Bugs:
(1) no longer allow empty jobs
(2) strip the header added by crontab package used in openSuSE and SuSE
(3) try not to leak temp files when things go wrong
(4) issue returning job names under certain conditions

Enhancements:
(1) Allow all special times not just reboot.
(2) Fail earlier by performing more input validation
(3) Add feature to allow removing cron file under /etc/cron.d

ToDo:
(1) Validate times (minute, hour, ...)
(2) Strip white space from fields such as name and job such that name=foo equals name=' foo'.
(3) More testing

system/cron

@@ -3,6 +3,7 @@
 #
 # (c) 2012, Dane Summers <dsummers@pinedesk.biz>
 # (c) 2013, Mike Grozak  <mike.grozak@gmail.com>
+# (c) 2013, Patrick Callahan <pmc@patrickcallahan.com>
 #
 # This file is part of Ansible
 #
@@ -18,17 +19,20 @@
 #
 # You should have received a copy of the GNU General Public License
 # along with Ansible.  If not, see <http://www.gnu.org/licenses/>.
-
+#
 # Cron Plugin: The goal of this plugin is to provide an indempotent method for
 # setting up cron jobs on a host. The script will play well with other manually
 # entered crons. Each cron job entered will be preceded with a comment
 # describing the job so that it can be found later, which is required to be
 # present in order for this plugin to find/modify the job.
+#
+# This module is based on python-crontab by Martin Owens.
+#
 
 DOCUMENTATION = """
 ---
 module: cron
-short_description: Manage crontab entries.
+short_description: Manage cron.d and crontab entries.
 description:
   - Use this module to manage crontab entries. This module allows you to create named
     crontab entries, update, or delete them.
@@ -40,80 +44,76 @@ options:
   name:
     description:
       - Description of a crontab entry.
-    required: true
+    required: false
-    default:
+    default: null
-    aliases: []
   user:
     description:
       - The specific user who's crontab should be modified.
     required: false
     default: root
-    aliases: []
   job:
     description:
-      - The command to execute.
+      - The command to execute. Required if state=present.
-      - Required if state=present.
     required: false
-    default: 
+    default: null
-    aliases: []
   state:
     description:
       - Whether to ensure the job is present or absent.
     required: false
     default: present
-    aliases: []
+    choices: [ "present", "absent" ]
   cron_file:
     description:
-      - If specified, uses this file in cron.d versus in the main crontab
+      - If specified, uses this file in cron.d instead of an individual user's crontab.
     required: false
-    default:
+    default: null
-    aliases: []
   backup:
     description:
-      - If set, then create a backup of the crontab before it is modified.
+      - If set, create a backup of the crontab before it is modified.
-      - The location of the backup is returned in the C(backup) variable by this module.
+        The location of the backup is returned in the C(backup) variable by this module.
     required: false
     default: false
-    aliases: []
   minute:
     description:
       - Minute when the job should run ( 0-59, *, */2, etc )
     required: false
     default: "*"
-    aliases: []
   hour:
     description:
       - Hour when the job should run ( 0-23, *, */2, etc )
     required: false
     default: "*"
-    aliases: []
   day:
     description:
       - Day of the month the job should run ( 1-31, *, */2, etc )
     required: false
     default: "*"
-    aliases: []
+    aliases: [ "dom" ]
   month:
     description:
       - Month of the year the job should run ( 1-12, *, */2, etc )
     required: false
     default: "*"
-    aliases: []
   weekday:
     description:
-      - Day of the week that the job should run ( 0-7 for Sunday - Saturday, or mon, tue, * etc )
+      - Day of the week that the job should run ( 0-7 for Sunday - Saturday, *, etc )
     required: false
     default: "*"
-    aliases: []
+    aliases: [ "dow" ]
-
   reboot:
     description:
-      - If the job should be run at reboot, will ignore minute, hour, day, and month settings in favour of C(@reboot)
+      - If the job should be run at reboot. This option is deprecated. Users should use special_time.
     version_added: "1.0"
     required: false
     default: "no"
     choices: [ "yes", "no" ]
-    aliases: []
+  special_time:
+    description:
+      - Special time specification nickname.
+    version_added: "1.3"
+    required: false
+    default: null
+    choices: [ "reboot", "yearly", "annually", "monthly", "weekly", "daily", "hourly" ]
 requirements:
   - cron
 author: Dane Summers
@@ -127,85 +127,192 @@ EXAMPLES = '''
 
 # Ensure an old job is no longer present. Removes any job that is prefixed
 # by "#Ansible: an old job" from the crontab
-- cron: name="an old job" cron job="/some/dir/job.sh" state=absent
+- cron: name="an old job" state=absent
 
 # Creates an entry like "@reboot /some/job.sh"
-- cron: name="a job for reboot" reboot=yes job="/some/job.sh"
+- cron: name="a job for reboot" special_time=reboot job="/some/job.sh"
 
+# Creates a cron file under /etc/cron.d
 - cron: name="yum autoupdate" weekday="2" minute=0 hour=12
         user="root" job="YUMINTERACTIVE=0 /usr/sbin/yum-autoupdate"
         cron_file=ansible_yum-autoupdate
+
+# Removes a cron file from under /etc/cron.d
+- cron: cron_file=ansible_yum-autoupdate state=absent
 '''
 
+import os
 import re
 import tempfile
-import os
 
-def get_jobs_file(module, user, tmpfile, cron_file):
+CRONCMD = "/usr/bin/crontab"
+
+class CronTabError(Exception):
+    pass
+
+class CronTab(object):
+    """
+        CronTab object to write time based crontab file
+
+        user      - the user of the crontab (defaults to root)
+        cron_file - a cron file under /etc/cron.d
+    """
+    def __init__(self, module, user=None, cron_file=None):
+        self.module    = module
+        self.user      = user
+        self.root      = (os.getuid() == 0)
+        self.lines     = None
+        self.ansible   = "#Ansible: "
+
+        # select whether we dump additional debug info through syslog
+        self.syslogging = False
+
         if cron_file:
-        cmd = "cp -fp /etc/cron.d/%s %s" % (cron_file, tmpfile)
+            self.cron_file = '/etc/cron.d/%s' % cron_file
+        else:
+            self.cron_file = None
+
+        self.read()
+
+    def read(self):
+        # Read in the crontab from the system
+        self.lines = []
+        if self.cron_file:
+            # read the cronfile
+            try:
+                f = open(self.cron_file, 'r')
+                self.lines = f.read().splitlines()
+                f.close()
+            except IOError as e:
+                # cron file does not exist
+                return
+            except:
+                raise CronTabError("Unexpected error:", sys.exc_info()[0])
         else:
-        cmd = "crontab -l %s > %s" % (user,tmpfile)
+            (rc, out, err) = self.module.run_command(self._read_user_execute())
 
-    return module.run_command(cmd)
+            if rc != 0 and rc != 1: # 1 can mean that there are no jobs.
+                raise CronTabError("Unable to read crontab")
 
-def install_jobs(module, user, tmpfile, cron_file):
+            lines = out.splitlines()
-    if cron_file:
+            count = 0
-        cron_file = '/etc/cron.d/%s' % cron_file
+            for l in lines:
-        module.atomic_move(tmpfile, cron_file)
+                if count > 2 or (not re.match( r'# DO NOT EDIT THIS FILE - edit the master and reinstall.', l) and
+                                 not re.match( r'# \(/tmp/.*installed on.*\)', l) and
+                                 not re.match( r'# \(.*version.*\)', l)):
+                    self.lines.append(l)
+                count += 1
+
+    def log_message(self, message):
+        if self.syslogging:
+            syslog.syslog(syslog.LOG_NOTICE, 'ansible: "%s"' % message)
+
+    def is_empty(self):
+        if len(self.lines) == 0:
+            return True
         else:
-        cmd = "crontab %s %s" % (user, tmpfile)
+            return False
-        return module.run_command(cmd)
+
+    def write(self, backup_file=None):
+        """
+        Write the crontab to the system. Saves all information.
+        """
+        if backup_file:
+            fileh = open(backup_file, 'w')
+        elif self.cron_file:
+            fileh = open(self.cron_file, 'w')
+        else:
+            filed, path = tempfile.mkstemp(prefix='crontab')
+            fileh = os.fdopen(filed, 'w')
+
+        fileh.write(self.render())
+        fileh.close()
+
+        # return if making a backup
+        if backup_file:
+            return
+
+        # Add the entire crontab back to the user crontab
+        if not self.cron_file:
+            # os.system(self._write_execute(path))
+            (rc, out, err) = self.module.run_command(self._write_execute(path))
+            os.unlink(path)
 
-def get_jobs(tmpfile):
+            if rc != 0:
-    lines = open(tmpfile).read().splitlines()
+                self.module.fail_json(msg=err)
+
+    def add_job(self, name, job):
+        # Add the comment
+        self.lines.append("%s%s" % (self.ansible, name))
+
+        # Add the job
+        self.lines.append("%s" % (job))
+
+    def update_job(self, name, job):
+        return self._update_job(name, job, self.do_add_job)
+
+    def do_add_job(self, lines, comment, job):
+        lines.append(comment)
+
+        lines.append("%s" % (job))
+
+    def remove_job(self, name):
+        return self._update_job(name, "", self.do_remove_job)
+
+    def do_remove_job(self, lines, comment, job):
+        return None
+
+    def remove_job_file(self):
+        try:
+            os.unlink(self.cron_file)
+        except OSError as e:
+            # cron file does not exist
+            return
+        except:
+            raise CronTabError("Unexpected error:", sys.exc_info()[0])
+
+    def find_job(self, name):
         comment = None
-    jobs = []
+        for l in self.lines:
-    for l in lines:
             if comment is not None:
-            jobs.append([comment,l])
+                if comment == name:
+                    return [comment, l]
+                else:
                     comment = None
-        elif re.match( r'#Ansible: ',l):
+            elif re.match( r'%s' % self.ansible, l):
-            comment = re.sub( r'#Ansible: ', '', l)
+                comment = re.sub( r'%s' % self.ansible, '', l)
-    return jobs
-
-def find_job(name,tmpfile):
-    jobs = get_jobs(tmpfile)
-    for j in jobs:
-        if j[0] == name:
-            return j
-    return []
 
-def add_job(module,name,job,tmpfile):
+        return []
-    f = open(tmpfile, 'a')
-    f.write("#Ansible: %s\n%s\n" % (name, job))
-    f.close()
 
-def update_job(name,job,tmpfile):
+    def get_cron_job(self,minute,hour,day,month,weekday,job,special):
-    return _update_job(name,job,tmpfile,do_add_job)
+        if special:
+            if self.cron_file:
+                return "@%s %s %s" % (special, self.user, job)
+            else:
+                return "@%s %s" % (special, job)
+        else:
+            if self.cron_file:
+                return "%s %s %s %s %s %s %s" % (minute,hour,day,month,weekday,self.user,job)
+            else:
+                return "%s %s %s %s %s %s" % (minute,hour,day,month,weekday,job)
 
-def do_add_job(lines, comment, job):
+        return None
-    lines.append(comment)
-    lines.append(job)
 
-def remove_job(name,tmpfile):
+    def get_jobnames(self):
-    return _update_job(name, "", tmpfile, do_remove_job)
+        jobnames = []
 
-def do_remove_job(lines,comment,job):
+        for l in self.lines:
-    return None
+            if re.match( r'%s' % self.ansible, l):
+                jobnames.append(re.sub( r'%s' % self.ansible, '', l))
 
-def remove_job_file(cron_file):
+        return jobnames
-    fname = "/etc/cron.d/%s" % (cron_file)
-    os.unlink(fname)
 
-def _update_job(name,job,tmpfile,addlinesfunction):
+    def _update_job(self, name, job, addlinesfunction):
-    ansiblename = "#Ansible: %s" % (name)
+        ansiblename = "%s%s" % (self.ansible, name)
-    f = open(tmpfile)
-    lines = f.read().splitlines()
-    f.close()
         newlines = []
         comment = None
-    for l in lines:
+
+        for l in self.lines:
             if comment is not None:
                 addlinesfunction(newlines, comment, job)
                 comment = None
@@ -213,38 +320,65 @@ def _update_job(name,job,tmpfile,addlinesfunction):
                 comment = l
             else:
                 newlines.append(l)
-    f = open(tmpfile, 'w')
+
-    for l in newlines:
+        self.lines = newlines
-        f.write(l)
-        f.write('\n')
-    f.close()
 
         if len(newlines) == 0:
             return True
         else:
             return False # TODO add some more error testing
 
-def get_cron_job(minute,hour,day,month,weekday,job,user,cron_file,reboot):
+    def render(self):
-    if reboot:
+        """
-        if cron_file:
+        Render this crontab as it would be in the crontab.
-            return "@reboot %s %s" % (user, job)
+        """
-        else:
+        crons = []
-            return "@reboot %s" % (job)
+        for cron in self.lines:
-    else:
+            crons.append(cron)
-        if cron_file:
-            return "%s %s %s %s %s %s %s" % (minute,hour,day,month,weekday,user,job)
-        else:
-            return "%s %s %s %s %s %s" % (minute,hour,day,month,weekday,job)
 
-    return None
+        result = '\n'.join(crons)
+        if result and result[-1] not in ['\n', '\r']:
+            result += '\n'
+        return result
+
+    def _read_file_execute(self):
+        """
+        Returns the command line for reading a crontab
+        """
+        return "%s -l%s" % (CRONCMD, self._user_execute())
+
+    def _read_user_execute(self):
+        """
+        Returns the command line for reading a crontab
+        """
+        return "%s -l %s" % (CRONCMD, self._user_execute())
+
+    def _write_execute(self, path):
+        """
+        Return the command line for writing a crontab
+        """
+        return "%s %s %s" % (CRONCMD, path, self._user_execute())
+
+    def _user_execute(self):
+        """
+        User command switches to append to the read and write commands.
+        """
+        if self.user:
+            return "%s %s" % (' -u ', str(self.user))
+        return ''
+
+#==================================================
 
 def main():
     # The following example playbooks:
-    # - action: cron name="check dirs" hour="5,2" job="ls -alh > /dev/null"
+    #
+    # - cron: name="check dirs" hour="5,2" job="ls -alh > /dev/null"
+    #
     # - name: do the job
-    #   action: name="do the job" cron hour="5,2" job="/some/dir/job.sh"
+    #   cron: name="do the job" hour="5,2" job="/some/dir/job.sh"
+    #
     # - name: no job
-    #   action: name="an old job" cron job="/some/dir/job.sh" state=absent
+    #   cron: name="an old job" state=absent
     #
     # Would produce:
     # # Ansible: check dirs
@@ -252,19 +386,9 @@ def main():
     # # Ansible: do the job
     # * * 5,2 * * /some/dir/job.sh
 
-    # Function:
-    # 1. dump the existing cron:
-    #    crontab -l -u <user> > /tmp/tmpfile
-    # 2. search for comment "^# Ansible: <name>" followed by a cron.
-    # 3. if absent: remove if present (and say modified), otherwise return with no mod.
-    # 4. if present: if the same return no mod, if not present add (and say mod), if different add (and say mod)
-    # 5. Install new cron (if mod):
-    #    crontab -u <user> /tmp/tmpfile
-    # 6. return mod
-
     module = AnsibleModule(
         argument_spec = dict(
-            name=dict(required=True),
+            name=dict(required=False),
             user=dict(required=False),
             job=dict(required=False),
             cron_file=dict(required=False),
@@ -272,99 +396,113 @@ def main():
             backup=dict(default=False, type='bool'),
             minute=dict(default='*'),
             hour=dict(default='*'),
-            day=dict(default='*'),
+            day=dict(aliases=['dom'], default='*'),
             month=dict(default='*'),
-            weekday=dict(default='*'),
+            weekday=dict(aliases=['dow'], default='*'),
-            reboot=dict(required=False, default=False, type='bool')
+            reboot=dict(required=False, default=False, type='bool'),
-        )
+            special_time=dict(required=False,
+                              default=None,
+                              choices=["reboot", "yearly", "annually", "monthly", "weekly", "daily", "hourly"],
+                              type='str')
+        ),
+        supports_check_mode = False,
     )
 
-    backup     = module.params['backup']
     name         = module.params['name']
     user         = module.params['user']
     job          = module.params['job']
     cron_file    = module.params['cron_file']
+    state        = module.params['state']
+    backup       = module.params['backup']
     minute       = module.params['minute']
     hour         = module.params['hour']
     day          = module.params['day']
     month        = module.params['month']
     weekday      = module.params['weekday']
     reboot       = module.params['reboot']
-    state      = module.params['state']
+    special_time = module.params['special_time']
-    do_install = module.params['state'] == 'present'
+    do_install   = state == 'present'
+
     changed      = False
+    res_args     = dict()
 
-    if reboot and (True in [(x != '*') for x in [minute, hour, day, month, weekday]]):
+    crontab = CronTab(module, user, cron_file)
-        module.fail_json(msg="You must specify either reboot=True or any of minute, hour, day, month, weekday")
 
-    if cron_file:
+    if crontab.syslogging:
+        syslog.openlog('ansible-%s' % os.path.basename(__file__))
+        syslog.syslog(syslog.LOG_NOTICE, 'cron instantiated - name: "%s"' % name)
+
+    # --- user input validation ---
+
+    if (special_time or reboot) and \
+       (True in [(x != '*') for x in [minute, hour, day, month, weekday]]):
+        module.fail_json(msg="You must specify time and date fields or special time.")
+
+    if cron_file and do_install:
         if not user:
             module.fail_json(msg="To use file=... parameter you must specify user=... as well")
-    else:
-        if not user:
-            user = ""
-        else:
-            user = "-u %s" % (user)
 
-    job = get_cron_job(minute,hour,day,month,weekday,job,user,cron_file,reboot)
+    if reboot and special_time:
-    rc, out, err, rm, status = (0, None, None, None, None)
+        module.fail_json(msg="reboot and special_time are mutually exclusive")
+
+    if name is None and do_install:
+        module.fail_json(msg="You must specify 'name' to install a new cron job")
+
     if job is None and do_install:
         module.fail_json(msg="You must specify 'job' to install a new cron job")
 
-    tmpfile = tempfile.NamedTemporaryFile()
+    if reboot:
-    (rc, out, err) = get_jobs_file(module,user,tmpfile.name, cron_file)
+        if special_time:
+            module.fail_json(msg="reboot and special_time are mutually exclusive")
+        else:
+            special_time = "reboot"
 
-    if rc != 0 and rc != 1: # 1 can mean that there are no jobs.
+    # if requested make a backup before making a change
-        module.fail_json(msg=err)
+    if backup:
+        (backuph, backup_file) = tempfile.mkstemp(prefix='crontab')
+        crontab.write(backup_file)
 
-    (handle,backupfile) = tempfile.mkstemp(prefix='crontab')
+    if crontab.cron_file and not do_install:
-    (rc, out, err) = get_jobs_file(module,user,backupfile, cron_file)
+        crontab.remove_job_file()
-    if rc != 0 and rc != 1:
+        changed = True
-        module.fail_json(msg=err)
+        module.exit_json(changed=changed,cron_file=cron_file,state=state)
+
+    job = crontab.get_cron_job(minute, hour, day, month, weekday, job, special_time)
+    old_job = crontab.find_job(name)
 
-    old_job = find_job(name,backupfile)
     if do_install:
         if len(old_job) == 0:
-            add_job(module,name,job,tmpfile.name)
+            crontab.add_job(name, job)
             changed = True
         if len(old_job) > 0 and old_job[1] != job:
-            update_job(name,job,tmpfile.name)
+            crontab.update_job(name, job)
             changed = True
     else:
         if len(old_job) > 0:
-            # if rm is true after the next line, file will be deleted afterwards
+            crontab.remove_job(name)
-            rm = remove_job(name,tmpfile.name)
             changed = True
-        else:
+
-            # there is no old_jobs for deletion - we should leave everything
+    res_args = dict(
-            # as is. If the file is empty, it will be removed later
+        jobs = crontab.get_jobnames(), changed = changed
-            tmpfile.close()
+    )
-            # the file created by mks should be deleted explicitly
-            os.unlink(backupfile)
-            module.exit_json(changed=changed,cron_file=cron_file,state=state)
 
     if changed:
-        # If the file is empty - remove it
+        crontab.write()
-        if rm and cron_file:
+
-            remove_job_file(cron_file)
+    # retain the backup only if crontab or cron file have changed
-        else:
     if backup:
-                module.backup_local(backupfile)
+        if changed:
-            (rc, out, err) = install_jobs(module,user,tmpfile.name, cron_file)
+            res_args['backup_file'] = backup_file
-            if (rc != 0):
+        else:
-                module.fail_json(msg=err)
+            os.unlink(backup_file)
 
-    # get the list of jobs in file
+    if cron_file:
-    jobnames = []
+        res_args['cron_file'] = cron_file
-    for j in get_jobs(tmpfile.name):
-        jobnames.append(j[0])
-    tmpfile.close()
 
-    if not backup:
+    module.exit_json(**res_args)
-        os.unlink(backupfile)
+
-        module.exit_json(changed=changed,jobs=jobnames)
+    # --- should never get here
-    else:
+    module.exit_json(msg="Unable to execute cron task.")
-        module.exit_json(changed=changed,jobs=jobnames,backup=backupfile)
 
 # include magic from lib/ansible/module_common.py
 #<<INCLUDE_ANSIBLE_MODULE_COMMON>>