Repository navigation
feat(gitops-update): support per-cluster env suffix variants (-st / -mt) - #423
Conversation
Adds two additive, backward-compatible fields to the deployment matrix
manifest to express clusters that host multiple parallel variants per
environment (e.g. single-tenant -st + multi-tenant -mt on benedita):
clusters:
benedita:
env_suffixes: ["-st", "-mt"]
suffix_excludes_envs: ["sandbox"]
The gitops-update workflow expands each tag-derived env (dev/stg/prd/sandbox)
against the cluster's env_suffixes, except envs listed in suffix_excludes_envs
which stay bare. The expanded env name flows through unchanged to the helmfile
path resolution and the ArgoCD app name template, so 'benedita-midaz-dev-st'
resolves naturally without any new workflow inputs.
Cross apps (those with app_helmfile_env set, e.g. forge: cross) bypass the
suffix expansion - they update once at the override path and their sync target
uses the override env, so 'benedita-forge-cross' resolves correctly when the
cluster renames its ArgoCD apps to follow the {server}-{app}-{env} convention.
Defaults preserve byte-identical behavior for firmino/clotilde/anacleto:
env_suffixes defaults to [""] (single empty-suffix expansion), suffix_excludes_envs
defaults to []. Tag-derived env list for those clusters is unchanged.
Also extends the deployment-matrix lint action to validate the new fields
plus the pre-existing app_helmfile_env field (which was previously unvalidated).
Closes #422
|
Warning Review limit reached
More reviews will be available in 32 minutes and 9 seconds. Learn how PR review limits work. To continue reviewing without waiting, enable usage-based billing in the billing tab. ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based credits. 🚦 How do rate limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yml Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (3)
WalkthroughThe PR adds per-cluster environment suffix and context handling to GitOps deployment matrix resolution, updates workflow inputs and runtime sync behavior, and refreshes the matching configuration and documentation. It also removes the deprecated Firmino/Clotilde override inputs. ChangesEnvironment suffix variants and context directories
🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested labels
Suggested reviewers
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
lerian-studio
added
size/M
🛡️ CodeQL Analysis ResultsLanguages analyzed: Found 2 issue(s): 2 Medium
🔍 View full scan logs | 🛡️ Security tab |
🔍 PR Validation Summary✅ PR Mergeable — no blocking failures
|
🔍 Lint Analysis
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Warning
CodeRabbit couldn't request changes on this pull request because it doesn't have sufficient GitHub permissions.
Please grant CodeRabbit Pull requests: Read and write permission and re-run the review.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
docs/gitops-update-workflow.md (1)
1-163:⚠️ Potential issue | 🟠 MajorAdd Lerian branding header to file start.
All
docs/*.mdfiles must begin with the Lerian HTML header block per repository conventions:<table border="0"><tr><td><img src="https://github.com/LerianStudio.png" width="64" height="64"/></td><td><h1>GitOps Update Workflow</h1></td></tr></table>Currently the file starts with a markdown heading instead.
🤖 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 `@docs/gitops-update-workflow.md` around lines 1 - 163, Add the Lerian branding header block at the very beginning of the gitops-update-workflow.md file, before the current markdown heading. Insert an HTML table containing the Lerian Studio logo image (from https://github.com/LerianStudio.png with dimensions 64x64) and the GitOps Update Workflow title. This header block should follow the repository conventions for all docs/*.md files and precede the existing markdown content.
🤖 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/gitops-update.yml:
- Around line 565-568: The early return statement triggered by the `-z
"$suffixes"` condition is exiting the function before context-based environment
expansion occurs. This causes the function to skip generating context-specific
target environments (e.g., `{context}/...` patterns) even when context expansion
is required. Restructure the logic so that the context expansion logic executes
regardless of whether suffixes is empty, either by moving the context expansion
before the early return check or by removing the early return and letting the
function continue through the context expansion logic when suffixes is empty.
---
Outside diff comments:
In `@docs/gitops-update-workflow.md`:
- Around line 1-163: Add the Lerian branding header block at the very beginning
of the gitops-update-workflow.md file, before the current markdown heading.
Insert an HTML table containing the Lerian Studio logo image (from
https://github.com/LerianStudio.png with dimensions 64x64) and the GitOps Update
Workflow title. This header block should follow the repository conventions for
all docs/*.md files and precede the existing markdown content.
🪄 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: Path: .coderabbit.yml
Review profile: ASSERTIVE
Plan: Pro
Run ID: da93ccc9-3fd6-4bf8-be2a-e188e05d741d
📒 Files selected for processing (5)
.github/workflows/gitops-update.yml.github/workflows/go-release.ymlconfig/deployment-matrix.ymldocs/gitops-update-workflow.mdsrc/lint/deployment-matrix/action.yml
There was a problem hiding this comment.
Actionable comments posted: 5
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
.github/workflows/gitops-update.yml (1)
247-253: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winTreat removed force-off inputs as a caller-breaking API change.
deploy_in_firminoanddeploy_in_clotildewere removed from the reusable workflow interface and no longer wired here; existing callers that still pass them will fail before this job starts. If this is not being released as a breaking version, keep the inputs as deprecated no-ops or provide a compatibility path. As per path instructions, “Flag any removed or renamed input/output as a breaking change for callers.”Also applies to: 296-309
🤖 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/gitops-update.yml around lines 247 - 253, The inputs `deploy_in_firmino` and `deploy_in_clotilde` have been removed from the workflow interface, causing existing callers that still pass these inputs to fail. To maintain backward compatibility, either re-add these inputs to the workflow's input definitions but mark them as deprecated with a note that they are no longer used, or implement a compatibility path that gracefully handles the removal without breaking existing callers. This ensures a smoother migration path rather than a hard breaking change.Source: Path instructions
🤖 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/go-release.yml:
- Line 144: The workflow inputs deploy_in_firmino and deploy_in_clotilde have
been removed from the workflow_call inputs section, which breaks the contract
for any external callers still providing these inputs. To maintain backward
compatibility during a transition period, re-add deploy_in_firmino and
deploy_in_clotilde as inputs to workflow_call with type boolean, mark them as
deprecated through documentation or comments, and ensure they are accepted but
not used (no-op). Alternatively, if this is an intentional breaking change,
explicitly communicate it in release notes and version bumps rather than
silently removing the inputs.
- Line 196: The AWS_MIGRATIONS_ROLE_ARN is declared as an optional input but is
being used unconditionally in the Configure AWS credentials step, causing
failures when the secret is not provided during beta/rc/prod tag deployments.
Either mark AWS_MIGRATIONS_ROLE_ARN as required: true in the input declaration,
or add an if condition to the Configure AWS credentials step that checks whether
the AWS_MIGRATIONS_ROLE_ARN secret is present before executing the step. Choose
the approach that best fits your deployment requirements for when credential
configuration should be skipped.
- Line 432: The `enable_gitops_artifacts` field on line 432 uses the `|| true`
pattern which incorrectly coerces explicit false values to true, preventing
callers from disabling this feature. Replace the `|| true` pattern with the
correct default pattern already used on line 436 for `normalize_to_filter`,
which properly allows explicit false values to be respected. Apply the same fix
to line 444 for `helm_detect_env_changes` which has the identical issue.
In `@config/deployment-matrix.yml`:
- Around line 119-125: The comment in the Benedita configuration contradicts the
actual suffix expansion behavior. The configuration defines env_suffixes:
["-st", "-mt"] which applies the "-mt" suffix to all non-excluded base
environments (dev, stg, prd), meaning dev-mt and prd-mt are actually generated
by the current schema, not excluded. Either update the comment to correctly
state that dev-mt, stg-mt, and prd-mt are all generated by the current
configuration, or redesign the suffix_excludes_envs logic to support
per-environment suffix restrictions (e.g., excluding "-mt" only from dev and prd
while allowing it for stg) if dev-mt and prd-mt should not actually exist.
In `@docs/gitops-update-workflow.md`:
- Line 329: The description of the `<server>` parameter in the
gitops-update-workflow.md file hardcodes specific cluster names (anacleto and
benedita) which will become outdated as the deployment manifest changes. Remove
the hardcoded cluster names from this description and instead make it reference
the dynamic cluster resolution from the deployment matrix, ensuring it aligns
with the manifest-driven approach described in later examples that resolve
clusters from config/deployment-matrix.yml. The description should indicate that
servers come from the deployment matrix with optional filtering, rather than
listing specific cluster instances.
---
Outside diff comments:
In @.github/workflows/gitops-update.yml:
- Around line 247-253: The inputs `deploy_in_firmino` and `deploy_in_clotilde`
have been removed from the workflow interface, causing existing callers that
still pass these inputs to fail. To maintain backward compatibility, either
re-add these inputs to the workflow's input definitions but mark them as
deprecated with a note that they are no longer used, or implement a
compatibility path that gracefully handles the removal without breaking existing
callers. This ensures a smoother migration path rather than a hard breaking
change.
🪄 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: Path: .coderabbit.yml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 2e384cdd-ad8e-4eb2-bb86-dd40e0e60b29
📒 Files selected for processing (4)
.github/workflows/gitops-update.yml.github/workflows/go-release.ymlconfig/deployment-matrix.ymldocs/gitops-update-workflow.md
There was a problem hiding this comment.
Caution
Inline review comments failed to post. This is likely due to GitHub's internal server error or limits when posting large numbers of comments. If you are seeing this consistently it is likely a permissions issue. Please check "Moderation" -> "Code review limits" under your organization settings.
Actionable comments posted: 5
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
.github/workflows/gitops-update.yml (1)
247-253: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winTreat removed force-off inputs as a caller-breaking API change.
deploy_in_firminoanddeploy_in_clotildewere removed from the reusable workflow interface and no longer wired here; existing callers that still pass them will fail before this job starts. If this is not being released as a breaking version, keep the inputs as deprecated no-ops or provide a compatibility path. As per path instructions, “Flag any removed or renamed input/output as a breaking change for callers.”Also applies to: 296-309
🤖 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/gitops-update.yml around lines 247 - 253, The inputs `deploy_in_firmino` and `deploy_in_clotilde` have been removed from the workflow interface, causing existing callers that still pass these inputs to fail. To maintain backward compatibility, either re-add these inputs to the workflow's input definitions but mark them as deprecated with a note that they are no longer used, or implement a compatibility path that gracefully handles the removal without breaking existing callers. This ensures a smoother migration path rather than a hard breaking change.Source: Path instructions
🤖 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/go-release.yml:
- Line 144: The workflow inputs deploy_in_firmino and deploy_in_clotilde have
been removed from the workflow_call inputs section, which breaks the contract
for any external callers still providing these inputs. To maintain backward
compatibility during a transition period, re-add deploy_in_firmino and
deploy_in_clotilde as inputs to workflow_call with type boolean, mark them as
deprecated through documentation or comments, and ensure they are accepted but
not used (no-op). Alternatively, if this is an intentional breaking change,
explicitly communicate it in release notes and version bumps rather than
silently removing the inputs.
- Line 196: The AWS_MIGRATIONS_ROLE_ARN is declared as an optional input but is
being used unconditionally in the Configure AWS credentials step, causing
failures when the secret is not provided during beta/rc/prod tag deployments.
Either mark AWS_MIGRATIONS_ROLE_ARN as required: true in the input declaration,
or add an if condition to the Configure AWS credentials step that checks whether
the AWS_MIGRATIONS_ROLE_ARN secret is present before executing the step. Choose
the approach that best fits your deployment requirements for when credential
configuration should be skipped.
- Line 432: The `enable_gitops_artifacts` field on line 432 uses the `|| true`
pattern which incorrectly coerces explicit false values to true, preventing
callers from disabling this feature. Replace the `|| true` pattern with the
correct default pattern already used on line 436 for `normalize_to_filter`,
which properly allows explicit false values to be respected. Apply the same fix
to line 444 for `helm_detect_env_changes` which has the identical issue.
In `@config/deployment-matrix.yml`:
- Around line 119-125: The comment in the Benedita configuration contradicts the
actual suffix expansion behavior. The configuration defines env_suffixes:
["-st", "-mt"] which applies the "-mt" suffix to all non-excluded base
environments (dev, stg, prd), meaning dev-mt and prd-mt are actually generated
by the current schema, not excluded. Either update the comment to correctly
state that dev-mt, stg-mt, and prd-mt are all generated by the current
configuration, or redesign the suffix_excludes_envs logic to support
per-environment suffix restrictions (e.g., excluding "-mt" only from dev and prd
while allowing it for stg) if dev-mt and prd-mt should not actually exist.
In `@docs/gitops-update-workflow.md`:
- Line 329: The description of the `<server>` parameter in the
gitops-update-workflow.md file hardcodes specific cluster names (anacleto and
benedita) which will become outdated as the deployment manifest changes. Remove
the hardcoded cluster names from this description and instead make it reference
the dynamic cluster resolution from the deployment matrix, ensuring it aligns
with the manifest-driven approach described in later examples that resolve
clusters from config/deployment-matrix.yml. The description should indicate that
servers come from the deployment matrix with optional filtering, rather than
listing specific cluster instances.
---
Outside diff comments:
In @.github/workflows/gitops-update.yml:
- Around line 247-253: The inputs `deploy_in_firmino` and `deploy_in_clotilde`
have been removed from the workflow interface, causing existing callers that
still pass these inputs to fail. To maintain backward compatibility, either
re-add these inputs to the workflow's input definitions but mark them as
deprecated with a note that they are no longer used, or implement a
compatibility path that gracefully handles the removal without breaking existing
callers. This ensures a smoother migration path rather than a hard breaking
change.
🪄 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: Path: .coderabbit.yml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 2e384cdd-ad8e-4eb2-bb86-dd40e0e60b29
📒 Files selected for processing (4)
.github/workflows/gitops-update.yml.github/workflows/go-release.ymlconfig/deployment-matrix.ymldocs/gitops-update-workflow.md
🛑 Comments failed to post (5)
.github/workflows/go-release.yml (3)
144-144: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Breaking caller contract by removing workflow inputs
Line 144 removes
deploy_in_firminoanddeploy_in_clotildefromworkflow_call. Callers still sending these keys will fail validation at dispatch time. Keep deprecated no-op inputs for a transition window or ship this only with explicit breaking-change release signaling.As per path instructions, “Flag any removed or renamed input/output as a breaking change for callers.”
🤖 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/go-release.yml at line 144, The workflow inputs deploy_in_firmino and deploy_in_clotilde have been removed from the workflow_call inputs section, which breaks the contract for any external callers still providing these inputs. To maintain backward compatibility during a transition period, re-add deploy_in_firmino and deploy_in_clotilde as inputs to workflow_call with type boolean, mark them as deprecated through documentation or comments, and ensure they are accepted but not used (no-op). Alternatively, if this is an intentional breaking change, explicitly communicate it in release notes and version bumps rather than silently removing the inputs.Source: Path instructions
196-196: 🩺 Stability & Availability | 🟠 Major
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash set -euo pipefail FILE=".github/workflows/go-release.yml" echo "== workflow_call.secrets block ==" sed -n '190,206p' "$FILE" echo echo "== S3 credential bootstrap step ==" sed -n '308,320p' "$FILE"Repository: LerianStudio/github-actions-shared-workflows
Length of output: 1280
🏁 Script executed:
#!/bin/bash set -euo pipefail FILE=".github/workflows/go-release.yml" echo "== Full secrets block (190-215) ==" sed -n '190,215p' "$FILE" echo echo "== S3 upload job context (295-330) ==" sed -n '295,330p' "$FILE" echo echo "== Input declarations for s3_uploads ==" sed -n '1,100p' "$FILE" | grep -A 20 's3_uploads' || echo "No s3_uploads in first 100 lines, searching broader..." rg 's3_uploads' "$FILE" -B 2 -A 10 --no-heading | head -50Repository: LerianStudio/github-actions-shared-workflows
Length of output: 4570
🏁 Script executed:
#!/bin/bash set -euo pipefail FILE=".github/workflows/go-release.yml" echo "== Looking for s3-upload composite reference ==" grep -n "s3-upload" "$FILE" || echo "No direct reference found" echo echo "== Check if src/ has composite actions ==" find . -name "action.yml" -o -name "action.yaml" | head -20 echo echo "== If s3-upload composite exists ==" find . -path "*/s3-upload/*" -o -path "*/s3*upload*" 2>/dev/null | grep -E "\.(yml|yaml)$" | head -10Repository: LerianStudio/github-actions-shared-workflows
Length of output: 1617
🏁 Script executed:
#!/bin/bash set -euo pipefail echo "== s3-upload.yml workflow (first 100 lines) ==" sed -n '1,100p' ".github/workflows/s3-upload.yml" echo echo "== Search for aws_role_arn or role-arn in s3-upload.yml ==" grep -n "role-arn\|aws_role\|AWS_ROLE" ".github/workflows/s3-upload.yml" || echo "No aws_role_arn field found" echo echo "== Full inputs section of s3-upload.yml ==" sed -n '/^ inputs:/,/^ /p' ".github/workflows/s3-upload.yml" | head -80Repository: LerianStudio/github-actions-shared-workflows
Length of output: 4168
🏁 Script executed:
#!/bin/bash set -euo pipefail FILE=".github/workflows/go-release.yml" echo "== Full Configure AWS credentials step (line 312-317) ==" sed -n '312,325p' "$FILE" echo echo "== Check step condition and surrounding context ==" sed -n '305,330p' "$FILE" echo echo "== Verify AWS_MIGRATIONS_ROLE_ARN secret declaration ==" sed -n '190,210p' "$FILE" | grep -A 1 "AWS_MIGRATIONS_ROLE_ARN"Repository: LerianStudio/github-actions-shared-workflows
Length of output: 1946
Make
AWS_MIGRATIONS_ROLE_ARNsecret required or guard credential setup on secret presence
AWS_MIGRATIONS_ROLE_ARNis declared as optional (line 196) but unconditionally referenced in theConfigure AWS credentialsstep (line 316). When a tag matches an environment pattern (beta/rc/prod), the step runs and fails if the secret is not provided. Either mark the secretrequired: trueor add an explicit condition to skip credential setup when the secret is absent.🤖 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/go-release.yml at line 196, The AWS_MIGRATIONS_ROLE_ARN is declared as an optional input but is being used unconditionally in the Configure AWS credentials step, causing failures when the secret is not provided during beta/rc/prod tag deployments. Either mark AWS_MIGRATIONS_ROLE_ARN as required: true in the input declaration, or add an if condition to the Configure AWS credentials step that checks whether the AWS_MIGRATIONS_ROLE_ARN secret is present before executing the step. Choose the approach that best fits your deployment requirements for when credential configuration should be skipped.
432-432: 🎯 Functional Correctness | 🟠 Major
🧩 Analysis chain
🌐 Web query:
In GitHub Actions expression syntax, what does${{ false || true }}evaluate to, and what pattern preserves explicitfalsewhile defaulting undefined totrue?💡 Result:
${{ false || true }} evaluates to true. Reason: In GitHub Actions expressions, the logical OR operator (||) returns the value that corresponds to the first truthy operand; since the left operand is
false(falsy) and the right operand istrue(truthy), the overall expression evaluates totrue[1][2]. Pattern to preserve explicit false while defaulting undefined to true: Useinputs.<name> || trueis NOT safe for preserving an explicit false, becausefalseis falsy and would be replaced bytrue. Instead, use the “explicit false” pattern based on ternary-like short-circuiting:${{ inputs.<name> == false && false || true }}Explanation: If the input is explicitlyfalse, theinputs.<name> == falsepredicate is true, so the left branch yieldsfalse; otherwise (includingtrueandundefined), it yieldstrue[2].Citations:
- 1: https://docs.github.com/en/enterprise-server@3.12/actions/learn-github-actions/expressions
- 2: https://docs.github.com/actions/reference/evaluate-expressions-in-workflows-and-actions
🏁 Script executed:
# First, locate and inspect the workflow file fd -name "go-release.yml" .github/workflows/ -x cat -n {} | head -100Repository: LerianStudio/github-actions-shared-workflows
Length of output: 413
🏁 Script executed:
# Search for the problematic pattern around lines 432 and 444 fd -name "go-release.yml" .github/workflows/ -x sed -n '425,450p' {}Repository: LerianStudio/github-actions-shared-workflows
Length of output: 413
🏁 Script executed:
# Check if there are other instances of the same problematic pattern in the file fd -name "go-release.yml" .github/workflows/ -x rg "|| true" {} -nRepository: LerianStudio/github-actions-shared-workflows
Length of output: 413
🏁 Script executed:
# Locate the workflow file find .github/workflows -name "go-release.yml"Repository: LerianStudio/github-actions-shared-workflows
Length of output: 215
🏁 Script executed:
# Inspect lines around 432 and 444 sed -n '425,450p' .github/workflows/go-release.ymlRepository: LerianStudio/github-actions-shared-workflows
Length of output: 1627
🏁 Script executed:
# Search for the || true pattern grep -n "|| true" .github/workflows/go-release.ymlRepository: LerianStudio/github-actions-shared-workflows
Length of output: 356
🏁 Script executed:
# Check the full context around those lines and see if there are other similar patterns sed -n '420,460p' .github/workflows/go-release.yml | cat -nRepository: LerianStudio/github-actions-shared-workflows
Length of output: 2456
🏁 Script executed:
# Search for all instances of the pattern used on line 435 (toJSON comparison) grep -n "toJSON.*== 'false'" .github/workflows/go-release.ymlRepository: LerianStudio/github-actions-shared-workflows
Length of output: 293
🏁 Script executed:
# Search for all || true and || false patterns grep -n "||" .github/workflows/go-release.yml | head -20Repository: LerianStudio/github-actions-shared-workflows
Length of output: 1597
Fix boolean defaults to allow explicit
falseon lines 432 and 444The
|| truepattern coerces explicitfalsetotrue, preventing callers from disablingenable_gitops_artifactsandhelm_detect_env_changes. The file already uses the correct pattern on line 436 (normalize_to_filter); apply it consistently here.Required changes
- enable_gitops_artifacts: ${{ matrix.group.enable_gitops_artifacts || true }} + enable_gitops_artifacts: ${{ toJSON(matrix.group.enable_gitops_artifacts) == 'false' && 'false' || 'true' }}- helm_detect_env_changes: ${{ matrix.group.helm_detect_env_changes || true }} + helm_detect_env_changes: ${{ toJSON(matrix.group.helm_detect_env_changes) == 'false' && 'false' || 'true' }}🤖 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/go-release.yml at line 432, The `enable_gitops_artifacts` field on line 432 uses the `|| true` pattern which incorrectly coerces explicit false values to true, preventing callers from disabling this feature. Replace the `|| true` pattern with the correct default pattern already used on line 436 for `normalize_to_filter`, which properly allows explicit false values to be respected. Apply the same fix to line 444 for `helm_detect_env_changes` which has the identical issue.config/deployment-matrix.yml (1)
119-125: 🎯 Functional Correctness | 🟠 Major
🧩 Analysis chain
🏁 Script executed:
# First, let's read the actual file to see the context around lines 119-125 cat -n config/deployment-matrix.yml | sed -n '110,135p'Repository: LerianStudio/github-actions-shared-workflows
Length of output: 1596
🏁 Script executed:
# Also check the overall structure of the file to understand the benedita section ast-grep outline config/deployment-matrix.yml --view expandedRepository: LerianStudio/github-actions-shared-workflows
Length of output: 225
🏁 Script executed:
# Search for how env_suffixes and suffix_excludes_envs are used elsewhere rg "env_suffixes|suffix_excludes_envs" -A 2 -B 2Repository: LerianStudio/github-actions-shared-workflows
Length of output: 182
🏁 Script executed:
# Look for references to dev-mt, prd-mt, stg-mt in the repository rg "dev-mt|prd-mt|stg-mt" --type yml --type yamlRepository: LerianStudio/github-actions-shared-workflows
Length of output: 210
🏁 Script executed:
# Search for files that reference env_suffixes or deployment-matrix rg "env_suffixes|deployment-matrix" -lRepository: LerianStudio/github-actions-shared-workflows
Length of output: 182
🏁 Script executed:
# Look for workflow files that might process the benedita configuration fd "\.ya?ml$" --type f | head -20Repository: LerianStudio/github-actions-shared-workflows
Length of output: 915
🏁 Script executed:
# Search for references to benedita and how it's used rg "benedita" --type yaml --type yml -B 2 -A 2Repository: LerianStudio/github-actions-shared-workflows
Length of output: 210
🏁 Script executed:
# Check if there's any code that processes the deployment matrix rg "dev-mt|prd-mt" -rRepository: LerianStudio/github-actions-shared-workflows
Length of output: 596
🏁 Script executed:
# Search for deployment-matrix usage in shell/JavaScript files rg "deployment-matrix" -A 3 -B 3Repository: LerianStudio/github-actions-shared-workflows
Length of output: 182
🏁 Script executed:
# Look for any scripts that process environment suffixes rg "env_suffix" -A 2 -B 2Repository: LerianStudio/github-actions-shared-workflows
Length of output: 182
🏁 Script executed:
# Check if there's any conditional logic for different environments in the codebase rg "dev-st|stg-st|prd-st" -A 2 -B 2Repository: LerianStudio/github-actions-shared-workflows
Length of output: 182
🏁 Script executed:
# Look at the full benedita section to understand the context better sed -n '119,200p' config/deployment-matrix.ymlRepository: LerianStudio/github-actions-shared-workflows
Length of output: 1967
🏁 Script executed:
# Search for any references to mt (multi-tenant) across the entire repo rg "\-mt" | head -30Repository: LerianStudio/github-actions-shared-workflows
Length of output: 182
🏁 Script executed:
# Check the log from the -mt search to see if there were any results tail -100 /tmp/coderabbit-shell-logs/shell-output-2yRspnRepository: LerianStudio/github-actions-shared-workflows
Length of output: 182
🏁 Script executed:
# Look for workflow files that might use the deployment-matrix find . -name "*.yml" -o -name "*.yaml" | xargs grep -l "benedita" 2>/dev/null | head -10Repository: LerianStudio/github-actions-shared-workflows
Length of output: 291
🏁 Script executed:
# Check if there's a script or action that processes deployment-matrix find . -type f \( -name "*.js" -o -name "*.ts" -o -name "*.sh" \) | xargs grep -l "deployment-matrix" 2>/dev/nullRepository: LerianStudio/github-actions-shared-workflows
Length of output: 186
🏁 Script executed:
# Look at the entire deployment-matrix.yml structure head -150 config/deployment-matrix.ymlRepository: LerianStudio/github-actions-shared-workflows
Length of output: 7117
🏁 Script executed:
# Check the gitops-update workflow to see how env_suffixes is actually processed sed -n '1,100p' .github/workflows/gitops-update.ymlRepository: LerianStudio/github-actions-shared-workflows
Length of output: 5038
🏁 Script executed:
# Look at the deployment-matrix action to understand how it processes the config cat src/lint/deployment-matrix/action.ymlRepository: LerianStudio/github-actions-shared-workflows
Length of output: 11156
Resolve the Benedita suffix contract mismatch.
Line 122 states
dev-mtandprd-mtdo not exist, butenv_suffixes: ["-st", "-mt"]applies-mtto every non-excluded base env. The schema documentation confirms this expansion behavior: all tag-derived envs (dev/stg/prd/sandbox) receive all suffixes unless listed insuffix_excludes_envs. With current config, production tags will generate dev-mt, stg-mt, and prd-mt deployments, contradicting the comment.The
app_helmfile_envper-app override does not provide per-env suffix control—it skips suffixes entirely for that app. Either update the comment if dev-mt and prd-mt exist, or implement per-env suffix configuration before enabling-mtglobally.🤖 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 `@config/deployment-matrix.yml` around lines 119 - 125, The comment in the Benedita configuration contradicts the actual suffix expansion behavior. The configuration defines env_suffixes: ["-st", "-mt"] which applies the "-mt" suffix to all non-excluded base environments (dev, stg, prd), meaning dev-mt and prd-mt are actually generated by the current schema, not excluded. Either update the comment to correctly state that dev-mt, stg-mt, and prd-mt are all generated by the current configuration, or redesign the suffix_excludes_envs logic to support per-environment suffix restrictions (e.g., excluding "-mt" only from dev and prd while allowing it for stg) if dev-mt and prd-mt should not actually exist.docs/gitops-update-workflow.md (1)
329-329: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Keep the
<server>description fully manifest-driven.The hard-coded
anacleto/beneditasnapshot will drift as soon as the manifest changes, and it clashes with the later examples that resolve a different cluster set fromconfig/deployment-matrix.yml.♻️ Proposed fix
-- `<server>`: any cluster resolved from the deployment matrix (current set: `anacleto`, `benedita`), minus those force-off via `deploy_in_<cluster>: false` +- `<server>`: any cluster resolved from the deployment matrix, minus those force-off via `deploy_in_<cluster>: false`📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.- `<server>`: any cluster resolved from the deployment matrix, minus those force-off via `deploy_in_<cluster>: false`🤖 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 `@docs/gitops-update-workflow.md` at line 329, The description of the `<server>` parameter in the gitops-update-workflow.md file hardcodes specific cluster names (anacleto and benedita) which will become outdated as the deployment manifest changes. Remove the hardcoded cluster names from this description and instead make it reference the dynamic cluster resolution from the deployment matrix, ensuring it aligns with the manifest-driven approach described in later examples that resolve clusters from config/deployment-matrix.yml. The description should indicate that servers come from the deployment matrix with optional filtering, rather than listing specific cluster instances.
…pository from org var
lerian-studio
added
size/L
…ively from GITOPS_RUNNERS org var
There was a problem hiding this comment.
Warning
CodeRabbit couldn't request changes on this pull request because it doesn't have sufficient GitHub permissions.
Please grant CodeRabbit Pull requests: Read and write permission and re-run the review.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
.github/workflows/go-release.yml (1)
117-139: 🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy liftKeep
go-releasecallers compatible or document the breaking migration.This wrapper also removed the Firmino/Clotilde force-off inputs and changed GitOps defaults, but
docs/go-release-workflow.mdstill lists the old inputs and fixed defaults. Existinggo-releasecallers following those docs will pass invalid inputs or rely on missing org variables. Keep deprecated inputs until the next breaking release, or update the docs and release notes with an explicit migration path. As per path instructions, “Flag any removed or renamed input/output as a breaking change for callers.”Also applies to: 469-475
🤖 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/go-release.yml around lines 117 - 139, The go-release workflow has removed inputs for Firmino/Clotilde force-off and changed GitOps defaults, but the documentation in docs/go-release-workflow.md still references the old inputs and defaults, causing existing callers to break. Either restore the removed inputs (like Firmino/Clotilde force-off controls) as deprecated with warnings until the next major release to maintain backward compatibility, or update docs/go-release-workflow.md with explicit migration instructions for all removed/renamed inputs (such as the changes to update_sandbox, gitops_artifact_pattern, gitops_yaml_key_mappings, deployment_matrix_ref, and gitops_runner_type) and document this as a breaking change in release notes.Source: Path instructions
.github/workflows/gitops-update.yml (1)
4-21: 🗄️ Data Integrity & Integration | 🟠 MajorUpdate docs for removed
deploy_in_*inputs and mark as breaking change.Inputs
deploy_in_firminoanddeploy_in_clotildehave been removed from the workflow definition, but the documentation still references them:
docs/go-release-workflow.md:58-59— parameter table lists both removed inputsdocs/gitops-update-workflow.md:438-439— example code passes the removed inputsExternal callers relying on these inputs will fail silently on workflow call (GitHub rejects unknown inputs). Either retain the inputs with deprecation warnings until a major version bump, or update all caller docs and examples immediately and document this as a breaking change per the path guidelines: "Flag any removed or renamed input/output as a breaking change for callers."
🤖 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/gitops-update.yml around lines 4 - 21, The workflow inputs deploy_in_firmino and deploy_in_clotilde have been removed from the workflow definition, but documentation still references them in two locations: docs/go-release-workflow.md (lines 58-59) in the parameter table and docs/gitops-update-workflow.md (lines 438-439) in the example code. To fix this, either retain these inputs with deprecation warnings in the workflow definition and update documentation to note deprecation, or remove all references to these inputs from both documentation files and mark this as a breaking change in the workflow documentation and release notes to inform external callers that they must update their calls to this workflow.Source: Path instructions
🤖 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/gitops-update.yml:
- Around line 48-50: The runner input has an empty string as default, which
causes runs-on to be empty when neither the input parameter nor the
GITOPS_RUNNERS environment variable is provided, resulting in job scheduling
failures. Update the default value from an empty string to include a fallback
chain that allows the input and GITOPS_RUNNERS variable to override, but
ultimately defaults to blacksmith-4vcpu-ubuntu-2404 as the concrete runner.
Apply this same fallback pattern to all job definitions in the workflow that
reference the runner input (also at lines 102 and 842).
---
Outside diff comments:
In @.github/workflows/gitops-update.yml:
- Around line 4-21: The workflow inputs deploy_in_firmino and deploy_in_clotilde
have been removed from the workflow definition, but documentation still
references them in two locations: docs/go-release-workflow.md (lines 58-59) in
the parameter table and docs/gitops-update-workflow.md (lines 438-439) in the
example code. To fix this, either retain these inputs with deprecation warnings
in the workflow definition and update documentation to note deprecation, or
remove all references to these inputs from both documentation files and mark
this as a breaking change in the workflow documentation and release notes to
inform external callers that they must update their calls to this workflow.
In @.github/workflows/go-release.yml:
- Around line 117-139: The go-release workflow has removed inputs for
Firmino/Clotilde force-off and changed GitOps defaults, but the documentation in
docs/go-release-workflow.md still references the old inputs and defaults,
causing existing callers to break. Either restore the removed inputs (like
Firmino/Clotilde force-off controls) as deprecated with warnings until the next
major release to maintain backward compatibility, or update
docs/go-release-workflow.md with explicit migration instructions for all
removed/renamed inputs (such as the changes to update_sandbox,
gitops_artifact_pattern, gitops_yaml_key_mappings, deployment_matrix_ref, and
gitops_runner_type) and document this as a breaking change in release notes.
🪄 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: Path: .coderabbit.yml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 818b5f95-36d7-49f4-8a6e-4d0306891398
📒 Files selected for processing (2)
.github/workflows/gitops-update.yml.github/workflows/go-release.yml
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
.github/workflows/gitops-update.yml (2)
4-21: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winTreat the removed workflow inputs as a breaking caller contract.
Removing
deploy_in_firminoanddeploy_in_clotildemakes existing callers that still pass thosewith:keys fail workflow validation. Either keep them as deprecated no-op inputs for compatibility, or mark this release as breaking and update the workflow docs that still publish those inputs and the oldgitops_repositorydefault. As per path instructions, “Flag any removed or renamed input/output as a breaking change for callers.”🤖 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/gitops-update.yml around lines 4 - 21, The workflow has removed the inputs deploy_in_firmino and deploy_in_clotilde without maintaining backward compatibility, which breaks existing callers passing these keys via with:. Either restore these inputs as no-op boolean inputs in the workflow_call inputs section (with appropriate deprecation notes in their descriptions) to maintain caller compatibility, or document this as a breaking change in the release notes and update any workflow documentation that references the old gitops_repository default behavior and these removed inputs.Source: Path instructions
192-204: 🔒 Security & Privacy | 🟠 Major | ⚡ Quick winMove
yq_versionthroughenvbefore using it in shell.Line 199 expands a caller-controlled workflow input directly into the
run:script. A quoted value can break the shell assignment before validation. Map it throughenv, validate the tag shape, and only skip installation when the existing binary matches the requested version. As per coding guidelines, “Never interpolate untrusted user input directly intorun:commands. Pass untrusted values through environment variables instead.”Proposed fix
- name: Install yq shell: bash + env: + YQ_VERSION: ${{ inputs.yq_version }} run: | set -euo pipefail - if command -v yq &>/dev/null; then + if [[ ! "$YQ_VERSION" =~ ^v[0-9]+[.][0-9]+[.][0-9]+$ ]]; then + echo "::error::Invalid yq_version '$YQ_VERSION'. Expected vX.Y.Z." + exit 1 + fi + if command -v yq &>/dev/null && yq --version | grep -q "version ${YQ_VERSION}$"; then echo "yq already installed: $(yq --version)" else - YQ_VERSION="${{ inputs.yq_version }}" YQ_BIN="yq_linux_amd64" YQ_URL="https://github.com/mikefarah/yq/releases/download/${YQ_VERSION}/${YQ_BIN}" sudo curl -L "$YQ_URL" -o /usr/local/bin/yq🤖 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/gitops-update.yml around lines 192 - 204, The "Install yq" step directly interpolates the caller-controlled input.yq_version into the shell run command, which is a security risk. Instead, pass the yq_version input through the env section of the step, then reference it as an environment variable (like $YQ_VERSION) within the shell script. Add validation to ensure the version tag is in an expected format before using it in the download URL, and update the version check logic to compare the requested version against the output of the already-installed yq binary (if present) to determine whether reinstallation is needed.Sources: Coding guidelines, Linters/SAST tools
🤖 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.
Outside diff comments:
In @.github/workflows/gitops-update.yml:
- Around line 4-21: The workflow has removed the inputs deploy_in_firmino and
deploy_in_clotilde without maintaining backward compatibility, which breaks
existing callers passing these keys via with:. Either restore these inputs as
no-op boolean inputs in the workflow_call inputs section (with appropriate
deprecation notes in their descriptions) to maintain caller compatibility, or
document this as a breaking change in the release notes and update any workflow
documentation that references the old gitops_repository default behavior and
these removed inputs.
- Around line 192-204: The "Install yq" step directly interpolates the
caller-controlled input.yq_version into the shell run command, which is a
security risk. Instead, pass the yq_version input through the env section of the
step, then reference it as an environment variable (like $YQ_VERSION) within the
shell script. Add validation to ensure the version tag is in an expected format
before using it in the download URL, and update the version check logic to
compare the requested version against the output of the already-installed yq
binary (if present) to determine whether reinstallation is needed.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yml
Review profile: ASSERTIVE
Plan: Pro
Run ID: f35c6e0a-511c-4fb1-9652-941669b96328
📒 Files selected for processing (2)
.github/workflows/gitops-update.yml.github/workflows/go-release.yml
💤 Files with no reviewable changes (1)
- .github/workflows/go-release.yml
… and runner fallback
…v instead of run interpolation
There was a problem hiding this comment.
Warning
CodeRabbit couldn't request changes on this pull request because it doesn't have sufficient GitHub permissions.
Please grant CodeRabbit Pull requests: Read and write permission and re-run the review.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (5)
docs/gitops-update-workflow.md (2)
115-115: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
gitops_repositorydefault is out of sync with workflow behavior.Line 115 still documents a fixed default repo, but the workflow contract in this PR moved to empty input + runtime/org fallback resolution. This is a caller-facing interface mismatch and can mislead adopters during upgrades.
Suggested doc fix
-| `gitops_repository` | string | `LerianStudio/midaz-firmino-gitops` | GitOps repository to update | +| `gitops_repository` | string | `""` | GitOps repository to update. When empty, falls back to org-level variable/default resolution in the workflow runtime. |🤖 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 `@docs/gitops-update-workflow.md` at line 115, The documented default for gitops_repository is out of sync with the workflow behavior; update the gitops_repository entry in gitops-update-workflow.md to reflect that the input is now empty by default and resolved at runtime using the org fallback logic. Make sure the docs match the workflow contract introduced in this PR, and keep the description aligned with the actual behavior of the gitops_repository handling.
283-283: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winAnacleto statement contradicts the new section above.
Line 283 says Anacleto behavior is byte-identical when fields are absent, but this same doc now introduces explicit
env_contexts/env_suffixesbehavior for Anacleto. Keep only unchanged clusters in this sentence, or reword as a conditional generic rule.🤖 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 `@docs/gitops-update-workflow.md` at line 283, The Anacleto wording conflicts with the new env_contexts/env_suffixes behavior described above, so update this sentence to avoid claiming byte-identical behavior when those fields are absent. Keep the unchanged cluster names only if the rule still applies universally, or rephrase it as a generic conditional statement that matches the documented default behavior in the same section..github/workflows/gitops-update.yml (3)
890-907: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick winFail fast when ArgoCD connection inputs are missing.
After switching to
vars.ARGOCD_URLandsecrets.ARGOCD_TOKEN, an unset var/secret becomes an empty CLI argument. The currentargocd app gethandling converts that misconfiguration into a warning and exits0, so sync can be silently skipped.Proposed fix
set -uo pipefail + if [[ -z "$ARGOCD_URL" || -z "$ARGOCD_TOKEN" ]]; then + echo "::error::enable_argocd_sync requires vars.ARGOCD_URL and secrets.ARGOCD_TOKEN to be set." + exit 1 + fi + PRUNE_FLAG="" if [[ "$ARGOCD_PRUNE" == "true" ]]; then PRUNE_FLAG="--prune" fi🤖 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/gitops-update.yml around lines 890 - 907, Fail fast in the GitOps update step when ARGOCD_URL or ARGOCD_TOKEN is missing instead of letting argocd app get proceed with empty arguments and return a warning. In the workflow run block that uses ARGOCD_URL, ARGOCD_TOKEN, and APP_NAME, add an explicit validation before calling argocd app get and exit nonzero with a clear error if either input is unset/empty, so the resolve_argocd_app and sync path cannot silently skip due to misconfiguration.
4-21: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winTreat the removed callable inputs as a breaking API change.
deploy_in_firmino,deploy_in_clotilde, andrunner_typewere removed from theworkflow_callsurface, so existing external callers that still pass them will fail before the workflow starts. If this is not a semver-breaking release, keep deprecated compatibility inputs and preserve the force-off filtering path for one release; otherwise document and release it as breaking. As per path instructions, “Flag any removed or renamed input/output as a breaking change for callers.”Also applies to: 46-47, 260-264, 306-320
🤖 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/gitops-update.yml around lines 4 - 21, The workflow_call surface in gitops-update.yml has removed caller-facing inputs, which is a breaking API change for external reusable workflow consumers. In the workflow definition and any related input handling paths, restore backward-compatible deprecated inputs for deploy_in_firmino, deploy_in_clotilde, and runner_type, and keep the existing force-off filtering behavior in the update logic (including the reusable workflow’s input mapping and component selection flow) so old callers still work for one release. If compatibility is not intended, make sure the change is explicitly treated and documented as a breaking release for callers.Source: Path instructions
196-205: 🩺 Stability & Availability | 🟠 MajorRequire the requested yq version before skipping install.
command -v yqaccepts any runner-provided binary, so a preinstalled v3/custom build can bypassinputs.yq_versionand break the lateryq -o=json/strenv()calls. Compareyq --versionagainst the requested version, or always install the requested release.🤖 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/gitops-update.yml around lines 196 - 205, The yq install check is too permissive because command -v yq will skip installation for any preinstalled binary, even if it is the wrong major/version for the later yq -o=json and strenv() usage. Update the install logic in the workflow step that references inputs.yq_version and yq --version so it verifies the existing yq matches the requested version before skipping, or otherwise always download and install the requested release into ~/.local/bin.
🤖 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 `@docs/gitops-update-workflow.md`:
- Line 35: The GitOps workflow docs incorrectly list ARGOCD_URL as a required
secret, but the runtime resolves the ArgoCD URL from org/repo variables while
only ARGOCD_TOKEN is secret-backed. Update the required-secrets wording in the
workflow guide to remove ARGOCD_URL from the secrets list and clearly document
that the URL comes from variables, matching the behavior in the GitOps workflow
documentation section.
---
Outside diff comments:
In @.github/workflows/gitops-update.yml:
- Around line 890-907: Fail fast in the GitOps update step when ARGOCD_URL or
ARGOCD_TOKEN is missing instead of letting argocd app get proceed with empty
arguments and return a warning. In the workflow run block that uses ARGOCD_URL,
ARGOCD_TOKEN, and APP_NAME, add an explicit validation before calling argocd app
get and exit nonzero with a clear error if either input is unset/empty, so the
resolve_argocd_app and sync path cannot silently skip due to misconfiguration.
- Around line 4-21: The workflow_call surface in gitops-update.yml has removed
caller-facing inputs, which is a breaking API change for external reusable
workflow consumers. In the workflow definition and any related input handling
paths, restore backward-compatible deprecated inputs for deploy_in_firmino,
deploy_in_clotilde, and runner_type, and keep the existing force-off filtering
behavior in the update logic (including the reusable workflow’s input mapping
and component selection flow) so old callers still work for one release. If
compatibility is not intended, make sure the change is explicitly treated and
documented as a breaking release for callers.
- Around line 196-205: The yq install check is too permissive because command -v
yq will skip installation for any preinstalled binary, even if it is the wrong
major/version for the later yq -o=json and strenv() usage. Update the install
logic in the workflow step that references inputs.yq_version and yq --version so
it verifies the existing yq matches the requested version before skipping, or
otherwise always download and install the requested release into ~/.local/bin.
In `@docs/gitops-update-workflow.md`:
- Line 115: The documented default for gitops_repository is out of sync with the
workflow behavior; update the gitops_repository entry in
gitops-update-workflow.md to reflect that the input is now empty by default and
resolved at runtime using the org fallback logic. Make sure the docs match the
workflow contract introduced in this PR, and keep the description aligned with
the actual behavior of the gitops_repository handling.
- Line 283: The Anacleto wording conflicts with the new
env_contexts/env_suffixes behavior described above, so update this sentence to
avoid claiming byte-identical behavior when those fields are absent. Keep the
unchanged cluster names only if the rule still applies universally, or rephrase
it as a generic conditional statement that matches the documented default
behavior in the same section.
🪄 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: Path: .coderabbit.yml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 06c5375a-561d-4d0b-9413-0530e4099a02
📒 Files selected for processing (2)
.github/workflows/gitops-update.ymldocs/gitops-update-workflow.md
… update features list
All comments addressed: env_contexts early return fixed, runner fallback added, ARGOCD_URL moved to variables in docs, Lerian branding header added.
…lear lint warnings
… for kustomize cluster resolution
4 participants
Summary
Adds per-cluster env-suffix expansion to the
gitops-updateworkflow so clusters that host multiple parallel variants per environment (e.g. Benedita with single-tenant-stand multi-tenant-mt) can be onboarded without forking the workflow.Closes #422.
Manifest change (additive)
Both fields are optional:
env_suffixes[""]suffix_excludes_envs[]Resolution example
Manifest: benedita declares
env_suffixes: ["-st", "-mt"],suffix_excludes_envs: ["sandbox"].v1.2.3-beta.1)devdev-st,dev-mtv1.2.3-rc.1)stgstg-st,stg-mtv1.2.3)dev stg prd sandboxdev-st,dev-mt,stg-st,stg-mt,prd-st,prd-mt,sandboxv1.2.3-sandbox.1)sandboxsandboxFor firmino/clotilde/anacleto (no manifest change),
env_suffixesdefaults to[""], so the final env list equals the tag-derived list verbatim. Byte-identical behavior to today.Workflow changes
resolve_clustersstep — extended to emit a per-cluster JSON map ofenv_suffixesandsuffix_excludes_envsresolved from the manifest (with defaults applied).apply_tagsstep — new helperexpand_envs_for_serverruns the cross-product of the tag-derived env list × the cluster'senv_suffixes, withsuffix_excludes_envshonored. The expanded env name flows through unchanged to:environments/{server}/helmfile/applications/{ENV}/{app}/values.yamlwhereENVis nowdev-stetc.{server}-{app}-{env}by default) —benedita-midaz-dev-stresolves naturally with no new workflow inputs.Cross-app handling — when an app has
app_helmfile_envset (e.g.forge: cross), the suffix expansion is bypassed and the sync target's env equals the override value (cross). This makesbenedita-forge-crossthe correct ArgoCD app name once the cross apps are renamed to follow the standard convention.Sync-matrix dedup — the updated
(server, env)pairs are now de-duplicated before being emitted as the ArgoCD sync matrix. This prevents cross apps (visited once per outer env iteration) from being queued for sync multiple times.Kustomize layout — unchanged. The kustomize branch already bypasses the env list when its sentinel env (
-) is in effect;expand_envs_for_serverreturns the input unchanged in that case.Lint extension
src/lint/deployment-matrix/action.ymlwas previously validating onlyversion,apps.registry, andclusters.<name>.apps. This PR extends it to validate:env_suffixes(list of strings, unique, non-empty list)suffix_excludes_envs(list of valid env names; warns if pointing at unknown env or used withoutenv_suffixes)app_helmfile_env(pre-existing field, now validated: dict whose keys must reference apps in the cluster'sappslist)Testing
Simulated the full env resolution offline against the updated manifest for all four clusters × four tag types. Output verified manually:
Workflow and action YAML pass
actionlintclean.Extended lint passes on the updated manifest (
violations=0 warnings=0).Out of scope
app_env_suffixesin a follow-up when an actual MT-only or ST-only app emerges.benedita-forge→benedita-forge-cross, etc.) — handled separately in the gitops repo by the cluster owner.Reviewer checklist
config/deployment-matrix.ymlreads sensibly and the schema comments make the new fields discoverablegitops-update.ymlpreserves existing behavior for firmino/clotilde/anacleto (defaultenv_suffixes=[""]produces no env list change)expand_envs_for_servercorrectly handles edge cases: emptyENV_CONFIG, kustomize sentinel-, missing fieldssort -uand the resulting JSON array preserves the (server, env) shapedocs/gitops-update-workflow.mddescribes the new fields and resolution rulessrc/lint/deployment-matrix/action.ymlcovers the new fields and the pre-existingapp_helmfile_envSummary by CodeRabbit
env_contexts,env_suffixes, andsuffix_excludes_envs, with improved ArgoCD app-name normalization.sandboxbased on the update mode.app_helmfile_env.(server, env)entries.