Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

the fix can't be this easy #3262

Closed
wants to merge 4 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 18 additions & 0 deletions integrationTest/integration_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -2351,11 +2351,29 @@ func makeTests() []*TestGroup {
a("testdefined", "9.9.9.9"),
ignore("testignore", "", ""),
).ExpectNoChanges(),
tc("VERIFY PREVIOUS",
a("testignore", "8.8.8.8"),
a("testdefined", "9.9.9.9"),
).ExpectNoChanges(),

tc("Verify nothing changed",
a("testignore", "8.8.8.8"),
a("testdefined", "9.9.9.9"),
).ExpectNoChanges(),
tc("VERIFY PREVIOUS",
a("testignore", "8.8.8.8"),
a("testdefined", "9.9.9.9"),
).ExpectNoChanges(),

tc("ignore with change",
//a("testignore", "8.8.8.8"),
a("testdefined", "2.2.2.2"),
ignore("testignore", "", ""),
),
tc("VERIFY PREVIOUS",
a("testignore", "8.8.8.8"),
a("testdefined", "2.2.2.2"),
).ExpectNoChanges(),
),

// OVH features
Expand Down
43 changes: 33 additions & 10 deletions pkg/diff2/diff2.go
Original file line number Diff line number Diff line change
Expand Up @@ -205,31 +205,49 @@ func ByRecord(existing models.Records, dc *models.DomainConfig, compFunc Compara
// }
//
// Example providers include: BIND, AUTODNS
func ByZone(existing models.Records, dc *models.DomainConfig, compFunc ComparableFunc) ([]string, bool, int, error) {
// Only return the messages. The caller has the list of records needed to build the new zone.
instructions, actualChangeCount, err := byHelper(analyzeByRecord, existing, dc, compFunc)
return justMsgs(instructions), actualChangeCount > 0, actualChangeCount, err
func ByZone(existing models.Records, dc *models.DomainConfig, compFunc ComparableFunc) (ByResults, error) {
// Only return the messages and a list of records needed to build the new zone.
result, err := byHelperStruct(analyzeByRecord, existing, dc, compFunc)
result.Msgs = justMsgs(result.Instructions)
return result, err
}

//
// ByResults is the results of ByZone() and perhaps someday all the By*() functions.
// It is partially populated by // byHelperStruct() and partially by the By*()
// functions that use it.
type ByResults struct {
// Fields filled in by byHelperStruct():
Instructions ChangeList // Instructions to turn existing into desired.
ActualChangeCount int // Number of actual changes, not including REPORTs.
HasChanges bool // True if there are any changes.
DesiredPlus models.Records // Desired + foreign + ignored
// Fields filled in by ByZone():
Msgs []string // Just the messages from the instructions.
}

// byHelper does 90% of the work for the By*() calls.
// byHelper is like byHelperStruct but has a signature that is compatible with legacy code.
// Deprecated: Use byHelperStruct instead.
func byHelper(fn func(cc *CompareConfig) (ChangeList, int), existing models.Records, dc *models.DomainConfig, compFunc ComparableFunc) (ChangeList, int, error) {
result, err := byHelperStruct(fn, existing, dc, compFunc)
return result.Instructions, result.ActualChangeCount, err
}

// byHelperStruct does 90% of the work for the By*() calls.
func byHelperStruct(fn func(cc *CompareConfig) (ChangeList, int), existing models.Records, dc *models.DomainConfig, compFunc ComparableFunc) (ByResults, error) {
// Process NO_PURGE/ENSURE_ABSENT and IGNORE*().
desired, msgs, err := handsoff(
desiredPlus, msgs, err := handsoff(
dc.Name,
existing, dc.Records, dc.EnsureAbsent,
dc.Unmanaged,
dc.UnmanagedUnsafe,
dc.KeepUnknown,
)
if err != nil {
return nil, 0, err
return ByResults{}, err
}

// Regroup existing/desiredd for easy comparison:
cc := NewCompareConfig(dc.Name, existing, desired, compFunc)
cc := NewCompareConfig(dc.Name, existing, desiredPlus, compFunc)

// Analyze and generate the instructions:
instructions, actualChangeCount := fn(cc)
Expand All @@ -245,5 +263,10 @@ func byHelper(fn func(cc *CompareConfig) (ChangeList, int), existing models.Reco
instructions = append([]Change{chg}, instructions...)
}

return instructions, actualChangeCount, nil
return ByResults{
Instructions: instructions,
ActualChangeCount: actualChangeCount,
HasChanges: actualChangeCount > 0,
DesiredPlus: desiredPlus,
}, nil
}
6 changes: 4 additions & 2 deletions providers/autodns/autoDnsProvider.go
Original file line number Diff line number Diff line change
Expand Up @@ -77,16 +77,18 @@ func (api *autoDNSProvider) GetZoneRecordsCorrections(dc *models.DomainConfig, e

var corrections []*models.Correction

msgs, changed, actualChangeCount, err := diff2.ByZone(existingRecords, dc, nil)
result, err := diff2.ByZone(existingRecords, dc, nil)
if err != nil {
return nil, 0, err
}
msgs, changed, actualChangeCount := result.Msgs, result.HasChanges, result.ActualChangeCount

if changed {

msgs = append(msgs, "Zone update for "+domain)
msg := strings.Join(msgs, "\n")

nameServers, zoneTTL, resourceRecords := recordsToNative(dc.Records)
nameServers, zoneTTL, resourceRecords := recordsToNative(result.DesiredPlus)

corrections = append(corrections,
&models.Correction{
Expand Down
8 changes: 4 additions & 4 deletions providers/bind/bindProvider.go
Original file line number Diff line number Diff line change
Expand Up @@ -211,7 +211,7 @@ func ParseZoneContents(content string, zoneName string, zonefileName string) (mo
return foundRecords, nil
}

func (n *bindProvider) EnsureZoneExists(_ string) error {
func (c *bindProvider) EnsureZoneExists(_ string) error {
return nil
}

Expand Down Expand Up @@ -247,12 +247,12 @@ func (c *bindProvider) GetZoneRecordsCorrections(dc *models.DomainConfig, foundR
}

var msgs []string
var err error
var actualChangeCount int
msgs, changes, actualChangeCount, err = diff2.ByZone(foundRecords, dc, nil)
result, err := diff2.ByZone(foundRecords, dc, nil)
if err != nil {
return nil, 0, err
}
msgs, changes, actualChangeCount = result.Msgs, result.HasChanges, result.ActualChangeCount
if !changes {
return nil, 0, nil
}
Expand Down Expand Up @@ -299,7 +299,7 @@ func (c *bindProvider) GetZoneRecordsCorrections(dc *models.DomainConfig, foundR
// Beware that if there are any fake types, then they will
// be commented out on write, but we don't reverse that when
// reading, so there will be a diff on every invocation.
err = prettyzone.WriteZoneFileRC(zf, dc.Records, dc.Name, 0, comments)
err = prettyzone.WriteZoneFileRC(zf, result.DesiredPlus, dc.Name, 0, comments)

if err != nil {
return fmt.Errorf("failed WriteZoneFile: %w", err)
Expand Down
5 changes: 3 additions & 2 deletions providers/mythicbeasts/mythicbeastsProvider.go
Original file line number Diff line number Diff line change
Expand Up @@ -115,10 +115,11 @@ func zoneFileToRecords(r io.Reader, origin string) (models.Records, error) {

// GetZoneRecordsCorrections returns a list of corrections that will turn existing records into dc.Records.
func (n *mythicBeastsProvider) GetZoneRecordsCorrections(dc *models.DomainConfig, actual models.Records) ([]*models.Correction, int, error) {
msgs, changes, actualChangeCount, err := diff2.ByZone(actual, dc, nil)
result, err := diff2.ByZone(actual, dc, nil)
if err != nil {
return nil, 0, err
}
msgs, changes, actualChangeCount := result.Msgs, result.HasChanges, result.ActualChangeCount

var corrections []*models.Correction
if changes {
Expand All @@ -127,7 +128,7 @@ func (n *mythicBeastsProvider) GetZoneRecordsCorrections(dc *models.DomainConfig
Msg: strings.Join(msgs, "\n"),
F: func() error {
var b strings.Builder
for _, record := range dc.Records {
for _, record := range result.DesiredPlus {
switch rr := record.ToRR().(type) {
case *dns.SSHFP:
// "Hex strings [for SSHFP] must be in lower-case", per Mythic Beasts API docs.
Expand Down
7 changes: 4 additions & 3 deletions providers/realtimeregister/realtimeregisterProvider.go
Original file line number Diff line number Diff line change
Expand Up @@ -107,10 +107,11 @@ func (api *realtimeregisterAPI) GetZoneRecords(domain string, meta map[string]st
}

func (api *realtimeregisterAPI) GetZoneRecordsCorrections(dc *models.DomainConfig, existing models.Records) ([]*models.Correction, int, error) {
msgs, changes, actualChangeCount, err := diff2.ByZone(existing, dc, nil)
result, err := diff2.ByZone(existing, dc, nil)
if err != nil {
return nil, 0, err
}
msgs, changes, actualChangeCount := result.Msgs, result.HasChanges, result.ActualChangeCount

var corrections []*models.Correction

Expand Down Expand Up @@ -149,8 +150,8 @@ func (api *realtimeregisterAPI) GetZoneRecordsCorrections(dc *models.DomainConfi
&models.Correction{
Msg: strings.Join(msgs, "\n"),
F: func() error {
records := make([]Record, len(dc.Records))
for i, r := range dc.Records {
records := make([]Record, len(result.DesiredPlus))
for i, r := range result.DesiredPlus {
records[i] = toRecord(r)
}
zone := &Zone{Records: records, Dnssec: dnssec}
Expand Down
3 changes: 2 additions & 1 deletion providers/sakuracloud/records.go
Original file line number Diff line number Diff line change
Expand Up @@ -65,10 +65,11 @@ func (s *sakuracloudProvider) GetZoneRecordsCorrections(dc *models.DomainConfig,
}
}

msgs, changes, actualChangeCount, err := diff2.ByZone(existing, dc, nil)
result, err := diff2.ByZone(existing, dc, nil)
if err != nil {
return nil, 0, err
}
msgs, changes, actualChangeCount := result.Msgs, result.HasChanges, result.ActualChangeCount
if !changes {
return nil, actualChangeCount, nil
}
Expand Down
Loading