Skip to content
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

Dockerized application for local development, testing and deployment #293

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

pindaroso
Copy link

Relates to:

Distribution.

Risks

Low. Could potentially cause confusion to those unfamiliar with Docker.

Background

What does this PR do?

This PR aims to improve Eliza ergonomics by providing a one liner for getting up and running via Docker.

What kind of change is this?

Development or infrastructure support.

Why are we doing this? Any context or related work?

Improve Eliza accessibility.

Documentation changes needed?

Updated README.

Testing

Where should a reviewer start?

Detailed testing steps

docker:build
docker:run
docker:bash

Deploy Notes

We could potentially add CI/CD. Or an official Docker image. Thoughts?

Database changes

N/A

@tebayoso
Copy link

tebayoso commented Nov 13, 2024

Dockerfile Audit and Suggested Improvements

The current Dockerfile implementation has several areas that could be improved to enhance performance, security, and maintainability. Here’s a detailed list of suggested changes:

  1. No .dockerignore File

    • Without a .dockerignore, unnecessary files and folders (e.g., .git, node_modules, logs) are included in the Docker build context. This increases image size and build time.
    • Suggested Fix: Add a .dockerignore file to exclude files not needed in the container.
  2. Inefficient Layer Caching

    • Dependencies are installed multiple times, which prevents effective caching. Additionally, using separate ADD commands for configuration files results in multiple layers.
    • Suggested Fix: Consolidate the ADD commands for configuration files and install dependencies in a single step. Run pnpm i only once after adding all configuration files.
  3. Redundant RUN pnpm i Commands

    • Dependencies are installed three times throughout the Dockerfile, increasing build time.
    • Suggested Fix: Install dependencies once, after copying all necessary configuration files, to streamline the build process.
  4. Lack of Multi-Stage Build for Production

    • A single-stage build may result in unnecessary development dependencies being included in the final image, which increases image size and introduces security risks.
    • Suggested Fix: Use a multi-stage build to separate build dependencies from the final production image, ensuring only essential files are included.
  5. Unoptimized CMD Command

    • The CMD ["tail", "-f", "/dev/null"] keeps the container idle and suggests the container isn’t configured to run the application.
    • Suggested Fix: Replace CMD with the actual command to start the application, such as CMD ["pnpm", "start"].
  6. Direct Inclusion of Environment Variables (.env File)

    • Adding the .env file with ADD exposes sensitive information in the Docker image. This can pose security risks if the image is pushed to a registry.
    • Suggested Fix: Use Docker secrets or pass environment variables at runtime to secure sensitive data.
  7. Lack of Non-Root User

    • The container runs as the root user by default, which can lead to security vulnerabilities if exploited.
    • Suggested Fix: Create a non-root user and run the application under this user. Example:
      RUN adduser -D appuser
      USER appuser
  8. Missing Port Exposure

    • If the application is meant to run a service (e.g., a web server), the container should expose the relevant port.
    • Suggested Fix: Add an EXPOSE <port> statement for documentation and networking purposes.
  9. Lack of .npmrc Caching

    • If .npmrc contains specific registry or authentication settings, it should be added before dependency installation for caching purposes and authentication consistency.
    • Suggested Fix: Add .npmrc before running pnpm i to leverage caching and reduce unnecessary network requests.
  10. Unused docs Directory

    • Adding docs to the image increases its size without being used by the application.
    • Suggested Fix: Remove the docs directory from the Dockerfile unless it’s necessary for runtime.
  11. Inconsistent Dependency Installation

    • Multiple installations of dependencies without clearing caches or pinning versions may lead to conflicts or unexpected updates.
    • Suggested Fix: Ensure dependencies are installed consistently in one step, and pin specific versions where possible.
  12. Missing Health Checks

    • Without a health check, monitoring systems like Kubernetes cannot accurately assess the container’s health.
    • Suggested Fix: Add a health check for better monitoring, e.g.:
      HEALTHCHECK CMD curl -f http://localhost:<port> || exit 1

Suggested Refactored Dockerfile

Here’s a refactored Dockerfile with the suggested improvements:

# Stage 1: Build dependencies in a temporary stage
FROM node:22-alpine AS builder

# Install required global dependencies
RUN apk add --no-cache python3 make g++ \
    && npm install -g [email protected]

# Set working directory
WORKDIR /app

# Copy essential config files
COPY pnpm-workspace.yaml package.json .npmrc pnpm-lock.yaml ./

# Install dependencies
RUN pnpm install --frozen-lockfile

# Copy source code
COPY tsconfig.json ./
COPY docs ./docs
COPY packages ./packages
COPY scripts ./scripts
COPY characters ./characters
COPY src ./src

# Optional: build step if using TypeScript or other build process
# RUN pnpm build

# Stage 2: Production image
FROM node:22-alpine

# Install dependencies required for the final runtime
RUN apk add --no-cache python3 make g++ \
    && npm install -g [email protected]

# Create a non-root user and switch to it
RUN adduser -D appuser
USER appuser

# Set working directory
WORKDIR /app

# Copy built files from the builder stage
COPY --from=builder /app /app

# Expose application port if running a web server
EXPOSE 3000  # Adjust port as needed

# Add health check to ensure the app is running
HEALTHCHECK --interval=30s --timeout=10s --start-period=5s CMD curl -f http://localhost:3000 || exit 1

# Set environment variables to configure runtime model settings
ENV NODE_ENV=production
# Add more default environment variables if needed, e.g., DISCORD_APPLICATION_ID, etc.

# Default command to run the application
CMD ["pnpm", "start"]

RUN pnpm i

# Add the documentation
ADD docs /app/docs
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

docs needed in the docker?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants