-
Notifications
You must be signed in to change notification settings - Fork 5.3k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Git tags not resolving to the right commits #20083
Comments
This breaking change has been introduced between
Broken in I am suspecting this commit to have caused this regression. da6c2e9 |
@PaulSonOfLars here is good article with explanation of differences between lightweight and annotated tags: Here is the same issue which happened before to Argo CD: We also experience this issue right now in our org after Argo CD update. |
@aenshin-pp thanks for the reference! Sadly, I don't think this is to do with lightweight/annotated tags - I did come across the PR you've linked while I was investigating, and double checked that I am definitely using lightweight tags for all purposes here. I can confirm this because if I pin to a tag I opened up #20096 and wrote some tests to confirm my assumptions, and can confirm the issue; argocd seems to be interpreting genuine non-semver git tags as semver constraints, which are then being smudged into using "newer versions". |
@anandf that seems surprising, given that commit doesnt seem to have anything to do with tags or git, and argocd didnt have semver resolution at the time - that was a feature introduced in v12... Hopefully i havent completely misunderstood the issue here. |
@PaulSonOfLars got it. Your issue sounds more complicated than mine :) The issue relates to annotated tags and easy to reproduce which @anandf has confirmed. |
@anandf for me it seems to be the other way round 🤔: v2.11.0-rc3 ( v2.11.0 ( |
Hmm, I noticed that with rc3 that it starts with |
@blakepettersson Thanks for looking into this! Unsure if this was made clear from the above conversation, but just to avoid any confusion, as it seems youre investigating the wrong repro - The repro I've provided in the issue description is only using lightweight tags; adding a new tag starting with the same number will then "bump" to the new commit, while showing the old tag, because tags are being treated as semver constraints rather than an actual tag |
@PaulSonOfLars I'm aware that this issue is different from yours, I just wanted to give this a quick spin. I'll take a look at your issue in a bit - preliminarily my thinking is that your PR should be merged as-is, and then a subsequent PR would try to consolidate the semver resolution between Helm/Git/(and soon OCI). Will confirm my initial thinking in a bit 😄 |
Ok great - thanks for clarifying! Just wanted to make sure :) FWIW - I agree with breaking it down into two steps, and have some code ready for that second PR already. Since I'm already familiar with that corner of the code, I figured I might as well! |
@anandf @aenshin-pp I cc @alexmt |
Describe the bug
Pinning to a git tag uses the wrong commit.
To Reproduce
An example app to reproduce this is available here: https://github.com/PaulSonOfLars/test-argo-app
Note that the repo has two tags:
2024-09-22-test
, against commit4650a611850c03600e423d8385ab237308a8ff05
2024-09-22-test-2
, against commit6650b7ff386c9cb8a2e199ae1fd78571aa3ac336
The app is pinned to
2024-09-22-test
, so SHOULD be using commit4650...
. However, it is not - see below;Expected behavior
ArgoCD should use the commit that the tag is pinned to (
4650...
). However, it is using6650...
instead.Screenshots
See git history below, with commits etc.
Version
v2.12.3+6b9cd82
Logs
Have enabled debug logs, but can't see anything of note.
My initial reaction is that this may be an issue to do #17566; somehow the tags above are being treated as semver, when they shouldn't be. This then leads to the tags being updated to later commits, even though we expect them to be fixed.
This would explain why ArgoCD uses the second tag, rather than HEAD.
Does that sound about right, or am I barking up the wrong tree?
The text was updated successfully, but these errors were encountered: