wip!

commands/previewPush.go

@@ -178,6 +178,7 @@ DomainLoop:
 		nameservers.AddNSRecords(domain)
 
 		for _, provider := range providersWithExistingZone {
+			// FIXME(tlim): Test this without the copying.  Just dc := domain
 			dc, err := domain.Copy()
 			if err != nil {
 				return err

pkg/zonerecs/zonerecords.go

@@ -0,0 +1,32 @@
+package zonerecs
+
+import "github.com/StackExchange/dnscontrol/v3/models"
+
+// CorrectZoneRecords calls both GetZoneRecords, does any
+// post-processing, and then calls GetZoneRecordsCorrections.  The
+// name sucks because all the good names were taken.
+func CorrectZoneRecords(driver models.DNSProvider, dc *models.DomainConfig) ([]*models.Correction, error) {
+	existingRecords, err := driver.GetZoneRecords(dc.Name)
+	if err != nil {
+		return nil, err
+	}
+
+	// downcase
+	models.PostProcessRecords(existingRecords)
+
+	// Copy dc so that any corrections code that wants to
+	// modify the records may. For example, if the provider only
+	// supports certain TTL values, it will adjust the ones in
+	// dc.Records.
+	dc, err = dc.Copy()
+	if err != nil {
+		return nil, err
+	}
+
+	// punycode
+	dc.Punycode()
+	// FIXME(tlim) It is a waste to PunyCode every iteration.
+	// This should be moved to where the JavaScript is processed.
+
+	return driver.GetZoneRecordsCorrections(dc, existingRecords)
+}

providers/namedotcom/records.go

@@ -54,6 +54,11 @@ func (n *namedotcomProvider) GetDomainCorrections(dc *models.DomainConfig) ([]*m
 	// Normalize
 	models.PostProcessRecords(actual)
 
+	return n.GetZoneRecordsCorrections(dc, actual)
+}
+
+func (n *namedotcomProvider) GetZoneRecordsCorrections(dc *models.DomainConfig, actual models.Records) ([]*models.Correction, error) {
+
 	var corrections []*models.Correction
 	var differ diff.Differ
 	if !diff2.EnableDiff2 {

providers/netcup/netcupProvider.go

@@ -72,8 +72,23 @@ func (api *netcupProvider) GetDomainCorrections(dc *models.DomainConfig) ([]*mod
 	if err != nil {
 		return nil, err
 	}
-
 	dc.Punycode()
+
+	// Check existing set
+	existingRecords, err := api.GetZoneRecords(dc.Name)
+	if err != nil {
+		return nil, err
+	}
+
+	// Normalize
+	models.PostProcessRecords(existingRecords)
+	// no need for txtutil.SplitSingleLongTxt in function GetDomainCorrections
+	// txtutil.SplitSingleLongTxt(dc.Records) // Autosplit long TXT records
+
+	return api.GetDomainCorrections(dc)
+}
+
+func (api *netcupProvider) GetZoneRecordsCorrections(dc *models.DomainConfig, existingRecords models.Records) ([]*models.Correction, error) {
 	domain := dc.Name
 
 	// Setting the TTL is not supported for netcup
@@ -90,19 +105,9 @@ func (api *netcupProvider) GetDomainCorrections(dc *models.DomainConfig) ([]*mod
 	}
 	dc.Records = newRecords
 
-	// Check existing set
-	existingRecords, err := api.GetZoneRecords(domain)
-	if err != nil {
-		return nil, err
-	}
-
-	// Normalize
-	models.PostProcessRecords(existingRecords)
-	// no need for txtutil.SplitSingleLongTxt in function GetDomainCorrections
-	// txtutil.SplitSingleLongTxt(dc.Records) // Autosplit long TXT records
-
 	var corrections []*models.Correction
 	var create, del, modify diff.Changeset
+	var err error
 	if !diff2.EnableDiff2 {
 		differ := diff.New(dc)
 		_, create, del, modify, err = differ.IncrementalDiff(existingRecords)

providers/netlify/netlifyProvider.go

@@ -191,8 +191,14 @@ func (n *netlifyProvider) GetDomainCorrections(dc *models.DomainConfig) ([]*mode
 	models.PostProcessRecords(records)
 	removeOtherApexNS(dc)
 
+	return n.GetZoneRecordsCorrections(dc, records)
+}
+
+func (n *netlifyProvider) GetZoneRecordsCorrections(dc *models.DomainConfig, records models.Records) ([]*models.Correction, error) {
+
 	var corrections []*models.Correction
 	var create, del, modify diff.Changeset
+	var err error
 	if !diff2.EnableDiff2 {
 		differ := diff.New(dc)
 		_, create, del, modify, err = differ.IncrementalDiff(records)

providers/ns1/ns1Provider.go

@@ -143,7 +143,6 @@ func (n *nsone) GetDomainCorrections(dc *models.DomainConfig) ([]*models.Correct
 	//dc.CombineMXs()
 
 	domain := dc.Name
-	corrections := []*models.Correction{}
 
 	// Get existing records
 	existingRecords, err := n.GetZoneRecords(domain)
@@ -154,6 +153,13 @@ func (n *nsone) GetDomainCorrections(dc *models.DomainConfig) ([]*models.Correct
 	//  Normalize
 	models.PostProcessRecords(existingRecords)
 
+	return n.GetZoneRecordsCorrections(dc, existingRecords)
+}
+
+func (n *nsone) GetZoneRecordsCorrections(dc *models.DomainConfig, existingRecords models.Records) ([]*models.Correction, error) {
+	corrections := []*models.Correction{}
+	domain := dc.Name
+
 	// add DNSSEC-related corrections
 	if dnssecCorrections := n.getDomainCorrectionsDNSSEC(domain, dc.AutoDNSSEC); dnssecCorrections != nil {
 		corrections = append(corrections, dnssecCorrections)

providers/oracle/oracleProvider.go

@@ -222,6 +222,12 @@ func (o *oracleProvider) GetDomainCorrections(dc *models.DomainConfig) ([]*model
 	models.PostProcessRecords(existingRecords)
 	txtutil.SplitSingleLongTxt(dc.Records) // Autosplit long TXT records
 
+	return o.GetZoneRecordsCorrections(dc, existingRecords)
+}
+
+func (o *oracleProvider) GetZoneRecordsCorrections(dc *models.DomainConfig, existingRecords models.Records) ([]*models.Correction, error) {
+	var err error
+
 	// Ensure we don't emit changes for attempted modification of built-in apex NSs
 	for _, rec := range dc.Records {
 		if rec.Type != "NS" {
@@ -293,7 +299,7 @@ func (o *oracleProvider) GetDomainCorrections(dc *models.DomainConfig) ([]*model
 		return []*models.Correction{{
 			Msg: desc,
 			F: func() error {
-				return o.patch(createRecords, deleteRecords, domain)
+				return o.patch(createRecords, deleteRecords, dc.Name)
 			},
 		}}, nil
 	}

providers/ovh/ovhProvider.go

@@ -118,9 +118,7 @@ func (c *ovhProvider) GetZoneRecords(domain string) (models.Records, error) {
 }
 
 func (c *ovhProvider) GetDomainCorrections(dc *models.DomainConfig) ([]*models.Correction, error) {
-	var err error
+	err := dc.Punycode()
-
-	err = dc.Punycode()
 	if err != nil {
 		return nil, err
 	}
@@ -133,7 +131,13 @@ func (c *ovhProvider) GetDomainCorrections(dc *models.DomainConfig) ([]*models.C
 	// Normalize
 	models.PostProcessRecords(actual)
 
+	return c.GetZoneRecordsCorrections(dc, actual)
+}
+
+func (c *ovhProvider) GetZoneRecordsCorrections(dc *models.DomainConfig, actual models.Records) ([]*models.Correction, error) {
+
 	var corrections []*models.Correction
+	var err error
 	if !diff2.EnableDiff2 {
 		corrections, err = c.getDiff1DomainCorrections(dc, actual)
 	} else {

providers/packetframe/api.go

@@ -19,7 +19,7 @@ var defaultNameServerNames = []string{
 	"ns2.packetframe.com",
 }
 
-type zone struct {
+type zoneInfo struct {
 	ID         string   `json:"id"`
 	Zone       string   `json:"zone"`
 	Users      []string `json:"users"`
@@ -28,7 +28,7 @@ type zone struct {
 
 type domainResponse struct {
 	Data struct {
-		Zones []zone `json:"zones"`
+		Zones []zoneInfo `json:"zones"`
 	} `json:"data"`
 	Message string `json:"message"`
 	Success bool   `json:"success"`
@@ -58,7 +58,7 @@ type domainRecord struct {
 }
 
 func (api *packetframeProvider) fetchDomainList() error {
-	api.domainIndex = map[string]zone{}
+	api.domainIndex = map[string]zoneInfo{}
 	dr := &domainResponse{}
 	endpoint := "dns/zones"
 	if err := api.get(endpoint, dr); err != nil {

providers/packetframe/packetframeProvider.go

@@ -19,7 +19,7 @@ type packetframeProvider struct {
 	client      *http.Client
 	baseURL     *url.URL
 	token       string
-	domainIndex map[string]zone
+	domainIndex map[string]zoneInfo
 }
 
 // newPacketframe creates the provider.
@@ -60,19 +60,28 @@ func (api *packetframeProvider) GetNameservers(domain string) ([]*models.Nameser
 	return models.ToNameservers(defaultNameServerNames)
 }
 
-// GetZoneRecords gets the records of a zone and returns them in RecordConfig format.
+func (api *packetframeProvider) getZone(domain string) (*zoneInfo, error) {
-func (api *packetframeProvider) GetZoneRecords(domain string) (models.Records, error) {
-
 	if api.domainIndex == nil {
 		if err := api.fetchDomainList(); err != nil {
 			return nil, err
 		}
 	}
-	zone, ok := api.domainIndex[domain+"."]
+	z, ok := api.domainIndex[domain+"."]
 	if !ok {
 		return nil, fmt.Errorf("%q not a zone in Packetframe account", domain)
 	}
 
+	return &z, nil
+}
+
+// GetZoneRecords gets the records of a zone and returns them in RecordConfig format.
+func (api *packetframeProvider) GetZoneRecords(domain string) (models.Records, error) {
+
+	zone, err := api.getZone(domain)
+	if err != nil {
+		return nil, fmt.Errorf("no such zone %q in Packetframe account", domain)
+	}
+
 	records, err := api.getRecords(zone.ID)
 	if err != nil {
 		return nil, fmt.Errorf("could not load records for domain %q", domain)
@@ -100,13 +109,8 @@ func (api *packetframeProvider) GetDomainCorrections(dc *models.DomainConfig) ([
 
 	dc.Punycode()
 
-	if api.domainIndex == nil {
+	zone, err := api.getZone(dc.Name)
-		if err := api.fetchDomainList(); err != nil {
+	if err != nil {
-			return nil, err
-		}
-	}
-	zone, ok := api.domainIndex[dc.Name+"."]
-	if !ok {
 		return nil, fmt.Errorf("no such zone %q in Packetframe account", dc.Name)
 	}
 
@@ -124,6 +128,15 @@ func (api *packetframeProvider) GetDomainCorrections(dc *models.DomainConfig) ([
 	// Normalize
 	models.PostProcessRecords(existingRecords)
 
+	return api.GetZoneRecordsCorrections(dc, existingRecords)
+}
+
+func (api *packetframeProvider) GetZoneRecordsCorrections(dc *models.DomainConfig, existingRecords models.Records) ([]*models.Correction, error) {
+	zone, err := api.getZone(dc.Name)
+	if err != nil {
+		return nil, fmt.Errorf("no such zone %q in Packetframe account", dc.Name)
+	}
+
 	var corrections []*models.Correction
 	var create, dels, modify diff.Changeset
 	if !diff2.EnableDiff2 {

providers/porkbun/porkbunProvider.go

@@ -88,12 +88,17 @@ func (c *porkbunProvider) GetDomainCorrections(dc *models.DomainConfig) ([]*mode
 		return nil, err
 	}
 
-	// Block changes to NS records for base domain
-	checkNSModifications(dc)
-
 	// Normalize
 	models.PostProcessRecords(existingRecords)
 
+	return c.GetZoneRecordsCorrections(dc, existingRecords)
+}
+
+func (c *porkbunProvider) GetZoneRecordsCorrections(dc *models.DomainConfig, existingRecords models.Records) ([]*models.Correction, error) {
+
+	// Block changes to NS records for base domain
+	checkNSModifications(dc)
+
 	// Make sure TTL larger than the minimum TTL
 	for _, record := range dc.Records {
 		record.TTL = fixTTL(record.TTL)

providers/powerdns/dns.go

@@ -62,8 +62,13 @@ func (dsp *powerdnsProvider) GetDomainCorrections(dc *models.DomainConfig) ([]*m
 	}
 	models.PostProcessRecords(curRecords)
 
+	return dsp.GetZoneRecordsCorrections(dc, curRecords)
+}
+
+func (dsp *powerdnsProvider) GetZoneRecordsCorrections(dc *models.DomainConfig, curRecords models.Records) ([]*models.Correction, error) {
 	// create record diff by group
 	var keysToUpdate map[models.RecordKey][]string
+	var err error
 	if !diff2.EnableDiff2 {
 		keysToUpdate, err = (diff.New(dc)).ChangedGroups(curRecords)
 	} else {

providers/route53/route53Provider.go

@@ -133,6 +133,8 @@ func (r *route53Provider) ListZones() ([]string, error) {
 }
 
 func (r *route53Provider) getZones() error {
+	// TODO(tlim) This should memoize itself.
+
 	if r.zonesByDomain != nil {
 		return nil
 	}
@@ -222,6 +224,8 @@ func (r *route53Provider) GetZoneRecords(domain string) (models.Records, error)
 }
 
 func (r *route53Provider) getZone(dc *models.DomainConfig) (r53Types.HostedZone, error) {
+	// TODO(tlim) This should memoize itself.
+
 	if err := r.getZones(); err != nil {
 		return r53Types.HostedZone{}, err
 	}
@@ -283,9 +287,24 @@ func (r *route53Provider) GetDomainCorrections(dc *models.DomainConfig) ([]*mode
 	models.PostProcessRecords(existingRecords)
 	txtutil.SplitSingleLongTxt(dc.Records) // Autosplit long TXT records
 
+	return r.GetZoneRecordsCorrections(dc, existingRecords)
+}
+
+func (r *route53Provider) GetZoneRecordsCorrections(dc *models.DomainConfig, existingRecords models.Records) ([]*models.Correction, error) {
+
+	zone, err := r.getZone(dc)
+	if err != nil {
+		return nil, err
+	}
+
 	var corrections []*models.Correction
 	if !diff2.EnableDiff2 {
 
+		zone, err := r.getZone(dc)
+		if err != nil {
+			return nil, err
+		}
+
 		// diff
 		differ := diff.New(dc, getAliasMap)
 		namesToUpdate, err := differ.ChangedGroups(existingRecords)