mikaeld commented on code in PR #24588:
URL: https://github.com/apache/airflow/pull/24588#discussion_r1171883530


##########
chart/templates/_helpers.yaml:
##########
@@ -171,7 +171,7 @@ If release name contains chart name it will be used as a 
full name.
 - name: {{ .Values.dags.gitSync.containerName }}{{ if .is_init }}-init{{ end }}
   image: {{ template "git_sync_image" . }}
   imagePullPolicy: {{ .Values.images.gitSync.pullPolicy }}
-  securityContext: {{- include "localSecurityContext" .Values.dags.gitSync | 
nindent 4 }}
+  securityContext: {{ include "localPodSecurityContext" .Values.dags.gitSync | 
nindent 4 }}

Review Comment:
   ```suggestion
     securityContext: {{ include "containerSecurityContext" (list . 
.Values.dags.gitSync) | nindent 4 }}
   ```
   
   Other changes required :point_down: 
   
   ### values.yaml
   ```yaml
   dags:
     # [...]
     gitSync:
       # [...]
       securityContexts:
         container: {}
   ```
   
   and the equivalent schema in 
   ### values.schemas.json
   ```json
           "dags": {
               "description": "DAGs settings.",
               "type": "object",
               "x-docsSection": "Airflow",
               "additionalProperties": false,
               "properties": {
                   [...]
                   "gitSync": {
                       "description": "Git sync settings.",
                       "type": "object",
                       "additionalProperties": false,
                       "properties": {
                           [...]
                           "securityContexts": {
                               "description": "Security context definition for 
the git sync sidecar. If not set, the values from global `securityContexts` 
will be used.",
                               "type": "object",
                               "x-docsSection": "Kubernetes",
                               "properties": {
                                   "container": {
                                       "description": "Container security 
context definition for the git sync sidecar.",
                                       "type": "object",
                                       "$ref": 
"#/definitions/io.k8s.api.core.v1.SecurityContext",
                                       "default": {},
                                       "x-docsSection": "Kubernetes",
                                       "examples": [
                                           {
                                               "allowPrivilegeEscalation": 
false,
                                               "capabilities": {
                                                   "drop": [
                                                       "ALL"
                                                   ]
                                               }
                                           }
                                       ]
                                   }
                               }
                           },
   ```



##########
chart/templates/_helpers.yaml:
##########
@@ -729,85 +729,125 @@ server_tls_key_file = /etc/pgbouncer/server.key
 {{- end }}
 
 {{/*
-Set the default value for securityContext
-If no value is passed for securityContext or <node>.securityContext, defaults 
to global uid and gid.
+Set the default value for pod securityContext
+If no value is passed for securityContexts.pod or <node>.securityContexts.pod 
or legacy securityContext and <node>.securityContext, defaults to global uid 
and gid.
 
-    +------------------------+      +-----------------+      
+-------------------------+
-    | <node>.securityContext |  ->  | securityContext |  ->  | Values.uid + 
Values.gid |
-    +------------------------+      +-----------------+      
+-------------------------+
+    +-----------------------------+      +------------------------+      
+----------------------+      +-----------------+      
+-------------------------+
+    | <node>.securityContexts.pod |  ->  | <node>.securityContext |  ->  | 
securityContexts.pod |  ->  | securityContext |  ->  | Values.uid + Values.gid |
+    +-----------------------------+      +------------------------+      
+----------------------+      +-----------------+      
+-------------------------+
 
-Values are not accumulated meaning that if runAsUser is set to 10 in 
<node>.securityContext,
+Values are not accumulated meaning that if runAsUser is set to 10 in 
<node>.securityContexts.pod,
 any extra values set to securityContext or uid+gid will be ignored.
 
 The template can be called like so:
-   include "airflowSecurityContext" (list . .Values.webserver)
+   include "airflowPodSecurityContext" (list . .Values.webserver)
 
 Where `.` is the global variables scope and `.Values.webserver` the local 
variables scope for the webserver template.
 */}}
-{{- define "airflowSecurityContext" -}}
-  {{- $ := index . 0 }}
+{{- define "airflowPodSecurityContext" -}}
+  {{- $ := index . 0 -}}
   {{- with index . 1 }}
-    {{- if .securityContext }}
-      {{- toYaml .securityContext }}
-    {{- else if $.Values.securityContext }}
-      {{- toYaml $.Values.securityContext }}
-    {{- else }}
+    {{- if .securityContexts.pod -}}
+{{ toYaml .securityContexts.pod | print }}
+    {{- else if .securityContext -}}
+{{ toYaml .securityContext | print }}
+    {{- else if $.Values.securityContexts.pod -}}
+{{ toYaml $.Values.securityContexts.pod | print }}
+    {{- else if $.Values.securityContext -}}
+{{ toYaml $.Values.securityContext | print }}
+    {{- else -}}
 runAsUser: {{ $.Values.uid }}
 fsGroup: {{ $.Values.gid }}
     {{- end }}
   {{- end }}
 {{- end }}
 
 {{/*
-Set the default value for securityContext
-If no value is passed for securityContext or <node>.securityContext, defaults 
to UID in the local node.
+Set the default value for pod securityContext
+If no value is passed for <node>.securityContexts.pod or 
<node>.securityContext, defaults to UID in the local node.
 
-    +------------------------+     +-------------+
-    | <node>.securityContext |  >  | <node>.uid  |
-    +------------------------+     +-------------+
+    +-----------------------------+      +------------------------+      
+-------------+
+    | <node>.securityContexts.pod |  ->  | <node>.securityContext |  ->  | 
<node>.uid  |
+    +-----------------------------+      +------------------------+      
+-------------+
 
 The template can be called like so:
-  include "localSecurityContext" .Values.statsd
+  include "localPodSecurityContext" (list . .Values.schedule)
 
 It is important to pass the local variables scope to this template as it is 
used to determine the local node value for uid.
 */}}
-{{- define "localSecurityContext" -}}
-  {{- if .securityContext }}
-    {{- toYaml .securityContext }}
-  {{- else }}
-    {{- printf "runAsUser: %v" .uid }}
-  {{- end }}
-{{- end }}
+{{- define "localPodSecurityContext" -}}
+  {{- if .securityContexts.pod -}}
+{{ toYaml .securityContexts.pod | print }}
+  {{- else if .securityContext -}}
+{{ toYaml .securityContext | print }}
+  {{- else -}}
+runAsUser: {{ .uid }}
+  {{- end -}}
+{{- end -}}
 
 {{/*
 Set the default value for workers chown for persistent storage
-If no value is passed for securityContext or <node>.securityContext, defaults 
to global uid and gid.
+If no value is passed for securityContexts.pod or <node>.securityContexts.pod 
or legacy securityContext and <node>.securityContext, defaults to global uid 
and gid.
 The template looks for `runAsUser` and `fsGroup` specifically, any other 
parameter will be ignored.
 
-    +------------------------+      +-----------------+      
+-------------------------+
-    | <node>.securityContext |  ->  | securityContext |  ->  | Values.uid + 
Values.gid |
-    +------------------------+      +-----------------+      
+-------------------------+
+    +-----------------------------+      
+----------------------------------------------------+      
+------------------+      +-------------------------+
+    | <node>.securityContexts.pod |  ->  | securityContexts.pod | 
<node>.securityContexts.pod |  ->  | securityContexts |  ->  | Values.uid + 
Values.gid |
+    +-----------------------------+      
+----------------------------------------------------+      
+------------------+      +-------------------------+
 
-Values are not accumulated meaning that if runAsUser is set to 10 in 
<node>.securityContext,
-any extra values set to securityContext or uid+gid will be ignored.
+Values are not accumulated meaning that if runAsUser is set to 10 in 
<node>.securityContexts.pod,
+any extra values set to securityContexts or uid+gid will be ignored.
 
 The template can be called like so:
-   include "airflowSecurityContextIds" (list . .Values.workers)
+   include "airflowPodSecurityContextsIds" (list . .Values.webserver)
 
 Where `.` is the global variables scope and `.Values.workers` the local 
variables scope for the workers template.
 */}}
-{{- define "airflowSecurityContextIds" -}}
-  {{- $ := index . 0 }}
+{{- define "airflowPodSecurityContextsIds" -}}
+  {{- $ := index . 0 -}}
   {{- with index . 1 }}
-    {{- if .securityContext }}
-      {{- pluck "runAsUser" .securityContext | first | default $.Values.uid 
}}:{{ pluck "fsGroup" .securityContext | first | default $.Values.gid }}
-    {{- else if $.Values.securityContext }}
-      {{- pluck "runAsUser" $.Values.securityContext | first | default 
$.Values.uid }}:{{ pluck "fsGroup" $.Values.securityContext | first | default 
$.Values.gid }}
-    {{- else }}
-      {{- printf "%s:%s" $.Values.uid $.Values.gid }}
-    {{- end }}
-  {{- end }}
-{{- end }}
+    {{- if .securityContexts.pod -}}
+{{ pluck "runAsUser" .securityContexts.pod | first | default $.Values.uid 
}}:{{ pluck "fsGroup" .securityContexts.pod | first | default $.Values.gid }}
+    {{- else if $.Values.securityContext -}}
+{{ pluck "runAsUser" $.Values.securityContext | first | default $.Values.uid 
}}:{{ pluck "fsGroup" $.Values.securityContext | first | default $.Values.gid }}
+    {{- else if $.Values.securityContexts.pod -}}
+{{ pluck "runAsUser" $.Values.securityContexts.pod | first | default 
$.Values.uid }}:{{ pluck "fsGroup" $.Values.securityContexts.pod | first | 
default $.Values.gid }}
+    {{- else if $.Values.securityContext -}}
+{{ pluck "runAsUser" $.Values.securityContext | first | default $.Values.uid 
}}:{{ pluck "fsGroup" $.Values.securityContext | first | default $.Values.gid }}
+    {{- else -}}
+{{ $.Values.uid }}:{{ $.Values.gid }}
+    {{- end -}}
+  {{- end -}}
+{{- end -}}
+
+{{/*
+Set the default value for container securityContext
+If no value is passed for securityContexts.container or 
<node>.securityContexts.container, defaults to deny privileges escallation and 
dropping all POSIX capabilities.
+
+    +-----------------------------------+      +----------------------------+  
    +-----------------------------------------------------------+
+    | <node>.securityContexts.container |  ->  | securityContexts.container |  
->  | allowPrivilegesEscalation: false, capabilities.drop: [ALL]|

Review Comment:
   ```suggestion
       | <node>.securityContexts.container |  ->  | securityContexts.containers 
|  ->  | allowPrivilegesEscalation: false, capabilities.drop: [ALL]|
   ```
   Global container security contexts for containers is 
`securityContexts.containers` and not `securityContexts.container`



##########
chart/templates/_helpers.yaml:
##########
@@ -729,85 +729,125 @@ server_tls_key_file = /etc/pgbouncer/server.key
 {{- end }}
 
 {{/*
-Set the default value for securityContext
-If no value is passed for securityContext or <node>.securityContext, defaults 
to global uid and gid.
+Set the default value for pod securityContext
+If no value is passed for securityContexts.pod or <node>.securityContexts.pod 
or legacy securityContext and <node>.securityContext, defaults to global uid 
and gid.
 
-    +------------------------+      +-----------------+      
+-------------------------+
-    | <node>.securityContext |  ->  | securityContext |  ->  | Values.uid + 
Values.gid |
-    +------------------------+      +-----------------+      
+-------------------------+
+    +-----------------------------+      +------------------------+      
+----------------------+      +-----------------+      
+-------------------------+
+    | <node>.securityContexts.pod |  ->  | <node>.securityContext |  ->  | 
securityContexts.pod |  ->  | securityContext |  ->  | Values.uid + Values.gid |
+    +-----------------------------+      +------------------------+      
+----------------------+      +-----------------+      
+-------------------------+
 
-Values are not accumulated meaning that if runAsUser is set to 10 in 
<node>.securityContext,
+Values are not accumulated meaning that if runAsUser is set to 10 in 
<node>.securityContexts.pod,
 any extra values set to securityContext or uid+gid will be ignored.
 
 The template can be called like so:
-   include "airflowSecurityContext" (list . .Values.webserver)
+   include "airflowPodSecurityContext" (list . .Values.webserver)
 
 Where `.` is the global variables scope and `.Values.webserver` the local 
variables scope for the webserver template.
 */}}
-{{- define "airflowSecurityContext" -}}
-  {{- $ := index . 0 }}
+{{- define "airflowPodSecurityContext" -}}
+  {{- $ := index . 0 -}}
   {{- with index . 1 }}
-    {{- if .securityContext }}
-      {{- toYaml .securityContext }}
-    {{- else if $.Values.securityContext }}
-      {{- toYaml $.Values.securityContext }}
-    {{- else }}
+    {{- if .securityContexts.pod -}}
+{{ toYaml .securityContexts.pod | print }}
+    {{- else if .securityContext -}}
+{{ toYaml .securityContext | print }}
+    {{- else if $.Values.securityContexts.pod -}}
+{{ toYaml $.Values.securityContexts.pod | print }}
+    {{- else if $.Values.securityContext -}}
+{{ toYaml $.Values.securityContext | print }}
+    {{- else -}}
 runAsUser: {{ $.Values.uid }}
 fsGroup: {{ $.Values.gid }}
     {{- end }}
   {{- end }}
 {{- end }}
 
 {{/*
-Set the default value for securityContext
-If no value is passed for securityContext or <node>.securityContext, defaults 
to UID in the local node.
+Set the default value for pod securityContext
+If no value is passed for <node>.securityContexts.pod or 
<node>.securityContext, defaults to UID in the local node.
 
-    +------------------------+     +-------------+
-    | <node>.securityContext |  >  | <node>.uid  |
-    +------------------------+     +-------------+
+    +-----------------------------+      +------------------------+      
+-------------+
+    | <node>.securityContexts.pod |  ->  | <node>.securityContext |  ->  | 
<node>.uid  |
+    +-----------------------------+      +------------------------+      
+-------------+
 
 The template can be called like so:
-  include "localSecurityContext" .Values.statsd
+  include "localPodSecurityContext" (list . .Values.schedule)
 
 It is important to pass the local variables scope to this template as it is 
used to determine the local node value for uid.
 */}}
-{{- define "localSecurityContext" -}}
-  {{- if .securityContext }}
-    {{- toYaml .securityContext }}
-  {{- else }}
-    {{- printf "runAsUser: %v" .uid }}
-  {{- end }}
-{{- end }}
+{{- define "localPodSecurityContext" -}}
+  {{- if .securityContexts.pod -}}
+{{ toYaml .securityContexts.pod | print }}
+  {{- else if .securityContext -}}
+{{ toYaml .securityContext | print }}
+  {{- else -}}
+runAsUser: {{ .uid }}
+  {{- end -}}
+{{- end -}}
 
 {{/*
 Set the default value for workers chown for persistent storage
-If no value is passed for securityContext or <node>.securityContext, defaults 
to global uid and gid.
+If no value is passed for securityContexts.pod or <node>.securityContexts.pod 
or legacy securityContext and <node>.securityContext, defaults to global uid 
and gid.
 The template looks for `runAsUser` and `fsGroup` specifically, any other 
parameter will be ignored.
 
-    +------------------------+      +-----------------+      
+-------------------------+
-    | <node>.securityContext |  ->  | securityContext |  ->  | Values.uid + 
Values.gid |
-    +------------------------+      +-----------------+      
+-------------------------+
+    +-----------------------------+      
+----------------------------------------------------+      
+------------------+      +-------------------------+
+    | <node>.securityContexts.pod |  ->  | securityContexts.pod | 
<node>.securityContexts.pod |  ->  | securityContexts |  ->  | Values.uid + 
Values.gid |
+    +-----------------------------+      
+----------------------------------------------------+      
+------------------+      +-------------------------+
 
-Values are not accumulated meaning that if runAsUser is set to 10 in 
<node>.securityContext,
-any extra values set to securityContext or uid+gid will be ignored.
+Values are not accumulated meaning that if runAsUser is set to 10 in 
<node>.securityContexts.pod,
+any extra values set to securityContexts or uid+gid will be ignored.
 
 The template can be called like so:
-   include "airflowSecurityContextIds" (list . .Values.workers)
+   include "airflowPodSecurityContextsIds" (list . .Values.webserver)
 
 Where `.` is the global variables scope and `.Values.workers` the local 
variables scope for the workers template.
 */}}
-{{- define "airflowSecurityContextIds" -}}
-  {{- $ := index . 0 }}
+{{- define "airflowPodSecurityContextsIds" -}}
+  {{- $ := index . 0 -}}
   {{- with index . 1 }}
-    {{- if .securityContext }}
-      {{- pluck "runAsUser" .securityContext | first | default $.Values.uid 
}}:{{ pluck "fsGroup" .securityContext | first | default $.Values.gid }}
-    {{- else if $.Values.securityContext }}
-      {{- pluck "runAsUser" $.Values.securityContext | first | default 
$.Values.uid }}:{{ pluck "fsGroup" $.Values.securityContext | first | default 
$.Values.gid }}
-    {{- else }}
-      {{- printf "%s:%s" $.Values.uid $.Values.gid }}
-    {{- end }}
-  {{- end }}
-{{- end }}
+    {{- if .securityContexts.pod -}}
+{{ pluck "runAsUser" .securityContexts.pod | first | default $.Values.uid 
}}:{{ pluck "fsGroup" .securityContexts.pod | first | default $.Values.gid }}
+    {{- else if $.Values.securityContext -}}
+{{ pluck "runAsUser" $.Values.securityContext | first | default $.Values.uid 
}}:{{ pluck "fsGroup" $.Values.securityContext | first | default $.Values.gid }}
+    {{- else if $.Values.securityContexts.pod -}}
+{{ pluck "runAsUser" $.Values.securityContexts.pod | first | default 
$.Values.uid }}:{{ pluck "fsGroup" $.Values.securityContexts.pod | first | 
default $.Values.gid }}
+    {{- else if $.Values.securityContext -}}
+{{ pluck "runAsUser" $.Values.securityContext | first | default $.Values.uid 
}}:{{ pluck "fsGroup" $.Values.securityContext | first | default $.Values.gid }}
+    {{- else -}}
+{{ $.Values.uid }}:{{ $.Values.gid }}
+    {{- end -}}
+  {{- end -}}
+{{- end -}}
+
+{{/*
+Set the default value for container securityContext
+If no value is passed for securityContexts.container or 
<node>.securityContexts.container, defaults to deny privileges escallation and 
dropping all POSIX capabilities.
+
+    +-----------------------------------+      +----------------------------+  
    +-----------------------------------------------------------+
+    | <node>.securityContexts.container |  ->  | securityContexts.container |  
->  | allowPrivilegesEscalation: false, capabilities.drop: [ALL]|
+    +-----------------------------------+      +----------------------------+  
    +-----------------------------------------------------------+
+
+
+The template can be called like so:
+   include "containerSecurityContext" (list . .Values.statsd)
+
+Where `.` is the global variables scope and `.Values.webserver` the local 
variables scope for the webserver template.
+*/}}
+{{- define "containerSecurityContext" -}}
+  {{- $ := index . 0 -}}
+  {{- with index . 1 }}
+    {{- if .securityContexts.container -}}
+{{ toYaml .securityContexts.container | print }}
+    {{- else if $.Values.securityContexts.container -}}
+{{ toYaml $.Values.securityContexts.container | print }}

Review Comment:
   ```suggestion
       {{- else if $.Values.securityContexts.containers -}}
   {{ toYaml $.Values.securityContexts.containers | print }}
   ```
   Same thing here, `securityContexts.containers` and not 
`securityContexts.container`



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscr...@airflow.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org

Reply via email to