common - podOptions hostUsers not being set

truecharts / library-charts

Helm Library Charts for TrueCharts

Other

29 stars 36 forks source link

common - podOptions hostUsers not being set #823

Closed Kampe closed 3 months ago

Kampe commented 3 months ago

chart Name

common

Operating System

Talos-OS 1.7 Kubernetes 1.30

Deployment Method

Helm

Chart Version

latest

Kubernetes Events

n/a

chartlication Logs

N/A

Chart Configuration

podOptions: hostUsers: false

Describe the bug

podOptions: hostUsers: false

has no effect for any charts that utilize common.

To Reproduce

install any true chart and set podOptions.hostUsers

Expected Behavior

expected that the hostUsers field is set downstream on pods.

Screenshots

n/a

Additional Context

n/a

I've read and agree with the following

[X] I've checked all open and closed issues and my issue is not there.
[X] I've understand TrueNAS SCALE Apps are deprecated and TrueCharts does not accept TrueNAS SCALE related issues
[X] I've prefixed my issue title with [Chart-Name]

PrivatePuffin commented 3 months ago

Just to be clear: hostUsers, is automatically determined already based on weither it can be used. (basically: stateless, non-root applications). So in 99% of cases it isn't needed to be explicitly disabled.

So i'm not 100% sure why we even have this in the docs, maybe @stavros-k can shed a light on that?

stavros-k commented 3 months ago

Currently the way we have it like this:

Defaults to false
If it matches one of the conditions we set, we set it to true
If podOptions.hostUsers is true, we set it to true regardless of the calculations above
If $wokrload.podSpec.hostUsers is true, we set it to true regardless of the calculations above

Currently there is no way to set it to false if one of the conditions is matched.

TL;DR; currently users can manually only opt-in to hostUsers if the conditions are not met. There is no opt-out.

Here is the code for ref: https://github.com/truecharts/library-charts/blob/a1b7be52fce831b53104c3fb470172e10bf1c3ad/library/common/templates/lib/pod/_hostUsers.tpl#L13-L29

Here is the "conditions" for ref: https://github.com/truecharts/library-charts/blob/a1b7be52fce831b53104c3fb470172e10bf1c3ad/library/common/templates/lib/pod/_podSecurityContext.tpl#L4

PrivatePuffin commented 3 months ago

Yeah so this is expected behavior...

Kampe commented 3 months ago

This will become increasingly more of a problem if its not set as of Kubernetes 1.30 as it becomes a required field and is set automatically. This causes issues in any gitops tools as there's constantly state drift between the chart and what the API server is returning.

stavros-k commented 3 months ago

Functionality will change to this:

If the key is missing, or is not a bool, we will set it based on our predefined conditions. If the key is set and its a bool. We will use that value.