The previous commit deleted the $effective_provmethod override on the grounds
that %image_hash never carries a provmethod. That was wrong, and the review
caught it: makescript builds %image_hash, calls getImage() on it, and then
hands the SAME hashref to getScripts(), which fills provmethod for every
osimage from the osimage table. getDisklessNet() already reads that key the
same way. The override was live, not dead.
Restore it and say what is actually true in the comment. nodetype.provmethod is
frequently an osimage name rather than 'install', and resolving it is the point
of the lookup.
Also close the gap that made the wrong deletion so easy to ship: reverting
mkinstall's subiquity branch to its pre-fix body left the whole unit suite
green. debian_mkinstall_subiquity_branch.t lifts that branch out and drives it
inside a real loop, so the `next` it performs is the one under test, with
report_node_error and the getipaddr seam stood in for. It calls
subiquity_boot_params with no injected resolver, exactly as production does.
The branch is selected out of debian.pm by what it contains rather than by
where it sits -- there are four `if (using_subiquity(...))` in that file, and an
earlier draft of this test silently matched the wrong one and ran past its
block.
Now observable, each verified by mutation: swapping $pkgdir and $httpport at
the call site reddens the nfsroot assertion; reverting the branch wholesale
fails the extraction guard rather than passing.
Signed-off-by: Daniel Hilst <392820+dhilst@users.noreply.github.com>
The syncfiles deferral resolved the node's provmethod through
$image_hash{$osimgname}{provmethod} when the node names an osimage. makescript
fills %image_hash from getImage(), which stores pkglist, pkgdir, otherpkglist,
otherpkgdir and environvar -- and no provmethod. getScripts() has a separate
hash that does store one, which is where the pattern was copied from. So the
lookup was always undef, the override never fired, and the code claimed a
behaviour it did not have.
Pass $provmethod directly and say in the comment why there is nothing to
resolve it with. No behaviour changes -- the branch was inert -- so there is no
red to show first; what the deletion needs is coverage that the path it was
supposed to serve still works.
That is what the two new assertions do: an osimage-named provmethod with
nodesetstate 'install' still defers, and the same name with no nodesetstate is
not mistaken for a diskful install. nodesetstate is what carries the install
signal here, which is why the override was never load-bearing. Making the
deferral ignore nodesetstate and require provmethod eq 'install' reddens both.
Signed-off-by: Daniel Hilst <392820+dhilst@users.noreply.github.com>
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>
The subiquity path resolved $instserver with getipaddr and `next`ed the node
when that returned nothing. For a node whose noderes.xcatmaster is unset,
$instserver is the literal '!myipfn!' -- a placeholder that pxe.pm:176 and
grub2.pm:129 substitute with my_ip_facing($node) when they write the boot
config. Resolving it as a name always fails, so those nodes went from a working
numeric nfsroot to no boot config and a "Could not resolve the install server"
error.
noderes.5.rst:125 documents an unset xcatmaster as supported, and anaconda.pm
and sles.pm both already guard the placeholder with
`unless ($instserver eq '!myipfn!')`.
Exempt the placeholder, keeping the guard for a real name that does not
resolve -- klibc's nfsmount still cannot resolve one of those, which is what
the original commit was for.
The decision moves into subiquity_nfsroot_server(), taking the resolver as an
optional argument, so it can be driven directly instead of through a routine
that needs a management node.
Signed-off-by: Daniel Hilst <392820+dhilst@users.noreply.github.com>
Two ways the Subiquity install could fail without saying anything useful.
The boot flip read from the install monitor with no timeout. A monitor that accepts the
connection and then never answers -- which #7759 shows is a state it gets into -- blocked the read
forever, and with it the Subiquity late-command and the install. The retry loop could not help:
it never reached the retry. Both reads now take -t 10, so five attempts are bounded at roughly two
minutes and end in the failure that is already logged. The regression test stands up a listener
that accepts and holds the connection; removing the timeouts fails it.
mkinstall resolved the install server for nfsroot and fell back to the name when that failed:
"getipaddr($instserver) || $instserver". The name is the original defect -- klibc's nfsmount
cannot resolve one -- so the node panicked "can't parse IP address" at boot, on the node, with
nothing reported on the management node. The management node knows at template time, so it says
so there and skips the node, as the other unrecoverable per-node conditions in this routine do.
Signed-off-by: Daniel Hilst <392820+dhilst@users.noreply.github.com>
A node may list syncfiles in both postscripts and postbootscripts -- once for the install and
once for the booted node. The deferral prepended its own copy regardless, so the booted node ran
syncfiles twice in a row inside a marker block that made it look like xCAT had generated the
duplicate.
Keep the entry the node already has and still remove the install-time one, which is the copy
that cannot work in the in-target chroot.
Signed-off-by: Daniel Hilst <392820+dhilst@users.noreply.github.com>
toram copies the live filesystem into RAM, so the node needs memory for it on top of the
installer -- about 1.5G on 24.04 and growing per release. Nothing enforced or documented that.
A node with too little fails part-way and reboots into the installer, which looks like a
boot-flip failure rather than an out-of-memory condition, so the troubleshooting page says
where to look and how to raise vmmemory.
Signed-off-by: Daniel Hilst <392820+dhilst@users.noreply.github.com>
The comment blamed mawk: updateflag.awk needs gawk's |& and Ubuntu's /usr/bin/awk is mawk. That
is wrong on this path -- compute.subiquity.tmpl has installed gawk in the target since Ubuntu
20.04 support was added, so updateflag.awk would have found it.
The flip was going unanswered because xcatd's install monitor, which serves port 3002, dies and
is never re-forked. That is fixed in #7759. The /dev/tcp exchange here is hardening: it retries
and reports, rather than reinstalling in silence.
Signed-off-by: Daniel Hilst <392820+dhilst@users.noreply.github.com>
Three passes of the same reasoning had accumulated: in the code, in the POD of the routines the
previous commit extracted, and again in the test headers. Say each once, where the reader needs it.
The POD blocks were the worst of it -- extracting subiquity_kcmdline() and
defer_syncfiles_to_postboot() moved the essays out of the routines but did not shorten them. The
toram paragraph ran eleven lines for one kernel argument; the exact systemd-shutdown message and
the size of the 24.04 layers are colour, not reasoning.
The template's two comments and the apt-sources pair are trimmed the same way: the sources.list
explanation lives in Template.pm, and the test says which case it is checking and points there.
That the tests still pass after rewriting the template is the point of the previous commit -- a
test matching its text would have broken here.
47 comment and POD lines removed, no behaviour change.
Signed-off-by: Daniel Hilst <392820+dhilst@users.noreply.github.com>
The tests added with this fix matched regexes against the text of debian.pm, Postage.pm and
compute.subiquity.tmpl. A source match cannot tell whether the code it found ever runs: moving
the Subiquity command line into its own routine leaves every one of those assertions passing,
and reformatting a line fails them while the behaviour is untouched. One of them pinned Perl
syntax outright, qr/\$kcmdline\s*\.=\s*" ---";/, and another matched the text of a substitution.
Run the code instead.
Two decisions were lifted out of the routines that had grown around them, so a test can call
them: subiquity_kcmdline() in debian.pm builds the installer command line from its inputs, and
defer_syncfiles_to_postboot() in Postage.pm returns the adjusted postscript lists. Both are pure
and carry the reasoning that used to sit inline. The callers keep their behaviour exactly.
The template's two shell fragments are extracted and executed: the boot flip runs against a
stand-in for xcatd on the install-monitor port, and the resolv.conf step runs with a getent that
answers as the case requires.
Every assertion now fails when the behaviour it describes is removed, which is what the source
matches only appeared to do:
boot=casper dropped from the command line 1 assertion fails
toram dropped 1 assertion fails
nfsroot given the host name instead of the IP 2 assertions fail
the deferral made a no-op 7 assertions fail
the wrong token sent to xcatd 1 assertion fails
the failed-flip log line removed 3 assertions fail
the retry loop reduced to one attempt 2 assertions fail
resolv.conf given the host name 3 assertions fail
the resolution fallback removed 2 assertions fail
ubuntu_subiquity_bootparams.t is removed. Its three matches are covered by execution in
debian_subiquity_netboot.t, and it demonstrated the problem: it still passed after the command
line moved into another routine.
Signed-off-by: Daniel Hilst <392820+dhilst@users.noreply.github.com>
Boot the live installer correctly. Add boot=casper so casper actually processes
netboot=nfs instead of scanning local disks and panicking, resolve the install server
to a literal IP because casper mounts the live filesystem with klibc's nfsmount which
has no resolver, and add toram so casper copies the squashfs into RAM and unmounts the
NFS source. That last one is what lets the node reboot at all: with the NFS root still
mounted, systemd-shutdown waits forever on an lvm/pvscan wedged in uninterruptible I/O
on it and the node never power-cycles into the disk it just installed. casper has no
cmdline knob for NFS mount options -- it parses only nfsroot= and takes the whole value
as the path -- so toram is its supported way to avoid a network root.
Write the installer's resolv.conf nameserver as an IP. glibc's resolver discards a
hostname given on a nameserver line, so the xcatmaster name left the installer, and
the in-target apt that inherits the file, with no DNS at all.
Add the online archive through apt `sources:` on classic-sources releases, where
Subiquity renders the target sources.list from the install media alone and in-target
apt cannot find packages the ISO does not carry. Deb822 releases are excluded: their
primary mirror already lands in ubuntu.sources, so legacy .list files would configure
the same suites twice.
Flip the node to local-disk boot from the live installer over bash's /dev/tcp instead
of relying on updateflag.awk, which needs gawk's |& coprocess while Ubuntu's
/usr/bin/awk is mawk. The exchange is checked, and a failure is recorded in the
install log rather than silently PXE-looping into another install.
Defer syncfiles to the postbootscripts on the diskful install path, so it runs on the
booted node with sshd up rather than inside the in-target chroot where the MN cannot
reach it.
Make the test harness's provision wait overridable through WAIT_FOR_PROVISION; the
default stays short so a boot loop still fails fast.
Signed-off-by: Daniel Hilst <392820+dhilst@users.noreply.github.com>
The manual page and the usage message of nodestat give the option
-f|--usefping. The preprocessor of the request knows no option f, and the
handler of the request spells the long name useping, so --usefping does
nothing. An administrator who follows the manual page gets the nmap path, and
gets no message that says why.
The two places also read different specifications, so an option that one
place accepts can reach the other place and take a different meaning. Put the
specification in one routine, and let both places read that routine.
Give the name usefping to the option, and keep useping as a second name. That
spelling has worked since 2.14.2, so a site can have it in a script.
The long name of the fping option starts with the same letters as usemon, so
--use and --us become names that Getopt::Long cannot decide. Those two
abbreviations select usemon today. Keep them with usemon, or an administrator
who monitors with them loses the monitoring and gets no message.
The change has two other effects. The abbreviations --use and --us no longer
select fping as well, which they did only because the two places read
different specifications. The bundles -mf and -fm now select both options,
which they did not do before.
Recovered from the lenovobuild branch.
The genesis image goes into a file whose name ends with .lzma. The plugin
writes that file only when /usr/bin/lzma is there, and it falls back to gzip
when it is not.
Red Hat ships no lzma binary. On AlmaLinux 9 and on AlmaLinux 10 that test
fails, the plugin falls back to gzip, and it gives no message that says why.
The image is larger on each run of mknb. Debian and Ubuntu ship lzma as a
second name for xz, so those systems still get the smaller image.
Ask xz for the same container when lzma is absent. The command
"xz --format=lzma" writes the same bytes as "lzma", so the file keeps its
name, its container and its size. Keep the gzip fallback for a system that
has neither program.
Recovered from the lenovobuild branch, which asked xz for the xz container.
That container is not the lzma container, and the name of the file says lzma.
The scan wrote each disk into a file named after the identifier it
reported, wwn, path or neither, and read back the groups of one such
file only. Two disks that reported different identifiers therefore
never competed on their driver group: a disk without a WWN was dropped
as soon as another disk reported one, and when the last disk scanned
reported a WWN the readback opened the WWN files alone. A direct
attached boot disk that reports no WWN thus lost to a RAID volume that
reports one, which is the case the driver groups exist to decide.
Write every disk into the file of its driver group and keep the
identifier as the sort key inside that group, ranked so that a WWN
sorts ahead of a path and a path ahead of no identifier. The driver
group now decides first for every disk, the identifier still decides
between disks of one group, and no disk is dropped from the scan.
The device filter accepted sd, hd, vd and nvme names, so the xvd names
that a Xen guest presents never entered the scan. On such a guest the
whole detection ran on an empty list and the script fell through to the
xvda fallback, which takes the first Xen disk without looking at any of
them.
Accept the xvd names in the filter. A Xen disk now goes through the
same classification, kernel search and driver sort as any other disk,
so a guest with more than one disk gets a chosen disk rather than the
first one. The fallback stays for the case where the scan still finds
nothing.
The nvme branch of the filter is anchored at the same time, so a name
only matches when it starts with nvme.
The RHEL 10 support added a second copy of the script. The copy carries
the Xen fallback and a failure log that the common script does not, and
the common script carries the Intel RSTe/VROC fallback that the copy
does not. A RHEL 10 node whose OS disk is a VROC volume therefore fell
back to /dev/sda, and every later fix to disk selection had to be
written twice to reach both.
Fold the Xen fallback and the failure log into the common script and
delete the copy. The RHEL 10 installer includes the common script, as
the other installers already do. The failure log runs only when the
including script defines msgutil_r, because the subiquity path does
not.
The driver sort put the ahci and ata_piix controllers in the same
choice group as the PMC MaxRAID and megaraid_sas RAID controllers. On a
server with both, the sort could select a RAID data volume as the OS
install disk.
Move the RAID controllers to the second choice group. The direct
attached controllers hold the likely boot volume, and a server with
only RAID volumes still selects them from the second group. The SAS
host adapters move to the third group and every other driver to a new
fourth group, so the relative order of the remaining drivers does not
change. The RHEL 10 installer includes its own copy of the script, so
both carry the change.
Recovered from the lenovobuild branch.
The daemon redacted secret attributes on the joined command string. The
match failed when a value held a space. The match also failed for a "+="
splice assignment. The validate() path did not quote the arguments, so a
multi-word secret value kept its later words in syslog and in the
auditlog table. A password that a command receives through an option or
a positional operand was not redacted at all. The debug dispatch trace
wrote the raw arguments to syslog when site.xcatdebugmode was set.
Redact the argument vector before the daemon joins it. Add
redact_password_args for this task. The routine masks the value of a
secret attribute in any argument, at the start or embedded after another
token. An embedded secret assignment masks to the end of the argument,
because a shell value may hold quotes and spaces. The routine allows
spaces around the operator. It accepts the "=", "+=", ",=", "^=", "!=",
"=~" and "!~" operators that chdef, nodech and node selection use. It
masks a password option value in each form that Getopt::Long accepts: a
separate argument, a compact short option, a bundle of short options
with the "?" help letter, a "+" option prefix, a single-letter option
with two dashes, a long option, a long option with an equals sign, and
an abbreviated long option. The long-name match runs first, so a long
option keeps its name and masks its value. A walk over each bundle then
finds the first secret letter, so the mask always starts at the option
and the result does not depend on hash order. The walk knows which
other letters of a command take a value, so a secret letter inside such
a value does not redact and the audit text stays correct. The walk also
knows which letters take an integer, because the z/VM cpu option
consumes only its signed digits and the parser then continues the
bundle into the password option. The value stops match letter case,
because a bundle keeps short options case sensitive and an unknown
capital letter does not absorb the rest. The mkvm secret match ignores
letter case, because the z/VM parser keeps the Getopt::Long default for
long names. The mkhwconn match keeps letter case, because -p
is the hardware control point and -P is the password. The routine knows
the password options of bmcdiscover, switchdiscover, mkhwconn, mkvm,
createvcluster, lsvcluster and rmvcluster, the rspconfig password
assignments, the mkvm clone pw= operand, and the positional password
operands of chvm. It masks the site.value argument of tabch and chtab
when a selector or a site.key assignment names snmpc. An exact short
option that takes a non-secret value stays visible, so the PPC mkvm -p
profile is not an abbreviation of --password. The dispatch trace builds
its text from the redacted vector.
Add snmpc, productkey, prodkey.key, tokenid and token.tokenid to the
secret list, with community and pdu.community. The secret list holds
only attributes that map to a secret column, so key and sshkeydir stay
visible.
redact_password keeps a second pass over the joined string. This pass
masks an embedded secret assignment to the end of the line, because the
argument boundaries are gone after the join.
The commands.log response classifier marks a response sensitive when the
request was redacted. The argument vector pass sets that signal, so a
secret whose name has no "passw" text still marks its response.
xcatd redacts the request in commands.log but appends the command response
verbatim. A command whose output holds a secret writes it in clear text.
Examples are tabdump passwd, gettab of a passwd column, and getcredentials.
Collect the response into a per-command buffer. Set a sensitive flag when the
command is getcredentials, an argument names a password, or the request was
redacted. When the command finishes, replace the whole buffer if the flag is
set or the buffer still holds password content, then append the buffer. A
connection can carry more than one command, so the buffer is finalized at the
next command's start and at the end of the connection.
The buffer holds the full response, so a secret split across several callbacks
is also redacted. A per-callback check cannot do this.
The word-content check is a fallback. The request classification is the main
signal. A secret with no password marker, such as the output of an xdsh cat of
a shadow file, is a pre-existing leak of the root-only log. It is out of scope.
Recovered from the lenovobuild branch. Reimplemented against master.
xCAT::XML wraps XML::Simple. It blocks XML external entities. An external
entity can read a local file or a network resource.
XML::Simple with new_xml_parser() installs an ExternEnt handler. The
handler returns the system id, not the content.
XML::Simple 2.20 to 2.24 has no new_xml_parser(). On these versions xCAT
uses build_tree_xml_parser() instead. This function did not install the
handler. A SYSTEM entity then expanded. It leaked a local file into the
parsed data.
This change adds the same handler to build_tree_xml_parser(). Both paths
pass parser options in an array reference. XML::Parser->new ignores that
reference. The handler is therefore the real defense. This fix keeps
#6917, which retains the compatibility path for those releases.
This fix was recovered from the lenovobuild branch. See PR #6505.
Only syslog received the redacted arguments. The auditlog table was given the
raw string, so a password removed from syslog and from commands.log was still
written to the database, which persists and is readable by anything with access
to it.
Use the redacted text for both. This also covers the mkvm --password and -w
masking a few lines above, which the table did not have either.
redact_password only knew about bmcdiscover, mkhwconn and rspconfig, so setting
a password the ordinary way left it in the clear:
[Request] chdef node01 'bmcpassword=SEKRET'
Those secrets are carried as an assignment on whichever command happens to set
them, so match them by name rather than by command. The names are every
attribute Schema.pm maps to a password, passwd, authkey or privkey column, and
the columns themselves, since a table qualified assignment such as
passwd.password= is accepted too. An assignment may be written with spaces
around the equals sign and the value may contain spaces, so a quoted argument is
redacted to its closing quote.
Attributes such as key, which names a monitoring attribute, and sshkeydir, which
is a directory, are not secrets and are left readable. The existing per-command
table is kept for a flag like bmcdiscover -p, which carries its secret in the
following argument where there is no name to match.
makedhcp on a noderange sends the request to every service node running a dhcp
server, whether or not it serves any of the named nodes. On a cluster with many
service nodes each one does the work and reports on nodes that are not its
responsibility, for example warning that a node it does not serve has no mac
address.
Map the named nodes to their service nodes and skip the rest. Regenerating the
networks with -n still reaches every dhcp server, since a dynamic range is not
tied to a node, and if none of the named nodes can be mapped the request goes to
all of them as before rather than risk leaving one out.
The node to service node mapping reuses getSNformattedhash, which the
disjointdhcps path in this same function already uses.
Co-authored-by: Jarrod Johnson <10814490+jjohnson42@users.noreply.github.com>
syncfiles builds an xdcp subrequest with no username. xdcp takes its user from
-l, then from DSH_FROM_USERID, and otherwise leaves it unset, so the sync ends
up running as whatever identity xcatd happens to have rather than one the
request states.
That gap dates from 2c9bdf848, which removed the forceroot flag because any
caller could set it, and replaced it by passing the real username through.
updatenode was updated to do that; syncfiles was not. Its request comes from the
node, so there is no calling user to carry through, and root is what the sync
has to run as.
Name it explicitly, the same way updatenode does. This is a consistency and
hardening change: on a management node running as root the resulting identity is
the same either way, and no behavior difference is observable today.
Co-authored-by: Jarrod Johnson <10814490+jjohnson42@users.noreply.github.com>
packimage rejects -o, -p and -a before anything else runs, which leaves three
things below it that cannot be reached. The branch handling a missing image
name asks for -o and reports "Please specify a os version with the -o flag",
so passing the option is rejected and omitting it is reported as the option
being absent. The check for those options alongside an image name re-tests a
condition already rejected, and the block that built an image from them can no
longer be entered.
Report the missing image name instead, and drop the parts that cannot run.
Passing -o, -p or -a is rejected exactly as before, and packing a named image
is unchanged.
setdestiny reports the deprecated install, netboot and statelite states and
returns, and the legacy path that set nodetype.provmethod from the target sits
directly after that return. It has not run since the deprecation landed in
57aa41798, and it still reads like live code when following setdestiny.
Remove it. The deprecated states are still rejected exactly as before, and the
osimage path is untouched.
Daniel Hilst
Vinícius Ferrão