🌱 (chore): Enable wrapcheck linter

[kersten]

🌱 Enable wrapcheck linter

This change improves code quality by enforcing two new linting rule:

• wrapcheck ensures wrapped errors are properly handled and not silently ignored.

The motivation is to promote modern Go practices and ensure consistent error wrapping and checking across the codebase.

[k8s-ci-robot]

Hi @kersten. Thanks for your PR.

I'm waiting for a kubernetes-sigs member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository.

[camilamacedo86]

Hi @kersten

Thanks a lot for this PR — enabling linters like depguard and wrapcheck is a great step toward improving overall code quality and consistency 🙌

That said, regarding depguard specifically for github.com/pkg/errors:
This package is currently brought in indirectly by several upstream dependencies, including controller-runtime, client-go, and other Kubernetes-related libraries, as shown by:

go mod graph | grep github.com/pkg/errors
sigs.k8s.io/kubebuilder/testdata/project-v4 github.com/pkg/[email protected]
github.com/cert-manager/[email protected] github.com/pkg/[email protected]
gomodules.xyz/jsonpatch/[email protected] github.com/pkg/[email protected]
k8s.io/[email protected] github.com/pkg/[email protected]
k8s.io/[email protected] github.com/pkg/[email protected]
k8s.io/[email protected] github.com/pkg/[email protected]
k8s.io/[email protected] github.com/pkg/[email protected]
k8s.io/[email protected] github.com/pkg/[email protected]
sigs.k8s.io/[email protected] github.com/pkg/[email protected]
sigs.k8s.io/[email protected] github.com/pkg/[email protected]

Because of that, adding depguard with a deny rule for github.com/pkg/errors might not be very effective right now — it could lead to confusion or unnecessary friction, especially since we're not using it directly in our own codebase.

It might make more sense to enforce this rule only once those upstream dependencies have fully transitioned to the standard errors package, if that’s indeed their intended direction. (If you believe that’s the case, I’d recommend opening issues in the relevant upstream projects that are still using it.)

The addition of wrapcheck, however, shows make sense.

Thanks again for driving quality improvements like this! 💚

[kersten]

@camilamacedo86 Sounds good, I’ll remove the deny rule.

If we decide to go with wrapcheck, it would be great to keep an eye on PRs that introduce wrapped errors - just to make sure they don’t unintentionally use the previously mentioned package. It’s easy for someone to copy-paste code quickly, and not everyone is familiar with fmt.Errorf.

When they see linter messages like “please wrap the error,” they might end up googling and using something like errors.Wrap(err, "message") by mistake :)

[k8s-ci-robot]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: camilamacedo86, kersten

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

• OWNERS [camilamacedo86]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

[camilamacedo86]

Hi @kersten 👋

If we enable these linters, we'll need to update the codebase to ensure everything passes — see the failing job here:
https://github.com/kubernetes-sigs/kubebuilder/actions/runs/14177346899/job/39715844116?pr=4721

Also, it's worth noting that many of the failures appear to be false positives, or at least cases where the current code seems intentional and correct.

While linters are great tools for maintaining consistency and catching real issues, we also need to weigh their practical impact.
If a linter generates too many non-actionable or misleading errors, it can become more of a hurdle than a help — making it harder to contribute without adding real value.

So before moving forward, I think it’s worth asking:
Do we really want this linter, with this configuration?

Thanks again for looking into ways to improve the project! 🙏

[kersten]

Do we really want this linter, with this configuration?

Fair enough, I firstly removed depguard anyways because of much noise. But as I understood, wrapcheck is ok for you? I think it would make sense to enable this one, to have a better control of errors, and where they come from.

based on the comment above

[camilamacedo86]

Hi @kersten

Fair enough, I firstly removed depguard anyways because of much noise. But as I understood, wrapcheck is ok for you? I think it would make sense to enable this one, to have a better control of errors, and where they come from.

That is fine.
If we pass the CI check with this one, I am OK with getting this merged.
No problem at all.