Set up go workspace #6702

mook-as · 2024-04-11T21:29:52Z

This sets up a go workspace to try to keep all of our various go bits in sync.

Note that we disable it for some of the CI because we do a sparse checkout (so not all of the modules are there).

This also includes a ton of lint fixes that this uncovers :( Sorry about that. It might be a good idea to review commit-by-commit; some commits (like the one about defer + fatal) might be best reviewed skipping whitespace.

Nino-K · 2024-04-15T16:02:39Z

src/go/github-runner-monitor/cmd/root_other.go

+//go:build !unix
+
+/*
+Copyright © 2023 SUSE LLC


Nino-K · 2024-04-15T16:04:11Z

src/go/mock-wsl/lock_file_other.go

@@ -0,0 +1,12 @@
+//go:build !windows


We should include the license header for the sake of consistency.

Nino-K · 2024-04-15T16:08:17Z

src/go/nerdctl-stub/debugging_stub.go

@@ -0,0 +1,9 @@
+//go:build !debug


Same here, we should probably include the license header.

Nino-K · 2024-04-15T16:10:21Z

src/go/privileged-service/doc.go

@@ -0,0 +1,20 @@
+/*
+Copyright © 2022 SUSE LLC


Nino-K · 2024-04-15T16:11:05Z

src/go/rdctl/pkg/options/generated/doc.go

@@ -0,0 +1,21 @@
+/*
+Copyright © 2023 SUSE LLC


Nino-K · 2024-04-15T16:13:21Z

src/go/vtunnel/cmd/peer_linux.go

@@ -59,6 +60,8 @@ IP and port acting as a peer end of the tunnel.`,

 func init() {
 	peerCmd.Flags().String("config-path", "", "Path to the vtunnel's yaml configuration file")
-	peerCmd.MarkFlagRequired("config-path")
+	if err := peerCmd.MarkFlagRequired("config-path"); err != nil {
+		logrus.WithError(err).Fatal("Failed to set up flags")


Maybe the same here, lowercase.

Nino-K · 2024-04-15T16:14:06Z

src/go/wsl-helper/cmd/certificates_windows.go

@@ -64,6 +65,8 @@ var certificatesCmd = &cobra.Command{
 func init() {
 	certificatesCmd.Flags().StringSlice("stores", []string{"CA", "ROOT"}, "Certificate stores to enumerate")
 	certificatesViper.AutomaticEnv()
-	certificatesViper.BindPFlags(certificatesCmd.Flags())
+	if err := certificatesViper.BindPFlags(certificatesCmd.Flags()); err != nil {
+		logrus.WithError(err).Fatal("Failed to set up flags")


This one too, and the following.

Nino-K · 2024-04-15T16:15:56Z

src/go/wsl-helper/cmd/k3s_kubeconfig.go

@@ -53,6 +54,10 @@ type kubeConfig struct {
 	Extras map[string]interface{} `yaml:",inline"`
 }

+const (


does it have to be a multi-line style? can it be a single-line declaration since there is only one const?

Nino-K · 2024-04-15T16:18:43Z

src/go/wsl-helper/pkg/dockerproxy/models/doc.go

@@ -0,0 +1,2 @@
+// Package models contains the auto-generated OpenAPI models.


License header here too.

Nino-K · 2024-04-15T16:22:12Z

src/go/wsl-helper/pkg/dockerproxy/mungers/containers_create_linux.go

@@ -282,7 +282,7 @@ func (b *bindManager) mungeContainersCreateRequest(req *http.Request, contextVal

 // containersCreateResponseBody describes the contents of a /containers/create response.
 type containersCreateResponseBody struct {
-	Id       string
+	Id       string //nolint:stylecheck // Needs to match API


Should it use json struct tags instead?

Nino-K

LGTM, however, it looks like you have a merge conflict to deal with.

This ensures a cross-repo baseline of dependency versions. This also means the editors can better track cross-module definitions. Signed-off-by: Mark Yen <[email protected]>

Dependabot still fails even if the generated packages are listed as an ignored dependency. Instead, check in stub files that can be used as placeholders; they have no contents and therefore should not conflict with the actual generated data. Signed-off-by: Mark Yen <[email protected]>

This was already covered by "leave existing file" on line 76. Signed-off-by: Mark Yen <[email protected]>

`deadcode`, `structcheck`, and `varcheck` have all been marked as deprecated by golangci-lint; the recommended replacement is `unused` (which we're already using). Signed-off-by: Mark Yen <[email protected]>

- Remove gocyclo; in general, this is better replaced by human reviews. - Disable `gocritic`'s `unnamedResult` rule; we don't seem to care about it - Mark `os.FileMode` / `os.MkDir` as acceptable with magic numbers; those are file permissions, and typical usage uses permission bits. - Add specific exclusion rules: - Disable a case of `stylecheck` not being aware that Windows is a proper noun. - Disable `lll` for function declarations where it's clearer to have all of the parameters on a single line. - We copied `vsock_linux.go` from a different project; leave the terse `FIXME` comments alone. - When using `syscall` (or its moral equivalent) to call FFI, allow ignoring the result. Signed-off-by: Mark Yen <[email protected]>

This was triggered by the linter. Signed-off-by: Mark Yen <[email protected]>

When we call `log.Fatalf()` or similar that ends up calling `os.Exit()`, we don't run any deferred statements. This gets picked up by the linter as an issue. Work around this by wrapping the bulk in an inner function, and then calling `log.Fatalf()` outside of it (after all of the deferred statements have been run). Signed-off-by: Mark Yen <[email protected]>

Make packages build across all platforms (because the code analysis falls over if a dependency is not available on the given platform). Signed-off-by: Mark Yen <[email protected]>

These are lint errors that are more local in nature; for example: - io/ioutil is deprecated - name constants instead of magic numbers all over - avoid shadowing - explicitly ignore returned errors in defer statements Signed-off-by: Mark Yen <[email protected]>

Signed-off-by: Mark Yen <[email protected]>

mook-as requested a review from Nino-K April 11, 2024 21:29

Nino-K reviewed Apr 15, 2024

View reviewed changes

mook-as self-assigned this Apr 18, 2024

mook-as force-pushed the go-workspace branch from 517cd1e to 95c7b82 Compare April 19, 2024 18:18

mook-as requested a review from Nino-K April 19, 2024 18:19

Nino-K previously approved these changes Apr 22, 2024

View reviewed changes

mook-as dismissed Nino-K’s stale review via 40efb8c April 22, 2024 17:53

mook-as force-pushed the go-workspace branch from 95c7b82 to 40efb8c Compare April 22, 2024 17:53

mook-as requested a review from Nino-K April 22, 2024 18:14

mook-as added 10 commits April 25, 2024 09:51

Set up go workspace

7cc4806

This ensures a cross-repo baseline of dependency versions. This also means the editors can better track cross-module definitions. Signed-off-by: Mark Yen <[email protected]>

wsl-helper: remove duplicate test

4cebaa7

This was already covered by "leave existing file" on line 76. Signed-off-by: Mark Yen <[email protected]>

golangci: Remove obsolete linters

1152954

`deadcode`, `structcheck`, and `varcheck` have all been marked as deprecated by golangci-lint; the recommended replacement is `unused` (which we're already using). Signed-off-by: Mark Yen <[email protected]>

Check errors from cobra Bind*Flags / MarkFlagRequired

24222ce

This was triggered by the linter. Signed-off-by: Mark Yen <[email protected]>

Fix lint errors: cross-platform build

110e611

Make packages build across all platforms (because the code analysis falls over if a dependency is not available on the given platform). Signed-off-by: Mark Yen <[email protected]>

Fix lint errors

5bf17e3

These are lint errors that are more local in nature; for example: - io/ioutil is deprecated - name constants instead of magic numbers all over - avoid shadowing - explicitly ignore returned errors in defer statements Signed-off-by: Mark Yen <[email protected]>

Address review comments

e59b162

Signed-off-by: Mark Yen <[email protected]>

mook-as force-pushed the go-workspace branch from 40efb8c to e59b162 Compare April 25, 2024 16:52

Nino-K approved these changes Apr 25, 2024

View reviewed changes

mook-as enabled auto-merge April 25, 2024 16:54

mook-as merged commit 57191c5 into rancher-sandbox:main Apr 25, 2024
14 checks passed

mook-as deleted the go-workspace branch April 25, 2024 19:30

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Set up go workspace #6702

Set up go workspace #6702

mook-as commented Apr 11, 2024

Nino-K Apr 15, 2024

Nino-K Apr 15, 2024

Nino-K Apr 15, 2024

Nino-K Apr 15, 2024

Nino-K Apr 15, 2024

Nino-K Apr 15, 2024

Nino-K Apr 15, 2024

Nino-K Apr 15, 2024

Nino-K Apr 15, 2024

Nino-K Apr 15, 2024

Nino-K left a comment

		@@ -0,0 +1,2 @@
		// Package models contains the auto-generated OpenAPI models.

Set up go workspace #6702

Set up go workspace #6702

Conversation

mook-as commented Apr 11, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Nino-K left a comment

Choose a reason for hiding this comment