Skip to content

Commit a432151

Browse files
lucacomelucacome
authored
Enable more linters (#2545)
1 parent f74d70a commit a432151

20 files changed

+82
-66
lines changed

‎.golangci.yml

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -53,14 +53,19 @@ linters:
5353
enable:
5454
- asasalint
5555
- asciicheck
56+
- bidichk
5657
- copyloopvar
5758
- dupword
59+
- durationcheck
5860
- errcheck
61+
- errchkjson
5962
- errname
6063
- errorlint
6164
- fatcontext
6265
- ginkgolinter
6366
- gocheckcompilerdirectives
67+
- gochecksumtype
68+
- gocritic
6469
- gocyclo
6570
- godot
6671
- gofmt
@@ -76,6 +81,7 @@ linters:
7681
- loggercheck
7782
- makezero
7883
- misspell
84+
- musttag
7985
- nilerr
8086
- noctx
8187
- nolintlint
@@ -86,6 +92,7 @@ linters:
8692
- spancheck
8793
- staticcheck
8894
- stylecheck
95+
- tagalign
8996
- tenv
9097
- thelper
9198
- tparallel

‎internal/mode/provisioner/manager.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -133,7 +133,7 @@ func StartManager(cfg Config) error {
133133
statusUpdater,
134134
mgr.GetClient(),
135135
embeddedfiles.StaticModeDeploymentYAML,
136-
func() metav1.Time { return metav1.Now() },
136+
metav1.Now,
137137
)
138138

139139
eventLoop := events.NewEventLoop(

‎internal/mode/static/handler.go

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -471,8 +471,7 @@ func getGatewayAddresses(
471471
}
472472

473473
var addresses, hostnames []string
474-
switch gwSvc.Spec.Type {
475-
case v1.ServiceTypeLoadBalancer:
474+
if gwSvc.Spec.Type == v1.ServiceTypeLoadBalancer {
476475
for _, ingress := range gwSvc.Status.LoadBalancer.Ingress {
477476
if ingress.IP != "" {
478477
addresses = append(addresses, ingress.IP)

‎internal/mode/static/manager.go

Lines changed: 9 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -107,7 +107,8 @@ func StartManager(cfg config.Config) error {
107107
Namespace: cfg.GatewayPodConfig.Namespace,
108108
Name: cfg.ConfigName,
109109
}
110-
if err := registerControllers(ctx, cfg, mgr, recorder, logLevelSetter, eventCh, controlConfigNSName); err != nil {
110+
err = registerControllers(ctx, cfg, mgr, recorder, logLevelSetter, eventCh, controlConfigNSName)
111+
if err != nil {
111112
return err
112113
}
113114

@@ -149,9 +150,11 @@ func StartManager(cfg config.Config) error {
149150
processHandler := ngxruntime.NewProcessHandlerImpl(os.ReadFile, os.Stat)
150151

151152
// Ensure NGINX is running before registering metrics & starting the manager.
152-
if _, err := processHandler.FindMainProcess(ctx, ngxruntime.PidFileTimeout); err != nil {
153+
p, err := processHandler.FindMainProcess(ctx, ngxruntime.PidFileTimeout)
154+
if err != nil {
153155
return fmt.Errorf("NGINX is not running: %w", err)
154156
}
157+
cfg.Logger.V(1).Info("NGINX is running with PID", "pid", p)
155158

156159
var (
157160
ngxruntimeCollector ngxruntime.MetricsCollector = collectors.NewManagerNoopCollector()
@@ -162,11 +165,6 @@ func StartManager(cfg config.Config) error {
162165
var usageSecret *usage.Secret
163166

164167
if cfg.Plus {
165-
ngxPlusClient, err = ngxruntime.CreatePlusClient()
166-
if err != nil {
167-
return fmt.Errorf("error creating NGINX plus client: %w", err)
168-
}
169-
170168
if cfg.UsageReportConfig != nil {
171169
usageSecret = usage.NewUsageSecret()
172170
reporter, err := createUsageReporterJob(mgr.GetAPIReader(), cfg, usageSecret, nginxChecker.getReadyCh())
@@ -188,6 +186,10 @@ func StartManager(cfg config.Config) error {
188186
constLabels := map[string]string{"class": cfg.GatewayClassName}
189187
var ngxCollector prometheus.Collector
190188
if cfg.Plus {
189+
ngxPlusClient, err = ngxruntime.CreatePlusClient()
190+
if err != nil {
191+
return fmt.Errorf("error creating NGINX plus client: %w", err)
192+
}
191193
ngxCollector, err = collectors.NewNginxPlusMetricsCollector(ngxPlusClient, constLabels, promLogger)
192194
} else {
193195
ngxCollector = collectors.NewNginxMetricsCollector(constLabels, promLogger)

‎internal/mode/static/nginx/config/validation/common_test.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -48,7 +48,7 @@ func TestValidateEscapedStringNoVarExpansion(t *testing.T) {
4848

4949
func TestValidateValidHeaderName(t *testing.T) {
5050
t.Parallel()
51-
validator := func(value string) error { return validateHeaderName(value) }
51+
validator := validateHeaderName
5252

5353
testValidValuesForSimpleValidator(
5454
t,

‎internal/mode/static/nginx/file/folders.go

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -28,12 +28,12 @@ func ClearFolders(fileMgr ClearFoldersOSFileManager, paths []string) (removedFil
2828
}
2929

3030
for _, entry := range entries {
31-
path := filepath.Join(path, entry.Name())
32-
if err := fileMgr.Remove(path); err != nil {
33-
return removedFiles, fmt.Errorf("failed to remove %q: %w", path, err)
31+
entryPath := filepath.Join(path, entry.Name())
32+
if err := fileMgr.Remove(entryPath); err != nil {
33+
return removedFiles, fmt.Errorf("failed to remove %q: %w", entryPath, err)
3434
}
3535

36-
removedFiles = append(removedFiles, path)
36+
removedFiles = append(removedFiles, entryPath)
3737
}
3838
}
3939

‎internal/mode/static/nginx/runtime/manager.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -121,9 +121,9 @@ func (m *ManagerImpl) Reload(ctx context.Context, configVersion int) error {
121121

122122
// send HUP signal to the NGINX main process reload configuration
123123
// See https://nginx.org/en/docs/control.html
124-
if err := m.processHandler.Kill(pid); err != nil {
124+
if errP := m.processHandler.Kill(pid); errP != nil {
125125
m.metricsCollector.IncReloadErrors()
126-
return fmt.Errorf("failed to send the HUP signal to NGINX main: %w", err)
126+
return fmt.Errorf("failed to send the HUP signal to NGINX main: %w", errP)
127127
}
128128

129129
if err = m.verifyClient.WaitForCorrectVersion(

‎internal/mode/static/state/dataplane/configuration.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -279,12 +279,12 @@ func buildBackendGroups(servers []VirtualServer) []BackendGroup {
279279
for _, mr := range pr.MatchRules {
280280
group := mr.BackendGroup
281281

282-
key := key{
282+
k := key{
283283
nsname: group.Source,
284284
ruleIdx: group.RuleIdx,
285285
}
286286

287-
uniqueGroups[key] = group
287+
uniqueGroups[k] = group
288288
}
289289
}
290290
}

‎internal/mode/static/state/graph/backend_refs.go

Lines changed: 3 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -238,12 +238,10 @@ func validateBackendTLSPolicyMatchingAllBackends(backendRefs []BackendRef) *cond
238238
if referencePolicy == nil {
239239
// First reference, store the policy as reference
240240
referencePolicy = backendRef.BackendTLSPolicy
241-
} else {
241+
} else if checkPoliciesEqual(backendRef.BackendTLSPolicy.Source, referencePolicy.Source) {
242242
// Check if the policies match
243-
if checkPoliciesEqual(backendRef.BackendTLSPolicy.Source, referencePolicy.Source) {
244-
mismatch = true
245-
break
246-
}
243+
mismatch = true
244+
break
247245
}
248246
}
249247
if mismatch {

‎internal/mode/static/state/graph/backend_tls_policy.go

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -86,23 +86,27 @@ func validateBackendTLSPolicy(
8686

8787
caCertRefs := backendTLSPolicy.Spec.Validation.CACertificateRefs
8888
wellKnownCerts := backendTLSPolicy.Spec.Validation.WellKnownCACertificates
89-
if len(caCertRefs) > 0 && wellKnownCerts != nil {
89+
switch {
90+
case len(caCertRefs) > 0 && wellKnownCerts != nil:
9091
valid = false
9192
msg := "CACertificateRefs and WellKnownCACertificates are mutually exclusive"
9293
conds = append(conds, staticConds.NewPolicyInvalid(msg))
93-
} else if len(caCertRefs) > 0 {
94+
95+
case len(caCertRefs) > 0:
9496
if err := validateBackendTLSCACertRef(backendTLSPolicy, configMapResolver); err != nil {
9597
valid = false
9698
conds = append(conds, staticConds.NewPolicyInvalid(
9799
fmt.Sprintf("invalid CACertificateRef: %s", err.Error())))
98100
}
99-
} else if wellKnownCerts != nil {
101+
102+
case wellKnownCerts != nil:
100103
if err := validateBackendTLSWellKnownCACerts(backendTLSPolicy); err != nil {
101104
valid = false
102105
conds = append(conds, staticConds.NewPolicyInvalid(
103106
fmt.Sprintf("invalid WellKnownCACertificates: %s", err.Error())))
104107
}
105-
} else {
108+
109+
default:
106110
valid = false
107111
conds = append(conds, staticConds.NewPolicyInvalid("CACertRefs and WellKnownCACerts are both nil"))
108112
}

‎internal/mode/static/state/graph/backend_tls_policy_test.go

Lines changed: 12 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -122,7 +122,18 @@ func TestValidateBackendTLSPolicy(t *testing.T) {
122122
},
123123
}
124124

125-
localObjectRefTooManyCerts := append(localObjectRefNormalCase, localObjectRefInvalidName...)
125+
localObjectRefTooManyCerts := []gatewayv1.LocalObjectReference{
126+
{
127+
Kind: "ConfigMap",
128+
Name: "configmap",
129+
Group: "",
130+
},
131+
{
132+
Kind: "ConfigMap",
133+
Name: "invalid",
134+
Group: "",
135+
},
136+
}
126137

127138
getAncestorRef := func(ctlrName, parentName string) v1alpha2.PolicyAncestorStatus {
128139
return v1alpha2.PolicyAncestorStatus{

‎internal/mode/static/state/graph/gateway_listener.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -563,7 +563,7 @@ func GetAllowedRouteLabelSelector(l v1.Listener) *metav1.LabelSelector {
563563

564564
// matchesWildcard checks if hostname2 matches the wildcard pattern of hostname1.
565565
func matchesWildcard(hostname1, hostname2 string) bool {
566-
matchesWildcard := func(h1, h2 string) bool {
566+
mw := func(h1, h2 string) bool {
567567
if strings.HasPrefix(h1, "*.") {
568568
// Remove the "*." from h1
569569
h1 = h1[2:]
@@ -572,7 +572,7 @@ func matchesWildcard(hostname1, hostname2 string) bool {
572572
}
573573
return false
574574
}
575-
return matchesWildcard(hostname1, hostname2) || matchesWildcard(hostname2, hostname1)
575+
return mw(hostname1, hostname2) || mw(hostname2, hostname1)
576576
}
577577

578578
// haveOverlap checks for overlap between two hostnames.

‎internal/mode/static/state/graph/secret.go

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -44,11 +44,14 @@ func (r *secretResolver) resolve(nsname types.NamespacedName) error {
4444

4545
var validationErr error
4646

47-
if !exist {
47+
switch {
48+
case !exist:
4849
validationErr = errors.New("secret does not exist")
49-
} else if secret.Type != apiv1.SecretTypeTLS {
50+
51+
case secret.Type != apiv1.SecretTypeTLS:
5052
validationErr = fmt.Errorf("secret type must be %q not %q", apiv1.SecretTypeTLS, secret.Type)
51-
} else {
53+
54+
default:
5255
// A TLS Secret is guaranteed to have these data fields.
5356
_, err := tls.X509KeyPair(secret.Data[apiv1.TLSCertKey], secret.Data[apiv1.TLSPrivateKeyKey])
5457
if err != nil {

‎internal/mode/static/state/resolver/resolver.go

Lines changed: 2 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -155,11 +155,8 @@ func resolveEndpoints(
155155
// If the ServicePort has a non-zero integer TargetPort, the TargetPort integer value is returned.
156156
// Otherwise, the ServicePort port value is returned.
157157
func getDefaultPort(svcPort v1.ServicePort) int32 {
158-
switch svcPort.TargetPort.Type {
159-
case intstr.Int:
160-
if svcPort.TargetPort.IntVal != 0 {
161-
return svcPort.TargetPort.IntVal
162-
}
158+
if svcPort.TargetPort.Type == intstr.Int && svcPort.TargetPort.IntVal != 0 {
159+
return svcPort.TargetPort.IntVal
163160
}
164161

165162
return svcPort.Port

‎internal/mode/static/status/prepare_requests.go

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -68,7 +68,8 @@ func PrepareRouteRequests(
6868
r.Source.GetGeneration(),
6969
)
7070

71-
if r.RouteType == graph.RouteTypeHTTP {
71+
switch r.RouteType {
72+
case graph.RouteTypeHTTP:
7273
status := v1.HTTPRouteStatus{
7374
RouteStatus: routeStatus,
7475
}
@@ -80,7 +81,8 @@ func PrepareRouteRequests(
8081
}
8182

8283
reqs = append(reqs, req)
83-
} else if r.RouteType == graph.RouteTypeGRPC {
84+
85+
case graph.RouteTypeGRPC:
8486
status := v1.GRPCRouteStatus{
8587
RouteStatus: routeStatus,
8688
}
@@ -92,7 +94,8 @@ func PrepareRouteRequests(
9294
}
9395

9496
reqs = append(reqs, req)
95-
} else {
97+
98+
default:
9699
panic(fmt.Sprintf("Unknown route type: %s", r.RouteType))
97100
}
98101
}

‎internal/mode/static/telemetry/collector.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -245,10 +245,10 @@ func computeRouteCount(
245245

246246
for _, r := range routes {
247247
if r.RouteType == graph.RouteTypeHTTP {
248-
httpRouteCount = httpRouteCount + 1
248+
httpRouteCount++
249249
}
250250
if r.RouteType == graph.RouteTypeGRPC {
251-
grpcRouteCount = grpcRouteCount + 1
251+
grpcRouteCount++
252252
}
253253
}
254254

‎internal/mode/static/usage/job_worker_test.go

Lines changed: 3 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -58,8 +58,7 @@ func TestCreateUsageJobWorker(t *testing.T) {
5858
return nil
5959
},
6060
getCalls: func(_ context.Context, _ types.NamespacedName, object client.Object, _ ...client.GetOption) error {
61-
switch typedObject := object.(type) {
62-
case *v1.Namespace:
61+
if typedObject, ok := object.(*v1.Namespace); ok {
6362
typedObject.Name = metav1.NamespaceSystem
6463
typedObject.UID = "1234abcd"
6564
return nil
@@ -83,8 +82,7 @@ func TestCreateUsageJobWorker(t *testing.T) {
8382
{
8483
name: "collect node count fails",
8584
listCalls: func(_ context.Context, object client.ObjectList, _ ...client.ListOption) error {
86-
switch object.(type) {
87-
case *v1.NodeList:
85+
if _, ok := object.(*v1.NodeList); ok {
8886
return errors.New("failed to collect node list")
8987
}
9088
return nil
@@ -127,8 +125,7 @@ func TestCreateUsageJobWorker(t *testing.T) {
127125
return nil
128126
},
129127
getCalls: func(_ context.Context, _ types.NamespacedName, object client.Object, _ ...client.GetOption) error {
130-
switch object.(type) {
131-
case *v1.Namespace:
128+
if _, ok := object.(*v1.Namespace); ok {
132129
return errors.New("failed to collect namespace")
133130
}
134131
return nil

‎tests/framework/ngf.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -73,7 +73,7 @@ func InstallNGF(cfg InstallationConfig, extraArgs ...string) ([]byte, error) {
7373
}
7474

7575
args = append(args, setImageArgs(cfg)...)
76-
fullArgs := append(args, extraArgs...)
76+
fullArgs := append(args, extraArgs...) //nolint:gocritic
7777

7878
GinkgoWriter.Printf("Installing NGF with command: helm %v\n", strings.Join(fullArgs, " "))
7979

@@ -102,7 +102,7 @@ func UpgradeNGF(cfg InstallationConfig, extraArgs ...string) ([]byte, error) {
102102
}
103103

104104
args = append(args, setImageArgs(cfg)...)
105-
fullArgs := append(args, extraArgs...)
105+
fullArgs := append(args, extraArgs...) //nolint:gocritic
106106

107107
GinkgoWriter.Printf("Upgrading NGF with command: helm %v\n", strings.Join(fullArgs, " "))
108108

‎tests/suite/system_suite_test.go

Lines changed: 7 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -148,11 +148,12 @@ func setup(cfg setupConfig, extraInstallArgs ...string) {
148148
Skip("Graceful Recovery test must be run on Kind")
149149
}
150150

151-
if *versionUnderTest != "" {
151+
switch {
152+
case *versionUnderTest != "":
152153
version = *versionUnderTest
153-
} else if *imageTag != "" {
154+
case *imageTag != "":
154155
version = *imageTag
155-
} else {
156+
default:
156157
version = "edge"
157158
}
158159

@@ -203,11 +204,9 @@ func createNGFInstallConfig(cfg setupConfig, extraInstallArgs ...string) framewo
203204
}
204205
installCfg.ImageTag = *imageTag
205206
installCfg.ImagePullPolicy = *imagePullPolicy
206-
} else {
207-
if version == "edge" {
208-
chartVersion = "0.0.0-edge"
209-
installCfg.ChartVersion = chartVersion
210-
}
207+
} else if version == "edge" {
208+
chartVersion = "0.0.0-edge"
209+
installCfg.ChartVersion = chartVersion
211210
}
212211

213212
output, err := framework.InstallGatewayAPI(cfg.gwAPIVersion)

0 commit comments

Comments
 (0)