fix(endpoint): debug message when owner label is missing (#5788)

* fix: debug message of FilterEndpointsByOwnerID in case owner label is missing

* more consistent messages

* add unit test for changed debug message

---------

Co-authored-by: Pascal Bachor <[email protected]>

Author: bachorp

endpoint/endpoint.go

@@ -378,8 +378,11 @@ func (e *Endpoint) Describe() string {
 func FilterEndpointsByOwnerID(ownerID string, eps []*Endpoint) []*Endpoint {
 	filtered := []*Endpoint{}
 	for _, ep := range eps {
-		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)
+		endpointOwner, ok := ep.Labels[OwnerLabelKey]
+		if !ok {
+			log.Debugf(`Skipping endpoint %v because of missing owner label (required: "%s")`, ep, ownerID)
+		} else if 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)
 		}

internal/testutils/endpoint_test.go

@@ -23,6 +23,8 @@ import (
 	"sort"
 	"testing"
 
+	log "github.com/sirupsen/logrus"
+
 	"github.com/stretchr/testify/assert"
 	"sigs.k8s.io/external-dns/endpoint"
 )
@@ -489,3 +491,63 @@ func TestWithLabel(t *testing.T) {
 	assert.Equal(t, "orig", e.Labels["existing"])
 	assert.Equal(t, "val", e.Labels["new"])
 }
+
+func TestFilterEndpointsByOwnerIDLogging(t *testing.T) {
+	noOwner := &endpoint.Endpoint{}
+	ownedByFoo := &endpoint.Endpoint{
+		Labels: endpoint.Labels{
+			endpoint.OwnerLabelKey: "foo",
+		},
+	}
+	ownedByBar := &endpoint.Endpoint{
+		Labels: endpoint.Labels{
+			endpoint.OwnerLabelKey: "bar",
+		},
+	}
+	tests := []struct {
+		name         string
+		ownerID      string
+		endpoints    []*endpoint.Endpoint
+		messages     []string
+		messages_not []string
+		result       []*endpoint.Endpoint
+	}{
+		{
+			name:         "one_matches",
+			ownerID:      "foo",
+			endpoints:    []*endpoint.Endpoint{ownedByFoo},
+			messages:     []string{},
+			messages_not: []string{""},
+			result:       []*endpoint.Endpoint{ownedByFoo},
+		},
+		{
+			name:         "wrong_owner",
+			ownerID:      "foo",
+			endpoints:    []*endpoint.Endpoint{ownedByFoo, ownedByBar},
+			messages:     []string{"because owner id does not match"},
+			messages_not: []string{},
+			result:       []*endpoint.Endpoint{ownedByFoo},
+		},
+		{
+			name:         "no_owner",
+			ownerID:      "bar",
+			endpoints:    []*endpoint.Endpoint{noOwner, ownedByBar},
+			messages:     []string{"because of missing owner label"},
+			messages_not: []string{"because owner id does not match"},
+			result:       []*endpoint.Endpoint{ownedByBar},
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			hook := LogsUnderTestWithLogLevel(log.DebugLevel, t)
+			endpoint.FilterEndpointsByOwnerID(tt.ownerID, tt.endpoints)
+			for _, m := range tt.messages {
+				TestHelperLogContains(m, hook, t)
+			}
+			for _, m := range tt.messages_not {
+				TestHelperLogNotContains(m, hook, t)
+			}
+		})
+	}
+}