openSUSE / obs-service-go_modules

OBS Source Service to download, verify, and vendor Go module dependency sources
GNU General Public License v2.0
19 stars 18 forks source link

go mod verify can fail with committed go.work workspace modules (talos) #31

Closed johanneskastl closed 1 year ago

johanneskastl commented 1 year ago

I am trying to packages talosctl based on https://github.com/siderolabs/talos. But the verify step fails. Not sure if this is an error in the service or the talos go files?

merge: origin/v1.4.0 - not something we can merge
Already up to date.
db0f3001506694089b59d2a0e33781d5f8a0b93f
INFO:obs-service-go_modules:Running OBS Source Service: obs-service-go_modules
INFO:obs-service-go_modules:Using archive talos-1.4.0.obscpio
INFO:obs-service-go_modules:Extracting talos-1.4.0.obscpio to /tmp/tmpxgl36qur
INFO:obs-service-go_modules:Switching to /tmp/tmpxgl36qur
INFO:obs-service-go_modules:Detected basename talos-1.4.0 from archive
INFO:obs-service-go_modules:Using go.mod found at /tmp/tmpxgl36qur/talos-1.4.0/go.mod
INFO:obs-service-go_modules:go mod download
INFO:obs-service-go_modules:go mod vendor
INFO:obs-service-go_modules:go mod verify
ERROR:obs-service-go_modules:github.com/siderolabs/cloud-image-uploader : missing ziphash: open hash: no such file or directory
github.com/siderolabs/talos-hack-docgen : missing ziphash: open hash: no such file or directory
github.com/siderolabs/gotagsrewrite : missing ziphash: open hash: no such file or directory
module-sig-verify : missing ziphash: open hash: no such file or directory
github.com/siderolabs/structprotogen : missing ziphash: open hash: no such file or directory
github.com/siderolabs/talos/pkg/machinery : missing ziphash: open hash: no such file or directory
ERROR:obs-service-go_modules:go mod verify failed
[...]

Here is the _service file:

<services>
  <service name="obs_scm" mode="disabled">
    <param name="url">https://github.com/siderolabs/talos</param>
    <param name="scm">git</param>
    <param name="exclude">.git</param>
    <param name="revision">v1.4.0</param>
    <param name="versionformat">@PARENT_TAG@</param>
    <param name="changesgenerate">enable</param>
    <param name="versionrewrite-pattern">v(.*)</param>
  </service>
  <service name="set_version" mode="disabled">
    <param name="basename">talos</param>
  </service>
  <service name="tar" mode="buildtime"/>
  <service name="recompress" mode="buildtime">
    <param name="file">*.tar</param>
    <param name="compression">gz</param>
  </service>
  <service name="go_modules" mode="disabled">
    <param name="archive">talos-1.4.0.obscpio</param>
  </service>
</services>
johanneskastl commented 1 year ago

Any ideas, anyone?

johanneskastl commented 1 year ago

(Still happens with 1.4.6 of talos)

jfkw commented 1 year ago

Thanks for reporting. This is an issue with talos upstream's use of a go.work workspace for the module module-sig-verify in directory hack/module-sig-verify. I have verified that the behavior is the same with our packaged go1.21 and the upstream binary toolchain download. I do not yet know if many workspace mods will have this issue or if it is specific to module-sig-verify. Running go mod verify directly in hack/module-sig-verify fails with the same error, so I suspect it is specific.

There is currently an upstream Go issue that is directly related: https://github.com/golang/go/issues/62663

The initial workspaces proposal stated that go.work was not intended to be a committed file. Current upstream consensus seems to be that in certain limited circumstances and workflows, it can be useful to commit go.work workspaces to source code repositories. We may see a small number of projects doing this over time and will support that use case if possible.

I do not plan to add an option to skip the go mod verify step.

johanneskastl commented 1 year ago

Thanks for taking a look, @jfkw! Any workaround for the time being? I would really like to package talosctl, but without too much manual work...

jfkw commented 1 year ago

Is it only talosctl that is wanted for vendoring and packaging at this time? If so, it might be possible to remove parts of the large source tree ("Talos Linux is a modern Linux distribution built for Kubernetes"). It turns out only removing the committed go.work is necessary:

talos> rm go.work
talos> go mod verify
all modules verified

talos> go build ./cmd/talosctl/

talos> git status
 D go.work
?? talosctl
?? vendor/

talos> ./talosctl --help
A CLI for out-of-band management of Kubernetes nodes created by Talos

Usage:
  talosctl [command]

Available Commands:
  apply-config        Apply a new configuration to a node
  bootstrap           Bootstrap the etcd cluster on the specified node.
(...)

The obs-service-go_modules runs in a separate service phase before the rpmbuild %prep step could issue the rm command. We'll need a solution to remove or ignore the file at that time. obs_scm/tar_scm has exclude features that would do it while building the archive.

I'm open to supporting an "ignore go.work" feature in obs-service-go_modules, but that will first require some planning and more information on how Go upstream plans to handle the presence of go.work during go mod verify. If upstream added an argument to the command to ignore, we would support that. Or perhaps they will detect and ignore go.work transparently.

The talos workspace module is the example discussed in the upstream issue, it seems like it will get the necessary attention and an eventual fix at a date TBD.

johanneskastl commented 1 year ago

Thanks @jfkw , ignoring the go.work file in the obs_scm service seems to work.

jfkw commented 1 year ago

Good to hear, closing.

The plan is that our go mod verify usage will support the eventual Go upstream recommendation for handling committed go.work files and workspaces. That may be via an added argument to ignore, or transparent detection by the go mod verify command.