Only change expiration date if it is different

changelogs/fragments/user-expires.yaml

@@ -0,0 +1,2 @@
+bugfixes:
+  - user - only change the expiration time when necessary (https://github.com/ansible/ansible/issues/13235)

lib/ansible/modules/system/user.py

@@ -270,6 +270,7 @@ class User(object):
     platform = 'Generic'
     distribution = None
     SHADOWFILE = '/etc/shadow'
+    SHADOWFILE_EXPIRE_INDEX = 7
     DATE_FORMAT = '%Y-%m-%d'
 
     def __new__(cls, *args, **kwargs):
@@ -532,8 +533,16 @@ def modify_user_usermod(self):
             cmd.append(self.shell)
 
         if self.expires:
-            cmd.append('-e')
-            cmd.append(time.strftime(self.DATE_FORMAT, self.expires))
+            current_expires = self.user_password()[1]
+
+            # Convert days since Epoch to seconds since Epoch as struct_time
+            total_seconds = int(current_expires) * 86400
+            current_expires = time.gmtime(total_seconds)
+
+            # Compare year, month, and day only
+            if current_expires[:3] != self.expires[:3]:
+                cmd.append('-e')
+                cmd.append(time.strftime(self.DATE_FORMAT, self.expires))
 
         if self.password_lock:
             cmd.append('-L')
@@ -616,25 +625,30 @@ def user_info(self):
             return False
         info = self.get_pwd_info()
         if len(info[1]) == 1 or len(info[1]) == 0:
-            info[1] = self.user_password()
+            info[1] = self.user_password()[0]
         return info
 
     def user_password(self):
         passwd = ''
+        expires = ''
         if HAVE_SPWD:
             try:
                 passwd = spwd.getspnam(self.name)[1]
+                expires = spwd.getspnam(self.name)[7]
+                return passwd, expires
             except KeyError:
-                return passwd
+                return passwd, expires
+
         if not self.user_exists():
-            return passwd
+            return passwd, expires
         elif self.SHADOWFILE:
             # Read shadow file for user's encrypted password string
             if os.path.exists(self.SHADOWFILE) and os.access(self.SHADOWFILE, os.R_OK):
                 for line in open(self.SHADOWFILE).readlines():
                     if line.startswith('%s:' % self.name):
                         passwd = line.split(':')[1]
-        return passwd
+                        expires = line.split(':')[self.SHADOWFILE_EXPIRE_INDEX]
+        return passwd, expires
 
     def get_ssh_key_path(self):
         info = self.user_info()
@@ -767,6 +781,8 @@ class FreeBsdUser(User):
     platform = 'FreeBSD'
     distribution = None
     SHADOWFILE = '/etc/master.passwd'
+    SHADOWFILE_EXPIRE_INDEX = 6
+    DATE_FORMAT = '%d-%b-%Y'
 
     def remove_user(self):
         cmd = [
@@ -830,9 +846,8 @@ def create_user(self):
             cmd.append(self.login_class)
 
         if self.expires:
-            days = (time.mktime(self.expires) - time.time()) // 86400
             cmd.append('-e')
-            cmd.append(str(int(days)))
+            cmd.append(time.strftime(self.DATE_FORMAT, self.expires))
 
         # system cannot be handled currently - should we error if its requested?
         # create the user
@@ -932,8 +947,12 @@ def modify_user(self):
                 cmd.append(','.join(new_groups))
 
         if self.expires:
-            cmd.append('-e')
-            cmd.append(str(int(time.mktime(self.expires))))
+            current_expires = time.gmtime(int(self.user_password()[1]))
+
+            # Compare year, month, and day only
+            if current_expires[:3] != self.expires[:3]:
+                cmd.append('-e')
+                cmd.append(time.strftime(self.DATE_FORMAT, self.expires))
 
         # modify the user if cmd will do anything
         if cmd_len != len(cmd):

test/integration/targets/user/aliases

@@ -1 +1,2 @@
+destructive
 posix/ci/group1

test/integration/targets/user/tasks/main.yml

@@ -20,142 +20,229 @@
   shell: python -c 'import jinja2; print(jinja2.__version__)'
   register: jinja2_version
   delegate_to: localhost
-- debug: var=jinja2_version
+  changed_when: no
+
+- debug:
+    msg: "Jinja version: {{ jinja2_version.stdout }}, Python version: {{ ansible_python_version }}"
+
 
-##
 ## user add
-##
-#
+
 - name: remove the test user
   user:
-      name: ansibulluser
-      state: absent
+    name: ansibulluser
+    state: absent
 
 - name: try to create a user
   user:
-      name: ansibulluser
-      state: present
+    name: ansibulluser
+    state: present
   register: user_test0
-- debug: var=user_test0
+
+- debug:
+    var: user_test0
+    verbosity: 2
 
 - name: make a list of users
-  script: userlist.sh "{{ ansible_distribution }}"
+  script: userlist.sh {{ ansible_distribution }}
   register: user_names
-- debug: var=user_names
+
+- debug:
+    var: user_names
+    verbosity: 2
 
 - name: validate results for testcase 0
   assert:
-      that:
-          - 'user_test0.changed is defined'
-          - 'user_test0.changed'
-          - '"ansibulluser" in user_names.stdout_lines'
+    that:
+      - user_test0 is changed
+      - '"ansibulluser" in user_names.stdout_lines'
+
 
-##
 ## user check
-##
 
 - name: run existing user check tests
   user:
-      name: "{{ user_names.stdout_lines|random }}"
-      state: present
-      create_home: no
+    name: "{{ user_names.stdout_lines | random }}"
+    state: present
+    create_home: no
   with_sequence: start=1 end=5
   register: user_test1
-- debug: var=user_test1
+
+- debug:
+    var: user_test1
+    verbosity: 2
 
 - name: validate results for testcase 1
   assert:
-      that:
-          - 'user_test1.results is defined'
-          - 'user_test1.results|length == 5'
+    that:
+      - user_test1.results is defined
+      - user_test1.results | length == 5
 
 - name: validate changed results for testcase 1 (jinja >= 2.6)
   assert:
-      that:
-          - "user_test1.results|map(attribute='changed')|unique|list == [False]"
-          - "user_test1.results|map(attribute='state')|unique|list == ['present']"
-  when: "jinja2_version.stdout is version('2.6', '>=')"
+    that:
+      - user_test1.results | map(attribute='changed') | unique | list == [False]
+      - user_test1.results | map(attribute='state') | unique | list == ['present']
+  when: jinja2_version.stdout is version('2.6', '>=')
 
-- name: validate changed results for testcase 1 (jinja >= 2.6)
+- name: validate changed results for testcase 1 (jinja < 2.6)
   assert:
-      that:
-          - "not user_test1.results[0]['changed']"
-          - "not user_test1.results[1]['changed']"
-          - "not user_test1.results[2]['changed']"
-          - "not user_test1.results[3]['changed']"
-          - "not user_test1.results[4]['changed']"
-          - "user_test1.results[0]['state'] == 'present'"
-          - "user_test1.results[1]['state'] == 'present'"
-          - "user_test1.results[2]['state'] == 'present'"
-          - "user_test1.results[3]['state'] == 'present'"
-          - "user_test1.results[4]['state'] == 'present'"
-  when: "jinja2_version.stdout is version('2.6', '<')"
-
-##
+    that:
+      - "user_test1.results[0] is not changed"
+      - "user_test1.results[1] is not changed"
+      - "user_test1.results[2] is not changed"
+      - "user_test1.results[3] is not changed"
+      - "user_test1.results[4] is not changed"
+      - "user_test1.results[0]['state'] == 'present'"
+      - "user_test1.results[1]['state'] == 'present'"
+      - "user_test1.results[2]['state'] == 'present'"
+      - "user_test1.results[3]['state'] == 'present'"
+      - "user_test1.results[4]['state'] == 'present'"
+  when: jinja2_version.stdout is version('2.6', '<')
+
+
 ## user remove
-##
-
+
 - name: try to delete the user
   user:
-      name: ansibulluser
-      state: absent
-      force: true
+    name: ansibulluser
+    state: absent
+    force: true
   register: user_test2
+
 - name: make a new list of users
-  script: userlist.sh "{{ ansible_distribution }}"
+  script: userlist.sh {{ ansible_distribution }}
   register: user_names2
-- debug: var=user_names2
+
+- debug:
+    var: user_names2
+    verbosity: 2
+
 - name: validate results for testcase 2
   assert:
-      that:
-          - '"ansibulluser" not in user_names2.stdout_lines'
+    that:
+      - '"ansibulluser" not in user_names2.stdout_lines'
 
 
 - block:
-    - name: create non-system user on OSX to test the shell is set to /bin/bash
+    - name: create non-system user on macOS to test the shell is set to /bin/bash
       user:
-        name: osxuser
-      register: osxuser_output
+        name: macosuser
+      register: macosuser_output
 
     - name: validate the shell is set to /bin/bash
       assert:
         that:
-          - 'osxuser_output.shell == "/bin/bash"'
+          - 'macosuser_output.shell == "/bin/bash"'
 
     - name: cleanup
       user:
-        name: osxuser
+        name: macosuser
         state: absent
 
-    - name: create system user on OSX to test the shell is set to /usr/bin/false
+    - name: create system user on macos to test the shell is set to /usr/bin/false
       user:
-        name: osxuser
+        name: macosuser
         system: yes
-      register: osxuser_output
+      register: macosuser_output
 
     - name: validate the shell is set to /usr/bin/false
       assert:
         that:
-          - 'osxuser_output.shell == "/usr/bin/false"'
+          - 'macosuser_output.shell == "/usr/bin/false"'
 
     - name: cleanup
       user:
-        name: osxuser
+        name: macosuser
         state: absent
 
-    - name: create non-system user on OSX and set the shell to /bin/sh
+    - name: create non-system user on macos and set the shell to /bin/sh
       user:
-        name: osxuser
+        name: macosuser
         shell: /bin/sh
-      register: osxuser_output
+      register: macosuser_output
 
     - name: validate the shell is set to /bin/sh
       assert:
         that:
-          - 'osxuser_output.shell == "/bin/sh"'
+          - 'macosuser_output.shell == "/bin/sh"'
 
     - name: cleanup
       user:
-        name: osxuser
+        name: macosuser
         state: absent
   when: ansible_distribution == "MacOSX"
+
+
+## user expires
+# Date is March 3, 2050
+- name: Create user with expiration
+  user:
+    name: ansibulluser
+    state: present
+    expires: 2529881062
+  register: user_test_expires1
+
+- name: Create user with expiration again to ensure no change is made
+  user:
+    name: ansibulluser
+    state: present
+    expires: 2529881062
+  register: user_test_expires2
+
+- name: Ensure that account with expiration was created and did not change on subsequent run
+  assert:
+    that:
+      - user_test_expires1 is changed
+      - user_test_expires2 is not changed
+
+- name: Verify expiration date for Linux
+  block:
+    - name: LINUX | Get expiration date for ansibulluser
+      getent:
+        database: shadow
+        key: ansibulluser
+
+    - name: LINUX | Ensure proper expiration date was set
+      assert:
+        that:
+          - getent_shadow['ansibulluser'][6] == '29281'
+  when: ansible_os_family in ['RedHat', 'Debian', 'Suse']
+
+
+- name: Verify expiration date for BSD
+  block:
+    - name: BSD | Get expiration date for ansibulluser
+      shell: 'grep ansibulluser /etc/master.passwd | cut -d: -f 7'
+      changed_when: no
+      register: bsd_account_expiration
+
+    - name: BSD | Ensure proper expiration date was set
+      assert:
+        that:
+          - bsd_account_expiration.stdout == '2529878400'
+  when: ansible_os_family == 'FreeBSD'
+
+- name: Change timezone
+  timezone:
+    name: America/Denver
+  register: original_timezone
+
+- name: Change system timezone to make sure expiration comparison works properly
+  block:
+    - name: Create user with expiration again to ensure no change is made in a new timezone
+      user:
+        name: ansibulluser
+        state: present
+        expires: 2529881062
+      register: user_test_different_tz
+
+    - name: Ensure that no change was reported
+      assert:
+        that:
+          - user_test_different_tz is not changed
+
+  always:
+    - name: Restore original timezone - {{ original_timezone.diff.before.name }}
+      timezone:
+        name: "{{ original_timezone.diff.before.name }}"