From 663aac1a1b711386914f0755868b137f28058472 Mon Sep 17 00:00:00 2001
From: Shaun Crampton <shaun@tigera.io>
Date: Wed, 6 Dec 2023 15:11:37 +0000
Subject: [PATCH] Normalise selectors.

---
 felix/calc/rule_scanner.go            | 18 ++++++++++++++++--
 felix/calc/rule_scanner_test.go       |  8 +++++---
 felix/dataplane/linux/endpoint_mgr.go |  2 --
 libcalico-go/lib/selector/selector.go | 18 +++++++++++++++++-
 4 files changed, 38 insertions(+), 8 deletions(-)

diff --git a/felix/calc/rule_scanner.go b/felix/calc/rule_scanner.go
index 871601087e8..471d586d5f6 100644
--- a/felix/calc/rule_scanner.go
+++ b/felix/calc/rule_scanner.go
@@ -182,7 +182,15 @@ func (rs *RuleScanner) OnProfileInactive(key model.ProfileRulesKey) {
 }
 
 func (rs *RuleScanner) OnPolicyActive(key model.PolicyKey, policy *model.Policy) {
-	parsedRules := rs.updateRules(key, policy.InboundRules, policy.OutboundRules, policy.DoNotTrack, policy.PreDNAT, policy.Namespace, policy.Selector)
+	parsedRules := rs.updateRules(
+		key,
+		policy.InboundRules,
+		policy.OutboundRules,
+		policy.DoNotTrack,
+		policy.PreDNAT,
+		policy.Namespace,
+		selector.Normalise(policy.Selector),
+	)
 	rs.RulesUpdateCallbacks.OnPolicyActive(key, parsedRules)
 }
 
@@ -191,7 +199,13 @@ func (rs *RuleScanner) OnPolicyInactive(key model.PolicyKey) {
 	rs.RulesUpdateCallbacks.OnPolicyInactive(key)
 }
 
-func (rs *RuleScanner) updateRules(key interface{}, inbound, outbound []model.Rule, untracked, preDNAT bool, origNamespace string, origSelector string) (parsedRules *ParsedRules) {
+func (rs *RuleScanner) updateRules(
+	key interface{},
+	inbound, outbound []model.Rule,
+	untracked, preDNAT bool,
+	origNamespace string,
+	origSelector string,
+) (parsedRules *ParsedRules) {
 	log.Debugf("Scanning rules (%v in, %v out) for key %v",
 		len(inbound), len(outbound), key)
 	// Extract all the new selectors/named ports.
diff --git a/felix/calc/rule_scanner_test.go b/felix/calc/rule_scanner_test.go
index 1831ea9ab1c..d64374a2c8e 100644
--- a/felix/calc/rule_scanner_test.go
+++ b/felix/calc/rule_scanner_test.go
@@ -94,13 +94,15 @@ var _ = DescribeTable("RuleScanner rule conversion should generate correct Parse
 			Namespace:     "namespace",
 			InboundRules:  []model.Rule{modelRule},
 			OutboundRules: []model.Rule{},
+			Selector:      "a  ==  'A' ",
 		}
 		rs.OnPolicyActive(policyKey, policy)
 		Expect(ur.activeRules).To(Equal(map[model.Key]*ParsedRules{
 			policyKey: {
-				Namespace:     "namespace",
-				InboundRules:  []*ParsedRule{&expectedParsedRule},
-				OutboundRules: []*ParsedRule{},
+				Namespace:        "namespace",
+				InboundRules:     []*ParsedRule{&expectedParsedRule},
+				OutboundRules:    []*ParsedRule{},
+				OriginalSelector: "a == \"A\"",
 			},
 		}))
 		rs.OnPolicyInactive(policyKey)
diff --git a/felix/dataplane/linux/endpoint_mgr.go b/felix/dataplane/linux/endpoint_mgr.go
index 12fdf221aab..0077d13333d 100644
--- a/felix/dataplane/linux/endpoint_mgr.go
+++ b/felix/dataplane/linux/endpoint_mgr.go
@@ -371,8 +371,6 @@ func (m *endpointManager) OnUpdate(protoBufMsg interface{}) {
 		}
 		m.hostEndpointsDirty = true
 	case *proto.ActivePolicyUpdate:
-		// FIXME normalise selectors somewhere?  Here on in calc graph.
-		//       want "" to be equal to "all()" and for whitespace to be ignored.
 		newSel := msg.Policy.OriginalSelector
 		if oldSel, ok := m.activePolicySelectors[*msg.Id]; ok && oldSel == newSel {
 			// No change that we care about.
diff --git a/libcalico-go/lib/selector/selector.go b/libcalico-go/lib/selector/selector.go
index ebcea7281c1..7b91ba2fe10 100644
--- a/libcalico-go/lib/selector/selector.go
+++ b/libcalico-go/lib/selector/selector.go
@@ -14,7 +14,11 @@
 
 package selector
 
-import "github.com/projectcalico/calico/libcalico-go/lib/selector/parser"
+import (
+	"strings"
+
+	"github.com/projectcalico/calico/libcalico-go/lib/selector/parser"
+)
 
 // Selector represents a label selector.
 type Selector interface {
@@ -38,3 +42,15 @@ type Selector interface {
 func Parse(selector string) (sel Selector, err error) {
 	return parser.Parse(selector)
 }
+
+// Normalise converts the given selector to the form returned by
+// Selector.String(), i.e. "" is converted to "all()" and whitespace is
+// tidied up.  If the input string cannot be parsed, it is returned unaltered.
+func Normalise(selector string) string {
+	selector = strings.TrimSpace(selector)
+	parsed, err := Parse(selector)
+	if err != nil {
+		return selector
+	}
+	return parsed.String()
+}