search

golangci / golangci-lint

Fast linters runner for Go
https://golangci-lint.run
GNU General Public License v3.0
14.96k stars 1.35k forks source link

New errors when upgrading bumping google.golang.org/protobuf from 1.31.0 to 1.33.0 in argo workflow related code #4651

Closed mckornfield closed 3 months ago

mckornfield commented 3 months ago

Welcome

Description of the problem

When bumping to version 1.33.0 of google.golang.org/protobuf, we get a set of errors that we haven't seen before, but don't know the cause. The errors don't appear on version 1.31 or 1.32 of google.golang.org/protobuf but appear on 1.33.0 for the latest version of golanci-lint It seems to be related to the places where Argo Workflows (https://pkg.go.dev/github.com/argoproj/argo-workflows/v3@v3.5.5) are present in the code as well, I'll share a small, stripped down example.

Version of golangci-lint

```console tools/golangci-lint-new-v1.57.2/golangci-lint" --version golangci-lint has version 1.57.2 built with go1.22.1 from 77a8601a on 2024-03-28T19:01:11Z ```

Configuration

```yml linters: disable-all: true enable: - typecheck - govet - exportloopref - ineffassign - gosimple - gofmt - goimports - asasalint - bidichk - gosec # Enable these at a later point (fixing all issues produces a larger diff) # - errcheck # - unused # - staticcheck linters-settings: govet: settings: printf: funcs: - (github.com/gretellabs/monogretel/go/pkg/logging.Logger).Trace - (github.com/gretellabs/monogretel/go/pkg/logging.Logger).Info - (github.com/gretellabs/monogretel/go/pkg/logging.Logger).Warn - (github.com/gretellabs/monogretel/go/pkg/logging.Logger).Error gosimple: checks: - "all" - "-S1008" goimports: local-prefixes: github.com/gretellabs/monogretel gosec: excludes: - G404 # weak random number generator - G114 # Use of net/http serve function that has no support for setting timeouts ```

Go environment

```console $ go version && go env go version go1.21.7 darwin/arm64 GO111MODULE='' GOARCH='arm64' GOBIN='' GOCACHE='/Users/mckornfield/Library/Caches/go-build' GOENV='/Users/mckornfield/Library/Application Support/go/env' GOEXE='' GOEXPERIMENT='' GOFLAGS='' GOHOSTARCH='arm64' GOHOSTOS='darwin' GOINSECURE='' GOMODCACHE='/Users/mckornfield/go/pkg/mod' GONOPROXY='' GONOSUMDB='' GOOS='darwin' GOPATH='/Users/mckornfield/go' GOPRIVATE='' GOPROXY='https://proxy.golang.org,direct' GOROOT='/private/var/tmp/_bazel_mckornfield/ee01006115885add21842adb27a6a0ea/external/go_sdk' GOSUMDB='sum.golang.org' GOTMPDIR='' GOTOOLCHAIN='auto' GOTOOLDIR='/private/var/tmp/_bazel_mckornfield/ee01006115885add21842adb27a6a0ea/external/go_sdk/pkg/tool/darwin_arm64' GOVCS='' GOVERSION='go1.21.7' GCCGO='gccgo' AR='ar' CC='clang' CXX='clang++' CGO_ENABLED='1' GOMOD='/dev/null' GOWORK='/Users/mckornfield/repo/monogretel2/go.work' CGO_CFLAGS='-O2 -g' CGO_CPPFLAGS='' CGO_CXXFLAGS='-O2 -g' CGO_FFLAGS='-O2 -g' CGO_LDFLAGS='-O2 -g' PKG_CONFIG='pkg-config' GOGCCFLAGS='-fPIC -arch arm64 -pthread -fno-caret-diagnostics -Qunused-arguments -fmessage-length=0 -ffile-prefix-map=/var/folders/m3/_h02tfnx27x1nrh8c662m5xc0000gn/T/go-build1824552533=/tmp/go-build -gno-record-gcc-switches -fno-common' ```

Verbose output of running

```console "/Users/mckornfield/repo/monogretel2/go/../.tools/golangci-lint-new-v1.57.2/golangci-lint" cache clean "/Users/mckornfield/repo/monogretel2/go/../.tools/golangci-lint-new-v1.57.2/golangci-lint" run -v INFO [config_reader] Config search paths: [./ /Users/mckornfield/repo/monogretel2/go /Users/mckornfield/repo/monogretel2 /Users/mckornfield/repo /Users/mckornfield /Users /] INFO [config_reader] Used config file .golangci.yaml INFO [lintersdb] Active 9 linters: [asasalint bidichk exportloopref gofmt goimports gosec gosimple govet ineffassign] INFO [loader] Go packages loading at mode 575 (exports_file|files|name|compiled_files|deps|imports|types_sizes) took 1.302885625s INFO [runner/filename_unadjuster] Pre-built 0 adjustments in 33.868833ms INFO [linters_context/goanalysis] analyzers took 2m57.985359366s with top 10 stages: buildir: 1m51.322815792s, bidichk: 8.764515795s, goimports: 6.775047838s, ctrlflow: 4.59323989s, printf: 3.802954974s, inspect: 3.695072566s, fact_purity: 3.108077326s, gofmt: 2.768707716s, gosec: 1.990496875s, asasalint: 1.294749498s INFO [runner/skip_dirs] Skipped 17 issues from dir third_party/protoc-gen-openapi by pattern (^|/)third_party($|/) INFO [runner/skip_dirs] Skipped 1 issues from dir third_party/protoc-gen-openapi/generator by pattern (^|/)third_party($|/) INFO [runner/max_same_issues] 25/28 issues with text "missing type in composite literal" were hidden, use --max-same-issues INFO [runner] Issues before processing: 3528, after processing: 9 INFO [runner] Processors filtering stat (out/in): max_same_issues: 9/34, severity-rules: 9/9, path_prefixer: 9/9, sort_results: 9/9, invalid_issue: 3528/3528, skip_files: 3528/3528, max_per_file_from_linter: 34/34, filename_unadjuster: 3528/3528, nolint: 2550/2550, source_code: 9/9, exclude: 2593/2593, uniq_by_line: 34/2550, max_from_linter: 9/9, path_shortener: 9/9, path_prettifier: 3528/3528, skip_dirs: 3510/3528, autogenerated_exclude: 2593/3510, diff: 34/34, fixer: 9/9, cgo: 3528/3528, identifier_marker: 2593/2593, exclude-rules: 2550/2593 INFO [runner] processing took 68.514336ms with stages: exclude-rules: 28.324626ms, identifier_marker: 17.274459ms, autogenerated_exclude: 14.820417ms, path_prettifier: 4.6205ms, cgo: 1.216459ms, nolint: 1.181ms, skip_dirs: 695.959µs, source_code: 129.583µs, invalid_issue: 86.791µs, uniq_by_line: 76.041µs, filename_unadjuster: 73.75µs, max_same_issues: 8.792µs, max_per_file_from_linter: 1.792µs, max_from_linter: 1.583µs, path_shortener: 1.542µs, exclude: 334ns, fixer: 167ns, severity-rules: 166ns, sort_results: 125ns, diff: 84ns, path_prefixer: 83ns, skip_files: 83ns INFO [runner] linters took 16.362647s with stages: goanalysis_metalinter: 16.2939755s pipelinecontroller/argo/kubernetes.go:20:109: not enough arguments in call to utils.Must have (func[T any, O interface{*T; client.Object}](f k8sclient.Factory, opts ...client.ListOption) (k8sclient.Informer[T], error)) want (T, error) (typecheck) kubernetes.GetFactory(), client.InNamespace(namespace), client.MatchingLabelsSelector{Selector: labelSel})) ^ pipelinecontroller/workflow_runs.go:58:3: not enough arguments in call to utils.Must have (unknown type) want (T, error) (typecheck) )) ^ pipelinecontroller/workflow_runs.go:70:3: not enough arguments in call to utils.Must have (unknown type) want (T, error) (typecheck) )) ^ pipelinecontroller/pkg/argowfsecrets/extract_test.go:23:8: missing type in composite literal (typecheck) { ^ pipelinecontroller/pkg/argowfsecrets/extract_test.go:33:8: missing type in composite literal (typecheck) { ^ pipelinecontroller/pkg/argowfsecrets/extract_test.go:43:8: missing type in composite literal (typecheck) { ^ pipelinecontroller/workflowruns/compiler/template_test.go:103:2: expectedAwsConnString declared and not used (typecheck) expectedAwsConnString, _ := protos.ProtoToJson(buildAwsConnection()) ^ pipelinecontroller/workflowruns/compiler/template_test.go:104:2: expectedGretelConnString declared and not used (typecheck) expectedGretelConnString, _ := protos.ProtoToJson(buildGretelConnection()) ^ pipelinecontroller/workflowruns/compiler/dag.go:4:2: "fmt" imported and not used (typecheck) "fmt" ^ INFO File cache stats: 5 entries of total size 29.5KiB INFO Memory: 173 samples, avg is 1904.1MB, max is 3220.9MB INFO Execution took 17.733726167s make: *** [check-style] Error 1 ```

A minimal reproducible example or link to a public repository

```go package compiler import ( "fmt" wfv1 "github.com/argoproj/argo-workflows/v3/pkg/apis/workflow/v1alpha1" ) func (d *dagImpl) testMethod() (wfv1.DAGTask, error) { task := wfv1.DAGTask{ Name: "abc", Template: "abc", } task.Dependencies = []string{"abc"} task.Arguments = wfv1.Arguments{ Artifacts: wfv1.Artifacts{ { Name: d.options.ArtifactConfigProvider.RunManifestArtifactKey, From: fmt.Sprintf( "{{tasks.%s.outputs.artifacts.%s}}", "abc", d.options.ArtifactConfigProvider.RunManifestArtifactKey, ), }, }, } return task, nil } ```

Validation

boring-cyborg[bot] commented 3 months ago

Hey, thank you for opening your first Issue ! 🙂 If you would like to contribute we have a guide for contributors.

ldez commented 3 months ago
.golangci.yml ```yml linters: disable-all: true enable: - typecheck - govet - exportloopref - ineffassign - gosimple - gofmt - goimports - asasalint - bidichk - gosec # Enable these at a later point (fixing all issues produces a larger diff) # - errcheck # - unused # - staticcheck linters-settings: govet: settings: printf: funcs: - (github.com/gretellabs/monogretel/go/pkg/logging.Logger).Trace - (github.com/gretellabs/monogretel/go/pkg/logging.Logger).Info - (github.com/gretellabs/monogretel/go/pkg/logging.Logger).Warn - (github.com/gretellabs/monogretel/go/pkg/logging.Logger).Error gosimple: checks: - "all" - "-S1008" goimports: local-prefixes: github.com/gretellabs/monogretel gosec: excludes: - G404 # weak random number generator - G114 # Use of net/http serve function that has no support for setting timeouts ```
main.go ```go package compiler import ( "fmt" wfv1 "github.com/argoproj/argo-workflows/v3/pkg/apis/workflow/v1alpha1" ) type ArtifactConfigProvider struct { RunManifestArtifactKey string } type Options struct { ArtifactConfigProvider ArtifactConfigProvider } type dagImpl struct { options Options } func (d *dagImpl) testMethod() (wfv1.DAGTask, error) { task := wfv1.DAGTask{ Name: "abc", Template: "abc", } task.Dependencies = []string{"abc"} task.Arguments = wfv1.Arguments{ Artifacts: wfv1.Artifacts{ { Name: d.options.ArtifactConfigProvider.RunManifestArtifactKey, From: fmt.Sprintf( "{{tasks.%s.outputs.artifacts.%s}}", "abc", d.options.ArtifactConfigProvider.RunManifestArtifactKey, ), }, }, } return task, nil } ```
go.mod (google.golang.org/protobuf v1.33.0) ```go module github.com/golangci/sandbox go 1.21 require github.com/argoproj/argo-workflows/v3 v3.5.5 require ( github.com/davecgh/go-spew v1.1.2-0.20180830191138-d8f796af33cc // indirect github.com/emicklei/go-restful/v3 v3.10.0 // indirect github.com/go-logr/logr v1.2.3 // indirect github.com/go-openapi/jsonpointer v0.19.6 // indirect github.com/go-openapi/jsonreference v0.20.2 // indirect github.com/go-openapi/swag v0.22.3 // indirect github.com/gogo/protobuf v1.3.2 // indirect github.com/golang/protobuf v1.5.3 // indirect github.com/google/gnostic v0.6.9 // indirect github.com/google/gofuzz v1.2.0 // indirect github.com/grpc-ecosystem/grpc-gateway v1.16.0 // indirect github.com/josharian/intern v1.0.0 // indirect github.com/json-iterator/go v1.1.12 // indirect github.com/mailru/easyjson v0.7.7 // indirect github.com/modern-go/concurrent v0.0.0-20180306012644-bacd9c7ef1dd // indirect github.com/modern-go/reflect2 v1.0.2 // indirect github.com/sirupsen/logrus v1.9.3 // indirect golang.org/x/net v0.19.0 // indirect golang.org/x/oauth2 v0.13.0 // indirect golang.org/x/sys v0.15.0 // indirect golang.org/x/term v0.15.0 // indirect golang.org/x/text v0.14.0 // indirect golang.org/x/time v0.4.0 // indirect google.golang.org/appengine v1.6.8 // indirect google.golang.org/genproto v0.0.0-20231016165738-49dd2c1f3d0b // indirect google.golang.org/genproto/googleapis/api v0.0.0-20231016165738-49dd2c1f3d0b // indirect google.golang.org/genproto/googleapis/rpc v0.0.0-20231030173426-d783a09b4405 // indirect google.golang.org/grpc v1.59.0 // indirect google.golang.org/protobuf v1.31.0 // indirect gopkg.in/inf.v0 v0.9.1 // indirect gopkg.in/yaml.v2 v2.4.0 // indirect gopkg.in/yaml.v3 v3.0.1 // indirect k8s.io/api v0.24.3 // indirect k8s.io/apimachinery v0.24.3 // indirect k8s.io/client-go v0.24.3 // indirect k8s.io/klog/v2 v2.70.1 // indirect k8s.io/kube-openapi v0.0.0-20220627174259-011e075b9cb8 // indirect k8s.io/utils v0.0.0-20220713171938-56c0de1e6f5e // indirect sigs.k8s.io/json v0.0.0-20220713155537-f223a00ba0e2 // indirect sigs.k8s.io/structured-merge-diff/v4 v4.2.1 // indirect sigs.k8s.io/yaml v1.4.0 // indirect ) ``` 
$ go build   
# github.com/golang/protobuf/protoc-gen-go/descriptor
../../../../pkg/mod/github.com/golang/protobuf@v1.5.3/protoc-gen-go/descriptor/descriptor.pb.go:106:61: undefined: descriptorpb.Default_FileOptions_PhpGenericServi

It's not a golangci-lint problem, but a compilation problem related to compatibility of google.golang.org/protobuf with github.com/argoproj/argo-workflows

https://github.com/argoproj/argo-workflows/blob/c80b2e91ebd7e7f604e88442f45ec630380effa0/go.mod#L272