Skip to content

add docker & api route #125

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.

Sign up for GitHub

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

Closed
wvl94 wants to merge 2 commits into from
Closed

add docker & api route #125

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
f0b8ff4
add docker & api route
wvl94 Nov 13, 2024
ab656c6
docker update
wvl94 Nov 14, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 10 additions & 0 deletions Dockerfile
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
FROM node:19-alpine

WORKDIR /usr/src/app

RUN addgroup -S appgroup && adduser -S appuser -G appgroup
USER appuser

RUN apk add --no-cache git
COPY . .
CMD ["npm", "start", "--no-update-notifier"]
Comment on lines +1 to +10
Copy link

@firstmate firstmate bot Nov 14, 2024

Choose a reason for hiding this comment

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

alt text

Your Dockerfile should utilize multi-stage builds to enhance build efficiency and security by separating the build environment from the runtime environment. This will reduce the final image size and potential attack surface. Here's how you can modify your Dockerfile:

# Stage 1: Build
FROM node:19-alpine AS build
WORKDIR /usr/src/app
COPY package*.json ./
RUN npm install
COPY . .
RUN npm run build

# Stage 2: Production
FROM node:19-alpine
WORKDIR /usr/src/app
COPY --from=build /usr/src/app .
CMD ["npm", "start", "--no-update-notifier"]

(Based on guideline 'Dockerfiles should use multi-stage builds')

2 changes: 1 addition & 1 deletion src/config/appConfig.js
View file
Open in desktop
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 +4 to 5
Copy link

@firstmate firstmate bot Nov 14, 2024

Choose a reason for hiding this comment

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

alt text

You changed the default server port from 8080 to 80. This change seems important, so it might be smart to change the documentation accordingly.

For example, update the documentation to reflect the new default port:

## Running the Application

To start the application, use:

```bash
npm start

The application will now run on port 80 by default.

logLevel(){
return process.env.LOG_LEVEL || "info";
Expand Down
3 changes: 3 additions & 0 deletions src/routes/exampleRouter.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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 +13
Copy link

@firstmate firstmate bot Nov 14, 2024

Choose a reason for hiding this comment

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

alt text

The route '/get/:id' lacks permission checks, which are essential for security. You should modify it to include the grantAccessByPermissionMiddleware as follows:

router.route("/get/:id")
    .get(grantAccessByPermissionMiddleware([API_PERMISSIONS.PUBLIC_ENDPOINT]), exampleController.getById)

This ensures that only authorized users can access the endpoint.

(Based on guideline 'Routes should check permissions')


export default router;