From a30d8ce6fd1e0eb6d1cc706ad999ec589c8da888 Mon Sep 17 00:00:00 2001 From: Oleksandr Kuzminskyi Date: Fri, 31 Jul 2026 16:22:49 -0700 Subject: [PATCH 1/2] Own DPkg::Lock::Timeout in Puppet and bound the boot security upgrade Behaviour changes are in environments/development only; sandbox and modules/ follow after testing there. profile::apt_lock_timeout (new, declared by profile::repos with stage => init) writes /etc/apt/apt.conf.d/99-lock-timeout with an *unscoped* DPkg::Lock::Timeout. Ubuntu's shipped default is binary::apt::-scoped, so it covers the `apt` command only -- apt-get, which Puppet's package provider, cloud-init and the AWS agents all use, inherits nothing and fails instantly on a held lock. Measured on noble / apt 2.8.3 against a held frontend lock: apt-get install exits 100 in 0s without the unscoped key, 0 in 45s with it. The value comes from the apt_lock_timeout custom fact, defaulting to the 300 current AMIs ship. It manages the same path infrahouse-ubuntu-pro writes, because two drop-ins setting the key resolve by lexical filename order. The init stage is load-bearing: a drop-in applied halfway through the catalog does not help the Package resources Puppet already evaluated. Exec[gha-boot-security-upgrade] moves its retry logic into gha-boot-security-upgrade.sh with a hard cumulative deadline (480s budget, exec timeout 540) rather than tries/try_sleep. exec's timeout is per-attempt, so tries multiplies the worst case with no cumulative cap -- and an overrun is not merely a slow run: an ih-puppet resource failure exits 4/6 under --detailed-exitcodes, tripping ih-bootstrap's ERR trap into ABANDON. Retries are still needed because the apt lists lock is not covered by DPkg::Lock::Timeout and unattended-upgrade refuses to run concurrently with itself. Also corrects profile::unattended_upgrades' docstring in all three copies (comments only, no behaviour change): those units are masked by terraform-aws-cloud-init's bootcmd, not by the AMI. That misattribution caused a wrong root-cause analysis, so the correction is applied everywhere rather than left to the promotion cycle. The unmask execs are load-bearing until infrahouse/terraform-aws-cloud-init#91 lands. Refs #289, infrahouse/infrahouse-ubuntu-pro#20, infrahouse/terraform-aws-cloud-init#91 Co-Authored-By: Claude Opus 5 (1M context) --- debian/changelog | 6 ++ .../gha-boot-security-upgrade.sh | 64 +++++++++++++++++++ .../profile/manifests/apt_lock_timeout.pp | 44 +++++++++++++ .../profile/manifests/github_runner.pp | 29 ++++++++- .../modules/profile/manifests/repos.pp | 7 ++ .../profile/manifests/unattended_upgrades.pp | 45 +++++++++++-- .../profile/manifests/unattended_upgrades.pp | 45 +++++++++++-- .../profile/manifests/unattended_upgrades.pp | 45 +++++++++++-- 8 files changed, 261 insertions(+), 24 deletions(-) create mode 100644 environments/development/modules/profile/files/github_runner/gha-boot-security-upgrade.sh create mode 100644 environments/development/modules/profile/manifests/apt_lock_timeout.pp diff --git a/debian/changelog b/debian/changelog index 4788a22..5c4f836 100644 --- a/debian/changelog +++ b/debian/changelog @@ -1,3 +1,9 @@ +puppet-code (0.1.0-1build320) noble; urgency=medium + + * commit event. see changes history in git log + + -- root Fri, 31 Jul 2026 23:22:51 +0000 + puppet-code (0.1.0-1build319) noble; urgency=medium * commit event. see changes history in git log diff --git a/environments/development/modules/profile/files/github_runner/gha-boot-security-upgrade.sh b/environments/development/modules/profile/files/github_runner/gha-boot-security-upgrade.sh new file mode 100644 index 0000000..77cea06 --- /dev/null +++ b/environments/development/modules/profile/files/github_runner/gha-boot-security-upgrade.sh @@ -0,0 +1,64 @@ +#!/bin/bash +# +# One-shot security patching during runner provisioning, with a hard cumulative +# time bound. +# +# Why a script instead of exec's tries/try_sleep: Puppet's `timeout` is +# PER-ATTEMPT, so `tries` multiplies the worst case to +# tries * timeout + (tries-1) * try_sleep with no cumulative cap. That matters +# here because an overrun is not merely a slow run -- ih-puppet applies with +# --detailed-exitcodes and exits 4/6 when a resource fails, which trips +# ih-bootstrap.sh's `trap _ih_signal_abandon ERR` and ABANDONs the instance. So a +# retry budget that can exceed the bootstrap lifecycle hook is a fleet-churn bug, +# not a latency bug. Bounding total wall clock here lets one legitimately long +# upgrade use the whole window while still capping the worst case. +# +# What actually needs retrying: both commands below can fail within seconds under +# lock contention. +# - `apt-get update` takes /var/lib/apt/lists/lock, which DPkg::Lock::Timeout +# does NOT cover (measured: fails in ~1s even with the option set). +# - `unattended-upgrade` refuses to run concurrently with itself. +# Contenders are routine: the Inspector and GuardDuty agents each dpkg-install +# about a minute into every boot, squarely inside the provisioning window. +# +# Note ih-puppet already runs the catalog twice and only checks the second exit +# code, so a transient failure gets one free retry above this script too. +# +# Usage: gha-boot-security-upgrade.sh [budget_seconds] [marker_path] + +# Deliberately no `set -e`: failures of the apt commands are expected and handled +# by the retry loop below. +set -uo pipefail + +BUDGET="${1:-480}" +MARKER="${2:-/run/gha-boot-upgrade.done}" + +deadline=$(( $(date +%s) + BUDGET )) +attempt=0 + +while :; do + attempt=$(( attempt + 1 )) + remaining=$(( deadline - $(date +%s) )) + + if [ "$remaining" -le 0 ]; then + echo "gha-boot-security-upgrade: ${BUDGET}s budget exhausted after ${attempt} attempt(s)" >&2 + exit 1 + fi + + echo "gha-boot-security-upgrade: attempt ${attempt}, ${remaining}s of budget left" + + # Each command is capped at the remaining budget so a single slow command + # cannot overshoot the deadline. + if timeout "$remaining" apt-get update -qq && timeout "$remaining" unattended-upgrade; then + # Written only on success, so a failed upgrade simply retries on the next + # Puppet apply. Lives on tmpfs so it clears on a real boot. + touch "$MARKER" + echo "gha-boot-security-upgrade: succeeded on attempt ${attempt}" + exit 0 + fi + + # Only sleep if there will still be budget to use afterwards. + if [ $(( deadline - $(date +%s) )) -gt 15 ]; then + sleep 15 + fi +done diff --git a/environments/development/modules/profile/manifests/apt_lock_timeout.pp b/environments/development/modules/profile/manifests/apt_lock_timeout.pp new file mode 100644 index 0000000..b67f96c --- /dev/null +++ b/environments/development/modules/profile/manifests/apt_lock_timeout.pp @@ -0,0 +1,44 @@ +# @summary: Make apt-get wait for the dpkg lock instead of failing outright. +# +# Declared by profile::repos with `stage => init` so the drop-in exists before any +# Package resource in stage main. That ordering is the whole point: a drop-in +# applied halfway through the catalog does not help the Package resources Puppet +# already evaluated. +# +# Why this is needed at all: Ubuntu ships `binary::apt::DPkg::Lock::Timeout "120"`, +# and that scope applies ONLY to the `apt` command. Puppet's package provider, +# cloud-init and the AWS agents all shell out to `apt-get`, which inherits nothing +# and fails instantly on a held lock. Measured on noble/apt 2.8.3 against a held +# /var/lib/dpkg/lock-frontend: apt-get install exits 100 in 0s without the +# unscoped key, and 0 in 45s (waiting out a 40s lock) with it. +# +# Lock contention here is routine, not hypothetical: +# - the Inspector and GuardDuty agents each dpkg-install ~1 min into every boot +# - profile::unattended_upgrades deliberately unmasks and STARTS the apt-daily +# timers mid-catalog, so an unattended-upgrade can begin during the run +# +# NOTE: the path is deliberately the same file infrahouse-ubuntu-pro writes from +# its provision.sh. Two drop-ins both setting this key would resolve by lexical +# filename order, which is a silent trap -- so Puppet converges the AMI's file +# rather than racing a second one of its own. +# +# @param timeout +# Seconds apt-get waits for the dpkg lock. Sourced from the apt_lock_timeout +# custom fact (set via the cloud-init module's custom_facts), defaulting to the +# 300 that current AMIs ship. +# +# Keep it well inside the gha_runner bootstrap lifecycle hook (1200s, +# default_result ABANDON): a genuinely wedged lock costs this many seconds per +# Package resource, and the hook is not renewed during bootstrap. +class profile::apt_lock_timeout ( + Integer[1] $timeout = Integer(pick_default($facts['apt_lock_timeout'], 300)), +) { + + file { '/etc/apt/apt.conf.d/99-lock-timeout': + ensure => file, + owner => 'root', + group => 'root', + mode => '0644', + content => "DPkg::Lock::Timeout \"${timeout}\";\n", + } +} diff --git a/environments/development/modules/profile/manifests/github_runner.pp b/environments/development/modules/profile/manifests/github_runner.pp index 7f0e444..29f611d 100644 --- a/environments/development/modules/profile/manifests/github_runner.pp +++ b/environments/development/modules/profile/manifests/github_runner.pp @@ -65,12 +65,35 @@ # per boot rather than on every Puppet apply, and is written only on success, # so a failed upgrade simply retries on the next apply. This applies Ubuntu # security updates, which do not depend on the InfraHouse repos. + # + # The retry/bounding logic lives in the script rather than in exec's + # tries/try_sleep because exec's timeout is per-attempt, so tries would multiply + # the worst case with no cumulative cap. A resource failure here ABANDONs the + # instance (ih-puppet exits 4/6, ih-bootstrap's ERR trap signals ABANDON), so + # the total must stay inside the 1200s bootstrap hook budget -- nothing renews + # it, since gha-lifecycle-heartbeater.sh is a no-op outside Terminating:Wait. + $boot_upgrade_script = '/usr/local/bin/gha-boot-security-upgrade.sh' + $boot_upgrade_budget = 480 + + file { $boot_upgrade_script: + ensure => file, + owner => 'root', + group => 'root', + mode => '0755', + source => 'puppet:///modules/profile/github_runner/gha-boot-security-upgrade.sh', + } + exec { 'gha-boot-security-upgrade': - command => 'apt-get update -qq && unattended-upgrade && touch /run/gha-boot-upgrade.done', + command => "${boot_upgrade_script} ${boot_upgrade_budget}", path => '/usr/bin:/bin:/usr/sbin:/sbin', unless => 'test -f /run/gha-boot-upgrade.done', - timeout => 1200, - require => Class['profile::unattended_upgrades'], + # Slightly above the script's own budget so the script always gets to exit and + # log why it gave up, rather than being killed mid-report by Puppet. + timeout => $boot_upgrade_budget + 60, + require => [ + Class['profile::unattended_upgrades'], + File[$boot_upgrade_script], + ], } } diff --git a/environments/development/modules/profile/manifests/repos.pp b/environments/development/modules/profile/manifests/repos.pp index 63752fe..218d252 100644 --- a/environments/development/modules/profile/manifests/repos.pp +++ b/environments/development/modules/profile/manifests/repos.pp @@ -7,4 +7,11 @@ tries => 5, }, } + + # Also in the init stage: the lock timeout has to be in place before anything in + # stage main starts installing packages. See the class for why apt-get needs an + # unscoped key of its own. + class { 'profile::apt_lock_timeout': + stage => init, + } } diff --git a/environments/development/modules/profile/manifests/unattended_upgrades.pp b/environments/development/modules/profile/manifests/unattended_upgrades.pp index 745dc25..d324501 100644 --- a/environments/development/modules/profile/manifests/unattended_upgrades.pp +++ b/environments/development/modules/profile/manifests/unattended_upgrades.pp @@ -1,8 +1,27 @@ # @summary: Enable unattended-upgrades fleet-wide for automatic security updates. # -# Included by profile::base so every host receives security fixes. The base -# AMI ships the relevant systemd units masked; this profile unmasks and enables -# them and owns their configuration via Puppet. +# Included by profile::base so every host receives security fixes. This profile +# owns the unattended-upgrades configuration and asserts that the relevant units +# stay installed, enabled and running. +# +# The units ARE masked by the time this class runs, but the masking comes from +# cloud-init, not the AMI: terraform-aws-cloud-init's bootcmd runs +# `systemctl stop` + `systemctl mask` on apt-daily{,-upgrade}.{service,timer} and +# unattended-upgrades.service on every boot, before runcmd starts ih-puppet +# (terraform-aws-cloud-init#87 -- those timers race Puppet for the dpkg lock). +# The execs below undo it on every run, so the mask/unmask cycle repeats per boot. +# +# That masking predates vulnerability management and is obsolete policy. It is +# settled that unattended-upgrades IS wanted on these hosts and that Puppet is +# authoritative for it, so the unmask is deliberate, not a workaround. The +# cloud-init side will stop masking (terraform-aws-cloud-init#91); until it does, +# the execs below are what keeps unattended-upgrades running. Do not remove them +# before that lands -- and once it does, they become no-ops and can go. +# +# Meanwhile there is a small bounded patching gap: bootcmd masks on every boot, +# but runcmd/Puppet only runs at provisioning, so after a reboot of a long-lived +# instance the units stay masked until the next scheduled puppet apply -- at most +# ~30 min, since profile::puppet_apply runs at $m and $m+30. # # Hosts that must not be disrupted by an automatic service restart (e.g. # Elasticsearch nodes) keep unattended-upgrades running but drop their own @@ -19,10 +38,22 @@ # Units that drive automatic upgrades: # - the timers run the periodic download + upgrade # - unattended-upgrades.service applies pending upgrades on shutdown/boot - # The base AMI ships these masked. A masked apt-daily.service also prevents - # its timer from starting ("unit to trigger not loaded"), so the trigger - # .service units must be unmasked too even though we never run them directly. - # Puppet's service provider cannot unmask a unit, hence the execs. + # + # These arrive masked from cloud-init's bootcmd (see the class docstring), so + # the execs below are load-bearing, not defensive. Without them + # Service[$enabled_units] fails, because Puppet's service provider can neither + # start nor enable a masked unit -- and a failed resource makes ih-puppet exit + # 4 or 6 under --detailed-exitcodes, which trips ih-bootstrap's ERR trap and + # ABANDONs the instance. The service provider cannot unmask either, hence execs. + # + # A masked apt-daily.service additionally prevents its own timer from starting + # ("unit to trigger not loaded"), so the trigger .service units are listed here + # too even though we never run them directly. + # + # Consequence worth knowing: unmasking and starting the timers here means the + # timers are STARTED mid-catalog, so systemd evaluates their Persistent=true + # backlog at that moment. That is what made infrahouse-ubuntu-pro's stale timer + # stamps fire a catch-up unattended-upgrade during the Puppet run. $unmask_units = [ 'unattended-upgrades.service', 'apt-daily.service', diff --git a/environments/sandbox/modules/profile/manifests/unattended_upgrades.pp b/environments/sandbox/modules/profile/manifests/unattended_upgrades.pp index 745dc25..d324501 100644 --- a/environments/sandbox/modules/profile/manifests/unattended_upgrades.pp +++ b/environments/sandbox/modules/profile/manifests/unattended_upgrades.pp @@ -1,8 +1,27 @@ # @summary: Enable unattended-upgrades fleet-wide for automatic security updates. # -# Included by profile::base so every host receives security fixes. The base -# AMI ships the relevant systemd units masked; this profile unmasks and enables -# them and owns their configuration via Puppet. +# Included by profile::base so every host receives security fixes. This profile +# owns the unattended-upgrades configuration and asserts that the relevant units +# stay installed, enabled and running. +# +# The units ARE masked by the time this class runs, but the masking comes from +# cloud-init, not the AMI: terraform-aws-cloud-init's bootcmd runs +# `systemctl stop` + `systemctl mask` on apt-daily{,-upgrade}.{service,timer} and +# unattended-upgrades.service on every boot, before runcmd starts ih-puppet +# (terraform-aws-cloud-init#87 -- those timers race Puppet for the dpkg lock). +# The execs below undo it on every run, so the mask/unmask cycle repeats per boot. +# +# That masking predates vulnerability management and is obsolete policy. It is +# settled that unattended-upgrades IS wanted on these hosts and that Puppet is +# authoritative for it, so the unmask is deliberate, not a workaround. The +# cloud-init side will stop masking (terraform-aws-cloud-init#91); until it does, +# the execs below are what keeps unattended-upgrades running. Do not remove them +# before that lands -- and once it does, they become no-ops and can go. +# +# Meanwhile there is a small bounded patching gap: bootcmd masks on every boot, +# but runcmd/Puppet only runs at provisioning, so after a reboot of a long-lived +# instance the units stay masked until the next scheduled puppet apply -- at most +# ~30 min, since profile::puppet_apply runs at $m and $m+30. # # Hosts that must not be disrupted by an automatic service restart (e.g. # Elasticsearch nodes) keep unattended-upgrades running but drop their own @@ -19,10 +38,22 @@ # Units that drive automatic upgrades: # - the timers run the periodic download + upgrade # - unattended-upgrades.service applies pending upgrades on shutdown/boot - # The base AMI ships these masked. A masked apt-daily.service also prevents - # its timer from starting ("unit to trigger not loaded"), so the trigger - # .service units must be unmasked too even though we never run them directly. - # Puppet's service provider cannot unmask a unit, hence the execs. + # + # These arrive masked from cloud-init's bootcmd (see the class docstring), so + # the execs below are load-bearing, not defensive. Without them + # Service[$enabled_units] fails, because Puppet's service provider can neither + # start nor enable a masked unit -- and a failed resource makes ih-puppet exit + # 4 or 6 under --detailed-exitcodes, which trips ih-bootstrap's ERR trap and + # ABANDONs the instance. The service provider cannot unmask either, hence execs. + # + # A masked apt-daily.service additionally prevents its own timer from starting + # ("unit to trigger not loaded"), so the trigger .service units are listed here + # too even though we never run them directly. + # + # Consequence worth knowing: unmasking and starting the timers here means the + # timers are STARTED mid-catalog, so systemd evaluates their Persistent=true + # backlog at that moment. That is what made infrahouse-ubuntu-pro's stale timer + # stamps fire a catch-up unattended-upgrade during the Puppet run. $unmask_units = [ 'unattended-upgrades.service', 'apt-daily.service', diff --git a/modules/profile/manifests/unattended_upgrades.pp b/modules/profile/manifests/unattended_upgrades.pp index 745dc25..d324501 100644 --- a/modules/profile/manifests/unattended_upgrades.pp +++ b/modules/profile/manifests/unattended_upgrades.pp @@ -1,8 +1,27 @@ # @summary: Enable unattended-upgrades fleet-wide for automatic security updates. # -# Included by profile::base so every host receives security fixes. The base -# AMI ships the relevant systemd units masked; this profile unmasks and enables -# them and owns their configuration via Puppet. +# Included by profile::base so every host receives security fixes. This profile +# owns the unattended-upgrades configuration and asserts that the relevant units +# stay installed, enabled and running. +# +# The units ARE masked by the time this class runs, but the masking comes from +# cloud-init, not the AMI: terraform-aws-cloud-init's bootcmd runs +# `systemctl stop` + `systemctl mask` on apt-daily{,-upgrade}.{service,timer} and +# unattended-upgrades.service on every boot, before runcmd starts ih-puppet +# (terraform-aws-cloud-init#87 -- those timers race Puppet for the dpkg lock). +# The execs below undo it on every run, so the mask/unmask cycle repeats per boot. +# +# That masking predates vulnerability management and is obsolete policy. It is +# settled that unattended-upgrades IS wanted on these hosts and that Puppet is +# authoritative for it, so the unmask is deliberate, not a workaround. The +# cloud-init side will stop masking (terraform-aws-cloud-init#91); until it does, +# the execs below are what keeps unattended-upgrades running. Do not remove them +# before that lands -- and once it does, they become no-ops and can go. +# +# Meanwhile there is a small bounded patching gap: bootcmd masks on every boot, +# but runcmd/Puppet only runs at provisioning, so after a reboot of a long-lived +# instance the units stay masked until the next scheduled puppet apply -- at most +# ~30 min, since profile::puppet_apply runs at $m and $m+30. # # Hosts that must not be disrupted by an automatic service restart (e.g. # Elasticsearch nodes) keep unattended-upgrades running but drop their own @@ -19,10 +38,22 @@ # Units that drive automatic upgrades: # - the timers run the periodic download + upgrade # - unattended-upgrades.service applies pending upgrades on shutdown/boot - # The base AMI ships these masked. A masked apt-daily.service also prevents - # its timer from starting ("unit to trigger not loaded"), so the trigger - # .service units must be unmasked too even though we never run them directly. - # Puppet's service provider cannot unmask a unit, hence the execs. + # + # These arrive masked from cloud-init's bootcmd (see the class docstring), so + # the execs below are load-bearing, not defensive. Without them + # Service[$enabled_units] fails, because Puppet's service provider can neither + # start nor enable a masked unit -- and a failed resource makes ih-puppet exit + # 4 or 6 under --detailed-exitcodes, which trips ih-bootstrap's ERR trap and + # ABANDONs the instance. The service provider cannot unmask either, hence execs. + # + # A masked apt-daily.service additionally prevents its own timer from starting + # ("unit to trigger not loaded"), so the trigger .service units are listed here + # too even though we never run them directly. + # + # Consequence worth knowing: unmasking and starting the timers here means the + # timers are STARTED mid-catalog, so systemd evaluates their Persistent=true + # backlog at that moment. That is what made infrahouse-ubuntu-pro's stale timer + # stamps fire a catch-up unattended-upgrade during the Puppet run. $unmask_units = [ 'unattended-upgrades.service', 'apt-daily.service', From 90796bf7a6ffafdf914b2c43779b91044cbbcb56 Mon Sep 17 00:00:00 2001 From: Oleksandr Kuzminskyi Date: Fri, 31 Jul 2026 16:48:40 -0700 Subject: [PATCH 2/2] Simplify profile::apt_lock_timeout's parameter Integer(pick_default(...)) parses fine on Puppet 8 -- verified with `puppet parser validate` on 8.4.0 -- but the Puppet 4/5-era grammars still bundled in editor language servers reject it: after a bare capitalized name, which is a Type reference, that grammar accepts `[`, an operator, `<| |>`, CONSUMES or PRODUCES, but not `(`. It surfaced as a permanent false error in the IDE. Dropped the conversion and the type rather than swapping in a stricter type. $timeout is only interpolated into the drop-in's content, where an Integer and a String are indistinguishable, so there was never anything to convert or validate. This also matches the existing idiom for fact-sourced parameters -- see $deregistration_hookname in profile::github_runner::service. Refs #289 Co-Authored-By: Claude Opus 5 (1M context) --- debian/changelog | 6 ++++++ .../modules/profile/manifests/apt_lock_timeout.pp | 5 ++++- 2 files changed, 10 insertions(+), 1 deletion(-) diff --git a/debian/changelog b/debian/changelog index 5c4f836..eabc465 100644 --- a/debian/changelog +++ b/debian/changelog @@ -1,3 +1,9 @@ +puppet-code (0.1.0-1build321) noble; urgency=medium + + * commit event. see changes history in git log + + -- root Fri, 31 Jul 2026 23:48:42 +0000 + puppet-code (0.1.0-1build320) noble; urgency=medium * commit event. see changes history in git log diff --git a/environments/development/modules/profile/manifests/apt_lock_timeout.pp b/environments/development/modules/profile/manifests/apt_lock_timeout.pp index b67f96c..454a519 100644 --- a/environments/development/modules/profile/manifests/apt_lock_timeout.pp +++ b/environments/development/modules/profile/manifests/apt_lock_timeout.pp @@ -27,11 +27,14 @@ # custom fact (set via the cloud-init module's custom_facts), defaulting to the # 300 that current AMIs ship. # +# Interpolated as-is, so it makes no difference whether the fact arrives as an +# Integer or a String. +# # Keep it well inside the gha_runner bootstrap lifecycle hook (1200s, # default_result ABANDON): a genuinely wedged lock costs this many seconds per # Package resource, and the hook is not renewed during bootstrap. class profile::apt_lock_timeout ( - Integer[1] $timeout = Integer(pick_default($facts['apt_lock_timeout'], 300)), + $timeout = pick_default($facts['apt_lock_timeout'], 300), ) { file { '/etc/apt/apt.conf.d/99-lock-timeout':