sunchao commented on code in PR #7386:
URL: https://github.com/apache/datafusion-comet/pull/7386#discussion_r4437102176
##########
.github/workflows/docs.yaml:
##########
@@ -109,9 +139,9 @@ jobs:
-a \
--delete \
--exclude '/.git/' \
- ../docs/build/html/ \
+ ../site-output/ \
Review Comment:
**Protect `.git` regardless of its file type.** The trailing slash in
`--exclude '/.git/'` only matches a directory. In a disposable checkout, a
regular `site-output/.git` file made this exact rsync command delete the
checkout’s nonempty `.git` directory and replace it with the supplied file. Git
then follows that file’s `gitdir:` pointer, so subsequent Git commands can use
configuration supplied by the read-only builder in the job with `contents:
write`. `--exclude '/.git'` preserved the checkout in the same test. Please add
the regular-file case to the site-copy fixture.
##########
dev/ci/check-ci-config.py:
##########
@@ -1928,11 +1928,52 @@ def check_scheduled_workflows_stay_upstream():
return not failures
+def check_docs_permission_boundary():
+ """Keep dependency builds away from the site publishing token."""
+ path = WORKFLOWS / "docs.yaml"
+ jobs = job_blocks(path)
+ build = jobs.get("build-docs", "")
+ deploy = jobs.get("deploy-docs", "")
+ caller = job_blocks(CI_WORKFLOW).get("docs", "")
+ failures = []
+ for label, block, permission in (
+ ("caller", caller, "write"),
+ ("build-docs", build, "read"),
+ ("deploy-docs", deploy, "write"),
+ ):
+ if not re.search(rf"^ permissions:\n contents: {permission}$",
block, re.M):
+ failures.append(f"{label} must have contents: {permission}")
+ checkout_steps = re.findall(
+ r" - name: [^\n]+\n uses: actions/checkout@[^\n]+(?:\n
[^\n]+|\n with:)*",
+ build,
+ )
+ checkout_count = len(re.findall(r"uses: actions/checkout@", build))
+ if (not checkout_count or len(checkout_steps) != checkout_count
+ or any("persist-credentials: false" not in step for step in
checkout_steps)):
+ failures.append("every build checkout must disable persisted
credentials")
+ if not re.search(r"^ needs: build-docs$", deploy, re.M):
+ failures.append("deploy-docs must depend on the build")
+ uses = re.findall(r"^ uses: (\S+)", deploy, re.M)
+ if (len(uses) != 2 or not uses[0].startswith("actions/checkout@")
+ or not uses[1].startswith("actions/download-artifact@")):
+ failures.append("the writer may only check out asf-site and download
the site artifact")
+ if len(re.findall(r"^ run:", deploy, re.M)) != 1:
+ failures.append("the writer must have just the static publication
script")
Review Comment:
**Check unnamed workflow steps too.** Adding a valid `- run: bash
site-output/build.sh` step after the download still makes this check return
true. The regex counts `run:` only when it appears under a step name. Changing
the checkout ref from `asf-site` to `main` also passes. Can the check inspect
every step’s `run` or `uses` key and verify the writer’s checkout ref and path?
Both mutations make good negative tests.
##########
.github/workflows/docs.yaml:
##########
@@ -101,6 +99,38 @@ jobs:
# node, and a green build. This runs before the publish step for that
reason.
run: python3 dev/ci/check-mermaid.py --built docs/build/html
+ - name: Prepare site metadata
+ run: |
+ cp .asf.yaml docs/build/html/.asf.yaml
+ touch docs/build/html/.nojekyll
+
+ - name: Upload generated site
+ uses: actions/upload-artifact@v7
Review Comment:
**Use the retrying upload action for this handoff.** A transient
`FinalizeArtifact` 403 here drops the site after the five-profile docs build,
and the deploy job has nothing to download. The workflow README calls for
`./.github/actions/upload-artifact-retry` when a later job consumes an
artifact. The builder already has the local action checked out, and it accepts
`include-hidden-files` and `if-no-files-found`.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
