From cdc62f9efe07d9f13313a7bd7f96a353afda40da Mon Sep 17 00:00:00 2001 From: Dinar Valeev Date: Wed, 23 Mar 2022 18:37:15 +0100 Subject: [PATCH] Name servers should belong to zone they serve There is a best practice, glue record should belong to zone it serves. And records should be the same in dnsZone and edgeDNS edgeDNS: - test.example.com 30 IN NS ns1.test.example.com - ns1.test.example.com 30 IN A 1.1.1.1 k8gb-coredns: - test.example.com 30 IN NS ns1.test.example.com - ns1.test.example.com 30 IN A 1.1.1.1 This commit changes the way NS and Glue records are named in edgeDNS Signed-off-by: Dinar Valeev --- controllers/depresolver/depresolver_config.go | 5 +- controllers/depresolver/depresolver_test.go | 41 ++++++----- controllers/gslb_controller_test.go | 8 +-- controllers/internal/utils/fakedns_test.go | 6 +- controllers/providers/dns/external_test.go | 8 +-- controllers/providers/dns/infoblox_test.go | 72 +++++++++---------- 6 files changed, 71 insertions(+), 69 deletions(-) diff --git a/controllers/depresolver/depresolver_config.go b/controllers/depresolver/depresolver_config.go index a11d99954..751dfc394 100644 --- a/controllers/depresolver/depresolver_config.go +++ b/controllers/depresolver/depresolver_config.go @@ -391,7 +391,7 @@ func (c *Config) GetClusterHeartbeatFQDN(gslbName string) string { // DNS_ZONE k8gb-test.gslb.cloud.example.com // EDGE_DNS_ZONE: cloud.example.com // CLUSTER_GEOTAG: us -// will generate "gslb-ns-us-k8gb-test-gslb.cloud.example.com" +// will generate "gslb-ns-us-k8gb-test-gslb.k8gb-test.gslb.cloud.example.com" // If edgeDNSServer == localhost or 127.0.0.1 than edgeDNSServer is returned. // The function is private and expects only valid inputs. func getNsName(tag, dnsZone, edgeDNSZone, edgeDNSServer string) string { @@ -400,8 +400,7 @@ func getNsName(tag, dnsZone, edgeDNSZone, edgeDNSServer string) string { } const prefix = "gslb-ns" d := strings.TrimSuffix(dnsZone, "."+edgeDNSZone) - domainX := strings.ReplaceAll(d, ".", "-") - return fmt.Sprintf("%s-%s-%s.%s", prefix, tag, domainX, edgeDNSZone) + return fmt.Sprintf("%s-%s.%s.%s", prefix, tag, d, edgeDNSZone) } // getHeartbeatFQDN returns heartbeat for geo tag. diff --git a/controllers/depresolver/depresolver_test.go b/controllers/depresolver/depresolver_test.go index 91ba481b1..d4514d2b0 100644 --- a/controllers/depresolver/depresolver_test.go +++ b/controllers/depresolver/depresolver_test.go @@ -1166,9 +1166,9 @@ func TestNsServerNamesWithMultipleExtClusterGeoTag(t *testing.T) { // assert assert.NoError(t, err) assert.Len(t, config.GetExternalClusterNSNames(), 2) - assert.Equal(t, "gslb-ns-us-west-1-k8gb-test-preprod-gslb.cloud.example.com", config.GetClusterNSName()) - for k, v := range map[string]string{defaultClusterGeoTagUs2: "gslb-ns-us-east-1-k8gb-test-preprod-gslb.cloud.example.com", - defaultClusterGeoTagEu: "gslb-ns-eu-central-1-k8gb-test-preprod-gslb.cloud.example.com"} { + assert.Equal(t, "gslb-ns-us-west-1.k8gb-test-preprod.gslb.cloud.example.com", config.GetClusterNSName()) + for k, v := range map[string]string{defaultClusterGeoTagUs2: "gslb-ns-us-east-1.k8gb-test-preprod.gslb.cloud.example.com", + defaultClusterGeoTagEu: "gslb-ns-eu-central-1.k8gb-test-preprod.gslb.cloud.example.com"} { assert.Equal(t, config.GetExternalClusterNSNames()[k], v) } } @@ -1212,8 +1212,8 @@ func TestNsServerNamesWithOneExtClusterGeoTag(t *testing.T) { // assert assert.NoError(t, err) assert.Len(t, config.GetExternalClusterNSNames(), 1) - assert.Equal(t, "gslb-ns-us-west-1-k8gb-test-preprod-gslb.cloud.example.com", config.GetClusterNSName()) - assert.Equal(t, config.GetExternalClusterNSNames()["location-2"], "gslb-ns-location-2-k8gb-test-preprod-gslb.cloud.example.com") + assert.Equal(t, "gslb-ns-us-west-1.k8gb-test-preprod.gslb.cloud.example.com", config.GetClusterNSName()) + assert.Equal(t, config.GetExternalClusterNSNames()["location-2"], "gslb-ns-location-2.k8gb-test-preprod.gslb.cloud.example.com") } func TestNsServerNamesWithExtClusterGeoTagsContainingClusterGeoTag(t *testing.T) { @@ -1233,15 +1233,15 @@ func TestNsServerNamesWithExtClusterGeoTagsContainingClusterGeoTag(t *testing.T) // assert assert.NoError(t, err) assert.Len(t, config.GetExternalClusterNSNames(), 1) - assert.Equal(t, "gslb-ns-us-west-1-k8gb-test-preprod-gslb.cloud.example.com", config.GetClusterNSName()) - assert.Equal(t, config.GetExternalClusterNSNames()["location-2"], "gslb-ns-location-2-k8gb-test-preprod-gslb.cloud.example.com") + assert.Equal(t, "gslb-ns-us-west-1.k8gb-test-preprod.gslb.cloud.example.com", config.GetClusterNSName()) + assert.Equal(t, config.GetExternalClusterNSNames()["location-2"], "gslb-ns-location-2.k8gb-test-preprod.gslb.cloud.example.com") } func TestNsServerNamesLargeDNSZone(t *testing.T) { defer cleanup() // arrange DNSZone exceeds customConfig := predefinedConfig - customConfig.DNSZone = "k8gb-test-preprod-lorem-ipsum-donor-blah-blah-blah.gslb.cloud.example.com" + customConfig.DNSZone = "k8gb-test-preprod-lorem-ipsum-donor-blah-blah-blah-blah-blah-blah.gslb.cloud.example.com" customConfig.EdgeDNSZone = defaultEdgeDNSZone customConfig.ClusterGeoTag = "us" configureEnvVar(customConfig) @@ -1252,15 +1252,15 @@ func TestNsServerNamesLargeDNSZone(t *testing.T) { // assert assert.Error(t, err) - assert.Equal(t, "gslb-ns-us-k8gb-test-preprod-lorem-ipsum-donor-blah-blah-blah-gslb.cloud.example.com", config.GetClusterNSName()) + assert.Equal(t, "gslb-ns-us.k8gb-test-preprod-lorem-ipsum-donor-blah-blah-blah-blah-blah-blah.gslb.cloud.example.com", config.GetClusterNSName()) extNsNames := config.GetExternalClusterNSNames() - expectedExtNsNames := map[string]string{"za": "gslb-ns-za-k8gb-test-preprod-lorem-ipsum-donor-blah-blah-blah-gslb.cloud.example.com", - "eu": "gslb-ns-eu-k8gb-test-preprod-lorem-ipsum-donor-blah-blah-blah-gslb.cloud.example.com"} + expectedExtNsNames := map[string]string{"za": "gslb-ns-za.k8gb-test-preprod-lorem-ipsum-donor-blah-blah-blah-blah-blah-blah.gslb.cloud.example.com", + "eu": "gslb-ns-eu.k8gb-test-preprod-lorem-ipsum-donor-blah-blah-blah-blah-blah-blah.gslb.cloud.example.com"} assert.True(t, reflect.DeepEqual(extNsNames, expectedExtNsNames), "maps must be equal: \n %v\n %v", extNsNames, expectedExtNsNames) } func TestNsServerNamesWithLargeExtClusterGeoTag(t *testing.T) { - const largeGeoTag = "za-lorem-ipsum-donor-b-blah-lorem" + const largeGeoTag = "za-lorem-ipsum-donor-b-blah-lorem-blah-blah-blah-blah-blah" defer cleanup() // arrange customConfig := predefinedConfig @@ -1278,10 +1278,11 @@ func TestNsServerNamesWithLargeExtClusterGeoTag(t *testing.T) { // assert assert.Error(t, err) - assert.Equal(t, "gslb-ns-us-k8gb-test-preprod-gslb.cloud.example.com", config.GetClusterNSName()) + assert.Equal(t, "gslb-ns-us.k8gb-test-preprod.gslb.cloud.example.com", config.GetClusterNSName()) extNsNames := config.GetExternalClusterNSNames() - expectedExtNsNames := map[string]string{largeGeoTag: "gslb-ns-za-lorem-ipsum-donor-b-blah-lorem-k8gb-test-preprod-gslb.cloud.example.com", - "eu": "gslb-ns-eu-k8gb-test-preprod-gslb.cloud.example.com"} + expectedExtNsNames := map[string]string{ + largeGeoTag: "gslb-ns-za-lorem-ipsum-donor-b-blah-lorem-blah-blah-blah-blah-blah.k8gb-test-preprod.gslb.cloud.example.com", + "eu": "gslb-ns-eu.k8gb-test-preprod.gslb.cloud.example.com"} assert.True(t, reflect.DeepEqual(extNsNames, expectedExtNsNames), "maps must be equal: \n %v\n %v", extNsNames, expectedExtNsNames) } @@ -1292,7 +1293,7 @@ func TestNsServerNamesWithLargeClusterGeoTag(t *testing.T) { customConfig := predefinedConfig customConfig.DNSZone = defaultDNSZone customConfig.EdgeDNSZone = defaultEdgeDNSZone - customConfig.ClusterGeoTag = "us-lorem-ipsum-donor-blah-blah-blah-blah" + customConfig.ClusterGeoTag = "us-lorem-ipsum-donor-blah-blah-blah-blah-blah-blah-blah-blah" configureEnvVar(customConfig) resolver := NewDependencyResolver() @@ -1301,10 +1302,12 @@ func TestNsServerNamesWithLargeClusterGeoTag(t *testing.T) { // assert assert.Error(t, err) - assert.Equal(t, "gslb-ns-us-lorem-ipsum-donor-blah-blah-blah-blah-k8gb-test-preprod-gslb.cloud.example.com", config.GetClusterNSName()) + assert.Equal(t, + "gslb-ns-us-lorem-ipsum-donor-blah-blah-blah-blah-blah-blah-blah-blah.k8gb-test-preprod.gslb.cloud.example.com", + config.GetClusterNSName()) extNsNames := config.GetExternalClusterNSNames() - expectedExtNsNames := map[string]string{"za": "gslb-ns-za-k8gb-test-preprod-gslb.cloud.example.com", - "eu": "gslb-ns-eu-k8gb-test-preprod-gslb.cloud.example.com"} + expectedExtNsNames := map[string]string{"za": "gslb-ns-za.k8gb-test-preprod.gslb.cloud.example.com", + "eu": "gslb-ns-eu.k8gb-test-preprod.gslb.cloud.example.com"} assert.True(t, reflect.DeepEqual(extNsNames, expectedExtNsNames), "maps must be equal: \n %v\n %v", extNsNames, expectedExtNsNames) } diff --git a/controllers/gslb_controller_test.go b/controllers/gslb_controller_test.go index 14c8b4dde..1d08d35bf 100644 --- a/controllers/gslb_controller_test.go +++ b/controllers/gslb_controller_test.go @@ -786,13 +786,13 @@ func TestCreatesDNSNSRecordsForExtDNS(t *testing.T) { RecordTTL: 30, RecordType: "NS", Targets: externaldns.Targets{ - "gslb-ns-eu-cloud.example.com", - "gslb-ns-us-cloud.example.com", - "gslb-ns-za-cloud.example.com", + "gslb-ns-eu.cloud.example.com", + "gslb-ns-us.cloud.example.com", + "gslb-ns-za.cloud.example.com", }, }, { - DNSName: "gslb-ns-eu-cloud.example.com", + DNSName: "gslb-ns-eu.cloud.example.com", RecordTTL: 30, RecordType: "A", Targets: externaldns.Targets{ diff --git a/controllers/internal/utils/fakedns_test.go b/controllers/internal/utils/fakedns_test.go index 6b1de694b..4a416a0c7 100644 --- a/controllers/internal/utils/fakedns_test.go +++ b/controllers/internal/utils/fakedns_test.go @@ -89,9 +89,9 @@ func TestFakeDNSMultipleTXTRecords(t *testing.T) { func TestFakeDNSBasic(t *testing.T) { NewFakeDNS(testSettings). - AddNSRecord("blah.cloud.example.com.", "gslb-ns-us-cloud.example.com."). - AddNSRecord("blah.cloud.example.com.", "gslb-ns-uk-cloud.example.com."). - AddNSRecord("blah.cloud.example.com.", "gslb-ns-eu-cloud.example.com."). + AddNSRecord("blah.cloud.example.com.", "gslb-ns-us.cloud.example.com."). + AddNSRecord("blah.cloud.example.com.", "gslb-ns-uk.cloud.example.com."). + AddNSRecord("blah.cloud.example.com.", "gslb-ns-eu.cloud.example.com."). AddTXTRecord("First", "Second", "Banana"). AddTXTRecord("White", "Red", "Purple"). AddARecord("ip.blah.cloud.example.com.", net.IPv4(10, 0, 1, 5)). diff --git a/controllers/providers/dns/external_test.go b/controllers/providers/dns/external_test.go index 8ee33bd3a..57ecc73bd 100644 --- a/controllers/providers/dns/external_test.go +++ b/controllers/providers/dns/external_test.go @@ -75,9 +75,9 @@ var a = struct { "10.0.1.39", }, TargetNSNamesSorted: []string{ - "gslb-ns-eu-cloud.example.com", - "gslb-ns-us-cloud.example.com", - "gslb-ns-za-cloud.example.com", + "gslb-ns-eu.cloud.example.com", + "gslb-ns-us.cloud.example.com", + "gslb-ns-za.cloud.example.com", }, } @@ -96,7 +96,7 @@ var expectedDNSEndpoint = &externaldns.DNSEndpoint{ Targets: a.TargetNSNamesSorted, }, { - DNSName: "gslb-ns-us-cloud.example.com", + DNSName: "gslb-ns-us.cloud.example.com", RecordTTL: 30, RecordType: "A", Targets: a.TargetIPs, diff --git a/controllers/providers/dns/infoblox_test.go b/controllers/providers/dns/infoblox_test.go index 6810796ee..fb47a526b 100644 --- a/controllers/providers/dns/infoblox_test.go +++ b/controllers/providers/dns/infoblox_test.go @@ -75,17 +75,17 @@ var ( func TestCanFilterOutDelegatedZoneEntryAccordingFQDNProvided(t *testing.T) { // arrange delegateTo := []ibclient.NameServer{ - {Address: "10.0.0.1", Name: "gslb-ns-eu-cloud.example.com"}, - {Address: "10.0.0.2", Name: "gslb-ns-eu-cloud.example.com"}, - {Address: "10.0.0.3", Name: "gslb-ns-eu-cloud.example.com"}, - {Address: "10.1.0.1", Name: "gslb-ns-za-cloud.example.com"}, - {Address: "10.1.0.2", Name: "gslb-ns-za-cloud.example.com"}, - {Address: "10.1.0.3", Name: "gslb-ns-za-cloud.example.com"}, + {Address: "10.0.0.1", Name: "gslb-ns-eu.cloud.example.com"}, + {Address: "10.0.0.2", Name: "gslb-ns-eu.cloud.example.com"}, + {Address: "10.0.0.3", Name: "gslb-ns-eu.cloud.example.com"}, + {Address: "10.1.0.1", Name: "gslb-ns-za.cloud.example.com"}, + {Address: "10.1.0.2", Name: "gslb-ns-za.cloud.example.com"}, + {Address: "10.1.0.3", Name: "gslb-ns-za.cloud.example.com"}, } want := []ibclient.NameServer{ - {Address: "10.0.0.1", Name: "gslb-ns-eu-cloud.example.com"}, - {Address: "10.0.0.2", Name: "gslb-ns-eu-cloud.example.com"}, - {Address: "10.0.0.3", Name: "gslb-ns-eu-cloud.example.com"}, + {Address: "10.0.0.1", Name: "gslb-ns-eu.cloud.example.com"}, + {Address: "10.0.0.2", Name: "gslb-ns-eu.cloud.example.com"}, + {Address: "10.0.0.3", Name: "gslb-ns-eu.cloud.example.com"}, } customConfig := defaultConfig customConfig.EdgeDNSZone = "example.com" @@ -104,25 +104,25 @@ func TestCanFilterOutDelegatedZoneEntryAccordingFQDNProvided(t *testing.T) { func TestCanSanitizeDelegatedZone(t *testing.T) { // arrange local := []ibclient.NameServer{ - {Address: "10.0.0.3", Name: "gslb-ns-eu-cloud.example.com"}, - {Address: "10.0.0.1", Name: "gslb-ns-eu-cloud.example.com"}, - {Address: "10.0.0.2", Name: "gslb-ns-eu-cloud.example.com"}, + {Address: "10.0.0.3", Name: "gslb-ns-eu.cloud.example.com"}, + {Address: "10.0.0.1", Name: "gslb-ns-eu.cloud.example.com"}, + {Address: "10.0.0.2", Name: "gslb-ns-eu.cloud.example.com"}, } upstream := []ibclient.NameServer{ - {Address: "10.0.0.3", Name: "gslb-ns-eu-cloud.example.com"}, - {Address: "10.1.0.3", Name: "gslb-ns-za-cloud.example.com"}, - {Address: "10.0.0.1", Name: "gslb-ns-eu-cloud.example.com"}, - {Address: "10.1.0.2", Name: "gslb-ns-za-cloud.example.com"}, - {Address: "10.0.0.2", Name: "gslb-ns-eu-cloud.example.com"}, - {Address: "10.1.0.1", Name: "gslb-ns-za-cloud.example.com"}, + {Address: "10.0.0.3", Name: "gslb-ns-eu.cloud.example.com"}, + {Address: "10.1.0.3", Name: "gslb-ns-za.cloud.example.com"}, + {Address: "10.0.0.1", Name: "gslb-ns-eu.cloud.example.com"}, + {Address: "10.1.0.2", Name: "gslb-ns-za.cloud.example.com"}, + {Address: "10.0.0.2", Name: "gslb-ns-eu.cloud.example.com"}, + {Address: "10.1.0.1", Name: "gslb-ns-za.cloud.example.com"}, } want := []ibclient.NameServer{ - {Address: "10.0.0.1", Name: "gslb-ns-eu-cloud.example.com"}, - {Address: "10.0.0.2", Name: "gslb-ns-eu-cloud.example.com"}, - {Address: "10.0.0.3", Name: "gslb-ns-eu-cloud.example.com"}, - {Address: "10.1.0.1", Name: "gslb-ns-za-cloud.example.com"}, - {Address: "10.1.0.2", Name: "gslb-ns-za-cloud.example.com"}, - {Address: "10.1.0.3", Name: "gslb-ns-za-cloud.example.com"}, + {Address: "10.0.0.1", Name: "gslb-ns-eu.cloud.example.com"}, + {Address: "10.0.0.2", Name: "gslb-ns-eu.cloud.example.com"}, + {Address: "10.0.0.3", Name: "gslb-ns-eu.cloud.example.com"}, + {Address: "10.1.0.1", Name: "gslb-ns-za.cloud.example.com"}, + {Address: "10.1.0.2", Name: "gslb-ns-za.cloud.example.com"}, + {Address: "10.1.0.3", Name: "gslb-ns-za.cloud.example.com"}, } customConfig := defaultConfig customConfig.EdgeDNSZone = "example.com" @@ -141,20 +141,20 @@ func TestCanSanitizeDelegatedZone(t *testing.T) { func TestSortNameServer(t *testing.T) { delegateTo := []ibclient.NameServer{ - {Address: "10.0.0.3", Name: "gslb-ns-eu-cloud.example.com"}, - {Address: "10.1.0.3", Name: "gslb-ns-za-cloud.example.com"}, - {Address: "10.0.0.1", Name: "gslb-ns-eu-cloud.example.com"}, - {Address: "10.1.0.2", Name: "gslb-ns-za-cloud.example.com"}, - {Address: "10.0.0.2", Name: "gslb-ns-eu-cloud.example.com"}, - {Address: "10.1.0.1", Name: "gslb-ns-za-cloud.example.com"}, + {Address: "10.0.0.3", Name: "gslb-ns-eu.cloud.example.com"}, + {Address: "10.1.0.3", Name: "gslb-ns-za.cloud.example.com"}, + {Address: "10.0.0.1", Name: "gslb-ns-eu.cloud.example.com"}, + {Address: "10.1.0.2", Name: "gslb-ns-za.cloud.example.com"}, + {Address: "10.0.0.2", Name: "gslb-ns-eu.cloud.example.com"}, + {Address: "10.1.0.1", Name: "gslb-ns-za.cloud.example.com"}, } want := []ibclient.NameServer{ - {Address: "10.0.0.1", Name: "gslb-ns-eu-cloud.example.com"}, - {Address: "10.0.0.2", Name: "gslb-ns-eu-cloud.example.com"}, - {Address: "10.0.0.3", Name: "gslb-ns-eu-cloud.example.com"}, - {Address: "10.1.0.1", Name: "gslb-ns-za-cloud.example.com"}, - {Address: "10.1.0.2", Name: "gslb-ns-za-cloud.example.com"}, - {Address: "10.1.0.3", Name: "gslb-ns-za-cloud.example.com"}, + {Address: "10.0.0.1", Name: "gslb-ns-eu.cloud.example.com"}, + {Address: "10.0.0.2", Name: "gslb-ns-eu.cloud.example.com"}, + {Address: "10.0.0.3", Name: "gslb-ns-eu.cloud.example.com"}, + {Address: "10.1.0.1", Name: "gslb-ns-za.cloud.example.com"}, + {Address: "10.1.0.2", Name: "gslb-ns-za.cloud.example.com"}, + {Address: "10.1.0.3", Name: "gslb-ns-za.cloud.example.com"}, } sortZones(delegateTo) assert.Equal(t, want, delegateTo, "got:\n %q \n\n want:\n %q", delegateTo, want)