feat: Support intranet update environment checks and notification strategies#292
Conversation
faf0d75 to
4485329
Compare
strategies - Added logic to determine intranet updates (IsPrivateLastore) and changed the method for obtaining update sources; - Implemented pre-checks for the update environment, executing scripts to verify and report results; - Added pre-check logic for backups; - Optimized cron job logic to use different trigger commands during intranet updates; - Adjusted notification sending logic to use专属 icons and expiration times for intranet updates; - Enriched update status workflow event reporting, covering stages such as backup, download, and installation; - Added error and status reporting at multiple points to ensure the update platform accurately captures status; - Introduced automated periodic event handling to enhance auto-update capabilities. Task: https://pms.uniontech.com/task-view-385321.html
4485329 to
a2b7822
Compare
deepin pr auto review这份代码 diff 主要实现了对内网更新模式的支持,包括配置读取、更新流程控制、通知处理和平台上报等功能。我将从语法逻辑、代码质量、代码性能和代码安全四个方面进行审查。 1. 语法逻辑审查1.1 类型断言安全性问题在 v, err = c.dsLastoreManager.Value(0, dSettingsKeyIntranetUpdate)
if err != nil {
logger.Warning(err)
} else {
c.IntranetUpdate = v.Value().(bool) // 未检查类型断言是否成功
}问题:直接使用类型断言 v, err = c.dsLastoreManager.Value(0, dSettingsKeyIntranetUpdate)
if err != nil {
logger.Warning(err)
} else if boolVal, ok := v.Value().(bool); ok {
c.IntranetUpdate = boolVal
} else {
logger.Warningf("Invalid type for %s, expected bool", dSettingsKeyIntranetUpdate)
}1.2 全局变量初始化时机在 config := NewConfig(path.Join(system.VarLibDir, "config.json"))
logger.Info("intranet update:", config.IntranetUpdate)
system.IsPrivateLastore = config.IntranetUpdate问题: 1.3 条件判断冗余在 if IsPrivateLastore {
sourcePathList = append(sourcePathList, SystemUpdateSource)
} else {
for _, t := range AllCheckUpdateType() {
category := updateType & t
if category != 0 {
sourcePath := GetCategorySourceMap()[t]
sourcePathList = append(sourcePathList, sourcePath)
}
}
}问题:在 2. 代码质量审查2.1 魔法数字在 const (
NotifyExpireTimeoutDefault = -1
NotifyExpireTimeoutNoHide = 0
NotifyExpireTimeoutPrivate = 10000
NotifyExpireTimeoutPrivateLong = 600000
)问题: const (
NotifyExpireTimeoutDefault = -1
NotifyExpireTimeoutNoHide = 0
NotifyExpireTimeoutPrivate = 10 * time.Second
NotifyExpireTimeoutPrivateLong = 10 * time.Minute
)2.2 错误处理不一致在多处代码中,错误处理方式不一致: if err != nil {
logger.Warning(err)
}有些地方只记录日志,有些地方会返回错误,有些地方会忽略错误。 2.3 代码重复在多处代码中,通知发送逻辑重复: if system.IsPrivateLastore {
go m.sendNotify(updateNotifyShow, 0, "preferences-system", "", msg, nil, nil, system.NotifyExpireTimeoutPrivate)
} else {
hints := map[string]dbus.Variant{"x-deepin-action-view": dbus.MakeVariant("dde-control-center,-m,update")}
go m.sendNotify(updateNotifyShowOptional, 0, "preferences-system", "", msg, action, hints, system.NotifyExpireTimeoutDefault)
}建议:将通知发送逻辑封装成一个方法,减少重复代码: func (m *Manager) sendUpdateNotify(msg string, actions []string, hints map[string]dbus.Variant, expireTimeout int32) {
if system.IsPrivateLastore {
go m.sendNotify(updateNotifyShow, 0, "preferences-system", "", msg, nil, nil, system.NotifyExpireTimeoutPrivate)
} else {
go m.sendNotify(updateNotifyShowOptional, 0, "preferences-system", "", msg, actions, hints, expireTimeout)
}
}3. 代码性能审查3.1 不必要的 goroutine在 if system.IsPrivateLastore {
go func() {
out, err := exec.Command("/usr/bin/lastore-tools", "gatherinfo", "-type=post").CombinedOutput()
if err != nil {
logger.Warning(string(out))
}
}()
}问题:这个 goroutine 没有等待其完成,也没有处理其结果,可能会导致资源泄漏或错误被忽略。 3.2 随机延迟的实现在 if system.IsPrivateLastore {
// 独立客户端为了减少更新平台的瞬时负载,采取0-58秒内随机开始检查更新任务
time.Sleep(time.Duration(rand.Intn(59)) * time.Second)
}问题: if system.IsPrivateLastore {
r := rand.New(rand.NewSource(time.Now().UnixNano()))
time.Sleep(time.Duration(r.Intn(59)) * time.Second)
}3.3 文件操作性能在多处代码中,文件操作没有考虑性能: cacheFile := "/tmp/checkpolicy.cache"
_ = os.RemoveAll(cacheFile)问题:频繁的文件操作可能会影响性能,尤其是在高并发场景下。 4. 代码安全审查4.1 命令注入风险在多处代码中,直接使用 cmd := exec.Command("/bin/bash", preCheckFilePath)问题:如果 4.2 敏感信息泄露在多处代码中,错误信息可能包含敏感信息: logger.Warningf("Error executing command: %v, output: %v", err, errMsg)问题:直接记录错误输出可能会泄露敏感信息。 4.3 资源泄漏风险在多处代码中,使用了 go func() {
// ... 一些操作
}()问题:如果 goroutine 中有阻塞操作或资源分配,可能会导致资源泄漏。 5. 其他建议5.1 配置管理当前代码中,
5.2 测试覆盖建议为新增的内网更新逻辑添加单元测试和集成测试,特别是:
5.3 日志记录建议统一日志记录格式,添加更多上下文信息,便于问题排查: logger.Infof("Starting update source check, IsPrivateLastore: %v, UpdateMode: %v",
system.IsPrivateLastore, m.updatePlatform.Tp)总结这份代码实现了内网更新模式的基本功能,但在类型安全、错误处理、代码复用和并发安全等方面还有改进空间。建议:
这些改进将有助于提高代码的健壮性、可维护性和安全性。 |
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: electricface, qiuzhiqian The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
5a17dd3
into
linuxdeepin:intranet_update
and changed the method for obtaining update sources;
scripts to verify and report results;
intranet updates;
expiration times for intranet updates;
such as backup, download, and installation;
the update platform accurately captures status;
capabilities.
Task: https://pms.uniontech.com/task-view-385321.html