-
-
Notifications
You must be signed in to change notification settings - Fork 541
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
Custom Docker image - Pre-install pre-commit hooks inside image #622
Comments
Do I get it right that the error pops up when you build container image? If yes, please share your Docker build command with all the parameters used. |
To be able to reproduce your build, we need your ## init pre-commit
COPY .pre-commit-config.yaml .
RUN \
git init . && \
pre-commit install --install-hooks |
Sorry for being unclear about that. It builds fine but crashes on docker run. .pre-commit-config.yamlrepos:
- repo: https://github.com/pre-commit/pre-commit-hooks
rev: v4.5.0
hooks:
- id: end-of-file-fixer
- id: trailing-whitespace
- repo: https://github.com/antonbabenko/pre-commit-terraform
rev: v1.86.0
hooks:
- id: terraform_validate
- id: terraform_fmt
- id: terraform_docs
- id: terraform_tflint Running the following cmd generates the error reported previously: NB: ❯ id -u
1000
❯ id -g
1000 |
I'm not sure the solution is best as it's a bit of security hole and is anti-pattern, though given you create and populate |
ah ... I just feel kind of stupid .. I just added to the final RUN cmd I'm curious to know what you would have done differently to propose another solution. I think I understand what you mean by security hole but not sure about the anti pattern. |
Yep, given that your container is targeted to a wide audience of users with different usernames, the "give read/write to all for pre-commit cache dir" looks reasonable. Moreover perms are scope to the very specific dir, which decreases the probability that users may try and threaten the container for any reason. So, probably, if this works for all your users, you should be ok to go with this solution.
I mean that giving read-write to any arbitrary user is a security anti-pattern. Which, well, in your use case seems to be the inevitable measure to allow container to have a pre-baked pre-commit stuff. I'm closing this issue. Please let us know if you still think it's worth keeping it open. |
Describe the bug
Hi, not sure if exactly a bug but I feel like potentially misleading explanation in docs or more likely me missing the point completely. Anyway looking for help and not sure exactly where to ask.
I'm trying to build a container image based on the one featured in this repo: ghcr.io/antonbabenko/pre-commit-terraform.
Looking at #docker-usage I see that the recommended approach is to leave root user inside and leverage
"USERID=$(id -u):$(id -g)"
env var for the entrypoint script.So I'm fine with that.
Now all I need is to pre-install the config that I want to share. For that I found this answer from anthony sottile on stackoverflow.
Dockerfile
So I tried that and got this :
Error message
Which seems to be related to pre-commit running as root ? caused by my
pre-commit install-hooks
command as far as I understand.I feel like I have to create a user with specific uid and get rid of the entrypoint script. Missing the ability to handle any uid. Or maybe my installation isn't correct.
Do you have an idea of what would be the best approach in my case ?
Thanks a lot !
The text was updated successfully, but these errors were encountered: