-
Notifications
You must be signed in to change notification settings - Fork 43
Copy env variables to new docker properties for deploying in Linux/Docker #220
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
Conversation
WalkthroughA multi-stage Dockerfile is introduced for building and running a Java application, a new Docker-specific environment properties file is added, and the Changes
Sequence Diagram(s)sequenceDiagram
participant Developer
participant Docker Build (Maven)
participant Docker Runtime (JRE)
participant Application
Developer->>Docker Build (Maven): Build image (copy source, run Maven package)
Docker Build (Maven)->>Docker Runtime (JRE): Copy WAR file to runtime image
Developer->>Docker Runtime (JRE): Start container (java -jar app.war)
Docker Runtime (JRE)->>Application: Run application on port 8080
Possibly related PRs
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ Context from checks skipped due to timeout of 90000ms (3)
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
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: 4
🧹 Nitpick comments (3)
src/main/environment/common_ci.properties (1)
1-182
: Ensure newline at end of file
Add an empty newline at EOF to satisfy POSIX conventions and avoid issues with some editors or tools.Dockerfile (2)
9-11
: Optimize Docker layer caching
Reorder theCOPY
/RUN
steps to download dependencies before copying the full source tree. This reduces rebuild times when code changes but dependencies do not.
17-22
: Pin JRE base image tag
Consider using a fully qualified tag or digest (e.g.eclipse-temurin:17-jre-jammy
) to ensure reproducible builds instead of the moving:17-jre
.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
Dockerfile
(1 hunks)pom.xml
(2 hunks)src/main/environment/common_ci.properties
(1 hunks)src/main/environment/common_docker.properties
(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (3)
- GitHub Check: Build
- GitHub Check: Analyze (java)
- GitHub Check: Package-test
🔇 Additional comments (2)
pom.xml (1)
62-65
: Validate exclusion of default logging starter
You’ve excludedspring-boot-starter-logging
from the core starter. Ensure an alternative logging implementation is provided and that no required logging classes are inadvertently omitted.src/main/environment/common_docker.properties (1)
165-167
: Verify captcha placeholders resolution
The captcha keys use${...}
syntax; confirm that the Maven filtering or Docker entrypoint will replace them properly at runtime.
<plugin> | ||
<groupId>org.springframework.boot</groupId> | ||
<artifactId>spring-boot-maven-plugin</artifactId> | ||
<version>3.2.2</version> | ||
<executions> | ||
<execution> | ||
<goals> | ||
<goal>repackage</goal> | ||
</goals> | ||
</execution> | ||
</executions> | ||
</plugin> |
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
Consolidate duplicate Spring Boot Maven plugin
There are two <plugin>
entries for spring-boot-maven-plugin
(one without version at lines 511–514 and another with version at 661–672). Merge them into a single declaration with the <version>
and <execution>
to avoid redundancy.
Apply this diff in the <plugins>
section:
- <plugin>
- <groupId>org.springframework.boot</groupId>
- <artifactId>spring-boot-maven-plugin</artifactId>
- </plugin>
+ <plugin>
+ <groupId>org.springframework.boot</groupId>
+ <artifactId>spring-boot-maven-plugin</artifactId>
+ <version>3.2.2</version>
+ <executions>
+ <execution>
+ <goals>
+ <goal>repackage</goal>
+ </goals>
+ </execution>
+ </executions>
+ </plugin>
🤖 Prompt for AI Agents
In pom.xml around lines 511-514 and 661-672, there are two separate declarations
of the spring-boot-maven-plugin, one without a version and one with version
3.2.2 and execution configuration. Merge these two plugin entries into a single
plugin declaration that includes the version 3.2.2 and the execution block with
the repackage goal, removing the duplicate plugin entry to avoid redundancy.
--------------------------- | ||
fileBasePath =${MMU_FILE_BASE_PATH} |
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
Remove non-property separator
The dash line (---------------------------
) isn’t a valid key/value entry and will break parsing. Please delete it.
🤖 Prompt for AI Agents
In src/main/environment/common_docker.properties at lines 59 to 60, remove the
line containing only dashes (---------------------------) as it is not a valid
property entry and will cause parsing errors. Ensure the file only contains
valid key=value pairs without any separator lines.
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.
@drtechie Please remove the conflict markers and duplicate plugins in pom.xml file.
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: 0
♻️ Duplicate comments (1)
src/main/environment/common_ci.properties (1)
180-185
: Remove unresolved merge conflict markersThe conflict markers (
<<<<<<<
,=======
,>>>>>>>
) break the properties file syntax. Remove them and retain only:cors.allowed-origins[email protected]_ALLOWED_ORIGINS@
🧹 Nitpick comments (1)
src/main/environment/common_ci.properties (1)
64-66
: Consistent naming for file Base Path propertyThis key is defined in camelCase (
fileBasePath
) unlike the kebab-case and dot-notation style used elsewhere. Consider renaming it tofile-base-path
orfile.base-path
to maintain consistency across properties.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/main/environment/common_ci.properties
(3 hunks)src/main/environment/common_docker.properties
(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- src/main/environment/common_docker.properties
⏰ Context from checks skipped due to timeout of 90000ms (3)
- GitHub Check: Package-test
- GitHub Check: Analyze (java)
- GitHub Check: Build
🔇 Additional comments (2)
src/main/environment/common_ci.properties (2)
68-68
: Approve Redis host placeholder updateSwitching from a hardcoded
localhost
to@env.REDIS_HOST@
enables external configuration for different deployments.
170-170
: VerifyisProduction
flag value for CI environmentSetting
isProduction=true
in the CI properties may cause CI runs to be treated as production. Confirm that this aligns with the intended environment semantics and downstream code logic.
|
📋 Description
JIRA ID: AMM-1625
Proposed Changes:
${}
to ensure environment variables inside the system is used instead of injecting from Jenkins - used for Linux and Docker deployments✅ Type of Change
Summary by CodeRabbit
New Features
Chores