-
Notifications
You must be signed in to change notification settings - Fork 2.6k
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
loop through all hosted zones to match the domain record #3695
Conversation
|
Welcome @allurisravanth! |
/easycla |
/easycla |
you are right, I didn't consider that operations such as only call once Subject: [PATCH] fix alibaba cloud support sub domains
---
Index: provider/alibabacloud/alibaba_cloud_test.go
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/provider/alibabacloud/alibaba_cloud_test.go b/provider/alibabacloud/alibaba_cloud_test.go
--- a/provider/alibabacloud/alibaba_cloud_test.go (revision 1f6340a95c0f5a96cc5888196eb767943f96bea4)
+++ b/provider/alibabacloud/alibaba_cloud_test.go (date 1687322673526)
@@ -18,6 +18,7 @@
import (
"context"
+ "fmt"
"github.com/aliyun/alibaba-cloud-sdk-go/services/alidns"
"github.com/aliyun/alibaba-cloud-sdk-go/services/pvtz"
"testing"
@@ -392,44 +393,55 @@
p := newTestAlibabaCloudProvider(false)
endpoint := &endpoint.Endpoint{}
endpoint.DNSName = "www.example.org"
- rr, domain := p.splitDNSName(endpoint.DNSName)
+ rr, domain := p.splitDNSName(endpoint.DNSName, nil)
if rr != "www" || domain != "example.org" {
t.Errorf("Failed to splitDNSName for %s: rr=%s, domain=%s", endpoint.DNSName, rr, domain)
}
endpoint.DNSName = ".example.org"
- rr, domain = p.splitDNSName(endpoint.DNSName)
+ rr, domain = p.splitDNSName(endpoint.DNSName, nil)
if rr != "@" || domain != "example.org" {
t.Errorf("Failed to splitDNSName for %s: rr=%s, domain=%s", endpoint.DNSName, rr, domain)
}
endpoint.DNSName = "www"
- rr, domain = p.splitDNSName(endpoint.DNSName)
+ rr, domain = p.splitDNSName(endpoint.DNSName, nil)
if rr != "www" || domain != "" {
t.Errorf("Failed to splitDNSName for %s: rr=%s, domain=%s", endpoint.DNSName, rr, domain)
}
endpoint.DNSName = ""
- rr, domain = p.splitDNSName(endpoint.DNSName)
+ rr, domain = p.splitDNSName(endpoint.DNSName, nil)
if rr != "@" || domain != "" {
t.Errorf("Failed to splitDNSName for %s: rr=%s, domain=%s", endpoint.DNSName, rr, domain)
}
endpoint.DNSName = "_30000._tcp.container-service.top"
- rr, domain = p.splitDNSName(endpoint.DNSName)
+ rr, domain = p.splitDNSName(endpoint.DNSName, nil)
if rr != "_30000._tcp" || domain != "container-service.top" {
t.Errorf("Failed to splitDNSName for %s: rr=%s, domain=%s", endpoint.DNSName, rr, domain)
}
endpoint.DNSName = "container-service.top"
- rr, domain = p.splitDNSName(endpoint.DNSName)
+ rr, domain = p.splitDNSName(endpoint.DNSName, nil)
if rr != "@" || domain != "container-service.top" {
t.Errorf("Failed to splitDNSName for %s: rr=%s, domain=%s", endpoint.DNSName, rr, domain)
}
endpoint.DNSName = "a.b.container-service.top"
- rr, domain = p.splitDNSName(endpoint.DNSName)
+ rr, domain = p.splitDNSName(endpoint.DNSName, nil)
if rr != "a.b" || domain != "container-service.top" {
t.Errorf("Failed to splitDNSName for %s: rr=%s, domain=%s", endpoint.DNSName, rr, domain)
}
endpoint.DNSName = "a.b.c.container-service.top"
- rr, domain = p.splitDNSName(endpoint.DNSName)
+ rr, domain = p.splitDNSName(endpoint.DNSName, nil)
if rr != "a.b.c" || domain != "container-service.top" {
t.Errorf("Failed to splitDNSName for %s: rr=%s, domain=%s", endpoint.DNSName, rr, domain)
+ }
+ endpoint.DNSName = "a.b.c.container-service.top"
+ rr, domain = p.splitDNSName(endpoint.DNSName, []string{"c.container-service.top"})
+ if rr != "a.b" || domain != "c.container-service.top" {
+ t.Errorf("Failed to splitDNSName for %s: rr=%s, domain=%s", endpoint.DNSName, rr, domain)
+ }
+
+ endpoint.DNSName = "a.b.c.container-service.top"
+ rr, domain = p.splitDNSName(endpoint.DNSName, []string{"container-service.top", "c.container-service.top"})
+ if rr != "a.b" || domain != "c.container-service.top" {
+ t.Errorf("Failed to splitDNSName for %s: rr=%s, domain=%s", endpoint.DNSName, rr, domain)
}
}
Index: provider/alibabacloud/alibaba_cloud.go
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/provider/alibabacloud/alibaba_cloud.go b/provider/alibabacloud/alibaba_cloud.go
--- a/provider/alibabacloud/alibaba_cloud.go (revision 1f6340a95c0f5a96cc5888196eb767943f96bea4)
+++ b/provider/alibabacloud/alibaba_cloud.go (date 1687322655257)
@@ -20,6 +20,7 @@
"context"
"fmt"
"os"
+ "sort"
"strings"
"sync"
"time"
@@ -374,30 +375,19 @@
log.Infof("Retrieving Alibaba Cloud DNS Domain Records")
var results []alidns.Record
- if len(p.domainFilter.Filters) == 0 {
- domainNames, tmpErr := p.getDomainList()
- if tmpErr != nil {
- log.Errorf("AlibabaCloudProvider getDomainList error %v", tmpErr)
- return results, tmpErr
- }
- for _, tmpDomainName := range domainNames {
- tmpResults, err := p.getDomainRecords(tmpDomainName)
- if err != nil {
- log.Errorf("AlibabaCloudProvider getDomainRecords %s error %v", tmpDomainName, err)
- continue
- }
- results = append(results, tmpResults...)
- }
- } else {
- for _, domainName := range p.domainFilter.Filters {
- _, domainName = p.splitDNSName(domainName)
- tmpResults, err := p.getDomainRecords(domainName)
- if err != nil {
- log.Errorf("getDomainRecords %s error %v", domainName, err)
- continue
- }
- results = append(results, tmpResults...)
- }
+ hostedZoneNames, err := p.getDomainList()
+ if err != nil {
+ log.Errorf("AlibabaCloudProvider getDomainList error %v", err)
+ return results, err
+ }
+ var tmpResults []alidns.Record
+ for _, zone := range hostedZoneNames {
+ tmpResults, err = p.getDomainRecords(zone)
+ if err != nil {
+ log.Errorf("AlibabaCloudProvider getDomainRecords %q error %v", zone, err)
+ continue
+ }
+ results = append(results, tmpResults...)
}
log.Infof("Found %d Alibaba Cloud DNS record(s).", len(results))
return results, nil
@@ -409,6 +399,7 @@
request.PageSize = requests.NewInteger(defaultAlibabaCloudPageSize)
request.PageNumber = "1"
request.Scheme = defaultAlibabaCloudRequestScheme
+ needFilter := len(p.domainFilter.Filters) > 0
for {
resp, err := p.dnsClient.DescribeDomains(request)
if err != nil {
@@ -416,6 +407,9 @@
return nil, err
}
for _, tmpDomain := range resp.Domains.Domain {
+ if needFilter && !p.domainFilter.Match(tmpDomain.DomainName) {
+ continue
+ }
domainNames = append(domainNames, tmpDomain.DomainName)
}
nextPage := getNextPageNumber(resp.PageNumber, defaultAlibabaCloudPageSize, resp.TotalCount)
@@ -494,8 +488,8 @@
return value
}
-func (p *AlibabaCloudProvider) createRecord(endpoint *endpoint.Endpoint, target string) error {
- rr, domain := p.splitDNSName(endpoint.DNSName)
+func (p *AlibabaCloudProvider) createRecord(endpoint *endpoint.Endpoint, target string, zones []string) error {
+ rr, domain := p.splitDNSName(endpoint.DNSName, zones)
request := alidns.CreateAddDomainRecordRequest()
request.DomainName = domain
request.Type = endpoint.RecordType
@@ -528,9 +522,13 @@
}
func (p *AlibabaCloudProvider) createRecords(endpoints []*endpoint.Endpoint) error {
+ zones, err := p.getDomainList()
+ if err != nil {
+ return err
+ }
for _, endpoint := range endpoints {
for _, target := range endpoint.Targets {
- p.createRecord(endpoint, target)
+ p.createRecord(endpoint, target, zones)
}
}
return nil
@@ -616,6 +614,10 @@
}
func (p *AlibabaCloudProvider) updateRecords(recordMap map[string][]alidns.Record, endpoints []*endpoint.Endpoint) error {
+ zones, err := p.getDomainList()
+ if err != nil {
+ return err
+ }
for _, endpoint := range endpoints {
key := p.getRecordKeyByEndpoint(endpoint)
records := recordMap[key]
@@ -652,31 +654,56 @@
}
}
if !found {
- p.createRecord(endpoint, target)
+ p.createRecord(endpoint, target, zones)
}
}
}
return nil
}
-func (p *AlibabaCloudProvider) splitDNSName(fullName string) (rr string, domain string) {
- name := strings.TrimSuffix(fullName, ".")
- parts := strings.Split(name, ".")
- if len(parts) < 2 {
- rr = name
- domain = ""
- } else {
- domain = parts[len(parts)-2] + "." + parts[len(parts)-1]
- rrIndex := strings.Index(name, domain)
- if rrIndex < 1 {
- rrIndex = 1
- }
- rr = name[0 : rrIndex-1]
+func (p *AlibabaCloudProvider) splitDNSName(dnsName string, zones []string) (rr string, domain string) {
+ name := strings.TrimSuffix(dnsName, ".")
+
+ // sort zones by dot, make sure subdomain at first
+ sort.Slice(zones, func(i, j int) bool {
+ return strings.Count(zones[i], ".") > strings.Count(zones[j], ".")
+ })
+
+ found := false
+
+ for _, filter := range zones {
+ if strings.HasSuffix(name, "."+filter) {
+ rr = name[0 : len(name)-len(filter)-1]
+ domain = filter
+ found = true
+ break
+ } else if name == filter {
+ domain = filter
+ rr = ""
+ found = true
+ }
+ }
+
+ if !found {
+ parts := strings.Split(name, ".")
+ if len(parts) < 2 {
+ rr = name
+ domain = ""
+ } else {
+ domain = parts[len(parts)-2] + "." + parts[len(parts)-1]
+ rrIndex := strings.Index(name, domain)
+ if rrIndex < 1 {
+ rrIndex = 1
+ }
+ rr = name[0 : rrIndex-1]
+ }
}
+
if rr == "" {
rr = nullHostAlibabaCloud
}
- return
+
+ return rr, domain
}
func (p *AlibabaCloudProvider) matchVPC(zoneID string) bool {
@@ -844,7 +871,7 @@
}
func (p *AlibabaCloudProvider) createPrivateZoneRecord(zones map[string]*alibabaPrivateZone, endpoint *endpoint.Endpoint, target string) error {
- rr, domain := p.splitDNSName(endpoint.DNSName)
+ rr, domain := p.splitDNSName(endpoint.DNSName, keys(zones))
zone := zones[domain]
if zone == nil {
err := fmt.Errorf("failed to find private zone '%s'", domain)
@@ -913,8 +940,9 @@
}
func (p *AlibabaCloudProvider) deletePrivateZoneRecords(zones map[string]*alibabaPrivateZone, endpoints []*endpoint.Endpoint) error {
+ zoneNames := keys(zones)
for _, endpoint := range endpoints {
- rr, domain := p.splitDNSName(endpoint.DNSName)
+ rr, domain := p.splitDNSName(endpoint.DNSName, zoneNames)
zone := zones[domain]
if zone == nil {
@@ -1003,8 +1031,9 @@
}
func (p *AlibabaCloudProvider) updatePrivateZoneRecords(zones map[string]*alibabaPrivateZone, endpoints []*endpoint.Endpoint) error {
+ zoneNames := keys(zones)
for _, endpoint := range endpoints {
- rr, domain := p.splitDNSName(endpoint.DNSName)
+ rr, domain := p.splitDNSName(endpoint.DNSName, zoneNames)
zone := zones[domain]
if zone == nil {
err := fmt.Errorf("failed to find private zone '%s'", domain)
@@ -1059,3 +1088,11 @@
}
return nil
}
+
+func keys[T any](value map[string]T) []string {
+ var results []string
+ for k := range value {
+ results = append(results, k)
+ }
+ return results
+} |
@stan-chen i made the changes as suggested, it works now with our alicloud setup. Please do review when you get a chance |
/ok-to-test |
@allurisravanth everything looks fine. |
Just the one whitespace nit. |
/lgtm |
@szuecs can you please have a look once when you get some time. Our alicloud external-dns is broken for sometime now, hence bugging you as you are assigned to this PR. |
@johngmyers @szuecs can you please review the PR |
/approve |
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: allurisravanth, szuecs The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
/retest |
1 similar comment
/retest |
Description
loop through all domains to match the domain record
instead of matching domain records with filter query, the idea is to first match the record with the hosted zones. If there is no match, splitDNS method then proceeds to split the domain to it's respective rr and Domain Name
Fixes #3625
Checklist