Pull request: all: fix statip ip ck

Merge in DNS/adguard-home from fix-static-ip-check to master

Squashed commit of the following:

commit af365c106f3d620afc77492a06f5368611328f5f
Author: Ainar Garipov <A.Garipov@AdGuard.COM>
Date:   Mon Feb 15 18:55:35 2021 +0300

    all: doc changes

commit 922afb262458fc488e03cad232430d90c504f2f3
Merge: 43fec5fb dbcc55f5
Author: Ainar Garipov <A.Garipov@AdGuard.COM>
Date:   Mon Feb 15 18:53:31 2021 +0300

    Merge branch 'master' into fix-static-ip-check

commit 43fec5fb79f5c67b375da00aa73d11b3ed9ba3a4
Author: Ainar Garipov <A.Garipov@AdGuard.COM>
Date:   Mon Feb 15 18:37:16 2021 +0300

    all: fix statip ip ck

CHANGELOG.md

@@ -31,6 +31,8 @@ and this project adheres to
 
 ### Fixed
 
+- Error when enabling the DHCP server when AdGuard Home couldn't determine if
+  the machine has a static IP.
 - Optical issue on custom rules ([#2641]).
 - Occasional crashes during startup.
 - The field `"range_start"` in the `GET /control/dhcp/status` HTTP API response

internal/dhcpd/http.go

@@ -95,26 +95,40 @@ func (s *Server) enableDHCP(ifaceName string) (code int, err error) {
 	var hasStaticIP bool
 	hasStaticIP, err = sysutil.IfaceHasStaticIP(ifaceName)
 	if err != nil {
-		// ErrPermission may happen here on Linux systems where AdGuard
-		// Home is installed using Snap.  That doesn't necessarily mean
-		// that the machine doesn't have a static IP, so we can assume
-		// that it has and go on.  If the machine doesn't, we'll get an
-		// error later.
-		//
-		// See https://github.com/AdguardTeam/AdGuardHome/issues/2667.
 		if errors.Is(err, os.ErrPermission) {
+			// ErrPermission may happen here on Linux systems where
+			// AdGuard Home is installed using Snap.  That doesn't
+			// necessarily mean that the machine doesn't have
+			// a static IP, so we can assume that it has and go on.
+			// If the machine doesn't, we'll get an error later.
+			//
+			// See https://github.com/AdguardTeam/AdGuardHome/issues/2667.
+			//
+			// TODO(a.garipov): I was thinking about moving this
+			// into IfaceHasStaticIP, but then we wouldn't be able
+			// to log it.  Think about it more.
 			log.Info("error while checking static ip: %s; "+
 				"assuming machine has static ip and going on", err)
 			hasStaticIP = true
+		} else if errors.Is(err, sysutil.ErrNoStaticIPInfo) {
+			// Couldn't obtain a definitive answer.  Assume static
+			// IP an go on.
+			log.Info("can't check for static ip; " +
+				"assuming machine has static ip and going on")
+			hasStaticIP = true
 		} else {
-			return http.StatusInternalServerError, fmt.Errorf("checking static ip: %w", err)
+			err = fmt.Errorf("checking static ip: %w", err)
+
+			return http.StatusInternalServerError, err
 		}
 	}
 
 	if !hasStaticIP {
 		err = sysutil.IfaceSetStaticIP(ifaceName)
 		if err != nil {
-			return http.StatusInternalServerError, fmt.Errorf("setting static ip: %w", err)
+			err = fmt.Errorf("setting static ip: %w", err)
+
+			return http.StatusInternalServerError, err
 		}
 	}
 

internal/sysutil/net.go

@@ -5,10 +5,17 @@ import (
 	"os/exec"
 	"strings"
 
+	"github.com/AdguardTeam/AdGuardHome/internal/agherr"
 	"github.com/AdguardTeam/golibs/log"
 )
 
+// ErrNoStaticIPInfo is returned by IfaceHasStaticIP when no information about
+// the IP being static is available.
+const ErrNoStaticIPInfo agherr.Error = "no information about static ip"
+
 // IfaceHasStaticIP checks if interface is configured to have static IP address.
+// If it can't give a definitive answer, it returns false and an error for which
+// errors.Is(err, ErrNoStaticIPInfo) is true.
 func IfaceHasStaticIP(ifaceName string) (has bool, err error) {
 	return ifaceHasStaticIP(ifaceName)
 }

internal/sysutil/net_linux.go

@@ -21,7 +21,11 @@ import (
 const maxConfigFileSize = 1024 * 1024
 
 func ifaceHasStaticIP(ifaceName string) (has bool, err error) {
-	var f *os.File
+	// TODO(a.garipov): Currently, this function returns the first
+	// definitive result.  So if /etc/dhcpcd.conf has a static IP while
+	// /etc/network/interfaces doesn't, it will return true.  Perhaps this
+	// is not the most desirable behavior.
+
 	for _, check := range []struct {
 		checker  func(io.Reader, string) (bool, error)
 		filePath string
@@ -32,8 +36,11 @@ func ifaceHasStaticIP(ifaceName string) (has bool, err error) {
 		checker:  ifacesStaticConfig,
 		filePath: "/etc/network/interfaces",
 	}} {
+		var f *os.File
 		f, err = os.Open(check.filePath)
 		if err != nil {
+			// ErrNotExist can happen here if there is no such file.
+			// This is normal, as not every system uses those files.
 			if errors.Is(err, os.ErrNotExist) {
 				err = nil
 
@@ -52,12 +59,14 @@ func ifaceHasStaticIP(ifaceName string) (has bool, err error) {
 		defer fileReadCloser.Close()
 
 		has, err = check.checker(fileReadCloser, ifaceName)
-		if has || err != nil {
+		if err != nil {
-			break
+			return false, err
 		}
+
+		return has, nil
 	}
 
-	return has, err
+	return false, ErrNoStaticIPInfo
 }
 
 // dhcpcdStaticConfig checks if interface is configured by /etc/dhcpcd.conf to