Merge pull request #4856 from dtuck9/avoid-merging-cname-records

fix: do not merge CNAME with multiple targets
kubernetes-sigs · Jan 14, 2025 · 36478f1 · 36478f1
2 parents d0ae7cf + fe420f4
commit 36478f1
Show file tree

Hide file tree

Showing 2 changed files with 35 additions and 4 deletions.
diff --git a/source/service.go b/source/service.go
@@ -223,12 +223,19 @@ func (sc *serviceSource) Endpoints(ctx context.Context) ([]*endpoint.Endpoint, e
 			lastMergedEndpoint := len(mergedEndpoints) - 1
 			if mergedEndpoints[lastMergedEndpoint].DNSName == endpoints[i].DNSName &&
 				mergedEndpoints[lastMergedEndpoint].RecordType == endpoints[i].RecordType &&
+				mergedEndpoints[lastMergedEndpoint].RecordType != endpoint.RecordTypeCNAME && // It is against RFC-1034 for CNAME records to have multiple targets, so skip merging
 				mergedEndpoints[lastMergedEndpoint].SetIdentifier == endpoints[i].SetIdentifier &&
 				mergedEndpoints[lastMergedEndpoint].RecordTTL == endpoints[i].RecordTTL {
 				mergedEndpoints[lastMergedEndpoint].Targets = append(mergedEndpoints[lastMergedEndpoint].Targets, endpoints[i].Targets[0])
 			} else {
 				mergedEndpoints = append(mergedEndpoints, endpoints[i])
 			}
+
+			if mergedEndpoints[lastMergedEndpoint].DNSName == endpoints[i].DNSName &&
+				mergedEndpoints[lastMergedEndpoint].RecordType == endpoints[i].RecordType &&
+				mergedEndpoints[lastMergedEndpoint].RecordType == endpoint.RecordTypeCNAME {
+				log.Debugf("CNAME %s with multiple targets found", endpoints[i].DNSName)
+			}
 		}
 		endpoints = mergedEndpoints
 	}

diff --git a/source/service_test.go b/source/service_test.go
@@ -1260,13 +1260,37 @@ func testMultipleServicesEndpoints(t *testing.T) {
 			map[string]string{},
 			"",
 			map[string]map[string]string{
-				"a.elb.com": {hostnameAnnotationKey: "foo.example.org", SetIdentifierKey: "a"},
-				"b.elb.com": {hostnameAnnotationKey: "foo.example.org", SetIdentifierKey: "b"},
+				"1.2.3.5":  {hostnameAnnotationKey: "foo.example.org", SetIdentifierKey: "a"},
+				"10.1.1.3": {hostnameAnnotationKey: "foo.example.org", SetIdentifierKey: "b"},
 			},
 			[]string{},
 			[]*endpoint.Endpoint{
-				{DNSName: "foo.example.org", RecordType: endpoint.RecordTypeCNAME, Targets: endpoint.Targets{"a.elb.com"}, Labels: map[string]string{endpoint.ResourceLabelKey: "service/testing/fooa.elb.com"}, SetIdentifier: "a"},
-				{DNSName: "foo.example.org", RecordType: endpoint.RecordTypeCNAME, Targets: endpoint.Targets{"b.elb.com"}, Labels: map[string]string{endpoint.ResourceLabelKey: "service/testing/foob.elb.com"}, SetIdentifier: "b"},
+				{DNSName: "foo.example.org", RecordType: endpoint.RecordTypeA, Targets: endpoint.Targets{"1.2.3.5"}, Labels: map[string]string{endpoint.ResourceLabelKey: "service/testing/foo1.2.3.5"}, SetIdentifier: "a"},
+				{DNSName: "foo.example.org", RecordType: endpoint.RecordTypeA, Targets: endpoint.Targets{"10.1.1.3"}, Labels: map[string]string{endpoint.ResourceLabelKey: "service/testing/foo10.1.1.3"}, SetIdentifier: "b"},
+			},
+			false,
+		},
+		{
+			"test that services with CNAME types do not get merged together",
+			"",
+			"",
+			"testing",
+			"foo",
+			v1.ServiceTypeLoadBalancer,
+			"",
+			"",
+			false,
+			false,
+			map[string]string{},
+			"",
+			map[string]map[string]string{
+				"a.elb.com": {hostnameAnnotationKey: "foo.example.org"},
+				"b.elb.com": {hostnameAnnotationKey: "foo.example.org"},
+			},
+			[]string{},
+			[]*endpoint.Endpoint{
+				{DNSName: "foo.example.org", RecordType: endpoint.RecordTypeCNAME, Targets: endpoint.Targets{"a.elb.com"}, Labels: map[string]string{endpoint.ResourceLabelKey: "service/testing/fooa.elb.com"}},
+				{DNSName: "foo.example.org", RecordType: endpoint.RecordTypeCNAME, Targets: endpoint.Targets{"b.elb.com"}, Labels: map[string]string{endpoint.ResourceLabelKey: "service/testing/foob.elb.com"}},
 			},
 			false,
 		},