From 7eab0e4618728c0663c0c60aa07cc31f53e477f7 Mon Sep 17 00:00:00 2001 From: jcardozo-eth <131674798+jcardozo-eth@users.noreply.github.com> Date: Thu, 23 Jul 2026 00:12:35 +0200 Subject: [PATCH 01/12] docs(init): scope the socket escape-hatch comment to DOCKER_HOST The socket-derivation comment claimed an "exotic external socket is expressible via runner_service.environment", which overstates the reach. That environment only sets the manager service drop-in's DOCKER_HOST; the socket_mount volume and the healthcheck probe keep using the uid-derived path. Reword to state the escape hatch narrowly so it is not read as a general external-daemon switch. --- manifests/init.pp | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/manifests/init.pp b/manifests/init.pp index af4fd39..8442408 100644 --- a/manifests/init.pp +++ b/manifests/init.pp @@ -315,8 +315,11 @@ # Rootless runtime paths derived from the uid where known: the module # itself installs the daemon socket at /run/user//docker.sock, so the - # path is derivation, not configuration. An exotic external socket is - # expressible via runner_service.environment. + # path is derivation, not configuration. The one escape hatch is narrow: + # runner_service.environment can point the manager service's DOCKER_HOST at + # a socket elsewhere. Nothing else follows it — the socket_mount volume and + # the healthcheck probe keep using the derived path — so it is not a general + # "external daemon" switch. if $runner_account['uid'] =~ Integer { $runtime_dir = "/run/user/${runner_account['uid']}" $socket_path = "${runtime_dir}/docker.sock" From 1131e3a05f8eb4a9aa4f5751824d183ae05bf8d3 Mon Sep 17 00:00:00 2001 From: jcardozo-eth <131674798+jcardozo-eth@users.noreply.github.com> Date: Thu, 23 Jul 2026 00:17:30 +0200 Subject: [PATCH 02/12] docs(healthcheck): note the manager-service unit in the template Add a non-rendered EPP comment at the top of the healthcheck template recording that the manager-service check asserts the packaged gitlab-runner systemd unit (systemctl is-active gitlab-runner), the name the package defines rather than a module-derived value. Source-only, so the rendered script and its golden are unchanged. --- templates/healthcheck.sh.epp | 3 +++ 1 file changed, 3 insertions(+) diff --git a/templates/healthcheck.sh.epp b/templates/healthcheck.sh.epp index 6921806..73a702a 100644 --- a/templates/healthcheck.sh.epp +++ b/templates/healthcheck.sh.epp @@ -4,6 +4,9 @@ String $control_repository_path, String $control_repository_branch, | -%> +<%# The manager-service check below asserts the packaged gitlab-runner systemd + unit (systemctl is-active --quiet gitlab-runner): the unit name the + gitlab-runner package defines, not a module-derived value. Not rendered. -%> #!/usr/bin/env bash # # Managed by Puppet (rootless_gitlab_runner). Do not edit by hand — this file From f12d5d8b8062eee5fca97af76463567437f96d01 Mon Sep 17 00:00:00 2001 From: jcardozo-eth <131674798+jcardozo-eth@users.noreply.github.com> Date: Thu, 23 Jul 2026 00:20:29 +0200 Subject: [PATCH 03/12] refactor: derive the manager service name once and share it The manager's systemd unit name ('gitlab-runner') was a literal in service.pp and again in the healthcheck template. Derive it once as $service_name in init.pp and thread it into both. Value unchanged, so the compiled catalog and the healthcheck golden are byte-identical. --- manifests/init.pp | 5 +++++ manifests/self_update.pp | 1 + manifests/service.pp | 2 +- templates/healthcheck.sh.epp | 9 +++++---- 4 files changed, 12 insertions(+), 5 deletions(-) diff --git a/manifests/init.pp b/manifests/init.pp index 8442408..9dda0b5 100644 --- a/manifests/init.pp +++ b/manifests/init.pp @@ -353,6 +353,11 @@ default => $runner_service['environment'], } + # The runner manager's systemd unit name is the one the gitlab-runner package + # defines, not configuration. Derived once and shared by the service class + # and the healthcheck so the literal has a single home. + $service_name = 'gitlab-runner' + contain rootless_gitlab_runner::apt_repos contain rootless_gitlab_runner::packages contain rootless_gitlab_runner::user diff --git a/manifests/self_update.pp b/manifests/self_update.pp index d04e76e..59f6741 100644 --- a/manifests/self_update.pp +++ b/manifests/self_update.pp @@ -54,6 +54,7 @@ 'socket_path' => $rootless_gitlab_runner::socket_path, 'control_repository_path' => $control_repository_path, 'control_repository_branch' => $standalone['control_repository_branch'], + 'service_name' => $rootless_gitlab_runner::service_name, }), } diff --git a/manifests/service.pp b/manifests/service.pp index 4de3bf3..54b7498 100644 --- a/manifests/service.pp +++ b/manifests/service.pp @@ -8,7 +8,7 @@ # as root would contradict the module's identity — and the service name is # the package-defined unit, not configuration. if $rootless_gitlab_runner::runner_service['manage'] { - $service_name = 'gitlab-runner' + $service_name = $rootless_gitlab_runner::service_name $service_dropin_dir = "/etc/systemd/system/${service_name}.service.d" file { $service_dropin_dir: diff --git a/templates/healthcheck.sh.epp b/templates/healthcheck.sh.epp index 73a702a..547f395 100644 --- a/templates/healthcheck.sh.epp +++ b/templates/healthcheck.sh.epp @@ -3,10 +3,11 @@ String $socket_path, String $control_repository_path, String $control_repository_branch, + String $service_name, | -%> <%# The manager-service check below asserts the packaged gitlab-runner systemd - unit (systemctl is-active --quiet gitlab-runner): the unit name the - gitlab-runner package defines, not a module-derived value. Not rendered. -%> + unit: $service_name is the unit name the gitlab-runner package defines, + threaded in from the manifest rather than restated here. Not rendered. -%> #!/usr/bin/env bash # # Managed by Puppet (rootless_gitlab_runner). Do not edit by hand — this file @@ -33,8 +34,8 @@ as_runner() { "$@" } -systemctl is-active --quiet 'gitlab-runner' \ - || err "manager service 'gitlab-runner' is not active" +systemctl is-active --quiet '<%= $service_name %>' \ + || err "manager service '<%= $service_name %>' is not active" # The apply timer drives self-update; a disabled or stopped timer silently # halts convergence and would otherwise surface only later as checkout From c8c9f2dcadd7f0d39bf2c0bb0ab48a4fec047d4b Mon Sep 17 00:00:00 2001 From: jcardozo-eth <131674798+jcardozo-eth@users.noreply.github.com> Date: Thu, 23 Jul 2026 00:24:00 +0200 Subject: [PATCH 04/12] refactor: derive the systemd user unit directory once, bottom-up config.pp built the ~/.config/systemd/user drop-in parent chain top-down with dirname(), and rootless_docker.pp hardcoded the same path for the setuptool's generated docker.service. Derive the chain once in init.pp, built bottom-up (each directory its parent plus one element), and share $user_systemd_dir with both. Paths are unchanged, so the compiled catalog is byte-identical. --- manifests/config.pp | 20 ++++++++++++-------- manifests/init.pp | 10 ++++++++++ manifests/rootless_docker.pp | 6 ++++-- 3 files changed, 26 insertions(+), 10 deletions(-) diff --git a/manifests/config.pp b/manifests/config.pp index 8029fff..5950315 100644 --- a/manifests/config.pp +++ b/manifests/config.pp @@ -138,14 +138,18 @@ # side effects — and Puppet's file type never creates parents. Own the whole # parent chain explicitly (ordered after the bring-up by the class order), # so the first apply on a fresh host can always place the drop-in, even if a - # future setuptool changes what it creates. Puppet autorequires each managed - # parent, so the chain applies top-down. - $dropin_path = "${runner_home}/.config/systemd/user/docker.service.d/no-detach-netns.conf" - $dropin_dir = dirname($dropin_path) - $user_units_dir = dirname($dropin_dir) - $user_systemd_dir = dirname($user_units_dir) - $user_config_dir = dirname($user_systemd_dir) - file { [$user_config_dir, $user_systemd_dir, $user_units_dir, $dropin_dir]: + # future setuptool changes what it creates. The chain is derived bottom-up in + # init.pp (each directory its parent plus one element) and shared with the + # rootless bring-up; Puppet autorequires each managed parent, so it still + # applies top-down. Only the drop-in directory and file are leaves here. + $dropin_dir = "${rootless_gitlab_runner::user_systemd_dir}/docker.service.d" + $dropin_path = "${dropin_dir}/no-detach-netns.conf" + file { [ + $rootless_gitlab_runner::user_config_dir, + $rootless_gitlab_runner::user_config_systemd_dir, + $rootless_gitlab_runner::user_systemd_dir, + $dropin_dir, + ]: ensure => directory, owner => $runner_name, group => $runner_group, diff --git a/manifests/init.pp b/manifests/init.pp index 9dda0b5..96b7a09 100644 --- a/manifests/init.pp +++ b/manifests/init.pp @@ -358,6 +358,16 @@ # and the healthcheck so the literal has a single home. $service_name = 'gitlab-runner' + # The runner account's systemd user unit directory (~/.config/systemd/user) + # and its parents, built bottom-up from the home so each directory is its + # parent plus one path element (no dirname() walk back up). Puppet's file + # type never creates parents, so config.pp manages the whole chain to place + # the no-detach-netns drop-in; rootless_docker.pp reads $user_systemd_dir for + # the docker.service the setuptool generates there. + $user_config_dir = "${runner_account['home']}/.config" + $user_config_systemd_dir = "${user_config_dir}/systemd" + $user_systemd_dir = "${user_config_systemd_dir}/user" + contain rootless_gitlab_runner::apt_repos contain rootless_gitlab_runner::packages contain rootless_gitlab_runner::user diff --git a/manifests/rootless_docker.pp b/manifests/rootless_docker.pp index e0008dc..3bdb474 100644 --- a/manifests/rootless_docker.pp +++ b/manifests/rootless_docker.pp @@ -13,6 +13,8 @@ # defines; if it is absent the install exec below fails loud (a missing # command is a command failure, not a guard failure). $setuptool = '/usr/bin/dockerd-rootless-setuptool.sh' + # The user unit the setuptool generates, under the shared systemd user dir. + $docker_user_unit = "${rootless_gitlab_runner::user_systemd_dir}/docker.service" # A fresh docker-ce install leaves the rootful system daemon running: the # package postinst starts docker.service + docker.socket as root, and @@ -128,8 +130,8 @@ # success. Assert the user unit exists after install, turning a silent skip # into a hard failure. exec { 'rootless_gitlab_runner setuptool install': - command => "${setuptool} install && test -f ${runner_home}/.config/systemd/user/docker.service", - creates => "${runner_home}/.config/systemd/user/docker.service", + command => "${setuptool} install && test -f ${docker_user_unit}", + creates => $docker_user_unit, user => $runner_user, environment => $user_env, path => ['/usr/bin', '/bin'], From e62aa8b06e2d92394553f1b37b00a97539ccd542 Mon Sep 17 00:00:00 2001 From: jcardozo-eth <131674798+jcardozo-eth@users.noreply.github.com> Date: Thu, 23 Jul 2026 00:26:28 +0200 Subject: [PATCH 05/12] refactor: derive the Puppetfile path beside the other apply paths The apply template built the Puppetfile path inline from the control repository path, twice, while its siblings (manifest, module directory, Hiera config) arrive pre-derived from the manifest. Derive puppetfile_path in self_update.pp and pass it in, and record the Puppetfile in the control_repository_path option doc. Rendered script and catalog unchanged. --- REFERENCE.md | 9 +++++---- manifests/init.pp | 9 +++++---- manifests/self_update.pp | 12 ++++++------ templates/apply.sh.epp | 6 +++--- 4 files changed, 19 insertions(+), 17 deletions(-) diff --git a/REFERENCE.md b/REFERENCE.md index 3067e6f..59c4843 100644 --- a/REFERENCE.md +++ b/REFERENCE.md @@ -360,10 +360,11 @@ Options: (`/usr/local/sbin/rootless-gitlab-runner-apply`), the single definition of the apply command for timer-driven and manual runs. Default false. * **:control_repository_path** `Stdlib::Absolutepath`: Root-owned checkout of the control repository on the host (the apply and -self-update target). The apply's manifest, module directory and Hiera -configuration derive strictly from the documented layout beneath it -(`puppet/manifests/site.pp`, `puppet/modules`, `puppet/hiera.yaml`). -Default `/opt/gitlab-runner-infra`. +self-update target). The apply's manifest, module directory, Hiera +configuration and optional `Puppetfile` derive strictly from the documented +layout beneath it (`puppet/manifests/site.pp`, `puppet/modules`, +`puppet/hiera.yaml`, and `Puppetfile` at the root — r10k installs it each +tick when present). Default `/opt/gitlab-runner-infra`. * **:control_repository_branch** `String[1]`: Branch the self-update loop follows (protected, signed). Default `main`. * **:puppet_confdir** `Stdlib::Absolutepath`: Isolated Puppet confdir for the apply — never the central agent's. Default `/etc/gitlab-runner-infra/puppet`. diff --git a/manifests/init.pp b/manifests/init.pp index 96b7a09..9b56e46 100644 --- a/manifests/init.pp +++ b/manifests/init.pp @@ -196,10 +196,11 @@ # of the apply command for timer-driven and manual runs. Default false. # @option standalone [Stdlib::Absolutepath] :control_repository_path # Root-owned checkout of the control repository on the host (the apply and -# self-update target). The apply's manifest, module directory and Hiera -# configuration derive strictly from the documented layout beneath it -# (`puppet/manifests/site.pp`, `puppet/modules`, `puppet/hiera.yaml`). -# Default `/opt/gitlab-runner-infra`. +# self-update target). The apply's manifest, module directory, Hiera +# configuration and optional `Puppetfile` derive strictly from the documented +# layout beneath it (`puppet/manifests/site.pp`, `puppet/modules`, +# `puppet/hiera.yaml`, and `Puppetfile` at the root — r10k installs it each +# tick when present). Default `/opt/gitlab-runner-infra`. # @option standalone [String[1]] :control_repository_branch # Branch the self-update loop follows (protected, signed). Default `main`. # @option standalone [Stdlib::Absolutepath] :puppet_confdir diff --git a/manifests/self_update.pp b/manifests/self_update.pp index 59f6741..5a74e49 100644 --- a/manifests/self_update.pp +++ b/manifests/self_update.pp @@ -20,12 +20,12 @@ group => 'root', mode => '0755', content => epp('rootless_gitlab_runner/apply.sh.epp', { - 'control_repository_path' => $control_repository_path, - 'manifest_path' => "${control_repository_path}/puppet/manifests/site.pp", - 'module_directory' => "${control_repository_path}/puppet/modules", - 'hiera_config' => "${control_repository_path}/puppet/hiera.yaml", - 'confdir' => $standalone['puppet_confdir'], - 'vardir' => $standalone['puppet_vardir'], + 'puppetfile_path' => "${control_repository_path}/Puppetfile", + 'manifest_path' => "${control_repository_path}/puppet/manifests/site.pp", + 'module_directory' => "${control_repository_path}/puppet/modules", + 'hiera_config' => "${control_repository_path}/puppet/hiera.yaml", + 'confdir' => $standalone['puppet_confdir'], + 'vardir' => $standalone['puppet_vardir'], }), } } diff --git a/templates/apply.sh.epp b/templates/apply.sh.epp index 30101d9..9e17974 100644 --- a/templates/apply.sh.epp +++ b/templates/apply.sh.epp @@ -1,4 +1,4 @@ -<%- | String $control_repository_path, +<%- | String $puppetfile_path, String $manifest_path, String $module_directory, String $hiera_config, @@ -25,9 +25,9 @@ fi # Deliver module updates pinned in the consumer's Puppetfile each tick. # Without a Puppetfile (module vendored in the checkout) this is a no-op. -if [[ -f "<%= $control_repository_path %>/Puppetfile" ]]; then +if [[ -f "<%= $puppetfile_path %>" ]]; then r10k puppetfile install \ - --puppetfile "<%= $control_repository_path %>/Puppetfile" \ + --puppetfile "<%= $puppetfile_path %>" \ --moduledir "<%= $module_directory %>" fi From 158e2da8885d5417b03678895a47985bb166a70d Mon Sep 17 00:00:00 2001 From: jcardozo-eth <131674798+jcardozo-eth@users.noreply.github.com> Date: Thu, 23 Jul 2026 00:27:36 +0200 Subject: [PATCH 06/12] docs(init): explain the total selector's empty-environment arm The $real_service_environment selector's inner `undef => []` arm looks like it could render an empty Environment. Note that it is reached only when runner_service.manage is off (the guard above rejects that case while the service is managed), so [] is the never-consumed value that keeps the selector total, not a rendered empty Environment. --- manifests/init.pp | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/manifests/init.pp b/manifests/init.pp index 9b56e46..dee7ae3 100644 --- a/manifests/init.pp +++ b/manifests/init.pp @@ -346,6 +346,11 @@ ])) } + # The inner `undef => []` arm (environment unset and no derivable socket) is + # reached only when runner_service.manage is off: the fail-loud guard above + # rejects that exact combination while the service is managed. So [] is the + # never-consumed value that keeps the selector total — service.pp renders the + # drop-in only under manage — not a deliberately empty Environment. $real_service_environment = $runner_service['environment'] ? { undef => $socket_path ? { undef => [], From 76fcd563e3bd5675497eb87295f19a115c5568c9 Mon Sep 17 00:00:00 2001 From: jcardozo-eth <131674798+jcardozo-eth@users.noreply.github.com> Date: Thu, 23 Jul 2026 00:35:22 +0200 Subject: [PATCH 07/12] docs(init): align option doc types with the enforced structs The runner_service.environment option was documented as Array[String] while the struct enforces Array[Pattern[/\A[^\r\n]+\z/]], and standalone.self_update as a bare Hash while the struct spells out manage/apply_interval/apply_timeout. Match the doc types to the enforced types so REFERENCE states the real contract. --- REFERENCE.md | 10 +++++----- manifests/init.pp | 10 +++++----- 2 files changed, 10 insertions(+), 10 deletions(-) diff --git a/REFERENCE.md b/REFERENCE.md index 59c4843..7393517 100644 --- a/REFERENCE.md +++ b/REFERENCE.md @@ -317,11 +317,11 @@ Options: * **:manage** `Boolean`: Whether to manage the runner system service, its privilege-drop systemd drop-in, and the configuration directory's mode so a privilege-dropped manager can traverse to its configuration file. Default false. -* **:environment** `Optional[Array[String]]`: `Environment=` lines (KEY=value) rendered into the service drop-in. When +* **:environment** `Optional[Array[Pattern[/\A[^\r\n]+\z/]]]`: `Environment=` lines (KEY=value) rendered into the service drop-in. When unset, defaults to pointing DOCKER_HOST at the rootless docker socket -derived from `runner_account.uid`. Each line must be a single line — a -value containing a newline is rejected, so it cannot inject an extra -systemd directive into the drop-in. +derived from `runner_account.uid`. The `Pattern` enforces the type: each +line must be non-empty and single-line — a value containing a newline is +rejected, so it cannot inject an extra systemd directive into the drop-in. * **:timeout_stop_sec** `Optional[Variant[Integer[0], String[1]]]`: `TimeoutStopSec=` written into the manager service drop-in — how long systemd waits for a graceful drain before escalating to `SIGKILL`. Unset by default, so systemd's `DefaultTimeoutStopSec` (typically 90s) applies; @@ -376,7 +376,7 @@ non-login apply can find them. Default `/opt/puppetlabs/bin`; the AIO-reuse topology sets `/opt/puppetlabs/puppet/bin` (gem executables live there, not in the symlink farm). * **:healthcheck_interval** `String[1]`: systemd time span between healthcheck runs. Default `15min`. -* **:self_update** `Hash`: The self-update loop: `manage` (default false) installs a oneshot +* **:self_update** `Struct[{ manage => Boolean, apply_interval => String[1], apply_timeout => String[1] }]`: The self-update loop: `manage` (default false) installs a oneshot service + timer that fetch the control repository, verify the commit signature, reset to the remote branch, run `r10k puppetfile install` and re-apply — plus the healthcheck script + timer. Only valid on a diff --git a/manifests/init.pp b/manifests/init.pp index dee7ae3..da85624 100644 --- a/manifests/init.pp +++ b/manifests/init.pp @@ -173,12 +173,12 @@ # Whether to manage the runner system service, its privilege-drop systemd # drop-in, and the configuration directory's mode so a privilege-dropped # manager can traverse to its configuration file. Default false. -# @option runner_service [Optional[Array[String]]] :environment +# @option runner_service [Optional[Array[Pattern[/\A[^\r\n]+\z/]]]] :environment # `Environment=` lines (KEY=value) rendered into the service drop-in. When # unset, defaults to pointing DOCKER_HOST at the rootless docker socket -# derived from `runner_account.uid`. Each line must be a single line — a -# value containing a newline is rejected, so it cannot inject an extra -# systemd directive into the drop-in. +# derived from `runner_account.uid`. The `Pattern` enforces the type: each +# line must be non-empty and single-line — a value containing a newline is +# rejected, so it cannot inject an extra systemd directive into the drop-in. # @option runner_service [Optional[Variant[Integer[0], String[1]]]] :timeout_stop_sec # `TimeoutStopSec=` written into the manager service drop-in — how long # systemd waits for a graceful drain before escalating to `SIGKILL`. Unset @@ -217,7 +217,7 @@ # live there, not in the symlink farm). # @option standalone [String[1]] :healthcheck_interval # systemd time span between healthcheck runs. Default `15min`. -# @option standalone [Hash] :self_update +# @option standalone [Struct[{ manage => Boolean, apply_interval => String[1], apply_timeout => String[1] }]] :self_update # The self-update loop: `manage` (default false) installs a oneshot # service + timer that fetch the control repository, verify the commit # signature, reset to the remote branch, run `r10k puppetfile install` and From 4fc1e143b4a79a2f47682a06df7ec181acfc0d2c Mon Sep 17 00:00:00 2001 From: jcardozo-eth <131674798+jcardozo-eth@users.noreply.github.com> Date: Thu, 23 Jul 2026 00:38:55 +0200 Subject: [PATCH 08/12] test: merge struct_param via the deep_merge gem Hiera uses The struct_param helper mirrors the module's deep-merge lookup_options so resource-style overrides merge over the module defaults the way a Hiera consumer's partial hash would. The hand-rolled recursive merge replaced whole arrays; Hiera's deep strategy unions them and applies the '--' knockout prefix. Use the deep_merge gem that strategy is built on so the helper matches Hiera, deep-copying both operands since deep_merge! mutates its destination. --- spec/classes/rootless_gitlab_runner_spec.rb | 18 ++++++++++-------- 1 file changed, 10 insertions(+), 8 deletions(-) diff --git a/spec/classes/rootless_gitlab_runner_spec.rb b/spec/classes/rootless_gitlab_runner_spec.rb index 17c88a4..7e1b25a 100644 --- a/spec/classes/rootless_gitlab_runner_spec.rb +++ b/spec/classes/rootless_gitlab_runner_spec.rb @@ -1,5 +1,6 @@ require 'spec_helper' require 'yaml' +require 'deep_merge' # puppetlabs-apt compiles only on Debian-family facts; the suite otherwise runs # factless, so the apt-enabled and example contexts pin the single supported @@ -33,12 +34,6 @@ def example_yaml(*path) YAML.safe_load(File.read(File.expand_path(File.join(__dir__, '..', '..', 'examples', *path)))) end - def deep_merge(base, override) - base.merge(override) do |_k, old, new| - old.is_a?(Hash) && new.is_a?(Hash) ? deep_merge(old, new) : new - end - end - # rspec-puppet serializes a Ruby nil param value as the literal string # "nil"; a YAML `~` default must reach Puppet as undef instead. def undefize(value) @@ -50,9 +45,16 @@ def undefize(value) end # A struct parameter's module-data default with overrides merged deep over - # it, mirroring the deep-merge lookup_options a Hiera consumer gets. + # it, mirroring the deep-merge lookup_options a Hiera consumer gets (strategy: + # deep, knockout_prefix: '--'): scalars from the override win, sub-hashes + # merge recursively, arrays union across layers, and a '--'-prefixed element + # knocks the matching one out. Uses the same deep_merge gem Hiera's deep + # strategy is built on, so struct_param merges the way Hiera would rather than + # replacing whole arrays. Both operands are deep-copied first: deep_merge! + # mutates its destination, and MODULE_DATA must stay pristine across calls. def struct_param(name, overrides = {}) - undefize(deep_merge(MODULE_DATA.fetch("rootless_gitlab_runner::#{name}"), overrides)) + base = Marshal.load(Marshal.dump(MODULE_DATA.fetch("rootless_gitlab_runner::#{name}"))) + undefize(DeepMerge.deep_merge!(Marshal.load(Marshal.dump(overrides)), base, knockout_prefix: '--')) end context 'with defaults' do From 2d2400d17a0d92a216f86dfe3925ebd4f8c9a007 Mon Sep 17 00:00:00 2001 From: jcardozo-eth <131674798+jcardozo-eth@users.noreply.github.com> Date: Thu, 23 Jul 2026 00:41:36 +0200 Subject: [PATCH 09/12] test: dedupe rendered_config and the runner_account-uid setup rendered_config duplicated rendered_file's Sensitive-unwrap logic and carried a path parameter no caller passed; define it in terms of rendered_file at the fixed default path. Collapse the repeated struct_param('runner_account', 'uid' => N) boilerplate into one account_with_uid helper. Test-only; the suite is unchanged. --- spec/classes/rootless_gitlab_runner_spec.rb | 47 +++++++++++---------- 1 file changed, 25 insertions(+), 22 deletions(-) diff --git a/spec/classes/rootless_gitlab_runner_spec.rb b/spec/classes/rootless_gitlab_runner_spec.rb index 7e1b25a..e838c6b 100644 --- a/spec/classes/rootless_gitlab_runner_spec.rb +++ b/spec/classes/rootless_gitlab_runner_spec.rb @@ -16,19 +16,18 @@ ).freeze describe 'rootless_gitlab_runner' do - # config.toml content is Sensitive (it carries tokens); unwrap it for byte - # and pattern assertions. - def rendered_config(path = '/etc/gitlab-runner/config.toml') - content = catalogue.resource('File', path)[:content] - content.respond_to?(:unwrap) ? content.unwrap : content - end - # The rendered content of any managed File resource (unwrapped if Sensitive). def rendered_file(path) content = catalogue.resource('File', path)[:content] content.respond_to?(:unwrap) ? content.unwrap : content end + # config.toml content is Sensitive (it carries tokens); rendered_file unwraps + # it for byte and pattern assertions. The path is the module default. + def rendered_config + rendered_file('/etc/gitlab-runner/config.toml') + end + # Load and parse a YAML file from examples/. def example_yaml(*path) YAML.safe_load(File.read(File.expand_path(File.join(__dir__, '..', '..', 'examples', *path)))) @@ -57,6 +56,12 @@ def struct_param(name, overrides = {}) undefize(DeepMerge.deep_merge!(Marshal.load(Marshal.dump(overrides)), base, knockout_prefix: '--')) end + # The common runner_account shape: a derivable uid (the socket path derives + # from it) plus any extra keys. Most contexts need only the uid. + def account_with_uid(uid, extra = {}) + struct_param('runner_account', { 'uid' => uid }.merge(extra)) + end + context 'with defaults' do it { is_expected.to compile.with_all_deps } @@ -435,7 +440,7 @@ def struct_param(name, overrides = {}) end context 'with runner_account.manage' do - let(:params) { { 'runner_account' => struct_param('runner_account', 'manage' => true, 'uid' => 4242) } } + let(:params) { { 'runner_account' => account_with_uid(4242, 'manage' => true) } } it { is_expected.to compile.with_all_deps } it { is_expected.to contain_group('gitlab-runner').with('ensure' => 'present', 'system' => true) } @@ -463,7 +468,7 @@ def struct_param(name, overrides = {}) # it as the user's primary group instead of a same-named group. context 'with runner_account.manage and a differently named primary group' do let(:params) do - { 'runner_account' => struct_param('runner_account', 'manage' => true, 'uid' => 4242, 'group' => 'ci') } + { 'runner_account' => account_with_uid(4242, 'manage' => true, 'group' => 'ci') } end it { is_expected.to compile.with_all_deps } @@ -478,7 +483,7 @@ def struct_param(name, overrides = {}) context 'with rootless_docker.manage' do let(:params) do { 'rootless_docker' => struct_param('rootless_docker', 'manage' => true), - 'runner_account' => struct_param('runner_account', 'uid' => 4242) } + 'runner_account' => account_with_uid(4242) } end it { is_expected.to compile.with_all_deps } @@ -586,7 +591,7 @@ def struct_param(name, overrides = {}) let(:params) do { 'rootless_docker' => struct_param('rootless_docker', 'manage' => true, 'subid_start' => 300_000, 'subid_count' => 131_072), - 'runner_account' => struct_param('runner_account', 'manage' => true, 'uid' => 4242) } + 'runner_account' => account_with_uid(4242, 'manage' => true) } end it { is_expected.to compile.with_all_deps } @@ -611,7 +616,7 @@ def struct_param(name, overrides = {}) let(:facts) { UBUNTU_FACTS } let(:params) do { - 'runner_account' => struct_param('runner_account', 'manage' => true, 'uid' => 4242), + 'runner_account' => account_with_uid(4242, 'manage' => true), 'rootless_docker' => struct_param('rootless_docker', 'manage' => true), 'runner_service' => struct_param('runner_service', 'manage' => true), 'packages' => struct_param('packages', 'sources' => { 'manage' => true }), @@ -634,7 +639,7 @@ def struct_param(name, overrides = {}) context 'with runner_service.manage' do let(:params) do { 'runner_service' => struct_param('runner_service', 'manage' => true), - 'runner_account' => struct_param('runner_account', 'uid' => 4242) } + 'runner_account' => account_with_uid(4242) } end it { is_expected.to compile.with_all_deps } @@ -698,8 +703,7 @@ def struct_param(name, overrides = {}) context 'with a non-default runner account name and home' do let(:params) do { - 'runner_account' => struct_param('runner_account', - 'name' => 'ci-worker', 'uid' => 5000, 'home' => '/srv/ci-worker'), + 'runner_account' => account_with_uid(5000, 'name' => 'ci-worker', 'home' => '/srv/ci-worker'), 'runner_service' => struct_param('runner_service', 'manage' => true), } end @@ -735,9 +739,8 @@ def struct_param(name, overrides = {}) # fails to resolve a group that does not exist. context 'with a differently named primary group' do let(:params) do - super().merge('runner_account' => struct_param('runner_account', - 'name' => 'ci-worker', 'uid' => 5000, - 'home' => '/srv/ci-worker', 'group' => 'ci')) + super().merge('runner_account' => account_with_uid(5000, 'name' => 'ci-worker', + 'home' => '/srv/ci-worker', 'group' => 'ci')) end it { is_expected.to compile.with_all_deps } @@ -790,7 +793,7 @@ def struct_param(name, overrides = {}) context 'with standalone.self_update.manage' do let(:params) do { 'standalone' => struct_param('standalone', 'manage' => true, 'self_update' => { 'manage' => true }), - 'runner_account' => struct_param('runner_account', 'uid' => 4242) } + 'runner_account' => account_with_uid(4242) } end it { is_expected.to compile.with_all_deps } @@ -928,7 +931,7 @@ def struct_param(name, overrides = {}) context 'golden file: full two-runner config' do let(:params) do { - 'runner_account' => struct_param('runner_account', 'uid' => 4242), + 'runner_account' => account_with_uid(4242), 'runner_tokens' => sensitive({ 'runner_a' => 'glrt-GOLDEN-TOKEN-A', 'runner_b' => 'glrt-GOLDEN-TOKEN-B' }), # url + executor deliberately live in runner_defaults: the golden file @@ -980,7 +983,7 @@ def struct_param(name, overrides = {}) context 'rendered shell scripts (golden + shellcheck)' do let(:params) do { 'standalone' => struct_param('standalone', 'manage' => true, 'self_update' => { 'manage' => true }), - 'runner_account' => struct_param('runner_account', 'uid' => 4242) } + 'runner_account' => account_with_uid(4242) } end { @@ -1045,7 +1048,7 @@ def struct_param(name, overrides = {}) 'runner_service' => struct_param('runner_service', 'manage' => true, 'environment' => ["DOCKER_HOST=unix:///run/x\nExecStartPre=/bin/evil"]), - 'runner_account' => struct_param('runner_account', 'uid' => 2000), + 'runner_account' => account_with_uid(2000), } end From 23a81739b0cd16cfb3f65b5419f1ec5df6094318 Mon Sep 17 00:00:00 2001 From: jcardozo-eth <131674798+jcardozo-eth@users.noreply.github.com> Date: Thu, 23 Jul 2026 00:43:56 +0200 Subject: [PATCH 10/12] refactor: hoist the runner account identity locals once The account name and home were re-subscripted out of runner_account in five files (config, user, service, self_update, rootless_docker). Hoist $runner_name and $runner_home once in init.pp, beside the existing $runner_group, and have each concern read the local. Values unchanged, so the compiled catalog is byte-identical. --- manifests/config.pp | 9 +++++---- manifests/init.pp | 25 ++++++++++++++----------- manifests/rootless_docker.pp | 4 ++-- manifests/self_update.pp | 2 +- manifests/service.pp | 4 ++-- manifests/user.pp | 6 +++--- 6 files changed, 27 insertions(+), 23 deletions(-) diff --git a/manifests/config.pp b/manifests/config.pp index 5950315..769b93a 100644 --- a/manifests/config.pp +++ b/manifests/config.pp @@ -3,11 +3,12 @@ class rootless_gitlab_runner::config { assert_private() - $runner_name = $rootless_gitlab_runner::runner_account['name'] - # Primary group, defaulting to the account name (derived in init.pp). Owners - # stay the account name; only the group ownerships derive from it. + # Identity locals hoisted in init.pp. Owners stay the account name; only the + # group ownerships derive from $runner_group (the account's primary group, + # defaulting to the name). + $runner_name = $rootless_gitlab_runner::runner_name $runner_group = $rootless_gitlab_runner::runner_group - $runner_home = $rootless_gitlab_runner::runner_account['home'] + $runner_home = $rootless_gitlab_runner::runner_home $configuration_file_path = $rootless_gitlab_runner::configuration_file['path'] $socket_path = $rootless_gitlab_runner::socket_path diff --git a/manifests/init.pp b/manifests/init.pp index da85624..27fed37 100644 --- a/manifests/init.pp +++ b/manifests/init.pp @@ -302,14 +302,17 @@ ])) } - # The account's primary group, derived once and read as a group by every - # concern (the group resource and the user's gid in user.pp, and every - # managed file's group in config.pp). The data layer cannot express "same as - # the account name", so the default is an absent key and the fallback lives - # here: an unset group follows the account name — correct by construction - # where the module creates the account — while a set group names the - # differently named primary group of an externally provisioned account. - $runner_group = pick($runner_account['group'], $runner_account['name']) + # The runner account's identity, hoisted once so every concern reads a local + # instead of re-subscripting runner_account per file. The primary group is + # read as a group everywhere the account name used to double as one (the + # group resource and the user's gid in user.pp, every managed file's group in + # config.pp); it defaults to the account name, since the data layer cannot + # express "same as the name" — an unset group falls back here, correct by + # construction where the module creates the account, while a set group names + # the differently named primary group of an externally provisioned account. + $runner_name = $runner_account['name'] + $runner_home = $runner_account['home'] + $runner_group = pick($runner_account['group'], $runner_name) # Defaults merged under every runner entry; keys set on the entry win. $effective_runners = $runners.map |$r| { $runner_defaults + $r } @@ -333,8 +336,8 @@ # root Puppet provides neither XDG_RUNTIME_DIR nor DBUS_SESSION_BUS_ADDRESS, # and systemctl --user / the setuptool fail without them. $runner_user_env = [ - "HOME=${runner_account['home']}", - "USER=${runner_account['name']}", + "HOME=${runner_home}", + "USER=${runner_name}", "XDG_RUNTIME_DIR=${runtime_dir}", "DBUS_SESSION_BUS_ADDRESS=unix:path=${runtime_dir}/bus", ] @@ -370,7 +373,7 @@ # type never creates parents, so config.pp manages the whole chain to place # the no-detach-netns drop-in; rootless_docker.pp reads $user_systemd_dir for # the docker.service the setuptool generates there. - $user_config_dir = "${runner_account['home']}/.config" + $user_config_dir = "${runner_home}/.config" $user_config_systemd_dir = "${user_config_dir}/systemd" $user_systemd_dir = "${user_config_systemd_dir}/user" diff --git a/manifests/rootless_docker.pp b/manifests/rootless_docker.pp index 3bdb474..ee0f66c 100644 --- a/manifests/rootless_docker.pp +++ b/manifests/rootless_docker.pp @@ -4,8 +4,8 @@ assert_private() if $rootless_gitlab_runner::rootless_docker['manage'] { - $runner_user = $rootless_gitlab_runner::runner_account['name'] - $runner_home = $rootless_gitlab_runner::runner_account['home'] + $runner_user = $rootless_gitlab_runner::runner_name + $runner_home = $rootless_gitlab_runner::runner_home $runner_uid = $rootless_gitlab_runner::runner_account['uid'] $runtime_dir = $rootless_gitlab_runner::runtime_dir $user_env = $rootless_gitlab_runner::runner_user_env diff --git a/manifests/self_update.pp b/manifests/self_update.pp index 5a74e49..c49e6eb 100644 --- a/manifests/self_update.pp +++ b/manifests/self_update.pp @@ -49,7 +49,7 @@ group => 'root', mode => '0755', content => epp('rootless_gitlab_runner/healthcheck.sh.epp', { - 'runner_name' => $rootless_gitlab_runner::runner_account['name'], + 'runner_name' => $rootless_gitlab_runner::runner_name, 'runtime_dir' => $rootless_gitlab_runner::runtime_dir, 'socket_path' => $rootless_gitlab_runner::socket_path, 'control_repository_path' => $control_repository_path, diff --git a/manifests/service.pp b/manifests/service.pp index 54b7498..3adca5f 100644 --- a/manifests/service.pp +++ b/manifests/service.pp @@ -34,8 +34,8 @@ group => 'root', mode => '0644', content => epp('rootless_gitlab_runner/service-dropin.conf.epp', { - 'runner_name' => $rootless_gitlab_runner::runner_account['name'], - 'runner_home' => $rootless_gitlab_runner::runner_account['home'], + 'runner_name' => $rootless_gitlab_runner::runner_name, + 'runner_home' => $rootless_gitlab_runner::runner_home, 'configuration_file_path' => $rootless_gitlab_runner::configuration_file['path'], 'environment' => $rootless_gitlab_runner::real_service_environment, 'timeout_stop_sec' => $rootless_gitlab_runner::runner_service['timeout_stop_sec'], diff --git a/manifests/user.pp b/manifests/user.pp index 7aaab10..ea87662 100644 --- a/manifests/user.pp +++ b/manifests/user.pp @@ -6,8 +6,8 @@ $account = $rootless_gitlab_runner::runner_account if $account['manage'] { - $runner_name = $account['name'] - # Primary group, defaulting to the account name (derived in init.pp). + # Identity locals hoisted in init.pp; only manage and uid stay on $account. + $runner_name = $rootless_gitlab_runner::runner_name $runner_group = $rootless_gitlab_runner::runner_group group { $runner_group: @@ -22,7 +22,7 @@ system => true, uid => $account['uid'], gid => $runner_group, - home => $account['home'], + home => $rootless_gitlab_runner::runner_home, managehome => true, shell => '/bin/bash', require => Group[$runner_group], From 282fdc5e8e95d497dea6651468de9473a14fe77a Mon Sep 17 00:00:00 2001 From: jcardozo-eth <131674798+jcardozo-eth@users.noreply.github.com> Date: Thu, 23 Jul 2026 00:44:51 +0200 Subject: [PATCH 11/12] refactor(types): extract the apt-source struct into a named alias The docker and gitlab_runner sources under packages.sources declared the same inline Struct[{ location, key_source }] twice in the class signature. Extract it as Rootless_gitlab_runner::Apt_source and reference the alias in both. Same type constraint, so the compiled catalog is byte-identical; REFERENCE regenerates with the alias. --- REFERENCE.md | 28 ++++++++++++++++++++-------- manifests/init.pp | 10 ++-------- types/apt_source.pp | 11 +++++++++++ 3 files changed, 33 insertions(+), 16 deletions(-) create mode 100644 types/apt_source.pp diff --git a/REFERENCE.md b/REFERENCE.md index 7393517..7a89a29 100644 --- a/REFERENCE.md +++ b/REFERENCE.md @@ -26,6 +26,7 @@ ### Data types +* [`Rootless_gitlab_runner::Apt_source`](#Rootless_gitlab_runner--Apt_source): An apt source's location and signing-key endpoint. * [`Rootless_gitlab_runner::Username`](#Rootless_gitlab_runner--Username): A strict POSIX-portable Linux username. ## Classes @@ -234,14 +235,8 @@ Struct[{ install => Array[String[1]], sources => Struct[{ manage => Boolean, - docker => Struct[{ - location => Stdlib::HTTPUrl, - key_source => Stdlib::HTTPUrl, - }], - gitlab_runner => Struct[{ - location => Stdlib::HTTPUrl, - key_source => Stdlib::HTTPUrl, - }], + docker => Rootless_gitlab_runner::Apt_source, + gitlab_runner => Rootless_gitlab_runner::Apt_source, }], }] ``` @@ -428,6 +423,23 @@ the raw string to escape ## Data types +### `Rootless_gitlab_runner::Apt_source` + +`location` and `key_source` are verbatim `apt::source` parameter names: the +repository URL (suite = OS codename) and the URL of its armored signing key, +stored as an apt keyring. Both the Docker and GitLab Runner sources under +`packages.sources` share this shape, pointing at a vendor repository or a +mirror. + +Alias of + +```puppet +Struct[{ + location => Stdlib::HTTPUrl, + key_source => Stdlib::HTTPUrl, +}] +``` + ### `Rootless_gitlab_runner::Username` The runner user is interpolated raw into shell execs (the subuid/subgid diff --git a/manifests/init.pp b/manifests/init.pp index 27fed37..d495651 100644 --- a/manifests/init.pp +++ b/manifests/init.pp @@ -247,14 +247,8 @@ install => Array[String[1]], sources => Struct[{ manage => Boolean, - docker => Struct[{ - location => Stdlib::HTTPUrl, - key_source => Stdlib::HTTPUrl, - }], - gitlab_runner => Struct[{ - location => Stdlib::HTTPUrl, - key_source => Stdlib::HTTPUrl, - }], + docker => Rootless_gitlab_runner::Apt_source, + gitlab_runner => Rootless_gitlab_runner::Apt_source, }], }] $packages, Struct[{ diff --git a/types/apt_source.pp b/types/apt_source.pp new file mode 100644 index 0000000..8ba3996 --- /dev/null +++ b/types/apt_source.pp @@ -0,0 +1,11 @@ +# @summary An apt source's location and signing-key endpoint. +# +# `location` and `key_source` are verbatim `apt::source` parameter names: the +# repository URL (suite = OS codename) and the URL of its armored signing key, +# stored as an apt keyring. Both the Docker and GitLab Runner sources under +# `packages.sources` share this shape, pointing at a vendor repository or a +# mirror. +type Rootless_gitlab_runner::Apt_source = Struct[{ + location => Stdlib::HTTPUrl, + key_source => Stdlib::HTTPUrl, +}] From ddd441e592ae1421d980c84c4d88c6e0307c4d0d Mon Sep 17 00:00:00 2001 From: jcardozo-eth <131674798+jcardozo-eth@users.noreply.github.com> Date: Thu, 23 Jul 2026 10:32:50 +0200 Subject: [PATCH 12/12] docs(init): give packages.sources a struct-typed option doc packages.sources was documented as a bare Hash while the struct enforces manage plus the docker and gitlab_runner apt sources. Spell the contract out in the option type, referencing the Apt_source alias, so REFERENCE states the real shape like the other options. --- REFERENCE.md | 2 +- manifests/init.pp | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/REFERENCE.md b/REFERENCE.md index 7a89a29..4210947 100644 --- a/REFERENCE.md +++ b/REFERENCE.md @@ -248,7 +248,7 @@ Options: * **:install** `Array[String[1]]`: Packages to ensure installed. The empty default installs nothing. Install only: the module never removes packages absent from the list and never pins or upgrades. -* **:sources** `Hash`: The apt sources the `install` list installs from (Docker's and GitLab +* **:sources** `Struct[{ manage => Boolean, docker => Apt_source, gitlab_runner => Apt_source }]`: The apt sources the `install` list installs from (Docker's and GitLab Runner's, via `puppetlabs/apt`). `sources.manage` (default false) decides ownership: keep it off where apt sources are owned elsewhere (central configuration management, e.g. Foreman/Katello, or a mirror). The diff --git a/manifests/init.pp b/manifests/init.pp index d495651..4122f7b 100644 --- a/manifests/init.pp +++ b/manifests/init.pp @@ -134,7 +134,7 @@ # Packages to ensure installed. The empty default installs nothing. Install # only: the module never removes packages absent from the list and never # pins or upgrades. -# @option packages [Hash] :sources +# @option packages [Struct[{ manage => Boolean, docker => Apt_source, gitlab_runner => Apt_source }]] :sources # The apt sources the `install` list installs from (Docker's and GitLab # Runner's, via `puppetlabs/apt`). `sources.manage` (default false) decides # ownership: keep it off where apt sources are owned elsewhere (central