policy: Fix mapstate changes error in entry change comparison

[ upstream commit 3d3f69c ]

Datapath equalness of the old and new entry must be evaluated before
entries are merged, afterwards the entries will be the same and the key
is not added in ChangeState.Adds, which may lead to a policy map entry
not being updated during incremental updates.

Fixes: #26331

Signed-off-by: Jarno Rajahalme <jarno@isovalent.com>
Signed-off-by: Yutaro Hayakawa <yutaro.hayakawa@isovalent.com>

pkg/policy/mapstate.go

@@ -366,6 +366,7 @@ func (keys MapState) denyPreferredInsert(newKey Key, newEntry MapStateEntry, ide
 func (keys MapState) addKeyWithChanges(key Key, entry MapStateEntry, changes ChangeState) {
 	// Keep all owners that need this entry so that it is deleted only if all the owners delete their contribution
 	oldEntry, exists := keys[key]
+	var datapathEqual bool
 	if exists {
 		// Deny entry can only be overridden by another deny entry
 		if oldEntry.IsDeny && !entry.IsDeny {
@@ -381,6 +382,9 @@ func (keys MapState) addKeyWithChanges(key Key, entry MapStateEntry, changes Cha
 			changes.Old.insertIfNotExists(key, oldEntry)
 		}
 
+		// Compare for datapath equalness before merging, as the old entry is updated in
+		// place!
+		datapathEqual = oldEntry.DatapathEqual(&entry)
 		oldEntry.Merge(&entry)
 		keys[key] = oldEntry
 	} else {
@@ -393,7 +397,7 @@ func (keys MapState) addKeyWithChanges(key Key, entry MapStateEntry, changes Cha
 	}
 
 	// Record an incremental Add if desired and entry is new or changed
-	if changes.Adds != nil && (!exists || !oldEntry.DatapathEqual(&entry)) {
+	if changes.Adds != nil && (!exists || !datapathEqual) {
 		changes.Adds[key] = struct{}{}
 		// Key add overrides any previous delete of the same key
 		if changes.Deletes != nil {

pkg/policy/mapstate_test.go

@@ -1867,6 +1867,9 @@ func (ds *PolicyTestSuite) TestMapState_AddVisibilityKeys(c *check.C) {
 		for k, v := range tt.keys {
 			if v2, ok := old[k]; ok {
 				if equals, _ := checker.DeepEqual(v2, v); !equals {
+					if !v.DatapathEqual(&v2) {
+						wantAdds[k] = struct{}{}
+					}
 					wantOld[k] = v2
 				}
 			} else {
@@ -2110,6 +2113,7 @@ func (ds *PolicyTestSuite) TestMapState_AccumulateMapChangesOnVisibilityKeys(c *
 		},
 		visAdds: Keys{
 			HttpIngressKey(0): {},
+			HttpEgressKey(0):  {},
 		},
 		visOld: MapState{
 			// Old value for the modified entry