Helm chart: Git-sync v4 incorrect mix of branch, ref, rev
- Dominant language
- Python
- Stars
- 46.9k
- Forks
- 17.8k
- Avg merge
- 2d 10h
- Merged PRs (30d)
- 483
Description
### Official Helm Chart version
1.15.0 (latest released)
### Apache Airflow version
2.5.3
### Kubernetes Version
1.30.4-gke.1348000
### Helm Chart configuration
```yaml
dags:
gitSync:
branch: main
rev: HEAD
ref: v1.2.3
repo: git@github.com:/.git
depth: 1
enabled: true
sshKeySecret:
subPath: src//
knownHosts: |
github.com ssh-rsa
```
### Docker Image customizations
_No response_
### What happened
I pushed code to the `main` branch, and this was picked up by the `gitSync` sidecar and it updated the Airflow Dags to use the new code
### What you think should happen instead
It should not have updated the code, because the `ref` is "frozen" to version `v1.2.3`. This is a production environment, and that's the whole reason I froze it by pointing it at a Git tag - so that pushing new code won't automatically deploy until it is reviewed/tagged/released.
### How to reproduce
1. Have your git repo have a branch named `main`, and a tag named `v1.2.3` that points to `main`.
2. Set this in your `values.yaml`
```yaml
images:
gitSync:
repository: registry.k8s.io/git-sync/git-sync
tag: v4.1.0
dags:
gitSync:
branch: main
rev: HEAD
ref: v1.2.3
repo: git@github.com:/.git
enabled: true
```
3. Deploy this
4. Make a new commit to the `main` branch, and push it
5. You will see that the gitSync container updates the code with the new commit, even though it is supposed to be "frozen" to `v1.2.3`
### Anything else
I dug into this, and it's caused by incorrectly mapping 3 parameters to the gitSync container. The latest Helm chart (v1.15) by default uses this image
https://github.com/apache/airflow/blob/ff7463b1624d91fb6478ec367bbf84b88fb9b83d/chart/values.yaml#L124-L127
which I believe is from this source code here https://github.com/kubernetes/git-sync/tree/v4.1.0.
The 3 incorrect params are:
- branch
- rev
- ref
In v4.x of gitSync, the params `branch` and `rev` are deprecated, and only `ref` should be used. But if any of the deprecated params are specified, they override `ref`, which was the root cause of my issue.
These are explicitly marked as deprecated (from v3 to v4) in the migration guide [here](https://github.com/kubernetes/git-sync/blob/v4.1.0/v3-to-v4.md#sync-target---branch-and---rev-----ref), and inspecting the source code you can see they are:
- https://github.com/kubernetes/git-sync/blob/97c0d585a83057184821549b30945f2105cc8966/main.go#L306-L309
- https://github.com/kubernetes/git-sync/blob/97c0d585a83057184821549b30945f2105cc8966/main.go#L327-L329
- https://github.com/kubernetes/git-sync/blob/97c0d585a83057184821549b30945f2105cc8966/main.go#L421-L432
I fixed it using a hack, by setting `branch` and `rev` to empty strings, but this is not ideal.
### Are you willing to submit PR?
- [X] Yes I am willing to submit a PR!
### Code of Conduct
- [X] I agree to follow this project's [Code of Conduct](https://github.com/apache/airflow/blob/main/CODE_OF_CONDUCT.md)
Contributor guide
Research direction
Start with chart/values.yaml at the git-sync image configuration, then compare the chart's branch, rev, and ref mappings with the linked git-sync v4 migration guide and main.go entries. Reproduce the values.yaml setup using a main branch and v1.2.3 tag; done means changes pushed to main no longer update the deployment when ref remains v1.2.3.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- git, helm, kubernetes
- Domain
- devops, infrastructure
- Issue type
- Bug
- Difficulty
- 3/5
- Estimated time
- 1-2 days
- Activity status
- Active
- Clarity
- Clearly specified
- Newbie friendliness
- 68/100