Skip to content

feat(aws): add support for geoproximity routing #5347

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 4 commits into
base: master
Choose a base branch
from
Open

feat(aws): add support for geoproximity routing #5347

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
377f2c8
feat(aws): add support for geoproximity routing
prasadkatti May 1, 2025
1a5b4c9
remove the invalid test
prasadkatti May 1, 2025
901e340
make some changes based on review comments
prasadkatti May 2, 2025
7b7dac7
fix linting errors
prasadkatti May 2, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
97 changes: 87 additions & 10 deletions provider/aws/aws.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -53,16 +53,20 @@ const (
// providerSpecificEvaluateTargetHealth specifies whether an AWS ALIAS record
// has the EvaluateTargetHealth field set to true. Present iff the endpoint
// has a `providerSpecificAlias` value of `true`.
providerSpecificEvaluateTargetHealth = "aws/evaluate-target-health"
providerSpecificWeight = "aws/weight"
providerSpecificRegion = "aws/region"
providerSpecificFailover = "aws/failover"
providerSpecificGeolocationContinentCode = "aws/geolocation-continent-code"
providerSpecificGeolocationCountryCode = "aws/geolocation-country-code"
providerSpecificGeolocationSubdivisionCode = "aws/geolocation-subdivision-code"
providerSpecificMultiValueAnswer = "aws/multi-value-answer"
providerSpecificHealthCheckID = "aws/health-check-id"
sameZoneAlias = "same-zone"
providerSpecificEvaluateTargetHealth = "aws/evaluate-target-health"
providerSpecificWeight = "aws/weight"
providerSpecificRegion = "aws/region"
providerSpecificFailover = "aws/failover"
providerSpecificGeolocationContinentCode = "aws/geolocation-continent-code"
providerSpecificGeolocationCountryCode = "aws/geolocation-country-code"
providerSpecificGeolocationSubdivisionCode = "aws/geolocation-subdivision-code"
providerSpecificGeoProximityLocationAWSRegion = "aws/geoproximitylocation-aws-region"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

aws-region not required, just region

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have tried to match it with the field names in the GeoProximityLocation type

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

aws/geoproximitylocation-aws-region

does not looks right

providerSpecificGeoProximityLocationBias = "aws/geoproximitylocation-bias"
providerSpecificGeoProximityLocationCoordinates = "aws/geoproximitylocation-coordinates"
providerSpecificGeoProximityLocationLocalZoneGroup = "aws/geoproximitylocation-local-zone-group"
providerSpecificMultiValueAnswer = "aws/multi-value-answer"
providerSpecificHealthCheckID = "aws/health-check-id"
sameZoneAlias = "same-zone"
// Currently supported up to 10 health checks or hosted zones.
// https://docs.aws.amazon.com/Route53/latest/APIReference/API_ListTagsForResources.html#API_ListTagsForResources_RequestSyntax
batchSize = 10
Expand Down Expand Up @@ -542,6 +546,8 @@ func (p *AWSProvider) records(ctx context.Context, zones map[string]*profiledZon
ep.WithProviderSpecific(providerSpecificGeolocationSubdivisionCode, *r.GeoLocation.SubdivisionCode)
}
}
case r.GeoProximityLocation != nil:
handleGeoProximityLocationRecord(&r, ep)
default:
// one of the above needs to be set, otherwise SetIdentifier doesn't make sense
}
Expand All @@ -560,6 +566,22 @@ func (p *AWSProvider) records(ctx context.Context, zones map[string]*profiledZon
return endpoints, nil
}

func handleGeoProximityLocationRecord(r *route53types.ResourceRecordSet, ep *endpoint.Endpoint) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Here is suggested improvements

  1. Use Early Returns: If the GeoProximityLocation is nil, return early to avoid unnecessary checks.
  2. Slightly rewrite the logic, to simplify readability.
if r.GeoProximityLocation == nil {
		return
	}

	if region := aws.ToString(r.GeoProximityLocation.AWSRegion); region != "" {
		ep.WithProviderSpecific(providerSpecificGeoProximityLocationAWSRegion, region)
	}

	if bias := r.GeoProximityLocation.Bias; bias != nil {
		ep.WithProviderSpecific(providerSpecificGeoProximityLocationBias, fmt.Sprintf("%d", aws.ToInt32(bias)))
	}

	if coords := r.GeoProximityLocation.Coordinates; coords != nil {
		coordinates := fmt.Sprintf("%s,%s", aws.ToString(coords.Latitude), aws.ToString(coords.Longitude))
		ep.WithProviderSpecific(providerSpecificGeoProximityLocationCoordinates, coordinates)
	}

	if localZoneGroup := aws.ToString(r.GeoProximityLocation.LocalZoneGroup); localZoneGroup != "" {
		ep.WithProviderSpecific(providerSpecificGeoProximityLocationLocalZoneGroup, localZoneGroup)
	}

if r.GeoProximityLocation.AWSRegion != nil {
ep.WithProviderSpecific(providerSpecificGeoProximityLocationAWSRegion, aws.ToString(r.GeoProximityLocation.AWSRegion))
}
if r.GeoProximityLocation.Bias != nil {
ep.WithProviderSpecific(providerSpecificGeoProximityLocationBias, fmt.Sprintf("%d", aws.ToInt32(r.GeoProximityLocation.Bias)))
}
if r.GeoProximityLocation.Coordinates != nil {
coordinates := fmt.Sprintf("%s,%s", aws.ToString(r.GeoProximityLocation.Coordinates.Latitude), aws.ToString(r.GeoProximityLocation.Coordinates.Longitude))
ep.WithProviderSpecific(providerSpecificGeoProximityLocationCoordinates, coordinates)
}
if r.GeoProximityLocation.LocalZoneGroup != nil {
ep.WithProviderSpecific(providerSpecificGeoProximityLocationLocalZoneGroup, aws.ToString(r.GeoProximityLocation.LocalZoneGroup))
}
}

// Identify if old and new endpoints require DELETE/CREATE instead of UPDATE.
func (p *AWSProvider) requiresDeleteCreate(old *endpoint.Endpoint, newE *endpoint.Endpoint) bool {
// a change of a record type
Expand Down Expand Up @@ -832,12 +854,31 @@ func (p *AWSProvider) AdjustEndpoints(endpoints []*endpoint.Endpoint) ([]*endpoi
} else {
ep.DeleteProviderSpecificProperty(providerSpecificEvaluateTargetHealth)
}

adjustGeoProximityLocationEndpoint(ep)
}

endpoints = append(endpoints, aliasCnameAaaaEndpoints...)
return endpoints, nil
}

// if the endpoint is using geoproximity, set the bias to 0 if not set
// this is needed to avoid unnecessary Upserts if the desired endpoint doesn't specify a bias
func adjustGeoProximityLocationEndpoint(ep *endpoint.Endpoint) {
if ep.SetIdentifier != "" {
_, ok1 := ep.GetProviderSpecificProperty(providerSpecificGeoProximityLocationAWSRegion)
_, ok2 := ep.GetProviderSpecificProperty(providerSpecificGeoProximityLocationLocalZoneGroup)
_, ok3 := ep.GetProviderSpecificProperty(providerSpecificGeoProximityLocationCoordinates)

if ok1 || ok2 || ok3 {
// check if ep has bias property and if not, set it to 0
if _, ok := ep.GetProviderSpecificProperty(providerSpecificGeoProximityLocationBias); !ok {
ep.SetProviderSpecificProperty(providerSpecificGeoProximityLocationBias, "0")
}
}
}
}

// newChange returns a route53 Change
// returned Change is based on the given record by the given action, e.g.
// action=ChangeActionCreate returns a change for creation of the record and
Expand Down Expand Up @@ -926,6 +967,8 @@ func (p *AWSProvider) newChange(action route53types.ChangeAction, ep *endpoint.E
if useGeolocation {
change.ResourceRecordSet.GeoLocation = geolocation
}

buildChangeForGeoProximityEndpoint(change, ep)
}

if prop, ok := ep.GetProviderSpecificProperty(providerSpecificHealthCheckID); ok {
Expand All @@ -939,6 +982,40 @@ func (p *AWSProvider) newChange(action route53types.ChangeAction, ep *endpoint.E
return change
}

func buildChangeForGeoProximityEndpoint(change *Route53Change, ep *endpoint.Endpoint) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

all lines need to be covered in tests
Screenshot 2025-05-04 at 14 46 46

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
func buildChangeForGeoProximityEndpoint(change *Route53Change, ep *endpoint.Endpoint) {
func witchChangeForGeoProximityEndpoint(change *Route53Change, ep *endpoint.Endpoint) {

geoproximity := &route53types.GeoProximityLocation{}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This method complexity is quite high, we need to split it most likely, example

func buildChangeForGeoProximityEndpoin(......)
  geoproximity := &route53types.GeoProximityLocation{}
  isGeoproximity := false
  
  isGeoproximity = withGeoProximityAWSRegion(ep, geoproximity) || useGeoproximity
  isGeoproximity = withGeoProximityBias(ep, geoproximity) || useGeoproximity
  isGeoproximity = withGeoProximityCoordinates(ep, geoproximity) || useGeoproximity
  isGeoproximity = withGeoProximityLocalZoneGroup(ep, geoproximity) || useGeoproximity
  
  if isGeoproximity {
    change.ResourceRecordSet.GeoProximityLocation = geoproximity
  }
}


where
func withGeoProximityLocalZoneGroup(ep *endpoint.Endpoint, geoproximity *route53types.GeoProximityLocation) bool {
	if prop, ok := ep.GetProviderSpecificProperty(providerSpecificGeoProximityLocationLocalZoneGroup); ok {
		geoproximity.LocalZoneGroup = aws.String(prop)
		return true
	}
	return false
}

useGeoproximity := false
if prop, ok := ep.GetProviderSpecificProperty(providerSpecificGeoProximityLocationAWSRegion); ok {
geoproximity.AWSRegion = aws.String(prop)
useGeoproximity = true
}
if prop, ok := ep.GetProviderSpecificProperty(providerSpecificGeoProximityLocationBias); ok {
bias, _ := strconv.ParseInt(prop, 10, 32)
geoproximity.Bias = aws.Int32(int32(bias))
}
if prop, ok := ep.GetProviderSpecificProperty(providerSpecificGeoProximityLocationCoordinates); ok {
coordinates := strings.Split(prop, ",")
if len(coordinates) == 2 {
latitude := coordinates[0]
longitude := coordinates[1]
geoproximity.Coordinates = &route53types.Coordinates{
Latitude: aws.String(latitude),
Longitude: aws.String(longitude),
}
Comment on lines +999 to +1004
Copy link
Contributor

@ivankatliarchuk ivankatliarchuk May 4, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would be nice to add some validation here as well

type Coordinates struct {

	//  Specifies a coordinate of the north–south position of a geographic point on
	// the surface of the Earth (-90 - 90).
	//
	// This member is required.
	Latitude *string

	//  Specifies a coordinate of the east–west position of a geographic point on the
	// surface of the Earth (-180 - 180).
	//
	// This member is required.
	Longitude *string
}

Probably something like

// validateCoordinates checks if the given latitude and longitude are valid.
func (c *Coordinates) validate(lat, lon string) error {
	latitude, err := strconv.ParseFloat(lat, 64)
	if err != nil || latitude < -90 || latitude > 90 {
		return errors.New("invalid latitude: must be a number between -90 and 90")
	}

	longitude, err := strconv.ParseFloat(lon, 64)
	if err != nil || longitude < -180 || longitude > 180 {
		return errors.New("invalid longitude: must be a number between -180 and 180")
	}

	return nil
}

// Example usage
func (c *Coordinates) validateAndSet(coordinates string) (*route53types.Coordinates, error) {
	parts := strings.Split(coordinates, ",")
	if len(parts) != 2 {
		return nil, errors.New("invalid coordinates format: expected 'latitude,longitude'")
	}

	lat := strings.TrimSpace(parts[0])
	lon := strings.TrimSpace(parts[1])

	if err := validateCoordinates(lat, lon); err != nil {
		return nil, err
	}

	return &route53types.Coordinates{
		Latitude:  aws.String(lat),
		Longitude: aws.String(lon),
	}, nil
}

useGeoproximity = true
} else {
log.Errorf("Invalid coordinates format for %s: %s; expected format 'latitude,longitude'", providerSpecificGeoProximityLocationCoordinates, prop)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is debatable, but most likely should be log.Warn, to warn user so they need to change configuraiton

}
}
if prop, ok := ep.GetProviderSpecificProperty(providerSpecificGeoProximityLocationLocalZoneGroup); ok {
geoproximity.LocalZoneGroup = aws.String(prop)
useGeoproximity = true
}
if useGeoproximity {
change.ResourceRecordSet.GeoProximityLocation = geoproximity
}
}

// searches for `changes` that are contained in `queue` and returns the `changes` separated by whether they were found in the queue (`foundChanges`) or not (`notFoundChanges`)
func findChangesInQueue(changes Route53Changes, queue Route53Changes) (foundChanges, notFoundChanges Route53Changes) {
if queue == nil {
Expand Down
43 changes: 42 additions & 1 deletion provider/aws/aws_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -583,6 +583,42 @@ func TestAWSRecords(t *testing.T) {
SubdivisionCode: aws.String("NY"),
},
},
{
Name: aws.String("geoproximitylocation-region.zone-1.ext-dns-test-2.teapot.zalan.do."),
Type: route53types.RRTypeA,
TTL: aws.Int64(defaultTTL),
ResourceRecords: []route53types.ResourceRecord{{Value: aws.String("1.2.3.4")}},
SetIdentifier: aws.String("test-set-1"),
GeoProximityLocation: &route53types.GeoProximityLocation{
AWSRegion: aws.String("us-west-2"),
Bias: aws.Int32(10),
},
},
{
Name: aws.String("geoproximitylocation-localzone.zone-1.ext-dns-test-2.teapot.zalan.do."),
Type: route53types.RRTypeA,
TTL: aws.Int64(defaultTTL),
ResourceRecords: []route53types.ResourceRecord{{Value: aws.String("1.2.3.4")}},
SetIdentifier: aws.String("test-set-1"),
GeoProximityLocation: &route53types.GeoProximityLocation{
LocalZoneGroup: aws.String("usw2-pdx1-az1"),
Bias: aws.Int32(10),
},
},
{
Name: aws.String("geoproximitylocation-coordinates.zone-1.ext-dns-test-2.teapot.zalan.do."),
Type: route53types.RRTypeA,
TTL: aws.Int64(defaultTTL),
ResourceRecords: []route53types.ResourceRecord{{Value: aws.String("1.2.3.4")}},
SetIdentifier: aws.String("test-set-1"),
GeoProximityLocation: &route53types.GeoProximityLocation{
Coordinates: &route53types.Coordinates{
Latitude: aws.String("90"),
Longitude: aws.String("90"),
},
Bias: aws.Int32(0),
},
},
{
Name: aws.String("healthcheck-test.zone-1.ext-dns-test-2.teapot.zalan.do."),
Type: route53types.RRTypeCname,
Expand Down Expand Up @@ -636,6 +672,9 @@ func TestAWSRecords(t *testing.T) {
endpoint.NewEndpointWithTTL("geolocation-test.zone-1.ext-dns-test-2.teapot.zalan.do", endpoint.RecordTypeA, endpoint.TTL(defaultTTL), "1.2.3.4").WithSetIdentifier("test-set-1").WithProviderSpecific(providerSpecificGeolocationContinentCode, "EU"),
endpoint.NewEndpointWithTTL("geolocation-test.zone-1.ext-dns-test-2.teapot.zalan.do", endpoint.RecordTypeA, endpoint.TTL(defaultTTL), "4.3.2.1").WithSetIdentifier("test-set-2").WithProviderSpecific(providerSpecificGeolocationCountryCode, "DE"),
endpoint.NewEndpointWithTTL("geolocation-subdivision-test.zone-1.ext-dns-test-2.teapot.zalan.do", endpoint.RecordTypeA, endpoint.TTL(defaultTTL), "1.2.3.4").WithSetIdentifier("test-set-1").WithProviderSpecific(providerSpecificGeolocationSubdivisionCode, "NY"),
endpoint.NewEndpointWithTTL("geoproximitylocation-region.zone-1.ext-dns-test-2.teapot.zalan.do", endpoint.RecordTypeA, endpoint.TTL(defaultTTL), "1.2.3.4").WithSetIdentifier("test-set-1").WithProviderSpecific(providerSpecificGeoProximityLocationAWSRegion, "us-west-2").WithProviderSpecific(providerSpecificGeoProximityLocationBias, "10"),
endpoint.NewEndpointWithTTL("geoproximitylocation-localzone.zone-1.ext-dns-test-2.teapot.zalan.do", endpoint.RecordTypeA, endpoint.TTL(defaultTTL), "1.2.3.4").WithSetIdentifier("test-set-1").WithProviderSpecific(providerSpecificGeoProximityLocationLocalZoneGroup, "usw2-pdx1-az1").WithProviderSpecific(providerSpecificGeoProximityLocationBias, "10"),
endpoint.NewEndpointWithTTL("geoproximitylocation-coordinates.zone-1.ext-dns-test-2.teapot.zalan.do", endpoint.RecordTypeA, endpoint.TTL(defaultTTL), "1.2.3.4").WithSetIdentifier("test-set-1").WithProviderSpecific(providerSpecificGeoProximityLocationCoordinates, "90,90").WithProviderSpecific(providerSpecificGeoProximityLocationBias, "0"),
endpoint.NewEndpointWithTTL("healthcheck-test.zone-1.ext-dns-test-2.teapot.zalan.do", endpoint.RecordTypeCNAME, endpoint.TTL(defaultTTL), "foo.example.com").WithSetIdentifier("test-set-1").WithProviderSpecific(providerSpecificWeight, "10").WithProviderSpecific(providerSpecificHealthCheckID, "foo-bar-healthcheck-id").WithProviderSpecific(providerSpecificAlias, "false"),
endpoint.NewEndpointWithTTL("healthcheck-test.zone-1.ext-dns-test-2.teapot.zalan.do", endpoint.RecordTypeA, endpoint.TTL(defaultTTL), "4.3.2.1").WithSetIdentifier("test-set-2").WithProviderSpecific(providerSpecificWeight, "20").WithProviderSpecific(providerSpecificHealthCheckID, "abc-def-healthcheck-id"),
endpoint.NewEndpointWithTTL("mail.zone-1.ext-dns-test-2.teapot.zalan.do", endpoint.RecordTypeMX, endpoint.TTL(defaultTTL), "10 mailhost1.example.com", "20 mailhost2.example.com"),
Expand Down Expand Up @@ -670,6 +709,7 @@ func TestAWSAdjustEndpoints(t *testing.T) {
endpoint.NewEndpoint("cname-test-elb-no-alias.zone-2.ext-dns-test-2.teapot.zalan.do", endpoint.RecordTypeCNAME, "foo.eu-central-1.elb.amazonaws.com").WithProviderSpecific(providerSpecificAlias, "false"),
endpoint.NewEndpoint("cname-test-elb-no-eth.ext-dns-test-2.teapot.zalan.do", endpoint.RecordTypeCNAME, "foo.eu-central-1.elb.amazonaws.com").WithProviderSpecific(providerSpecificEvaluateTargetHealth, "false"), // eth = evaluate target health
endpoint.NewEndpoint("cname-test-elb-alias.zone-2.ext-dns-test-2.teapot.zalan.do", endpoint.RecordTypeCNAME, "foo.eu-central-1.elb.amazonaws.com").WithProviderSpecific(providerSpecificAlias, "true").WithProviderSpecific(providerSpecificEvaluateTargetHealth, "true"),
endpoint.NewEndpoint("a-test-geoproximity-no-bias.zone-1.ext-dns-test-2.teapot.zalan.do", endpoint.RecordTypeA, "8.8.8.8").WithSetIdentifier("test-set-1").WithProviderSpecific(providerSpecificGeoProximityLocationAWSRegion, "us-west-2"),
}

records, err := provider.AdjustEndpoints(records)
Expand All @@ -687,6 +727,7 @@ func TestAWSAdjustEndpoints(t *testing.T) {
endpoint.NewEndpoint("cname-test-elb-no-eth.ext-dns-test-2.teapot.zalan.do", endpoint.RecordTypeAAAA, "foo.eu-central-1.elb.amazonaws.com").WithProviderSpecific(providerSpecificAlias, "true").WithProviderSpecific(providerSpecificEvaluateTargetHealth, "false"), // eth = evaluate target health
endpoint.NewEndpoint("cname-test-elb-alias.zone-2.ext-dns-test-2.teapot.zalan.do", endpoint.RecordTypeA, "foo.eu-central-1.elb.amazonaws.com").WithProviderSpecific(providerSpecificAlias, "true").WithProviderSpecific(providerSpecificEvaluateTargetHealth, "true"),
endpoint.NewEndpoint("cname-test-elb-alias.zone-2.ext-dns-test-2.teapot.zalan.do", endpoint.RecordTypeAAAA, "foo.eu-central-1.elb.amazonaws.com").WithProviderSpecific(providerSpecificAlias, "true").WithProviderSpecific(providerSpecificEvaluateTargetHealth, "true"),
endpoint.NewEndpoint("a-test-geoproximity-no-bias.zone-1.ext-dns-test-2.teapot.zalan.do", endpoint.RecordTypeA, "8.8.8.8").WithSetIdentifier("test-set-1").WithProviderSpecific(providerSpecificGeoProximityLocationAWSRegion, "us-west-2").WithProviderSpecific(providerSpecificGeoProximityLocationBias, "0"),
})
}

Expand Down Expand Up @@ -1902,7 +1943,7 @@ func validateEndpoints(t *testing.T, provider *AWSProvider, endpoints []*endpoin

normalized, err := provider.AdjustEndpoints(endpoints)
assert.NoError(t, err)
assert.True(t, testutils.SameEndpoints(normalized, expected), "actual and normalized endpoints don't match. %+v:%+v", endpoints, normalized)
assert.True(t, testutils.SameEndpoints(normalized, expected), "normalized and expected endpoints don't match. %+v:%+v", normalized, expected)
}

func validateAWSZones(t *testing.T, zones map[string]*route53types.HostedZone, expected map[string]*route53types.HostedZone) {
Expand Down
Loading