[GitHub] [airflow] jedcunningham commented on a change in pull request #18249: Add support for securityContext per deployment
jedcunningham commented on a change in pull request #18249:
URL: https://github.com/apache/airflow/pull/18249#discussion_r770883736
##
File path: chart/templates/_helpers.yaml
##
@@ -616,3 +615,85 @@ Create the name of the cleanup service account to use
{{- end -}}
{{- $kubeVersion -}}
{{- end -}}
+
+{{/*
+Set the default value for securityContext
+If no value is passed for securityContext or .securityContext, defaults
to global uid and gid.
+
+++ +-+
+-+
+| .securityContext | -> | securityContext | -> | Values.uid +
Values.gid |
+++ +-+
+-+
+
+Values are not accumulated meaning that if runAsUser is set to 10 in
.securityContext,
+any extra values set to securityContext or uid+gid will be ignored.
+
+The template can be called like so:
+ include "airflowSecurityContext" (list . .Values.webserver)
+
+Where '.' is the global varriables scope and `.Values.webserver` the local
variables scope for the webserver template.
+*/}}
+{{- define "airflowSecurityContext" -}}
+ {{- $ := index . 0 -}}
+ {{- with index . 1 }}
+{{- if .securityContext -}}
+{{ toYaml .securityContext | 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 .securityContext, defaults
to UID in the local node.
+
+++ +-+
+| .securityContext | > | .uid |
+++ +-+
+
+The template can be called like so:
+ include "localSecurityContext" .Values.statsd
+
+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 | print }}
+ {{- else -}}
+runAsUser: {{ .uid }}
+ {{- end -}}
+{{- end -}}
+
+{{/*
+Set the default value for workers chown for persistent storage
+If no value is passed for securityContext or .securityContext, defaults
to global uid and gid.
+The template looks for `runAsUser` and `fsGroup` specifically, any other
parameter will be ignored.
+
+++ +-+
+-+
+| .securityContext | -> | securityContext | -> | Values.uid +
Values.gid |
+++ +-+
+-+
+
+Values are not accumulated meaning that if runAsUser is set to 10 in
.securityContext,
+any extra values set to securityContext or uid+gid will be ignored.
+
+The template can be called like so:
+ include "airflowSecurityContextIds" (list . .Values.workers)
+
+Where '.' is the global varriables scope and `.Values.workers` the local
variables scope for the workers template.
Review comment:
```suggestion
Where `.` is the global variables scope and `.Values.workers` the local
variables scope for the workers template.
```
##
File path: chart/templates/_helpers.yaml
##
@@ -616,3 +615,85 @@ Create the name of the cleanup service account to use
{{- end -}}
{{- $kubeVersion -}}
{{- end -}}
+
+{{/*
+Set the default value for securityContext
+If no value is passed for securityContext or .securityContext, defaults
to global uid and gid.
+
+++ +-+
+-+
+| .securityContext | -> | securityContext | -> | Values.uid +
Values.gid |
+++ +-+
+-+
+
+Values are not accumulated meaning that if runAsUser is set to 10 in
.securityContext,
+any extra values set to securityContext or uid+gid will be ignored.
+
+The template can be called like so:
+ include "airflowSecurityContext" (list . .Values.webserver)
+
+Where '.' is the global varriables scope and `.Values.webserver` the local
variables scope for the webserver template.
Review comment:
```suggestion
Where `.` is the global variables scope and `.Values.webserver` the local
variables scope for the webserver template.
```
--
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
[GitHub] [airflow] jedcunningham commented on a change in pull request #18249: Add support for securityContext per deployment
jedcunningham commented on a change in pull request #18249:
URL: https://github.com/apache/airflow/pull/18249#discussion_r770683086
##
File path: chart/templates/_helpers.yaml
##
@@ -616,3 +615,49 @@ Create the name of the cleanup service account to use
{{- end -}}
{{- $kubeVersion -}}
{{- end -}}
+{{/*
+Set the default value for securityContext
+If no value is passed for securityContext or .securityContext, defaults
to global uid and gid
+*/}}
+{{- define "globalSecurityContext" -}}
Review comment:
(I can't think of a better name than `localSecurityContext` - I think it
fits pretty well anyways)
--
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
[GitHub] [airflow] jedcunningham commented on a change in pull request #18249: Add support for securityContext per deployment
jedcunningham commented on a change in pull request #18249:
URL: https://github.com/apache/airflow/pull/18249#discussion_r770682265
##
File path: chart/templates/_helpers.yaml
##
@@ -616,3 +615,49 @@ Create the name of the cleanup service account to use
{{- end -}}
{{- $kubeVersion -}}
{{- end -}}
+{{/*
+Set the default value for securityContext
+If no value is passed for securityContext or .securityContext, defaults
to global uid and gid
+*/}}
+{{- define "globalSecurityContext" -}}
Review comment:
I like `airflowSecurityContext`.
I think I like having a separate `localSecurityContext`, as it does grab the
uid from a different object too (`$.Values.uid` vs `.uid`).
--
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
[GitHub] [airflow] jedcunningham commented on a change in pull request #18249: Add support for securityContext per deployment
jedcunningham commented on a change in pull request #18249:
URL: https://github.com/apache/airflow/pull/18249#discussion_r770669303
##
File path: chart/templates/_helpers.yaml
##
@@ -616,3 +615,49 @@ Create the name of the cleanup service account to use
{{- end -}}
{{- $kubeVersion -}}
{{- end -}}
+{{/*
+Set the default value for securityContext
+If no value is passed for securityContext or .securityContext, defaults
to global uid and gid
+*/}}
+{{- define "globalSecurityContext" -}}
Review comment:
It ultimately doesn't matter, but as of today, the example chart Helm
generates does use (namespaced) camel case.
`"example_chart.serviceAccountName"`
This is probably something worth making consistent eventually.
--
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
[GitHub] [airflow] jedcunningham commented on a change in pull request #18249: Add support for securityContext per deployment
jedcunningham commented on a change in pull request #18249:
URL: https://github.com/apache/airflow/pull/18249#discussion_r769827430
##
File path: chart/templates/_helpers.yaml
##
@@ -616,3 +615,50 @@ Create the name of the cleanup service account to use
{{- end -}}
{{- $kubeVersion -}}
{{- end -}}
+
+{{/*
+Set the default value for securityContext
+If no value is passed for secirityContext or .securityContext, defaults
to global uid and gid
+*/}}
+{{- define "globalSecurityContext" -}}
+ {{- $ := index . 0 -}}
+ {{- with index . 1 }}
+{{- if .securityContext -}}
+{{ toYaml .securityContext | 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 secirityContext or .securityContext, defaults
to UID in the local node
+*/}}
+{{- define "localSecurityContext" -}}
+ {{- 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 secirityContext or .securityContext, defaults
to global uid and gid
Review comment:
```suggestion
If no value is passed for securityContext or .securityContext,
defaults to global uid and gid
```
##
File path: chart/templates/_helpers.yaml
##
@@ -616,3 +615,50 @@ Create the name of the cleanup service account to use
{{- end -}}
{{- $kubeVersion -}}
{{- end -}}
+
+{{/*
+Set the default value for securityContext
+If no value is passed for secirityContext or .securityContext, defaults
to global uid and gid
+*/}}
+{{- define "globalSecurityContext" -}}
+ {{- $ := index . 0 -}}
+ {{- with index . 1 }}
+{{- if .securityContext -}}
+{{ toYaml .securityContext | 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 secirityContext or .securityContext, defaults
to UID in the local node
Review comment:
```suggestion
If no value is passed for securityContext or .securityContext,
defaults to UID in the local node
```
##
File path: chart/templates/_helpers.yaml
##
@@ -616,3 +615,50 @@ Create the name of the cleanup service account to use
{{- end -}}
{{- $kubeVersion -}}
{{- end -}}
+
+{{/*
+Set the default value for securityContext
+If no value is passed for secirityContext or .securityContext, defaults
to global uid and gid
Review comment:
```suggestion
If no value is passed for securityContext or .securityContext,
defaults to global uid and gid
```
--
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
[GitHub] [airflow] jedcunningham commented on a change in pull request #18249: Add support for securityContext per deployment
jedcunningham commented on a change in pull request #18249:
URL: https://github.com/apache/airflow/pull/18249#discussion_r742089969
##
File path: chart/templates/_helpers.yaml
##
@@ -610,3 +609,50 @@ Create the name of the cleanup service account to use
{{- end -}}
{{- $kubeVersion -}}
{{- end -}}
+
+{{/*
+Set the default value for securityContext
+If no value is passed for secirityCpntext or .securityContext, defaults
to global uid and gid
+*/}}
+{{- define "globalSecurityContext" -}}
+ {{- $ := index . 0 -}}
+ {{- with index . 1 }}
+{{- if .securityContext -}}
+{{ toYaml .securityContext | 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 secirityCpntext or .securityContext, defaults
to UID in the local node
Review comment:
```suggestion
If no value is passed for securityContext or .securityContext,
defaults to UID in the local node
```
##
File path: chart/templates/_helpers.yaml
##
@@ -610,3 +609,50 @@ Create the name of the cleanup service account to use
{{- end -}}
{{- $kubeVersion -}}
{{- end -}}
+
+{{/*
+Set the default value for securityContext
+If no value is passed for secirityCpntext or .securityContext, defaults
to global uid and gid
+*/}}
+{{- define "globalSecurityContext" -}}
+ {{- $ := index . 0 -}}
+ {{- with index . 1 }}
+{{- if .securityContext -}}
+{{ toYaml .securityContext | 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 secirityCpntext or .securityContext, defaults
to UID in the local node
+*/}}
+{{- define "localSecurityContext" -}}
+ {{- 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 secirityCpntext or .securityContext, defaults
to global uid and gid
Review comment:
```suggestion
If no value is passed for securityContext or .securityContext,
defaults to global uid and gid
```
##
File path: chart/templates/_helpers.yaml
##
@@ -610,3 +609,50 @@ Create the name of the cleanup service account to use
{{- end -}}
{{- $kubeVersion -}}
{{- end -}}
+
+{{/*
+Set the default value for securityContext
+If no value is passed for secirityCpntext or .securityContext, defaults
to global uid and gid
Review comment:
```suggestion
If no value is passed for securityContext or .securityContext,
defaults to global uid and gid
```
##
File path: chart/tests/test_security_context.py
##
@@ -0,0 +1,189 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied. See the License for the
+# specific language governing permissions and limitations
+# under the License.
+
+import jmespath
+
+from tests.helm_template_generator import render_chart
+
+
+class TestSCBackwardsCompatibility:
+def test_check_deployments_and_jobs(self):
+docs = render_chart(
+values={
+"uid": 3000,
+"gid": 30,
+"webserver": {"defaultUser": {"enabled": True}},
+"flower": {"enabled": True},
+"airflowVersion": "2.2.0",
+"executor": "CeleryKubernetesExecutor",
+},
+show_only=[
+"templates/flower/flower-deployment.yaml",
+"templates/scheduler/scheduler-deployment.yaml",
+"templates/triggerer/triggerer-deployment.yaml",
+"templates/webserver/webserver-deployment.yaml",
+"templates/workers/worker-deployment.yaml",
+"templates/jobs/create-user-job.yaml",
+"templates/jobs/migrate-database-job.yaml",
+],
+)
+
+assert 3000 ==
jmespath.search("spec.template.spec.securityContext.runAsUser", docs[0])
+asser
[GitHub] [airflow] jedcunningham commented on a change in pull request #18249: Add support for securityContext per deployment
jedcunningham commented on a change in pull request #18249:
URL: https://github.com/apache/airflow/pull/18249#discussion_r742089969
##
File path: chart/templates/_helpers.yaml
##
@@ -610,3 +609,50 @@ Create the name of the cleanup service account to use
{{- end -}}
{{- $kubeVersion -}}
{{- end -}}
+
+{{/*
+Set the default value for securityContext
+If no value is passed for secirityCpntext or .securityContext, defaults
to global uid and gid
+*/}}
+{{- define "globalSecurityContext" -}}
+ {{- $ := index . 0 -}}
+ {{- with index . 1 }}
+{{- if .securityContext -}}
+{{ toYaml .securityContext | 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 secirityCpntext or .securityContext, defaults
to UID in the local node
Review comment:
```suggestion
If no value is passed for securityContext or .securityContext,
defaults to UID in the local node
```
##
File path: chart/templates/_helpers.yaml
##
@@ -610,3 +609,50 @@ Create the name of the cleanup service account to use
{{- end -}}
{{- $kubeVersion -}}
{{- end -}}
+
+{{/*
+Set the default value for securityContext
+If no value is passed for secirityCpntext or .securityContext, defaults
to global uid and gid
+*/}}
+{{- define "globalSecurityContext" -}}
+ {{- $ := index . 0 -}}
+ {{- with index . 1 }}
+{{- if .securityContext -}}
+{{ toYaml .securityContext | 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 secirityCpntext or .securityContext, defaults
to UID in the local node
+*/}}
+{{- define "localSecurityContext" -}}
+ {{- 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 secirityCpntext or .securityContext, defaults
to global uid and gid
Review comment:
```suggestion
If no value is passed for securityContext or .securityContext,
defaults to global uid and gid
```
##
File path: chart/templates/_helpers.yaml
##
@@ -610,3 +609,50 @@ Create the name of the cleanup service account to use
{{- end -}}
{{- $kubeVersion -}}
{{- end -}}
+
+{{/*
+Set the default value for securityContext
+If no value is passed for secirityCpntext or .securityContext, defaults
to global uid and gid
Review comment:
```suggestion
If no value is passed for securityContext or .securityContext,
defaults to global uid and gid
```
##
File path: chart/tests/test_security_context.py
##
@@ -0,0 +1,189 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied. See the License for the
+# specific language governing permissions and limitations
+# under the License.
+
+import jmespath
+
+from tests.helm_template_generator import render_chart
+
+
+class TestSCBackwardsCompatibility:
+def test_check_deployments_and_jobs(self):
+docs = render_chart(
+values={
+"uid": 3000,
+"gid": 30,
+"webserver": {"defaultUser": {"enabled": True}},
+"flower": {"enabled": True},
+"airflowVersion": "2.2.0",
+"executor": "CeleryKubernetesExecutor",
+},
+show_only=[
+"templates/flower/flower-deployment.yaml",
+"templates/scheduler/scheduler-deployment.yaml",
+"templates/triggerer/triggerer-deployment.yaml",
+"templates/webserver/webserver-deployment.yaml",
+"templates/workers/worker-deployment.yaml",
+"templates/jobs/create-user-job.yaml",
+"templates/jobs/migrate-database-job.yaml",
+],
+)
+
+assert 3000 ==
jmespath.search("spec.template.spec.securityContext.runAsUser", docs[0])
+asser
[GitHub] [airflow] jedcunningham commented on a change in pull request #18249: Add support for securityContext per deployment
jedcunningham commented on a change in pull request #18249:
URL: https://github.com/apache/airflow/pull/18249#discussion_r742089969
##
File path: chart/templates/_helpers.yaml
##
@@ -610,3 +609,50 @@ Create the name of the cleanup service account to use
{{- end -}}
{{- $kubeVersion -}}
{{- end -}}
+
+{{/*
+Set the default value for securityContext
+If no value is passed for secirityCpntext or .securityContext, defaults
to global uid and gid
+*/}}
+{{- define "globalSecurityContext" -}}
+ {{- $ := index . 0 -}}
+ {{- with index . 1 }}
+{{- if .securityContext -}}
+{{ toYaml .securityContext | 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 secirityCpntext or .securityContext, defaults
to UID in the local node
Review comment:
```suggestion
If no value is passed for securityContext or .securityContext,
defaults to UID in the local node
```
##
File path: chart/templates/_helpers.yaml
##
@@ -610,3 +609,50 @@ Create the name of the cleanup service account to use
{{- end -}}
{{- $kubeVersion -}}
{{- end -}}
+
+{{/*
+Set the default value for securityContext
+If no value is passed for secirityCpntext or .securityContext, defaults
to global uid and gid
+*/}}
+{{- define "globalSecurityContext" -}}
+ {{- $ := index . 0 -}}
+ {{- with index . 1 }}
+{{- if .securityContext -}}
+{{ toYaml .securityContext | 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 secirityCpntext or .securityContext, defaults
to UID in the local node
+*/}}
+{{- define "localSecurityContext" -}}
+ {{- 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 secirityCpntext or .securityContext, defaults
to global uid and gid
Review comment:
```suggestion
If no value is passed for securityContext or .securityContext,
defaults to global uid and gid
```
##
File path: chart/templates/_helpers.yaml
##
@@ -610,3 +609,50 @@ Create the name of the cleanup service account to use
{{- end -}}
{{- $kubeVersion -}}
{{- end -}}
+
+{{/*
+Set the default value for securityContext
+If no value is passed for secirityCpntext or .securityContext, defaults
to global uid and gid
Review comment:
```suggestion
If no value is passed for securityContext or .securityContext,
defaults to global uid and gid
```
##
File path: chart/tests/test_security_context.py
##
@@ -0,0 +1,189 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied. See the License for the
+# specific language governing permissions and limitations
+# under the License.
+
+import jmespath
+
+from tests.helm_template_generator import render_chart
+
+
+class TestSCBackwardsCompatibility:
+def test_check_deployments_and_jobs(self):
+docs = render_chart(
+values={
+"uid": 3000,
+"gid": 30,
+"webserver": {"defaultUser": {"enabled": True}},
+"flower": {"enabled": True},
+"airflowVersion": "2.2.0",
+"executor": "CeleryKubernetesExecutor",
+},
+show_only=[
+"templates/flower/flower-deployment.yaml",
+"templates/scheduler/scheduler-deployment.yaml",
+"templates/triggerer/triggerer-deployment.yaml",
+"templates/webserver/webserver-deployment.yaml",
+"templates/workers/worker-deployment.yaml",
+"templates/jobs/create-user-job.yaml",
+"templates/jobs/migrate-database-job.yaml",
+],
+)
+
+assert 3000 ==
jmespath.search("spec.template.spec.securityContext.runAsUser", docs[0])
+asser
[GitHub] [airflow] jedcunningham commented on a change in pull request #18249: Add support for securityContext per deployment
jedcunningham commented on a change in pull request #18249:
URL: https://github.com/apache/airflow/pull/18249#discussion_r740594865
##
File path: chart/values.schema.json
##
@@ -1629,6 +1666,18 @@
"description": "Annotations to add to the triggerer pods.",
"type": "object",
"default": {}
+},
+"securityContext": {
+"description": "Global Pod security context as defined in
`Pod Security Context
`_.
If not set, the values from `securityContext` will be used.",
Review comment:
```suggestion
"description": "Triggerer Pod security context as
defined in `Pod Security Context
`_.
If not set, the values from `securityContext` will be used.",
```
Or similar, and for all the others that aren't actually global?
##
File path: chart/tests/test_security_context.py
##
@@ -0,0 +1,206 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied. See the License for the
+# specific language governing permissions and limitations
+# under the License.
+
+import jmespath
+
+from tests.helm_template_generator import render_chart
+
+
+class TestSCBackwardsCompatibility:
+def test_check_deployments_and_jobs(self):
+docs = render_chart(
+values={
+"uid": 3000,
+"gid": 30,
+"webserver": {"defaultUser": {"enabled": True}},
+"flower": {"enabled": True},
+"airflowVersion": "2.2.0",
+"executor": "CeleryKubernetesExecutor",
+},
+show_only=[
+"templates/flower/flower-deployment.yaml",
+"templates/scheduler/scheduler-deployment.yaml",
+"templates/triggerer/triggerer-deployment.yaml",
+"templates/webserver/webserver-deployment.yaml",
+"templates/workers/worker-deployment.yaml",
+"templates/jobs/create-user-job.yaml",
+"templates/jobs/migrate-database-job.yaml",
+],
+)
+
+for index in range(len(docs)):
+assert 3000 ==
jmespath.search("spec.template.spec.securityContext.runAsUser", docs[index])
+assert 30 ==
jmespath.search("spec.template.spec.securityContext.fsGroup", docs[index])
+
+def test_check_statsd_uid(self):
+docs = render_chart(
+values={"statsd": {"enabled": True, "uid": 3000}},
+show_only=["templates/statsd/statsd-deployment.yaml"],
+)
+
+for index in range(len(docs)):
+assert 3000 ==
jmespath.search("spec.template.spec.securityContext.runAsUser", docs[index])
Review comment:
```suggestion
assert 3000 ==
jmespath.search("spec.template.spec.securityContext.runAsUser", docs[0])
```
Both here and elsewhere.
##
File path: chart/tests/test_security_context.py
##
@@ -0,0 +1,206 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied. See the License for the
+# specific language governing permissions and limitations
+# under the License.
+
+import jmespath
+
+from tests.helm_template_generator import render_chart
+
+
+class TestSCBackwardsCompatibility:
+def test_check_deployments_and_jobs(self):
+docs = render_chart(
+values={
+"uid": 3000,
+"gid": 30,
+"webserver": {"defaultUser": {"enabled": True}},
+
[GitHub] [airflow] jedcunningham commented on a change in pull request #18249: Add support for securityContext per deployment
jedcunningham commented on a change in pull request #18249:
URL: https://github.com/apache/airflow/pull/18249#discussion_r740594865
##
File path: chart/values.schema.json
##
@@ -1629,6 +1666,18 @@
"description": "Annotations to add to the triggerer pods.",
"type": "object",
"default": {}
+},
+"securityContext": {
+"description": "Global Pod security context as defined in
`Pod Security Context
`_.
If not set, the values from `securityContext` will be used.",
Review comment:
```suggestion
"description": "Triggerer Pod security context as
defined in `Pod Security Context
`_.
If not set, the values from `securityContext` will be used.",
```
Or similar, and for all the others that aren't actually global?
##
File path: chart/tests/test_security_context.py
##
@@ -0,0 +1,206 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied. See the License for the
+# specific language governing permissions and limitations
+# under the License.
+
+import jmespath
+
+from tests.helm_template_generator import render_chart
+
+
+class TestSCBackwardsCompatibility:
+def test_check_deployments_and_jobs(self):
+docs = render_chart(
+values={
+"uid": 3000,
+"gid": 30,
+"webserver": {"defaultUser": {"enabled": True}},
+"flower": {"enabled": True},
+"airflowVersion": "2.2.0",
+"executor": "CeleryKubernetesExecutor",
+},
+show_only=[
+"templates/flower/flower-deployment.yaml",
+"templates/scheduler/scheduler-deployment.yaml",
+"templates/triggerer/triggerer-deployment.yaml",
+"templates/webserver/webserver-deployment.yaml",
+"templates/workers/worker-deployment.yaml",
+"templates/jobs/create-user-job.yaml",
+"templates/jobs/migrate-database-job.yaml",
+],
+)
+
+for index in range(len(docs)):
+assert 3000 ==
jmespath.search("spec.template.spec.securityContext.runAsUser", docs[index])
+assert 30 ==
jmespath.search("spec.template.spec.securityContext.fsGroup", docs[index])
+
+def test_check_statsd_uid(self):
+docs = render_chart(
+values={"statsd": {"enabled": True, "uid": 3000}},
+show_only=["templates/statsd/statsd-deployment.yaml"],
+)
+
+for index in range(len(docs)):
+assert 3000 ==
jmespath.search("spec.template.spec.securityContext.runAsUser", docs[index])
Review comment:
```suggestion
assert 3000 ==
jmespath.search("spec.template.spec.securityContext.runAsUser", docs[0])
```
Both here and elsewhere.
##
File path: chart/tests/test_security_context.py
##
@@ -0,0 +1,206 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied. See the License for the
+# specific language governing permissions and limitations
+# under the License.
+
+import jmespath
+
+from tests.helm_template_generator import render_chart
+
+
+class TestSCBackwardsCompatibility:
+def test_check_deployments_and_jobs(self):
+docs = render_chart(
+values={
+"uid": 3000,
+"gid": 30,
+"webserver": {"defaultUser": {"enabled": True}},
+
[GitHub] [airflow] jedcunningham commented on a change in pull request #18249: Add support for securityContext per deployment
jedcunningham commented on a change in pull request #18249:
URL: https://github.com/apache/airflow/pull/18249#discussion_r740594865
##
File path: chart/values.schema.json
##
@@ -1629,6 +1666,18 @@
"description": "Annotations to add to the triggerer pods.",
"type": "object",
"default": {}
+},
+"securityContext": {
+"description": "Global Pod security context as defined in
`Pod Security Context
`_.
If not set, the values from `securityContext` will be used.",
Review comment:
```suggestion
"description": "Triggerer Pod security context as
defined in `Pod Security Context
`_.
If not set, the values from `securityContext` will be used.",
```
Or similar, and for all the others that aren't actually global?
##
File path: chart/tests/test_security_context.py
##
@@ -0,0 +1,206 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied. See the License for the
+# specific language governing permissions and limitations
+# under the License.
+
+import jmespath
+
+from tests.helm_template_generator import render_chart
+
+
+class TestSCBackwardsCompatibility:
+def test_check_deployments_and_jobs(self):
+docs = render_chart(
+values={
+"uid": 3000,
+"gid": 30,
+"webserver": {"defaultUser": {"enabled": True}},
+"flower": {"enabled": True},
+"airflowVersion": "2.2.0",
+"executor": "CeleryKubernetesExecutor",
+},
+show_only=[
+"templates/flower/flower-deployment.yaml",
+"templates/scheduler/scheduler-deployment.yaml",
+"templates/triggerer/triggerer-deployment.yaml",
+"templates/webserver/webserver-deployment.yaml",
+"templates/workers/worker-deployment.yaml",
+"templates/jobs/create-user-job.yaml",
+"templates/jobs/migrate-database-job.yaml",
+],
+)
+
+for index in range(len(docs)):
+assert 3000 ==
jmespath.search("spec.template.spec.securityContext.runAsUser", docs[index])
+assert 30 ==
jmespath.search("spec.template.spec.securityContext.fsGroup", docs[index])
+
+def test_check_statsd_uid(self):
+docs = render_chart(
+values={"statsd": {"enabled": True, "uid": 3000}},
+show_only=["templates/statsd/statsd-deployment.yaml"],
+)
+
+for index in range(len(docs)):
+assert 3000 ==
jmespath.search("spec.template.spec.securityContext.runAsUser", docs[index])
Review comment:
```suggestion
assert 3000 ==
jmespath.search("spec.template.spec.securityContext.runAsUser", docs[0])
```
Both here and elsewhere.
##
File path: chart/tests/test_security_context.py
##
@@ -0,0 +1,206 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied. See the License for the
+# specific language governing permissions and limitations
+# under the License.
+
+import jmespath
+
+from tests.helm_template_generator import render_chart
+
+
+class TestSCBackwardsCompatibility:
+def test_check_deployments_and_jobs(self):
+docs = render_chart(
+values={
+"uid": 3000,
+"gid": 30,
+"webserver": {"defaultUser": {"enabled": True}},
+
[GitHub] [airflow] jedcunningham commented on a change in pull request #18249: Add support for securityContext per deployment
jedcunningham commented on a change in pull request #18249:
URL: https://github.com/apache/airflow/pull/18249#discussion_r722672775
##
File path: chart/templates/_helpers.yaml
##
@@ -610,3 +610,62 @@ Create the name of the cleanup service account to use
{{- end -}}
{{- $kubeVersion -}}
{{- end -}}
+
+{{/*
+Set the default podsecurity.securityContext
+If no value is passed, defaults to .Values.uid and .Values.gid
+This function is required fr backwards compatibility
+*/}}
+{{- define "defaultSecurityContext" -}}
+{{- if .Values.podSecurity.securityContext -}}
+ {{ .Values.podSecurity.securityContext | toJson }}
+{{- else -}}
+ {{- $result := dict "runAsUser" .Values.uid "fsGroup" .Values.gid }}
+ {{- $result | toJson }}
+{{- end -}}
+{{- end -}}
+
+{{/*
+Set the default podsecurity.securityContext
+If no value is passed, defaults to .Values.uid and .Values.gid
+This function is required fr backwards compatibility
+*/}}
+{{- define "defaultContainerSecurityContext" -}}
+{{- if .Values.podSecurity.containerSecurityContext -}}
+{{ .Values.podSecurity.containerSecurityContext | toJson }}
+{{- else -}}
+ {{- $result := dict "runAsUser" .Values.uid "runAsGroup" .Values.gid }}
+ {{- $result | toJson }}
+{{- end -}}
+{{- end -}}
+
+{{/*
+For gitSync and statsD, we use their respectice uid properties as fallback
+*/}}
+{{- define "gitSyncContainerSecurityContext" -}}
+{{- if .Values.dags.gitSync.containerSecurityContext -}}
+ {{ .Values.dags.gitSync.containerSecurityContext | toYaml }}
+{{- else if .Values.podSecurity.containerSecurityContext -}}
+ {{ .Values.podSecurity.containerSecurityContext | toYaml }}
+{{- else -}}
+runAsUser: {{ .Values.dags.gitSync.uid }}
+{{- end -}}
+{{- end -}}
+
+{{- define "statsdSecurityContext" -}}
+{{- if .Values.podSecurity.securityContext -}}
+ {{ .Values.podSecurity.securityContext | toJson }}
+{{- else -}}
+ {{- $result := dict "runAsUser" .Values.statsd.uid }}
+ {{- $result | toJson }}
Review comment:
Fair enough, makes sense, I should have known it wasn't as simple as it
seemed 👍
--
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
[GitHub] [airflow] jedcunningham commented on a change in pull request #18249: Add support for securityContext per deployment
jedcunningham commented on a change in pull request #18249:
URL: https://github.com/apache/airflow/pull/18249#discussion_r720453456
##
File path: chart/templates/_helpers.yaml
##
@@ -610,3 +610,62 @@ Create the name of the cleanup service account to use
{{- end -}}
{{- $kubeVersion -}}
{{- end -}}
+
+{{/*
+Set the default podsecurity.securityContext
+If no value is passed, defaults to .Values.uid and .Values.gid
+This function is required fr backwards compatibility
Review comment:
```suggestion
This function is required for backwards compatibility
```
##
File path: chart/templates/cleanup/cleanup-cronjob.yaml
##
@@ -22,6 +22,8 @@
{{- $nodeSelector := or .Values.cleanup.nodeSelector .Values.nodeSelector }}
{{- $affinity := or .Values.cleanup.affinity .Values.affinity }}
{{- $tolerations := or .Values.cleanup.tolerations .Values.tolerations }}
+{{- $securityContext := or .Values.cleanup.securityContext (include
"defaultSecurityContext" . | mustFromJson ) }}
+{{- $containerSecurityContext := or .Values.cleanup.containerSecurityContext
(include "defaultContainerSecurityContext" . | mustFromJson ) }}
Review comment:
```suggestion
{{- $securityContext := or .Values.cleanup.securityContext (include
"defaultSecurityContext" . | mustFromJson) }}
{{- $containerSecurityContext := or .Values.cleanup.containerSecurityContext
(include "defaultContainerSecurityContext" . | mustFromJson) }}
```
nit
##
File path: chart/templates/_helpers.yaml
##
@@ -610,3 +610,62 @@ Create the name of the cleanup service account to use
{{- end -}}
{{- $kubeVersion -}}
{{- end -}}
+
+{{/*
+Set the default podsecurity.securityContext
+If no value is passed, defaults to .Values.uid and .Values.gid
+This function is required fr backwards compatibility
+*/}}
+{{- define "defaultSecurityContext" -}}
+{{- if .Values.podSecurity.securityContext -}}
+ {{ .Values.podSecurity.securityContext | toJson }}
+{{- else -}}
+ {{- $result := dict "runAsUser" .Values.uid "fsGroup" .Values.gid }}
+ {{- $result | toJson }}
+{{- end -}}
+{{- end -}}
+
+{{/*
+Set the default podsecurity.securityContext
+If no value is passed, defaults to .Values.uid and .Values.gid
+This function is required fr backwards compatibility
+*/}}
+{{- define "defaultContainerSecurityContext" -}}
+{{- if .Values.podSecurity.containerSecurityContext -}}
+{{ .Values.podSecurity.containerSecurityContext | toJson }}
+{{- else -}}
+ {{- $result := dict "runAsUser" .Values.uid "runAsGroup" .Values.gid }}
+ {{- $result | toJson }}
+{{- end -}}
+{{- end -}}
+
+{{/*
+For gitSync and statsD, we use their respectice uid properties as fallback
+*/}}
+{{- define "gitSyncContainerSecurityContext" -}}
+{{- if .Values.dags.gitSync.containerSecurityContext -}}
+ {{ .Values.dags.gitSync.containerSecurityContext | toYaml }}
+{{- else if .Values.podSecurity.containerSecurityContext -}}
+ {{ .Values.podSecurity.containerSecurityContext | toYaml }}
+{{- else -}}
+runAsUser: {{ .Values.dags.gitSync.uid }}
+{{- end -}}
+{{- end -}}
+
+{{- define "statsdSecurityContext" -}}
+{{- if .Values.podSecurity.securityContext -}}
+ {{ .Values.podSecurity.securityContext | toJson }}
+{{- else -}}
+ {{- $result := dict "runAsUser" .Values.statsd.uid }}
+ {{- $result | toJson }}
Review comment:
Can this be consistent with how `gitsync` is handled (I think the
non-dict route is more readable)?. That should also work for setting more than
1, no?
##
File path: chart/templates/_helpers.yaml
##
@@ -610,3 +610,62 @@ Create the name of the cleanup service account to use
{{- end -}}
{{- $kubeVersion -}}
{{- end -}}
+
+{{/*
+Set the default podsecurity.securityContext
+If no value is passed, defaults to .Values.uid and .Values.gid
+This function is required fr backwards compatibility
+*/}}
+{{- define "defaultSecurityContext" -}}
+{{- if .Values.podSecurity.securityContext -}}
+ {{ .Values.podSecurity.securityContext | toJson }}
+{{- else -}}
+ {{- $result := dict "runAsUser" .Values.uid "fsGroup" .Values.gid }}
+ {{- $result | toJson }}
+{{- end -}}
+{{- end -}}
+
+{{/*
+Set the default podsecurity.securityContext
+If no value is passed, defaults to .Values.uid and .Values.gid
+This function is required fr backwards compatibility
+*/}}
+{{- define "defaultContainerSecurityContext" -}}
+{{- if .Values.podSecurity.containerSecurityContext -}}
+{{ .Values.podSecurity.containerSecurityContext | toJson }}
+{{- else -}}
+ {{- $result := dict "runAsUser" .Values.uid "runAsGroup" .Values.gid }}
Review comment:
I don't think we should set these at the container level. We should only
set it at the pod level, otherwise choices for
`podSecurity.securityContext.runAsUser` or
`scheduler.podSecurity.securityContext.runAsUser` will be just be overridden by
this default.
Similarly, if we want to set runAsGroup by default, we should set it only in
the `defaultSecurityCo
[GitHub] [airflow] jedcunningham commented on a change in pull request #18249: Add support for securityContext per deployment
jedcunningham commented on a change in pull request #18249:
URL: https://github.com/apache/airflow/pull/18249#discussion_r717791801
##
File path: chart/files/pod-template-file.kubernetes-helm-yaml
##
@@ -56,6 +56,7 @@ spec:
image: {{ template "pod_template_image" . }}
imagePullPolicy: {{ .Values.images.airflow.pullPolicy }}
name: base
+ securityContext: {{-
.Values.podTemplateSecurity.containerSecurityContext | default
(.Values.podSecurity.containerSecurityContext) | toYaml | nindent 4 }}
Review comment:
This should use `workers.containerSecurityContext` instead of having a
separate param.
##
File path: chart/values.yaml
##
@@ -383,6 +393,19 @@ workers:
maxSurge: "100%"
maxUnavailable: "50%"
+ # When not set, the values defined on podSecurity.securityContext will be
used
+ securityContext:
+runAsUser: 5
+fsGroup: 0
Review comment:
```suggestion
securityContext: {}
# runAsUser: 5
# fsGroup: 0
```
We shouldn't set these for all of the components, otherwise
`podSecurity.securityContext` will never actually be used and isn't an easy way
to set it across the board.
##
File path: chart/files/pod-template-file.kubernetes-helm-yaml
##
@@ -87,9 +88,7 @@ spec:
- name: {{ template "registry_secret" . }}
{{- end }}
restartPolicy: Never
- securityContext:
-runAsUser: {{ .Values.uid }}
-fsGroup: {{ .Values.gid }}
+ securityContext: {{- .Values.podTemplateSecurity.securityContext | default
(.Values.podSecurity.securityContext) | toYaml | nindent 4 }}
Review comment:
Same here, should use `workers.securityContext`.
--
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
[GitHub] [airflow] jedcunningham commented on a change in pull request #18249: Add support for securityContext per deployment
jedcunningham commented on a change in pull request #18249:
URL: https://github.com/apache/airflow/pull/18249#discussion_r717791801
##
File path: chart/files/pod-template-file.kubernetes-helm-yaml
##
@@ -56,6 +56,7 @@ spec:
image: {{ template "pod_template_image" . }}
imagePullPolicy: {{ .Values.images.airflow.pullPolicy }}
name: base
+ securityContext: {{-
.Values.podTemplateSecurity.containerSecurityContext | default
(.Values.podSecurity.containerSecurityContext) | toYaml | nindent 4 }}
Review comment:
This should use `workers.containerSecurityContext` instead of having a
separate param.
##
File path: chart/values.yaml
##
@@ -383,6 +393,19 @@ workers:
maxSurge: "100%"
maxUnavailable: "50%"
+ # When not set, the values defined on podSecurity.securityContext will be
used
+ securityContext:
+runAsUser: 5
+fsGroup: 0
Review comment:
```suggestion
securityContext: {}
# runAsUser: 5
# fsGroup: 0
```
We shouldn't set these for all of the components, otherwise
`podSecurity.securityContext` will never actually be used and isn't an easy way
to set it across the board.
##
File path: chart/files/pod-template-file.kubernetes-helm-yaml
##
@@ -87,9 +88,7 @@ spec:
- name: {{ template "registry_secret" . }}
{{- end }}
restartPolicy: Never
- securityContext:
-runAsUser: {{ .Values.uid }}
-fsGroup: {{ .Values.gid }}
+ securityContext: {{- .Values.podTemplateSecurity.securityContext | default
(.Values.podSecurity.securityContext) | toYaml | nindent 4 }}
Review comment:
Same here, should use `workers.securityContext`.
--
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
[GitHub] [airflow] jedcunningham commented on a change in pull request #18249: Add support for securityContext per deployment
jedcunningham commented on a change in pull request #18249:
URL: https://github.com/apache/airflow/pull/18249#discussion_r708827609
##
File path: chart/values.schema.json
##
@@ -70,6 +70,332 @@
"default": "2.1.3",
"x-docsSection": "Common"
},
+"podSecurity": {
+"description": "Set security contexts for certain containers",
+"type": "object",
+"x-docsSection": "Kubernetes",
+"additionalProperties": false,
+"properties": {
+"default": {
+"description": "Default global security context.",
+"type": "object",
+"additionalProperties": false,
+"properties": {
+"securityContext": {
+"description": "Global Pod security context as
defined in
https://kubernetes.io/docs/reference/generated/kubernetes-api/v1.18/#podsecuritycontext-v1-core";,
+"type": "object",
+"default": "See values.yaml",
Review comment:
```suggestion
"default": {"runAsUser": 5, "fsGroup": 0,
"runAsGroup": 0},
```
I think we should bring the actual defaults into the docs instead.
##
File path: chart/files/pod-template-file.kubernetes-helm-yaml
##
@@ -45,6 +45,7 @@ spec:
{{- end }}
containers:
- args: []
+ securityContext: {{- omit
.Values.podSecurity.pod_template.containerSecurityContext "enabled" | default
(.Values.podSecurity.default.containerSecurityContext) | toYaml | nindent 8 }}
Review comment:
Instead of this `omit` pattern, would this work instead? I think it will
and is more intuitive imo.
values.yaml:
```
podSecurity.pod_template.containerSecurityContext: {}
```
and:
```suggestion
securityContext: {{-
.Values.podSecurity.pod_template.containerSecurityContext | default
(.Values.podSecurity.default.containerSecurityContext) | toYaml | nindent 8 }}
```
--
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
