Skip to content

Conversation

@shawn-hurley
Copy link
Contributor

@shawn-hurley shawn-hurley commented Nov 11, 2025

  • This will save alot of time downloading maven artifacts

Summary by CodeRabbit

  • Chores
    • Optimized build process configuration for improved build performance.

* This will save alot of time downloading maven artifacts

Signed-off-by: Shawn Hurley <shawn@hurley.page>
@coderabbitai
Copy link

coderabbitai bot commented Nov 11, 2025

Walkthrough

The Dockerfile's addon-build stage was optimized by replacing Maven's install goal with package, adding a cache mount for the Maven repository at /root/.m2, and updating the COPY instruction to pull artifacts directly from the app's target directory instead of the Maven repository.

Changes

Cohort / File(s) Summary
Build Optimization
Dockerfile
Changed Maven build from clean install to clean package with added repository cache mount (--mount=type=cache,id=m2_repo,uid=1001,target=/root/.m2); updated COPY path from Maven repository (/root/.m2/repository/...) to app target directory (/app/java-analyzer-bundle.core/target/...)

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

  • Single file modification with straightforward build optimization changes
  • Cache mount configuration and path updates are relatively low-risk changes
  • Verify that the new artifact path correctly references the Maven package output location
  • Confirm cache mount parameters (uid, target path) align with container user configuration

Suggested reviewers

  • pranavgaikwad
  • eemcmullan
  • aufi

Poem

🐰 A cache mount appears, swift as spring rain,
From install to package, Maven's refrain,
Direct paths now flow where repos once lay,
Builds faster they dance, hip-hop hooray! 🎉

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 accurately describes the main change: adding Maven cache mount capability to the Dockerfile build process to optimize build performance.
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)

25-26: Simplify redundant JAVA_HOME setup.

Line 25 sets JAVA_HOME as a RUN environment variable, but this only applies to that specific RUN command and is immediately followed by another RUN command that re-sets JAVA_HOME inline on line 26. Consider consolidating these into a single ENV directive to avoid repetition.

Apply this diff to clean up the redundancy:

-RUN export JAVA_HOME=/usr/lib/jvm/java-17-openjdk
-RUN --mount=type=cache,id=m2_repo,uid=1001,target=/root/.m2 JAVA_HOME=/usr/lib/jvm/java-17-openjdk mvn clean package -DskipTests=true
+ENV JAVA_HOME=/usr/lib/jvm/java-17-openjdk
+RUN --mount=type=cache,id=m2_repo,uid=1001,target=/root/.m2 mvn clean package -DskipTests=true
📜 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 e193492.

📒 Files selected for processing (1)
  • Dockerfile (2 hunks)
🔇 Additional comments (1)
Dockerfile (1)

52-52: LGTM: COPY path correctly updated for package goal.

The COPY instruction now pulls the packaged jar from /app/java-analyzer-bundle.core/target/, which is the correct output location when using mvn clean package instead of install.

COPY ./ /app/
RUN export JAVA_HOME=/usr/lib/jvm/java-17-openjdk
RUN JAVA_HOME=/usr/lib/jvm/java-17-openjdk mvn clean install -DskipTests=true
RUN --mount=type=cache,id=m2_repo,uid=1001,target=/root/.m2 JAVA_HOME=/usr/lib/jvm/java-17-openjdk mvn clean package -DskipTests=true
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

🧩 Analysis chain

Verify cache mount uid and assess Maven goal change.

The cache mount specifies uid=1001, but RUN commands execute as root (uid 0) by default. This uid mismatch may cause permission issues or prevent effective caching. Verify that the uid is intentional and correct.

Additionally, the change from mvn clean install to mvn clean package is a functional change: install writes artifacts to the local repository, while package only builds the jar. This optimization works if downstream stages only need the packaged jar (which line 52 confirms), but verify there are no other consumers expecting artifacts in /root/.m2/repository.

To verify the cache mount configuration, please confirm:

  • Is the Maven process expected to run as uid=1001, or should this be uid=0 for the root user?
  • Is there any dependency on artifacts being installed to the local Maven repository after the build?

🏁 Script executed:

#!/bin/bash
# Description: Verify Maven process user and check for any references to Maven repository artifacts

# Check if there are any references to the .m2 repository in the build process
rg -n "\.m2/repository" --type dockerfile

# Check for any USER directives in addon-build stage
rg -n "FROM registry.access.redhat.com/ubi9/ubi AS addon-build" -A 20 | grep -i "USER"

Length of output: 34


Let me adjust the search strategy to examine the Dockerfile more broadly.

Let me adjust the search strategy to examine the Dockerfile without relying on explicit file type detection.


🏁 Script executed:

#!/bin/bash
# Description: Examine Dockerfile structure and verify cache/Maven configuration

# Find and display the entire Dockerfile
echo "=== Full Dockerfile content ==="
fd -e Dockerfile -x cat

echo -e "\n=== Searching for .m2/repository references ==="
fd -e Dockerfile -x rg "\.m2/repository" -n

echo -e "\n=== Searching for USER directives ==="
fd -e Dockerfile -x rg "USER" -n -C 2

echo -e "\n=== Examining lines around cache mount and COPY (lines 20-30 and 50-55) ==="
fd -e Dockerfile -x sed -n '20,30p;50,55p'

Length of output: 196


🏁 Script executed:

#!/bin/bash
# Description: Examine Dockerfile structure and verify cache/Maven configuration

# Find and display the entire Dockerfile
echo "=== Full Dockerfile content ==="
fd -name Dockerfile -x cat

echo -e "\n=== Searching for .m2/repository references ==="
fd -name Dockerfile -x rg "\.m2/repository" -n

echo -e "\n=== Searching for USER directives ==="
fd -name Dockerfile -x rg "USER" -n -C 2

echo -e "\n=== Examining specific lines around cache mount and COPY ==="
fd -name Dockerfile -x sed -n '20,30p;50,55p' | cat -n

Length of output: 1105


Let me simplify the approach and use more basic commands.


🏁 Script executed:

#!/bin/bash
# Description: Examine Dockerfile with simpler syntax

echo "=== Locating Dockerfile ==="
find . -name "Dockerfile" -type f

echo -e "\n=== Full Dockerfile content ==="
find . -name "Dockerfile" -type f -exec cat {} \;

echo -e "\n=== Searching for .m2/repository references ==="
find . -name "Dockerfile" -type f -exec grep -n "\.m2/repository" {} +

echo -e "\n=== Searching for USER directives ==="
find . -name "Dockerfile" -type f -exec grep -n "USER" {} +

echo -e "\n=== Lines 20-55 from Dockerfile for context ==="
find . -name "Dockerfile" -type f -exec sed -n '20,55p' {} \;

Length of output: 6761


Fix cache mount uid mismatch—RUN commands execute as root, not uid 1001.

The cache mount specifies uid=1001, but the addon-build stage runs as root (uid 0) by default—no USER directive is set. This mismatch will prevent effective caching or cause permission issues. Change the cache mount to uid=0 to match the process user:

RUN --mount=type=cache,id=m2_repo,uid=0,target=/root/.m2 JAVA_HOME=/usr/lib/jvm/java-17-openjdk mvn clean package -DskipTests=true

The Maven goal change from install to package is correct—the subsequent COPY confirms only the jar artifact from target/ is needed, not repository artifacts.

🤖 Prompt for AI Agents
In Dockerfile around line 26, the cache mount uses uid=1001 but the RUN executes
as root (uid 0), causing permission/cache issues; change the mount's uid to 0
(or make the stage run as uid 1001) so the cache ownership matches the process
user, e.g., set uid=0 on the --mount or add a USER directive to run the stage as
1001, and keep the Maven goal as package since only the jar artifact is copied
later.

Copy link
Member

@aufi aufi left a comment

Choose a reason for hiding this comment

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

I'm not sure this is safe to do in full Konveyor installation by default.

While understanding performance benefits, there could be user roles&credentials for applications setup in Hub where sharing maven cache could expose artifacts between applications, which I'm not sure is wanted.

Keeping it as a comment in case I'm missed something and it is safe to do this.

@shawn-hurley
Copy link
Contributor Author

@aufi This only uses the build cache when you are building images in GitHub Actions, and I don't think it has anything to do with the Maven cache when running as a task pod in the hub.

Unless I am missing something, this should not be a problem.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants