Skip to content

Conversation

@thevilledev
Copy link
Contributor

@thevilledev thevilledev commented Jun 6, 2025

Manual cherry-pick #23287 for release-2.14 branch.

Updates Go version across all workflows, Dockerfiles and go.mod.

See:

Also includes #23301 due to circular dependency. The fix requires Go 1.24 and this branch requires the fix to pass Go tests.

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).

@thevilledev thevilledev requested review from a team as code owners June 6, 2025 03:51
@thevilledev
Copy link
Contributor Author

Interesting test failure: https://github.com/argoproj/argo-cd/actions/runs/15482413210/job/43590920713?pr=23294

As for linting, golangci-lint needs to be bumped to v2 and lint rules need to be updated. I updated just the linter and migrated the ruleset and it yields this:

607 issues:
* staticcheck: 491
* testifylint: 116

@thevilledev
Copy link
Contributor Author

The failing TLS test actually surfaces a real issue. It affects all release branches and should be fixed. The reason why this does not happen on master or release-3.0 branches is that there's drift between the tests in tls_test.go.

I'll remove the fix commit from this branch and issue separate PRs for affected branches instead. Tests will continue to fail on this PR until rebased with the fix.

@codecov
Copy link

codecov bot commented Jun 6, 2025

Codecov Report

❌ Patch coverage is 80.00000% with 3 lines in your changes missing coverage. Please review.
✅ Project coverage is 55.32%. Comparing base (5f89062) to head (5a43b1b).
⚠️ Report is 16 commits behind head on release-2.14.

Files with missing lines Patch % Lines
util/tls/tls.go 80.00% 2 Missing and 1 partial ⚠️
Additional details and impacted files
@@               Coverage Diff                @@
##           release-2.14   #23294      +/-   ##
================================================
- Coverage         55.36%   55.32%   -0.04%     
================================================
  Files               339      339              
  Lines             57361    57374      +13     
================================================
- Hits              31756    31745      -11     
- Misses            22915    22936      +21     
- Partials           2690     2693       +3     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@blakepettersson
Copy link
Member

@thevilledev could we do separate cherry-picks for golangci-lint and the Golang upgrade?

@thevilledev
Copy link
Contributor Author

@blakepettersson I think I hit a wall with not having a Go 1.24 compatible golangci-lint version of the v1 series. I can check again.

Another option I guess would be to upgrade the linter and loosen the lint rules temporarily in this PR.

@blakepettersson
Copy link
Member

@thevilledev from what I can see, golangci-lint is backwards compatible with versions older than what it has been built with, so we could do a PR with a new version of golangci-lint + 1.23 and then we just update this branch when that has been merged.

@thevilledev
Copy link
Contributor Author

Yup lets do that! I'll mark this as a draft in the meanwhile

Updates Go version across all workflows, Dockerfiles and go.mod

Signed-off-by: Ville Vesilehto <[email protected]>
@thevilledev thevilledev force-pushed the chore/release-2.14-go-1.24.4 branch from 8bf296a to c010e4c Compare June 6, 2025 20:58
@blakepettersson blakepettersson marked this pull request as ready for review June 6, 2025 21:04
@thevilledev
Copy link
Contributor Author

Integration tests fail because #23301 is not merged in yet, but it also depends on Go 1.24. We could merge this in and rebase it.

@thevilledev
Copy link
Contributor Author

Merged the fix PR here due to circular dependency. Ready to merge I think!

@blakepettersson
Copy link
Member

blakepettersson commented Jun 7, 2025

I see this (minor) thing in the failed action, I think that is an artifact from the previous lint PR

2025/06/07 05:33:18 internal error: package "io" without types was imported from "github.com/argoproj/argo-cd/v2/util/io"

@thevilledev
Copy link
Contributor Author

master seems to import the util/io package through alias, like:

utilio "github.com/argoproj/argo-cd/v3/util/io"

Seems like we need to do it here too?

@thevilledev
Copy link
Contributor Author

I'll open a separate PR to fix this 👍

@thevilledev
Copy link
Contributor Author

@blakepettersson any thoughts on upgrading mockery to the same version as the other branches? I think there's something fundamentally broken, as I get a ton of other similar errors if I keep commenting out mocks from .mockery.yaml:

2025/06/08 00:32:13 internal error: package "bytes" without types was imported from "github.com/argoproj/argo-cd/v2/util/helm"
...
2025/06/08 00:32:27 internal error: package "crypto/tls" without types was imported from "github.com/argoproj/argo-cd/v2/util/git"
...
2025/06/08 00:32:55 internal error: package "context" without types was imported from "github.com/argoproj/argo-cd/v2/pkg/apiclient/cluster"

I upgraded it to v3.2.5 locally and while the diff is significant, it's mostly because of variable name changes on the generated code (like rf -> returnFunc). I can open a PR with the upgrade if that's ok to upgrade.

thevilledev added a commit to thevilledev/argo-cd that referenced this pull request Jun 8, 2025
Upgrade mockery to latest v2 version to fix issues
with Go 1.24 argoproj#23294 like:

internal error: package "io" without types
was imported from "github.com/argoproj/argo-cd/v2/util/io"

Signed-off-by: Ville Vesilehto <[email protected]>
@thevilledev thevilledev force-pushed the chore/release-2.14-go-1.24.4 branch from a3cd12d to 149c0e8 Compare June 8, 2025 04:48
@thevilledev
Copy link
Contributor Author

I realised upgrading mockery just within the v2 series to the latest one (v2.53.4) fixes the issue with Go 1.24. I pushed it briefly to this branch just to check that the codegen tests pass. Now in #23316 so we can merge it separately.

@blakepettersson blakepettersson merged commit 3c68b26 into argoproj:release-2.14 Jun 8, 2025
32 of 36 checks passed
@blakepettersson
Copy link
Member

Thanks a lot @thevilledev!

@thevilledev
Copy link
Contributor Author

Any time - and thanks for the reviews!

@thevilledev thevilledev deleted the chore/release-2.14-go-1.24.4 branch June 9, 2025 05:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants