From 11ce0b6ece60fec02a98cb4571891c6b5842390a Mon Sep 17 00:00:00 2001 From: Angel Freire Date: Fri, 9 Jan 2026 13:31:02 -0300 Subject: [PATCH 1/2] fix(chart): allow revisionHistoryLimit to be set to 0 Use explicit nil check instead of truthy check to allow 0 as valid value. Signed-off-by: Angel Freire --- chart/templates/_helpers.yaml | 18 +++++++++++++++ .../api-server/api-server-deployment.yaml | 4 ++-- .../dag-processor-deployment.yaml | 4 ++-- chart/templates/flower/flower-deployment.yaml | 4 ++-- .../pgbouncer/pgbouncer-deployment.yaml | 4 ++-- .../scheduler/scheduler-deployment.yaml | 4 ++-- chart/templates/statsd/statsd-deployment.yaml | 4 ++-- .../triggerer/triggerer-deployment.yaml | 4 ++-- .../webserver/webserver-deployment.yaml | 4 ++-- .../templates/workers/worker-deployment.yaml | 4 ++-- .../airflow_core/test_api_server.py | 19 +++++++++++++++ .../airflow_core/test_dag_processor.py | 19 +++++++++++++++ .../helm_tests/airflow_core/test_scheduler.py | 19 +++++++++++++++ .../helm_tests/airflow_core/test_triggerer.py | 19 +++++++++++++++ .../helm_tests/airflow_core/test_worker.py | 23 +++++++++++++++++++ .../tests/helm_tests/other/test_flower.py | 19 +++++++++++++++ .../tests/helm_tests/other/test_pgbouncer.py | 19 +++++++++++++++ .../tests/helm_tests/other/test_statsd.py | 19 +++++++++++++++ .../helm_tests/webserver/test_webserver.py | 19 +++++++++++++++ 19 files changed, 211 insertions(+), 18 deletions(-) diff --git a/chart/templates/_helpers.yaml b/chart/templates/_helpers.yaml index e1e92cf84b5ac..60e8fc6140a1b 100644 --- a/chart/templates/_helpers.yaml +++ b/chart/templates/_helpers.yaml @@ -1172,3 +1172,21 @@ This helper function returns true if the executor can launch pods (Kubernetes-ba {{- print "true" -}} {{- end -}} {{- end -}} + +{{/* +Get revisionHistoryLimit with nil-aware fallback. +Unlike `or`, this properly handles 0 as a valid value. +Pass a list of values to check in order of priority. + +Usage: + include "airflow.revisionHistoryLimit" (list .Values.scheduler.revisionHistoryLimit .Values.revisionHistoryLimit) +*/}} +{{- define "airflow.revisionHistoryLimit" -}} + {{- $result := "" -}} + {{- range . -}} + {{- if and (eq $result "") (not (kindIs "invalid" .)) -}} + {{- $result = . -}} + {{- end -}} + {{- end -}} + {{- $result -}} +{{- end -}} diff --git a/chart/templates/api-server/api-server-deployment.yaml b/chart/templates/api-server/api-server-deployment.yaml index b4d905e587a91..66c7588f9077b 100644 --- a/chart/templates/api-server/api-server-deployment.yaml +++ b/chart/templates/api-server/api-server-deployment.yaml @@ -25,7 +25,7 @@ {{- $affinity := or .Values.apiServer.affinity .Values.affinity }} {{- $tolerations := or .Values.apiServer.tolerations .Values.tolerations }} {{- $topologySpreadConstraints := or .Values.apiServer.topologySpreadConstraints .Values.topologySpreadConstraints }} -{{- $revisionHistoryLimit := or .Values.apiServer.revisionHistoryLimit .Values.revisionHistoryLimit }} +{{- $revisionHistoryLimit := include "airflow.revisionHistoryLimit" (list .Values.apiServer.revisionHistoryLimit .Values.revisionHistoryLimit) }} {{- $securityContext := include "airflowPodSecurityContext" (list . .Values.apiServer) }} {{- $containerSecurityContext := include "containerSecurityContext" (list . .Values.apiServer) }} {{- $containerSecurityContextWaitForMigrations := include "containerSecurityContext" (list . .Values.apiServer.waitForMigrations) }} @@ -48,7 +48,7 @@ metadata: {{- end }} spec: replicas: {{ .Values.apiServer.replicas }} - {{- if $revisionHistoryLimit }} + {{- if ne $revisionHistoryLimit "" }} revisionHistoryLimit: {{ $revisionHistoryLimit }} {{- end }} strategy: diff --git a/chart/templates/dag-processor/dag-processor-deployment.yaml b/chart/templates/dag-processor/dag-processor-deployment.yaml index 69abd70eaca05..4fd69f971dc4b 100644 --- a/chart/templates/dag-processor/dag-processor-deployment.yaml +++ b/chart/templates/dag-processor/dag-processor-deployment.yaml @@ -30,7 +30,7 @@ {{- $affinity := or .Values.dagProcessor.affinity .Values.affinity }} {{- $tolerations := or .Values.dagProcessor.tolerations .Values.tolerations }} {{- $topologySpreadConstraints := or .Values.dagProcessor.topologySpreadConstraints .Values.topologySpreadConstraints }} -{{- $revisionHistoryLimit := or .Values.dagProcessor.revisionHistoryLimit .Values.revisionHistoryLimit }} +{{- $revisionHistoryLimit := include "airflow.revisionHistoryLimit" (list .Values.dagProcessor.revisionHistoryLimit .Values.revisionHistoryLimit) }} {{- $securityContext := include "airflowPodSecurityContext" (list . .Values.dagProcessor) }} {{- $containerSecurityContext := include "containerSecurityContext" (list . .Values.dagProcessor) }} {{- $containerSecurityContextLogGroomerSidecar := include "containerSecurityContext" (list . .Values.dagProcessor.logGroomerSidecar) }} @@ -54,7 +54,7 @@ metadata: {{- end }} spec: replicas: {{ .Values.dagProcessor.replicas }} - {{- if $revisionHistoryLimit }} + {{- if ne $revisionHistoryLimit "" }} revisionHistoryLimit: {{ $revisionHistoryLimit }} {{- end }} selector: diff --git a/chart/templates/flower/flower-deployment.yaml b/chart/templates/flower/flower-deployment.yaml index 50bfc1520ff94..ff5b2a0b03461 100644 --- a/chart/templates/flower/flower-deployment.yaml +++ b/chart/templates/flower/flower-deployment.yaml @@ -26,7 +26,7 @@ {{- $affinity := or .Values.flower.affinity .Values.affinity }} {{- $tolerations := or .Values.flower.tolerations .Values.tolerations }} {{- $topologySpreadConstraints := or .Values.flower.topologySpreadConstraints .Values.topologySpreadConstraints }} -{{- $revisionHistoryLimit := or .Values.flower.revisionHistoryLimit .Values.revisionHistoryLimit }} +{{- $revisionHistoryLimit := include "airflow.revisionHistoryLimit" (list .Values.flower.revisionHistoryLimit .Values.revisionHistoryLimit) }} {{- $securityContext := include "airflowPodSecurityContext" (list . .Values.flower) }} {{- $containerSecurityContext := include "containerSecurityContext" (list . .Values.flower) }} {{- $containerLifecycleHooks := or .Values.flower.containerLifecycleHooks .Values.containerLifecycleHooks }} @@ -48,7 +48,7 @@ metadata: {{- end }} spec: replicas: 1 - {{- if $revisionHistoryLimit }} + {{- if ne $revisionHistoryLimit "" }} revisionHistoryLimit: {{ $revisionHistoryLimit }} {{- end }} selector: diff --git a/chart/templates/pgbouncer/pgbouncer-deployment.yaml b/chart/templates/pgbouncer/pgbouncer-deployment.yaml index 74412f2d4ebf8..b568c259b1234 100644 --- a/chart/templates/pgbouncer/pgbouncer-deployment.yaml +++ b/chart/templates/pgbouncer/pgbouncer-deployment.yaml @@ -25,7 +25,7 @@ {{- $affinity := or .Values.pgbouncer.affinity .Values.affinity }} {{- $tolerations := or .Values.pgbouncer.tolerations .Values.tolerations }} {{- $topologySpreadConstraints := or .Values.pgbouncer.topologySpreadConstraints .Values.topologySpreadConstraints }} -{{- $revisionHistoryLimit := or .Values.pgbouncer.revisionHistoryLimit .Values.revisionHistoryLimit }} +{{- $revisionHistoryLimit := include "airflow.revisionHistoryLimit" (list .Values.pgbouncer.revisionHistoryLimit .Values.revisionHistoryLimit) }} {{- $securityContext := include "localPodSecurityContext" .Values.pgbouncer }} {{- $containerSecurityContext := include "externalContainerSecurityContext" .Values.pgbouncer }} {{- $containerSecurityContextMetricsExporter := include "externalContainerSecurityContext" .Values.pgbouncer.metricsExporterSidecar }} @@ -49,7 +49,7 @@ metadata: {{- end }} spec: replicas: {{ .Values.pgbouncer.replicas | default "1" }} - {{- if $revisionHistoryLimit }} + {{- if ne $revisionHistoryLimit "" }} revisionHistoryLimit: {{ $revisionHistoryLimit }} {{- end }} strategy: diff --git a/chart/templates/scheduler/scheduler-deployment.yaml b/chart/templates/scheduler/scheduler-deployment.yaml index a5c40a60734fb..c9eec470fb3f3 100644 --- a/chart/templates/scheduler/scheduler-deployment.yaml +++ b/chart/templates/scheduler/scheduler-deployment.yaml @@ -40,7 +40,7 @@ {{- $affinity := or .Values.scheduler.affinity .Values.affinity }} {{- $tolerations := or .Values.scheduler.tolerations .Values.tolerations }} {{- $topologySpreadConstraints := or .Values.scheduler.topologySpreadConstraints .Values.topologySpreadConstraints }} -{{- $revisionHistoryLimit := or .Values.scheduler.revisionHistoryLimit .Values.revisionHistoryLimit }} +{{- $revisionHistoryLimit := include "airflow.revisionHistoryLimit" (list .Values.scheduler.revisionHistoryLimit .Values.revisionHistoryLimit) }} {{- $securityContext := include "airflowPodSecurityContext" (list . .Values.scheduler) }} {{- $containerSecurityContext := include "containerSecurityContext" (list . .Values.scheduler) }} {{- $containerSecurityContextWaitForMigrations := include "containerSecurityContext" (list . .Values.scheduler.waitForMigrations) }} @@ -69,7 +69,7 @@ spec: serviceName: {{ include "airflow.fullname" . }}-scheduler {{- end }} replicas: {{ .Values.scheduler.replicas }} - {{- if $revisionHistoryLimit }} + {{- if ne $revisionHistoryLimit "" }} revisionHistoryLimit: {{ $revisionHistoryLimit }} {{- end }} {{- if and $stateful .Values.scheduler.updateStrategy }} diff --git a/chart/templates/statsd/statsd-deployment.yaml b/chart/templates/statsd/statsd-deployment.yaml index 57ab8b477a980..6fef2cb71c6f2 100644 --- a/chart/templates/statsd/statsd-deployment.yaml +++ b/chart/templates/statsd/statsd-deployment.yaml @@ -25,7 +25,7 @@ {{- $affinity := or .Values.statsd.affinity .Values.affinity }} {{- $tolerations := or .Values.statsd.tolerations .Values.tolerations }} {{- $topologySpreadConstraints := or .Values.statsd.topologySpreadConstraints .Values.topologySpreadConstraints }} -{{- $revisionHistoryLimit := or .Values.statsd.revisionHistoryLimit .Values.revisionHistoryLimit }} +{{- $revisionHistoryLimit := include "airflow.revisionHistoryLimit" (list .Values.statsd.revisionHistoryLimit .Values.revisionHistoryLimit) }} {{- $securityContext := include "localPodSecurityContext" .Values.statsd }} {{- $containerSecurityContext := include "externalContainerSecurityContext" .Values.statsd }} {{- $containerLifecycleHooks := .Values.statsd.containerLifecycleHooks }} @@ -47,7 +47,7 @@ metadata: {{- end }} spec: replicas: 1 - {{- if $revisionHistoryLimit }} + {{- if ne $revisionHistoryLimit "" }} revisionHistoryLimit: {{ $revisionHistoryLimit }} {{- end }} selector: diff --git a/chart/templates/triggerer/triggerer-deployment.yaml b/chart/templates/triggerer/triggerer-deployment.yaml index 6b140eeffa32d..9799b095f6368 100644 --- a/chart/templates/triggerer/triggerer-deployment.yaml +++ b/chart/templates/triggerer/triggerer-deployment.yaml @@ -29,7 +29,7 @@ {{- $affinity := or .Values.triggerer.affinity .Values.affinity }} {{- $tolerations := or .Values.triggerer.tolerations .Values.tolerations }} {{- $topologySpreadConstraints := or .Values.triggerer.topologySpreadConstraints .Values.topologySpreadConstraints }} -{{- $revisionHistoryLimit := or .Values.triggerer.revisionHistoryLimit .Values.revisionHistoryLimit }} +{{- $revisionHistoryLimit := include "airflow.revisionHistoryLimit" (list .Values.triggerer.revisionHistoryLimit .Values.revisionHistoryLimit) }} {{- $securityContext := include "airflowPodSecurityContext" (list . .Values.triggerer) }} {{- $containerSecurityContext := include "containerSecurityContext" (list . .Values.triggerer) }} {{- $containerSecurityContextWaitForMigrations := include "containerSecurityContext" (list . .Values.triggerer.waitForMigrations) }} @@ -59,7 +59,7 @@ spec: {{- if not $keda }} replicas: {{ .Values.triggerer.replicas }} {{- end }} - {{- if $revisionHistoryLimit }} + {{- if ne $revisionHistoryLimit "" }} revisionHistoryLimit: {{ $revisionHistoryLimit }} {{- end }} selector: diff --git a/chart/templates/webserver/webserver-deployment.yaml b/chart/templates/webserver/webserver-deployment.yaml index f1d4b2d92ee4f..7c7396058bf9a 100644 --- a/chart/templates/webserver/webserver-deployment.yaml +++ b/chart/templates/webserver/webserver-deployment.yaml @@ -25,7 +25,7 @@ {{- $affinity := or .Values.webserver.affinity .Values.affinity }} {{- $tolerations := or .Values.webserver.tolerations .Values.tolerations }} {{- $topologySpreadConstraints := or .Values.webserver.topologySpreadConstraints .Values.topologySpreadConstraints }} -{{- $revisionHistoryLimit := or .Values.webserver.revisionHistoryLimit .Values.revisionHistoryLimit }} +{{- $revisionHistoryLimit := include "airflow.revisionHistoryLimit" (list .Values.webserver.revisionHistoryLimit .Values.revisionHistoryLimit) }} {{- $securityContext := include "airflowPodSecurityContext" (list . .Values.webserver) }} {{- $containerSecurityContext := include "containerSecurityContext" (list . .Values.webserver) }} {{- $containerSecurityContextWaitForMigrations := include "containerSecurityContext" (list . .Values.webserver.waitForMigrations) }} @@ -50,7 +50,7 @@ spec: {{- if not .Values.webserver.hpa.enabled }} replicas: {{ .Values.webserver.replicas }} {{- end}} - {{- if $revisionHistoryLimit }} + {{- if ne $revisionHistoryLimit "" }} revisionHistoryLimit: {{ $revisionHistoryLimit }} {{- end }} strategy: diff --git a/chart/templates/workers/worker-deployment.yaml b/chart/templates/workers/worker-deployment.yaml index 9eaa3331f8311..1c042db0fc455 100644 --- a/chart/templates/workers/worker-deployment.yaml +++ b/chart/templates/workers/worker-deployment.yaml @@ -28,7 +28,7 @@ {{- $affinity := or .Values.workers.affinity .Values.affinity }} {{- $tolerations := or .Values.workers.tolerations .Values.tolerations }} {{- $topologySpreadConstraints := or .Values.workers.topologySpreadConstraints .Values.topologySpreadConstraints }} -{{- $revisionHistoryLimit := or .Values.workers.celery.revisionHistoryLimit .Values.workers.revisionHistoryLimit .Values.revisionHistoryLimit }} +{{- $revisionHistoryLimit := include "airflow.revisionHistoryLimit" (list .Values.workers.celery.revisionHistoryLimit .Values.workers.revisionHistoryLimit .Values.revisionHistoryLimit) }} {{- $securityContext := include "airflowPodSecurityContext" (list . .Values.workers) }} {{- $containerSecurityContext := include "containerSecurityContext" (list . .Values.workers) }} {{- $containerSecurityContextPersistence := include "containerSecurityContext" (list . .Values.workers.persistence) }} @@ -65,7 +65,7 @@ spec: {{- if and (not $keda) (not $hpa) }} replicas: {{ .Values.workers.celery.replicas | default .Values.workers.replicas }} {{- end }} - {{- if $revisionHistoryLimit }} + {{- if ne $revisionHistoryLimit "" }} revisionHistoryLimit: {{ $revisionHistoryLimit }} {{- end }} {{- if and $persistence (or .Values.workers.celery.persistence.persistentVolumeClaimRetentionPolicy .Values.workers.persistence.persistentVolumeClaimRetentionPolicy) }} diff --git a/helm-tests/tests/helm_tests/airflow_core/test_api_server.py b/helm-tests/tests/helm_tests/airflow_core/test_api_server.py index 377c4b9703025..78b5abdae78d5 100644 --- a/helm-tests/tests/helm_tests/airflow_core/test_api_server.py +++ b/helm-tests/tests/helm_tests/airflow_core/test_api_server.py @@ -50,6 +50,25 @@ def test_revision_history_limit(self, revision_history_limit, global_revision_hi expected_result = revision_history_limit if revision_history_limit else global_revision_history_limit assert jmespath.search("spec.revisionHistoryLimit", docs[0]) == expected_result + @pytest.mark.parametrize( + ("revision_history_limit", "global_revision_history_limit", "expected"), + [(0, None, 0), (None, 0, 0), (0, 10, 0)], + ) + def test_revision_history_limit_zero( + self, revision_history_limit, global_revision_history_limit, expected + ): + """Test that revisionHistoryLimit can be set to 0.""" + values = {"apiServer": {}} + if revision_history_limit is not None: + values["apiServer"]["revisionHistoryLimit"] = revision_history_limit + if global_revision_history_limit is not None: + values["revisionHistoryLimit"] = global_revision_history_limit + docs = render_chart( + values=values, + show_only=["templates/api-server/api-server-deployment.yaml"], + ) + assert jmespath.search("spec.revisionHistoryLimit", docs[0]) == expected + def test_should_add_scheme_to_liveness_and_readiness_and_startup_probes(self): docs = render_chart( values={ diff --git a/helm-tests/tests/helm_tests/airflow_core/test_dag_processor.py b/helm-tests/tests/helm_tests/airflow_core/test_dag_processor.py index 3f72718027756..996b7aff5b035 100644 --- a/helm-tests/tests/helm_tests/airflow_core/test_dag_processor.py +++ b/helm-tests/tests/helm_tests/airflow_core/test_dag_processor.py @@ -624,6 +624,25 @@ def test_revision_history_limit(self, revision_history_limit, global_revision_hi expected_result = revision_history_limit or global_revision_history_limit assert jmespath.search("spec.revisionHistoryLimit", docs[0]) == expected_result + @pytest.mark.parametrize( + ("revision_history_limit", "global_revision_history_limit", "expected"), + [(0, None, 0), (None, 0, 0), (0, 10, 0)], + ) + def test_revision_history_limit_zero( + self, revision_history_limit, global_revision_history_limit, expected + ): + """Test that revisionHistoryLimit can be set to 0.""" + values = {"dagProcessor": {"enabled": True}} + if revision_history_limit is not None: + values["dagProcessor"]["revisionHistoryLimit"] = revision_history_limit + if global_revision_history_limit is not None: + values["revisionHistoryLimit"] = global_revision_history_limit + docs = render_chart( + values=values, + show_only=["templates/dag-processor/dag-processor-deployment.yaml"], + ) + assert jmespath.search("spec.revisionHistoryLimit", docs[0]) == expected + @pytest.mark.parametrize("command", [None, ["custom", "command"]]) @pytest.mark.parametrize("args", [None, ["custom", "args"]]) def test_command_and_args_overrides(self, command, args): diff --git a/helm-tests/tests/helm_tests/airflow_core/test_scheduler.py b/helm-tests/tests/helm_tests/airflow_core/test_scheduler.py index c8db632b99726..06dfe7c13e61c 100644 --- a/helm-tests/tests/helm_tests/airflow_core/test_scheduler.py +++ b/helm-tests/tests/helm_tests/airflow_core/test_scheduler.py @@ -348,6 +348,25 @@ def test_revision_history_limit(self, revision_history_limit, global_revision_hi expected_result = revision_history_limit or global_revision_history_limit assert jmespath.search("spec.revisionHistoryLimit", docs[0]) == expected_result + @pytest.mark.parametrize( + ("revision_history_limit", "global_revision_history_limit", "expected"), + [(0, None, 0), (None, 0, 0), (0, 10, 0)], + ) + def test_revision_history_limit_zero( + self, revision_history_limit, global_revision_history_limit, expected + ): + """Test that revisionHistoryLimit can be set to 0.""" + values = {"scheduler": {}} + if revision_history_limit is not None: + values["scheduler"]["revisionHistoryLimit"] = revision_history_limit + if global_revision_history_limit is not None: + values["revisionHistoryLimit"] = global_revision_history_limit + docs = render_chart( + values=values, + show_only=["templates/scheduler/scheduler-deployment.yaml"], + ) + assert jmespath.search("spec.revisionHistoryLimit", docs[0]) == expected + def test_should_create_valid_affinity_tolerations_and_node_selector(self): docs = render_chart( values={ diff --git a/helm-tests/tests/helm_tests/airflow_core/test_triggerer.py b/helm-tests/tests/helm_tests/airflow_core/test_triggerer.py index b3c9e55881608..be14e770cb284 100644 --- a/helm-tests/tests/helm_tests/airflow_core/test_triggerer.py +++ b/helm-tests/tests/helm_tests/airflow_core/test_triggerer.py @@ -75,6 +75,25 @@ def test_revision_history_limit(self, revision_history_limit, global_revision_hi expected_result = revision_history_limit or global_revision_history_limit assert jmespath.search("spec.revisionHistoryLimit", docs[0]) == expected_result + @pytest.mark.parametrize( + ("revision_history_limit", "global_revision_history_limit", "expected"), + [(0, None, 0), (None, 0, 0), (0, 10, 0)], + ) + def test_revision_history_limit_zero( + self, revision_history_limit, global_revision_history_limit, expected + ): + """Test that revisionHistoryLimit can be set to 0.""" + values = {"triggerer": {"enabled": True}} + if revision_history_limit is not None: + values["triggerer"]["revisionHistoryLimit"] = revision_history_limit + if global_revision_history_limit is not None: + values["revisionHistoryLimit"] = global_revision_history_limit + docs = render_chart( + values=values, + show_only=["templates/triggerer/triggerer-deployment.yaml"], + ) + assert jmespath.search("spec.revisionHistoryLimit", docs[0]) == expected + def test_disable_wait_for_migration(self): docs = render_chart( values={ diff --git a/helm-tests/tests/helm_tests/airflow_core/test_worker.py b/helm-tests/tests/helm_tests/airflow_core/test_worker.py index 82a48a82423f4..123a579568c7a 100644 --- a/helm-tests/tests/helm_tests/airflow_core/test_worker.py +++ b/helm-tests/tests/helm_tests/airflow_core/test_worker.py @@ -133,6 +133,29 @@ def test_revision_history_limit_overwrite(self, worker_values, global_limit, exp assert jmespath.search("spec.revisionHistoryLimit", docs[0]) == expected + @pytest.mark.parametrize( + ("worker_values", "global_limit", "expected"), + [ + ({}, 0, 0), + ({"revisionHistoryLimit": 0}, None, 0), + ({"celery": {"revisionHistoryLimit": 0}}, None, 0), + ({"revisionHistoryLimit": 0}, 10, 0), + ({"celery": {"revisionHistoryLimit": 0}}, 10, 0), + ({"revisionHistoryLimit": 0, "celery": {"revisionHistoryLimit": 0}}, 10, 0), + ], + ) + def test_revision_history_limit_zero(self, worker_values, global_limit, expected): + """Test that revisionHistoryLimit can be set to 0.""" + values = {"workers": worker_values} + if global_limit is not None: + values["revisionHistoryLimit"] = global_limit + docs = render_chart( + values=values, + show_only=["templates/workers/worker-deployment.yaml"], + ) + + assert jmespath.search("spec.revisionHistoryLimit", docs[0]) == expected + def test_should_add_extra_containers(self): docs = render_chart( values={ diff --git a/helm-tests/tests/helm_tests/other/test_flower.py b/helm-tests/tests/helm_tests/other/test_flower.py index acf34d58355c8..b423df50fdb07 100644 --- a/helm-tests/tests/helm_tests/other/test_flower.py +++ b/helm-tests/tests/helm_tests/other/test_flower.py @@ -69,6 +69,25 @@ def test_revision_history_limit(self, revision_history_limit, global_revision_hi expected_result = revision_history_limit or global_revision_history_limit assert jmespath.search("spec.revisionHistoryLimit", docs[0]) == expected_result + @pytest.mark.parametrize( + ("revision_history_limit", "global_revision_history_limit", "expected"), + [(0, None, 0), (None, 0, 0), (0, 10, 0)], + ) + def test_revision_history_limit_zero( + self, revision_history_limit, global_revision_history_limit, expected + ): + """Test that revisionHistoryLimit can be set to 0.""" + values = {"flower": {"enabled": True}} + if revision_history_limit is not None: + values["flower"]["revisionHistoryLimit"] = revision_history_limit + if global_revision_history_limit is not None: + values["revisionHistoryLimit"] = global_revision_history_limit + docs = render_chart( + values=values, + show_only=["templates/flower/flower-deployment.yaml"], + ) + assert jmespath.search("spec.revisionHistoryLimit", docs[0]) == expected + @pytest.mark.parametrize( ("airflow_version", "expected_arg"), [ diff --git a/helm-tests/tests/helm_tests/other/test_pgbouncer.py b/helm-tests/tests/helm_tests/other/test_pgbouncer.py index 857ae069c3e42..df6dbe66e0816 100644 --- a/helm-tests/tests/helm_tests/other/test_pgbouncer.py +++ b/helm-tests/tests/helm_tests/other/test_pgbouncer.py @@ -129,6 +129,25 @@ def test_revision_history_limit(self, revision_history_limit, global_revision_hi expected_result = revision_history_limit or global_revision_history_limit assert jmespath.search("spec.revisionHistoryLimit", docs[0]) == expected_result + @pytest.mark.parametrize( + ("revision_history_limit", "global_revision_history_limit", "expected"), + [(0, None, 0), (None, 0, 0), (0, 10, 0)], + ) + def test_revision_history_limit_zero( + self, revision_history_limit, global_revision_history_limit, expected + ): + """Test that revisionHistoryLimit can be set to 0.""" + values = {"pgbouncer": {"enabled": True}} + if revision_history_limit is not None: + values["pgbouncer"]["revisionHistoryLimit"] = revision_history_limit + if global_revision_history_limit is not None: + values["revisionHistoryLimit"] = global_revision_history_limit + docs = render_chart( + values=values, + show_only=["templates/pgbouncer/pgbouncer-deployment.yaml"], + ) + assert jmespath.search("spec.revisionHistoryLimit", docs[0]) == expected + def test_scheduler_name(self): docs = render_chart( values={"pgbouncer": {"enabled": True}, "schedulerName": "airflow-scheduler"}, diff --git a/helm-tests/tests/helm_tests/other/test_statsd.py b/helm-tests/tests/helm_tests/other/test_statsd.py index 21815aff5e48d..711b7cd49b6a6 100644 --- a/helm-tests/tests/helm_tests/other/test_statsd.py +++ b/helm-tests/tests/helm_tests/other/test_statsd.py @@ -104,6 +104,25 @@ def test_revision_history_limit(self, revision_history_limit, global_revision_hi expected_result = revision_history_limit or global_revision_history_limit assert jmespath.search("spec.revisionHistoryLimit", docs[0]) == expected_result + @pytest.mark.parametrize( + ("revision_history_limit", "global_revision_history_limit", "expected"), + [(0, None, 0), (None, 0, 0), (0, 10, 0)], + ) + def test_revision_history_limit_zero( + self, revision_history_limit, global_revision_history_limit, expected + ): + """Test that revisionHistoryLimit can be set to 0.""" + values = {"statsd": {"enabled": True}} + if revision_history_limit is not None: + values["statsd"]["revisionHistoryLimit"] = revision_history_limit + if global_revision_history_limit is not None: + values["revisionHistoryLimit"] = global_revision_history_limit + docs = render_chart( + values=values, + show_only=["templates/statsd/statsd-deployment.yaml"], + ) + assert jmespath.search("spec.revisionHistoryLimit", docs[0]) == expected + def test_scheduler_name(self): docs = render_chart( values={"schedulerName": "airflow-scheduler"}, diff --git a/helm-tests/tests/helm_tests/webserver/test_webserver.py b/helm-tests/tests/helm_tests/webserver/test_webserver.py index ed4d7ea5710b7..ca64e27998aac 100644 --- a/helm-tests/tests/helm_tests/webserver/test_webserver.py +++ b/helm-tests/tests/helm_tests/webserver/test_webserver.py @@ -136,6 +136,25 @@ def test_revision_history_limit(self, revision_history_limit, global_revision_hi expected_result = revision_history_limit if revision_history_limit else global_revision_history_limit assert jmespath.search("spec.revisionHistoryLimit", docs[0]) == expected_result + @pytest.mark.parametrize( + ("revision_history_limit", "global_revision_history_limit", "expected"), + [(0, None, 0), (None, 0, 0), (0, 10, 0)], + ) + def test_revision_history_limit_zero( + self, revision_history_limit, global_revision_history_limit, expected + ): + """Test that revisionHistoryLimit can be set to 0.""" + values = {"webserver": {}, "airflowVersion": "2.10.5"} + if revision_history_limit is not None: + values["webserver"]["revisionHistoryLimit"] = revision_history_limit + if global_revision_history_limit is not None: + values["revisionHistoryLimit"] = global_revision_history_limit + docs = render_chart( + values=values, + show_only=["templates/webserver/webserver-deployment.yaml"], + ) + assert jmespath.search("spec.revisionHistoryLimit", docs[0]) == expected + @pytest.mark.parametrize( "values", [ From de56a9fbd09f1710cb5e0a06e5ace4538be26945 Mon Sep 17 00:00:00 2001 From: Angel Freire Date: Fri, 9 Jan 2026 23:47:46 -0300 Subject: [PATCH 2/2] fix(chart): use boolean flag to avoid type comparison error in revisionHistoryLimit helper The previous implementation compared $result with "" using eq, which fails when $result is assigned an integer value (like 0) because Go templates don't allow comparing different types. Fixed by using a boolean $found flag instead of type-unsafe string comparison. Co-Authored-By: Claude Opus 4.5 --- chart/templates/_helpers.yaml | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/chart/templates/_helpers.yaml b/chart/templates/_helpers.yaml index 60e8fc6140a1b..bfcabd834ee72 100644 --- a/chart/templates/_helpers.yaml +++ b/chart/templates/_helpers.yaml @@ -1183,9 +1183,11 @@ Usage: */}} {{- define "airflow.revisionHistoryLimit" -}} {{- $result := "" -}} + {{- $found := false -}} {{- range . -}} - {{- if and (eq $result "") (not (kindIs "invalid" .)) -}} + {{- if and (not $found) (not (kindIs "invalid" .)) -}} {{- $result = . -}} + {{- $found = true -}} {{- end -}} {{- end -}} {{- $result -}}