Dockerfile: build backend and frontend into container#40
Dockerfile: build backend and frontend into container#40techbuzzz merged 8 commits intotechbuzzz:mainfrom
Conversation
There was a problem hiding this comment.
Pull request overview
This pull request adds a multi-stage Dockerfile that builds both the Vue.js frontend and Go backend into a single container image, and updates the backend server to serve the frontend static files. This enables a unified deployment model where the backend can optionally serve the frontend directly.
Changes:
- Added multi-stage Dockerfile with frontend (Node.js) and backend (Go) build stages
- Backend now serves static files from
./web/distwith SPA fallback routing - Static file serving is optional - backend runs standalone if frontend is not built
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 8 comments.
| File | Description |
|---|---|
| Dockerfile | Added 3-stage build: frontend (Node.js/Vue), backend (Go), and final Alpine image with both artifacts |
| cmd/server/main.go | Added static file serving with SPA routing support, registered before MCP catch-all routes |
| } | ||
|
|
||
| // Try to serve the requested file | ||
| path := distDir + req.URL.Path |
There was a problem hiding this comment.
Using string concatenation to construct file paths is vulnerable to path traversal attacks and doesn't handle path separators correctly across operating systems. Import "path/filepath" and use filepath.Join with filepath.Clean to safely construct the path. For example: path := filepath.Join(distDir, filepath.Clean(req.URL.Path)). This prevents attackers from using paths like "/../../../etc/passwd" to access files outside the intended directory. This follows the established convention in the codebase (see cmd/server/migrations_test.go and internal/task/store.go).
| # Stage 1: Build Frontend (Vue.js) | ||
| FROM node:18-alpine AS frontend-builder | ||
|
|
||
| WORKDIR /app | ||
|
|
||
| # Copy frontend package files | ||
| COPY web/package*.json ./ | ||
| RUN npm install | ||
|
|
||
| # Copy frontend source | ||
| COPY web/ ./ | ||
|
|
||
| # Build frontend for production | ||
| RUN npm run build |
There was a problem hiding this comment.
Consider adding a .dockerignore file to exclude unnecessary files from the Docker build context. This will reduce build time and image size by excluding files like node_modules, .git, test files, and development artifacts. Create a .dockerignore with entries such as: .git, node_modules, web/node_modules, .test, coverage., .vscode, .idea, *.md (except those explicitly copied), and postgres_data/.
| # Copy frontend source | ||
| COPY web/ ./ |
There was a problem hiding this comment.
The COPY command copies the entire web/ directory including any locally built files, node_modules, or other artifacts that might exist in the build context. Since npm install has already been run at line 8, this could cause conflicts or include unnecessary files. Consider using a .dockerignore file to exclude web/node_modules, web/dist, and other build artifacts from the build context. Alternatively, copy only the necessary source files explicitly (e.g., COPY web/src ./src, COPY web/public ./public, etc.).
| # Copy frontend source | |
| COPY web/ ./ | |
| # Copy frontend source and config (exclude node_modules, dist, etc.) | |
| COPY web/src ./src | |
| COPY web/public ./public | |
| COPY web/vite.config.* ./ | |
| COPY web/tsconfig*.json ./ | |
| COPY web/.eslintrc* ./ | |
| COPY web/postcss.config.* ./ | |
| COPY web/tailwind.config.* ./ |
| // Serve static files from web/dist (if exists) - BEFORE catch-all routes | ||
| distDir := "./web/dist" | ||
| if _, err := os.Stat(distDir); err == nil { | ||
| log.Println("Serving frontend from ./web/dist") | ||
| fs := http.FileServer(http.Dir(distDir)) | ||
| r.PathPrefix("/").Handler(http.HandlerFunc(func(w http.ResponseWriter, req *http.Request) { | ||
| // Skip static serving for API/MCP routes | ||
| if strings.HasPrefix(req.URL.Path, "/api") || | ||
| strings.HasPrefix(req.URL.Path, "/mcp") || | ||
| strings.HasPrefix(req.URL.Path, "/ws") || | ||
| strings.HasPrefix(req.URL.Path, "/a2a") || | ||
| strings.HasPrefix(req.URL.Path, "/.well-known") || | ||
| req.URL.Path == "/health" { | ||
| // Let other handlers handle it | ||
| http.NotFound(w, req) | ||
| return | ||
| } | ||
|
|
||
| // Try to serve the requested file | ||
| path := distDir + req.URL.Path | ||
| if _, err := os.Stat(path); os.IsNotExist(err) { | ||
| // File doesn't exist, serve index.html for SPA routing | ||
| http.ServeFile(w, req, distDir+"/index.html") | ||
| return | ||
| } | ||
| // File exists, serve it | ||
| fs.ServeHTTP(w, req) | ||
| })) |
There was a problem hiding this comment.
The new static file serving logic lacks test coverage. Given that the codebase has integration tests for handlers (see tests/a2a/integration_test.go), consider adding integration tests for the static file serving functionality. Tests should cover: serving existing static files, serving index.html for non-existent paths (SPA routing), handling of API/MCP route exclusions, and path traversal attack prevention.
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
No description provided.