cherry-pick: all: imp updater

Merge in DNS/adguard-home from imp-updater to master

Squashed commit of the following:

commit 6ed487359e56a35b36f13dcbf2efbf2a7a2d8734
Author: Ainar Garipov <A.Garipov@AdGuard.COM>
Date:   Thu Jun 9 16:29:35 2022 +0300

    all: imp logs, err handling

commit e930044cb619a43e5a44c230dadbe2228e9a93f5
Author: Ainar Garipov <A.Garipov@AdGuard.COM>
Date:   Thu Jun 9 15:53:35 2022 +0300

    all: imp updater

internal/aghalg/nullbool.go

@@ -0,0 +1,59 @@
+package aghalg
+
+import (
+	"bytes"
+	"encoding/json"
+	"fmt"
+)
+
+// NullBool is a nullable boolean.  Use these in JSON requests and responses
+// instead of pointers to bool.
+type NullBool uint8
+
+// NullBool values
+const (
+	NBNull NullBool = iota
+	NBTrue
+	NBFalse
+)
+
+// String implements the fmt.Stringer interface for NullBool.
+func (nb NullBool) String() (s string) {
+	switch nb {
+	case NBNull:
+		return "null"
+	case NBTrue:
+		return "true"
+	case NBFalse:
+		return "false"
+	}
+
+	return fmt.Sprintf("!invalid NullBool %d", uint8(nb))
+}
+
+// BoolToNullBool converts a bool into a NullBool.
+func BoolToNullBool(cond bool) (nb NullBool) {
+	if cond {
+		return NBTrue
+	}
+
+	return NBFalse
+}
+
+// type check
+var _ json.Unmarshaler = (*NullBool)(nil)
+
+// UnmarshalJSON implements the json.Unmarshaler interface for *NullBool.
+func (nb *NullBool) UnmarshalJSON(b []byte) (err error) {
+	if len(b) == 0 || bytes.Equal(b, []byte("null")) {
+		*nb = NBNull
+	} else if bytes.Equal(b, []byte("true")) {
+		*nb = NBTrue
+	} else if bytes.Equal(b, []byte("false")) {
+		*nb = NBFalse
+	} else {
+		return fmt.Errorf("unmarshalling json data into aghalg.NullBool: bad value %q", b)
+	}
+
+	return nil
+}

internal/aghalg/nullbool_test.go

@@ -1,9 +1,10 @@
-package dhcpd
+package aghalg_test
 
 import (
 	"encoding/json"
 	"testing"
 
+	"github.com/AdguardTeam/AdGuardHome/internal/aghalg"
 	"github.com/AdguardTeam/golibs/testutil"
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
@@ -14,37 +15,37 @@ func TestNullBool_UnmarshalJSON(t *testing.T) {
 		name       string
 		wantErrMsg string
 		data       []byte
-		want       nullBool
+		want       aghalg.NullBool
 	}{{
 		name:       "empty",
 		wantErrMsg: "",
 		data:       []byte{},
-		want:       nbNull,
+		want:       aghalg.NBNull,
 	}, {
 		name:       "null",
 		wantErrMsg: "",
 		data:       []byte("null"),
-		want:       nbNull,
+		want:       aghalg.NBNull,
 	}, {
 		name:       "true",
 		wantErrMsg: "",
 		data:       []byte("true"),
-		want:       nbTrue,
+		want:       aghalg.NBTrue,
 	}, {
 		name:       "false",
 		wantErrMsg: "",
 		data:       []byte("false"),
-		want:       nbFalse,
+		want:       aghalg.NBFalse,
 	}, {
 		name:       "invalid",
-		wantErrMsg: `invalid nullBool value "invalid"`,
+		wantErrMsg: `unmarshalling json data into aghalg.NullBool: bad value "invalid"`,
 		data:       []byte("invalid"),
-		want:       nbNull,
+		want:       aghalg.NBNull,
 	}}
 
 	for _, tc := range testCases {
 		t.Run(tc.name, func(t *testing.T) {
-			var got nullBool
+			var got aghalg.NullBool
 			err := got.UnmarshalJSON(tc.data)
 			testutil.AssertErrorMsg(t, tc.wantErrMsg, err)
 
@@ -53,9 +54,9 @@ func TestNullBool_UnmarshalJSON(t *testing.T) {
 	}
 
 	t.Run("json", func(t *testing.T) {
-		want := nbTrue
+		want := aghalg.NBTrue
 		var got struct {
-			A nullBool
+			A aghalg.NullBool
 		}
 
 		err := json.Unmarshal([]byte(`{"A":true}`), &got)

internal/dhcpd/http.go

@@ -10,6 +10,7 @@ import (
 	"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"
@@ -145,7 +146,7 @@ type dhcpServerConfigJSON struct {
 	V4            *v4ServerConfJSON `json:"v4"`
 	V6            *v6ServerConfJSON `json:"v6"`
 	InterfaceName string            `json:"interface_name"`
-	Enabled       nullBool          `json:"enabled"`
+	Enabled       aghalg.NullBool   `json:"enabled"`
 }
 
 func (s *Server) handleDHCPSetConfigV4(
@@ -156,7 +157,7 @@ func (s *Server) handleDHCPSetConfigV4(
 	}
 
 	v4Conf := v4JSONToServerConf(conf.V4)
-	v4Conf.Enabled = conf.Enabled == nbTrue
+	v4Conf.Enabled = conf.Enabled == aghalg.NBTrue
 	if len(v4Conf.RangeStart) == 0 {
 		v4Conf.Enabled = false
 	}
@@ -183,7 +184,7 @@ func (s *Server) handleDHCPSetConfigV6(
 	}
 
 	v6Conf := v6JSONToServerConf(conf.V6)
-	v6Conf.Enabled = conf.Enabled == nbTrue
+	v6Conf.Enabled = conf.Enabled == aghalg.NBTrue
 	if len(v6Conf.RangeStart) == 0 {
 		v6Conf.Enabled = false
 	}
@@ -206,7 +207,7 @@ func (s *Server) handleDHCPSetConfigV6(
 
 func (s *Server) handleDHCPSetConfig(w http.ResponseWriter, r *http.Request) {
 	conf := &dhcpServerConfigJSON{}
-	conf.Enabled = boolToNullBool(s.conf.Enabled)
+	conf.Enabled = aghalg.BoolToNullBool(s.conf.Enabled)
 	conf.InterfaceName = s.conf.InterfaceName
 
 	err := json.NewDecoder(r.Body).Decode(conf)
@@ -230,7 +231,7 @@ func (s *Server) handleDHCPSetConfig(w http.ResponseWriter, r *http.Request) {
 		return
 	}
 
-	if conf.Enabled == nbTrue && !v4Enabled && !v6Enabled {
+	if conf.Enabled == aghalg.NBTrue && !v4Enabled && !v6Enabled {
 		aghhttp.Error(r, w, http.StatusBadRequest, "dhcpv4 or dhcpv6 configuration must be complete")
 
 		return
@@ -243,8 +244,8 @@ func (s *Server) handleDHCPSetConfig(w http.ResponseWriter, r *http.Request) {
 		return
 	}
 
-	if conf.Enabled != nbNull {
+	if conf.Enabled != aghalg.NBNull {
-		s.conf.Enabled = conf.Enabled == nbTrue
+		s.conf.Enabled = conf.Enabled == aghalg.NBTrue
 	}
 
 	if conf.InterfaceName != "" {
@@ -279,11 +280,11 @@ func (s *Server) handleDHCPSetConfig(w http.ResponseWriter, r *http.Request) {
 
 type netInterfaceJSON struct {
 	Name         string   `json:"name"`
-	GatewayIP    net.IP   `json:"gateway_ip"`
 	HardwareAddr string   `json:"hardware_address"`
+	Flags        string   `json:"flags"`
+	GatewayIP    net.IP   `json:"gateway_ip"`
 	Addrs4       []net.IP `json:"ipv4_addresses"`
 	Addrs6       []net.IP `json:"ipv6_addresses"`
-	Flags        string   `json:"flags"`
 }
 
 func (s *Server) handleDHCPInterfaces(w http.ResponseWriter, r *http.Request) {

internal/dhcpd/nullbool.go

@@ -1,58 +0,0 @@
-package dhcpd
-
-import (
-	"bytes"
-	"fmt"
-)
-
-// nullBool is a nullable boolean.  Use these in JSON requests and responses
-// instead of pointers to bool.
-//
-// TODO(a.garipov): Inspect uses of *bool, move this type into some new package
-// if we need it somewhere else.
-type nullBool uint8
-
-// nullBool values
-const (
-	nbNull nullBool = iota
-	nbTrue
-	nbFalse
-)
-
-// String implements the fmt.Stringer interface for nullBool.
-func (nb nullBool) String() (s string) {
-	switch nb {
-	case nbNull:
-		return "null"
-	case nbTrue:
-		return "true"
-	case nbFalse:
-		return "false"
-	}
-
-	return fmt.Sprintf("!invalid nullBool %d", uint8(nb))
-}
-
-// boolToNullBool converts a bool into a nullBool.
-func boolToNullBool(cond bool) (nb nullBool) {
-	if cond {
-		return nbTrue
-	}
-
-	return nbFalse
-}
-
-// UnmarshalJSON implements the json.Unmarshaler interface for *nullBool.
-func (nb *nullBool) UnmarshalJSON(b []byte) (err error) {
-	if len(b) == 0 || bytes.Equal(b, []byte("null")) {
-		*nb = nbNull
-	} else if bytes.Equal(b, []byte("true")) {
-		*nb = nbTrue
-	} else if bytes.Equal(b, []byte("false")) {
-		*nb = nbFalse
-	} else {
-		return fmt.Errorf("invalid nullBool value %q", b)
-	}
-
-	return nil
-}

internal/home/controlupdate.go

@@ -12,6 +12,7 @@ import (
 	"syscall"
 	"time"
 
+	"github.com/AdguardTeam/AdGuardHome/internal/aghalg"
 	"github.com/AdguardTeam/AdGuardHome/internal/aghhttp"
 	"github.com/AdguardTeam/AdGuardHome/internal/aghnet"
 	"github.com/AdguardTeam/AdGuardHome/internal/updater"
@@ -147,8 +148,8 @@ type versionResponse struct {
 // setAllowedToAutoUpdate sets CanAutoUpdate to true if AdGuard Home is actually
 // allowed to perform an automatic update by the OS.
 func (vr *versionResponse) setAllowedToAutoUpdate() (err error) {
-	if vr.CanAutoUpdate == nil || !*vr.CanAutoUpdate {
+	if vr.CanAutoUpdate != aghalg.NBTrue {
-		return
+		return nil
 	}
 
 	tlsConf := &tlsConfigSettings{}
@@ -162,7 +163,7 @@ func (vr *versionResponse) setAllowedToAutoUpdate() (err error) {
 		}
 	}
 
-	vr.CanAutoUpdate = &canUpdate
+	vr.CanAutoUpdate = aghalg.BoolToNullBool(canUpdate)
 
 	return nil
 }

internal/updater/check.go

@@ -5,9 +5,9 @@ import (
 	"fmt"
 	"io"
 	"net/http"
-	"strings"
 	"time"
 
+	"github.com/AdguardTeam/AdGuardHome/internal/aghalg"
 	"github.com/AdguardTeam/AdGuardHome/internal/aghio"
 	"github.com/AdguardTeam/golibs/errors"
 )
@@ -17,11 +17,12 @@ const versionCheckPeriod = 8 * time.Hour
 
 // VersionInfo contains information about a new version.
 type VersionInfo struct {
-	CanAutoUpdate        *bool  `json:"can_autoupdate,omitempty"`
 	NewVersion      string `json:"new_version,omitempty"`
 	Announcement    string `json:"announcement,omitempty"`
 	AnnouncementURL string `json:"announcement_url,omitempty"`
-	SelfUpdateMinVersion string `json:"-"`
+	// TODO(a.garipov): See if the frontend actually still cares about
+	// nullability.
+	CanAutoUpdate aghalg.NullBool `json:"can_autoupdate,omitempty"`
 }
 
 // MaxResponseSize is responses on server's requests maximum length in bytes.
@@ -67,15 +68,13 @@ func (u *Updater) VersionInfo(forceRecheck bool) (vi VersionInfo, err error) {
 }
 
 func (u *Updater) parseVersionResponse(data []byte) (VersionInfo, error) {
-	var canAutoUpdate bool
 	info := VersionInfo{
-		CanAutoUpdate: &canAutoUpdate,
+		CanAutoUpdate: aghalg.NBFalse,
 	}
 	versionJSON := map[string]string{
 		"version":          "",
 		"announcement":     "",
 		"announcement_url": "",
-		"selfupdate_min_version": "",
 	}
 	err := json.Unmarshal(data, &versionJSON)
 	if err != nil {
@@ -91,14 +90,9 @@ func (u *Updater) parseVersionResponse(data []byte) (VersionInfo, error) {
 	info.NewVersion = versionJSON["version"]
 	info.Announcement = versionJSON["announcement"]
 	info.AnnouncementURL = versionJSON["announcement_url"]
-	info.SelfUpdateMinVersion = versionJSON["selfupdate_min_version"]
 
 	packageURL, ok := u.downloadURL(versionJSON)
-	if ok &&
+	info.CanAutoUpdate = aghalg.BoolToNullBool(ok && info.NewVersion != u.version)
-		info.NewVersion != u.version &&
-		strings.TrimPrefix(u.version, "v") >= strings.TrimPrefix(info.SelfUpdateMinVersion, "v") {
-		canAutoUpdate = true
-	}
 
 	u.newVersion = info.NewVersion
 	u.packageURL = packageURL

internal/updater/updater.go

@@ -104,11 +104,14 @@ func NewUpdater(conf *Config) *Updater {
 }
 
 // Update performs the auto-update.
-func (u *Updater) Update() error {
+func (u *Updater) Update() (err error) {
 	u.mu.Lock()
 	defer u.mu.Unlock()
 
-	err := u.prepare()
+	log.Info("updater: updating")
+	defer func() { log.Info("updater: finished; errors: %v", err) }()
+
+	err = u.prepare()
 	if err != nil {
 		return err
 	}
@@ -178,7 +181,12 @@ func (u *Updater) prepare() (err error) {
 	u.backupExeName = filepath.Join(u.backupDir, exeName)
 	u.updateExeName = filepath.Join(u.updateDir, exeName)
 
-	log.Info("Updating from %s to %s.  URL:%s", version.Version(), u.newVersion, u.packageURL)
+	log.Debug(
+		"updater: updating from %s to %s using url: %s",
+		version.Version(),
+		u.newVersion,
+		u.packageURL,
+	)
 
 	// TODO(a.garipov): Use os.Args[0] instead?
 	u.currentExeName = filepath.Join(u.workDir, exeName)
@@ -194,7 +202,7 @@ func (u *Updater) unpack() error {
 	var err error
 	_, pkgNameOnly := filepath.Split(u.packageURL)
 
-	log.Debug("updater: unpacking the package")
+	log.Debug("updater: unpacking package")
 	if strings.HasSuffix(pkgNameOnly, ".zip") {
 		u.unpackedFiles, err = zipFileUnpack(u.packageName, u.updateDir)
 		if err != nil {
@@ -229,7 +237,7 @@ func (u *Updater) check() error {
 }
 
 func (u *Updater) backup() error {
-	log.Debug("updater: backing up the current configuration")
+	log.Debug("updater: backing up current configuration")
 	_ = os.Mkdir(u.backupDir, 0o755)
 	err := copyFile(u.confName, filepath.Join(u.backupDir, "AdGuardHome.yaml"))
 	if err != nil {
@@ -252,7 +260,7 @@ func (u *Updater) replace() error {
 		return fmt.Errorf("copySupportingFiles(%s, %s) failed: %s", u.updateDir, u.workDir, err)
 	}
 
-	log.Debug("updater: renaming: %s -> %s", u.currentExeName, u.backupExeName)
+	log.Debug("updater: renaming: %s to %s", u.currentExeName, u.backupExeName)
 	err = os.Rename(u.currentExeName, u.backupExeName)
 	if err != nil {
 		return err
@@ -268,7 +276,7 @@ func (u *Updater) replace() error {
 		return err
 	}
 
-	log.Debug("updater: renamed: %s -> %s", u.updateExeName, u.currentExeName)
+	log.Debug("updater: renamed: %s to %s", u.updateExeName, u.currentExeName)
 
 	return nil
 }
@@ -297,7 +305,7 @@ func (u *Updater) downloadPackageFile(url, filename string) (err error) {
 		return fmt.Errorf("http request failed: %w", err)
 	}
 
-	log.Debug("updater: reading HTTP body")
+	log.Debug("updater: reading http body")
 	// This use of ReadAll is now safe, because we limited body's Reader.
 	body, err := io.ReadAll(r)
 	if err != nil {
@@ -343,7 +351,7 @@ func tarGzFileUnpackOne(outDir string, tr *tar.Reader, hdr *tar.Header) (name st
 	}
 
 	if hdr.Typeflag != tar.TypeReg {
-		log.Debug("updater: %s: unknown file type %d, skipping", name, hdr.Typeflag)
+		log.Info("updater: %s: unknown file type %d, skipping", name, hdr.Typeflag)
 
 		return "", nil
 	}
@@ -364,7 +372,7 @@ func tarGzFileUnpackOne(outDir string, tr *tar.Reader, hdr *tar.Header) (name st
 		return "", fmt.Errorf("io.Copy(): %w", err)
 	}
 
-	log.Tracef("updater: created file %s", outputName)
+	log.Debug("updater: created file %q", outputName)
 
 	return name, nil
 }
@@ -440,7 +448,7 @@ func zipFileUnpackOne(outDir string, zf *zip.File) (name string, err error) {
 			return "", fmt.Errorf("os.Mkdir(%q): %w", outputName, err)
 		}
 
-		log.Tracef("created directory %q", outputName)
+		log.Debug("updater: created directory %q", outputName)
 
 		return "", nil
 	}
@@ -457,7 +465,7 @@ func zipFileUnpackOne(outDir string, zf *zip.File) (name string, err error) {
 		return "", fmt.Errorf("io.Copy(): %w", err)
 	}
 
-	log.Tracef("created file %s", outputName)
+	log.Debug("updater: created file %q", outputName)
 
 	return name, nil
 }
@@ -516,7 +524,7 @@ func copySupportingFiles(files []string, srcdir, dstdir string) error {
 			return err
 		}
 
-		log.Debug("updater: copied: %q -> %q", src, dst)
+		log.Debug("updater: copied: %q to %q", src, dst)
 	}
 
 	return nil

internal/updater/updater_test.go

@@ -10,6 +10,7 @@ import (
 	"strconv"
 	"testing"
 
+	"github.com/AdguardTeam/AdGuardHome/internal/aghalg"
 	"github.com/AdguardTeam/AdGuardHome/internal/aghtest"
 	"github.com/AdguardTeam/AdGuardHome/internal/version"
 	"github.com/AdguardTeam/golibs/testutil"
@@ -92,10 +93,7 @@ func TestUpdateGetVersion(t *testing.T) {
 	assert.Equal(t, "v0.103.0-beta.2", info.NewVersion)
 	assert.Equal(t, "AdGuard Home v0.103.0-beta.2 is now available!", info.Announcement)
 	assert.Equal(t, "https://github.com/AdguardTeam/AdGuardHome/internal/releases", info.AnnouncementURL)
-	assert.Equal(t, "v0.0", info.SelfUpdateMinVersion)
+	assert.Equal(t, aghalg.NBTrue, info.CanAutoUpdate)
-	if assert.NotNil(t, info.CanAutoUpdate) {
-		assert.True(t, *info.CanAutoUpdate)
-	}
 
 	// check cached
 	_, err = u.VersionInfo(false)
@@ -290,10 +288,7 @@ func TestUpdater_VersionInto_ARM(t *testing.T) {
 	assert.Equal(t, "v0.103.0-beta.2", info.NewVersion)
 	assert.Equal(t, "AdGuard Home v0.103.0-beta.2 is now available!", info.Announcement)
 	assert.Equal(t, "https://github.com/AdguardTeam/AdGuardHome/internal/releases", info.AnnouncementURL)
-	assert.Equal(t, "v0.0", info.SelfUpdateMinVersion)
+	assert.Equal(t, aghalg.NBTrue, info.CanAutoUpdate)
-	if assert.NotNil(t, info.CanAutoUpdate) {
-		assert.True(t, *info.CanAutoUpdate)
-	}
 }
 
 func TestUpdater_VersionInto_MIPS(t *testing.T) {
@@ -330,8 +325,5 @@ func TestUpdater_VersionInto_MIPS(t *testing.T) {
 	assert.Equal(t, "v0.103.0-beta.2", info.NewVersion)
 	assert.Equal(t, "AdGuard Home v0.103.0-beta.2 is now available!", info.Announcement)
 	assert.Equal(t, "https://github.com/AdguardTeam/AdGuardHome/internal/releases", info.AnnouncementURL)
-	assert.Equal(t, "v0.0", info.SelfUpdateMinVersion)
+	assert.Equal(t, aghalg.NBTrue, info.CanAutoUpdate)
-	if assert.NotNil(t, info.CanAutoUpdate) {
-		assert.True(t, *info.CanAutoUpdate)
-	}
 }

scripts/make/build-release.sh

@@ -363,6 +363,7 @@ else
 fi
 readonly announcement_url
 
+# TODO(a.garipov): Remove "selfupdate_min_version" in future versions.
 rm -f "$version_json"
 echo "{
   \"version\": \"${version}\",