GCLOUD: Support "private domains" plus many bugfixes (#2482)

Co-authored-by: Tom Limoncelli <tlimoncelli@stackoverflow.com>

documentation/providers/gcloud.md

@@ -90,6 +90,37 @@ will enable it on your account, responding with a list of names to use in the `n
 
 > `name_server_set` only applies on `create-domains` at the moment. Additional work needs to be done to support it during `push`
 
+## Private Domains
+
+This optional feature allows for the instantiation of Google Cloud DNS zones with the `Visibility` field set to `private` and with specific Google Cloud VPC Networks granted visibility to the zone.
+
+Example:
+
+{% code title="dnsconfig.js" %}
+```javascript
+var REG_NAMECOM = NewRegistrar("name.com");
+var DSP_GCLOUD = NewDnsProvider("gcloud", {
+    "visibility": "private",
+    "networks": [
+        "https://www.googleapis.com/compute/v1/projects/mydnsproject/global/networks/myvpcnetwork",
+        "my2ndvpcnetwork"
+    ]
+});
+
+D("example.tld", REG_NAMECOM, DnsProvider(DSP_GCLOUD),
+    A("test", "1.2.3.4")
+);
+```
+{% endcode %}
+
+> `visiblity` and `networks` only applies on `create-domains` at the moment. Neither setting is enforced by the provider after a zone is created.  Additional work is required to support modifications to `networks` visibility during `push`, however the API will not permit `visibility` to be modified on an existing zone.
+
+> `networks` may be specified using the network name if the VPC network exists in `project_id`
+
+> multiple network urls may be specified in `networks`
+
+> split horizon zones using the `GCLOUD` provider are currently only supported when the providers' credentials target separate `project_id` values
+
 # Debugging credentials
 
 You can test your `creds.json` entry with the command: `dnscontrol check-creds foo GCLOUD` where `foo` is the name of key used in `creds.json`.  Error messages you might see:

integrationTest/integration_test.go

@@ -1294,6 +1294,7 @@ func makeTests(t *testing.T) []*TestGroup {
 				//"GANDI_V5",      // Their API is so damn slow. We'll add it back as needed.
 				//"HEDNS",         // No paging done. No need to test.
 				//"MSDNS",         // No paging done. No need to test.
+				"GCLOUD",
 				"HEXONET",
 				"HOSTINGDE", // Pages.
 				"ROUTE53",   // Batches up changes in pages.

providers/gcloud/gcloudProvider.go

@@ -5,7 +5,7 @@ import (
 	"encoding/json"
 	"fmt"
 	"log"
-	"net/http"
+	"regexp"
 	"strings"
 	"time"
 
@@ -21,6 +21,8 @@ import (
 	"google.golang.org/api/option"
 )
 
+const selfLinkBasePath = "https://www.googleapis.com/compute/v1/projects/"
+
 var features = providers.DocumentationNotes{
 	providers.CanGetZones:            providers.Can(),
 	providers.CanUseAlias:            providers.Can(),
@@ -36,6 +38,12 @@ var features = providers.DocumentationNotes{
 	providers.DocOfficiallySupported: providers.Can(),
 }
 
+var (
+	visibilityCheck  = regexp.MustCompile("^(public|private)$")
+	networkURLCheck  = regexp.MustCompile("^" + selfLinkBasePath + "[a-z][-a-z0-9]{4,28}[a-z0-9]/global/networks/[a-z]([-a-z0-9]{0,61}[a-z0-9])?$")
+	networkNameCheck = regexp.MustCompile("^[a-z]([-a-z0-9]{0,61}[a-z0-9])?$")
+)
+
 func sPtr(s string) *string {
 	return &s
 }
@@ -56,6 +64,9 @@ type gcloudProvider struct {
 	// diff1
 	oldRRsMap   map[string]map[key]*gdns.ResourceRecordSet
 	zoneNameMap map[string]string
+	// provider metadata fields
+	Visibility string   `json:"visibility"`
+	Networks   []string `json:"networks"`
 }
 
 type errNoExist struct {
@@ -73,28 +84,22 @@ func New(cfg map[string]string, metadata json.RawMessage) (providers.DNSServiceP
 	// fix it if we find that.
 
 	ctx := context.Background()
-	var hc *http.Client
+	var opt option.ClientOption
 	if key, ok := cfg["private_key"]; ok {
 		cfg["private_key"] = strings.Replace(key, "\\n", "\n", -1)
 		raw, err := json.Marshal(cfg)
 		if err != nil {
 			return nil, err
 		}
-		config, err := gauth.JWTConfigFromJSON(raw, "https://www.googleapis.com/auth/ndev.clouddns.readwrite")
+		config, err := gauth.JWTConfigFromJSON(raw, gdns.NdevClouddnsReadwriteScope)
 		if err != nil {
 			return nil, err
 		}
-		hc = config.Client(ctx)
+		opt = option.WithTokenSource(config.TokenSource(ctx))
 	} else {
-		var err error
+		opt = option.WithScopes(gdns.NdevClouddnsReadwriteScope)
-		hc, err = gauth.DefaultClient(ctx, "https://www.googleapis.com/auth/ndev.clouddns.readwrite")
-		if err != nil {
-			return nil, fmt.Errorf("no creds.json private_key found and ADC failed with:\n%s", err)
 	}
-	}
+	dcli, err := gdns.NewService(ctx, opt)
-	// FIXME(tlim): Is it a problem that ctx is included with hc and in
-	// the call to NewService?  Seems redundant.
-	dcli, err := gdns.NewService(ctx, option.WithHTTPClient(hc))
 	if err != nil {
 		return nil, err
 	}
@@ -111,17 +116,53 @@ func New(cfg map[string]string, metadata json.RawMessage) (providers.DNSServiceP
 		oldRRsMap:     map[string]map[key]*gdns.ResourceRecordSet{},
 		zoneNameMap:   map[string]string{},
 	}
+	if len(metadata) != 0 {
+		err := json.Unmarshal(metadata, g)
+		if err != nil {
+			return nil, err
+		}
+		if len(g.Visibility) != 0 {
+			if ok := visibilityCheck.MatchString(g.Visibility); !ok {
+				return nil, fmt.Errorf("GCLOUD :visibility set but not one of \"public\" or \"private\"")
+			}
+			printer.Printf("GCLOUD :visibility %s configured\n", g.Visibility)
+		}
+		for i, v := range g.Networks {
+			if ok := networkURLCheck.MatchString(v); ok {
+				// the user specified a fully qualified network url
+				continue
+			}
+			if ok := networkNameCheck.MatchString(v); !ok {
+				return nil, fmt.Errorf("GCLOUD :networks set but %s does not appear to be a valid network name or url", v)
+			}
+			// assume target vpc network exists in the same project as the dns zones
+			g.Networks[i] = fmt.Sprintf("%s%s/global/networks/%s", selfLinkBasePath, g.project, v)
+		}
+	}
 	return g, g.loadZoneInfo()
 }
 
 func (g *gcloudProvider) loadZoneInfo() error {
+	// TODO(asn-iac): In order to fully support split horizon domains within the same GCP project,
+	// need to parse the zone Visibility field from *ManagedZone, but currently
+	// gcloudProvider.zones is map[string]*gdns.ManagedZone
+	// where the map keys are the zone dns names. A given GCP project can have
+	// multiple zones of the same dns name.
 	if g.zones != nil {
 		return nil
 	}
 	g.zones = map[string]*gdns.ManagedZone{}
 	pageToken := ""
 	for {
+	retry:
 		resp, err := g.client.ManagedZones.List(g.project).PageToken(pageToken).Do()
+		var check *googleapi.ServerResponse
+		if resp != nil {
+			check = &resp.ServerResponse
+		}
+		if retryNeeded(check, err) {
+			goto retry
+		}
 		if err != nil {
 			return err
 		}
@@ -203,6 +244,20 @@ func (g *gcloudProvider) getZoneSets(domain string) (models.Records, error) {
 	return existingRecords, err
 }
 
+type msgs struct {
+	Additions, Deletions []string
+}
+
+type orderedChanges struct {
+	Change *gdns.Change
+	Msgs   msgs
+}
+
+type correctionValues struct {
+	Change *gdns.Change
+	Msgs   string
+}
+
 // GetZoneRecordsCorrections returns a list of corrections that will turn existing records into dc.Records.
 func (g *gcloudProvider) GetZoneRecordsCorrections(dc *models.DomainConfig, existingRecords models.Records) ([]*models.Correction, error) {
 
@@ -229,30 +284,25 @@ func (g *gcloudProvider) GetZoneRecordsCorrections(dc *models.DomainConfig, exis
 		return nil, fmt.Errorf("incdiff error: %w", err)
 	}
 
-	changedKeys := map[key]bool{}
+	changedKeys := map[key]string{}
-	var msgs []string
 	for _, c := range create {
-		msgs = append(msgs, fmt.Sprint(c))
+		changedKeys[keyForRec(c.Desired)] = fmt.Sprintln(c)
-		changedKeys[keyForRec(c.Desired)] = true
 	}
 	for _, d := range delete {
-		msgs = append(msgs, fmt.Sprint(d))
+		changedKeys[keyForRec(d.Existing)] = fmt.Sprintln(d)
-		changedKeys[keyForRec(d.Existing)] = true
 	}
 	for _, m := range modify {
-		msgs = append(msgs, fmt.Sprint(m))
+		changedKeys[keyForRec(m.Existing)] = fmt.Sprintln(m)
-		changedKeys[keyForRec(m.Existing)] = true
 	}
 	if len(changedKeys) == 0 {
 		return nil, nil
 	}
-	chg := &gdns.Change{Kind: "dns#change"}
+	chg := orderedChanges{Change: &gdns.Change{}, Msgs: msgs{}}
-	for ck := range changedKeys {
+	// create slices of Deletions and Additions
-		// remove old version (if present)
+	// that can be split into properly ordered batches
-		if old, ok := oldRRs[ck]; ok {
+	// if necessary.  Retain the string messages from
-			chg.Deletions = append(chg.Deletions, old)
+	// differ in the same order
-		}
+	for ck, msg := range changedKeys {
-		// collect records to replace with
 		newRRs := &gdns.ResourceRecordSet{
 			Name: ck.Name,
 			Type: ck.Type,
@@ -265,22 +315,74 @@ func (g *gcloudProvider) GetZoneRecordsCorrections(dc *models.DomainConfig, exis
 			}
 		}
 		if len(newRRs.Rrdatas) > 0 {
-			chg.Additions = append(chg.Additions, newRRs)
+			// if we have Rrdatas because the key from differ
+			// exists in normalized config,
+			// check whether the key also has data in oldRRs.
+			// if so, this is actually a modify operation, insert
+			// the Addition and Deletion at the beginning of the slices
+			// to ensure they are executed in the same batch
+			if old, ok := oldRRs[ck]; ok {
+				chg.Change.Additions = append([]*gdns.ResourceRecordSet{newRRs}, chg.Change.Additions...)
+				chg.Change.Deletions = append([]*gdns.ResourceRecordSet{old}, chg.Change.Deletions...)
+				chg.Msgs.Additions = append([]string{msg}, chg.Msgs.Additions...)
+				chg.Msgs.Deletions = append([]string{""}, chg.Msgs.Deletions...)
+			} else {
+				// otherwise this is a pure Addition
+				chg.Change.Additions = append(chg.Change.Additions, newRRs)
+				chg.Msgs.Additions = append(chg.Msgs.Additions, msg)
+			}
+		} else {
+			// there is no Rrdatas from normalized config for this key.
+			// it must be a Deletion, use the ResourceRecordSet from
+			// oldRRs
+			if old, ok := oldRRs[ck]; ok {
+				chg.Change.Deletions = append(chg.Change.Deletions, old)
+				chg.Msgs.Deletions = append(chg.Msgs.Deletions, msg)
+			}
 		}
 	}
 
-	// FIXME(tlim): Google will return an error if too many changes are
+	// create a slice of Changes in batches of at most
-	// specified in a single request. We should split up very large
+	// 1000 Deletions and 1000 Additions per Change.
-	// batches.  This can be reliably reproduced with the 1201
+	// create a slice of strings that aligns with the batch
-	// integration test.  The error you get is:
+	// to output with each correction/Change
-	// googleapi: Error 403: The change would exceed quota for additions per change., quotaExceeded
+	const batchMax = 1000
-	//log.Printf("PAUSE STT = %+v %v\n", err, resp)
+	setBatchLen := func(len int) int {
-	//log.Printf("PAUSE ERR = %+v %v\n", err, resp)
+		if len > batchMax {
-
+			return batchMax
+		}
+		return len
+	}
+	chgSet := []correctionValues{}
+	for len(chg.Change.Deletions) > 0 {
+		b := setBatchLen(len(chg.Change.Deletions))
+		chgSet = append(chgSet, correctionValues{Change: &gdns.Change{Deletions: chg.Change.Deletions[:b:b], Kind: "dns#change"}, Msgs: "\n" + strings.Join(chg.Msgs.Deletions[:b:b], "")})
+		chg.Change.Deletions = chg.Change.Deletions[b:]
+		chg.Msgs.Deletions = chg.Msgs.Deletions[b:]
+	}
+	for i := 0; len(chg.Change.Additions) > 0; i++ {
+		b := setBatchLen(len(chg.Change.Additions))
+		if len(chgSet) == i {
+			chgSet = append(chgSet, correctionValues{Change: &gdns.Change{Additions: chg.Change.Additions[:b:b], Kind: "dns#change"}, Msgs: "\n" + strings.Join(chg.Msgs.Additions[:b:b], "")})
+		} else {
+			chgSet[i].Change.Additions = chg.Change.Additions[:b:b]
+			chgSet[i].Msgs += strings.Join(chg.Msgs.Additions[:b:b], "")
+		}
+		chg.Change.Additions = chg.Change.Additions[b:]
+		chg.Msgs.Additions = chg.Msgs.Additions[b:]
+	}
+	// create a Correction for each gdns.Change
+	// that needs to be executed
+	corrections := []*models.Correction{}
+	makeCorrection := func(chg *gdns.Change, msgs string) {
 		runChange := func() error {
 		retry:
 			resp, err := g.client.Changes.Create(g.project, zoneName, chg).Do()
-		if retryNeeded(resp, err) {
+			var check *googleapi.ServerResponse
+			if resp != nil {
+				check = &resp.ServerResponse
+			}
+			if retryNeeded(check, err) {
 				goto retry
 			}
 			if err != nil {
@@ -288,12 +390,17 @@ func (g *gcloudProvider) GetZoneRecordsCorrections(dc *models.DomainConfig, exis
 			}
 			return nil
 		}
-
+		corrections = append(corrections,
-	return []*models.Correction{{
+			&models.Correction{
-		Msg: strings.Join(msgs, "\n"),
+				Msg: strings.TrimSuffix(msgs, "\n"),
 				F:   runChange,
-	}}, nil
+			})
+	}
+	for _, v := range chgSet {
+		makeCorrection(v.Change, v.Msgs)
+	}
 
+	return corrections, nil
 }
 
 func nativeToRecord(set *gdns.ResourceRecordSet, rec, origin string) (*models.RecordConfig, error) {
@@ -326,7 +433,15 @@ func (g *gcloudProvider) getRecords(domain string) ([]*gdns.ResourceRecordSet, s
 		if pageToken != "" {
 			call = call.PageToken(pageToken)
 		}
+	retry:
 		resp, err := call.Do()
+		var check *googleapi.ServerResponse
+		if resp != nil {
+			check = &resp.ServerResponse
+		}
+		if retryNeeded(check, err) {
+			goto retry
+		}
 		if err != nil {
 			return nil, "", err
 		}
@@ -354,24 +469,33 @@ func (g *gcloudProvider) EnsureZoneExists(domain string) error {
 		return nil
 	}
 	var mz *gdns.ManagedZone
+	printer.Printf("Adding zone for %s to gcloud account ", domain)
+	mz = &gdns.ManagedZone{
+		DnsName:     domain + ".",
+		Name:        "zone-" + strings.Replace(domain, ".", "-", -1),
+		Description: "zone added by dnscontrol",
+	}
 	if g.nameServerSet != nil {
-		printer.Printf("Adding zone for %s to gcloud account with name_server_set %s\n", domain, *g.nameServerSet)
+		mz.NameServerSet = *g.nameServerSet
-		mz = &gdns.ManagedZone{
+		printer.Printf("with name_server_set %s ", *g.nameServerSet)
-			DnsName:       domain + ".",
-			NameServerSet: *g.nameServerSet,
-			Name:          "zone-" + strings.Replace(domain, ".", "-", -1),
-			Description:   "zone added by dnscontrol",
 	}
-	} else {
+	if len(g.Visibility) != 0 {
-		printer.Printf("Adding zone for %s to gcloud account \n", domain)
+		mz.Visibility = g.Visibility
-		mz = &gdns.ManagedZone{
+		printer.Printf("with %s visibility ", g.Visibility)
-			DnsName:     domain + ".",
+		// prevent possible GCP resource name conflicts when split horizon can be properly implemented
-			Name:        "zone-" + strings.Replace(domain, ".", "-", -1),
+		mz.Name = strings.Replace(mz.Name, "zone-", "zone-"+g.Visibility+"-", 1)
-			Description: "zone added by dnscontrol",
 	}
+	if g.Networks != nil {
+		mzn := make([]*gdns.ManagedZonePrivateVisibilityConfigNetwork, len(g.Networks))
+		printer.Printf("for network(s) ")
+		for _, v := range g.Networks {
+			printer.Printf("%s ", v)
+			mzn = append(mzn, &gdns.ManagedZonePrivateVisibilityConfigNetwork{NetworkUrl: v})
 		}
-	g.zones = nil // reset cache
+		mz.PrivateVisibilityConfig = &gdns.ManagedZonePrivateVisibilityConfig{Networks: mzn}
-	_, err = g.client.ManagedZones.Create(g.project, mz).Do()
+	}
+	printer.Printf("\n")
+	g.zones[domain+"."], err = g.client.ManagedZones.Create(g.project, mz).Do()
 	return err
 }
 
@@ -383,7 +507,7 @@ const maxBackoff = time.Minute * 3      // Maximum backoff delay
 var backoff = initialBackoff
 var backoff404 = false // Set if the last call requested a retry of a 404
 
-func retryNeeded(resp *gdns.Change, err error) bool {
+func retryNeeded(resp *googleapi.ServerResponse, err error) bool {
 	if err != nil {
 		return false // Not an error.
 	}