Skip to content

Conversation

@jmle
Copy link
Collaborator

@jmle jmle commented Nov 12, 2025

Bring in always the latest version of the generated maven index data

Summary by CodeRabbit

  • Chores
    • Updated the build process to dynamically retrieve the latest maven-index-data release from GitHub instead of using a fixed version, ensuring more current data is available in deployments.

Signed-off-by: Juan Manuel Leflet Estrada <[email protected]>
@jmle jmle requested a review from aufi November 12, 2025 15:14
@coderabbitai
Copy link

coderabbitai bot commented Nov 12, 2025

Walkthrough

The Dockerfile's maven-index-data download mechanism is updated from a hardcoded v0.0.1 version reference to a dynamic retrieval system that queries GitHub's releases API for the latest available version, extracts the download URL, and downloads accordingly.

Changes

Cohort / File(s) Summary
Dockerfile Dynamic Maven-Index Retrieval
Dockerfile
Replaced hardcoded maven-index-data v0.0.1 download with GitHub releases API query to fetch and download the latest release version dynamically.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~8 minutes

  • Verify GitHub API query syntax and reliability for extracting latest release download URL
  • Confirm URL parsing and extraction logic handles edge cases (malformed responses, network issues)
  • Validate that cleanup steps (unzip, remove temporary files) execute correctly with dynamic URLs
  • Test that the Docker build succeeds and results in the same final state as the hardcoded approach

Possibly related PRs

  • ✨ Bring in maven index from release #158: Introduced the initial maven-index-data download stage in the Dockerfile; this PR makes the version retrieval dynamic via GitHub releases API rather than hardcoding a specific version.

Suggested reviewers

  • aufi
  • shawn-hurley
  • pranavgaikwad

Poem

🐰 No more versions carved in stone,
The API speaks—the latest is known!
GitHub whispers secrets so fine,
Docker builds with releases that shine,
Dynamic magic, one command at a time! ✨

Pre-merge checks and finishing touches

✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly and accurately summarizes the main change: updating to use the latest maven-index data version instead of a hardcoded one.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
✨ Finishing touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🧹 Nitpick comments (1)
Dockerfile (1)

31-34: Consider using jq for robust JSON parsing.

Parsing GitHub's JSON API response with grep and sed is fragile and error-prone. If the response format changes or contains unexpected characters, extraction may fail silently. The container already installs zip; consider adding jq for reliable JSON parsing. While the current regex may work for typical responses, jq provides better maintainability and resilience.

Example refactor using jq:

 FROM registry.access.redhat.com/ubi9/ubi-minimal AS index-download
-RUN microdnf install -y wget zip && microdnf clean all && rm -rf /var/cache/dnf
+RUN microdnf install -y wget zip jq && microdnf clean all && rm -rf /var/cache/dnf
 WORKDIR /maven-index-data
-RUN DOWNLOAD_URL=$(wget --quiet -O - https://api.github.com/repos/konveyor/maven-search-index/releases/latest | grep '"browser_download_url".*maven-index-data.*\.zip' | sed -E 's/.*"browser_download_url": "([^"]+)".*/\1/') && \
+RUN DOWNLOAD_URL=$(wget --quiet -O - https://api.github.com/repos/konveyor/maven-search-index/releases/latest | jq -r '.assets[] | select(.name | contains("maven-index-data") and endswith(".zip")) | .browser_download_url' | head -1) && \
+    test -n "$DOWNLOAD_URL" || (echo "Failed to extract download URL"; exit 1) && \
     wget --quiet ${DOWNLOAD_URL} -O maven-index-data.zip && \
     unzip maven-index-data.zip && \
     rm maven-index-data.zip
📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between be3f65c and 7f30e3b.

📒 Files selected for processing (1)
  • Dockerfile (1 hunks)

Comment on lines +31 to +34
RUN DOWNLOAD_URL=$(wget --quiet -O - https://api.github.com/repos/konveyor/maven-search-index/releases/latest | grep '"browser_download_url".*maven-index-data.*\.zip' | sed -E 's/.*"browser_download_url": "([^"]+)".*/\1/') && \
wget --quiet ${DOWNLOAD_URL} -O maven-index-data.zip && \
unzip maven-index-data.zip && \
rm maven-index-data.zip
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical

Add error handling for API query and URL extraction failures.

If the GitHub API call fails, the grep doesn't match, or sed fails to extract a valid URL, DOWNLOAD_URL will be empty. The subsequent wget ${DOWNLOAD_URL} will execute with no argument, leading to silent failures or undefined behavior. Docker RUN instructions do not enable set -e by default, so failures in the grep/sed pipeline won't halt the build—the build will succeed despite a missing download.

Apply this diff to add explicit error handling:

 RUN DOWNLOAD_URL=$(wget --quiet -O - https://api.github.com/repos/konveyor/maven-search-index/releases/latest | grep '"browser_download_url".*maven-index-data.*\.zip' | sed -E 's/.*"browser_download_url": "([^"]+)".*/\1/') && \
+    if [ -z "$DOWNLOAD_URL" ]; then echo "Failed to extract download URL from GitHub API"; exit 1; fi && \
     wget --quiet ${DOWNLOAD_URL} -O maven-index-data.zip && \
     unzip maven-index-data.zip && \
     rm maven-index-data.zip

Alternatively, use a more robust approach:

-RUN DOWNLOAD_URL=$(wget --quiet -O - https://api.github.com/repos/konveyor/maven-search-index/releases/latest | grep '"browser_download_url".*maven-index-data.*\.zip' | sed -E 's/.*"browser_download_url": "([^"]+)".*/\1/') && \
+RUN DOWNLOAD_URL=$(wget --quiet -O - https://api.github.com/repos/konveyor/maven-search-index/releases/latest | grep -o '"browser_download_url":"[^"]*maven-index-data[^"]*\.zip[^"]*"' | head -1 | cut -d'"' -f4) && \
+    test -n "$DOWNLOAD_URL" || (echo "Failed to extract download URL"; exit 1) && \
     wget --quiet ${DOWNLOAD_URL} -O maven-index-data.zip && \
     unzip maven-index-data.zip && \
     rm maven-index-data.zip
📝 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.

Suggested change
RUN DOWNLOAD_URL=$(wget --quiet -O - https://api.github.com/repos/konveyor/maven-search-index/releases/latest | grep '"browser_download_url".*maven-index-data.*\.zip' | sed -E 's/.*"browser_download_url": "([^"]+)".*/\1/') && \
wget --quiet ${DOWNLOAD_URL} -O maven-index-data.zip && \
unzip maven-index-data.zip && \
rm maven-index-data.zip
RUN DOWNLOAD_URL=$(wget --quiet -O - https://api.github.com/repos/konveyor/maven-search-index/releases/latest | grep '"browser_download_url".*maven-index-data.*\.zip' | sed -E 's/.*"browser_download_url": "([^"]+)".*/\1/') && \
if [ -z "$DOWNLOAD_URL" ]; then echo "Failed to extract download URL from GitHub API"; exit 1; fi && \
wget --quiet ${DOWNLOAD_URL} -O maven-index-data.zip && \
unzip maven-index-data.zip && \
rm maven-index-data.zip
🤖 Prompt for AI Agents
In Dockerfile around lines 31 to 34, the RUN pipeline that queries the GitHub
API and extracts a download URL can produce an empty DOWNLOAD_URL (if the API
call, grep, or sed fails) which then causes wget to run with no argument and the
build to continue silently; update this block to explicitly fail the build on
any error by enabling errexit for the command (or test the extracted variable),
validate that DOWNLOAD_URL is non-empty before calling wget (exit with a clear
error message if empty), and prefer using a robust API parsing command (e.g.,
curl/wget with non-zero-exit check and jq to extract the browser_download_url)
so that any HTTP or parsing failure stops the Docker build and surfaces a
helpful error.

@jmle jmle requested a review from eemcmullan November 12, 2025 15:18
@jmle jmle added the cherry-pick/release-0.8 This PR should be cherry-picked to release-0.8 branch label Nov 12, 2025
@jmle jmle merged commit 7e0cb5d into konveyor:main Nov 12, 2025
5 of 8 checks passed
@jmle
Copy link
Collaborator Author

jmle commented Nov 12, 2025

Quality, license and security are reporting back as incomplete, but inner status is green. Merging.

github-actions bot pushed a commit that referenced this pull request Nov 12, 2025
Signed-off-by: Juan Manuel Leflet Estrada <[email protected]>
Signed-off-by: Cherry Picker <[email protected]>
jmle added a commit that referenced this pull request Nov 12, 2025
Signed-off-by: Juan Manuel Leflet Estrada <[email protected]>
Signed-off-by: Cherry Picker <[email protected]>
Co-authored-by: Juan Manuel Leflet Estrada <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

cherry-pick/release-0.8 This PR should be cherry-picked to release-0.8 branch

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants