chore(web): validate Re:Earth configuration on container startup #1390
Conversation
|
Caution Review failedThe pull request is closed. WalkthroughThe pull request introduces changes to the web service's Docker configuration and deployment script. A new command is added to the Dockerfile to install the Changes
Sequence DiagramsequenceDiagram
participant Dockerfile
participant BuildStage
participant Script
Dockerfile->>BuildStage: Install jq utility
BuildStage->>Script: Run configuration script
Script->>Script: Generate config file with envsubst
Script->>Script: Validate JSON using jq
alt JSON is valid
Script-->>BuildStage: Continue deployment
else JSON is invalid
Script--xBuildStage: Exit with error
end
Possibly related PRs
Suggested reviewers
Poem
📜 Recent review detailsConfiguration used: .coderabbit.yaml 📒 Files selected for processing (2)
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
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
Documentation and Community
|
✅ Deploy Preview for reearth-web ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
KeisukeYamashita
requested review from
airslice and
mkumbobeaty
as code owners
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
web/docker/40-envsubst-on-reearth-config.sh (1)
39-42: Enhance error reporting for JSON validation failures.While the validation logic is correct, we could provide more helpful error messages by showing the specific JSON parsing error.
Consider this improvement:
-if ! jq empty "$_REEARTH_CONFIG_OUTPUT_FILE" > /dev/null 2>&1; then - echo "Invalid JSON configuration file $_REEARTH_CONFIG_OUTPUT_FILE" >&2 +if ! error=$(jq empty "$_REEARTH_CONFIG_OUTPUT_FILE" 2>&1); then + echo "Invalid JSON configuration file $_REEARTH_CONFIG_OUTPUT_FILE:" >&2 + echo "$error" >&2 exit 1 fiweb/Dockerfile (1)
71-73: Consider pinning the jq version for reproducible builds.While the current implementation works, pinning the jq version would ensure reproducible builds and prevent potential issues from unexpected updates.
RUN --mount=type=cache,target=/var/cache/apk \ - apk add --no-cache jq + apk add --no-cache jq=1.7.1-r0
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
web/Dockerfile(1 hunks)web/docker/40-envsubst-on-reearth-config.sh(1 hunks)
🔇 Additional comments (1)
web/Dockerfile (1)
71-73: LGTM! Clean implementation using build cache.The implementation follows Docker best practices by using cache mount and no-cache flag.
Signed-off-by: KeisukeYamashita <[email protected]>
rot1024
force-pushed
the
validate-reearth-config-on-startup
branch
from
4ebb8df to
b582480
Compare
rot1024
changed the title
Overview
What I've done
I added
jqto the Dockerfile so that the container can validate the Re:Earth configuration file (format: JSON) on startup.This will prevent the container from starting when the required environment variables aren't configured and improve the reliability of our deployment workflow.
In the current implementation, the container will start and run even if the configuration file is broken.
What I haven't done
How I tested
Which point I want you to review particularly
Memo
jqfor Alpine is about650KB, which is comparatively tiny and worth adding.Summary by CodeRabbit
New Features
jqutility installation in the Dockerfile to support JSON parsing.Bug Fixes