Skip to content

Commit

Permalink
Sync golangci-lint configuration to core CAPI
Browse files Browse the repository at this point in the history
  • Loading branch information
chrischdi committed Aug 7, 2023
1 parent f1341ab commit c1b76c6
Show file tree
Hide file tree
Showing 10 changed files with 253 additions and 41 deletions.
270 changes: 241 additions & 29 deletions .golangci.yml
Original file line number Diff line number Diff line change
@@ -1,15 +1,30 @@
run:
timeout: 10m
skip-files:
- "zz_generated.*\\.go$"
- "contrib/.*"
- "apis/v1alpha3/.*"
- "apis/v1alpha4/.*"
allow-parallel-runners: true

linters:
disable-all: true
enable:
- asasalint
- asciicheck
- bidichk
- bodyclose
# - containedctx
- dogsled
# - dupword
- durationcheck
- errcheck
# - errchkjson
- exportloopref
- gci
# - ginkgolinter
- goconst
- gocritic
- gocyclo
- godot
- gofmt
- goimports
Expand All @@ -24,6 +39,7 @@ linters:
- nilerr
- noctx
- nolintlint
- nosprintfhostport
- prealloc
- predeclared
- revive
Expand All @@ -35,37 +51,233 @@ linters:
- unconvert
- unparam
- unused
- usestdlibvars
- whitespace

linters-settings:
gci:
local-prefixes: sigs.k8s.io/cluster-api-provider-vsphere
run:
skip-files:
- ".*zz_generated.*\\.go"
- "contrib/.*"
- "apis/v1alpha3/.*"
- "apis/v1alpha4/.*"
timeout: 5m
issue:
max-same-issues: 0
max-per-linter: 0
local-prefixes: "sigs.k8s.io/cluster-api-provider-vsphere"
godot:
# declarations - for top level declaration comments (default);
# toplevel - for top level comments;
# all - for all comments.
scope: toplevel
exclude:
- '^ \+.*'
- '^ ANCHOR.*'
# gocritic:
# enabled-tags:
# - diagnostic
# - experimental
# - performance
# disabled-checks:
# - appendAssign
# - dupImport # https://github.com/go-critic/go-critic/issues/845
# - evalOrder
# - ifElseChain
# - octalLiteral
# - regexpSimplify
# - sloppyReassign
# - truncateCmp
# - typeDefFirst
# - unnamedResult
# - unnecessaryDefer
# - whyNoLint
# - wrapperFunc
# - rangeValCopy
# - hugeParam
# importas:
# no-unaliased: true
# alias:
# # Kubernetes
# - pkg: k8s.io/api/core/v1
# alias: corev1
# - pkg: k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1
# alias: apiextensionsv1
# - pkg: k8s.io/apimachinery/pkg/apis/meta/v1
# alias: metav1
# - pkg: k8s.io/apimachinery/pkg/api/errors
# alias: apierrors
# - pkg: k8s.io/apimachinery/pkg/util/errors
# alias: kerrors
# - pkg: k8s.io/component-base/logs/api/v1
# alias: logsv1
# # Controller Runtime
# - pkg: sigs.k8s.io/controller-runtime
# alias: ctrl
# # CAPV
# - pkg: sigs.k8s.io/cluster-api-provider-vsphere/apis/v1beta1
# alias: infrav1
# - pkg: sigs.k8s.io/cluster-api-provider-vsphere/apis/v1alpha3
# alias: infrav1alpha3
# - pkg: sigs.k8s.io/cluster-api-provider-vsphere/apis/v1alpha4
# alias: infrav1alpha4
# - pkg: sigs.k8s.io/cluster-api-provider-vsphere/apis/vmware/v1beta1
# alias: vmwarev1
# # VMware Operator
# - pkg: "github.com/vmware-tanzu/vm-operator/api/v1alpha1"
# alias: vmoprv1
# # CABPK
# - pkg: sigs.k8s.io/cluster-api/bootstrap/kubeadm/api/v1beta1
# alias: bootstrapv1
# # KCP
# - pkg: sigs.k8s.io/cluster-api/controlplane/kubeadm/api/v1beta1
# alias: controlplanev1
# # CAPI
# - pkg: sigs.k8s.io/cluster-api/api/v1alpha3
# alias: clusterv1alpha3
# - pkg: sigs.k8s.io/cluster-api/api/v1alpha4
# alias: clusterv1alpha4
# - pkg: sigs.k8s.io/cluster-api/api/v1beta1
# alias: clusterv1
# # CAPI exp
# - pkg: sigs.k8s.io/cluster-api/exp/api/v1beta1
# alias: expv1
# # CAPI exp addons
# - pkg: sigs.k8s.io/cluster-api/exp/addons/api/v1beta1
# alias: addonsv1
# nolintlint:
# allow-unused: false
# allow-leading-space: false
# require-specific: true
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: dot-imports
- name: error-return
- name: error-strings
- name: error-naming
- name: exported
- name: if-return
- name: increment-decrement
- name: var-naming
- name: var-declaration
- name: package-comments
- name: range
- name: receiver-naming
- name: time-naming
- name: unexported-return
- name: indent-error-flow
- name: errorf
- name: empty-block
- name: superfluous-else
- name: unused-parameter
- name: unreachable-code
- name: redefines-builtin-id
#
# Rules in addition to the recommended configuration above.
#
- name: bool-literal-in-expr
- name: constant-logical-expr

issues:
max-same-issues: 0
max-issues-per-linter: 0
# We are disabling default golangci exclusions because we want to help reviewers to focus on reviewing the most relevant
# changes in PRs and avoid nitpicking.
# exclude-use-default: false
exclude-rules:
# Specific exclude rules for deprecated fields that are still part of the codebase. These
# should be removed as the referenced deprecated item is removed from the project.
- linters:
- staticcheck
text: "SA1019: failureDomain.AutoConfigure is deprecated"
- path: "test/e2e/*"
linters:
- gosec
text: "G106:"
- linters:
- revive
text: "dot-imports"
path: ".*test.*"
- linters:
- stylecheck
text: "ST1001"
path: ".*test.*"
# Specific exclude rules for deprecated fields that are still part of the codebase. These
# should be removed as the referenced deprecated item is removed from the project.

# Exclude-rules shared with CAPI
# Specific exclude rules for deprecated packages that are still part of the codebase. These
# should be removed as the referenced deprecated packages are removed from the project.
- linters:
- staticcheck
text: "SA1019: .* is deprecated: This package will be removed in one of the next releases."
- linters:
- revive
text: "exported: exported method .*\\.(Reconcile|SetupWithManager|SetupWebhookWithManager) should have comment or be unexported"
- linters:
- errcheck
text: Error return value of .((os\.)?std(out|err)\..*|.*Close|.*Flush|os\.Remove(All)?|.*print(f|ln)?|os\.(Un)?Setenv). is not checked
# Exclude some packages or code to require comments, for example test code, or fake clients.
- linters:
- revive
text: exported (method|function|type|const) (.+) should have comment or be unexported
source: (func|type).*Fake.*
- linters:
- revive
text: exported (method|function|type|const) (.+) should have comment or be unexported
path: fake_\.go
# Disable unparam "always receives" which might not be really
# useful when building libraries.
- linters:
- unparam
text: always receives

# Dot imports for gomega and ginkgo are allowed
# within test files and test utils.
- path: _test\.go
text: should not use dot imports
- path: ^test/.*.go
text: should not use dot imports
- path: _test\.go
text: cyclomatic complexity
# Append should be able to assign to a different var/slice.
- linters:
- gocritic
text: "appendAssign: append result not assigned to the same slice"
# Disable linters for conversion
- linters:
- staticcheck
text: "SA1019: in.(.+) is deprecated"
path: .*(api|types)\/.*\/conversion.*\.go$
- linters:
- revive
# Checking if an error is nil to just after return the error or nil is redundant
text: "if-return: redundant if ...; err != nil check, just return error instead"
# Ignoring stylistic checks for generated code
path: .*(api|types|test)\/.*\/conversion.*\.go$
- linters:
- revive
# Exported function and methods should have comments. This warns on undocumented exported functions and methods.
text: exported (method|function|type|const) (.+) should have comment or be unexported
# Ignoring stylistic checks for generated code
path: .*(api|types|test)\/.*\/conversion.*\.go$
- linters:
- revive
# This rule warns when initialism, variable or package naming conventions are not followed.
text: "var-naming: don't use underscores in Go names;"
# Ignoring stylistic checks for generated code
path: .*(api|types|test)\/.*\/conversion.*\.go$
- linters:
- revive
# By convention, receiver names in a method should reflect their identity.
text: "receiver-naming: receiver name"
# Ignoring stylistic checks for generated code
path: .*(api|types)\/.*\/conversion.*\.go$
- linters:
- stylecheck
text: "ST1003: should not use underscores in Go names;"
path: .*(api|types|test)\/.*\/conversion.*\.go$
- linters:
- stylecheck
text: "ST1016: methods on the same type should have the same receiver name"
path: .*(api|types)\/.*\/conversion.*\.go$
# We don't care about defer in for loops in test files.
- linters:
- gocritic
text: "deferInLoop: Possible resource leak, 'defer' is called in the 'for' loop"
path: _test\.go

# Exclude-rules unique for CAPV
- linters:
- staticcheck
text: "SA1019: failureDomain.AutoConfigure is deprecated"
- path: "test/e2e/*"
linters:
- gosec
text: "G106:"
- linters:
- revive
text: "dot-imports"
path: ".*test.*"
- linters:
- stylecheck
text: "ST1001"
path: ".*test.*"
4 changes: 2 additions & 2 deletions controllers/clustermodule_reconciler_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -370,7 +370,7 @@ func TestReconciler_fetchMachineOwnerObjects(t *testing.T) {
})
}

//nolint:unparam

func machineDeployment(name, namespace, cluster string) *clusterv1.MachineDeployment {
return &clusterv1.MachineDeployment{
TypeMeta: metav1.TypeMeta{
Expand All @@ -384,7 +384,7 @@ func machineDeployment(name, namespace, cluster string) *clusterv1.MachineDeploy
}
}

//nolint:unparam

func controlPlane(name, namespace, cluster string) *controlplanev1.KubeadmControlPlane {
return &controlplanev1.KubeadmControlPlane{
TypeMeta: metav1.TypeMeta{
Expand Down
4 changes: 2 additions & 2 deletions controllers/serviceaccount_controller_suite_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -102,7 +102,7 @@ func assertServiceAccountAndUpdateSecret(ctx goctx.Context, ctrlClient client.Cl
Expect(ctrlClient.Update(ctx, secret)).To(Succeed())
}

func assertTargetSecret(ctx goctx.Context, guestClient client.Client, namespace, name string) { //nolint
func assertTargetSecret(ctx goctx.Context, guestClient client.Client, namespace, name string) {
secret := &corev1.Secret{}
assertEventuallyExistsInNamespace(ctx, guestClient, namespace, name, secret)
EventuallyWithOffset(2, func() []byte {
Expand Down Expand Up @@ -157,7 +157,7 @@ func assertRoleBinding(_ *helpers.UnitTestContextForController, ctrlClient clien
}

// assertProviderServiceAccountsCondition asserts the condition on the ProviderServiceAccount CR.
func assertProviderServiceAccountsCondition(vCluster *vmwarev1.VSphereCluster, status corev1.ConditionStatus, message string, reason string, severity clusterv1.ConditionSeverity) { //nolint
func assertProviderServiceAccountsCondition(vCluster *vmwarev1.VSphereCluster, status corev1.ConditionStatus, message string, reason string, severity clusterv1.ConditionSeverity) {
c := conditions.Get(vCluster, vmwarev1.ProviderServiceAccountsReadyCondition)
Expect(c).NotTo(BeNil())
Expect(c.Status).To(Equal(status))
Expand Down
2 changes: 1 addition & 1 deletion controllers/svcdiscovery_controller_suite_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -81,7 +81,7 @@ func assertHeadlessSvc(ctx context.Context, guestClient client.Client, namespace
Expect(headlessSvc.Spec.Ports[0].TargetPort.IntVal).To(Equal(int32(supervisorAPIServerPort)))
}

func assertHeadlessSvcWithNoEndpoints(ctx context.Context, guestClient client.Client, namespace, name string) { //nolint
func assertHeadlessSvcWithNoEndpoints(ctx context.Context, guestClient client.Client, namespace, name string) {
assertHeadlessSvc(ctx, guestClient, namespace, name)
headlessEndpoints := &corev1.Endpoints{}
assertEventuallyDoesNotExistInNamespace(ctx, guestClient, namespace, name, headlessEndpoints)
Expand Down
4 changes: 2 additions & 2 deletions controllers/vsphereclusteridentity_controller_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -160,7 +160,7 @@ var _ = Describe("VSphereClusterIdentity Reconciler", func() {
return false
}

if i.Status.Ready == false && conditions.GetReason(i, infrav1.CredentialsAvailableCondidtion) == infrav1.SecretAlreadyInUseReason {
if !i.Status.Ready && conditions.GetReason(i, infrav1.CredentialsAvailableCondidtion) == infrav1.SecretAlreadyInUseReason {
return true
}
return false
Expand All @@ -184,7 +184,7 @@ var _ = Describe("VSphereClusterIdentity Reconciler", func() {
return false
}

if i.Status.Ready == false && conditions.GetReason(i, infrav1.CredentialsAvailableCondidtion) == infrav1.SecretNotAvailableReason {
if !i.Status.Ready && conditions.GetReason(i, infrav1.CredentialsAvailableCondidtion) == infrav1.SecretNotAvailableReason {
return true
}
return false
Expand Down
2 changes: 1 addition & 1 deletion pkg/services/govmomi/cluster/vmgroup.go
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@ type VMGroup struct {
// Add a VSphere VM object to the VM Group.
func (vg VMGroup) Add(ctx context.Context, vmObj types.ManagedObjectReference) (*object.Task, error) {
vms := vg.listVMs()
vg.ClusterVmGroup.Vm = append(vms, vmObj) //nolint:gocritic
vg.ClusterVmGroup.Vm = append(vms, vmObj)

spec := &types.ClusterConfigSpecEx{
GroupSpec: []types.ClusterGroupSpec{
Expand Down
2 changes: 1 addition & 1 deletion test/e2e/common.go
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ type GlobalInput struct {
E2EConfig *clusterctl.E2EConfig
}

//nolint:unparam

func defaultConfigCluster(clusterName, namespace, flavor string, controlPlaneNodeCount, workerNodeCount int64,
input GlobalInput) clusterctl.ConfigClusterInput {
configClusterInput := clusterctl.ConfigClusterInput{
Expand Down
2 changes: 1 addition & 1 deletion test/helpers/framework.go
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ import (
"sigs.k8s.io/cluster-api/test/framework/clusterctl"
)

// Util functions to interact with the clusterctl e2e framework
// Util functions to interact with the clusterctl e2e framework.

func LoadE2EConfig(configPath string) (*clusterctl.E2EConfig, error) {
config := clusterctl.LoadE2EConfig(goctx.TODO(), clusterctl.LoadE2EConfigInput{ConfigPath: configPath})
Expand Down
Loading

0 comments on commit c1b76c6

Please sign in to comment.