Skip to content

Commit 212c0f6

Browse files
authored
Run golang-ci lint for all the modules (#2081)
1 parent fc2ed96 commit 212c0f6

File tree

13 files changed

+59
-44
lines changed

13 files changed

+59
-44
lines changed

.github/workflows/lint.yml

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,10 @@ jobs:
2323
lint:
2424
name: Lint
2525
runs-on: ubuntu-22.04
26+
strategy:
27+
fail-fast: false
28+
matrix:
29+
directory: [., tests] # we need to run golangci-lint for every module https://github.com/golangci/golangci-lint/issues/828
2630
steps:
2731
- name: Checkout Repository
2832
uses: actions/checkout@a5ac7e51b41094c92402da3b24376905380afc29 # v4.1.6
@@ -36,6 +40,7 @@ jobs:
3640
uses: golangci/golangci-lint-action@a4f60bb28d35aeee14e6880718e0c85ff1882e64 # v6.0.1
3741
with:
3842
args: --timeout 10m0s
43+
working-directory: ${{ matrix.directory }}
3944

4045
njs-lint:
4146
name: NJS Lint

.golangci.yml

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,6 @@ linters-settings:
1717
- name: error-strings
1818
- name: errorf
1919
- name: exported
20-
- name: if-return
2120
- name: increment-decrement
2221
- name: indent-error-flow
2322
- name: package-comments

.pre-commit-config.yaml

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -41,6 +41,13 @@ repos:
4141
rev: v1.59.0
4242
hooks:
4343
- id: golangci-lint-full
44+
name: golangci-lint-root
45+
alias: golangci-lint-root
46+
47+
- id: golangci-lint-full
48+
name: golangci-lint-tests
49+
alias: golangci-lint-tests
50+
entry: bash -c 'cd tests && golangci-lint run --fix --config $OLDPWD/.golangci.yml'
4451

4552
# Rules are in .markdownlint-cli2.yaml file
4653
# See https://github.com/DavidAnson/markdownlint/blob/main/doc/Rules.md for rule descriptions

Makefile

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,9 @@ GO_LINKER_FlAGS_VARS = -X main.version=${VERSION} -X main.commit=${GIT_COMMIT} -
2525
GO_LINKER_FLAGS_OPTIMIZATIONS = -s -w
2626
GO_LINKER_FLAGS = $(GO_LINKER_FLAGS_OPTIMIZATIONS) $(GO_LINKER_FlAGS_VARS)
2727

28+
# tools versions
29+
GOLANGCI_LINT_VERSION := $(shell awk '/repo:.*golangci-lint/{getline; if ($$1 == "rev:") {sub(/^v/, "", $$2); print $$2}}' $(SELF_DIR).pre-commit-config.yaml)
30+
2831
# variables that can be overridden by the user
2932
PREFIX ?= nginx-gateway-fabric## The name of the NGF image. For example, nginx-gateway-fabric
3033
NGINX_PREFIX ?= $(PREFIX)/nginx## The name of the nginx image. For example: nginx-gateway-fabric/nginx
@@ -168,9 +171,14 @@ njs-fmt: ## Run prettier against the njs httpmatches module
168171
vet: ## Run go vet against code
169172
go vet ./...
170173

174+
.PHONY: check-golangci-lint
175+
check-golangci-lint:
176+
@golangci-lint --version || (code=$$?; printf "\033[0;31mError\033[0m: there was a problem with golangci-lint. Follow the docs to install it https://golangci-lint.run/welcome/install/\n"; exit $$code)
177+
@golangci-lint --version | grep -q $(GOLANGCI_LINT_VERSION) || (printf "\033[0;33mWarning\033[0m: your golangci-lint version is different from the one specified in .pre-commit-config.yaml. The recommended version is $(GOLANGCI_LINT_VERSION)\n")
178+
171179
.PHONY: lint
172-
lint: ## Run golangci-lint against code
173-
docker run --pull always --rm -v $(CURDIR):/nginx-gateway-fabric -w /nginx-gateway-fabric -v $(shell go env GOCACHE):/cache/go -e GOCACHE=/cache/go -e GOLANGCI_LINT_CACHE=/cache/go -v $(shell go env GOPATH)/pkg:/go/pkg golangci/golangci-lint:latest golangci-lint --color always run
180+
lint: check-golangci-lint ## Run golangci-lint against code
181+
golangci-lint run
174182

175183
.PHONY: unit-test
176184
unit-test: ## Run unit tests for the go code

internal/mode/static/handler.go

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -282,9 +282,9 @@ func (h *eventHandlerImpl) updateStatuses(ctx context.Context, logger logr.Logge
282282
func (h *eventHandlerImpl) parseAndCaptureEvent(ctx context.Context, logger logr.Logger, event interface{}) {
283283
switch e := event.(type) {
284284
case *events.UpsertEvent:
285-
filterKey := objectFilterKey(e.Resource, client.ObjectKeyFromObject(e.Resource))
285+
upFilterKey := objectFilterKey(e.Resource, client.ObjectKeyFromObject(e.Resource))
286286

287-
if filter, ok := h.objectFilters[filterKey]; ok {
287+
if filter, ok := h.objectFilters[upFilterKey]; ok {
288288
filter.upsert(ctx, logger, e.Resource)
289289
if !filter.captureChangeInGraph {
290290
return
@@ -293,9 +293,9 @@ func (h *eventHandlerImpl) parseAndCaptureEvent(ctx context.Context, logger logr
293293

294294
h.cfg.processor.CaptureUpsertChange(e.Resource)
295295
case *events.DeleteEvent:
296-
filterKey := objectFilterKey(e.Type, e.NamespacedName)
296+
delFilterKey := objectFilterKey(e.Type, e.NamespacedName)
297297

298-
if filter, ok := h.objectFilters[filterKey]; ok {
298+
if filter, ok := h.objectFilters[delFilterKey]; ok {
299299
filter.delete(ctx, logger, e.NamespacedName)
300300
if !filter.captureChangeInGraph {
301301
return
@@ -359,14 +359,14 @@ func (h *eventHandlerImpl) updateUpstreamServers(
359359
}
360360

361361
for _, u := range conf.Upstreams {
362-
upstream := upstream{
362+
confUpstream := upstream{
363363
name: u.Name,
364364
servers: ngxConfig.ConvertEndpoints(u.Endpoints),
365365
}
366366

367-
if u, ok := prevUpstreams[upstream.name]; ok {
368-
if !serversEqual(upstream.servers, u.Peers) {
369-
upstreams = append(upstreams, upstream)
367+
if u, ok := prevUpstreams[confUpstream.name]; ok {
368+
if !serversEqual(confUpstream.servers, u.Peers) {
369+
upstreams = append(upstreams, confUpstream)
370370
}
371371
}
372372
}

tests/Makefile

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,6 @@ GW_API_VERSION ?= $(shell sed -n 's/.*ref=v\(.*\)/\1/p' ../config/crd/gateway-ap
99
GW_API_PREV_VERSION ?= 1.0.0## Supported Gateway API version from previous NGF release
1010
GW_SERVICE_TYPE = NodePort## Service type to use for the gateway
1111
GW_SVC_GKE_INTERNAL = false
12-
K8S_VERSION ?= latest## Kubernetes version to use. Expected format: 1.24 (major.minor) or latest
1312
NGF_VERSION ?= $(shell git describe --tags $(shell git rev-list --tags --max-count=1))## NGF version to be tested (defaults to latest tag)
1413
PULL_POLICY = Never## Pull policy for the images
1514
PROVISIONER_MANIFEST = conformance/provisioner/provisioner.yaml
@@ -114,7 +113,7 @@ stop-longevity-test: nfr-test ## Stop the longevity test and collects results
114113
--label-filter "nfr" $(GINKGO_FLAGS) ./suite -- --gateway-api-version=$(GW_API_VERSION) \
115114
--gateway-api-prev-version=$(GW_API_PREV_VERSION) --image-tag=$(TAG) --version-under-test=$(NGF_VERSION) \
116115
--plus-enabled=$(PLUS_ENABLED) --ngf-image-repo=$(PREFIX) --nginx-image-repo=$(NGINX_PREFIX) --nginx-plus-image-repo=$(NGINX_PLUS_PREFIX) \
117-
--pull-policy=$(PULL_POLICY) --k8s-version=$(K8S_VERSION) --service-type=$(GW_SERVICE_TYPE) \
116+
--pull-policy=$(PULL_POLICY) --service-type=$(GW_SERVICE_TYPE) \
118117
--is-gke-internal-lb=$(GW_SVC_GKE_INTERNAL)
119118

120119
.PHONY: test
@@ -124,7 +123,7 @@ test: ## Runs the functional tests on your default k8s cluster
124123
--gateway-api-version=$(GW_API_VERSION) --gateway-api-prev-version=$(GW_API_PREV_VERSION) \
125124
--image-tag=$(TAG) --version-under-test=$(NGF_VERSION) \
126125
--plus-enabled=$(PLUS_ENABLED) --ngf-image-repo=$(PREFIX) --nginx-image-repo=$(NGINX_PREFIX) --nginx-plus-image-repo=$(NGINX_PLUS_PREFIX) \
127-
--pull-policy=$(PULL_POLICY) --k8s-version=$(K8S_VERSION) --service-type=$(GW_SERVICE_TYPE) \
126+
--pull-policy=$(PULL_POLICY) --service-type=$(GW_SERVICE_TYPE) \
128127
--is-gke-internal-lb=$(GW_SVC_GKE_INTERNAL)
129128

130129
.PHONY: test-with-plus

tests/framework/ngf.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -43,7 +43,7 @@ func InstallGatewayAPI(apiVersion string) ([]byte, error) {
4343
}
4444

4545
// UninstallGatewayAPI uninstalls the specified version of the Gateway API resources.
46-
func UninstallGatewayAPI(apiVersion, k8sVersion string) ([]byte, error) {
46+
func UninstallGatewayAPI(apiVersion string) ([]byte, error) {
4747
apiPath := fmt.Sprintf("%s/v%s/standard-install.yaml", gwInstallBasePath, apiVersion)
4848

4949
output, err := exec.Command("kubectl", "delete", "-f", apiPath).CombinedOutput()

tests/framework/portforward.go

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3,13 +3,12 @@ package framework
33
import (
44
"bytes"
55
"fmt"
6+
"log/slog"
67
"net/http"
78
"net/url"
89
"path"
910
"time"
1011

11-
"log/slog"
12-
1312
"k8s.io/client-go/rest"
1413
"k8s.io/client-go/tools/portforward"
1514
"k8s.io/client-go/transport/spdy"

tests/framework/prometheus.go

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -61,6 +61,7 @@ func InstallPrometheus(
6161

6262
scrapeInterval := fmt.Sprintf("%ds", int(cfg.ScrapeInterval.Seconds()))
6363

64+
// nolint:gosec
6465
output, err = exec.Command(
6566
"helm",
6667
"install",
@@ -134,13 +135,12 @@ const (
134135

135136
// PrometheusInstance represents a Prometheus instance in the cluster.
136137
type PrometheusInstance struct {
138+
apiClient v1.API
137139
podIP string
138140
podName string
139141
podNamespace string
140-
portForward bool
141142
queryTimeout time.Duration
142-
143-
apiClient v1.API
143+
portForward bool
144144
}
145145

146146
// PortForward starts port forwarding to the Prometheus instance.
@@ -165,7 +165,7 @@ func (ins *PrometheusInstance) getAPIClient() (v1.API, error) {
165165
}
166166

167167
cfg := api.Config{
168-
Address: fmt.Sprintf("%s", endpoint),
168+
Address: endpoint,
169169
}
170170

171171
c, err := api.NewClient(cfg)
@@ -227,7 +227,9 @@ func (ins *PrometheusInstance) QueryRange(query string, promRange v1.Range) (mod
227227
}
228228

229229
// QueryRangeWithCtx sends a range query to Prometheus with the specified context.
230-
func (ins *PrometheusInstance) QueryRangeWithCtx(ctx context.Context, query string, promRange v1.Range) (model.Value, error) {
230+
func (ins *PrometheusInstance) QueryRangeWithCtx(ctx context.Context,
231+
query string, promRange v1.Range,
232+
) (model.Value, error) {
231233
if err := ins.ensureAPIClient(); err != nil {
232234
return nil, err
233235
}

tests/framework/resourcemanager.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -307,7 +307,7 @@ func (rm *ResourceManager) WaitForAppsToBeReady(namespace string) error {
307307
}
308308

309309
// WaitForAppsToBeReadyWithCtx waits for all apps in the specified namespace to be ready or
310-
// until the provided context is cancelled.
310+
// until the provided context is canceled.
311311
func (rm *ResourceManager) WaitForAppsToBeReadyWithCtx(ctx context.Context, namespace string) error {
312312
if err := rm.WaitForPodsToBeReady(ctx, namespace); err != nil {
313313
return err
@@ -325,7 +325,7 @@ func (rm *ResourceManager) WaitForAppsToBeReadyWithCtx(ctx context.Context, name
325325
}
326326

327327
// WaitForPodsToBeReady waits for all Pods in the specified namespace to be ready or
328-
// until the provided context is cancelled.
328+
// until the provided context is canceled.
329329
func (rm *ResourceManager) WaitForPodsToBeReady(ctx context.Context, namespace string) error {
330330
return wait.PollUntilContextCancel(
331331
ctx,

tests/suite/graceful_recovery_test.go

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -191,7 +191,8 @@ func checkContainerRestart(ngfPodName, containerName string, currentRestartCount
191191
}
192192

193193
if restartCount != currentRestartCount+1 {
194-
return fmt.Errorf("expected current restart count: %d to match incremented restart count: %d", restartCount, currentRestartCount+1)
194+
return fmt.Errorf("expected current restart count: %d to match incremented restart count: %d",
195+
restartCount, currentRestartCount+1)
195196
}
196197

197198
return nil
@@ -314,7 +315,7 @@ func getContainerRestartCount(ngfPodName, containerName string) (int, error) {
314315

315316
var ngfPod core.Pod
316317
if err := k8sClient.Get(ctx, types.NamespacedName{Namespace: ngfNamespace, Name: ngfPodName}, &ngfPod); err != nil {
317-
return 0, fmt.Errorf("error retriving NGF Pod: %w", err)
318+
return 0, fmt.Errorf("error retrieving NGF Pod: %w", err)
318319
}
319320

320321
var restartCount int
@@ -333,7 +334,7 @@ func runNodeDebuggerJob(ngfPodName, jobScript string) (*v1.Job, error) {
333334

334335
var ngfPod core.Pod
335336
if err := k8sClient.Get(ctx, types.NamespacedName{Namespace: ngfNamespace, Name: ngfPodName}, &ngfPod); err != nil {
336-
return nil, fmt.Errorf("error retriving NGF Pod: %w", err)
337+
return nil, fmt.Errorf("error retrieving NGF Pod: %w", err)
337338
}
338339

339340
b, err := resourceManager.GetFileContents("graceful-recovery/node-debugger-job.yaml")

tests/suite/scale_test.go

Lines changed: 11 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -571,7 +571,7 @@ var _ = Describe("Scale test", Ordered, Label("nfr", "scale"), func() {
571571
const testName = "TestScale_Listeners"
572572

573573
testResultsDir := filepath.Join(resultsDir, testName)
574-
Expect(os.MkdirAll(testResultsDir, 0755)).To(Succeed())
574+
Expect(os.MkdirAll(testResultsDir, 0o755)).To(Succeed())
575575

576576
objects, err := framework.GenerateScaleListenerObjects(httpListenerCount, false /*non-tls*/)
577577
Expect(err).ToNot(HaveOccurred())
@@ -734,22 +734,18 @@ type bucket struct {
734734
}
735735

736736
type scaleTestResults struct {
737-
Name string
738-
739-
ReloadCount int
740-
ReloadErrsCount int
741-
ReloadAvgTime int
742-
ReloadBuckets []bucket
743-
744-
EventsCount int
745-
EventsAvgTime int
746-
EventsBuckets []bucket
747-
748-
NGFErrors int
749-
NginxErrors int
750-
737+
Name string
738+
EventsBuckets []bucket
739+
ReloadBuckets []bucket
740+
EventsAvgTime int
741+
EventsCount int
751742
NGFContainerRestarts int
743+
NGFErrors int
752744
NginxContainerRestarts int
745+
NginxErrors int
746+
ReloadAvgTime int
747+
ReloadCount int
748+
ReloadErrsCount int
753749
}
754750

755751
const scaleResultTemplate = `

tests/suite/system_suite_test.go

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -48,7 +48,6 @@ var (
4848
gatewayAPIPrevVersion = flag.String(
4949
"gateway-api-prev-version", "", "Supported Gateway API version for previous NGF release",
5050
)
51-
k8sVersion = flag.String("k8s-version", "latest", "Version of k8s being tested on")
5251
// Configurable NGF installation variables. Helm values will be used as defaults if not specified.
5352
ngfImageRepository = flag.String("ngf-image-repo", "", "Image repo for NGF control plane")
5453
nginxImageRepository = flag.String("nginx-image-repo", "", "Image repo for NGF data plane")
@@ -214,7 +213,7 @@ func teardown(relName string) {
214213
output, err := framework.UninstallNGF(cfg, k8sClient)
215214
Expect(err).ToNot(HaveOccurred(), string(output))
216215

217-
output, err = framework.UninstallGatewayAPI(*gatewayAPIVersion, *k8sVersion)
216+
output, err = framework.UninstallGatewayAPI(*gatewayAPIVersion)
218217
Expect(err).ToNot(HaveOccurred(), string(output))
219218

220219
ctx, cancel := context.WithTimeout(context.Background(), 5*time.Minute)

0 commit comments

Comments
 (0)