Skip to content

Conversation

@dengyh
Copy link
Collaborator

@dengyh dengyh commented Jan 20, 2026

No description provided.

normal-wls and others added 9 commits January 20, 2026 15:33
* feat:  pipeline tree 协议转换基础实现
# Reviewed, transaction id: 39164

* feat: workflow config update #ignore
…BlueKing#195)

* feat: 数据转换 DataModel 到 PipelineTree 初步实现 # 184
# Reviewed, transaction id: 40085

* fix: add unit test for data_model to pipeline_tree TencentBlueKing#184
# Reviewed, transaction id: 40179
# Reviewed, transaction id: 40263

(cherry picked from commit 7e5faef)
…#199)

* feat: pipeline converters 目录结构调整 TencentBlueKing#184
# Reviewed, transaction id: 40345

* feat: pipeline converters 目录结构调整 TencentBlueKing#184
# Reviewed, transaction id: 40346
* feat: gateway协议转换基本实现 TencentBlueKing#184
# Reviewed, transaction id: 40354

* fix: 协议调整优化及问题修复 TencentBlueKing#184
# Reviewed, transaction id: 40538

* fix: gateway协议改造reviewed修改 TencentBlueKing#184
# Reviewed, transaction id: 40601
…#204)

* feat: pipeline converters 目录结构调整 TencentBlueKing#184
# Reviewed, transaction id: 40345

* feat: pipeline converters 目录结构调整 TencentBlueKing#184
# Reviewed, transaction id: 40346

* feat: 支持 json to data model 数据转换 TencentBlueKing#184
# Reviewed, transaction id: 40711

* feat: 集成 CI 配置调整 TencentBlueKing#184

* fix: fix flake8 TencentBlueKing#184
# Reviewed, transaction id: 40718
…ing#206)

* feat: 实现gateway协议json到data_model的转换 TencentBlueKing#184
# Reviewed, transaction id: 40866

* fix: 修改converters模块为惰性加载 TencentBlueKing#184
# Reviewed, transaction id: 40909

* fix: 修改converters模块加载方式 TencentBlueKing#184
# Reviewed, transaction id: 40971

* fix: 修复循环引用报错问题 TencentBlueKing#184
# Reviewed, transaction id: 40993
Copy link

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

代码审查总结

本次 PR 新增了流程导入功能,整体架构清晰。发现以下需关注的问题:

🚨 严重问题

  1. 文件上传安全 (bkflow/template/views.py:838): 缺少文件大小限制,存在 DoS 风险
  2. 异常处理过宽 (bkflow/template/views.py:848): 捕获所有异常可能掩盖真实问题,难以排查

⚡ 性能与设计

  1. 通配符导入 (bkflow/pipeline_converter/converters/init.py:3-4): from .xxx import * 可能导致命名空间污染

⚠️ 代码质量

  1. 常量定义不当 (bkflow/pipeline_converter/constants.py:14-18): 类属性 GATEWAYS 应移至类外作为模块常量
  2. TODO 标记 (bkflow/pipeline_converter/converters/data_model_to_web_pipeline/component.py:17): 生产代码包含未完成功能

✨ 建议

  • 添加文件大小限制 (如 10MB) 和内容校验
  • 细化异常类型,分别处理解析错误、转换错误等
  • 移除通配符导入,明确列出导出项
  • 完成或移除 TODO 标记的功能

共标注 5 处关键点,优先处理安全相关问题。

@github-actions
Copy link

📍 关键问题标注

🚨 严重 - 文件上传安全 (bkflow/template/views.py:838)

缺少文件大小限制,攻击者可上传超大文件导致服务 DoS。
建议: 添加 MAX_UPLOAD_SIZE 限制 (如 10MB),并在 Django settings 配置 DATA_UPLOAD_MAX_MEMORY_SIZE

🚨 严重 - 异常处理过宽 (bkflow/template/views.py:848)

except Exception 捕获所有异常会掩盖真实错误,难以排查问题。
建议: 分别处理具体异常类型

except (ValidationError, ParseError) as e:
    logger.error(f"Parse failed: {e}")
except ConversionError as e:
    logger.error(f"Conversion failed: {e}")

⚡ 性能 - 通配符导入 (bkflow/pipeline_converter/converters/init.py:3-4)

from .xxx import * 会污染命名空间且影响代码可读性。
建议: 明确列出导出项: __all__ = ['JsonToDataModelConverter', 'DataModelToPipelineTreeConverter']

⚠️ 代码质量 - 常量定义 (bkflow/pipeline_converter/constants.py:14-18)

GATEWAYS 定义为 Enum 类属性不符合 Python 惯例。
建议: 移至模块级: GATEWAY_TYPES = [NodeTypes.PARALLEL_GATEWAY.value, ...]

⚠️ 代码质量 - TODO 标记 (bkflow/pipeline_converter/converters/data_model_to_web_pipeline/component.py:17)

生产代码包含 # TODO: 可配置 未完成功能。
建议: 完成 hook 配置功能,或创建 issue 跟踪后移除此注释

@codecov-commenter
Copy link

Codecov Report

❌ Patch coverage is 61.69265% with 172 lines in your changes missing coverage. Please review.
⚠️ Please upload report for BASE (master@011cdfe). Learn more about missing BASE report.

Files with missing lines Patch % Lines
.../converters/data_model_to_web_pipeline/pipeline.py 24.35% 59 Missing ⚠️
...r/converters/data_model_to_web_pipeline/gateway.py 54.05% 17 Missing ⚠️
...converter/converters/json_to_data_model/gateway.py 52.94% 16 Missing ⚠️
bkflow/template/views/template.py 41.66% 14 Missing ⚠️
bkflow/pipeline_converter/converters/base.py 70.45% 13 Missing ⚠️
...ne_converter/converters/json_to_data_model/node.py 56.00% 11 Missing ⚠️
...converters/data_model_to_web_pipeline/component.py 46.66% 8 Missing ⚠️
...onverter/converters/json_to_data_model/pipeline.py 50.00% 8 Missing ⚠️
...rter/converters/data_model_to_web_pipeline/node.py 68.18% 7 Missing ⚠️
...w/pipeline_converter/file_handlers/json_handler.py 40.00% 6 Missing ⚠️
... and 5 more
Additional details and impacted files
@@            Coverage Diff            @@
##             master     #589   +/-   ##
=========================================
  Coverage          ?   79.48%           
=========================================
  Files             ?      255           
  Lines             ?    13627           
  Branches          ?        0           
=========================================
  Hits              ?    10832           
  Misses            ?     2795           
  Partials          ?        0           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

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.

5 participants