kubernetes-sigs / e2e-framework

A Go framework for end-to-end testing of components running in Kubernetes clusters.
Apache License 2.0
517 stars 101 forks source link

imdario/mergo module rename is creating issues with go mod tidy #442

Open jgalliers opened 3 months ago

jgalliers commented 3 months ago

What happened?

The module author of mergo renamed their module as described in their readme from github.com/imdario/mergo to dario.cat/mergo. A similar issue has been filed with kubernetes.

This change breaks transitive dependencies because replace directives do not apply to modules outside the current codebase. For eg, the below is usually what you would do to address this scenario:

replace github.com/imdario/mergo => dario.cat/mergo v1.0.0

What did you expect to happen?

After running go get -u and using the above replace statement, go mod tidy should succeed. Unfortunately this results in the following error:

go: finding module for package dario.cat/mergo
go: found dario.cat/mergo in dario.cat/mergo v1.0.0
go: dario.cat/mergo@v1.0.0 used for two different module paths (dario.cat/mergo and github.com/imdario/mergo)

The result of go mod why shows the following

go mod why github.com/imdario/mergo
# github.com/imdario/mergo
[local module name]/test
[local module name]/test.test
sigs.k8s.io/e2e-framework/pkg/envconf
sigs.k8s.io/e2e-framework/klient
sigs.k8s.io/e2e-framework/klient/conf
k8s.io/client-go/tools/clientcmd
github.com/imdario/mergo

How can we reproduce it (as minimally and precisely as possible)?

Referencing the e2e framework in a new module and then attempting to update/tidy should be sufficient to trigger a warning, if not the error

Anything elese we need to know?

Ideally the replace statement mentioned above could be added to the e2e framework go.mod to ensure upstream consumers are not presented with errors or warnings about conflicting package imports.

E2E Provider Used

kind

e2e-framework Version

v0.4.0

OS version

```console # On Linux: $ cat /etc/os-release NAME="Ubuntu" VERSION="20.04.6 LTS (Focal Fossa)" ID=ubuntu ID_LIKE=debian PRETTY_NAME="Ubuntu 20.04.6 LTS" VERSION_ID="20.04" HOME_URL="https://www.ubuntu.com/" SUPPORT_URL="https://help.ubuntu.com/" BUG_REPORT_URL="https://bugs.launchpad.net/ubuntu/" PRIVACY_POLICY_URL="https://www.ubuntu.com/legal/terms-and-policies/privacy-policy" VERSION_CODENAME=focal UBUNTU_CODENAME=focal $ uname -a Linux AU16820 5.15.153.1-microsoft-standard-WSL2 #1 SMP Fri Mar 29 23:14:13 UTC 2024 x86_64 x86_64 x86_64 GNU/Linux
harshanarayana commented 2 months ago

Thanks for reporting this @jgalliers

@vladimirvivien @cpanato I see there is some work happening on the client-go for the clientcmd that is intended to remove this package entirely. Until then, shall we add a replace directive to help avoid this complication for others ?

I did some quick check to see if we can get rid of the cliendcmd use entirely, but that was not a small one and had to re-do a bunch of what is being done by the clientcmd. Not sure if that is worth it for short duration fix.

vladimirvivien commented 2 months ago

@harshanarayana I think for now, let's do a package replace to keep it simple. Once clientcmd is completley removed, then we can revisit (not sure how soon that will be).