From 26254a966210dd855e0e78d4027a96de1a473519 Mon Sep 17 00:00:00 2001 From: Michael Weibel Date: Thu, 25 Jun 2026 13:58:38 +0200 Subject: [PATCH] chore: adjust linting configuration --- .golangci.yml | 120 ++++++++++++------ internal/cloudscale/client.go | 2 +- internal/cloudscale/flavors.go | 2 +- internal/cloudscale/flavors_test.go | 11 +- .../cloudscalecluster_controller.go | 2 - .../cloudscalecluster_network_test.go | 2 +- .../cloudscalemachine_controller.go | 2 - .../controller/cloudscalemachine_server.go | 2 +- .../cloudscalemachinetemplate_controller.go | 2 +- internal/credentials/credentials_test.go | 5 +- internal/observability/trace_context.go | 2 + internal/observability/trace_context_test.go | 1 + internal/testutils/cluster_scope.go | 5 +- internal/testutils/fake_client.go | 2 +- .../cloudscaleclustertemplate_webhook.go | 1 - .../webhook/v1beta2/webhook_suite_test.go | 9 +- test/e2e/e2e_suite_test.go | 2 +- test/e2e/log_collector.go | 36 ++++-- 18 files changed, 132 insertions(+), 76 deletions(-) diff --git a/.golangci.yml b/.golangci.yml index bb567c6..1ecb9e4 100644 --- a/.golangci.yml +++ b/.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 \ No newline at end of file diff --git a/internal/cloudscale/client.go b/internal/cloudscale/client.go index 86e0900..f1f4e05 100644 --- a/internal/cloudscale/client.go +++ b/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 diff --git a/internal/cloudscale/flavors.go b/internal/cloudscale/flavors.go index 9776baf..e9b54c9 100644 --- a/internal/cloudscale/flavors.go +++ b/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) diff --git a/internal/cloudscale/flavors_test.go b/internal/cloudscale/flavors_test.go index 5397635..d8b557a 100644 --- a/internal/cloudscale/flavors_test.go +++ b/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")) diff --git a/internal/controller/cloudscalecluster_controller.go b/internal/controller/cloudscalecluster_controller.go index 5d7da24..30531d3 100644 --- a/internal/controller/cloudscalecluster_controller.go +++ b/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() diff --git a/internal/controller/cloudscalecluster_network_test.go b/internal/controller/cloudscalecluster_network_test.go index 513c475..36d91c4 100644 --- a/internal/controller/cloudscalecluster_network_test.go +++ b/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 }, } diff --git a/internal/controller/cloudscalemachine_controller.go b/internal/controller/cloudscalemachine_controller.go index f758585..acfbb6f 100644 --- a/internal/controller/cloudscalemachine_controller.go +++ b/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() diff --git a/internal/controller/cloudscalemachine_server.go b/internal/controller/cloudscalemachine_server.go index 7c1353c..5fc1014 100644 --- a/internal/controller/cloudscalemachine_server.go +++ b/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) diff --git a/internal/controller/cloudscalemachinetemplate_controller.go b/internal/controller/cloudscalemachinetemplate_controller.go index bb8a388..c2dee31 100644 --- a/internal/controller/cloudscalemachinetemplate_controller.go +++ b/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 diff --git a/internal/credentials/credentials_test.go b/internal/credentials/credentials_test.go index 6930693..81ffe5f 100644 --- a/internal/credentials/credentials_test.go +++ b/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)) }) } diff --git a/internal/observability/trace_context.go b/internal/observability/trace_context.go index 92b8356..ed44efd 100644 --- a/internal/observability/trace_context.go +++ b/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} } diff --git a/internal/observability/trace_context_test.go b/internal/observability/trace_context_test.go index ca5d9cd..71453b6 100644 --- a/internal/observability/trace_context_test.go +++ b/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) diff --git a/internal/testutils/cluster_scope.go b/internal/testutils/cluster_scope.go index 9fd3390..f3a1c81 100644 --- a/internal/testutils/cluster_scope.go +++ b/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" diff --git a/internal/testutils/fake_client.go b/internal/testutils/fake_client.go index d354c89..f23a797 100644 --- a/internal/testutils/fake_client.go +++ b/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 diff --git a/internal/webhook/v1beta2/cloudscaleclustertemplate_webhook.go b/internal/webhook/v1beta2/cloudscaleclustertemplate_webhook.go index 78876be..1233b74 100644 --- a/internal/webhook/v1beta2/cloudscaleclustertemplate_webhook.go +++ b/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") diff --git a/internal/webhook/v1beta2/webhook_suite_test.go b/internal/webhook/v1beta2/webhook_suite_test.go index 924f1f6..45da5f3 100644 --- a/internal/webhook/v1beta2/webhook_suite_test.go +++ b/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 diff --git a/test/e2e/e2e_suite_test.go b/test/e2e/e2e_suite_test.go index 387b1e9..8ee62a3 100644 --- a/test/e2e/e2e_suite_test.go +++ b/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{ diff --git a/test/e2e/log_collector.go b/test/e2e/log_collector.go index e6ee81d..2fae435 100644 --- a/test/e2e/log_collector.go +++ b/test/e2e/log_collector.go @@ -23,7 +23,7 @@ import ( "fmt" "net" "os" - osExec "os/exec" + "os/exec" "path/filepath" "golang.org/x/crypto/ssh" @@ -50,7 +50,9 @@ func (c CloudscaleLogCollector) CollectMachineLog(ctx context.Context, _ client. if err != nil { return fmt.Errorf("SSH dial to %s (%s): %w", m.Name, ip, err) } - defer sshClient.Close() + defer func(sshClient *ssh.Client) { + _ = sshClient.Close() + }(sshClient) if err := os.MkdirAll(outputPath, 0750); err != nil { return fmt.Errorf("create output dir: %w", err) @@ -76,7 +78,7 @@ func (c CloudscaleLogCollector) CollectMachineLog(ctx context.Context, _ client. } // Collect /var/log/pods as a tarball and extract locally - if err := sshCollectPods(sshClient, filepath.Join(outputPath, "pods")); err != nil { + if err := sshCollectPods(ctx, sshClient, filepath.Join(outputPath, "pods")); err != nil { logger.V(1).Info("Failed to collect pod logs", "error", err) } @@ -108,7 +110,7 @@ func sshDial(host string) (*ssh.Client, error) { return nil, fmt.Errorf("SSH_AUTH_SOCK not set; ssh-agent is required for log collection") } - conn, err := net.Dial("unix", sock) + conn, err := net.Dial("unix", sock) //nolint:gosec,noctx // G704 does not apply, unix filesystem. This is test code if err != nil { return nil, fmt.Errorf("connect to ssh-agent: %w", err) } @@ -124,7 +126,7 @@ func sshDial(host string) (*ssh.Client, error) { client, err := ssh.Dial("tcp", net.JoinHostPort(host, "22"), config) if err != nil { - conn.Close() + _ = conn.Close() return nil, err } return client, nil @@ -136,7 +138,9 @@ func sshRunToFile(client *ssh.Client, command, outputFile string) error { if err != nil { return err } - defer session.Close() + defer func(session *ssh.Session) { + _ = session.Close() + }(session) output, err := session.CombinedOutput(command) if err != nil { @@ -151,12 +155,14 @@ func sshRunToFile(client *ssh.Client, command, outputFile string) error { } // sshCollectPods tars /var/log/pods on the remote and extracts it locally. -func sshCollectPods(client *ssh.Client, outputDir string) error { +func sshCollectPods(ctx context.Context, client *ssh.Client, outputDir string) error { session, err := client.NewSession() if err != nil { return err } - defer session.Close() + defer func(session *ssh.Session) { + _ = session.Close() + }(session) tarData, err := session.CombinedOutput("sudo tar -cf - -C /var/log/pods . 2>/dev/null") if err != nil { @@ -176,17 +182,19 @@ func sshCollectPods(client *ssh.Client, outputDir string) error { if err != nil { return err } - defer os.Remove(tmpFile.Name()) + defer func(name string) { + _ = os.Remove(name) + }(tmpFile.Name()) if _, err := tmpFile.Write(tarData); err != nil { - tmpFile.Close() + _ = tmpFile.Close() return err } - tmpFile.Close() + _ = tmpFile.Close() // Use local tar to extract cmd := fmt.Sprintf("tar -xf %s -C %s", tmpFile.Name(), outputDir) - return runLocalCommand(cmd) + return runLocalCommand(ctx, cmd) } // writeFile writes data to a file, creating parent directories as needed. @@ -198,6 +206,6 @@ func writeFile(path string, data []byte) error { } // runLocalCommand runs a shell command on the local machine. -func runLocalCommand(command string) error { - return osExec.Command("sh", "-c", command).Run() //nolint:gosec // E2E test helper with controlled input. +func runLocalCommand(ctx context.Context, command string) error { + return exec.CommandContext(ctx, "sh", "-c", command).Run() //nolint:gosec // E2E test helper with controlled input. }