From 7b9cfa94f89525cc1aeecb42ed2b1139166fc813 Mon Sep 17 00:00:00 2001 From: Ainar Garipov Date: Thu, 9 Jun 2022 17:47:05 +0300 Subject: [PATCH] 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 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 --- 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 + 9 files changed, 123 insertions(+), 124 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/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}\",