ci(web): dockerize web application #1222
Conversation
|
Note Currently processing new changes in this PR. This may take a few minutes, please wait... 📒 Files selected for processing (10)
WalkthroughThe pull request introduces extensive modifications across several workflow files related to Docker image building, deployment, and configuration for a web application. Key changes include the creation of new jobs for building and deploying server and web components, updates to environment variables, and the introduction of new configuration templates. The workflow files have been restructured to support parallel execution, enhance modularity, and facilitate deployment to services like AWS and Google Cloud Platform. Additionally, a Changes
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
Tip CodeRabbit can approve the review once all CodeRabbit's comments are resolved.Enable the 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,
mkumbobeaty and
rot1024
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: 17
🧹 Outside diff range and nitpick comments (11)
web/docker/reearth_config.json.template (1)
12-12: Document configuration structure and add validationFields like
extensionUrlsandpluginslikely expect specific data structures (arrays/objects), but their expected format is unclear.Consider:
- Adding a JSON schema to validate the configuration
- Including comments in the template to document:
- Required vs optional fields
- Expected data types and formats
- Example values
Example schema structure:
{ "$schema": "http://json-schema.org/draft-07/schema#", "type": "object", "required": ["api", "auth0Domain"], "properties": { "extensionUrls": { "type": "array", "items": { "type": "string", "format": "uri" } }, "plugins": { "type": "array", "items": { "type": "object" } } } }Also applies to: 16-16
web/docker/40-envsubst-on-reearth-config.sh (1)
1-25: Consider startup performance and failure modes in Docker context.As this script runs during container initialization (indicated by the
40-prefix), consider:
- Adding a timeout mechanism to prevent hanging during startup
- Implementing health checks to verify successful config generation
- Documenting required environment variables in a README or template file
- Adding logging to container stdout/stderr for better observability
This will improve the reliability and maintainability of the Dockerized application.
web/docker/nginx.conf.template (1)
1-15: Consider adding cloud-specific headers to JSON logging.Since this application is being deployed to Cloud Run, consider adding these additional fields to enhance security monitoring and debugging capabilities:
x_forwarded_for: To track the original client IP through proxiesx_cloud_trace_context: For Google Cloud trace correlationhost: To identify which host handled the requestlog_format json escape=json '{' '"body_bytes_sent": "$body_bytes_sent",' '"http_referer": "$http_referer",' '"http_user_agent": "$http_user_agent",' '"remote_ip": "$remote_addr",' '"remote_user": "$remote_user",' '"request": "$request",' '"request_id": "$request_id",' '"request_method": "$request_method",' '"request_time": "$request_time",' '"request_uri": "$request_uri",' '"server_name": "$server_name",' '"status": "$status",' - '"time": "$time_iso8601"' + '"time": "$time_iso8601",' + '"x_forwarded_for": "$http_x_forwarded_for",' + '"x_cloud_trace_context": "$http_x_cloud_trace_context",' + '"host": "$host"' }';.github/workflows/deploy_web_nightly.yml (2)
7-9: LGTM! Consider adding documentation for environment variables.The Docker image environment variables are well-structured and follow naming conventions.
Consider adding comments to document the purpose of these environment variables:
+ # Docker Hub image for nightly builds IMAGE: reearth/reearth-visualizer-web:nightly + # GCP Artifact Registry image for Cloud Run deployment IMAGE_GCP: us-central1-docker.pkg.dev/reearth-oss/reearth/reearth-visualizer-web:nightly
14-15: Track the removal of legacy deployment.The TODO comment is a good indicator, but it would be better to track this as a follow-up task.
Would you like me to create a GitHub issue to track the removal of the GCS-based deployment after the Cloud Run migration is complete?
.github/workflows/ci_docker_build_push.yml (2)
Line range hint
21-145: Consider using reusable workflows to reduce duplication.The
serverandwebjobs share almost identical steps and logic. Consider extracting the common build and push logic into a reusable workflow to improve maintainability.Example structure:
# .github/workflows/docker-build-push.yml name: Docker Build and Push on: workflow_call: inputs: working-directory: required: true image-name: required: true secrets: dockerhub-username: required: true dockerhub-token: required: true jobs: build: # Common build and push logic hereThen reference it in the main workflow:
jobs: server: uses: ./.github/workflows/docker-build-push.yml with: working-directory: server image-name: reearth/reearth-visualizer secrets: dockerhub-username: ${{ secrets.DOCKERHUB_USERNAME }} dockerhub-token: ${{ secrets.DOCKERHUB_TOKEN }}🧰 Tools
🪛 actionlint
100-100: the runner of "actions/checkout@v2" action is too old to run on GitHub Actions. update the action's version to fix this issue
(action)
117-117: workflow command "set-output" was deprecated. use
echo "{name}={value}" >> $GITHUB_OUTPUTinstead: https://docs.github.com/en/actions/using-workflows/workflow-commands-for-github-actions(deprecated-commands)
117-117: workflow command "set-output" was deprecated. use
echo "{name}={value}" >> $GITHUB_OUTPUTinstead: https://docs.github.com/en/actions/using-workflows/workflow-commands-for-github-actions(deprecated-commands)
117-117: workflow command "set-output" was deprecated. use
echo "{name}={value}" >> $GITHUB_OUTPUTinstead: https://docs.github.com/en/actions/using-workflows/workflow-commands-for-github-actions(deprecated-commands)
137-137: the runner of "docker/build-push-action@v2" action is too old to run on GitHub Actions. update the action's version to fix this issue
(action)
Line range hint
66-77: Track technical debt for Cloud Run migration.The TODO comment and temporary fetch step for reearth-web release should be tracked as technical debt to ensure it's removed after the Cloud Run migration.
Would you like me to create a GitHub issue to track this cleanup task? The issue would include:
- Removal of the fetch step
- Cleanup of related web assets
- Verification steps for Cloud Run migration completion
web/vite.config.ts (2)
60-60: LGTM! Consider optimizing the commit hash retrieval for containers.The change to use
process.env.GITHUB_SHAis good and container-friendly. However, we could optimize the implementation further.Consider this optimization to avoid unnecessary git commands in containers:
- let commitHash = ""; - try { - commitHash = execSync("git rev-parse HEAD").toString().trimEnd(); - } catch { - // noop - } + const getCommitHash = () => { + if (process.env.GITHUB_SHA) return process.env.GITHUB_SHA; + try { + return execSync("git rev-parse HEAD").toString().trimEnd(); + } catch { + return ""; + } + }; + + const commitHash = getCommitHash();This approach:
- Prioritizes the environment variable
- Only executes git command if needed
- Remains backward compatible
Line range hint
63-65: Adapt server configuration for container environments.Since this application is being Dockerized for Cloud Run deployment, consider these container-specific adjustments:
- Cloud Run requires listening on
PORTenvironment variable:server: { - port: 3000 + port: parseInt(process.env.PORT || "3000", 10) },
- Add a comment documenting container-specific environment variables:
// Container-specific environment variables: // - PORT: Server port (required by Cloud Run) // - GITHUB_SHA: Build commit hash.github/workflows/build_web.yml (1)
236-236: Remove trailing spaces.There are trailing spaces at the end of line 236.
🧰 Tools
🪛 yamllint
[error] 236-236: trailing spaces
(trailing-spaces)
web/Dockerfile (1)
33-34: Typographical error in commentThere's a typo in line 33. The comment reads "Quite the Nginx startup logs." It should be "Quiet the Nginx startup logs."
Apply this diff to correct the typo:
-# Quite the Nginx startup logs. +# Quiet the Nginx startup logs.
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
📒 Files selected for processing (10)
.github/workflows/build_web.yml(2 hunks).github/workflows/ci_docker_build_push.yml(4 hunks).github/workflows/deploiy_aws.yml(2 hunks).github/workflows/deploy_web_nightly.yml(2 hunks)web/.dockerignore(1 hunks)web/Dockerfile(1 hunks)web/docker/40-envsubst-on-reearth-config.sh(1 hunks)web/docker/nginx.conf.template(1 hunks)web/docker/reearth_config.json.template(1 hunks)web/vite.config.ts(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- web/.dockerignore
🧰 Additional context used
🪛 actionlint
.github/workflows/build_web.yml
195-195: workflow command "set-output" was deprecated. use echo "{name}={value}" >> $GITHUB_OUTPUT instead: https://docs.github.com/en/actions/using-workflows/workflow-commands-for-github-actions
(deprecated-commands)
195-195: workflow command "set-output" was deprecated. use echo "{name}={value}" >> $GITHUB_OUTPUT instead: https://docs.github.com/en/actions/using-workflows/workflow-commands-for-github-actions
(deprecated-commands)
195-195: workflow command "set-output" was deprecated. use echo "{name}={value}" >> $GITHUB_OUTPUT instead: https://docs.github.com/en/actions/using-workflows/workflow-commands-for-github-actions
(deprecated-commands)
.github/workflows/ci_docker_build_push.yml
100-100: the runner of "actions/checkout@v2" action is too old to run on GitHub Actions. update the action's version to fix this issue
(action)
117-117: workflow command "set-output" was deprecated. use echo "{name}={value}" >> $GITHUB_OUTPUT instead: https://docs.github.com/en/actions/using-workflows/workflow-commands-for-github-actions
(deprecated-commands)
117-117: workflow command "set-output" was deprecated. use echo "{name}={value}" >> $GITHUB_OUTPUT instead: https://docs.github.com/en/actions/using-workflows/workflow-commands-for-github-actions
(deprecated-commands)
117-117: workflow command "set-output" was deprecated. use echo "{name}={value}" >> $GITHUB_OUTPUT instead: https://docs.github.com/en/actions/using-workflows/workflow-commands-for-github-actions
(deprecated-commands)
137-137: the runner of "docker/build-push-action@v2" action is too old to run on GitHub Actions. update the action's version to fix this issue
(action)
.github/workflows/deploiy_aws.yml
51-51: property "events" is not defined in object type {action: string; action_path: string; action_ref: string; action_repository: string; action_status: string; actor: string; actor_id: string; api_url: string; base_ref: string; env: string; event: object; event_name: string; event_path: string; graphql_url: string; head_ref: string; job: string; job_workflow_sha: string; path: string; ref: string; ref_name: string; ref_protected: string; ref_type: string; repository: string; repository_id: string; repository_owner: string; repository_owner_id: string; repositoryurl: string; retention_days: number; run_attempt: string; run_id: string; run_number: string; secret_source: string; server_url: string; sha: string; token: string; triggering_actor: string; workflow: string; workflow_ref: string; workflow_sha: string; workspace: string}
(expression)
52-52: property "events" is not defined in object type {action: string; action_path: string; action_ref: string; action_repository: string; action_status: string; actor: string; actor_id: string; api_url: string; base_ref: string; env: string; event: object; event_name: string; event_path: string; graphql_url: string; head_ref: string; job: string; job_workflow_sha: string; path: string; ref: string; ref_name: string; ref_protected: string; ref_type: string; repository: string; repository_id: string; repository_owner: string; repository_owner_id: string; repositoryurl: string; retention_days: number; run_attempt: string; run_id: string; run_number: string; secret_source: string; server_url: string; sha: string; token: string; triggering_actor: string; workflow: string; workflow_ref: string; workflow_sha: string; workspace: string}
(expression)
60-60: shellcheck reported issue in this script: SC2086:info:1:13: Double quote to prevent globbing and word splitting
(shellcheck)
60-60: shellcheck reported issue in this script: SC2086:info:2:12: Double quote to prevent globbing and word splitting
(shellcheck)
60-60: shellcheck reported issue in this script: SC2086:info:2:63: Double quote to prevent globbing and word splitting
(shellcheck)
60-60: shellcheck reported issue in this script: SC2086:info:3:53: Double quote to prevent globbing and word splitting
(shellcheck)
.github/workflows/deploy_web_nightly.yml
48-48: shellcheck reported issue in this script: SC2086:info:1:13: Double quote to prevent globbing and word splitting
(shellcheck)
48-48: shellcheck reported issue in this script: SC2086:info:2:12: Double quote to prevent globbing and word splitting
(shellcheck)
48-48: shellcheck reported issue in this script: SC2086:info:2:19: Double quote to prevent globbing and word splitting
(shellcheck)
48-48: shellcheck reported issue in this script: SC2086:info:3:13: Double quote to prevent globbing and word splitting
(shellcheck)
53-53: shellcheck reported issue in this script: SC2086:info:2:11: Double quote to prevent globbing and word splitting
(shellcheck)
53-53: shellcheck reported issue in this script: SC2086:info:3:12: Double quote to prevent globbing and word splitting
(shellcheck)
🪛 yamllint
.github/workflows/build_web.yml
[error] 236-236: trailing spaces
(trailing-spaces)
🔇 Additional comments (10)
web/docker/reearth_config.json.template (1)
3-6: Verify security measures for sensitive configuration
This configuration contains sensitive values (auth tokens, client IDs). When deploying to Cloud Run:
- Ensure these values are stored in Secret Manager
- Verify proper access controls are in place
- Consider adding input validation for these environment variables
Let's check the deployment workflow for proper secret handling:
Also applies to: 15-15
✅ Verification successful
Let me check the deployment workflow for proper secret handling in more detail:
Sensitive configuration values are properly handled through GitHub Secrets
The verification shows that:
- Auth0 credentials (audience, client ID, domain) are properly stored as GitHub Secrets and referenced securely in workflows
- Deployment workflows use GitHub's secret management system for all sensitive values
- No hardcoded sensitive values were found in the workflows
- The configuration template appropriately uses environment variables that are populated from secrets during deployment
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check if secrets are properly referenced in deployment workflows
rg -l 'REEARTH_WEB_(AUTH0|CESIUM)' .github/workflows/
# Check for potential hardcoded sensitive values
rg -i '(token|key|secret|password|credential)' .github/workflows/
Length of output: 8111
Script:
#!/bin/bash
# Check how reearth config is handled in deployment
rg -A 5 'reearth_config\.json' .github/workflows/
# Check for environment variable handling in deployment
rg -A 5 'REEARTH_WEB_(AUTH0|CESIUM)' .github/workflows/
Length of output: 915
web/docker/nginx.conf.template (1)
17-17: Verify environment variables in deployment workflow.
The configuration depends on two environment variables:
${PORT}: Used for the listening port${REAL_IP_HEADER}: Used for real IP configuration
Let's verify these are properly set in the deployment workflow:
Also applies to: 20-20
✅ Verification successful
Environment variables are properly configured
The environment variables are properly set in the Dockerfile:
PORT=8080(default Cloud Run port)REAL_IP_HEADER=X-Forwarded-For(standard header for forwarded requests)
These default values provide a secure and standard configuration for the nginx setup. The X-Forwarded-For header is the conventional choice for Cloud Run deployments to preserve client IP addresses through the load balancer.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check if environment variables are properly set in workflow files
# Test: Search for PORT and REAL_IP_HEADER environment variable settings
echo "Checking for PORT environment variable..."
rg -l "PORT=" .github/workflows/
echo "Checking for REAL_IP_HEADER environment variable..."
rg -l "REAL_IP_HEADER=" .github/workflows/
Length of output: 281
Script:
#!/bin/bash
# Let's try a broader search for these environment variables in workflow files
# and docker-compose files, as they might be defined there
echo "Searching for PORT in workflow files (any context)..."
rg -i "PORT" .github/workflows/ -A 2 -B 2
echo "Searching for REAL_IP_HEADER in workflow files (any context)..."
rg -i "REAL_IP_HEADER" .github/workflows/ -A 2 -B 2
echo "Checking docker-compose files for environment variables..."
fd -e yml -e yaml docker-compose --exec rg -i "PORT|REAL_IP_HEADER" {}
echo "Checking Dockerfile for environment variables..."
fd Dockerfile --exec rg -i "PORT|REAL_IP_HEADER" {}
echo "Checking for .env files or environment templates..."
fd -e env -e env.template --exec cat {}
Length of output: 2183
.github/workflows/deploy_web_nightly.yml (1)
35-44: LGTM! GCP authentication and setup is properly configured.
The authentication and Cloud SDK setup follows Google's recommended practices.
.github/workflows/deploiy_aws.yml (2)
14-20: Well-structured environment variable organization!
Good job separating and documenting the environment variables for server and web components. The naming convention is consistent and clear.
44-63: Clarify deployment strategy regarding AWS and Cloud Run.
The PR objectives mention deploying to Cloud Run, but this job deploys to AWS ECR. Please clarify:
- Is AWS ECR being used as an intermediate registry for Cloud Run?
- Or is this a parallel deployment strategy?
This understanding would help ensure the deployment workflow aligns with the intended architecture.
🧰 Tools
🪛 actionlint
51-51: property "events" is not defined in object type {action: string; action_path: string; action_ref: string; action_repository: string; action_status: string; actor: string; actor_id: string; api_url: string; base_ref: string; env: string; event: object; event_name: string; event_path: string; graphql_url: string; head_ref: string; job: string; job_workflow_sha: string; path: string; ref: string; ref_name: string; ref_protected: string; ref_type: string; repository: string; repository_id: string; repository_owner: string; repository_owner_id: string; repositoryurl: string; retention_days: number; run_attempt: string; run_id: string; run_number: string; secret_source: string; server_url: string; sha: string; token: string; triggering_actor: string; workflow: string; workflow_ref: string; workflow_sha: string; workspace: string}
(expression)
52-52: property "events" is not defined in object type {action: string; action_path: string; action_ref: string; action_repository: string; action_status: string; actor: string; actor_id: string; api_url: string; base_ref: string; env: string; event: object; event_name: string; event_path: string; graphql_url: string; head_ref: string; job: string; job_workflow_sha: string; path: string; ref: string; ref_name: string; ref_protected: string; ref_type: string; repository: string; repository_id: string; repository_owner: string; repository_owner_id: string; repositoryurl: string; retention_days: number; run_attempt: string; run_id: string; run_number: string; secret_source: string; server_url: string; sha: string; token: string; triggering_actor: string; workflow: string; workflow_ref: string; workflow_sha: string; workspace: string}
(expression)
60-60: shellcheck reported issue in this script: SC2086:info:1:13: Double quote to prevent globbing and word splitting
(shellcheck)
60-60: shellcheck reported issue in this script: SC2086:info:2:12: Double quote to prevent globbing and word splitting
(shellcheck)
60-60: shellcheck reported issue in this script: SC2086:info:2:63: Double quote to prevent globbing and word splitting
(shellcheck)
60-60: shellcheck reported issue in this script: SC2086:info:3:53: Double quote to prevent globbing and word splitting
(shellcheck)
.github/workflows/build_web.yml (1)
214-224: Verify Docker build process without web release fetch steps.
The steps to fetch and extract the web release are commented out, but they seem necessary for the Docker build process. Please:
- Verify that the Docker build can succeed without these steps
- Document the migration plan for transitioning from the old to the new job
- Consider keeping these steps active until the old job is removed to ensure a smooth transition
✅ Verification successful
Keep the web release fetch steps commented out - they are not needed for the new job
The commented-out steps in the build-docker-image job should remain commented out because:
- The old job
build-docker-image-oldusesserver/Dockerfilewhich needs the web files (COPY web* /reearth/web/) - The new job
build-docker-imageusesweb/Dockerfilewhich builds directly from source, not requiring the web release files - The migration strategy is correct - keep both jobs running until the old one is removed, with each using its appropriate build process
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check if Dockerfile in web directory expects these files
rg -A 5 'COPY.*web' web/Dockerfile
Length of output: 36
Script:
#!/bin/bash
# Let's first check if the Dockerfile exists in the repository
fd Dockerfile
# Then search for any COPY commands related to web or reearth-web
rg -A 5 'COPY.*(?:reearth-)?web' --type yaml --type dockerfile
# Also check the build workflow for context
cat .github/workflows/build_web.yml
Length of output: 9695
Script:
#!/bin/bash
# Let's check the server and web Dockerfiles
cat server/Dockerfile
cat web/Dockerfile
# Also check if there are any references to web directory in server's Dockerfile
rg -A 5 'web' server/Dockerfile
Length of output: 3300
web/Dockerfile (4)
1-28: Efficient multi-stage build with cache optimization
The build stage is well-structured, leveraging multi-stage builds and cache optimization using --mount options effectively. The use of corepack and yarn ensures consistent package management and reproducible builds.
45-61: Verify default environment variable values
Ensure that the application correctly handles the default environment variable values set to null and empty arrays like []. These values may need to be parsed or handled appropriately within the application to prevent unexpected behavior.
If necessary, update the application code to handle these defaults or consider setting them to more suitable default values.
62-65: Proper file ownership and permissions
Setting file ownership during the COPY commands with --chown=nginx:nginx is appropriate and ensures that Nginx can access the files without permission issues.
67-68: Appropriate entrypoint and command settings
Using the default Nginx entrypoint script and running Nginx in the foreground with daemon off; is standard practice for containerized Nginx applications.
KeisukeYamashita
force-pushed
the
dockerize-web
branch
from
78d3fb2 to
dc07c4c
Compare
KeisukeYamashita
force-pushed
the
dockerize-web
branch
from
dc07c4c to
d37868b
Compare
There was a problem hiding this comment.
Caution
Inline review comments failed to post. This is likely due to GitHub's limits when posting large numbers of comments.
Actionable comments posted: 6
🧹 Outside diff range and nitpick comments (6)
.github/workflows/deploy_web_nightly.yml (2)
8-9: Parameterize the GCP region in image path.The GCP region is hardcoded in the
IMAGE_GCPvariable. Consider extracting it to a separate environment variable for better maintainability:- IMAGE_GCP: us-central1-docker.pkg.dev/reearth-oss/reearth/reearth-visualizer-web:nightly + GCP_REGION: us-central1 + IMAGE_GCP: ${GCP_REGION}-docker.pkg.dev/reearth-oss/reearth/reearth-visualizer-web:nightly
14-15: Track GCS migration timeline.The TODO comment indicates this job will be removed after Cloud Run migration, but there's no clear timeline. Consider creating a tracking issue for this technical debt.
Would you like me to help create a GitHub issue to track the GCS to Cloud Run migration timeline?
.github/workflows/deploiy_aws.yml (1)
Line range hint
1-1: Fix typo in workflow filename.The workflow filename contains a typo: "deploiy_aws.yml" should be "deploy_aws.yml".
Rename the file to fix the typo:
git mv .github/workflows/deploiy_aws.yml .github/workflows/deploy_aws.ymlweb/Dockerfile (3)
19-29: Add build-time security measuresConsider adding build-time security scanning:
--mount=type=bind,source=src,target=src \ --mount=type=cache,target=/root/.yarn,sharing=locked \ + yarn audit && \ yarn build
63-69: Optimize for Cloud Run deploymentThe setup is good, but consider these Cloud Run optimizations:
- Add health check endpoint
- Configure graceful shutdown
Add to your Nginx config template:
# Health check endpoint for Cloud Run location /health { access_log off; return 200 'healthy\n'; } # Graceful shutdown configuration http { proxy_send_timeout 120s; proxy_read_timeout 120s; }
1-69: Consider Cloud Run specific architectural improvementsWhile the Dockerfile is well-structured, consider these architectural improvements for Cloud Run:
- Implement container startup/shutdown hooks for better lifecycle management
- Add structured logging configuration for better observability
- Consider implementing Cloud Run's container contract fully (https://cloud.google.com/run/docs/container-contract)
- Add container vulnerability scanning in CI/CD pipeline
🛑 Comments failed to post (6)
.github/workflows/deploy_web_nightly.yml (1)
35-46: 🛠️ Refactor suggestion
Add environment variable validation.
Add checks for required environment variables at the start of the job to fail fast if any are missing:
deploy_test: name: Deploy app to test env runs-on: ubuntu-latest if: github.event.repository.full_name == 'reearth/reearth-visualizer' + env: + REQUIRED_VARS: "IMAGE IMAGE_GCP GCP_SA_KEY" steps: + - name: Validate environment + run: | + for var in $REQUIRED_VARS; do + if [ -z "${!var}" ]; then + echo "Error: $var is not set" + exit 1 + fi + done - uses: google-github-actions/auth@v0📝 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.deploy_test: name: Deploy app to test env runs-on: ubuntu-latest if: github.event.repository.full_name == 'reearth/reearth-visualizer' env: REQUIRED_VARS: "IMAGE IMAGE_GCP GCP_SA_KEY" steps: - name: Validate environment run: | for var in $REQUIRED_VARS; do if [ -z "${!var}" ]; then echo "Error: $var is not set" exit 1 fi done - uses: google-github-actions/auth@v0 with: credentials_json: ${{ secrets.GCP_SA_KEY }} - name: Set up Cloud SDK uses: google-github-actions/setup-gcloud@v0 - name: Configure docker run: gcloud auth configure-docker us-central1-docker.pkg.dev --quiet.github/workflows/deploiy_aws.yml (1)
51-52:
⚠️ Potential issueFix incorrect GitHub context syntax.
The expression
github.eventsis incorrect. GitHub Actions usesgithub.event.Apply this diff:
- aws-access-key-id: ${{ secrets.AWS_ACCESS_KEY_ID || github.events.inputs.keyId }} - aws-secret-access-key: ${{ secrets.AWS_SECRET_ACCESS_KEY || github.events.inputs.secretAccessKey }} + aws-access-key-id: ${{ secrets.AWS_ACCESS_KEY_ID || github.event.inputs.keyId }} + aws-secret-access-key: ${{ secrets.AWS_SECRET_ACCESS_KEY || github.event.inputs.secretAccessKey }}📝 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.aws-access-key-id: ${{ secrets.AWS_ACCESS_KEY_ID || github.event.inputs.keyId }} aws-secret-access-key: ${{ secrets.AWS_SECRET_ACCESS_KEY || github.event.inputs.secretAccessKey }}🧰 Tools
🪛 actionlint
51-51: property "events" is not defined in object type {action: string; action_path: string; action_ref: string; action_repository: string; action_status: string; actor: string; actor_id: string; api_url: string; base_ref: string; env: string; event: object; event_name: string; event_path: string; graphql_url: string; head_ref: string; job: string; job_workflow_sha: string; path: string; ref: string; ref_name: string; ref_protected: string; ref_type: string; repository: string; repository_id: string; repository_owner: string; repository_owner_id: string; repositoryurl: string; retention_days: number; run_attempt: string; run_id: string; run_number: string; secret_source: string; server_url: string; sha: string; token: string; triggering_actor: string; workflow: string; workflow_ref: string; workflow_sha: string; workspace: string}
(expression)
52-52: property "events" is not defined in object type {action: string; action_path: string; action_ref: string; action_repository: string; action_status: string; actor: string; actor_id: string; api_url: string; base_ref: string; env: string; event: object; event_name: string; event_path: string; graphql_url: string; head_ref: string; job: string; job_workflow_sha: string; path: string; ref: string; ref_name: string; ref_protected: string; ref_type: string; repository: string; repository_id: string; repository_owner: string; repository_owner_id: string; repositoryurl: string; retention_days: number; run_attempt: string; run_id: string; run_number: string; secret_source: string; server_url: string; sha: string; token: string; triggering_actor: string; workflow: string; workflow_ref: string; workflow_sha: string; workspace: string}
(expression)
web/Dockerfile (3)
33-62: 🛠️ Refactor suggestion
Enhance environment variable configuration
While the Cloud Run port configuration is correct, consider:
- Adding runtime validation for required variables
- Documenting required vs optional variables
- Using more secure defaults
Consider adding a startup script:
#!/bin/sh # Add to /docker-entrypoint.d/30-validate-env.sh required_vars="REEARTH_WEB_API REEARTH_WEB_AUTH0_DOMAIN" for var in $required_vars; do if [ "${!var}" = "null" ]; then echo "Error: $var must be set" exit 1 fi done
30-32:
⚠️ Potential issueFix invalid Nginx version and enhance security
The specified Nginx version 1.27 doesn't exist. The latest stable version is 1.25.
-FROM nginx:1.27-alpine +FROM nginx:1.25-alpine + +# Add security headers +RUN echo "security.conf" > /etc/nginx/conf.d/security.conf && \ + echo "add_header X-Frame-Options DENY;" >> /etc/nginx/conf.d/security.conf && \ + echo "add_header X-Content-Type-Options nosniff;" >> /etc/nginx/conf.d/security.conf && \ + echo "add_header X-XSS-Protection \"1; mode=block\";" >> /etc/nginx/conf.d/security.conf📝 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 nginx:1.25-alpine WORKDIR /usr/share/nginx/html # Add security headers RUN echo "security.conf" > /etc/nginx/conf.d/security.conf && \ echo "add_header X-Frame-Options DENY;" >> /etc/nginx/conf.d/security.conf && \ echo "add_header X-Content-Type-Options nosniff;" >> /etc/nginx/conf.d/security.conf && \ echo "add_header X-XSS-Protection \"1; mode=block\";" >> /etc/nginx/conf.d/security.conf
1-8: 🛠️ Refactor suggestion
Consider security and resource optimization improvements
- Add a non-root user for better security
- Adjust NODE_OPTIONS based on Cloud Run's memory constraints
- Use more specific version pinning
FROM node:20.18.0-slim AS builder WORKDIR /app +RUN groupadd -r appuser && useradd -r -g appuser appuser +USER appuser + -ARG NODE_OPTIONS="--max-old-space-size=4096" +ARG NODE_OPTIONS="--max-old-space-size=2048" ARG GITHUB_SHA ENV NODE_OPTIONS=$NODE_OPTIONS ENV GITHUB_SHA=$GITHUB_SHA📝 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 node:20.18.0-slim AS builder # Create non-root user RUN groupadd -r appuser && useradd -r -g appuser appuser WORKDIR /app # Ensure proper ownership of the working directory RUN chown appuser:appuser /app # Switch to non-root user USER appuser ARG NODE_OPTIONS="--max-old-space-size=2048" ARG GITHUB_SHA ENV NODE_OPTIONS=$NODE_OPTIONS ENV GITHUB_SHA=$GITHUB_SHA.github/workflows/ci_docker_build_push.yml (1)
89-145: 🛠️ Refactor suggestion
Consider using reusable workflows to reduce duplication.
The
webjob largely mirrors theserverjob's structure. To improve maintainability and reduce duplication, consider:
- Creating a reusable workflow for the common Docker build and push logic
- Parameterizing the differences (image name, context directory, etc.)
- Calling the reusable workflow from both jobs
Example structure:
# .github/workflows/docker-build-push.yml name: Docker Build and Push on: workflow_call: inputs: image_name: required: true context: required: true secrets: dockerhub_username: required: true dockerhub_token: required: true jobs: build: runs-on: ubuntu-latest steps: # Common build steps hereUsage:
jobs: server: uses: ./.github/workflows/docker-build-push.yml with: image_name: reearth/reearth-visualizer context: server secrets: dockerhub_username: ${{ secrets.DOCKERHUB_USERNAME }} dockerhub_token: ${{ secrets.DOCKERHUB_TOKEN }}🧰 Tools
🪛 actionlint
100-100: the runner of "actions/checkout@v2" action is too old to run on GitHub Actions. update the action's version to fix this issue
(action)
117-117: workflow command "set-output" was deprecated. use
echo "{name}={value}" >> $GITHUB_OUTPUTinstead: https://docs.github.com/en/actions/using-workflows/workflow-commands-for-github-actions(deprecated-commands)
117-117: workflow command "set-output" was deprecated. use
echo "{name}={value}" >> $GITHUB_OUTPUTinstead: https://docs.github.com/en/actions/using-workflows/workflow-commands-for-github-actions(deprecated-commands)
117-117: workflow command "set-output" was deprecated. use
echo "{name}={value}" >> $GITHUB_OUTPUTinstead: https://docs.github.com/en/actions/using-workflows/workflow-commands-for-github-actions(deprecated-commands)
137-137: the runner of "docker/build-push-action@v2" action is too old to run on GitHub Actions. update the action's version to fix this issue
(action)
KeisukeYamashita
force-pushed
the
dockerize-web
branch
from
d37868b to
35a5951
Compare
KeisukeYamashita
force-pushed
the
dockerize-web
branch
from
35a5951 to
0b43a59
Compare
There was a problem hiding this comment.
Caution
Inline review comments failed to post. This is likely due to GitHub's limits when posting large numbers of comments.
Actionable comments posted: 4
🧹 Outside diff range and nitpick comments (8)
web/docker/40-envsubst-on-reearth-config.sh (3)
6-6: Document container-specific path assumptions.The hard-coded paths assume a specific nginx container structure. Consider adding a comment to document these container-specific assumptions to help with maintenance and debugging.
-_REEARTH_HTML_FILE="/usr/share/nginx/html/index.html" +# Paths specific to nginx container structure +_REEARTH_HTML_FILE="/usr/share/nginx/html/index.html" # nginx default html directory -_REEARTH_CONFIG_TEMPLATE_FILE="/opt/reearth/reearth_config.json.template" -_REEARTH_CONFIG_OUTPUT_FILE="/usr/share/nginx/html/reearth_config.json" +# Configuration file paths +_REEARTH_CONFIG_TEMPLATE_FILE="/opt/reearth/reearth_config.json.template" # mounted template +_REEARTH_CONFIG_OUTPUT_FILE="/usr/share/nginx/html/reearth_config.json" # accessible by nginxAlso applies to: 9-10
12-22: Improve function documentation.The current comment doesn't fully explain the function's purpose and behavior. Consider adding more detailed documentation:
-# Wrap with "" if the value doesn't start with '{[' and end with ']}' (JSON) or "null". +# wrap_reearth_variables processes environment variables with REEARTH_WEB prefix: +# - Empty values are wrapped in quotes: "" +# - JSON objects/arrays (starting with { or [ and ending with } or ]) are preserved as-is +# - Boolean values (true/false) and null are preserved as-is +# - All other values are wrapped in quotes for proper JSON string formatting +# This ensures all environment variables are properly formatted for the JSON config file.
1-25: Consider Cloud Run deployment requirements.Since this script is part of the containerization for Cloud Run deployment, consider the following Cloud Run specific recommendations:
- Cloud Run injects environment variables at runtime, ensure the script handles dynamic configuration properly
- Cloud Run requires the application to respect PORT environment variable
- Consider adding a health check endpoint configuration
Would you like me to provide specific examples for these Cloud Run optimizations?
.github/workflows/deploy_web_nightly.yml (2)
7-9: Consider parameterizing the GCP region in environment variables.The GCP region is hardcoded in
IMAGE_GCP. Consider:
- Adding
GCP_REGIONas an environment variable- Using the variable in
IMAGE_GCPdefinition+ GCP_REGION: us-central1 IMAGE: reearth/reearth-visualizer-web:nightly - IMAGE_GCP: us-central1-docker.pkg.dev/reearth-oss/reearth/reearth-visualizer-web:nightly + IMAGE_GCP: ${{ env.GCP_REGION }}-docker.pkg.dev/reearth-oss/reearth/reearth-visualizer-web:nightly
35-39: Add health checks and rollback strategy.The deployment configuration should include:
- Health checks to verify deployment success
- Rollback strategy for failed deployments
deploy_test: name: Deploy app to test env runs-on: ubuntu-latest if: github.event.repository.full_name == 'reearth/reearth-visualizer' + environment: + name: test + url: ${{ steps.deploy.outputs.url }}.github/workflows/deploiy_aws.yml (2)
14-20: Fix typo in workflow filename.The workflow filename contains a typo: "deploiy_aws.yml" should be "deploy_aws.yml". This could cause confusion and make it harder to find the file.
44-63: Consider adding job dependencies if order matters.The
deploy_webjob runs independently ofdeploy_server. If the deployment order matters (e.g., if the web frontend depends on the server API), consider adding a dependency using theneedskeyword:deploy_web: needs: deploy_server name: Deploy web to AWS # ... rest of the job configuration🧰 Tools
🪛 actionlint
51-51: property "events" is not defined in object type {action: string; action_path: string; action_ref: string; action_repository: string; action_status: string; actor: string; actor_id: string; api_url: string; base_ref: string; env: string; event: object; event_name: string; event_path: string; graphql_url: string; head_ref: string; job: string; job_workflow_sha: string; path: string; ref: string; ref_name: string; ref_protected: string; ref_type: string; repository: string; repository_id: string; repository_owner: string; repository_owner_id: string; repositoryurl: string; retention_days: number; run_attempt: string; run_id: string; run_number: string; secret_source: string; server_url: string; sha: string; token: string; triggering_actor: string; workflow: string; workflow_ref: string; workflow_sha: string; workspace: string}
(expression)
52-52: property "events" is not defined in object type {action: string; action_path: string; action_ref: string; action_repository: string; action_status: string; actor: string; actor_id: string; api_url: string; base_ref: string; env: string; event: object; event_name: string; event_path: string; graphql_url: string; head_ref: string; job: string; job_workflow_sha: string; path: string; ref: string; ref_name: string; ref_protected: string; ref_type: string; repository: string; repository_id: string; repository_owner: string; repository_owner_id: string; repositoryurl: string; retention_days: number; run_attempt: string; run_id: string; run_number: string; secret_source: string; server_url: string; sha: string; token: string; triggering_actor: string; workflow: string; workflow_ref: string; workflow_sha: string; workspace: string}
(expression)
60-60: shellcheck reported issue in this script: SC2086:info:1:13: Double quote to prevent globbing and word splitting
(shellcheck)
60-60: shellcheck reported issue in this script: SC2086:info:2:12: Double quote to prevent globbing and word splitting
(shellcheck)
60-60: shellcheck reported issue in this script: SC2086:info:2:63: Double quote to prevent globbing and word splitting
(shellcheck)
60-60: shellcheck reported issue in this script: SC2086:info:3:53: Double quote to prevent globbing and word splitting
(shellcheck)
.github/workflows/ci_docker_build_push.yml (1)
136-145: Optimize build cache configuration.The current cache configuration uses GitHub Actions cache (
type=gha). For Docker builds, consider using more efficient caching strategies.Consider enhancing the build-push action configuration:
- name: Build and push docker image uses: docker/build-push-action@v4 with: context: web platforms: ${{ steps.options.outputs.platforms }} push: true build-args: VERSION=${{ steps.options.outputs.version }} tags: ${{ steps.options.outputs.tags }} cache-from: type=gha - cache-to: type=gha,mode=max + cache-to: type=gha,mode=max,image-manifest=true + provenance: falseThis configuration:
- Adds image manifest caching for better layer reuse
- Disables provenance to reduce build time (enable if SLSA compliance is required)
🛑 Comments failed to post (4)
.github/workflows/deploy_web_nightly.yml (1)
52-58: 🛠️ Refactor suggestion
Add deployment verification and monitoring.
The deployment step should include:
- Health check verification
- Basic monitoring setup
- name: Deploy to Cloud Run + id: deploy run: | gcloud run deploy reearth-visualizer-web \ --image $IMAGE_GCP \ --region $GCP_REGION \ --platform managed \ - --quiet + --quiet \ + --format='get(status.url)' > url.txt + echo "url=$(cat url.txt)" >> $GITHUB_OUTPUT + # Verify deployment + DEPLOY_URL=$(cat url.txt) + MAX_RETRIES=5 + COUNT=0 + echo "Verifying deployment..." + while [ $COUNT -lt $MAX_RETRIES ]; do + if curl -s -f -o /dev/null "$DEPLOY_URL"; then + echo "Deployment verified successfully" + exit 0 + fi + COUNT=$((COUNT + 1)) + sleep 10 + done + echo "Deployment verification failed" + 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.- name: Deploy to Cloud Run id: deploy run: | gcloud run deploy reearth-visualizer-web \ --image $IMAGE_GCP \ --region $GCP_REGION \ --platform managed \ --quiet \ --format='get(status.url)' > url.txt echo "url=$(cat url.txt)" >> $GITHUB_OUTPUT # Verify deployment DEPLOY_URL=$(cat url.txt) MAX_RETRIES=5 COUNT=0 echo "Verifying deployment..." while [ $COUNT -lt $MAX_RETRIES ]; do if curl -s -f -o /dev/null "$DEPLOY_URL"; then echo "Deployment verified successfully" exit 0 fi COUNT=$((COUNT + 1)) sleep 10 done echo "Deployment verification failed" exit 1🧰 Tools
🪛 actionlint
53-53: shellcheck reported issue in this script: SC2086:info:2:11: Double quote to prevent globbing and word splitting
(shellcheck)
53-53: shellcheck reported issue in this script: SC2086:info:3:12: Double quote to prevent globbing and word splitting
(shellcheck)
.github/workflows/deploiy_aws.yml (1)
51-52:
⚠️ Potential issueFix invalid context reference in AWS credentials configuration.
The expression
github.events.inputsis incorrect. In GitHub Actions, the correct context reference isgithub.event.inputs(singular "event").Apply this diff to fix the credentials configuration:
- aws-access-key-id: ${{ secrets.AWS_ACCESS_KEY_ID || github.events.inputs.keyId }} - aws-secret-access-key: ${{ secrets.AWS_SECRET_ACCESS_KEY || github.events.inputs.secretAccessKey }} + aws-access-key-id: ${{ secrets.AWS_ACCESS_KEY_ID || github.event.inputs.keyId }} + aws-secret-access-key: ${{ secrets.AWS_SECRET_ACCESS_KEY || github.event.inputs.secretAccessKey }}📝 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.aws-access-key-id: ${{ secrets.AWS_ACCESS_KEY_ID || github.event.inputs.keyId }} aws-secret-access-key: ${{ secrets.AWS_SECRET_ACCESS_KEY || github.event.inputs.secretAccessKey }}🧰 Tools
🪛 actionlint
51-51: property "events" is not defined in object type {action: string; action_path: string; action_ref: string; action_repository: string; action_status: string; actor: string; actor_id: string; api_url: string; base_ref: string; env: string; event: object; event_name: string; event_path: string; graphql_url: string; head_ref: string; job: string; job_workflow_sha: string; path: string; ref: string; ref_name: string; ref_protected: string; ref_type: string; repository: string; repository_id: string; repository_owner: string; repository_owner_id: string; repositoryurl: string; retention_days: number; run_attempt: string; run_id: string; run_number: string; secret_source: string; server_url: string; sha: string; token: string; triggering_actor: string; workflow: string; workflow_ref: string; workflow_sha: string; workspace: string}
(expression)
52-52: property "events" is not defined in object type {action: string; action_path: string; action_ref: string; action_repository: string; action_status: string; actor: string; actor_id: string; api_url: string; base_ref: string; env: string; event: object; event_name: string; event_path: string; graphql_url: string; head_ref: string; job: string; job_workflow_sha: string; path: string; ref: string; ref_name: string; ref_protected: string; ref_type: string; repository: string; repository_id: string; repository_owner: string; repository_owner_id: string; repositoryurl: string; retention_days: number; run_attempt: string; run_id: string; run_number: string; secret_source: string; server_url: string; sha: string; token: string; triggering_actor: string; workflow: string; workflow_ref: string; workflow_sha: string; workspace: string}
(expression)
.github/workflows/ci_docker_build_push.yml (1)
89-98: 🛠️ Refactor suggestion
Ensure consistent configuration between server and web jobs.
The web job configuration largely mirrors the server job, which is good for maintainability. However, there are some inconsistencies in the action versions used.
Consider:
- Using consistent action versions across both jobs
- Extracting common configuration into reusable workflow
.github/workflows/build_web.yml (1)
189-213: 🛠️ Refactor suggestion
Update deprecated GitHub Actions commands.
The workflow uses deprecated
set-outputcommands. These should be updated to use the new$GITHUB_OUTPUTsyntax.Apply this diff:
fi - echo "::set-output name=platforms::$PLATFORMS" - echo "::set-output name=version::$VERSION" - echo "::set-output name=tags::$TAGS" + echo "platforms=$PLATFORMS" >> $GITHUB_OUTPUT + echo "version=$VERSION" >> $GITHUB_OUTPUT + echo "tags=$TAGS" >> $GITHUB_OUTPUT📝 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.- name: Get options id: options env: TAG: ${{ github.event.inputs.new_tag_short && github.event.inputs.new_tag_short != 'blank' && github.event.inputs.new_tag_short || '' }} NAME: ${{ github.event.inputs.name }} SHA: ${{ github.event.inputs.sha_short }} run: | if [[ -n $TAG ]]; then PLATFORMS=linux/amd64,linux/arm64 VERSION=$TAG TAGS=$IMAGE_NAME:$TAG if [[ ! $TAG =~ '-' ]]; then TAGS+=,${IMAGE_NAME}:${TAG%.*} TAGS+=,${IMAGE_NAME}:${TAG%%.*} TAGS+=,${IMAGE_NAME}:latest fi else PLATFORMS=linux/amd64 VERSION=$SHA TAGS=$IMAGE_NAME:$NAME fi echo "platforms=$PLATFORMS" >> $GITHUB_OUTPUT echo "version=$VERSION" >> $GITHUB_OUTPUT echo "tags=$TAGS" >> $GITHUB_OUTPUT🧰 Tools
🪛 actionlint
195-195: workflow command "set-output" was deprecated. use
echo "{name}={value}" >> $GITHUB_OUTPUTinstead: https://docs.github.com/en/actions/using-workflows/workflow-commands-for-github-actions(deprecated-commands)
195-195: workflow command "set-output" was deprecated. use
echo "{name}={value}" >> $GITHUB_OUTPUTinstead: https://docs.github.com/en/actions/using-workflows/workflow-commands-for-github-actions(deprecated-commands)
195-195: workflow command "set-output" was deprecated. use
echo "{name}={value}" >> $GITHUB_OUTPUTinstead: https://docs.github.com/en/actions/using-workflows/workflow-commands-for-github-actions(deprecated-commands)
Signed-off-by: KeisukeYamashita <19yamashita15@gmail.com>
rot1024
force-pushed
the
dockerize-web
branch
from
0b43a59 to
adf6fd3
Compare
KeisukeYamashita
changed the title
Overview
I've Dockerized the web application to be portable and deployable to container runtimes (e.g. Cloud Run, Kubernetes). We aim to host the web on Cloud Run.
What I've done
I've containerised the application and implemented the deployment workflow.
What I haven't done
I haven't decommissioned our GCS stuff yet.
I've implemented it separately from the existing system so that we can deploy, test, and switch over without any impact.
How I tested
I've built in manually:
$ docker build . -t reearth-visualizer-web && docker run -e AUTH0_AUDIENCE="https://api.cms.dev.reearth.io" -e AUTH0_CLIENT_ID="xxx" -e AUTH0_DOMAIN="https://reearth-dev.auth0.com" -p 8080:8080 reearth-visualizer-webWhich point I want you to review particularly
Memo
Summary by CodeRabbit
Release Notes
New Features
Improvements
Chores
.dockerignorefile to streamline Docker builds by excluding unnecessary files.