ci: fix auto deploy site failed #2616
Conversation
WalkthroughThe pull request introduces several updates to the auto-deployment workflow and environment configuration for a project. Key changes include modifications to permissions in the workflow file, updates to multiple action versions, and the addition of a new environment configuration file. The build command in the Changes
Possibly related PRs
Suggested reviewers
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
ci: set --max-old-space-size ci: add .env.pages fix: fix deploy fail
kagol
force-pushed
the
kagol/auto-deploy-site
branch
from
f5e2697 to
0bacaae
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Caution
Inline review comments failed to post. This is likely due to GitHub's limits when posting large numbers of comments.
Actionable comments posted: 1
🧹 Outside diff range and nitpick comments (1)
.github/workflows/auto-deploy-site.yml (1)
59-62: Add newline at end of fileThe deployment configuration looks good, but please add a newline at the end of the file to comply with YAML best practices.
url: ${{ steps.deployment.outputs.page_url }} +🧰 Tools
🪛 yamllint (1.35.1)
[error] 62-62: no new line character at the end of file
(new-line-at-end-of-file)
🛑 Comments failed to post (1)
.github/workflows/auto-deploy-site.yml (1)
46-48:
⚠️ Potential issueSecurity: Restrict directory permissions
The
chmod 777command grants full read/write/execute permissions to all users, which is overly permissive. Consider using more restrictive permissions.- - run: sudo chmod -R 777 ./dist + - run: sudo chmod -R 755 ./dist📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.- run: sudo chmod -R 755 ./dist - run: cp ./examples/sites/dist/assets/_commonjsHelpers*.js ./_site/assets - run: cp ./_site/index.html ./_site/404.html
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Outside diff range and nitpick comments (2)
.github/workflows/auto-deploy-site.yml (2)
38-38: Investigate root cause of memory issuesWhile increasing the Node.js memory limit might resolve the immediate issue, it's important to investigate why the build process requires such a large amount of memory.
Consider:
- Analyzing the build process for memory leaks
- Implementing incremental builds
- Optimizing asset bundling and processing
Would you like assistance in profiling the build process to identify memory-intensive operations?
60-62: Add newline at end of fileAdd a newline character at the end of the file to comply with POSIX standards and avoid potential issues with text processing tools.
environment: name: github-pages - url: ${{ steps.deployment.outputs.page_url }} + url: ${{ steps.deployment.outputs.page_url }} +🧰 Tools
🪛 yamllint (1.35.1)
[error] 62-62: no new line character at the end of file
(new-line-at-end-of-file)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (3)
.github/workflows/auto-deploy-site.yml(2 hunks)examples/sites/env/.env.pages(1 hunks)examples/sites/package.json(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- examples/sites/package.json
- examples/sites/env/.env.pages
🧰 Additional context used
🪛 yamllint (1.35.1)
.github/workflows/auto-deploy-site.yml
[error] 62-62: no new line character at the end of file
(new-line-at-end-of-file)
🔇 Additional comments (2)
.github/workflows/auto-deploy-site.yml (2)
26-27: Verify the necessity of write permissions
The addition of contents: write permissions at the job level overrides the more restrictive root-level permissions. While this might be needed for the new file operations, it's generally recommended to follow the principle of least privilege.
Let's verify if these permissions are strictly necessary:
✅ Verification successful
Write permissions are necessary for the workflow operations
The workflow requires write permissions as it performs file operations including:
- Modifying file permissions with
chmod - Copying files with
cpoperations - Uploading artifacts using
actions/upload-pages-artifact
These operations, particularly the file modifications and artifact uploads, require write access to the repository contents.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check if there are any git operations or file modifications in the workflow
rg -A 5 "git |checkout|commit|push|chmod|cp|mv" .github/workflows/auto-deploy-site.yml
Length of output: 635
40-40: Verify compatibility of major action version updates
Multiple actions have been updated to newer major versions:
configure-pages: v2 → v5upload-pages-artifact: v1 → v3deploy-pages: v1 → v4
These major version jumps might introduce breaking changes.
Let's verify the compatibility:
Also applies to: 50-50, 59-59
✅ Verification successful
Version updates are compatible and safe to use
Based on the release notes:
configure-pages@v5: Breaking changes only affect Next.js configurations, which isn't used in this workflowupload-pages-artifact@v3: No breaking changes in latest release (v3.0.1)deploy-pages@v4: No breaking changes in latest release (v4.0.5)
The workflow structure is simple with a single dependency chain (deploy needs build), and all actions are being used according to their latest stable versions. The updates are safe to proceed with.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check for any other actions in the workflow that might need updates
rg "uses: actions/" .github/workflows/auto-deploy-site.yml
# Check if these actions are used in other workflow files
fd -e yml -e yaml . .github/workflows/ --exec rg "actions/(configure|upload|deploy)-pages@"
Length of output: 437
Script:
#!/bin/bash
# Let's check the changelog or release notes for these actions in their respective repositories
gh api repos/actions/configure-pages/releases/latest
gh api repos/actions/upload-pages-artifact/releases/latest
gh api repos/actions/deploy-pages/releases/latest
# Also check if there are any other dependencies on these actions
rg -A 2 -B 2 "needs:" .github/workflows/auto-deploy-site.yml
Length of output: 8931
| - run: sudo chmod -R 777 ./dist | ||
| - run: cp ./examples/sites/dist/assets/_commonjsHelpers*.js ./_site/assets | ||
| - run: cp ./_site/index.html ./_site/404.html |
There was a problem hiding this comment.
Security and architectural concerns in file operations
Several concerns with the new file operations:
- Using
chmod 777is overly permissive and a security risk - Manual file copying might indicate build configuration issues
- Duplicating index.html as 404.html could be handled more elegantly
Consider these alternatives:
- Use more restrictive permissions:
chmod 644for files andchmod 755for directories - Configure the build process to output files in the correct location
- Handle 404 pages through proper routing configuration
Would you like assistance in implementing any of these improvements?
| @@ -31,27 +33,30 @@ | |||
| with: | |||
| version: 9 | |||
| - name: Install deps | |||
| run: pnpm -F @opentiny/vue-docs i --no-frozen-lockfile | |||
| run: pnpm i --no-frozen-lockfile | |||
There was a problem hiding this comment.
🛠️ Refactor suggestion
Consider scoping dependency installation
The removal of the package filter -F @opentiny/vue-docs means all dependencies will be installed, which could increase build time and resource usage unnecessarily.
Consider using a more targeted installation:
-run: pnpm i --no-frozen-lockfile
+run: pnpm i --no-frozen-lockfile -F @opentiny/vue-docs... 📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| run: pnpm i --no-frozen-lockfile | |
| run: pnpm i --no-frozen-lockfile -F @opentiny/vue-docs... |
PR
PR Checklist
Please check if your PR fulfills the following requirements:
PR Type
What kind of change does this PR introduce?
What is the current behavior?
Issue Number: N/A
What is the new behavior?
Does this PR introduce a breaking change?
Other information
Summary by CodeRabbit
New Features
Improvements
These changes aim to streamline deployment and enhance the configuration experience for users.