From e738508d7a5e09e257724c557069000506e055ad Mon Sep 17 00:00:00 2001 From: Ainar Garipov Date: Thu, 9 Jun 2022 17:47:05 +0300 Subject: [PATCH] Pull request: all: imp updater Merge in DNS/adguard-home from imp-updater to master Squashed commit of the following: commit 6ed487359e56a35b36f13dcbf2efbf2a7a2d8734 Author: Ainar Garipov Date: Thu Jun 9 16:29:35 2022 +0300 all: imp logs, err handling commit e930044cb619a43e5a44c230dadbe2228e9a93f5 Author: Ainar Garipov Date: Thu Jun 9 15:53:35 2022 +0300 all: imp updater --- client/src/__locales/en.json | 2 +- internal/aghalg/nullbool.go | 59 +++++++++++++++++++++ internal/{dhcpd => aghalg}/nullbool_test.go | 23 ++++---- internal/dhcpd/http.go | 19 +++---- internal/dhcpd/nullbool.go | 58 -------------------- internal/home/controlupdate.go | 7 +-- internal/updater/check.go | 30 +++++------ internal/updater/updater.go | 34 +++++++----- internal/updater/updater_test.go | 16 ++---- scripts/make/build-release.sh | 1 + 10 files changed, 124 insertions(+), 125 deletions(-) create mode 100644 internal/aghalg/nullbool.go rename internal/{dhcpd => aghalg}/nullbool_test.go (72%) delete mode 100644 internal/dhcpd/nullbool.go diff --git a/client/src/__locales/en.json b/client/src/__locales/en.json index a5c21109..dd885677 100644 --- a/client/src/__locales/en.json +++ b/client/src/__locales/en.json @@ -445,7 +445,7 @@ "access_blocked_title": "Disallowed domains", "access_blocked_desc": "Not to be confused with filters. AdGuard Home drops DNS queries matching these domains, and these queries don't even appear in the query log. You can specify exact domain names, wildcards, or URL filter rules, e.g. \"example.org\", \"*.example.org\", or \"||example.org^\" correspondingly.", "access_settings_saved": "Access settings successfully saved", - "updates_checked": "Updates successfully checked", + "updates_checked": "A new version of AdGuard Home is available", "updates_version_equal": "AdGuard Home is up-to-date", "check_updates_now": "Check for updates now", "dns_privacy": "DNS Privacy", diff --git a/internal/aghalg/nullbool.go b/internal/aghalg/nullbool.go new file mode 100644 index 00000000..3c5633e3 --- /dev/null +++ b/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 +} diff --git a/internal/dhcpd/nullbool_test.go b/internal/aghalg/nullbool_test.go similarity index 72% rename from internal/dhcpd/nullbool_test.go rename to internal/aghalg/nullbool_test.go index 549df608..0fe7f203 100644 --- a/internal/dhcpd/nullbool_test.go +++ b/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) diff --git a/internal/dhcpd/http.go b/internal/dhcpd/http.go index e340addb..62a08f66 100644 --- a/internal/dhcpd/http.go +++ b/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 { - s.conf.Enabled = conf.Enabled == nbTrue + if conf.Enabled != aghalg.NBNull { + 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) { diff --git a/internal/dhcpd/nullbool.go b/internal/dhcpd/nullbool.go deleted file mode 100644 index b07f6768..00000000 --- a/internal/dhcpd/nullbool.go +++ /dev/null @@ -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 -} diff --git a/internal/home/controlupdate.go b/internal/home/controlupdate.go index ae469598..08ddd455 100644 --- a/internal/home/controlupdate.go +++ b/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 { - return + if vr.CanAutoUpdate != aghalg.NBTrue { + return nil } tlsConf := &tlsConfigSettings{} @@ -162,7 +163,7 @@ func (vr *versionResponse) setAllowedToAutoUpdate() (err error) { } } - vr.CanAutoUpdate = &canUpdate + vr.CanAutoUpdate = aghalg.BoolToNullBool(canUpdate) return nil } diff --git a/internal/updater/check.go b/internal/updater/check.go index ec7176b2..2bac6153 100644 --- a/internal/updater/check.go +++ b/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:"-"` + NewVersion string `json:"new_version,omitempty"` + Announcement string `json:"announcement,omitempty"` + AnnouncementURL string `json:"announcement_url,omitempty"` + // 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": "", + "version": "", + "announcement": "", + "announcement_url": "", } 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.NewVersion != u.version && - strings.TrimPrefix(u.version, "v") >= strings.TrimPrefix(info.SelfUpdateMinVersion, "v") { - canAutoUpdate = true - } + info.CanAutoUpdate = aghalg.BoolToNullBool(ok && info.NewVersion != u.version) u.newVersion = info.NewVersion u.packageURL = packageURL diff --git a/internal/updater/updater.go b/internal/updater/updater.go index 0cc49f9e..d975d977 100644 --- a/internal/updater/updater.go +++ b/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 diff --git a/internal/updater/updater_test.go b/internal/updater/updater_test.go index 3a29d277..771fb6d4 100644 --- a/internal/updater/updater_test.go +++ b/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) - if assert.NotNil(t, info.CanAutoUpdate) { - assert.True(t, *info.CanAutoUpdate) - } + assert.Equal(t, aghalg.NBTrue, 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) - if assert.NotNil(t, info.CanAutoUpdate) { - assert.True(t, *info.CanAutoUpdate) - } + assert.Equal(t, aghalg.NBTrue, 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) - if assert.NotNil(t, info.CanAutoUpdate) { - assert.True(t, *info.CanAutoUpdate) - } + assert.Equal(t, aghalg.NBTrue, info.CanAutoUpdate) } diff --git a/scripts/make/build-release.sh b/scripts/make/build-release.sh index 36fc98f7..2cf95086 100644 --- a/scripts/make/build-release.sh +++ b/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}\",