Skip to content

feat: Add konflux-kargo-bot ability to automerge kargo app in stage#533

Merged
openshift-merge-bot[bot] merged 1 commit into
redhat-appstudio:mainfrom
flacatus:move_app
Jul 3, 2026
Merged

feat: Add konflux-kargo-bot ability to automerge kargo app in stage#533
openshift-merge-bot[bot] merged 1 commit into
redhat-appstudio:mainfrom
flacatus:move_app

Conversation

@flacatus

@flacatus flacatus commented Jul 2, 2026

Copy link
Copy Markdown
Contributor

No description provided.

@openshift-ci

openshift-ci Bot commented Jul 2, 2026

Copy link
Copy Markdown

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: flacatus

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Details Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@openshift-ci openshift-ci Bot added the approved label Jul 2, 2026
@qodo-for-redhat-appstudio

Copy link
Copy Markdown

PR Summary by Qodo

Enable stage auto-merge for kargo via git-merge-pr in staging pipeline

✨ Enhancement ⚙️ Configuration changes 🕐 20-40 Minutes

Grey Divider

AI Description

• Add explicit reviewers list to kargo component OWNERS.
• Replace PR wait step with PR merge step in stage ring-1 staging pipeline.
• Deploy Argo CD from the merged commit SHA instead of the pre-merge PR commit.
Diagram

graph TD
  P["Stage ring-1 pipeline"] --> O["open-pr step"] --> M["git-merge-pr (wait=true)"] --> A["argocd-update"] --> C["Argo CD"]
  O --> R["Git repo"]
  M --> R
  A --> R
Loading
High-Level Assessment

The following are alternative approaches to this PR:

1. Use GitHub native auto-merge (branch protection + auto-merge)
  • ➕ Moves merge policy/enforcement to the VCS layer
  • ➕ Reduces custom pipeline logic and bot permissions complexity
  • ➖ Still needs coordination with pipeline to ensure Argo CD deploys the merged SHA
  • ➖ May not fit environments where merges must be initiated only by automation
2. Keep git-wait-for-pr and deploy PR commit (no merge in pipeline)
  • ➕ Avoids merge-side effects inside the promotion workflow
  • ➕ Simpler failure modes (no conflicts/merge method issues during promotion)
  • ➖ Stage could run an unmerged commit, diverging from what lands on the target branch
  • ➖ Does not satisfy the stated goal of automerging in stage

Recommendation: The PR’s approach (merge in the pipeline and deploy the merged commit) is the most direct way to guarantee stage reflects what actually landed in the branch. Ensure the bot has the right merge permissions, the intended merge method (merge/squash/rebase) is enforced, and failure behavior is acceptable when merges are blocked (conflicts, missing approvals, branch protection).

Files changed (2) +10 / -3

Enhancement (1) +4 / -3
stage-ring-1-staging.yamlMerge PRs during stage promotion and deploy merged commit +4/-3

Merge PRs during stage promotion and deploy merged commit

• Replaces the git-wait-for-pr step with git-merge-pr (wait=true) in the staging ring-1 pipeline. Updates the Argo CD desiredRevision input to use the merged commit output from the merge step.

components/kargo/internal-production/projects/kargo-infra-common/base/stage-ring-1-staging.yaml

Other (1) +6 / -0
OWNERSAdd reviewers list to kargo OWNERS +6/-0

Add reviewers list to kargo OWNERS

• Introduces a reviewers section mirroring the existing approvers list. This clarifies who can be requested for review without changing approval authority.

components/kargo/OWNERS

@qodo-for-redhat-appstudio

qodo-for-redhat-appstudio Bot commented Jul 2, 2026

Copy link
Copy Markdown

Code Review by Qodo

🐞 Bugs (0) 📘 Rule violations (1) 📜 Skill insights (0)

Context used
✅ Compliance rules (platform): 5 rules

Grey Divider


Action required

1. Skipped step output reference ✓ Resolved 🐞 Bug ☼ Reliability
Description
argocd-update always dereferences outputs['merge-pr'].commit even though the merge-pr step is
conditional and will be skipped when git-open-pr ends in Errored (continueOnError=true). This can
cause the promotion to fail when rendering/evaluating the argocd-update config due to the missing
merge-pr outputs.
Code

components/kargo/internal-production/projects/kargo-infra-common/base/stage-ring-1-staging.yaml[R84-93]

+        - uses: git-merge-pr
          if: ${{ status('open-pr') != 'Errored' }}
-          as: wait-for-pr
+          as: merge-pr
          config:
            repoURL: ${{ vars.repoURL }}
            prNumber: ${{ outputs['open-pr'].pr.id }}
+            wait: true
        - uses: argocd-update
          as: argocd-update
          config:
Relevance

⭐⭐ Medium

No prior reviews found about guarding outputs from skipped conditional steps; unclear team
enforcement.

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
The Stage explicitly allows git-open-pr to error (continueOnError: true), then conditionally
runs git-merge-pr only when open-pr is not Errored, but the subsequent argocd-update always
reads outputs['merge-pr'].commit, which won’t exist when merge-pr is skipped.

components/kargo/internal-production/projects/kargo-infra-common/base/stage-ring-1-staging.yaml[71-99]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
`argocd-update` references `${{ outputs['merge-pr'].commit }}` unconditionally, but `merge-pr` only runs when `status('open-pr') != 'Errored'`. Because `git-open-pr` is configured with `continueOnError: true`, it can legitimately end in `Errored`, causing `merge-pr` to be skipped and leaving no `outputs['merge-pr']` to reference.

## Issue Context
This is in the `ring-1-staging` Stage promotion template. The pipeline currently allows `git-open-pr` to error without stopping the promotion, but later steps assume merge outputs exist.

## Fix Focus Areas
- components/kargo/internal-production/projects/kargo-infra-common/base/stage-ring-1-staging.yaml[71-99]

## Suggested fix options
1. Add the same `if: ${{ status('open-pr') != 'Errored' }}` guard to `argocd-update` (and any later steps that depend on `merge-pr` outputs), so the template never references missing outputs.
2. Alternatively, remove `continueOnError: true` from `git-open-pr` so an `Errored` open-pr stops the promotion before downstream steps evaluate.
3. If you intentionally want to proceed on open-pr errors, implement a safe fallback for `desiredRevision` that does not depend on `merge-pr` outputs when `merge-pr` is skipped.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools



Informational

2. Stage manifest misnamed stage-ring-1-staging 📘 Rule violation ⚙ Maintainability
Description
The modified Kubernetes manifest file stage-ring-1-staging.yaml contains a single primary resource
of kind Stage, but the filename base does not match the lowercase kind name (stage). This
violates the required manifest naming convention and can cause ambiguity and inconsistent resource
organization.
Code

components/kargo/internal-production/projects/kargo-infra-common/base/stage-ring-1-staging.yaml[R84-90]

+        - uses: git-merge-pr
          if: ${{ status('open-pr') != 'Errored' }}
-          as: wait-for-pr
+          as: merge-pr
          config:
            repoURL: ${{ vars.repoURL }}
            prNumber: ${{ outputs['open-pr'].pr.id }}
+            wait: true
Relevance

⭐ Low

Same Stage kind-to-filename rename suggestion was rejected for this exact file in PR #484.

PR-#484

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
PR Compliance ID 1100 requires that each modified Kubernetes manifest file containing a single
primary resource kind be named exactly after that kind (lowercased). The file shows kind: Stage
while the filename base is stage-ring-1-staging, which is not equal to stage.

Rule 1100: Name Kubernetes manifest files after single primary Kind
components/kargo/internal-production/projects/kargo-infra-common/base/stage-ring-1-staging.yaml[1-5]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
A Kubernetes manifest with `kind: Stage` is stored in `stage-ring-1-staging.yaml`, which violates the rule requiring manifest filenames (case-insensitive, excluding extension) to exactly match the lowercase resource kind.

## Issue Context
This file is a single-resource manifest (`kind: Stage`). To comply, rename the file to `stage.yaml` (or `stage.yml`) and update any references (e.g., kustomize `resources:` entries) that point to the old filename.

## Fix Focus Areas
- components/kargo/internal-production/projects/kargo-infra-common/base/stage-ring-1-staging.yaml[1-5]
- components/kargo/internal-production/projects/kargo-infra-common/base/stage-ring-1-staging.yaml[84-90]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


Grey Divider

Qodo Logo

Signed-off-by: flacatus <flacatus@redhat.com>
@p8r-the-gr8

Copy link
Copy Markdown
Contributor

/lgtm

@openshift-ci openshift-ci Bot added the lgtm label Jul 3, 2026
@openshift-merge-bot openshift-merge-bot Bot merged commit 876bac3 into redhat-appstudio:main Jul 3, 2026
6 checks passed
@flacatus flacatus deleted the move_app branch July 3, 2026 09:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants