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

Handle file permissions in a cleaner way while building Docker images #3238

Merged
merged 2 commits into from Apr 15, 2023

Conversation

knu
Copy link
Member

@knu knu commented Mar 19, 2023

  • Use COPY --chown wisely
  • Switch USER earlier so that generated files are owned by the user

knu added 2 commits March 19, 2023 22:13
- Use COPY --chown wisely
- Switch USER earlier so that generated files are owned by the user
@knu knu requested a review from dsander March 19, 2023 14:17
Copy link
Collaborator

@dsander dsander left a comment

Choose a reason for hiding this comment

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

Looks a lot cleaner! The changes make sense to me, but permissions can be sneaky. I haven't used it in a long time, but https://github.com/huginn/huginn_docker_specs used to be pretty good in catching regressions, can you check if everything still works with it?

COPY --chown="$UID:0" vendor/gems/ /app/vendor/gems/

ARG ADDITIONAL_GEMS=
ENV ADDITIONAL_GEMS=$ADDITIONAL_GEMS
Copy link
Collaborator

Choose a reason for hiding this comment

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

Nice idea, this allows one to bake agent gems into their docker image to avoid the additional bundle install time at startup?

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes, actually I didn't notice that the startup process ran bundle install, and building my own Docker image has been how I build and use Huginn, and thought this should be everybody's option. 😊

@dsander
Copy link
Collaborator

dsander commented Mar 25, 2023

I have updated the docker specs repository and ran them against this PR, everything is still green 🎉

Also noticed that this brings down the image sizes again, great job!

@knu
Copy link
Member Author

knu commented Apr 15, 2023

@dsander Thanks for that! I tried to update the specs but had a hard time fixing them. 😆

@knu knu merged commit b0ff38d into master Apr 15, 2023
12 checks passed
@knu knu deleted the docker_file_permissions branch April 15, 2023 11:01
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.

None yet

2 participants