perf: 空间配置恢复默认值修改为批量操作 --story=132989396#651
perf: 空间配置恢复默认值修改为批量操作 --story=132989396#651guohelu wants to merge 47 commits intoTencentBlueKing:developfrom
Conversation
* feat: 节点配置添加凭证选择 --story=125449007 # Reviewed, transaction id: 66075 * fix: 关闭label提示下划线 --story=125449007 # Reviewed, transaction id: 66078 * feat: 节点配置添加凭证选择 --story=125449007 # Reviewed, transaction id: 66104 * fix: 凭证选择校验优化 --story=125449007 # Reviewed, transaction id: 66163
* fix: 任务和流程表格增加hover和无数据时填充-- * fix: 流程和任务列表新建标签后回填 * fix: 标签选择器样式调整 * fix: 新建标签增加校验条件 * fix: 国际化内容补全
# Reviewed, transaction id: 75439
# Reviewed, transaction id: 75444
feat: 标签管理功能优化
新增 bkflow.statistics 模块,包含: - 6 张统计数据模型(任务/节点/模板/每日汇总/插件汇总) - 模板和任务采集器,通过 Django Signal 触发 Celery 异步写入 - 预聚合定时任务(每日汇总、插件汇总、过期清理) - 系统级和空间级两套 REST API(概览/趋势/排行/失败分析) - 独立数据库路由、回填管理命令、Django Admin 配置 Made-with: Cursor
新增 _CustomSpan、create_execution_span、_create_span_with_custom_id、 clean_plugin_span_outputs 等函数,修复 span 层级关系,支持预生成 plugin_span_id 用于 method span 正确嵌套,精简冗余属性,增加 propagate_attributes 幂等优化。 Made-with: Cursor
更新插件基类 BKFlowBaseService:span 名称增加 .plugin. 命名空间, _get_trace_context 返回 plugin_span_id,_end_plugin_span 后调用 clean_plugin_span_outputs 清理内部属性,execute/schedule 传递 plugin_span_id 给 method span。 Made-with: Cursor
TaskOperation.start() 使用 create_execution_span 替代原有 get_current_trace_context 方式,创建独立的执行级根 span 并 注入 root_pipeline_data,不再依赖外部 trace context 传递。 Made-with: Cursor
Made-with: Cursor
为 collectors、conf、signals、tasks、views、models、serializers、 db_router、backfill command 等 12 个核心文件补充类/方法 docstring 和关键逻辑的行内注释,提升代码可读性。 Made-with: Cursor
1. 恢复 end_plugin_span/plugin_method_span 中的 plugin.success 属性 2. 对 error_message 增加 [:1000] 截断保护 3. 关键 span 创建失败日志从 debug 提升为 warning 4. _CustomSpan 补充 SDK 兼容性风险注释 5. 旧设计文档标注已被新方案取代 Made-with: Cursor
* feat: 新增支持简化协议JSON创建流程接口 --story=132674359 # Conflicts: # bkflow/apigw/docs/apigw-docs.zip * fix: 重命名& 补充tests和docs --story=132674359 * fix: tests 修复 --story=132674359 * fix: 优化数据库查询 --story=132674359 * fix: tests 修复 & trigger_ids 防空值 --story=132674359 * fix: 作用域类型和作用域值 修复 --story=132674359
新增 _CustomSpan、create_execution_span、_create_span_with_custom_id、 clean_plugin_span_outputs 等函数,修复 span 层级关系,支持预生成 plugin_span_id 用于 method span 正确嵌套,精简冗余属性,增加 propagate_attributes 幂等优化。 Made-with: Cursor
更新插件基类 BKFlowBaseService:span 名称增加 .plugin. 命名空间, _get_trace_context 返回 plugin_span_id,_end_plugin_span 后调用 clean_plugin_span_outputs 清理内部属性,execute/schedule 传递 plugin_span_id 给 method span。 Made-with: Cursor
TaskOperation.start() 使用 create_execution_span 替代原有 get_current_trace_context 方式,创建独立的执行级根 span 并 注入 root_pipeline_data,不再依赖外部 trace context 传递。 Made-with: Cursor
Made-with: Cursor
1. 恢复 end_plugin_span/plugin_method_span 中的 plugin.success 属性 2. 对 error_message 增加 [:1000] 截断保护 3. 关键 span 创建失败日志从 debug 提升为 warning 4. _CustomSpan 补充 SDK 兼容性风险注释 5. 旧设计文档标注已被新方案取代 Made-with: Cursor
审批插件的 plugin_schedule 在成功路径上未调用 finish_schedule(), 导致基类 BKFlowBaseService.schedule() 中 is_schedule_finished() 始终 为 False,_end_plugin_span() 从未被触发,bkflow.plugin.approve 父级 Span 缺失,approve.execute 和 approve.schedule 直接暴露在最外层。 Made-with: Cursor
…abel feat: 标签管理功能优化 (TencentBlueKing#579)
# Reviewed, transaction id: 77017
There was a problem hiding this comment.
Code Review Summary
PR 新增了 SpaceConfigAdminViewSet.batch_delete 批量删除空间配置接口,以及对应的 SpaceConfigBatchDeleteSerializer。
主要问题
- 🚨 关键数据完整性风险:
batch_delete直接调用SpaceConfig.objects.filter(id__in=ids).delete(),绕过了SpaceConfigManager.delete_space_config()中对REF类型配置的引擎同步逻辑(TaskComponentClient.delete_engine_config),可能导致引擎侧残留孤立配置。 ⚠️ Serializer 缺少allow_empty=False校验,空列表会导致无意义的 delete 调用。⚠️ 日志信息使用中文,项目规范要求批量/清理操作日志使用英文。
整体思路正确,建议复用 Manager 层的 delete_space_config 逻辑(或新增 batch_delete_space_config 方法)来保证 REF 类型配置的引擎同步一致性。
| try: | ||
| SpaceConfig.objects.filter(id__in=ids).delete() | ||
| except Exception as e: | ||
| err_msg = f"批量删除空间配置失败: {str(e)}" |
There was a problem hiding this comment.
"batch delete space configs failed: {e}"。
There was a problem hiding this comment.
Code Review: PR #651
🔒 Security Issue — Space ID Not Validated
bkflow/space/models.py:177 in batch_delete():
instances = self.filter(id__in=inst_ids)The filter only checks ID but NOT space_id. This allows users to delete configs from other spaces if they know the IDs. Add constraint: .filter(id__in=inst_ids, space_id=space_id)
⚡ Logic Error — Redundant Delete
bkflow/space/models.py:187–190:
instances.delete() # Line 187 deletes ALL instances
if ref_instances_ids:
self.filter(id__in=ref_instances_ids).delete() # Line 190 — already deleted!Line 187 deletes all instances including REF types. Line 190's second delete is unreachable. Remove line 190 entirely since instances are already gone.
⚠️ Chinese Error Message — Use English
bkflow/space/views.py:453:
err_msg = f"批量删除空间配置失败: {str(e)}"Project convention requires English log messages. Change to: f"Failed to batch delete space configs: {str(e)}"
🔐 Input Validation Missing
bkflow/space/serializers.py:86–88 SpaceConfigBatchDeleteSerializer:
No validation that IDs belong to the given space_id. Add a validate() method to verify ownership before the model layer processes deletion. This prevents unauthorized access if a user guesses valid config IDs.
✨ Test Coverage
No tests added for batch delete. Consider adding:
- Test REF vs non-REF config deletion
- Verify remote API call for REF types
- Ensure atomic rollback on API failure
Summary: PR follows batch operation pattern correctly (POST method matches batch_delete_tasks), but has a permission gap (space_id not filtered) and logic redundancy (double delete). These should be fixed before merge. Test coverage would strengthen confidence in the refactoring.
# Reviewed, transaction id: 77026
There was a problem hiding this comment.
Code Review Summary
PR 将空间配置恢复默认值的操作改为批量删除,新增 SpaceConfigAdminViewSet.batch_delete 接口和 SpaceConfigBatchDeleteSerializer,并在 Manager 层实现了 batch_delete 方法。
先前评审问题追踪
- ✅ REF 类型引擎同步:
batch_delete已在 Manager 层正确处理 REF 类型配置的引擎侧同步删除 - ✅ 空列表校验:Serializer 已添加
min_length=1 - ✅ 冗余二次删除:已修复,当前仅一次
instances.delete()调用
当前仍存在的问题
- 🔒 space_id 未参与过滤(Critical):
batch_delete的self.filter(id__in=inst_ids)未限制space_id,用户传入其他空间的配置 ID 可跨空间删除 ⚠️ 中文日志消息(Minor):views.py:453错误日志使用中文,项目规范要求批量操作日志统一使用英文
其他观察
- 测试文件变更为标签模块的格式化修正和断言优化,与主功能无关,无问题
- 使用 POST 方法进行批量删除与项目中
batch_delete_tasks等现有模式一致 - 建议为
batch_delete补充单元测试(REF/非 REF 类型、事务回滚场景)
| try: | ||
| SpaceConfig.objects.batch_delete(inst_ids=ids, space_id=space_id) | ||
| except Exception as e: | ||
| err_msg = f"批量删除空间配置失败: {str(e)}" |
There was a problem hiding this comment.
f"batch delete space configs failed: {e}"。
# Reviewed, transaction id: 77030
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## develop #651 +/- ##
==========================================
Coverage ? 82.99%
==========================================
Files ? 272
Lines ? 16112
Branches ? 0
==========================================
Hits ? 13372
Misses ? 2740
Partials ? 0 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Code Review Summary
PR 将空间配置恢复默认值的操作改为批量删除,新增 SpaceConfigAdminViewSet.batch_delete 接口、SpaceConfigBatchDeleteSerializer 和 Manager 层 batch_delete 方法。同时修复了测试中的时间竞争问题和标签引用计数断言的顺序依赖。
先前评审问题追踪
| 问题 | 状态 |
|---|---|
| REF 类型引擎同步遗漏 | ✅ 已解决 — 逻辑已移入 Manager batch_delete |
ListField 缺少 min_length |
✅ 已解决 — 已添加 min_length=1 |
space_id 过滤缺失(跨空间删除风险) |
✅ 已解决 — 已添加 space_id=space_id |
| 日志应使用英文 | ❌ 未修复 — 仍为中文 |
本轮新发现
- ⚡
models.py:180-184—ref_instances_ids计算在事务外,存在竞态风险 ⚠️ views.py:453— 日志仍使用中文,不符合项目规范
测试改动
test_legacy.py:将VALID_DATETIME_TEST_CASE改为动态生成时间,修复模块加载时的时间竞争 — 改动合理test_label_viewset.py:用集合比较替代有序断言 — 改动合理
总体来看,先前的关键问题(REF 同步、跨空间删除)已修复,剩余 2 个 Minor/Important 级别建议。
| inst.id | ||
| for inst in instances | ||
| if SpaceConfigHandler.get_config(inst.name).value_type == SpaceConfigValueType.REF.value | ||
| ] |
There was a problem hiding this comment.
⚡ ref_instances_ids 的计算在 transaction.atomic() 之外,存在竞态窗口:在迭代 queryset 和进入事务之间,数据可能被并发请求修改。建议将此列表推导移到 with transaction.atomic(): 块内部,与 delete_space_config 保持一致的事务保护模式。
| try: | ||
| SpaceConfig.objects.batch_delete(inst_ids=ids, space_id=space_id) | ||
| except Exception as e: | ||
| err_msg = f"批量删除空间配置失败: {str(e)}" |
There was a problem hiding this comment.
f"batch delete space configs failed: {e}"。
|
noahchi seems not to be a GitHub user. You need a GitHub account to be able to sign the CLA. If you have already a GitHub account, please add the email address used for this commit to your account. You have signed the CLA already but the status is still pending? Let us recheck it. |
Reviewed, transaction id: 77017