Skip to content

Commit

Permalink
Route53 update configuration of target health checks
Browse files Browse the repository at this point in the history
  • Loading branch information
@johngmyers
johngmyers committed Jun 10, 2023
1 parent 4b15f20 commit 55bbb29
Show file tree
Hide file tree
Showing 4 changed files with 83 additions and 89 deletions.
30 changes: 25 additions & 5 deletions endpoint/endpoint.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -222,11 +222,7 @@ func (e *Endpoint) WithSetIdentifier(setIdentifier string) *Endpoint {
// warrant its own field on the Endpoint object itself. It differs from Labels in the fact that it's
// not persisted in the Registry but only kept in memory during a single record synchronization.
func (e *Endpoint) WithProviderSpecific(key, value string) *Endpoint {
if e.ProviderSpecific == nil {
e.ProviderSpecific = ProviderSpecific{}
}

e.ProviderSpecific = append(e.ProviderSpecific, ProviderSpecificProperty{Name: key, Value: value})
e.SetProviderSpecificProperty(key, value)
return e
}

Expand All @@ -240,6 +236,30 @@ func (e *Endpoint) GetProviderSpecificProperty(key string) (string, bool) {
return "", false
}

// SetProviderSpecificProperty sets the value of a ProviderSpecificProperty.
func (e *Endpoint) SetProviderSpecificProperty(key string, value string) {
for i, providerSpecific := range e.ProviderSpecific {
if providerSpecific.Name == key {
e.ProviderSpecific[i] = ProviderSpecificProperty{
Name: key,
Value: value,
}
return
}
}

e.ProviderSpecific = append(e.ProviderSpecific, ProviderSpecificProperty{Name: key, Value: value})
}

func (e *Endpoint) DeleteProviderSpecificProperty(key string) {
for i, providerSpecific := range e.ProviderSpecific {
if providerSpecific.Name == key {
e.ProviderSpecific = append(e.ProviderSpecific[:i], e.ProviderSpecific[i+1:]...)
return
}
}
}

func (e *Endpoint) String() string {
return fmt.Sprintf("%s %d IN %s %s %s %s", e.DNSName, e.RecordTTL, e.RecordType, e.SetIdentifier, e.Targets, e.ProviderSpecific)
}
Expand Down
65 changes: 37 additions & 28 deletions provider/aws/aws.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -47,10 +47,12 @@ const (
// As we are using the standard AWS client, this should already be compliant.
// Hence, ifever AWS decides to raise this limit, we will automatically reduce the pressure on rate limits
route53PageSize = "300"
// provider specific key that designates whether an AWS ALIAS record has the EvaluateTargetHealth
// field set to true.
providerSpecificAlias = "alias"
providerSpecificTargetHostedZone = "aws/target-hosted-zone"
// providerSpecificAlias specifies whether a CNAME endpoint maps to an AWS ALIAS record.
providerSpecificAlias = "alias"
providerSpecificTargetHostedZone = "aws/target-hosted-zone"
// 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"
Expand Down Expand Up @@ -283,13 +285,6 @@ func NewAWSProvider(awsConfig AWSConfig) (*AWSProvider, error) {
return provider, nil
}

func (p *AWSProvider) PropertyValuesEqual(name string, previous string, current string) bool {
if name == "aws/evaluate-target-health" {
return true
}
return p.BaseProvider.PropertyValuesEqual(name, previous, current)
}

// Zones returns the list of hosted zones.
func (p *AWSProvider) Zones(ctx context.Context) (map[string]*route53.HostedZone, error) {
if p.zonesCache.zones != nil && time.Since(p.zonesCache.age) < p.zonesCache.duration {
Expand Down Expand Up @@ -390,7 +385,11 @@ func (p *AWSProvider) records(ctx context.Context, zones map[string]*route53.Hos
targets[idx] = aws.StringValue(rr.Value)
}

newEndpoints = append(newEndpoints, endpoint.NewEndpointWithTTL(wildcardUnescape(aws.StringValue(r.Name)), aws.StringValue(r.Type), ttl, targets...))
ep := endpoint.NewEndpointWithTTL(wildcardUnescape(aws.StringValue(r.Name)), aws.StringValue(r.Type), ttl, targets...)
if aws.StringValue(r.Type) == endpoint.RecordTypeCNAME {
ep = ep.WithProviderSpecific(providerSpecificAlias, "false")
}
newEndpoints = append(newEndpoints, ep)
}

if r.AliasTarget != nil {
Expand Down Expand Up @@ -466,8 +465,12 @@ func (p *AWSProvider) requiresDeleteCreate(old *endpoint.Endpoint, new *endpoint
}

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

// a set identifier change
Expand Down Expand Up @@ -667,23 +670,29 @@ func (p *AWSProvider) newChanges(action string, endpoints []*endpoint.Endpoint)
func (p *AWSProvider) AdjustEndpoints(endpoints []*endpoint.Endpoint) []*endpoint.Endpoint {
for _, ep := range endpoints {
alias := false
if aliasString, ok := ep.GetProviderSpecificProperty(providerSpecificAlias); ok {
if ep.RecordType != endpoint.RecordTypeCNAME {
ep.DeleteProviderSpecificProperty(providerSpecificAlias)
} else if aliasString, ok := ep.GetProviderSpecificProperty(providerSpecificAlias); ok {
alias = aliasString == "true"
} else if useAlias(ep, p.preferCNAME) {
alias = true
log.Debugf("Modifying endpoint: %v, setting %s=true", ep, providerSpecificAlias)
ep.ProviderSpecific = append(ep.ProviderSpecific, endpoint.ProviderSpecificProperty{
Name: providerSpecificAlias,
Value: "true",
})
if !alias && aliasString != "false" {
ep.SetProviderSpecificProperty(providerSpecificAlias, "false")
}
} else {
alias = useAlias(ep, p.preferCNAME)
log.Debugf("Modifying endpoint: %v, setting %s=%v", ep, providerSpecificAlias, alias)
ep.SetProviderSpecificProperty(providerSpecificAlias, strconv.FormatBool(alias))
}

if _, ok := ep.GetProviderSpecificProperty(providerSpecificEvaluateTargetHealth); alias && !ok {
log.Debugf("Modifying endpoint: %v, setting %s=%t", ep, providerSpecificEvaluateTargetHealth, p.evaluateTargetHealth)
ep.ProviderSpecific = append(ep.ProviderSpecific, endpoint.ProviderSpecificProperty{
Name: providerSpecificEvaluateTargetHealth,
Value: fmt.Sprintf("%t", p.evaluateTargetHealth),
})
if alias {
if prop, ok := ep.GetProviderSpecificProperty(providerSpecificEvaluateTargetHealth); ok {
if prop != "true" && prop != "false" {
ep.SetProviderSpecificProperty(providerSpecificEvaluateTargetHealth, "false")
}
} else {
ep.SetProviderSpecificProperty(providerSpecificEvaluateTargetHealth, strconv.FormatBool(p.evaluateTargetHealth))
}
} else {
ep.DeleteProviderSpecificProperty(providerSpecificEvaluateTargetHealth)
}
}
return endpoints
Expand Down
70 changes: 14 additions & 56 deletions provider/aws/aws_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -494,7 +494,7 @@ func TestAWSRecords(t *testing.T) {
records, err := provider.Records(context.Background())
require.NoError(t, err)

validateEndpoints(t, records, []*endpoint.Endpoint{
validateEndpoints(t, provider, records, []*endpoint.Endpoint{
endpoint.NewEndpointWithTTL("list-test.zone-1.ext-dns-test-2.teapot.zalan.do", endpoint.RecordTypeA, endpoint.TTL(recordTTL), "1.2.3.4"),
endpoint.NewEndpointWithTTL("list-test.zone-2.ext-dns-test-2.teapot.zalan.do", endpoint.RecordTypeA, endpoint.TTL(recordTTL), "8.8.8.8"),
endpoint.NewEndpointWithTTL("*.wildcard-test.zone-2.ext-dns-test-2.teapot.zalan.do", endpoint.RecordTypeA, endpoint.TTL(recordTTL), "8.8.8.8"),
Expand All @@ -511,7 +511,7 @@ func TestAWSRecords(t *testing.T) {
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(providerSpecificGeolocationContinentCode, "EU"),
endpoint.NewEndpointWithTTL("geolocation-test.zone-1.ext-dns-test-2.teapot.zalan.do", endpoint.RecordTypeA, endpoint.TTL(recordTTL), "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(recordTTL), "1.2.3.4").WithSetIdentifier("test-set-1").WithProviderSpecific(providerSpecificGeolocationSubdivisionCode, "NY"),
endpoint.NewEndpointWithTTL("healthcheck-test.zone-1.ext-dns-test-2.teapot.zalan.do", endpoint.RecordTypeCNAME, endpoint.TTL(recordTTL), "foo.example.com").WithSetIdentifier("test-set-1").WithProviderSpecific(providerSpecificWeight, "10").WithProviderSpecific(providerSpecificHealthCheckID, "foo-bar-healthcheck-id"),
endpoint.NewEndpointWithTTL("healthcheck-test.zone-1.ext-dns-test-2.teapot.zalan.do", endpoint.RecordTypeCNAME, endpoint.TTL(recordTTL), "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(recordTTL), "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(recordTTL), "10 mailhost1.example.com", "20 mailhost2.example.com"),
})
Expand All @@ -529,11 +529,11 @@ func TestAWSAdjustEndpoints(t *testing.T) {
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
}

provider.AdjustEndpoints(records)
records = provider.AdjustEndpoints(records)

validateEndpoints(t, records, []*endpoint.Endpoint{
validateEndpoints(t, provider, records, []*endpoint.Endpoint{
endpoint.NewEndpoint("a-test.zone-1.ext-dns-test-2.teapot.zalan.do", endpoint.RecordTypeA, "8.8.8.8"),
endpoint.NewEndpoint("cname-test.zone-1.ext-dns-test-2.teapot.zalan.do", endpoint.RecordTypeCNAME, "foo.example.com"),
endpoint.NewEndpoint("cname-test.zone-1.ext-dns-test-2.teapot.zalan.do", endpoint.RecordTypeCNAME, "foo.example.com").WithProviderSpecific(providerSpecificAlias, "false"),
endpoint.NewEndpoint("cname-test-alias.zone-1.ext-dns-test-2.teapot.zalan.do", endpoint.RecordTypeCNAME, "alias-target.zone-2.ext-dns-test-2.teapot.zalan.do").WithProviderSpecific(providerSpecificAlias, "true").WithProviderSpecific(providerSpecificEvaluateTargetHealth, "true"),
endpoint.NewEndpoint("cname-test-elb.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("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"),
Expand Down Expand Up @@ -1426,7 +1426,7 @@ func TestAWSsubmitChanges(t *testing.T) {
records, err := provider.Records(ctx)
require.NoError(t, err)

validateEndpoints(t, records, endpoints)
validateEndpoints(t, provider, records, endpoints)
}

func TestAWSsubmitChangesError(t *testing.T) {
Expand Down Expand Up @@ -1607,8 +1607,11 @@ func TestAWSBatchChangeSetExceedingNameChange(t *testing.T) {
require.Equal(t, 0, len(batchCs))
}

func validateEndpoints(t *testing.T, endpoints []*endpoint.Endpoint, expected []*endpoint.Endpoint) {
func validateEndpoints(t *testing.T, provider *AWSProvider, endpoints []*endpoint.Endpoint, expected []*endpoint.Endpoint) {
assert.True(t, testutils.SameEndpoints(endpoints, expected), "actual and expected endpoints don't match. %+v:%+v", endpoints, expected)

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

func validateAWSZones(t *testing.T, zones map[string]*route53.HostedZone, expected map[string]*route53.HostedZone) {
Expand Down Expand Up @@ -1840,51 +1843,6 @@ func TestAWSSuitableZones(t *testing.T) {
}
}

func TestAWSHealthTargetAnnotation(tt *testing.T) {
comparator := func(name, previous, current string) bool {
return previous == current
}
for _, test := range []struct {
name string
current *endpoint.Endpoint
desired *endpoint.Endpoint
propertyComparator func(name, previous, current string) bool
shouldUpdate bool
}{
{
name: "skip AWS target health",
current: &endpoint.Endpoint{
RecordType: "A",
DNSName: "foo.com",
ProviderSpecific: []endpoint.ProviderSpecificProperty{
{Name: "aws/evaluate-target-health", Value: "true"},
},
},
desired: &endpoint.Endpoint{
DNSName: "foo.com",
RecordType: "A",
ProviderSpecific: []endpoint.ProviderSpecificProperty{
{Name: "aws/evaluate-target-health", Value: "false"},
},
},
propertyComparator: comparator,
shouldUpdate: false,
},
} {
tt.Run(test.name, func(t *testing.T) {
provider := &AWSProvider{}
plan := &plan.Plan{
Current: []*endpoint.Endpoint{test.current},
Desired: []*endpoint.Endpoint{test.desired},
PropertyComparator: provider.PropertyValuesEqual,
ManagedRecords: []string{endpoint.RecordTypeA, endpoint.RecordTypeCNAME},
}
plan = plan.Calculate()
assert.Equal(t, test.shouldUpdate, len(plan.Changes.UpdateNew) == 1)
})
}
}

func createAWSZone(t *testing.T, provider *AWSProvider, zone *route53.HostedZone) {
params := &route53.CreateHostedZoneInput{
CallerReference: aws.String("external-dns.alpha.kubernetes.io/test-zone"),
Expand All @@ -1908,7 +1866,7 @@ func setAWSRecords(t *testing.T, provider *AWSProvider, records []*route53.Resou
endpoints, err := provider.Records(ctx)
require.NoError(t, err)

validateEndpoints(t, endpoints, []*endpoint.Endpoint{})
validateEndpoints(t, provider, endpoints, []*endpoint.Endpoint{})

var changes Route53Changes
for _, record := range records {
Expand Down Expand Up @@ -2071,13 +2029,13 @@ func TestRequiresDeleteCreate(t *testing.T) {
provider, _ := newAWSProvider(t, endpoint.NewDomainFilter([]string{"foo.bar."}), provider.NewZoneIDFilter([]string{}), provider.NewZoneTypeFilter(""), defaultEvaluateTargetHealth, false, nil)

oldRecordType := endpoint.NewEndpointWithTTL("recordType", endpoint.RecordTypeA, endpoint.TTL(recordTTL), "8.8.8.8")
newRecordType := endpoint.NewEndpointWithTTL("recordType", endpoint.RecordTypeCNAME, endpoint.TTL(recordTTL), "bar")
newRecordType := endpoint.NewEndpointWithTTL("recordType", endpoint.RecordTypeCNAME, endpoint.TTL(recordTTL), "bar").WithProviderSpecific(providerSpecificAlias, "false")

assert.False(t, provider.requiresDeleteCreate(oldRecordType, oldRecordType), "actual and expected endpoints don't match. %+v:%+v", oldRecordType, oldRecordType)
assert.True(t, provider.requiresDeleteCreate(oldRecordType, newRecordType), "actual and expected endpoints don't match. %+v:%+v", oldRecordType, newRecordType)

oldCNAMEAlias := endpoint.NewEndpointWithTTL("CNAMEAlias", endpoint.RecordTypeCNAME, endpoint.TTL(recordTTL), "bar")
newCNAMEAlias := endpoint.NewEndpointWithTTL("CNAMEAlias", endpoint.RecordTypeCNAME, endpoint.TTL(recordTTL), "bar.us-east-1.elb.amazonaws.com")
oldCNAMEAlias := endpoint.NewEndpointWithTTL("CNAMEAlias", endpoint.RecordTypeCNAME, endpoint.TTL(recordTTL), "bar").WithProviderSpecific(providerSpecificAlias, "false")
newCNAMEAlias := endpoint.NewEndpointWithTTL("CNAMEAlias", endpoint.RecordTypeCNAME, endpoint.TTL(recordTTL), "bar.us-east-1.elb.amazonaws.com").WithProviderSpecific(providerSpecificAlias, "true")

assert.False(t, provider.requiresDeleteCreate(oldCNAMEAlias, oldCNAMEAlias), "actual and expected endpoints don't match. %+v:%+v", oldCNAMEAlias, oldCNAMEAlias.DNSName)
assert.True(t, provider.requiresDeleteCreate(oldCNAMEAlias, newCNAMEAlias), "actual and expected endpoints don't match. %+v:%+v", oldCNAMEAlias, newCNAMEAlias)
Expand Down
7 changes: 7 additions & 0 deletions provider/provider.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,13 @@ type Provider interface {
Records(ctx context.Context) ([]*endpoint.Endpoint, error)
ApplyChanges(ctx context.Context, changes *plan.Changes) error
PropertyValuesEqual(name string, previous string, current string) bool
// AdjustEndpoints canonicalizes a set of candidate endpoints.
// It is called with a set of candidate endpoints obtained from the various sources.
// It returns a set modified as required by the provider. The provider is responsible for
// adding, removing, and modifying the ProviderSpecific properties to match
// the endpoints that the provider returns in `Records` so that the change plan will not have
// unnecessary (potentially failing) changes. It may also modify other fields, add, or remove
// Endpoints. It is permitted to modify the supplied endpoints.
AdjustEndpoints(endpoints []*endpoint.Endpoint) []*endpoint.Endpoint
GetDomainFilter() endpoint.DomainFilterInterface
}
Expand Down

0 comments on commit 55bbb29

Please sign in to comment.