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

fix(dhcp): split the per-node xNBA UEFI branch dhcpd could not parse

Browse Source 
The per-node statement for a UEFI node running netboot=xnba matched its two
architecture ids with a parenthesised alternation:

    else if <user class> and (option client-architecture = 00:09
                              or option client-architecture = 00:07) { ... }

ISC dhcpd has no parenthesised grouping in its expression grammar, so that
condition cannot parse. The node-level second stage it guards -- the
per-node .uefi script that keeps two machines chainloading at the same
moment from running the same script -- has never been reachable.

Written as two branches instead, one per architecture id, which is how the
per-network chain in BootPolicy.pm already spells the same test.

dhcp_isc_expression_grouping.t is a guard rather than a test of this branch:
the per-node statements are built inline in addnode against a live database
and cannot be called from a unit test, so it scans the plugin for the one
construct that produces the failure and checks the rendered per-network
chain and the user class test for it as well. dhcpd's tokens only -- Perl's
own parenthesised `exists` and `not` are excluded, and a bareword `option`
after an opening paren cannot be Perl.
This commit is contained in:
Daniel Hilst
2026-09-10 11:13:27 -03:00
parent 6ecd1994fb
commit d77322289b
2 changed files with 79 additions and 1 deletions
Download Patch File Download Diff File Expand all files Collapse all files
xCAT-server/lib/xcat/plugins/dhcp.pm
+6 -1
View File
@@ -1188,7 +1188,12 @@ sub addnode
$lstatements = 'if ' . $xnba_user_class . ' and option client-architecture = 00:00 { always-broadcast on; filename = \"http://' . $nxtsrv . $portsuffix . '/tftpboot/xcat/xnba/nodes/' . $node . '\"; } else if option client-architecture = 00:07 or option client-architecture = 00:09 { filename = \"\"; option vendor-class-identifier \"PXEClient\"; } else if option client-architecture = 00:00 { filename = \"xcat/xnba.kpxe\"; } else { filename = \"\"; }' . $lstatements; #Only PXE compliant clients should ever receive xNBA
}
} elsif ($douefi and $chainent->{currstate} ne "boot" and $chainent->{currstate} ne "iscsiboot") {
$lstatements = 'if ' . $xnba_user_class . ' and option client-architecture = 00:00 { always-broadcast on; filename = \"http://' . $nxtsrv . $portsuffix . '/tftpboot/xcat/xnba/nodes/' . $node . '\"; } else if ' . $xnba_user_class . ' and (option client-architecture = 00:09 or option client-architecture = 00:07) { filename = \"http://' . $nxtsrv . $portsuffix . '/tftpboot/xcat/xnba/nodes/' . $node . '.uefi\"; } else if option client-architecture = 00:07 { filename = \"xcat/xnba.efi\"; } else if option client-architecture = 00:00 { filename = \"xcat/xnba.kpxe\"; } else { filename = \"\"; }' . $lstatements; #Only PXE compliant clients should ever receive xNBA
# The two UEFI architecture ids are written as separate
# branches rather than one parenthesised alternation:
# dhcpd's expression grammar has no grouping, so
# `... and (a or b) {` is a parse error.
my $uefi_second_stage = 'filename = \"http://' . $nxtsrv . $portsuffix . '/tftpboot/xcat/xnba/nodes/' . $node . '.uefi\";';
$lstatements = 'if ' . $xnba_user_class . ' and option client-architecture = 00:00 { always-broadcast on; filename = \"http://' . $nxtsrv . $portsuffix . '/tftpboot/xcat/xnba/nodes/' . $node . '\"; } else if ' . $xnba_user_class . ' and option client-architecture = 00:09 { ' . $uefi_second_stage . ' } else if ' . $xnba_user_class . ' and option client-architecture = 00:07 { ' . $uefi_second_stage . ' } else if option client-architecture = 00:07 { filename = \"xcat/xnba.efi\"; } else if option client-architecture = 00:00 { filename = \"xcat/xnba.kpxe\"; } else { filename = \"\"; }' . $lstatements; #Only PXE compliant clients should ever receive xNBA
} else {
$lstatements = 'if ' . $xnba_user_class . ' and option client-architecture = 00:00 { filename = \"http://' . $nxtsrv . $portsuffix . '/tftpboot/xcat/xnba/nodes/' . $node . '\"; } else if option client-architecture = 00:00 { filename = \"xcat/xnba.kpxe\"; } else { filename = \"\"; }' . $lstatements; #Only PXE compliant clients should ever receive xNBA
}
xCAT-test/unit/dhcp_isc_expression_grouping.t
+73
View File
@@ -0,0 +1,73 @@
#!/usr/bin/env perl
use strict;
use warnings;
use FindBin;
use lib "$FindBin::Bin/../../perl-xCAT";
use Test::More;
use xCAT::DHCP::BootPolicy;
# ISC dhcpd's expression grammar has no parenthesised grouping. dhcp-eval(5)
# documents exactly three boolean forms -- `not E`, `E1 and E2`, `E1 or E2` --
# and nothing that groups them, so a condition written as
#
# if (option user-class-identifier = "xNBA" or ...) and option client-architecture = 00:00 {
#
# is rejected at the opening paren:
#
# /etc/dhcp/dhcpd.conf line 11: left brace expected.
#
# followed by a cascade of "expecting a parameter or declaration" at every
# `} else` after it. The daemon does not start, so the whole cluster stops
# answering DHCP -- not just the branch that was mis-written.
#
# There is no way to unit test the per-node statements in dhcp.pm directly:
# they are built inline inside addnode against a live database. What can be
# checked cheaply is that no generated ISC condition anywhere in the plugin
# groups a boolean with parentheses, which is the only construct that produces
# this failure. A function call -- substring(...), suffix(...), binary-to-ascii
# -- is fine and is deliberately not matched: `option` never follows an opening
# paren in a call, only in a grouped comparison.
#
# The tokens looked for are dhcpd's, not Perl's: `exists` and `not` are left out
# because the plugin's own Perl uses them parenthesised on nearly every page,
# and a bareword `option` or `filename` immediately after `(` cannot be Perl --
# a Perl variable there would carry its sigil.
my $grouping = qr/\b(?:if|and|or)\s+\(\s*(?:option|substring|suffix|hardware|packet|filename)\b/;
my $plugin = "$FindBin::Bin/../../xCAT-server/lib/xcat/plugins/dhcp.pm";
open( my $fh, '<', $plugin ) or BAIL_OUT("cannot read $plugin: $!");
my @offenders;
while ( my $line = <$fh> ) {
next if $line =~ /^\s*#/;
push @offenders, "$.: $line" if $line =~ $grouping;
}
close $fh;
is_deeply( \@offenders, [],
'no ISC condition in the dhcp plugin groups a boolean with parentheses' )
or diag("dhcpd cannot parse these:\n@offenders");
# The same rule for the per-network architecture chain, checked against what it
# actually renders rather than against its source.
my $rendered = join '', @{ xCAT::DHCP::BootPolicy->isc_client_architecture_lines(
next_server => '192.0.2.10',
portsuffix => ':8080',
net => '192.0.2.0',
prefix => 24,
) };
unlike( $rendered, $grouping,
'the rendered per-network architecture chain groups nothing' );
# And the user class test itself, which is spliced into conditions that already
# carry a trailing `and option client-architecture = ...`.
foreach my $quote ( '"', '\\"' ) {
my $test = xCAT::DHCP::BootPolicy->isc_xnba_user_class_test(quote => $quote);
unlike( $test, qr/^\(/,
'the xNBA user class test is a single ungrouped expression' );
}
done_testing();