Skip to content

Commit 2079be9

Browse files
AshleyDumaineAshleyDumaine
authored
Merge pull request #486 from linode/nilaway
[CI] add nilaway check
2 parents 52dbb71 + fb1f520 commit 2079be9

File tree

9 files changed

+157
-41
lines changed

9 files changed

+157
-41
lines changed

‎.custom-gcl.yaml‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
version: v2.5.0
2+
name: golangci-lint-nilaway
3+
destination: ./bin
4+
plugins:
5+
- module: "go.uber.org/nilaway"
6+
import: "go.uber.org/nilaway/cmd/gclplugin"
7+
version: "v0.0.0-20250821055425-361559d802f0"

‎.github/workflows/build-test.yml‎

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -73,7 +73,10 @@ jobs:
7373
run: make vet
7474

7575
- name: lint
76-
uses: golangci/golangci-lint-action@v8
76+
run: make lint
77+
78+
- name: nilcheck
79+
run: make nilcheck
7780

7881
- name: Helm Lint
7982
run: make helm-lint

‎.golangci-nilaway.yml‎

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,23 @@
1+
version: "2"
2+
run:
3+
issues-exit-code: 1
4+
output:
5+
formats:
6+
text:
7+
path: stdout
8+
linters:
9+
default: none
10+
enable:
11+
- nilaway
12+
settings:
13+
custom:
14+
nilaway:
15+
type: "module"
16+
description: Static analysis tool to detect potential nil panics in Go code.
17+
settings:
18+
include-pkgs: ""
19+
exclude-file-docstrings: ignore_autogenerated
20+
issues:
21+
max-issues-per-linter: 0
22+
max-same-issues: 0
23+
new: false

‎Makefile‎

Lines changed: 31 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,9 @@ LOCALBIN ?= $(CACHE_BIN)
99
DEVBOX_BIN ?= $(DEVBOX_PACKAGES_DIR)/bin
1010
HELM ?= $(LOCALBIN)/helm
1111
HELM_VERSION ?= v3.16.3
12+
GOLANGCI_LINT ?= $(LOCALBIN)/golangci-lint
13+
GOLANGCI_LINT_VERSION ?= v2.5.0
14+
GOLANGCI_LINT_NILAWAY ?= $(CACHE_BIN)/golangci-lint-nilaway
1215

1316
#####################################################################
1417
# Dev Setup
@@ -31,7 +34,6 @@ LINODE_URL ?= https://api.linode.com
3134
KUBECONFIG_PATH ?= $(CURDIR)/test-cluster-kubeconfig.yaml
3235
SUBNET_KUBECONFIG_PATH ?= $(CURDIR)/subnet-testing-kubeconfig.yaml
3336
MGMT_KUBECONFIG_PATH ?= $(CURDIR)/mgmt-cluster-kubeconfig.yaml
34-
GOLANGCI_LINT_VERSION ?= v2.5.0
3537

3638
# if the $DEVBOX_PACKAGES_DIR env variable exists that means we are within a devbox shell and can safely
3739
# use devbox's bin for our tools
@@ -65,8 +67,12 @@ vet: fmt
6567
go vet ./...
6668

6769
.PHONY: lint
68-
lint:
69-
docker run --rm -w /workdir -v $(PWD):/workdir golangci/golangci-lint:$(GOLANGCI_LINT_VERSION) golangci-lint run -c .golangci.yml --fix
70+
lint: golangci-lint
71+
$(GOLANGCI_LINT) run -c .golangci.yml --fix
72+
73+
.PHONY: lint
74+
nilcheck: golangci-lint-nilaway ## Run nilaway against code.
75+
$(GOLANGCI_LINT_NILAWAY) run -c .golangci-nilaway.yml
7076

7177
.PHONY: gosec
7278
gosec: ## Run gosec against code.
@@ -273,3 +279,25 @@ helm-template: helm
273279
#Verify template works when region and apiToken are passed, and when it is passed as reference.
274280
@$(HELM) template foo deploy/chart --set apiToken="apiToken",region="us-east" > /dev/null
275281
@$(HELM) template foo deploy/chart --set secretRef.apiTokenRef="apiToken",secretRef.name="api",secretRef.regionRef="us-east" > /dev/null
282+
283+
.PHONY: kubectl
284+
kubectl: $(KUBECTL) ## Download kubectl locally if necessary.
285+
$(KUBECTL): $(LOCALBIN)
286+
curl -fsSL https://dl.k8s.io/release/$(KUBECTL_VERSION)/bin/$(OS)/$(ARCH_SHORT)/kubectl -o $(KUBECTL)
287+
chmod +x $(KUBECTL)
288+
289+
.PHONY: clusterctl
290+
clusterctl: $(CLUSTERCTL) ## Download clusterctl locally if necessary.
291+
$(CLUSTERCTL): $(LOCALBIN)
292+
curl -fsSL https://github.com/kubernetes-sigs/cluster-api/releases/download/$(CLUSTERCTL_VERSION)/clusterctl-$(OS)-$(ARCH_SHORT) -o $(CLUSTERCTL)
293+
chmod +x $(CLUSTERCTL)
294+
295+
.phony: golangci-lint-nilaway
296+
golangci-lint-nilaway: $(GOLANGCI_LINT_NILAWAY)
297+
$(GOLANGCI_LINT_NILAWAY): $(GOLANGCI_LINT) # Build golangci-lint-nilaway from custom configuration.
298+
$(GOLANGCI_LINT) custom
299+
300+
.phony: golangci-lint
301+
golangci-lint: $(GOLANGCI_LINT)
302+
$(GOLANGCI_LINT): # Build golangci-lint from tools folder.
303+
GOBIN=$(LOCALBIN) go install github.com/golangci/golangci-lint/v2/cmd/golangci-lint@$(GOLANGCI_LINT_VERSION)

‎cloud/linode/loadbalancers_test.go‎

Lines changed: 75 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -609,7 +609,7 @@ func testCreateNodeBalancerWithInvalidFirewall(t *testing.T, client *linodego.Cl
609609
}
610610
expectedError := "strconv.Atoi: parsing \"qwerty\": invalid syntax"
611611
err := testCreateNodeBalancer(t, client, f, annotations, nil)
612-
if err.Error() != expectedError {
612+
if err != nil && err.Error() != expectedError {
613613
t.Fatalf("expected a %s error, got %v", expectedError, err)
614614
}
615615
}
@@ -748,7 +748,9 @@ func testUpdateNodeBalancerWithVPCBackend(t *testing.T, client *linodego.Client,
748748
if err != nil {
749749
t.Errorf("EnsureLoadBalancer returned an error: %s", err)
750750
}
751-
svc.Status.LoadBalancer = *lbStatus
751+
if lbStatus != nil {
752+
svc.Status.LoadBalancer = *lbStatus
753+
}
752754

753755
stubServiceUpdate(fakeClientset, svc)
754756
svc.SetAnnotations(map[string]string{
@@ -834,7 +836,9 @@ func testCreateNodeBalancerWithVPCOnlySubnetFlag(t *testing.T, client *linodego.
834836
if err != nil {
835837
t.Errorf("EnsureLoadBalancer returned an error: %s", err)
836838
}
837-
svc.Status.LoadBalancer = *lbStatus
839+
if lbStatus != nil {
840+
svc.Status.LoadBalancer = *lbStatus
841+
}
838842

839843
stubService(fakeClientset, svc)
840844
svc.SetAnnotations(map[string]string{
@@ -927,7 +931,9 @@ func testCreateNodeBalancerWithVPCNoFlagOrAnnotation(t *testing.T, client *linod
927931
if err != nil {
928932
t.Errorf("EnsureLoadBalancer returned an error: %s", err)
929933
}
930-
svc.Status.LoadBalancer = *lbStatus
934+
if lbStatus != nil {
935+
svc.Status.LoadBalancer = *lbStatus
936+
}
931937

932938
stubService(fakeClientset, svc)
933939
svc.SetAnnotations(map[string]string{
@@ -1017,7 +1023,9 @@ func testCreateNodeBalancerWithVPCAnnotationOnly(t *testing.T, client *linodego.
10171023
if err != nil {
10181024
t.Errorf("EnsureLoadBalancer returned an error: %s", err)
10191025
}
1020-
svc.Status.LoadBalancer = *lbStatus
1026+
if lbStatus != nil {
1027+
svc.Status.LoadBalancer = *lbStatus
1028+
}
10211029

10221030
stubServiceUpdate(fakeClientset, svc)
10231031
svc.SetAnnotations(map[string]string{})
@@ -1101,7 +1109,9 @@ func testCreateNodeBalancerWithVPCOnlySubnetIDFlag(t *testing.T, client *linodeg
11011109
if err != nil {
11021110
t.Errorf("EnsureLoadBalancer returned an error: %s", err)
11031111
}
1104-
svc.Status.LoadBalancer = *lbStatus
1112+
if lbStatus != nil {
1113+
svc.Status.LoadBalancer = *lbStatus
1114+
}
11051115

11061116
stubService(fakeClientset, svc)
11071117

@@ -1247,7 +1257,9 @@ func testUpdateLoadBalancerAddNode(t *testing.T, client *linodego.Client, f *fak
12471257
if err != nil {
12481258
t.Errorf("EnsureLoadBalancer returned an error %s", err)
12491259
}
1250-
svc.Status.LoadBalancer = *lbStatus
1260+
if lbStatus != nil {
1261+
svc.Status.LoadBalancer = *lbStatus
1262+
}
12511263

12521264
stubService(fakeClientset, svc)
12531265

@@ -1412,7 +1424,9 @@ func testUpdateLoadBalancerAddAnnotation(t *testing.T, client *linodego.Client,
14121424
if err != nil {
14131425
t.Errorf("EnsureLoadBalancer returned an error: %s", err)
14141426
}
1415-
svc.Status.LoadBalancer = *lbStatus
1427+
if lbStatus != nil {
1428+
svc.Status.LoadBalancer = *lbStatus
1429+
}
14161430

14171431
stubService(fakeClientset, svc)
14181432
svc.SetAnnotations(map[string]string{
@@ -1488,7 +1502,9 @@ func testUpdateLoadBalancerAddPortAnnotation(t *testing.T, client *linodego.Clie
14881502
if err != nil {
14891503
t.Errorf("EnsureLoadBalancer returned an error: %s", err)
14901504
}
1491-
svc.Status.LoadBalancer = *lbStatus
1505+
if lbStatus != nil {
1506+
svc.Status.LoadBalancer = *lbStatus
1507+
}
14921508
stubServiceUpdate(fakeClientset, svc)
14931509

14941510
svc.SetAnnotations(map[string]string{
@@ -1601,7 +1617,9 @@ func testVeryLongServiceName(t *testing.T, client *linodego.Client, _ *fakeAPI)
16011617
if err != nil {
16021618
t.Errorf("EnsureLoadBalancer returned an error: %s", err)
16031619
}
1604-
svc.Status.LoadBalancer = *lbStatus
1620+
if lbStatus != nil {
1621+
svc.Status.LoadBalancer = *lbStatus
1622+
}
16051623
stubServiceUpdate(fakeClientset, svc)
16061624

16071625
svc.SetAnnotations(map[string]string{
@@ -1669,7 +1687,9 @@ func testUpdateLoadBalancerAddTags(t *testing.T, client *linodego.Client, _ *fak
16691687
if err != nil {
16701688
t.Errorf("EnsureLoadBalancer returned an error: %s", err)
16711689
}
1672-
svc.Status.LoadBalancer = *lbStatus
1690+
if lbStatus != nil {
1691+
svc.Status.LoadBalancer = *lbStatus
1692+
}
16731693
stubService(fakeClientset, svc)
16741694

16751695
testTags := "test,new,tags"
@@ -1756,7 +1776,9 @@ func testUpdateLoadBalancerAddTLSPort(t *testing.T, client *linodego.Client, _ *
17561776
if err != nil {
17571777
t.Errorf("EnsureLoadBalancer returned an error: %s", err)
17581778
}
1759-
svc.Status.LoadBalancer = *lbStatus
1779+
if lbStatus != nil {
1780+
svc.Status.LoadBalancer = *lbStatus
1781+
}
17601782

17611783
stubServiceUpdate(fakeClientset, svc)
17621784
svc.Spec.Ports = append(svc.Spec.Ports, extraPort)
@@ -1963,7 +1985,9 @@ func testUpdateLoadBalancerAddNewFirewall(t *testing.T, client *linodego.Client,
19631985
if err != nil {
19641986
t.Errorf("EnsureLoadBalancer returned an error: %s", err)
19651987
}
1966-
svc.Status.LoadBalancer = *lbStatus
1988+
if lbStatus != nil {
1989+
svc.Status.LoadBalancer = *lbStatus
1990+
}
19671991
stubServiceUpdate(fakeClientset, svc)
19681992
fwClient := services.LinodeClient{Client: client}
19691993
fw, err := fwClient.CreateFirewall(t.Context(), linodego.FirewallCreateOptions{
@@ -1987,7 +2011,7 @@ func testUpdateLoadBalancerAddNewFirewall(t *testing.T, client *linodego.Client,
19872011
}()
19882012

19892013
svc.SetAnnotations(map[string]string{
1990-
annotations.AnnLinodeCloudFirewallID: strconv.Itoa(fw.ID),
2014+
annotations.AnnLinodeCloudFirewallID: strconv.Itoa(fw.ID), //nolint:nilaway // fw should not be nil if no err
19912015
})
19922016

19932017
err = lb.UpdateLoadBalancer(t.Context(), "linodelb", svc, nodes)
@@ -2063,7 +2087,9 @@ func testUpdateLoadBalancerAddNewFirewallACL(t *testing.T, client *linodego.Clie
20632087
if err != nil {
20642088
t.Errorf("EnsureLoadBalancer returned an error: %s", err)
20652089
}
2066-
svc.Status.LoadBalancer = *lbStatus
2090+
if lbStatus != nil {
2091+
svc.Status.LoadBalancer = *lbStatus
2092+
}
20672093
stubService(fakeClientset, svc)
20682094

20692095
nb, err := lb.getNodeBalancerByStatus(t.Context(), svc)
@@ -2203,7 +2229,9 @@ func testUpdateLoadBalancerDeleteFirewallRemoveACL(t *testing.T, client *linodeg
22032229
if err != nil {
22042230
t.Errorf("EnsureLoadBalancer returned an error: %s", err)
22052231
}
2206-
svc.Status.LoadBalancer = *lbStatus
2232+
if lbStatus != nil {
2233+
svc.Status.LoadBalancer = *lbStatus
2234+
}
22072235
stubService(fakeClientset, svc)
22082236

22092237
nb, err := lb.getNodeBalancerByStatus(t.Context(), svc)
@@ -2302,7 +2330,9 @@ func testUpdateLoadBalancerUpdateFirewallRemoveACLaddID(t *testing.T, client *li
23022330
if err != nil {
23032331
t.Errorf("EnsureLoadBalancer returned an error: %s", err)
23042332
}
2305-
svc.Status.LoadBalancer = *lbStatus
2333+
if lbStatus != nil {
2334+
svc.Status.LoadBalancer = *lbStatus
2335+
}
23062336
stubService(fakeClientset, svc)
23072337

23082338
nb, err := lb.getNodeBalancerByStatus(t.Context(), svc)
@@ -2350,7 +2380,7 @@ func testUpdateLoadBalancerUpdateFirewallRemoveACLaddID(t *testing.T, client *li
23502380
}()
23512381

23522382
svc.SetAnnotations(map[string]string{
2353-
annotations.AnnLinodeCloudFirewallID: strconv.Itoa(fw.ID),
2383+
annotations.AnnLinodeCloudFirewallID: strconv.Itoa(fw.ID), //nolint:nilaway // fw should not be nil if no err
23542384
})
23552385

23562386
err = lb.UpdateLoadBalancer(t.Context(), "linodelb", svc, nodes)
@@ -2448,7 +2478,7 @@ func testUpdateLoadBalancerUpdateFirewallRemoveIDaddACL(t *testing.T, client *li
24482478
}()
24492479

24502480
svc.SetAnnotations(map[string]string{
2451-
annotations.AnnLinodeCloudFirewallID: strconv.Itoa(fw.ID),
2481+
annotations.AnnLinodeCloudFirewallID: strconv.Itoa(fw.ID), //nolint:nilaway // fw should not be nil if no err
24522482
})
24532483

24542484
defer func() {
@@ -2460,7 +2490,9 @@ func testUpdateLoadBalancerUpdateFirewallRemoveIDaddACL(t *testing.T, client *li
24602490
if err != nil {
24612491
t.Errorf("EnsureLoadBalancer returned an error: %s", err)
24622492
}
2463-
svc.Status.LoadBalancer = *lbStatus
2493+
if lbStatus != nil {
2494+
svc.Status.LoadBalancer = *lbStatus
2495+
}
24642496
stubServiceUpdate(fakeClientset, svc)
24652497

24662498
nb, err := lb.getNodeBalancerByStatus(t.Context(), svc)
@@ -2581,7 +2613,9 @@ func testUpdateLoadBalancerUpdateFirewallACL(t *testing.T, client *linodego.Clie
25812613
if err != nil {
25822614
t.Errorf("EnsureLoadBalancer returned an error: %s", err)
25832615
}
2584-
svc.Status.LoadBalancer = *lbStatus
2616+
if lbStatus != nil {
2617+
svc.Status.LoadBalancer = *lbStatus
2618+
}
25852619
stubService(fakeClientset, svc)
25862620

25872621
nb, err := lb.getNodeBalancerByStatus(t.Context(), svc)
@@ -2823,14 +2857,16 @@ func testUpdateLoadBalancerUpdateFirewall(t *testing.T, client *linodego.Client,
28232857
}()
28242858

28252859
svc.SetAnnotations(map[string]string{
2826-
annotations.AnnLinodeCloudFirewallID: strconv.Itoa(fw.ID),
2860+
annotations.AnnLinodeCloudFirewallID: strconv.Itoa(fw.ID), //nolint:nilaway // fw should not be nil if no err
28272861
})
28282862

28292863
lbStatus, err := lb.EnsureLoadBalancer(t.Context(), "linodelb", svc, nodes)
28302864
if err != nil {
28312865
t.Errorf("EnsureLoadBalancer returned an error: %s", err)
28322866
}
2833-
svc.Status.LoadBalancer = *lbStatus
2867+
if lbStatus != nil {
2868+
svc.Status.LoadBalancer = *lbStatus
2869+
}
28342870
stubService(fakeClientset, svc)
28352871

28362872
nb, err := lb.getNodeBalancerByStatus(t.Context(), svc)
@@ -2956,15 +2992,17 @@ func testUpdateLoadBalancerDeleteFirewallRemoveID(t *testing.T, client *linodego
29562992
}()
29572993

29582994
svc.SetAnnotations(map[string]string{
2959-
annotations.AnnLinodeCloudFirewallID: strconv.Itoa(fw.ID),
2995+
annotations.AnnLinodeCloudFirewallID: strconv.Itoa(fw.ID), //nolint:nilaway // fw should not be nil if no err
29602996
})
29612997

29622998
stubService(fakeClientset, svc)
29632999
lbStatus, err := lb.EnsureLoadBalancer(t.Context(), "linodelb", svc, nodes)
29643000
if err != nil {
29653001
t.Errorf("EnsureLoadBalancer returned an error: %s", err)
29663002
}
2967-
svc.Status.LoadBalancer = *lbStatus
3003+
if lbStatus != nil {
3004+
svc.Status.LoadBalancer = *lbStatus
3005+
}
29683006
stubServiceUpdate(fakeClientset, svc)
29693007

29703008
nb, err := lb.getNodeBalancerByStatus(t.Context(), svc)
@@ -3077,6 +3115,12 @@ func testUpdateLoadBalancerAddReservedIP(t *testing.T, client *linodego.Client,
30773115
}
30783116

30793117
status, _, err := lb.GetLoadBalancer(t.Context(), clusterName, svc)
3118+
if err != nil {
3119+
t.Fatalf("failed to get loadbalancer status: %s", err)
3120+
}
3121+
if status == nil {
3122+
t.Fatalf("no loadbalancer status returned")
3123+
}
30803124
if status.Ingress[0].IP != initialIP {
30813125
t.Fatalf("IP should not have changed in service status: %s", err)
30823126
}
@@ -4334,6 +4378,9 @@ func testEnsureExistingLoadBalancer(t *testing.T, client *linodego.Client, _ *fa
43344378
if err != nil {
43354379
t.Fatalf("failed to create nodebalancer: %s", err)
43364380
}
4381+
if getLBStatus == nil {
4382+
t.Fatalf("failed to get nodebalancer")
4383+
}
43374384
if !exists {
43384385
t.Fatal("Node balancer not found")
43394386
}
@@ -4981,7 +5028,9 @@ func testGetLoadBalancer(t *testing.T, client *linodego.Client, _ *fakeAPI) {
49815028
defer func() { _ = lb.EnsureLoadBalancerDeleted(t.Context(), "linodelb", svc) }()
49825029

49835030
lbStatus := makeLoadBalancerStatus(svc, nb)
4984-
svc.Status.LoadBalancer = *lbStatus
5031+
if lbStatus != nil {
5032+
svc.Status.LoadBalancer = *lbStatus
5033+
}
49855034
stubService(fakeClientset, svc)
49865035
stubService(fakeClientset, svc2)
49875036

0 commit comments

Comments
 (0)