Skip to content

feat: Support intranet update environment checks and notification strategies#292

Merged
electricface merged 1 commit into
linuxdeepin:intranet_updatefrom
electricface:swt/dev-private
Jan 21, 2026
Merged

feat: Support intranet update environment checks and notification strategies#292
electricface merged 1 commit into
linuxdeepin:intranet_updatefrom
electricface:swt/dev-private

Conversation

@electricface
Copy link
Copy Markdown
Member

  • 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

@electricface electricface force-pushed the swt/dev-private branch 2 times, most recently from faf0d75 to 4485329 Compare January 20, 2026 10:14
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
@deepin-ci-robot
Copy link
Copy Markdown

deepin pr auto review

这份代码 diff 主要实现了对内网更新模式的支持,包括配置读取、更新流程控制、通知处理和平台上报等功能。我将从语法逻辑、代码质量、代码性能和代码安全四个方面进行审查。

1. 语法逻辑审查

1.1 类型断言安全性问题

src/internal/config/config.go 中:

v, err = c.dsLastoreManager.Value(0, dSettingsKeyIntranetUpdate)
if err != nil {
    logger.Warning(err)
} else {
    c.IntranetUpdate = v.Value().(bool)  // 未检查类型断言是否成功
}

问题:直接使用类型断言 v.Value().(bool) 没有检查是否成功,如果类型不匹配会引发 panic。
建议

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 全局变量初始化时机

src/lastore-daemon/main.go 中:

config := NewConfig(path.Join(system.VarLibDir, "config.json"))
logger.Info("intranet update:", config.IntranetUpdate)
system.IsPrivateLastore = config.IntranetUpdate

问题system.IsPrivateLastore 是一个全局变量,在 main 函数中初始化。如果其他包在 init 函数中使用了这个变量,可能会出现初始化顺序问题。
建议:考虑使用单例模式或依赖注入来管理这个全局状态,或者确保所有使用该变量的地方都在 main 函数之后执行。

1.3 条件判断冗余

src/internal/system/update_type.go 中:

if IsPrivateLastore {
    sourcePathList = append(sourcePathList, SystemUpdateSource)
} else {
    for _, t := range AllCheckUpdateType() {
        category := updateType & t
        if category != 0 {
            sourcePath := GetCategorySourceMap()[t]
            sourcePathList = append(sourcePathList, sourcePath)
        }
    }
}

问题:在 IsPrivateLastore 为 true 时,直接使用 SystemUpdateSource,但后续代码仍然会检查 len(sourcePathList) 是否为 1,可能导致逻辑不一致。
建议:确保在 IsPrivateLastore 为 true 时,sourcePathList 的处理逻辑与其他分支一致,或者明确说明两种模式下的差异。

2. 代码质量审查

2.1 魔法数字

src/internal/system/system.go 中:

const (
    NotifyExpireTimeoutDefault     = -1
    NotifyExpireTimeoutNoHide      = 0
    NotifyExpireTimeoutPrivate     = 10000
    NotifyExpireTimeoutPrivateLong = 600000
)

问题10000600000 是魔法数字,没有明确单位。
建议:添加注释说明单位(毫秒),或者使用 time.Duration 类型:

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

src/lastore-daemon/main.go 中:

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 随机延迟的实现

src/lastore-daemon/manager_update.go 中:

if system.IsPrivateLastore {
    // 独立客户端为了减少更新平台的瞬时负载,采取0-58秒内随机开始检查更新任务
    time.Sleep(time.Duration(rand.Intn(59)) * time.Second)
}

问题rand.Intn(59) 使用的是全局的随机数生成器,不是并发安全的。
建议:使用 rand.New(rand.NewSource(time.Now().UnixNano())) 创建独立的随机数生成器,或者使用 math/rand/v2 中的并发安全实现:

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)

问题:频繁的文件操作可能会影响性能,尤其是在高并发场景下。
建议:考虑使用缓存或批量处理文件操作,减少 I/O 次数。

4. 代码安全审查

4.1 命令注入风险

在多处代码中,直接使用 exec.Command 执行外部命令:

cmd := exec.Command("/bin/bash", preCheckFilePath)

问题:如果 preCheckFilePath 来自用户输入或不可信来源,可能会导致命令注入。
建议:确保 preCheckFilePath 是可信的,或者对其进行验证和清理。

4.2 敏感信息泄露

在多处代码中,错误信息可能包含敏感信息:

logger.Warningf("Error executing command: %v, output: %v", err, errMsg)

问题:直接记录错误输出可能会泄露敏感信息。
建议:对错误信息进行过滤或脱敏处理,避免记录敏感信息。

4.3 资源泄漏风险

在多处代码中,使用了 go 关键字启动 goroutine,但没有确保其资源释放:

go func() {
    // ... 一些操作
}()

问题:如果 goroutine 中有阻塞操作或资源分配,可能会导致资源泄漏。
建议:确保 goroutine 能够正确退出,或者使用 context 来控制其生命周期。

5. 其他建议

5.1 配置管理

当前代码中,IsPrivateLastore 是一个全局变量,建议:

  1. 将其封装在配置管理器中,通过方法访问
  2. 添加配置变更监听机制,支持动态更新

5.2 测试覆盖

建议为新增的内网更新逻辑添加单元测试和集成测试,特别是:

  1. 配置读取逻辑
  2. 更新流程控制
  3. 通知发送逻辑
  4. 平台上报逻辑

5.3 日志记录

建议统一日志记录格式,添加更多上下文信息,便于问题排查:

logger.Infof("Starting update source check, IsPrivateLastore: %v, UpdateMode: %v", 
    system.IsPrivateLastore, m.updatePlatform.Tp)

总结

这份代码实现了内网更新模式的基本功能,但在类型安全、错误处理、代码复用和并发安全等方面还有改进空间。建议:

  1. 加强类型断言的安全性检查
  2. 统一错误处理策略
  3. 减少代码重复,提高可维护性
  4. 优化并发安全,避免资源泄漏
  5. 加强安全防护,防止命令注入和信息泄露
  6. 添加完善的测试覆盖
  7. 改进日志记录,便于问题排查

这些改进将有助于提高代码的健壮性、可维护性和安全性。

@deepin-ci-robot
Copy link
Copy Markdown

[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.

Details Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@electricface electricface merged commit 5a17dd3 into linuxdeepin:intranet_update Jan 21, 2026
15 of 16 checks passed
@electricface electricface deleted the swt/dev-private branch January 21, 2026 01:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants