Merge pull request #1318 from openark/ignore-hostame-ip-regexp

Fix filter match logic, be strict about IP addresses

go/inst/instance_dao.go

@@ -669,7 +669,7 @@ func ReadTopologyInstanceBufferable(instanceKey *InstanceKey, bufferWrites bool,
 
 				replicaKey, err := NewResolveInstanceKey(host, port)
 				if err == nil && replicaKey.IsValid() {
-					if !RegexpMatchPatterns(replicaKey.StringCode(), config.Config.DiscoveryIgnoreReplicaHostnameFilters) {
+					if !FiltersMatchInstanceKey(replicaKey, config.Config.DiscoveryIgnoreReplicaHostnameFilters) {
 						instance.AddReplicaKey(replicaKey)
 					}
 					foundByShowSlaveHosts = true
@@ -699,7 +699,7 @@ func ReadTopologyInstanceBufferable(instanceKey *InstanceKey, bufferWrites bool,
 						logReadTopologyInstanceError(instanceKey, "ResolveHostname: processlist", resolveErr)
 					}
 					replicaKey := InstanceKey{Hostname: cname, Port: instance.Key.Port}
-					if !RegexpMatchPatterns(replicaKey.StringCode(), config.Config.DiscoveryIgnoreReplicaHostnameFilters) {
+					if !FiltersMatchInstanceKey(&replicaKey, config.Config.DiscoveryIgnoreReplicaHostnameFilters) {
 						instance.AddReplicaKey(&replicaKey)
 					}
 					return err
@@ -1476,7 +1476,7 @@ func ReadProblemInstances(clusterName string) ([](*Instance), error) {
 		if instance.IsDowntimed {
 			skip = true
 		}
-		if RegexpMatchPatterns(instance.Key.StringCode(), config.Config.ProblemIgnoreHostnameFilters) {
+		if FiltersMatchInstanceKey(&instance.Key, config.Config.ProblemIgnoreHostnameFilters) {
 			skip = true
 		}
 		if !skip {
@@ -1637,7 +1637,7 @@ func ReadClusterNeutralPromotionRuleInstances(clusterName string) (neutralInstan
 func filterOSCInstances(instances [](*Instance)) [](*Instance) {
 	result := [](*Instance){}
 	for _, instance := range instances {
-		if RegexpMatchPatterns(instance.Key.StringCode(), config.Config.OSCIgnoreHostnameFilters) {
+		if FiltersMatchInstanceKey(&instance.Key, config.Config.OSCIgnoreHostnameFilters) {
 			continue
 		}
 		if instance.IsBinlogServer() {
@@ -1989,11 +1989,11 @@ func InjectUnseenMasters() error {
 	for _, masterKey := range unseenMasterKeys {
 		masterKey := masterKey
 
-		if RegexpMatchPatterns(masterKey.StringCode(), config.Config.DiscoveryIgnoreMasterHostnameFilters) {
+		if FiltersMatchInstanceKey(&masterKey, config.Config.DiscoveryIgnoreMasterHostnameFilters) {
 			log.Debugf("InjectUnseenMasters: skipping discovery of %+v because it matches DiscoveryIgnoreMasterHostnameFilters", masterKey)
 			continue
 		}
-		if RegexpMatchPatterns(masterKey.StringCode(), config.Config.DiscoveryIgnoreHostnameFilters) {
+		if FiltersMatchInstanceKey(&masterKey, config.Config.DiscoveryIgnoreHostnameFilters) {
 			log.Debugf("InjectUnseenMasters: skipping discovery of %+v because it matches DiscoveryIgnoreHostnameFilters", masterKey)
 			continue
 		}

go/inst/instance_topology.go

@@ -19,7 +19,6 @@ package inst
 import (
 	"fmt"
 	goos "os"
-	"regexp"
 	"sort"
 	"strings"
 	"sync"
@@ -2159,10 +2158,8 @@ func IsBannedFromBeingCandidateReplica(replica *Instance) bool {
 		log.Debugf("instance %+v is banned because of promotion rule", replica.Key)
 		return true
 	}
-	for _, filter := range config.Config.PromotionIgnoreHostnameFilters {
-		if matched, _ := regexp.MatchString(filter, replica.Key.Hostname); matched {
-			return true
-		}
+	if FiltersMatchInstanceKey(&replica.Key, config.Config.PromotionIgnoreHostnameFilters) {
+		return true
 	}
 	return false
 }

go/inst/instance_topology_dao.go

@@ -532,7 +532,6 @@ func WaitForExecBinlogCoordinatesToReach(instanceKey *InstanceKey, coordinates *
 			return instance, false, nil
 		}
 	}
-	return instance, exactMatch, err
 }
 
 // StartReplicationUntilMasterCoordinates issuesa START SLAVE UNTIL... statement on given instance

go/inst/instance_utils.go

@@ -17,6 +17,7 @@
 package inst
 
 import (
+	"net"
 	"regexp"
 	"strconv"
 	"strings"
@@ -249,11 +250,24 @@ func IsSmallerBinlogFormat(binlogFormat string, otherBinlogFormat string) bool {
 	return false
 }
 
-// RegexpMatchPatterns returns true if s matches any of the provided regexpPatterns
-func RegexpMatchPatterns(s string, regexpPatterns []string) bool {
-	for _, filter := range regexpPatterns {
-		if matched, err := regexp.MatchString(filter, s); err == nil && matched {
-			return true
+// FiltersMatchInstanceKey returns true if given instance key matches any one of given filters.
+// A filter could be:
+// - An IP address, in which case we compare exact value
+// - Any other string, in which case we compare via regular expression
+func FiltersMatchInstanceKey(instanceKey *InstanceKey, filters []string) bool {
+	for _, filter := range filters {
+		switch {
+		case net.ParseIP(filter) != nil:
+			// If the filter is an IP address, expect complete match.
+			// This is to avoid matching 10.0.0.3 with 10.0.0.38 if we
+			// were to compare via regexp
+			if filter == instanceKey.Hostname {
+				return true
+			}
+		default:
+			if matched, _ := regexp.MatchString(filter, instanceKey.StringCode()); matched {
+				return true
+			}
 		}
 	}
 	return false

go/inst/instance_utils_test.go

@@ -20,11 +20,17 @@ func TestRegexpMatchPatterns(t *testing.T) {
 		{"hostname", []string{"ho.tname"}, true},
 		{"hostname", []string{"ho.tname2"}, false},
 		{"hostname", []string{"ho.*me"}, true},
+		{"10.0.0.3", []string{"10.0.0.3"}, true},
+		{"10.0.0.3", []string{"10.0.0.3:3306"}, true},
+		{"10.0.0.3", []string{"10.0.0.38"}, false},
 	}
 
 	for _, p := range patterns {
-		if match := RegexpMatchPatterns(p.s, p.patterns); match != p.expected {
-			t.Errorf("RegexpMatchPatterns failed with: %q, %+v, got: %+v, expected: %+v", p.s, p.patterns, match, p.expected)
-		}
+		t.Run(p.s, func(t *testing.T) {
+			k := &InstanceKey{Hostname: p.s, Port: 3306}
+			if match := FiltersMatchInstanceKey(k, p.patterns); match != p.expected {
+				t.Errorf("FiltersMatchInstanceKey failed with: %q, %+v, got: %+v, expected: %+v", p.s, p.patterns, match, p.expected)
+			}
+		})
 	}
 }

go/logic/orchestrator.go

@@ -190,7 +190,7 @@ func DiscoverInstance(instanceKey inst.InstanceKey) {
 		log.Debugf("discoverInstance: skipping discovery of %+v because it is set to be forgotten", instanceKey)
 		return
 	}
-	if inst.RegexpMatchPatterns(instanceKey.StringCode(), config.Config.DiscoveryIgnoreHostnameFilters) {
+	if inst.FiltersMatchInstanceKey(&instanceKey, config.Config.DiscoveryIgnoreHostnameFilters) {
 		log.Debugf("discoverInstance: skipping discovery of %+v because it matches DiscoveryIgnoreHostnameFilters", instanceKey)
 		return
 	}
@@ -284,7 +284,7 @@ func DiscoverInstance(instanceKey inst.InstanceKey) {
 		replicaKey := replicaKey // not needed? no concurrency here?
 
 		// Avoid noticing some hosts we would otherwise discover
-		if inst.RegexpMatchPatterns(replicaKey.StringCode(), config.Config.DiscoveryIgnoreReplicaHostnameFilters) {
+		if inst.FiltersMatchInstanceKey(&replicaKey, config.Config.DiscoveryIgnoreReplicaHostnameFilters) {
 			continue
 		}
 
@@ -294,7 +294,7 @@ func DiscoverInstance(instanceKey inst.InstanceKey) {
 	}
 	// Investigate master:
 	if instance.MasterKey.IsValid() {
-		if !inst.RegexpMatchPatterns(instance.MasterKey.StringCode(), config.Config.DiscoveryIgnoreMasterHostnameFilters) {
+		if !inst.FiltersMatchInstanceKey(&instance.MasterKey, config.Config.DiscoveryIgnoreMasterHostnameFilters) {
 			discoveryQueue.Push(instance.MasterKey)
 		}
 	}