Skip to content

Commit

Permalink
refactor(provider/aws): Refactor createUpdateChanges() with helper re…
Browse files Browse the repository at this point in the history 
…quiresDeleteCreate() to see if change is UPSERT capable.
  • Loading branch information
@jessegonzalez
jessegonzalez committed Jan 10, 2023
1 parent b747cc8 commit 569571a
Show file tree
Hide file tree
Showing 2 changed files with 42 additions and 6 deletions.
40 changes: 34 additions & 6 deletions provider/aws/aws.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -435,19 +435,47 @@ func (p *AWSProvider) UpdateRecords(ctx context.Context, updates, current []*end
return p.submitChanges(ctx, p.createUpdateChanges(updates, current), zones)
}

// Identify if old and new endpoints require DELETE/CREATE instead of UPDATE.
func (p *AWSProvider) requiresDeleteCreate(old *endpoint.Endpoint, new *endpoint.Endpoint) bool {

// a change of record type
if old.RecordType != new.RecordType {
return true
}

// an ALIAS record change to/from a CNAME
if old.RecordType == endpoint.RecordTypeCNAME && useAlias(old, p.preferCNAME) != useAlias(new, p.preferCNAME) {
return true
}

// a change of routing policy
// default to true for geolocation properties if any geolocation property exists in old/new but not the other
for _, propType := range [7]string{providerSpecificWeight, providerSpecificRegion, providerSpecificFailover,
providerSpecificFailover, providerSpecificGeolocationContinentCode, providerSpecificGeolocationCountryCode,
providerSpecificGeolocationSubdivisionCode} {
_, oldPolicy := old.GetProviderSpecificProperty(propType)
_, newPolicy := old.GetProviderSpecificProperty(propType)
if oldPolicy != newPolicy {
return true
}
}

// a set identifier change
if old.SetIdentifier != new.SetIdentifier {
return true
}

return false
}

func (p *AWSProvider) createUpdateChanges(newEndpoints, oldEndpoints []*endpoint.Endpoint) []*route53.Change {
var deletes []*endpoint.Endpoint
var creates []*endpoint.Endpoint
var updates []*endpoint.Endpoint

for i, new := range newEndpoints {
old := oldEndpoints[i]
if new.RecordType != old.RecordType ||
// Handle the case where an AWS ALIAS record is changing to/from a CNAME.
(old.RecordType == endpoint.RecordTypeCNAME && useAlias(old, p.preferCNAME) != useAlias(new, p.preferCNAME)) ||
// Handle the case where an AWS record is changing to/from simple to other routing policies
((old.SetIdentifier == "" && new.SetIdentifier != "") || (old.SetIdentifier != "" && new.SetIdentifier == "")) {
// The record type changed or the routing policy change, so UPSERT will fail. Instead perform a DELETE followed by a CREATE.
if p.requiresDeleteCreate(old, new) {
deletes = append(deletes, old)
creates = append(creates, new)
} else {
Expand Down
8 changes: 8 additions & 0 deletions provider/aws/aws_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -529,6 +529,8 @@ func TestAWSApplyChanges(t *testing.T) {
endpoint.NewEndpointWithTTL("weighted-to-simple.zone-1.ext-dns-test-2.teapot.zalan.do", endpoint.RecordTypeA, endpoint.TTL(recordTTL), "1.2.3.4").WithSetIdentifier("weighted-to-simple").WithProviderSpecific(providerSpecificWeight, "10"),
endpoint.NewEndpointWithTTL("simple-to-weighted.zone-1.ext-dns-test-2.teapot.zalan.do", endpoint.RecordTypeA, endpoint.TTL(recordTTL), "1.2.3.4"),
endpoint.NewEndpointWithTTL("policy-change.zone-1.ext-dns-test-2.teapot.zalan.do", endpoint.RecordTypeA, endpoint.TTL(recordTTL), "1.2.3.4").WithSetIdentifier("policy-change").WithProviderSpecific(providerSpecificWeight, "10"),
endpoint.NewEndpointWithTTL("set-identifier-change.zone-1.ext-dns-test-2.teapot.zalan.do", endpoint.RecordTypeA, endpoint.TTL(recordTTL), "1.2.3.4").WithSetIdentifier("before").WithProviderSpecific(providerSpecificWeight, "10"),
endpoint.NewEndpointWithTTL("set-identifier-no-change.zone-1.ext-dns-test-2.teapot.zalan.do", endpoint.RecordTypeA, endpoint.TTL(recordTTL), "1.2.3.4").WithSetIdentifier("no-change").WithProviderSpecific(providerSpecificWeight, "10"),
})

createRecords := []*endpoint.Endpoint{
Expand All @@ -550,6 +552,8 @@ func TestAWSApplyChanges(t *testing.T) {
endpoint.NewEndpoint("weighted-to-simple.zone-1.ext-dns-test-2.teapot.zalan.do", endpoint.RecordTypeA, "1.2.3.4").WithSetIdentifier("weighted-to-simple").WithProviderSpecific(providerSpecificWeight, "10"),
endpoint.NewEndpoint("simple-to-weighted.zone-1.ext-dns-test-2.teapot.zalan.do", endpoint.RecordTypeA, "1.2.3.4"),
endpoint.NewEndpoint("policy-change.zone-1.ext-dns-test-2.teapot.zalan.do", endpoint.RecordTypeA, "1.2.3.4").WithSetIdentifier("policy-change").WithProviderSpecific(providerSpecificWeight, "10"),
endpoint.NewEndpoint("set-identifier-change.zone-1.ext-dns-test-2.teapot.zalan.do", endpoint.RecordTypeA, "1.2.3.4").WithSetIdentifier("before").WithProviderSpecific(providerSpecificWeight, "10"),
endpoint.NewEndpoint("set-identifier-no-change.zone-1.ext-dns-test-2.teapot.zalan.do", endpoint.RecordTypeA, "1.2.3.4").WithSetIdentifier("no-change").WithProviderSpecific(providerSpecificWeight, "10"),
}
updatedRecords := []*endpoint.Endpoint{
endpoint.NewEndpoint("update-test.zone-1.ext-dns-test-2.teapot.zalan.do", endpoint.RecordTypeA, "1.2.3.4"),
Expand All @@ -562,6 +566,8 @@ func TestAWSApplyChanges(t *testing.T) {
endpoint.NewEndpoint("weighted-to-simple.zone-1.ext-dns-test-2.teapot.zalan.do", endpoint.RecordTypeA, "1.2.3.4"),
endpoint.NewEndpoint("simple-to-weighted.zone-1.ext-dns-test-2.teapot.zalan.do", endpoint.RecordTypeA, "1.2.3.4").WithSetIdentifier("simple-to-weighted").WithProviderSpecific(providerSpecificWeight, "10"),
endpoint.NewEndpoint("policy-change.zone-1.ext-dns-test-2.teapot.zalan.do", endpoint.RecordTypeA, "1.2.3.4").WithSetIdentifier("policy-change").WithProviderSpecific(providerSpecificRegion, "us-east-1"),
endpoint.NewEndpoint("set-identifier-change.zone-1.ext-dns-test-2.teapot.zalan.do", endpoint.RecordTypeA, "1.2.3.4").WithSetIdentifier("after").WithProviderSpecific(providerSpecificWeight, "10"),
endpoint.NewEndpoint("set-identifier-no-change.zone-1.ext-dns-test-2.teapot.zalan.do", endpoint.RecordTypeA, "1.2.3.4").WithSetIdentifier("no-change").WithProviderSpecific(providerSpecificWeight, "10"),
}

deleteRecords := []*endpoint.Endpoint{
Expand Down Expand Up @@ -608,6 +614,8 @@ func TestAWSApplyChanges(t *testing.T) {
endpoint.NewEndpointWithTTL("weighted-to-simple.zone-1.ext-dns-test-2.teapot.zalan.do", endpoint.RecordTypeA, endpoint.TTL(recordTTL), "1.2.3.4"),
endpoint.NewEndpointWithTTL("simple-to-weighted.zone-1.ext-dns-test-2.teapot.zalan.do", endpoint.RecordTypeA, endpoint.TTL(recordTTL), "1.2.3.4").WithSetIdentifier("simple-to-weighted").WithProviderSpecific(providerSpecificWeight, "10"),
endpoint.NewEndpointWithTTL("policy-change.zone-1.ext-dns-test-2.teapot.zalan.do", endpoint.RecordTypeA, endpoint.TTL(recordTTL), "1.2.3.4").WithSetIdentifier("policy-change").WithProviderSpecific(providerSpecificRegion, "us-east-1"),
endpoint.NewEndpointWithTTL("set-identifier-change.zone-1.ext-dns-test-2.teapot.zalan.do", endpoint.RecordTypeA, endpoint.TTL(recordTTL), "1.2.3.4").WithSetIdentifier("after").WithProviderSpecific(providerSpecificWeight, "10"),
endpoint.NewEndpointWithTTL("set-identifier-no-change.zone-1.ext-dns-test-2.teapot.zalan.do", endpoint.RecordTypeA, endpoint.TTL(recordTTL), "1.2.3.4").WithSetIdentifier("no-change").WithProviderSpecific(providerSpecificWeight, "10"),
})
}
}
Expand Down

0 comments on commit 569571a

Please sign in to comment.