airflow icon indicating copy to clipboard operation
airflow copied to clipboard

Helm chart can't remove .securityContexts.runAsUser and securityContexts.fsGroup for Openshift compatibility

Open philthynz opened this issue 1 year ago • 14 comments

Official Helm Chart version

1.15.0 (latest released)

Apache Airflow version

2.9.3

Kubernetes Version

v1.26.15+4818370

Helm Chart configuration

securityContexts: {
  pod: {
    runAsUser: 
    fsGroup:  
  }
}

Docker Image customizations

No response

What happened

.securityContexts.pod.runAsUser and securityContexts.pod.fsGroup should allow us to remove the values entirely for openshift compatibility. Bitnami have done the same in their image https://github.com/bitnami/charts/blob/f7bd58e4b2842e0bf1bf2dcd0288beea98dd87a9/bitnami/airflow/values.yaml#L29

This is needed when users like myself do not have cluster admin rights to assign scc's to service accounts that can use any UID and GID. By default Openshift applies the restricted-v2 SCC, which doesn't allow any id for runAsUser and fsGroup.

What you think should happen instead

Allow .runAsUser and fsGroup values to be omitted or allow .securityContexts block to be removed so Openshifts SCC policy will apply the defaults.

How to reproduce

In an openshift cluster, try to deploy as a project admin.

Anything else

No response

Are you willing to submit PR?

  • [ ] Yes I am willing to submit a PR!

Code of Conduct

philthynz avatar Aug 21 '24 05:08 philthynz

Thanks for opening your first issue here! Be sure to follow the issue template! If you are willing to raise PR to address this issue please do so, no need to wait for approval.

boring-cyborg[bot] avatar Aug 21 '24 06:08 boring-cyborg[bot]

Feel free to contribute it - otherwise it will have to wait for someone to pick it up (marked it as good-first-issue since there are some clear examples it can be based on)

potiuk avatar Aug 21 '24 13:08 potiuk

cc @nevcohen maybe you be intreated in fixing this one?

eladkal avatar Aug 22 '24 06:08 eladkal

Actually, a good fix would be to just remove the hard coded runAsUser and fsGroup. Openshift by default will apply the restricted-v2 SCC policy, which doesn't allow this template to be deployed. Openshift will add those values.

Security context block would also be good if it can be removed entirely for other clusters that apply SCC rules.

Maybe a bool and condition for runAsUser and fsGroup that are omitted when openshift=true?

philthynz avatar Aug 22 '24 06:08 philthynz

Hi @philthynz ,

I hope you're doing well.

I am working on adapting the Helm chart to support OpenShift compatibility, particularly in relation to runAsUser and fsGroup settings. Based on your suggestion, I plan to make the following changes:

Add an openshift flag: This flag will determine whether to adapt the security context for OpenShift.

Conditional Inclusion of securityContext: In the Helm chart templates, I will conditionally include or exclude the securityContext block based on the openshift flag and adaptSecurityContext setting.

If openshift=true: The securityContext block will be omitted, allowing OpenShift to apply its default security settings. For other Kubernetes environments: The securityContext block will include runAsUser and fsGroup settings if they are specified. I’d like to confirm if this approach aligns with your expectations and the best practices for handling OpenShift compatibility. Additionally, if there are any other considerations or adjustments you recommend, please let me know.

Thanks

deveshgoyal1000 avatar Aug 22 '24 13:08 deveshgoyal1000

Hi @deveshgoyal1000 ,

Thanks for jumping in. I am well thanks.

Yes your plan will work. I thought it was more simple to allow runAsUser and fsGroup values to be null. Openshift would accept that too. As an example:

securityContext: {
  runAsUser: 
  fsGroup:  
}

Blank values are accepted in restricted-v2.

But you're right. The underlying issue here is that the securityContexts block can't be fully omitted to allow Openshift to apply the default SCC config.

As a workaround we are using helm template and then a find and replace to make runAsUser and fsGroup blank.

Do you mind if I ask your time frame?

philthynz avatar Aug 22 '24 18:08 philthynz

I can get this done 1-2 if but which approch should i do making runAsUser and fsGroup values to be null or Conditionally including or excluding the securityContext block based on this flag and the adaptSecurityContext setting. and updating deployment templates with

{{- if and (eq .Values.global.compatibility.openshift.adaptSecurityContext "auto") (eq .Values.global.compatibility.openshift.openshift true) }}
securityContext: {}
{{- else }}
securityContext:
  runAsUser: {{ .Values.containerSecurityContext.runAsUser | default "" }}
  fsGroup: {{ .Values.podSecurityContext.fsGroup | default "" }}
{{- end }}

deveshgoyal1000 avatar Aug 23 '24 14:08 deveshgoyal1000

Hey @deveshgoyal1000 many thanks for this.

I would take the bitnami approach. Because users still may want to specify other attributes in the security context that Openshift would allow.

See here: https://blog.bitnami.com/2024/04/bitnami-helm-charts-are-now-more-secure.html?m=1

Screenshot_20240824_090230_Chrome

The runAsUser, runAsGroup, and fsGroup ahould be automatically removed. I.e, not set.

philthynz avatar Aug 23 '24 21:08 philthynz

Hey @philthynz Thank you for pointing me to the Bitnami approach it looks great and aligns perfectly with our goals. I'll proceed with this method to ensure the Helm charts automatically remove runAsUser, runAsGroup, and fsGroup when deployed on OpenShift. To ensure I’m on the right track, here’s the updated conditional logic that I’ll be applying :

{{- if and (eq .Values.global.compatibility.openshift.adaptSecurityContext "auto") (eq .Values.global.compatibility.openshift.enabled true) }}
containerSecurityContext: {}
podSecurityContext: {}
{{- else }}
containerSecurityContext:
  runAsUser: {{ .Values.containerSecurityContext.runAsUser | default "" }}
  runAsGroup: {{ .Values.containerSecurityContext.runAsGroup | default "" }}
podSecurityContext:
  fsGroup: {{ .Values.podSecurityContext.fsGroup | default "" }}
{{- end }}

This approach separates the containerSecurityContext and podSecurityContext, ensuring that OpenShift-specific settings are handled correctly, while also allowing other Kubernetes environments to use the specified values.

Please let me know if this aligns with your expectations or if there are any further adjustments needed. Thanks!

deveshgoyal1000 avatar Aug 24 '24 20:08 deveshgoyal1000

Yea blank values will work. I've tested that. Many thanks.

philthynz avatar Aug 24 '24 20:08 philthynz

Oh, that's nice! Thanks for confirming.

deveshgoyal1000 avatar Aug 24 '24 22:08 deveshgoyal1000

Hello, I saw this conversation, and I saw that now you can go and configure the uids and gid to "" in most places I have looked at. For example at general, statsd and redis. So maybe the only thing we need is to add to the docs?

Avihais12344 avatar Sep 02 '24 17:09 Avihais12344

Hi @deveshgoyal1000 , Do we have a PR in progress for this? We use openshift and are looking forward to these changes being added in 🙂

sre42 avatar Dec 09 '24 08:12 sre42

Hi yes,

Hi @deveshgoyal1000 , Do we have a PR in progress for this? We use openshift and are looking forward to these changes being added in 🙂

Hi, Thanks for the reminder! It seems this slipped past me. I'll start working on it and have a PR ready shortly.

deveshgoyal1000 avatar Dec 18 '24 07:12 deveshgoyal1000

Hi @deveshgoyal1000 any progress on this?

mateocolina avatar Feb 21 '25 21:02 mateocolina

I do have a PR in place to add SC to all pods with minimal permissions. https://github.com/apache/airflow/pull/47356

I can add logic to that as @deveshgoyal1000 above mentions from the same way bitnami does.

JKrehling avatar Mar 04 '25 19:03 JKrehling