From a95c2741a7e3e5bfe8775bf937a3709217b76da0 Mon Sep 17 00:00:00 2001 From: Eugene Burkov Date: Wed, 31 Aug 2022 16:24:02 +0300 Subject: [PATCH] dhcpd: separate os files --- internal/dhcpd/dhcpd.go | 21 ++---- internal/dhcpd/{http.go => http_others.go} | 71 +++---------------- internal/dhcpd/http_windows.go | 55 ++++++++++++++ .../{http_test.go => http_windows_test.go} | 11 ++- internal/dhcpd/server.go | 4 ++ internal/dhcpd/v4.go | 21 +++--- 6 files changed, 95 insertions(+), 88 deletions(-) rename internal/dhcpd/{http.go => http_others.go} (87%) create mode 100644 internal/dhcpd/http_windows.go rename internal/dhcpd/{http_test.go => http_windows_test.go} (63%) diff --git a/internal/dhcpd/dhcpd.go b/internal/dhcpd/dhcpd.go index a085e6562da..006424e29d8 100644 --- a/internal/dhcpd/dhcpd.go +++ b/internal/dhcpd/dhcpd.go @@ -6,12 +6,12 @@ import ( "fmt" "net" "path/filepath" - "runtime" "time" "github.com/AdguardTeam/AdGuardHome/internal/aghhttp" "github.com/AdguardTeam/golibs/log" "github.com/AdguardTeam/golibs/netutil" + "github.com/AdguardTeam/golibs/timeutil" ) const ( @@ -21,6 +21,11 @@ const ( // TODO(e.burkov): Remove it when static leases determining mechanism // will be improved. leaseExpireStatic = 1 + + // DefaultDHCPLeaseTTL is the default time-to-live for leases. + DefaultDHCPLeaseTTL = uint32(timeutil.Day / time.Second) + // DefaultDHCPTimeoutICMP is the default timeout for waiting ICMP responses. + DefaultDHCPTimeoutICMP = 1000 ) var webHandlersRegistered = false @@ -205,19 +210,7 @@ func Create(conf *ServerConfig) (s *Server, err error) { } if !webHandlersRegistered && s.conf.HTTPRegister != nil { - if runtime.GOOS == "windows" { - // Our DHCP server doesn't work on Windows yet, so - // signal that to the front with an HTTP 501. - // - // TODO(a.garipov): This needs refactoring. We - // shouldn't even try and initialize a DHCP server on - // Windows, but there are currently too many - // interconnected parts--such as HTTP handlers and - // frontend--to make that work properly. - s.registerNotImplementedHandlers() - } else { - s.registerHandlers() - } + s.registerHandlers() webHandlersRegistered = true } diff --git a/internal/dhcpd/http.go b/internal/dhcpd/http_others.go similarity index 87% rename from internal/dhcpd/http.go rename to internal/dhcpd/http_others.go index a8f0c108835..ed99c35ee37 100644 --- a/internal/dhcpd/http.go +++ b/internal/dhcpd/http_others.go @@ -1,3 +1,5 @@ +//go:build aix || darwin || dragonfly || linux || netbsd || solaris || freebsd || openbsd + package dhcpd import ( @@ -8,14 +10,12 @@ import ( "net/http" "os" "strings" - "time" "github.com/AdguardTeam/AdGuardHome/internal/aghalg" "github.com/AdguardTeam/AdGuardHome/internal/aghhttp" "github.com/AdguardTeam/AdGuardHome/internal/aghnet" "github.com/AdguardTeam/golibs/errors" "github.com/AdguardTeam/golibs/log" - "github.com/AdguardTeam/golibs/timeutil" ) type v4ServerConfJSON struct { @@ -81,6 +81,7 @@ func (s *Server) handleDHCPStatus(w http.ResponseWriter, r *http.Request) { status.StaticLeases = s.Leases(LeasesStatic) w.Header().Set("Content-Type", "application/json") + err := json.NewEncoder(w).Encode(status) if err != nil { aghhttp.Error( @@ -497,20 +498,16 @@ func (s *Server) handleDHCPAddStaticLease(w http.ResponseWriter, r *http.Request return } - ip4 := l.IP.To4() - if ip4 == nil { + var srv DHCPServer + if ip4 := l.IP.To4(); ip4 != nil { + l.IP = ip4 + srv = s.srv4 + } else { l.IP = l.IP.To16() - - err = s.srv6.AddStaticLease(l) - if err != nil { - aghhttp.Error(r, w, http.StatusBadRequest, "%s", err) - } - - return + srv = s.srv6 } - l.IP = ip4 - err = s.srv4.AddStaticLease(l) + err = srv.AddStaticLease(l) if err != nil { aghhttp.Error(r, w, http.StatusBadRequest, "%s", err) @@ -555,13 +552,6 @@ func (s *Server) handleDHCPRemoveStaticLease(w http.ResponseWriter, r *http.Requ } } -const ( - // DefaultDHCPLeaseTTL is the default time-to-live for leases. - DefaultDHCPLeaseTTL = uint32(timeutil.Day / time.Second) - // DefaultDHCPTimeoutICMP is the default timeout for waiting ICMP responses. - DefaultDHCPTimeoutICMP = 1000 -) - func (s *Server) handleReset(w http.ResponseWriter, r *http.Request) { err := s.Stop() if err != nil { @@ -622,44 +612,3 @@ func (s *Server) registerHandlers() { s.conf.HTTPRegister(http.MethodPost, "/control/dhcp/reset", s.handleReset) s.conf.HTTPRegister(http.MethodPost, "/control/dhcp/reset_leases", s.handleResetLeases) } - -// jsonError is a generic JSON error response. -// -// TODO(a.garipov): Merge together with the implementations in .../home and -// other packages after refactoring the web handler registering. -type jsonError struct { - // Message is the error message, an opaque string. - Message string `json:"message"` -} - -// notImplemented returns a handler that replies to any request with an HTTP 501 -// Not Implemented status and a JSON error with the provided message msg. -// -// TODO(a.garipov): Either take the logger from the server after we've -// refactored logging or make this not a method of *Server. -func (s *Server) notImplemented(msg string) (f func(http.ResponseWriter, *http.Request)) { - return func(w http.ResponseWriter, _ *http.Request) { - w.Header().Set("Content-Type", "application/json") - w.WriteHeader(http.StatusNotImplemented) - - err := json.NewEncoder(w).Encode(&jsonError{ - Message: msg, - }) - if err != nil { - log.Debug("writing 501 json response: %s", err) - } - } -} - -func (s *Server) registerNotImplementedHandlers() { - h := s.notImplemented("dhcp is not supported on windows") - - s.conf.HTTPRegister(http.MethodGet, "/control/dhcp/status", h) - s.conf.HTTPRegister(http.MethodGet, "/control/dhcp/interfaces", h) - s.conf.HTTPRegister(http.MethodPost, "/control/dhcp/set_config", h) - s.conf.HTTPRegister(http.MethodPost, "/control/dhcp/find_active_dhcp", h) - s.conf.HTTPRegister(http.MethodPost, "/control/dhcp/add_static_lease", h) - s.conf.HTTPRegister(http.MethodPost, "/control/dhcp/remove_static_lease", h) - s.conf.HTTPRegister(http.MethodPost, "/control/dhcp/reset", h) - s.conf.HTTPRegister(http.MethodPost, "/control/dhcp/reset_leases", h) -} diff --git a/internal/dhcpd/http_windows.go b/internal/dhcpd/http_windows.go new file mode 100644 index 00000000000..8241e419103 --- /dev/null +++ b/internal/dhcpd/http_windows.go @@ -0,0 +1,55 @@ +//go:build windows + +package dhcpd + +import ( + "encoding/json" + "net/http" + + "github.com/AdguardTeam/AdGuardHome/internal/aghos" + "github.com/AdguardTeam/golibs/log" +) + +// jsonError is a generic JSON error response. +// +// TODO(a.garipov): Merge together with the implementations in .../home and +// other packages after refactoring the web handler registering. +type jsonError struct { + // Message is the error message, an opaque string. + Message string `json:"message"` +} + +// notImplemented returns a handler that replies to any request with an HTTP 501 +// Not Implemented status and a JSON error with the provided message msg. +// +// TODO(a.garipov): Either take the logger from the server after we've +// refactored logging or make this not a method of *Server. +func (s *Server) notImplemented(w http.ResponseWriter, r *http.Request) { + w.Header().Set("Content-Type", "application/json") + w.WriteHeader(http.StatusNotImplemented) + + err := json.NewEncoder(w).Encode(&jsonError{ + Message: aghos.Unsupported("dhcp").Error(), + }) + if err != nil { + log.Debug("writing 501 json response: %s", err) + } +} + +// registerHandlers sets the handlers for DHCP HTTP API that always respond with +// an HTTP 501, since DHCP server doesn't work on Windows yet. +// +// TODO(a.garipov): This needs refactoring. We shouldn't even try and +// initialize a DHCP server on Windows, but there are currently too many +// interconnected parts--such as HTTP handlers and frontend--to make that work +// properly. +func (s *Server) registerHandlers() { + s.conf.HTTPRegister(http.MethodGet, "/control/dhcp/status", s.notImplemented) + s.conf.HTTPRegister(http.MethodGet, "/control/dhcp/interfaces", s.notImplemented) + s.conf.HTTPRegister(http.MethodPost, "/control/dhcp/set_config", s.notImplemented) + s.conf.HTTPRegister(http.MethodPost, "/control/dhcp/find_active_dhcp", s.notImplemented) + s.conf.HTTPRegister(http.MethodPost, "/control/dhcp/add_static_lease", s.notImplemented) + s.conf.HTTPRegister(http.MethodPost, "/control/dhcp/remove_static_lease", s.notImplemented) + s.conf.HTTPRegister(http.MethodPost, "/control/dhcp/reset", s.notImplemented) + s.conf.HTTPRegister(http.MethodPost, "/control/dhcp/reset_leases", s.notImplemented) +} diff --git a/internal/dhcpd/http_test.go b/internal/dhcpd/http_windows_test.go similarity index 63% rename from internal/dhcpd/http_test.go rename to internal/dhcpd/http_windows_test.go index 120e02a3a27..22c26da01a2 100644 --- a/internal/dhcpd/http_test.go +++ b/internal/dhcpd/http_windows_test.go @@ -1,23 +1,28 @@ +//go:build windows + package dhcpd import ( + "fmt" "net/http" "net/http/httptest" "testing" + "github.com/AdguardTeam/AdGuardHome/internal/aghos" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" ) func TestServer_notImplemented(t *testing.T) { s := &Server{} - h := s.notImplemented("never!") w := httptest.NewRecorder() r, err := http.NewRequest(http.MethodGet, "/unsupported", nil) require.NoError(t, err) - h(w, r) + s.notImplemented(w, r) assert.Equal(t, http.StatusNotImplemented, w.Code) - assert.Equal(t, `{"message":"never!"}`+"\n", w.Body.String()) + + wantStr := fmt.Sprintf("{%q:%q}", "message", aghos.Unsupported("dhcp")) + assert.JSONEq(t, wantStr, w.Body.String()) } diff --git a/internal/dhcpd/server.go b/internal/dhcpd/server.go index be88804b749..32723468666 100644 --- a/internal/dhcpd/server.go +++ b/internal/dhcpd/server.go @@ -80,6 +80,8 @@ type V4ServerConf struct { notify func(uint32) } +// TODO(e.burkov): !! validation + // V6ServerConf - server configuration type V6ServerConf struct { Enabled bool `yaml:"-" json:"-"` @@ -101,3 +103,5 @@ type V6ServerConf struct { // Server calls this function when leases data changes notify func(uint32) } + +// TODO(e.burkov): !! validation diff --git a/internal/dhcpd/v4.go b/internal/dhcpd/v4.go index 5eec00bd85c..2ed525c28ca 100644 --- a/internal/dhcpd/v4.go +++ b/internal/dhcpd/v4.go @@ -1233,18 +1233,19 @@ func (s *v4Server) Stop() (err error) { // Create DHCPv4 server func v4Create(conf V4ServerConf) (srv DHCPServer, err error) { - s := &v4Server{} - s.conf = conf - s.leaseHosts = stringutil.NewSet() + s := &v4Server{ + conf: conf, + leaseHosts: stringutil.NewSet(), + } - // TODO(a.garipov): Don't use a disabled server in other places or just - // use an interface. + // TODO(a.garipov): Don't use a disabled server in other places or just use + // an interface. if !conf.Enabled { return s, nil } - var routerIP net.IP - routerIP, err = tryTo4(s.conf.GatewayIP) + var gatewayIP net.IP + gatewayIP, err = tryTo4(s.conf.GatewayIP) if err != nil { return s, fmt.Errorf("dhcpv4: %w", err) } @@ -1257,7 +1258,7 @@ func v4Create(conf V4ServerConf) (srv DHCPServer, err error) { copy(subnetMask, s.conf.SubnetMask.To4()) s.conf.subnet = &net.IPNet{ - IP: routerIP, + IP: gatewayIP, Mask: subnetMask, } s.conf.broadcastIP = aghnet.BroadcastFromIPNet(s.conf.subnet) @@ -1267,9 +1268,9 @@ func v4Create(conf V4ServerConf) (srv DHCPServer, err error) { return s, fmt.Errorf("dhcpv4: %w", err) } - if s.conf.ipRange.contains(routerIP) { + if s.conf.ipRange.contains(gatewayIP) { return s, fmt.Errorf("dhcpv4: gateway ip %v in the ip range: %v-%v", - routerIP, + gatewayIP, conf.RangeStart, conf.RangeEnd, )