Split docs preview: reusable Docker workflow + local dotnet workflow#3064
Split docs preview: reusable Docker workflow + local dotnet workflow#3064
Conversation
- preview-build.yml: workflow_call only; single Docker path (remove docs-builder repo branches); unify validate/build step IDs for downstream steps. - docs-preview-local.yml: PR/push workflow with bootstrap + dotnet run for elastic/docs-builder (preserves fork guards and disabled deploy blocks). - preview-cleanup.yml: TODO for future migration to elastic/docs-actions. Made-with: Cursor
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds a new in-repo GitHub Actions workflow Sequence Diagram(s)sequenceDiagram
participant GitHub as "GitHub (events)"
participant Runner as "Actions Runner\n(GitHub Actions)"
participant Assembler as "assembler-match\naction"
participant ChangedFiles as "changed-files\naction + github-script"
participant DocsBuilder as "docs-builder\n(dotnet / assembler)"
participant AWS as "AWS Auth / S3"
participant LinkIndex as "update-link-index\naction"
GitHub->>Runner: trigger workflow (pull_request / push / tag)
Runner->>Assembler: run match job -> compute content-source flags
Runner->>ChangedFiles: run check job -> detect changed files & outputs
alt non-fork & MATCH == "true" & changes detected
Runner->>DocsBuilder: run build job -> checkout ref and run validations
DocsBuilder->>DocsBuilder: validate redirects
DocsBuilder->>DocsBuilder: strict doc build
DocsBuilder->>DocsBuilder: validate inbound links
DocsBuilder->>DocsBuilder: validate path-prefixes
DocsBuilder->>AWS: authenticate (upload/invalidate steps present but disabled)
DocsBuilder->>LinkIndex: update link index (on eligible push runs)
end
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches✨ Simplify code
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.github/workflows/docs-preview-local.yml:
- Around line 16-21: The ref filter is using regex-style patterns (e.g.,
'\d+.\d+.\d+', '\d+.\d+', '\d+.x') but GitHub Actions expects globs; replace
those with glob equivalents—for numeric-only matches use '[0-9]*.[0-9]*.[0-9]*'
and 'v[0-9]*.[0-9]*.[0-9]*', and for broader matching use '*.*.*', '*.*', '*.x'
(or 'v*.*.*' for tag variants) in the same keys where the current patterns
appear (the branch/tag filter entries shown).
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: d65590ee-1de5-4048-9317-828f3551b660
📒 Files selected for processing (3)
.github/workflows/docs-preview-local.yml.github/workflows/preview-build.yml.github/workflows/preview-cleanup.yml
…w-local GitHub Actions matches branches/tags with fnmatch-style globs, not regex. Replace regex-style entries with *.*.*, *.*, *.x for branches and *.*.*, *.* for tags. Made-with: Cursor
…h filters Made-with: Cursor
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
.github/workflows/docs-preview-local.yml (1)
340-344: AWS auth step lacks explicitMATCHand fork guards.This step relies on
steps.internal-validate-path-prefixes.outcome == 'success'for gating, which transitively provides the guards since that step checksenv.MATCH == 'true'and the fork condition. While currently safe, this implicit dependency is fragile—future edits to upstream conditions could inadvertently expose this step.Consider adding explicit checks for consistency with other steps:
Suggested fix
- uses: elastic/docs-builder/.github/actions/aws-auth@main if: > + env.MATCH == 'true' && !cancelled() && needs.check.outputs.any_modified != 'false' + && (github.event_name != 'pull_request' || github.event.pull_request.head.repo.full_name == github.repository) && steps.internal-validate-path-prefixes.outcome == 'success'🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/docs-preview-local.yml around lines 340 - 344, The AWS auth step's if condition is relying indirectly on steps.internal-validate-path-prefixes for MATCH and fork guarding; update the step's if expression to explicitly include the same guards: keep the existing cancelled() and needs.check.outputs.any_modified checks, and add an explicit check that env.MATCH == 'true' and an explicit non-fork guard (verify the pull request run’s repo matches the base repo, e.g., compare github.event.pull_request.head.repo.full_name to github.repository or equivalent) so the step cannot run when MATCH is false or the workflow is triggered from a fork.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.github/workflows/docs-preview-local.yml:
- Around line 142-144: The current code uses glob.create() and
ignoreGlobber.glob() (creating ignoreGlobber and ignoredPaths) which performs
filesystem globbing and returns empty for pull_request events; change this to
treat IGNORE_PATTERNS as pattern strings and perform pattern matching against
each file path instead of creating a globber. Replace the
ignoreGlobber/ignoredPaths flow by splitting process.env.IGNORE_PATTERNS into
patterns and use a matcher (e.g., minimatch or simple string prefix checks) to
test each file path when filtering (refer to the variables ignorePatterns,
ignoreGlobber, ignoredPaths and the use of glob.create) so filtering works even
when the filesystem isn’t checked out.
---
Nitpick comments:
In @.github/workflows/docs-preview-local.yml:
- Around line 340-344: The AWS auth step's if condition is relying indirectly on
steps.internal-validate-path-prefixes for MATCH and fork guarding; update the
step's if expression to explicitly include the same guards: keep the existing
cancelled() and needs.check.outputs.any_modified checks, and add an explicit
check that env.MATCH == 'true' and an explicit non-fork guard (verify the pull
request run’s repo matches the base repo, e.g., compare
github.event.pull_request.head.repo.full_name to github.repository or
equivalent) so the step cannot run when MATCH is false or the workflow is
triggered from a fork.
🪄 Autofix (Beta)
❌ Autofix failed (check again to retry)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: aa7e1471-bf66-4f4b-93a7-c5a3bfdb76f7
📒 Files selected for processing (1)
.github/workflows/docs-preview-local.yml
| const ignorePatterns = process.env.IGNORE_PATTERNS; | ||
| const ignoreGlobber = await glob.create(ignorePatterns); | ||
| const ignoredPaths = new Set(await ignoreGlobber.glob()); |
There was a problem hiding this comment.
Filesystem glob won't filter files for PR events.
glob.create() searches the filesystem, but for pull_request events, checkout doesn't run (see line 117). The ignoredPaths set will be empty, so .github/** and README.md files won't be filtered from the detailed output.
Use pattern matching (e.g., minimatch or simple string checks) instead of filesystem globbing:
Suggested fix
const ignorePatterns = process.env.IGNORE_PATTERNS;
- const ignoreGlobber = await glob.create(ignorePatterns);
- const ignoredPaths = new Set(await ignoreGlobber.glob());
+ const patterns = ignorePatterns.split('\n').map(p => p.trim()).filter(Boolean);
+
+ function shouldIgnore(filename) {
+ return patterns.some(pattern => {
+ if (pattern.endsWith('/**')) {
+ const prefix = pattern.slice(0, -3);
+ return filename.startsWith(prefix + '/') || filename === prefix;
+ }
+ return filename === pattern;
+ });
+ }Then replace line 167:
- const filteredFiles = allFiles.filter(file => !ignoredPaths.has(file.filename));
+ const filteredFiles = allFiles.filter(file => !shouldIgnore(file.filename));🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.github/workflows/docs-preview-local.yml around lines 142 - 144, The current
code uses glob.create() and ignoreGlobber.glob() (creating ignoreGlobber and
ignoredPaths) which performs filesystem globbing and returns empty for
pull_request events; change this to treat IGNORE_PATTERNS as pattern strings and
perform pattern matching against each file path instead of creating a globber.
Replace the ignoreGlobber/ignoredPaths flow by splitting
process.env.IGNORE_PATTERNS into patterns and use a matcher (e.g., minimatch or
simple string prefix checks) to test each file path when filtering (refer to the
variables ignorePatterns, ignoreGlobber, ignoredPaths and the use of
glob.create) so filtering works even when the filesystem isn’t checked out.
… a trigger) Made-with: Cursor
Remove merge_group and workflow_dispatch steps/branches; match job outputs use pr + push only; simplify PR file-list script to pull_request API path. Made-with: Cursor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.github/workflows/docs-preview-local.yml:
- Around line 31-33: The concurrency group currently uses head.ref which can
collide across forks; update the concurrency group expression (the
concurrency.group key) to include a PR-unique identifier when handling
pull_request events—e.g., use github.event.pull_request.number (or
github.event.pull_request.node_id) combined with the head.ref or github.ref so
runs for different PRs never share the same group; modify the expression that
builds docs-preview-local-${{ ... }} to conditionally include
github.event.pull_request.number for pull_request events and fall back to
github.ref for other events.
🪄 Autofix (Beta)
❌ Autofix failed (check again to retry)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 1708334f-146b-477f-90f2-1098adb32364
📒 Files selected for processing (1)
.github/workflows/docs-preview-local.yml
| concurrency: | ||
| group: docs-preview-local-${{ github.event.pull_request.head.ref || github.ref }} | ||
| cancel-in-progress: ${{ startsWith(github.event_name, 'pull_request') }} |
There was a problem hiding this comment.
Make the concurrency key truly per-PR.
Line 32 groups PR runs by head.ref, so two PRs from different forks with the same branch name will cancel each other. That breaks the intended per-PR cancellation behavior.
Suggested fix
concurrency:
- group: docs-preview-local-${{ github.event.pull_request.head.ref || github.ref }}
+ group: docs-preview-local-${{ github.event.pull_request.number || github.ref }}
cancel-in-progress: ${{ startsWith(github.event_name, 'pull_request') }}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.github/workflows/docs-preview-local.yml around lines 31 - 33, The
concurrency group currently uses head.ref which can collide across forks; update
the concurrency group expression (the concurrency.group key) to include a
PR-unique identifier when handling pull_request events—e.g., use
github.event.pull_request.number (or github.event.pull_request.node_id) combined
with the head.ref or github.ref so runs for different PRs never share the same
group; modify the expression that builds docs-preview-local-${{ ... }} to
conditionally include github.event.pull_request.number for pull_request events
and fall back to github.ref for other events.
There was a problem hiding this comment.
♻️ Duplicate comments (2)
.github/workflows/docs-preview-local.yml (2)
32-34:⚠️ Potential issue | 🟠 MajorConcurrency key collision risk remains unaddressed.
PRs from different forks using the same branch name will share a concurrency group and cancel each other. Use PR number for unique grouping.
Suggested fix
concurrency: - group: docs-preview-local-${{ github.event.pull_request.head.ref || github.ref }} + group: docs-preview-local-${{ github.event.pull_request.number || github.ref }} cancel-in-progress: ${{ github.event_name == 'pull_request' }}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/docs-preview-local.yml around lines 32 - 34, The concurrency group expression currently uses "docs-preview-local-${{ github.event.pull_request.head.ref || github.ref }}" which can collide for forks using the same branch name; update the group to include the PR number when available (e.g., "docs-preview-local-${{ github.event.pull_request.number || github.ref }}") so each pull request has a unique concurrency key, keeping "cancel-in-progress" behavior unchanged; modify the "group" value in the concurrency block accordingly.
129-131:⚠️ Potential issue | 🟠 MajorFilesystem glob won't filter files for PR events.
glob.create()searches the local filesystem, but checkout doesn't run forpull_requestevents (see line 104). TheignoredPathsset will be empty, so.github/**andREADME.mdfiles won't actually be filtered from the output at line 142.Use pattern matching against the filename strings instead of filesystem globbing:
Suggested fix
const ignorePatterns = process.env.IGNORE_PATTERNS; - const ignoreGlobber = await glob.create(ignorePatterns); - const ignoredPaths = new Set(await ignoreGlobber.glob()); + const patterns = ignorePatterns.split('\n').map(p => p.trim()).filter(Boolean); + + function shouldIgnore(filename) { + return patterns.some(pattern => { + if (pattern.endsWith('/**')) { + const prefix = pattern.slice(0, -3); + return filename.startsWith(prefix + '/') || filename === prefix; + } + return filename === pattern; + }); + }Then at line 142:
- const filteredFiles = allFiles.filter(file => !ignoredPaths.has(file.filename)); + const filteredFiles = allFiles.filter(file => !shouldIgnore(file.filename));🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/docs-preview-local.yml around lines 129 - 131, The current code uses glob.create(ignorePatterns) and ignoreGlobber.glob() to build ignoredPaths which relies on filesystem presence (ignoreGlobber and ignoredPaths), but for PR events the repo isn't checked out so this yields an empty set; change the logic to treat ignorePatterns as glob patterns and match them against the filename strings directly (i.e., remove the filesystem globbing step using glob.create/ignoreGlobber and instead compile or use a glob-match function to test each file path string before output/filtering where ignoredPaths is used). Locate references to ignorePatterns, ignoreGlobber, ignoredPaths and the filtering site (the code that filters files later) and replace the filesystem glob approach with per-filename pattern matching.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In @.github/workflows/docs-preview-local.yml:
- Around line 32-34: The concurrency group expression currently uses
"docs-preview-local-${{ github.event.pull_request.head.ref || github.ref }}"
which can collide for forks using the same branch name; update the group to
include the PR number when available (e.g., "docs-preview-local-${{
github.event.pull_request.number || github.ref }}") so each pull request has a
unique concurrency key, keeping "cancel-in-progress" behavior unchanged; modify
the "group" value in the concurrency block accordingly.
- Around line 129-131: The current code uses glob.create(ignorePatterns) and
ignoreGlobber.glob() to build ignoredPaths which relies on filesystem presence
(ignoreGlobber and ignoredPaths), but for PR events the repo isn't checked out
so this yields an empty set; change the logic to treat ignorePatterns as glob
patterns and match them against the filename strings directly (i.e., remove the
filesystem globbing step using glob.create/ignoreGlobber and instead compile or
use a glob-match function to test each file path string before output/filtering
where ignoredPaths is used). Locate references to ignorePatterns, ignoreGlobber,
ignoredPaths and the filtering site (the code that filters files later) and
replace the filesystem glob approach with per-filename pattern matching.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 99b7a4a4-e754-4e25-8059-3f4d3f1ce9aa
📒 Files selected for processing (1)
.github/workflows/docs-preview-local.yml
|
Note Autofix is a beta feature. Expect some limitations and changes as we gather feedback and continue to improve it. An unexpected error occurred while generating fixes: Resource not accessible by integration - https://docs.github.com/rest/git/trees#create-a-tree |
|
Note Autofix is a beta feature. Expect some limitations and changes as we gather feedback and continue to improve it. An unexpected error occurred while generating fixes: Resource not accessible by integration - https://docs.github.com/rest/git/trees#create-a-tree |
1 participant
Summary
This change aligns
elastic/docs-builderwith the broader migration away from monolithic preview workflows (docs-eng-team#474).preview-build.ymlis nowworkflow_callonly (single Docker-based path; allgithub.repository == 'elastic/docs-builder'branches removed). Repositories that still call this reusable workflow are unchanged.docs-preview-local.ymlruns onpull_request/pushin this repo only, using bootstrap +dotnet runso PRs validate the in-tree CLI (same behavior as the former special-casing).preview-cleanup.ymlincludes a TODO to migrate toelastic/docs-actionslater; behavior is unchanged.Follow-up intent
We intend to remove
preview-build.ymlandpreview-cleanup.ymlfrom this repository once all downstream consumers have migrated toelastic/docs-actions(or equivalent) and no longer reference these workflows. Until then, they remain for backward compatibility.Ops
preview-buildno longer runs automatically on PRs in this repo.Testing
docs-preview-localon this PR.Made with Cursor