[GitHub] [airflow] jedcunningham commented on a change in pull request #22182: Add securityContext config for Redis to helm chart

[GitBox <gi...@apache.org>]

jedcunningham commented on a change in pull request #22182:
URL: https://github.com/apache/airflow/pull/22182#discussion_r837797114



##########
File path: chart/values.schema.json
##########
@@ -3764,6 +3764,19 @@
                             }
                         }
                     }
+                },

Review comment:
       ```suggestion
                   },
                   "uid": {
                       "description": "Redis run as user parameter.",
                       "type": "integer",
                       "default": 999
                   },
   ```
   
   Which means we also need `uid`.

##########
File path: chart/templates/redis/redis-statefulset.yaml
##########
@@ -22,6 +22,7 @@
 {{- $nodeSelector := or .Values.redis.nodeSelector .Values.nodeSelector }}
 {{- $affinity := or .Values.redis.affinity .Values.affinity }}
 {{- $tolerations := or .Values.redis.tolerations .Values.tolerations }}
+{{- $securityContext := include "airflowSecurityContext" (list . .Values.redis) }}

Review comment:
       ```suggestion
   {{- $securityContext := include "localSecurityContext" .Values.redis }}
   ```
   
   This should use `localSecurityContext` instead. We don't want the global Airflow security context to apply to Redis.

##########
File path: chart/values.schema.json
##########
@@ -3764,6 +3764,19 @@
                             }
                         }
                     }
+                },
+                "securityContext": {
+                    "description": "Security context for the cleanup job pod. If not set, the values from `securityContext` will be used.",
+                    "type": "object",
+                    "$ref": "#/definitions/io.k8s.api.core.v1.PodSecurityContext",
+                    "default": {},
+                    "examples": [
+                        {
+                            "runAsUser": 50000,

Review comment:
       ```suggestion
                               "runAsUser": 999,
   ```
   
   This will be a better example, as this is the uid of the `redis` user.

##########
File path: chart/values.yaml
##########
@@ -1328,6 +1328,11 @@ redis:
   affinity: {}
   tolerations: []
 
+  # When not set, the values defined in the global securityContext will be used

Review comment:
       ```suggestion
     uid: 999
     # If not set, `redis.uid` will be used
   ```

##########
File path: tests/charts/test_security_context.py
##########
@@ -33,6 +33,7 @@ def test_check_deployments_and_jobs(self):
             },
             show_only=[
                 "templates/flower/flower-deployment.yaml",
+                "templates/redis/redis-statefulset.yaml",

Review comment:
       Instead, we should refactor `test_check_statsd_uid` to be like this:
   
   ```
       # Test containerSecurity priority over uid under components using localSecurityContext
       def test_check_local_uid(self):
           component_contexts = {"uid": 3000, "securityContext": {"runAsUser": 7000}}
           docs = render_chart(
               values={
                   "redis": {**component_contexts},
                   "statsd": {"enabled": True, **component_contexts},
               },
               show_only=[
                   "templates/statsd/statsd-deployment.yaml",
                   "templates/redis/redis-statefulset.yaml",
               ],
           )
   
           for doc in docs:
               assert 7000 == jmespath.search("spec.template.spec.securityContext.runAsUser", doc)
   ```

##########
File path: tests/charts/test_security_context.py
##########
@@ -33,6 +33,7 @@ def test_check_deployments_and_jobs(self):
             },
             show_only=[
                 "templates/flower/flower-deployment.yaml",
+                "templates/redis/redis-statefulset.yaml",

Review comment:
       ```suggestion
   ```




-- 
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-unsubscribe@airflow.apache.org

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