Property automation: cloud-sync trigger, workflow hardening, iceberg override cleanup - #1847
Property automation: cloud-sync trigger, workflow hardening, iceberg override cleanup#1847JakeSCahill wants to merge 3 commits into
Conversation
✅ Deploy Preview for redpanda-docs-preview ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
📝 WalkthroughWalkthroughThe documentation workflow now handles cloud-property-sync dispatches with separate concurrency, cached pip installation, tag resolution from Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant GitHubDispatch
participant UpdatePropertyDocs
participant AntoraYml
participant PullRequest
GitHubDispatch->>UpdatePropertyDocs: trigger-cloud-property-sync
UpdatePropertyDocs->>AntoraYml: read latest-redpanda-tag
AntoraYml-->>UpdatePropertyDocs: documentation tag
UpdatePropertyDocs->>UpdatePropertyDocs: generate documentation
UpdatePropertyDocs->>PullRequest: create or update cloud-sync PR
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
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 |
|
Companion cloudv2 dispatch PR: redpanda-data/cloudv2#28588 (sends the |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
.github/workflows/update-property-docs.yaml (1)
129-133: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winRegenerate the old baseline on the initial sync.
The required
--regenerate-old-baselineflag is missing, so the first regeneration after docs-extensions-and-macros#233 will not perform the required baseline migration. Add it for that initial cloud-sync run.🤖 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/update-property-docs.yaml around lines 129 - 133, Update the doc-tools property-docs generation command in the initial cloud-sync workflow to include the required --regenerate-old-baseline flag, preserving the existing tag, partials, cloud-support, and overrides options.
🤖 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.
Inline comments:
In @.github/workflows/update-property-docs.yaml:
- Line 82: Harden the tag handling in the workflow: at
.github/workflows/update-property-docs.yaml lines 82-82, pass dispatch inputs
through env, read quoted shell variables, and reject control characters before
writing tag to GITHUB_OUTPUT; at lines 144-144, pass steps.tag.outputs.tag
through env and assign it via TAG="$TAG" instead of embedding it directly in the
script.
- Around line 151-159: Validate CLOUD_COMMIT_SHA as a commit hash before the
output-writing block uses it, and reject or sanitize any value containing
newlines or other invalid characters. Update the conditional around
CLOUD_COMMIT_SHA in the workflow’s body generation so only validated values are
emitted, preserving the existing BODY_EOF output format for valid hashes.
---
Outside diff comments:
In @.github/workflows/update-property-docs.yaml:
- Around line 129-133: Update the doc-tools property-docs generation command in
the initial cloud-sync workflow to include the required
--regenerate-old-baseline flag, preserving the existing tag, partials,
cloud-support, and overrides options.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 7784a342-9ebc-4dc0-9df0-4a9ee71bc263
📒 Files selected for processing (2)
.github/workflows/update-property-docs.yamldocs-data/property-overrides.json
micheleRP
left a comment
micheleRP
left a comment
There was a problem hiding this comment.
Docs-team-standards review
Files reviewed: 2 (.github/workflows/update-property-docs.yaml +110/−12, docs-data/property-overrides.json +2/−8)
Overall assessment: No .adoc files changed, so the style checklist doesn't apply — reviewed as automation and override logic. The workflow hardening is solid and the injection-proofing is genuinely good. Two things need attention before the first dispatch, and the sequencing notes in the description are now out of date in a way worth correcting rather than acting on.
What this PR does
Adds a second repository_dispatch event (trigger-cloud-property-sync) so cloud property availability changes can trigger a same-tag regen without waiting for a Redpanda release. Cloud-sync runs resolve the tag from antora.yml, bypass the newer-tag guard, and land on a fixed branch with an extra label. Also adds npm/pip caching, a concurrency group, and passes all dispatch-controlled values through env instead of interpolating them into shell. Separately deletes a duplicate iceberg override keyed by an old property name and migrates its config_scope to the canonical entry.
Jira ticket alignment
Tickets: DOC-1886 (cloud-sync trigger and hardening), DOC-1889 (override cleanup)
Both addressed. The workflow half maps cleanly to DOC-1886's scope; the override half does what DOC-1889 asks. No generated files touched, consistent with the rule that property partials are only ever regenerated.
Critical issues
None confirmed. Everything checkable came back correct:
- Tag resolution —
latest-redpanda-tag: 'v26.2.1'is the only match inantora.yml, so thegrep | awk | trchain yieldsv26.2.1with no collision risk, and the new^[A-Za-z0-9._-]+$guard accepts it. - Concurrency expression —
action == '…' && 'cloud-sync' || inputs.tag || client_payload.tag || github.refresolves correctly for all three trigger types (traced each). - Token wiring —
ACTIONS_BOT_TOKENalready comes from Secrets Manager, so cloud-sync PRs will run the repo's checks rather than silently skipping them the wayGITHUB_TOKENauto-PRs do. - Link safety of the rename — no prose xref anywhere in
docs/modulesorcloud-docs/modulespoints at#iceberg_rest_catalog_aws_credentials_source. The only occurrences are the two shipped JSON attachments (redpanda-properties-v26.1.14.json,-v26.2.1.json) and the override file itself, so the rename breaks no hand-written links.
Suggestions
-
The
cloud-property-synclabel does not exist in this repo.gh label listshows onlyauto-docs(and unrelatedcloud separation), there's no label config file in.github/, and no other workflow references the new name. Whetherpeter-evans/create-pull-request@v6auto-creates it or fails the step isn't documented either way in GitHub's REST reference. Either way the failure mode is bad: the label is applied after the full clone-and-regen, so a first dispatch could burn the whole run and produce no PR.- Fix: create the label (with a colour and description) before the first dispatch. Cheap insurance, and better than an auto-created label with no description.
-
The
--regenerate-old-baselineinstruction can't be satisfied by this workflow. The flag does exist on docs-extensions-and-macrosmain(bin/doc-tools.js:894), but it's absent from the installed 5.3.1 tree, it only affects the--diffcode path, and this workflow never passes--diff. So "the first regen after #233 must use--regenerate-old-baseline" describes a manual local run, not anything the automation does.- Fix: say so explicitly in the description or a comment on the workflow, otherwise the next person will look for it in the workflow, not find it, and assume it was dropped. It also needs a version bump past the current pin to be available at all.
-
The sequencing caveat is already satisfied — the override deletion is safe to merge now. docs-extensions-and-macros#233 merged 2026-07-31T15:12Z and is an ancestor of
main(compare reportsbehind_by: 0); its name-resolution fix is present in the installed package, where theBasicInfoTransformerdocstring documents the new priority order (first string-literal parameter, falling back toname_in_file). 5.3.1 published 19:17Z the same day, andpackage-lock.jsonpins 5.3.2. So there's no window where the old-named property loses its description override — that risk closed on 2026-07-31. Worth updating the note so nobody holds the PR waiting on it. -
Confirm the
config_scope: clustermigration lands one entry, not two.iceberg_rest_catalog_credentials_sourcecurrently renders intopic-properties.adoc:440, betweenflush.msandinitial.retention.local.target.bytes— an underscore-named property among dotted topic properties, which reads like an existing misfiling. After the rename, the cluster property takes that same name, so the surviving override withconfig_scope: clusteris doing more than "nothing valuable is lost" implies: it decides where a name that currently appears in the topic file gets filed.- Fix: run one local regen before merging and confirm the property appears once, under cluster scope. This is the one change here whose outcome can't be determined by reading.
-
Cloud-sync and release runs aren't serialized against each other. The groups are
<workflow>-cloud-syncand<workflow>-<tag>, so both can run at once, each regenerating the same partials on a different branch. CI is fine; whichever PR merges second conflicts or clobbers. Worth a comment acknowledging it, or key both paths on the resolved tag. -
Cloud-sync PRs get a title identical to release PRs (
auto-docs: Update property docs for tag v26.2.1), since onlybranch,labels, andbodybranch inpr_meta. That defeats part of the point of distinguishing them — you can't tell which trigger fired from the PR list. Consider a distinct title for the cloud-sync path. -
Nit: the new bypass branch writes
>> $GITHUB_OUTPUTunquoted while the tag step you just hardened uses>> "$GITHUB_OUTPUT".
Impact on other files
- The property-docs review guide (docs-team-standards) documents the auto-docs flow as "Opens a PR titled
auto-docs: Update property docs for tag <version>". There's now a second trigger, a second label, and a long-lived fixed branch that behaves differently from the per-tag release branches. Reviewers triaging these PRs need that context. - The paired dispatch-sender PR is still a placeholder in the description. The
trigger-cloud-property-syncevent type and thecommit_shapayload key form a contract that can't be verified from this side until that PR exists. Worth landing them together, or at least confirming the sender uses those exact names. - No generated property partials touched, correctly — regen happens through the automation.
What works well
- Routing every dispatch-controlled value (
EVENT_ACTION,INPUT_TAG,PAYLOAD_TAG,CLOUD_COMMIT_SHA) throughenvinstead of direct interpolation is the right call on a workflow that accepts third-party dispatches, and the hex validation oncommit_shacloses the heredoc-terminator injection intoGITHUB_OUTPUTrather than just sanitising for display. - The tag charset check happens before the value reaches
GITHUB_OUTPUT, so downstream steps can't be fed a crafted tag. - The comments explain why rather than restating the YAML — the guard-bypass rationale and the pip cache key choice both pre-empt the obvious reviewer question.
- Failing loudly when
latest-redpanda-tagis missing, instead of regenerating against an empty tag.
2 participants
Implements the docs-repo side of the property automation fixes for DOC-1886 and DOC-1889.
Workflow: cloud-sync trigger and hardening (DOC-1886)
Changes to
.github/workflows/update-property-docs.yaml:trigger-cloud-property-syncdispatch event. When cloudv2 changes cloud property availability metadata, it can dispatch this event to regenerate the property docs without a new Redpanda release. This pairs with a cloudv2 workflow PR that sends the dispatch (placeholder — link will be added). The contract this side expects: event typetrigger-cloud-property-sync, optional payload keycommit_sha(hex, 7-40 chars). The rpai docs sender that went live 2026-08-03 uses the same repository-dispatch pattern from cloudv2, so the credential path is proven.latest-redpanda-taginantora.ymlinstead of the dispatch payload, and fails with a clear message if the attribute is missing. Regen always targets the currently published version.update-property-docs-cloud-sync) so repeat dispatches update one open PR instead of colliding with the release branch. Cloud-sync PRs get acloud-property-synclabel alongsideauto-docs, and the PR body includes the triggering cloudv2commit_shafor traceability (guarded when empty).cancel-in-progress: false).cache: npmonactions/setup-node, plus a pip cache (~/.cache/pip) keyed onpackage-lock.json(the pinned doc-tools version determines the Python requirements it installs at run time). The Redpanda clone is deliberately not cached.Override cleanup: iceberg credentials source (DOC-1889)
docs-data/property-overrides.jsonhad two entries for the same property:iceberg_rest_catalog_aws_credentials_sourceentry. It is keyed by the property's old name/alias. After fix(property-extractor): derive property names from the registered name literal, not the C++ member docs-extensions-and-macros#233 merges, that key matches nothing and would mint a phantom stub. Its description override is redundant because the accepted values render from the enum.iceberg_rest_catalog_credentials_sourceentry with itsrelated_topics, and migrated"config_scope": "cluster"from the deleted entry so nothing valuable is lost.Sequencing notes
The override deletion is sequenced with docs-extensions-and-macros#233.Resolved: #233 shipped in doc-tools 5.3.1 on 2026-07-31 and this repo's lockfile is past it, so the override deletion has no interim risk and does not need to wait.--regenerate-old-baselineapplies only to manual local--diffruns (it rebuilds a stale committed baseline). This workflow never passes--diff, so the flag is deliberately absent from it — a comment in the workflow now says so.No generated files are touched in this PR. Regen happens via the automation.
🤖 Generated with Claude Code