Changeset 710

Timestamp:

03/17/16 15:06:32 (9 years ago)

Author:

Kris Deugau

Message:

/trunk

Refine/fix behaviour of updateRevSet() and chained subs

• properly represent add, update, and delete actions all in one set
• do nothing on null "updates" to keep log noise down
• properly target requests to a single location to avoid stomping unrelated records

Location:

trunk

Files:

• DNSDB.pm (modified) (3 diffs)
• dns-rpc.cgi (modified) (8 diffs)

trunk/DNSDB.pm

@@ -5183 +5183 @@
 ## DNSDB::getZonesByCIDR()
 # Get a list of zone names and IDs that records for a passed CIDR block are within.
+# Optionally restrict to a specific location/view
 sub getZonesByCIDR {
   my $self = shift;
@@ -5188 +5189 @@
   my %args = @_;
 
-  my $result = $dbh->selectall_arrayref("SELECT rdns_id,revnet FROM revzones WHERE revnet >>= ? OR revnet <<= ?",
-        { Slice => {} }, ($args{cidr}, $args{cidr}) );
+  my $sql = "SELECT rdns_id,revnet,default_location FROM revzones WHERE revnet >>= ? OR revnet <<= ?".
+        ($args{location} ? " AND default_location = ?" : '');
+  my @svals = ($args{cidr}, $args{cidr});
+  push @svals, $args{location} if $args{location};
+
+  my $result = $dbh->selectall_arrayref($sql, { Slice => {} }, @svals );
   return $result;
 } # end getZonesByCIDR()
@@ -5351 +5356 @@
 ##fixme:  serial
       $dbh->do("INSERT INTO domains (domain,group_id,status) VALUES (?,?,?)", undef,
-        ($zone, $group, $args{status}) );
+        ($zone, $group, $args{status}) ) or die $dbh->errstr;
       # get domain id so we can do the records
       ($zone_id) = $dbh->selectrow_array("SELECT currval('domains_domain_id_seq')");

trunk/dns-rpc.cgi

@@ -137 +137 @@
 ##
 
+##
+## Internal utility subs
+##
+
 # Check RPC ACL
 sub _aclcheck {
@@ -174 +178 @@
 }
 
-# set ttl to zone defailt minttl if none is specified
+# set ttl to zone default minttl if none is specified
 sub _ttlcheck {
   my $argref = shift;
@@ -182 +186 @@
   }
 }
+
+# Check if the hashrefs passed in refer to identical record data, so we can skip
+# the actual update if nothing has actually changed.  This is mainly useful for
+# reducing log noise due to chained calls orginating with updateRevSet() since
+# "many" records could be sent for update but only one or two have actually changed.
+sub _checkRecMod {
+  my $oldrec = shift;
+  my $newrec = shift;
+
+  # Because we don't know which fields we've even been passed
+  no warnings qw(uninitialized);
+
+  my $modflag = 0;
+  # order by most common change.  host should be first, due to rDNS RPC calls
+  for my $field qw(host type val) {
+    return 1 if (
+        defined($newrec->{$field}) &&
+        $oldrec->{$field} ne $newrec->{$field} );
+  }
+
+  return 0;
+} # _checRecMod
+
+
+##
+## Shims for DNSDB core subs
+##
 
 #sub connectDB {
@@ -534 +565 @@
 } # rpc_updateRec
 
+
 # Takes a passed CIDR block and DNS pattern;  adds a new record or updates the record(s) affected
 sub addOrUpdateRevRec {
@@ -540 +572 @@
   _commoncheck(\%args, 'y');
   my $cidr = new NetAddr::IP $args{cidr};
-
-##fixme:  Minor edge case; if we receive calls one after the other to update
-# to the same thing, we bulk out the log with useless notices.  Leaving this
-# for future development since this should be rare in practice.
 
   my $zonelist = $dnsdb->getZonesByCIDR(%args);
@@ -570 +598 @@
                 || $rec->{type} == 65282 || $rec->{type} == 65283 || $rec->{type} == 65284;
           next unless $rec->{val} eq $filt;     # make sure we really update the record we want to update.
+          my %newrec = (host => $args{name}, val => $args{cidr}, type => $args{type});
           rpc_updateRec(defrec =>'n', revrec => 'y', id => $rec->{record_id},
-            parent_id => $zonelist->[0]->{rdns_id}, address => "$cidr", %args);
+                parent_id => $zonelist->[0]->{rdns_id}, address => "$cidr", %args)
+                if _checkRecMod($rec, \%newrec);        # and only do the update if there really is something to change
           $flag = 1;
           last; # only do one record.
@@ -623 +653 @@
     next unless $key =~ m{^host_((?:[\d.]+|[\da-f:]+)(?:/\d+)?)$};
     my $ip = $1;
-    push @ret, addOrUpdateRevRec(cidr => $ip, name => $args{$key}, %args);
-  }
+    push @ret, addOrUpdateRevRec(%args, cidr => $ip, name => $args{$key});
+  }
+
+  # now we check the parts of the block that didn't get passed to see if they should be deleted
+  my $block = new NetAddr::IP $args{cidr};
+  foreach my $ip (@{$block->splitref(32)}) {
+    my $bare = $ip->addr;
+    next if $args{"host_$bare"};
+    delByCIDR(delforward => 1, delsubs => 0, cidr => $bare, location => $args{location},
+        rpcuser => $args{rpcuser}, rpcsystem => $args{rpcsystem});
+  }
+
 ##fixme:  what about errors?  what about warnings?
   return \@ret;
@@ -901 +941 @@
           my $reccidr = new NetAddr::IP $rec->{val};
           next unless $cidr == $reccidr;
+          # restrict to the right location
+          next unless ( defined($rec->{locname}) && defined($args{location}) &&
+                      $rec->{locname} eq $args{location} );
           next unless $rec->{type} == 12 || $rec->{type} == 65280 || $rec->{type} == 65281 ||
                       $rec->{type} == 65282 || $rec->{type} == 65283 ||$rec->{type} == 65284;