From cb10e8050c942e055e7260c03c5cbf998e66dd79 Mon Sep 17 00:00:00 2001 From: Maskym Vavilov Date: Tue, 16 Apr 2024 14:09:22 +0100 Subject: [PATCH] GH-545 restrict default geo to local --- controllers/dns_helper.go | 33 ++-- ...dnspolicy_controller_multi_cluster_test.go | 141 ++++++++++++++++-- ...nspolicy_controller_single_cluster_test.go | 16 -- pkg/common/common.go | 3 + 4 files changed, 144 insertions(+), 49 deletions(-) diff --git a/controllers/dns_helper.go b/controllers/dns_helper.go index 7d6d20f8d..2fd4c5f71 100644 --- a/controllers/dns_helper.go +++ b/controllers/dns_helper.go @@ -248,28 +248,27 @@ func (dh *dnsHelper) getLoadBalancedEndpoints(mcgTarget *multicluster.GatewayTar } endpoints = append(endpoints, clusterEndpoints...) - //Create lbName CNAME (lb-a1b2.shop.example.com -> default.lb-a1b2.shop.example.com) - ep = createOrUpdateEndpoint(lbName, []string{geoLbName}, kuadrantdnsv1alpha1.CNAMERecordType, string(geoCode), DefaultCnameTTL, currentEndpoints) - - //Deal with the default geo endpoint first - if geoCode.IsDefaultCode() { - defaultEndpoint = ep - // continue here as we will add the `defaultEndpoint` later - continue - } else if (geoCode == mcgTarget.GetDefaultGeo()) || defaultEndpoint == nil { - // Ensure that a `defaultEndpoint` is always set, but the expected default takes precedence - defaultEndpoint = createOrUpdateEndpoint(lbName, []string{geoLbName}, kuadrantdnsv1alpha1.CNAMERecordType, "default", DefaultCnameTTL, currentEndpoints) + if !geoCode.IsDefaultCode() { + //Deal with the default geo endpoint + if geoCode == mcgTarget.GetDefaultGeo() { + // Ensure that `defaultEndpoint` is set only if geo of the current cluster is desired default geo + defaultEndpoint = createOrUpdateEndpoint(lbName, []string{geoLbName}, kuadrantdnsv1alpha1.CNAMERecordType, "default", DefaultCnameTTL, currentEndpoints) + } + //Create lbName CNAME (lb-a1b2.shop.example.com -> default.lb-a1b2.shop.example.com) + ep = createOrUpdateEndpoint(lbName, []string{geoLbName}, kuadrantdnsv1alpha1.CNAMERecordType, string(geoCode), DefaultCnameTTL, currentEndpoints) + ep.SetProviderSpecificProperty(kuadrantdnsv1alpha1.ProviderSpecificGeoCode, string(geoCode)) + endpoints = append(endpoints, ep) } - ep.SetProviderSpecificProperty(kuadrantdnsv1alpha1.ProviderSpecificGeoCode, string(geoCode)) - - endpoints = append(endpoints, ep) } if len(endpoints) > 0 { - // Add the `defaultEndpoint`, this should always be set by this point if `endpoints` isn't empty - defaultEndpoint.SetProviderSpecificProperty(kuadrantdnsv1alpha1.ProviderSpecificGeoCode, string(v1alpha1.WildcardGeo)) - endpoints = append(endpoints, defaultEndpoint) + + // Add the `defaultEndpoint`, this should be set only if current cluster geo is the desired default geo + if defaultEndpoint != nil { + defaultEndpoint.SetProviderSpecificProperty(kuadrantdnsv1alpha1.ProviderSpecificGeoCode, string(v1alpha1.WildcardGeo)) + endpoints = append(endpoints, defaultEndpoint) + } //Create gwListenerHost CNAME (shop.example.com -> lb-a1b2.shop.example.com) ep = createOrUpdateEndpoint(hostname, []string{lbName}, kuadrantdnsv1alpha1.CNAMERecordType, "", DefaultCnameTTL, currentEndpoints) endpoints = append(endpoints, ep) diff --git a/controllers/dnspolicy_controller_multi_cluster_test.go b/controllers/dnspolicy_controller_multi_cluster_test.go index 8ed0e51a6..f0aadd203 100644 --- a/controllers/dnspolicy_controller_multi_cluster_test.go +++ b/controllers/dnspolicy_controller_multi_cluster_test.go @@ -244,14 +244,6 @@ var _ = Describe("DNSPolicy Multi Cluster", func() { "SetIdentifier": Equal(""), "RecordTTL": Equal(externaldns.TTL(60)), })), - PointTo(MatchFields(IgnoreExtras, Fields{ - "DNSName": Equal("klb.test.example.com"), - "Targets": ConsistOf("default.klb.test.example.com"), - "RecordType": Equal("CNAME"), - "SetIdentifier": Equal("default"), - "RecordTTL": Equal(externaldns.TTL(300)), - "ProviderSpecific": Equal(externaldns.ProviderSpecific{{Name: "geo-code", Value: "*"}}), - })), PointTo(MatchFields(IgnoreExtras, Fields{ "DNSName": Equal(TestHostOne), "Targets": ConsistOf("klb.test.example.com"), @@ -300,14 +292,6 @@ var _ = Describe("DNSPolicy Multi Cluster", func() { "SetIdentifier": Equal(""), "RecordTTL": Equal(externaldns.TTL(60)), })), - PointTo(MatchFields(IgnoreExtras, Fields{ - "DNSName": Equal("klb.example.com"), - "Targets": ConsistOf("default.klb.example.com"), - "RecordType": Equal("CNAME"), - "SetIdentifier": Equal("default"), - "RecordTTL": Equal(externaldns.TTL(300)), - "ProviderSpecific": Equal(externaldns.ProviderSpecific{{Name: "geo-code", Value: "*"}}), - })), PointTo(MatchFields(IgnoreExtras, Fields{ "DNSName": Equal(TestHostWildcard), "Targets": ConsistOf("klb.example.com"), @@ -680,6 +664,131 @@ var _ = Describe("DNSPolicy Multi Cluster", func() { }) + Context("geo+weighted with no default geo", func() { + + BeforeEach(func() { + dnsPolicy = v1alpha1.NewDNSPolicy("test-dns-policy", testNamespace). + WithTargetGateway(TestGatewayName). + WithRoutingStrategy(v1alpha1.LoadBalancedRoutingStrategy) + Expect(k8sClient.Create(ctx, dnsPolicy)).To(Succeed()) + }) + + It("should create dns records", func() { + Eventually(func(g Gomega, ctx context.Context) { + recordList := &kuadrantdnsv1alpha1.DNSRecordList{} + err := k8sClient.List(ctx, recordList, &client.ListOptions{Namespace: testNamespace}) + g.Expect(err).NotTo(HaveOccurred()) + g.Expect(recordList.Items).To(HaveLen(2)) + + dnsRecord := &kuadrantdnsv1alpha1.DNSRecord{} + err = k8sClient.Get(ctx, client.ObjectKey{Name: recordName, Namespace: testNamespace}, dnsRecord) + g.Expect(err).NotTo(HaveOccurred()) + + wildcardDnsRecord := &kuadrantdnsv1alpha1.DNSRecord{} + err = k8sClient.Get(ctx, client.ObjectKey{Name: wildcardRecordName, Namespace: testNamespace}, wildcardDnsRecord) + g.Expect(err).NotTo(HaveOccurred()) + + g.Expect(*dnsRecord).To( + MatchFields(IgnoreExtras, Fields{ + "ObjectMeta": HaveField("Name", recordName), + "Spec": MatchFields(IgnoreExtras, Fields{ + "OwnerID": Equal(&clusterID), + "ManagedZoneRef": HaveField("Name", "mz-example-com"), + "Endpoints": ConsistOf( + PointTo(MatchFields(IgnoreExtras, Fields{ + "DNSName": Equal(clusterOneIDHash + "-" + gwHash + ".klb.test.example.com"), + "Targets": ConsistOf(TestIPAddressOne), + "RecordType": Equal("A"), + "SetIdentifier": Equal(""), + "RecordTTL": Equal(externaldns.TTL(60)), + })), + PointTo(MatchFields(IgnoreExtras, Fields{ + "DNSName": Equal("default.klb.test.example.com"), + "Targets": ConsistOf(clusterOneIDHash + "-" + gwHash + ".klb.test.example.com"), + "RecordType": Equal("CNAME"), + "SetIdentifier": Equal(clusterOneIDHash + "-" + gwHash + ".klb.test.example.com"), + "RecordTTL": Equal(externaldns.TTL(60)), + "ProviderSpecific": Equal(externaldns.ProviderSpecific{{Name: "weight", Value: "120"}}), + })), + PointTo(MatchFields(IgnoreExtras, Fields{ + "DNSName": Equal("default.klb.test.example.com"), + "Targets": ConsistOf(clusterTwoIDHash + "-" + gwHash + ".klb.test.example.com"), + "RecordType": Equal("CNAME"), + "SetIdentifier": Equal(clusterTwoIDHash + "-" + gwHash + ".klb.test.example.com"), + "RecordTTL": Equal(externaldns.TTL(60)), + "ProviderSpecific": Equal(externaldns.ProviderSpecific{{Name: "weight", Value: "120"}}), + })), + PointTo(MatchFields(IgnoreExtras, Fields{ + "DNSName": Equal(clusterTwoIDHash + "-" + gwHash + ".klb.test.example.com"), + "Targets": ConsistOf(TestIPAddressTwo), + "RecordType": Equal("A"), + "SetIdentifier": Equal(""), + "RecordTTL": Equal(externaldns.TTL(60)), + })), + PointTo(MatchFields(IgnoreExtras, Fields{ + "DNSName": Equal(TestHostOne), + "Targets": ConsistOf("klb.test.example.com"), + "RecordType": Equal("CNAME"), + "SetIdentifier": Equal(""), + "RecordTTL": Equal(externaldns.TTL(300)), + })), + ), + }), + }), + ) + + g.Expect(*wildcardDnsRecord).To( + MatchFields(IgnoreExtras, Fields{ + "ObjectMeta": HaveField("Name", wildcardRecordName), + "Spec": MatchFields(IgnoreExtras, Fields{ + "OwnerID": Equal(&clusterID), + "ManagedZoneRef": HaveField("Name", "mz-example-com"), + "Endpoints": ConsistOf( + PointTo(MatchFields(IgnoreExtras, Fields{ + "DNSName": Equal(clusterOneIDHash + "-" + gwHash + ".klb.example.com"), + "Targets": ConsistOf(TestIPAddressOne), + "RecordType": Equal("A"), + "SetIdentifier": Equal(""), + "RecordTTL": Equal(externaldns.TTL(60)), + })), + PointTo(MatchFields(IgnoreExtras, Fields{ + "DNSName": Equal("default.klb.example.com"), + "Targets": ConsistOf(clusterOneIDHash + "-" + gwHash + ".klb.example.com"), + "RecordType": Equal("CNAME"), + "SetIdentifier": Equal(clusterOneIDHash + "-" + gwHash + ".klb.example.com"), + "RecordTTL": Equal(externaldns.TTL(60)), + "ProviderSpecific": Equal(externaldns.ProviderSpecific{{Name: "weight", Value: "120"}}), + })), + PointTo(MatchFields(IgnoreExtras, Fields{ + "DNSName": Equal("default.klb.example.com"), + "Targets": ConsistOf(clusterTwoIDHash + "-" + gwHash + ".klb.example.com"), + "RecordType": Equal("CNAME"), + "SetIdentifier": Equal(clusterTwoIDHash + "-" + gwHash + ".klb.example.com"), + "RecordTTL": Equal(externaldns.TTL(60)), + "ProviderSpecific": Equal(externaldns.ProviderSpecific{{Name: "weight", Value: "120"}}), + })), + PointTo(MatchFields(IgnoreExtras, Fields{ + "DNSName": Equal(clusterTwoIDHash + "-" + gwHash + ".klb.example.com"), + "Targets": ConsistOf(TestIPAddressTwo), + "RecordType": Equal("A"), + "SetIdentifier": Equal(""), + "RecordTTL": Equal(externaldns.TTL(60)), + })), + PointTo(MatchFields(IgnoreExtras, Fields{ + "DNSName": Equal(TestHostWildcard), + "Targets": ConsistOf("klb.example.com"), + "RecordType": Equal("CNAME"), + "SetIdentifier": Equal(""), + "RecordTTL": Equal(externaldns.TTL(300)), + })), + ), + }), + }), + ) + }, TestTimeoutMedium, TestRetryIntervalMedium, ctx).Should(Succeed()) + }) + }) + }) }) diff --git a/controllers/dnspolicy_controller_single_cluster_test.go b/controllers/dnspolicy_controller_single_cluster_test.go index ea7d1ac08..d06607392 100644 --- a/controllers/dnspolicy_controller_single_cluster_test.go +++ b/controllers/dnspolicy_controller_single_cluster_test.go @@ -212,14 +212,6 @@ var _ = Describe("DNSPolicy Single Cluster", func() { "RecordTTL": Equal(externaldns.TTL(60)), "ProviderSpecific": Equal(externaldns.ProviderSpecific{{Name: "weight", Value: "120"}}), })), - PointTo(MatchFields(IgnoreExtras, Fields{ - "DNSName": Equal("klb.test.example.com"), - "Targets": ConsistOf("default." + "klb.test.example.com"), - "RecordType": Equal("CNAME"), - "SetIdentifier": Equal("default"), - "RecordTTL": Equal(externaldns.TTL(300)), - "ProviderSpecific": Equal(externaldns.ProviderSpecific{{Name: "geo-code", Value: "*"}}), - })), PointTo(MatchFields(IgnoreExtras, Fields{ "DNSName": Equal(TestHostOne), "Targets": ConsistOf("klb.test.example.com"), @@ -253,14 +245,6 @@ var _ = Describe("DNSPolicy Single Cluster", func() { "RecordTTL": Equal(externaldns.TTL(60)), "ProviderSpecific": Equal(externaldns.ProviderSpecific{{Name: "weight", Value: "120"}}), })), - PointTo(MatchFields(IgnoreExtras, Fields{ - "DNSName": Equal("klb.example.com"), - "Targets": ConsistOf("default." + "klb.example.com"), - "RecordType": Equal("CNAME"), - "SetIdentifier": Equal("default"), - "RecordTTL": Equal(externaldns.TTL(300)), - "ProviderSpecific": Equal(externaldns.ProviderSpecific{{Name: "geo-code", Value: "*"}}), - })), PointTo(MatchFields(IgnoreExtras, Fields{ "DNSName": Equal(TestHostWildcard), "Targets": ConsistOf("klb.example.com"), diff --git a/pkg/common/common.go b/pkg/common/common.go index be182b43e..9b33a90b9 100644 --- a/pkg/common/common.go +++ b/pkg/common/common.go @@ -23,6 +23,9 @@ import ( "github.com/martinlindhe/base36" "sigs.k8s.io/controller-runtime/pkg/client" + gatewayapiv1 "sigs.k8s.io/gateway-api/apis/v1" + + "github.com/kuadrant/kuadrant-operator/pkg/library/utils" ) // TODO: move the const to a proper place, or get it from config