From 2b3da1b6080ecd32cdde0cb007cc0839208383e3 Mon Sep 17 00:00:00 2001
From: Leonardo Quatrocchi <leonardo.quatrocchi@caylent.com>
Date: Fri, 29 Mar 2024 23:27:25 -0300
Subject: [PATCH] Fix for duplicated endpoints and unit tests

---
 controller/controller.go  |  2 ++
 endpoint/endpoint.go      | 30 +++++++++++++++++++-----------
 endpoint/endpoint_test.go | 20 ++++++++------------
 3 files changed, 29 insertions(+), 23 deletions(-)

diff --git a/controller/controller.go b/controller/controller.go
index a3ef2e8b2..4edf72ebf 100644
--- a/controller/controller.go
+++ b/controller/controller.go
@@ -210,6 +210,7 @@ func (c *Controller) RunOnce(ctx context.Context) error {
 		return err
 	}
 
+	records = endpoint.RemoveDuplicates(records)
 	registryEndpointsTotal.Set(float64(len(records)))
 	regARecords, regAAAARecords := countAddressRecords(records)
 	registryARecords.Set(float64(regARecords))
@@ -230,6 +231,7 @@ func (c *Controller) RunOnce(ctx context.Context) error {
 	verifiedARecords.Set(float64(vARecords))
 	verifiedAAAARecords.Set(float64(vAAAARecords))
 	endpoints, err = c.Registry.AdjustEndpoints(endpoints)
+	endpoints = endpoint.RemoveDuplicates(endpoints)
 	if err != nil {
 		return fmt.Errorf("adjusting endpoints: %w", err)
 	}
diff --git a/endpoint/endpoint.go b/endpoint/endpoint.go
index 9c918cfb7..3d6d9161c 100644
--- a/endpoint/endpoint.go
+++ b/endpoint/endpoint.go
@@ -293,22 +293,11 @@ func (e *Endpoint) String() string {
 // only endpoints that match.
 func FilterEndpointsByOwnerID(ownerID string, eps []*Endpoint) []*Endpoint {
 	filtered := []*Endpoint{}
-	// Initialize the map for detecting duplicated endpoints
-	visited := make(map[EndpointKey]bool)
-
 	for _, ep := range eps {
-		key := ep.Key()
-		// Using EndpointKey to generate the primary key using DNSName, RecordType & SetIdentifier.
-		if visited[key] { //Do not contain duplicated endpoints
-			log.Debugf(`Skipping duplicated endpoint %v `, ep)
-			continue
-		}
 		if endpointOwner, ok := ep.Labels[OwnerLabelKey]; !ok || endpointOwner != ownerID {
 			log.Debugf(`Skipping endpoint %v because owner id does not match, found: "%s", required: "%s"`, ep, endpointOwner, ownerID)
 		} else {
 			filtered = append(filtered, ep)
-			visited[key] = true
-			log.Debugf(`Added endpoint %v because owner id matches, found: "%s", required: "%s"`, ep, endpointOwner, ownerID)
 		}
 	}
 
@@ -354,3 +343,22 @@ type DNSEndpointList struct {
 	metav1.ListMeta `json:"metadata,omitempty"`
 	Items           []DNSEndpoint `json:"items"`
 }
+
+// Apply filter based on EndpointKey (using DNSName, RecordType & SetIdentifier)
+func RemoveDuplicates(filtered []*Endpoint) []*Endpoint {
+	visited := make(map[EndpointKey]bool)
+	result := []*Endpoint{}
+
+	for _, ep := range filtered {
+		key := ep.Key()
+
+		if !visited[key] {
+			result = append(result, ep)
+			visited[key] = true
+		} else {
+			log.Debugf(`Skipping duplicated endpoint: %v`, ep)
+		}
+	}
+
+	return result
+}
diff --git a/endpoint/endpoint_test.go b/endpoint/endpoint_test.go
index 9f009a4b4..57584d993 100644
--- a/endpoint/endpoint_test.go
+++ b/endpoint/endpoint_test.go
@@ -217,7 +217,7 @@ func TestIsOwnedBy(t *testing.T) {
 	}
 }
 
-func TestDuplicatedFilterEndpointsByOwnerID(t *testing.T) {
+func TestDuplicatedEndpointsWithSimpleZone(t *testing.T) {
 	foo1 := &Endpoint{
 		DNSName:    "foo.com",
 		RecordType: RecordTypeA,
@@ -241,8 +241,7 @@ func TestDuplicatedFilterEndpointsByOwnerID(t *testing.T) {
 	}
 
 	type args struct {
-		ownerID string
-		eps     []*Endpoint
+		eps []*Endpoint
 	}
 	tests := []struct {
 		name string
@@ -252,7 +251,6 @@ func TestDuplicatedFilterEndpointsByOwnerID(t *testing.T) {
 		{
 			name: "filter values",
 			args: args{
-				ownerID: "foo",
 				eps: []*Endpoint{
 					foo1,
 					foo2,
@@ -266,14 +264,14 @@ func TestDuplicatedFilterEndpointsByOwnerID(t *testing.T) {
 	}
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
-			if got := FilterEndpointsByOwnerID(tt.args.ownerID, tt.args.eps); !reflect.DeepEqual(got, tt.want) {
-				t.Errorf("FilterEndpointsByOwnerID() = %v, want %v", got, tt.want)
+			if got := RemoveDuplicates(tt.args.eps); !reflect.DeepEqual(got, tt.want) {
+				t.Errorf("RemoveDuplicates() = %v, want %v", got, tt.want)
 			}
 		})
 	}
 }
 
-func TestZonesDuplicatedFilterEndpointsByOwnerID(t *testing.T) {
+func TestDuplicatedEndpointsWithOverlappingZones(t *testing.T) {
 	foo1 := &Endpoint{
 		DNSName:    "internal.foo.com",
 		RecordType: RecordTypeA,
@@ -318,8 +316,7 @@ func TestZonesDuplicatedFilterEndpointsByOwnerID(t *testing.T) {
 	}
 
 	type args struct {
-		ownerID string
-		eps     []*Endpoint
+		eps []*Endpoint
 	}
 	tests := []struct {
 		name string
@@ -329,7 +326,6 @@ func TestZonesDuplicatedFilterEndpointsByOwnerID(t *testing.T) {
 		{
 			name: "filter values",
 			args: args{
-				ownerID: "foo",
 				eps: []*Endpoint{
 					foo1,
 					foo2,
@@ -347,8 +343,8 @@ func TestZonesDuplicatedFilterEndpointsByOwnerID(t *testing.T) {
 	}
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
-			if got := FilterEndpointsByOwnerID(tt.args.ownerID, tt.args.eps); !reflect.DeepEqual(got, tt.want) {
-				t.Errorf("FilterEndpointsByOwnerID() = %v, want %v", got, tt.want)
+			if got := RemoveDuplicates(tt.args.eps); !reflect.DeepEqual(got, tt.want) {
+				t.Errorf("RemoveDuplicates() = %v, want %v", got, tt.want)
 			}
 		})
 	}