Add workflow to upgrade dependency versions#13952
Add workflow to upgrade dependency versions#13952PasanT9 wants to merge 2 commits intowso2:masterfrom
Conversation
|
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:
WalkthroughA new GitHub Actions workflow automates dependency upgrades by resolving target versions (with optional overrides), extracting axis2/axiom from Synapse, updating POMs and product/filter properties, validating changes (XML/Maven), and creating or updating a pull request with summaries; supports dry-run and debug modes. Changes
Sequence Diagram(s)sequenceDiagram
participant User as User (manual trigger)
participant Workflow as Actions Workflow
participant GHAPI as GitHub API
participant Repo as Repository (code)
participant Synapse as Synapse repo
participant Maven as Maven/Validator
participant PR as GitHub PR
User->>Workflow: workflow_dispatch (versions, dry-run, debug)
Workflow->>GHAPI: resolve target versions (query tags)
GHAPI-->>Workflow: tags / version candidates
Workflow->>Synapse: checkout synapse (if needed)
Synapse-->>Workflow: axis2/axiom versions
Workflow->>Repo: checkout repo + apply POM/product/filter updates
Repo-->>Workflow: patched files
Workflow->>Maven: XML & maven validation
Maven-->>Workflow: validation results
alt changes detected and not dry-run
Workflow->>PR: create or update PR with summary & labels
PR-->>Workflow: PR URL / status
else dry-run or no changes
Workflow-->>User: summary only
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In @.github/workflows/dependency-upgrade.yaml:
- Line 30: Replace both occurrences of the checkout action that read "uses:
actions/checkout@v3" with "uses: actions/checkout@v4" to ensure the workflow
uses the newer Node.js runtime; update each instance (the one shown as "uses:
actions/checkout@v3" and the second occurrence later in the file) so both
references point to actions/checkout@v4.
- Line 46: The current TAG assignment uses the tags API with per_page=1 which
returns most-recently-pushed tag (not highest semver); change the logic that
sets TAG (and the kernel tag resolution at the other location) to fetch a larger
page (e.g., per_page=100 or paginate), extract tag names (the jq query currently
using .[0].name), then semver-sort the list (e.g., sort -V) and pick the highest
version (last entry) as TAG; alternatively, use the Releases API
(/repos/{owner}/{repo}/releases/latest) when the repo publishes releases and
fall back to the semver-sorted tags approach. Ensure you update the TAG
assignment command and the kernel tag resolution code path to use the same
semver-sorting approach.
🧹 Nitpick comments (3)
.github/workflows/dependency-upgrade.yaml (3)
54-78: Consider addingactions/setup-javabefore Maven commands.Both jobs use
mvn help:evaluate(lines 60, 155) but neither sets up a specific JDK. Whileubuntu-latestbundles a JDK, the pre-installed version may not match the project's requirements and can change without notice. Adding an explicitactions/setup-javastep makes the workflow deterministic and avoids surprise breakages.Proposed fix (add before the "Decide whether…" step in each job)
- name: Set up JDK uses: actions/setup-java@v4 with: distribution: temurin java-version: '17' # adjust to match project requirement
98-101:exit 0does not prevent the PR-creation step from running.When
git diff --quietdetects no changes, this step exits successfully, butUPGRADE_APIMGTremainstruein the environment, so the "Create/update PR" step (line 103) still executes.peter-evans/create-pull-requesthappens to no-op when there are no changes, so this isn't a functional bug — but the intent is unclear and the PR action still runs unnecessarily.The same pattern appears in the kernel job at lines 231–234.
Proposed fix — clear the flag so the PR step is truly skipped
if git diff --quiet -- "${FILES[@]}"; then echo "No APIMGT upgrade needed" - exit 0 + echo "UPGRADE_APIMGT=" >> "$GITHUB_ENV" + exit 0 fiApply a similar change in the kernel job (line 232):
if git diff --quiet -- "${POMS[@]}" "${PRODUCTS[@]}" "${FILTERS[@]}"; then echo "No KERNEL upgrade needed" - exit 0 + echo "UPGRADE_KERNEL=" >> "$GITHUB_ENV" + exit 0 fi
131-143: Minor:is49()regex allows trailing characters — may accept unexpected inputs.The regex
^4\.9\.[0-9]+.*$matches any string starting with4.9.<digits>followed by anything (e.g.,4.9.1-malicious). If the intent is to only allow standard Maven versions like4.9.25or4.9.25-SNAPSHOT, tighten the pattern.Proposed tighter pattern
- is49() { [[ "$1" =~ ^4\.9\.[0-9]+.*$ ]]; } + is49() { [[ "$1" =~ ^4\.9\.[0-9]+(-[A-Za-z0-9.]+)?$ ]]; }
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In @.github/workflows/dependency-upgrade.yaml:
- Around line 43-44: Update the hardcoded OWNER variable value from "pasant9" to
"wso2" wherever the OWNER is set (e.g., the OWNER="pasant9" assignments in the
workflow), ensuring the REPO variable (REPO="carbon-apimgt") remains unchanged;
apply the same replacement for all occurrences referenced in the workflow so
tags resolve against the canonical wso2 organization instead of the personal
fork.
d6177c8 to
a4d36bb
Compare
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Fix all issues with AI agents
In @.github/workflows/dependency-upgrade.yaml:
- Around line 324-325: The sed commands currently match any <product> or
<feature> element's version attribute and replace it with ${KERNEL_VERSION},
which will mistakenly overwrite non-kernel component versions; change the
replacement to target only kernel-related entries by narrowing the match (e.g.,
only match elements whose id/name contains the kernel feature id or group) or
switch to an XML-aware approach such as using xmlstarlet to select specific
<product> or <feature> nodes by a known kernel identifier and update their
`@version` with ${KERNEL_VERSION}; update the commands that reference sed and
${KERNEL_VERSION} accordingly so only kernel feature/product elements are
modified.
- Around line 454-456: The SUMMARY variable is being updated inside a subshell
because of the pipeline "git diff --name-only | while read -r file; do ...
done", so assignments to SUMMARY never reach the parent shell; change the loop
to avoid the pipe-induced subshell (e.g., feed git diff output into the while
loop via process substitution or input redirection) so that updates to SUMMARY
inside the loop (the block that appends "- \`$file\`") persist after the loop
completes.
- Around line 114-117: The workflow currently injects inputs directly into run
scripts (e.g., uses `${{ inputs.apimgtVersion }}` when calling
validate_version), which allows script injection; change these run steps to pass
inputs via env: (e.g., set APIMGT_VERSION: ${{ inputs.apimgtVersion }}) and then
reference the value inside the script as $APIMGT_VERSION when calling
validate_version (and do the same for KERNEL_VERSION, APIMAPPS_VERSION,
SYNAPSE_VERSION); apply this pattern everywhere `${{ inputs.* }}` is used inside
run: blocks (including the "Resolve dependency versions" step that calls
validate_version) so the shell sees only environment variables like
$APIMGT_VERSION rather than pre-expanded input expressions.
🧹 Nitpick comments (3)
.github/workflows/dependency-upgrade.yaml (3)
13-15: Schedule is commented out — remember to uncomment before merging.The weekly cron trigger is currently disabled. Ensure this is re-enabled when the workflow is ready for production.
380-384:exit 0terminates the entire step — fragile if more sections are added later.If a maintainer appends another dependency section after synapse in this step,
exit 0will silently skip it. Wrap the update logic in anifblock instead.Proposed fix
- # Only prevent *synapse* downgrade (optional, but sensible) - if ! is_version_higher "$CURRENT_SYNAPSE" "$T_SYN"; then - echo "::notice::Target synapse is lower; skipping." - exit 0 - fi - - echo "::notice::Updating synapse trio to match synapse POM:" + if ! is_version_higher "$CURRENT_SYNAPSE" "$T_SYN"; then + echo "::notice::Target synapse is lower; skipping." + else + echo "::notice::Updating synapse trio to match synapse POM:" ... + fi
472-492: PRs created withGITHUB_TOKENwon't trigger downstream CI workflows.GitHub Actions purposely prevents
GITHUB_TOKEN-authored events from triggering other workflows to avoid recursive loops. If CI checks (builds, tests) need to run on the auto-created PR, you'll need a GitHub App token or a PAT with appropriate scopes instead ofsecrets.GITHUB_TOKEN.
| validate_version "${{ inputs.apimgtVersion }}" "carbon-apimgt" | ||
| validate_version "${{ inputs.kernelVersion }}" "carbon-kernel" | ||
| validate_version "${{ inputs.apimAppsVersion }}" "apim-apps" | ||
| validate_version "${{ inputs.synapseVersion }}" "wso2-synapse" |
There was a problem hiding this comment.
Script injection via ${{ inputs.* }} — use environment variables instead.
${{ inputs.apimgtVersion }} and the other input expressions are interpolated directly into the shell script by the workflow engine before bash runs. A specially crafted input (e.g., containing $(…) or backticks) executes arbitrary commands, bypassing the validate_version check entirely.
Pass inputs through env: and reference $VAR in the script:
Proposed fix
- name: Validate inputs
shell: bash
+ env:
+ INPUT_APIMGT: ${{ inputs.apimgtVersion }}
+ INPUT_KERNEL: ${{ inputs.kernelVersion }}
+ INPUT_APPS: ${{ inputs.apimAppsVersion }}
+ INPUT_SYNAPSE: ${{ inputs.synapseVersion }}
run: |
set -euo pipefail
validate_version() {
local version="$1"
local name="$2"
if [[ -n "$version" ]] && [[ ! "$version" =~ ^[0-9]+\.[0-9]+\.[0-9]+(-.*)?$ ]]; then
echo "::error::Invalid version format for $name: $version"
exit 1
fi
}
- validate_version "${{ inputs.apimgtVersion }}" "carbon-apimgt"
- validate_version "${{ inputs.kernelVersion }}" "carbon-kernel"
- validate_version "${{ inputs.apimAppsVersion }}" "apim-apps"
- validate_version "${{ inputs.synapseVersion }}" "wso2-synapse"
+ validate_version "$INPUT_APIMGT" "carbon-apimgt"
+ validate_version "$INPUT_KERNEL" "carbon-kernel"
+ validate_version "$INPUT_APPS" "apim-apps"
+ validate_version "$INPUT_SYNAPSE" "wso2-synapse"The same pattern must be applied everywhere ${{ inputs.* }} appears inside run: blocks — notably the "Resolve dependency versions" step (lines 181, 184, 187, 190).
🤖 Prompt for AI Agents
In @.github/workflows/dependency-upgrade.yaml around lines 114 - 117, The
workflow currently injects inputs directly into run scripts (e.g., uses `${{
inputs.apimgtVersion }}` when calling validate_version), which allows script
injection; change these run steps to pass inputs via env: (e.g., set
APIMGT_VERSION: ${{ inputs.apimgtVersion }}) and then reference the value inside
the script as $APIMGT_VERSION when calling validate_version (and do the same for
KERNEL_VERSION, APIMAPPS_VERSION, SYNAPSE_VERSION); apply this pattern
everywhere `${{ inputs.* }}` is used inside run: blocks (including the "Resolve
dependency versions" step that calls validate_version) so the shell sees only
environment variables like $APIMGT_VERSION rather than pre-expanded input
expressions.
| sed -i -E "s/(<product[^>]*\bversion=\")([^\"]+)(\")/\1${KERNEL_VERSION}\3/g" "$f" | ||
| sed -i -E "s/(<feature[^>]*\bversion=\")([^\"]+)(\")/\1${KERNEL_VERSION}\3/g" "$f" |
There was a problem hiding this comment.
Overly broad sed replaces version= in every <product> and <feature> element.
These regexes update the version attribute on all <product …> and <feature …> tags in the carbon.product files, not only the kernel-related entries. If these XML files contain features from other components (e.g., apimgt features), their versions will be silently overwritten with the kernel version.
Consider narrowing the match — e.g., only replacing versions on elements that reference a known kernel feature ID, or using an XPath-based tool (xmlstarlet) to target specific elements.
🤖 Prompt for AI Agents
In @.github/workflows/dependency-upgrade.yaml around lines 324 - 325, The sed
commands currently match any <product> or <feature> element's version attribute
and replace it with ${KERNEL_VERSION}, which will mistakenly overwrite
non-kernel component versions; change the replacement to target only
kernel-related entries by narrowing the match (e.g., only match elements whose
id/name contains the kernel feature id or group) or switch to an XML-aware
approach such as using xmlstarlet to select specific <product> or <feature>
nodes by a known kernel identifier and update their `@version` with
${KERNEL_VERSION}; update the commands that reference sed and ${KERNEL_VERSION}
accordingly so only kernel feature/product elements are modified.
| git diff --name-only | while read -r file; do | ||
| SUMMARY+="- \`$file\`\n" | ||
| done |
There was a problem hiding this comment.
Bug: SUMMARY modifications inside while loop are lost due to pipe-induced subshell.
git diff --name-only | while read … runs the while body in a subshell. Assignments to SUMMARY inside the loop never propagate back to the parent shell. The "Modified Files" section of the PR body will always be empty.
Use process substitution or command substitution instead:
Proposed fix
SUMMARY+="### Modified Files\n"
- git diff --name-only | while read -r file; do
+ while read -r file; do
SUMMARY+="- \`$file\`\n"
- done
+ done < <(git diff --name-only)
SUMMARY+="\n"📝 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.
| git diff --name-only | while read -r file; do | |
| SUMMARY+="- \`$file\`\n" | |
| done | |
| SUMMARY+="### Modified Files\n" | |
| while read -r file; do | |
| SUMMARY+="- \`$file\`\n" | |
| done < <(git diff --name-only) | |
| SUMMARY+="\n" |
🤖 Prompt for AI Agents
In @.github/workflows/dependency-upgrade.yaml around lines 454 - 456, The
SUMMARY variable is being updated inside a subshell because of the pipeline "git
diff --name-only | while read -r file; do ... done", so assignments to SUMMARY
never reach the parent shell; change the loop to avoid the pipe-induced subshell
(e.g., feed git diff output into the while loop via process substitution or
input redirection) so that updates to SUMMARY inside the loop (the block that
appends "- \`$file\`") persist after the loop completes.
This PR adds an automated GitHub Actions workflow that performs weekly (or manually triggered) dependency checks for carbon-apimgt and carbon-kernel. When newer versions are available, the workflow updates the corresponding pom.xml, carbon.product, and filter.properties files across all modules and automatically opens or updates pull requests with the changes. This streamlines version maintenance and reduces manual effort.
Summary by CodeRabbit