From 61e37112c689788edb0066f23cafb5f0144870db Mon Sep 17 00:00:00 2001 From: craig Date: Mon, 21 Oct 2024 11:46:10 +0100 Subject: [PATCH] add code to handle GEO- prefix for conintents for AWS Signed-off-by: craig rh-pre-commit.version: 2.2.0 rh-pre-commit.check-secrets: ENABLED change geo codes in test --- internal/provider/aws/aws.go | 21 +++- internal/provider/aws/aws_test.go | 111 +++++++++++++++++++++ internal/provider/{iso3166.go => codes.go} | 24 +++++ test/e2e/multi_record_test.go | 2 +- test/e2e/provider_errors_test.go | 2 +- test/e2e/single_record_test.go | 2 +- test/e2e/suite_test.go | 2 +- 7 files changed, 157 insertions(+), 7 deletions(-) create mode 100644 internal/provider/aws/aws_test.go rename internal/provider/{iso3166.go => codes.go} (81%) diff --git a/internal/provider/aws/aws.go b/internal/provider/aws/aws.go index 2d3141aa..f99ab8ac 100644 --- a/internal/provider/aws/aws.go +++ b/internal/provider/aws/aws.go @@ -49,6 +49,7 @@ const ( awsEvaluateTargetHealth = false awsPreferCNAME = true awsZoneCacheDuration = 0 * time.Second + providerContinentPrefix = "GEO-" ) type Route53DNSProvider struct { @@ -122,20 +123,34 @@ func (p *Route53DNSProvider) AdjustEndpoints(endpoints []*externaldnsendpoint.En if err != nil { return nil, err } - + p.logger.V(1).Info("adjusting aws endpoints") for _, ep := range endpoints { if prop, ok := ep.GetProviderSpecificProperty(v1alpha1.ProviderSpecificWeight); ok { ep.DeleteProviderSpecificProperty(v1alpha1.ProviderSpecificWeight) ep.WithProviderSpecific(providerSpecificWeight, prop) + p.logger.V(1).Info("set provider specific weight", "endpoint", ep) } if prop, ok := ep.GetProviderSpecificProperty(v1alpha1.ProviderSpecificGeoCode); ok { ep.DeleteProviderSpecificProperty(v1alpha1.ProviderSpecificGeoCode) + prop = strings.ToUpper(prop) + if strings.HasPrefix(prop, providerContinentPrefix) { + continent := strings.Replace(prop, providerContinentPrefix, "", -1) + if !provider.IsContinentCode(continent) { + return nil, fmt.Errorf("unexpected continent code. %s", continent) + } + ep.WithProviderSpecific(providerSpecificGeolocationContinentCode, continent) + p.logger.V(1).Info("set provider specific continent code base GEO- prefix", "endpoint", ep) + continue + } + if provider.IsISO3166Alpha2Code(prop) || prop == "*" { ep.WithProviderSpecific(providerSpecificGeolocationCountryCode, prop) - } else { - ep.WithProviderSpecific(providerSpecificGeolocationContinentCode, prop) + p.logger.V(1).Info("set provider specific country code", "endpoint", ep) + continue } + //if we get to here there is a value we cannot use + return nil, fmt.Errorf("unexpected geo code. Prefix with %s for continents or use ISO_3166 Alpha 2 supported code for countries", providerContinentPrefix) } } return endpoints, nil diff --git a/internal/provider/aws/aws_test.go b/internal/provider/aws/aws_test.go new file mode 100644 index 00000000..d412768d --- /dev/null +++ b/internal/provider/aws/aws_test.go @@ -0,0 +1,111 @@ +package aws + +import ( + "testing" + + "sigs.k8s.io/external-dns/endpoint" + externaldnsendpoint "sigs.k8s.io/external-dns/endpoint" + + "github.com/kuadrant/dns-operator/api/v1alpha1" +) + +const recordTTL = 300 + +func TestAWSAdjustEndpoints(t *testing.T) { + testCases := []struct { + Name string + Endpoints []*externaldnsendpoint.Endpoint + Validate func(t *testing.T, eps []*externaldnsendpoint.Endpoint, err error) + }{ + { + Name: "test geo prefix continent code endpoint success", + Endpoints: []*externaldnsendpoint.Endpoint{ + endpoint.NewEndpointWithTTL("geolocation-test.zone-1.ext-dns-test-2.teapot.zalan.do", endpoint.RecordTypeA, endpoint.TTL(recordTTL), "1.2.3.4").WithSetIdentifier("test-set-1").WithProviderSpecific(v1alpha1.ProviderSpecificGeoCode, "GEO-EU"), + }, + Validate: func(t *testing.T, eps []*externaldnsendpoint.Endpoint, err error) { + if err != nil { + t.Fatalf("did not expect an error but got %s", err) + } + if len(eps) != 1 { + t.Fatalf("expected 1 endpoint but got %v", len(eps)) + val, ok := eps[0].GetProviderSpecificProperty(providerSpecificGeolocationContinentCode) + if !ok { + t.Fatalf("exected a provider specific contintinent code to be set but got none") + } + if val != "EU" { + t.Fatalf("continent code set but expected the EU got %s ", val) + } + } + + }, + }, + { + Name: "test none geo prefixed continent value and none ISO_3166 to return error", + Endpoints: []*externaldnsendpoint.Endpoint{ + endpoint.NewEndpointWithTTL("geolocation-test.zone-1.ext-dns-test-2.teapot.zalan.do", endpoint.RecordTypeA, endpoint.TTL(recordTTL), "1.2.3.4").WithSetIdentifier("test-set-1").WithProviderSpecific(v1alpha1.ProviderSpecificGeoCode, "EU"), + }, + Validate: func(t *testing.T, eps []*externaldnsendpoint.Endpoint, err error) { + if err == nil { + t.Fatalf("expected an error but got none") + } + if len(eps) != 0 { + t.Fatalf("expected no endpoints but got %v", len(eps)) + } + + }, + }, + { + Name: "test valid ISO_3166 success for country code", + Validate: func(t *testing.T, eps []*externaldnsendpoint.Endpoint, err error) { + if err != nil { + t.Fatalf("did not expect an error but got %s", err) + } + if len(eps) != 1 { + t.Fatalf("expected 1 endpoint but got %v", len(eps)) + } + val, ok := eps[0].GetProviderSpecificProperty(providerSpecificGeolocationCountryCode) + if !ok { + t.Fatalf("exected a provider specific country code to be set but got none") + } + if val != "IE" { + t.Fatalf("continent code set but expected the IE got %s ", val) + } + }, + Endpoints: []*externaldnsendpoint.Endpoint{ + endpoint.NewEndpointWithTTL("geolocation-test.zone-1.ext-dns-test-2.teapot.zalan.do", endpoint.RecordTypeA, endpoint.TTL(recordTTL), "1.2.3.4").WithSetIdentifier("test-set-1").WithProviderSpecific(v1alpha1.ProviderSpecificGeoCode, "IE"), + }, + }, + { + Name: "test geo prefix lower case continent code endpoint success", + Endpoints: []*externaldnsendpoint.Endpoint{ + endpoint.NewEndpointWithTTL("geolocation-test.zone-1.ext-dns-test-2.teapot.zalan.do", endpoint.RecordTypeA, endpoint.TTL(recordTTL), "1.2.3.4").WithSetIdentifier("test-set-1").WithProviderSpecific(v1alpha1.ProviderSpecificGeoCode, "geo-eu"), + }, + Validate: func(t *testing.T, eps []*externaldnsendpoint.Endpoint, err error) { + if err != nil { + t.Fatalf("did not expect an error but got %s", err) + } + + if len(eps) != 1 { + t.Fatalf("expected 1 endpoint but got %v", len(eps)) + val, ok := eps[0].GetProviderSpecificProperty(providerSpecificGeolocationContinentCode) + if !ok { + t.Fatalf("exected a provider specific contintinent code to be set but got none") + } + if val != "EU" { + t.Fatalf("continent code set but expected the EU got %s ", val) + } + } + + }, + }, + } + + for _, testCase := range testCases { + t.Run(testCase.Name, func(t *testing.T) { + r53Prov := &Route53DNSProvider{} + adjusted, err := r53Prov.AdjustEndpoints(testCase.Endpoints) + testCase.Validate(t, adjusted, err) + + }) + } +} diff --git a/internal/provider/iso3166.go b/internal/provider/codes.go similarity index 81% rename from internal/provider/iso3166.go rename to internal/provider/codes.go index c70696b1..32181873 100644 --- a/internal/provider/iso3166.go +++ b/internal/provider/codes.go @@ -270,3 +270,27 @@ func GetISO3166Alpha2Codes() []string { func IsISO3166Alpha2Code(code string) bool { return slice.ContainsString(GetISO3166Alpha2Codes(), code) } + +const ( + AWS_CONTINENT_CODE_AFRICA = "AF" + AWS_CONTINENT_CODE_ANTARTICA = "AN" + AWS_CONTINENT_CODE_ASIA = "AS" + AWS_CONTINENT_CODE_EUROPE = "EU" + AWS_CONTINENT_CODE_OCEANIA = "OC" + AWS_CONTINENT_CODE_NORTH_AMERICA = "NA" + AWS_CONTINENT_CODE_SOUTH_AMERICA = "SA" +) + +var AWSContinentCodes = []string{ + AWS_CONTINENT_CODE_AFRICA, + AWS_CONTINENT_CODE_ANTARTICA, + AWS_CONTINENT_CODE_ASIA, + AWS_CONTINENT_CODE_EUROPE, + AWS_CONTINENT_CODE_OCEANIA, + AWS_CONTINENT_CODE_NORTH_AMERICA, + AWS_CONTINENT_CODE_SOUTH_AMERICA, +} + +func IsContinentCode(code string) bool { + return slice.ContainsString(AWSContinentCodes, code) +} diff --git a/test/e2e/multi_record_test.go b/test/e2e/multi_record_test.go index a9964b76..e3478f04 100644 --- a/test/e2e/multi_record_test.go +++ b/test/e2e/multi_record_test.go @@ -57,7 +57,7 @@ var _ = Describe("Multi Record Test", Labels{"multi_record"}, func() { weighted = "Weighted" } else { geoCode1 = "US" - geoCode2 = "EU" + geoCode2 = "GEO-EU" weighted = "weighted" } }) diff --git a/test/e2e/provider_errors_test.go b/test/e2e/provider_errors_test.go index 0ef5bb4a..0d267e0e 100644 --- a/test/e2e/provider_errors_test.go +++ b/test/e2e/provider_errors_test.go @@ -127,7 +127,7 @@ var _ = Describe("DNSRecord Provider Errors", Labels{"provider_errors"}, func() validGeoCode = "GEO-NA" } else { //AWS - expectedProviderErr = "Value 'notageocode' with length = '11' is not facet-valid with respect to length '2' for type 'ContinentCode'" + expectedProviderErr = "unexpected geo code. Prefix with GEO- for continents or use ISO_3166 Alpha 2 supported code for countries" validGeoCode = "US" } diff --git a/test/e2e/single_record_test.go b/test/e2e/single_record_test.go index d34d29f2..07e66b12 100644 --- a/test/e2e/single_record_test.go +++ b/test/e2e/single_record_test.go @@ -646,7 +646,7 @@ var _ = Describe("Single Record Test", Labels{"single_record"}, func() { } else if testDNSProvider == "azure" { SetTestEnv("testGeoCode", "GEO-EU") } else { - SetTestEnv("testGeoCode", "EU") + SetTestEnv("testGeoCode", "GEO-EU") } SetTestEnv("TEST_DNS_PROVIDER_SECRET_NAME", testDNSProviderSecret.Name) diff --git a/test/e2e/suite_test.go b/test/e2e/suite_test.go index fe9a3b24..11c1053e 100644 --- a/test/e2e/suite_test.go +++ b/test/e2e/suite_test.go @@ -125,7 +125,7 @@ var _ = BeforeSuite(func(ctx SpecContext) { testSuiteID = "dns-op-e2e-" + GenerateName() - geoCode := "EU" + geoCode := "GEO-EU" if testDNSProvider == "google" { geoCode = "europe-west1" }