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.

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

Jump to bottom

Run as non-root #221

Merged
merged 1 commit into from
Jan 22, 2024
Merged

Run as non-root #221

merged 1 commit into from
Jan 22, 2024

Conversation

iaindillingham
Copy link
Member

Closes #196

@iaindillingham iaindillingham force-pushed the iaindillingham/docker-image-non-root-user branch 2 times, most recently from bd4d6de to ca08b15 Compare January 17, 2024 15:41
madwort
madwort reviewed Jan 17, 2024
View reviewed changes
docker/Dockerfile Outdated Show resolved Hide resolved
madwort
madwort previously requested changes Jan 17, 2024
View reviewed changes
Copy link
Contributor

@madwort madwort left a comment

Choose a reason for hiding this comment

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

sorry, think we need to use a different uid

@rebkwok
Copy link
Contributor

rebkwok commented Jan 17, 2024

Does actions-registry write anything (other than the db, which I assume is in a mounted volume in the dokku app)? This was the thing that caused most problems for opencodelists. If everything it needs write access to is in a mounted storage volume, the sysadmin ansible playbook will do the work of making sure the non-root user owns it.

@iaindillingham iaindillingham force-pushed the iaindillingham/docker-image-non-root-user branch from ca08b15 to 7553665 Compare January 17, 2024 17:29
@iaindillingham
Copy link
Member Author

Thanks for your comments, @madwort and @rebkwok. I've added a user/group for actions-registry on dokku3 and updated docker/Dockerfile. See ebmdatalab/sysadmin#322.

Does actions-registry write anything?

Other than the DB, no, it doesn't.

rebkwok
rebkwok requested changes Jan 17, 2024
View reviewed changes
docker/Dockerfile Outdated
@@ -154,6 +154,9 @@ LABEL org.opencontainers.image.created=$BUILD_DATE
ARG GITREF=unknown
LABEL org.opencontainers.image.revision=$GITREF

ARG USERID=10004
ARG GROUPID=10004
Copy link
Contributor

Choose a reason for hiding this comment

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

Sorry, this uid is taken (by ebmbot); 10005 should be OK

Copy link
Member Author

Choose a reason for hiding this comment

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

Thanks, @rebkwok. I've updated to 10005.

@iaindillingham iaindillingham force-pushed the iaindillingham/docker-image-non-root-user branch from 7553665 to 0097dd2 Compare January 18, 2024 09:26
@iaindillingham iaindillingham mentioned this pull request Jan 18, 2024
Closed
@iaindillingham iaindillingham dismissed madwort’s stale review January 22, 2024 12:23

First, thanks for your review! Becky requested the same change and I've updated the PR (and also updated dokku3 in a related PR). Becky has approved the PR but you're away until Wednesday, so you can't reasonably do the same. As such, I'm going to dismiss your review and merge.

@iaindillingham iaindillingham merged commit 4cb5958 into main Jan 22, 2024
6 checks passed
@iaindillingham iaindillingham deleted the iaindillingham/docker-image-non-root-user branch January 22, 2024 12:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@madwort madwort madwort left review comments

@rebkwok rebkwok rebkwok approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Run as non-root
3 participants
@iaindillingham @rebkwok @madwort