Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
10 changes: 5 additions & 5 deletions endpoint/endpoint.go
Original file line number Diff line number Diff line change
Expand Up @@ -52,9 +52,9 @@ const (
// RecordTypeNAPTR is a RecordType enum value
RecordTypeNAPTR = "NAPTR"

// TODO: review source/annotations package to consolidate alias key definitions;
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

AliasKey and ProviderSpecificAlias are related in that the annotation value set via AliasKey
is eventually converted into a ProviderSpecific property named ProviderSpecificAlias by the
source/annotations package.

However, they are separate definitions serving different purposes — one is a Kubernetes annotation key with a full prefix, the other is an internal property name — so keeping them as independent constants is the right approach.

The original TODO has been removed accordingly.

// currently duplicated here to avoid circular dependency.
providerSpecificAlias = "alias"
// ProviderSpecificAlias indicates whether a CNAME endpoint maps to a
// provider-native alias record (e.g. AWS ALIAS).
ProviderSpecificAlias = "alias"

// ProviderSpecificRecordType is the provider-specific property name used to
// request a particular DNS record type (e.g. "ptr") on an endpoint.
Expand Down Expand Up @@ -535,7 +535,7 @@ func (e *Endpoint) RequestedRecordType() (string, bool) {
// 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 {
if _, ok := e.GetBoolProviderSpecificProperty(ProviderSpecificAlias); ok {
log.Warnf("Endpoint %s of type %s does not support alias records", e.DNSName, e.RecordType)
return false
}
Expand All @@ -558,7 +558,7 @@ func (e *Endpoint) CheckEndpoint() bool {

// isAlias returns true if the endpoint has the alias provider-specific property set to true.
func (e *Endpoint) isAlias() bool {
val, ok := e.GetBoolProviderSpecificProperty(providerSpecificAlias)
val, ok := e.GetBoolProviderSpecificProperty(ProviderSpecificAlias)
return ok && val
}

Expand Down
2 changes: 1 addition & 1 deletion endpoint/endpoint_benchmark_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ import (

var (
providerSpecificKeys = []string{
"alias",
ProviderSpecificAlias,
"provider/target-hosted-zone",
"provider/evaluate-target-health",
"provider/weight",
Expand Down
28 changes: 14 additions & 14 deletions endpoint/endpoint_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -576,14 +576,14 @@ func TestRetainProviderProperties(t *testing.T) {
name: "provider agnostic properties without prefix are retained",
endpoint: Endpoint{
ProviderSpecific: []ProviderSpecificProperty{
{Name: "alias", Value: "true"},
{Name: ProviderSpecificAlias, Value: "true"},
{Name: "aws/evaluate-target-health", Value: "true"},
{Name: "coredns/group", Value: "my-group"},
},
},
provider: "aws",
expected: []ProviderSpecificProperty{
{Name: "alias", Value: "true"},
{Name: ProviderSpecificAlias, Value: "true"},
{Name: "aws/evaluate-target-health", Value: "true"},
},
},
Expand All @@ -608,12 +608,12 @@ func TestRetainProviderProperties(t *testing.T) {
ProviderSpecific: []ProviderSpecificProperty{
{Name: "external-dns.alpha.kubernetes.io/cloudflare-tags", Value: "tag1"},
{Name: "aws/evaluate-target-health", Value: "true"},
{Name: "alias", Value: "false"},
{Name: ProviderSpecificAlias, Value: "false"},
},
},
provider: "cloudflare",
expected: []ProviderSpecificProperty{
{Name: "alias", Value: "false"},
{Name: ProviderSpecificAlias, Value: "false"},
{Name: "aws/evaluate-target-health", Value: "true"},
{Name: "external-dns.alpha.kubernetes.io/cloudflare-tags", Value: "tag1"},
},
Expand Down Expand Up @@ -1246,7 +1246,7 @@ func TestCheckEndpoint(t *testing.T) {
DNSName: "example.com",
RecordType: RecordTypeA,
Targets: Targets{"my-elb-123.us-east-1.elb.amazonaws.com"},
ProviderSpecific: ProviderSpecific{{Name: providerSpecificAlias, Value: "true"}},
ProviderSpecific: ProviderSpecific{{Name: ProviderSpecificAlias, Value: "true"}},
},
expected: true,
},
Expand All @@ -1256,7 +1256,7 @@ func TestCheckEndpoint(t *testing.T) {
DNSName: "example.com",
RecordType: RecordTypeAAAA,
Targets: Targets{"dualstack.my-elb-123.us-east-1.elb.amazonaws.com"},
ProviderSpecific: ProviderSpecific{{Name: providerSpecificAlias, Value: "true"}},
ProviderSpecific: ProviderSpecific{{Name: ProviderSpecificAlias, Value: "true"}},
},
expected: true,
},
Expand All @@ -1266,7 +1266,7 @@ func TestCheckEndpoint(t *testing.T) {
DNSName: "example.com",
RecordType: RecordTypeCNAME,
Targets: Targets{"d111111abcdef8.cloudfront.net"},
ProviderSpecific: ProviderSpecific{{Name: providerSpecificAlias, Value: "true"}},
ProviderSpecific: ProviderSpecific{{Name: ProviderSpecificAlias, Value: "true"}},
},
expected: true,
},
Expand All @@ -1276,7 +1276,7 @@ func TestCheckEndpoint(t *testing.T) {
DNSName: "example.com",
RecordType: RecordTypeMX,
Targets: Targets{"10 mail.example.com"},
ProviderSpecific: ProviderSpecific{{Name: providerSpecificAlias, Value: "true"}},
ProviderSpecific: ProviderSpecific{{Name: ProviderSpecificAlias, Value: "true"}},
},
expected: false,
},
Expand All @@ -1286,7 +1286,7 @@ func TestCheckEndpoint(t *testing.T) {
DNSName: "example.com",
RecordType: RecordTypeTXT,
Targets: Targets{"v=spf1 ~all"},
ProviderSpecific: ProviderSpecific{{Name: providerSpecificAlias, Value: "true"}},
ProviderSpecific: ProviderSpecific{{Name: ProviderSpecificAlias, Value: "true"}},
},
expected: false,
},
Expand All @@ -1296,7 +1296,7 @@ func TestCheckEndpoint(t *testing.T) {
DNSName: "example.com",
RecordType: RecordTypeNS,
Targets: Targets{"ns1.example.com"},
ProviderSpecific: ProviderSpecific{{Name: providerSpecificAlias, Value: "true"}},
ProviderSpecific: ProviderSpecific{{Name: ProviderSpecificAlias, Value: "true"}},
},
expected: false,
},
Expand All @@ -1306,7 +1306,7 @@ func TestCheckEndpoint(t *testing.T) {
DNSName: "_sip._tcp.example.com",
RecordType: RecordTypeSRV,
Targets: Targets{"10 5 5060 sip.example.com."},
ProviderSpecific: ProviderSpecific{{Name: providerSpecificAlias, Value: "true"}},
ProviderSpecific: ProviderSpecific{{Name: ProviderSpecificAlias, Value: "true"}},
},
expected: false,
},
Expand All @@ -1316,7 +1316,7 @@ func TestCheckEndpoint(t *testing.T) {
DNSName: "example.com",
RecordType: RecordTypeMX,
Targets: Targets{"10 mail.example.com"},
ProviderSpecific: ProviderSpecific{{Name: providerSpecificAlias, Value: "false"}},
ProviderSpecific: ProviderSpecific{{Name: ProviderSpecificAlias, Value: "false"}},
},
expected: false,
},
Expand Down Expand Up @@ -1459,7 +1459,7 @@ func TestCheckEndpoint_AliasWarningLog(t *testing.T) {
DNSName: "example.com",
RecordType: RecordTypeMX,
Targets: Targets{"10 mail.example.com"},
ProviderSpecific: ProviderSpecific{{Name: providerSpecificAlias, Value: "true"}},
ProviderSpecific: ProviderSpecific{{Name: ProviderSpecificAlias, Value: "true"}},
},
wantLog: true,
},
Expand All @@ -1469,7 +1469,7 @@ func TestCheckEndpoint_AliasWarningLog(t *testing.T) {
DNSName: "example.com",
RecordType: RecordTypeA,
Targets: Targets{"my-elb-123.us-east-1.elb.amazonaws.com"},
ProviderSpecific: ProviderSpecific{{Name: providerSpecificAlias, Value: "true"}},
ProviderSpecific: ProviderSpecific{{Name: ProviderSpecificAlias, Value: "true"}},
},
wantLog: false,
},
Expand Down
6 changes: 3 additions & 3 deletions plan/plan_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -162,7 +162,7 @@ func (suite *PlanTestSuite) SetupTest() {
},
ProviderSpecific: endpoint.ProviderSpecific{
endpoint.ProviderSpecificProperty{
Name: "alias",
Name: endpoint.ProviderSpecificAlias,
Value: "false",
},
endpoint.ProviderSpecificProperty{
Expand All @@ -184,7 +184,7 @@ func (suite *PlanTestSuite) SetupTest() {
Value: "false",
},
endpoint.ProviderSpecificProperty{
Name: "alias",
Name: endpoint.ProviderSpecificAlias,
Value: "false",
},
},
Expand All @@ -198,7 +198,7 @@ func (suite *PlanTestSuite) SetupTest() {
},
ProviderSpecific: endpoint.ProviderSpecific{
endpoint.ProviderSpecificProperty{
Name: "alias",
Name: endpoint.ProviderSpecificAlias,
Value: "false",
},
},
Expand Down
38 changes: 18 additions & 20 deletions provider/aws/aws.go
Original file line number Diff line number Diff line change
Expand Up @@ -49,13 +49,11 @@ const (
// when fewer items are returned, and still paginate accordingly.
// As we are using the standard AWS client, this should already be compliant.
// Hence, if AWS ever decides to raise this limit, we will automatically reduce the pressure on rate limits
route53PageSize int32 = 300
// providerSpecificAlias specifies whether a CNAME endpoint maps to an AWS ALIAS record.
providerSpecificAlias = "alias"
providerSpecificTargetHostedZone = "aws/target-hosted-zone"
route53PageSize int32 = 300
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`.
// has a `endpoint.ProviderSpecificAlias` value of `true`.
providerSpecificEvaluateTargetHealth = "aws/evaluate-target-health"
providerSpecificWeight = "aws/weight"
providerSpecificRegion = "aws/region"
Expand Down Expand Up @@ -542,7 +540,7 @@ func (p *AWSProvider) records(ctx context.Context, zones map[string]*profiledZon

ep := endpoint.NewEndpointWithTTL(name, string(r.Type), ttl, targets...)
if r.Type == endpoint.RecordTypeCNAME {
ep = ep.WithProviderSpecific(providerSpecificAlias, "false")
ep = ep.WithProviderSpecific(endpoint.ProviderSpecificAlias, "false")
}
newEndpoints = append(newEndpoints, ep)
}
Expand All @@ -555,7 +553,7 @@ func (p *AWSProvider) records(ctx context.Context, zones map[string]*profiledZon
ep := endpoint.
NewEndpointWithTTL(name, string(r.Type), ttl, *r.AliasTarget.DNSName).
WithProviderSpecific(providerSpecificEvaluateTargetHealth, fmt.Sprintf("%t", r.AliasTarget.EvaluateTargetHealth)).
WithProviderSpecific(providerSpecificAlias, "true")
WithProviderSpecific(endpoint.ProviderSpecificAlias, "true")
newEndpoints = append(newEndpoints, ep)
}

Expand Down Expand Up @@ -630,8 +628,8 @@ func (p *AWSProvider) requiresDeleteCreate(old *endpoint.Endpoint, newE *endpoin

// an ALIAS record change to/from an A
if old.RecordType == endpoint.RecordTypeA {
oldAlias, _ := old.GetProviderSpecificProperty(providerSpecificAlias)
newAlias, _ := newE.GetProviderSpecificProperty(providerSpecificAlias)
oldAlias, _ := old.GetProviderSpecificProperty(endpoint.ProviderSpecificAlias)
newAlias, _ := newE.GetProviderSpecificProperty(endpoint.ProviderSpecificAlias)
if oldAlias != newAlias {
return true
}
Expand Down Expand Up @@ -862,7 +860,7 @@ func (p *AWSProvider) adjustEndpointAndNewAaaaIfNeeded(ep *endpoint.Endpoint) *e
case endpoint.RecordTypeCNAME:
p.adjustCNAMERecord(ep)
adjustGeoProximityLocationEndpoint(ep)
if isAlias, _ := ep.GetBoolProviderSpecificProperty(providerSpecificAlias); isAlias {
if isAlias, _ := ep.GetBoolProviderSpecificProperty(endpoint.ProviderSpecificAlias); isAlias {
aaaa = ep.DeepCopy()
aaaa.RecordType = endpoint.RecordTypeAAAA
}
Expand Down Expand Up @@ -890,30 +888,30 @@ func (p *AWSProvider) adjustAliasRecord(ep *endpoint.Endpoint) {
}

func (p *AWSProvider) adjustAandAAAARecord(ep *endpoint.Endpoint) {
isAlias, _ := ep.GetBoolProviderSpecificProperty(providerSpecificAlias)
isAlias, _ := ep.GetBoolProviderSpecificProperty(endpoint.ProviderSpecificAlias)
if isAlias {
p.adjustAliasRecord(ep)
} else {
ep.DeleteProviderSpecificProperty(providerSpecificAlias)
ep.DeleteProviderSpecificProperty(endpoint.ProviderSpecificAlias)
ep.DeleteProviderSpecificProperty(providerSpecificEvaluateTargetHealth)
}
}

func (p *AWSProvider) adjustCNAMERecord(ep *endpoint.Endpoint) {
isAlias, exists := ep.GetBoolProviderSpecificProperty(providerSpecificAlias)
isAlias, exists := ep.GetBoolProviderSpecificProperty(endpoint.ProviderSpecificAlias)

// fallback to determining alias based on preferCNAME if not explicitly set
if !exists {
isAlias = useAlias(ep, p.preferCNAME)
log.Debugf("Modifying endpoint: %v, setting %s=%v", ep, providerSpecificAlias, isAlias)
ep.SetProviderSpecificProperty(providerSpecificAlias, strconv.FormatBool(isAlias))
log.Debugf("Modifying endpoint: %v, setting %s=%v", ep, endpoint.ProviderSpecificAlias, isAlias)
ep.SetProviderSpecificProperty(endpoint.ProviderSpecificAlias, strconv.FormatBool(isAlias))
}

// if not an alias, ensure alias properties are adjusted accordingly
if !isAlias {
if exists {
// normalize to string "false" when provider specific alias is set to false or other non-true value
ep.SetProviderSpecificProperty(providerSpecificAlias, "false")
ep.SetProviderSpecificProperty(endpoint.ProviderSpecificAlias, "false")
}
ep.DeleteProviderSpecificProperty(providerSpecificEvaluateTargetHealth)
}
Expand All @@ -928,11 +926,11 @@ func (p *AWSProvider) adjustCNAMERecord(ep *endpoint.Endpoint) {
func (p *AWSProvider) adjustOtherRecord(ep *endpoint.Endpoint) {
// TODO: fix For records other than A, AAAA, and CNAME, if an alias record is set, the alias record processing is not performed.
// This will be fixed in another PR.
if isAlias, _ := ep.GetBoolProviderSpecificProperty(providerSpecificAlias); isAlias {
if isAlias, _ := ep.GetBoolProviderSpecificProperty(endpoint.ProviderSpecificAlias); isAlias {
p.adjustAliasRecord(ep)
ep.DeleteProviderSpecificProperty(providerSpecificAlias)
ep.DeleteProviderSpecificProperty(endpoint.ProviderSpecificAlias)
} else {
ep.DeleteProviderSpecificProperty(providerSpecificAlias)
ep.DeleteProviderSpecificProperty(endpoint.ProviderSpecificAlias)
ep.DeleteProviderSpecificProperty(providerSpecificEvaluateTargetHealth)
}
}
Expand Down Expand Up @@ -1394,7 +1392,7 @@ func useAlias(ep *endpoint.Endpoint, preferCNAME bool) bool {
// 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 {
isAlias, _ := ep.GetBoolProviderSpecificProperty(providerSpecificAlias)
isAlias, _ := ep.GetBoolProviderSpecificProperty(endpoint.ProviderSpecificAlias)
if isAlias && slices.Contains([]string{endpoint.RecordTypeA, endpoint.RecordTypeAAAA}, ep.RecordType) && len(ep.Targets) > 0 {
// alias records can only point to canonical hosted zones (e.g. to ELBs) or other records in the same zone

Expand Down
Loading
Loading