XD-DENG commented on code in PR #62178:
URL: https://github.com/apache/airflow/pull/62178#discussion_r2893087922
##########
chart/templates/dag-processor/dag-processor-deployment.yaml:
##########
@@ -80,6 +80,9 @@ spec:
checksum/airflow-config: {{ include (print $.Template.BasePath
"/configmaps/configmap.yaml") . | sha256sum }}
checksum/extra-configmaps: {{ include (print $.Template.BasePath
"/configmaps/extra-configmaps.yaml") . | sha256sum }}
checksum/extra-secrets: {{ include (print $.Template.BasePath
"/secrets/extra-secrets.yaml") . | sha256sum }}
+ {{- if and (semverCompare ">=3.0.0" .Values.airflowVersion) (not
.Values.jwtSecretName) }}
Review Comment:
The `jwt-secret.yaml` template has a three-part guard (lines 23-24):
```
{{- if semverCompare ">=3.0.0" .Values.airflowVersion }}
{{- if and .Values.apiServer.enabled (not .Values.jwtSecretName) }}
```
The Secret is only created when all three conditions are true:
1. airflowVersion >= 3.0.0
2. .Values.apiServer.enabled == true
3. No custom .Values.jwtSecretName is provided
With the PR's condition, if a user sets `apiServer.enabled: false`:
- The `jwt-secret.yaml` template renders as empty (no Secret resource is
produced)
- But the PR's condition `(semverCompare ">=3.0.0")` AND `(not
.Values.jwtSecretName)` still evaluates to `true`
- So `{{ include ... "/secrets/jwt-secret.yaml" . | sha256sum }}` will
compute a checksum of the empty/comment-only output
- The annotation gets added to the pod spec pointing at a Secret that
doesn't exist
This won't cause a runtime error (it's just a pod annotation, not a volume
mount), but it's still wrong in two ways:
1. Unnecessary pod restarts: Any unrelated Helm value change could subtly
alter the rendered empty template (e.g., whitespace), changing the checksum and
triggering a spurious rolling restart.
2. Semantic confusion: The annotation implies the pod depends on a
jwt-secret that isn't deployed.
The scheduler template already handles this correctly. The fix can simple
— add `.Values.apiServer.enabled` to match:
` {{- if and (semverCompare ">=3.0.0" .Values.airflowVersion)
.Values.apiServer.enabled (not .Values.jwtSecretName) }}`
Please let me know if this is making sense to you. Thanks
--
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]
