ci: docker testnet #58
Conversation
fmorency
temporarily deployed
to
Production - Testnet
GitHub Actions
Inactive
WalkthroughA new GitHub Actions workflow named Changes
Sequence Diagram(s)sequenceDiagram
participant Developer
participant GitHub Actions
participant Docker Hub
Developer->>GitHub Actions: Push to main branch
GitHub Actions->>GitHub Actions: Checkout Code
GitHub Actions->>GitHub Actions: Set Up Docker Buildx
GitHub Actions->>Docker Hub: Log in
GitHub Actions->>GitHub Actions: Create .env file
GitHub Actions->>Docker Hub: Build and Push Docker Image
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
CodeRabbit Configuration File (
|
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #58 +/- ##
=======================================
Coverage 59.07% 59.07%
=======================================
Files 148 148
Lines 13860 13860
=======================================
Hits 8188 8188
Misses 5672 5672 ☔ View full report in Codecov by Sentry. 🚨 Try these New Features:
|
fmorency
had a problem deploying
to
Production - Testnet
GitHub Actions
Error
fmorency
temporarily deployed
to
Production - Testnet
GitHub Actions
Inactive
fmorency
temporarily deployed
to
Production - Testnet
GitHub Actions
Inactive
fmorency
temporarily deployed
to
Production - Testnet
GitHub Actions
Inactive
fmorency
had a problem deploying
to
Production - Testnet
GitHub Actions
Error
fmorency
had a problem deploying
to
Production - Testnet
GitHub Actions
Error
fmorency
had a problem deploying
to
Production - Testnet
GitHub Actions
Error
fmorency
temporarily deployed
to
Production - Testnet
GitHub Actions
Inactive
fmorency
had a problem deploying
to
Production - Testnet
GitHub Actions
Error
fmorency
had a problem deploying
to
Production - Testnet
GitHub Actions
Error
fmorency
had a problem deploying
to
Production - Testnet
GitHub Actions
Error
fmorency
had a problem deploying
to
Production - Testnet
GitHub Actions
Error
fmorency
temporarily deployed
to
Production - Testnet
GitHub Actions
Inactive
fmorency
had a problem deploying
to
Production - Testnet
GitHub Actions
Error
fmorency
temporarily deployed
to
Production - Testnet
GitHub Actions
Inactive
fmorency
had a problem deploying
to
Production - Testnet
GitHub Actions
Error
fmorency
had a problem deploying
to
Production - Testnet
GitHub Actions
Failure
fmorency
had a problem deploying
to
Production - Testnet
GitHub Actions
Error
fmorency
temporarily deployed
to
Production - Testnet
GitHub Actions
Inactive
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Outside diff range and nitpick comments (5)
next.config.js (1)
Most configured image domains and patterns are actively used, but some can be removed
Based on the codebase analysis, while many of the configured domains are actively used in components like
DenomImage.tsxand various dynamic URL constructions, some domains in the configuration appear unused:
- Domains that can be safely removed from the configuration:
imgix.netstaticflickr.comtwimg.compinimg.comdropboxusercontent.comThe remaining domains are either directly referenced in the code or used in dynamic image URL construction, particularly for validator logos, IPFS content, and chain-related images.
🔗 Analysis chain
Line range hint
8-67: Verify necessity of configured image domains and patternsThe configuration includes a substantial list of image domains and remote patterns. While this provides flexibility, it's recommended to review and verify that all these domains are actively required by the application.
Let me generate additional scripts to check for actual usage of the Next.js Image component and any dynamic image URLs in the codebase.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for image references in the codebase to verify domain usage # Look for image URLs or domain references in code echo "Searching for image domain usage in code..." rg -g '!{node_modules,build,.next}/*' -i -A 2 \ 'imgur\.com|cloudfront\.net|jsdelivr\.net|githubusercontent\.com|amazonaws\.com|googleapis\.com|cloudinary\.com|unsplash\.com|giphy\.com|istockphoto\.com' # Look for next/image component usage echo "Searching for Next.js Image component usage..." ast-grep --pattern 'Image src={$_}'Length of output: 3447
Script:
#!/bin/bash # Search for Next.js Image component usage with different pattern variations echo "Searching for Next.js Image component usage patterns..." ast-grep --pattern 'import { Image } from "next/image"' ast-grep --pattern 'import Image from "next/image"' # Search for Image component implementations echo "Searching for Image component implementations..." rg -g '!{node_modules,build,.next}/*' '<Image' -A 2 # Search for dynamic image URL construction echo "Searching for dynamic image URL patterns..." rg -g '!{node_modules,build,.next}/*' -i 'https?://(.*?)\${' -A 2 rg -g '!{node_modules,build,.next}/*' -i '`https?://' -A 2Length of output: 6731
.github/workflows/docker.yml (3)
3-7: Consider adding PR trigger for testing buildsWhile it's good to limit production image pushes to the main branch, consider adding a
pull_requesttrigger that builds but doesn't push the image. This would help catch Docker-related issues earlier in the development cycle.on: push: branches: - main + pull_request: + branches: + - main
41-46: Enhance Docker build configurationConsider the following improvements for better security and performance:
- Add SHA digest for image immutability
- Specify platform for multi-arch support
- Enable build caching
- name: Build and push Docker image uses: docker/build-push-action@v6 with: context: . push: true - tags: lifted/manifest-app:testnet + tags: | + lifted/manifest-app:testnet + lifted/manifest-app:testnet-${{ github.sha }} + platforms: linux/amd64,linux/arm64 + cache-from: type=gha + cache-to: type=gha,mode=max
8-12: Add workflow timeouts and error handlingConsider adding:
- Workflow timeout to prevent hung builds
- Explicit error handling for Docker operations
jobs: build-and-push: runs-on: ubuntu-latest environment: 'Production - Testnet' + timeout-minutes: 30 + + # Cancel any running workflow in the same branch + concurrency: + group: ${{ github.workflow }}-${{ github.ref }} + cancel-in-progress: trueDockerfile (1)
52-53: Avoid hardcoding user and group IDsHardcoding UID and GID can lead to conflicts on different systems or when running in various environments. Consider omitting the UID and GID specifications to allow the system to assign them automatically.
Apply this diff to let the system assign the IDs:
-RUN addgroup --system --gid 1001 nodejs -RUN adduser --system --uid 1001 nextjs +RUN addgroup --system nodejs +RUN adduser --system nextjs
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (5)
.github/workflows/docker.yml(1 hunks).gitignore(1 hunks)Dockerfile(1 hunks)next.config.js(1 hunks)package.json(2 hunks)
✅ Files skipped from review due to trivial changes (2)
- .gitignore
- package.json
🧰 Additional context used
🪛 actionlint
.github/workflows/docker.yml
27-27: shellcheck reported issue in this script: SC2129:style:2:1: Consider using { cmd1; cmd2; } >> file instead of individual redirects
(shellcheck)
🔇 Additional comments (5)
next.config.js (1)
3-3: LGTM! Standalone output configuration is appropriate for Docker builds
The addition of output: 'standalone' is correct and aligns with Next.js best practices for containerization. This configuration will generate a self-contained output that includes all necessary dependencies, making it ideal for multi-stage Docker builds.
.github/workflows/docker.yml (1)
8-11: LGTM! Good environment segregation
The explicit environment setting ensures proper secret isolation for the testnet deployment.
Dockerfile (3)
70-70: Confirm the usage of HOSTNAME environment variable
Setting HOSTNAME="0.0.0.0" may not configure the server to listen on all interfaces as intended. Typically, HOST or HOST_IP is used for binding to an IP address.
Check if your application uses the HOSTNAME variable to bind the server. If not, consider updating the environment variable:
-ENV HOSTNAME="0.0.0.0"
+ENV HOST="0.0.0.0"Ensure that the application reads the HOST environment variable to bind to the correct interface.
59-60: Ensure ownership and permissions are correctly set when copying files
When using --chown in the COPY command, confirm that your Docker version supports this feature and that the ownership change is applied as expected.
Verify that the ownership is correctly set in the image. If --chown is not supported, you might need to add a chown command after copying:
COPY --from=builder /app/.next/standalone ./
COPY --from=builder /app/.next/static ./.next/static
+RUN chown -R nextjs:nodejs ./38-38: Verify the correctness of the Bun build command
Ensure that bun run build executes the build script as intended. The command assumes that build is defined in your package.json scripts. If not, the build may fail.
Run the following script to confirm that the build script exists:
If the build script is not defined, consider adding it to your package.json.
✅ Verification successful
Build script is correctly defined in package.json
The build script is properly defined in package.json as "next build", which is the correct command for building a Next.js application. The Dockerfile's usage of bun run build will correctly execute this script.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check if the build script is defined in package.json.
# Expected output: The build script command.
cat package.json | jq '.scripts.build'
Length of output: 51
| - name: Create .env file | ||
| run: | | ||
| touch .env | ||
| echo "NEXT_PUBLIC_CHAIN=${{ vars.NEXT_PUBLIC_CHAIN }}" >> .env | ||
| echo "NEXT_PUBLIC_CHAIN_ID=${{ vars.NEXT_PUBLIC_CHAIN_ID }}" >> .env | ||
| echo "NEXT_PUBLIC_TESTNET_CHAIN_ID=${{ vars.NEXT_PUBLIC_TESTNET_CHAIN_ID }}" >> .env | ||
| echo "NEXT_PUBLIC_MAINNET_RPC_URL=${{ vars.NEXT_PUBLIC_MAINNET_RPC_URL }}" >> .env | ||
| echo "NEXT_PUBLIC_TESTNET_RPC_URL=${{ vars.NEXT_PUBLIC_TESTNET_RPC_URL }}" >> .env | ||
| echo "NEXT_PUBLIC_MAINNET_API_URL=${{ vars.NEXT_PUBLIC_MAINNET_API_URL }}" >> .env | ||
| echo "NEXT_PUBLIC_TESTNET_API_URL=${{ vars.NEXT_PUBLIC_TESTNET_API_URL }}" >> .env | ||
| echo "NEXT_PUBLIC_ABLY_API_KEY=${{ secrets.NEXT_PUBLIC_ABLY_API_KEY }}" >> .env | ||
| echo "NEXT_PUBLIC_WALLETCONNECT_KEY=${{ secrets.NEXT_PUBLIC_WALLETCONNECT_KEY }}" >> .env | ||
| echo "NEXT_PUBLIC_WEB3_CLIENT_ID=${{ secrets.NEXT_PUBLIC_WEB3_CLIENT_ID }}" >> .env | ||
| cat .env | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion
Remove sensitive environment file logging
The cat .env command will expose secrets in the workflow logs. This is a security risk.
- cat .envImprove environment file creation
The current implementation has several areas for improvement:
- Use a here-document for better readability and efficiency
- Add validation for required variables
- name: Create .env file
run: |
- touch .env
- echo "NEXT_PUBLIC_CHAIN=${{ vars.NEXT_PUBLIC_CHAIN }}" >> .env
- echo "NEXT_PUBLIC_CHAIN_ID=${{ vars.NEXT_PUBLIC_CHAIN_ID }}" >> .env
- # ... more echoes
+ cat << EOF > .env
+ NEXT_PUBLIC_CHAIN=${{ vars.NEXT_PUBLIC_CHAIN }}
+ NEXT_PUBLIC_CHAIN_ID=${{ vars.NEXT_PUBLIC_CHAIN_ID }}
+ NEXT_PUBLIC_TESTNET_CHAIN_ID=${{ vars.NEXT_PUBLIC_TESTNET_CHAIN_ID }}
+ NEXT_PUBLIC_MAINNET_RPC_URL=${{ vars.NEXT_PUBLIC_MAINNET_RPC_URL }}
+ NEXT_PUBLIC_TESTNET_RPC_URL=${{ vars.NEXT_PUBLIC_TESTNET_RPC_URL }}
+ NEXT_PUBLIC_MAINNET_API_URL=${{ vars.NEXT_PUBLIC_MAINNET_API_URL }}
+ NEXT_PUBLIC_TESTNET_API_URL=${{ vars.NEXT_PUBLIC_TESTNET_API_URL }}
+ NEXT_PUBLIC_ABLY_API_KEY=${{ secrets.NEXT_PUBLIC_ABLY_API_KEY }}
+ NEXT_PUBLIC_WALLETCONNECT_KEY=${{ secrets.NEXT_PUBLIC_WALLETCONNECT_KEY }}
+ NEXT_PUBLIC_WEB3_CLIENT_ID=${{ secrets.NEXT_PUBLIC_WEB3_CLIENT_ID }}
+ EOF
+
+ # Validate required variables
+ missing_vars=()
+ while IFS= read -r line; do
+ var_name=$(echo "$line" | cut -d= -f1)
+ var_value=$(echo "$line" | cut -d= -f2)
+ if [ -z "$var_value" ]; then
+ missing_vars+=("$var_name")
+ fi
+ done < .env
+
+ if [ ${#missing_vars[@]} -ne 0 ]; then
+ echo "Error: Missing required variables: ${missing_vars[*]}"
+ exit 1
+ fi📝 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: Create .env file | |
| run: | | |
| touch .env | |
| echo "NEXT_PUBLIC_CHAIN=${{ vars.NEXT_PUBLIC_CHAIN }}" >> .env | |
| echo "NEXT_PUBLIC_CHAIN_ID=${{ vars.NEXT_PUBLIC_CHAIN_ID }}" >> .env | |
| echo "NEXT_PUBLIC_TESTNET_CHAIN_ID=${{ vars.NEXT_PUBLIC_TESTNET_CHAIN_ID }}" >> .env | |
| echo "NEXT_PUBLIC_MAINNET_RPC_URL=${{ vars.NEXT_PUBLIC_MAINNET_RPC_URL }}" >> .env | |
| echo "NEXT_PUBLIC_TESTNET_RPC_URL=${{ vars.NEXT_PUBLIC_TESTNET_RPC_URL }}" >> .env | |
| echo "NEXT_PUBLIC_MAINNET_API_URL=${{ vars.NEXT_PUBLIC_MAINNET_API_URL }}" >> .env | |
| echo "NEXT_PUBLIC_TESTNET_API_URL=${{ vars.NEXT_PUBLIC_TESTNET_API_URL }}" >> .env | |
| echo "NEXT_PUBLIC_ABLY_API_KEY=${{ secrets.NEXT_PUBLIC_ABLY_API_KEY }}" >> .env | |
| echo "NEXT_PUBLIC_WALLETCONNECT_KEY=${{ secrets.NEXT_PUBLIC_WALLETCONNECT_KEY }}" >> .env | |
| echo "NEXT_PUBLIC_WEB3_CLIENT_ID=${{ secrets.NEXT_PUBLIC_WEB3_CLIENT_ID }}" >> .env | |
| cat .env | |
| - name: Create .env file | |
| run: | | |
| cat << EOF > .env | |
| NEXT_PUBLIC_CHAIN=${{ vars.NEXT_PUBLIC_CHAIN }} | |
| NEXT_PUBLIC_CHAIN_ID=${{ vars.NEXT_PUBLIC_CHAIN_ID }} | |
| NEXT_PUBLIC_TESTNET_CHAIN_ID=${{ vars.NEXT_PUBLIC_TESTNET_CHAIN_ID }} | |
| NEXT_PUBLIC_MAINNET_RPC_URL=${{ vars.NEXT_PUBLIC_MAINNET_RPC_URL }} | |
| NEXT_PUBLIC_TESTNET_RPC_URL=${{ vars.NEXT_PUBLIC_TESTNET_RPC_URL }} | |
| NEXT_PUBLIC_MAINNET_API_URL=${{ vars.NEXT_PUBLIC_MAINNET_API_URL }} | |
| NEXT_PUBLIC_TESTNET_API_URL=${{ vars.NEXT_PUBLIC_TESTNET_API_URL }} | |
| NEXT_PUBLIC_ABLY_API_KEY=${{ secrets.NEXT_PUBLIC_ABLY_API_KEY }} | |
| NEXT_PUBLIC_WALLETCONNECT_KEY=${{ secrets.NEXT_PUBLIC_WALLETCONNECT_KEY }} | |
| NEXT_PUBLIC_WEB3_CLIENT_ID=${{ secrets.NEXT_PUBLIC_WEB3_CLIENT_ID }} | |
| EOF | |
| # Validate required variables | |
| missing_vars=() | |
| while IFS= read -r line; do | |
| var_name=$(echo "$line" | cut -d= -f1) | |
| var_value=$(echo "$line" | cut -d= -f2) | |
| if [ -z "$var_value" ]; then | |
| missing_vars+=("$var_name") | |
| fi | |
| done < .env | |
| if [ ${#missing_vars[@]} -ne 0 ]; then | |
| echo "Error: Missing required variables: ${missing_vars[*]}" | |
| exit 1 | |
| fi |
🧰 Tools
🪛 actionlint
27-27: shellcheck reported issue in this script: SC2129:style:2:1: Consider using { cmd1; cmd2; } >> file instead of individual redirects
(shellcheck)
| WORKDIR /app | ||
|
|
||
| # Install dependencies based on the preferred package manager | ||
| COPY package.json yarn.lock* package-lock.json* pnpm-lock.yaml* .npmrc* bun.lockb ./ |
There was a problem hiding this comment.
Security Concern: Avoid copying .npmrc* files into the image
Copying .npmrc* files into the Docker image can unintentionally expose sensitive information, such as authentication tokens or registry credentials. It's safer to exclude .npmrc* from the COPY command.
Apply this diff to address the issue:
-COPY package.json yarn.lock* package-lock.json* pnpm-lock.yaml* .npmrc* bun.lockb ./
+COPY package.json yarn.lock* package-lock.json* pnpm-lock.yaml* bun.lockb ./📝 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.
| COPY package.json yarn.lock* package-lock.json* pnpm-lock.yaml* .npmrc* bun.lockb ./ | |
| COPY package.json yarn.lock* package-lock.json* pnpm-lock.yaml* bun.lockb ./ |
| RUN \ | ||
| if [ -f .env ]; then echo ".env file found, continuing..."; else echo ".env file not found, exiting..."; exit 1; fi |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Avoid including .env file during the build process
Requiring and copying the .env file during the build can expose sensitive environment variables within the image. It's more secure to handle environment variables at runtime rather than during the build.
Modify the Dockerfile to remove the dependency on the .env file during build:
RUN \
- if [ -f .env ]; then echo ".env file found, continuing..."; else echo ".env file not found, exiting..."; exit 1; fi
+ echo "Starting the build process..."
# Remove the command that deletes the .env file since it won't be included
-RUN rm -rf .envAdjust your application configuration to use environment variables provided at runtime rather than relying on an .env file within the image.
📝 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.
| RUN \ | |
| if [ -f .env ]; then echo ".env file found, continuing..."; else echo ".env file not found, exiting..."; exit 1; fi | |
| RUN \ | |
| echo "Starting the build process..." |
| FROM base AS builder | ||
| WORKDIR /app | ||
| COPY --from=deps /app/node_modules ./node_modules | ||
| COPY . . |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Prevent copying unnecessary or sensitive files into the image
Using COPY . . may include unnecessary files and potentially sensitive information like .env, .git, or local development artifacts. It's recommended to specify the files and directories needed for the build or use a .dockerignore file to exclude unwanted files.
Create a .dockerignore file with the following content to exclude unnecessary files:
# .dockerignore
.git
.gitignore
Dockerfile
README.md
.env
node_modules
Alternatively, adjust the COPY command to include only the necessary files and directories:
-COPY . .
+COPY pages ./pages
+COPY public ./public
+COPY components ./components
+COPY next.config.js ./
+COPY package.json ./Committable suggestion skipped: line range outside the PR's diff.
This PR builds and push the
lifted/manifest-app:testnetDocker image on DockerHub.https://hub.docker.com/repository/docker/lifted/manifest-app/general
Relates #59
Summary by CodeRabbit
Release Notes
New Features
Bug Fixes
.gitignorerules for environment files.Chores
package.jsonfor better alignment with the new project identity.