Skip to content

Commit c48baad

Browse files
vinceprivincepri
committed
Refactor golangci-lint, enable more linters and add github action
Signed-off-by: Vince Prignano <vincepri@vmware.com>
1 parent 44a4c03 commit c48baad

File tree

127 files changed

+1338
-1345
lines changed

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

127 files changed

+1338
-1345
lines changed

‎.github/workflows/golangci-lint.yml

+23
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,23 @@
1+
name: golangci-lint
2+
on:
3+
pull_request:
4+
types: [opened, edited, synchronize, reopened]
5+
branches:
6+
- main
7+
- master
8+
jobs:
9+
golangci:
10+
name: lint
11+
runs-on: ubuntu-latest
12+
strategy:
13+
matrix:
14+
working-directory:
15+
- ""
16+
- tools/setup-envtest
17+
steps:
18+
- uses: actions/checkout@v2
19+
- name: golangci-lint
20+
uses: golangci/golangci-lint-action@v2
21+
with:
22+
version: v1.40.1
23+
working-directory: ${{matrix.working-directory}}

‎.golangci.yml

+123-29
Original file line numberDiff line numberDiff line change
@@ -1,36 +1,130 @@
1-
run:
2-
deadline: 5m
3-
linters-settings:
4-
lll:
5-
line-length: 170
6-
dupl:
7-
threshold: 400
8-
issues:
9-
# don't skip warning about doc comments
10-
exclude-use-default: false
11-
12-
# restore some of the defaults
13-
# (fill in the rest as needed)
14-
exclude-rules:
15-
- linters: [errcheck]
16-
text: "Error return value of .((os\\.)?std(out|err)\\..*|.*Close|.*Flush|os\\.Remove(All)?|.*printf?|os\\.(Un)?Setenv). is not checked"
171
linters:
182
disable-all: true
193
enable:
20-
- misspell
21-
- structcheck
22-
- golint
23-
- govet
4+
- asciicheck
5+
- bodyclose
246
- deadcode
7+
- depguard
8+
- dogsled
259
- errcheck
26-
- varcheck
27-
- unparam
28-
- ineffassign
29-
- nakedret
10+
- exportloopref
11+
- goconst
12+
- gocritic
3013
- gocyclo
31-
- dupl
14+
- godot
15+
- gofmt
3216
- goimports
33-
- golint
34-
# disabled:
35-
# - goconst is overly aggressive
36-
# - lll generally just complains about flag help & error strings that are human-readable
17+
- goprintffuncname
18+
- gosec
19+
- gosimple
20+
- govet
21+
- ifshort
22+
- importas
23+
- ineffassign
24+
- misspell
25+
- nakedret
26+
- nilerr
27+
- nolintlint
28+
- prealloc
29+
- revive
30+
- rowserrcheck
31+
- staticcheck
32+
- structcheck
33+
- stylecheck
34+
- typecheck
35+
- unconvert
36+
- unparam
37+
- varcheck
38+
- whitespace
39+
40+
linters-settings:
41+
ifshort:
42+
# Maximum length of variable declaration measured in number of characters, after which linter won't suggest using short syntax.
43+
max-decl-chars: 50
44+
importas:
45+
no-unaliased: true
46+
alias:
47+
# Kubernetes
48+
- pkg: k8s.io/api/core/v1
49+
alias: corev1
50+
- pkg: k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1
51+
alias: apiextensionsv1
52+
- pkg: k8s.io/apimachinery/pkg/apis/meta/v1
53+
alias: metav1
54+
- pkg: k8s.io/apimachinery/pkg/api/errors
55+
alias: apierrors
56+
- pkg: k8s.io/apimachinery/pkg/util/errors
57+
alias: kerrors
58+
# Controller Runtime
59+
- pkg: sigs.k8s.io/controller-runtime
60+
alias: ctrl
61+
staticcheck:
62+
go: "1.16"
63+
stylecheck:
64+
go: "1.16"
65+
66+
issues:
67+
max-same-issues: 0
68+
max-issues-per-linter: 0
69+
# We are disabling default golangci exclusions because we want to help reviewers to focus on reviewing the most relevant
70+
# changes in PRs and avoid nitpicking.
71+
exclude-use-default: false
72+
# List of regexps of issue texts to exclude, empty list by default.
73+
exclude:
74+
# The following are being worked on to remove their exclusion. This list should be reduced or go away all together over time.
75+
# If it is decided they will not be addressed they should be moved above this comment.
76+
- Subprocess launch(ed with variable|ing should be audited)
77+
- (G204|G104|G307)
78+
- "ST1000: at least one file in a package should have a package comment"
79+
exclude-rules:
80+
- linters:
81+
- gosec
82+
text: "G108: Profiling endpoint is automatically exposed on /debug/pprof"
83+
- linters:
84+
- revive
85+
text: "exported: exported method .*\\.(Reconcile|SetupWithManager|SetupWebhookWithManager) should have comment or be unexported"
86+
- linters:
87+
- errcheck
88+
text: Error return value of .((os\.)?std(out|err)\..*|.*Close|.*Flush|os\.Remove(All)?|.*print(f|ln)?|os\.(Un)?Setenv). is not checked
89+
# With Go 1.16, the new embed directive can be used with an un-named import,
90+
# revive (previously, golint) only allows these to be imported in a main.go, which wouldn't work for us.
91+
# This directive allows the embed package to be imported with an underscore everywhere.
92+
- linters:
93+
- revive
94+
source: _ "embed"
95+
# Exclude some packages or code to require comments, for example test code, or fake clients.
96+
- linters:
97+
- revive
98+
text: exported (method|function|type|const) (.+) should have comment or be unexported
99+
source: (func|type).*Fake.*
100+
- linters:
101+
- revive
102+
text: exported (method|function|type|const) (.+) should have comment or be unexported
103+
path: fake_\.go
104+
# Disable unparam "always receives" which might not be really
105+
# useful when building libraries.
106+
- linters:
107+
- unparam
108+
text: always receives
109+
# Dot imports for gomega or ginkgo are allowed
110+
# within test files.
111+
- path: _test\.go
112+
text: should not use dot imports
113+
- path: _test\.go
114+
text: cyclomatic complexity
115+
- path: _test\.go
116+
text: "G107: Potential HTTP request made with variable url"
117+
# Append should be able to assign to a different var/slice.
118+
- linters:
119+
- gocritic
120+
text: "appendAssign: append result not assigned to the same slice"
121+
- linters:
122+
- gocritic
123+
text: "singleCaseSwitch: should rewrite switch statement to if statement"
124+
125+
run:
126+
timeout: 10m
127+
skip-files:
128+
- "zz_generated.*\\.go$"
129+
- ".*conversion.*\\.go$"
130+
allow-parallel-runners: true

‎Makefile

+12-12
Original file line numberDiff line numberDiff line change
@@ -65,29 +65,29 @@ test-tools: ## tests the tools codebase (setup-envtest)
6565
## Binaries
6666
## --------------------------------------
6767

68-
$(GOLANGCI_LINT): $(TOOLS_DIR)/go.mod # Build golangci-lint from tools folder.
69-
cd $(TOOLS_DIR) && go build -tags=tools -o bin/golangci-lint github.com/golangci/golangci-lint/cmd/golangci-lint
70-
7168
$(GO_APIDIFF): $(TOOLS_DIR)/go.mod # Build go-apidiff from tools folder.
7269
cd $(TOOLS_DIR) && go build -tags=tools -o bin/go-apidiff github.com/joelanford/go-apidiff
7370

7471
$(CONTROLLER_GEN): $(TOOLS_DIR)/go.mod # Build controller-gen from tools folder.
7572
cd $(TOOLS_DIR) && go build -tags=tools -o bin/controller-gen sigs.k8s.io/controller-tools/cmd/controller-gen
7673

74+
$(GOLANGCI_LINT): .github/workflows/golangci-lint.yml # Download golanci-lint using hack script into tools folder.
75+
hack/ensure-golangci-lint.sh \
76+
-b $(TOOLS_BIN_DIR) \
77+
$(shell cat .github/workflows/golangci-lint.yml | grep version | sed 's/.*version: //')
78+
7779
## --------------------------------------
7880
## Linting
7981
## --------------------------------------
8082

81-
.PHONY: lint-libs
82-
lint-libs: $(GOLANGCI_LINT) ## Lint library codebase.
83-
$(GOLANGCI_LINT) run -v
84-
85-
.PHONY: lint-tools
86-
lint-tools: $(GOLANGCI_LINT) ## Lint tools codebase.
87-
cd tools/setup-envtest && $(GOLANGCI_LINT) run -v
88-
8983
.PHONY: lint
90-
lint: lint-libs lint-tools
84+
lint: $(GOLANGCI_LINT) ## Lint codebase
85+
$(GOLANGCI_LINT) run -v $(GOLANGCI_LINT_EXTRA_ARGS)
86+
cd tools/setup-envtest; $(GOLANGCI_LINT) run -v $(GOLANGCI_LINT_EXTRA_ARGS)
87+
88+
.PHONY: lint-fix
89+
lint-fix: $(GOLANGCI_LINT) ## Lint the codebase and run auto-fixers if supported by the linter.
90+
GOLANGCI_LINT_EXTRA_ARGS=--fix $(MAKE) lint
9191

9292
## --------------------------------------
9393
## Generate

‎alias.go

+5-5
Original file line numberDiff line numberDiff line change
@@ -45,7 +45,7 @@ type Result = reconcile.Result
4545
// A Manager is required to create Controllers.
4646
type Manager = manager.Manager
4747

48-
// Options are the arguments for creating a new Manager
48+
// Options are the arguments for creating a new Manager.
4949
type Options = manager.Options
5050

5151
// SchemeBuilder builds a new Scheme for mapping go types to Kubernetes GroupVersionKinds.
@@ -55,7 +55,7 @@ type SchemeBuilder = scheme.Builder
5555
type GroupVersion = schema.GroupVersion
5656

5757
// GroupResource specifies a Group and a Resource, but does not force a version. This is useful for identifying
58-
// concepts during lookup stages without having partially valid types
58+
// concepts during lookup stages without having partially valid types.
5959
type GroupResource = schema.GroupResource
6060

6161
// TypeMeta describes an individual object in an API response or request
@@ -89,18 +89,18 @@ var (
8989
//
9090
// * In-cluster config if running in cluster
9191
//
92-
// * $HOME/.kube/config if exists
92+
// * $HOME/.kube/config if exists.
9393
GetConfig = config.GetConfig
9494

9595
// ConfigFile returns the cfg.File function for deferred config file loading,
9696
// this is passed into Options{}.From() to populate the Options fields for
9797
// the manager.
9898
ConfigFile = cfg.File
9999

100-
// NewControllerManagedBy returns a new controller builder that will be started by the provided Manager
100+
// NewControllerManagedBy returns a new controller builder that will be started by the provided Manager.
101101
NewControllerManagedBy = builder.ControllerManagedBy
102102

103-
// NewWebhookManagedBy returns a new webhook builder that will be started by the provided Manager
103+
// NewWebhookManagedBy returns a new webhook builder that will be started by the provided Manager.
104104
NewWebhookManagedBy = builder.WebhookManagedBy
105105

106106
// NewManager returns a new Manager for creating Controllers.

‎doc.go

+1-1
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,7 @@ limitations under the License.
2828
// The main entrypoint for controller-runtime is this root package, which
2929
// contains all of the common types needed to get started building controllers:
3030
// import (
31-
// controllers "sigs.k8s.io/controller-runtime"
31+
// ctrl "sigs.k8s.io/controller-runtime"
3232
// )
3333
//
3434
// The examples in this package walk through a basic controller setup. The

‎example_test.go

+19-19
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,7 @@ import (
2424

2525
appsv1 "k8s.io/api/apps/v1"
2626
corev1 "k8s.io/api/core/v1"
27-
controllers "sigs.k8s.io/controller-runtime"
27+
ctrl "sigs.k8s.io/controller-runtime"
2828
"sigs.k8s.io/controller-runtime/pkg/client"
2929
)
3030

@@ -34,17 +34,17 @@ import (
3434
// ReplicaSetReconciler.
3535
//
3636
// * Start the application.
37-
// TODO(pwittrock): Update this example when we have better dependency injection support
37+
// TODO(pwittrock): Update this example when we have better dependency injection support.
3838
func Example() {
39-
var log = controllers.Log.WithName("builder-examples")
39+
var log = ctrl.Log.WithName("builder-examples")
4040

41-
manager, err := controllers.NewManager(controllers.GetConfigOrDie(), controllers.Options{})
41+
manager, err := ctrl.NewManager(ctrl.GetConfigOrDie(), ctrl.Options{})
4242
if err != nil {
4343
log.Error(err, "could not create manager")
4444
os.Exit(1)
4545
}
4646

47-
err = controllers.
47+
err = ctrl.
4848
NewControllerManagedBy(manager). // Create the Controller
4949
For(&appsv1.ReplicaSet{}). // ReplicaSet is the Application API
5050
Owns(&corev1.Pod{}). // ReplicaSet owns Pods created by it
@@ -54,7 +54,7 @@ func Example() {
5454
os.Exit(1)
5555
}
5656

57-
if err := manager.Start(controllers.SetupSignalHandler()); err != nil {
57+
if err := manager.Start(ctrl.SetupSignalHandler()); err != nil {
5858
log.Error(err, "could not start manager")
5959
os.Exit(1)
6060
}
@@ -72,15 +72,15 @@ func Example() {
7272
// ReplicaSetReconciler.
7373
//
7474
// * Start the application.
75-
// TODO(pwittrock): Update this example when we have better dependency injection support
75+
// TODO(pwittrock): Update this example when we have better dependency injection support.
7676
func Example_updateLeaderElectionDurations() {
77-
var log = controllers.Log.WithName("builder-examples")
77+
var log = ctrl.Log.WithName("builder-examples")
7878
leaseDuration := 100 * time.Second
7979
renewDeadline := 80 * time.Second
8080
retryPeriod := 20 * time.Second
81-
manager, err := controllers.NewManager(
82-
controllers.GetConfigOrDie(),
83-
controllers.Options{
81+
manager, err := ctrl.NewManager(
82+
ctrl.GetConfigOrDie(),
83+
ctrl.Options{
8484
LeaseDuration: &leaseDuration,
8585
RenewDeadline: &renewDeadline,
8686
RetryPeriod: &retryPeriod,
@@ -90,7 +90,7 @@ func Example_updateLeaderElectionDurations() {
9090
os.Exit(1)
9191
}
9292

93-
err = controllers.
93+
err = ctrl.
9494
NewControllerManagedBy(manager). // Create the Controller
9595
For(&appsv1.ReplicaSet{}). // ReplicaSet is the Application API
9696
Owns(&corev1.Pod{}). // ReplicaSet owns Pods created by it
@@ -100,7 +100,7 @@ func Example_updateLeaderElectionDurations() {
100100
os.Exit(1)
101101
}
102102

103-
if err := manager.Start(controllers.SetupSignalHandler()); err != nil {
103+
if err := manager.Start(ctrl.SetupSignalHandler()); err != nil {
104104
log.Error(err, "could not start manager")
105105
os.Exit(1)
106106
}
@@ -117,28 +117,28 @@ type ReplicaSetReconciler struct {
117117
//
118118
// * Read the ReplicaSet
119119
// * Read the Pods
120-
// * Set a Label on the ReplicaSet with the Pod count
121-
func (a *ReplicaSetReconciler) Reconcile(ctx context.Context, req controllers.Request) (controllers.Result, error) {
120+
// * Set a Label on the ReplicaSet with the Pod count.
121+
func (a *ReplicaSetReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Result, error) {
122122
// Read the ReplicaSet
123123
rs := &appsv1.ReplicaSet{}
124124
err := a.Get(ctx, req.NamespacedName, rs)
125125
if err != nil {
126-
return controllers.Result{}, err
126+
return ctrl.Result{}, err
127127
}
128128

129129
// List the Pods matching the PodTemplate Labels
130130
pods := &corev1.PodList{}
131131
err = a.List(ctx, pods, client.InNamespace(req.Namespace), client.MatchingLabels(rs.Spec.Template.Labels))
132132
if err != nil {
133-
return controllers.Result{}, err
133+
return ctrl.Result{}, err
134134
}
135135

136136
// Update the ReplicaSet
137137
rs.Labels["pod-count"] = fmt.Sprintf("%v", len(pods.Items))
138138
err = a.Update(context.TODO(), rs)
139139
if err != nil {
140-
return controllers.Result{}, err
140+
return ctrl.Result{}, err
141141
}
142142

143-
return controllers.Result{}, nil
143+
return ctrl.Result{}, nil
144144
}

0 commit comments

Comments
 (0)