Bugfix: Providers with batched updates might not report the correct n…

…umber of changes (#3108)

.github/workflows/pr_test.yml

@@ -96,7 +96,7 @@ jobs:
     if: github.ref != 'refs/heads/master' && github.ref != 'refs/heads/main'
     runs-on: ubuntu-latest
     container:
-      image: golang:1.22
+      image: golang:1.23
     needs:
     - integration-test-providers
     env:

.github/workflows/release_draft.yml

@@ -42,7 +42,7 @@ jobs:
     - name: Set up Go
       uses: actions/setup-go@v5
       with:
-        go-version: ^1.22
+        go-version: ^1.23
 
 # Stringer is needed because .goreleaser includes "go generate ./..."
     - name: Install stringer

commands/ppreviewPush.go

@@ -240,7 +240,7 @@ func prun(args PPreviewArgs, push bool, interactive bool, out printer.CLI, repor
 			out.StartDNSProvider(provider.Name, skip)
 			if !skip {
 				corrections := zone.GetCorrections(provider.Name)
-				numActions := countActions(corrections)
+				numActions := zone.GetChangeCount(provider.Name)
 				totalCorrections += numActions
 				out.EndProvider2(provider.Name, numActions)
 				reportItems = append(reportItems, genReportItem(zone.Name, corrections, provider.Name))
@@ -253,7 +253,7 @@ func prun(args PPreviewArgs, push bool, interactive bool, out printer.CLI, repor
 		out.StartRegistrar(zone.RegistrarName, !skip)
 		if skip {
 			corrections := zone.GetCorrections(zone.RegistrarInstance.Name)
-			numActions := countActions(corrections)
+			numActions := zone.GetChangeCount(zone.RegistrarInstance.Name)
 			out.EndProvider2(zone.RegistrarName, numActions)
 			totalCorrections += numActions
 			reportItems = append(reportItems, genReportItem(zone.Name, corrections, zone.RegistrarName))
@@ -281,15 +281,15 @@ func prun(args PPreviewArgs, push bool, interactive bool, out printer.CLI, repor
 	return nil
 }
 
-func countActions(corrections []*models.Correction) int {
-	r := 0
-	for _, c := range corrections {
-		if c.F != nil {
-			r++
-		}
-	}
-	return r
-}
+//func countActions(corrections []*models.Correction) int {
+//	r := 0
+//	for _, c := range corrections {
+//		if c.F != nil {
+//			r++
+//		}
+//	}
+//	return r
+//}
 
 func whichZonesToProcess(domains []*models.DomainConfig, filter string) []*models.DomainConfig {
 	if filter == "" || filter == "all" {
@@ -364,7 +364,7 @@ func optimizeOrder(zones []*models.DomainConfig) []*models.DomainConfig {
 func oneZone(zone *models.DomainConfig, args PPreviewArgs, zc *zoneCache) {
 	// Fix the parent zone's delegation: (if able/needed)
 	//zone.NameserversMutex.Lock()
-	delegationCorrections := generateDelegationCorrections(zone, zone.DNSProviderInstances, zone.RegistrarInstance)
+	delegationCorrections, dcCount := generateDelegationCorrections(zone, zone.DNSProviderInstances, zone.RegistrarInstance)
 	//zone.NameserversMutex.Unlock()
 
 	// Loop over the (selected) providers configured for that zone:
@@ -378,13 +378,15 @@ func oneZone(zone *models.DomainConfig, args PPreviewArgs, zc *zoneCache) {
 		}
 
 		// Update the zone's records at the provider:
-		zoneCor, rep := generateZoneCorrections(zone, provider)
+		zoneCor, rep, actualChangeCount := generateZoneCorrections(zone, provider)
 		zone.StoreCorrections(provider.Name, rep)
 		zone.StoreCorrections(provider.Name, zoneCor)
+		zone.IncrementChangeCount(provider.Name, actualChangeCount)
 	}
 
 	// Do the delegation corrections after the zones are updated.
 	zone.StoreCorrections(zone.RegistrarInstance.Name, delegationCorrections)
+	zone.IncrementChangeCount(zone.RegistrarInstance.Name, dcCount)
 }
 
 func whichProvidersToProcess(providers []*models.DNSProviderInstance, filter string) []*models.DNSProviderInstance {
@@ -533,32 +535,32 @@ func generatePopulateCorrections(provider *models.DNSProviderInstance, zoneName
 	}}
 }
 
-func generateZoneCorrections(zone *models.DomainConfig, provider *models.DNSProviderInstance) ([]*models.Correction, []*models.Correction) {
-	reports, zoneCorrections, err := zonerecs.CorrectZoneRecords(provider.Driver, zone)
+func generateZoneCorrections(zone *models.DomainConfig, provider *models.DNSProviderInstance) ([]*models.Correction, []*models.Correction, int) {
+	reports, zoneCorrections, actualChangeCount, err := zonerecs.CorrectZoneRecords(provider.Driver, zone)
 	if err != nil {
-		return []*models.Correction{{Msg: fmt.Sprintf("Domain %q provider %s Error: %s", zone.Name, provider.Name, err)}}, nil
+		return []*models.Correction{{Msg: fmt.Sprintf("Domain %q provider %s Error: %s", zone.Name, provider.Name, err)}}, nil, 0
 	}
-	return zoneCorrections, reports
+	return zoneCorrections, reports, actualChangeCount
 }
 
-func generateDelegationCorrections(zone *models.DomainConfig, providers []*models.DNSProviderInstance, _ *models.RegistrarInstance) []*models.Correction {
+func generateDelegationCorrections(zone *models.DomainConfig, providers []*models.DNSProviderInstance, _ *models.RegistrarInstance) ([]*models.Correction, int) {
 	//fmt.Printf("DEBUG: generateDelegationCorrections start zone=%q nsList = %v\n", zone.Name, zone.Nameservers)
 	nsList, err := nameservers.DetermineNameserversForProviders(zone, providers, true)
 	if err != nil {
-		return msg(fmt.Sprintf("DtermineNS: zone %q; Error: %s", zone.Name, err))
+		return msg(fmt.Sprintf("DetermineNS: zone %q; Error: %s", zone.Name, err)), 0
 	}
 	zone.Nameservers = nsList
 	nameservers.AddNSRecords(zone)
 
 	if len(zone.Nameservers) == 0 && zone.Metadata["no_ns"] != "true" {
-		return []*models.Correction{{Msg: fmt.Sprintf("No nameservers declared for domain %q; skipping registrar. Add {no_ns:'true'} to force", zone.Name)}}
+		return []*models.Correction{{Msg: fmt.Sprintf("No nameservers declared for domain %q; skipping registrar. Add {no_ns:'true'} to force", zone.Name)}}, 0
 	}
 
 	corrections, err := zone.RegistrarInstance.Driver.GetRegistrarCorrections(zone)
 	if err != nil {
-		return msg(fmt.Sprintf("zone %q; Rprovider %q; Error: %s", zone.Name, zone.RegistrarInstance.Name, err))
+		return msg(fmt.Sprintf("zone %q; Rprovider %q; Error: %s", zone.Name, zone.RegistrarInstance.Name, err)), 0
 	}
-	return corrections
+	return corrections, len(corrections)
 }
 
 func msg(s string) []*models.Correction {

commands/previewPush.go

@@ -247,17 +247,17 @@ func run(args PreviewArgs, push bool, interactive bool, out printer.CLI, report
 					continue
 				}
 
-				reports, corrections, err := zonerecs.CorrectZoneRecords(provider.Driver, domain)
-				out.EndProvider(provider.Name, len(corrections), err)
+				reports, corrections, actualChangeCount, err := zonerecs.CorrectZoneRecords(provider.Driver, domain)
+				out.EndProvider(provider.Name, actualChangeCount, err)
 				if err != nil {
 					anyErrors = true
 					return
 				}
-				totalCorrections += len(corrections)
+				totalCorrections += actualChangeCount
 				printReports(domain.Name, provider.Name, reports, out, push, notifier)
 				reportItems = append(reportItems, ReportItem{
 					Domain:      domain.Name,
-					Corrections: len(corrections),
+					Corrections: actualChangeCount,
 					Provider:    provider.Name,
 				})
 				anyErrors = printOrRunCorrections(domain.Name, provider.Name, corrections, out, push, interactive, notifier) || anyErrors

go.mod

@@ -1,6 +1,6 @@
 module github.com/StackExchange/dnscontrol/v4
 
-go 1.22.1
+go 1.23.0
 
 retract v4.8.0
 

integrationTest/integration_test.go

@@ -228,13 +228,13 @@ func makeChanges(t *testing.T, prv providers.DNSServiceProvider, dc *models.Doma
 		}
 
 		// get and run corrections for first time
-		_, corrections, err := zonerecs.CorrectZoneRecords(prv, dom)
+		_, corrections, actualChangeCount, err := zonerecs.CorrectZoneRecords(prv, dom)
 		if err != nil {
 			t.Fatal(fmt.Errorf("runTests: %w", err))
 		}
 		if tst.Changeless {
-			if count := len(corrections); count != 0 {
-				t.Logf("Expected 0 corrections on FIRST run, but found %d.", count)
+			if actualChangeCount != 0 {
+				t.Logf("Expected 0 corrections on FIRST run, but found %d.", actualChangeCount)
 				for i, c := range corrections {
 					t.Logf("UNEXPECTED #%d: %s", i, c.Msg)
 				}
@@ -261,12 +261,12 @@ func makeChanges(t *testing.T, prv providers.DNSServiceProvider, dc *models.Doma
 		}
 
 		// run a second time and expect zero corrections
-		_, corrections, err = zonerecs.CorrectZoneRecords(prv, dom2)
+		_, corrections, actualChangeCount, err = zonerecs.CorrectZoneRecords(prv, dom2)
 		if err != nil {
 			t.Fatal(err)
 		}
-		if count := len(corrections); count != 0 {
-			t.Logf("Expected 0 corrections on second run, but found %d.", count)
+		if actualChangeCount != 0 {
+			t.Logf("Expected 0 corrections on second run, but found %d.", actualChangeCount)
 			for i, c := range corrections {
 				t.Logf("UNEXPECTED #%d: %s", i, c.Msg)
 			}
@@ -353,7 +353,7 @@ func TestDualProviders(t *testing.T) {
 	run := func() {
 		dom, _ := dc.Copy()
 
-		rs, cs, err := zonerecs.CorrectZoneRecords(p, dom)
+		rs, cs, _, err := zonerecs.CorrectZoneRecords(p, dom)
 		if err != nil {
 			t.Fatal(err)
 		}
@@ -378,12 +378,12 @@ func TestDualProviders(t *testing.T) {
 	run()
 	// run again to make sure no corrections
 	t.Log("Running again to ensure stability")
-	rs, cs, err := zonerecs.CorrectZoneRecords(p, dc)
+	rs, cs, actualChangeCount, err := zonerecs.CorrectZoneRecords(p, dc)
 	if err != nil {
 		t.Fatal(err)
 	}
-	if count := len(cs); count != 0 {
-		t.Logf("Expect no corrections on second run, but found %d.", count)
+	if actualChangeCount != 0 {
+		t.Logf("Expect no corrections on second run, but found %d.", actualChangeCount)
 		for i, c := range rs {
 			t.Logf("INFO#%d:\n%s", i+1, c.Msg)
 		}

models/domain.go

@@ -49,9 +49,10 @@ type DomainConfig struct {
 	RawRecords []RawRecordConfig `json:"rawrecords,omitempty"`
 
 	// Pending work to do for each provider.  Provider may be a registrar or DSP.
-	pendingCorrectionsMutex sync.Mutex                 // Protect pendingCorrections*
-	pendingCorrections      map[string]([]*Correction) // Work to be done for each provider
-	pendingCorrectionsOrder []string                   // Call the providers in this order
+	pendingCorrectionsMutex  sync.Mutex                 // Protect pendingCorrections*
+	pendingCorrections       map[string]([]*Correction) // Work to be done for each provider
+	pendingCorrectionsOrder  []string                   // Call the providers in this order
+	pendingActualChangeCount map[string](int)           // Number of changes to report (cumulative)
 }
 
 // GetSplitHorizonNames returns the domain's name, uniquename, and tag.
@@ -176,3 +177,23 @@ func (dc *DomainConfig) GetCorrections(providerName string) []*Correction {
 	}
 	return nil
 }
+
+// IncrementChangeCount accumulates change count in a thread-safe way.
+func (dc *DomainConfig) IncrementChangeCount(providerName string, delta int) {
+	dc.pendingCorrectionsMutex.Lock()
+	defer dc.pendingCorrectionsMutex.Unlock()
+
+	if dc.pendingActualChangeCount == nil {
+		// First time storing anything.
+		dc.pendingActualChangeCount = make(map[string](int))
+	}
+	dc.pendingActualChangeCount[providerName] += delta
+}
+
+// GetChangeCount accumulates change count in a thread-safe way.
+func (dc *DomainConfig) GetChangeCount(providerName string) int {
+	dc.pendingCorrectionsMutex.Lock()
+	defer dc.pendingCorrectionsMutex.Unlock()
+
+	return dc.pendingActualChangeCount[providerName]
+}

models/provider.go

@@ -4,7 +4,7 @@ package models
 type DNSProvider interface {
 	GetNameservers(domain string) ([]*Nameserver, error)
 	GetZoneRecords(domain string, meta map[string]string) (Records, error)
-	GetZoneRecordsCorrections(dc *DomainConfig, existing Records) ([]*Correction, error)
+	GetZoneRecordsCorrections(dc *DomainConfig, existing Records) ([]*Correction, int, error)
 }
 
 // Registrar is an interface for Registrar plug-ins.

pkg/acme/acme.go

@@ -276,7 +276,7 @@ func (c *certManager) getCorrections(d *models.DomainConfig) ([]*models.Correcti
 		if err != nil {
 			return nil, err
 		}
-		reports, corrections, err := zonerecs.CorrectZoneRecords(p.Driver, dc)
+		reports, corrections, _, err := zonerecs.CorrectZoneRecords(p.Driver, dc)
 		if err != nil {
 			return nil, err
 		}

pkg/diff/diff.go

@@ -20,10 +20,10 @@ type Changeset []Correlation
 // Differ is an interface for computing the difference between two zones.
 type Differ interface {
 	// IncrementalDiff performs a diff on a record-by-record basis, and returns a sets for which records need to be created, deleted, or modified.
-	IncrementalDiff(existing []*models.RecordConfig) (reportMsgs []string, create, toDelete, modify Changeset, err error)
+	IncrementalDiff(existing []*models.RecordConfig) (reportMsgs []string, create, toDelete, modify Changeset, actualChangeCount int, err error)
 	// 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, []string, error)
+	ChangedGroups(existing []*models.RecordConfig) (map[models.RecordKey][]string, []string, int, error)
 }
 
 type differ struct {

pkg/diff/diff2compat.go

@@ -32,10 +32,10 @@ type differCompat struct {
 
 // IncrementalDiff usees pkg/diff2 to generate output compatible with systems
 // still using NewCompat().
-func (d *differCompat) IncrementalDiff(existing []*models.RecordConfig) (reportMsgs []string, toCreate, toDelete, toModify Changeset, err error) {
-	instructions, err := diff2.ByRecord(existing, d.dc, nil)
+func (d *differCompat) IncrementalDiff(existing []*models.RecordConfig) (reportMsgs []string, toCreate, toDelete, toModify Changeset, actualChangeCount int, err error) {
+	instructions, actualChangeCount, err := diff2.ByRecord(existing, d.dc, nil)
 	if err != nil {
-		return nil, nil, nil, nil, err
+		return nil, nil, nil, nil, 0, err
 	}
 
 	for _, inst := range instructions {
@@ -71,11 +71,11 @@ func GenerateMessageCorrections(msgs []string) (corrections []*models.Correction
 }
 
 // ChangedGroups provides the same results as IncrementalDiff but grouped by key.
-func (d *differCompat) ChangedGroups(existing []*models.RecordConfig) (map[models.RecordKey][]string, []string, error) {
+func (d *differCompat) ChangedGroups(existing []*models.RecordConfig) (map[models.RecordKey][]string, []string, int, error) {
 	changedKeys := map[models.RecordKey][]string{}
-	toReport, toCreate, toDelete, toModify, err := d.IncrementalDiff(existing)
+	toReport, toCreate, toDelete, toModify, actualChangeCount, err := d.IncrementalDiff(existing)
 	if err != nil {
-		return nil, nil, err
+		return nil, nil, 0, err
 	}
 	for _, c := range toCreate {
 		changedKeys[c.Desired.Key()] = append(changedKeys[c.Desired.Key()], c.String())
@@ -86,5 +86,5 @@ func (d *differCompat) ChangedGroups(existing []*models.RecordConfig) (map[model
 	for _, m := range toModify {
 		changedKeys[m.Desired.Key()] = append(changedKeys[m.Desired.Key()], m.String())
 	}
-	return changedKeys, toReport, nil
+	return changedKeys, toReport, actualChangeCount, nil
 }