api go module results in ambiguous import error

bradbl commented 4 years ago

The go modules document describes a procedure for "carving out" modules from a parent module in a multi-module repository. To prevent ambiguous imports at compile-time, the new module should depend upon the parent.

From other closed issues, I understand the Vault team's guidance is to only import the api or sdk go modules. However, transitive dependencies within large repositories make this difficult in practice.

Here is one example of how this can (and is in our case) happening.

Some code in a repository imports an open source project which declares a dependency on github.com/hashicorp/vault itself.
The mono-repository as a whole now has a transitive dependency on github.com/hashicorp/vault.
Future code attempting to import github.com/hashicorp/vault/api hits the "ambiguous import" error at compile time.

jefferai commented 4 years ago

There's nothing for us to do here...if you are seeing this problem, please ask the other project to update their dependencies. We've been exposing go modules in a released version for nearly four months and in master for several months longer.

rski commented 4 years ago

hi, I spent the better part of my day trying to wrap my head around this issue, and this is where I've gotten:

TL;DR please cut off a v2 release at the same code as the latest 1.x release, and use semantic import versioning

Here is the much longer explanation:

We have a repo that imports hashicorp/terraform at the latest release, 0.12.x. This is the line from the go.mod file:

     github.com/hashicorp/terraform v0.12.25

We only import a couple of packages from it. That is all fine. Until today, we didn't actually depend on vault, although terraform does have it in its go mod and at an old version.

To wit,

$ go mod why github.com/hashicorp/vault
# github.com/hashicorp/vault
(main module does not need package github.com/hashicorp/vault)
$ rg "vault " go.sum
725:github.com/hashicorp/vault v0.10.4

This changed today, when we needed to depend on vault/api. As per the usual go module workflow, you add a file with the import, you run a go command that tickles modules into action and your module gets pulled down. But lo and behold!

$ cat mypackage/mypackage.go
package mypackage

import _ "github.com/hashicorp/vault/api"
$ go test ./mypackage/
mypackage/mypackage.go:3:8: ambiguous import: found package github.com/hashicorp/vault/api in multiple modules:
    github.com/hashicorp/vault v0.10.4 (/home/rski/go/pkg/mod/github.com/hashicorp/vault@v0.10.4/api)
    github.com/hashicorp/vault/api v1.0.5-0.20200317185738-82f498082f02 (/home/rski/go/pkg/mod/github.com/hashicorp/vault/api@v1.0.5-0.20200317185738-82f498082f02)

This is where I entered the rabbithole, found the dependency on vault in terraform and kind of pieced these things together.

And that's when I came to this issue to post this, right? Oh, I wish.

With mypackage/ in place, go mod tidy always failed with the ambiguous import error for me. Fine. On a completely new container go1.13.8 container with an empty modcache and fresh copy of the repo, same.

But, on a colleague's laptop? It worked! Black magic. After a bit of poking and tinkering, we found out that he somehow already had vault@1.4something. Go liked that, but still, we don't depend on vault so a recent vault won't end up in our go.mod.

I wanted to see that for myself, so I did a go get github.com/hashicorp/vault to put it in my modcache. Well, that failed. So yeah, it turns out if the go module cache is already in some state, go getting a newer version of vault doesn't work, and fails in itself with an ambiguous import error:

// inside the repo that has mypackage and the terraform imports
go get -v github.com/hashicorp/vault
go: github.com/hashicorp/vault upgrade => v1.4.2
go: downloading github.com/hashicorp/vault v1.4.2
go: downloading github.com/fatih/color v1.9.0
go: downloading github.com/hashicorp/consul-template v0.25.0
go: downloading github.com/hashicorp/consul/api v1.4.0
go: downloading github.com/mattn/go-isatty v0.0.12
go: downloading github.com/hashicorp/vault-plugin-secrets-openldap v0.1.3-0.20200518214608-746aba5fead6
go: downloading github.com/hashicorp/vault-plugin-database-mongodbatlas v0.1.2-0.20200520204052-f840e9d4895c
../../pkg/mod/github.com/hashicorp/vault@v1.4.2/command/operator_init.go:14:2: ambiguous import: found package github.com/hashicorp/consul/api in multiple modules:
    github.com/hashicorp/consul v0.0.0-20171026175957-610f3c86a089 (/home/rski/go/pkg/mod/github.com/hashicorp/consul@v0.0.0-20171026175957-610f3c86a089/api)
    github.com/hashicorp/consul/api v1.4.0 (/home/rski/go/pkg/mod/github.com/hashicorp/consul/api@v1.4.0)

Now, even though that this failed, vault@1.4.2 did make it in my module cache, so go test ./mypackage works.

I see these potential ways out of this:

Wait for terraform to upgrade to a newer vault. Realistically that seems like it's going to be a while. The 0.13 beta was announced less than a week ago. Unfortunately we can't wait for terraform to be fixed.
We can have a shim package that is basically empty apart from a go.mod that pulls a 1.x vault. Then our existing repo can import that with no cost. This would probably force vault1.x to be put in the modcache and fix go test mypackage. I've not tested it so I'm not sure it would work. I will try it later. Problem is, if someone starts using terraform + vault, things will be at an impasse again.
Cut off a 2.x hashicorp/vault at where the 1.x is now and move 1.x development there. Same exact code, moving to it is trivial for users and should very much fix the issue since semantic import versioning will make the imports unambiguous, and go will no longer get confused by vault 0.x.

I would even go as far as calling that last one what should have happened when the vault repo was changed to have multiple modules, since it effectively broke compatibility with the existing 1.x packaging. Of course going back in time is not possible, hindsight is 2020 and all that, but by doing this now, all of this can be fixed.

If you managed to read everything I've written so far, thank you :)

ncabatoff commented 4 years ago

Hi @rski,

Please open an issue on Terraform and ask them to update their dependency. They should be using github.com/hashicorp/vault/sdk rather than github.com/hashicorp/vault. When they added this dependency I don't think sdk existed yet.

rski commented 4 years ago

will do. I still think cutting off a 2.x release from 1.4.x should also be done.

rski commented 4 years ago

I should explain that a bit more; There's going to be things out there using terraform 0.12 for quite a while, including but not limited to our own branches in our repo. Suggesting that everyone in the world updates their vault dependency on all active branches is not realistic. Imo the right solution is a major release. That is exactly the point of semver and how modules work. When an incompatible thing happens, a new major release needs to be cut. Usually that revolves around APIs, in this case it's the distribution mechanism.

ncabatoff commented 4 years ago

No one should be importing github.com/hashicorp/vault. It's not intended to be a dependency, we're using go.mod solely to manage our own dependencies here. I agree it would be desirable to improve the versioning of the modules we do expect to be dependencies (api and sdk), but I'm not sure when or if that will happen. Having them as submodules in a single repo complicates things, or so I'm given to understand, I haven't dug deep into the issue.

rski commented 4 years ago

hhm, I'm not really sure what the uses of the various vault packages are. Sure if vault is not to be depended on, that's fine. I don't think we were trying to do that. However, turning the vault/api module into vault/api/v2 fixes the ambiguous import error immediately, cheaply and for everyone without having to wait for terrraform to do a release. Go will never try to import it from vault 0.4.x (the old mono-module) since vault 0.4.x doesn't have api/v2, just api. Totally different things from the perspective of modules :)

jefferai commented 4 years ago

I should note that this is not limited to Vault. Terraform is also pulling in a much older version of Consul whereas Consul also has api/ and sdk/ packages, and has for a very long time now. This needs to be solved on the Terraform side.

rski commented 4 years ago

that's unfortunate. I did manage to work around this with the shim package. Basically what I did was:

    The mechanics:
    - create a subfolder
    - go mod init in there
    - add a single go file
    - add an `import _ ...vault"`
    - go mod tidy in the subfolder folder
    - remove the vault import so that it is not a source level dependency, only via modules
    - make ROOT/go.mod point to it
    - import it in tools to transiently depend on vault 1.4.2
    - go mod tidy at root

Hopefully this will help the next person. I will unfollow the terraform issue, I have no stakes in it. (tools is a package in our repo as in what https://github.com/golang/go/wiki/Modules#how-can-i-track-tool-dependencies-for-a-module describes)

rski commented 4 years ago

apparently just adding this replace works: github.com/hashicorp/vault => github.com/hashicorp/vault v1.4.2

hashicorp / vault

api go module results in ambiguous import error #7848