logrotate: don't use filename to generate config file

I noticed on our hosts some logrotate files named '*.1234.conf' --
these are coming from callers of logrotate role specifying
'/var/log/program/*.log', where the '*' is turning into a literal
filename.  I didn't really consider this case.

Having a file-name starting with '*' may technically be fine, but is a
bad idea for everyone's sanity and it's potential to foot-gun some
sort of operation that suddenly wipes out a lot more than you wanted
to.

Let's just use the hash of the name to be unambiguous and still
idempotent.  Make it more git-ish by using the same 7 digits as a
default short-hash.

Change-Id: I13d376f85a25a7b8c3a0bc0dcbabd916e8a9774a

playbooks/roles/logrotate/README.rst

@@ -14,12 +14,15 @@ not an exhaustive list of directives (contributions are welcome).
 
 .. zuul:rolevar:: logrotate_file_name
 
-   The log file on disk to rotate
+   The full path to log file on disk to rotate.  May be a wild-card;
+   e.g. ``/var/log/progname/*.log``.
 
 .. zuul:rolevar:: logrotate_config_file_name
-   :default: Unique name based on :zuul:rolevar::`logrotate.logrotate_file_name`
+   :default: Unique name based on the hash of :zuul:rolevar::`logrotate.logrotate_file_name`
 
-   The name of the configuration file in ``/etc/logrotate.d``
+   The name of the configuration file in ``/etc/logrotate.d``.  If
+   this is specified, it is up to the caller to ensure it is unique
+   across all calls of this role.
 
 .. zuul:rolevar:: logrotate_compress
    :default: yes

playbooks/roles/logrotate/tasks/main.yaml

@@ -15,10 +15,19 @@
   when: logrotate_frequency == 'size'
 
 # Hash the full path to avoid any conflicts but remain idempotent.
-# "/var/log/ansible/ansible.log" becomes "ansible.log.37237.conf" for example
 - name: Create a unique config name
   set_fact:
-    logrotate_generated_config_file_name: "{{ logrotate_file_name | basename }}.{{ (logrotate_file_name|hash('sha1'))[0:5] }}.conf"
+    # NOTE(ianw) 2023-02-13 : we missed that this makes files with
+    # names like "*.1234.conf" when using wild-cards.  Below we have
+    # dropped using the file-name component.  After we've removed them
+    # we can drop this.
+    _old_logrotate_generated_config_file_name: "{{ logrotate_file_name | basename }}.{{ (logrotate_file_name|hash('sha1'))[0:5] }}.conf"
+    logrotate_generated_config_file_name:  "{{ (logrotate_file_name | hash('sha1'))[0:6] }}.conf"
+
+- name: Clear out potentially confusing config files
+  file:
+    state: absent
+    path: '{{ _old_logrotate_generated_config_file_name }}'
 
 - name: 'Install {{ logrotate_file_name }} rotatation config file'
   template:

testinfra/test_base.py

@@ -130,7 +130,9 @@ def test_logrotate(host):
     '''
     ansible_vars = host.ansible.get_variables()
     if ansible_vars['inventory_hostname'].startswith('bridge'):
-        cfg_file = host.file("/etc/logrotate.d/ansible.log.37237.conf")
+        # Generated for idempotence by logrotate role; hash of
+        # "/var/log/ansible/ansible.log"
+        cfg_file = host.file("/etc/logrotate.d/372374.conf")
         assert cfg_file.exists
         assert cfg_file.contains('/var/log/ansible/ansible.log')