Remove some odd behaviour of cached SNMP config hints (L. Ferguson)

Netdisco/Changes

@@ -4,6 +4,10 @@
 
   * Add validate_remote_user setting to check proxied users are known
 
+  [BUG FIXES]
+
+  * Remove some odd behaviour of cached SNMP config hints (L. Ferguson)
+
 2.034000 - 2016-10-03
 
   [NEW FEATURES]

Netdisco/lib/App/Netdisco/DB.pm

@@ -11,7 +11,7 @@ __PACKAGE__->load_namespaces(
 );
 
 our # try to hide from kwalitee
-  $VERSION = 40; # schema version used for upgrades, keep as integer
+  $VERSION = 41; # schema version used for upgrades, keep as integer
 
 use Path::Class;
 use File::Basename;

Netdisco/lib/App/Netdisco/DB/Result/Community.pm

@@ -11,7 +11,9 @@ __PACKAGE__->add_columns(
   { data_type => "inet", is_nullable => 0 },
   "snmp_comm_rw",
   { data_type => "text", is_nullable => 1 },
-  "snmp_auth_tag",
+  "snmp_auth_tag_read",
+  { data_type => "text", is_nullable => 1 },
+  "snmp_auth_tag_write",
   { data_type => "text", is_nullable => 1 },
 );
 __PACKAGE__->set_primary_key("ip");

Netdisco/lib/App/Netdisco/DB/ResultSet/Device.pm

@@ -591,7 +591,8 @@ sub delete {
 
   $schema->resultset('Community')->search({
     ip => { '-in' => $devices->as_query },
-    snmp_auth_tag => undef,
+    snmp_auth_tag_read  => undef,
+    snmp_auth_tag_write => undef,
   })->delete;
 
   $schema->resultset('Community')->search(

Netdisco/lib/App/Netdisco/DB/schema_versions/App-Netdisco-DB-40-41-PostgreSQL.sql

@@ -0,0 +1,7 @@
+BEGIN;
+
+ALTER TABLE community RENAME COLUMN snmp_auth_tag TO snmp_auth_tag_read;
+
+ALTER TABLE community ADD COLUMN snmp_auth_tag_write text;
+
+COMMIT;

Netdisco/lib/App/Netdisco/Util/SNMP.pm

@@ -191,7 +191,7 @@ sub _try_read {
 
   # regardless of device in storage, save the hint
   $device->update_or_create_related('community',
-    {snmp_auth_tag => $comm->{tag}}) if $comm->{tag};
+    {snmp_auth_tag_read => $comm->{tag}}) if $comm->{tag};
 
   return $info;
 }
@@ -209,7 +209,7 @@ sub _try_write {
 
   # one of these two cols must be set
   $device->update_or_create_related('community', {
-    ($comm->{tag} ? (snmp_auth_tag => $comm->{tag}) : ()),
+    ($comm->{tag} ? (snmp_auth_tag_write => $comm->{tag}) : ()),
     ($comm->{community} ? (snmp_comm_rw => $comm->{community}) : ()),
   });
 
@@ -258,9 +258,11 @@ sub _get_mibdirs_content {
 sub _build_communities {
   my ($device, $mode) = @_;
   $mode ||= 'read';
+  my $seen_tags = {}; # for cleaning community table
 
   my $config = (setting('snmp_auth') || []);
-  my $stored_tag = eval { $device->community->snmp_auth_tag };
+  my $tag_name = 'snmp_auth_tag_'. $mode;
+  my $stored_tag = eval { $device->community->$tag_name };
   my $snmp_comm_rw = eval { $device->community->snmp_comm_rw };
   my @communities = ();
 
@@ -289,6 +291,7 @@ sub _build_communities {
 
       # defaults
       $stanza->{tag} ||= $tag;
+      ++$seen_tags->{ $stanza->{tag} };
       $stanza->{read} = 1 if !exists $stanza->{read};
       $stanza->{only} ||= ['any'];
       $stanza->{only} = [$stanza->{only}] if ref '' eq ref $stanza->{only};
@@ -298,7 +301,8 @@ sub _build_communities {
            and !exists $stanza->{community};
 
       if ($stanza->{$mode} and check_acl($device->ip, $stanza->{only})) {
-          if ($stored_tag and $stored_tag eq $stanza->{tag}) {
+          if ($device->in_storage and
+            $stored_tag and $stored_tag eq $stanza->{tag}) {
               # last known-good by tag
               unshift @communities, $stanza
           }
@@ -308,6 +312,11 @@ sub _build_communities {
       }
   }
 
+  # clean the community table of obsolete tags
+  if ($stored_tag and !exists $seen_tags->{ $stored_tag }) {
+      eval { $device->community->update({$tag_name => undef}) };
+  }
+
   # legacy config (note: read strings tried before write)
   if ($mode eq 'read') {
       push @communities, map {{
@@ -340,7 +349,7 @@ sub _get_external_community {
       $cmd =~ s/\%HOST\%/$host/egi;
       $cmd =~ s/\%IP\%/$ip/egi;
 
-      my $result = `$cmd`;
+      my $result = `$cmd`; # BACKTICKS
       return () unless defined $result and length $result;
 
       my @lines = split (m/\n/, $result);
@@ -383,7 +392,7 @@ sub snmp_comm_reindex {
       my @comms = _build_communities($device, 'read');
       foreach my $c (@comms) {
           next unless $c->{tag}
-            and $c->{tag} eq (eval { $device->community->snmp_auth_tag } || '');
+            and $c->{tag} eq (eval { $device->community->snmp_auth_tag_read } || '');
           $prefix = $c->{context_prefix} and last;
       }
       $prefix ||= 'vlan-';