Merge pull request #2173 from shiftstack/linter

🌱 Bump golangci-lint to v1.61.0

Author: k8s-ci-robot

.golangci.yml

@@ -5,13 +5,12 @@ linters:
   - asciicheck
   - bidichk
   - bodyclose
+  - copyloopvar
   - cyclop
-#  - depguard
   - dogsled
   - dupword
   - durationcheck
   - errcheck
-  - exportloopref
   - forbidigo
   - gci
   - goconst

controllers/openstackcluster_controller.go

@@ -20,6 +20,7 @@ import (
 	"context"
 	"errors"
 	"fmt"
+	"math"
 	"time"
 
 	"github.com/gophercloud/gophercloud/v2/openstack/networking/v2/networks"
@@ -759,7 +760,10 @@ func reconcileProvisionedNetworkComponents(networkingService *networking.Service
 // cluster spec.
 func reconcileControlPlaneEndpoint(scope *scope.WithLogger, networkingService *networking.Service, openStackCluster *infrav1.OpenStackCluster, clusterResourceName string) error {
 	// Calculate the port that we will use for the API server
-	apiServerPort := getAPIServerPort(openStackCluster)
+	apiServerPort, err := getAPIServerPort(openStackCluster)
+	if err != nil {
+		return err
+	}
 
 	// host must be set by a matching control plane endpoint provider below
 	var host string
@@ -774,7 +778,7 @@ func reconcileControlPlaneEndpoint(scope *scope.WithLogger, networkingService *n
 			return err
 		}
 
-		terminalFailure, err := loadBalancerService.ReconcileLoadBalancer(openStackCluster, clusterResourceName, apiServerPort)
+		terminalFailure, err := loadBalancerService.ReconcileLoadBalancer(openStackCluster, clusterResourceName, int(apiServerPort))
 		if err != nil {
 			handleUpdateOSCError(openStackCluster, fmt.Errorf("failed to reconcile load balancer: %w", err), terminalFailure)
 			return fmt.Errorf("failed to reconcile load balancer: %w", err)
@@ -819,21 +823,27 @@ func reconcileControlPlaneEndpoint(scope *scope.WithLogger, networkingService *n
 
 	openStackCluster.Spec.ControlPlaneEndpoint = &clusterv1.APIEndpoint{
 		Host: host,
-		Port: int32(apiServerPort),
+		Port: apiServerPort,
 	}
 
 	return nil
 }
 
 // getAPIServerPort returns the port to use for the API server based on the cluster spec.
-func getAPIServerPort(openStackCluster *infrav1.OpenStackCluster) int {
+func getAPIServerPort(openStackCluster *infrav1.OpenStackCluster) (int32, error) {
 	switch {
 	case openStackCluster.Spec.ControlPlaneEndpoint != nil && openStackCluster.Spec.ControlPlaneEndpoint.IsValid():
-		return int(openStackCluster.Spec.ControlPlaneEndpoint.Port)
+		return openStackCluster.Spec.ControlPlaneEndpoint.Port, nil
 	case openStackCluster.Spec.APIServerPort != nil:
-		return *openStackCluster.Spec.APIServerPort
+		// XXX: This highlights that we have missing validation on
+		// APIServerPort. We should ideally add validation and change its type
+		// to int32.
+		if *openStackCluster.Spec.APIServerPort > math.MaxInt32 {
+			return 0, fmt.Errorf("value of apiServerPort is larger than %d", math.MaxInt32)
+		}
+		return int32(*openStackCluster.Spec.APIServerPort), nil //nolint:gosec
 	}
-	return 6443
+	return 6443, nil
 }
 
 func (r *OpenStackClusterReconciler) SetupWithManager(ctx context.Context, mgr ctrl.Manager, options controller.Options) error {

controllers/openstackcluster_controller_test.go

@@ -467,7 +467,7 @@ func Test_getAPIServerPort(t *testing.T) {
 	tests := []struct {
 		name             string
 		openStackCluster *infrav1.OpenStackCluster
-		want             int
+		want             int32
 	}{
 		{
 			name:             "default",
@@ -498,7 +498,7 @@ func Test_getAPIServerPort(t *testing.T) {
 	}
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
-			if got := getAPIServerPort(tt.openStackCluster); got != tt.want {
+			if got, _ := getAPIServerPort(tt.openStackCluster); got != tt.want {
 				t.Errorf("getAPIServerPort() = %v, want %v", got, tt.want)
 			}
 		})

controllers/openstackfloatingippool_controller.go

@@ -125,7 +125,6 @@ func (r *OpenStackFloatingIPPoolReconciler) Reconcile(ctx context.Context, req c
 	}
 
 	for _, claim := range claims.Items {
-		claim := claim
 		log := log.WithValues("claim", claim.Name)
 		if !claim.ObjectMeta.DeletionTimestamp.IsZero() {
 			continue

controllers/openstackserver_controller.go

@@ -117,7 +117,7 @@ func (r *OpenStackServerReconciler) Reconcile(ctx context.Context, req ctrl.Requ
 		// Propagate terminal errors
 		terminalError := &capoerrors.TerminalError{}
 		if errors.As(reterr, &terminalError) {
-			conditions.MarkFalse(openStackServer, infrav1.InstanceReadyCondition, terminalError.Reason, clusterv1.ConditionSeverityError, terminalError.Message)
+			conditions.MarkFalse(openStackServer, infrav1.InstanceReadyCondition, terminalError.Reason, clusterv1.ConditionSeverityError, "%s", terminalError.Message)
 		}
 
 		if err := patchServer(ctx, patchHelper, openStackServer); err != nil {
@@ -465,7 +465,7 @@ func (r *OpenStackServerReconciler) getOrCreateServer(ctx context.Context, logge
 		instanceStatus, err = computeService.GetInstanceStatus(*openStackServer.Status.InstanceID)
 		if err != nil {
 			logger.Info("Unable to get OpenStack instance", "name", openStackServer.Name)
-			conditions.MarkFalse(openStackServer, infrav1.InstanceReadyCondition, infrav1.OpenStackErrorReason, clusterv1.ConditionSeverityError, err.Error())
+			conditions.MarkFalse(openStackServer, infrav1.InstanceReadyCondition, infrav1.OpenStackErrorReason, clusterv1.ConditionSeverityError, "%s", err.Error())
 			return nil, err
 		}
 	}
@@ -494,7 +494,7 @@ func (r *OpenStackServerReconciler) getOrCreateServer(ctx context.Context, logge
 		instanceSpec.Name = openStackServer.Name
 		instanceStatus, err = computeService.CreateInstance(openStackServer, instanceSpec, portIDs)
 		if err != nil {
-			conditions.MarkFalse(openStackServer, infrav1.InstanceReadyCondition, infrav1.InstanceCreateFailedReason, clusterv1.ConditionSeverityError, err.Error())
+			conditions.MarkFalse(openStackServer, infrav1.InstanceReadyCondition, infrav1.InstanceCreateFailedReason, clusterv1.ConditionSeverityError, "%s", err.Error())
 			openStackServer.Status.InstanceState = &infrav1.InstanceStateError
 			return nil, fmt.Errorf("create OpenStack instance: %w", err)
 		}

hack/tools/Makefile

@@ -15,7 +15,7 @@
 ROOT_DIR_RELATIVE := ../..
 include $(ROOT_DIR_RELATIVE)/common.mk
 
-GOLANGCI_LINT_VERSION ?= v1.57.2
+GOLANGCI_LINT_VERSION ?= v1.61.0
 
 # GOTESTSUM version without the leading 'v'
 GOTESTSUM_VERSION ?= 1.11.0

orc/.golangci.yml

@@ -19,9 +19,9 @@ issues:
 linters:
   disable-all: true
   enable:
+    - copyloopvar
     - dupl
     - errcheck
-    - exportloopref
     - ginkgolinter
     - goconst
     - gocyclo

orc/Makefile

@@ -177,7 +177,7 @@ GOLANGCI_LINT = $(LOCALBIN)/golangci-lint
 KUSTOMIZE_VERSION ?= v5.4.2
 CONTROLLER_TOOLS_VERSION ?= v0.15.0
 ENVTEST_VERSION ?= release-0.18
-GOLANGCI_LINT_VERSION ?= v1.59.1
+GOLANGCI_LINT_VERSION ?= v1.61.0
 
 .PHONY: kustomize
 kustomize: $(KUSTOMIZE) ## Download kustomize locally if necessary.

pkg/webhooks/openstackmachinetemplate_webhook_test.go

@@ -184,7 +184,6 @@ func TestOpenStackMachineTemplate_ValidateUpdate(t *testing.T) {
 	}
 
 	for _, tt := range tests {
-		tt := tt
 		t.Run(tt.name, func(t *testing.T) {
 			t.Parallel()
 

pkg/webhooks/openstackserver_webhook_test.go

@@ -112,7 +112,6 @@ func TestOpenStackServer_ValidateUpdate(t *testing.T) {
 	}
 
 	for _, tt := range tests {
-		tt := tt
 		t.Run(tt.name, func(t *testing.T) {
 			t.Parallel()
 

test/e2e/shared/exec.go

@@ -146,7 +146,7 @@ func executeCommand(ctx context.Context, p commandParameter) error {
 		return fmt.Errorf("unable to send command %q: %s", "sudo "+p.cmd.cmd, err)
 	}
 	result := strings.TrimSuffix(stdoutBuf.String(), "\n") + "\n" + strings.TrimSuffix(stderrBuf.String(), "\n")
-	if err := os.WriteFile(logFile, []byte(result), os.ModePerm); err != nil {
+	if err := os.WriteFile(logFile, []byte(result), 0o600); err != nil {
 		return fmt.Errorf("error writing log file: %s", err)
 	}
 	Debugf(p.debug, "finished executing cmd %q on machine %s", p.cmd.cmd, p.machineIP)