CNR: Initial Performance improvement; golint review (#3391)

Author: KaiSchwarz-cnic

documentation/provider/cnr.md

@@ -19,7 +19,11 @@ Example:
     "apilogin": "your-cnr-account-id",
     "apipassword": "your-cnr-account-password",
     "apientity": "LIVE", // for the LIVE system; use "OTE" for the OT&E system
-    "debugmode": "0", // set it to "1" to get debug output of the communication with our Backend System API
+    // --- debugmode ---
+    // "0" -> turned off (default)
+    // "1" -> turned on, basic logging of the changes reflected as API command parameters for the CNR API
+    // "2" -> turned on, most verbose level - showing the detailed CNR API communication
+    "debugmode": "0"
   }
 }
 ```
@@ -46,23 +50,27 @@ Here a working example for our OT&E System:
 {% endhint %}
 
 With the above CentralNic Reseller entry in `creds.json`, you can run the
-integration tests as follows:
+integration tests or by specifying the data per environment vars as follows:
 
-```shell
-dnscontrol get-zones --format=nameonly cnr CNR all
-```
 ```shell
 # Review the output.  Pick one domain and set CNR_DOMAIN.
 export CNR_DOMAIN=yodream.com            # Pick a domain name.
 export CNR_ENTITY=OTE
 export CNR_UID=test.user
 export CNR_PW=test.passw0rd
+export CNR_DEBUGMODE=2
 cd integrationTest              # NOTE: Not needed if already in that subdirectory
 go test -v -verbose -profile CNR
 ```
 
 ## Usage
 
+Fetch a list of all DNSZones:
+
+```shell
+dnscontrol get-zones --format=nameonly cnr CNR all
+```
+
 Here's an example DNS Configuration `dnsconfig.js` using our provider module.
 Even though it shows how you use us as Domain Registrar AND DNS Provider, we don't force you to do that.
 You are free to decide if you want to use both of our provider technology or just one of them.

providers/cnr/cnrProvider.go

@@ -15,8 +15,8 @@ var (
 	version = "dev"
 )
 
-// CNRClient describes a connection to the CNR API.
-type CNRClient struct {
+// Client describes a connection to the CNR API.
+type Client struct {
 	conf        map[string]string
 	APILogin    string
 	APIPassword string
@@ -56,8 +56,8 @@ var features = providers.DocumentationNotes{
 	providers.CanUseTLSA:  providers.Can(),
 }
 
-func newProvider(conf map[string]string) (*CNRClient, error) {
-	api := &CNRClient{
+func newProvider(conf map[string]string) (*Client, error) {
+	api := &Client{
 		conf:   conf,
 		client: cnrcl.NewAPIClient(),
 	}

providers/cnr/domains.go

@@ -3,36 +3,25 @@ package cnr
 // EnsureZoneExists returns an error
 // * if access to dnszone is not allowed (not authorized) or
 // * if it doesn't exist and creating it fails
-func (n *CNRClient) EnsureZoneExists(domain string) error {
-	r := n.client.Request(map[string]interface{}{
-		"COMMAND": "StatusDNSZone",
+func (n *Client) EnsureZoneExists(domain string) error {
+	command := map[string]interface{}{
+		"COMMAND": "AddDNSZone",
 		"DNSZONE": domain,
-	})
-	code := r.GetCode()
-	if code == 545 {
-		command := map[string]interface{}{
-			"COMMAND": "AddDNSZone",
-			"DNSZONE": domain,
-		}
-		if n.APIEntity == "OTE" {
-			command["SOATTL"] = "33200"
-			command["SOASERIAL"] = "0000000000"
-		}
-		// Create the zone
-		r = n.client.Request(command)
-		if !r.IsSuccess() {
-			return n.GetCNRApiError("Failed to create not existing zone ", domain, r)
-		}
-	} else if code == 531 {
-		return n.GetCNRApiError("Not authorized to manage dnszone", domain, r)
-	} else if r.IsError() || r.IsTmpError() {
-		return n.GetCNRApiError("Error while checking status of dnszone", domain, r)
 	}
-	return nil
+	if n.APIEntity == "OTE" {
+		command["SOATTL"] = "33200"
+		command["SOASERIAL"] = "0000000000"
+	}
+	// Create the zone
+	r := n.client.Request(command)
+	if r.GetCode() == 549 || r.IsSuccess() {
+		return nil
+	}
+	return n.GetAPIError("Failed to create not existing zone ", domain, r)
 }
 
 // ListZones lists all the
-func (n *CNRClient) ListZones() ([]string, error) {
+func (n *Client) ListZones() ([]string, error) {
 	var zones []string
 
 	// Basic
@@ -42,7 +31,7 @@ func (n *CNRClient) ListZones() ([]string, error) {
 	})
 	for _, r := range rs {
 		if r.IsError() {
-			return nil, n.GetCNRApiError("Error while QueryDNSZoneList", "Basic", &r)
+			return nil, n.GetAPIError("Error while QueryDNSZoneList", "Basic", &r)
 		}
 		zoneColumn := r.GetColumn("DNSZONE")
 		if zoneColumn != nil {

providers/cnr/error.go

@@ -6,7 +6,7 @@ import (
 	"github.com/centralnicgroup-opensource/rtldev-middleware-go-sdk/v5/response"
 )
 
-// GetCNRApiError returns an error including API error code and error description.
-func (n *CNRClient) GetCNRApiError(format string, objectid string, r *response.Response) error {
+// GetAPIError returns an error including API error code and error description.
+func (n *Client) GetAPIError(format string, objectid string, r *response.Response) error {
 	return fmt.Errorf(format+" %q. [%v %s]", objectid, r.GetCode(), r.GetDescription())
 }

providers/cnr/nameservers.go

@@ -18,7 +18,7 @@ var defaultNameservers = []*models.Nameserver{
 var nsRegex = regexp.MustCompile(`ns([1-3]{1})[0-9]+\.rrpproxy\.net`)
 
 // GetNameservers gets the nameservers set on a domain.
-func (n *CNRClient) GetNameservers(domain string) ([]*models.Nameserver, error) {
+func (n *Client) GetNameservers(domain string) ([]*models.Nameserver, error) {
 	// NOTE: This information is taken over from HX and adapted to CNR... might be wrong...
 	// This is an interesting edge case. CNR expects you to SET the nameservers to ns[1-3].rrpproxy.net,
 	// but it will internally set it to (ns1xyz|ns2uvw|ns3asd).rrpproxy.net, where xyz/uvw/asd is a uniqueish number.
@@ -41,14 +41,14 @@ func (n *CNRClient) GetNameservers(domain string) ([]*models.Nameserver, error)
 	return models.ToNameservers(toUse)
 }
 
-func (n *CNRClient) getNameserversRaw(domain string) ([]string, error) {
+func (n *Client) getNameserversRaw(domain string) ([]string, error) {
 	r := n.client.Request(map[string]interface{}{
 		"COMMAND": "StatusDomain",
 		"DOMAIN":  domain,
 	})
 	code := r.GetCode()
 	if code != 200 {
-		return nil, n.GetCNRApiError("Could not get status for domain", domain, r)
+		return nil, n.GetAPIError("Could not get status for domain", domain, r)
 	}
 	nsColumn := r.GetColumn("NAMESERVER")
 	if nsColumn == nil {
@@ -61,7 +61,7 @@ func (n *CNRClient) getNameserversRaw(domain string) ([]string, error) {
 }
 
 // GetRegistrarCorrections gathers corrections that would being n to match dc.
-func (n *CNRClient) GetRegistrarCorrections(dc *models.DomainConfig) ([]*models.Correction, error) {
+func (n *Client) GetRegistrarCorrections(dc *models.DomainConfig) ([]*models.Correction, error) {
 	nss, err := n.getNameserversRaw(dc.Name)
 	if err != nil {
 		return nil, err
@@ -87,7 +87,7 @@ func (n *CNRClient) GetRegistrarCorrections(dc *models.DomainConfig) ([]*models.
 	return nil, nil
 }
 
-func (n *CNRClient) updateNameservers(ns []string, domain string) func() error {
+func (n *Client) updateNameservers(ns []string, domain string) func() error {
 	return func() error {
 		cmd := map[string]interface{}{
 			"COMMAND": "ModifyDomain",

providers/cnr/records.go

@@ -14,8 +14,8 @@ import (
 	"github.com/StackExchange/dnscontrol/v4/pkg/txtutil"
 )
 
-// CNRRecord covers an individual DNS resource record.
-type CNRRecord struct {
+// Record covers an individual DNS resource record.
+type Record struct {
 	// DomainName is the zone that the record belongs to.
 	DomainName string
 	// Host is the hostname relative to the zone: e.g. for a record for blog.example.org, domain would be "example.org" and host would be "blog".
@@ -36,7 +36,7 @@ type CNRRecord struct {
 }
 
 // GetZoneRecords gets the records of a zone and returns them in RecordConfig format.
-func (n *CNRClient) GetZoneRecords(domain string, meta map[string]string) (models.Records, error) {
+func (n *Client) GetZoneRecords(domain string, meta map[string]string) (models.Records, error) {
 	records, err := n.getRecords(domain)
 	if err != nil {
 		return nil, err
@@ -50,7 +50,7 @@ func (n *CNRClient) GetZoneRecords(domain string, meta map[string]string) (model
 }
 
 // GetZoneRecordsCorrections returns a list of corrections that will turn existing records into dc.Records.
-func (n *CNRClient) GetZoneRecordsCorrections(dc *models.DomainConfig, actual models.Records) ([]*models.Correction, int, error) {
+func (n *Client) GetZoneRecordsCorrections(dc *models.DomainConfig, actual models.Records) ([]*models.Correction, int, error) {
 	toReport, create, del, mod, actualChangeCount, err := diff.NewCompat(dc).IncrementalDiff(actual)
 	if err != nil {
 		return nil, 0, err
@@ -82,7 +82,7 @@ func (n *CNRClient) GetZoneRecordsCorrections(dc *models.DomainConfig, actual mo
 		changes = true
 		fmt.Fprintln(buf, d)
 		key := fmt.Sprintf("DELRR%d", delrridx)
-		oldRecordString := n.deleteRecordString(d.Existing.Original.(*CNRRecord))
+		oldRecordString := n.deleteRecordString(d.Existing.Original.(*Record))
 		params[key] = oldRecordString
 		fmt.Fprintf(&builder, "\033[31m- %s = %s\033[0m\n", key, oldRecordString)
 		delrridx++
@@ -92,7 +92,7 @@ func (n *CNRClient) GetZoneRecordsCorrections(dc *models.DomainConfig, actual mo
 		fmt.Fprintln(buf, chng)
 		// old record deletion
 		key := fmt.Sprintf("DELRR%d", delrridx)
-		oldRecordString := n.deleteRecordString(chng.Existing.Original.(*CNRRecord))
+		oldRecordString := n.deleteRecordString(chng.Existing.Original.(*Record))
 		params[key] = oldRecordString
 		fmt.Fprintf(&builder, "\033[31m- %s = %s\033[0m\n", key, oldRecordString)
 		delrridx++
@@ -123,7 +123,7 @@ func (n *CNRClient) GetZoneRecordsCorrections(dc *models.DomainConfig, actual mo
 	return corrections, actualChangeCount, nil
 }
 
-func toRecord(r *CNRRecord, origin string) *models.RecordConfig {
+func toRecord(r *Record, origin string) *models.RecordConfig {
 	rc := &models.RecordConfig{
 		Type:     r.Type,
 		TTL:      r.TTL,
@@ -159,7 +159,7 @@ func toRecord(r *CNRRecord, origin string) *models.RecordConfig {
 }
 
 // updateZoneBy updates the zone with the provided changes.
-func (n *CNRClient) updateZoneBy(params map[string]interface{}, domain string) error {
+func (n *Client) updateZoneBy(params map[string]interface{}, domain string) error {
 	zone := domain
 	cmd := map[string]interface{}{
 		"COMMAND": "ModifyDNSZone",
@@ -170,14 +170,14 @@ func (n *CNRClient) updateZoneBy(params map[string]interface{}, domain string) e
 	}
 	r := n.client.Request(cmd)
 	if !r.IsSuccess() {
-		return n.GetCNRApiError("Error while updating zone", zone, r)
+		return n.GetAPIError("Error while updating zone", zone, r)
 	}
 	return nil
 }
 
-// deleteRecordString constructs the record string based on the provided CNRRecord.
-func (n *CNRClient) getRecords(domain string) ([]*CNRRecord, error) {
-	var records []*CNRRecord
+// deleteRecordString constructs the record string based on the provided Record.
+func (n *Client) getRecords(domain string) ([]*Record, error) {
+	var records []*Record
 
 	// Command to find out the total numbers of resource records for the zone
 	// so that the follow-up query can be done with the correct limit
@@ -186,7 +186,8 @@ func (n *CNRClient) getRecords(domain string) ([]*CNRRecord, error) {
 		"DNSZONE": domain,
 		"ORDERBY": "type",
 		"FIRST":   "0",
-		"LIMIT":   "1",
+		"LIMIT":   "10000",
+		"WIDE":    "1",
 	}
 	r := n.client.Request(cmd)
 
@@ -201,29 +202,16 @@ func (n *CNRClient) getRecords(domain string) ([]*CNRRecord, error) {
 				}
 			} else {
 				// Return specific error if the zone does not exist
-				return nil, n.GetCNRApiError("Use `dnscontrol create-domains` to create not-existing zone", domain, r)
+				return nil, n.GetAPIError("Use `dnscontrol create-domains` to create not-existing zone", domain, r)
 			}
 		}
 		// Return general error for any other issues
-		return nil, n.GetCNRApiError("Failed loading resource records for zone", domain, r)
+		return nil, n.GetAPIError("Failed loading resource records for zone", domain, r)
 	}
 	totalRecords := r.GetRecordsTotalCount()
 	if totalRecords <= 0 {
 		return nil, nil
 	}
-	limitation := 100
-	totalRecords += limitation
-
-	// finally request all resource records available for the zone
-	cmd["LIMIT"] = strconv.Itoa(totalRecords)
-	cmd["WIDE"] = "1"
-	r = n.client.Request(cmd)
-
-	// Check if the request was successful
-	if !r.IsSuccess() {
-		// Return general error for any other issues
-		return nil, n.GetCNRApiError("Failed loading resource records for zone", domain, r)
-	}
 
 	// loop over the records array
 	rrs := r.GetRecords()
@@ -265,8 +253,8 @@ func (n *CNRClient) getRecords(domain string) ([]*CNRRecord, error) {
 			fqdn = fmt.Sprintf("%s.%s.", data["NAME"], domain)
 		}
 
-		// Initialize a new CNRRecord
-		record := &CNRRecord{
+		// Initialize a new Record
+		record := &Record{
 			DomainName: domain,
 			Host:       data["NAME"],
 			Fqdn:       fqdn,
@@ -286,7 +274,7 @@ func (n *CNRClient) getRecords(domain string) ([]*CNRRecord, error) {
 }
 
 // Function to create record string from given RecordConfig for the ADDRR# API parameter
-func (n *CNRClient) createRecordString(rc *models.RecordConfig, domain string) (string, error) {
+func (n *Client) createRecordString(rc *models.RecordConfig, domain string) (string, error) {
 	host := rc.GetLabel()
 	answer := ""
 
@@ -339,8 +327,8 @@ func (n *CNRClient) createRecordString(rc *models.RecordConfig, domain string) (
 	return str, nil
 }
 
-// deleteRecordString constructs the record string based on the provided CNRRecord.
-func (n *CNRClient) deleteRecordString(record *CNRRecord) string {
+// deleteRecordString constructs the record string based on the provided Record.
+func (n *Client) deleteRecordString(record *Record) string {
 	// Initialize values slice
 	values := []string{
 		record.Host,
@@ -375,6 +363,7 @@ func isNoPopulate() bool {
 }
 
 // Function to check if debug mode is enabled
-func (n *CNRClient) isDebugOn() bool {
-	return n.conf["debugmode"] == "1" || n.conf["debugmode"] == "2"
+func (n *Client) isDebugOn() bool {
+	debugMode, exists := n.conf["debugmode"]
+	return exists && (debugMode == "1" || debugMode == "2")
 }