ROSA-745: per-repo dependency automation config#93
Conversation
|
@MitaliBhalla: This pull request references ROSA-745 which is a valid jira issue. Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the initiative to target the "5.0.0" version, but no target version was set. DetailsIn response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository. |
|
Skipping CI for Draft Pull Request. |
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughAdds Dependabot Go module update settings and a Dependabot-only workflow that enables squash auto-merge for safe updates, comments on major updates, and logs the decision. ChangesDependabot Update Automation
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes 🚥 Pre-merge checks | ✅ 15✅ Passed checks (15 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: MitaliBhalla The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
dea10db to
778ed0b
Compare
There was a problem hiding this comment.
Actionable comments posted: 6
🧹 Nitpick comments (1)
.github/workflows/dependabot-auto-merge.yml (1)
44-47: 🔒 Security & Privacy | 🔵 Trivial | ⚡ Quick winMove metadata outputs through env vars before shell use.
Direct
${{ ... }}expansion insiderunscripts and JSON payloads is brittle and can become shell/JSON injection if the metadata shape expands later.Also applies to: 89-89, 106-106, 111-118
🤖 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/dependabot-auto-merge.yml around lines 44 - 47, The workflow is using direct metadata interpolation inside shell and JSON payloads, which is brittle and may become injection-prone as the metadata expands. Update the affected steps in the dependabot-auto-merge workflow so the metadata from the metadata step is first mapped into env vars, then referenced from those env vars in the shell script and JSON construction. Apply this consistently where the current `${{ steps.metadata.outputs... }}` values are used, including the auto-merge logging and payload-building sections.Source: 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.
Inline comments:
In @.github/workflows/dependabot-auto-merge.yml:
- Around line 108-124: The final auto-merge decision log in the “Log Auto-Merge
Decision” step should reflect the actual enable result, not just the update type
and label checks. Update the logic around the decision path that follows the
GraphQL mutation so it logs ENABLED only when the auto-merge action truly
succeeded; if the mutation fails and the workflow falls back to manual review,
the log should show DISABLED or manual-review instead. Use the existing step
outputs and result state in the workflow to base the message on the real
outcome.
- Around line 92-106: The “Comment on Major Version Updates” step can post the
same major-update message multiple times because the workflow runs on several PR
events, including rebases. Update the logic in the major-update comment step to
detect whether the PR already has this specific comment before calling the
GitHub comments API, and skip posting if it exists. Use the existing “Comment on
Major Version Updates” step and the `${{ github.event.pull_request.number
}}`/`steps.metadata.outputs.*` context to keep the check scoped to the current
PR.
- Around line 75-77: The success check in the GraphQL auto-merge step only looks
at the HTTP status, so it can falsely report success when GitHub returns HTTP
200 with a GraphQL errors array. Update the response handling in the dependabot
auto-merge workflow step to inspect the parsed GraphQL payload after the
request, and only print the success message when there are no errors and the
expected data indicates auto-merge was enabled. Use the existing
response-handling block around the GraphQL request and the cat
/tmp/response.json output to verify and surface any GraphQL errors instead of
treating 200 as success.
- Around line 19-24: The Dependabot auto-merge job includes an unnecessary
Checkout code step and uses a mutable action tag for Fetch Dependabot Metadata.
Remove the actions/checkout usage from this job entirely, and update the
dependabot/fetch-metadata reference in the workflow to a full commit SHA so the
action is pinned. Keep the change localized to the dependabot-auto-merge
workflow and preserve the existing metadata step behavior.
- Around line 7-11: The dependabot auto-merge workflow is requesting extra
GITHUB_TOKEN scopes that it does not use. Update the permissions block in the
dependabot-auto-merge workflow to keep only the scopes needed for the merge
flow, and remove the unused checks and actions permissions from the workflow’s
permission configuration.
- Around line 16-17: The Dependabot-only workflow gate is using the trigger
actor instead of the pull request author, so update the condition in the
dependabot auto-merge workflow to check the PR author via
github.event.pull_request.user.login. Also add a fork guard using
github.event.pull_request.head.repo.full_name == github.repository so only PRs
originating from the same repository are allowed; adjust the existing if
expression on the workflow job/step that currently references github.actor and
github.repository.
---
Nitpick comments:
In @.github/workflows/dependabot-auto-merge.yml:
- Around line 44-47: The workflow is using direct metadata interpolation inside
shell and JSON payloads, which is brittle and may become injection-prone as the
metadata expands. Update the affected steps in the dependabot-auto-merge
workflow so the metadata from the metadata step is first mapped into env vars,
then referenced from those env vars in the shell script and JSON construction.
Apply this consistently where the current `${{ steps.metadata.outputs... }}`
values are used, including the auto-merge logging and payload-building sections.
🪄 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: Repository: openshift/coderabbit/.coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: 40ece3d7-f6b6-44dd-8f07-3085b9506e78
📒 Files selected for processing (2)
.github/dependabot.yml.github/workflows/dependabot-auto-merge.yml
✅ Files skipped from review due to trivial changes (1)
- .github/dependabot.yml
778ed0b to
982290a
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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/dependabot-auto-merge.yml:
- Around line 70-84: The PR comment posting in the dependabot auto-merge
workflow ignores whether the GitHub API actually succeeded. Update both
comment-posting blocks around the jq/curl pipeline to capture the HTTP status
from the curl request, and use that status to warn or fail when it is not 201.
Keep the fix localized to the comment creation step so the workflow only reports
success when the POST really created the issue comment.
- Around line 25-27: The auto-merge workflow condition in dependabot-auto-merge
should only handle the update types that dependabot/fetch-metadata actually
emits. Remove the unreachable semver-digest branch from the update-type check
and also delete the matching semver-digest case arm in the workflow logic so the
condition and branching stay aligned with the supported semver-major/minor/patch
values.
🪄 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: Repository: openshift/coderabbit/.coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: b5412777-b28f-427c-99a5-df827547167b
📒 Files selected for processing (2)
.github/dependabot.yml.github/workflows/dependabot-auto-merge.yml
✅ Files skipped from review due to trivial changes (1)
- .github/dependabot.yml
982290a to
c83bd8a
Compare
|
Addressed CodeRabbit review in latest push:
Thanks @coderabbitai |
|
Correction: the allow-list/groups note applies to osdctl only — this PR only includes the shared workflow hardening. |
|
Tip For best results, initiate chat on the files or code changes.
Appreciate you addressing the review comments. ✏️ Learnings added
|
c83bd8a to
27a73f2
Compare
There was a problem hiding this comment.
🧹 Nitpick comments (1)
.github/workflows/dependabot-auto-merge.yml (1)
12-13: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winBound this automation job with a timeout.
The workflow performs only metadata/API operations; without
timeout-minutes, a stuck action orcurlcall can hold the job until the platform default timeout.Suggested change
auto-merge: runs-on: ubuntu-latest + timeout-minutes: 10 if: |🤖 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/dependabot-auto-merge.yml around lines 12 - 13, The dependabot auto-merge workflow job lacks an explicit timeout, so a stalled step can run until the platform default limit. Add a timeout limit directly on the auto-merge job in dependabot-auto-merge.yml, keeping the change scoped to the auto-merge job definition so the metadata/API workflow cannot hang indefinitely.
🤖 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.
Nitpick comments:
In @.github/workflows/dependabot-auto-merge.yml:
- Around line 12-13: The dependabot auto-merge workflow job lacks an explicit
timeout, so a stalled step can run until the platform default limit. Add a
timeout limit directly on the auto-merge job in dependabot-auto-merge.yml,
keeping the change scoped to the auto-merge job definition so the metadata/API
workflow cannot hang indefinitely.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository: openshift/coderabbit/.coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: c3ad7b1f-8308-470e-9ae6-d0928dd48327
📒 Files selected for processing (2)
.github/dependabot.yml.github/workflows/dependabot-auto-merge.yml
🚧 Files skipped from review as they are similar to previous changes (1)
- .github/dependabot.yml
|
@MitaliBhalla: all tests passed! Full PR test history. Your PR dashboard. DetailsInstructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. I understand the commands that are listed here. |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #93 +/- ##
=====================================
Coverage 0.00% 0.00%
=====================================
Files 34 34
Lines 1594 1594
=====================================
Misses 1594 1594 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
Summary
ROSA-745 — per-repo dependency automation for backplane-tools (not on boilerplate).
Follows the backplane-cli pilot: Dependabot gomod (grouped) + GHA auto-merge for patch/minor/digest after required prow checks pass.
Draft — merge after openshift/release#80263 is live and branch-protector has cycled (~6h).
Depends on
ci/prow/*on default branchTest plan
Summary by CodeRabbit