Skip to content

feat: 空间配置优化-后端接口 --story=1070120217132989396 - #838

Open
kaedePing wants to merge 1 commit into
TencentBlueKing:feat/space-config-redesignfrom
kaedePing:feat/optimize_space_config
Open

feat: 空间配置优化-后端接口 --story=1070120217132989396#838
kaedePing wants to merge 1 commit into
TencentBlueKing:feat/space-config-redesignfrom
kaedePing:feat/optimize_space_config

Conversation

@kaedePing

Copy link
Copy Markdown
Collaborator
  • 优化api/space/admin/space_config/config_meta接口 增加配置项的group/help/ui/verifiable字段返回
  • 增加api/space/admin/space_config/verify 校验前端的数据,目前只支持 uniform_api 字段的校验 # Reviewed, transaction id: 83605

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Code Review Summary

整体变更为空间配置增加了声明式元数据(group/help/ui/verifiable)和 verify 接口,结构清晰。以下是需要关注的问题:

级别 问题
🚨 Critical list_data.get('apis') 可能为 None 导致 TypeError
⚠️ Important verify 循环遍历所有分类所有 API 发起 HTTP 请求,无上限保护
⚠️ Important create 方法中残留调试代码 a=request.data
✨ Minor 多处赋值缺少 PEP8 空格

建议修复后合入。

Comment thread bkflow/space/configs.py Outdated

# 取某个分类的api
if not api_list:
for api in list_data.get('apis'):

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ list_data.get('apis') 可能返回 None(接口无数据或字段缺失),此时 for api in None 会抛 TypeError。建议改为 list_data.get('apis') or []

Comment thread bkflow/space/configs.py

# 3. 调用 meta 接口 → 取 list 返回的前 5 个 api 的 meta_url
samples = []
for item in api_list:

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚡ 注释说“取前 5 个 api 的 meta_url”,但实际遍历了 api_list 全量(无 [:5] 切片)。若某分类有大量 API,会发起大量 HTTP 请求。建议加 api_list[:5] 限制。

Comment thread bkflow/space/configs.py Outdated
)
if not cat_result.result:
raise ValidationError(f"[uniform_api verify] categories 接口请求失败: {cat_result.message}")
categories=cat_result.json_resp["data"]

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

✨ PEP8:赋值号两侧缺少空格。应为 categories = cat_result.json_resp["data"]。下方 category_length=api_length= 等同理。

Comment thread bkflow/space/configs.py

api_list = []
api_length=0
for category in categories:

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚡ 外层 for 循环对每个 category 发起 HTTP 请求,若分类数量多则 verify 耗时不可控。建议加上分类数量上限(如 categories[:10])或总请求超时保护。

Comment thread bkflow/space/views.py Outdated

def create(self, request, *args, **kwargs):
serializer = self.get_serializer(data=request.data)
a=request.data

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ a=request.data 看起来是调试残留,变量命名无意义且未提供额外价值。应还原为 serializer = self.get_serializer(data=request.data)

Comment thread bkflow/space/configs.py Outdated
if not api_categories_url:
raise ValidationError(f"[uniform_api verify] api_key={api_key} 未配置 api_categories_url")

logger.info(f"待测试api.meta_url: {meta_url},api_categories_url:{api_categories_url}")

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

✨ 日志应使用英文保持一致性(项目规范要求运行时日志使用英文)。建议改为 logger.info(f"[uniform_api verify] meta_url: {meta_url}, api_categories_url: {api_categories_url}")

Comment thread bkflow/space/configs.py Outdated
app_secret=content["bk_app_secret"],
username=operator or "admin",
)
logger.info(f"测试用凭证.credential_name: {credential_name},app_code: {content['bk_app_code']},app_secret: ******,username:{operator}")

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🔒 虽然 app_secret 已手动脱敏为 ******,但 app_code 也属于应用凭证的一部分。另外建议统一使用 copy.deepcopy + 脱敏工具函数,避免遗漏。

@kaedePing
kaedePing force-pushed the feat/optimize_space_config branch 2 times, most recently from db0f8f0 to 5a47cca Compare July 28, 2026 09:25

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Incremental Code Review

对比上次审查,以下问题已修复:

  • list_data.get('apis') 为 None 导致 TypeError — 已通过 or [] 修复
  • create 方法中调试残留 a=request.data — 已移除

新发现的问题:

级别 问题
🚨 Critical 测试断言与实现返回结构不匹配,test_uniform_api_verify_success 必然失败
⚠️ Important off-by-one:len(samples) > 5 实际允许 6 条再退出,与注释"最多 5 个"矛盾
⚠️ Important cat_result.json_resp["data"] 直接下标访问,若响应异常将抛 KeyError 且无上下文
⚡ Performance 上次提到的 category 循环无上限保护仍未处理

整体结构良好,建议修复 Critical 项后合入。

assert response.status_code == 200
payload = response.data.get("data", {})
assert payload["ok"] is True
assert payload["preview"]["total"] == 2

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🚨 测试与实现结构不匹配。verify 方法返回 {"api_key": ..., "api_length": ..., "samples": [...]},view 包装为 {"ok": True, "data": verify_data}。经 finalize_responseresponse.data.get("data") = {"ok": True, "data": {...}},不存在 preview 键,此测试必定 KeyError。需要将断言改为 payload["data"]["api_length"]payload["data"]["samples"],或修改 verify 的返回结构。

Comment thread bkflow/space/configs.py
# 3. 调用 meta 接口 → 取 list 返回的最多 5 个 api 的 meta_url
samples = []
for item in api_list:
if len(samples) > 5:

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Off-by-one:len(samples) > 5 在 samples 已有 6 条时才 break,实际会收集 6 条。注释说"最多 5 个",应改为 if len(samples) >= 5: break

Comment thread bkflow/space/configs.py Outdated
)
if not cat_result.result:
raise ValidationError(f"[uniform_api verify] categories 接口请求失败: {cat_result.message}")
categories = cat_result.json_resp["data"]

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ json_resp["data"] 直接下标访问,若接口返回格式异常会抛 KeyError 且无业务上下文。建议改为 .get("data") 并判空,与下方 line 514 的 .get("data", {}) 保持一致风格。

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Code Review Summary (增量审查)

已解决的问题

原问题 状态
list_data.get('apis') 可能为 None ✅ 已修复,使用 or []
a=request.data 调试残留 ✅ 已修复,代码已移除

新发现的问题

级别 文件 问题
⚠️ Important test_space_views.py:497 测试断言字段与实际返回不匹配,测试会失败
⚠️ Important configs.py:525 Off-by-one:> 5 实际会收集 6 条
✨ Minor test_config_metadata.py 新增文件缺少开源协议头

未修复但已有评论的问题(不重复报告)

  • categories 循环无上限保护
  • api_list 未切片
  • 日志中文 / PEP8 空格

建议修复 Important 级别问题后合入。

Comment thread bkflow/space/configs.py
# 3. 调用 meta 接口 → 取 list 返回的最多 5 个 api 的 meta_url
samples = []
for item in api_list:
if len(samples) > 5:

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Off-by-one:len(samples) > 5 会在收集到第 6 条后才 break,实际最多收集 6 个 sample。应改为 >= 5 以匹配注释中「最多 5 个」的意图。

assert response.status_code == 200
payload = response.data.get("data", {})
assert payload["ok"] is True
assert payload["preview"]["total"] == 2

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ 测试断言 payload["preview"]["total"]payload["preview"]["sample"],但 verify() 返回的 key 是 api_lengthsamples(且无 preview 包装层)。此测试会抛 KeyError 失败,需与实现对齐。

@@ -0,0 +1,101 @@
"""

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

✨ 新增 Python 文件缺少项目要求的开源协议声明头(参考同目录其他文件)。

@kaedePing
kaedePing force-pushed the feat/optimize_space_config branch 2 times, most recently from 3113497 to cc09a8b Compare July 28, 2026 10:08

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Code Review Summary (增量审查)

已解决的问题

原问题 状态
🚨 list_data.get('apis') 可能为 None 导致 TypeError ✅ 已修复,使用 or []
🚨 a=request.data 调试残留 ✅ 已修复,代码已移除
🚨 测试断言字段与实现返回结构不匹配 ✅ 已修复,测试现在使用正确的 key(ok/data/category_length/api_length/samples
⚠️ cat_result.json_resp["data"] 直接下标访问可能抛 KeyError ✅ 已修复,改为 .get('data') or []

仍存在的问题(已有评论,不重复报告)

级别 文件 问题
⚠️ Important configs.py:525 Off-by-one:> 5 实际允许收集 6 条,应为 >= 5
✨ Minor test_config_metadata.py:1 新增文件缺少开源协议头
⚡ Minor configs.py:501 categories 循环无上限保护

总结

本次推送修复了之前审查中报告的所有 Critical 问题,代码质量有明显提升。剩余 off-by-one 问题(> 5>= 5)建议在合入前修复,属于 Important 级别。整体功能设计合理,verify 接口的错误处理完备,权限继承自 ViewSet 类级配置无遗漏。

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Code Review Summary (增量审查)

已解决的问题

原问题 状态
list_data.get('apis') 可能为 None ✅ 已修复,使用 or []
a=request.data 调试残留 ✅ 已修复,代码已移除
测试断言与实现返回结构不匹配(preview key 不存在) ✅ 已修复,断言现在正确使用 payload["data"]["category_length"]
json_resp["data"] 直接下标访问 ✅ 已修复,改为 .get('data') or []

新发现的问题

级别 文件 问题
⚠️ Important views.py:425 用户可控的 params dict 直接作为 **kwargs 传入 verify,可能覆盖/冲突已有参数
✨ Minor configs.py:227 ui["help"] 缺少 _() 国际化包装,与其他配置项不一致

未修复但已有评论的问题(不重复报告)

  • Off-by-one:len(samples) > 5 实际允许 6 条
  • categories 循环无上限保护
  • api_list 未切片
  • 新增测试文件缺少开源协议头
  • 日志使用中文

整体代码结构清晰,建议处理 Important 级别的 params 注入问题后合入。

Comment thread bkflow/space/views.py
operation_summary="验证空间配置",
request_body=SpaceConfigVerifySerializer,
)
@action(detail=False, methods=["POST"])

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ params 来自用户输入,直接 **params 展开可能与显式传入的 space_id/value 冲突导致 TypeError,或注入非预期参数。建议过滤已知 key:params.pop("space_id", None); params.pop("value", None)

Comment thread bkflow/space/configs.py
"true_value": "true",
"false_value": "false",
"help": "关闭后到期即失效,需重新获取"
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

✨ 此处 "help" 缺少 _() 国际化包装,其他配置项的 ui["help"] 均使用了 _()。建议改为 "help": _("关闭后到期即失效,需重新获取")

@kaedePing
kaedePing force-pushed the feat/optimize_space_config branch 2 times, most recently from 30e0f11 to 397cf69 Compare July 29, 2026 01:48

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Code Review Summary (增量审查)

已解决的问题

原问题 状态
list_data.get('apis') 可能为 None 导致 TypeError ✅ 已修复,使用 or []
a=request.data 调试残留 ✅ 已修复,代码已移除
测试断言与实现返回结构不匹配(preview key) ✅ 已修复,断言已对齐
params 直接展开可能注入非预期参数 ✅ 已修复,pop space_id/value
ui["help"] 缺少 _() 国际化 ✅ 已修复
新测试文件缺少开源协议头 ✅ 已修复
json_resp["data"] 直接下标访问 ✅ 已修复,使用 .get('data') or []
PEP8 赋值号空格 ✅ 已修复

未解决的已有评论(不再重复报告)

  • Off-by-one len(samples) > 5(应为 >= 5
  • categories 循环无上限保护
  • 运行时日志含中文

新发现的问题

级别 文件 问题
⚠️ 逻辑 configs.py:457 查询 cred_config 缺少 is_deleted=False 过滤,与 line 428 不一致,可能读到已软删的配置

整体:本轮提交修复了绝大多数前次审查问题,代码质量明显提升。仅余一个新发现的查询一致性问题。

Comment thread bkflow/space/configs.py
# 获取测试用的凭证
if not credential_name:
cred_config = SpaceConfig.objects.filter(
space_id=space_id, name=ApiGatewayCredentialConfig.name).first()

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ 此处查询缺少 is_deleted=False 过滤,与 line 428-430 的同类查询不一致。可能导致读取到已软删除的凭证配置。建议补充:SpaceConfig.objects.filter(space_id=space_id, name=ApiGatewayCredentialConfig.name, is_deleted=False).first()

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Code Review Summary (增量审查)

已解决的问题

原问题 状态
🚨 list_data.get('apis') 可能为 None 导致 TypeError ✅ 已修复,使用 or []
🚨 a=request.data 调试残留 ✅ 已修复,代码已移除
🚨 测试断言字段与实现返回结构不匹配 ✅ 已修复,测试现在使用正确的 key
⚠️ json_resp["data"] 直接下标访问可能抛 KeyError ✅ 已修复,改为 .get('data') or []
⚠️ params 注入风险(可覆盖 space_id/value) ✅ 已修复,添加了 params.pop() 过滤
test_config_metadata.py 缺少开源协议头 ✅ 已修复,协议头已添加
ui["help"] 缺少 _() 国际化包装 ✅ 已修复

仍存在的问题(已有评论,不重复报告)

级别 文件 问题
⚠️ Important configs.py:526 Off-by-one:len(samples) > 5 实际允许收集 6 条,应为 >= 5
⚡ Minor configs.py:502 categories 循环无上限保护,极端情况下请求数不可控

总结

本次推送修复了之前审查中报告的全部 Critical 和绝大部分 Important 问题,代码质量明显提升。新增的 verify 接口错误处理完备,权限继承自 ViewSet 类级别配置(AdminPermission | SpaceSuperuserPermission)无遗漏,测试覆盖了正常/异常/不支持三种路径。

剩余 off-by-one 问题(> 5>= 5)建议在合入前修复,属于 Important 级别但修改量极小。

- 优化api/space/admin/space_config/config_meta接口 增加配置项的group/help/ui/verifiable字段返回
- 增加api/space/admin/space_config/verify 校验前端的数据,目前只支持 uniform_api 字段的校验
# Reviewed, transaction id: 83619
@kaedePing
kaedePing force-pushed the feat/optimize_space_config branch 2 times, most recently from 8d15760 to f89a535 Compare July 29, 2026 01:57

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Code Review Summary (增量审查)

已解决的问题

原问题 状态
🚨 list_data.get('apis') 可能为 None 导致 TypeError ✅ 已修复,使用 or []
🚨 a=request.data 调试残留 ✅ 已修复
🚨 测试断言字段与实现返回结构不匹配 ✅ 已修复
⚠️ json_resp["data"] 直接下标访问 ✅ 已修复,使用 .get('data') or []
⚠️ params 注入风险 ✅ 已修复,pop space_id/value
✨ 测试文件缺少开源协议头 ✅ 已修复
ui["help"] 缺少 _() 国际化 ✅ 已修复

仍存在的问题

级别 文件 问题
⚠️ Important configs.py:524 Off-by-one:> 5 实际允许 6 条,应为 >= 5
⚠️ Important configs.py:463 Credential 查询缺少 is_deleted=False,可能读到已软删凭证

关于之前 is_deleted 评论的澄清

之前在 line 456 指出 SpaceConfig 查询缺少 is_deleted=False,但实际上 SpaceConfig 不继承 CommonModel没有 is_deleted 字段,因此无需过滤。真正需要关注的是 line 463 的 Credential 查询——Credential 继承自 CommonModel(含 is_deleted),且 credential/resolver.py:56views.py:88 均使用了 is_deleted=False 过滤。

总结

代码质量持续提升,之前报告的 Critical 问题全部修复。剩余 2 个 Important 级别问题均为小改动,建议修复后合入。

Comment thread bkflow/space/configs.py
# 3. 调用 meta 接口 → 取 list 返回的最多 5 个 api 的 meta_url
samples = []
for item in api_list:
if len(samples) > 5:

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Off-by-one 仍未修复:len(samples) > 5 在已收集 6 条后才 break,实际最多 6 条。应改为 >= 5 以匹配注释「最多 5 个」的意图。

Comment thread bkflow/space/configs.py
if not credential_name:
raise ValidationError("[uniform_api verify] 空间未配置默认网关凭证,无法测试")

credential = Credential.objects.filter(space_id=space_id, name=credential_name).first()

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Credential 继承自 CommonModel(有 is_deleted 字段),此处缺少 is_deleted=False 过滤,可能读到已软删除的凭证。参考 credential/resolver.py:56views.py:88 的写法,建议改为:

credential = Credential.objects.filter(space_id=space_id, name=credential_name, is_deleted=False).first()

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Code Review Summary (增量审查)

已解决的问题

原问题 状态
🚨 list_data.get('apis') 可能为 None 导致 TypeError ✅ 已修复,使用 or []
🚨 a=request.data 调试残留 ✅ 已修复,代码已移除
🚨 测试断言字段与实现返回结构不匹配 ✅ 已修复,key 已对齐
⚠️ json_resp["data"] 直接下标访问可能抛 KeyError ✅ 已修复,改为 .get('data') or []
⚠️ params 直接展开可注入 space_id/value ✅ 已修复,已 pop 敏感 key
test_config_metadata.py 缺少开源协议头 ✅ 已修复
ui["help"] 缺少 _() 国际化包装 ✅ 已修复
⚠️ 此前评论在 line 456 建议 SpaceConfigis_deleted=False ✅ 此为误报——SpaceConfig 继承 models.Model,无 is_deleted 字段

仍存在的问题(已有评论,不重复报告)

级别 文件 问题
⚠️ Important configs.py:524 Off-by-one:len(samples) > 5 实际允许收集 6 条,应为 >= 5
⚡ Minor configs.py:500 categories 循环无上限保护

新发现的问题

无新增问题。

总结

本次推送已修复此前审查中报告的全部 Critical 和绝大部分 Important 问题。代码结构清晰,verify 接口错误处理路径完备,权限继承自 ViewSet 类级配置(AdminPermission | SpaceSuperuserPermission)合理。此前关于 SpaceConfig 需加 is_deleted=False 的评论为误报(该模型无此字段)。

唯一剩余的 Important 级别问题是 off-by-one(> 5>= 5),修改量极小,建议合入前修复。

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.

1 participant