From d5d89ade6ca191d8ac87ba7183ae069cd7e339c9 Mon Sep 17 00:00:00 2001 From: Daniel Hilst <392820+dhilst@users.noreply.github.com> Date: Tue, 1 Sep 2026 08:01:17 -0300 Subject: [PATCH] test(ubuntu): assert on what subiquity is handed, not on how it is written ubuntu_2604_pkglist.t matched regexes against Template.pm's `push @lines` literals, and ubuntu_subiquity_storage.t was source greps end to end. Both pass when the code is moved somewhere it never runs, and both break on a reformat that changes nothing: reordering two keys inside the efi-part stanza -- identical emitted config -- fails the old efi-part match. The apt config is now rendered by calling ubuntu_subiquity_apt_config and asserting on its output. Only the two collaborators that read the xCAT database are stubbed; the release branch is driven for real through the media directory the renderer parses, so 26.04, 24.04 and pre-Deb822 each render their own shape. The Release-index requirement is exercised against a real directory instead of matched as a shell fragment. The partitioning moves to ubuntu_subiquity_storage.t, which owns that topic and was asserting the same facts by grep. The script cannot run here -- it stops syslog and carries xCAT template markers -- so the block that writes the partition file is lifted out and executed with its one bracket test shadowed by a shell function, which bash resolves ahead of the builtin, leaving the script's own condition unmodified. The emitted curtin config is parsed into id => attributes, so the assertions survive reindentation and reordering. Both extractions BAIL_OUT if their anchors stop matching, and the partition-file redirect is asserted to have been pointed at the scratch tree twice before anything runs, so a rewrite that stops matching fails loudly instead of writing to /tmp. Verified by mutation: dropping grub_device, formatting the ESP ext4, moving its mount point, dropping the bios_grub flag, indenting the block off column 0, dropping Enabled: no, rendering a live cdrom source on 26.04, dropping the Check-Date waiver, dropping the offline fallback and dropping the Release requirement each redden the suite; the pure key reorder that broke the old assertion leaves it green. Signed-off-by: Daniel Hilst <392820+dhilst@users.noreply.github.com> --- xCAT-test/unit/ubuntu_2604_pkglist.t | 119 ++++++++++++++++---- xCAT-test/unit/ubuntu_subiquity_storage.t | 130 ++++++++++++++++++---- 2 files changed, 203 insertions(+), 46 deletions(-) diff --git a/xCAT-test/unit/ubuntu_2604_pkglist.t b/xCAT-test/unit/ubuntu_2604_pkglist.t index 29f6fa8e5..4fcd51188 100644 --- a/xCAT-test/unit/ubuntu_2604_pkglist.t +++ b/xCAT-test/unit/ubuntu_2604_pkglist.t @@ -3,8 +3,12 @@ use warnings; use FindBin; use File::Spec; +use File::Temp (); use Test::More; +use lib "$FindBin::Bin/../../xCAT-server/lib/perl"; +use lib "$FindBin::Bin/../../perl-xCAT"; + use lib "$FindBin::Bin/../.."; use BuildUtils (); @@ -56,33 +60,100 @@ like( $template, qr/#UBUNTU_SUBIQUITY_APT_CONFIG#/, 'subiquity apt configuration like( $template, qr/package_update: false/, 'subiquity install does not require online package update' ); like( $template, qr/package_upgrade: false/, 'subiquity install does not require online package upgrade' ); -my $template_module = File::Spec->catfile( $repo_root, 'xCAT-server/lib/perl/xCAT/Template.pm' ); -open( my $module_fh, '<', $template_module ) or die "Unable to read $template_module: $!"; -my $module = do { local $/; <$module_fh> }; -close($module_fh); +# ---------------------------------------------------- the rendered apt config -- +# What Subiquity is handed, not how Template.pm is written. These assertions used +# to grep the `push @lines, '...'` literals out of Template.pm, which passed on any +# reordering and would have kept passing had the renderer moved somewhere it never +# runs. Only two collaborators are stubbed -- the two that read the xCAT database. +# The release branch is driven for real, by the media directory the renderer parses. +require xCAT::Template; -like( $module, qr/URIs: http:\/\/xcat\.invalid\/disabled.*Enabled: no/s, 'subiquity renderer disables duplicate archive sources when Subiquity provides cdrom.sources' ); -like( $module, qr/sources_list: \|/, 'subiquity renderer owns Deb822 install media sources when Subiquity does not provide cdrom.sources' ); -like( $module, qr/URIs: file:\/\/\/cdrom/, 'subiquity renderer can use the mounted install media as the primary mirror' ); -like( $module, qr/Check-Date: no/, 'subiquity renderer avoids cdrom Check-Date conflicts' ); -like( $module, qr/fallback: offline-install/, 'subiquity renderer can complete without external apt mirrors' ); -like( $module, qr/geoip: false/, 'subiquity renderer does not require external geoip lookup' ); -like( $module, qr/- updates.*- backports.*- security/s, 'subiquity renderer disables online update suites' ); -like( $module, qr/mirror-selection:/, 'subiquity renderer keeps classic mirror-selection fallback for older Ubuntu releases' ); -like( $module, qr/Types: deb.*URIs: \$source.*Suites: \.\/.*Components:.*Trusted: yes/s, 'subiquity renderer includes trusted local xCAT otherpkgdir repositories in Deb822 sources_list' ); -like( $module, qr/deb \[trusted=yes\] \$source \.\/"/, 'subiquity renderer keeps classic source-list fallback for older Ubuntu releases' ); -like( $module, qr/-f "\$path\/Release"/, 'subiquity renderer requires indexed otherpkgdir repositories' ); +our @otherpkg_sources; +{ + no warnings qw(redefine once); + *xCAT::Template::ubuntu_subiquity_apt_mirror = sub { return $main::apt_mirror }; + *xCAT::Template::ubuntu_subiquity_otherpkg_sources = sub { return @main::otherpkg_sources }; +} -my $subiquity_pre = File::Spec->catfile( - $repo_root, - 'xCAT-server/share/xcat/install/scripts/pre.ubuntu.subiquity' -); -open( my $pre_fh, '<', $subiquity_pre ) or die "Unable to read $subiquity_pre: $!"; -my $pre = do { local $/; <$pre_fh> }; -close($pre_fh); +our $apt_mirror = ''; -like( $pre, qr/id: efi-part\s+type: partition\s+device: disk-detected\s+size: 512M\s+flag: boot\s+number: 1\s+preserve: false\s+grub_device: true/s, 'subiquity UEFI storage marks the EFI partition as grub device' ); -like( $pre, qr/id: efi-part-fs\s+type: format\s+fstype: fat32\s+volume: efi-part/s, 'subiquity UEFI storage formats ESP as fat32' ); +sub apt_config_for { + my ( $media_dir, %args ) = @_; + local $apt_mirror = $args{mirror} // ''; + local @otherpkg_sources = @{ $args{sources} || [] }; + return xCAT::Template::ubuntu_subiquity_apt_config($media_dir); +} + +# 26.04's Subiquity writes its own cdrom.sources. A second file:///cdrom source +# collides with it, so the rendered one is present but inactive. +my $noble_plus = apt_config_for('ubuntu26.04'); +like( $noble_plus, qr{^\s+URIs: http://xcat\.invalid/disabled$}m, + 'on 26.04 the install-media source is rendered inert' ); +like( $noble_plus, qr/^\s+Enabled: no$/m, + 'and is explicitly disabled so Subiquity does not fetch from it' ); +unlike( $noble_plus, qr{file:///cdrom}, + 'so it cannot collide with the cdrom.sources Subiquity generates' ); + +# 24.04 has Deb822 but no generated cdrom.sources, so xCAT owns the media source. +my $noble = apt_config_for('ubuntu24.04'); +like( $noble, qr/^\s+sources_list: \|$/m, + 'on 24.04 xCAT owns the Deb822 sources_list' ); +like( $noble, qr{^\s+URIs: file:///cdrom$}m, + 'and points it at the mounted install media' ); +like( $noble, qr/^\s+Check-Date: no$/m, + 'and waives Check-Date, which the media index would otherwise fail' ); + +# Before Deb822 the same intent is expressed with mirror-selection. +my $focal = apt_config_for('ubuntu20.04'); +like( $focal, qr/^\s+mirror-selection:$/m, + 'older releases keep the classic mirror-selection form' ); +like( $focal, qr{^\s+- uri: file:/cdrom$}m, + 'still served from the install media' ); +unlike( $focal, qr/sources_list: \|/, + 'and are not given a Deb822 block they cannot parse' ); + +# Common to every offline render: no external mirror is required to finish. +foreach my $case ( [ '26.04', $noble_plus ], [ '24.04', $noble ], [ '20.04', $focal ] ) { + my ( $name, $rendered ) = @{$case}; + like( $rendered, qr/^\s+fallback: offline-install$/m, + "$name completes without an external apt mirror" ); + like( $rendered, qr/^\s+geoip: false$/m, + "$name does not wait on a geoip lookup" ); + like( $rendered, qr/^\s+disable_suites:\n\s+- updates\n\s+- backports\n\s+- security$/m, + "$name has the online update suites disabled" ); +} + +# otherpkgdir repositories reach the installer in whichever form the release reads. +my $with_otherpkgs = apt_config_for( 'ubuntu24.04', sources => ['http://mn/otherpkg'] ); +like( $with_otherpkgs, + qr/^\s+Types: deb\n\s+URIs: http:\/\/mn\/otherpkg\n\s+Suites: \.\/\n\s+Components:\n\s+Trusted: yes$/m, + 'Deb822 releases get the xCAT repository as a trusted Deb822 stanza' ); + +my $classic_otherpkgs = apt_config_for( 'ubuntu20.04', sources => ['http://mn/otherpkg'] ); +like( $classic_otherpkgs, qr{source: "deb \[trusted=yes\] http://mn/otherpkg \./"}, + 'and older releases get the same repository as a one-line source' ); + +# An online mirror turns the offline handling off entirely. +my $online = apt_config_for( 'ubuntu24.04', mirror => 'http://archive.example/ubuntu' ); +like( $online, qr{^\s+- uri: http://archive\.example/ubuntu$}m, + 'a configured mirror becomes the primary' ); +unlike( $online, qr/fallback: offline-install/, + 'and the offline fallback is not rendered alongside it' ); +unlike( $online, qr/disable_suites/, + 'nor are updates and security disabled on an online install' ); + +# The reason a bare directory is not offered as a repository: apt needs an index. +my $repo_dir = File::Temp::tempdir( CLEANUP => 1 ); +ok( !xCAT::Template::ubuntu_subiquity_local_apt_repo($repo_dir), + 'an empty directory is not treated as an apt repository' ); +open( my $pkgs_fh, '>', File::Spec->catfile( $repo_dir, 'Packages' ) ) or die $!; +close($pkgs_fh); +ok( !xCAT::Template::ubuntu_subiquity_local_apt_repo($repo_dir), + 'nor is one with packages but no Release index' ); +open( my $rel_fh, '>', File::Spec->catfile( $repo_dir, 'Release' ) ) or die $!; +close($rel_fh); +ok( xCAT::Template::ubuntu_subiquity_local_apt_repo($repo_dir), + 'an indexed directory is' ); # The releases the deb builder serves by default. Read from BuildUtils, which is where # the builder itself reads them, rather than matched against the source that sets them: diff --git a/xCAT-test/unit/ubuntu_subiquity_storage.t b/xCAT-test/unit/ubuntu_subiquity_storage.t index 844a493c6..288296db0 100644 --- a/xCAT-test/unit/ubuntu_subiquity_storage.t +++ b/xCAT-test/unit/ubuntu_subiquity_storage.t @@ -1,6 +1,16 @@ #!/usr/bin/env perl +# The partitioning pre.ubuntu.subiquity hands to Subiquity. +# +# Every assertion here used to be a regex against the script's own text, which +# passes when the block is moved somewhere it never runs and fails when the YAML +# is reindented or its keys reordered -- a pure reformat broke the old efi-part +# match while the emitted config was identical. The block is executed instead and +# the config it writes is parsed, so the assertions are about what Subiquity gets. use strict; use warnings; + +use File::Spec; +use File::Temp (); use Test::More; my $pre_path = defined $ENV{XCATROOT} ? "$ENV{XCATROOT}/share/xcat/install/scripts/pre.ubuntu.subiquity" : ''; @@ -11,35 +21,111 @@ plan skip_all => "pre.ubuntu.subiquity not found" unless -f $pre_path; my $script = do { local $/; open my $fh, '<', $pre_path or die $!; <$fh> }; -# Shell syntax check -my $rc = system("bash -n $pre_path 2>/dev/null"); -is($rc, 0, 'pre.ubuntu.subiquity passes bash -n syntax check'); +is( system("bash -n $pre_path 2>/dev/null"), 0, + 'pre.ubuntu.subiquity passes bash -n syntax check' ); -# UEFI storage layout checks -like($script, qr/if \[ -d \/sys\/firmware\/efi \]/, 'script detects UEFI via /sys/firmware/efi'); +# ------------------------------------------------------ the emitted partitioning -- +# pre.ubuntu.subiquity cannot run here -- it stops syslog and carries xCAT template +# markers -- so the block that writes the partition file is lifted out and executed, +# with its one bracket test shadowed to choose the firmware branch and its output +# redirected into a scratch tree. Both substitutions are asserted: if either stops +# matching, this bails out rather than silently covering nothing or writing to /tmp. +my ($storage_block) = $script =~ /(^if \[ -d \/sys\/firmware\/efi \]; then\n.*?\n^fi$)/ms; +BAIL_OUT('the firmware branch that writes the partition file no longer matches') + unless $storage_block; -# UEFI: grub_device on EFI partition, NOT on disk -like($script, qr/id: efi-part.*grub_device: true/s, 'UEFI: grub_device on EFI partition'); +my $brackets = () = $storage_block =~ /\[ /g; +BAIL_OUT("the partitioning block now has $brackets bracket tests; the shadow below covers one") + unless $brackets == 1; -# UEFI: EFI partition formatted as fat32 -like($script, qr/id: efi-part-fs.*fstype: fat32/s, 'UEFI: EFI partition formatted fat32'); +my $sandbox = File::Temp::tempdir( CLEANUP => 1 ); +my $partfile = File::Spec->catfile( $sandbox, 'partitionfile' ); +my $rewrites = ( $storage_block =~ s{/tmp/partitionfile}{$partfile}g ); +BAIL_OUT("expected two partition-file redirects to sandbox, rewrote $rewrites") + unless $rewrites == 2; -# UEFI: EFI partition mounted at /boot/efi -like($script, qr/efi-part-mount.*path: \/boot\/efi/s, 'UEFI: EFI partition mounted at /boot/efi'); +my %YAML_FOR; -# BIOS storage layout checks -like($script, qr/id: bios-grub.*flag: bios_grub/s, 'BIOS: has bios_grub partition'); -like($script, qr/id: disk-detected.*grub_device: true/s, 'BIOS: grub_device on disk'); +sub partition_yaml_for { return $YAML_FOR{ $_[0] }; } -# Both paths must have storage: version: 1 -my @storage_version = ($script =~ /storage:\s*\n\s*version:\s*1/g); -is(scalar @storage_version, 2, 'both UEFI and BIOS have storage: version: 1'); +sub partition_config_for { + my ($firmware) = @_; + my $script = File::Spec->catfile( $sandbox, 'storage.sh' ); + open( my $fh, '>', $script ) or die "Unable to write $script: $!"; + # `[` is shadowed rather than the condition rewritten: bash resolves a function + # ahead of the builtin, so the script's own test runs unmodified. + print {$fh} <<"SHELL"; +INSTALL_DISK=/dev/sdz +logger() { :; } +[() { + case "\$1 \$2" in + "-d /sys/firmware/efi") return @{[ $firmware eq 'uefi' ? 0 : 1 ]} ;; + *) builtin echo "unexpected bracket test: \$*" >&2; builtin return 2 ;; + esac +} +$storage_block +SHELL + close($fh); -# Both paths write to /tmp/partitionfile -my @partfile = ($script =~ /\/tmp\/partitionfile/g); -cmp_ok(scalar @partfile, '>=', 2, 'both paths write to /tmp/partitionfile'); + unlink $partfile; + system( 'bash', $script ) == 0 + or BAIL_OUT("the extracted partitioning block failed to run for $firmware"); + open( my $out_fh, '<', $partfile ) + or BAIL_OUT("the partitioning block wrote no file for $firmware: $!"); + my $yaml = do { local $/; <$out_fh> }; + close($out_fh); + $YAML_FOR{$firmware} = $yaml; -# Storage at column 0 (for re-serialized autoinstall.yaml) -like($script, qr/^storage:\n version: 1/m, 'storage block starts at column 0'); + # Parse the curtin config into id => {key => value} so the assertions survive + # reordering and reindentation, which is what the source match could not do. + my %entry; + my $current; + foreach my $line ( split /\n/, $yaml ) { + if ( $line =~ /^\s+- id:\s*(\S+)/ ) { + $current = $1; + $entry{$current} = { id => $1 }; + } + elsif ( defined $current && $line =~ /^\s+(\S+):\s*(\S*)\s*$/ ) { + $entry{$current}{$1} = $2; + } + } + return \%entry; +} + +my $uefi = partition_config_for('uefi'); +is( $uefi->{'efi-part'}{type}, 'partition', 'UEFI installs get an EFI partition' ); +is( $uefi->{'efi-part'}{device}, 'disk-detected', 'on the detected install disk' ); +is( $uefi->{'efi-part'}{size}, '512M', 'sized for an ESP' ); +is( $uefi->{'efi-part'}{flag}, 'boot', 'flagged bootable' ); +is( $uefi->{'efi-part'}{number}, '1', 'as the first partition' ); +is( $uefi->{'efi-part'}{grub_device}, 'true', 'and it is where grub is installed' ); +is( $uefi->{'efi-part-fs'}{type}, 'format', 'the ESP is formatted' ); +is( $uefi->{'efi-part-fs'}{fstype}, 'fat32', 'as fat32, which firmware can read' ); +is( $uefi->{'efi-part-fs'}{volume}, 'efi-part', 'on the partition just created' ); +is( $uefi->{'efi-part-mount'}{path}, '/boot/efi', 'and mounted where the kernel expects it' ); + +# The BIOS branch must not carry the ESP, and puts grub on the disk itself. +my $bios = partition_config_for('bios'); +ok( !exists $bios->{'efi-part'}, 'BIOS installs get no EFI partition' ); +is( $bios->{'bios-grub'}{flag}, 'bios_grub', 'they get a bios_grub partition instead' ); +is( $bios->{'disk-detected'}{grub_device}, 'true', 'and grub is installed to the disk' ); + +foreach my $firmware ( [ UEFI => $uefi ], [ BIOS => $bios ] ) { + my ( $name, $config ) = @{$firmware}; + is( $config->{'root-part-fs'}{fstype}, 'ext4', "$name root filesystem is ext4" ); + is( $config->{'root-part-mount'}{path}, '/', "$name mounts root at /" ); + is( $config->{'swap-part-fs'}{fstype}, 'swap', "$name has swap formatted" ); + is( $config->{'root-part'}{size}, '-1', + "$name gives the remaining space to root" ); +} + + +# Subiquity re-serializes autoinstall.yaml and appends this file, so the block has +# to start at column 0 -- asserted on what was written, not on the heredoc. +foreach my $firmware ( [ UEFI => 'uefi' ], [ BIOS => 'bios' ] ) { + my ( $name, $key ) = @{$firmware}; + like( partition_yaml_for($key), qr/\Astorage:\n version: 1\n/, + "$name config starts at column 0 with storage: version: 1" ); +} done_testing();