xCAT/xcat-core
2
0
Fork 0
mirror of https://github.com/xcat2/xcat-core.git synced 2026-09-04 20:17:55 +00:00
Code Issues Releases Wiki Activity

fix(debian): connect the subiquity helpers to mkinstall, and close the sandbox guard

Browse Source 
Two things the review found, both in code added by this branch.

Reverting mkinstall's call site -- putting xCAT::NetworkUtils->getipaddr back
in place of subiquity_nfsroot_server, the exact regression the fix removes --
left the entire unit suite green. The helpers were covered; nothing linked them
to production. Compose the two steps in subiquity_boot_params(), which takes its
inputs and returns either a command line or the reason there isn't one, so the
composition can be driven; mkinstall keeps report_node_error and the loop's
`next`. That same revert now reddens 6 of 9 assertions.

The test stubs xCAT::NetworkUtils::getipaddr deliberately. Without it a call
site that bypassed the injected resolver died on a missing module -- a red, but
for the wrong reason. With it, bypassing the resolver returns the wrong answer,
which is what the assertions are there to catch.

The resolv.conf sandbox guard matched `/etc/` with a trailing slash, so the one
respelling its own comment names -- `etcdir=/etc; rm -f "$etcdir/resolv.conf"`
-- walked straight past it and the fragment would rm the runner's real
resolv.conf, as root in CI. `/etc\b` catches it: applying that respelling now
BAIL_OUTs instead of running.

Signed-off-by: Daniel Hilst <392820+dhilst@users.noreply.github.com>
This commit is contained in:
Daniel Hilst
2026-09-01 21:09:31 -03:00
parent da0bfdbd22
commit b784aa782c
3 changed files with 56 additions and 10 deletions
Download Patch File Download Diff File Expand all files Collapse all files
xCAT-server/lib/xcat/plugins/debian.pm
+44 -9
View File
@@ -584,6 +584,42 @@ sub subiquity_kcmdline {
return $kcmdline;
}
#-------------------------------------------------------------------------------
=head3 subiquity_boot_params
Resolve the install server and build the Subiquity command line, or say why not.
The two steps are composed here rather than in mkinstall so the composition can be driven:
mkinstall needs a management node, and the decision that matters -- which install server
ends up in nfsroot -- is exactly what a regression would change. The caller keeps the side
effects: reporting the error and skipping the node.
Arguments:
$base the command line built so far
$instserver the install server name, address, or the '!myipfn!' placeholder
$pkgdir the install media path exported over NFS
$httpport the xCAT HTTP port
$node the node being installed
$resolver optional coderef, for tests; passed through to subiquity_nfsroot_server
Returns:
($kcmdline, undef) on success, or (undef, $message) when the server does not resolve
=cut
#-------------------------------------------------------------------------------
sub subiquity_boot_params {
my ($base, $instserver, $pkgdir, $httpport, $node, $resolver) = @_;
my $nfsip = subiquity_nfsroot_server($instserver, $resolver);
return (undef, "Could not resolve the install server '$instserver' to an address. "
. "The Ubuntu live installer mounts its root with klibc nfsmount, which cannot "
. "resolve names, so nfsroot must be an address.")
unless $nfsip;
return (subiquity_kcmdline($base, $nfsip, $pkgdir, $instserver, $httpport, $node), undef);
}
sub mkinstall {
xCAT::MsgUtils->message("S", "Doing debian mkinstall");
my $request = shift;
@@ -1063,17 +1099,16 @@ sub mkinstall {
my $kcmdline = "nofb utf8 auto xcatd=" . $instserver;
if (using_subiquity($os,$tmplfile)) {
# Fail here rather than handing casper a name: klibc's nfsmount cannot resolve
# one, so the node would panic "can't parse IP address" at boot, on the node,
# with nothing said on the management node. '!myipfn!' is exempt -- pxe.pm and
# grub2.pm turn it into an address when they write the boot config.
my $nfsip = subiquity_nfsroot_server($instserver);
unless ($nfsip) {
xCAT::MsgUtils->report_node_error($callback, $node,
"Could not resolve the install server '$instserver' to an address. The Ubuntu live installer mounts its root with klibc nfsmount, which cannot resolve names, so nfsroot must be an address.");
# Fail rather than hand casper a name: klibc's nfsmount cannot resolve one, so
# the node would panic "can't parse IP address" at boot, on the node, with
# nothing said on the management node.
my ($subiquity_cmdline, $subiquity_error) =
subiquity_boot_params($kcmdline, $instserver, $pkgdir, $httpport, $node);
if ($subiquity_error) {
xCAT::MsgUtils->report_node_error($callback, $node, $subiquity_error);
next;
}
$kcmdline = subiquity_kcmdline($kcmdline, $nfsip, $pkgdir, $instserver, $httpport, $node);
$kcmdline = $subiquity_cmdline;
} else {
$kcmdline .= " url=http://${instserver}:$httpport/install/autoinst/$node";
$kcmdline .= " mirror/http/hostname=${instserver}:$httpport";
xCAT-test/unit/debian_subiquity_boot_params.t
+9
View File
@@ -35,6 +35,15 @@ for my $name (qw(subiquity_nfsroot_server subiquity_kcmdline subiquity_boot_para
$body .= "$sub\n";
}
# Stand in for the module the routine falls back to when no resolver is injected. Without this
# a call site that bypassed the injected resolver would die on a missing module -- a red, but
# for the wrong reason. With it, bypassing the resolver produces the WRONG ANSWER instead of an
# exception, which is what the assertions below are meant to catch.
{
package xCAT::NetworkUtils;
sub getipaddr { return undef }
}
{
package T;
eval "$body; 1" or main::BAIL_OUT("could not eval the subiquity helpers: $@");
xCAT-test/unit/ubuntu_resolvconf_ip.t
+3 -1
View File
@@ -37,7 +37,9 @@ sub write_resolv_conf {
# fail a test. Sandboxing by rewriting paths is fragile by nature -- respelling the path in
# the template as, say, `etcdir=/etc; rm -f "$etcdir/resolv.conf"` slips straight past the
# substitution above. Refuse to execute anything that still points outside the scratch tree.
if ($script =~ m{(?<!\Q$root\E)/etc/}) {
# \b not "/etc/": the respelling this guard exists to catch -- `etcdir=/etc; rm -f
# "$etcdir/resolv.conf"` -- has no slash after /etc, so requiring one let it straight past.
if ($script =~ m{(?<!\Q$root\E)/etc\b}) {
BAIL_OUT('the /etc rewrite no longer covers the fragment; refusing to run it as root');
}