From 6e4384371294e012400c4158a0b1cc9cbf87730f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Vin=C3=ADcius=20Ferr=C3=A3o?= <2031761+viniciusferrao@users.noreply.github.com> Date: Sun, 23 Aug 2026 11:08:24 -0300 Subject: [PATCH 1/4] refactor(utils): centralize comma-list membership --- perl-xCAT/xCAT/Utils.pm | 13 +++++++++++++ xCAT-server/lib/perl/xCAT/SvrUtils.pm | 13 ++----------- 2 files changed, 15 insertions(+), 11 deletions(-) diff --git a/perl-xCAT/xCAT/Utils.pm b/perl-xCAT/xCAT/Utils.pm index 6c969e2c5..b1c1c1c4b 100644 --- a/perl-xCAT/xCAT/Utils.pm +++ b/perl-xCAT/xCAT/Utils.pm @@ -5045,3 +5045,16 @@ sub strim{ $str =~ s/^\s+|\s+$//g; return $str; } + +# Return true when a value is an exact element of a comma-delimited list. +sub comma_list_contains { + my $class = shift; + my $list = shift; + my $value = shift; + + return 0 unless defined($list) && defined($value); + foreach my $item (split /,/, $list) { + return 1 if $class->strim($item) eq $value; + } + return 0; +} diff --git a/xCAT-server/lib/perl/xCAT/SvrUtils.pm b/xCAT-server/lib/perl/xCAT/SvrUtils.pm index e1a8df247..d28a456d4 100644 --- a/xCAT-server/lib/perl/xCAT/SvrUtils.pm +++ b/xCAT-server/lib/perl/xCAT/SvrUtils.pm @@ -1798,13 +1798,13 @@ sub ensure_nfs_export_option { for my $tok (@tokens) { if ($tok =~ /^-(.+)/) { my $opts = $1; - unless (_option_list_has($opts, $option)) { + unless (xCAT::Utils->comma_list_contains($opts, $option)) { $tok = "-$opts,$option"; $line_changed = 1; } } elsif ($tok =~ /^([^(]+)\(([^)]*)\)$/) { my ($client, $opts) = ($1, $2); - unless (_option_list_has($opts, $option)) { + unless (xCAT::Utils->comma_list_contains($opts, $option)) { $tok = "$client($opts,$option)"; $line_changed = 1; } @@ -1844,15 +1844,6 @@ sub _nfs_export_files { return @files; } -sub _option_list_has { - my ($list, $option) = @_; - foreach my $item (split /,/, $list) { - $item =~ s/^\s+|\s+$//g; - return 1 if $item eq $option; - } - return 0; -} - #------------------------------------------------------------------------------------------- # Common method to send info back to the client # The last two args are optional, though $allerrornodes will unlikely be there without $node From 5d39fc30d41dcddafb699878037534fb511d7eb6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Vin=C3=ADcius=20Ferr=C3=A3o?= <2031761+viniciusferrao@users.noreply.github.com> Date: Sun, 23 Aug 2026 11:08:59 -0300 Subject: [PATCH 2/4] test(utils): cover comma-list membership --- xCAT-test/unit/utils_comma_list.t | 39 +++++++++++++++++++++++++++++++ 1 file changed, 39 insertions(+) create mode 100644 xCAT-test/unit/utils_comma_list.t diff --git a/xCAT-test/unit/utils_comma_list.t b/xCAT-test/unit/utils_comma_list.t new file mode 100644 index 000000000..4a9e98373 --- /dev/null +++ b/xCAT-test/unit/utils_comma_list.t @@ -0,0 +1,39 @@ +#!/usr/bin/env perl +use strict; +use warnings; + +use FindBin; +use lib "$FindBin::Bin/../../perl-xCAT"; + +use Test::More; + +use xCAT::Utils; + +my @cases = ( + [undef, 'pdu', 0, 'undefined list does not match'], + ['', 'pdu', 0, 'empty list does not match'], + ['pdu', undef, 0, 'undefined target does not match'], + ['pdu', 'pdu', 1, 'single value matches'], + ['pdu,openbmc', 'openbmc', 1, 'comma-list value matches'], + [' pdu , openbmc ', 'pdu', 1, 'leading and trailing item whitespace is ignored'], + [' pdu , openbmc ', 'openbmc', 1, 'whitespace is ignored for later items'], + ['0', '0', 1, 'defined zero matches'], + ['not-pdu', 'pdu', 0, 'hyphenated prefix is not a match'], + ['pdu-extra', 'pdu', 0, 'hyphenated suffix is not a match'], + ['pdu.extra', 'pdu', 0, 'dotted suffix is not a match'], + ['.pdu,pdu.,-pdu,pdu-', '.pdu', 1, 'leading punctuation matches exactly'], + ['.pdu,pdu.,-pdu,pdu-', 'pdu.', 1, 'trailing punctuation matches exactly'], + ['.pdu,pdu.,-pdu,pdu-', '-pdu', 1, 'leading hyphen matches exactly'], + ['.pdu,pdu.,-pdu,pdu-', 'pdu-', 1, 'trailing hyphen matches exactly'], +); + +foreach my $case (@cases) { + my ($list, $value, $expected, $description) = @$case; + is( + xCAT::Utils->comma_list_contains($list, $value), + $expected, + $description, + ); +} + +done_testing(); From 3aa826df925f61feecd7ac2d958becf48bdff622 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Vin=C3=ADcius=20Ferr=C3=A3o?= <2031761+viniciusferrao@users.noreply.github.com> Date: Sun, 23 Aug 2026 11:09:26 -0300 Subject: [PATCH 3/4] fix(dbobjutils): match exact only-if values --- perl-xCAT/xCAT/DBobjUtils.pm | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/perl-xCAT/xCAT/DBobjUtils.pm b/perl-xCAT/xCAT/DBobjUtils.pm index f0eeec592..55bdb3a42 100644 --- a/perl-xCAT/xCAT/DBobjUtils.pm +++ b/perl-xCAT/xCAT/DBobjUtils.pm @@ -506,7 +506,9 @@ sub getobjdefs if (defined($check_attr) && defined($check_value)) { # if the object value is not the value we need # to match then try the next only_if value - next if (!($objhash{$objname}{$check_attr} =~ /\b$check_value\b/)); + next unless xCAT::Utils->comma_list_contains( + $objhash{$objname}{$check_attr}, + $check_value); } $objhash{$objname}{'objtype'} = $objtype; @@ -2875,12 +2877,12 @@ sub _only_if_value_matches foreach my $source ($attrs_ref, $dbattrs_ref) { next unless $source; my $value = $source->{$check_attr}; - return 1 if defined($value) && $value =~ /\b\Q$check_value\E\b/; + return 1 if xCAT::Utils->comma_list_contains($value, $check_value); } foreach my $group (keys %{ $groupattrs_ref || {} }) { my $value = $groupattrs_ref->{$group}{$check_attr}; - return 1 if defined($value) && $value =~ /\b\Q$check_value\E\b/; + return 1 if xCAT::Utils->comma_list_contains($value, $check_value); } return 0; From 21755f8f93a4e9df4aaa0f7d967685efc96a0de3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Vin=C3=ADcius=20Ferr=C3=A3o?= <2031761+viniciusferrao@users.noreply.github.com> Date: Sun, 23 Aug 2026 11:12:46 -0300 Subject: [PATCH 4/4] test(dbobjutils): cover exact only-if value matching --- xCAT-test/unit/dbobjutils_only_if.t | 181 ++++++++++++++++++++++++++-- 1 file changed, 173 insertions(+), 8 deletions(-) diff --git a/xCAT-test/unit/dbobjutils_only_if.t b/xCAT-test/unit/dbobjutils_only_if.t index 8f9ffe02e..bc75752d9 100644 --- a/xCAT-test/unit/dbobjutils_only_if.t +++ b/xCAT-test/unit/dbobjutils_only_if.t @@ -22,6 +22,16 @@ my @failures = xCAT::DBobjUtils->validate_only_if_attrs('node01', 'node', \%miss is(scalar @failures, 1, 'bmc without mgt fails only_if validation'); like($failures[0]->{message}, qr/mgt value is .*openbmc/, 'failure explains accepted mgt values'); +my %substring_openbmc = ( + objtype => 'node', + groups => 'test', + mgt => 'openbmc-redfish', + bmc => '10.0.0.1', +); +@failures = xCAT::DBobjUtils->validate_only_if_attrs('node01', 'node', \%substring_openbmc, {}); +is(scalar @failures, 1, 'substring-only mgt value fails bmc only_if validation'); +like($failures[0]->{message}, qr/mgt value is .*openbmc/, 'substring rejection explains accepted mgt values'); + my %explicit_openbmc = ( objtype => 'node', groups => 'test', @@ -49,6 +59,138 @@ my %groupattrs = (openbmcgrp => { mgt => 'openbmc' }); @failures = xCAT::DBobjUtils->validate_only_if_attrs('node01', 'node', \%group_openbmc, {}, \%groupattrs); is(scalar @failures, 0, 'group mgt=openbmc satisfies bmc only_if validation'); +sub check_exact_only_if_value { + my ($source, $actual, $required, $matches, $description) = @_; + my (%attrs, %dbattrs, %groupattrs); + + $attrs{selector} = $actual if $source eq 'explicit'; + $dbattrs{selector} = $actual if $source eq 'database'; + $groupattrs{exactgroup}{selector} = $actual if $source eq 'group'; + + is( + xCAT::DBobjUtils::_only_if_value_matches( + \%attrs, + \%dbattrs, + \%groupattrs, + 'selector', + $required, + ), + $matches, + "$source source $description", + ); +} + +my @exact_match_cases = ( + ['pdu', 'pdu', 1, 'matches a single value'], + ['openbmc,pdu', 'pdu', 1, 'matches an exact list element'], + [' openbmc , pdu ', 'pdu', 1, 'matches a whitespace-padded list element'], + [0, '0', 1, 'matches a defined zero'], + ['not-pdu', 'pdu', 0, 'rejects a hyphenated prefix'], + ['pdu-extra', 'pdu', 0, 'rejects a hyphenated suffix'], + ['pdu.extra', 'pdu', 0, 'rejects a dotted suffix'], + ['.pdu', 'pdu', 0, 'rejects a punctuated prefix'], + ['pdu.', 'pdu', 0, 'rejects a punctuated suffix'], + ['.pdu', '.pdu', 1, 'matches a leading-punctuation value exactly'], + ['pdu.', 'pdu.', 1, 'matches a trailing-punctuation value exactly'], + ['', 'pdu', 0, 'rejects an empty value'], + [undef, 'pdu', 0, 'rejects an undefined value'], +); + +foreach my $source (qw(explicit database group)) { + check_exact_only_if_value($source, @$_) for @exact_match_cases; +} + +is( + xCAT::DBobjUtils::_only_if_value_matches( + { selector => 'not-pdu' }, + { selector => 'pdu.extra' }, + { exactgroup => { selector => ' openbmc , pdu ' } }, + 'selector', + 'pdu', + ), + 1, + 'later exact source match preserves source OR semantics', +); + +sub check_literal_only_if_read { + my ($selector, $expected_payload, $description) = @_; + + local $xCAT::Schema::defspec{node} = { + objkey => 'name', + attrs => [ + { + attr_name => 'selector', + tabentry => 'route_source.selector', + access_tabentry => 'route_source.node=attr:name', + }, + { + attr_name => 'payload', + only_if => 'selector=axb', + tabentry => 'route_expected.payload', + access_tabentry => 'route_expected.node=attr:name', + }, + { + attr_name => 'payload', + only_if => 'selector=a.b', + tabentry => 'route_wrong.payload', + access_tabentry => 'route_wrong.node=attr:name', + }, + { + attr_name => 'payload', + only_if => 'selector=not-pdu', + tabentry => 'route_not_pdu.payload', + access_tabentry => 'route_not_pdu.node=attr:name', + }, + { + attr_name => 'payload', + only_if => 'selector=pdu.extra', + tabentry => 'route_pdu_extra.payload', + access_tabentry => 'route_pdu_extra.node=attr:name', + }, + { + attr_name => 'payload', + only_if => 'selector=pdu', + tabentry => 'route_pdu.payload', + access_tabentry => 'route_pdu.node=attr:name', + }, + ], + }; + + my @tables = qw( + route_source route_expected route_wrong route_not_pdu + route_pdu_extra route_pdu + ); + local @xCAT::Schema::tabspec{@tables}; + @xCAT::Schema::tabspec{@tables} = map { { nodecol => 'node' } } @tables; + + no warnings 'redefine'; + local *xCAT::DBobjUtils::getobjattrs = sub { + return ( + route_source => { node01 => { selector => $selector } }, + route_expected => { node01 => { payload => 'literal axb' } }, + route_wrong => { node01 => { payload => 'literal a.b' } }, + route_not_pdu => { node01 => { payload => 'literal not-pdu' } }, + route_pdu_extra => { node01 => { payload => 'literal pdu.extra' } }, + route_pdu => { node01 => { payload => 'literal pdu' } }, + ); + }; + + local $::ATTRLIST = ''; + my %objects = (node01 => 'node'); + my %defs = xCAT::DBobjUtils->getobjdefs( + \%objects, + 0, + ['selector', 'payload'], + ); + + is($defs{node01}{payload}, $expected_payload, $description); +} + +check_literal_only_if_read('axb', 'literal axb', 'read routing treats regex punctuation literally'); +check_literal_only_if_read('not-pdu', 'literal not-pdu', 'read routing rejects a hyphenated substring match'); +check_literal_only_if_read('pdu.extra', 'literal pdu.extra', 'read routing rejects a dotted substring match'); +check_literal_only_if_read('other,pdu', 'literal pdu', 'read routing accepts an exact comma-list element'); + { package DBobjUtilsOnlyIf::TableRecorder; @@ -66,7 +208,7 @@ is(scalar @failures, 0, 'group mgt=openbmc satisfies bmc only_if validation'); } sub check_literal_only_if_routing { - my ($source) = @_; + my ($source, $selector, $expected_table, $description) = @_; my @writes; local $xCAT::Schema::defspec{routing_fixture} = { @@ -95,6 +237,24 @@ sub check_literal_only_if_routing { tabentry => 'route_expected.payload', access_tabentry => 'route_expected.node=attr:name', }, + { + attr_name => 'payload', + only_if => 'selector=pdu', + tabentry => 'route_pdu.payload', + access_tabentry => 'route_pdu.node=attr:name', + }, + { + attr_name => 'payload', + only_if => 'selector=not-pdu', + tabentry => 'route_not_pdu.payload', + access_tabentry => 'route_not_pdu.node=attr:name', + }, + { + attr_name => 'payload', + only_if => 'selector=pdu.extra', + tabentry => 'route_pdu_extra.payload', + access_tabentry => 'route_pdu_extra.node=attr:name', + }, ], }; @@ -102,9 +262,9 @@ sub check_literal_only_if_routing { local *xCAT::DBobjUtils::getobjdefs = sub { my ($class, $objects) = @_; - return (node01 => { selector => 'axb' }) + return (node01 => { selector => $selector }) if $source eq 'database' && exists $objects->{node01}; - return (literalgroup => { selector => 'axb' }) + return (literalgroup => { selector => $selector }) if $source eq 'group' && exists $objects->{literalgroup}; return (); }; @@ -123,22 +283,27 @@ sub check_literal_only_if_routing { objtype => 'routing_fixture', payload => 'stored', ); - $attrs{selector} = 'axb' if $source eq 'explicit'; + $attrs{selector} = $selector if $source eq 'explicit'; $attrs{groups} = 'literalgroup' if $source eq 'group'; my %objects = (node01 => \%attrs); my $rc = xCAT::DBobjUtils->setobjdefs(\%objects); - is($rc, 0, "$source source passes only_if validation"); + is($rc, 0, "$source source $description passes only_if validation"); my @payload_tables = sort map { $_->{table} } grep { exists $_->{updates}{payload} } @writes; is_deeply( \@payload_tables, - ['route_expected'], - "$source source routes through only the literal-matching only_if entry", + [$expected_table], + "$source source $description routes through only the exact only_if entry", ); } -check_literal_only_if_routing($_) for qw(explicit database group); +foreach my $source (qw(explicit database group)) { + check_literal_only_if_routing($source, 'axb', 'route_expected', 'literal value'); + check_literal_only_if_routing($source, 'not-pdu', 'route_not_pdu', 'hyphenated value'); + check_literal_only_if_routing($source, 'pdu.extra', 'route_pdu_extra', 'dotted value'); + check_literal_only_if_routing($source, 'other,pdu', 'route_pdu', 'comma-list value'); +} done_testing();