argoproj
chore(repo-server): unify semver resolution in new versions subpackage
Unify semver resolution across helm/OCI/git into a single package, to ensure we don't end up with mismatched logic for different services
Follow up to #20096, as requested here. cc @blakepettersson
Checklist:
- [ ] Either (a) I've created an enhancement proposal and discussed it with the community, (b) this is a bug fix, or (c) this does not need to be in the release notes.
- [ ] The title of the PR states what changed and the related issues number (used for the release note).
- [ ] The title of the PR conforms to the Toolchain Guide
- [ ] I've included "Closes [ISSUE #]" or "Fixes [ISSUE #]" in the description to automatically close the associated issue.
- [ ] I've updated both the CLI and UI to expose my feature, or I plan to submit a second PR with them.
- [ ] Does this PR require documentation updates?
- [ ] I've updated documentation as required by this PR.
- [ ] I have signed off all my commits as required by DCO
- [ ] I have written unit and/or e2e tests for my change. PRs without these are unlikely to be merged.
- [ ] My build is green (troubleshooting builds).
- [ ] My new feature complies with the feature status guidelines.
- [ ] I have added a brief description of why this PR is necessary and/or what this PR solves.
- [ ] Optional. My organization is added to USERS.md.
- [ ] Optional. For bug fixes, I've indicated what older releases this fix should be cherry-picked into (this may or may not happen depending on risk/complexity).
:exclamation: Preview Environment undeploy from Bunnyshell failed
See: Environment Details | Pipeline Logs
Available commands (reply to this comment):
- :rocket:
/bns:deployto redeploy the environment - :x:
/bns:deleteto try again to remove the environment
![bunnyshell[bot] avatar](https://avatars.githubusercontent.com/in/338154?v=4 "Published by a pub.dev verified publisher"){kind=small}
:x: Preview Environment deleted from Bunnyshell
Available commands (reply to this comment):
- :rocket:
/bns:deployto deploy the environment
@blakepettersson thanks for the review! I've reverted things back to entries and added in a private struct for the tags methods. Also fixed some tests which were failing due to new error messages. Let me know if you'd like me to poke at anything else!
Codecov Report
:x: Patch coverage is 83.33333% with 14 lines in your changes missing coverage. Please review.
:white_check_mark: Project coverage is 59.95%. Comparing base (92589de) to head (dbc5c35).
:warning: Report is 480 commits behind head on master.
Additional details and impacted files
@@ Coverage Diff @@
## master #20216 +/- ##
==========================================
+ Coverage 59.91% 59.95% +0.03%
==========================================
Files 344 344
Lines 57715 57721 +6
==========================================
+ Hits 34582 34605 +23
+ Misses 20375 20344 -31
- Partials 2758 2772 +14
:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.
![codecov[bot] avatar](https://avatars.githubusercontent.com/in/254?v=4 "Published by a pub.dev verified publisher"){kind=small}
Hey!
Is there any update on this? We have a pending bug that we would like fixed, but we are waiting on this PR :smile:
https://github.com/argoproj/argo-cd/pull/20862
Hey, how it is going? I really appreciate your work because i really need this feature. Hope you don't forgot about it.
Have merged in latest master to simplify merging, as well as updated the PR to fix the issues raised in #21818.
I believe this fix belongs here rather than in a separate PR, as while this error was raised against a Helm chart, it will also happen in git repos, making this a great time to unify the two and avoid the duplicate code.
Hello guys. Could you please share the possible time when this PR can be merged?
We depend on the fix (which is in this PR) for this issue - https://github.com/argoproj/argo-cd/issues/21818 At the moment we are blocked. We cannot upgrade ArgoCD to 2.14 because of the issue, at the same time we cannot upgrade k8s to 1.31 because ArgoCD 2.13 is not supported in k8s 1.31. https://argo-cd.readthedocs.io/en/stable/operator-manual/installation/#tested-versions
Appreciate for your help. Thanks
Not sure what the context of the http: server gave HTTP response to HTTPS client error might be; I'm guessing it's because I've removed the short-circuit in the helm version selection, in favour of checking the available tags, rather than blindly returning a version we don't know exists.
Let me know if theres something to be done to fix the test, or if you'd like me to undo add back that shortcircuit.
@PaulSonOfLars I would say let's re-add the short circuit for now and we can look into removing it at some other time.
@PaulSonOfLars I would say let's re-add the short circuit for now and we can look into removing it at some other time.
@blakepettersson Sure. I've left a note explaining context for future users. MIght be of interest to before v3 comes out.
Not sure what the test failure is about now, looks like some codegen for a piece I haven't touched?
@crenshaw-dev @blakepettersson friendly nudge on the above.
This will help unify semver resolution across all providers (both git and helm), creating a more consistent and predictable approach, as well as resolve the issue #21818. In case the concern is breaking changes, I would hope that the push to v3 would be a good excuse to get this merged in and tested. What do you think?
@PaulSonOfLars I'm all for it, I just don't have enough powers to merge it. Will ping some others to see if they can.
Hi,
we've updated to 2.14 and have now hit this issue :-( We now have multiple kubernetes clusters with some apps not being installed with errors like " invalid revision '33.03.138'".... which is a REALLY big issue.
Please merge this and do a new release ASAP.
@crenshaw-dev could you please help in merging this? We're are really stuck with errors like:
Failed to load target state: failed to generate manifest for source 1 of 1: rpc error: code = Unknown desc = invalid revision '33.03.138': constraint Parser Error
Thanks
I have also just come across this issue, and i can't deploy an important helm chart into argo because of it.
Failed to load target state: failed to generate manifest for source 2 of 2: rpc error: code =
Unknown desc = invalid revision '4.0-*': improper constraint: 4.0.002
@PaulSonOfLars there's a failing unit test, otherwise we're good to go
Thank you @agaudreault! <3 Got it @blakepettersson - fixed the test, should be good to go now.
Here's to hoping you can reenable the automerge! Edit: Aha, didn't refresh. Can see that you could :)
@blakepettersson Would it be possible to have this backported to 3.0? We currently cannot upgrade because we can't determine the impact to our customers, we would have to check that all their charts are compliant with the strict semantic versioning from before the fixes in this PR.
@rouke-broersma this is a chore, not a fix. Is this fixing a problem in 3.0?
