-
Notifications
You must be signed in to change notification settings - Fork 2.8k
feat(aws): add aws/alias-disable-a and aws/alias-disable-aaaa provider-specific options #5997
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -66,6 +66,8 @@ const ( | |
| providerSpecificGeoProximityLocationLocalZoneGroup = "aws/geoproximity-local-zone-group" | ||
| providerSpecificMultiValueAnswer = "aws/multi-value-answer" | ||
| providerSpecificHealthCheckID = "aws/health-check-id" | ||
| providerSpecificAliasDisableA = "aws/alias-disable-a" | ||
| providerSpecificAliasDisableAAAA = "aws/alias-disable-aaaa" | ||
| sameZoneAlias = "same-zone" | ||
| // Currently supported up to 10 health checks or hosted zones. | ||
| // https://docs.aws.amazon.com/Route53/latest/APIReference/API_ListTagsForResources.html#API_ListTagsForResources_RequestSyntax | ||
|
|
@@ -863,19 +865,32 @@ func (p *AWSProvider) AdjustEndpoints(endpoints []*endpoint.Endpoint) ([]*endpoi | |
| ep.SetProviderSpecificProperty(providerSpecificEvaluateTargetHealth, strconv.FormatBool(p.evaluateTargetHealth)) | ||
| } | ||
|
|
||
| if ep.RecordType == endpoint.RecordTypeCNAME { | ||
| // This needs to match two records from Route53, one alias for 'A' (IPv4) | ||
| // and one alias for 'AAAA' (IPv6). | ||
| aliasCnameAaaaEndpoints = append(aliasCnameAaaaEndpoints, &endpoint.Endpoint{ | ||
| DNSName: ep.DNSName, | ||
| Targets: ep.Targets, | ||
| RecordType: endpoint.RecordTypeAAAA, | ||
| RecordTTL: ep.RecordTTL, | ||
| Labels: ep.Labels, | ||
| ProviderSpecific: ep.ProviderSpecific, | ||
| SetIdentifier: ep.SetIdentifier, | ||
| }) | ||
| ep.RecordType = endpoint.RecordTypeA | ||
| disableA := aliasDisableARecord(ep) | ||
| disableAaaa := aliasDisableAaaaRecord(ep) | ||
| disableAlias := disableA && disableAaaa | ||
| enableAandAAAA := !disableA && !disableAaaa | ||
|
|
||
| if ep.RecordType == endpoint.RecordTypeCNAME && !disableAlias { | ||
| if disableA { | ||
| ep.RecordType = endpoint.RecordTypeAAAA | ||
| } | ||
| if disableAaaa { | ||
| ep.RecordType = endpoint.RecordTypeA | ||
| } | ||
| if enableAandAAAA { | ||
| // Add a new endpoint for the AAAA record | ||
| aliasCnameAaaaEndpoints = append(aliasCnameAaaaEndpoints, &endpoint.Endpoint{ | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Make sure not to use &endpoint.Endpoint{}, but methods available in the package. This will create tech debt for us.
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'd say it should be clone or similar function in Endpoint package, to make sure we do not mutate original
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. |
||
| DNSName: ep.DNSName, | ||
| Targets: ep.Targets, | ||
| RecordType: endpoint.RecordTypeAAAA, | ||
| RecordTTL: ep.RecordTTL, | ||
| Labels: ep.Labels, | ||
| ProviderSpecific: ep.ProviderSpecific, | ||
| SetIdentifier: ep.SetIdentifier, | ||
| }) | ||
| // Modify the original endpoint to be the A record | ||
| ep.RecordType = endpoint.RecordTypeA | ||
| } | ||
| } | ||
| } else { | ||
| ep.DeleteProviderSpecificProperty(providerSpecificEvaluateTargetHealth) | ||
|
|
@@ -888,6 +903,16 @@ func (p *AWSProvider) AdjustEndpoints(endpoints []*endpoint.Endpoint) ([]*endpoi | |
| return endpoints, nil | ||
| } | ||
|
|
||
| func aliasDisableARecord(ep *endpoint.Endpoint) bool { | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. My opinion, we will benefit to have a generic,centralised method on Endpoint object for all annotations that contains booleans like true/false
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. something like
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. That does sound like a good idea.
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ok |
||
| disable, ok := ep.GetProviderSpecificProperty(providerSpecificAliasDisableA) | ||
| return ok && disable == "true" | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Use constants instead of strings for "true". |
||
| } | ||
|
|
||
| func aliasDisableAaaaRecord(ep *endpoint.Endpoint) bool { | ||
| disable, ok := ep.GetProviderSpecificProperty(providerSpecificAliasDisableAAAA) | ||
| return ok && disable == "true" | ||
| } | ||
|
|
||
| // if the endpoint is using geoproximity, set the bias to 0 if not set | ||
| // this is needed to avoid unnecessary Upserts if the desired endpoint doesn't specify a bias | ||
| func adjustGeoProximityLocationEndpoint(ep *endpoint.Endpoint) { | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Something does not sound here. This most likely could and should be a method on it's own, so we could add tests just for this method
something like