-
Notifications
You must be signed in to change notification settings - Fork 1.2k
add docker file #1220
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
add docker file #1220
Conversation
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. |
WalkthroughAdds Docker build and compose artifacts, a root Changes
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)
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
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Poem
✨ Finishing Touches🧪 Generate unit tests
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 |
OK |
There was a problem hiding this 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.mdAlso 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
📒 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.
There was a problem hiding this 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/retriesImprove 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 timeoutsAdds 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
⛔ 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 correctRespects project-configured URL when present; sensible fallback otherwise.
mvnw.cmd (1)
60-80
: LGTM: robust JAVA_HOME validationClear diagnostics and early exit are helpful for Windows users.
mvnw (1)
206-214
: LGTM: graceful fallback to auto-download when wrapper JAR is absentGood UX: logs verbosely when enabled and avoids hard-failing on missing binary.
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"; | ||
|
There was a problem hiding this comment.
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
anddistributionSha256Sum
entries tomaven-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.
// 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 ... | ||
} | ||
} | ||
} |
There was a problem hiding this comment.
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.
// 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 ... | |
} | |
} | |
} |
try { | ||
downloadFileFromURL(url, outputFile); | ||
System.out.println("Done"); | ||
System.exit(0); | ||
} catch (Throwable e) { |
There was a problem hiding this comment.
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.
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.
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(); | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
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.
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 |
There was a problem hiding this comment.
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.
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" ( |
There was a problem hiding this comment.
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.
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% | ||
) | ||
) |
There was a problem hiding this comment.
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.
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.
There was a problem hiding this 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.jarPlease 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
📒 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
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"] |
There was a problem hiding this comment.
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.
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.
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
Documentation
Chores