eks-distro icon indicating copy to clipboard operation
eks-distro copied to clipboard

catch go mod download overwriting changes to vendor

Open jaxesn opened this issue 9 months ago • 5 comments

Issue #, if available:

Description of changes:

From pretty much the beginning of eks-d and then eks-a after it, we have run go mod download before building. Some upstream projects, such as k/k, actually check in their vendor folder so in those cases we do not technically need to run go mod vendor (and in the case of k/k we actually do not, never have) but we do anyway to be consistent and avoid having to have made a case by case decision. This was also important for reproducible builds, there is some oddities with go dependencies that have been download vs checked in and pulled. I have previous PRs which talk about this. Always running go mod vendor ensured all projects would build regardless of if the vendor dir was checked in or not.

About a year ago, the harbor build in eks-a was updated with a patch that actually did patch something in the vendor directory. At that time we added GO_MODS_VENDORED in https://github.com/aws/eks-anywhere-build-tooling/pull/2078 to allow projects to skip running go mod vendor.

Recently a patch was added to CCM for versions 1.28+ which 1) adds the vendor directory and 2) patches something in that directory. As is, this patch is not actually taking affect because it gets overwritten when the go mod vendor call is ran. The fix, which @markapruett is taking, is to port over the skipping mechanism from eks-a to eks-d and setting GO_MODS_VENDORED to true for the ccm build.

While discussing with Mark this nuance, I was thinking of ways we could catch this and avoid the silent "failure". This is my attempt, if the vendor dir exists before running go mod vendor then use git to determine if anything changed in that folder due to go mod vendor, if so, then error.

With this change it will require that any time we patch a go.mod/sum file for a project with vendored deps, we need to make sure the patch includes the vendor folder updates, otherwise git will see a difference and trigger this error. The fix should be pretty easy/obvious in this case.

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

jaxesn avatar May 10 '24 17:05 jaxesn