utafrali commented on code in PR #6523:
URL: https://github.com/apache/fineract/pull/6523#discussion_r4123423942


##########
Dockerfile:
##########
@@ -0,0 +1,39 @@
+# Fineract image aligned with the epara microservice pattern.
+# Pin both images by digest in practice (Renovate/Dependabot keep them fresh).
+FROM eclipse-temurin:17-jdk-alpine-3.23 AS builder
+# The pipeline stages the boot jar as app.jar (skips the -plain jar).
+ARG JAR_FILE=app.jar
+WORKDIR /build
+COPY ${JAR_FILE} application.jar
+COPY agent/applicationinsights-agent-3.4.14.jar applicationinsights-agent.jar
+
+# Spring Boot 3.3+ syntax. On older Boot versions use:

Review Comment:
   This COPY references `agent/applicationinsights-agent-3.4.14.jar`, but that 
file is not included in this PR. The build will fail immediately on any CI 
environment that does not already have this file staged next to the Dockerfile. 
The jar needs to either be downloaded in the builder stage (e.g. via `curl` or 
Maven) or bundled in the repo alongside this file.



##########
Dockerfile:
##########
@@ -0,0 +1,39 @@
+# Fineract image aligned with the epara microservice pattern.
+# Pin both images by digest in practice (Renovate/Dependabot keep them fresh).
+FROM eclipse-temurin:17-jdk-alpine-3.23 AS builder
+# The pipeline stages the boot jar as app.jar (skips the -plain jar).
+ARG JAR_FILE=app.jar
+WORKDIR /build
+COPY ${JAR_FILE} application.jar
+COPY agent/applicationinsights-agent-3.4.14.jar applicationinsights-agent.jar
+
+# Spring Boot 3.3+ syntax. On older Boot versions use:

Review Comment:
   The Azure Application Insights agent 
(`applicationinsights-agent-3.4.14.jar`) is a Microsoft-specific observability 
library. It is hardcoded in the `COPY`, baked into the image in line 31, set as 
the default role name in the `ENV` block, and unconditionally loaded via 
`-javaagent` in the `ENTRYPOINT`. Apache Fineract is a vendor-neutral 
open-source project — shipping an Azure-only agent as a required runtime 
component is not appropriate for the main repository. If observability agents 
are desirable, the agent path should be optional (e.g. injected via 
`JAVA_TOOL_OPTIONS` at deploy time) and the image should not depend on any 
vendor-specific jar at build time.



##########
Dockerfile:
##########
@@ -0,0 +1,39 @@
+# Fineract image aligned with the epara microservice pattern.

Review Comment:
   The opening comment references "the epara microservice pattern", which 
suggests this Dockerfile was authored for an internal or organisation-specific 
deployment pipeline, not for the Apache Fineract project itself. Apache 
Fineract already maintains its own Docker infrastructure. This file appears to 
be a private-use artifact submitted to the wrong repository.



##########
Dockerfile:
##########
@@ -0,0 +1,39 @@
+# Fineract image aligned with the epara microservice pattern.
+# Pin both images by digest in practice (Renovate/Dependabot keep them fresh).
+FROM eclipse-temurin:17-jdk-alpine-3.23 AS builder
+# The pipeline stages the boot jar as app.jar (skips the -plain jar).

Review Comment:
   The comment on line 2 says to pin images by digest, but both `FROM` lines 
use mutable tags (`eclipse-temurin:17-jdk-alpine-3.23` and 
`eclipse-temurin:17-jre-alpine-3.23`). Mutable tags mean two builds a week 
apart can silently pull different base layers. Pin with a digest: `FROM 
eclipse-temurin:17-jdk-alpine-3.23@sha256:<digest>` to get reproducible, 
auditable builds.



##########
Dockerfile:
##########
@@ -0,0 +1,39 @@
+# Fineract image aligned with the epara microservice pattern.
+# Pin both images by digest in practice (Renovate/Dependabot keep them fresh).
+FROM eclipse-temurin:17-jdk-alpine-3.23 AS builder
+# The pipeline stages the boot jar as app.jar (skips the -plain jar).
+ARG JAR_FILE=app.jar
+WORKDIR /build
+COPY ${JAR_FILE} application.jar
+COPY agent/applicationinsights-agent-3.4.14.jar applicationinsights-agent.jar
+
+# Spring Boot 3.3+ syntax. On older Boot versions use:
+#   java -Djarmode=layertools -jar application.jar extract --destination 
extracted
+RUN java -Djarmode=tools -jar application.jar extract --layers --launcher 
--destination extracted
+
+FROM eclipse-temurin:17-jre-alpine-3.23
+
+ENV TZ=Etc/UTC
+RUN ln -snf /usr/share/zoneinfo/$TZ /etc/localtime && echo $TZ > /etc/timezone 
\
+ && addgroup -S -g 10001 fineract \
+ && adduser -S -D -H -u 10001 -G fineract fineract
+
+# See notes: check whether the JVM actually uses this bundle.
+ADD ca-bundle.crt /etc/pki/tls/certs/ca-bundle.crt
+
+WORKDIR /app

Review Comment:
   `ca-bundle.crt` is referenced here but is also missing from this PR — same 
build-breaking problem as the agent jar. Additionally, `/etc/pki/tls/certs/` is 
a Red Hat / CentOS path convention. The base image is Alpine, which stores CA 
certificates at `/etc/ssl/certs/ca-certificates.crt`. Placing the bundle under 
`/etc/pki/...` on Alpine will not update the system trust store; the JVM will 
continue to use its own bundled cacerts unless explicitly pointed elsewhere. 
Use `COPY ca-bundle.crt /usr/local/share/ca-certificates/ca-bundle.crt` 
followed by `RUN update-ca-certificates` to actually register it.



##########
Dockerfile:
##########
@@ -0,0 +1,39 @@
+# Fineract image aligned with the epara microservice pattern.
+# Pin both images by digest in practice (Renovate/Dependabot keep them fresh).
+FROM eclipse-temurin:17-jdk-alpine-3.23 AS builder
+# The pipeline stages the boot jar as app.jar (skips the -plain jar).
+ARG JAR_FILE=app.jar
+WORKDIR /build
+COPY ${JAR_FILE} application.jar
+COPY agent/applicationinsights-agent-3.4.14.jar applicationinsights-agent.jar
+
+# Spring Boot 3.3+ syntax. On older Boot versions use:
+#   java -Djarmode=layertools -jar application.jar extract --destination 
extracted
+RUN java -Djarmode=tools -jar application.jar extract --layers --launcher 
--destination extracted
+
+FROM eclipse-temurin:17-jre-alpine-3.23
+
+ENV TZ=Etc/UTC
+RUN ln -snf /usr/share/zoneinfo/$TZ /etc/localtime && echo $TZ > /etc/timezone 
\
+ && addgroup -S -g 10001 fineract \
+ && adduser -S -D -H -u 10001 -G fineract fineract
+
+# See notes: check whether the JVM actually uses this bundle.
+ADD ca-bundle.crt /etc/pki/tls/certs/ca-bundle.crt
+
+WORKDIR /app

Review Comment:
   Use `COPY` instead of `ADD` here. `ADD` has implicit decompression and 
URL-fetching behaviour that is not needed for a plain local file copy. `COPY` 
makes the intent explicit and is the Dockerfile best-practice recommendation.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to