Skip to content

Commit

Permalink
Fix for duplicated endpoints and unit tests
Browse files Browse the repository at this point in the history
  • Loading branch information
leonardocaylent committed Mar 30, 2024
1 parent 7fe2d3f commit 2b3da1b
Show file tree
Hide file tree
Showing 3 changed files with 29 additions and 23 deletions.
2 changes: 2 additions & 0 deletions controller/controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -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))
Expand All @@ -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)
}
Expand Down
30 changes: 19 additions & 11 deletions endpoint/endpoint.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
}

Expand Down Expand Up @@ -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
}
20 changes: 8 additions & 12 deletions endpoint/endpoint_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -241,8 +241,7 @@ func TestDuplicatedFilterEndpointsByOwnerID(t *testing.T) {
}

type args struct {
ownerID string
eps []*Endpoint
eps []*Endpoint
}
tests := []struct {
name string
Expand All @@ -252,7 +251,6 @@ func TestDuplicatedFilterEndpointsByOwnerID(t *testing.T) {
{
name: "filter values",
args: args{
ownerID: "foo",
eps: []*Endpoint{
foo1,
foo2,
Expand All @@ -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,
Expand Down Expand Up @@ -318,8 +316,7 @@ func TestZonesDuplicatedFilterEndpointsByOwnerID(t *testing.T) {
}

type args struct {
ownerID string
eps []*Endpoint
eps []*Endpoint
}
tests := []struct {
name string
Expand All @@ -329,7 +326,6 @@ func TestZonesDuplicatedFilterEndpointsByOwnerID(t *testing.T) {
{
name: "filter values",
args: args{
ownerID: "foo",
eps: []*Endpoint{
foo1,
foo2,
Expand All @@ -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)
}
})
}
Expand Down

0 comments on commit 2b3da1b

Please sign in to comment.