From c93084e6234f8b128e3dcced6d1afafd8772ddd5 Mon Sep 17 00:00:00 2001
From: Marc Brugger <github@bakito.ch>
Date: Wed, 12 Apr 2023 20:02:55 +0200
Subject: [PATCH] Only sync dhcp config if it is valid (#184)

* handle new install page redirect location

* only sync dhcp config if valid
---
 .gitignore              |  1 +
 pkg/sync/sync.go        |  2 +-
 pkg/sync/sync_test.go   |  9 ++++++++-
 pkg/types/dhcp.go       | 12 ++++++++++++
 pkg/types/types_test.go | 31 +++++++++++++++++++++++++++++++
 5 files changed, 53 insertions(+), 2 deletions(-)

diff --git a/.gitignore b/.gitignore
index 47e5d0a..e4f6e9b 100644
--- a/.gitignore
+++ b/.gitignore
@@ -7,3 +7,4 @@ main
 tmp
 bin
 config.yaml
+*.log
diff --git a/pkg/sync/sync.go b/pkg/sync/sync.go
index b6b70fa..d246959 100644
--- a/pkg/sync/sync.go
+++ b/pkg/sync/sync.go
@@ -490,7 +490,7 @@ func (w *worker) syncDHCPServer(osc *types.DHCPServerConfig, rc client.Client, r
 		return nil
 	}
 	sc, err := rc.DHCPServerConfig()
-	if w.cfg.Features.DHCP.ServerConfig {
+	if w.cfg.Features.DHCP.ServerConfig && osc.HasConfig() {
 		if err != nil {
 			return err
 		}
diff --git a/pkg/sync/sync_test.go b/pkg/sync/sync_test.go
index af50482..677407b 100644
--- a/pkg/sync/sync_test.go
+++ b/pkg/sync/sync_test.go
@@ -2,6 +2,7 @@ package sync
 
 import (
 	"errors"
+	"net"
 
 	"github.com/bakito/adguardhome-sync/pkg/client"
 	clientmock "github.com/bakito/adguardhome-sync/pkg/mocks/client"
@@ -433,11 +434,17 @@ var _ = Describe("Sync", func() {
 				rsc *types.DHCPServerConfig
 			)
 			BeforeEach(func() {
-				osc = &types.DHCPServerConfig{}
+				osc = &types.DHCPServerConfig{V4: &types.V4ServerConfJSON{
+					GatewayIP:  net.IPv4(1, 2, 3, 4),
+					RangeStart: net.IPv4(1, 2, 3, 5),
+					RangeEnd:   net.IPv4(1, 2, 3, 6),
+					SubnetMask: net.IPv4(255, 255, 255, 0),
+				}}
 				rsc = &types.DHCPServerConfig{}
 				w.cfg.Features.DHCP.StaticLeases = false
 			})
 			It("should have no changes", func() {
+				rsc.V4 = osc.V4
 				cl.EXPECT().DHCPServerConfig().Return(rsc, nil)
 				err := w.syncDHCPServer(osc, cl, types.AdGuardInstance{})
 				Ω(err).ShouldNot(HaveOccurred())
diff --git a/pkg/types/dhcp.go b/pkg/types/dhcp.go
index a2860b7..b63b516 100644
--- a/pkg/types/dhcp.go
+++ b/pkg/types/dhcp.go
@@ -33,6 +33,10 @@ func (c *DHCPServerConfig) Equals(o *DHCPServerConfig) bool {
 	return string(a) == string(b)
 }
 
+func (c *DHCPServerConfig) HasConfig() bool {
+	return (c.V4 != nil && c.V4.isValid()) || (c.V6 != nil && c.V6.isValid())
+}
+
 // V4ServerConfJSON v4 server conf
 type V4ServerConfJSON struct {
 	GatewayIP     net.IP `json:"gateway_ip"`
@@ -42,6 +46,10 @@ type V4ServerConfJSON struct {
 	LeaseDuration uint32 `json:"lease_duration"`
 }
 
+func (j V4ServerConfJSON) isValid() bool {
+	return j.GatewayIP != nil && j.SubnetMask != nil && j.RangeStart != nil && j.RangeEnd != nil
+}
+
 // V6ServerConfJSON v6 server conf
 type V6ServerConfJSON struct {
 	RangeStart    net.IP `json:"range_start"`
@@ -49,6 +57,10 @@ type V6ServerConfJSON struct {
 	LeaseDuration uint32 `json:"lease_duration"`
 }
 
+func (j V6ServerConfJSON) isValid() bool {
+	return j.RangeStart != nil && j.RangeEnd != nil
+}
+
 // Leases slice of leases type
 type Leases []Lease
 
diff --git a/pkg/types/types_test.go b/pkg/types/types_test.go
index 998a007..8ecdb8a 100644
--- a/pkg/types/types_test.go
+++ b/pkg/types/types_test.go
@@ -420,5 +420,36 @@ var _ = Describe("Types", func() {
 				Ω(dc1.Clone().Equals(dc1)).Should(BeTrue())
 			})
 		})
+		Context("HasConfig", func() {
+			It("should not have a config", func() {
+				dc1 := &types.DHCPServerConfig{
+					V4: &types.V4ServerConfJSON{},
+					V6: &types.V6ServerConfJSON{},
+				}
+				Ω(dc1.HasConfig()).Should(BeFalse())
+			})
+			It("should not have a v4 config", func() {
+				dc1 := &types.DHCPServerConfig{
+					V4: &types.V4ServerConfJSON{
+						GatewayIP:  net.IPv4(1, 2, 3, 4),
+						RangeStart: net.IPv4(1, 2, 3, 5),
+						RangeEnd:   net.IPv4(1, 2, 3, 6),
+						SubnetMask: net.IPv4(255, 255, 255, 0),
+					},
+					V6: &types.V6ServerConfJSON{},
+				}
+				Ω(dc1.HasConfig()).Should(BeTrue())
+			})
+			It("should not have a v6 config", func() {
+				dc1 := &types.DHCPServerConfig{
+					V4: &types.V4ServerConfJSON{},
+					V6: &types.V6ServerConfJSON{
+						RangeStart: net.IPv4(1, 2, 3, 5),
+						RangeEnd:   net.IPv4(1, 2, 3, 6),
+					},
+				}
+				Ω(dc1.HasConfig()).Should(BeTrue())
+			})
+		})
 	})
 })