chore: adjust linting configuration

.golangci.yml

@@ -1,55 +1,97 @@
 version: "2"
 run:
   allow-parallel-runners: true
+  build-tags:
+    - e2e
 linters:
   default: none
   enable:
-    - copyloopvar
-    - dupl
-    - errcheck
-    - ginkgolinter
-    - gocyclo
-    - govet
-    - ineffassign
-    - lll
-    - modernize
-    - misspell
-    - nakedret
-    - prealloc
-    - revive
-    - staticcheck
-    - unconvert
-    - unparam
-    - unused
+    - asasalint # warns about passing []any to func(...any) without expanding it
+    - asciicheck # non ascii symbols
+    - copyloopvar # copying loop variables
+    - errcheck  # unchecked errors
+    - exhaustive # exhaustiveness of enum switch statements
+    - ginkgolinter # ginkgo and gomega - only if ginkgo/gomega is used
+    - gocritic # bugs, performance, style
+    - gocyclo # cyclomatic complexity of functions
+    - godoclint # go documentation linting against best practices
+    - gosec # potential security problems
+    - govet # basically 'go vet'
+    - importas # enforces consistent import aliases.
+    - ineffassign # ineffectual assignments
+    - loggercheck # check for even key/value pairs in logger calls
+    - modernize # suggest simplifications to Go code, using modern language and library features
+    - misspell # spelling checks
+    - nakedret  # naked returns (named return parameters and an empty return)
+    - noctx  # http requests without context.Context
+    - nolintlint  # badly formatted nolint directives
+    - predeclared # shadowing predeclared identifiers
+    - promlinter # only if prom is used for creating metrics
+    - revive # better version of golint
+    - spancheck # only if OpenTelemetry is used
+    - staticcheck # many static checks
+    - thelper # test helpers not starting with t.Helper()
+    - unconvert # unnecessary type conversions
+    - unparam # unused function parameters
+    - unused # unused constants, variables,functions, types
+    - usestdlibvars # using variables/constants from the standard library
+    - usetesting # reports uses of functions with replacement inside the testing package
+    - whitespace # unnecessary newlines
   settings:
-    revive:
-      rules:
-        - name: comment-spacings
-        - name: import-shadowing
+    loggercheck:
+      kitlog: false
+      slog: false
+      zap: false
+      require-string-key: true
+      no-printf-like: true
     modernize:
       disable:
         - omitzero
+    gocritic:
+      disabled-checks:
+        - ifElseChain # disabled ifElseChain because this is purely stylistic
+    revive:
+      rules:
+        # The following rules are recommended https://github.com/mgechev/revive#recommended-configuration
+        - name: blank-imports
+        - name: context-as-argument
+        - name: context-keys-type
+        - name: error-return
+        - name: error-strings
+        - name: error-naming
+        - name: if-return
+        - name: increment-decrement
+        - name: var-naming
+        - name: var-declaration
+        - name: range
+        - name: receiver-naming
+        - name: time-naming
+        - name: unexported-return
+        - name: indent-error-flow
+        - name: errorf
+        - name: empty-block
+        - name: superfluous-else
+        - name: unreachable-code
+        - name: redefines-builtin-id
+    staticcheck:
+      dot-import-whitelist:
+        - fmt
+        - github.com/onsi/gomega
+        - github.com/onsi/ginkgo/v2
   exclusions:
     generated: lax
-    rules:
-      - linters:
-          - lll
-        path: api/*
-      - linters:
-          - dupl
-          - lll
-        path: internal/*
-    paths:
-      - third_party$
-      - builtin$
-      - examples$
+    warn-unused: true
 formatters:
   enable:
-    - gofmt
-    - goimports
+    - goimports # ensures imports are organized
+    - gofmt # Check if the code is formatted according to 'gofmt' command.
+  settings:
+    goimports:
+      # A list of prefixes, which, if set, checks import paths
+      # with the given prefixes are grouped after 3rd-party packages.
+      # Default: []
+      local-prefixes:
+        - github.com/cloudscale-ch/cluster-api-provider-cloudscale
   exclusions:
     generated: lax
-    paths:
-      - third_party$
-      - builtin$
-      - examples$
+    warn-unused: true

internal/cloudscale/client.go

@@ -132,7 +132,7 @@ func IsNotFound(err error) bool {
 	if ok := errors.As(err, &errResp); !ok {
 		return false
 	}
-	return errResp.StatusCode == 404
+	return errResp.StatusCode == http.StatusNotFound
 }
 
 // IsFloatingIPNoPublicInterface returns true if the error indicates the target

internal/cloudscale/flavors.go

@@ -48,7 +48,7 @@ func (fi *FlavorInfo) GetFlavor(slug string) *cloudscalesdk.Flavor {
 
 // GetCapacity returns the resource capacity for a flavor slug.
 // rootVolumeSizeGB is used to calculate ephemeral-storage.
-func (fi *FlavorInfo) GetCapacity(slug string, rootVolumeSizeGB int) (corev1.ResourceList, error) {
+func (fi *FlavorInfo) GetCapacity(slug string) (corev1.ResourceList, error) {
 	flavor, ok := fi.flavors[slug]
 	if !ok {
 		return nil, fmt.Errorf("unknown flavor: %s", slug)

internal/cloudscale/flavors_test.go

@@ -10,7 +10,6 @@ import (
 )
 
 func TestFlavorInfo(t *testing.T) {
-	g := NewWithT(t)
 	flavors := []cloudscalesdk.Flavor{
 		{
 			Slug:      "small",
@@ -30,12 +29,14 @@ func TestFlavorInfo(t *testing.T) {
 	fi := NewFlavorInfo(flavors)
 
 	t.Run("IsValidFlavor", func(t *testing.T) {
+		g := NewWithT(t)
 		g.Expect(fi.IsValidFlavor("small")).To(BeTrue())
 		g.Expect(fi.IsValidFlavor("gpu-large")).To(BeTrue())
 		g.Expect(fi.IsValidFlavor("non-existent")).To(BeFalse())
 	})
 
 	t.Run("GetFlavor", func(t *testing.T) {
+		g := NewWithT(t)
 		f := fi.GetFlavor("small")
 		g.Expect(f).NotTo(BeNil())
 		g.Expect(f.Slug).To(Equal("small"))
@@ -45,28 +46,30 @@ func TestFlavorInfo(t *testing.T) {
 	})
 
 	t.Run("GetCapacity", func(t *testing.T) {
+		g := NewWithT(t)
 		// Test standard flavor
-		capSmall, err := fi.GetCapacity("small", 20)
+		capSmall, err := fi.GetCapacity("small")
 		g.Expect(err).NotTo(HaveOccurred())
 		g.Expect(capSmall[corev1.ResourceCPU]).To(Equal(resource.MustParse("2")))
 		g.Expect(capSmall[corev1.ResourceMemory]).To(Equal(resource.MustParse("4Gi")))
 		g.Expect(capSmall[ResourceNvidiaGPU]).To(BeZero())
 
 		// Test GPU flavor
-		capGPU, err := fi.GetCapacity("gpu-large", 100)
+		capGPU, err := fi.GetCapacity("gpu-large")
 		g.Expect(err).NotTo(HaveOccurred())
 		g.Expect(capGPU[corev1.ResourceCPU]).To(Equal(resource.MustParse("8")))
 		g.Expect(capGPU[corev1.ResourceMemory]).To(Equal(resource.MustParse("32Gi")))
 		g.Expect(capGPU[ResourceNvidiaGPU]).To(Equal(resource.MustParse("1")))
 
 		// Test unknown flavor
-		capUnknown, err := fi.GetCapacity("unknown", 20)
+		capUnknown, err := fi.GetCapacity("unknown")
 		g.Expect(err).To(HaveOccurred())
 		g.Expect(capUnknown).To(BeNil())
 		g.Expect(err.Error()).To(ContainSubstring("unknown flavor: unknown"))
 	})
 
 	t.Run("GetAllFlavors", func(t *testing.T) {
+		g := NewWithT(t)
 		slugs := fi.GetAllFlavors()
 		g.Expect(slugs).To(HaveLen(2))
 		g.Expect(slugs).To(ContainElement("small"))

internal/controller/cloudscalecluster_controller.go

@@ -194,8 +194,6 @@ func (r *CloudscaleClusterReconciler) reconcileNormal(ctx context.Context, clust
 
 // reconcileDelete handles deletion of cloudscale.ch infrastructure.
 // Resources are deleted in reverse order of creation: Load Balancer -> Network.
-//
-//nolint:unparam // Returns ctrl.Result for consistency with reconcile pattern
 func (r *CloudscaleClusterReconciler) reconcileDelete(ctx context.Context, clusterScope *scope.ClusterScope) (ctrl.Result, error) {
 	ctx, logger, done := observability.StartSpanWithLogger(ctx, "controllers.CloudscaleClusterReconciler.reconcileDelete")
 	defer done()

internal/controller/cloudscalecluster_network_test.go

@@ -1008,7 +1008,7 @@ func TestDeleteNetwork_RequeuesOnLBPoolMembersError(t *testing.T) {
 	networkService := &testutils.MockNetworkService{
 		DeleteFn: func(ctx context.Context, id string) error {
 			//goland:noinspection GoErrorStringFormat
-			return fmt.Errorf("There are still one or more load balancer pool members in this network.")
+			return fmt.Errorf("There are still one or more load balancer pool members in this network.") //nolint:revive // this is an actual response from the API
 		},
 	}
 

internal/controller/cloudscalemachine_controller.go

@@ -249,8 +249,6 @@ func (r *CloudscaleMachineReconciler) setReadyCondition(machine *infrastructurev
 }
 
 // reconcileDelete handles deletion of CloudscaleMachine.
-//
-//nolint:unparam // Returns ctrl.Result for consistency with reconcile pattern
 func (r *CloudscaleMachineReconciler) reconcileDelete(ctx context.Context, machineScope *scope.MachineScope) (ctrl.Result, error) {
 	ctx, logger, done := observability.StartSpanWithLogger(ctx, "controllers.CloudscaleMachineReconciler.reconcileDelete")
 	defer done()

internal/controller/cloudscalemachine_server.go

@@ -351,7 +351,7 @@ func ipFamilyToUseIPV6(ipFamily *infrastructurev1beta2.IPFamily) *bool {
 		return nil
 	}
 	switch *ipFamily {
-	case infrastructurev1beta2.IPFamilyDualStack:
+	case infrastructurev1beta2.IPFamilyDualStack, infrastructurev1beta2.IPFamilyIPv6:
 		return new(true)
 	case infrastructurev1beta2.IPFamilyIPv4:
 		return new(false)

internal/controller/cloudscalemachinetemplate_controller.go

@@ -73,7 +73,7 @@ func (r *CloudscaleMachineTemplateReconciler) Reconcile(ctx context.Context, req
 	}
 
 	// Get capacity for the flavor
-	capacity, err := r.FlavorInfo.GetCapacity(flavor, template.Spec.Template.Spec.RootVolumeSize)
+	capacity, err := r.FlavorInfo.GetCapacity(flavor)
 	if err != nil {
 		// Unknown flavor - don't populate capacity
 		// flavor is already validated in webhook - this branch is only here for defensive coding reasons

internal/credentials/credentials_test.go

@@ -29,7 +29,6 @@ import (
 )
 
 func TestGetToken(t *testing.T) {
-
 	tests := []struct {
 		name      string
 		secret    *corev1.Secret
@@ -141,9 +140,9 @@ func TestGetToken(t *testing.T) {
 			if tt.wantErr {
 				g.Expect(err).To(HaveOccurred())
 				return
-			} else {
-				g.Expect(err).ToNot(HaveOccurred())
 			}
+
+			g.Expect(err).ToNot(HaveOccurred())
 			g.Expect(token).To(Equal(tt.wantToken))
 		})
 	}

internal/observability/trace_context.go

@@ -35,6 +35,8 @@ type traceContextSink struct {
 	span trace.Span
 }
 
+var _ logr.LogSink = &traceContextSink{}
+
 func newTraceContextSink(base logr.LogSink, span trace.Span) logr.LogSink {
 	return &traceContextSink{base: base, span: span}
 }

internal/observability/trace_context_test.go

@@ -150,6 +150,7 @@ func TestTraceContextSink_InfoAddsTraceIDs(t *testing.T) {
 func TestTraceContextSink_InfoNoopSpanLeavesKVsUnchanged(t *testing.T) {
 	g := NewWithT(t)
 	_, span := tracenoop.NewTracerProvider().Tracer("test").Start(context.Background(), "op")
+	defer span.End()
 
 	base := newRecordingSink()
 	sink := newTraceContextSink(base, span)

internal/testutils/cluster_scope.go

@@ -1,11 +1,10 @@
 package testutils
 
 import (
-	corev1 "k8s.io/api/core/v1"
-	clusterv1 "sigs.k8s.io/cluster-api/api/core/v1beta2"
-
 	"github.com/go-logr/logr"
+	corev1 "k8s.io/api/core/v1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	clusterv1 "sigs.k8s.io/cluster-api/api/core/v1beta2"
 
 	infrastructurev1beta2 "github.com/cloudscale-ch/cluster-api-provider-cloudscale/api/v1beta2"
 	"github.com/cloudscale-ch/cluster-api-provider-cloudscale/internal/cloudscale"

internal/testutils/fake_client.go

@@ -1,13 +1,13 @@
 package testutils
 
 import (
+	corev1 "k8s.io/api/core/v1"
 	"k8s.io/apimachinery/pkg/runtime"
 	"sigs.k8s.io/cluster-api/api/core/v1beta2"
 	"sigs.k8s.io/controller-runtime/pkg/client"
 	"sigs.k8s.io/controller-runtime/pkg/client/fake"
 
 	infrastructurev1beta2 "github.com/cloudscale-ch/cluster-api-provider-cloudscale/api/v1beta2"
-	corev1 "k8s.io/api/core/v1"
 )
 
 // NewFakeClient returns a fake client with corev1, clusterv1, and infrastructurev1beta2

internal/webhook/v1beta2/cloudscaleclustertemplate_webhook.go

@@ -31,7 +31,6 @@ import (
 	"github.com/cloudscale-ch/cluster-api-provider-cloudscale/internal/cloudscale"
 )
 
-// nolint:unused
 // log is for logging in this package.
 var cloudscaleclustertemplatelog = logf.Log.WithName("cloudscaleclustertemplate-resource")
 

internal/webhook/v1beta2/webhook_suite_test.go

@@ -122,7 +122,14 @@ func TestMain(m *testing.M) {
 	addrPort := fmt.Sprintf("%s:%d", webhookInstallOptions.LocalServingHost, webhookInstallOptions.LocalServingPort)
 	deadline := time.Now().Add(30 * time.Second)
 	for time.Now().Before(deadline) {
-		conn, dialErr := tls.DialWithDialer(dialer, "tcp", addrPort, &tls.Config{InsecureSkipVerify: true}) //nolint:gosec
+		d := tls.Dialer{
+			NetDialer: dialer,
+			Config: &tls.Config{
+				InsecureSkipVerify: true, //nolint:gosec // testing code
+			},
+		}
+
+		conn, dialErr := d.DialContext(ctx, "tcp", addrPort)
 		if dialErr == nil {
 			_ = conn.Close()
 			break

test/e2e/e2e_suite_test.go

@@ -127,7 +127,7 @@ var _ = SynchronizedBeforeSuite(func() []byte {
 	if artifactFolder == "" {
 		artifactFolder = filepath.Join(os.TempDir(), "capcs-e2e-artifacts")
 	}
-	Expect(os.MkdirAll(artifactFolder, 0755)).To(Succeed())
+	Expect(os.MkdirAll(artifactFolder, 0750)).To(Succeed())
 
 	By("Creating a clusterctl local repository")
 	clusterctlConfigPath = clusterctl.CreateRepository(ctx, clusterctl.CreateRepositoryInput{