-
Notifications
You must be signed in to change notification settings - Fork 0
add docker & api route #119
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,12 @@ | ||
FROM node:19-alpine | ||
ENV PORT 8080 | ||
|
||
WORKDIR /usr/src/app | ||
|
||
RUN addgroup -S appgroup && adduser -S appuser -G appgroup | ||
USER appuser | ||
|
||
RUN apk add --no-cache git | ||
COPY . . | ||
EXPOSE 8080 | ||
CMD ["npm", "start", "--no-update-notifier"] | ||
Comment on lines
+1
to
+12
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Consider using multi-stage builds in your Dockerfile to reduce the final image size and improve security. You can create a builder stage to compile your application and then copy the necessary artifacts to a smaller base image. Here's a simplified example: FROM node:19-alpine AS builder
WORKDIR /usr/src/app
COPY package*.json ./
RUN npm install
COPY . .
FROM node:19-alpine
WORKDIR /usr/src/app
COPY --from=builder /usr/src/app .
EXPOSE 8080
CMD ["npm", "start", "--no-update-notifier"] (Based on guideline 'Dockerfiles should use multi-stage builds') |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,7 +1,7 @@ | ||
class Configuration { | ||
|
||
serverPort() { | ||
return process.env.PORT || 8080; | ||
return process.env.PORT || 80; | ||
} | ||
Comment on lines
3
to
5
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Avoid hardcoding the server port in - return process.env.PORT || 8080;
+ return process.env.PORT || process.env.DEFAULT_PORT || 80; (Based on guideline 'No hardcoded configuration or secrets') |
||
logLevel(){ | ||
return process.env.LOG_LEVEL || "info"; | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -9,4 +9,7 @@ const router = express.Router(); | |
router.route("/:id") | ||
.get(grantAccessByPermissionMiddleware([API_PERMISSIONS.PUBLIC_ENDPOINT]), exampleController.getById) | ||
|
||
router.route("/get/:id").get(exampleController.getById) | ||
|
||
|
||
Comment on lines
+12
to
+14
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ensure that all routes check permissions using the - router.route("/get/:id").get(exampleController.getById)
+ router.route("/get/:id").get(grantAccessByPermissionMiddleware([API_PERMISSIONS.PUBLIC_ENDPOINT]), exampleController.getById) This will help maintain proper access control across your routes. (Based on guideline 'Routes should check permissions') |
||
export default router; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You changed the application port to 8080 in the Dockerfile. This change seems important, so it might be smart to change the documentation accordingly. Additionally, the default port in
appConfig.js
was changed to 80. Ensure the documentation reflects the correct ports for running the application.Example documentation update:
The application will be accessible at
http://localhost:8080
.Configuration
The application runs on port 80 by default unless specified otherwise.