[fix] Ensure log files are owned by www-data #627

CHANGES.md

@@ -4,6 +4,14 @@
 
 Work in progress.
 
+### Bugfixes
+
+- Ensured log files in the OpenWISP log directory (e.g. `/opt/openwisp2/log/`)
+  are owned by the application user (`www-data`) instead of being created as
+  `root`, which prevented the application processes (uWSGI, celery, daphne,
+  Django) from writing to them
+  ([#627](https://github.com/openwisp/ansible-openwisp2/issues/627))
+
 ## Version 25.10.2 [2026-01-28]
 
 # Bugfixes

molecule/resources/verify.yml

@@ -7,7 +7,7 @@
 
   tasks:
     - name: Run a specific subset of tests
       command: >
         /opt/openwisp2/env/bin/python /opt/openwisp2/manage.py test --keepdb --exclude skip_prod \
             openwisp_controller.pki.tests.test_admin \
             openwisp_controller.pki.tests.test_models \
@@ -38,28 +38,107 @@
       changed_when: false
 
     - name: Check if redis-server is running
       command: systemctl status redis-server
       changed_when: false
 
     - name: Chcke if redis is running
       command: systemctl status redis
       changed_when: false
 
     - name: Check Openwisp
       block:
         - name: Check if OpenWISP is running
           uri:
             url: "https://{{ inventory_hostname }}/admin/login/?next=/admin/"
             validate_certs: false
       rescue:
         - name: Get OpenWisp log
           command: "tail -n 500 {{ openwisp2_path }}/log/*.log"
           register: openwisp_log
 
         - name: Show OpenWisp log
           debug:
             var: openwisp_log
 
+    # Regression tests for
+    # https://github.com/openwisp/ansible-openwisp2/issues/627
+    # The log directory and the supervisor/Django log files must be owned by the
+    # application user (www-data) so the services running as www-data can write
+    # to them. (nginx*.log are intentionally excluded: they are created by the
+    # nginx master process running as root.)
+    - name: Get ownership of the OpenWISP log directory
+      ansible.builtin.stat:
+        path: /opt/openwisp2/log
+      register: openwisp2_log_dir
+
+    - name: Ensure the OpenWISP log directory is owned by www-data
+      ansible.builtin.assert:
+        that:
+          - openwisp2_log_dir.stat.exists
+          - openwisp2_log_dir.stat.isdir
+          - openwisp2_log_dir.stat.pw_name == 'www-data'
+          - openwisp2_log_dir.stat.gr_name == 'www-data'
+        fail_msg: >-
+          /opt/openwisp2/log is not owned by www-data:www-data
+          (owner={{ openwisp2_log_dir.stat.pw_name | default('?') }}:{{
+          openwisp2_log_dir.stat.gr_name | default('?') }})
+        quiet: true
+
+    - name: Get ownership of the OpenWISP log files
+      ansible.builtin.stat:
+        path: "/opt/openwisp2/log/{{ item }}"
+      loop:
+        - openwisp2.log
+        - uwsgi.log
+        - celery.log
+        - celerybeat.log
+        - celery-network.log
+        - celery-monitoring.log
+        - celery-firmware-upgrader.log
+        - daphne.log
+      register: openwisp2_log_files
+
+    - name: Ensure OpenWISP log files exist and are owned by www-data
+      ansible.builtin.assert:
+        that:
+          - item.stat.exists
+          - item.stat.pw_name == 'www-data'
+          - item.stat.gr_name == 'www-data'
+        fail_msg: >-
+          {{ item.item }} is missing or not owned by www-data:www-data
+          (exists={{ item.stat.exists }}, owner={{
+          item.stat.pw_name | default('?') }}:{{
+          item.stat.gr_name | default('?') }})
+        quiet: true
+      loop: "{{ openwisp2_log_files.results }}"
+      loop_control:
+        label: "{{ item.item }}"
+
+    - name: Read the www-data crontab
+      ansible.builtin.command: crontab -u www-data -l
+      register: www_data_crontab
+      changed_when: false
+
+    - name: Read the root crontab
+      ansible.builtin.command: crontab -u root -l
+      register: root_crontab
+      changed_when: false
+      failed_when: false
+
+    - name: Ensure OpenWISP cron jobs run as www-data and not as root
+      ansible.builtin.assert:
+        that:
+          - "'manage.py clearsessions' in www_data_crontab.stdout"
+          - "'manage.py update_topology' in www_data_crontab.stdout"
+          - "'manage.py save_snapshot' in www_data_crontab.stdout"
+          - "'manage.py clearsessions' not in root_crontab.stdout"
+          - "'manage.py update_topology' not in root_crontab.stdout"
+          - "'manage.py save_snapshot' not in root_crontab.stdout"
+        fail_msg: >-
+          OpenWISP cron jobs are not installed in the www-data crontab as
+          expected (www-data crontab: {{ www_data_crontab.stdout }})
+        quiet: true
+
     - name: Check if FreeRADIUS is listening on WPA Enterprise site ports
       shell: "netstat -tuln | grep -Eq '1822|1823|18230'"
       register: freeradius_eap_ports  # Register the output and return code

tasks/cron.yml

@@ -1,9 +1,14 @@
 ---
+# NOTE: these cron jobs run manage.py commands which use Django's logging and
+# may create/rotate openwisp2.log. They are installed in the www-data user's
+# crontab (user: "{{ www_user }}") so they run as the application user and do
+# not create root-owned log files.
 - name: Install topology update cron
   when: openwisp2_network_topology
   become: true
   ansible.builtin.cron:
     name: "Update toplogies"
+    user: "{{ www_user }}"
     day: "{{ openwisp2_topology_update_frequency.day }}"
     hour: "{{ openwisp2_topology_update_frequency.hour }}"
     minute: "{{ openwisp2_topology_update_frequency.minute }}"
@@ -14,6 +19,7 @@
   become: true
   ansible.builtin.cron:
     name: "Save snapshots of topologies"
+    user: "{{ www_user }}"
     day: "{{ openwisp2_topology_save_snapshot_frequency.day }}"
     hour: "{{ openwisp2_topology_save_snapshot_frequency.hour }}"
     minute: "{{ openwisp2_topology_save_snapshot_frequency.minute }}"
@@ -23,7 +29,22 @@
   become: true
   ansible.builtin.cron:
     name: "clearsessions cronjob"
+    user: "{{ www_user }}"
     day: "*"
     hour: "04"
     minute: "30"
     job: "{{ virtualenv_path }}/bin/python {{ openwisp2_path }}/manage.py clearsessions"
+
+# Older versions of this role installed these cron jobs in root's crontab.
+# Remove the stale root-owned entries so they don't keep running as root and
+# creating root-owned log files after the upgrade.
+- name: Remove obsolete root-owned cron jobs
+  become: true
+  ansible.builtin.cron:
+    name: "{{ item }}"
+    user: root
+    state: absent
+  loop:
+    - "Update toplogies"
+    - "Save snapshots of topologies"
+    - "clearsessions cronjob"

tasks/django.yml

@@ -17,12 +17,49 @@
   ansible.builtin.file:
     path: "{{ openwisp2_path }}/log"
     state: directory
+    owner: "{{ www_user }}"
     group: "{{ www_group }}"
     mode: "0770"
     recurse: true
   tags:
     - molecule-idempotence-notest
 
+# Supervisord and Django (RotatingFileHandler) both create their log files on
+# first run. Supervisord runs as root and would otherwise create these files as
+# root:root, and Django tasks running as root would do the same for
+# openwisp2.log. Pre-creating the files with the correct ownership ensures the
+# www-data processes (uWSGI, celery, daphne, Django) can always write to them.
+- name: Ensure openwisp2 log files are owned by "{{ www_user }}"
+  ansible.builtin.file:
+    path: "{{ openwisp2_path }}/log/{{ item }}"
+    state: touch
+    owner: "{{ www_user }}"
+    group: "{{ www_group }}"
+    mode: "0640"
+    # keep the task idempotent (touch reports "changed" otherwise)
+    access_time: preserve
+    modification_time: preserve
+  # Only pre-create the log files for the services that are actually enabled,
+  # matching the conditions used to install the supervisor programs in
+  # supervisor.yml, so we don't leave empty log files for disabled features.
+  loop: >-
+    {{
+      ['openwisp2.log', 'uwsgi.log', 'celery.log']
+      + (['celerybeat.log'] if openwisp2_celerybeat else [])
+      + (['celery-network.log'] if openwisp2_celery_network else [])
+      + (['celery-monitoring.log']
+         if (openwisp2_monitoring and openwisp2_celery_monitoring) else [])
+      + (['celery-firmware-upgrader.log']
+         if (openwisp2_firmware_upgrader and openwisp2_celery_firmware_upgrader)
+         else [])
+      + (['daphne.log'] if openwisp2_daphne_install else [])
+    }}
+  # Services (e.g. supervisord running as root) may rotate/recreate these log
+  # files between converge runs, so skip the idempotence check for this task,
+  # consistent with the log directory task above.
+  tags:
+    - molecule-idempotence-notest
+
 - name: Create custom static directory
   ansible.builtin.file:
     path: "{{ openwisp2_path }}/static_custom"
@@ -224,6 +261,10 @@
     mode: "0754"
 
 - name: Load initial data
+  # Run as the application user so that any log file (e.g. openwisp2.log)
+  # touched by Django during startup is not created as root.
+  become: true
+  become_user: "{{ www_user }}"
   environment:
     PRIVATE_KEY: "{{ default_private_ssh_key.stdout | default(None) }}"
     PUBLIC_KEY: "{{ default_public_ssh_key.stdout | default(None) }}"