diff --git a/perl-xCAT/xCAT/DBobjUtils.pm b/perl-xCAT/xCAT/DBobjUtils.pm index efd20e92f..5e1474e85 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; @@ -2863,12 +2865,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; diff --git a/perl-xCAT/xCAT/Utils.pm b/perl-xCAT/xCAT/Utils.pm index cd1d41220..4b08e226b 100644 --- a/perl-xCAT/xCAT/Utils.pm +++ b/perl-xCAT/xCAT/Utils.pm @@ -5066,3 +5066,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 diff --git a/xCAT-test/unit/dbobjutils_only_if.t b/xCAT-test/unit/dbobjutils_only_if.t index 910cbd616..cb95162be 100644 --- a/xCAT-test/unit/dbobjutils_only_if.t +++ b/xCAT-test/unit/dbobjutils_only_if.t @@ -23,6 +23,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', @@ -50,6 +60,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; @@ -67,7 +209,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} = { @@ -96,6 +238,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', + }, ], }; @@ -103,9 +263,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 (); }; @@ -124,23 +284,28 @@ 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'); +} my %external_entries; foreach my $type (keys %xCAT::ExtTab::ext_defspec) { 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();