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

Dockerfile non-root user #36

Closed
wants to merge 13 commits into from
Closed

Dockerfile non-root user #36

wants to merge 13 commits into from

Conversation

gpmayorga
Copy link

@gpmayorga gpmayorga commented Feb 29, 2024

Change the Dockefile to run as non-root by default.

Copy link
Contributor

@filo87 filo87 left a comment

Choose a reason for hiding this comment

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

I would also update the node FROM image to a more generic 18-alpine so we get a better upgrading.

For the username lets stick to node, so we can use it also for outflux.

Platform specification to amd64 might break Development on AppleSilicon

Dockerfile Outdated
RUN yarn install --production=true --frozen-lockfile

FROM node:18.14-alpine as prod-build

WORKDIR /usr/src/app

# Create a non-root user and switch to it
# Note: You might need to adjust permissions more restrictively depending on your app's specific needs
RUN adduser -D indexer && chown -R indexer:indexer /usr/src/app
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
RUN adduser -D indexer && chown -R indexer:indexer /usr/src/app
RUN adduser -D node && chown -R node:node /usr/src/app

Dockerfile Outdated
# Create a non-root user and switch to it
# Note: You might need to adjust permissions more restrictively depending on your app's specific needs
RUN adduser -D indexer && chown -R indexer:indexer /usr/src/app
USER indexer
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
USER indexer
USER node

Dockerfile Outdated
RUN adduser -D indexer && chown -R indexer:indexer /usr/src/app
USER indexer

# Copy necessary files
COPY package*.json ./
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
COPY package*.json ./
COPY --chown=node:node package*.json ./

Copy link
Author

Choose a reason for hiding this comment

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

redundant, as we have already selected USER before hand and therefore everything below it will inherit this user's permissions

Dockerfile Outdated
RUN adduser -D indexer && chown -R indexer:indexer /usr/src/app
USER indexer

# Copy necessary files
COPY package*.json ./
COPY yarn*.lock ./
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
COPY yarn*.lock ./
COPY --chown=node:node yarn*.lock ./

Copy link
Author

Choose a reason for hiding this comment

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

redundant, as we have already selected USER before hand and therefore everything below it will inherit this user's permissions

Dockerfile Outdated
RUN adduser -D indexer && chown -R indexer:indexer /usr/src/app
USER indexer

# Copy necessary files
COPY package*.json ./
COPY yarn*.lock ./
COPY --from=build /usr/src/app/dist ./dist
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
COPY --from=build /usr/src/app/dist ./dist
COPY --from=build --chown=node:node /usr/src/app/dist ./dist

Copy link
Author

Choose a reason for hiding this comment

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

redundant, as we have already selected USER before hand and therefore everything below it will inherit this user's permissions

Dockerfile Outdated
RUN adduser -D indexer && chown -R indexer:indexer /usr/src/app
USER indexer

# Copy necessary files
COPY package*.json ./
COPY yarn*.lock ./
COPY --from=build /usr/src/app/dist ./dist
COPY --from=prod-install /usr/src/app/node_modules ./node_modules
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
COPY --from=prod-install /usr/src/app/node_modules ./node_modules
COPY --from=prod-install --chown=node:node /usr/src/app/node_modules ./node_modules

Copy link
Author

Choose a reason for hiding this comment

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

redundant, as we have already selected USER before hand and therefore everything below it will inherit this user's permissions

Dockerfile Outdated
@@ -1,37 +1,55 @@
FROM node:18.14-alpine as install
# --platform=linux/amd64 is necessary when building on M1/M3 Macintosh locally
FROM --platform=linux/amd64 node:18.14-alpine as install
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
FROM --platform=linux/amd64 node:18.14-alpine as install
FROM --platform=linux/amd64 node:18-alpine as install

Copy link
Contributor

Choose a reason for hiding this comment

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

Lets pack in also some better upgrading of node

Copy link
Contributor

Choose a reason for hiding this comment

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

Also I do not know if specifying the platform is a good idea...because this could break development on AppleSilicon

Copy link
Author

Choose a reason for hiding this comment

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

quite the opposite, AppleSilicon can run Linux based arch containers as the engine is fully virtualized (a Linux VM running on your mac) even though sometimes this might not work.
This way, if you build a container and push from your laptop, it will not be Mac specific which is a better outcome than pushing a docker container with the wrong arch instead

Dockerfile Outdated
RUN yarn build

FROM node:18.14-alpine as prod-install
FROM --platform=linux/amd64 node:18.14-alpine as prod-install
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
FROM --platform=linux/amd64 node:18.14-alpine as prod-install
FROM --platform=linux/amd64 node:18-alpine as prod-install

Dockerfile Outdated
COPY package*.json ./
COPY yarn*.lock ./

# Install only production dependencies
RUN yarn install --production=true --frozen-lockfile

FROM node:18.14-alpine as prod-build
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
FROM node:18.14-alpine as prod-build
FROM node:18-alpine as prod-build

Dockerfile Outdated
COPY package*.json ./
COPY yarn*.lock ./

# Install dependencies
RUN yarn install --production=false

FROM node:18.14-alpine as build
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
FROM node:18.14-alpine as build
FROM node:18-alpine as build

@centrifuge centrifuge closed this by deleting the head repository May 8, 2024
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.

3 participants