ci: Add makefile target to verify the local module cache

[shahzadlone]

Resolves #762

Description

• Introduces a Makefile target that will fail when go mod verify reports any changes to module cache, because by default the go mod verify command just prints the modules that were tampered with (doesn't fail or return an error).

Limitations

No way to specify go mod verify to be a read-only op (i.e. in some cases might modify go.sum file.

• cmd/go: 'mod verify' should not modify the go.mod file golang/go#31372
• Use of "go mod verify" in Makefile results in unreproducible builds cosmos/cosmos-sdk#4165

How has this been tested?

Ensured that CI will fail upon

Specify the platform(s) on which this was tested:

• Manjaro WSL2

[AndrewSisley]

No way to specify go mod verify to be a read-only op (i.e. in some cases might modify go.sum file.

Are you sure that it will not modify anything if this step passes? If 'all modules verified' is printed out?

And are you sure there is no more reliable way of checking that it passes other than with a simple grep?

These questions aside (the first one being a blocker for me), I'm happy, but suggest waiting on someone else to chime in first before merge

[shahzadlone]

Are you sure that it will not modify anything if this step passes? If 'all modules verified' is printed out?

Here is what their documentation says:

"go mod verify may download go.mod files in order to perform minimal version selection. It will use go.sum to verify those files, and it may add go.sum entries for missing hashes."

In my testing I haven't seen it modify anything (i.e. go.sum) so far.

And are you sure there is no more reliable way of checking that it passes other than with a simple grep?

I couldn't find a flag or another option that would make it fail, happy to take another approach if there are suggestions.

[AndrewSisley]

"go mod verify may download go.mod files in order to perform minimal version selection. It will use go.sum to verify those files, and it may add go.sum entries for missing hashes."

In my testing I haven't seen it modify anything (i.e. go.sum) so far.

😅 okay lol, I guess we can try it out and drop it if this starts being a nuisance. I wont mark this as approved though, and will let others keep chatting on this first (as I'm still very 50-50 on this RE cost-benefit, and I guess this isn't blocking any other work).

I couldn't find a flag or another option that would make it fail, happy to take another approach if there are suggestions.

Okay no worries :P

[orpheuslummis]

Here's a middle ground idea:

• do provide make verify
• do not use it as part of build target
• do verify when building a binary for release, and make that fact known (for trustworthiness) - ideally - that opens up the question of how to have reproducible builds and proofs of them
• when/if Go provides readonly go mod verify, we can add the step as part of build target

[codecov]

Codecov Report

Merging #775 (c535c14) into develop (55fca49) will decrease coverage by 0.57%.
The diff coverage is n/a.

❗ Current head c535c14 differs from pull request most recent head baa0e1f. Consider uploading reports for the commit baa0e1f to get more accurate results

@@             Coverage Diff             @@
##           develop     #775      +/-   ##
===========================================
- Coverage    59.11%   58.53%   -0.58%     
===========================================
  Files          153      150       -3     
  Lines        16977    16900      -77     
===========================================
- Hits         10036     9893     -143     
- Misses        6023     6083      +60     
- Partials       918      924       +6     

Impacted Files	Coverage Δ
db/collection_update.go	42.99% <0.00%> (-12.49%)	⬇️
query/graphql/planner/planner.go	74.83% <0.00%> (-2.91%)	⬇️
query/graphql/planner/select.go	75.00% <0.00%> (-1.45%)	⬇️
query/graphql/planner/sum.go	84.97% <0.00%> (-0.60%)	⬇️
query/graphql/planner/count.go	95.60% <0.00%> (-0.36%)	⬇️
query/graphql/schema/manager.go	97.05% <0.00%> (-0.29%)	⬇️
core/enumerable/skip.go
core/enumerable/concat.go
core/enumerable/select.go
query/graphql/schema/generate.go	83.24% <0.00%> (+0.81%)	⬆️
... and 3 more

[shahzadlone]

Here's a middle ground idea:

• do provide make verify
• do not use it as part of build target
• do verify when building a binary for release, and make that fact known (for trustworthiness) - ideally - that opens up the question of how to have reproducible builds and proofs of them
• when/if Go provides readonly go mod verify, we can add the step as part of build target

Happy with this suggestion. I have removed the build step, and fixed the makefile target to show the failure as suggested.

[fredcarle]

LGTM

[orpheuslummis]

Thanks, I've added the corresponding step to the release process document so next time we produce a binary we manually do the step... To be automated later.

[orpheuslummis]

The Go 1.18 release notes indicate that go mod verify "no longer automatically update the go.mod and go.sum files". https://go.dev/doc/go1.18#go-command I will revisit this issue after Go 1.20 lands so we can just include it as part of the normal build, so the user can benefit from it locally.