Repository navigation
[ci] Fix docs validation notification: use Jobs API instead of artifacts - #18053
Conversation
|
|
||
| permissions: | ||
| contents: read | ||
| pull-requests: write |
There was a problem hiding this comment.
@v1v is there any inconvenient on setting this permission in Github workflows?
I see there are other workflows setting that permission, but I'd like to double check it for this workflow.
This would be needed to post comment in Pull Requests (keep me honest here @theletterf )
There was a problem hiding this comment.
Worst thing that might happen from a fork is someone writing a different comment on their own PR.
There was a problem hiding this comment.
We follow the least-permissive approach, so permissions are at the job level rather than at the top-level. That's my suggestion here.
OTOH, this won't work for forked PRs, this GitHub workflow runs for PRs in an unprivileged environment. Maybe it will be interesting to provide a workaround for those forked PRs with a job summary:
Or, we could support the pull-request/workflow_run approach:
There was a problem hiding this comment.
Let me check — might split it again.
There was a problem hiding this comment.
Reworked — dropped pull-requests: write from the validation workflow entirely. Now using the two-workflow split: the notification workflow downloads the validation run's logs via the API and greps for docs-builder's error/warning summary line. Permissions are at job level.
There was a problem hiding this comment.
@v1v Thanks for the feedback — reworked the approach:
- Validation workflow stays read-only (
contents: readonly), keepscontinue-on-error: true, nopull-requests: write - Notification workflow (
workflow_runtrigger) downloads the validation run's logs via the API and greps for docs-builder's summary line (N Errors / N Warnings). This works regardless ofcontinue-on-errormasking step conclusions - Permissions are at job level per least-permissive approach
- Works for fork PRs since the
workflow_runtrigger runs in the base branch context
Could you take another look?
There was a problem hiding this comment.
This won't add any GitHub comments with the outcome. If you are looking for that feature, you could use the upload-artefact/download-artifact steps between the two different workflows.
I recall I saw something like that in the past
There was a problem hiding this comment.
@v1v Reworked per your suggestions:
- Validation workflow stays read-only (
contents: readonly), nopull-requests: write. Usesupload-artifact@v7to upload a signal file with the PR number when docs-builder finds errors - Notification workflow uses
download-artifact@v8withrun-idandgithub-tokento download the artifact cross-workflow. Permissions at job level (pull-requests: write,actions: read) - Artifact upload confirmed working in test PR [test] Intentional doc errors to test validation + notification #18049
Could you take another look?
There was a problem hiding this comment.
@v1v Updated again based on further review:
- Validation workflow:
contents: readonly, no write permissions. Usesupload-artifact@v7to upload a signal file when docs-builder finds errors.PR_NUMBERpassed as env var (no inline${{ }}in shell) - Notification workflow:
download-artifact@v8withrun-id+github-tokenfor cross-workflow download. Permissions at job level (pull-requests: write,actions: read) - Fork safety: The notification workflow verifies the PR number from the artifact matches the actual PR for the head SHA (
GET /repos/{repo}/commits/{sha}/pulls), preventing a forked PR from targeting a different PR's comments - Fork support: Works for fork PRs — artifacts are stored in the upstream repo, and
download-artifact@v8can access them from theworkflow_runcontext
Artifact upload confirmed in test PR #18049.
aadab2b to
30705d2
Compare
theletterf
changed the title
30705d2 to
f891c3c
Compare
![macroscopeapp[bot]](https://avatars.githubusercontent.com/in/900172?s=60&v=4){kind=small}
…rkflow Use upload-artifact@v7 / download-artifact@v8 for cross-workflow signaling. Validation workflow uploads a "docs-validation-failed" artifact with the PR number when docs-builder finds errors. Notification workflow downloads it and posts a comment. - Validation workflow: contents: read only, continue-on-error: true - Notification workflow: permissions at job level (pull-requests: write, actions: read), runs via workflow_run in base branch context - Artifact confirmed uploaded in test PR #18049 Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
f891c3c to
88d7cc1
Compare
| permissions: | ||
| pull-requests: write | ||
| actions: read |
| name: docs-validation-failed | ||
| path: validate-result/pr_number | ||
| if-no-files-found: ignore | ||
| retention-days: 1 |
Summary
Reworks the notification mechanism for the package docs validation workflow. When docs-builder finds errors, a comment is posted on the PR mentioning
@elastic/integration-docs.How it works
Validation workflow (
validate-package-docs.yml): When docs-builder exits non-zero, uploads adocs-validation-failedartifact containing the PR number. Usesupload-artifact@v7. Stays non-blocking (continue-on-error: true) withcontents: readonly.Notification workflow (
notify-package-docs-failure.yml): Triggered byworkflow_run. Downloads the artifact viadownload-artifact@v8withrun-idandgithub-tokenfor cross-workflow access. If artifact exists, posts a comment. Permissions at job level (pull-requests: write,actions: read).Security
pull_requesttrigger,contents: readonly — safe for forksworkflow_runtrigger, runs in base branch context — safe for forksTesting
Artifact upload confirmed in test PR #18049.
🤖 Generated with Claude Code