Do not declare a volume for sshKeySecret if dag persistence is enabled (

apache#22913)

* Do not declare a volume for sshKeySecret if dag persistence is enabled

In scheduler and triggerer components, git-sync-ssh-key volume was created even
when persistence is enabled. This PR fixes that and added tests
in other components to avoid regression

chart/templates/scheduler/scheduler-deployment.yaml

@@ -253,10 +253,10 @@ spec:
         {{- else if .Values.dags.gitSync.enabled }}
         - name: dags
           emptyDir: {}
-        {{- end }}
-        {{- if and  .Values.dags.gitSync.enabled  .Values.dags.gitSync.sshKeySecret }}
+        {{- if .Values.dags.gitSync.sshKeySecret }}
         {{- include "git_sync_ssh_key_volume" . | indent 8 }}
         {{- end }}
+        {{- end}}
 {{- end }}
 {{- if .Values.scheduler.extraVolumes }}
 {{ toYaml .Values.scheduler.extraVolumes | indent 8 }}

chart/templates/triggerer/triggerer-deployment.yaml

@@ -199,10 +199,10 @@ spec:
         {{- else if .Values.dags.gitSync.enabled }}
         - name: dags
           emptyDir: {}
-        {{- end }}
-        {{- if and .Values.dags.gitSync.enabled .Values.dags.gitSync.sshKeySecret }}
+        {{- if .Values.dags.gitSync.sshKeySecret }}
         {{- include "git_sync_ssh_key_volume" . | indent 8 }}
         {{- end }}
+        {{- end }}
         {{- if .Values.triggerer.extraVolumes }}
         {{- toYaml .Values.triggerer.extraVolumes | nindent 8 }}
         {{- end }}

tests/charts/test_git_sync_scheduler.py

@@ -131,6 +131,24 @@ def test_validate_if_ssh_params_are_added(self):
             "secret": {"secretName": "ssh-secret", "defaultMode": 288},
         } in jmespath.search("spec.template.spec.volumes", docs[0])
 
+    def test_validate_sshkeysecret_not_added_when_persistence_is_enabled(self):
+        docs = render_chart(
+            values={
+                "dags": {
+                    "gitSync": {
+                        "enabled": True,
+                        "containerName": "git-sync-test",
+                        "sshKeySecret": "ssh-secret",
+                        "knownHosts": None,
+                        "branch": "test-branch",
+                    },
+                    "persistence": {"enabled": True},
+                }
+            },
+            show_only=["templates/scheduler/scheduler-deployment.yaml"],
+        )
+        assert "git-sync-ssh-key" not in jmespath.search("spec.template.spec.volumes[].name", docs[0])
+
     def test_should_set_username_and_pass_env_variables(self):
         docs = render_chart(
             values={

tests/charts/test_git_sync_triggerer.py

@@ -0,0 +1,42 @@
+# 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 unittest
+
+import jmespath
+
+from tests.charts.helm_template_generator import render_chart
+
+
+class GitSyncTriggererTest(unittest.TestCase):
+    def test_validate_sshkeysecret_not_added_when_persistence_is_enabled(self):
+        docs = render_chart(
+            values={
+                "dags": {
+                    "gitSync": {
+                        "enabled": True,
+                        "containerName": "git-sync-test",
+                        "sshKeySecret": "ssh-secret",
+                        "knownHosts": None,
+                        "branch": "test-branch",
+                    },
+                    "persistence": {"enabled": True},
+                }
+            },
+            show_only=["templates/triggerer/triggerer-deployment.yaml"],
+        )
+        assert "git-sync-ssh-key" not in jmespath.search("spec.template.spec.volumes[].name", docs[0])

tests/charts/test_git_sync_webserver.py

@@ -170,3 +170,21 @@ def test_resources_are_configurable(self):
             "spec.template.spec.containers[1].resources.requests.memory", docs[0]
         )
         assert "300m" == jmespath.search("spec.template.spec.containers[1].resources.requests.cpu", docs[0])
+
+    def test_validate_sshkeysecret_not_added_when_persistence_is_enabled(self):
+        docs = render_chart(
+            values={
+                "dags": {
+                    "gitSync": {
+                        "enabled": True,
+                        "containerName": "git-sync-test",
+                        "sshKeySecret": "ssh-secret",
+                        "knownHosts": None,
+                        "branch": "test-branch",
+                    },
+                    "persistence": {"enabled": True},
+                }
+            },
+            show_only=["templates/webserver/webserver-deployment.yaml"],
+        )
+        assert "git-sync-ssh-key" not in jmespath.search("spec.template.spec.volumes[].name", docs[0])

tests/charts/test_git_sync_worker.py

@@ -112,3 +112,22 @@ def test_resources_are_configurable(self):
             "spec.template.spec.containers[1].resources.requests.memory", docs[0]
         )
         assert "300m" == jmespath.search("spec.template.spec.containers[1].resources.requests.cpu", docs[0])
+
+    def test_validate_sshkeysecret_not_added_when_persistence_is_enabled(self):
+        docs = render_chart(
+            values={
+                "dags": {
+                    "gitSync": {
+                        "enabled": True,
+                        "containerName": "git-sync-test",
+                        "sshKeySecret": "ssh-secret",
+                        "knownHosts": None,
+                        "branch": "test-branch",
+                    },
+                    "persistence": {"enabled": True},
+                }
+            },
+            show_only=["templates/workers/worker-deployment.yaml"],
+        )
+
+        assert "git-sync-ssh-key" not in jmespath.search("spec.template.spec.volumes[].name", docs[0])