Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yml Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (2)
WalkthroughThe pinned-actions linter is updated with a path-aware pinning policy that enforces version requirements based on action type. Documentation and input definitions are revised accordingly, and validation logic is refactored to apply category-specific rules for internal vs external actions. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes 🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
Contributor
🔍 Lint Analysis
|
Contributor
🛡️ CodeQL Analysis ResultsLanguages analyzed: ✅ No security issues found. 🔍 View full scan logs | 🛡️ Security tab |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
github-actions
bot
deleted the
feat/pinned-actions-composite-reusable-split
branch
bedatty
added a commit
that referenced
this pull request
Apr 17, 2026
* feat(gitops-update): add manifest-driven cluster topology with anacleto support Introduce config/deployment-matrix.yaml as the single source of truth for which apps deploy to which clusters. The workflow now reads the manifest at the same pinned ref as itself (sparse checkout) and resolves the cluster set from app_name. Adds anacleto as the third deployment target. deploy_in_<cluster> inputs become force-off overrides — they subtract clusters from the manifest-resolved set but cannot add a cluster the manifest does not list. This prevents accidental cross-cluster spillover while still allowing emergency containment. Adds src/lint/deployment-matrix composite (Python embedded, follows the composite-schema pattern) that validates schema, app/cluster integrity, duplicates, and orphan apps. Wired into self-pr-validation as a gated job that only runs when config/deployment-matrix.yaml changes. The manifest topology was inferred empirically from the GitOps repo by cross-referencing folder presence with CI commit history — only apps that are real callers of this workflow are included (excludes apps managed manually like underwriter, jd-mock-api, mock-btg-server, control-plane, platform-console, ledger, dockerhub-secret). * fix(ci): pin actions/checkout to SHA and replace sed with bash substitution Address PR #212 lint failures: 1. Pin all `actions/checkout@v6` occurrences in self-pr-validation.yml to the SHA already used in gitops-update.yml. Required by pinned-actions lint for external (non-LerianStudio) actions. Also clears pre-existing tech debt in this file that surfaced because the new deployment-matrix job touched it. 2. Replace `echo "$RESOLVED" | sed 's/^/ - /'` with a bash `while read` loop in the resolve_clusters step. Fixes shellcheck SC2001 (prefer bash parameter expansion over sed for simple substitutions). * fix(security): address CodeQL findings on PR #212 Resolves 6 medium-severity findings from github-advanced-security: CODE INJECTION (4 findings — actions/code-injection/medium): - Move `${{ github.workflow_ref }}` to step env: WORKFLOW_REF - Bonus: replace `echo | sed -E 's|.*@||'` with bash `${VAR##*@}` - Eliminates injection vectors at lines 106 + 108 - Move resolve_clusters outputs (has_clusters, clusters) to step env: HAS_CLUSTERS + RESOLVED_SERVERS in apply_tags step - Move inputs.yaml_key_mappings + inputs.configmap_updates to step env: MAPPINGS + CONFIGMAP_MAPPINGS - Replace `${{ env.IS_BETA/RC/PRODUCTION/SANDBOX }}` with direct `$IS_BETA/...` (already in job-level env, no need to re-interpolate) - Replace `${{ github.ref }}` with `${GITHUB_REF}` (auto-set by runner) UNTRUSTED CHECKOUT (2 findings — actions/untrusted-checkout/medium): - Add `persist-credentials: false` to manifest sparse checkout (read-only, no credentials needed, never executes code from this checkout) - Document trust model inline for the GitOps repo checkout (workflow_call is not triggered by untrusted PRs; inputs.gitops_repository comes from trusted internal callers; MANAGE_TOKEN is required for the subsequent commit/push step, so we cannot drop persist-credentials there) * fix(gitops-update): address CodeRabbit review findings 1. [CRITICAL] Replace `github.workflow_ref` with `github.job_workflow_sha` for manifest checkout. In reusable workflows, github.workflow_ref points to the CALLER's workflow file/ref, not the called reusable workflow — my previous design would have failed for every external caller. `job_workflow_sha` is the commit SHA of the running reusable workflow, which is exactly what we need. Bonus: SHA is more secure than textual ref, and removes the need for the `Resolve shared-workflows ref` step entirely (−18 lines). 2. [HIGH] Remove `|| true` from the RESOLVED pipeline. Silenced yq/jq failures would collapse into the "app not registered" warning path, hiding real manifest/query errors. Now fails fast on parse errors; empty RESOLVED from a successful query remains the legitimate "no matching clusters" case (handled explicitly below). 3. [MEDIUM] Rename config/deployment-matrix.yaml → .yml to match the repo convention (77 .yml files vs 2 .yaml). Updated all references: workflow input default, self-pr-validation gate, composite default, README docs, and the workflow doc. 4. [LOW] Add prominent migration callout to docs about deploy_in_* semantic change — apps must be in the manifest; inputs only subtract. Declined: per-cluster warning when deploy_in_<cluster>: true but app is absent from that cluster's manifest list. Inputs default to true, so this would fire for every app missing from any cluster on every run — noise without signal. Existing "app in zero clusters" warning already covers the actionable case. * fix(ci): work around actionlint schema gap for job_workflow_sha actionlint v1.7.x (pinned via raven-actions/actionlint@v2.1.2) does not yet include `github.job_workflow_sha` in its GitHub context schema, triggering a false-positive "property not defined" error on the previous direct reference. Replace the inline `${{ github.job_workflow_sha }}` expression with an intermediate step that reads the equivalent auto-set env var GITHUB_JOB_WORKFLOW_SHA and exports it as a step output. Functionally identical (the runner populates both from the same source) but the `steps.X.outputs.Y` expression is recognized by actionlint. Also adds a defensive guard that fails fast if GITHUB_JOB_WORKFLOW_SHA is empty — which would mean the workflow is being called outside a reusable-workflow context, catching that misconfiguration loudly. * fix(gitops-update): map github.job_workflow_sha via env: instead of assuming auto env var GITHUB_JOB_WORKFLOW_SHA is not exposed automatically by the runner. The github.job_workflow_sha context must be mapped explicitly through the step's env: block like any other context value. Prior implementation relied on a nonexistent auto env var and failed with 'is this job really running as part of a reusable workflow?' on every execution. Validated against real run: https://github.com/LerianStudio/plugin-br-pix-indirect-btg/actions/runs/24458387402/job/71466177318 * fix(gitops-update): hardcode manifest ref for PR#212 testing Drops the 'Resolve reusable workflow SHA' step entirely — github.job_workflow_sha is empty when evaluated inside a job of a reusable workflow invoked via jobs.X.uses (empirically confirmed on run 24461037331). Three prior attempts to source that SHA all failed for different reasons: - parsing github.workflow_ref: points to the caller, not the reusable - GITHUB_JOB_WORKFLOW_SHA env var: does not exist - github.job_workflow_sha context: empty in this evaluation context This commit is a TEMP workaround for end-to-end validation: manifest checkout is hardcoded to the feature branch. Before merging #212 this will be replaced with a proper 'deployment_matrix_ref' input (default 'main'). * fix(gitops-update): inline argocd sync with visible stderr instead of external action The LerianStudio/github-actions-argocd-sync action suppresses stderr via '> /dev/null 2>&1' on every CLI invocation. Any failure (auth, permission, network, malformed URL, expired token) is rendered indistinguishable from 'app does not exist' and skipped silently when skip-if-not-exists=true. Replaces the external action with inline argocd CLI calls that surface the real error output. Preserves the skip-if-not-exists semantics (warn + exit 0 on app get failure), but syncs fail the job loudly. * test(gitops-matrix): remove plugin-br-pix-indirect-btg from clotilde to validate resolution Temporary change for end-to-end testing of the manifest-driven gitops pipeline on PR #212. Expected behavior on next beta of plugin-br-pix-indirect-btg: - resolve_clusters: {firmino, anacleto} (clotilde dropped) - values.yaml updated only in firmino/dev and anacleto/dev - argocd_sync fan-out: 2 jobs (firmino-*-dev, anacleto-*-dev) Revert this commit before merging #212. * refactor(gitops-update): replace hardcoded manifest ref with input and restore matrix - Adds deployment_matrix_ref input (default 'main'). Callers on pinned tags get the latest manifest automatically; test runs can override via the input without editing the workflow. - Drops the temporary hardcoded ref to the feature branch. - Restores plugin-br-pix-indirect-btg in the clotilde cluster (removed temporarily during exclusion-validation test). End-to-end validation completed against plugin-br-pix-indirect-btg: - v1.5.2-beta.9: full fan-out to firmino + clotilde + anacleto, sync OK - v1.5.2-beta.10: manifest exclusion respected (firmino + anacleto only) * feat(deployment-matrix): add label and skip release on matrix-only changes - New 'deployment-matrix' label auto-applied by the labeler on PRs that touch config/deployment-matrix.yml. - config/deployment-matrix.yml added to self-release.yml paths-ignore: since callers resolve the manifest from main at runtime (via the deployment_matrix_ref input with default 'main'), matrix-only changes propagate to all callers without requiring a new release tag. - Mixed commits that touch the matrix plus workflow/action code still trigger a release as usual. * docs(gitops-update): document deployment_matrix_ref input and main-default resolution Addresses CodeRabbit feedback on PR #221. The workflow no longer checks out the manifest at the same ref as itself — it defaults to 'main' (via the deployment_matrix_ref input) so manifest updates propagate to every caller without bumping the pinned workflow tag. - Lead paragraph: replace 'same pinned ref' description. - Optional inputs table: add deployment_matrix_ref row. - 'How it works' step 2: rewrite to reflect the new behavior and rationale. * fix(gptchangelog): resolve contributors via github api instead of email local-part * feat(self-release): generate changelog after stable release on main * fix(pinned-actions): enforce composite vs reusable pinning policy (#231) * fix(codeql-reporter): filter dismissed and fixed alerts from PR comment * fix(self-release): force-update floating major tag on stable release (#230) * feat(self-release): force-update floating major tag on stable release * refactor(update-major-tag): extract major-tag logic into composite * feat(update-major-tag): expose skip and tag-updated outputs * fix(update-major-tag): qualify tag refs to avoid branch/tag ambiguity * fix(actions): rename deprecated app-id input to client-id (#228) * chore(go-pr-analysis): resolve lint debt (SHA-pin, trailing spaces, shellcheck)
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
Extends the
pinned-actionscomposite linter to materialize therepository-wide pinning policy:
warn-patterns/src/@vNfloating major (or@develop/@mainfor testing)/.github/workflows/@vN.M.P[-pre]exact pin (or@develop/@mainfor testing)warn-patternsbut neither path patternChanges
src/lint/pinned-actions/action.yml: split the internal-org branch intocomposite vs reusable-workflow logic. Each emits a distinct, actionable
warning pointing at the expected pattern. External SHA enforcement is
unchanged.
src/lint/pinned-actions/README.md: rewrite to document the policy,fix the outdated
ignore-patternsinput (actual input iswarn-patterns), and add correct/incorrect examples.Rationale
Composites are small building blocks —
@v1lets callers track thelatest stable major without bumping per patch (the major tag is moved
atomically by
self-release.ymlon each stable release — see #230).Reusable workflows orchestrate entire pipelines; exact pins force
explicit, auditable caller updates.
Behavior on existing callers
Non-breaking. Callers currently using
@v1.24.1for internal compositeswill start seeing a warning line in
self-pr-validation.ymloutput, butthe step does not fail. The bulk migration to
@v1is scoped to afollow-up PR.
Type of Change
feat: New workflow or new input/output/step in an existing workflowfix: Bug fix in a workflow (incorrect behavior, broken step, wrong condition)perf: Performance improvement (e.g. caching, parallelism, reduced steps)refactor: Internal restructuring with no behavior changedocs: Documentation only (README, docs/, inline comments)ci: Changes to self-CI (workflows under.github/workflows/that run on this repo)chore: Dependency bumps, config updates, maintenancetest: Adding or updating testsBREAKING CHANGE: Callers must update their configuration after this PRBreaking Changes
None. Inputs (
files,warn-patterns) and outputs are unchanged; externalenforcement is unchanged; internal policy moves from a single regex to a
split regex but still at WARNING severity — existing CI pipelines do not
fail.
Testing
(composite OK / composite WARN / reusable OK / reusable WARN with
branches, exact versions, major-only, and pre-release tags)
@developor the beta tagCaller repo / workflow run: Will be validated on the next push to
this branch via
self-pr-validation.yml→pinned-actionsjob. Existing@v1.24.1refs for internal composites will produce (non-blocking) warnings.Related Issues
Closes #
Summary by CodeRabbit
Release Notes
Documentation
filesformat now comma-separated;warn-patternsreplacesignore-patternsValidation Updates