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

Run as non-root users #121

Merged
merged 6 commits into from
Nov 22, 2023
Merged

Run as non-root users #121

merged 6 commits into from
Nov 22, 2023

Conversation

aquarat
Copy link
Contributor

@aquarat aquarat commented Nov 20, 2023

Closes #117

As per the linked issue this PR adds largely a copy of the process used in the airseeker-v2 Dockerfile to make the "app" run as a non-privileged user.

I tried to use variable substitution to reduce the duplication but Dockerfiles are seemingly very limited in that area.

@aquarat aquarat self-assigned this Nov 21, 2023
@aquarat aquarat linked an issue Nov 21, 2023 that may be closed by this pull request
@aquarat aquarat changed the base branch from main to 115-fix-signed-response-schema November 21, 2023 11:35
@aquarat aquarat requested a review from Siegrift November 21, 2023 11:35
@aquarat aquarat marked this pull request as ready for review November 21, 2023 11:36
Base automatically changed from 115-fix-signed-response-schema to main November 21, 2023 11:46
Copy link
Collaborator

@Siegrift Siegrift left a comment

Choose a reason for hiding this comment

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

👍 LGTM.

Didn't test, but there is an e2e test that uses these docker images so if that passes we are good.

Dockerfile Outdated
@@ -34,20 +34,40 @@ RUN pnpm run --recursive build

# Create a separate stage for pusher package. We create a temporary stage for deployment and then copy the result into
# the final stage. Only the production dependencies and package implementation is part of this last stage.
ENV name="deployed-pusher"
Copy link
Collaborator

Choose a reason for hiding this comment

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

Do we use the ENV (and the similar one below) somewhere?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

ah, no, I'll remove it. I tried to use this ENV originally to reduce duplication/single source of truth but Dockerfiles seemingly can't work this way.

@aquarat aquarat merged commit 71ec7f7 into main Nov 22, 2023
4 checks passed
@aquarat aquarat deleted the 117-run-as-non-root-user branch November 22, 2023 15:22
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.

Run pusher and signed API as non root docker user
2 participants