Fix Vale CI file scoping and remove dead eBPF tutorial link#2580
Fix Vale CI file scoping and remove dead eBPF tutorial link#2580ctauchen merged 2 commits intotigera:mainfrom
Conversation
✅ Deploy Preview succeeded!Built without sensitive environment variables
To edit notification comments on pull requests, go to your Netlify project configuration. |
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
✅ Deploy Preview for calico-docs-preview-next ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
Add separator: " " to the vale-action step so it correctly parses the space-separated file list instead of falling back to scanning the entire repo. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
ctauchen
force-pushed
the
vale-deadlink-fix
branch
from
0a37b5a to
f67332f
Compare
There was a problem hiding this comment.
Pull request overview
This PR makes two unrelated but useful improvements to the repository: it fixes the Vale CI workflow to scope linting to only changed .md/.mdx files (rather than the entire repo), and removes a dead tigera.io/tutorials eBPF workshop link from four high-connection-workloads.mdx documentation files across Calico OSS versions.
Changes:
- Vale CI workflow (
vale.yml) updated to detect changed markdown files, build a JSON array of them, skip all heavy steps when no markdown changed, and pass the file list to the Vale action and the ripgrep pre-processing step. - Dead eBPF/XDP tutorial link removed from
high-connection-workloads.mdxacross three versioned releases (3.29, 3.30, 3.31) and the unversioned (calico/) copy.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
.github/workflows/vale.yml |
Adds changed-file detection, skip logic, and scopes all steps to only run on changed markdown files |
calico/network-policy/extreme-traffic/high-connection-workloads.mdx |
Removes dead eBPF/XDP tutorial link from unversioned docs |
calico_versioned_docs/version-3.29/network-policy/extreme-traffic/high-connection-workloads.mdx |
Removes dead eBPF/XDP tutorial link from v3.29 docs |
calico_versioned_docs/version-3.30/network-policy/extreme-traffic/high-connection-workloads.mdx |
Removes dead eBPF/XDP tutorial link from v3.30 docs |
calico_versioned_docs/version-3.31/network-policy/extreme-traffic/high-connection-workloads.mdx |
Removes dead eBPF/XDP tutorial link from v3.31 docs |
| FILES=$(git diff --name-only --diff-filter=d ${{ github.event.pull_request.base.sha }}...${{ github.event.pull_request.head.sha }} -- '*.md' '*.mdx') | ||
| if [ -z "$FILES" ]; then | ||
| echo "skip=true" >> "$GITHUB_OUTPUT" | ||
| echo 'files=all' >> "$GITHUB_OUTPUT" |
There was a problem hiding this comment.
When no markdown files are changed (skip=true), the code sets files=all on line 18. However, all subsequent steps that consume this output are gated on steps.changed.outputs.skip == 'false', so this value is never actually used. The string 'all' being stored as a fallback is misleading and potentially dangerous: if the skip condition ever fails to propagate correctly or is evaluated unexpectedly, the Vale action would receive files: all (a literal string, not a valid file path) rather than the intended JSON array, causing unpredictable behavior. The files=all line should simply be omitted from the skip=true branch since it serves no purpose.
| echo 'files=all' >> "$GITHUB_OUTPUT" |
| run: | | ||
| sudo apt-get install ripgrep | ||
| FILES="${{ steps.changed.outputs.files }}" | ||
| FILES=$(echo '${{ steps.changed.outputs.files }}' | jq -r '.[]' | tr '\n' ' ') |
There was a problem hiding this comment.
On line 43, ${{ steps.changed.outputs.files }} is interpolated directly into the run shell script. This is a GitHub Actions security anti-pattern: any content in that output is injected verbatim into the shell command before it is executed. While the files output is derived from git diff output (which in a pull_request event is determined by the branch's file paths), it is still best practice to pass step outputs via environment variables rather than direct expression interpolation in run scripts, to prevent shell injection. The value should be passed through an environment variable (e.g., env: FILES_JSON: ${{ steps.changed.outputs.files }}) and then referenced as $FILES_JSON in the shell.
| FILES=$(echo '${{ steps.changed.outputs.files }}' | jq -r '.[]' | tr '\n' ' ') | ||
| if [ -n "$FILES" ]; then | ||
| rg -l0 '\$\[[^\]]*\]' -- $FILES | xargs -0 perl -i -pe 's/\$\[[^\]]*\]/PICKLEVAR/g' || true |
There was a problem hiding this comment.
On line 45, the $FILES variable is unquoted in the rg command. After tr '\n' ' ' converts newlines to spaces, the resulting space-separated list is word-split by the shell when passed to rg. If any filename contains spaces (or glob characters), it will be incorrectly split into multiple arguments, causing rg to fail or search the wrong paths. The variable should either be quoted or the file list should be processed in a way that preserves filenames with spaces (e.g., use readarray/null-delimited processing).
| FILES=$(echo '${{ steps.changed.outputs.files }}' | jq -r '.[]' | tr '\n' ' ') | |
| if [ -n "$FILES" ]; then | |
| rg -l0 '\$\[[^\]]*\]' -- $FILES | xargs -0 perl -i -pe 's/\$\[[^\]]*\]/PICKLEVAR/g' || true | |
| FILES=$(echo '${{ steps.changed.outputs.files }}' | jq -r '.[]') | |
| if [ -n "$FILES" ]; then | |
| printf '%s\n' "$FILES" | rg -l0 '\$\[[^\]]*\]' --files-from - | xargs -0 perl -i -pe 's/\$\[[^\]]*\]/PICKLEVAR/g' || true |
Summary
getInput().trim()stripping the separatorTest plan
.md/.mdxfiles, not the entire repo🤖 Generated with Claude Code