diff --git a/perl-xCAT/xCAT/DBobjUtils.pm b/perl-xCAT/xCAT/DBobjUtils.pm index 476a02f56..8036e0e5a 100644 --- a/perl-xCAT/xCAT/DBobjUtils.pm +++ b/perl-xCAT/xCAT/DBobjUtils.pm @@ -959,6 +959,24 @@ sub setobjdefs %DBgroupsattr=xCAT::DBobjUtils->getobjdefs(\%tmpghash); } + my @only_if_failures = xCAT::DBobjUtils->validate_only_if_attrs( + $objname, + $type, + $objhash{$objname}, + $DBattrvals{$objname} || {}, + \%DBgroupsattr, + ); + if (@only_if_failures) { + foreach my $failure (@only_if_failures) { + my $rsp; + $rsp->{data}->[0] = $failure->{message}; + xCAT::MsgUtils->message("E", $rsp, $::callback); + } + $objhash{$objname}{error} = 1; + $ret = 1; + next; + } + # check FINALATTRS to see if all the attrs are valid foreach my $attr (keys %{ $objhash{$objname} }) { @@ -1032,7 +1050,16 @@ sub setobjdefs # as well as the attrs for this object that may be # already set in DB - if (!($objhash{$objname}{$check_attr}) && !($DBattrvals{$objname}{$check_attr})) { + # Also check group inheritance for the condition + my $grp_has_check_attr = 0; + foreach my $tmpgrp (@tmplgrplist) { + if ($DBgroupsattr{$tmpgrp}{$check_attr}) { + $grp_has_check_attr = 1; + last; + } + } + + if (!($objhash{$objname}{$check_attr}) && !($DBattrvals{$objname}{$check_attr}) && !$grp_has_check_attr) { # if I didn't already check for this attr my $rsp; @@ -1063,7 +1090,17 @@ sub setobjdefs next; } - if (!($objhash{$objname}{$check_attr} =~ /\b$check_value\b/) && !($DBattrvals{$objname}{$check_attr} =~ /\b$check_value\b/)) { + my $grp_matches_check_value = 0; + foreach my $tmpgrp (@tmplgrplist) { + if ($DBgroupsattr{$tmpgrp}{$check_attr} && $DBgroupsattr{$tmpgrp}{$check_attr} =~ /\b$check_value\b/) { + $grp_matches_check_value = 1; + last; + } + } + + my $obj_val = $objhash{$objname}{$check_attr} || ''; + my $db_val = $DBattrvals{$objname}{$check_attr} || ''; + if (!($obj_val =~ /\b$check_value\b/) && !($db_val =~ /\b$check_value\b/) && !$grp_matches_check_value) { if ($invalidattr->{$attr_name}->{valid} != 1) { $invalidattr->{$attr_name}->{valid} = 0; $invalidattr->{$attr_name}->{condition}=$check_attr; @@ -1211,6 +1248,7 @@ sub setobjdefs } my $rsp; + my $object_failed_validation = 0; foreach my $att (keys %$invalidattr) { my $pickvalidattr=0; if ($invalidattr->{$att}->{valid} != 1) { @@ -1230,33 +1268,17 @@ sub setobjdefs $conditionlist->{$conditionkey}=~s/,/ or /g; push @{ $rsp->{data} }, "Cannot set the attr=\'$att\' attribute unless $invalidattr->{$att}->{condition} value is $conditionlist->{$conditionkey}."; xCAT::MsgUtils->message("E", $rsp, $::callback); + $object_failed_validation = 1; } } } - - - # TODO - need to get back to this - if (0) { - # - # check to see if all the attrs got set - # - - my @errlist; - foreach $a (@attrprovided) { - - # is this attr was not set then add it to the error list - if (!grep(/^$a$/, @setattrlist)) { - push(@errlist, $a); - $ret = 2; - } - + if ($object_failed_validation) { + $objhash{$objname}{error} = 1; + delete $objhash{$objname}{updated}; + foreach my $table (keys %allupdates) { + delete $allupdates{$table}{$objname}; } - if ($ret == 2) { - my $rsp; - $rsp->{data}->[0] = "Could not set the following attributes for the \'$objname\' definition in the xCAT database: \'@errlist\'"; - xCAT::MsgUtils->message("E", $rsp, $::callback); - } - + $ret = 1; } } # end - foreach object @@ -2766,4 +2788,107 @@ sub collapsenicsattr() } } +#---------------------------------------------------------------------------- + +=head3 validate_only_if_attrs + + Validate Schema.pm only_if rules for attributes being set on one + object. The object is valid when any schema entry for the attribute is + unconditional or when one only_if condition is satisfied by the new + object attributes, existing database attributes, or group attributes. + + Arguments: + $objname - object name + $type - object type + $attrs_ref - attributes being set + $dbattrs_ref - existing attributes to allow as conditions + $groupattrs_ref - optional group attributes, keyed by group name + + Returns: + @failures: + ({ attr => ..., message => ... }, ...) + +=cut + +#----------------------------------------------------------------------------- +sub validate_only_if_attrs +{ + my ($class, $objname, $type, $attrs_ref, $dbattrs_ref, $groupattrs_ref) = @_; + my @failures; + my $datatype = $xCAT::Schema::defspec{$type}; + return @failures unless $datatype; + + $attrs_ref ||= {}; + $dbattrs_ref ||= {}; + + my %groupattrs; + if ($groupattrs_ref) { + %groupattrs = %$groupattrs_ref; + } elsif ($attrs_ref->{groups}) { + my %groups; + foreach my $group (split(",", $attrs_ref->{groups})) { + $groups{$group} = "group"; + } + %groupattrs = xCAT::DBobjUtils->getobjdefs(\%groups) if %groups; + } + + my %attr_has_unconditional; + my %attr_has_valid_condition; + my %invalidattr; + my %conditionlist; + + foreach my $this_attr (@{ $datatype->{'attrs'} }) { + my $attr_name = $this_attr->{attr_name}; + next unless defined($attrs_ref->{$attr_name}); + + if (!exists($this_attr->{only_if})) { + $attr_has_unconditional{$attr_name} = 1; + next; + } + + next unless $this_attr->{only_if} =~ /^([^!=]+)=(.+)$/; + my ($check_attr, $check_value) = ($1, $2); + + if (_only_if_value_matches($attrs_ref, $dbattrs_ref, \%groupattrs, $check_attr, $check_value)) { + $attr_has_valid_condition{$attr_name} = 1; + next; + } + + next if $attr_has_valid_condition{$attr_name}; + $invalidattr{$attr_name}{condition} = $check_attr; + $conditionlist{$check_attr}{$check_value} = 1; + } + + foreach my $att (keys %invalidattr) { + next if $attr_has_unconditional{$att}; + next if $attr_has_valid_condition{$att}; + my $conditionkey = $invalidattr{$att}{condition}; + my $condlist = join(" or ", sort keys %{ $conditionlist{$conditionkey} }); + push @failures, { + attr => $att, + message => "Cannot set the attr=\'$att\' attribute unless $conditionkey value is $condlist.", + }; + } + + return @failures; +} + +sub _only_if_value_matches +{ + my ($attrs_ref, $dbattrs_ref, $groupattrs_ref, $check_attr, $check_value) = @_; + + 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/; + } + + 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 0; +} + 1; diff --git a/xCAT-server/lib/xcat/plugins/DBobjectdefs.pm b/xCAT-server/lib/xcat/plugins/DBobjectdefs.pm index 52e5b5845..c9c41cb31 100644 --- a/xCAT-server/lib/xcat/plugins/DBobjectdefs.pm +++ b/xCAT-server/lib/xcat/plugins/DBobjectdefs.pm @@ -1671,14 +1671,23 @@ sub defmk # Pull all the pieces together for the final hash # - combines the command line attrs and input file attrs if provided # + my $command_level_error = $error ? 1 : 0; if (&setFINALattrs != 0) { $error = 1; + $command_level_error = 1; + } + + if ($command_level_error) + { + %::FINALATTRS = (); } # If no object definitions ended up in FINALATTRS, there is nothing to create - if (!$error && !%::FINALATTRS) + if (!%::FINALATTRS) { + return 1 if $error; + my $rsp; my $is_node_type = (grep { $_ eq "node" } @::finalTypeList) || ($::opt_t && $::opt_t =~ /\bnode\b/); @@ -1790,34 +1799,107 @@ sub defmk } - # if object already exists - if (defined($objTypeListsHash{$type}{$obj}) && ($objTypeListsHash{$type}{$obj} == 1)) + my $object_exists = (defined($objTypeListsHash{$type}{$obj}) && ($objTypeListsHash{$type}{$obj} == 1)); + if ($object_exists && !$::opt_f) { - if ($::opt_f) + # won't remove the old one unless the force option is used + my $rsp; + $rsp->{data}->[0] = "A definition for \'$obj\' already exists. No changes will be made. Run again with \'-f\' option to force replace."; + xCAT::MsgUtils->message("W", $rsp, $::callback); + delete $::FINALATTRS{$obj}; + next; + } + + # Reject object-level validation failures before -f removes an old + # definition or group handling mutates member nodes. + if (($type eq "node") && (!defined($::FINALATTRS{$obj}{groups}) || !$::FINALATTRS{$obj}{groups})) + { + my $rsp; + $rsp->{data}->[0] = "Attribute \'groups\' is not specified for node \'$obj\', skipping to the next node."; + xCAT::MsgUtils->message("E", $rsp, $::callback); + $error = 1; + delete $::FINALATTRS{$obj}; + next; + } + + if ($type eq 'group') + { + my $is_dynamic_group = 0; + if (!$::FINALATTRS{$obj}{grouptype}) { - # remove the old object - my %objhash; - $objhash{$obj} = $type; - if (xCAT::DBobjUtils->rmobjdefs(\%objhash) != 0) + if ($::opt_d) { - $error = 1; - my $rsp; - $rsp->{data}->[0] = "Could not remove the definition for \'$obj\'."; - xCAT::MsgUtils->message("E", $rsp, $::callback); + if (scalar(keys %{ $::FINALATTRS{$obj} }) > 1) + { + my $rsp; + $rsp->{data}->[0] = "Can not assign attributes to dynamic node group \'$obj\'."; + xCAT::MsgUtils->message("E", $rsp, $::callback); + $error = 1; + delete $::FINALATTRS{$obj}; + next; + } + $is_dynamic_group = 1; } } - else + elsif ($::FINALATTRS{$obj}{grouptype} eq 'dynamic') { - - # won't remove the old one unless the force option is used - my $rsp; - $rsp->{data}->[0] = "A definition for \'$obj\' already exists. No changes will be made. Run again with \'-f\' option to force replace."; - xCAT::MsgUtils->message("W", $rsp, $::callback); - delete $::FINALATTRS{$obj}; - next; - + $is_dynamic_group = 1; } + if ($is_dynamic_group && !$::FINALATTRS{$obj}{wherevals} && !$::opt_w) + { + my $rsp; + $rsp->{data}->[0] = "The \'where\' attributes and values were not provided for dynamic group \'$obj\'."; + $rsp->{data}->[1] = "Skipping to the next group."; + xCAT::MsgUtils->message("E", $rsp, $::callback); + $error = 1; + delete $::FINALATTRS{$obj}; + next; + } + + if (!$is_dynamic_group && $::opt_w && $::FINALATTRS{$obj}{members}) + { + my $rsp; + $rsp->{data}->[0] = "Cannot use a list of members together with the \'-w\' option."; + xCAT::MsgUtils->message("E", $rsp, $::callback); + $error = 1; + delete $::FINALATTRS{$obj}; + next; + } + } + + my @only_if_failures = xCAT::DBobjUtils->validate_only_if_attrs( + $obj, + $type, + $::FINALATTRS{$obj}, + {}, + ); + if (@only_if_failures) + { + foreach my $failure (@only_if_failures) { + my $rsp; + $rsp->{data}->[0] = $failure->{message}; + xCAT::MsgUtils->message("E", $rsp, $::callback); + } + $error = 1; + delete $::FINALATTRS{$obj}; + next; + } + + if ($object_exists && $::opt_f) + { + # remove the old object after the replacement definition is known valid + my %objhash; + $objhash{$obj} = $type; + if (xCAT::DBobjUtils->rmobjdefs(\%objhash) != 0) + { + $error = 1; + my $rsp; + $rsp->{data}->[0] = "Could not remove the definition for \'$obj\'."; + xCAT::MsgUtils->message("E", $rsp, $::callback); + delete $::FINALATTRS{$obj}; + next; + } } # need to handle group definitions - special! @@ -2021,6 +2103,7 @@ sub defmk $rsp->{data}->[0] = "Attribute \'groups\' is not specified for node \'$obj\', skipping to the next node."; xCAT::MsgUtils->message("E", $rsp, $::callback); $error = 1; + delete $::FINALATTRS{$obj}; next; } @@ -4835,4 +4918,3 @@ sub isobjnamevalid{ } 1; - diff --git a/xCAT-test/autotest/testcase/mkdef/cases0 b/xCAT-test/autotest/testcase/mkdef/cases0 index 16a18eefe..f61dc1861 100644 --- a/xCAT-test/autotest/testcase/mkdef/cases0 +++ b/xCAT-test/autotest/testcase/mkdef/cases0 @@ -415,6 +415,7 @@ end start:mkdef_node_no_attrs description:mkdef -t node with no attributes should fail and mention groups label:mn_only,ci_test,db +cmd:rmdef -t node testnode_noattr cmd:mkdef -t node -o testnode_noattr check:rc!=0 check:output=~groups @@ -425,12 +426,13 @@ end start:mkdef_node_no_groups description:mkdef -t node with attributes but no groups should fail label:mn_only,ci_test,db +cmd:rmdef -t node testnode_nogrp cmd:mkdef -t node -o testnode_nogrp ip=10.0.0.99 check:rc!=0 check:output=~groups cmd:lsdef testnode_nogrp check:rc!=0 -cmd:rmdef -t node -o testnode_nogrp +cmd:rmdef -t node testnode_nogrp end start:mkdef_nonnode_no_attrs @@ -439,6 +441,163 @@ label:mn_only,ci_test,db cmd:mkdef -t network -o testnet_noattr check:rc!=0 check:output=~no attributes were specified -cmd:rmdef -t network -o testnet_noattr +cmd:rmdef -t network testnet_noattr end +start:mkdef_validation_error_no_partial_object +description:mkdef should not create a partial node when an attribute fails validation +label:mn_only,ci_test,db +cmd:rmdef -t node testnode_partial +cmd:mkdef -t node -o testnode_partial groups=test bmc=10.0.0.1 +check:rc!=0 +check:output=~Cannot set +cmd:lsdef testnode_partial +check:rc!=0 +cmd:rmdef -t node testnode_partial +end + +start:mkdef_f_validation_error_preserves_old +description:mkdef -f with validation error should preserve original object +label:mn_only,ci_test,db +cmd:rmdef -t node testnode_keepme +cmd:mkdef -t node -o testnode_keepme groups=oldgroup mgt=openbmc bmc=1.2.3.4 +check:rc==0 +cmd:mkdef -f -t node -o testnode_keepme groups=newgroup bmc=5.6.7.8 +check:rc!=0 +cmd:lsdef -i groups testnode_keepme +check:rc==0 +check:output=~groups=oldgroup +cmd:lsdef -i mgt testnode_keepme +check:rc==0 +check:output=~mgt=openbmc +cmd:lsdef -i bmc testnode_keepme +check:rc==0 +check:output=~bmc=1.2.3.4 +cmd:rmdef -t node testnode_keepme +end + +start:mkdef_f_missing_groups_preserves_old +description:mkdef -f without groups should preserve original node +label:mn_only,ci_test,db +cmd:rmdef -t node testnode_keepgrp +cmd:mkdef -t node -o testnode_keepgrp groups=oldgroup mgt=openbmc bmc=1.2.3.4 +check:rc==0 +cmd:mkdef -f -t node -o testnode_keepgrp mgt=openbmc bmc=5.6.7.8 +check:rc!=0 +cmd:lsdef -i groups testnode_keepgrp +check:rc==0 +check:output=~groups=oldgroup +cmd:rmdef -t node testnode_keepgrp +end + +start:mkdef_only_if_satisfied_explicit +description:mkdef with only_if condition explicitly satisfied should succeed +label:mn_only,ci_test,db +cmd:rmdef -t node testnode_onlyif +cmd:mkdef -t node -o testnode_onlyif groups=test mgt=openbmc bmc=10.0.0.1 +check:rc==0 +cmd:lsdef -i bmc testnode_onlyif +check:rc==0 +check:output=~bmc=10.0.0.1 +cmd:rmdef -t node testnode_onlyif +end + +start:mkdef_inherited_validation_via_group +description:mkdef with only_if satisfied by group inheritance should write the attr +label:mn_only,ci_test,db +cmd:rmdef -t node testnode_inherit +cmd:chtab node=openbmcgrp nodehm.mgt=openbmc +check:rc==0 +cmd:mkdef -t node -o testnode_inherit groups=openbmcgrp bmc=10.0.0.1 +check:rc==0 +cmd:lsdef -i bmc testnode_inherit +check:rc==0 +check:output=~bmc=10.0.0.1 +cmd:rmdef -t node testnode_inherit +cmd:chtab -d node=openbmcgrp nodehm +end + +start:mkdef_f_only_if_failure_preserves_old_bmc +description:mkdef -f with only_if failure should preserve original bmc value +label:mn_only,ci_test,db +cmd:rmdef -t node testnode_keepbmc +cmd:mkdef -t node -o testnode_keepbmc groups=oldgroup mgt=openbmc bmc=1.2.3.4 +check:rc==0 +cmd:mkdef -f -t node -o testnode_keepbmc groups=newgroup bmc=5.6.7.8 +check:rc!=0 +cmd:lsdef -i bmc testnode_keepbmc +check:rc==0 +check:output=~bmc=1.2.3.4 +cmd:rmdef -t node testnode_keepbmc +end + +start:mkdef_multi_object_valid_invalid +description:single mkdef command with one valid and one invalid object (per-object atomic) +label:mn_only,ci_test,db +cmd:rmdef -t node testnode_good +cmd:rmdef -t node testnode_bad +cmd:printf "testnode_good:\n objtype=node\n groups=test\n mgt=openbmc\n bmc=10.0.0.1\n\ntestnode_bad:\n objtype=node\n groups=test\n bmc=10.0.0.2\n" | mkdef -z +check:rc!=0 +cmd:lsdef -i bmc testnode_good +check:rc==0 +check:output=~bmc=10.0.0.1 +cmd:lsdef testnode_bad +check:rc!=0 +cmd:rmdef -t node testnode_good +cmd:rmdef -t node testnode_bad +end + +start:mkdef_f_dynamic_group_missing_wherevals_preserves_old +description:mkdef -f with grouptype=dynamic but no wherevals should preserve old group +label:mn_only,ci_test,db +cmd:rmdef -t node testmember1 +cmd:rmdef -t group testkeepgrp +cmd:mkdef -t node -o testmember1 groups=all +check:rc==0 +cmd:mkdef -t group -o testkeepgrp members=testmember1 +check:rc==0 +cmd:mkdef -f -t group -o testkeepgrp grouptype=dynamic +check:rc!=0 +cmd:lsdef -t group testkeepgrp +check:rc==0 +cmd:lsdef -s testkeepgrp +check:output=~testmember1 +cmd:rmdef -t group testkeepgrp +cmd:rmdef -t node testmember1 +end + +start:mkdef_f_dynamic_group_members_w_conflict_preserves_old +description:mkdef -f -d -w with members should preserve old group +label:mn_only,ci_test,db +cmd:rmdef -t node testmember2 +cmd:rmdef -t group testkeepgrp2 +cmd:mkdef -t node -o testmember2 groups=all +check:rc==0 +cmd:mkdef -t group -o testkeepgrp2 members=testmember2 +check:rc==0 +cmd:mkdef -f -t group -o testkeepgrp2 -d -w mgt==openbmc members=testmember2 +check:rc!=0 +cmd:lsdef -t group testkeepgrp2 +check:rc==0 +cmd:lsdef -s testkeepgrp2 +check:output=~testmember2 +cmd:rmdef -t group testkeepgrp2 +cmd:rmdef -t node testmember2 +end + +start:mkdef_failed_group_no_member_mutation +description:failed group creation should not mutate member node groups +label:mn_only,ci_test,db +cmd:rmdef -t node testmember3 +cmd:rmdef -t group testbadgrp +cmd:mkdef -t node -o testmember3 groups=all +check:rc==0 +cmd:mkdef -t group -o testbadgrp members=testmember3 bmc=10.0.0.1 +check:rc!=0 +cmd:lsdef -t group testbadgrp +check:rc!=0 +cmd:lsdef -i groups testmember3 +check:output!~testbadgrp +cmd:rmdef -t node testmember3 +cmd:rmdef -t group testbadgrp +end diff --git a/xCAT-test/unit/dbobjutils_only_if.t b/xCAT-test/unit/dbobjutils_only_if.t new file mode 100644 index 000000000..b3d0740c8 --- /dev/null +++ b/xCAT-test/unit/dbobjutils_only_if.t @@ -0,0 +1,52 @@ +#!/usr/bin/env perl +use strict; +use warnings; + +BEGIN { + $ENV{XCATCFG} ||= 'SQLite:/tmp'; +} + +use FindBin; +use lib "$FindBin::Bin/../../perl-xCAT"; + +use Test::More; + +use xCAT::DBobjUtils; + +my %missing_mgt = ( + objtype => 'node', + groups => 'test', + bmc => '10.0.0.1', +); +my @failures = xCAT::DBobjUtils->validate_only_if_attrs('node01', 'node', \%missing_mgt, {}); +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 %explicit_openbmc = ( + objtype => 'node', + groups => 'test', + mgt => 'openbmc', + bmc => '10.0.0.1', +); +@failures = xCAT::DBobjUtils->validate_only_if_attrs('node01', 'node', \%explicit_openbmc, {}); +is(scalar @failures, 0, 'explicit mgt=openbmc satisfies bmc only_if validation'); + +my %existing_openbmc = ( + objtype => 'node', + groups => 'test', + bmc => '10.0.0.1', +); +my %dbattrs = (mgt => 'openbmc'); +@failures = xCAT::DBobjUtils->validate_only_if_attrs('node01', 'node', \%existing_openbmc, \%dbattrs); +is(scalar @failures, 0, 'existing mgt=openbmc satisfies bmc only_if validation'); + +my %group_openbmc = ( + objtype => 'node', + groups => 'openbmcgrp', + bmc => '10.0.0.1', +); +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'); + +done_testing();