Bump golangci-lint to v2 (#2979)

* Remove errant unicode

Signed-off-by: Stephen Finucane <stephenfin@redhat.com>

* Bump golangci-lint to latest version

Done by bumping the version in the Makefile and running the
'golangci-lint migrate' command

  go run github.com/golangci/golangci-lint/v2/cmd/golangci-lint@v2.3.1 migrate

This yields a number of news error from staticcheck, listed below, which
we temporarily ignore pending a fix.

  ST1005: error strings should not be capitalized
  ST1023: should omit type .* from declaration; it will be inferred from the right-hand side
  QF1001: could apply De Morgan's law
  QF1003: could use tagged switch on .*
  QF1004: could use strings.ReplaceAll instead
  QF1004: could merge conditional assignment into variable declaration
  QF1008: could remove embedded field ".*" from selector
  QF1011: could omit type .* from declaration; it will be inferred from the right-hand side

We also add some additional configuration to remove the limits on the
number of issues shown by linters, since this will make fixing these
things easier, and remove configuration around concurrency since this
will be auto-configured by default.

Signed-off-by: Stephen Finucane <stephenfin@redhat.com>

* lint: Use lowercase error strings

Signed-off-by: Stephen Finucane <stephenfin@redhat.com>

* lint: Remove redundant type info

Signed-off-by: Stephen Finucane <stephenfin@redhat.com>

* lint: Prefer switch statements over conditional ladders

Signed-off-by: Stephen Finucane <stephenfin@redhat.com>

* lint: Simplify conditionals

Signed-off-by: Stephen Finucane <stephenfin@redhat.com>

* lint: Prefer strings.ReplaceAll to string.Replace with n=-1

Signed-off-by: Stephen Finucane <stephenfin@redhat.com>

* lint: Remove embedded fields from selectors

Signed-off-by: Stephen Finucane <stephenfin@redhat.com>

---------

Signed-off-by: Stephen Finucane <stephenfin@redhat.com>

Author: stephenfin

.golangci.yaml

@@ -1,10 +1,30 @@
-run:
-  concurrency: 2
-  timeout: 10m0s
+version: "2"
+output:
+  formats:
+    text:
+      path: stdout
+      colors: false
+issues:
+  max-same-issues: 0
+  max-issues-per-linter: 0
 linters:
+  exclusions:
+    generated: lax
+    presets:
+      - comments
+      - common-false-positives
+      - legacy
+      - std-error-handling
+    paths:
+      - third_party$
+      - builtin$
+      - examples$
+formatters:
   enable:
     - gofmt
-output:
-  formats:
-    - format: line-number
-      path: stdout
+  exclusions:
+    generated: lax
+    paths:
+      - third_party$
+      - builtin$
+      - examples$

Makefile

@@ -81,7 +81,7 @@ $(BUILD_CMDS): $(SOURCES)
 test: unit functional
 
 check: work
-	go run github.com/golangci/golangci-lint/cmd/golangci-lint@v1.63.4 run --timeout=20m ./...
+	go run github.com/golangci/golangci-lint/v2/cmd/golangci-lint@v2.3.1 run --timeout=20m ./...
 
 unit: work
 	go test -tags=unit $(shell go list ./... | sed -e '/sanity/ { N; d; }' | sed -e '/tests/ {N; d;}') $(TESTARGS)

pkg/autohealing/cloudprovider/openstack/provider.go

@@ -584,7 +584,7 @@ func (provider CloudProvider) UpdateHealthStatus(ctx context.Context, masters []
 	if err != nil {
 		return fmt.Errorf("failed to build health status reason for cluster %s, error: %v", clusterName, err)
 	}
-	healthStatusReason := strings.Replace(string(jsonDumps), "\"", "'", -1)
+	healthStatusReason := strings.ReplaceAll(string(jsonDumps), "\"", "'")
 
 	updateOpts := []clusters.UpdateOptsBuilder{
 		clusters.UpdateOpts{

pkg/autohealing/healthcheck/plugin_endpoint.go

@@ -139,12 +139,13 @@ func (check *EndpointCheck) Check(ctx context.Context, node NodeInfo, controller
 
 	var client *http.Client
 	protocol := strings.ToLower(check.Protocol)
-	if protocol == "https" {
+	switch protocol {
+	case "https":
 		tr := &http.Transport{
 			TLSClientConfig: &tls.Config{InsecureSkipVerify: true},
 		}
 		client = &http.Client{Transport: tr, Timeout: time.Second * 5}
-	} else if protocol == "http" {
+	case "http":
 		client = &http.Client{Timeout: time.Second * 5}
 	}
 

pkg/csi/cinder/openstack/openstack_volumes.go

@@ -183,7 +183,7 @@ func (os *OpenStack) DeleteVolume(ctx context.Context, volumeID string) error {
 		return err
 	}
 	if used {
-		return fmt.Errorf("Cannot delete the volume %q, it's still attached to a node", volumeID)
+		return fmt.Errorf("cannot delete the volume %q, it's still attached to a node", volumeID)
 	}
 
 	mc := metrics.NewMetricContext("volume", "delete")
@@ -259,7 +259,7 @@ func (os *OpenStack) WaitDiskAttached(ctx context.Context, instanceID string, vo
 	})
 
 	if wait.Interrupted(err) {
-		err = fmt.Errorf("Volume %q failed to be attached within the allotted time", volumeID)
+		err = fmt.Errorf("volume %q failed to be attached within the allotted time", volumeID)
 	}
 
 	return err
@@ -285,14 +285,14 @@ func (os *OpenStack) WaitVolumeTargetStatus(ctx context.Context, volumeID string
 		}
 		for _, eState := range volumeErrorStates {
 			if vol.Status == eState {
-				return false, fmt.Errorf("Volume is in Error State : %s", vol.Status)
+				return false, fmt.Errorf("volume is in Error State : %s", vol.Status)
 			}
 		}
 		return false, nil
 	})
 
 	if wait.Interrupted(waitErr) {
-		waitErr = fmt.Errorf("Timeout on waiting for volume %s status to be in %v", volumeID, tStatus)
+		waitErr = fmt.Errorf("timeout on waiting for volume %s status to be in %v", volumeID, tStatus)
 	}
 
 	return waitErr
@@ -352,7 +352,7 @@ func (os *OpenStack) WaitDiskDetached(ctx context.Context, instanceID string, vo
 	})
 
 	if wait.Interrupted(err) {
-		err = fmt.Errorf("Volume %q failed to detach within the allotted time", volumeID)
+		err = fmt.Errorf("volume %q failed to detach within the allotted time", volumeID)
 	}
 
 	return err

pkg/csi/cinder/utils.go

@@ -86,7 +86,7 @@ func ParseEndpoint(ep string) (string, string, error) {
 			return s[0], s[1], nil
 		}
 	}
-	return "", "", fmt.Errorf("Invalid endpoint: %v", ep)
+	return "", "", fmt.Errorf("invalid endpoint: %v", ep)
 }
 
 func logGRPC(ctx context.Context, req interface{}, info *grpc.UnaryServerInfo, handler grpc.UnaryHandler) (interface{}, error) {

pkg/csi/manila/manilaclient/builder.go

@@ -66,14 +66,14 @@ func New(ctx context.Context, o *client.AuthOpts, userAgent string, extraUserAge
 		Availability: o.EndpointType,
 	})
 	if err != nil {
-		return nil, fmt.Errorf("failed to create Manila v2 client: %v", err)
+		return nil, fmt.Errorf("failed to create manila v2 client: %v", err)
 	}
 
 	// Check client's and server's versions for compatibility
 
 	client.Microversion = minimumManilaVersion
 	if err = validateManilaClient(ctx, client); err != nil {
-		return nil, fmt.Errorf("Manila v2 client validation failed: %v", err)
+		return nil, fmt.Errorf("manila v2 client validation failed: %v", err)
 	}
 
 	return &Client{c: client}, nil

pkg/identity/keystone/authenticator.go

@@ -54,7 +54,7 @@ func NewKeystoner(client *gophercloud.ServiceClient) *Keystoner {
 
 // revive:disable:unexported-return
 func (k *Keystoner) GetTokenInfo(ctx context.Context, token string) (*tokenInfo, error) {
-	k.client.ProviderClient.SetToken(token)
+	k.client.SetToken(token)
 	ret := tokens.Get(ctx, k.client, token)
 
 	tokenUser, err := ret.ExtractUser()
@@ -94,7 +94,7 @@ func (k *Keystoner) GetTokenInfo(ctx context.Context, token string) (*tokenInfo,
 // revive:enable:unexported-return
 
 func (k *Keystoner) GetGroups(ctx context.Context, token string, userID string) ([]string, error) {
-	k.client.ProviderClient.SetToken(token)
+	k.client.SetToken(token)
 	allGroupPages, err := users.ListGroups(k.client, userID).AllPages(ctx)
 	if err != nil {
 		return nil, fmt.Errorf("failed to get user groups from Keystone: %v", err)

pkg/identity/keystone/authorizer.go

@@ -64,7 +64,7 @@ func getAllowed(definition string, str string) (sets.Set[string], error) {
 	} else if strings.Index(definition, "[") == 0 && strings.Index(definition, "]") == (len(definition)-1) {
 		// "['namespace1', 'namespace2']"
 		var items []string
-		if err := json.Unmarshal([]byte(strings.Replace(definition, "'", "\"", -1)), &items); err != nil {
+		if err := json.Unmarshal([]byte(strings.ReplaceAll(definition, "'", "\"")), &items); err != nil {
 			klog.V(4).Infof("Skip the permission definition %s", definition)
 			return nil, fmt.Errorf("")
 		}
@@ -78,7 +78,7 @@ func getAllowed(definition string, str string) (sets.Set[string], error) {
 	} else if strings.Index(definition, "!") == 0 && strings.Index(definition, "[") == 1 && strings.Index(definition, "]") == (len(definition)-1) {
 		// "!['namespace1', 'namespace2']"
 		var items []string
-		if err := json.Unmarshal([]byte(strings.Replace(definition[1:], "'", "\"", -1)), &items); err != nil {
+		if err := json.Unmarshal([]byte(strings.ReplaceAll(definition[1:], "'", "\"")), &items); err != nil {
 			klog.V(4).Infof("Skip the permission definition %s", definition)
 			return nil, fmt.Errorf("")
 		}
@@ -219,8 +219,8 @@ func nonResourceMatches(p policy, a authorizer.Attributes) bool {
 	}
 	if *p.NonResourceSpec.NonResourcePath != "*" && *p.NonResourceSpec.NonResourcePath != a.GetPath() &&
 		// Allow a trailing * subpath match
-		!(strings.HasSuffix(*p.NonResourceSpec.NonResourcePath, "*") &&
-			strings.HasPrefix(a.GetPath(), strings.TrimRight(*p.NonResourceSpec.NonResourcePath, "*"))) {
+		(!strings.HasSuffix(*p.NonResourceSpec.NonResourcePath, "*") ||
+			!strings.HasPrefix(a.GetPath(), strings.TrimRight(*p.NonResourceSpec.NonResourcePath, "*"))) {
 		return false
 	}
 	allowed := match(p.Match, a)

pkg/identity/keystone/keystone.go

@@ -265,7 +265,8 @@ func (k *Auth) Handler(w http.ResponseWriter, r *http.Request) {
 		return
 	}
 
-	if kind == "TokenReview" {
+	switch kind {
+	case "TokenReview":
 		var token = data["spec"].(map[string]interface{})["token"].(string)
 		userInfo := k.authenticateToken(ctx, w, r, token, data)
 
@@ -277,9 +278,9 @@ func (k *Auth) Handler(w http.ResponseWriter, r *http.Request) {
 				klog.Errorf("an error occurred during data synchronization: %v", err)
 			}
 		}
-	} else if kind == "SubjectAccessReview" {
+	case "SubjectAccessReview":
 		k.authorizeToken(w, r, data)
-	} else {
+	default:
 		http.Error(w, fmt.Sprintf("unknown kind/apiVersion %q %q", kind, apiVersion), http.StatusBadRequest)
 	}
 }