Miretpl commented on code in PR #63276:
URL: https://github.com/apache/airflow/pull/63276#discussion_r2914240259
##########
chart/templates/scheduler/scheduler-deployment.yaml:
##########
@@ -344,6 +344,9 @@ spec:
{{- if or .Values.dags.gitSync.sshKeySecret
.Values.dags.gitSync.sshKey}}
{{- include "git_sync_ssh_key_volume" . | indent 8 }}
{{- end }}
+ {{- if .Values.dags.gitSync.credentialsSecret }}
Review Comment:
```suggestion
{{- if and $localOrDagProcessorDisabled .Values.dags.gitSync.enabled
.Values.dags.gitSync.credentialsSecret }}
```
following git sync container falgs for scheduler
##########
chart/values.schema.json:
##########
@@ -1076,7 +1076,7 @@
"tag": {
"description": "The gitSync image tag.",
"type": "string",
- "default": "v4.4.2"
+ "default": "v4.6.0"
Review Comment:
I believe this should be in a separate PR as it is unrelated change for this
one.
##########
chart/values.schema.json:
##########
@@ -10801,7 +10801,7 @@
}
},
"credentialsSecret": {
- "description": "Name of a Secret containing the
repo `GIT_SYNC_USERNAME` and `GIT_SYNC_PASSWORD`.",
+ "description": "Name of a Secret containing
`GIT_SYNC_USERNAME`, `GITSYNC_USERNAME`, `GIT_SYNC_PASSWORD`, and
`GITSYNC_PASSWORD` keys. The password keys are mounted as files and used via
`*_PASSWORD_FILE` env vars so rotated credentials can be re-read.",
Review Comment:
You probably need to create a new field dedicated for the file as this
breaks backward-compatibility.
##########
chart/templates/workers/worker-deployment.yaml:
##########
@@ -482,6 +482,9 @@ spec:
{{- if or .Values.dags.gitSync.sshKeySecret
.Values.dags.gitSync.sshKey}}
{{- include "git_sync_ssh_key_volume" . | indent 8 }}
{{- end }}
+ {{- if .Values.dags.gitSync.credentialsSecret }}
Review Comment:
```suggestion
{{- if and .Values.dags.gitSync.enabled (not
.Values.dags.persistence.enabled) }}
```
following git sync container falgs for workers
##########
chart/templates/_helpers.yaml:
##########
@@ -247,16 +255,10 @@ If release name contains chart name it will be used as a
full name.
secretKeyRef:
name: {{ .Values.dags.gitSync.credentialsSecret | quote }}
key: GITSYNC_USERNAME
- - name: GIT_SYNC_PASSWORD
- valueFrom:
- secretKeyRef:
- name: {{ .Values.dags.gitSync.credentialsSecret | quote }}
- key: GIT_SYNC_PASSWORD
- - name: GITSYNC_PASSWORD
- valueFrom:
- secretKeyRef:
- name: {{ .Values.dags.gitSync.credentialsSecret | quote }}
- key: GITSYNC_PASSWORD
+ - name: GIT_SYNC_PASSWORD_FILE
+ value: "/etc/git-secret/credentials/GIT_SYNC_PASSWORD"
+ - name: GITSYNC_PASSWORD_FILE
+ value: "/etc/git-secret/credentials/GITSYNC_PASSWORD"
Review Comment:
This change breaks backward-compatibility. You could prevent that by making
if condition on these envs where, by default, old behaviour will be in place.
##########
chart/templates/triggerer/triggerer-deployment.yaml:
##########
@@ -300,6 +300,9 @@ spec:
{{- if or .Values.dags.gitSync.sshKeySecret
.Values.dags.gitSync.sshKey}}
{{- include "git_sync_ssh_key_volume" . | nindent 8 }}
{{- end }}
+ {{- if .Values.dags.gitSync.credentialsSecret }}
Review Comment:
```suggestion
{{- if and .Values.dags.gitSync.enabled (not
.Values.dags.persistence.enabled) }}
```
following git sync container falgs for triggerer
##########
chart/templates/_helpers.yaml:
##########
@@ -204,6 +204,14 @@ If release name contains chart name it will be used as a
full name.
defaultMode: 288
{{- end }}
+{{/* Git credentials volume */}}
Review Comment:
```suggestion
{{/* Git credentials volume */}}
```
--
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]
