buildsys: extend external-files to vendor go modules

[jpmcb]

Issue number:

Closes #2052
Closes #2053

Description of changes:

buildsys: extend external-files to vendor go modules

Using the `bundle-*` keys on
`packag.metadata.build-package.external-files`, buildsys can vendor Go
dependencies into a new archive (leaving the prestine upstream archive
intact). Both archives may then be used to build the package in an
isolated environment.

toosl/docker-go now also retrieves the `GOSUMDB` environment variable
from the host.

Co-authored-by: John McBride <jpmmcb@amazon.com>
Co-authored-by: Samuel Karp <skarp@amazon.com>
Signed-off-by: John McBride <jpmmcb@amazon.com>

---

hotdog: use new buildsys go module support

Co-authored-by: John McBride <jpmmcb@amazon.com>
Co-authored-by: Samuel Karp <skarp@amazon.com>
Signed-off-by: John McBride <jpmmcb@amazon.com>

---

oci-add-hooks: use new buildsys go module support

Co-authored-by: John McBride <jpmmcb@amazon.com>
Co-authored-by: Samuel Karp <skarp@amazon.com>
Signed-off-by: John McBride <jpmmcb@amazon.com>

This patch carries many of the changes from #2053 but adds recommendations from subsequent code reviews.

The external-files interface has been extended to now take several optional `bundle-* modifiers that enable it to consume an upstream archive, unpack it, vendor it's go module dependencies, and create a new archive with those vendored go dependencies.

Further, this enables a possible future paradigm of vendoring many modules from a single external-files. With this PR, only "go" is supported in bundled-modules.

Testing done:

Builds 👍🏼

cargo make -e PACKAGE=oci-add-hooks build-package
cargo make -e PACKAGE=hotdog build-package

and launching an ecs instance looks 👍🏼

cargo make -e BUILDSYS_VARIANT=aws-ecs-1

Terms of contribution:

By submitting this pull request, I agree that this contribution is dual-licensed under the terms of both the Apache License, version 2.0, and the MIT license.

[jpmcb]

Whoops! A few files need rustfmt - please hold!

Force pushed to address failing GitHub actions needing rustfmt-ing

[jpmcb]

Force pushed to address cargo make check-lints and cargo make check-fmt

[jpmcb]

Force pushed to address not having to unwrap here:

- dg_args
-         .go_mod_cache 
-         .to_str()
-         .context(error::InputFileSnafu)
-         .unwrap()
+ dg_args
+        .go_mod_cache
+        .to_str()
+        .context(error::InputFileSnafu)?

[stmcginnis]

Since it looks like you will need to make other updates, added a couple small nits to the list.

[jpmcb]

Force pushed to address nits / spelling mistakes ✏️

[stmcginnis]

I've found some issues with using go mod vendor for pulling in dependencies for projects that do not using vendoring for dependency management. Not sure if there's a better way for us to handle it than current plans, but thought I should add my findings here so others are aware and we can possibly think about a better approach.

Vendoring approach

I found these issues as I'm trying to build the ecr-credential-provider binary out of /~https://github.com/kubernetes/cloud-provider-aws. If I try to build it isolated (airgapped, offline, etc), I first run go mod vendor to create and populate a vendor directory. But then when I try to build the project, it appears the vendor directory is too deep or something, so I get this error:

$ make ecr-credential-provider
GO111MODULE=on CGO_ENABLED=0 GOOS=linux GOARCH=amd64 GOPROXY=direct go build \
	-trimpath \
	-ldflags="-w -s -X k8s.io/component-base/version.gitVersion=v1.22.3-dirty" \
	-o=ecr-credential-provider \
	cmd/ecr-credential-provider/*.go
make: /bin/bash: Argument list too long
make: *** [Makefile:38: ecr-credential-provider] Error 127

go mod download Approach

This led me to explore a more module-centric solution to capturing all deps to be able to build offline.

I first tried to capture dependencies into a separate directory so I could use that to copy into the offline build.

GOCACHE="$(mktemp -d)"
GOPATH="$(mktemp -d)"

GOPATH="${GOPATH}" go mod download
cp -rp "${GOPATH}"/pkg/mod/cache/download/* "${GOCACHE}/"

docker run --rm --network none \
    -v "${GOCACHE}":/gocache \
    -v ${PWD}:/build -w /build \
    -e GOCACHE=/cache \
    -e GOPROXY=file:///gocache golang:1.18 \
    make ecr-credential-provider

This had a couple issues. For some reason, more often than not the go mod download would exit with failure being unable to find one of the dependencies:

go: github.com/matttproud/golang_protobuf_extensions@v1.0.2-0.20181231171920-c182affec369 requires
	golang.org/x/sync@v0.0.0-20181221193216-37e7f081c4d4: invalid version: unknown revision 37e7f081c4d4

If I reran the command a few times, eventually it would succeed and get everything downloaded.

But the second issue then was that go mod download doesn't go deep. The downloaded dependencies wouldn't actually have everything needed to build the project, so running in an offline environment would error out stating there were things missing:

...
cmd/ecr-credential-provider/main.go:32:2: github.com/aws/aws-sdk-go@v1.43.32: reading file:///gocache/github.com/aws/aws-sdk-go/@v/v1.43.32.zip: no such file or directory
cmd/ecr-credential-provider/main.go:34:2: k8s.io/apimachinery@v0.22.8: reading file:///gocache/k8s.io/apimachinery/@v/v0.22.8.zip: no such file or directory
cmd/ecr-credential-provider/plugin.go:28:2: k8s.io/apimachinery@v0.22.8: reading file:///gocache/k8s.io/apimachinery/@v/v0.22.8.zip: no such file or directory
...

Double Build Approach

I was able to get everything downloaded by first building the project while still connected:

GOPATH="${GOPATH}" GOPROXY="file:///${GOCACHE}" make ecr-credential-provider

then building again isolated:

rm ecr-credential-provider

docker run --rm --network none \
    -v "${GOCACHE}":/gocache \
    -v ${PWD}:/build -w /build \
    -e GOCACHE=/cache \
    -e GOPROXY=file:///gocache golang:1.18 \
    make ecr-credential-provider

It is a small project that doesn't take long to build, but that seems a little redundant. If we did end up having this codified in the way buildsys handles go projects, this definitely would not be a good thing to do for all go dependencies.

"Thorough" Approach

What ended up working the best was to "force" go mod download into actually following all dependencies. This is what I ended up with that would consistently work:

GOCACHE="$(mktemp -d)"
GOPATH="$(mktemp -d)"

go mod graph | \
    awk '{print $2}' | \
    sort -u | \
    GOPATH="${GOPATH}" xargs go mod download
cp -rp "${GOPATH}"/pkg/mod/cache/download/* "${GOCACHE}/"

docker run --rm --network none \
    -v "${GOCACHE}":/gocache \
    -v ${PWD}:/build -w /build \
    -e GOCACHE=/cache \
    -e GOPROXY=file:///gocache golang:1.18 \
    make ecr-credential-provider

The downside is the go mod graph call does take some time since it is actually thorough. I think we could mitigate that by having a local (non-isolated) GOCACHE that buildsys could use while downloading the dependencies. Then we shouldn't need to download the same packages repeatedly.

[jpmcb]

Added another commit to address latest review comments:

Now, when buildsys vendors the go dependencies, it places them into a separate archive. The pristine upstream archive can then be used in conjunction with the dependency archive to create the package.

Adds sane defaults for several of the bundling options.

---

I will plan to squash this into a co-authored commit upon approval, but this was a bit more of a change that seemed to warrant it's own intermittent commit

[jpmcb]

@stmcginnis - I think something is wrong with the cloud-provider-aws v1.22.3 tag and they have an out of date go.mod file.

---

I am able to pull down the tar with

❯ wget /~https://github.com/kubernetes/cloud-provider-aws/archive/refs/tags/v1.22.3.tar.gz

and unpack it with:

❯ tar -zxvf v1.22.3.tar.gz

Then, going into that directory, I can vendor the code. But when I attempt to manually run a go build I get the following:

cd cloud-provider-aws-1.22.3
go mod vendor
GO111MODULE=on CGO_ENABLED=0 GOOS=linux GOARCH=arm64 GOPROXY=https://proxy.golang.org,direct go build -x \
        -trimpath \
        -ldflags="-w -s -X k8s.io/component-base/version.gitVersion=" \
        -o=ecr-credential-provider \
        cmd/ecr-credential-provider/*.go
WORK=/tmp/go-build4216558415
go: updates to go.mod needed, disabled by -mod=vendor
        (Go version in go.mod is at least 1.14 and vendor directory exists.)
        to update it:
        go mod tidy

which is super weird. It's asking me to run a go mod tidy? I'd expect that to "just work" and there not need to be an update to the module dependencies. Switching over to the actual repository, I checked out their v1.22.3 tag and see the same problem with the command above.

So, running go mod tidy on their git repo does make it dirty and show the dependencies are out of date:

diff --git a/go.mod b/go.mod
index 27a8a4a..b19147b 100644
--- a/go.mod
+++ b/go.mod
@@ -6,6 +6,7 @@ require (
        github.com/aws/aws-sdk-go v1.43.32
        github.com/golang/mock v1.4.4
        github.com/google/go-cmp v0.5.5
+       github.com/spf13/pflag v1.0.5
        github.com/stretchr/testify v1.7.0
        gopkg.in/gcfg.v1 v1.2.0
        k8s.io/api v0.22.8
@@ -66,7 +67,6 @@ require (
        github.com/prometheus/common v0.26.0 // indirect
        github.com/prometheus/procfs v0.6.0 // indirect
        github.com/spf13/cobra v1.1.3 // indirect
-       github.com/spf13/pflag v1.0.5 // indirect
        github.com/stretchr/objx v0.2.0 // indirect
        go.etcd.io/etcd/api/v3 v3.5.0 // indirect
        go.etcd.io/etcd/client/pkg/v3 v3.5.0 // indirect
@@ -106,7 +106,6 @@ require (
        gopkg.in/yaml.v2 v2.4.0 // indirect
        gopkg.in/yaml.v3 v3.0.0-20210107192922-496545a6307b // indirect
        k8s.io/apiserver v0.22.8 // indirect
-       k8s.io/controller-manager v0.22.8 // indirect
        k8s.io/gengo v0.0.0-20201214224949-b6c5ce23f027 // indirect
        k8s.io/kube-openapi v0.0.0-20211109043538-20434351676c // indirect
        sigs.k8s.io/apiserver-network-proxy/konnectivity-client v0.0.30 // indirect

The pflag one is especially interesting since it does seems they use it directly in their controller:

❯ rg pflag
...

pkg/controllers/options/tagging_controller.go
18:     "github.com/spf13/pflag"
29:func (o *TaggingControllerOptions) AddFlags(fs *pflag.FlagSet) {

But checking out their master branch, cleaning everything up, and running through the sequence again works fine without any problem in their go.mod:

❯ git checkout master

❯ rm ecr-credential-provider && rm -rf vendor

❯ git reset --hard origin/master
HEAD is now at c72f89b Merge pull request #471 from wangamel/master

❯ GO111MODULE=on CGO_ENABLED=0 GOOS=linux GOARCH=arm64 GOPROXY=https://proxy.golang.org,direct go build -x \
        -trimpath \
        -ldflags="-w -s -X k8s.io/component-base/version.gitVersion=" \
        -o=ecr-credential-provider \
        cmd/ecr-credential-provider/*.go

❯ ll ecr-credential-provider
-rwxr-xr-x. 1 fedora fedora 14M Aug 26 22:53 ecr-credential-provider

---

My presumption is their v1.22.3 tag had an out of date go.mod which causes go mod vendor to get the wrong dependencies which then prevents a build.

I also wonder if make was eating all of that error output as an argument list?

Can we ask them to cut a new, up to date tag / tar with the right go.mod?

[stmcginnis]

My presumption is their v1.22.3 tag had an out of date go.mod which causes go mod vendor to get the wrong dependencies which then prevents a build.

Go would update the go.mod file, so I had assumed that would resolve any issues before vendoring. But I think you're right there. I found there is a newer v1.24.1. tag and the dependency issues have since been cleaned up (with one minor change still stale).

That seems to be enough that it fixed whatever was happening though:

$ git branch
* (HEAD detached at v1.24.1)
  master
$ go mod tidy
$ git status
HEAD detached at v1.24.1
Changes not staged for commit:
  (use "git add <file>..." to update what will be committed)
  (use "git restore <file>..." to discard changes in working directory)
	modified:   go.mod

no changes added to commit (use "git add" and/or "git commit -a")
$ PAGER= git diff
diff --git a/go.mod b/go.mod
index 76e3379..8c6d8ca 100644
--- a/go.mod
+++ b/go.mod
@@ -8,6 +8,7 @@ require (
 	github.com/google/go-cmp v0.5.8
 	github.com/spf13/pflag v1.0.5
 	github.com/stretchr/testify v1.7.2
+	golang.org/x/time v0.0.0-20220210224613-90d013bbcef8
 	gopkg.in/gcfg.v1 v1.2.3
 	k8s.io/api v0.24.2
 	k8s.io/apimachinery v0.24.2
@@ -94,7 +95,6 @@ require (
 	golang.org/x/sys v0.0.0-20220209214540-3681064d5158 // indirect
 	golang.org/x/term v0.0.0-20210927222741-03fcf44c2211 // indirect
 	golang.org/x/text v0.3.7 // indirect
-	golang.org/x/time v0.0.0-20220210224613-90d013bbcef8 // indirect
 	golang.org/x/tools v0.1.10-0.20220218145154-897bd77cd717 // indirect
 	golang.org/x/xerrors v0.0.0-20200804184101-5ec99f83aff1 // indirect
 	google.golang.org/appengine v1.6.7 // indirect
$ go mod vendor 
$ docker run --rm --network none -v ${PWD}:/build -w /build golang:1.18 make ecr-credential-provider
GO111MODULE=on CGO_ENABLED=0 GOOS=linux GOARCH=arm64 GOPROXY=https://proxy.golang.org,direct go build \
	-trimpath \
	-ldflags="-w -s -X k8s.io/component-base/version.gitVersion=v1.24.1-dirty" \
	-o=ecr-credential-provider \
	cmd/ecr-credential-provider/*.go
$ ls ecr-credential-provider
ecr-credential-provider

Glad this is at least captured here as I think we may run into other weirdness like this as we add other go packages. But looks like I can at least proceed for now.

And this change looks good to me. Again, just wanted to capture some weirdness so there's some record of it.

[jpmcb]

Glad this is at least captured here as I think we may run into other weirdness like this as we add other go packages. But looks like I can at least proceed for now.

@stmcginnis Awesome!!! 👏🏼 glad that works!! And yes, glad it's captured here

[stmcginnis]

Tested this out with the work I'm doing. I'm still working through other issues, but I've gotten past the part covered by this and everything appears to be working well.

[bcressey]

Nice work! Lots of nits but I don't see anything major missing.

[jpmcb]

Added another commit which reworks how the bundling script is used by docker-go:

Now, instead of attempting to feed a long script delimited by && or \, the script lives as a const string that gets added as a temporary file to the package directory. This file is then accessible by the container and easily run with bash -c ./docker-go-script.sh. Further, the bundling script now expects several environment variables which are loaded through a new --additional-docker-args option in the docker-go tool; this enables us to use the -e arguments for docker to load up environment variables into the container.

Further documentation in comments in code.

[bcressey]

Looks good. In terms of the commits:

• I'd "unsquash" the first one and have two separate commits for the hotdog and oci-add-hooks changes.
• There's at least one typo in the second commit message
• You can squash all the buildsys commits together as the first patch in the series (probably you were planning to do this).

[jpmcb]

Force pushed to flesh out the commit structure per @bcressey's recommendations and address a few nits!

Removed changes to tools/docker-go which added the --additional-docker-args and instead, the go module script uses a few placeholder variables that are populated before the temporary file is put in the package directory.

[bcressey]

Yay!

[stmcginnis]

Tested with #2377 and it looks like everything works as expected.