GitSync Credentials Secret requires 2 different sets of data fields

[johnstonmatt]

Official Helm Chart version

1.15.0 (latest released)

Apache Airflow version

chart default

Kubernetes Version

1.28

Helm Chart configuration

targetRevision: 1.15.0
destinationNamespace: airflow
repoURL: 'https://airflow.apache.org'
chart: airflow
values:
  executor: LocalKubernetesExecutor
  data:
    metadataSecretName: postgresql-connection-string-secret
  postgresql:
    enabled: false
  dags:
    gitSync:
      enabled: true
      repo: 'https://github.com/johnstonmatt/demo-dag-bag'
      credentialsSecret: airflow-git-credentials
      branch: main
      wait: 40
      subPath: dags
    # less relevant but true
    enableBuiltInSecretEnvVars:
      AIRFLOW__CORE__FERNET_KEY: false
      AIRFLOW__WEBSERVER__SECRET_KEY: false
    fernetKeySecretName: airflow-fernet-key
    webserverSecretKeySecretName: airflow-webserver-key
    config:
      webserver:
        expose_config: 'True'
        instance_name: Polyseam
        enable_proxy_fix: 'True'
        base_url: 'https://airflow.bug-repro.mj.cndi.link'
      operators:
        default_owner: Polyseam
    logs:
      persistence:
        enabled: true
        size: 15Gi
        storageClassName: rwm
    createUserJob:
      useHelmHooks: false
      applyCustomEnv: false
    migrateDatabaseJob:
      applyCustomEnv: false
      useHelmHooks: false
      jobAnnotations:
        "argocd.argoproj.io/hook": Sync      

apiVersion: v1
kind: Secret
metadata:
  name: airflow-git-credentials
  namespace: airflow
stringData:
  GIT_SYNC_USERNAME: 'foousername'
  GIT_SYNC_PASSWORD: 'barpassword'
  # newly required by airflow-triggerer, in addition to the pre-existing keys above
  GITSYNC_USERNAME: 'foousername'
  GITSYNC_PASSWORD: 'barpassword'

Docker Image customizations

No response

What happened

airflow-triggerer needs the gitsync credentials to be formatted as GITSYNC_USERNAME and GITSYNC_PASSWORD, while the rest of the chart seems to require GIT_SYNC_USERNAME and GIT_SYNC_PASSWORD

What you think should happen instead

the airflow helm chart should adhere to only one format rather than requiring both

How to reproduce

If you need to reproduce:

1. Use cndi to install airflow via ArgoCD: cndi create ghusername/my-repo -t airflow
2. after cluster is deployed and working, update from targetRevision: '1.11.0' to targetRevision: '1.15.0'

Anything else

No response

Are you willing to submit PR?

• Yes I am willing to submit a PR!

Code of Conduct

• I agree to follow this project's Code of Conduct

[boring-cyborg]

Thanks for opening your first issue here! Be sure to follow the issue template! If you are willing to raise PR to address this issue please do so, no need to wait for approval.

[romsharon98]

what git-sync version are u using?
the difference between this GIT_SYNC_USERNAME and this GITSYNC_USERNAME is the git-sync version.
this GIT_SYNC_USERNAME is supported by git-sync 3.6 and GITSYNC_USERNAME is supported by git-sync 4
maybe somehow you have different git-sync version on scheduler and triggerer?

[tobimichael96]

We encountered the same problem right now. I think this here is the problem: https://github.com/apache/airflow/blob/d122ebf3a6aae2c96c95a1d63db9a98aa52e4763/chart/templates/_helpers.yaml#L235C37-L235C54

If you specify the secret name, it requires both keys to be there.

[romsharon98]

when upgrading from 1.11 to 1.15 you also upgrade from git-sync 3.6.9 to 4.1.0 therefore you need to add those environment to your secret GITSYNC_USERNAME GITSYNC_PASSWORD.
it also documented here https://github.com/romsharon98/airflow/blob/main/chart/values.yaml#L2738.
also check that the scheduler and the triggerer are having the same git-sync version (maybe on of them didn't upgrade)
Let me know if I missed something.

[tobimichael96]

Yes, but looking how the git_sync_container object is defined in the _helpers.yaml, it does not matter what git-sync version you are using. It requires GITSYNC_* and GIT_SYNC_* being available in the secret.

So what you are describing is what the application side expects, but that is not in sync with the helm-chart code.

In my opinion that requires a change in the helm chart.

[tobimichael96]

I created a PR to provide an example on how to solve this: #43264