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

make image not using root user #2380

Merged
merged 4 commits into from
Aug 12, 2021
Merged

make image not using root user #2380

merged 4 commits into from
Aug 12, 2021

Conversation

wkloucek
Copy link
Contributor

@wkloucek wkloucek commented Aug 11, 2021

Description

The owncloud/ocis docker image now uses a non root user and enables you to set a different user with the docker --user parameter. The default user has the UID 1000 is part of a group with the GID 1000.

This is a breaking change for existing docker deployments. The permission on the files and folders in persistent volumes need to be changed to the UID and GID used for oCIS (default 1000:1000 if not changed by the user).

Related Issue

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)
  • Technical debt
  • Tests only (no source changes)

@update-docs
Copy link

update-docs bot commented Aug 11, 2021

Thanks for opening this pull request! The maintainers of this repository would appreciate it if you would create a changelog item based on your changes.

@wkloucek wkloucek mentioned this pull request Aug 11, 2021
9 tasks
@wkloucek
Copy link
Contributor Author

@xoxys what do you think about that change? It will allow you to change the UID of your user with the docker --user run option. But the user needs to care about volume permissions and so on...

The alternative would be some init script which cares about permissions of files and then starts oCIS as a non root user.
Any opinions?

@xoxys
Copy link
Contributor

xoxys commented Aug 12, 2021

Yeah, would love to see it running with a non-root user!

  1. I would not mess around with chown init foo, the user can handle it and changing the user later again is pretty uncommon
  2. There were some issues in the past using Kubernetes/OpenShift where it was required to use USER 700 instead of the username. Not sure if this is still true and what the exact issue was.

@wkloucek
Copy link
Contributor Author

  1. There were some issues in the past using Kubernetes/OpenShift where it was required to use USER 700 instead of the username. Not sure if this is still true and what the exact issue was.

This is an easy change. Any opinions on the UID itself? I picked 700 randomly as a default...

@xoxys
Copy link
Contributor

xoxys commented Aug 12, 2021

The first non-root/non-system UID used by Alpine is 1000 so I would use a UID >= 1000.

@wkloucek wkloucek marked this pull request as ready for review August 12, 2021 13:09
@wkloucek wkloucek requested review from refs, xoxys and micbar August 12, 2021 13:10
Copy link
Contributor

@xoxys xoxys left a comment

Choose a reason for hiding this comment

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

LGTM

@sonarcloud
Copy link

sonarcloud bot commented Aug 12, 2021

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
No Duplication information No Duplication information

@butonic butonic merged commit 8bda1d1 into master Aug 12, 2021
@delete-merged-branch delete-merged-branch bot deleted the non-root-docker-image branch August 12, 2021 20:06
ownclouders pushed a commit that referenced this pull request Aug 12, 2021
Merge: bbac85d 41b26a3
Author: Jörn Friedrich Dreyer <[email protected]>
Date:   Thu Aug 12 22:06:27 2021 +0200

    Merge pull request #2380 from owncloud/non-root-docker-image

    make image not using root user
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