-
Notifications
You must be signed in to change notification settings - Fork 3
feat: codeagent standrad 支持 cnb平台 #17
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
Summary of ChangesHello @wwcchh0123, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! 此拉取请求旨在扩展CodeAgent Standard的功能,使其能够支持CNB平台。通过引入新的Terraform变量和更新设置脚本,用户现在可以轻松地在部署CodeAgent实例时配置CNB平台的连接参数,从而实现与CNB平台的集成。 Highlights
🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console. Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Code Review
This pull request introduces support for configuring a new "CNB" platform within the CodeAgent setup, alongside the existing GitLab integration. The changes involve adding new Terraform variables for CNB's base URL, API URL, webhook secret, and token, and updating the codeagent_setup.sh script to process these variables. The setup script now utilizes yq for YAML configuration updates, replacing previous sed commands for GitLab and adding new logic for CNB. Review comments indicate that the yq installation method in the script needs to be revised to ensure the correct Go version is installed, as the apt package provides an incompatible Python version. Additionally, the review suggests that the CNB webhook secret and token updates should be conditional to prevent inadvertently clearing existing values and that multiple yq calls for CNB configuration should be merged for better performance.
| echo "yq not found, installing via apt..." | ||
| apt-get update -qq | ||
| apt-get install -y yq |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
apt-get install -y yq 安装的 yq (通常是 Python 包装器) 与脚本中使用的 yq 命令语法 (Go 版本 v4+) 不兼容,这将导致脚本执行失败。
脚本中使用的语法 (例如 .path.key = "value") 是 Mike Farah 开发的 Go 版本 yq 的语法。而 Debian/Ubuntu 的 apt 源中的 yq 包是另一个基于 Python 的工具,其语法不同。
为了确保安装正确的版本,建议直接从 GitHub release 下载二进制文件。
| echo "yq not found, installing via apt..." | |
| apt-get update -qq | |
| apt-get install -y yq | |
| echo "yq not found, installing correct version from GitHub..." | |
| # The yq syntax used requires the Go version, apt provides an incompatible python version. | |
| wget https://github.com/mikefarah/yq/releases/latest/download/yq_linux_amd64 -O /usr/bin/yq | |
| chmod +x /usr/bin/yq |
| yq -i -y ".platforms.cnb.instances.cool.base_url = \"$CNB_BASE_URL\"" "$CODEAGENT_CONF" | ||
| yq -i -y ".platforms.cnb.instances.cool.api_url = \"$CNB_API_URL\"" "$CODEAGENT_CONF" | ||
| yq -i -y ".platforms.cnb.instances.cool.webhook_secret = \"$CNB_WEBHOOK_SECRET\"" "$CODEAGENT_CONF" | ||
| yq -i -y ".platforms.cnb.instances.cool.token = \"$CNB_TOKEN\"" "$CODEAGENT_CONF" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
这部分配置更新存在两个问题:
- (高危)
CNB_WEBHOOK_SECRET和CNB_TOKEN是可选的,但这里是无条件更新。如果这两个变量为空,会把配置文件中已有的值清空。 - (优化) 多次调用
yq会有性能开销,可以合并为一次调用。
建议修改为只在变量存在时才更新,并且可以考虑将多个更新操作合并。
| yq -i -y ".platforms.cnb.instances.cool.base_url = \"$CNB_BASE_URL\"" "$CODEAGENT_CONF" | |
| yq -i -y ".platforms.cnb.instances.cool.api_url = \"$CNB_API_URL\"" "$CODEAGENT_CONF" | |
| yq -i -y ".platforms.cnb.instances.cool.webhook_secret = \"$CNB_WEBHOOK_SECRET\"" "$CODEAGENT_CONF" | |
| yq -i -y ".platforms.cnb.instances.cool.token = \"$CNB_TOKEN\"" "$CODEAGENT_CONF" | |
| yq -i -y ".platforms.cnb.instances.cool.base_url = \"$CNB_BASE_URL\" | .platforms.cnb.instances.cool.api_url = \"$CNB_API_URL\"" "$CODEAGENT_CONF" | |
| if [ -n "$CNB_WEBHOOK_SECRET" ]; then | |
| yq -i -y ".platforms.cnb.instances.cool.webhook_secret = \"$CNB_WEBHOOK_SECRET\"" "$CODEAGENT_CONF" | |
| fi | |
| if [ -n "$CNB_TOKEN" ]; then | |
| yq -i -y ".platforms.cnb.instances.cool.token = \"$CNB_TOKEN\"" "$CODEAGENT_CONF" | |
| fi |
| echo " - Base URL: $CNB_BASE_URL" | ||
| echo " - API URL: $CNB_API_URL" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Security: Sensitive URL logging
Logging CNB URLs exposes internal infrastructure details to /var/log/codeagent_setup.log. Consider redacting these like the secrets:
| echo " - Base URL: $CNB_BASE_URL" | |
| echo " - API URL: $CNB_API_URL" | |
| echo "✓ CNB platform configuration completed" | |
| echo " - Base URL: [CONFIGURED]" | |
| echo " - API URL: [CONFIGURED]" |
| yq -i -y ".platforms.cnb.instances.cool.webhook_secret = \"$CNB_WEBHOOK_SECRET\"" "$CODEAGENT_CONF" | ||
| yq -i -y ".platforms.cnb.instances.cool.token = \"$CNB_TOKEN\"" "$CODEAGENT_CONF" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Code Quality: Missing conditional checks
Unlike GitLab config (lines 82-91), CNB unconditionally overwrites webhook_secret and token even if empty. This could clear existing values. Add conditional checks:
| yq -i -y ".platforms.cnb.instances.cool.webhook_secret = \"$CNB_WEBHOOK_SECRET\"" "$CODEAGENT_CONF" | |
| yq -i -y ".platforms.cnb.instances.cool.token = \"$CNB_TOKEN\"" "$CODEAGENT_CONF" | |
| if [ -n "$CNB_WEBHOOK_SECRET" ]; then | |
| yq -i -y ".platforms.cnb.instances.cool.webhook_secret = \"$CNB_WEBHOOK_SECRET\"" "$CODEAGENT_CONF" | |
| fi | |
| if [ -n "$CNB_TOKEN" ]; then | |
| yq -i -y ".platforms.cnb.instances.cool.token = \"$CNB_TOKEN\"" "$CODEAGENT_CONF" | |
| fi |
| # Backup if not already backed up | ||
| if [ ! -f "$CODEAGENT_CONF.bak.cnb" ]; then | ||
| cp "$CODEAGENT_CONF" "$CODEAGENT_CONF.bak.cnb" | ||
| fi |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Performance: Redundant backup
GitLab section already creates .bak at line 75. This creates unnecessary duplicate backups and extra I/O. Consider removing this CNB-specific backup.
|
Great work adding CNB platform support! The implementation follows the GitLab pattern well and properly marks sensitive variables. Critical issue: Documentation is missing - README and See inline comments for security and code quality improvements. |
No description provided.