From 06a1cc3d38f1af9a4baf79fba8750dc6b37ae90c Mon Sep 17 00:00:00 2001 From: Jan-Philipp Benecke Date: Mon, 1 Mar 2021 13:09:49 +0100 Subject: [PATCH] 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 | 6 ++++++ integrationTest/providers.json | 7 ++++--- pkg/diff/diff.go | 24 ++++++++++++++++++++++-- providers/powerdns/powerdnsProvider.go | 19 ++++++++++--------- 4 files changed, 42 insertions(+), 14 deletions(-) diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml index 7f5907f6e..d264327c5 100644 --- a/.github/workflows/build.yml +++ b/.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 diff --git a/integrationTest/providers.json b/integrationTest/providers.json index f811fdf85..588b11cfb 100644 --- a/integrationTest/providers.json +++ b/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", diff --git a/pkg/diff/diff.go b/pkg/diff/diff.go index 9c6d258c2..b065f7918 100644 --- a/pkg/diff/diff.go +++ b/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. diff --git a/providers/powerdns/powerdnsProvider.go b/providers/powerdns/powerdnsProvider.go index e5efcea56..52740b775 100644 --- a/providers/powerdns/powerdnsProvider.go +++ b/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) }