Address review comments

This commit is contained in:
Alfred Krohmer 2021-04-05 21:27:26 +02:00
parent 4345ce6a31
commit 0d4cf9915c
3 changed files with 47 additions and 6 deletions

View File

@ -25,6 +25,12 @@ import (
/** test utility functions for endpoints verifications */
type byNames endpoint.ProviderSpecific
func (p byNames) Len() int { return len(p) }
func (p byNames) Swap(i, j int) { p[i], p[j] = p[j], p[i] }
func (p byNames) Less(i, j int) bool { return p[i].Name < p[j].Name }
type byAllFields []*endpoint.Endpoint
func (b byAllFields) Len() int { return len(b) }
@ -102,5 +108,9 @@ func SamePlanChanges(a, b map[string][]*endpoint.Endpoint) bool {
// SameProviderSpecific verifies that two maps contain the same string/string key/value pairs
func SameProviderSpecific(a, b endpoint.ProviderSpecific) bool {
return reflect.DeepEqual(a, b)
sa := a
sb := b
sort.Sort(byNames(sa))
sort.Sort(byNames(sb))
return reflect.DeepEqual(sa, sb)
}

View File

@ -402,8 +402,6 @@ func (p *AWSProvider) DeleteRecords(ctx context.Context, endpoints []*endpoint.E
func (p *AWSProvider) doRecords(ctx context.Context, action string, endpoints []*endpoint.Endpoint) error {
zones, err := p.Zones(ctx)
p.AdjustEndpoints(endpoints)
if err != nil {
return errors.Wrapf(err, "failed to list zones, aborting %s doRecords action", action)
}
@ -412,6 +410,9 @@ func (p *AWSProvider) doRecords(ctx context.Context, action string, endpoints []
if err != nil {
log.Errorf("failed to list records while preparing %s doRecords action: %s", action, err)
}
p.AdjustEndpoints(endpoints)
return p.submitChanges(ctx, p.newChanges(action, endpoints, records, zones), zones)
}
@ -561,11 +562,16 @@ func (p *AWSProvider) newChanges(action string, endpoints []*endpoint.Endpoint,
return changes
}
// AdjustEndpoints modifies the provided endpoints (coming from various sources) to match
// the endpoints that the provider returns in `Records` so that the change plan will not have
// unneeded (potentially failing) changes.
// Example: CNAME endpoints pointing to ELBs will have a `alias` provider-specific property
// added to match the endpoints generated from existing alias records in Route53.
func (p *AWSProvider) AdjustEndpoints(endpoints []*endpoint.Endpoint) []*endpoint.Endpoint {
for _, ep := range endpoints {
alias := false
if _, ok := ep.GetProviderSpecificProperty(providerSpecificAlias); ok {
alias = true
if aliasString, ok := ep.GetProviderSpecificProperty(providerSpecificAlias); ok {
alias = aliasString.Value == "true"
} else if useAlias(ep, p.preferCNAME) {
alias = true
log.Debugf("Modifying endpoint: %v, setting %s=true", ep, providerSpecificAlias)
@ -843,7 +849,8 @@ func useAlias(ep *endpoint.Endpoint, preferCNAME bool) bool {
return false
}
// isAWSAlias determines if a given hostname is an AWS Alias record
// isAWSAlias determines if a given endpoint is supposed to create an AWS Alias record
// and (if so) returns the target hosted zone ID
func isAWSAlias(ep *endpoint.Endpoint) string {
prop, exists := ep.GetProviderSpecificProperty(providerSpecificAlias)
if exists && prop.Value == "true" && ep.RecordType == endpoint.RecordTypeCNAME && len(ep.Targets) > 0 {

View File

@ -354,6 +354,30 @@ func TestAWSRecords(t *testing.T) {
})
}
func TestAWSAdjustEndpoints(t *testing.T) {
provider, _ := newAWSProvider(t, endpoint.NewDomainFilter([]string{"ext-dns-test-2.teapot.zalan.do."}), provider.NewZoneIDFilter([]string{}), provider.NewZoneTypeFilter(""), defaultEvaluateTargetHealth, false, []*endpoint.Endpoint{})
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-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"),
endpoint.NewEndpoint("cname-test-elb.zone-2.ext-dns-test-2.teapot.zalan.do", endpoint.RecordTypeCNAME, "foo.eu-central-1.elb.amazonaws.com"),
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
}
provider.AdjustEndpoints(records)
validateEndpoints(t, 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-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"),
endpoint.NewEndpoint("cname-test-elb-no-eth.ext-dns-test-2.teapot.zalan.do", endpoint.RecordTypeCNAME, "foo.eu-central-1.elb.amazonaws.com").WithProviderSpecific(providerSpecificAlias, "true").WithProviderSpecific(providerSpecificEvaluateTargetHealth, "false"), // eth = evaluate target health
})
}
func TestAWSCreateRecords(t *testing.T) {
customTTL := endpoint.TTL(60)
provider, _ := newAWSProvider(t, endpoint.NewDomainFilter([]string{"ext-dns-test-2.teapot.zalan.do."}), provider.NewZoneIDFilter([]string{}), provider.NewZoneTypeFilter(""), defaultEvaluateTargetHealth, false, []*endpoint.Endpoint{})