jedcunningham commented on code in PR #24588:
URL: https://github.com/apache/airflow/pull/24588#discussion_r941662272
##########
chart/values.schema.json:
##########
@@ -1907,6 +1935,34 @@
}
]
},
+ "securityContexts": {
+ "description": "Security context definition. The values in
this parameter will be used when `securityContexts` is not defined for specific
Pods/Container",
Review Comment:
```suggestion
"description": "Security context definition for the
scheduler.",
```
##########
chart/templates/NOTES.txt:
##########
@@ -176,6 +176,14 @@ DEPRECATION WARNING:
{{- if not (or .Values.webserverSecretKey
.Values.webserverSecretKeySecretName) }}
+{{- if .Values.securityContext }}
+
+ DEPRECATION WARNING:
+ `securityContext` has been renamed to `securityContexts`, to be enabled on
container and pod level.
Review Comment:
We also need the corresponding update to the `airflowSecurityContext` helper
to use the new config also.
##########
chart/values.yaml:
##########
@@ -35,12 +35,21 @@ revisionHistoryLimit: ~
uid: 50000
gid: 0
-# Default security context for airflow
+# Default security context for airflow (deprecated, use below in the future)
securityContext: {}
# runAsUser: 50000
# fsGroup: 0
# runAsGroup: 0
+# Detailed default security context for airflow deployments
+securityContexts:
+ pod: {}
+ containers:
+ allowPrivilegeEscalation: false
+ capabilities:
+ drop:
+ - ALL
Review Comment:
This is the default, yeah? Not sure we need to expand it here.
##########
chart/values.schema.json:
##########
@@ -100,6 +100,34 @@
}
]
},
+ "securityContexts": {
+ "description": "Security context definition. The values in this
parameter will be used when `securityContexts` is not defined for specific
Pods/Container",
Review Comment:
```suggestion
"description": "Security context definition. The values in this
parameter will be used when `securityContexts` is not defined for specific
Pods/Container.",
```
Missing some punctuation.
##########
tests/charts/test_scheduler.py:
##########
@@ -304,6 +304,38 @@ def test_logs_persistence_changes_volume(self,
log_persistence_values, expected_
assert {"name": "logs", **expected_volume} in
jmespath.search("spec.template.spec.volumes", docs[0])
+ def test_scheduler_securityContexts_are_configurable(self):
+ docs = render_chart(
+ values={
+ "scheduler": {
+ "securityContexts": {
+ "pod": {
+ "fsGroup": 1000,
+ 'runAsGroup': 1000,
+ 'runAsNonRoot': "true",
+ 'runAsUser': 1000,
+ },
+ "container": {
+ "allowPrivilegeEscalation": "false",
+ 'readOnlyRootFilesystem': "true",
+ },
+ }
+ },
+ },
+ show_only=["templates/scheduler/scheduler-deployment.yaml"],
+ )
+ assert "false" == jmespath.search(
+
"spec.template.spec.containers[0].securityContext.allowPrivilegeEscalation",
docs[0]
+ )
+ assert "true" == jmespath.search(
+
"spec.template.spec.containers[0].securityContext.readOnlyRootFilesystem",
docs[0]
+ )
Review Comment:
```suggestion
assert {"allowPrivilegeEscalation": False,
"readOnlyRootFilesystem": True} == jmespath.search(
"spec.template.spec.containers[0].securityContext", docs[0]
)
```
Might be cleaner to check the whole thing in 1 go (probably need some
formatting, but you get the idea).
##########
chart/values.schema.json:
##########
@@ -1907,6 +1935,34 @@
}
]
},
+ "securityContexts": {
+ "description": "Security context definition. The values in
this parameter will be used when `securityContexts` is not defined for specific
Pods/Container",
+ "type": "object",
+ "x-docsSection": "Kubernetes",
+ "properties": {
+ "pod": {
+ "description": "Pod security context definition.
The values in this parameter will be used when `securityContexts` is not
defined for specific Pods",
+ "type": "object",
+ "$ref":
"#/definitions/io.k8s.api.core.v1.PodSecurityContext",
+ "default": {},
+ "x-docsSection": "Kubernetes",
+ "examples": [
+ {
+ "runAsUser": 50000,
+ "runAsGroup": 0,
+ "fsGroup": 0
+ }
+ ]
+ },
+ "container": {
+ "description": "Container security context
definition. The values in this parameter will be used when `securityContexts`
is not defined for specific containers",
Review Comment:
```suggestion
"description": "Container security context
definition for the scheduler.",
```
##########
chart/values.schema.json:
##########
@@ -1907,6 +1935,34 @@
}
]
},
+ "securityContexts": {
+ "description": "Security context definition. The values in
this parameter will be used when `securityContexts` is not defined for specific
Pods/Container",
+ "type": "object",
+ "x-docsSection": "Kubernetes",
+ "properties": {
+ "pod": {
+ "description": "Pod security context definition.
The values in this parameter will be used when `securityContexts` is not
defined for specific Pods",
Review Comment:
```suggestion
"description": "Pod security context definition
for the scheduler.",
```
##########
chart/values.schema.json:
##########
@@ -100,6 +100,34 @@
}
]
},
+ "securityContexts": {
+ "description": "Security context definition. The values in this
parameter will be used when `securityContexts` is not defined for specific
Pods/Container",
+ "type": "object",
+ "x-docsSection": "Kubernetes",
+ "properties": {
+ "pod": {
+ "description": "Pod security context definition. The
values in this parameter will be used when `securityContexts` is not defined
for specific Pods",
+ "type": "object",
+ "$ref":
"#/definitions/io.k8s.api.core.v1.PodSecurityContext",
+ "default": {},
+ "x-docsSection": "Kubernetes",
+ "examples": [
+ {
+ "runAsUser": 50000,
+ "runAsGroup": 0,
+ "fsGroup": 0
+ }
+ ]
+ },
+ "container": {
+ "description": "Container security context definition. The
values in this parameter will be used when `securityContexts` is not defined
for specific containers",
+ "type": "object",
+ "$ref": "#/definitions/io.k8s.api.core.v1.SecurityContext",
+ "default": {},
Review Comment:
There is a default though?
##########
tests/charts/test_scheduler.py:
##########
@@ -304,6 +304,38 @@ def test_logs_persistence_changes_volume(self,
log_persistence_values, expected_
assert {"name": "logs", **expected_volume} in
jmespath.search("spec.template.spec.volumes", docs[0])
+ def test_scheduler_securityContexts_are_configurable(self):
+ docs = render_chart(
+ values={
+ "scheduler": {
+ "securityContexts": {
+ "pod": {
+ "fsGroup": 1000,
+ 'runAsGroup': 1000,
Review Comment:
```suggestion
'runAsGroup': 2000,
```
Might be nice to use different ints here.
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
