jedcunningham commented on code in PR #31865:
URL: https://github.com/apache/airflow/pull/31865#discussion_r1237863627
##########
tests/charts/security/test_security_context.py:
##########
@@ -210,6 +210,36 @@ def test_gitsync_sidecar_and_init_container(self):
docs[index],
)
+ # Test securityContexts for main containers
+ def test_global_security_context(self):
+ ctx_value_pod = {"runAsUser": 7000}
+ ctx_value_container = {"allowPrivilegeEscalation": False}
+ docs = render_chart(
+ values={"securityContexts": {"containers": ctx_value_container,
"pod": ctx_value_pod}},
+ show_only=[
+ "templates/flower/flower-deployment.yaml",
+ "templates/scheduler/scheduler-deployment.yaml",
+ "templates/webserver/webserver-deployment.yaml",
+ "templates/workers/worker-deployment.yaml",
+ "templates/jobs/create-user-job.yaml",
+ "templates/jobs/migrate-database-job.yaml",
+ "templates/triggerer/triggerer-deployment.yaml",
+ "templates/statsd/statsd-deployment.yaml",
+ "templates/redis/redis-statefulset.yaml",
+ ],
+ )
+
+ for index in range(len(docs) - 2):
Review Comment:
Hmm, is the order of the docs deterministic? Might be worth a comment why we
treat them differently.
##########
chart/templates/statsd/statsd-deployment.yaml:
##########
@@ -89,7 +88,7 @@ spec:
- name: statsd
image: {{ template "statsd_image" . }}
imagePullPolicy: {{ .Values.images.statsd.pullPolicy }}
- securityContext: {{ $containerSecurityContext | nindent 12 }}
+ securityContext: {{- toYaml
.Values.statsd.securityContexts.container | nindent 12 }}
Review Comment:
This breaks backcompat on `stastd.securityContext`.
##########
chart/templates/redis/redis-statefulset.yaml:
##########
@@ -80,7 +79,7 @@ spec:
- name: redis
image: {{ template "redis_image" . }}
imagePullPolicy: {{ .Values.images.redis.pullPolicy }}
- securityContext: {{ $containerSecurityContext | nindent 12 }}
+ securityContext: {{- toYaml .Values.redis.securityContexts.container
| nindent 12 }}
Review Comment:
This breaks backcompat on `redis.securityContext`.
##########
chart/values.yaml:
##########
@@ -1629,7 +1629,6 @@ statsd:
uid: 65534
# When not set, `statsd.uid` will be used
- # When not set, the values defined in the global securityContext will be used
Review Comment:
Also need to remove this comment from values.schema.json.
##########
tests/charts/security/test_security_context.py:
##########
@@ -210,6 +210,36 @@ def test_gitsync_sidecar_and_init_container(self):
docs[index],
)
+ # Test securityContexts for main containers
+ def test_global_security_context(self):
+ ctx_value_pod = {"runAsUser": 7000}
+ ctx_value_container = {"allowPrivilegeEscalation": False}
+ docs = render_chart(
+ values={"securityContexts": {"containers": ctx_value_container,
"pod": ctx_value_pod}},
+ show_only=[
+ "templates/flower/flower-deployment.yaml",
+ "templates/scheduler/scheduler-deployment.yaml",
+ "templates/webserver/webserver-deployment.yaml",
+ "templates/workers/worker-deployment.yaml",
+ "templates/jobs/create-user-job.yaml",
+ "templates/jobs/migrate-database-job.yaml",
+ "templates/triggerer/triggerer-deployment.yaml",
+ "templates/statsd/statsd-deployment.yaml",
+ "templates/redis/redis-statefulset.yaml",
+ ],
+ )
+
+ for index in range(len(docs) - 2):
+ assert ctx_value_container == jmespath.search(
+ "spec.template.spec.containers[0].securityContext", docs[index]
+ )
+ assert ctx_value_pod ==
jmespath.search("spec.template.spec.securityContext", docs[index])
+ for index in range(len(docs) - 2, len(docs)):
+ assert ctx_value_container != jmespath.search(
+ "spec.template.spec.containers[0].securityContext", docs[index]
+ )
+ assert ctx_value_pod !=
jmespath.search("spec.template.spec.securityContext", docs[index])
Review Comment:
Or, maybe provide values for statds and redis and test those are applied
properly too?
##########
tests/charts/security/test_security_context.py:
##########
@@ -210,6 +210,36 @@ def test_gitsync_sidecar_and_init_container(self):
docs[index],
)
+ # Test securityContexts for main containers
+ def test_global_security_context(self):
+ ctx_value_pod = {"runAsUser": 7000}
+ ctx_value_container = {"allowPrivilegeEscalation": False}
+ docs = render_chart(
+ values={"securityContexts": {"containers": ctx_value_container,
"pod": ctx_value_pod}},
+ show_only=[
+ "templates/flower/flower-deployment.yaml",
+ "templates/scheduler/scheduler-deployment.yaml",
+ "templates/webserver/webserver-deployment.yaml",
+ "templates/workers/worker-deployment.yaml",
+ "templates/jobs/create-user-job.yaml",
+ "templates/jobs/migrate-database-job.yaml",
+ "templates/triggerer/triggerer-deployment.yaml",
+ "templates/statsd/statsd-deployment.yaml",
+ "templates/redis/redis-statefulset.yaml",
+ ],
+ )
+
+ for index in range(len(docs) - 2):
+ assert ctx_value_container == jmespath.search(
+ "spec.template.spec.containers[0].securityContext", docs[index]
+ )
+ assert ctx_value_pod ==
jmespath.search("spec.template.spec.securityContext", docs[index])
+ for index in range(len(docs) - 2, len(docs)):
+ assert ctx_value_container != jmespath.search(
+ "spec.template.spec.containers[0].securityContext", docs[index]
+ )
+ assert ctx_value_pod !=
jmespath.search("spec.template.spec.securityContext", docs[index])
Review Comment:
```suggestion
assert {} == jmespath.search(
"spec.template.spec.containers[0].securityContext",
docs[index]
)
assert {} ==
jmespath.search("spec.template.spec.securityContext", docs[index])
```
Probably better to check what it is, vs isn't?
--
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]
