Skip to content

Conversation

ChaosYao
Copy link

@ChaosYao ChaosYao commented Sep 7, 2025

Motivation:

Explain the context, and why you're making that change.
To make others understand what is the problem you're trying to solve.

Modification:

Describe the idea and modifications you've done.

Result:

Fixes #.

If there is no issue then describe the changes introduced by this PR.

Summary by CodeRabbit

  • New Features

    • Added Docker-based container image and runtime with environment-configurable defaults.
    • Added Docker Compose for easy local startup (app reachable on port 8008).
    • Added Maven Wrapper tooling (cross-platform launcher and downloader) for consistent builds.
  • Documentation

    • Added Docker guide with local run and cloud deployment instructions, including multi-arch notes.
  • Chores

    • Added .dockerignore to reduce Docker build context and improve build efficiency.

Copy link

sofastack-cla bot commented Sep 7, 2025

Hi @ChaosYao, welcome to SOFAStack community, Please sign Contributor License Agreement!

After you signed CLA, we will automatically sync the status of this pull request in 3 minutes.

Copy link
Contributor

coderabbitai bot commented Sep 7, 2025

Walkthrough

Adds Docker build and compose artifacts, a root .dockerignore, Maven Wrapper scripts (mvnw, mvnw.cmd) and properties, and a Java-based Maven wrapper downloader utility; the Dockerfile builds the jraft-example module and produces a runtime image running /app.jar.

Changes

Cohort / File(s) Summary
Docker build & docs
Dockerfile, .dockerignore, README.Docker.md, compose.yaml
Introduces a two-stage Dockerfile that builds the jraft-example module and produces /app.jar for a JRE runtime; adds a root .dockerignore; adds compose.yaml with service mapping port 8008 and commented DB example; adds README.Docker.md with local and registry deployment instructions.
Maven Wrapper scripts & config
mvnw, mvnw.cmd, .mvn/wrapper/maven-wrapper.properties
Adds cross-platform Maven Wrapper launchers for Unix and Windows; configures distributionUrl and wrapperUrl (Alibaba mirror) in .mvn/wrapper/maven-wrapper.properties; includes logic to download wrapper JAR when missing.
Maven Wrapper downloader (Java)
.mvn/wrapper/MavenWrapperDownloader.java
New CLI utility to download maven-wrapper.jar into .mvn/wrapper; reads optional wrapperUrl property, supports HTTP basic auth via MVNW_USERNAME/MVNW_PASSWORD, logs progress, and exits with success/failure codes.

Sequence Diagram(s)

sequenceDiagram
  autonumber
  actor Dev as Developer
  participant Build as Docker Build
  participant Maven as Maven (mvnw)
  participant Image as Runtime Image
  Dev->>Build: docker build .
  Build->>Maven: run mvnw -B -DskipTests -pl jraft-example -am package (cache /root/.m2)
  Maven-->>Build: produce jraft-example/target/${FINAL_NAME}.jar
  Build->>Image: COPY --from=build /app.jar
  Dev->>Image: docker run / compose up --build
  Image-->>Dev: runs java -jar /app.jar (exposes 8081 / 8008 mapping)
Loading
sequenceDiagram
  autonumber
  actor User as Invoker (mvnw / mvnw.cmd)
  participant Script as Wrapper Script
  participant FS as .mvn/wrapper
  participant Downloader as Downloader (curl/wget/PowerShell/Java)
  Script->>FS: check for maven-wrapper.jar
  alt jar present
    Script->>Script: exec java -cp maven-wrapper.jar org.apache.maven.wrapper.MavenWrapperMain
  else jar missing
    Script->>Downloader: request wrapperUrl (from properties or default)
    Downloader-->>FS: write maven-wrapper.jar
    Script->>Script: exec wrapper with downloaded jar
  end
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

Poem

I nibble strings and fetch the jar, 🐇
A Docker den to carry far.
Compose ties ports, the build runs neat,
Wrapper hops in to make builds complete.
Little paws drum — the image is fleet.

✨ Finishing Touches
🧪 Generate unit tests
  • 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.

@ChaosYao
Copy link
Author

ChaosYao commented Sep 7, 2025

OK

Copy link
Contributor

@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: 2

🧹 Nitpick comments (7)
.dockerignore (1)

26-33: Don’t ignore your own build/compose descriptors.

Ignoring Dockerfile*, compose.* and docs is surprising and may confuse future contributors. Recommend keeping these in context; they’re tiny and occasionally copied into images.

Apply this diff to drop those patterns:

-**/docker-compose*
-**/compose.y*ml
-**/Dockerfile*
-LICENSE
-README.md

Also applies to: 28-28

Dockerfile (2)

52-56: Minor doc typo.

“eclipse-turmin” → “eclipse-temurin”.

-# The example below uses eclipse-turmin's JRE image as the foundation for running the app.
+# The example below uses eclipse-temurin's JRE image as the foundation for running the app.

21-25: Dependency caching may be incomplete for multi-module builds.

Binding only the root pom.xml often isn’t enough for go-offline; child module POMs aren’t visible, so deps won’t fully cache. Not a blocker (the package stage will fetch what’s missing), but it weakens caching.

Consider copying all pom.xml files first (root + modules) before running go-offline, then copying the rest of the sources.

Also applies to: 39-43

README.Docker.md (2)

6-6: Fix markdownlint: wrap the bare URL.

-Your application will be available at http://localhost:8008.
+Your application will be available at <http://localhost:8008>.

1-19: Clarify that a runnable module and port 8008 are assumptions.

This repository may not expose an HTTP server by default. Note that users must pick a runnable module and align EXPOSE/compose port.

-### Building and running your application
+### Building and running your application
+Note: Ensure the project builds a runnable JAR (e.g., via a demo/example module) and that it listens on the port exposed in Dockerfile (default 8008). Update the port if your module uses a different one.
compose.yaml (2)

11-16: Tag the built image for reuse and CI caching.

Optional but useful for local dev and registries.

   server:
-    build:
-      context: .
+    image: sofastack/sofa-jraft:dev
+    build:
+      context: .
     ports:
       - 8008:8008

21-23: Use Compose V2 syntax in comments for consistency.

-# a password of your choosing to it before running `docker-compose up`.
+# a password of your choosing to it before running `docker compose up`.
📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between ad9f79e and d76f5be.

📒 Files selected for processing (4)
  • .dockerignore (1 hunks)
  • Dockerfile (1 hunks)
  • README.Docker.md (1 hunks)
  • compose.yaml (1 hunks)
🧰 Additional context used
🪛 markdownlint-cli2 (0.17.2)
README.Docker.md

6-6: Bare URL used

(MD034, no-bare-urls)

⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
  • GitHub Check: sca
🔇 Additional comments (3)
.dockerignore (1)

7-62: Overall patterns look good.

Solid coverage of VCS, IDE, and build artifacts; keeps context lean.

Dockerfile (1)

57-71: Confirm Dockerfile targets a runnable uber-jar listening on 8008

  • This is an aggregator project (packaging pom) with modules including jraft-example, whose POM only configures the standard maven-jar-plugin (no shade or Spring Boot plugin) and may not produce a fat JAR. Ensure the Dockerfile builds and runs the example module’s executable JAR (or add a shading/boot plugin) and that the application actually binds to port 8008 (see Dockerfile 57–71, 75–77).
compose.yaml (1)

10-16: Confirm the exposed port matches the actual app.

If the runnable module doesn’t bind to 8008, adjust here and in the Dockerfile.

@sofastack-cla sofastack-cla bot added size/XL and removed size/L labels Sep 8, 2025
Copy link
Contributor

@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: 7

🧹 Nitpick comments (3)
.mvn/wrapper/MavenWrapperDownloader.java (1)

48-52: Guard against missing argv[0]

Add a quick args-length check to avoid ArrayIndexOutOfBounds if invoked incorrectly.

Apply:

-    File baseDirectory = new File(args[0]);
+    if (args == null || args.length == 0) {
+        System.err.println("Usage: MavenWrapperDownloader <baseDirectory>");
+        System.exit(1);
+    }
+    File baseDirectory = new File(args[0]);
mvnw (2)

231-239: Harden wget path with timeouts/retries

Improve resilience and reduce hanging in CI.

Apply:

-        if [ -z "$MVNW_USERNAME" ] || [ -z "$MVNW_PASSWORD" ]; then
-            wget "$jarUrl" -O "$wrapperJarPath"
-        else
-            wget --http-user=$MVNW_USERNAME --http-password=$MVNW_PASSWORD "$jarUrl" -O "$wrapperJarPath"
+        if [ -z "$MVNW_USERNAME" ] || [ -z "$MVNW_PASSWORD" ]; then
+            wget -q --timeout=15 --tries=3 "$jarUrl" -O "$wrapperJarPath"
+        else
+            wget -q --timeout=15 --tries=3 --http-user="$MVNW_USERNAME" --http-password="$MVNW_PASSWORD" "$jarUrl" -O "$wrapperJarPath"
         fi

244-248: Harden curl path with fail-fast, redirects, and timeouts

Adds safer defaults and better error propagation.

Apply:

-            curl -o "$wrapperJarPath" "$jarUrl" -f
+            curl -fsSL --connect-timeout 15 --retry 3 -o "$wrapperJarPath" "$jarUrl"
         else
-            curl --user $MVNW_USERNAME:$MVNW_PASSWORD -o "$wrapperJarPath" "$jarUrl" -f
+            curl -fsSL --connect-timeout 15 --retry 3 --user "$MVNW_USERNAME:$MVNW_PASSWORD" -o "$wrapperJarPath" "$jarUrl"
         fi
📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between d76f5be and 036bdc9.

⛔ Files ignored due to path filters (1)
  • .mvn/wrapper/maven-wrapper.jar is excluded by !**/*.jar
📒 Files selected for processing (4)
  • .mvn/wrapper/MavenWrapperDownloader.java (1 hunks)
  • .mvn/wrapper/maven-wrapper.properties (1 hunks)
  • mvnw (1 hunks)
  • mvnw.cmd (1 hunks)
✅ Files skipped from review due to trivial changes (1)
  • .mvn/wrapper/maven-wrapper.properties
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
  • GitHub Check: sca
🔇 Additional comments (3)
.mvn/wrapper/MavenWrapperDownloader.java (1)

53-64: LGTM: wrapperUrl override logic is correct

Respects project-configured URL when present; sensible fallback otherwise.

mvnw.cmd (1)

60-80: LGTM: robust JAVA_HOME validation

Clear diagnostics and early exit are helpful for Windows users.

mvnw (1)

206-214: LGTM: graceful fallback to auto-download when wrapper JAR is absent

Good UX: logs verbosely when enabled and avoids hard-failing on missing binary.

Comment on lines +23 to +29
private static final String WRAPPER_VERSION = "0.5.6";
/**
* Default URL to download the maven-wrapper.jar from, if no 'downloadUrl' is provided.
*/
private static final String DEFAULT_DOWNLOAD_URL = "https://repo.maven.apache.org/maven2/io/takari/maven-wrapper/"
+ WRAPPER_VERSION + "/maven-wrapper-" + WRAPPER_VERSION + ".jar";

Copy link
Contributor

Choose a reason for hiding this comment

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

💡 Verification agent

🧩 Analysis chain

Outdated Takari wrapper default; consider upgrading to Apache Maven Wrapper 3.x

0.5.6 is old and lacks built-in hardening (checksum verification, better mirrors). Prefer regenerating scripts via the Maven Wrapper Plugin (Apache 3.x) rather than maintaining this downloader.


🌐 Web query:

What is the latest Apache Maven Wrapper release, and does it support verifying wrapper and distribution checksums out of the box?

💡 Result:

Latest Apache Maven Wrapper release: 3.3.3. [1]

Yes — it supports verifying both the wrapper JAR and the downloaded Maven distribution out of the box. Add SHA‑256 sums (hex, lowercase) to maven-wrapper.properties using the keys wrapperSha256Sum and distributionSha256Sum to enable verification. [2][1]

Sources:
[1] Apache Maven Wrapper download (3.3.3).
[2] Apache Maven Wrapper documentation — checksum verification instructions.


Upgrade to Apache Maven Wrapper 3.3.3 and enable checksum verification

  • Update WRAPPER_VERSION in .mvn/wrapper/MavenWrapperDownloader.java from "0.5.6" to "3.3.3"
  • Add wrapperSha256Sum and distributionSha256Sum entries to maven-wrapper.properties for built-in SHA-256 verification
🤖 Prompt for AI Agents
.mvn/wrapper/MavenWrapperDownloader.java lines 23-29: the project is still
referencing the old wrapper version "0.5.6" and the repo needs checksum
verification enabled; update WRAPPER_VERSION to "3.3.3" in this file and then
add wrapperSha256Sum and distributionSha256Sum entries to the
.mvn/wrapper/maven-wrapper.properties file (use the official SHA-256 sums for
the maven-wrapper jar and the Maven distribution you want) so the wrapper
performs built-in SHA-256 verification when downloading artifacts.

Comment on lines +53 to +75
// If the maven-wrapper.properties exists, read it and check if it contains a custom
// wrapperUrl parameter.
File mavenWrapperPropertyFile = new File(baseDirectory, MAVEN_WRAPPER_PROPERTIES_PATH);
String url = DEFAULT_DOWNLOAD_URL;
if(mavenWrapperPropertyFile.exists()) {
FileInputStream mavenWrapperPropertyFileInputStream = null;
try {
mavenWrapperPropertyFileInputStream = new FileInputStream(mavenWrapperPropertyFile);
Properties mavenWrapperProperties = new Properties();
mavenWrapperProperties.load(mavenWrapperPropertyFileInputStream);
url = mavenWrapperProperties.getProperty(PROPERTY_NAME_WRAPPER_URL, url);
} catch (IOException e) {
System.out.println("- ERROR loading '" + MAVEN_WRAPPER_PROPERTIES_PATH + "'");
} finally {
try {
if(mavenWrapperPropertyFileInputStream != null) {
mavenWrapperPropertyFileInputStream.close();
}
} catch (IOException e) {
// Ignore ...
}
}
}
Copy link
Contributor

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Plumb expected SHA-256 from properties (wrapperSha256Sum)

Read an optional checksum from maven-wrapper.properties for later verification.

Apply:

         File mavenWrapperPropertyFile = new File(baseDirectory, MAVEN_WRAPPER_PROPERTIES_PATH);
-        String url = DEFAULT_DOWNLOAD_URL;
+        String url = DEFAULT_DOWNLOAD_URL;
+        String expectedSha256 = null;
         if(mavenWrapperPropertyFile.exists()) {
             FileInputStream mavenWrapperPropertyFileInputStream = null;
             try {
                 mavenWrapperPropertyFileInputStream = new FileInputStream(mavenWrapperPropertyFile);
                 Properties mavenWrapperProperties = new Properties();
                 mavenWrapperProperties.load(mavenWrapperPropertyFileInputStream);
                 url = mavenWrapperProperties.getProperty(PROPERTY_NAME_WRAPPER_URL, url);
+                expectedSha256 = mavenWrapperProperties.getProperty("wrapperSha256Sum");

And add this constant near Line 46:

-    private static final String PROPERTY_NAME_WRAPPER_URL = "wrapperUrl";
+    private static final String PROPERTY_NAME_WRAPPER_URL = "wrapperUrl";
+    private static final String PROPERTY_NAME_WRAPPER_SHA256 = "wrapperSha256Sum";
📝 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
// If the maven-wrapper.properties exists, read it and check if it contains a custom
// wrapperUrl parameter.
File mavenWrapperPropertyFile = new File(baseDirectory, MAVEN_WRAPPER_PROPERTIES_PATH);
String url = DEFAULT_DOWNLOAD_URL;
if(mavenWrapperPropertyFile.exists()) {
FileInputStream mavenWrapperPropertyFileInputStream = null;
try {
mavenWrapperPropertyFileInputStream = new FileInputStream(mavenWrapperPropertyFile);
Properties mavenWrapperProperties = new Properties();
mavenWrapperProperties.load(mavenWrapperPropertyFileInputStream);
url = mavenWrapperProperties.getProperty(PROPERTY_NAME_WRAPPER_URL, url);
} catch (IOException e) {
System.out.println("- ERROR loading '" + MAVEN_WRAPPER_PROPERTIES_PATH + "'");
} finally {
try {
if(mavenWrapperPropertyFileInputStream != null) {
mavenWrapperPropertyFileInputStream.close();
}
} catch (IOException e) {
// Ignore ...
}
}
}
// ------------------------------------------------------------------------
// Constants
// ------------------------------------------------------------------------
private static final String PROPERTY_NAME_WRAPPER_URL = "wrapperUrl";
private static final String PROPERTY_NAME_WRAPPER_SHA256 = "wrapperSha256Sum";
// ...
// If the maven-wrapper.properties exists, read it and check if it contains a custom
// wrapperUrl parameter.
File mavenWrapperPropertyFile = new File(baseDirectory, MAVEN_WRAPPER_PROPERTIES_PATH);
String url = DEFAULT_DOWNLOAD_URL;
String expectedSha256 = null;
if (mavenWrapperPropertyFile.exists()) {
FileInputStream mavenWrapperPropertyFileInputStream = null;
try {
mavenWrapperPropertyFileInputStream = new FileInputStream(mavenWrapperPropertyFile);
Properties mavenWrapperProperties = new Properties();
mavenWrapperProperties.load(mavenWrapperPropertyFileInputStream);
url = mavenWrapperProperties.getProperty(PROPERTY_NAME_WRAPPER_URL, url);
expectedSha256 = mavenWrapperProperties.getProperty(PROPERTY_NAME_WRAPPER_SHA256);
} catch (IOException e) {
System.out.println("- ERROR loading '" + MAVEN_WRAPPER_PROPERTIES_PATH + "'");
} finally {
try {
if (mavenWrapperPropertyFileInputStream != null) {
mavenWrapperPropertyFileInputStream.close();
}
} catch (IOException e) {
// Ignore ...
}
}
}

Comment on lines +86 to +90
try {
downloadFileFromURL(url, outputFile);
System.out.println("Done");
System.exit(0);
} catch (Throwable e) {
Copy link
Contributor

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Pass expected checksum into downloader

Wire the value through to the download method.

Apply:

-            downloadFileFromURL(url, outputFile);
+            downloadFileFromURL(url, outputFile, expectedSha256);
📝 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
try {
downloadFileFromURL(url, outputFile);
System.out.println("Done");
System.exit(0);
} catch (Throwable e) {
try {
downloadFileFromURL(url, outputFile, expectedSha256);
System.out.println("Done");
System.exit(0);
} catch (Throwable e) {
🤖 Prompt for AI Agents
.mvn/wrapper/MavenWrapperDownloader.java around lines 86-90: the code calls
downloadFileFromURL(url, outputFile) but does not pass the expected checksum
through to the downloader; update the call to include the expected checksum
(e.g., downloadFileFromURL(url, outputFile, expectedChecksum)), ensure
expectedChecksum is obtained from the surrounding context/arguments, then update
the downloadFileFromURL method signature and all internal calls to accept and
use the checksum for verification after download.

Comment on lines +97 to +115
private static void downloadFileFromURL(String urlString, File destination) throws Exception {
if (System.getenv("MVNW_USERNAME") != null && System.getenv("MVNW_PASSWORD") != null) {
String username = System.getenv("MVNW_USERNAME");
char[] password = System.getenv("MVNW_PASSWORD").toCharArray();
Authenticator.setDefault(new Authenticator() {
@Override
protected PasswordAuthentication getPasswordAuthentication() {
return new PasswordAuthentication(username, password);
}
});
}
URL website = new URL(urlString);
ReadableByteChannel rbc;
rbc = Channels.newChannel(website.openStream());
FileOutputStream fos = new FileOutputStream(destination);
fos.getChannel().transferFrom(rbc, 0, Long.MAX_VALUE);
fos.close();
rbc.close();
}
Copy link
Contributor

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

⚠️ Potential issue

Add timeouts, try-with-resources, and SHA-256 verification

Harden network I/O and verify the downloaded JAR to mitigate supply-chain/mitM risks.

Apply:

-    private static void downloadFileFromURL(String urlString, File destination) throws Exception {
+    private static void downloadFileFromURL(String urlString, File destination, String expectedSha256) throws Exception {
         if (System.getenv("MVNW_USERNAME") != null && System.getenv("MVNW_PASSWORD") != null) {
             String username = System.getenv("MVNW_USERNAME");
             char[] password = System.getenv("MVNW_PASSWORD").toCharArray();
             Authenticator.setDefault(new Authenticator() {
                 @Override
                 protected PasswordAuthentication getPasswordAuthentication() {
                     return new PasswordAuthentication(username, password);
                 }
             });
         }
-        URL website = new URL(urlString);
-        ReadableByteChannel rbc;
-        rbc = Channels.newChannel(website.openStream());
-        FileOutputStream fos = new FileOutputStream(destination);
-        fos.getChannel().transferFrom(rbc, 0, Long.MAX_VALUE);
-        fos.close();
-        rbc.close();
+        final URL url = new URL(urlString);
+        final java.net.HttpURLConnection conn = (java.net.HttpURLConnection) url.openConnection();
+        conn.setInstanceFollowRedirects(true);
+        conn.setConnectTimeout(15_000);
+        conn.setReadTimeout(30_000);
+        final java.security.MessageDigest md = java.security.MessageDigest.getInstance("SHA-256");
+        try (InputStream in = conn.getInputStream();
+             BufferedInputStream bin = new BufferedInputStream(in);
+             FileOutputStream fos = new FileOutputStream(destination)) {
+            byte[] buf = new byte[8192];
+            int r;
+            while ((r = bin.read(buf)) != -1) {
+                md.update(buf, 0, r);
+                fos.write(buf, 0, r);
+            }
+        }
+        if (expectedSha256 != null && !expectedSha256.trim().isEmpty()) {
+            String actual = toHex(md.digest());
+            if (!actual.equalsIgnoreCase(expectedSha256.trim())) {
+                // remove bad file
+                try { destination.delete(); } catch (Throwable ignore) {}
+                throw new IOException("SHA-256 mismatch for maven-wrapper.jar. Expected "
+                    + expectedSha256 + ", got " + actual);
+            }
+        }
     }

Add this helper method anywhere below Line 115:

private static String toHex(byte[] bytes) {
    StringBuilder sb = new StringBuilder(bytes.length * 2);
    for (byte b : bytes) sb.append(String.format("%02x", b));
    return sb.toString();
}
🤖 Prompt for AI Agents
In .mvn/wrapper/MavenWrapperDownloader.java around lines 97 to 115, the
downloadFileFromURL method currently performs unprotected network I/O and file
writes without timeouts, try-with-resources, or integrity checks; update it to
use URLConnection/HttpURLConnection with connect and read timeouts, replace
manual stream/channel handling with try-with-resources for the
InputStream/ReadableByteChannel and FileOutputStream to ensure proper closing,
compute the SHA-256 digest of the downloaded bytes and verify it against an
expected checksum (throwing an exception on mismatch), and add the provided
private static String toHex(byte[] bytes) helper method below line 115 to
convert the digest to hex for comparison.

Comment on lines +215 to +223
jarUrl="$MVNW_REPOURL/io/takari/maven-wrapper/0.5.6/maven-wrapper-0.5.6.jar"
else
jarUrl="https://repo.maven.apache.org/maven2/io/takari/maven-wrapper/0.5.6/maven-wrapper-0.5.6.jar"
fi
while IFS="=" read key value; do
case "$key" in (wrapperUrl) jarUrl="$value"; break ;;
esac
done < "$BASE_DIR/.mvn/wrapper/maven-wrapper.properties"
if [ "$MVNW_VERBOSE" = true ]; then
Copy link
Contributor

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Add checksum verification or adopt Wrapper 3.x

Current script does not validate the JAR content. Either:

  • adopt Apache Maven Wrapper 3.x (recommended), or
  • read wrapperSha256Sum from .mvn/wrapper/maven-wrapper.properties and verify via sha256sum (if available) after download.

Here’s a minimal snippet (non-diff) you can insert after download:

if command -v sha256sum >/dev/null 2>&1; then
  expected="$(awk -F= '/^wrapperSha256Sum[[:space:]]*=/{print $2}' "$BASE_DIR/.mvn/wrapper/maven-wrapper.properties" | tr -d '[:space:]')"
  if [ -n "$expected" ]; then
    actual="$(sha256sum "$wrapperJarPath" | awk '{print $1}')"
    [ "${actual,,}" = "${expected,,}" ] || { echo "SHA-256 mismatch for maven-wrapper.jar"; rm -f "$wrapperJarPath"; exit 1; }
  fi
fi
🤖 Prompt for AI Agents
In mvnw around lines 215 to 223, the script downloads the maven-wrapper JAR but
does not verify its integrity; add checksum verification or upgrade to Wrapper
3.x. Fix by reading wrapperSha256Sum from
$BASE_DIR/.mvn/wrapper/maven-wrapper.properties after download and, if sha256sum
is available, compute the downloaded JAR's SHA-256 and compare
(case-insensitive) to the expected value, removing the file and exiting non‑zero
on mismatch; alternatively replace the script with the official Maven Wrapper
3.x implementation which includes secure verification.

Comment on lines +123 to +139
set DOWNLOAD_URL="https://repo.maven.apache.org/maven2/io/takari/maven-wrapper/0.5.6/maven-wrapper-0.5.6.jar"

FOR /F "tokens=1,2 delims==" %%A IN ("%MAVEN_PROJECTBASEDIR%\.mvn\wrapper\maven-wrapper.properties") DO (
IF "%%A"=="wrapperUrl" SET DOWNLOAD_URL=%%B
)

@REM Extension to allow automatically downloading the maven-wrapper.jar from Maven-central
@REM This allows using the maven wrapper in projects that prohibit checking in binary data.
if exist %WRAPPER_JAR% (
if "%MVNW_VERBOSE%" == "true" (
echo Found %WRAPPER_JAR%
)
) else (
if not "%MVNW_REPOURL%" == "" (
SET DOWNLOAD_URL="%MVNW_REPOURL%/io/takari/maven-wrapper/0.5.6/maven-wrapper-0.5.6.jar"
)
if "%MVNW_VERBOSE%" == "true" (
Copy link
Contributor

Choose a reason for hiding this comment

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

💡 Verification agent

🧩 Analysis chain

Prefer Apache Maven Wrapper 3.x scripts

Modern wrapper scripts provide checksum verification and other hardening out of the box. Regenerate via the Maven Wrapper Plugin instead of maintaining custom logic here.


🌐 Web query:

How to regenerate Apache Maven Wrapper (mvn -N wrapper:wrapper) and which properties enable checksum verification?

💡 Result:

Run the wrapper goal (no reactor) to (re)generate the files:
mvn -N wrapper:wrapper
You can pass options to set the distribution/version/type and to enable checksum verification. The two properties that enable SHA‑256 checks are:

  • wrapperSha256Sum — expected SHA‑256 of the maven-wrapper.jar
  • distributionSha256Sum — expected SHA‑256 of the downloaded Maven distribution

Example (regenerate and embed checksums):
mvn -N wrapper:wrapper -DdistributionUrl=https://repo.maven.apache.org/maven2/org/apache/maven/apache-maven/3.9.6/apache-maven-3.9.6-bin.zip -DdistributionSha256Sum= -DwrapperSha256Sum=

Sources: Apache Maven Wrapper docs (wrapper mojo + overview). [1] [2]


Use official Maven Wrapper 3.x with embedded checksum verification
Replace the custom download logic in mvnw.cmd by regenerating the wrapper via the Maven Wrapper Plugin:

mvn -N wrapper:wrapper \
  -DdistributionUrl=https://repo.maven.apache.org/maven2/org/apache/maven/apache-maven/3.9.6/apache-maven-3.9.6-bin.zip \
  -DdistributionSha256Sum=<sha256> \
  -DwrapperSha256Sum=<sha256>

This embeds SHA-256 checks for both the Maven distribution and the maven-wrapper.jar. Commit the generated scripts and remove the existing manual download logic.

🤖 Prompt for AI Agents
In mvnw.cmd around lines 123 to 139, the script contains custom download logic
for maven-wrapper.jar without embedded SHA-256 verification; replace this manual
logic by regenerating the official Maven Wrapper that includes checksum
validation. Run the Maven Wrapper Plugin command to regenerate the wrapper
scripts with distributionUrl and both distributionSha256Sum and wrapperSha256Sum
provided, then commit the generated mvnw, mvnw.cmd and .mvn/wrapper/* files and
remove the existing custom download code so the project uses the official 3.x
wrapper with built-in checksum checks.

Comment on lines +144 to +154
powershell -Command "&{"^
"$webclient = new-object System.Net.WebClient;"^
"if (-not ([string]::IsNullOrEmpty('%MVNW_USERNAME%') -and [string]::IsNullOrEmpty('%MVNW_PASSWORD%'))) {"^
"$webclient.Credentials = new-object System.Net.NetworkCredential('%MVNW_USERNAME%', '%MVNW_PASSWORD%');"^
"}"^
"[Net.ServicePointManager]::SecurityProtocol = [Net.SecurityProtocolType]::Tls12; $webclient.DownloadFile('%DOWNLOAD_URL%', '%WRAPPER_JAR%')"^
"}"
if "%MVNW_VERBOSE%" == "true" (
echo Finished downloading %WRAPPER_JAR%
)
)
Copy link
Contributor

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Add post-download SHA-256 verification (PowerShell)

Verify the jar against wrapperSha256Sum in .mvn/wrapper/maven-wrapper.properties to prevent tampering.

Apply after Line 151:

     if "%MVNW_VERBOSE%" == "true" (
         echo Finished downloading %WRAPPER_JAR%
     )
+
+    rem Verify SHA-256 if provided
+    powershell -Command "$ErrorActionPreference = 'Stop';"^
+      "$props = Get-Content '%MAVEN_PROJECTBASEDIR%\.mvn\wrapper\maven-wrapper.properties' -ErrorAction SilentlyContinue;"^
+      "$expected = ($props | Where-Object { $_ -match '^wrapperSha256Sum\s*=' } | ForEach-Object { ($_ -split '=',2)[1].Trim() });"^
+      "if ($expected) {"^
+      "  $actual = (Get-FileHash -Algorithm SHA256 -Path %WRAPPER_JAR%).Hash.ToLower();"^
+      "  if ($actual -ne $expected.ToLower()) { Remove-Item -Force %WRAPPER_JAR%; Write-Error 'SHA-256 mismatch for maven-wrapper.jar'; exit 1 }"^
+      "}"
📝 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
powershell -Command "&{"^
"$webclient = new-object System.Net.WebClient;"^
"if (-not ([string]::IsNullOrEmpty('%MVNW_USERNAME%') -and [string]::IsNullOrEmpty('%MVNW_PASSWORD%'))) {"^
"$webclient.Credentials = new-object System.Net.NetworkCredential('%MVNW_USERNAME%', '%MVNW_PASSWORD%');"^
"}"^
"[Net.ServicePointManager]::SecurityProtocol = [Net.SecurityProtocolType]::Tls12; $webclient.DownloadFile('%DOWNLOAD_URL%', '%WRAPPER_JAR%')"^
"}"
if "%MVNW_VERBOSE%" == "true" (
echo Finished downloading %WRAPPER_JAR%
)
)
powershell -Command "&{"^
"$webclient = new-object System.Net.WebClient;"^
"if (-not ([string]::IsNullOrEmpty('%MVNW_USERNAME%') -and [string]::IsNullOrEmpty('%MVNW_PASSWORD%'))) {"^
"$webclient.Credentials = new-object System.Net.NetworkCredential('%MVNW_USERNAME%', '%MVNW_PASSWORD%');"^
"}"^
"[Net.ServicePointManager]::SecurityProtocol = [Net.SecurityProtocolType]::Tls12; $webclient.DownloadFile('%DOWNLOAD_URL%', '%WRAPPER_JAR%')"^
"}"
if "%MVNW_VERBOSE%" == "true" (
echo Finished downloading %WRAPPER_JAR%
)
rem Verify SHA-256 if provided
powershell -Command "$ErrorActionPreference = 'Stop';"^
"$props = Get-Content '%MAVEN_PROJECTBASEDIR%\.mvn\wrapper\maven-wrapper.properties' -ErrorAction SilentlyContinue;"^
"$expected = ($props | Where-Object { $_ -match '^wrapperSha256Sum\s*=' } | ForEach-Object { ($_ -split '=',2)[1].Trim() });"^
"if ($expected) {"^
" $actual = (Get-FileHash -Algorithm SHA256 -Path %WRAPPER_JAR%).Hash.ToLower();"^
" if ($actual -ne $expected.ToLower()) { Remove-Item -Force %WRAPPER_JAR%; Write-Error 'SHA-256 mismatch for maven-wrapper.jar'; exit 1 }"^
"}"
)
🤖 Prompt for AI Agents
In mvnw.cmd around lines 144 to 154, after the powershell download block (after
line 151), add a PowerShell step that reads wrapperSha256Sum from
.mvn/wrapper/maven-wrapper.properties, computes the SHA-256 of %WRAPPER_JAR%
(e.g. Get-FileHash -Algorithm SHA256), normalizes both values (remove
whitespace, toLowerInvariant) and compares them; if the property is missing or
the hashes don’t match, delete the downloaded %WRAPPER_JAR%, emit an error
message and exit the script with a non-zero code (exit /b 1); if they match,
emit a success/verified message and continue. Ensure the new PowerShell snippet
runs in the same Windows cmd context and preserves existing verbosity behavior.

Copy link
Contributor

@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

♻️ Duplicate comments (1)
Dockerfile (1)

13-14: Good fix: copy project into WORKDIR rather than a nested subdir.

This resolves the earlier issue where Maven saw src under an extra prefix and failed to locate pom.xml and modules.

🧹 Nitpick comments (5)
Dockerfile (5)

16-17: BuildKit-specific cache mount: confirm CI supports it or add a fallback.

RUN --mount=type=cache,... requires Docker BuildKit. If the CI/builders don’t enable BuildKit, this layer will fail. Either document the requirement (e.g., DOCKER_BUILDKIT=1) or add a non-BuildKit fallback.

Apply if you prefer a portable fallback:

- RUN --mount=type=cache,target=/root/.m2 \
-     ./mvnw -B -DskipTests -pl jraft-example -am package
+ # Fallback without BuildKit cache; slower but portable
+ RUN ./mvnw -B -DskipTests -pl jraft-example -am package

19-21: Jar selection is brittle; avoid a second Maven invocation and handle classifiers.

help:evaluate adds another wrapper run and may not match shaded/boot repackaged classifiers. Prefer copying the first non-.original jar from the module’s target dir.

-RUN APP=$(./mvnw -q -DforceStdout -pl jraft-example help:evaluate -Dexpression=project.build.finalName) && \
-    cp jraft-example/target/${APP}.jar /app.jar
+RUN set -e; \
+  for f in jraft-example/target/*.jar; do \
+    case "$f" in *.original.jar) continue;; esac; \
+    cp "$f" /app.jar; \
+    break; \
+  done; \
+  test -s /app.jar

Please also verify which artifact your module produces (plain, shaded, or Spring Boot repackage) and ensure the above picks the right one.


33-34: Expose port aligns with 8081; compose shows 8008 — please reconcile.

If docker-compose maps 8008:8008, the container won’t receive traffic unless the app actually listens on 8008. Align EXPOSE, app server port, and compose mapping.

Apply one of:

  • Update EXPOSE to 8008 if the app listens on 8008.
  • Or update compose to map host 8008 to container 8081 (8008:8081).

8-11: Minor: don’t mask errors when normalizing mvnw line endings.

|| true hides real failures (e.g., missing mvnw). Also run sed before chmod.

-COPY mvnw .
-RUN chmod +x mvnw && sed -i 's/\r$//' mvnw || true
+COPY mvnw .
+RUN sed -i 's/\r$//' mvnw && chmod +x mvnw

13-17: Optional build cache layering to speed rebuilds.

For faster incremental builds, copy only pom files first to warm dependency cache, then add sources.

-COPY . .
-# 仅构建 jraft-example(并自动构建其依赖模块)
-RUN --mount=type=cache,target=/root/.m2 \
-    ./mvnw -B -DskipTests -pl jraft-example -am package
+# Pre-cache deps with only POMs
+COPY pom.xml pom.xml
+COPY jraft-example/pom.xml jraft-example/pom.xml
+RUN --mount=type=cache,target=/root/.m2 ./mvnw -B -pl jraft-example -am -DskipTests -DskipITs -q dependency:go-offline
+# Now add the rest and build
+COPY . .
+RUN --mount=type=cache,target=/root/.m2 ./mvnw -B -DskipTests -pl jraft-example -am package
📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 036bdc9 and 1c4579f.

📒 Files selected for processing (1)
  • Dockerfile (1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
  • GitHub Check: sca

Comment on lines +24 to +34
FROM eclipse-temurin:8-jre-jammy
WORKDIR /
# 数据目录(日志/快照)
VOLUME ["/data/raft"]
# 统一一些默认环境变量(示例程序若支持,可自行解析)
ENV NODE_ID=1 \
PEERS="1@node1:8081,2@node2:8081,3@node3:8081" \
DATA_DIR=/data/raft
COPY --from=build /app.jar /app.jar
EXPOSE 8081
ENTRYPOINT ["java","-jar","/app.jar"]
Copy link
Contributor

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Run as non-root and fix ownership of the data volume.

Current image runs as root and declares a writable VOLUME. Switch to a non-root user and set ownership to avoid runtime permission issues.

 FROM eclipse-temurin:8-jre-jammy
 WORKDIR /
 # 数据目录(日志/快照)
 VOLUME ["/data/raft"]
@@
 COPY --from=build /app.jar /app.jar
+RUN useradd -r -u 10001 app && \
+    chown -R app:app /data/raft /app.jar
+USER 10001
 EXPOSE 8081
 ENTRYPOINT ["java","-jar","/app.jar"]
📝 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
FROM eclipse-temurin:8-jre-jammy
WORKDIR /
# 数据目录(日志/快照)
VOLUME ["/data/raft"]
# 统一一些默认环境变量(示例程序若支持,可自行解析)
ENV NODE_ID=1 \
PEERS="1@node1:8081,2@node2:8081,3@node3:8081" \
DATA_DIR=/data/raft
COPY --from=build /app.jar /app.jar
EXPOSE 8081
ENTRYPOINT ["java","-jar","/app.jar"]
FROM eclipse-temurin:8-jre-jammy
WORKDIR /
# 数据目录(日志/快照)
VOLUME ["/data/raft"]
# 统一一些默认环境变量(示例程序若支持,可自行解析)
ENV NODE_ID=1 \
PEERS="1@node1:8081,2@node2:8081,3@node3:8081" \
DATA_DIR=/data/raft
COPY --from=build /app.jar /app.jar
RUN useradd -r -u 10001 app && \
chown -R app:app /data/raft /app.jar
USER 10001
EXPOSE 8081
ENTRYPOINT ["java","-jar","/app.jar"]
🤖 Prompt for AI Agents
In Dockerfile around lines 24 to 34, the image runs as root while declaring a
writable VOLUME which can lead to permission issues; create a non-root user and
group (e.g. appuser), ensure /data/raft exists and is chowned to that user
(perform mkdir -p /data/raft and chown -R appuser:appgroup /data/raft) before
declaring the VOLUME or at least before switching user, and then switch to that
non-root user with USER so the container runs without root while retaining
proper ownership of the data directory.

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

Successfully merging this pull request may close these issues.

1 participant