From 992b321d34c90e3d6774fc24a4a9dbbad610b708 Mon Sep 17 00:00:00 2001
From: Michael Nairn <mnairn@redhat.com>
Date: Mon, 29 Apr 2024 17:36:24 +0100
Subject: [PATCH] plan: Fix plan manipulating desired

Anywhere the plan is manipulating endpoints needs to work with a copy to
ensure that it doesn't change the passed in spec value. Was causing an
issue where the status was being updated incorrectly due to the
dnsrecord spec endpoints changing.
---
 internal/external-dns/plan/plan.go | 14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/internal/external-dns/plan/plan.go b/internal/external-dns/plan/plan.go
index 5752efe3..ef96c044 100644
--- a/internal/external-dns/plan/plan.go
+++ b/internal/external-dns/plan/plan.go
@@ -437,13 +437,14 @@ func (e *managedRecordSetChanges) calculateDesired(update *endpointUpdate) {
 	}
 
 	currentCopy := update.current.DeepCopy()
+	desiredCopy := update.desired.DeepCopy()
 
 	// A Records can be merged, but we remove the known previous target values first in order to ensure potentially stale values are removed
 	if update.current.RecordType == endpoint.RecordTypeA {
 		if update.previous != nil {
 			removeEndpointTargets(update.previous.Targets, currentCopy)
 		}
-		mergeEndpointTargets(update.desired, currentCopy)
+		mergeEndpointTargets(desiredCopy, currentCopy)
 	}
 
 	// CNAME records can be merged, it's expected that the provider implementation understands that a CNAME might have
@@ -452,16 +453,14 @@ func (e *managedRecordSetChanges) calculateDesired(update *endpointUpdate) {
 		if update.previous != nil {
 			removeEndpointTargets(update.previous.Targets, currentCopy)
 		}
-		mergeEndpointTargets(update.desired, currentCopy)
+		mergeEndpointTargets(desiredCopy, currentCopy)
 
 		//ToDo manirn Check this is actually needed, and if it is add a test that requires it to be here
-		if len(update.desired.Targets) <= 1 {
+		if len(desiredCopy.Targets) <= 1 {
 			log.Debugf("skipping check for managed dnsNames for CNAME with single target value")
 			return
 		}
 
-		desiredCopy := update.desired.DeepCopy()
-
 		// Calculate if any of the new desired targets are also managed dnsNames within this record set.
 		// If a target is not managed, do nothing and continue with the current targets.
 		// If a target is managed:
@@ -476,7 +475,7 @@ func (e *managedRecordSetChanges) calculateDesired(update *endpointUpdate) {
 
 				// If the target has no owners we can just remove it
 				if len(tOwners) == 0 {
-					removeEndpointTarget(t, update.desired)
+					removeEndpointTarget(t, desiredCopy)
 					break
 				}
 
@@ -490,13 +489,14 @@ func (e *managedRecordSetChanges) calculateDesired(update *endpointUpdate) {
 						}
 					}
 					if !hasMutualOwner {
-						removeEndpointTarget(t, update.desired)
+						removeEndpointTarget(t, desiredCopy)
 					}
 				}
 
 			}
 		}
 	}
+	update.desired = desiredCopy
 }
 
 func removeEndpointTarget(target string, endpoint *endpoint.Endpoint) {