ci: notify on push failure of the plugin catalog workflow #52
Loading…
Add table
Add a link
Reference in a new issue
No description provided.
Delete branch "ci/notify-on-push-failure"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
What
Adds the estate-standard
notify-on-push-failurestep to.woodpecker/ci.yaml, the plugin catalog's test workflow. It runs onpushtomain, so until now a red catalog build onmainpaged nobody.Audit finding (cluster-gitops
ci-coverage-audit, census row cg#886)The census in cluster-gitops#886 lists ci-plugins under
no-notify-step. I reproduced the verdict with the audit's ownaudit_workflow()(scripts/ci-coverage-audit.pyat cluster-gitopsorigin/master, withci-coverage-inventory.yamlandapps-oss/ci/secret-inventory.yamlfrom the same commit), fed the local.woodpecker/*.yamlinstead of the Woodpecker API. Before:After this change:
Risk: R2 (CI path)
notify-on-push-failurestep shape used across argus and atlas (for exampleargus/.woodpecker/test.yaml,catalog-check.yaml):notify:v0.2.5@sha256:90a891d0…,topic: ci,ntfy_token: {from_secret: ntfy-token},when: event: push, status: [failure]. No new secret, plugin, or mechanism.ntfy-token(theci-notifyntfy user, write-only on topicci). The secret extension serves it only to the allow-listed notify digest and only forpush/manual/cron, and it refusespull_requestevents unconditionally. The step hassettings:only and noenvironment:orcommands:, so it stays a plugin step under Woodpecker'sIsPlugin()check.ntfy-tokenallow-list entry. A future notify bump in the inventory leaves both digests valid during the transition window. After that window this step must move with the rest of the fleet, or the audit reportsnotify-image-stale.mainafter merge. This PR'spull_requestpipeline skips it, so the first run that exercises the step is the merge push. On success the step is skipped. Check that the merge pipeline compiles and showsnotify-on-push-failureas skipped. If secret resolution fails, the push pipeline goeserror, and you revert.Validation
RESULT: PASS (0 finding(s)).woodpecker-cli lint .woodpecker/(woodpeckerci/woodpecker-cli:v3):ci.yamlandpr-review.yamlboth✅ Config is valid.shellcheck -S warningover the six entrypoints from theshellcheckstep: exit 0.bash plugins/notify/test/notify.test.sh:passed 172, failed 0.plugin-testsstep locally (Go, Python, and the other shell suites). The change doesn't touch them, and this PR's CI runs them.🤖 Generated with Claude Code
Review Findings
YAML syntax error (line 16): The
ntfy_tokensetting uses incorrect syntax. Should be:Current syntax
{from_secret: ntfy-token}is JSON-style and may cause parsing errors.Unquoted variable substitutions (lines 13-14): Consider quoting environment variables to prevent word splitting:
This is already correct, no change needed.
Image digest verification: The notify plugin uses a specific SHA256 digest which is good for security. Consider verifying this matches the expected v0.2.5 tag.
Minor: Step only runs on push failures - Confirm this is intentional. The step won't trigger on PR failures or other event types.
Critical issue: Fix the YAML syntax on line 16 before merging.
Thanks. I'm not changing anything; here's why for each point:
ntfy_token: {from_secret: ntfy-token}is a YAML flow mapping. It parses to the same structure as the block form, so it isn't a syntax error. The same form is used in every compliant sibling step (for exampleargus/.woodpecker/test.yaml) and in this repo'spr-review.yaml.woodpecker-cli lint .woodpecker/reportsci.yamlas✅ Config is valid, and this head'sci/woodpecker/pr/cipipeline compiled and passed.ntfy-tokenimage allow-list entry in cluster-gitopsapps-oss/ci/secret-inventory.yaml(notify:v0.2.5@sha256:90a891d0…). The secret extension serves the token only to that exact digest.ntfy-tokenonpull_requestevents, and ci-coverage-audit only requires failure notification for non-PR events (push/manual/tag/deployment/cron/release). This workflow's only non-PR trigger ispushtomain.