PowerDNS: fix order problems of delete corrections & some cleanup (#1153)

Signed-off-by: Jan-Philipp Benecke <jan-philipp.benecke@jpbe.de>

pkg/diff/diff.go

@@ -27,8 +27,6 @@ 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.
@@ -315,24 +313,6 @@ 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/dnssec.go

@@ -9,7 +9,7 @@ import (
 
 // getDNSSECCorrections returns corrections that update a domain's DNSSEC state.
 func (api *powerdnsProvider) getDNSSECCorrections(dc *models.DomainConfig) ([]*models.Correction, error) {
-	cryptokeys, getErr := api.client.Cryptokeys().ListCryptokeys(context.Background(), api.ServerName, dc.Name)
+	zoneCryptokeys, getErr := api.client.Cryptokeys().ListCryptokeys(context.Background(), api.ServerName, dc.Name)
 	if getErr != nil {
 		return nil, getErr
 	}
@@ -18,8 +18,8 @@ func (api *powerdnsProvider) getDNSSECCorrections(dc *models.DomainConfig) ([]*m
 	hasEnabledKey := false
 	var keyID int
 
-	if len(cryptokeys) > 0 {
-		for _, cryptoKey := range cryptokeys {
+	if len(zoneCryptokeys) > 0 {
+		for _, cryptoKey := range zoneCryptokeys {
 			if cryptoKey.Active && cryptoKey.Published {
 				hasEnabledKey = true
 				keyID = cryptoKey.ID

providers/powerdns/powerdnsProvider.go

@@ -4,9 +4,6 @@ import (
 	"context"
 	"encoding/json"
 	"fmt"
-	"net/http"
-	"strings"
-
 	"github.com/StackExchange/dnscontrol/v3/models"
 	"github.com/StackExchange/dnscontrol/v3/pkg/diff"
 	"github.com/StackExchange/dnscontrol/v3/providers"
@@ -14,6 +11,8 @@ import (
 	pdns "github.com/mittwald/go-powerdns"
 	"github.com/mittwald/go-powerdns/apis/zones"
 	"github.com/mittwald/go-powerdns/pdnshttp"
+	"net/http"
+	"strings"
 )
 
 var features = providers.DocumentationNotes{
@@ -107,11 +106,11 @@ func (api *powerdnsProvider) GetNameservers(string) ([]*models.Nameserver, error
 // ListZones returns all the zones in an account
 func (api *powerdnsProvider) ListZones() ([]string, error) {
 	var result []string
-	zones, err := api.client.Zones().ListZones(context.Background(), api.ServerName)
+	myZones, err := api.client.Zones().ListZones(context.Background(), api.ServerName)
 	if err != nil {
 		return result, err
 	}
-	for _, zone := range zones {
+	for _, zone := range myZones {
 		result = append(result, zone.Name)
 	}
 	return result, nil
@@ -147,59 +146,71 @@ func (api *powerdnsProvider) GetZoneRecords(domain string) (models.Records, erro
 func (api *powerdnsProvider) GetDomainCorrections(dc *models.DomainConfig) ([]*models.Correction, error) {
 	var corrections []*models.Correction
 
-	// record corrections
+	// get current zone records
 	curRecords, err := api.GetZoneRecords(dc.Name)
 	if err != nil {
 		return nil, err
 	}
 
 	// post-process records
-	dc.Punycode()
+	if err := dc.Punycode(); err != nil {
+		return nil, err
+	}
 	models.PostProcessRecords(curRecords)
 
 	// create record diff by group
-	keysToUpdate, err := (diff.New(dc)).ChangedGroupsDeleteFirst(curRecords)
+	keysToUpdate, err := (diff.New(dc)).ChangedGroups(curRecords)
 	if err != nil {
 		return nil, err
 	}
 	desiredRecords := dc.Records.GroupedByKey()
 
-	// create corrections by group
+	var cuCorrections []*models.Correction
+	var dCorrections []*models.Correction
+
+	// add create/update and delete corrections separately
 	for label, msgs := range keysToUpdate {
 		labelName := label.NameFQDN + "."
 		labelType := label.Type
+		msgJoined := strings.Join(msgs, "\n   ")
 
 		if _, ok := desiredRecords[label]; !ok {
-			// nothing found, must be a delete
-			corrections = append(corrections, &models.Correction{
-				Msg: strings.Join(msgs, "\n   "),
+			// no record found so delete it
+			dCorrections = append(dCorrections, &models.Correction{
+				Msg: msgJoined,
 				F: func() error {
 					return api.client.Zones().RemoveRecordSetFromZone(context.Background(), api.ServerName, dc.Name, labelName, labelType)
 				},
 			})
 		} else {
-			// needs to be a create or update
+			// record found so create or update it
 			ttl := desiredRecords[label][0].TTL
-			records := []zones.Record{}
+			var records []zones.Record
 			for _, recordContent := range desiredRecords[label] {
 				records = append(records, zones.Record{
 					Content: recordContent.GetTargetCombined(),
 				})
 			}
-			corrections = append(corrections, &models.Correction{
-				Msg: strings.Join(msgs, "\n   "),
+			cuCorrections = append(cuCorrections, &models.Correction{
+				Msg: msgJoined,
 				F: func() error {
 					return api.client.Zones().AddRecordSetToZone(context.Background(), api.ServerName, dc.Name, zones.ResourceRecordSet{
 						Name:       labelName,
 						Type:       labelType,
 						TTL:        int(ttl),
 						Records:    records,
+						ChangeType: zones.ChangeTypeReplace,
 					})
 				},
 			})
 		}
 	}
 
+	// append corrections in the right order
+	// delete corrections must be run first to avoid correlations with existing RR
+	corrections = append(corrections, dCorrections...)
+	corrections = append(corrections, cuCorrections...)
+
 	// DNSSec corrections
 	dnssecCorrections, err := api.getDNSSECCorrections(dc)
 	if err != nil {