go/build pipeline: Move go mod tidy after cd to modroot dir AND Validate existence of go.mod in modroot

[debasishbsws]

• fix: Move go mod tidy after cd to modroot dir

  • Previously, in the go/build pipeline, if modroot was set and tidy=true, the go mod tidy command ran in the home directory instead of the modroot directory.

• feat: Validate existence of go.mod in modroot

  • Added a check to ensure go.mod exists in the modroot directory, ensuring modroot is set correctly. This will help the reliability of go/bump automation as it also set its modroot by looking into the go/build pipeline modroot.

Notes: this will result in build failure of PR if there is any package where the modroot is set incorrectly.

[hectorj2f]

I cannot see the need for these changes, if go mod is not found go mod tidy or go/bump will fail anyway.

[hectorj2f]

The default error is also pretty descriptive today telling us that there is not a go.mod file on the specified location.

[debasishbsws]

@hectorj2f You're correct that go mod tidy or go/bump will fail if the go.mod file is missing. However, the issue is that the go/build pipeline does not fail if the modroot is set incorrectly, as running go mod tidy is not enabled by default.

Since go/bump inherits the modroot from go/build, an incorrectly set modroot can result in an automated go/bump with the wrong modroot, leading to failed CVE rem PR.

See the fix here, where previously, an incorrectly set modroot didn’t cause any build failure. Slack conversation link

[luhring]

I believe this change is needed! It enforces correct setup of the go/build pipeline and helps us avoid toil from broken automated PRs that depend on go/build's modroot being set correctly.

But I'm concern about how breaking of a change this could be at the outset. @debasishbsws have you looked at which of our packages this might break the build for? This could be a disruptive change that brings our build system to a halt, if we're not careful.

[hectorj2f]

@hectorj2f You're correct that go mod tidy or go/bump will fail if the go.mod file is missing. However, the issue is that the go/build pipeline does not fail if the modroot is set incorrectly, as running go mod tidy is not enabled by default.

👍🏻

[debasishbsws]

But I'm concern about how breaking of a change this could be at the outset. @debasishbsws have you looked at which of our packages this might break the build for? This could be a disruptive change that brings our build system to a halt, if we're not careful.

@luhring I was thinking of identifying packages this might break but that need looking into the source code or try to build all the go package which use go/build pipeline. But I don't think this could be very disruptive. As we don't build every package every day. We will encounter this build failure with clear message in the package updates or CVE rem PR. And at that time we have to look and try to fix the package.

[luhring]

But I'm concern about how breaking of a change this could be at the outset. @debasishbsws have you looked at which of our packages this might break the build for? This could be a disruptive change that brings our build system to a halt, if we're not careful.

@luhring I was thinking of identifying packages this might break but that need looking into the source code or try to build all the go package which use go/build pipeline. But I don't think this could be very disruptive. As we don't build every package every day. We will encounter this build failure with clear message in the package updates or CVE rem PR. And at that time we have to look and try to fix the package.

Right, I think once this causes a build to fail, we can put the time into to fix it once and for all. And then things will be awesome for that package.

But for me at least, I don't have a good sense of the total time that will require, and how soon we'll need to invest it. For example, if it's just an occasional extra 5 minutes, a few times a week for the next few months, then I agree that's not too bad. But if it starts block all our CVE fixes and requires dozens of hours a day from multiple staff members for the foreseeable future, that would be terrible, of course.

So my thinking is that we don't know where on that spectrum this would fall. Does that make sense? Do you have a better sense than I do about how this will play out for us if we merge this today?