diff --git a/docs/annotations/annotations.md b/docs/annotations/annotations.md index 4275c15686..d58ace08b7 100644 --- a/docs/annotations/annotations.md +++ b/docs/annotations/annotations.md @@ -305,6 +305,9 @@ Additional annotations implemented by specific providers: If the value of this annotation is `true`, specifies that CNAME records generated by the resource should instead be alias records. +This annotation is only supported on A, AAAA, and CNAME record types. Endpoints with other +record types (e.g. MX, SRV, TXT) that have this annotation set will be rejected. + **Supported providers:** - **AWS**: This annotation is only relevant if the `--aws-prefer-cname` flag is specified. diff --git a/endpoint/endpoint.go b/endpoint/endpoint.go index 94bae10f7f..03e5e74b4f 100644 --- a/endpoint/endpoint.go +++ b/endpoint/endpoint.go @@ -455,9 +455,20 @@ func RemoveDuplicates(endpoints []*Endpoint) []*Endpoint { return result } +// TODO: review source/annotations package to consolidate alias key definitions; +// currently duplicated here to avoid circular dependency. +const providerSpecificAlias = "alias" + // TODO: rename to Validate // CheckEndpoint Check if endpoint is properly formatted according to RFC standards func (e *Endpoint) CheckEndpoint() bool { + if !e.supportsAlias() { + if _, ok := e.GetBoolProviderSpecificProperty(providerSpecificAlias); ok { + log.Warnf("Endpoint %s of type %s does not support alias records", e.DNSName, e.RecordType) + return false + } + } + switch recordType := e.RecordType; recordType { case RecordTypeMX: return e.Targets.ValidateMXRecord() @@ -467,6 +478,15 @@ func (e *Endpoint) CheckEndpoint() bool { return true } +func (e *Endpoint) supportsAlias() bool { + switch e.RecordType { + case RecordTypeA, RecordTypeAAAA, RecordTypeCNAME: + return true + default: + return false + } +} + // WithMinTTL sets the endpoint's TTL to the given value if the current TTL is not configured. func (e *Endpoint) WithMinTTL(ttl int64) { if !e.RecordTTL.IsConfigured() && ttl > 0 { diff --git a/endpoint/endpoint_test.go b/endpoint/endpoint_test.go index e946f45d52..2145ab313c 100644 --- a/endpoint/endpoint_test.go +++ b/endpoint/endpoint_test.go @@ -19,8 +19,11 @@ package endpoint import ( "fmt" "reflect" + "strings" "testing" + log "github.com/sirupsen/logrus" + "github.com/sirupsen/logrus/hooks/test" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" "sigs.k8s.io/external-dns/pkg/events" @@ -939,6 +942,95 @@ func TestCheckEndpoint(t *testing.T) { }, expected: true, }, + { + description: "A record with alias=true is valid", + endpoint: Endpoint{ + DNSName: "example.com", + RecordType: RecordTypeA, + Targets: Targets{"my-elb-123.us-east-1.elb.amazonaws.com"}, + ProviderSpecific: ProviderSpecific{{Name: providerSpecificAlias, Value: "true"}}, + }, + expected: true, + }, + { + description: "AAAA record with alias=true is valid", + endpoint: Endpoint{ + DNSName: "example.com", + RecordType: RecordTypeAAAA, + Targets: Targets{"dualstack.my-elb-123.us-east-1.elb.amazonaws.com"}, + ProviderSpecific: ProviderSpecific{{Name: providerSpecificAlias, Value: "true"}}, + }, + expected: true, + }, + { + description: "CNAME record with alias=true is valid", + endpoint: Endpoint{ + DNSName: "example.com", + RecordType: RecordTypeCNAME, + Targets: Targets{"d111111abcdef8.cloudfront.net"}, + ProviderSpecific: ProviderSpecific{{Name: providerSpecificAlias, Value: "true"}}, + }, + expected: true, + }, + { + description: "MX record with alias=true is invalid", + endpoint: Endpoint{ + DNSName: "example.com", + RecordType: RecordTypeMX, + Targets: Targets{"10 mail.example.com"}, + ProviderSpecific: ProviderSpecific{{Name: providerSpecificAlias, Value: "true"}}, + }, + expected: false, + }, + { + description: "TXT record with alias=true is invalid", + endpoint: Endpoint{ + DNSName: "example.com", + RecordType: RecordTypeTXT, + Targets: Targets{"v=spf1 ~all"}, + ProviderSpecific: ProviderSpecific{{Name: providerSpecificAlias, Value: "true"}}, + }, + expected: false, + }, + { + description: "NS record with alias=true is invalid", + endpoint: Endpoint{ + DNSName: "example.com", + RecordType: RecordTypeNS, + Targets: Targets{"ns1.example.com"}, + ProviderSpecific: ProviderSpecific{{Name: providerSpecificAlias, Value: "true"}}, + }, + expected: false, + }, + { + description: "SRV record with alias=true is invalid", + endpoint: Endpoint{ + DNSName: "_sip._tcp.example.com", + RecordType: RecordTypeSRV, + Targets: Targets{"10 5 5060 sip.example.com."}, + ProviderSpecific: ProviderSpecific{{Name: providerSpecificAlias, Value: "true"}}, + }, + expected: false, + }, + { + description: "MX record with alias=false is also invalid", + endpoint: Endpoint{ + DNSName: "example.com", + RecordType: RecordTypeMX, + Targets: Targets{"10 mail.example.com"}, + ProviderSpecific: ProviderSpecific{{Name: providerSpecificAlias, Value: "false"}}, + }, + expected: false, + }, + { + description: "MX record without alias property is valid", + endpoint: Endpoint{ + DNSName: "example.com", + RecordType: RecordTypeMX, + Targets: Targets{"10 mail.example.com"}, + }, + expected: true, + }, } for _, tt := range tests { @@ -949,6 +1041,69 @@ func TestCheckEndpoint(t *testing.T) { } } +func TestCheckEndpoint_AliasWarningLog(t *testing.T) { + tests := []struct { + name string + ep Endpoint + wantLog bool + }{ + { + name: "unsupported type with alias logs warning", + ep: Endpoint{ + DNSName: "example.com", + RecordType: RecordTypeMX, + Targets: Targets{"10 mail.example.com"}, + ProviderSpecific: ProviderSpecific{{Name: providerSpecificAlias, Value: "true"}}, + }, + wantLog: true, + }, + { + name: "supported type with alias does not log", + ep: Endpoint{ + DNSName: "example.com", + RecordType: RecordTypeA, + Targets: Targets{"my-elb-123.us-east-1.elb.amazonaws.com"}, + ProviderSpecific: ProviderSpecific{{Name: providerSpecificAlias, Value: "true"}}, + }, + wantLog: false, + }, + { + name: "unsupported type without alias does not log", + ep: Endpoint{ + DNSName: "example.com", + RecordType: RecordTypeMX, + Targets: Targets{"10 mail.example.com"}, + }, + wantLog: false, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + logger, hook := test.NewNullLogger() + log.AddHook(hook) + log.SetOutput(logger.Out) + log.SetLevel(log.WarnLevel) + + tt.ep.CheckEndpoint() + + warnMsg := "does not support alias records" + found := false + for _, entry := range hook.AllEntries() { + if strings.Contains(entry.Message, warnMsg) && entry.Level == log.WarnLevel { + found = true + } + } + + if tt.wantLog { + assert.True(t, found, "Expected warning log message not found") + } else { + assert.False(t, found, "Unexpected warning log message found") + } + }) + } +} + func TestEndpoint_WithRefObject(t *testing.T) { ep := &Endpoint{} ref := &events.ObjectReference{ diff --git a/source/wrappers/dedupsource_test.go b/source/wrappers/dedupsource_test.go index 9ebaa2f8a7..1629a4d18e 100644 --- a/source/wrappers/dedupsource_test.go +++ b/source/wrappers/dedupsource_test.go @@ -253,6 +253,29 @@ func TestDedupEndpointsValidation(t *testing.T) { {DNSName: "example.org", RecordType: endpoint.RecordTypeA, Targets: endpoint.Targets{"1.2.3.4"}}, }, }, + { + name: "MX record with alias=true is filtered out", + endpoints: []*endpoint.Endpoint{ + {DNSName: "example.org", RecordType: endpoint.RecordTypeMX, Targets: endpoint.Targets{"10 mail.example.org"}, ProviderSpecific: endpoint.ProviderSpecific{{Name: "alias", Value: "true"}}}, + }, + expected: []*endpoint.Endpoint{}, + }, + { + name: "A record with alias=true is kept", + endpoints: []*endpoint.Endpoint{ + {DNSName: "example.org", RecordType: endpoint.RecordTypeA, Targets: endpoint.Targets{"192.168.1.1"}, ProviderSpecific: endpoint.ProviderSpecific{{Name: "alias", Value: "true"}}}, + }, + expected: []*endpoint.Endpoint{ + {DNSName: "example.org", RecordType: endpoint.RecordTypeA, Targets: endpoint.Targets{"192.168.1.1"}, ProviderSpecific: endpoint.ProviderSpecific{{Name: "alias", Value: "true"}}}, + }, + }, + { + name: "SRV record with alias=true is filtered out", + endpoints: []*endpoint.Endpoint{ + {DNSName: "_sip._tcp.example.org", RecordType: endpoint.RecordTypeSRV, Targets: endpoint.Targets{"10 5 5060 sip.example.org."}, ProviderSpecific: endpoint.ProviderSpecific{{Name: "alias", Value: "true"}}}, + }, + expected: []*endpoint.Endpoint{}, + }, { name: "mixed valid and invalid TXT, A, AAAA records", endpoints: []*endpoint.Endpoint{ @@ -290,21 +313,45 @@ func TestDedupEndpointsValidation(t *testing.T) { } func TestDedupSource_WarnsOnInvalidEndpoint(t *testing.T) { - hook := testutils.LogsUnderTestWithLogLevel(log.WarnLevel, t) - - invalidEndpoint := &endpoint.Endpoint{ - DNSName: "example.org", - RecordType: endpoint.RecordTypeSRV, - SetIdentifier: "default/svc/my-service", - Targets: endpoint.Targets{"10 mail.example.org"}, + tests := []struct { + name string + endpoint *endpoint.Endpoint + wantLogMsg string + }{ + { + name: "invalid SRV record", + endpoint: &endpoint.Endpoint{ + DNSName: "example.org", + RecordType: endpoint.RecordTypeSRV, + SetIdentifier: "default/svc/my-service", + Targets: endpoint.Targets{"10 mail.example.org"}, + }, + wantLogMsg: "Skipping endpoint [default/svc/my-service:example.org] due to invalid configuration [SRV:10 mail.example.org]", + }, + { + name: "unsupported alias on MX record", + endpoint: &endpoint.Endpoint{ + DNSName: "example.org", + RecordType: endpoint.RecordTypeMX, + Targets: endpoint.Targets{"10 mail.example.org"}, + ProviderSpecific: endpoint.ProviderSpecific{{Name: "alias", Value: "true"}}, + }, + wantLogMsg: "Endpoint example.org of type MX does not support alias records", + }, } - mockSource := new(testutils.MockSource) - mockSource.On("Endpoints").Return([]*endpoint.Endpoint{invalidEndpoint}, nil) + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + hook := testutils.LogsUnderTestWithLogLevel(log.WarnLevel, t) - src := NewDedupSource(mockSource) - _, err := src.Endpoints(context.Background()) - require.NoError(t, err) + mockSource := new(testutils.MockSource) + mockSource.On("Endpoints").Return([]*endpoint.Endpoint{tt.endpoint}, nil) - testutils.TestHelperLogContains("Skipping endpoint [default/svc/my-service:example.org] due to invalid configuration [SRV:10 mail.example.org]", hook, t) + src := NewDedupSource(mockSource) + _, err := src.Endpoints(context.Background()) + require.NoError(t, err) + + testutils.TestHelperLogContains(tt.wantLogMsg, hook, t) + }) + } }