MSDNS: be more efficient with ttl changes (#2251)

Co-authored-by: Tom Limoncelli <tal@whatexit.org>

pkg/diff2/analyze.go

@@ -265,9 +265,15 @@ func diffTargets(existing, desired []targetConfig) ChangeList {
 
 		m := color.YellowString("± MODIFY %s %s %s", dr.NameFQDN, dr.Type, humanDiff(existing[i], desired[i]))
 
-		instructions = append(instructions,
+		mkc := mkChange(dr.NameFQDN, dr.Type, []string{m}, models.Records{er}, models.Records{dr})
-			mkChange(dr.NameFQDN, dr.Type, []string{m}, models.Records{er}, models.Records{dr}),
+		if len(existing) == 1 && len(desired) == 1 {
-		)
+			// If the tdata has exactly 1 item, drop a hint to the providers.
+			// For example, MSDNS can use a more efficient command if it knows
+			// that `Get-DnsServerResourceRecord -Name FOO -RRType A` will
+			// return exactly one record.
+			mkc.HintRecordSetLen1 = true
+		}
+		instructions = append(instructions, mkc)
 	}
 
 	// any left-over existing are deletes

pkg/diff2/diff2.go

@@ -43,6 +43,13 @@ type Change struct {
 	// HintOnlyTTL is true only if (.Type == diff2.CHANGE) && (there is
 	// exactly 1 record being updated) && (the only change is the TTL)
 	HintOnlyTTL bool
+
+	// HintRecordSetLen1 is true only if (.Type == diff2.CHANGE) &&
+	// (there is exactly 1 record at this RecordSet).
+	// For example, MSDNS can use a more efficient command if it knows
+	// that `Get-DnsServerResourceRecord -Name FOO -RRType A` will
+	// return exactly one record.
+	HintRecordSetLen1 bool
 }
 
 /*

providers/msdns/corrections.go

@@ -61,10 +61,20 @@ func (client *msdnsProvider) GenerateDomainCorrections(dc *models.DomainConfig,
 		case diff2.CHANGE:
 			oldrec := change.Old[0]
 			newrec := change.New[0]
+			var f func(dnsserver string, zonename string, oldrec *models.RecordConfig, newrec *models.RecordConfig) error
+			if change.HintOnlyTTL && change.HintRecordSetLen1 {
+				// If we're only changing the TTL, and there is exactly one
+				// record of type oldrec.Type at this label, then we can do the
+				// TTL change in one command instead of deleting and re-creating
+				// the record.
+				f = client.modifyRecordTTL
+			} else {
+				f = client.modifyOneRecord
+			}
 			corr = &models.Correction{
 				Msg: msgsJoined,
 				F: func() error {
-					return client.modifyOneRecord(client.dnsserver, dc.Name, oldrec, newrec)
+					return f(client.dnsserver, dc.Name, oldrec, newrec)
 				},
 			}
 		case diff2.DELETE:
@@ -97,6 +107,10 @@ func (client *msdnsProvider) modifyOneRecord(dnsserver, zonename string, oldrec,
 	return client.shell.RecordModify(dnsserver, zonename, oldrec, newrec)
 }
 
+func (client *msdnsProvider) modifyRecordTTL(dnsserver, zonename string, oldrec, newrec *models.RecordConfig) error {
+	return client.shell.RecordModifyTTL(dnsserver, zonename, oldrec, newrec.TTL)
+}
+
 func (client *msdnsProvider) deleteRec(dnsserver, domainname string, cor diff.Correlation) *models.Correction {
 	rec := cor.Existing
 	return &models.Correction{

providers/msdns/powershell.go

@@ -224,7 +224,7 @@ func (psh *psHandle) RecordDelete(dnsserver, domain string, rec *models.RecordCo
 func generatePSDelete(dnsserver, domain string, rec *models.RecordConfig) string {
 
 	var b bytes.Buffer
-	fmt.Fprintf(&b, `echo DELETE "%s" "%s" "[target]"`, rec.Type, rec.Name)
+	fmt.Fprintf(&b, `echo DELETE "%s" "%s" %q`, rec.Type, rec.Name, rec.GetTargetCombined())
 	fmt.Fprintf(&b, " ; ")
 
 	if rec.Type == "NAPTR" {
@@ -283,7 +283,7 @@ func (psh *psHandle) RecordCreate(dnsserver, domain string, rec *models.RecordCo
 
 func generatePSCreate(dnsserver, domain string, rec *models.RecordConfig) string {
 	var b bytes.Buffer
-	fmt.Fprintf(&b, `echo CREATE "%s" "%s" "[target]"`, rec.Type, rec.Name)
+	fmt.Fprintf(&b, `echo CREATE "%s" "%s" %q`, rec.Type, rec.Name, rec.GetTargetCombined())
 	fmt.Fprintf(&b, " ; ")
 
 	if rec.Type == "NAPTR" {
@@ -372,6 +372,47 @@ func generatePSModify(dnsserver, domain string, old, rec *models.RecordConfig) s
 	// command.
 }
 
+func (psh *psHandle) RecordModifyTTL(dnsserver, domain string, old *models.RecordConfig, newTTL uint32) error {
+	c := generatePSModifyTTL(dnsserver, domain, old, newTTL)
+	//eLog(c)
+	_, stderr, err := psh.shell.Execute(c)
+	if err != nil {
+		printer.Printf("PowerShell code was:\nSTART\n%s\nEND\n", c)
+		return err
+	}
+	if stderr != "" {
+		printer.Printf("STDERROR = %q\n", stderr)
+		printer.Printf("PowerShell code was:\nSTART\n%s\nEND\n", c)
+		return fmt.Errorf("unexpected stderr from PSModify: %q", stderr)
+	}
+	return nil
+}
+
+func generatePSModifyTTL(dnsserver, domain string, rec *models.RecordConfig, newTTL uint32) string {
+	var b bytes.Buffer
+	fmt.Fprintf(&b, `echo MODIFY-TTL "%s" "%s" %q ttl=%d->%d`, rec.Name, rec.Type, rec.GetTargetCombined(), rec.TTL, newTTL)
+	fmt.Fprintf(&b, " ; ")
+
+	fmt.Fprint(&b, `Get-DnsServerResourceRecord`)
+	if dnsserver != "" {
+		fmt.Fprintf(&b, ` -ComputerName "%s"`, dnsserver)
+	}
+	fmt.Fprintf(&b, ` -ZoneName "%s"`, domain)
+	fmt.Fprintf(&b, ` -Name "%s"`, rec.GetLabel())
+	fmt.Fprintf(&b, ` -RRType %s`, rec.Type)
+	fmt.Fprint(&b, ` | ForEach-Object { $NewRecord = $_.Clone() ;`)
+	fmt.Fprintf(&b, `$NewRecord.TimeToLive = New-TimeSpan -Seconds %d`, newTTL)
+	fmt.Fprintf(&b, " ; ")
+	fmt.Fprintf(&b, `Set-DnsServerResourceRecord`)
+	if dnsserver != "" {
+		fmt.Fprintf(&b, ` -ComputerName "%s"`, dnsserver)
+	}
+	fmt.Fprint(&b, ` -NewInputObject $NewRecord -OldInputObject $_`)
+	fmt.Fprintf(&b, ` -ZoneName "%s"`, domain)
+
+	return b.String()
+}
+
 // Note about the old generatePSModify:
 //
 // The old method is to generate PowerShell code that extracts the resource

providers/msdns/powershell_test.go

@@ -146,16 +146,16 @@ func Test_generatePSModify(t *testing.T) {
 		want string
 	}{
 		{name: "A", args: args{domain: "example.com", dnsserver: "", old: recA1, rec: recA2},
-			want: `echo DELETE "A" "@" "[target]" ; Remove-DnsServerResourceRecord -Force -ZoneName "example.com" -Name "@" -RRType "A" -RecordData "1.2.3.4" ; echo CREATE "A" "@" "[target]" ; Add-DnsServerResourceRecord -ZoneName "example.com" -Name "@" -TimeToLive $(New-TimeSpan -Seconds 0) -A -IPv4Address "10.20.30.40"`,
+			want: `echo DELETE "A" "@" "1.2.3.4" ; Remove-DnsServerResourceRecord -Force -ZoneName "example.com" -Name "@" -RRType "A" -RecordData "1.2.3.4" ; echo CREATE "A" "@" "10.20.30.40" ; Add-DnsServerResourceRecord -ZoneName "example.com" -Name "@" -TimeToLive $(New-TimeSpan -Seconds 0) -A -IPv4Address "10.20.30.40"`,
 		},
 		{name: "MX1", args: args{domain: "example.com", dnsserver: "", old: recMX1, rec: recMX2},
-			want: `echo DELETE "MX" "@" "[target]" ; Remove-DnsServerResourceRecord -Force -ZoneName "example.com" -Name "@" -RRType "MX" -RecordData 5,"foo.com." ; echo CREATE "MX" "@" "[target]" ; Add-DnsServerResourceRecord -ZoneName "example.com" -Name "@" -TimeToLive $(New-TimeSpan -Seconds 0) -MX -MailExchange "foo2.com." -Preference 50`,
+			want: `echo DELETE "MX" "@" "5 foo.com." ; Remove-DnsServerResourceRecord -Force -ZoneName "example.com" -Name "@" -RRType "MX" -RecordData 5,"foo.com." ; echo CREATE "MX" "@" "50 foo2.com." ; Add-DnsServerResourceRecord -ZoneName "example.com" -Name "@" -TimeToLive $(New-TimeSpan -Seconds 0) -MX -MailExchange "foo2.com." -Preference 50`,
 		},
 		{name: "A-remote", args: args{domain: "example.com", dnsserver: "myremote", old: recA1, rec: recA2},
-			want: `echo DELETE "A" "@" "[target]" ; Remove-DnsServerResourceRecord -ComputerName "myremote" -Force -ZoneName "example.com" -Name "@" -RRType "A" -RecordData "1.2.3.4" ; echo CREATE "A" "@" "[target]" ; Add-DnsServerResourceRecord -ComputerName "myremote" -ZoneName "example.com" -Name "@" -TimeToLive $(New-TimeSpan -Seconds 0) -A -IPv4Address "10.20.30.40"`,
+			want: `echo DELETE "A" "@" "1.2.3.4" ; Remove-DnsServerResourceRecord -ComputerName "myremote" -Force -ZoneName "example.com" -Name "@" -RRType "A" -RecordData "1.2.3.4" ; echo CREATE "A" "@" "10.20.30.40" ; Add-DnsServerResourceRecord -ComputerName "myremote" -ZoneName "example.com" -Name "@" -TimeToLive $(New-TimeSpan -Seconds 0) -A -IPv4Address "10.20.30.40"`,
 		},
 		{name: "MX1-remote", args: args{domain: "example.com", dnsserver: "yourremote", old: recMX1, rec: recMX2},
-			want: `echo DELETE "MX" "@" "[target]" ; Remove-DnsServerResourceRecord -ComputerName "yourremote" -Force -ZoneName "example.com" -Name "@" -RRType "MX" -RecordData 5,"foo.com." ; echo CREATE "MX" "@" "[target]" ; Add-DnsServerResourceRecord -ComputerName "yourremote" -ZoneName "example.com" -Name "@" -TimeToLive $(New-TimeSpan -Seconds 0) -MX -MailExchange "foo2.com." -Preference 50`,
+			want: `echo DELETE "MX" "@" "5 foo.com." ; Remove-DnsServerResourceRecord -ComputerName "yourremote" -Force -ZoneName "example.com" -Name "@" -RRType "MX" -RecordData 5,"foo.com." ; echo CREATE "MX" "@" "50 foo2.com." ; Add-DnsServerResourceRecord -ComputerName "yourremote" -ZoneName "example.com" -Name "@" -TimeToLive $(New-TimeSpan -Seconds 0) -MX -MailExchange "foo2.com." -Preference 50`,
 		},
 	}
 	for _, tt := range tests {

providers/msdns/types.go

@@ -14,6 +14,7 @@ type DNSAccessor interface {
 	RecordCreate(dnsserver, domain string, rec *models.RecordConfig) error
 	RecordDelete(dnsserver, domain string, rec *models.RecordConfig) error
 	RecordModify(dnsserver, domain string, old, rec *models.RecordConfig) error
+	RecordModifyTTL(dnsserver, domain string, old *models.RecordConfig, newTTL uint32) error
 }
 
 // nativeRecord the JSON received from PowerShell when listing all DNS