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

Updated to correct file ownership #4

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

rsbisho2
Copy link

@rsbisho2 rsbisho2 commented Jun 6, 2020

I had to make this change in order to have all files in the /home/foundry/app directory owned by the user foundry. They were previously owned by root. This caused issues when trying to update 0.6.0 to 0.6.1 since the app user could not modify the files in /home/foundry/app

I had to make this change in order to have all files in the /home/foundry/app directory owned by the user foundry.  They were previously owned by root.  This caused issues when trying to update 0.6.0 to 0.6.1 since the app user could not modify the files in /home/foundry/app
@rsbisho2
Copy link
Author

Hey guys - any feedback on this? It worked for me, but I don't know if this is best practice or not.
Thanks

@felipearmat
Copy link

felipearmat commented Sep 5, 2020

This seems a good aproach, has anyone approved its merge?

@rsbisho2
Copy link
Author

rsbisho2 commented Sep 5, 2020

I don't think so. Not sure if this is active or not.

@foresto
Copy link

foresto commented Sep 25, 2020

This approach looks needlessly complex to me. It adds multiple extra instructions, adds at least one extra layer, and goes counter to best practices by switching USER back and forth.

Couldn't the same effect be accomplished simply by using --chown=$UID:$GID in the existing COPY instruction? So, in Dockerfile, replace this:

COPY . .

With this:

COPY --chown=$UID:$GID . .

@rsbisho2
Copy link
Author

No idea. I'm dockerfile novice. As long as foundry is the owner of that directory, that should work. That was the issue I was trying to solve.

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