From ebcce887257b90be415fe95d4ea0c9ddfa0af37f Mon Sep 17 00:00:00 2001 From: Mikhail Fedosin Date: Tue, 6 Jun 2023 23:35:22 +0200 Subject: [PATCH] fix: better linting This commit enables additional linters and removes unused (commented) ones. All issues reported by linters are resolved either with "make lint-fix" or manually. --- .golangci.yaml | 37 +++++++++++-------- api/v1alpha1/provider_types.go | 3 +- cmd/main.go | 9 +++-- internal/controller/component_customizer.go | 4 +- .../controller/component_customizer_test.go | 6 ++- .../controller/genericprovider_controller.go | 13 ++++--- .../genericprovider_controller_test.go | 4 +- internal/controller/phases.go | 3 +- internal/controller/preflight_checks_test.go | 7 ++-- internal/controller/suite_test.go | 6 +-- internal/envtest/environment.go | 5 +-- internal/webhook/bootstrapprovider_webhook.go | 3 +- .../webhook/controlplaneprovider_webhook.go | 3 +- internal/webhook/coreprovider_webhook.go | 3 +- .../webhook/infrastructureprovider_webhook.go | 3 +- webhook/alias.go | 12 ++---- 16 files changed, 62 insertions(+), 59 deletions(-) diff --git a/.golangci.yaml b/.golangci.yaml index 1dc377789..9f174b501 100644 --- a/.golangci.yaml +++ b/.golangci.yaml @@ -19,32 +19,29 @@ linters: - typecheck - unused # Additional linters + - asasalint - asciicheck - bidichk - bodyclose - contextcheck - # - cyclop - dogsled - # - dupl - durationcheck - errname - # - errorlint + - errorlint - exhaustive - # - exportloopref + - exportloopref - forcetypeassert - # - funlen - # - gochecknoglobals - # - gocognit + - ginkgolinter - goconst - gocritic - gocyclo - godot - # - goerr113 - gofmt + - gofumpt - goheader - goimports - goprintffuncname - # - gosec + - gosec - importas - makezero - misspell @@ -57,14 +54,23 @@ linters: - nolintlint - prealloc - predeclared - # - revive + - promlinter + - reassign + - rowserrcheck + - sqlclosecheck + - stylecheck - tagliatelle - tenv + - testableexamples + - thelper + - tparallel - unconvert - unparam + - usestdlibvars + - wastedassign - whitespace - # - wrapcheck - wsl + linters-settings: goheader: values: @@ -141,8 +147,9 @@ issues: # Exclude some linters from running on tests files. - path: _test\.go linters: - - gocyclo - - dupl - gosec - - gochecknoglobals - - goerr113 + - path: internal/envtest/environment.go + linters: + - dogsled + - gosec + - wsl diff --git a/api/v1alpha1/provider_types.go b/api/v1alpha1/provider_types.go index 8529afb6b..cd35b53d0 100644 --- a/api/v1alpha1/provider_types.go +++ b/api/v1alpha1/provider_types.go @@ -19,9 +19,8 @@ package v1alpha1 import ( corev1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - ctrlconfigv1 "sigs.k8s.io/controller-runtime/pkg/config/v1alpha1" - clusterv1 "sigs.k8s.io/cluster-api/api/v1beta1" + ctrlconfigv1 "sigs.k8s.io/controller-runtime/pkg/config/v1alpha1" ) const ( diff --git a/cmd/main.go b/cmd/main.go index 04076da39..f24b1c328 100644 --- a/cmd/main.go +++ b/cmd/main.go @@ -42,8 +42,6 @@ import ( "sigs.k8s.io/controller-runtime/pkg/controller" "sigs.k8s.io/controller-runtime/pkg/healthz" - // +kubebuilder:scaffold:imports - operatorv1 "sigs.k8s.io/cluster-api-operator/api/v1alpha1" providercontroller "sigs.k8s.io/cluster-api-operator/internal/controller" ) @@ -130,7 +128,12 @@ func main() { klog.Infof("Profiler listening for requests at %s", profilerAddress) go func() { - klog.Info(http.ListenAndServe(profilerAddress, nil)) + server := &http.Server{ + Addr: profilerAddress, + ReadHeaderTimeout: 3 * time.Second, + } + + klog.Info(server.ListenAndServe()) }() } diff --git a/internal/controller/component_customizer.go b/internal/controller/component_customizer.go index 006462a4f..ac1ec9eda 100644 --- a/internal/controller/component_customizer.go +++ b/internal/controller/component_customizer.go @@ -41,9 +41,7 @@ const ( defaultVerbosity = 1 ) -var ( - bool2Str = map[bool]string{true: "true", false: "false"} -) +var bool2Str = map[bool]string{true: "true", false: "false"} // customizeObjectsFn apply provider specific customization to a list of manifests. func customizeObjectsFn(provider genericprovider.GenericProvider) func(objs []unstructured.Unstructured) ([]unstructured.Unstructured, error) { diff --git a/internal/controller/component_customizer_test.go b/internal/controller/component_customizer_test.go index 7e4358641..7967fd480 100644 --- a/internal/controller/component_customizer_test.go +++ b/internal/controller/component_customizer_test.go @@ -29,8 +29,9 @@ import ( "k8s.io/apimachinery/pkg/util/intstr" configv1alpha1 "k8s.io/component-base/config/v1alpha1" "k8s.io/utils/pointer" - operatorv1 "sigs.k8s.io/cluster-api-operator/api/v1alpha1" ctrlconfigv1 "sigs.k8s.io/controller-runtime/pkg/config/v1alpha1" + + operatorv1 "sigs.k8s.io/cluster-api-operator/api/v1alpha1" ) func TestCustomizeDeployment(t *testing.T) { @@ -479,7 +480,8 @@ func TestCustomizeDeployment(t *testing.T) { "--sync-period=25200s", "--profiler-address=localhost:1234", "--v=5", - "--feature-gates=ANOTHER=false,TEST=true"}, + "--feature-gates=ANOTHER=false,TEST=true", + }, LivenessProbe: &corev1.Probe{ ProbeHandler: corev1.ProbeHandler{ HTTPGet: &corev1.HTTPGetAction{ diff --git a/internal/controller/genericprovider_controller.go b/internal/controller/genericprovider_controller.go index 03a2d04d4..5f21d1bb8 100644 --- a/internal/controller/genericprovider_controller.go +++ b/internal/controller/genericprovider_controller.go @@ -18,6 +18,7 @@ package controller import ( "context" + "errors" "fmt" "reflect" @@ -142,9 +143,9 @@ func (r *GenericProviderReconciler) reconcile(ctx context.Context, provider gene for _, phase := range phases { res, err = phase(ctx) if err != nil { - se, ok := err.(*PhaseError) - if ok { - conditions.Set(provider, conditions.FalseCondition(se.Type, se.Reason, se.Severity, err.Error())) + var pe *PhaseError + if errors.As(err, &pe) { + conditions.Set(provider, conditions.FalseCondition(pe.Type, pe.Reason, pe.Severity, err.Error())) } } @@ -174,9 +175,9 @@ func (r *GenericProviderReconciler) reconcileDelete(ctx context.Context, provide for _, phase := range phases { res, err = phase(ctx) if err != nil { - se, ok := err.(*PhaseError) - if ok { - conditions.Set(provider, conditions.FalseCondition(se.Type, se.Reason, se.Severity, err.Error())) + var pe *PhaseError + if errors.As(err, &pe) { + conditions.Set(provider, conditions.FalseCondition(pe.Type, pe.Reason, pe.Severity, err.Error())) } } diff --git a/internal/controller/genericprovider_controller_test.go b/internal/controller/genericprovider_controller_test.go index 52ad22250..8c274a990 100644 --- a/internal/controller/genericprovider_controller_test.go +++ b/internal/controller/genericprovider_controller_test.go @@ -25,12 +25,12 @@ import ( metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" utilruntime "k8s.io/apimachinery/pkg/util/runtime" + clusterv1 "sigs.k8s.io/cluster-api/api/v1beta1" + clusterctlv1 "sigs.k8s.io/cluster-api/cmd/clusterctl/api/v1alpha3" "sigs.k8s.io/controller-runtime/pkg/client" operatorv1 "sigs.k8s.io/cluster-api-operator/api/v1alpha1" "sigs.k8s.io/cluster-api-operator/internal/controller/genericprovider" - clusterv1 "sigs.k8s.io/cluster-api/api/v1beta1" - clusterctlv1 "sigs.k8s.io/cluster-api/cmd/clusterctl/api/v1alpha3" ) const ( diff --git a/internal/controller/phases.go b/internal/controller/phases.go index 88933b733..692ec1f5c 100644 --- a/internal/controller/phases.go +++ b/internal/controller/phases.go @@ -18,6 +18,7 @@ package controller import ( "context" + "errors" "fmt" "net/url" "strings" @@ -410,7 +411,7 @@ func (p *phaseReconciler) install(ctx context.Context) (reconcile.Result, error) if err := clusterClient.ProviderComponents().Create(p.components.Objs()); err != nil { reason := "Install failed" - if err == wait.ErrWaitTimeout { + if errors.Is(err, wait.ErrWaitTimeout) { reason = "Timed out waiting for deployment to become ready" } diff --git a/internal/controller/preflight_checks_test.go b/internal/controller/preflight_checks_test.go index f7e750c85..e4a2615fa 100644 --- a/internal/controller/preflight_checks_test.go +++ b/internal/controller/preflight_checks_test.go @@ -24,10 +24,11 @@ import ( . "github.com/onsi/gomega" corev1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - operatorv1 "sigs.k8s.io/cluster-api-operator/api/v1alpha1" - "sigs.k8s.io/cluster-api-operator/internal/controller/genericprovider" clusterv1 "sigs.k8s.io/cluster-api/api/v1beta1" "sigs.k8s.io/controller-runtime/pkg/client/fake" + + operatorv1 "sigs.k8s.io/cluster-api-operator/api/v1alpha1" + "sigs.k8s.io/cluster-api-operator/internal/controller/genericprovider" ) func TestPreflightChecks(t *testing.T) { @@ -650,7 +651,7 @@ func TestPreflightChecks(t *testing.T) { } // Check if proper condition is returned - gs.Expect(len(tc.providers[0].GetStatus().Conditions)).To(Equal(1)) + gs.Expect(tc.providers[0].GetStatus().Conditions).To(HaveLen(1)) gs.Expect(tc.providers[0].GetStatus().Conditions[0].Type).To(Equal(tc.expectedCondition.Type)) gs.Expect(tc.providers[0].GetStatus().Conditions[0].Status).To(Equal(tc.expectedCondition.Status)) gs.Expect(tc.providers[0].GetStatus().Conditions[0].Message).To(Equal(tc.expectedCondition.Message)) diff --git a/internal/controller/suite_test.go b/internal/controller/suite_test.go index 4e297a581..7c166460b 100644 --- a/internal/controller/suite_test.go +++ b/internal/controller/suite_test.go @@ -24,11 +24,11 @@ import ( . "github.com/onsi/ginkgo/v2" . "github.com/onsi/gomega" - operatorv1 "sigs.k8s.io/cluster-api-operator/api/v1alpha1" - "sigs.k8s.io/cluster-api-operator/internal/envtest" ctrl "sigs.k8s.io/controller-runtime" "sigs.k8s.io/controller-runtime/pkg/controller" - // +kubebuilder:scaffold:imports + + operatorv1 "sigs.k8s.io/cluster-api-operator/api/v1alpha1" + "sigs.k8s.io/cluster-api-operator/internal/envtest" ) const ( diff --git a/internal/envtest/environment.go b/internal/envtest/environment.go index 22aea0741..28281bbdb 100644 --- a/internal/envtest/environment.go +++ b/internal/envtest/environment.go @@ -100,7 +100,7 @@ type Environment struct { // usually the environment is initialized in a suite_test.go file within a `BeforeSuite` ginkgo block. func New(uncachedObjs ...client.Object) *Environment { // Get the root of the current file to use in CRD paths. - _, filename, _, _ := goruntime.Caller(0) //nolint + _, filename, _, _ := goruntime.Caller(0) root := path.Join(path.Dir(filename), "..", "..") crdPaths := []string{ filepath.Join(root, "config", "crd", "bases"), @@ -114,7 +114,7 @@ func New(uncachedObjs ...client.Object) *Environment { env := &envtest.Environment{ Scheme: scheme.Scheme, ErrorIfCRDPathMissing: true, - //CRDInstallOptions: envtest.CRDInstallOptions{CleanUpAfterUse: true}, + // CRDInstallOptions: envtest.CRDInstallOptions{CleanUpAfterUse: true}, CRDDirectoryPaths: crdPaths, } @@ -232,7 +232,6 @@ func (e *Environment) CleanupAndWait(ctx context.Context, objs ...client.Object) return false, nil }) - if err != nil { errs = append(errs, fmt.Errorf("key %s, %s is not being deleted from the testenv client cache: %w", o.GetObjectKind().GroupVersionKind().String(), key, err)) } diff --git a/internal/webhook/bootstrapprovider_webhook.go b/internal/webhook/bootstrapprovider_webhook.go index 836748db5..9efedce46 100644 --- a/internal/webhook/bootstrapprovider_webhook.go +++ b/internal/webhook/bootstrapprovider_webhook.go @@ -26,8 +26,7 @@ import ( operatorv1 "sigs.k8s.io/cluster-api-operator/api/v1alpha1" ) -type BootstrapProviderWebhook struct { -} +type BootstrapProviderWebhook struct{} func (r *BootstrapProviderWebhook) SetupWebhookWithManager(mgr ctrl.Manager) error { return ctrl.NewWebhookManagedBy(mgr). diff --git a/internal/webhook/controlplaneprovider_webhook.go b/internal/webhook/controlplaneprovider_webhook.go index a3ce7bed6..14dbc5182 100644 --- a/internal/webhook/controlplaneprovider_webhook.go +++ b/internal/webhook/controlplaneprovider_webhook.go @@ -26,8 +26,7 @@ import ( operatorv1 "sigs.k8s.io/cluster-api-operator/api/v1alpha1" ) -type ControlPlaneProviderWebhook struct { -} +type ControlPlaneProviderWebhook struct{} func (r *ControlPlaneProviderWebhook) SetupWebhookWithManager(mgr ctrl.Manager) error { return ctrl.NewWebhookManagedBy(mgr). diff --git a/internal/webhook/coreprovider_webhook.go b/internal/webhook/coreprovider_webhook.go index b9a132931..66ecf8f6c 100644 --- a/internal/webhook/coreprovider_webhook.go +++ b/internal/webhook/coreprovider_webhook.go @@ -26,8 +26,7 @@ import ( operatorv1 "sigs.k8s.io/cluster-api-operator/api/v1alpha1" ) -type CoreProviderWebhook struct { -} +type CoreProviderWebhook struct{} func (r *CoreProviderWebhook) SetupWebhookWithManager(mgr ctrl.Manager) error { return ctrl.NewWebhookManagedBy(mgr). diff --git a/internal/webhook/infrastructureprovider_webhook.go b/internal/webhook/infrastructureprovider_webhook.go index 94051f5b7..0d9e6dfcf 100644 --- a/internal/webhook/infrastructureprovider_webhook.go +++ b/internal/webhook/infrastructureprovider_webhook.go @@ -26,8 +26,7 @@ import ( operatorv1 "sigs.k8s.io/cluster-api-operator/api/v1alpha1" ) -type InfrastructureProviderWebhook struct { -} +type InfrastructureProviderWebhook struct{} func (r *InfrastructureProviderWebhook) SetupWebhookWithManager(mgr ctrl.Manager) error { return ctrl.NewWebhookManagedBy(mgr). diff --git a/webhook/alias.go b/webhook/alias.go index 22ed0409c..1ad2ae6ec 100644 --- a/webhook/alias.go +++ b/webhook/alias.go @@ -21,29 +21,25 @@ import ( ctrl "sigs.k8s.io/controller-runtime" ) -type BootstrapProviderWebhook struct { -} +type BootstrapProviderWebhook struct{} func (r *BootstrapProviderWebhook) SetupWebhookWithManager(mgr ctrl.Manager) error { return (&internalwebhook.BootstrapProviderWebhook{}).SetupWebhookWithManager(mgr) } -type ControlPlaneProviderWebhook struct { -} +type ControlPlaneProviderWebhook struct{} func (r *ControlPlaneProviderWebhook) SetupWebhookWithManager(mgr ctrl.Manager) error { return (&internalwebhook.ControlPlaneProviderWebhook{}).SetupWebhookWithManager(mgr) } -type CoreProviderWebhook struct { -} +type CoreProviderWebhook struct{} func (r *CoreProviderWebhook) SetupWebhookWithManager(mgr ctrl.Manager) error { return (&internalwebhook.CoreProviderWebhook{}).SetupWebhookWithManager(mgr) } -type InfrastructureProviderWebhook struct { -} +type InfrastructureProviderWebhook struct{} func (r *InfrastructureProviderWebhook) SetupWebhookWithManager(mgr ctrl.Manager) error { return (&internalwebhook.InfrastructureProviderWebhook{}).SetupWebhookWithManager(mgr)