POWERDNS: Some minor fixes for ALIAS and TXTMulti and integration testing (#1065)

* POWERDNS: Some minor fixes for ALIAS and integration testing
* POWERDNS: Readd missing error handling

.github/workflows/build.yml

@@ -45,6 +45,7 @@ jobs:
         - DIGITALOCEAN
         - GANDI_V5
         - INWX
+        - POWERDNS
 # Bring-Your-Own-Secrets:
 # To reduce the risk of secrets being logged by third-parties, secrets
 # come from the account of the fork.  For example, the PR submitted by
@@ -108,6 +109,11 @@ jobs:
       INWX_DOMAIN: ${{ secrets.INWX_DOMAIN }}
       INWX_PASSWORD: ${{ secrets.INWX_PASSWORD }}
       INWX_USER: ${{ secrets.INWX_USER }}
+#
+      POWERDNS_DOMAIN: ${{ secrets.POWERDNS_DOMAIN }}
+      POWERDNS_APIURL: ${{ secrets.POWERDNS_APIURL }}
+      POWERDNS_APIKEY: ${{ secrets.POWERDNS_APIKEY }}
+      POWERDNS_SERVERNAME: ${{ secrets.POWERDNS_SERVERNAME }}
     steps:
     - name: Checkout repo
       uses: actions/checkout@v2

integrationTest/providers.json

@@ -142,9 +142,10 @@
     "domain": "$OVH_DOMAIN"
   },
   "POWERDNS": {
-    "apikey": "$POWERDNS_APIKEY",
-    "apiurl": "$POWERDNS_APIURL",
-    "servername": "$POWERDNS_SERVERNAME"
+    "apiKey": "$POWERDNS_APIKEY",
+    "apiUrl": "$POWERDNS_APIURL",
+    "serverName": "$POWERDNS_SERVERNAME",
+    "domain": "$POWERDNS_DOMAIN"
   },
   "ROUTE53": {
     "KeyId": "$ROUTE53_KEY_ID",

pkg/diff/diff.go

@@ -27,6 +27,8 @@ type Differ interface {
 	// ChangedGroups performs a diff more appropriate for providers with a "RecordSet" model, where all records with the same name and type are grouped.
 	// Individual record changes are often not useful in such scenarios. Instead we return a map of record keys to a list of change descriptions within that group.
 	ChangedGroups(existing []*models.RecordConfig) (map[models.RecordKey][]string, error)
+	// ChangedGroupsDeleteFirst is the same as ChangedGroups but it sorts the deletions to the first postion
+	ChangedGroupsDeleteFirst(existing []*models.RecordConfig) (map[models.RecordKey][]string, error)
 }
 
 // New is a constructor for a Differ.
@@ -262,14 +264,14 @@ func CorrectionLess(c []*models.Correction, i, j int) bool {
 
 func (d *differ) ChangedGroups(existing []*models.RecordConfig) (map[models.RecordKey][]string, error) {
 	changedKeys := map[models.RecordKey][]string{}
-	_, create, delete, modify, err := d.IncrementalDiff(existing)
+	_, create, toDelete, modify, err := d.IncrementalDiff(existing)
 	if err != nil {
 		return nil, err
 	}
 	for _, c := range create {
 		changedKeys[c.Desired.Key()] = append(changedKeys[c.Desired.Key()], c.String())
 	}
-	for _, d := range delete {
+	for _, d := range toDelete {
 		changedKeys[d.Existing.Key()] = append(changedKeys[d.Existing.Key()], d.String())
 	}
 	for _, m := range modify {
@@ -278,6 +280,24 @@ func (d *differ) ChangedGroups(existing []*models.RecordConfig) (map[models.Reco
 	return changedKeys, nil
 }
 
+func (d *differ) ChangedGroupsDeleteFirst(existing []*models.RecordConfig) (map[models.RecordKey][]string, error) {
+	changedKeys := map[models.RecordKey][]string{}
+	_, create, toDelete, modify, err := d.IncrementalDiff(existing)
+	if err != nil {
+		return nil, err
+	}
+	for _, d := range toDelete {
+		changedKeys[d.Existing.Key()] = append(changedKeys[d.Existing.Key()], d.String())
+	}
+	for _, c := range create {
+		changedKeys[c.Desired.Key()] = append(changedKeys[c.Desired.Key()], c.String())
+	}
+	for _, m := range modify {
+		changedKeys[m.Desired.Key()] = append(changedKeys[m.Desired.Key()], m.String())
+	}
+	return changedKeys, nil
+}
+
 // DebugKeyMapMap debug prints the results from ChangedGroups.
 func DebugKeyMapMap(note string, m map[models.RecordKey][]string) {
 	// The output isn't pretty but it is useful.

providers/powerdns/powerdnsProvider.go

@@ -3,7 +3,6 @@ package powerdns
 import (
 	"context"
 	"encoding/json"
-	"errors"
 	"fmt"
 	"net/http"
 	"strings"
@@ -18,7 +17,7 @@ import (
 )
 
 var features = providers.DocumentationNotes{
-	providers.CanUseAlias:            providers.Can(),
+	providers.CanUseAlias:            providers.Can("Needs to be enabled in PowerDNS first", "https://doc.powerdns.com/authoritative/guides/alias.html"),
 	providers.CanUseCAA:              providers.Can(),
 	providers.CanUsePTR:              providers.Can(),
 	providers.CanUseSRV:              providers.Can(),
@@ -28,6 +27,9 @@ var features = providers.DocumentationNotes{
 	providers.DocCreateDomains:       providers.Can(),
 	providers.DocOfficiallySupported: providers.Cannot(),
 	providers.CanGetZones:            providers.Can(),
+	providers.CanUseTXTMulti:         providers.Can(),
+	providers.DocDualHost:            providers.Can(),
+	providers.CanUseNAPTR:            providers.Can(),
 }
 
 func init() {
@@ -153,7 +155,7 @@ func (api *powerdnsProvider) GetDomainCorrections(dc *models.DomainConfig) ([]*m
 	models.PostProcessRecords(curRecords)
 
 	// create record diff by group
-	keysToUpdate, err := (diff.New(dc)).ChangedGroups(curRecords)
+	keysToUpdate, err := (diff.New(dc)).ChangedGroupsDeleteFirst(curRecords)
 	if err != nil {
 		return nil, err
 	}
@@ -173,6 +175,7 @@ func (api *powerdnsProvider) GetDomainCorrections(dc *models.DomainConfig) ([]*m
 				},
 			})
 		} else {
+			// needs to be a create or update
 			ttl := desiredRecords[label][0].TTL
 			records := []zones.Record{}
 			for _, recordContent := range desiredRecords[label] {
@@ -240,6 +243,8 @@ func toRecordConfig(domain string, r zones.Record, ttl int, name string, rtype s
 
 	content := r.Content
 	switch rtype {
+	case "ALIAS":
+		return rc, rc.SetTarget(r.Content)
 	case "CNAME", "NS":
 		return rc, rc.SetTarget(dnsutil.AddOrigin(content, domain))
 	case "CAA":
@@ -248,12 +253,8 @@ func toRecordConfig(domain string, r zones.Record, ttl int, name string, rtype s
 		return rc, rc.SetTargetMXString(content)
 	case "SRV":
 		return rc, rc.SetTargetSRVString(content)
-	case "TXT":
-		// Remove quotes if it is a TXT record.
-		if !strings.HasPrefix(content, `"`) || !strings.HasSuffix(content, `"`) {
-			return nil, errors.New("unexpected lack of quotes in TXT record from PowerDNS")
-		}
-		return rc, rc.SetTargetTXT(content[1 : len(content)-1])
+	case "NAPTR":
+		return rc, rc.SetTargetNAPTRString(content)
 	default:
 		return rc, rc.PopulateFromString(rtype, content, domain)
 	}