developmentseed / osm-seed

A collection of Dockerfiles to run a containerized version of OpenStreetMap
https://devseed.com/osm-seed-chart/
MIT License
154 stars 36 forks source link

Add security context with fallback #282

Closed srudin closed 1 year ago

srudin commented 1 year ago

As discussed here: https://github.com/developmentseed/osm-seed/issues/281

This PR is adding the security context for all containers. It is possible to define the runAsUser / runAsGroup / fsGroup for every container individually. If nothing is set on a container a global value is used. If no global value is configured the root user is used (as is now).

I think this should be rather undisputed. ;-) Tested except I cannot set the root user in my environment so not sure if explicitely setting default 0 works (but would surprise me if not).

batpad commented 1 year ago

This looks fantastic to me. Thank you @srudin .

Will give this a day to see if @Rub21 has any concerns. Also tagging in @yuvipanda since he knows more about these things than me in case there's any concerns with this approach.

@srudin let's give this a day for @Rub21 or @yuvipanda to leave any comments, else am good to approve and merge this 👍 thanks again!

yuvipanda commented 1 year ago

Instead of allowing overrides on a 'per-leaf' level, I would suggest allowing overrides of the entire securityContext object, like https://github.com/jupyterhub/zero-to-jupyterhub-k8s/blob/23bf6a109fe9dd8fdabdbbaaa4c7d8abaf751951/jupyterhub/templates/scheduling/user-scheduler/deployment.yaml#L96 (or via | toJson on a single line to avoid having to count indents).

This allows admins to override any part of securityContext rather than just the 3 specified here, without requiring changes in the upstream code every time someone wants to customize something.

srudin commented 1 year ago

I see the benefit of that and I did it already at some other places. However I also think that the fallback per container -> full project is valuable as well and I didn't find a way to use the "with" syntax and the fallback "{{ value | default fallback }}" together. Do you know of a way to do that?

srudin commented 1 year ago

@yuvipanda Any feedback or examples regarding my question?

batpad commented 1 year ago

I didn't find a way to use the "with" syntax and the fallback "{{ value | default fallback }}" together.

@srudin do you have a bit more specific example of exactly what you were trying to do, what you tried, and what did not work?

batpad commented 1 year ago

It would be nice to do as @yuvipanda suggested, if that seems possible - allowing more flexible over-rides seems to make sense.

srudin commented 1 year ago

@batpad I didn't question that. But I think a fallback also makes sense and I only know how to do that with single values. So an example of the more flexible approach with a fallback would be appreciated.

galewis2 commented 1 year ago

Do we know which uids/gids work for Nominatim for example? With the current build, it looks like /app/ is owned by root, so for example, /app/address-levels.json cannot be downloaded

yuvipanda commented 1 year ago

@srudin if you are talking about how to set defaults, we put them in values.yaml like https://github.com/jupyterhub/zero-to-jupyterhub-k8s/blob/46a632f5d07ba8520e5fb5e7539b3f7b89c5fcc7/jupyterhub/values.yaml#L88. But otherwise it's not entirely clear to me what the question is.