Repository: StackExchange/dnscontrol
Pull request #1241 (2021-08-27 09:04:18)

Add R53_ZONE as an domain identifier
Using R53_ZONE allows you to differentiate between split horizon domains across zones.

## But why?
Well, imagine you have a hosted public zone called enrise.com and a private zone for enrise.com specific to a VPC. different enrise.com zones across different VPCs. In these examples DNSControl doesn't allow you to specify the zone by ID. This MR addresses this by using the `R53_ZONE` directive inside the `D` directive

tlimoncelli: * Please rebase
* Please integrate my suggested rewrite of the doc.
blackshadev: > * Please rebase
> * Please integrate my suggested rewrite of the doc.

I dont see any comment about the doc rewrite, could you elaborate?
tlimoncelli: I guess the suggestion about the docs didn't get saved the first time.  Take a look now.
blackshadev: > I guess the suggestion about the docs didn't get saved the first time. Take a look now.

Done
tlimoncelli: Sorry for the delay, things have been busy here with Ida.

I think this PR is ready for merge.  If you agree, rebase and confirm that it is ready.
blackshadev: > Sorry for the delay, things have been busy here with Ida.
> 
> I think this PR is ready for merge. If you agree, rebase and confirm that it is ready.

I rebased it (again). And yes it is ready to merge.
tlimoncelli: Awesome!  Thanks for this new feature!  I'm sure everyone that uses AWS will appreciate it!

Patch:
diff --git a/docs/_functions/record/R53_ZONE.md b/docs/_functions/record/R53_ZONE.md
index 0400063a..ad848be6 100644
--- a/docs/_functions/record/R53_ZONE.md
+++ b/docs/_functions/record/R53_ZONE.md
@@ -4,8 +4,12 @@ parameters:
   - zone_id
 ---
 
-R53_ZONE sets the required Route53 hosted zone id in a R53_ALIAS record.
+R53_ZONE lets you specify the AWS Zone ID for an entire domain (D()) or a specific R53_ALIAS() record.
+
+When used with D(), it sets the zone id of the domain. This can be used to differentiate between split horizon domains in public and private zones.
+
+When used with R53_ALIAS() it sets the required Route53 hosted zone id in a R53_ALIAS record. See [https://stackexchange.github.io/dnscontrol/js#R53_ALIAS](R53_ALIAS's documentation) for details.
+
+
 
-This directive has no impact when used in anything else than a R53_ALIAS.
 
-Please refer to the R53_ALIAS directive for usage.
\ No newline at end of file
diff --git a/pkg/js/helpers.js b/pkg/js/helpers.js
index 8ce180db..6af989d8 100644
--- a/pkg/js/helpers.js
+++ b/pkg/js/helpers.js
@@ -26,6 +26,10 @@ function initialize() {
     defaultArgs = [];
 }
 
+function _isDomain(d) {
+  return _.isArray(d.nameservers) && _.isArray(d.records) && _.isString(d.name);
+}
+
 // Returns an array of domains which were registered so far during runtime
 // Own function for compatibility reasons or if some day special processing would be required.
 function getConfiguredDomains() {
@@ -276,8 +280,10 @@ var R53_ALIAS = recordBuilder('R53_ALIAS', {
 
 // R53_ZONE(zone_id)
 function R53_ZONE(zone_id) {
-    return function(r) {
-        if (_.isObject(r.r53_alias)) {
+    return function (r) {
+        if (_isDomain(r)) {
+            r.meta.zone_id = zone_id;
+        } else if (_.isObject(r.r53_alias)) {
             r.r53_alias['zone_id'] = zone_id;
         } else {
             r.r53_alias = { zone_id: zone_id };
diff --git a/providers/route53/route53Provider.go b/providers/route53/route53Provider.go
index d467bc22..a40f88ba 100644
--- a/providers/route53/route53Provider.go
+++ b/providers/route53/route53Provider.go
@@ -25,7 +25,8 @@ type route53Provider struct {
 	client          *r53.Route53
 	registrar       *r53d.Route53Domains
 	delegationSet   *string
-	zones           map[string]*r53.HostedZone
+	zonesById       map[string]*r53.HostedZone
+	zonesByDomain   map[string]*r53.HostedZone
 	originalRecords []*r53.ResourceRecordSet
 }
 
@@ -119,7 +120,7 @@ func withRetry(f func() error) {
 func (r *route53Provider) ListZones() ([]string, error) {
 	var zones []string
 	// Assumes r.zones was filled already by newRoute53().
-	for i := range r.zones {
+	for i := range r.zonesByDomain {
 		zones = append(zones, i)
 	}
 	return zones, nil
@@ -127,7 +128,8 @@ func (r *route53Provider) ListZones() ([]string, error) {
 
 func (r *route53Provider) getZones() error {
 	var nextMarker *string
-	r.zones = make(map[string]*r53.HostedZone)
+	r.zonesByDomain = make(map[string]*r53.HostedZone)
+	r.zonesById = make(map[string]*r53.HostedZone)
 	for {
 		var out *r53.ListHostedZonesOutput
 		var err error
@@ -143,7 +145,8 @@ func (r *route53Provider) getZones() error {
 		}
 		for _, z := range out.HostedZones {
 			domain := strings.TrimSuffix(*z.Name, ".")
-			r.zones[domain] = z
+			r.zonesByDomain[domain] = z
+			r.zonesById[parseZoneId(*z.Id)] = z
 		}
 		if out.NextMarker != nil {
 			nextMarker = out.NextMarker
@@ -154,19 +157,27 @@ func (r *route53Provider) getZones() error {
 	return nil
 }
 
-type errNoExist struct {
+type errDomainNoExist struct {
 	domain string
 }
 
-func (e errNoExist) Error() string {
+type errZoneNoExist struct {
+	zoneId string
+}
+
+func (e errDomainNoExist) Error() string {
 	return fmt.Sprintf("Domain %s not found in your route 53 account", e.domain)
 }
 
+func (e errZoneNoExist) Error() string {
+	return fmt.Sprintf("Zone with id %s not found in your route 53 account", e.zoneId)
+}
+
 func (r *route53Provider) GetNameservers(domain string) ([]*models.Nameserver, error) {
 
-	zone, ok := r.zones[domain]
+	zone, ok := r.zonesByDomain[domain]
 	if !ok {
-		return nil, errNoExist{domain}
+		return nil, errDomainNoExist{domain}
 	}
 	var z *r53.GetHostedZoneOutput
 	var err error
@@ -187,14 +198,31 @@ func (r *route53Provider) GetNameservers(domain string) ([]*models.Nameserver, e
 	return models.ToNameservers(nss)
 }
 
-// GetZoneRecords gets the records of a zone and returns them in RecordConfig format.
 func (r *route53Provider) GetZoneRecords(domain string) (models.Records, error) {
+	if zone, ok := r.zonesByDomain[domain]; ok {
+		return r.getZoneRecords(zone)
+	}
 
-	zone, ok := r.zones[domain]
-	if !ok {
-		return nil, errNoExist{domain}
+	return nil, errDomainNoExist{domain}
+}
+
+func (r *route53Provider) getZone(dc *models.DomainConfig) (*r53.HostedZone, error) {
+	if zoneId, ok := dc.Metadata["zone_id"]; ok {
+		zone, ok := r.zonesById[zoneId]
+		if !ok {
+			return nil, errZoneNoExist{zoneId}
+		}
+		return zone, nil
 	}
 
+	if zone, ok := r.zonesByDomain[dc.Name]; ok {
+		return zone, nil
+	}
+
+	return nil, errDomainNoExist{dc.Name}
+}
+
+func (r *route53Provider) getZoneRecords(zone *r53.HostedZone) (models.Records, error) {
 	records, err := r.fetchRecordSets(zone.Id)
 	if err != nil {
 		return nil, err
@@ -203,7 +231,7 @@ func (r *route53Provider) GetZoneRecords(domain string) (models.Records, error)
 
 	var existingRecords = []*models.RecordConfig{}
 	for _, set := range records {
-		rts, err := nativeToRecords(set, domain)
+		rts, err := nativeToRecords(set, unescape(zone.Name))
 		if err != nil {
 			return nil, err
 		}
@@ -217,14 +245,14 @@ func (r *route53Provider) GetDomainCorrections(dc *models.DomainConfig) ([]*mode
 
 	var corrections = []*models.Correction{}
 
-	existingRecords, err := r.GetZoneRecords(dc.Name)
+	zone, err := r.getZone(dc)
 	if err != nil {
 		return nil, err
 	}
 
-	zone, ok := r.zones[dc.Name]
-	if !ok {
-		return nil, errNoExist{dc.Name}
+	existingRecords, err := r.getZoneRecords(zone)
+	if err != nil {
+		return nil, err
 	}
 
 	for _, want := range dc.Records {
@@ -492,6 +520,11 @@ func getZoneID(zone *r53.HostedZone, r *models.RecordConfig) string {
 	if zoneID == "" {
 		zoneID = aws.StringValue(zone.Id)
 	}
+	return parseZoneId(zoneID)
+}
+
+/** Removes "/hostedzone/"" prefix from AWS ZoneId */
+func parseZoneId(zoneID string) string {
 	if strings.HasPrefix(zoneID, "/hostedzone/") {
 		zoneID = strings.TrimPrefix(zoneID, "/hostedzone/")
 	}
@@ -613,7 +646,7 @@ func unescape(s *string) string {
 }
 
 func (r *route53Provider) EnsureDomainExists(domain string) error {
-	if _, ok := r.zones[domain]; ok {
+	if _, ok := r.zonesByDomain[domain]; ok {
 		return nil
 	}
 	if r.delegationSet != nil {


Test patch:
diff --git a/pkg/js/parse_tests/040-r53-zone.js b/pkg/js/parse_tests/040-r53-zone.js
new file mode 100644
index 00000000..5f6b0963
--- /dev/null
+++ b/pkg/js/parse_tests/040-r53-zone.js
@@ -0,0 +1,7 @@
+D('foo.com', 'none', R53_ZONE('Z2FTEDLFRTZ'));
+D(
+    'foo.com!internal',
+    'none',
+    R53_ZONE('Z2FTEDLFRTF'),
+    R53_ALIAS('atest', 'A', 'foo.com.', R53_ZONE('Z2FTEDLFRTZ'))
+);
diff --git a/pkg/js/parse_tests/040-r53-zone.json b/pkg/js/parse_tests/040-r53-zone.json
new file mode 100644
index 00000000..08ab64f2
--- /dev/null
+++ b/pkg/js/parse_tests/040-r53-zone.json
@@ -0,0 +1,34 @@
+{
+  "registrars": [],
+  "dns_providers": [],
+  "domains": [
+    {
+      "name": "foo.com",
+      "registrar": "none",
+      "dnsProviders": {},
+      "records": [],
+      "meta": {
+        "zone_id": "Z2FTEDLFRTZ"
+      }
+    },
+    {
+      "name": "foo.com!internal",
+      "registrar": "none",
+      "dnsProviders": {},
+      "records": [
+        {
+          "type": "R53_ALIAS",
+          "name": "atest",
+          "r53_alias": {
+            "type": "A",
+            "zone_id": "Z2FTEDLFRTZ"
+          },
+          "target": "foo.com."
+        }
+      ],
+      "meta": {
+        "zone_id": "Z2FTEDLFRTF"
+      }
+    }
+  ]
+}