Integration tests should test diff2's IGNORE* (#2292)

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

integrationTest/integration_test.go

@@ -209,6 +209,7 @@ func makeChanges(t *testing.T, prv providers.DNSServiceProvider, dc *models.Doma
 		}
 		dom.IgnoredNames = tst.IgnoredNames
 		dom.IgnoredTargets = tst.IgnoredTargets
+		dom.Unmanaged = tst.Unmanaged
 		models.PostProcessRecords(dom.Records)
 		dom2, _ := dom.Copy()
 
@@ -229,9 +230,11 @@ func makeChanges(t *testing.T, prv providers.DNSServiceProvider, dc *models.Doma
 			if *verbose {
 				t.Log("\n" + c.Msg)
 			}
-			err = c.F()
-			if err != nil {
-				t.Fatal(err)
+			if c.F != nil { // F == nil if there is just a msg, no action.
+				err = c.F()
+				if err != nil {
+					t.Fatal(err)
+				}
 			}
 		}
 
@@ -245,8 +248,8 @@ func makeChanges(t *testing.T, prv providers.DNSServiceProvider, dc *models.Doma
 		if err != nil {
 			t.Fatal(err)
 		}
-		if len(corrections) != 0 {
-			t.Logf("Expected 0 corrections on second run, but found %d.", len(corrections))
+		if count := zonerecs.CountActionable(corrections); count != 0 {
+			t.Logf("Expected 0 corrections on second run, but found %d.", count)
 			for i, c := range corrections {
 				t.Logf("UNEXPECTED #%d: %s", i, c.Msg)
 			}
@@ -362,8 +365,8 @@ func TestDualProviders(t *testing.T) {
 	if err != nil {
 		t.Fatal(err)
 	}
-	if len(cs) != 0 {
-		t.Logf("Expect no corrections on second run, but found %d.", len(cs))
+	if count := zonerecs.CountActionable(cs); count != 0 {
+		t.Logf("Expect no corrections on second run, but found %d.", count)
 		for i, c := range cs {
 			t.Logf("#%d: %s", i, c.Msg)
 		}
@@ -385,6 +388,7 @@ type TestCase struct {
 	Records        []*models.RecordConfig
 	IgnoredNames   []*models.IgnoreName
 	IgnoredTargets []*models.IgnoreTarget
+	Unmanaged      []*models.UnmanagedConfig
 }
 
 func SetLabel(r *models.RecordConfig, label, domain string) {
@@ -585,15 +589,24 @@ func tc(desc string, recs ...*models.RecordConfig) *TestCase {
 	var records []*models.RecordConfig
 	var ignoredNames []*models.IgnoreName
 	var ignoredTargets []*models.IgnoreTarget
+	var unmanagedItems []*models.UnmanagedConfig
 	for _, r := range recs {
 		if r.Type == "IGNORE_NAME" {
 			ignoredNames = append(ignoredNames, &models.IgnoreName{Pattern: r.GetLabel(), Types: r.GetTargetField()})
+			unmanagedItems = append(unmanagedItems, &models.UnmanagedConfig{
+				LabelPattern: r.GetLabel(),
+				RTypePattern: r.GetTargetField(),
+			})
 		} else if r.Type == "IGNORE_TARGET" {
 			rec := &models.IgnoreTarget{
 				Pattern: r.GetLabel(),
 				Type:    r.GetTargetField(),
 			}
 			ignoredTargets = append(ignoredTargets, rec)
+			unmanagedItems = append(unmanagedItems, &models.UnmanagedConfig{
+				RTypePattern:  r.GetTargetField(),
+				TargetPattern: r.GetLabel(),
+			})
 		} else {
 			records = append(records, r)
 		}
@@ -603,6 +616,7 @@ func tc(desc string, recs ...*models.RecordConfig) *TestCase {
 		Records:        records,
 		IgnoredNames:   ignoredNames,
 		IgnoredTargets: ignoredTargets,
+		Unmanaged:      unmanagedItems,
 	}
 }
 

pkg/diff2/diff2.go

@@ -186,7 +186,14 @@ func ByZone(existing models.Records, dc *models.DomainConfig, compFunc Comparabl
 
 	// Only return the messages.  The caller has the list of records needed to build the new zone.
 	instructions, err := byHelper(analyzeByRecord, existing, dc, compFunc)
-	return justMsgs(instructions), len(instructions) != 0, err
+	changes := false
+	for i, _ := range instructions {
+		//fmt.Printf("DEBUG: ByZone #%d: %v\n", i, ii)
+		if instructions[i].Type != REPORT {
+			changes = true
+		}
+	}
+	return justMsgs(instructions), changes, err
 }
 
 //

pkg/zonerecs/zonerecords.go

@@ -32,3 +32,16 @@ func CorrectZoneRecords(driver models.DNSProvider, dc *models.DomainConfig) ([]*
 
 	return driver.GetZoneRecordsCorrections(dc, existingRecords)
 }
+
+// CountActionable returns the number of corrections that have
+// actions.  It is like `len(corrections)` but doesn't count any
+// corrections that are purely informational. (i.e. `.F` is nil)
+func CountActionable(corrections []*models.Correction) int {
+	count := 0
+	for i, _ := range corrections {
+		if corrections[i].F != nil {
+			count++
+		}
+	}
+	return count
+}

providers/route53/route53Provider.go

@@ -483,6 +483,7 @@ func (r *route53Provider) GetZoneRecordsCorrections(dc *models.DomainConfig, exi
 		return nil, err
 	}
 	instructions = reorderInstructions(instructions)
+	wasReport := false
 	for _, inst := range instructions {
 		instNameFQDN := inst.Key.NameFQDN
 		instType := inst.Key.Type
@@ -492,6 +493,7 @@ func (r *route53Provider) GetZoneRecordsCorrections(dc *models.DomainConfig, exi
 
 		case diff2.REPORT:
 			chg = r53Types.Change{}
+			wasReport = true
 
 		case diff2.CREATE:
 			fallthrough
@@ -545,19 +547,33 @@ func (r *route53Provider) GetZoneRecordsCorrections(dc *models.DomainConfig, exi
 	}
 
 	addCorrection := func(msg string, req *r53.ChangeResourceRecordSetsInput) {
-		corrections = append(corrections,
-			&models.Correction{
-				Msg: msg,
-				F: func() error {
-					var err error
-					req.HostedZoneId = zone.Id
-					withRetry(func() error {
-						_, err = r.client.ChangeResourceRecordSets(context.Background(), req)
+
+		if wasReport {
+
+			// Add a "msg only" correction.
+			corrections = append(corrections,
+				&models.Correction{
+					Msg: msg,
+				})
+
+		} else {
+
+			// Add a function to execute.
+			corrections = append(corrections,
+				&models.Correction{
+					Msg: msg,
+					F: func() error {
+						var err error
+						req.HostedZoneId = zone.Id
+						withRetry(func() error {
+							_, err = r.client.ChangeResourceRecordSets(context.Background(), req)
+							return err
+						})
 						return err
-					})
-					return err
-				},
-			})
+					},
+				})
+
+		}
 
 	}