Deduplicate networks (#13)

* Deduplicate networks

* Addressed comments

cmd/network-crawler/network-crawler.go

@@ -62,16 +62,24 @@ func run() error {
 		flagBucketName       = flag.String("bucket-name", "", "GCS bucket name to upload external networks to")
 		flagDryRun           = flag.Bool("dry-run", false, "Skip uploading external networks to GCS")
 		flagSkippedProviders skippedProviderFlag
+		flagVerbose          bool
+		flagVerboseUsage     = "Prints extra debug message"
 	)
 	skippedProvidersUsage :=
 		fmt.Sprintf("Comma separated list of providers. Currently acceptable providers are: %v", common.AllProviders())
 	flag.Var(&flagSkippedProviders, "skipped-providers", skippedProvidersUsage)
+	flag.BoolVar(&flagVerbose, "verbose", flagVerbose, flagVerboseUsage)
+	flag.BoolVar(&flagVerbose, "v", flagVerbose, flagVerboseUsage+" (shorthand)")
 	flag.Parse()
 
 	if flagBucketName == nil || *flagBucketName == "" {
 		return common.NoBucketNameSpecified()
 	}
 
+	if flagVerbose {
+		common.SetVerbose()
+	}
+
 	if *flagDryRun {
 		log.Print("Dry run specified. Instead of uploading the content to bucket will just print to stdout.")
 	}

pkg/common/envvars.go

@@ -0,0 +1,15 @@
+package common
+
+var (
+	verbose = false
+)
+
+// Verbose returns if verbose options is set
+func Verbose() bool {
+	return verbose
+}
+
+// SetVerbose enables verbose mode
+func SetVerbose() {
+	verbose = true
+}

pkg/common/errors.go

@@ -74,3 +74,13 @@ func ErroneousPrefixOrderingError(bucketName string, prefixes []string) error {
 func NoBucketNameSpecified() error {
 	return errors.New("bucket name not specified")
 }
+
+// RegionNetworksNotFound is returned when a region networks spec is not found
+func RegionNetworksNotFound(region string) error {
+	return fmt.Errorf("region networks for region %s not found", region)
+}
+
+// ServiceNetworksNotFound is returned when a service networks spec is not found
+func ServiceNetworksNotFound(service string) error {
+	return fmt.Errorf("service networks for service %s not found", service)
+}

pkg/common/utils/test_utils.go → pkg/common/testutils/test_utils.go

@@ -1,4 +1,4 @@
-package utils
+package testutils
 
 import (
 	"testing"

pkg/common/types.go

@@ -1,9 +1,13 @@
 package common
 
 import (
+	"fmt"
+	"log"
 	"net"
+	"strings"
 
 	"github.com/pkg/errors"
+	"github.com/stackrox/external-network-pusher/pkg/common/utils"
 )
 
 // NetworkCrawler defines an interface for the implementation
@@ -37,25 +41,122 @@ type RegionNetworkDetail struct {
 type ProviderNetworkRanges struct {
 	ProviderName   string                 `json:"providerName"`
 	RegionNetworks []*RegionNetworkDetail `json:"regionNetworks"`
+
+	// prefixToRegionServiceNames is used to remove "redundant" network
+	// Redundancy is determined by user's predicate while adding a new IP prefix.
+	// More about the predicate function below.
+	prefixToRegionServiceNames map[string][]*RegionServicePair
 }
 
 // ExternalNetworkSources contains all the external networks for all providers
 type ExternalNetworkSources struct {
 	ProviderNetworks []*ProviderNetworkRanges `json:"providerNetworks"`
 }
 
+// RegionServicePair is a tuple of region and service names
+type RegionServicePair struct {
+	Region  string
+	Service string
+}
+
+// Equals checks if two RegionServicePairs are equal
+func (r *RegionServicePair) Equals(p *RegionServicePair) bool {
+	return r.Region == p.Region && r.Service == p.Service
+}
+
+// String returns the string representation of a RegionServicePair
+func (r *RegionServicePair) String() string {
+	return fmt.Sprintf("{%s, %s}", r.Region, r.Service)
+}
+
+// IsRedundantRegionServicePairFn is a predicate function to determine
+// if a new region service pair should be added to output or not.
+// For example, if an IP address belongs to multiple region/service pairs,
+// user needs to provide a predicate function which looks at the pairs that are
+// already recorded in ProviderNetworkRanges and the new pair that is about
+// to be added, then decide if the new pair should be added as well or not.
+// The existing pairs are given one by one to the user.
+//
+// The return value indicates which pair to remove. There could be three
+// different return outcomes. Remove the new pair, remove the existing pair, or keep
+// both. The returned pair is first checked with the new pair before checking with
+// the existing pair. In case of keeping both pairs, a nil value should be returned
+type IsRedundantRegionServicePairFn func(
+	newPair *RegionServicePair,
+	existingPair *RegionServicePair,
+) (*RegionServicePair, error)
+
+// GetDefaultRegionServicePairRedundancyCheck returns the default check
+// Default check checks if region and service names are the same
+func GetDefaultRegionServicePairRedundancyCheck() IsRedundantRegionServicePairFn {
+	return func(
+		newPair *RegionServicePair,
+		existingPair *RegionServicePair,
+	) (*RegionServicePair, error) {
+		if newPair.Equals(existingPair) {
+			return newPair, nil
+		}
+
+		return nil, nil
+	}
+}
+
+// NewProviderNetworkRanges returns a new instance of ProviderNetworkRanges
+func NewProviderNetworkRanges(providerName string) *ProviderNetworkRanges {
+	return &ProviderNetworkRanges{
+		ProviderName:               providerName,
+		RegionNetworks:             make([]*RegionNetworkDetail, 0),
+		prefixToRegionServiceNames: make(map[string][]*RegionServicePair),
+	}
+}
+
 // AddIPPrefix adds the specified IP prefix to the region and service name pair
 // returns error if the IP given is not a valid IP prefix
-func (p *ProviderNetworkRanges) AddIPPrefix(region, service, ipPrefix string) error {
+func (p *ProviderNetworkRanges) AddIPPrefix(region, service, ipPrefix string, fn IsRedundantRegionServicePairFn) error {
 	ip, prefix, err := net.ParseCIDR(ipPrefix)
 	if err != nil || ip == nil || prefix == nil {
 		return errors.Wrapf(err, "failed to parse address: %s", ip)
 	}
-	if ip.To4() != nil {
-		p.addIPPrefix(region, service, ipPrefix, true)
-	} else {
-		p.addIPPrefix(region, service, ipPrefix, false)
+	isIPv4 := ip.To4() != nil
+
+	// Check redundancy
+	existingPairs, ok := p.prefixToRegionServiceNames[ipPrefix]
+	if ok {
+		newPair := RegionServicePair{Region: region, Service: service}
+		for _, pair := range existingPairs {
+			redundantPair, err := fn(&newPair, pair)
+			if err != nil {
+				return err
+			}
+			if redundantPair != nil {
+				if redundantPair == &newPair {
+					// The new pair is redundant. Not adding it
+					return nil
+				}
+				// Check did not match with the new pair. Removing an old pair and continue pruning
+				err := p.removeIPPrefix(redundantPair.Region, redundantPair.Service, ipPrefix, isIPv4)
+				if err != nil {
+					return err
+				}
+			}
+		}
 	}
+
+	if existingPairs := p.prefixToRegionServiceNames[ipPrefix]; Verbose() && len(existingPairs) > 0 {
+		strs := make([]string, 0, len(existingPairs))
+		for _, pair := range existingPairs {
+			strs = append(strs, pair.String())
+		}
+		log.Printf(
+			"Multple usages found for CIDR: %s. About to add region: %s and service: %s."+
+				" Existing region and service pairs are: %s",
+			ipPrefix,
+			region,
+			service,
+			strings.Join(strs, ", "))
+	}
+
+	p.addIPPrefix(region, service, ipPrefix, isIPv4)
 	return nil
 }
 
@@ -91,4 +192,96 @@ func (p *ProviderNetworkRanges) addIPPrefix(region, service, ip string, isIPv4 b
 	} else {
 		serviceIPRanges.IPv6Prefixes = append(serviceIPRanges.IPv6Prefixes, ip)
 	}
+
+	// Update cache
+	p.prefixToRegionServiceNames[ip] =
+		append(p.prefixToRegionServiceNames[ip], &RegionServicePair{Region: region, Service: service})
+}
+
+func (p *ProviderNetworkRanges) removeIPPrefix(region, service, ip string, isIPv4 bool) error {
+	var regionNetwork *RegionNetworkDetail
+	regionIndex := -1
+	for i, network := range p.RegionNetworks {
+		if network.RegionName == region {
+			regionIndex = i
+			regionNetwork = network
+			break
+		}
+	}
+	if regionNetwork == nil {
+		return RegionNetworksNotFound(region)
+	}
+
+	var serviceIPRanges *ServiceIPRanges
+	serviceIndex := -1
+	for i, ips := range regionNetwork.ServiceNetworks {
+		if ips.ServiceName == service {
+			serviceIPRanges = ips
+			serviceIndex = i
+			break
+		}
+	}
+	if serviceIPRanges == nil {
+		return ServiceNetworksNotFound(service)
+	}
+
+	serviceIPRanges.removeIPPrefix(ip, isIPv4)
+	if serviceIPRanges.isEmpty() {
+		// Remove this service networks spec
+		regionNetwork.ServiceNetworks = SvcIPRangesSliceRemove(regionNetwork.ServiceNetworks, serviceIndex)
+	}
+	if regionNetwork.isEmpty() {
+		// Remove this region
+		p.RegionNetworks = RgnNetDetSliceRemove(p.RegionNetworks, regionIndex)
+	}
+
+	// Delete from cache as well
+	deletingIndex := -1
+	existingPairs := p.prefixToRegionServiceNames[ip]
+	for i, pair := range existingPairs {
+		if pair.Region == region && pair.Service == service {
+			deletingIndex = i
+			break
+		}
+	}
+	if deletingIndex == -1 {
+		// Not found in cache. No-op
+		return nil
+	}
+
+	p.prefixToRegionServiceNames[ip] = RgnSvcPairSliceRemove(p.prefixToRegionServiceNames[ip], deletingIndex)
+	return nil
+}
+
+func (s *ServiceIPRanges) removeIPPrefix(deletingIP string, isIPv4 bool) {
+	deletingIndex := -1
+	var deletingSlice *[]string
+	if isIPv4 {
+		deletingSlice = &s.IPv4Prefixes
+	} else {
+		deletingSlice = &s.IPv6Prefixes
+	}
+
+	for i, ip := range *deletingSlice {
+		if ip == deletingIP {
+			deletingIndex = i
+			break
+		}
+	}
+
+	if deletingIndex == -1 {
+		// Deleting an element that does not exist. No-op.
+		return
+	}
+
+	// Move the last element to the deleting position and truncate
+	*deletingSlice = utils.StrSliceRemove(*deletingSlice, deletingIndex)
+}
+
+func (s *ServiceIPRanges) isEmpty() bool {
+	return len(s.IPv4Prefixes)+len(s.IPv6Prefixes) == 0
+}
+
+func (r *RegionNetworkDetail) isEmpty() bool {
+	return len(r.ServiceNetworks) == 0
 }

pkg/common/types_util.go

@@ -0,0 +1,36 @@
+package common
+
+import (
+	"log"
+)
+
+// (sigh, only if using reflection could be deemed "idigomatic"...)
+// Also don't put these into pkg/common/utils/util.go since utils package should not
+// depend on application specifics (for example, this common package).
+
+// RgnSvcPairSliceRemove removes an element from a RegionServicePair slice at the specified index
+func RgnSvcPairSliceRemove(in []*RegionServicePair, i int) []*RegionServicePair {
+	if i < 0 || i >= len(in) {
+		log.Panicf("Index out of bound: %d", i)
+	}
+	in[i] = in[len(in)-1]
+	return in[:len(in)-1]
+}
+
+// SvcIPRangesSliceRemove removes an element from a ServiceIPRanges slice at the specified index
+func SvcIPRangesSliceRemove(in []*ServiceIPRanges, i int) []*ServiceIPRanges {
+	if i < 0 || i >= len(in) {
+		log.Panicf("Index out of bound: %d", i)
+	}
+	in[i] = in[len(in)-1]
+	return in[:len(in)-1]
+}
+
+// RgnNetDetSliceRemove removes an element from a RegionNetworkDetail slice at the specified index
+func RgnNetDetSliceRemove(in []*RegionNetworkDetail, i int) []*RegionNetworkDetail {
+	if i < 0 || i >= len(in) {
+		log.Panicf("Index out of bound: %d", i)
+	}
+	in[i] = in[len(in)-1]
+	return in[:len(in)-1]
+}

pkg/common/utils/util.go

@@ -1,14 +1,18 @@
 package utils
 
 import (
+	"log"
 	"strings"
 )
 
 // ToCompoundName accepts list of tags and returns a compound name.
 // It ignores any empty tag (i.e.: empty string)
 // If the final element only contains one string, then that string
 // is returned as the compound name
-func ToCompoundName(tags ...string) string {
+func ToCompoundName(delim string, tags ...string) string {
+	if delim == "" {
+		delim = "/"
+	}
 	filtered := make([]string, 0, len(tags))
 	for _, tag := range tags {
 		if tag != "" {
@@ -21,5 +25,22 @@ func ToCompoundName(tags ...string) string {
 	if len(filtered) == 1 {
 		return filtered[0]
 	}
-	return strings.Join(filtered, "-")
+	return strings.Join(filtered, delim)
+}
+
+// ToTags splits the compound name to a list of individual tags (names).
+func ToTags(delim, compoundName string) []string {
+	if delim == "" {
+		delim = "/"
+	}
+	return strings.Split(compoundName, delim)
+}
+
+// StrSliceRemove removes an element from a string slice at the specified index
+func StrSliceRemove(in []string, i int) []string {
+	if i < 0 || i >= len(in) {
+		log.Panicf("Index out of bound: %d", i)
+	}
+	in[i] = in[len(in)-1]
+	return in[:len(in)-1]
 }