Makes unattended-upgrades sequencing more similar to cron-apt

install_files/ansible-base/roles/common/defaults/main.yml

@@ -53,3 +53,11 @@ unused_packages:
   - wireless-tools
   - wpasupplicant
   - snapd
+
+# Template declaration for setting the upgrade time to a predictable time,
+# matching the 'daily_reboot_time' time via sdconfig.
+unattended_upgrades_timer_overrides:
+  - src: apt-daily-timer-override.j2
+    dest: /etc/systemd/system/apt-daily.timer.d/override.conf
+  - src: apt-daily-upgrade-timer-override.j2
+    dest: /etc/systemd/system/apt-daily-upgrade.timer.d/override.conf

install_files/ansible-base/roles/common/handlers/main.yml

@@ -10,3 +10,7 @@
 - name: update apt cache
   apt:
     update_cache: yes
+
+- name: systemd daemon-reload
+  systemd:
+    daemon_reload: yes

install_files/ansible-base/roles/common/tasks/main.yml

@@ -27,6 +27,7 @@
   when:
     - ansible_distribution_release == "focal"
   tags:
+    - ua
     - reboot
 
 - include: remove_unused_packages.yml

install_files/ansible-base/roles/common/tasks/unattended_upgrades.yml

@@ -2,6 +2,24 @@
 # Configuration for unattended upgrades is almost exclusively managed by the
 # securedrop-config package under Focal.
 
+- name: Create override dirs for apt-daily timers
+  file:
+    state: directory
+    mode: "0755"
+    path: "{{ item.dest|dirname }}"
+  with_items: "{{ unattended_upgrades_timer_overrides }}"
+
+- name: Add overrides for apt-daily timers
+  template:
+    src: "{{ item.src }}"
+    dest: "{{ item.dest }}"
+    mode: "0644"
+  notify: systemd daemon-reload
+  with_items: "{{ unattended_upgrades_timer_overrides }}"
+
+  # Ensure daemon-reload has happened before starting/enabling
+- meta: flush_handlers
+
 - name: Ensure apt-daily and apt-daily-upgrade services are unmasked, started and enabled.
   systemd:
     name: "{{ item }}"

install_files/ansible-base/roles/common/templates/80securedrop.j2

@@ -1,8 +1,7 @@
 {% if ansible_distribution_release == "focal" %}
-// If automatic reboot is enabled and needed, reboot at the specific
-// time instead of immediately
-//  Default: "now"
-Unattended-Upgrade::Automatic-Reboot-Time "{{ daily_reboot_time }}:00";
+// Reboot should happen after nightly upgrades. Timing of upgrade
+// is configured via apt.daily.timer
+Unattended-Upgrade::Automatic-Reboot-Time "now";
 {% endif %}
 // Don't install packages from "Recommends" field, we'll manage dependencies
 // explicitly to avoid pulling in packages from e.g. universe.

install_files/ansible-base/roles/common/templates/apt-daily-timer-override.j2

@@ -0,0 +1,5 @@
+[Timer]
+OnCalendar=
+OnCalendar=*-*-* {{ (daily_reboot_time|int - 1) % 24 }}:00
+RandomizedDelaySec=20m
+Persistent=true

install_files/ansible-base/roles/common/templates/apt-daily-upgrade-timer-override.j2

@@ -0,0 +1,4 @@
+[Timer]
+OnCalendar=
+OnCalendar=*-*-* {{ daily_reboot_time }}:00
+RandomizedDelaySec=20m

install_files/securedrop-config-focal/opt/securedrop/50unattended-upgrades

@@ -62,5 +62,7 @@ Unattended-Upgrade::Automatic-Reboot-WithUsers "true";
 // Here we set the dpkg options to force the old conffile if it's already present
 // or force the default config if no config is present
 // see https://github.com/freedomofpress/securedrop/pull/911
-Dpkg::Options "force-confdef";
-Dpkg::Options "force-confold";
+Dpkg::Options {
+    "--force-confdef";
+    "--force-confold";
+}

molecule/testinfra/common/test_automatic_updates.py

@@ -187,18 +187,45 @@ def test_cron_apt_cron_jobs(host, cron_job):
         assert not f.exists
 
 
-def test_unattended_upgrades_config(host):
+apt_config_options = {
+    "APT::Install-Recommends": "false",
+    "Dpkg::Options": [
+        "--force-confold",
+        "--force-confdef",
+    ],
+    "APT::Periodic::Update-Package-Lists": "1",
+    "APT::Periodic::Unattended-Upgrade": "1",
+    "APT::Periodic::AutocleanInterval": "1",
+    "Unattended-Upgrade::AutoFixInterruptedDpkg": "true",
+    "Unattended-Upgrade::Automatic-Reboot": "true",
+    "Unattended-Upgrade::Automatic-Reboot-Time": "now",
+    "Unattended-Upgrade::Automatic-Reboot-WithUsers": "true",
+    "Unattended-Upgrade::Origins-Pattern": [
+        "origin=${distro_id},archive=${distro_codename}",
+        "origin=${distro_id},archive=${distro_codename}-security",
+        "origin=${distro_id},archive=${distro_codename}-updates",
+        "origin=SecureDrop,codename=${distro_codename}",
+    ],
+}
+
+
+@pytest.mark.parametrize("k, v", apt_config_options.items())
+def test_unattended_upgrades_config(host, k, v):
     """
     Ensures the 50unattended-upgrades config is correct only under Ubuntu Focal
     """
-    f = host.file('/etc/apt/apt.conf.d/50unattended-upgrades')
-    if host.system_info.codename != "xenial":
-        assert f.is_file
-        assert f.user == "root"
-        assert f.mode == 0o644
-        assert f.contains("origin=SecureDrop,codename=${distro_codename}")
-        assert f.contains('Dpkg::Options "force-confold";')
-        assert f.contains('Dpkg::Options "force-confdef";')
+    if host.system_info.codename == "xenial":
+        return True
+    # Dump apt config to inspect end state, apt will build config
+    # from all conf files on disk, e.g. 80securedrop.
+    c = host.run("apt-config dump --format '%v%n' {}".format(k))
+    assert c.rc == 0
+    # Some values are lists, so support that in the params
+    if hasattr(v, "__getitem__"):
+        for i in v:
+            assert i in c.stdout
+    else:
+        assert v in c.stdout
 
 
 def test_unattended_securedrop_specific(host):
@@ -218,25 +245,6 @@ def test_unattended_securedrop_specific(host):
         assert not f.contains("Automatic-Reboot-Time")
 
 
-@pytest.mark.parametrize('option', [
-  'APT::Periodic::Update-Package-Lists "1";',
-  'APT::Periodic::Unattended-Upgrade "1";',
-  'APT::Periodic::AutocleanInterval "1";',
- ])
-def test_auto_upgrades_config(host, option):
-    """
-    Ensures the 20auto-upgrades config is correct only under Ubuntu Focal
-    """
-    f = host.file('/etc/apt/apt.conf.d/20auto-upgrades')
-    if host.system_info.codename == "xenial":
-        assert not f.exists
-    else:
-        assert f.is_file
-        assert f.user == "root"
-        assert f.mode == 0o644
-        assert f.contains('^{}$'.format(option))
-
-
 def test_unattended_upgrades_functional(host):
     """
     Ensure unatteded-upgrades completes successfully and ensures all packages
@@ -275,6 +283,20 @@ def test_apt_daily_services_and_timers_enabled(host, service):
             assert s.is_enabled
 
 
+def test_apt_daily_timer_schedule(host):
+    if host.system_info.codename != "xenial":
+        c = host.run("systemctl show apt-daily.timer")
+        assert "TimersCalendar={ OnCalendar=*-*-* 03:00:00 ;" in c.stdout
+        assert "RandomizedDelayUSec=20m" in c.stdout
+
+
+def test_apt_daily_upgrade_timer_schedule(host):
+    if host.system_info.codename != "xenial":
+        c = host.run("systemctl show apt-daily-upgrade.timer")
+        assert "TimersCalendar={ OnCalendar=*-*-* 04:00:00 ;" in c.stdout
+        assert "RandomizedDelayUSec=20m" in c.stdout
+
+
 def test_reboot_required_cron(host):
     """
     Unatteded-upgrades does not reboot the system if the updates don't require it.