fix(dhcp): match the xNBA user class without parenthesised grouping

isc_xnba_user_class_test wrapped its two alternatives in parentheses so the
trailing `and option client-architecture = ...` would bind to the whole
alternation rather than to the second branch alone. ISC dhcpd has no
parenthesised grouping in its expression grammar, so every generated
dhcpd.conf became unparseable:

    /etc/dhcp/.../dhcpd.conf line 11: left brace expected.
        if (option user-class-identifier = "xNBA" or substring(option user-cl
    ^

followed by a cascade of "expecting a parameter or declaration" at each
`} else`, and a daemon that will not start at all.

suffix() gives the same coverage in a single expression that needs no
grouping: it returns the last four bytes, which are "xNBA" whether the
client sent the bare string or the RFC 3004 length-prefixed "\x04xNBA".

The unit test now pins the whole expression rather than each alternative,
and asserts the two properties that broke the daemon -- no leading paren,
no `or` -- so a future rewrite cannot reintroduce grouping unnoticed.

perl-xCAT/xCAT/DHCP/BootPolicy.pm

@@ -140,6 +140,12 @@ sub kea_s390x_network_classes {
 # forever, which is why the Kea policy has always accepted both
 # (xnba_user_class_test) and why this one has to as well.
 #
+# suffix() takes the last N bytes, so one expression covers both encodings: the
+# bare "xNBA" is its own last four bytes, and the RFC 3004 form "\x04xNBA" ends
+# in the same four. It has to be a single expression, because dhcpd's grammar
+# has no parenthesised grouping -- writing the two forms as `(a or b)` is a
+# parse error ("left brace expected") that stops the daemon from starting.
+#
 # `quote` is the quoting the caller's context needs: a plain " for a config
 # file written directly, and \" for a statement that reaches dhcpd through
 # omshell.
@@ -148,8 +154,7 @@ sub isc_xnba_user_class_test {
 
     my $q = defined( $opts{quote} ) ? $opts{quote} : '"';
 
-    return "(option user-class-identifier = ${q}xNBA${q}"
-      . " or substring(option user-class-identifier, 1, 4) = ${q}xNBA${q})";
+    return "suffix(option user-class-identifier, 4) = ${q}xNBA${q}";
 }
 
 sub isc_client_architecture_lines {

xCAT-test/dhcptest/spec.md

@@ -229,9 +229,11 @@ Scenario Outline: The user class is recognised however the client encodes it
     | RFC 3004 length-prefixed, as the RFC says |
   # Kea accepts both: kea_xnba_user_class_test tests option[77].text, the raw
   # hex, and the length-prefixed substring. ISC accepts both through
-  # isc_xnba_user_class_test, which pairs the bare comparison with
-  # `substring(option user-class-identifier, 1, 4)` -- option 77 is declared as
-  # a plain string there (dhcp.pm), so offset 1 skips the RFC 3004 length byte.
+  # isc_xnba_user_class_test, which compares the last four bytes:
+  # `suffix(option user-class-identifier, 4) = "xNBA"` is true of the bare
+  # string and of "\x04xNBA" alike. It has to be one expression rather than an
+  # alternation -- dhcpd's grammar has no parenthesised grouping, so `if (a or
+  # b) and c {` is a parse error and the daemon will not start.
 
 Scenario: A known node's second stage is addressed to that node
   Given a node whose netboot method is xnba

xCAT-test/unit/dhcp_isc_client_arch.t

@@ -71,6 +71,12 @@ like($rendered, qr/filename "\/yaboot";\n\s*\}\n\z/, 'the policy ends with the e
 # and never says why. The Kea side of this plugin has always accepted both
 # (kea_xnba_user_class_test); the ISC side has to agree, or the same machine
 # boots on one backend and loops on the other.
+#
+# suffix() takes the last four bytes, which is "xNBA" either way: the bare
+# string is its own suffix, and "\x04xNBA" ends in the same four bytes. One
+# expression rather than an alternation, because dhcpd has no parenthesised
+# grouping -- `if (a or b) and c {` is rejected outright with "left brace
+# expected" and the daemon does not start.
 foreach my $case (
     [ '"',   'a config file written directly' ],
     [ '\\"', 'a statement passed through omshell' ],
@@ -79,17 +85,13 @@ foreach my $case (
     my ($quote, $context) = @{$case};
     my $test = xCAT::DHCP::BootPolicy->isc_xnba_user_class_test(quote => $quote);
 
-    like( $test, qr/\Qoption user-class-identifier = ${quote}xNBA${quote}\E/,
-        "the bare user class is matched, for $context" );
-    like( $test, qr/\Qsubstring(option user-class-identifier, 1, 4) = ${quote}xNBA${quote}\E/,
-        "the RFC 3004 length-prefixed user class is matched, for $context" );
+    is( $test, "suffix(option user-class-identifier, 4) = ${quote}xNBA${quote}",
+        "both encodings are matched by one suffix test, for $context" );
 
-    # dhcpd's `if` is a single expression: without the parentheses the trailing
-    # `and option client-architecture = ...` binds to the second alternative
-    # only, and every xNBA client is served the first branch whatever its
-    # architecture.
-    like( $test, qr/^\(.*\)$/s,
-        "the alternation is parenthesised so it can be and-ed with a further test, for $context" );
+    unlike( $test, qr/^\(/,
+        "the test is not wrapped in parentheses dhcpd cannot parse, for $context" );
+    unlike( $test, qr/\bor\b/,
+        "the test is a single expression, needing no grouping, for $context" );
 }
 
 is( xCAT::DHCP::BootPolicy->isc_xnba_user_class_test(),
@@ -99,11 +101,11 @@ is( xCAT::DHCP::BootPolicy->isc_xnba_user_class_test(),
 # ...and the per-network policy uses it, rather than its own bare comparison.
 foreach my $arch (qw(00:00 00:09 00:07)) {
     like( $rendered,
-        qr/\Qsubstring(option user-class-identifier, 1, 4) = "xNBA")\E and option client-architecture = \Q$arch\E/,
+        qr/\Qsuffix(option user-class-identifier, 4) = "xNBA"\E and option client-architecture = \Q$arch\E/,
         "the xNBA branch for client architecture $arch accepts both encodings" );
 }
 
-unlike( $rendered, qr/(?<!\()option user-class-identifier = "xNBA" and/,
+unlike( $rendered, qr/option user-class-identifier = "xNBA" and/,
     'no xNBA branch is left matching the bare encoding alone' );
 
 done_testing();