From 441876fab9af84c6646637c6bd8d35a3a72d7f6d Mon Sep 17 00:00:00 2001 From: Sandwich Date: Thu, 28 May 2026 17:25:23 +0200 Subject: [PATCH] refactor(global_defaults): single source of truth for family-default resolution --- roles/global_defaults/defaults/main.yml | 6 +++ .../tasks/_apply_family_defaults.yml | 25 +++++++++++ .../tasks/_normalize_system.yml | 39 ++++------------ .../global_defaults/tasks/_validate_input.yml | 2 +- roles/global_defaults/tasks/system.yml | 45 ++++--------------- roles/global_defaults/tasks/validation.yml | 2 +- 6 files changed, 49 insertions(+), 70 deletions(-) create mode 100644 roles/global_defaults/tasks/_apply_family_defaults.yml diff --git a/roles/global_defaults/defaults/main.yml b/roles/global_defaults/defaults/main.yml index 3ec408a..f75f7b5 100644 --- a/roles/global_defaults/defaults/main.yml +++ b/roles/global_defaults/defaults/main.yml @@ -203,6 +203,12 @@ hypervisor_required_fields: hypervisor: [] system: [] +# Family default content mirror URLs, used when content.url is empty. +content_mirror_defaults: + debian: "https://deb.debian.org/debian/" + ubuntu: "http://archive.ubuntu.com/ubuntu/" + ubuntu-lts: "http://archive.ubuntu.com/ubuntu/" + # Hypervisor-to-disk device prefix mapping for virtual machines. # Physical installs must set system.disks[].device explicitly. hypervisor_disk_device_map: diff --git a/roles/global_defaults/tasks/_apply_family_defaults.yml b/roles/global_defaults/tasks/_apply_family_defaults.yml new file mode 100644 index 0000000..b2b6c2f --- /dev/null +++ b/roles/global_defaults/tasks/_apply_family_defaults.yml @@ -0,0 +1,25 @@ +--- +# Shared by both the fresh-run path (_normalize_system.yml) and the pre-computed +# enrichment path (system.yml) so the family-default rules live in one place. +- name: Apply family defaults to system_cfg + vars: + _os: "{{ system_cfg.os | default('') | string | lower }}" + ansible.builtin.set_fact: + system_cfg: >- + {{ + system_cfg | combine({ + 'content': { + 'source': system_cfg.content.source + if (system_cfg.content.source | default('') | string | trim | length > 0) + else ('dvd' if _os == 'rhel' else 'mirror'), + 'url': system_cfg.content.url + if (system_cfg.content.url | default('') | string | trim | length > 0) + else (content_mirror_defaults[_os] | default('')), + }, + 'features': {'firewall': {'backend': + system_cfg.features.firewall.backend + if (system_cfg.features.firewall.backend | default('') | string | trim | length > 0) + else ('ufw' if _os in os_family_debian else 'firewalld') + }}, + }, recursive=True) + }} diff --git a/roles/global_defaults/tasks/_normalize_system.yml b/roles/global_defaults/tasks/_normalize_system.yml index c3442a3..94e5f6b 100644 --- a/roles/global_defaults/tasks/_normalize_system.yml +++ b/roles/global_defaults/tasks/_normalize_system.yml @@ -10,29 +10,19 @@ if (system_raw.name | default('') | string | trim | length) > 0 else inventory_hostname }} - _mirror_defaults: - debian: "https://deb.debian.org/debian/" - ubuntu: "http://archive.ubuntu.com/ubuntu/" - ubuntu-lts: "http://archive.ubuntu.com/ubuntu/" ansible.builtin.set_fact: system_cfg: - # --- Identity & platform --- type: "{{ system_type }}" os: "{{ system_os_input if system_os_input | length > 0 else (physical_default_os if system_type == 'physical' else '') }}" version: "{{ system_raw.version | default('') | string }}" filesystem: "{{ system_raw.filesystem | default('') | string | lower }}" name: "{{ system_name }}" id: "{{ system_raw.id | default('') | string }}" - # --- VM sizing (ignored for physical) --- cpus: "{{ [system_raw.cpus | default(0) | int, 0] | max }}" memory: "{{ [system_raw.memory | default(0) | int, 0] | max }}" balloon: "{{ [system_raw.balloon | default(0) | int, 0] | max }}" - # --- Network --- - # Flat fields (bridge, ip, etc.) and interfaces[] express the same primary NIC. - # When only flat fields are set, a synthetic interfaces[] entry is built below. - # When interfaces[] is set, the flat ip/prefix/gateway are backfilled from - # interfaces[0] so consumers reading the flat fields (e.g. the post-reboot - # reconnect block) still work. + # Flat fields and interfaces[] describe the same primary NIC: each is + # backfilled from the other so consumers reading either form still work. network: bridge: >- {{ @@ -87,20 +77,13 @@ else [] ) }} - # --- Locale & environment --- timezone: "{{ system_raw.timezone | string }}" locale: "{{ system_raw.locale | string }}" keymap: "{{ system_raw.keymap | string }}" content: - source: >- - {%- set s = system_raw.content.source | default('') | string | lower | trim -%} - {%- if s | length > 0 -%}{{ s }} - {%- elif (system_raw.os | default('') | string | lower) == 'rhel' -%}dvd - {%- else -%}mirror{%- endif -%} - url: >- - {%- set u = system_raw.content.url | default('') | string | trim -%} - {%- if u | length > 0 -%}{{ u }} - {%- else -%}{{ _mirror_defaults[system_raw.os | default('') | string | lower] | default('') }}{%- endif -%} + # Family defaults for empty source/url are applied by _apply_family_defaults.yml. + source: "{{ system_raw.content.source | default('') | string | lower | trim }}" + url: "{{ system_raw.content.url | default('') | string | trim }}" proxy: "{{ system_raw.content.proxy | default('') | string | trim }}" gpgcheck: "{{ system_raw.content.gpgcheck | default(true) | bool }}" satellite: @@ -129,13 +112,11 @@ | reject('equalto', '') | list }} - # --- Storage & accounts --- disks: "{{ system_raw.disks | default([]) }}" users: "{{ system_raw.users | default({}) }}" root: password: "{{ system_raw.root.password | string }}" shell: "{{ system_raw.root.shell | default('/bin/bash') | string }}" - # --- LUKS disk encryption --- luks: enabled: "{{ system_raw.luks.enabled | bool }}" passphrase: "{{ system_raw.luks.passphrase | string }}" @@ -153,7 +134,6 @@ iter: "{{ system_raw.luks.iter | int }}" bits: "{{ system_raw.luks.bits | int }}" pbkdf: "{{ system_raw.luks.pbkdf | string }}" - # --- Feature flags --- features: cloud_init: "{{ system_raw.features.cloud_init | default(false) | bool }}" cis: @@ -165,10 +145,8 @@ enabled: "{{ system_raw.features.selinux.enabled | bool }}" firewall: enabled: "{{ system_raw.features.firewall.enabled | bool }}" - backend: >- - {{ (system_raw.features.firewall.backend | default('') | string | lower | trim) - if (system_raw.features.firewall.backend | default('') | string | lower | trim | length > 0) - else ('ufw' if (system_raw.os | default('') | string | lower) in ['debian', 'ubuntu', 'ubuntu-lts'] else 'firewalld') }} + # Empty backend is family-resolved by _apply_family_defaults.yml. + backend: "{{ system_raw.features.firewall.backend | default('') | string | lower | trim }}" toolkit: "{{ system_raw.features.firewall.toolkit | string | lower }}" ssh: enabled: "{{ system_raw.features.ssh.enabled | bool }}" @@ -225,8 +203,7 @@ if (system_raw.features.peripherals.enabled | string | lower) == 'auto' else (system_raw.features.peripherals.enabled | bool) }} - # fingerprint/camera/audio/bluetooth stay tri-state ('auto'|'true'|'false') - # because the 'auto' branch is resolved at install time using detection results. + # Kept tri-state ('auto'|'true'|'false'): 'auto' resolves at install time from detection. fingerprint: >- {{ 'auto' diff --git a/roles/global_defaults/tasks/_validate_input.yml b/roles/global_defaults/tasks/_validate_input.yml index 7c2281c..f4a8080 100644 --- a/roles/global_defaults/tasks/_validate_input.yml +++ b/roles/global_defaults/tasks/_validate_input.yml @@ -44,7 +44,7 @@ label: "system.features.{{ item }}" ansible.builtin.assert: that: - - (system.features[item] | default({})) is mapping + - (system_defaults.features[item] is not mapping) or ((system.features[item] | default({})) is mapping) fail_msg: "system.features.{{ item }} must be a dictionary." quiet: true diff --git a/roles/global_defaults/tasks/system.yml b/roles/global_defaults/tasks/system.yml index c80f31e..11c1a6b 100644 --- a/roles/global_defaults/tasks/system.yml +++ b/roles/global_defaults/tasks/system.yml @@ -1,10 +1,7 @@ --- -# Two code paths: -# 1. Fresh run (system_cfg undefined): normalize from raw `system` input. -# 2. Pre-computed (system_cfg already set, e.g. from main project's deploy_iac): -# merge with bootstrap system_defaults to fill missing fields (luks, features, -# etc.) that bootstrap expects but the main project doesn't set, then derive -# convenience facts (hostname, os, os_version). +# Fresh run normalizes raw `system` input. A pre-computed system_cfg (from the main +# project's deploy_iac) is instead merged with system_defaults to fill the fields +# bootstrap expects, then convenience facts are derived. - name: Normalize system and disk configuration when: system_cfg is not defined block: @@ -50,37 +47,6 @@ ansible.builtin.set_fact: system_cfg: "{{ system_defaults | combine(system | default({}), recursive=True) | combine(system_cfg, recursive=True) }}" -- name: Apply family defaults (content source, firewall backend) for pre-computed system_cfg - when: - - system_cfg is defined - - _bootstrap_needs_enrichment | default(false) | bool - vars: - # Same family resolution as _normalize_system.yml - kept in sync manually. - _mirror_defaults: - debian: "https://deb.debian.org/debian/" - ubuntu: "http://archive.ubuntu.com/ubuntu/" - ubuntu-lts: "http://archive.ubuntu.com/ubuntu/" - _os: "{{ system_cfg.os | default('') | string | lower }}" - ansible.builtin.set_fact: - system_cfg: >- - {{ - system_cfg | combine({ - 'content': { - 'source': system_cfg.content.source - if (system_cfg.content.source | default('') | string | trim | length > 0) - else ('dvd' if _os == 'rhel' else 'mirror'), - 'url': system_cfg.content.url - if (system_cfg.content.url | default('') | string | trim | length > 0) - else (_mirror_defaults[_os] | default('')), - }, - 'features': {'firewall': {'backend': - system_cfg.features.firewall.backend - if (system_cfg.features.firewall.backend | default('') | string | trim | length > 0) - else ('ufw' if _os in ['debian', 'ubuntu', 'ubuntu-lts'] else 'firewalld') - }}, - }, recursive=True) - }} - - name: Populate primary network fields from first interface (pre-computed) when: - system_cfg is defined @@ -117,3 +83,8 @@ - system_cfg is defined - install_drive is not defined ansible.builtin.include_tasks: _normalize_disks.yml + +# Runs on every path before validation, so an empty firewall.backend / content.source +# resolves to the family default even when system_cfg arrived pre-computed. +- name: Apply family defaults (content source, firewall backend) + ansible.builtin.include_tasks: _apply_family_defaults.yml diff --git a/roles/global_defaults/tasks/validation.yml b/roles/global_defaults/tasks/validation.yml index d814d93..2861c3a 100644 --- a/roles/global_defaults/tasks/validation.yml +++ b/roles/global_defaults/tasks/validation.yml @@ -96,7 +96,7 @@ quiet: true - name: Validate system.features leaf schemas - loop: "{{ system_defaults.features | dict2items }}" + loop: "{{ system_defaults.features | dict2items | selectattr('value', 'mapping') }}" loop_control: label: "system.features.{{ item.key }}" vars: