Skip to content

Commit c0b0e4b

Browse files
u-kaiu-kai
authored
refactor(aws): extract and restructure alias-handling logic to enable safe upcoming fixes (#6021)
* test: add regression test to ensure behavior before refactor Signed-off-by: u-kai <[email protected]> * refactor aws adjustEndpointAndAaaaIfNeeded Signed-off-by: u-kai <[email protected]> * test(aws): add comprehensive tests and remove old logic Signed-off-by: u-kai <[email protected]> * refactor(aws): simplify AdjustEndpoints record-type dispatch Signed-off-by: u-kai <[email protected]> --------- Signed-off-by: u-kai <[email protected]>
1 parent 7579ce2 commit c0b0e4b

File tree

3 files changed

+594
-68
lines changed

3 files changed

+594
-68
lines changed

‎internal/testutils/endpoint.go‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -63,6 +63,9 @@ func (b byAllFields) Less(i, j int) bool {
6363
// SameEndpoint returns true if two endpoints are same
6464
// considers example.org. and example.org DNSName/Target as different endpoints
6565
func SameEndpoint(a, b *endpoint.Endpoint) bool {
66+
if a == nil || b == nil {
67+
return a == b
68+
}
6669
return a.DNSName == b.DNSName && a.Targets.Same(b.Targets) && a.RecordType == b.RecordType && a.SetIdentifier == b.SetIdentifier &&
6770
a.Labels[endpoint.OwnerLabelKey] == b.Labels[endpoint.OwnerLabelKey] && a.RecordTTL == b.RecordTTL &&
6871
a.Labels[endpoint.ResourceLabelKey] == b.Labels[endpoint.ResourceLabelKey] &&

‎provider/aws/aws.go‎

Lines changed: 80 additions & 51 deletions
Original file line numberDiff line numberDiff line change
@@ -820,65 +820,94 @@ func (p *AWSProvider) AdjustEndpoints(endpoints []*endpoint.Endpoint) ([]*endpoi
820820
var aliasCnameAaaaEndpoints []*endpoint.Endpoint
821821

822822
for _, ep := range endpoints {
823-
alias := false
823+
if aaaa := p.adjustEndpointAndNewAaaaIfNeeded(ep); aaaa != nil {
824+
aliasCnameAaaaEndpoints = append(aliasCnameAaaaEndpoints, aaaa)
825+
}
826+
}
827+
return append(endpoints, aliasCnameAaaaEndpoints...), nil
828+
}
824829

825-
if aliasString, ok := ep.GetProviderSpecificProperty(providerSpecificAlias); ok {
826-
alias = aliasString == "true"
827-
if alias {
828-
if !slices.Contains([]string{endpoint.RecordTypeA, endpoint.RecordTypeAAAA, endpoint.RecordTypeCNAME}, ep.RecordType) {
829-
ep.DeleteProviderSpecificProperty(providerSpecificAlias)
830-
}
831-
} else {
832-
if ep.RecordType == endpoint.RecordTypeCNAME {
833-
if aliasString != "false" {
834-
ep.SetProviderSpecificProperty(providerSpecificAlias, "false")
835-
}
836-
} else {
837-
ep.DeleteProviderSpecificProperty(providerSpecificAlias)
838-
}
839-
}
840-
} else if ep.RecordType == endpoint.RecordTypeCNAME {
841-
alias = useAlias(ep, p.preferCNAME)
842-
log.Debugf("Modifying endpoint: %v, setting %s=%v", ep, providerSpecificAlias, alias)
843-
ep.SetProviderSpecificProperty(providerSpecificAlias, strconv.FormatBool(alias))
830+
func (p *AWSProvider) adjustEndpointAndNewAaaaIfNeeded(ep *endpoint.Endpoint) *endpoint.Endpoint {
831+
var aaaa *endpoint.Endpoint
832+
switch ep.RecordType {
833+
case endpoint.RecordTypeA, endpoint.RecordTypeAAAA:
834+
p.adjustAandAAAARecord(ep)
835+
case endpoint.RecordTypeCNAME:
836+
p.adjustCNAMERecord(ep)
837+
adjustGeoProximityLocationEndpoint(ep)
838+
if isAlias, _ := ep.GetBoolProviderSpecificProperty(providerSpecificAlias); isAlias {
839+
aaaa = ep.DeepCopy()
840+
aaaa.RecordType = endpoint.RecordTypeAAAA
844841
}
842+
return aaaa
843+
default:
844+
p.adjustOtherRecord(ep)
845+
}
846+
adjustGeoProximityLocationEndpoint(ep)
847+
return aaaa
848+
}
845849

846-
if alias {
847-
if ep.RecordTTL.IsConfigured() {
848-
log.Debugf("Modifying endpoint: %v, setting ttl=%v", ep, defaultTTL)
849-
ep.RecordTTL = defaultTTL
850-
}
851-
if prop, ok := ep.GetProviderSpecificProperty(providerSpecificEvaluateTargetHealth); ok {
852-
if prop != "true" && prop != "false" {
853-
ep.SetProviderSpecificProperty(providerSpecificEvaluateTargetHealth, "false")
854-
}
855-
} else {
856-
ep.SetProviderSpecificProperty(providerSpecificEvaluateTargetHealth, strconv.FormatBool(p.evaluateTargetHealth))
857-
}
850+
func (p *AWSProvider) adjustAliasRecord(ep *endpoint.Endpoint) {
851+
if ep.RecordTTL.IsConfigured() {
852+
log.Debugf("Modifying endpoint: %v, setting ttl=%v", ep, defaultTTL)
853+
ep.RecordTTL = defaultTTL
854+
}
858855

859-
if ep.RecordType == endpoint.RecordTypeCNAME {
860-
// This needs to match two records from Route53, one alias for 'A' (IPv4)
861-
// and one alias for 'AAAA' (IPv6).
862-
aliasCnameAaaaEndpoints = append(aliasCnameAaaaEndpoints, &endpoint.Endpoint{
863-
DNSName: ep.DNSName,
864-
Targets: ep.Targets,
865-
RecordType: endpoint.RecordTypeAAAA,
866-
RecordTTL: ep.RecordTTL,
867-
Labels: ep.Labels,
868-
ProviderSpecific: ep.ProviderSpecific,
869-
SetIdentifier: ep.SetIdentifier,
870-
})
871-
ep.RecordType = endpoint.RecordTypeA
872-
}
873-
} else {
874-
ep.DeleteProviderSpecificProperty(providerSpecificEvaluateTargetHealth)
856+
if enable, exists := ep.GetBoolProviderSpecificProperty(providerSpecificEvaluateTargetHealth); exists {
857+
// normalize to string "true"/"false"
858+
ep.SetProviderSpecificProperty(providerSpecificEvaluateTargetHealth, strconv.FormatBool(enable))
859+
} else {
860+
// if not set, use provider default
861+
ep.SetProviderSpecificProperty(providerSpecificEvaluateTargetHealth, strconv.FormatBool(p.evaluateTargetHealth))
862+
}
863+
}
864+
865+
func (p *AWSProvider) adjustAandAAAARecord(ep *endpoint.Endpoint) {
866+
isAlias, _ := ep.GetBoolProviderSpecificProperty(providerSpecificAlias)
867+
if isAlias {
868+
p.adjustAliasRecord(ep)
869+
} else {
870+
ep.DeleteProviderSpecificProperty(providerSpecificAlias)
871+
ep.DeleteProviderSpecificProperty(providerSpecificEvaluateTargetHealth)
872+
}
873+
}
874+
875+
func (p *AWSProvider) adjustCNAMERecord(ep *endpoint.Endpoint) {
876+
isAlias, exists := ep.GetBoolProviderSpecificProperty(providerSpecificAlias)
877+
878+
// fallback to determining alias based on preferCNAME if not explicitly set
879+
if !exists {
880+
isAlias = useAlias(ep, p.preferCNAME)
881+
log.Debugf("Modifying endpoint: %v, setting %s=%v", ep, providerSpecificAlias, isAlias)
882+
ep.SetProviderSpecificProperty(providerSpecificAlias, strconv.FormatBool(isAlias))
883+
}
884+
885+
// if not an alias, ensure alias properties are adjusted accordingly
886+
if !isAlias {
887+
if exists {
888+
// normalize to string "false" when provider specific alias is set to false or other non-true value
889+
ep.SetProviderSpecificProperty(providerSpecificAlias, "false")
875890
}
891+
ep.DeleteProviderSpecificProperty(providerSpecificEvaluateTargetHealth)
892+
}
876893

877-
adjustGeoProximityLocationEndpoint(ep)
894+
// if an alias, convert to A record and adjust alias properties
895+
if isAlias {
896+
ep.RecordType = endpoint.RecordTypeA
897+
p.adjustAliasRecord(ep)
878898
}
899+
}
879900

880-
endpoints = append(endpoints, aliasCnameAaaaEndpoints...)
881-
return endpoints, nil
901+
func (p *AWSProvider) adjustOtherRecord(ep *endpoint.Endpoint) {
902+
// TODO: fix For records other than A, AAAA, and CNAME, if an alias record is set, the alias record processing is not performed.
903+
// This will be fixed in another PR.
904+
if isAlias, _ := ep.GetBoolProviderSpecificProperty(providerSpecificAlias); isAlias {
905+
p.adjustAliasRecord(ep)
906+
ep.DeleteProviderSpecificProperty(providerSpecificAlias)
907+
} else {
908+
ep.DeleteProviderSpecificProperty(providerSpecificAlias)
909+
ep.DeleteProviderSpecificProperty(providerSpecificEvaluateTargetHealth)
910+
}
882911
}
883912

884913
// if the endpoint is using geoproximity, set the bias to 0 if not set

0 commit comments

Comments
 (0)