search

georchestra / mapstore2-georchestra

geOrchestra newest viewer
Other
6 stars 23 forks source link

Run tomcat as non-root user #612

Closed jeanpommier closed 3 weeks ago

jeanpommier commented 1 year ago

update based on PR https://github.com/georchestra/mapstore2-georchestra/pull/442

jeanpommier commented 1 year ago

@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)

jeanpommier commented 1 year ago

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

pmauduit commented 1 year ago

Can you explain why for ?

it looks like because of https://github.com/georchestra/georchestra/issues/4071

edevosc2c commented 1 year ago

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's a standard thing in docker images to allow anyone to set UID and GID at build time. It doesn't hurt to add it.

On top of that, you have just one line/one place to change the UID and GID in case it's needed.

edevosc2c commented 12 months ago

Note: Once https://github.com/georchestra/mapstore2-georchestra/pull/671 will be merged, there will be a need here to add after RUN mkdir -p /docker-entrypoint.d this line:

RUN chown tomcat:tomcat /docker-entrypoint.d

Otherwise, the copy in this script won't work: https://github.com/georchestra/mapstore2-georchestra/pull/671/files#diff-1b0bf6d59703af25ba177c67baa3686a4aa1846098b91e7ff32375e0f4eb43eaR10

jeanpommier commented 2 months ago

Hi, Sorry I had left this PR untidy. I've updated the PR according to your suggestions @edevosc2c. Does it look OK to you ?

edevosc2c commented 1 month ago

We need to fix the CI before merge if possible.

edevosc2c commented 1 month ago

hey @jeanpommier can you rebase against master? this should fix the CI because geosolutions fixed it: https://github.com/georchestra/mapstore2-georchestra/commit/fe581a2ccc070ca433b2701cf914ac3fa70a4a36#diff-7c5b1aec3fd401c4634f1d2e303fa0b61fef81dd181322616a7058e833328a50R74

jeanpommier commented 3 weeks ago

hey @jeanpommier can you rebase against master? this should fix the CI because geosolutions fixed it: fe581a2#diff-7c5b1aec3fd401c4634f1d2e303fa0b61fef81dd181322616a7058e833328a50R74

Hi @edevosc2c Done, thanks

edevosc2c commented 2 weeks ago

will probably be great to create a new mapstore release

landryb commented 1 week ago

there will be an rc for 2024.02 soon, see #714. unless you want to cut a release from the previous 'stable' branch.

landryb commented 1 week ago

actually there has been one in https://github.com/georchestra/mapstore2-georchestra/releases/tag/2024.02.00-RC1-geOrchestra

edevosc2c commented 1 week ago

Yes we want to backport that to previous versions. Some of our clients want this change.