Apply DNS server update only on changes

[hurricanehrndz]

Add configuration tracking and comparison logic to avoid updating the DNS server when the configuration hasn't changed. This prevents potential DNS resolution disruptions and unnecessary processing cycles.

Changes:

• Add lastDNSConfig field to Engine struct to track applied configuration
• Implement dnsConfigsEqual() for deep comparison of DNS configurations
• Only call UpdateDNSServer when configuration actually changes

Describe your changes

Issue ticket number and link

Stack

Checklist

• Is it a bug fix
• Is a typo/documentation fix
• Is a feature enhancement
• It is a refactor
• Created tests that fail without the change (if possible)

By submitting this pull request, you confirm that you have read and agree to the terms of the Contributor License Agreement.

Documentation

Select exactly one:

• I added/updated documentation for this change
• Documentation is not needed for this change (explain why)

Docs PR URL (required if "docs added" is checked)

Paste the PR link from https://github.com/netbirdio/docs here:

https://github.com/netbirdio/docs/pull/__

[Copilot]

Pull Request Overview

This PR optimizes DNS server updates by tracking and comparing DNS configurations to avoid unnecessary updates when the configuration hasn't changed. This prevents potential DNS resolution disruptions and reduces unnecessary processing overhead.

Key changes:

• Added configuration tracking via a lastDNSConfig field in the Engine struct
• Implemented deep comparison logic through dnsConfigsEqual() function
• Modified update logic to only call UpdateDNSServer when configuration differs

---

_{Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.}

[Copilot]

Direct indexing without bounds checking could cause a panic. Although line 1276 checks length equality, if CustomZones order differs between configs with same length, this still represents unequal configs but the function may not detect it correctly. Consider using a map-based comparison or iterating with explicit bounds checks.

Suggested change

	for i, zoneA := range a.CustomZones {
	zoneB := b.CustomZones[i]
	if zoneA.Domain != zoneB.Domain {
	return false
	}
	if len(zoneA.Records) != len(zoneB.Records) {
	return false
	}
	for j, recordA := range zoneA.Records {
	recordB := zoneB.Records[j]
	if recordA.Name != recordB.Name ||
	recordA.Type != recordB.Type ||
	recordA.Class != recordB.Class ||
	recordA.TTL != recordB.TTL ||
	recordA.RData != recordB.RData {
	return false
	}
	}
	}
	for i, nsGroupA := range a.NameServerGroups {
	nsGroupB := b.NameServerGroups[i]
	if nsGroupA.Primary != nsGroupB.Primary ||
	nsGroupA.SearchDomainsEnabled != nsGroupB.SearchDomainsEnabled {
	return false
	}
	if len(nsGroupA.Domains) != len(nsGroupB.Domains) {
	return false
	}
	for j, domainA := range nsGroupA.Domains {
	if domainA != nsGroupB.Domains[j] {
	return false
	}
	}
	if len(nsGroupA.NameServers) != len(nsGroupB.NameServers) {
	return false
	}
	for j, nsA := range nsGroupA.NameServers {
	nsB := nsGroupB.NameServers[j]
	if nsA.IP != nsB.IP ||
	nsA.NSType != nsB.NSType ||
	nsA.Port != nsB.Port {
	// Compare CustomZones by domain, order-independent
	zonesA := make(map[string]nbdns.CustomZone, len(a.CustomZones))
	for _, zone := range a.CustomZones {
	zonesA[zone.Domain] = zone
	}
	zonesB := make(map[string]nbdns.CustomZone, len(b.CustomZones))
	for _, zone := range b.CustomZones {
	zonesB[zone.Domain] = zone
	}
	if len(zonesA) != len(zonesB) {
	return false
	}
	for domain, zoneA := range zonesA {
	zoneB, ok := zonesB[domain]
	if !ok {
	return false
	}
	if len(zoneA.Records) != len(zoneB.Records) {
	return false
	}
	// Compare records order-independently
	type recordKey struct {
	Name string
	Type string
	Class string
	TTL uint32
	RData string
	}
	recordsA := make(map[recordKey]struct{}, len(zoneA.Records))
	for _, record := range zoneA.Records {
	k := recordKey{
	Name: record.Name,
	Type: record.Type,
	Class: record.Class,
	TTL: record.TTL,
	RData: record.RData,
	}
	recordsA[k] = struct{}{}
	}
	recordsB := make(map[recordKey]struct{}, len(zoneB.Records))
	for _, record := range zoneB.Records {
	k := recordKey{
	Name: record.Name,
	Type: record.Type,
	Class: record.Class,
	TTL: record.TTL,
	RData: record.RData,
	}
	recordsB[k] = struct{}{}
	}
	if len(recordsA) != len(recordsB) {
	return false
	}
	for k := range recordsA {
	if _, ok := recordsB[k]; !ok {
	return false
	}
	}
	}
	// Compare NameServerGroups order-independently by Primary and Domains
	type nsGroupKey struct {
	Primary bool
	SearchDomainsEnabled bool
	Domains string // concatenated domains
	}
	nsGroupsA := make(map[nsGroupKey]nbdns.NameServerGroup, len(a.NameServerGroups))
	for _, nsGroup := range a.NameServerGroups {
	domains := make([]string, len(nsGroup.Domains))
	copy(domains, nsGroup.Domains)
	sort.Strings(domains)
	key := nsGroupKey{
	Primary: nsGroup.Primary,
	SearchDomainsEnabled: nsGroup.SearchDomainsEnabled,
	Domains: strings.Join(domains, ","),
	}
	nsGroupsA[key] = nsGroup
	}
	nsGroupsB := make(map[nsGroupKey]nbdns.NameServerGroup, len(b.NameServerGroups))
	for _, nsGroup := range b.NameServerGroups {
	domains := make([]string, len(nsGroup.Domains))
	copy(domains, nsGroup.Domains)
	sort.Strings(domains)
	key := nsGroupKey{
	Primary: nsGroup.Primary,
	SearchDomainsEnabled: nsGroup.SearchDomainsEnabled,
	Domains: strings.Join(domains, ","),
	}
	nsGroupsB[key] = nsGroup
	}
	if len(nsGroupsA) != len(nsGroupsB) {
	return false
	}
	for key, nsGroupA := range nsGroupsA {
	nsGroupB, ok := nsGroupsB[key]
	if !ok {
	return false
	}
	// Compare NameServers order-independently
	if len(nsGroupA.NameServers) != len(nsGroupB.NameServers) {
	return false
	}
	type nsKey struct {
	IP string
	NSType string
	Port uint32
	}
	nsA := make(map[nsKey]struct{}, len(nsGroupA.NameServers))
	for _, ns := range nsGroupA.NameServers {
	k := nsKey{
	IP: ns.IP,
	NSType: ns.NSType,
	Port: ns.Port,
	}
	nsA[k] = struct{}{}
	}
	nsB := make(map[nsKey]struct{}, len(nsGroupB.NameServers))
	for _, ns := range nsGroupB.NameServers {
	k := nsKey{
	IP: ns.IP,
	NSType: ns.NSType,
	Port: ns.Port,
	}
	nsB[k] = struct{}{}
	}
	if len(nsA) != len(nsB) {
	return false
	}
	for k := range nsA {
	if _, ok := nsB[k]; !ok {

[Copilot]

Direct indexing assumes order matters for equality. If the same NameServerGroups exist in different orders, they would be considered unequal. This may be intentional, but could lead to unnecessary DNS server updates if the order changes but content remains the same. Consider whether order-independent comparison is needed.

Suggested change

	for i, nsGroupA := range a.NameServerGroups {
	nsGroupB := b.NameServerGroups[i]
	if nsGroupA.Primary != nsGroupB.Primary ||
	nsGroupA.SearchDomainsEnabled != nsGroupB.SearchDomainsEnabled {
	return false
	}
	if len(nsGroupA.Domains) != len(nsGroupB.Domains) {
	return false
	}
	for j, domainA := range nsGroupA.Domains {
	if domainA != nsGroupB.Domains[j] {
	return false
	}
	}
	if len(nsGroupA.NameServers) != len(nsGroupB.NameServers) {
	return false
	}
	for j, nsA := range nsGroupA.NameServers {
	nsB := nsGroupB.NameServers[j]
	if nsA.IP != nsB.IP ||
	nsA.NSType != nsB.NSType ||
	nsA.Port != nsB.Port {
	return false
	}
	}
	// Compare NameServerGroups in an order-independent way
	if !nameServerGroupsEqual(a.NameServerGroups, b.NameServerGroups) {
	return false

[Copilot]

Direct indexing without bounds checking. While line 1309 checks length equality, consider using an explicit range check for defensive programming to prevent potential index out of bounds panics.

Suggested change

	if domainA != nsGroupB.Domains[j] {
	if j >= len(nsGroupB.Domains) || domainA != nsGroupB.Domains[j] {

[Copilot]

Direct indexing without bounds checking. Although line 1317 verifies length equality, adding explicit bounds verification would make the code more defensive against potential runtime panics.

[sonarqubecloud]

Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud