From b725800948c3c2332a409fc7d7e4d449ffccf2d6 Mon Sep 17 00:00:00 2001
From: Michael Nairn <mnairn@redhat.com>
Date: Tue, 19 Sep 2023 13:12:11 +0100
Subject: [PATCH] Fix google dns for records with multiple targets

Fix `toResourceRecordSets` in google dns provider to make sure that ony
a single record set is created for each unique dnsName. Issue was
introduced here
https://github.com/Kuadrant/multicluster-gateway-controller/pull/401/commits/c57366eb6ee1123e266bdb543e598303f9ddb3a0#diff-eea59a6fb9851390f6210a03f57ba88ea1628c32d20d526998600135a1642bedR494
---
 pkg/dns/google/google.go      |   2 +-
 pkg/dns/google/google_test.go | 146 ++++++++++++++++++++++++++++++++++
 2 files changed, 147 insertions(+), 1 deletion(-)

diff --git a/pkg/dns/google/google.go b/pkg/dns/google/google.go
index 49ebbb3b9..a269d29c4 100644
--- a/pkg/dns/google/google.go
+++ b/pkg/dns/google/google.go
@@ -491,8 +491,8 @@ func toResourceRecordSets(allEndpoints []*v1alpha1.Endpoint) []*dnsv1.ResourceRe
 				}
 				record.RoutingPolicy.Geo.Items = append(record.RoutingPolicy.Geo.Items, item)
 			}
-			records = append(records, record)
 		}
+		records = append(records, record)
 	}
 	return records
 }
diff --git a/pkg/dns/google/google_test.go b/pkg/dns/google/google_test.go
index e26256fd0..fc9e1ed84 100644
--- a/pkg/dns/google/google_test.go
+++ b/pkg/dns/google/google_test.go
@@ -359,6 +359,152 @@ func Test_toResourceRecordSets(t *testing.T) {
 				},
 			},
 		},
+		{
+			name: "Successful test weight round robin with multiple targets",
+			args: args{
+				allEndpoints: []*v1alpha1.Endpoint{
+					{
+						DNSName:    "2c71gf.lb-4ej5le.unittest.google.hcpapps.net",
+						RecordType: "A",
+						RecordTTL:  60,
+						Targets: v1alpha1.Targets{
+							"0.0.0.0",
+						},
+						SetIdentifier: "",
+					},
+					{
+						DNSName:    "lrnse3.lb-4ej5le.unittest.google.hcpapps.net",
+						RecordType: "A",
+						RecordTTL:  60,
+						Targets: v1alpha1.Targets{
+							"0.0.0.1",
+						},
+						SetIdentifier: "",
+					},
+					{
+						DNSName:       "default.lb-4ej5le.unittest.google.hcpapps.net",
+						RecordType:    "CNAME",
+						SetIdentifier: "2c71gf.lb-4ej5le.unittest.google.hcpapps.net",
+						RecordTTL:     60,
+						Targets: v1alpha1.Targets{
+							"2c71gf.lb-4ej5le.unittest.google.hcpapps.net",
+						},
+						ProviderSpecific: v1alpha1.ProviderSpecific{
+							v1alpha1.ProviderSpecificProperty{
+								Name:  "weight",
+								Value: "120",
+							},
+						},
+					},
+					{
+						DNSName:       "default.lb-4ej5le.unittest.google.hcpapps.net",
+						RecordType:    "CNAME",
+						SetIdentifier: "lrnse3.lb-4ej5le.unittest.google.hcpapps.net",
+						RecordTTL:     60,
+						Targets: v1alpha1.Targets{
+							"lrnse3.lb-4ej5le.unittest.google.hcpapps.net",
+						},
+						ProviderSpecific: v1alpha1.ProviderSpecific{
+							v1alpha1.ProviderSpecificProperty{
+								Name:  "weight",
+								Value: "120",
+							},
+						},
+					},
+					{
+						DNSName:       "lb-4ej5le.unittest.google.hcpapps.net",
+						RecordType:    "CNAME",
+						SetIdentifier: "default",
+						Targets: []string{
+							"default.lb-4ej5le.unittest.google.hcpapps.net",
+						},
+						RecordTTL: 300,
+						ProviderSpecific: v1alpha1.ProviderSpecific{
+							v1alpha1.ProviderSpecificProperty{
+								Name:  "geo-code",
+								Value: "*",
+							},
+						},
+					},
+					{
+						DNSName:    "unittest.google.hcpapps.net",
+						RecordType: "CNAME",
+						RecordTTL:  300,
+						Targets: []string{
+							"lb-4ej5le.unittest.google.hcpapps.net",
+						},
+						SetIdentifier: "",
+					},
+				},
+			},
+			want: []*dnsv1.ResourceRecordSet{
+				{
+					Name: "2c71gf.lb-4ej5le.unittest.google.hcpapps.net.",
+					Rrdatas: []string{
+						"0.0.0.0",
+					},
+					Ttl:  60,
+					Type: "A",
+				},
+				{
+					Name: "lrnse3.lb-4ej5le.unittest.google.hcpapps.net.",
+					Rrdatas: []string{
+						"0.0.0.1",
+					},
+					Ttl:  60,
+					Type: "A",
+				},
+				{
+					Name: "default.lb-4ej5le.unittest.google.hcpapps.net.",
+					RoutingPolicy: &dnsv1.RRSetRoutingPolicy{
+						Wrr: &dnsv1.RRSetRoutingPolicyWrrPolicy{
+							Items: []*dnsv1.RRSetRoutingPolicyWrrPolicyWrrPolicyItem{
+								{
+									Rrdatas: []string{
+										"2c71gf.lb-4ej5le.unittest.google.hcpapps.net.",
+									},
+									Weight: 120,
+								},
+								{
+									Rrdatas: []string{
+										"lrnse3.lb-4ej5le.unittest.google.hcpapps.net.",
+									},
+									Weight: 120,
+								},
+							},
+						},
+					},
+					Ttl:  60,
+					Type: "CNAME",
+				},
+				{
+					Name: "lb-4ej5le.unittest.google.hcpapps.net.",
+					RoutingPolicy: &dnsv1.RRSetRoutingPolicy{
+						Geo: &dnsv1.RRSetRoutingPolicyGeoPolicy{
+							EnableFencing: false,
+							Items: []*dnsv1.RRSetRoutingPolicyGeoPolicyGeoPolicyItem{
+								{
+									Location: "europe-west1",
+									Rrdatas: []string{
+										"default.lb-4ej5le.unittest.google.hcpapps.net.",
+									},
+								},
+							},
+						},
+					},
+					Ttl:  300,
+					Type: "CNAME",
+				},
+				{
+					Name: "unittest.google.hcpapps.net.",
+					Rrdatas: []string{
+						"lb-4ej5le.unittest.google.hcpapps.net.",
+					},
+					Ttl:  300,
+					Type: "CNAME",
+				},
+			},
+		},
 	}
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {