feature(veinmind-runner): add harbor webhook#116
Conversation
| log.Error(err) | ||
| return | ||
| } | ||
| err = pullImagesFromHarbor(option.authInfo, imageNames) |
There was a problem hiding this comment.
我建议不要处理每一个action都拉取镜像
除此之后还要考虑镜像缓存定时清理的问题
|
@MonkeyCode-AI review it |
MonkeyCode-AI
left a comment
MonkeyCode-AI
left a comment
There was a problem hiding this comment.
PR添加了Harbor webhook功能，使系统能够响应Harbor的事件并自动扫描相关镜像。整体实现较为完整，但也存在一些可以改进的地方：
- 安全性方面：密码比较应使用加密安全的方法。
- 可靠性方面：应添加重试机制以应对网络波动。
- 可维护性方面：错误日志应更详细，邮件内容应更友好。
建议根据评审意见进行修改。
| return | ||
| } | ||
| go func() { | ||
| handleHarborWebhookReportEvents(eventListCh, hpolicy, |
There was a problem hiding this comment.
在 checkPassword 函数中，密码比较使用了简单的字符串比较，容易受到时序攻击。建议使用加密安全的比较方法，例如 crypto/subtle 包中的 ConstantTimeCompare 函数。
// 原代码
if c.Request.Header.Get("Authorization") == password {
return nil
}
// 建议修改为
import "crypto/subtle"
if subtle.ConstantTimeCompare([]byte(c.Request.Header.Get("Authorization")), []byte(password)) == 1 {
return nil
}|
|
||
| // get secrect from Authorization field and check | ||
| func checkPassword(c *gin.Context, password string) error { | ||
| if password == "" { |
There was a problem hiding this comment.
在 parseHarborwebhookPostdata 函数中，读取请求体后没有关闭它。虽然在这个上下文中可能不会造成问题，但作为良好实践，应该确保在使用完 ioutil.ReadAll 后关闭请求体。
// 建议修改
func parseHarborwebhookPostdata(c *gin.Context) (action.PullandPushData, error) {
postData := &action.PullandPushData{}
data, err := ioutil.ReadAll(c.Request.Body)
if err != nil {
return action.PullandPushData{}, err
}
defer c.Request.Body.Close() // 添加这行
if err := json.Unmarshal(data, &postData); err != nil {
return action.PullandPushData{}, err
}
return *postData, nil
}| Auths: []auth.Auth{authentity}} | ||
| dockerclient, err := runtime.NewDockerClient(runtime.WithAuth(authConfig)) | ||
| if err != nil { | ||
| return err |
There was a problem hiding this comment.
在 pullImagesFromHarbor 函数中，当拉取镜像失败时，仅记录错误并继续处理下一个镜像。建议考虑添加重试机制或更详细的错误处理，以便更好地应对网络波动或临时性错误。
// 原代码
_, err := dockerclient.Pull(img)
if err != nil {
log.Error(err)
continue
}
// 建议添加重试逻辑
for i := 0; i < 3; i++ { // 重试3次
_, err := dockerclient.Pull(img)
if err == nil {
break
}
log.Warnf("Failed to pull image %s, retrying... (%d/3)", img, i+1)
time.Sleep(time.Second * 2) // 等待2秒后重试
}
if err != nil {
log.Error(err)
continue
}| eventListCh := make(chan []reporter.ReportEvent, 1) | ||
| var result []reporter.ReportEvent | ||
| for _, img := range imageNames { | ||
| report, err := scan.ScanLocalImage(ctx, img, |
There was a problem hiding this comment.
在 HandleWebhookImagePush 函数中，当扫描镜像遇到错误时，只是记录错误并继续处理下一个镜像。建议添加更详细的错误信息，以便更好地诊断问题。
// 原代码
report, err := scan.ScanLocalImage(ctx, img,
policy.EnabledPlugins, policy.PluginParams)
if err != nil {
log.Error(err)
continue
}
// 建议修改为
report, err := scan.ScanLocalImage(ctx, img,
policy.EnabledPlugins, policy.PluginParams)
if err != nil {
log.Errorf("Failed to scan image %s: %v", img, err)
continue
}| m.SetHeader("To", mailconf.Subscriber...) | ||
| m.SetHeader("Subject", "Harbor webhook Report") | ||
| m.SetBody("text/plain", fmt.Sprintf("%#v", events)) | ||
| if err := d.DialAndSend(m); err != nil { |
There was a problem hiding this comment.
在 sendReport2Mail 函数中，邮件内容直接使用了 fmt.Sprintf("%#v", events)，这可能不是用户友好的格式。建议对报告事件进行格式化处理，使其更易于阅读。
// 原代码
m.SetBody("text/plain", fmt.Sprintf("%#v", events))
// 建议修改为
var formattedEvents strings.Builder
for _, event := range events {
formattedEvents.WriteString(fmt.Sprintf("Event: %+v\n", event))
}
m.SetBody("text/plain", formattedEvents.String())
|
⏳ MonkeyCode-AI 正在分析，请稍等片刻... |
|
@MonkeyCode-AI 来 review |
❌ 代码审查需要修改: 本次 PR 功能扩展方向明确（新增 Harbor webhook、策略告警与邮件通知），但整体质量未达可合入标准：存在至少一处关键的类型/指针使用风险，且配置必填与合法性校验不足，可能导致运行期直接失败或行为不确定。建议优先补齐配置校验与错误/HTTP 响应处理、完善服务生命周期与资源控制（请求体大小、并发/超时、优雅停机），并对依赖升级与新增邮件相关依赖进行必要性与安全评估并补充回归测试。另需澄清 PR 元信息与实际 diff 不一致的问题后再继续审查与合并。 📊 统计
🔴 严重 (1)可能错误地对 server 取地址导致类型不匹配
|
4 participants
No description provided.