Skip to content

[release-1.10] ci: auto-update bundle manifests on same-repo PRs - #3228

Merged
rm3l merged 1 commit into
redhat-developer:release-1.10from
rm3l:cherry-pick/release-1.10/ci/auto-update-bundle-manifests-on-pr
Jul 20, 2026
Merged

[release-1.10] ci: auto-update bundle manifests on same-repo PRs#3228
rm3l merged 1 commit into
redhat-developer:release-1.10from
rm3l:cherry-pick/release-1.10/ci/auto-update-bundle-manifests-on-pr

Conversation

@rm3l

@rm3l rm3l commented Jul 20, 2026

Copy link
Copy Markdown
Member

Manual cherry-pick of #3169

@rm3l
rm3l requested a review from a team as a code owner July 20, 2026 16:26
@sonarqubecloud

Copy link
Copy Markdown

@rhdh-qodo-merge

Copy link
Copy Markdown

PR Summary by Qodo

CI: Auto-regenerate and push bundle manifests for same-repo PRs

✨ Enhancement ⚙️ Configuration changes 🕐 10-20 Minutes

Grey Divider

AI Description

• Regenerate bundle manifests during PR validation and detect drift (ignoring createdAt churn).
• Auto-commit and push updated manifests back to same-repo PR branches using a bot token.
• Fail the workflow only when manifests remain out-of-sync after the best-effort auto-push.
Diagram

graph TD
  A(["pull_request event"]) --> B["Job: Validate bundle manifests"] --> C["Checkout PR branch"] --> D["Regenerate manifests (make)"] --> E{"Diff ignoring createdAt?"}
  E -->|"no changes"| F["Pass"]
  E -->|"changed"| G{"Same-repo PR?"} -->|"yes"| H["Auto-commit + push"] --> I{"Push succeeded?"}
  I -->|"yes"| F
  I -->|"no"| J["Fail: out of sync"]
  G -->|"no"| J

  subgraph Legend
    direction LR
    _evt(["CI trigger"]) ~~~ _step["Workflow step"] ~~~ _dec{"Decision"}
  end
Loading
High-Level Assessment

The following are alternative approaches to this PR:

1. Keep current behavior (fail-only drift check)
  • ➕ Simplest security posture (no write token usage in CI)
  • ➕ Clear developer responsibility to regenerate artifacts
  • ➖ More manual churn and PR back-and-forth for generated artifacts
  • ➖ Higher likelihood of CI failures late in review
2. Use pull_request_target + checkout head SHA
  • ➕ Can use write permissions without requiring a custom bot secret
  • ➕ Common pattern for commenting/labeling workflows
  • ➖ Higher security risk if misconfigured (runs in base-repo context)
  • ➖ Requires careful hardening and strict checkout of head SHA only
3. Package logic as a reusable action/composite
  • ➕ Reuses the same drift/autopush logic across branches/repos
  • ➕ Easier to test and version workflow behavior
  • ➖ Extra maintenance overhead for action versioning
  • ➖ Doesn’t materially improve this single-workflow change

Recommendation: The PR’s approach is a good trade-off: it keeps the check strict, but removes toil by auto-pushing only for same-repo PRs (forks remain fail-only for safety). The key thing to preserve is the security boundary: ensure pushes only happen when head.repo == base repo and credentials are explicitly controlled (as implemented via persist-credentials: false plus an explicit bot token).

Files changed (1) +42 / -6

Other (1) +42 / -6
pr-bundle-diff-checks.yamlRegenerate, detect, and auto-push bundle manifest updates on PRs +42/-6

Regenerate, detect, and auto-push bundle manifest updates on PRs

• The workflow now checks out the PR head branch explicitly, regenerates bundle/installers, and records whether meaningful diffs exist while ignoring createdAt-only changes. If diffs are detected and the PR is from the same repository, it attempts a best-effort bot commit and push; otherwise it fails with a clear out-of-sync error.

.github/workflows/pr-bundle-diff-checks.yaml

@rm3l
rm3l merged commit c566336 into redhat-developer:release-1.10 Jul 20, 2026
5 of 6 checks passed
@rm3l
rm3l deleted the cherry-pick/release-1.10/ci/auto-update-bundle-manifests-on-pr branch July 20, 2026 16:28
@rhdh-qodo-merge

Copy link
Copy Markdown

Code Review by Qodo

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

Grey Divider


Remediation recommended

1. Untracked changes not detected 🐞 Bug ≡ Correctness
Description
The change detector uses only git diff, which ignores untracked files; if regeneration creates new
files under bundle/, config/, or dist/, the workflow can incorrectly set changed=false, skip
auto-push, and pass despite being out of sync. The Makefile targets write generated outputs via
redirects (e.g., > dist/.../install.yaml) and operator-sdk bundle generation, which can introduce
new files.
Code

.github/workflows/pr-bundle-diff-checks.yaml[R39-51]

+      - name: Check for changes
+        id: check
+        run: |
          # Since operator-sdk 1.26.0, `make bundle` changes the `createdAt` field from the bundle every time we run it.
          # The `git diff` below checks if only the createdAt field has changed. If it is the only change, it is ignored.
          # Inspired from https://github.com/operator-framework/operator-sdk/issues/6285#issuecomment-1415350333
          if git diff --quiet -I'^    createdAt: ' bundle config dist; then
            echo "✅ Bundle manifests are up to date"
+            echo "changed=false" >> "$GITHUB_OUTPUT"
+          else
+            echo "Bundle manifests are out of sync"
+            echo "changed=true" >> "$GITHUB_OUTPUT"
+          fi
Relevance

⭐⭐ Medium

No clear historical evidence of reviewers requesting untracked-file detection (git status/ls-files)
in workflows.

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
The workflow’s change detection uses git diff only (tracked-file diff) and has no `git
status/git ls-files --others` check; the invoked Makefile targets generate files by writing to
dist/ and generating bundle content, which can introduce new files that git diff will not report
if untracked.

.github/workflows/pr-bundle-diff-checks.yaml[39-51]
Makefile[269-280]
Makefile[300-318]

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

### Issue description
The workflow’s “Check for changes” step relies on `git diff ... bundle config dist`, which does not report untracked files. If `make bundles build-installers` produces a newly generated file (e.g., a new manifest/installer output) that isn’t already tracked, the workflow will incorrectly treat the repo as unchanged.

### Issue Context
The Make targets used here generate outputs by writing files (e.g., `> dist/.../install.yaml`) and by running operator-sdk bundle generation, which can create new artifacts.

### Fix Focus Areas
- .github/workflows/pr-bundle-diff-checks.yaml[39-51]
- .github/workflows/pr-bundle-diff-checks.yaml[78-99]

### Suggested fix
- Extend the check to also detect untracked files under the same paths. For example:
 - Keep the current `git diff --quiet -I ... -- bundle config dist` for tracked changes, AND
 - Add an untracked check like `git ls-files --others --exclude-standard -- bundle config dist`.
- Set `changed=true` if either tracked diffs exist (beyond createdAt) OR untracked files exist.
- Optionally, enhance the failure message to also list untracked files (so contributors see what was generated but not committed).

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



Informational

2. Workflow run blocks lack strict mode 📘 Rule violation ☼ Reliability
Description
The newly added GitHub Actions run scripts do not explicitly enable strict shell mode (e.g., `set
-euo pipefail`). This can allow unset variables or silent failures to go unnoticed, causing flaky CI
behavior and harder-to-debug failures.
Code

.github/workflows/pr-bundle-diff-checks.yaml[R61-65]

+        run: |
+          if [[ -z "${RHDH_BOT_TOKEN}" || -z "${HEAD_REF}" ]]; then
+            echo "::warning::RHDH_BOT_TOKEN or HEAD_REF is not set, skipping auto-push"
            exit 0
          fi
Relevance

⭐ Low

Strict-mode suggestion (set -euo pipefail) was only partially accepted; PR #3169 didn’t adopt strict
mode.

PR-#3169

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
Compliance ID 5 requires hardened shell scripting with strict modes like set -euo pipefail. The
run blocks added in this PR (e.g., Check for changes and `Auto-commit and push updated bundle
manifests`) do not include an explicit strict-mode line at the start of the script.

.github/workflows/pr-bundle-diff-checks.yaml[41-52]
.github/workflows/pr-bundle-diff-checks.yaml[61-76]
Best Practice: Repository guidelines

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

## Issue description
The workflow `run` scripts added/modified in this PR do not explicitly enable strict shell options (notably `-u`). The compliance checklist requires strict mode, quoting, and robust scripting practices for shell blocks.

## Issue Context
Although GitHub Actions may apply some default shell flags depending on runner/shell, this workflow should be self-contained and explicitly set strict mode to prevent unset-variable bugs and improve failure visibility.

## Fix Focus Areas
- .github/workflows/pr-bundle-diff-checks.yaml[41-52]
- .github/workflows/pr-bundle-diff-checks.yaml[61-76]

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


3. Overprivileged GITHUB_TOKEN 🐞 Bug ⛨ Security
Description
The workflow grants job-wide contents: write, making the default GITHUB_TOKEN write-capable
during execution of PR-checked-out code (e.g., make bundles build-installers). This is unnecessary
because the workflow push path uses RHDH_BOT_TOKEN, and it increases the blast radius of any
compromise in earlier steps.
Code

.github/workflows/pr-bundle-diff-checks.yaml[R19-20]

+    permissions:
+      contents: write
Relevance

⭐ Low

Similar least-privilege workflow-permissions suggestion was rejected in PR #2141; team kept
permissions: contents: write.

PR-#2141

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
The job explicitly grants contents: write (write-capable GITHUB_TOKEN) and then runs build
commands from the checked-out PR branch; however, the only push operation is authenticated with
RHDH_BOT_TOKEN, so contents: write on GITHUB_TOKEN is not required for the shown behavior.

.github/workflows/pr-bundle-diff-checks.yaml[19-20]
.github/workflows/pr-bundle-diff-checks.yaml[23-37]
.github/workflows/pr-bundle-diff-checks.yaml[53-71]

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

### Issue description
The job sets `permissions: contents: write`, granting a write-capable `GITHUB_TOKEN` to all steps (including PR-controlled build commands). The workflow’s push uses `RHDH_BOT_TOKEN`, so this write permission is not required and violates least privilege.

### Issue Context
- The PR branch is checked out and build commands are executed from that branch.
- The auto-push step authenticates via a bot PAT (`RHDH_BOT_TOKEN`), not `GITHUB_TOKEN`.

### Fix Focus Areas
- .github/workflows/pr-bundle-diff-checks.yaml[19-20]
- .github/workflows/pr-bundle-diff-checks.yaml[23-37]
- .github/workflows/pr-bundle-diff-checks.yaml[53-71]

### Suggested fix
- Change job permissions to the minimum required, e.g. `permissions: { contents: read }`.
- Keep the PAT-based push as-is (or, if you later switch to `GITHUB_TOKEN` pushing, then re-evaluate and scope permissions to only what’s needed).

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


Grey Divider

Qodo Logo

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant