enable linting and bingo management of the lint tool (#1524)

* enable linting and bingo management of the lint tool

Signed-off-by: Jordan Keister <[email protected]>

* automated lint fixes

Signed-off-by: Jordan Keister <[email protected]>

* manual test fixes

Signed-off-by: Jordan <[email protected]>

* linter feature code
- errcheck
- stylecheck
- nestif
- prealloc
- errorlint
- nonamedreturns
- gosec
- gosimple
- govet
- importas
- staticcheck
- unused

Signed-off-by: Jordan Keister <[email protected]>

* update bingo to work with go version stampede

Signed-off-by: Jordan Keister <[email protected]>

* review revisions

Signed-off-by: Jordan Keister <[email protected]>

---------

Signed-off-by: Jordan Keister <[email protected]>
Signed-off-by: Jordan <[email protected]>

Author: grokspawn

.bingo/Variables.mk

@@ -29,6 +29,12 @@ $(GINKGO): $(BINGO_DIR)/ginkgo.mod
 	@echo "(re)installing $(GOBIN)/ginkgo-v2.22.2"
 	@cd $(BINGO_DIR) && GOWORK=off $(GO) build -mod=mod -modfile=ginkgo.mod -o=$(GOBIN)/ginkgo-v2.22.2 "github.com/onsi/ginkgo/v2/ginkgo"
 
+GOLANGCI_LINT := $(GOBIN)/golangci-lint-v1.63.4
+$(GOLANGCI_LINT): $(BINGO_DIR)/golangci-lint.mod
+	@# Install binary/ries using Go 1.14+ build command. This is using bwplotka/bingo-controlled, separate go module with pinned dependencies.
+	@echo "(re)installing $(GOBIN)/golangci-lint-v1.63.4"
+	@cd $(BINGO_DIR) && GOWORK=off $(GO) build -mod=mod -modfile=golangci-lint.mod -o=$(GOBIN)/golangci-lint-v1.63.4 "github.com/golangci/golangci-lint/cmd/golangci-lint"
+
 GORELEASER := $(GOBIN)/goreleaser-v1.26.2
 $(GORELEASER): $(BINGO_DIR)/goreleaser.mod
 	@# Install binary/ries using Go 1.14+ build command. This is using bwplotka/bingo-controlled, separate go module with pinned dependencies.

.bingo/golangci-lint.mod

@@ -0,0 +1,5 @@
+module _ // Auto generated by https://github.com/bwplotka/bingo. DO NOT EDIT
+
+go 1.22.5
+
+require github.com/golangci/golangci-lint v1.63.4 // cmd/golangci-lint

.bingo/golangci-lint.sum

@@ -12,6 +12,8 @@ BINGO="${GOBIN}/bingo-v0.9.0"
 
 GINKGO="${GOBIN}/ginkgo-v2.22.2"
 
+GOLANGCI_LINT="${GOBIN}/golangci-lint-v1.63.4"
+
 GORELEASER="${GOBIN}/goreleaser-v1.26.2"
 
 PROTOC_GEN_GO_GRPC="${GOBIN}/protoc-gen-go-grpc-v1.3.0"

.bingo/variables.env

@@ -0,0 +1,83 @@
+########
+# NOTE
+#
+#  This file is duplicated in the following repos:
+#    - operator-framework/kubectl-operator
+#    - operator-framework/catalogd
+#    - operator-framework/operator-controller
+#    - operator-framework/operator-registry
+#
+#  If you are making a change, please make it in ALL
+#  of the above repositories!
+#
+#  TODO: Find a way to have a shared golangci config.
+########
+
+run:
+  # Default timeout is 1m, up to give more room
+  timeout: 4m
+
+linters:
+  enable:
+  - asciicheck
+  - bodyclose
+  - errorlint
+  - gci
+  - gofmt
+  - govet
+  - gosec
+  - importas
+  - misspell
+  - nestif
+  - nonamedreturns
+  - prealloc
+  - stylecheck
+  - testifylint
+  - tparallel
+  - unconvert
+  - unparam
+  - unused
+  - whitespace
+
+linters-settings:
+  gci:
+    sections:
+      - standard
+      - dot
+      - default
+      - prefix(github.com/operator-framework)
+      - localmodule
+    custom-order: true
+
+  errorlint:
+    errorf: false
+
+  importas:
+    alias:
+    - pkg: k8s.io/apimachinery/pkg/apis/meta/v1
+      alias: metav1
+    - pkg: k8s.io/apimachinery/pkg/api/errors
+      alias: apierrors
+    - pkg: k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1
+      alias: apiextensionsv1
+    - pkg: k8s.io/apimachinery/pkg/util/runtime
+      alias: utilruntime
+    - pkg: "^k8s\\.io/api/([^/]+)/(v[^/]+)$"
+      alias: $1$2
+    - pkg: sigs.k8s.io/controller-runtime
+      alias: ctrl
+    - pkg: github.com/blang/semver/v4
+      alias: bsemver
+
+issues:
+  # exclusion rules, mostly to avoid functionally complete areas and reduce unit test noise
+  exclude-rules:
+    - path: _test\.go
+      linters:
+        - unused
+        - govet
+
+
+output:
+  formats:
+    - format: tab

.golangci.yaml

@@ -102,8 +102,14 @@ image-upstream:
 	docker build -f upstream-example.Dockerfile .
 
 .PHONY: lint
-lint:
-	find . -type f -name '*.go' ! -name '*.pb.go' -print0 | xargs -0 goimports -w
+#lint:
+#	find . -type f -name '*.go' ! -name '*.pb.go' -print0 | xargs -0 goimports -w
+lint: $(GOLANGCI_LINT)
+	$(GOLANGCI_LINT) run $(GOLANGCI_LINT_ARGS)
+
+.PHONY: fix-lint
+fix-lint: $(GOLANGCI_LINT)
+	$(GOLANGCI_LINT) run --fix $(GOLANGCI_LINT_ARGS)
 
 .PHONY: bingo-upgrade
 bingo-upgrade: $(BINGO) #EXHELP Upgrade tools

Makefile

@@ -125,7 +125,7 @@ func TestMigrate(t *testing.T) {
 				return
 			}
 			actualFS := os.DirFS(s.migrate.OutputDir)
-			fs.WalkDir(actualFS, ".", func(path string, d fs.DirEntry, err error) error {
+			require.NoError(t, fs.WalkDir(actualFS, ".", func(path string, d fs.DirEntry, err error) error {
 				require.NoError(t, err)
 				if d.IsDir() {
 					return nil
@@ -136,7 +136,7 @@ func TestMigrate(t *testing.T) {
 				require.True(t, ok, "output directory contained unexpected file %q", path)
 				require.Equal(t, expectedData, string(actualData))
 				return nil
-			})
+			}))
 		})
 	}
 }
@@ -291,7 +291,6 @@ relatedImages:
   name: operator
 schema: olm.bundle
 `
-
 }
 
 func migrateBarCatalogSqlite() string {

alpha/action/migrate_test.go

@@ -8,10 +8,12 @@ import (
 	"testing"
 
 	"github.com/google/go-cmp/cmp"
+	"github.com/stretchr/testify/require"
+
 	"github.com/operator-framework/api/pkg/operators/v1alpha1"
+
 	"github.com/operator-framework/operator-registry/alpha/declcfg"
 	"github.com/operator-framework/operator-registry/alpha/property"
-	"github.com/stretchr/testify/require"
 )
 
 func TestMigrations(t *testing.T) {
@@ -57,7 +59,7 @@ func TestMigrations(t *testing.T) {
 
 	for _, test := range tests {
 		t.Run(test.name, func(t *testing.T) {
-			var config declcfg.DeclarativeConfig = unmigratedCatalogFBC()
+			var config = unmigratedCatalogFBC()
 
 			for _, m := range test.migrators.Migrations {
 				err := m.Migrate(&config)

alpha/action/migrations/migrations_test.go

@@ -70,10 +70,13 @@ func (r Render) Run(ctx context.Context) (*declcfg.DeclarativeConfig, error) {
 		if err != nil {
 			return nil, fmt.Errorf("create registry: %v", err)
 		}
-		defer reg.Destroy()
+		defer func() {
+			_ = reg.Destroy()
+		}()
 		r.Registry = reg
 	}
 
+	// nolint:prealloc
 	var cfgs []declcfg.DeclarativeConfig
 	for _, ref := range r.Refs {
 		cfg, err := r.renderReference(ctx, ref)
@@ -123,6 +126,7 @@ func (r Render) renderReference(ctx context.Context, ref string) (*declcfg.Decla
 	if err != nil {
 		return r.imageToDeclcfg(ctx, ref)
 	}
+	// nolint:nestif
 	if stat.IsDir() {
 		dirEntries, err := os.ReadDir(ref)
 		if err != nil {
@@ -178,6 +182,7 @@ func (r Render) imageToDeclcfg(ctx context.Context, imageRef string) (*declcfg.D
 	}
 
 	var cfg *declcfg.DeclarativeConfig
+	// nolint:nestif
 	if dbFile, ok := labels[containertools.DbLocationLabel]; ok {
 		if !r.AllowedRefMask.Allowed(RefSqliteImage) {
 			return nil, fmt.Errorf("cannot render sqlite image: %w", ErrNotAllowed)
@@ -279,6 +284,7 @@ func populateDBRelatedImages(ctx context.Context, cfg *declcfg.DeclarativeConfig
 	}
 	defer rows.Close()
 
+	// nolint:staticcheck
 	images := map[string]sets.String{}
 	for rows.Next() {
 		var (
@@ -326,10 +332,10 @@ func bundleToDeclcfg(bundle *registry.Bundle) (*declcfg.Bundle, error) {
 		return nil, fmt.Errorf("get related images for bundle %q: %v", bundle.Name, err)
 	}
 
-	var csvJson []byte
+	var csvJSON []byte
 	for _, obj := range bundle.Objects {
 		if obj.GetKind() == "ClusterServiceVersion" {
-			csvJson, err = json.Marshal(obj)
+			csvJSON, err = json.Marshal(obj)
 			if err != nil {
 				return nil, fmt.Errorf("marshal CSV JSON for bundle %q: %v", bundle.Name, err)
 			}
@@ -344,7 +350,7 @@ func bundleToDeclcfg(bundle *registry.Bundle) (*declcfg.Bundle, error) {
 		Properties:    props,
 		RelatedImages: relatedImages,
 		Objects:       objs,
-		CsvJSON:       string(csvJson),
+		CsvJSON:       string(csvJSON),
 	}, nil
 }
 

alpha/action/render.go

@@ -4,7 +4,6 @@ import (
 	"context"
 	"embed"
 	"encoding/json"
-	"errors"
 	"fmt"
 	"io/fs"
 	"os"
@@ -13,7 +12,6 @@ import (
 	"testing/fstest"
 	"text/template"
 
-	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
 	"k8s.io/apimachinery/pkg/util/yaml"
 
@@ -86,11 +84,11 @@ func TestRender(t *testing.T) {
 		image.SimpleReference("test.registry/foo-operator/foo-bundle:v0.1.0"): "testdata/foo-bundle-v0.1.0",
 		image.SimpleReference("test.registry/foo-operator/foo-bundle:v0.2.0"): "testdata/foo-bundle-v0.2.0",
 	}
-	assert.NoError(t, generateSqliteFile(dbFile, imageMap))
+	require.NoError(t, generateSqliteFile(dbFile, imageMap))
 	testMigrations := migrations.Migrations{
 		Migrations: []migrations.Migration{
 			fauxMigration{"faux-migration", "my help text", func(d *declcfg.DeclarativeConfig) error {
-				for i, _ := range d.Bundles {
+				for i := range d.Bundles {
 					d.Bundles[i].Name = fmt.Sprintf("%s-MIGRATED", d.Bundles[i].Name)
 				}
 				return nil
@@ -1235,7 +1233,7 @@ func TestAllowRefMask(t *testing.T) {
 		image.SimpleReference("test.registry/foo-operator/foo-bundle:v0.1.0"): "testdata/foo-bundle-v0.1.0",
 		image.SimpleReference("test.registry/foo-operator/foo-bundle:v0.2.0"): "testdata/foo-bundle-v0.2.0",
 	}
-	assert.NoError(t, generateSqliteFile(dbFile, imageMap))
+	require.NoError(t, generateSqliteFile(dbFile, imageMap))
 
 	specs := []spec{
 		{
@@ -1365,18 +1363,17 @@ func TestAllowRefMask(t *testing.T) {
 	for _, s := range specs {
 		t.Run(s.name, func(t *testing.T) {
 			_, err := s.render.Run(context.Background())
-			require.True(t, errors.Is(err, s.expectErr), "expected error %#v to be %#v", err, s.expectErr)
+			require.ErrorIs(t, err, s.expectErr, "expected error %#v to be %#v", err, s.expectErr)
 		})
 	}
 }
 
 func TestAllowRefMaskAllowed(t *testing.T) {
 	type spec struct {
-		name   string
-		mask   action.RefType
-		pass   []action.RefType
-		fail   []action.RefType
-		expect bool
+		name string
+		mask action.RefType
+		pass []action.RefType
+		fail []action.RefType
 	}
 
 	specs := []spec{