Support Go 1.19

[epk]

Description of your changes

Adds support for Go 1.19. This check now matches rook/rook's makelibs:

• https://github.com/rook/rook/blob/master/build/makelib/golang.mk#L51
• https://github.com/rook/rook/blob/master/build/makelib/golang.mk#L105-L108

Unlike #195, this does not enforce an upgrade to Go 1.19

I have:

• Read and followed Upbound's contribution process.
• Run make reviewable to ensure this PR is ready for review.
• Added backport release-x.y labels to auto-backport this PR, as appropriate.

How has this code been tested

[epk]

GO_REQUIRED_VERSION is not referenced in any repo:

• https://github.com/search?q=org%3Aupbound+GO_REQUIRED_VERSION&type=code
• https://github.com/search?q=org%3Acrossplane+GO_REQUIRED_VERSION&type=code

[muvaf]

@epk We used to have a list like this one, #140 is the PR that removed it. Could you take a look there and see whether the arguments for removal still holds?

cc @negz

[epk]

@epk We used to have a list like this one, #140 is the PR that removed it. Could you take a look there and see whether the arguments for removal still holds?

cc @negz

Sorry for the delay in getting back, I was taking a little break from the build submodule.

Looking at #140, Go 1.18 bump was where we had to pay the price of a newer version of Go breaking the library.

I was also running into instances where Github Actions or CircleCI pipelines were missing the Go setup step and were running older versions of Go (defaulting to the versions installed on Github or Circle CI runners) which produced different lint results and other errors compared to the local env. This is the reason why I added the version check/restriction. Go version in the go.mod alone was not sufficient to catch/surface this issue.

However, I am a bit torn on the version check/restriction. I generally agree with @negz's points in #140.

---

What are your thoughts on deferring to set GO_SUPPORTED_VERSION in the consuming projects? This is breaking change and would require all projects to set GO_SUPPORTED_VERSION in their makefiles but:

• We would have guardrail against the CI issues I stumbled upon
• We won't have to make a change to build when a new Go version comes out.

If we agree on this, it also makes sense to defer setting all the Go tool versions in the consuming projects as well.

[muvaf]

I think most of the issues were because many things have changed - even linters themselves and which linters are enabled - in go1.18 but I don't expect that to happen frequently that we need to have the guardrail in build submodule. My preference would be not have any check at all and rely on Go's internal checks regarding this.

[epk]

My preference would be not have any check at all and rely on Go's internal checks regarding this.

Go's internal checks are not sufficient for this purpose.

I was also running into instances where Github Actions or CircleCI pipelines were missing the Go setup step and were running older versions of Go (defaulting to the versions installed on Github or Circle CI runners) which produced different lint results and other errors compared to the local env

This was just a result of running same linters, garble versions but with different Go versiosn.

Go usually has two major releases a year, so if a project is on the latest version, they will only have to upgrade things once a year (if we keep GO_SUPPORTED_VERSIONS updated with the current, current-1 Go versions). GO_SUPPORTED_VERSIONS can also be overridden by consuming projects if they don't want to upgrade.

[epk]

Another example of why we need the check: https://github.com/upbound/up/commit/cd977caabcd0d21e9e8ab3bad35cea387e0d93b9

[negz]

I'm very reluctant to add this gate again. I don't think much has changed since last time we had it. I'm skeptical we'll do any better this time around WRT actually removing supported versions from this list as the submodule (and the projects that use it) change. I'm also not sure how valuable needing to explicitly permit newer versions of Go is.

Regarding the second point, it seems like the key value of this gate is that when a new version of Go is released someone trying to use that new version will get an explicit "this version isn't supported" error where they might have otherwise run into a bunch of arcane errors that are tricky to diagnose as being because they have a newer Go version than the project is designed for. The issue here is that (anecdotally) it's much more common for new Go versions to "just work" than to need tricky updates, which historically has made this list just an annoyance, or something everyone ends up just working around by running GO_SUPPORTED_VERSIONS=1.19 make for months before anyone bumps the build submodule.

If we do proceed with something like this I would prefer to require that each project be forced to set and maintain its own GO_SUPPORTED_VERSIONS per @epk's suggestion. I'd also suggest that we don't have a list of supported versions, but require exactly one version (the same one used by CI).

[negz]

@epk Another thing that could help here is to make GO_SUPPORTED_VERSION a warning rather than a fatal error - i.e. give folks a hint that they're using an unsupported version but don't block them.

[muvaf]

There are also false negatives like this one where an unrelated command errors out because the local go version is not supported.

Another thing that could help here is to make GO_SUPPORTED_VERSION a warning rather than a fatal error - i.e. give folks a hint that they're using an unsupported version but don't block them.

I like this option. I really don't want to have the list be the cause of error when it doesn't need to be and this happens a lot, i.e. 1.19 is actually compatible but not supported by the list or go is irrelevant but build submodule is called.

[epk]

There are also false negatives like this one where an unrelated command errors out because the local go version is not supported.

That's not a false negative, it is the issue of a CI workflow not running the required Go version (as I have pointed out above in a few instances). Would you want your publish/promote steps to build with a different version of Go than rest of the workflows?

EDIT: I don't know if make promote builds or just takes an existing build and promotes that

[muvaf]

Would you want your publish/promote steps to build with a different version of Go than rest of the workflows?

Go is not involved https://github.com/upbound/build/blob/master/makelib/image.mk#L206 .

Even if it was though, if go itself doesn't throw an error and I'm notified about the diff, then I'd be OK.