Re-enable isolated build deployment write on elastic/docs-builder#3270
Re-enable isolated build deployment write on elastic/docs-builder#3270Mpdreamz wants to merge 3 commits into
Conversation
Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com>
…eview-local Removes the `if: false` gates that were disabling deployment creation and status updates — these were left over from initial setup and are needed now that S3 uploads are active. Also adds explicit `head.repo.full_name` fork checks to the aws-auth and s3-upload steps so that security does not rely solely on the upstream dependency chain. Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com>
|
Warning Rate limit exceeded
To continue reviewing without waiting, purchase usage credits in the billing tab. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Enterprise Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughThis PR makes two complementary changes to the deployment infrastructure. The workflow file enables deployment operations by granting the build job 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches✨ Simplify code
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
.github/workflows/docs-preview-local.yml (1)
218-255:⚠️ Potential issue | 🟠 Major | ⚡ Quick win"Create Deployment" will fail noisily for fork PRs.
github.event.repository.forkrefers to the base repo (elastic/docs-builder), which is never a fork, so thebuildjob's top-level guard (if: github.event.repository.fork == false) doesn't exclude fork PRs. Forpull_requestevents from forks, GitHub restricts theGITHUB_TOKENto read-only regardless of whatpermissionsdeclares — thecreateDeploymentcall will 403, producing a failed step in every fork contributor's CI run.All other sensitive steps already carry the correct guard. Adding it here makes this consistent:
🔧 Proposed fix
- name: Create Deployment if: > env.MATCH == 'true' && needs.check.outputs.any_modified != 'false' && ( github.event_name == 'push' || github.event_name == 'pull_request' ) + && (github.event_name != 'pull_request' || github.event.pull_request.head.repo.full_name == github.repository)🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.github/workflows/docs-preview-local.yml around lines 218 - 255, The "Create Deployment" step (id: deployment, uses: actions/github-script@v8) must skip forked pull requests to avoid 403 from createDeployment/createDeploymentStatus; update the step's if condition to additionally require that for pull_request events the PR head repo is not a fork (e.g. add "&& (github.event_name != 'pull_request' || github.event.pull_request.head.repo.fork == false)") so the script only runs when the PR head repository is not a fork.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In @.github/workflows/docs-preview-local.yml:
- Around line 218-255: The "Create Deployment" step (id: deployment, uses:
actions/github-script@v8) must skip forked pull requests to avoid 403 from
createDeployment/createDeploymentStatus; update the step's if condition to
additionally require that for pull_request events the PR head repo is not a fork
(e.g. add "&& (github.event_name != 'pull_request' ||
github.event.pull_request.head.repo.fork == false)") so the script only runs
when the PR head repository is not a fork.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: e99812a3-e988-492c-ae9e-44a0ac42161e
📒 Files selected for processing (2)
.github/workflows/docs-preview-local.ymlsrc/tooling/docs-builder/Commands/Assembler/DeployCommands.cs
Why
Deployment creation and status updates were disabled with
if: falsegates left over from initial workflow setup. With S3 uploads now active, GitHub deployment status should reflect the actual preview state so contributors and reviewers can see deployment links directly on the PR.What
Re-enables the Create Deployment and Update deployment status steps by removing the
if: falseguards, and grants thebuildjobdeployments: writepermission.Also adds explicit
head.repo.full_name == github.repositoryfork checks to theaws-authands3-uploadsteps so that fork PR safety does not rely solely on the upstream step dependency chain.