-
Notifications
You must be signed in to change notification settings - Fork 25
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
Run tomcat as non-root user #612
Run tomcat as non-root user #612
Conversation
@pierrejego I believe we can merge this PR. We will need to document in the migration notes that the mapstore writable datadir needs to be writable by UID/GID 999 (for the docker version) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could you allow changing the UID and GID at build time?
You can do it with something like this:
ARG USER_GID=999
ARG USER_UID=999
RUN addgroup --gid ${USER_GID} tomcat && \
adduser --system -u ${USER_UID} --gid ${USER_GID} --no-create-home tomcat && \
chown -R ${USER_UID}:${USER_GID} /usr/local/tomcat
Hi @edevosc2c . Can you explain why for ? The idea is to run the georchestra official docker image, right ? Since this applies at build time, it means that to get different settings one would still have a build a custom image. I don't see the added value |
it looks like because of georchestra/georchestra#4071 |
It's a standard thing in docker images to allow anyone to set UID and GID at build time. On top of that, you have just one line/one place to change the UID and GID in case it's needed. |
Note: Once #671 will be merged, there will be a need here to add after
Otherwise, the copy in this script won't work: https://github.com/georchestra/mapstore2-georchestra/pull/671/files#diff-1b0bf6d59703af25ba177c67baa3686a4aa1846098b91e7ff32375e0f4eb43eaR10 |
bb9cf90
to
c65b297
Compare
Hi, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
good for merging
We need to fix the CI before merge if possible. |
hey @jeanpommier can you rebase against master? this should fix the CI because geosolutions fixed it: fe581a2#diff-7c5b1aec3fd401c4634f1d2e303fa0b61fef81dd181322616a7058e833328a50R74 |
Based on PR georchestra#442 Updated according to suggestions from @edevos on PR georchestra#612
c65b297
to
b17539d
Compare
Based on PR georchestra#442 Updated according to suggestions from @edevosc2c on PR georchestra#612
b17539d
to
4dd778e
Compare
Hi @edevosc2c |
will probably be great to create a new mapstore release |
there will be an rc for 2024.02 soon, see #714. unless you want to cut a release from the previous 'stable' branch. |
actually there has been one in https://github.com/georchestra/mapstore2-georchestra/releases/tag/2024.02.00-RC1-geOrchestra |
Yes we want to backport that to previous versions. Some of our clients want this change. |
update based on PR #442