diff --git a/cmd/viam-agent/main.go b/cmd/viam-agent/main.go index ffa6982..2a6e4a4 100644 --- a/cmd/viam-agent/main.go +++ b/cmd/viam-agent/main.go @@ -81,9 +81,8 @@ func commonMain() { globalLogger.SetLevel(logging.DEBUG) } - if opts.UpdateFirst { - utils.CLIWaitForUpdateCheck = true - } + utils.CLIWaitForUpdateCheck = opts.UpdateFirst + utils.DevMode = opts.DevMode // need to be root to go any further than this curUser, err := user.Current() diff --git a/manager.go b/manager.go index 23354b4..3eab129 100644 --- a/manager.go +++ b/manager.go @@ -169,6 +169,11 @@ func (m *Manager) SelfUpdate(ctx context.Context) (bool, error) { return false, ctx.Err() } + // Do not manage the agent while in dev mode. + if utils.DevMode { + return false, nil + } + _, err := m.GetConfig(ctx) if err != nil { return false, err @@ -195,17 +200,20 @@ func (m *Manager) SubsystemUpdates(ctx context.Context) { m.cfgMu.Lock() defer m.cfgMu.Unlock() - // Agent - needRestart, err := m.cache.UpdateBinary(ctx, SubsystemName) - if err != nil { - m.logger.Warn(err) - } - if needRestart { - _, err := InstallNewVersion(ctx, m.logger) + // Do not manage the agent while in dev mode. + if !utils.DevMode { + // Agent + needRestart, err := m.cache.UpdateBinary(ctx, SubsystemName) if err != nil { - m.logger.Warnw("running install of new agent version", "error", err) + m.logger.Warn(err) + } + if needRestart { + _, err := InstallNewVersion(ctx, m.logger) + if err != nil { + m.logger.Warnw("running install of new agent version", "error", err) + } + m.viamAgentNeedsRestart = true } - m.viamAgentNeedsRestart = true } // Viam Server @@ -254,7 +262,7 @@ func (m *Manager) SubsystemUpdates(ctx context.Context) { m.logger.Warn(err) } } else { - needRestart = m.sysConfig.Update(ctx, m.cfg) + needRestart := m.sysConfig.Update(ctx, m.cfg) if needRestart { if err := m.sysConfig.Stop(ctx); err != nil { m.logger.Warn(err) @@ -271,7 +279,7 @@ func (m *Manager) SubsystemUpdates(ctx context.Context) { m.logger.Warn(err) } } else { - needRestart = m.networking.Update(ctx, m.cfg) + needRestart := m.networking.Update(ctx, m.cfg) if needRestart { if err := m.networking.Stop(ctx); err != nil { m.logger.Warn(err) diff --git a/utils/config.go b/utils/config.go index 859a33b..1c4c5c9 100644 --- a/utils/config.go +++ b/utils/config.go @@ -65,6 +65,7 @@ var ( DefaultsFilePath = "/etc/viam-defaults.json" CLIDebug = false CLIWaitForUpdateCheck = false + DevMode = false ) func init() { diff --git a/utils/utils.go b/utils/utils.go index 60583b0..15b54d5 100644 --- a/utils/utils.go +++ b/utils/utils.go @@ -160,9 +160,11 @@ func DownloadFile(ctx context.Context, rawURL string, logger logging.Logger) (st logger.Infof("Starting download of %s", rawURL) parsedPath := parsedURL.Path - // don't want to accidentally overwrite anything in the cache directory by accident - // old agent versions used a different cache format, so it's possible we could be re-downloading ourself + // APP-8900: Somehow, versioned binaries can be downloaded over themselves. I think when the + // version cache file is out of sync with what's actually on disk. I don't know why we need a + // version cache file. var outPath string + outPath = filepath.Join(ViamDirs.Cache, path.Base(parsedPath)) for n := range 100 { var suffix string if n > 0 { @@ -350,15 +352,17 @@ func DecompressFile(inPath string) (outPath string, errRet error) { return outPath, errRet } -func GetFileSum(filepath string) (outSum []byte, errRet error) { - //nolint:gosec - in, err := os.Open(filepath) +func GetFileSum(path string) (outSum []byte, errRet error) { + resolvedPath, err := filepath.EvalSymlinks(path) if err != nil { return nil, err } - defer func() { - errRet = errors.Join(errRet, in.Close()) - }() + + in, err := os.Open(resolvedPath) + if err != nil { + return nil, err + } + defer goutils.UncheckedErrorFunc(in.Close) h := sha256.New() _, errRet = io.Copy(h, in) diff --git a/version_control.go b/version_control.go index 6d7109f..4ab04ba 100644 --- a/version_control.go +++ b/version_control.go @@ -9,6 +9,7 @@ import ( "errors" "fmt" "io/fs" + "net/url" "os" "path" "path/filepath" @@ -202,10 +203,8 @@ func (c *VersionCache) UpdateBinary(ctx context.Context, binary string) (bool, e return false, errw.Errorf("unknown binary name for update request: %s", binary) } - var needRestart, goodBytes bool - if data.brokenTarget { - return needRestart, nil + return false, nil } verData, ok := data.Versions[data.TargetVersion] @@ -216,17 +215,37 @@ func (c *VersionCache) UpdateBinary(ctx context.Context, binary string) (bool, e if targetVersion == "" { targetVersion = "[empty string]" } - return needRestart, errw.Errorf("version data %s not found for binary %s", targetVersion, binary) + return false, errw.Errorf("version data %s not found for binary %s", targetVersion, binary) } - isCustomURL := strings.HasPrefix(verData.Version, "customURL+") - shasum, err := utils.GetFileSum(verData.UnpackedPath) - if err == nil { - goodBytes = bytes.Equal(shasum, verData.UnpackedSHA) + var shasum []byte + var shaErr error + urlObj, urlErr := url.Parse(verData.URL) + //nolint:gocritic + if urlErr == nil && urlObj.Scheme == "file" { + shasum, shaErr = utils.GetFileSum(urlObj.Path) + } else if urlErr == nil && urlObj.Scheme == "https" { + // Google storage's `x-goog-hash` header only report crc32 hashes. We could compute that + // from the file on disk in. But do not do so currently. + // + // Alternatively we could compare the `last-modified` header with the on disk + // last-modified. We do not preserve the modification time when downloading files. So we + // can't rely on equality. But it should also be correct to redownload anytime the in-cloud + // last modified is newer than the on-disk last modified. So long as the system clock can be + // trusted. Which may not be the case for freshly restarted raspberry pis. + shaErr = errors.New("we do not know how to cache invalidate custom download urls") + } else { + shasum, shaErr = utils.GetFileSum(verData.UnpackedPath) + } + + var needRestart, bytesMatch bool + if shaErr == nil { + bytesMatch = bytes.Equal(shasum, verData.UnpackedSHA) } else if verData.UnpackedPath != "" { // custom file:// URLs with have an empty unpacked path; no need to warn - c.logger.Warnw("Could not calculate shasum", "path", verData.UnpackedPath, "error", err) + c.logger.Warnw("Could not calculate shasum", "path", verData.UnpackedPath, "shaErr", shaErr) } + isCustomURL := strings.HasPrefix(verData.Version, "customURL+") if data.TargetVersion == data.CurrentVersion { // if a known version, make sure the symlink is correct same, err := utils.CheckIfSame(verData.DlPath, verData.SymlinkPath) @@ -246,71 +265,78 @@ func (c *VersionCache) UpdateBinary(ctx context.Context, binary string) (bool, e } } - // TODO(APP-10012): don't leave bad checksum binary on disk + symlinked - if goodBytes { - return false, nil - } - // if we're here, we have a mismatched checksum, as likely the URL changed, so wipe it and recompute later if isCustomURL { verData.UnpackedSHA = []byte{} } } + // TODO(APP-10012): don't leave bad checksum binary on disk + symlinked + if bytesMatch { + return needRestart, nil + } + // this is a new version c.logger.Infof("new version (%s) found for %s", verData.Version, binary) + c.logger.Warnw("mismatched checksum, redownloading", + "expected", hex.EncodeToString(verData.UnpackedSHA), "actual", hex.EncodeToString(shasum), + "url", verData.URL, "shaErr", shaErr) - if !goodBytes { - c.logger.Warnw("mismatched checksum, redownloading", - "expected", hex.EncodeToString(verData.UnpackedSHA), "actual", hex.EncodeToString(shasum), - "url", verData.URL) - // download and record the sha of the download itself - verData.DlPath, err = utils.DownloadFile(ctx, verData.URL, c.logger) - if err != nil { - if isCustomURL { - data.brokenTarget = true - } - return needRestart, errw.Wrapf(err, "downloading %s", binary) - } - - actualSha, err := utils.GetFileSum(verData.DlPath) - if err != nil { - return needRestart, errw.Wrap(err, "getting file shasum") + // download and record the sha of the download itself + var err error + verData.DlPath, err = utils.DownloadFile(ctx, verData.URL, c.logger) + if err != nil { + if isCustomURL && urlObj.Scheme != "file" { + // If the binary was downloaded from the internet cache that link as bad forever. We + // don't want to redownload every five seconds to learn the binary still does not + // work. Even if the binary backing the URL eventually changes, unfortunately. + // + // However, if the URL represents a file on disk, we should keep retrying. Consider + // the case where the file was overwritten in place. Being this update logic is run + // every five seconds, it's common to catch a binary while its being + // compiled/written to. + data.brokenTarget = true } + return needRestart, errw.Wrapf(err, "downloading %s", binary) + } - // TODO handle compressed formats, for now, the raw download is the same file - verData.UnpackedPath = verData.DlPath - verData.DlSHA = actualSha + actualSha, err := utils.GetFileSum(verData.DlPath) + if err != nil { + return needRestart, errw.Wrap(err, "getting file shasum") + } - if len(verData.UnpackedSHA) <= 1 && isCustomURL { - // new custom download, so need to check the file is an executable binary and use locally generated sha - mtype, err := mimetype.DetectFile(verData.UnpackedPath) - if err != nil { - return needRestart, errw.Wrapf(err, "determining file type of download") - } + // TODO handle compressed formats, for now, the raw download is the same file + verData.UnpackedPath = verData.DlPath + verData.DlSHA = actualSha - expectedMimes := []string{"application/x-elf", "application/x-executable"} - if runtime.GOOS == "windows" { - expectedMimes = []string{"application/vnd.microsoft.portable-executable"} - } + if len(verData.UnpackedSHA) <= 1 && isCustomURL { + // new custom download, so need to check the file is an executable binary and use locally generated sha + mtype, err := mimetype.DetectFile(verData.UnpackedPath) + if err != nil { + return needRestart, errw.Wrapf(err, "determining file type of download") + } - if !slices.ContainsFunc(expectedMimes, mtype.Is) { - data.brokenTarget = true - return needRestart, errw.Errorf("downloaded file is %s, not %s, skipping", mtype, strings.Join(expectedMimes, ", ")) - } - verData.UnpackedSHA = actualSha + expectedMimes := []string{"application/x-elf", "application/x-executable"} + if runtime.GOOS == "windows" { + expectedMimes = []string{"application/vnd.microsoft.portable-executable"} } - if len(verData.UnpackedSHA) > 1 && !bytes.Equal(verData.UnpackedSHA, actualSha) { - // TODO(APP-10012): don't leave bad checksum file on disk - //nolint:goerr113 - return needRestart, fmt.Errorf( - "sha256 (%s) of downloaded file (%s) does not match provided (%s)", - base64.StdEncoding.EncodeToString(actualSha), - verData.UnpackedPath, - base64.StdEncoding.EncodeToString(verData.UnpackedSHA), - ) + if !slices.ContainsFunc(expectedMimes, mtype.Is) { + data.brokenTarget = true + return needRestart, errw.Errorf("downloaded file is %s, not %s, skipping", mtype, strings.Join(expectedMimes, ", ")) } + verData.UnpackedSHA = actualSha + } + + if len(verData.UnpackedSHA) > 1 && !bytes.Equal(verData.UnpackedSHA, actualSha) { + // TODO(APP-10012): don't leave bad checksum file on disk + //nolint:goerr113 + return needRestart, fmt.Errorf( + "sha256 (%s) of downloaded file (%s) does not match provided (%s)", + base64.StdEncoding.EncodeToString(actualSha), + verData.UnpackedPath, + base64.StdEncoding.EncodeToString(verData.UnpackedSHA), + ) } // chmod with execute permissions if the file is executable @@ -320,7 +346,7 @@ func (c *VersionCache) UpdateBinary(ctx context.Context, binary string) (bool, e } // symlink the extracted file to bin - if err = utils.ForceSymlink(verData.UnpackedPath, verData.SymlinkPath); err != nil { + if err := utils.ForceSymlink(verData.UnpackedPath, verData.SymlinkPath); err != nil { return needRestart, errw.Wrap(err, "creating symlink") }