Merge pull request #132 from mattfenwick/simplify-ports-fix

fix port simplification test case

Author: mattfenwick

pkg/matcher/portmatcher.go

@@ -2,9 +2,12 @@ package matcher
 
 import (
 	"encoding/json"
+	"sort"
+
+	collectionsjson "github.com/mattfenwick/collections/pkg/json"
+	"github.com/sirupsen/logrus"
 	v1 "k8s.io/api/core/v1"
 	"k8s.io/apimachinery/pkg/util/intstr"
-	"sort"
 )
 
 type PortMatcher interface {
@@ -100,12 +103,18 @@ func (s *SpecificPortMatcher) MarshalJSON() (b []byte, e error) {
 }
 
 func (s *SpecificPortMatcher) Combine(other *SpecificPortMatcher) *SpecificPortMatcher {
+	logrus.Debugf("SpecificPortMatcher Combined:\n%s\n", collectionsjson.MustMarshalToString([]interface{}{s, other}))
+
 	pps := append([]*PortProtocolMatcher{}, s.Ports...)
 	for _, otherPP := range other.Ports {
+		foundDuplicate := false
 		for _, pp := range pps {
 			if pp.Equals(otherPP) {
+				foundDuplicate = true
 				break
 			}
+		}
+		if !foundDuplicate {
 			pps = append(pps, otherPP)
 		}
 	}
@@ -126,6 +135,8 @@ func (s *SpecificPortMatcher) Combine(other *SpecificPortMatcher) *SpecificPortM
 	ranges := append(s.PortRanges, other.PortRanges...)
 	// TODO sort port ranges
 
+	logrus.Debugf("ports:\n%s\n", collectionsjson.MustMarshalToString(pps))
+
 	return &SpecificPortMatcher{Ports: pps, PortRanges: ranges}
 }
 

pkg/matcher/simplifier.go

@@ -1,8 +1,10 @@
 package matcher
 
 import (
+	"github.com/mattfenwick/collections/pkg/json"
 	"github.com/mattfenwick/collections/pkg/slice"
 	"github.com/pkg/errors"
+	"github.com/sirupsen/logrus"
 	"golang.org/x/exp/maps"
 )
 
@@ -35,6 +37,7 @@ func Simplify(matchers []PeerMatcher) []PeerMatcher {
 }
 
 func simplifyPortsForAllPeers(matchers []*PortsForAllPeersMatcher) *PortsForAllPeersMatcher {
+	logrus.Debugf("simplifyPortsForAllPeers:\n%s\n", json.MustMarshalToString(matchers))
 	if len(matchers) == 0 {
 		return nil
 	}

pkg/matcher/simplifier_tests.go

@@ -112,15 +112,18 @@ func RunSimplifierTests() {
 	})
 
 	Describe("Port Simplifier", func() {
+		port99 := intstr.FromInt(99)
+		port100 := intstr.FromInt(100)
+		port999 := intstr.FromInt(999)
+
 		It("should combine matchers correctly", func() {
-			port99 := intstr.FromInt(99)
 			allPortsOnSctp := &PortProtocolMatcher{
 				Port:     nil,
 				Protocol: v1.ProtocolSCTP,
 			}
 			port99OnUdp := &PortProtocolMatcher{
 				Port:     &port99,
-				Protocol: v1.ProtocolSCTP,
+				Protocol: v1.ProtocolSCTP, // TODO should this be udp?
 			}
 
 			allMatcher := &AllPortMatcher{}
@@ -136,5 +139,20 @@ func RunSimplifierTests() {
 			Expect(CombinePortMatchers(allPortsOnSctpMatcher, port99OnUdpMatcher)).To(Equal(combinedMatcher))
 			Expect(CombinePortMatchers(port99OnUdpMatcher, allPortsOnSctpMatcher)).To(Equal(combinedMatcher))
 		})
+
+		It("should combine matchers with multiple protocols correctly", func() {
+			tcp100 := &PortProtocolMatcher{Port: &port100, Protocol: v1.ProtocolTCP}
+			udp100 := &PortProtocolMatcher{Port: &port100, Protocol: v1.ProtocolUDP}
+			tcp999 := &PortProtocolMatcher{Port: &port999, Protocol: v1.ProtocolTCP}
+			udp999 := &PortProtocolMatcher{Port: &port999, Protocol: v1.ProtocolUDP}
+
+			matcher100 := &SpecificPortMatcher{Ports: []*PortProtocolMatcher{tcp100, udp100}}
+			matcher999 := &SpecificPortMatcher{Ports: []*PortProtocolMatcher{tcp999, udp999}}
+
+			expected := &SpecificPortMatcher{Ports: []*PortProtocolMatcher{tcp100, udp100, tcp999, udp999}}
+
+			Expect(CombinePortMatchers(matcher999, matcher100)).To(Equal(expected))
+			Expect(CombinePortMatchers(matcher100, matcher999)).To(Equal(expected))
+		})
 	})
 }