Skip to content

Commit 3c2cec8

Browse files
mmorel-35mmorel-35
authored
chore: enable several rules from go-critic (#1478)
* chore: enable-all rules from go-critic Signed-off-by: Matthieu MOREL <[email protected]> * chore: enable emptyStringTest rule from go-critic Signed-off-by: Matthieu MOREL <[email protected]> * chore: enable exposedSyncMutex rule from go-critic Signed-off-by: Matthieu MOREL <[email protected]> * chore: enable ifElseChain rule from go-critic Signed-off-by: Matthieu MOREL <[email protected]> * chore: enable nestingReduce rule from go-critic Signed-off-by: Matthieu MOREL <[email protected]> * chore: enable regexpMust rule from go-critic Signed-off-by: Matthieu MOREL <[email protected]> * chore: enable regexpSimplify rule from go-critic Signed-off-by: Matthieu MOREL <[email protected]> * chore: enable singleCaseSwitch rule from go-critic Signed-off-by: Matthieu MOREL <[email protected]> * chore: enable typeDefFirst rule from go-critic Signed-off-by: Matthieu MOREL <[email protected]> * chore: enable typeSwitchVar rule from go-critic Signed-off-by: Matthieu MOREL <[email protected]> * chore: enable unlambda rule from go-critic Signed-off-by: Matthieu MOREL <[email protected]> * fic paramTypeCombine Signed-off-by: Matthieu MOREL <[email protected]> --------- Signed-off-by: Matthieu MOREL <[email protected]>
1 parent b827dd9 commit 3c2cec8

File tree

9 files changed

+86
-80
lines changed

9 files changed

+86
-80
lines changed

‎.golangci.yml‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ version: "2"
22
linters:
33
enable:
44
- errorlint
5+
- gocritic
56
- misspell
67
- revive
78
- sloglint
@@ -11,6 +12,11 @@ linters:
1112
errcheck:
1213
exclude-functions:
1314
- (net/http.ResponseWriter).Write
15+
gocritic:
16+
enable-all: true
17+
disabled-checks:
18+
- hugeParam
19+
- unnamedResult
1420
revive:
1521
rules:
1622
- name: unused-parameter

‎collector/collector.go‎

Lines changed: 12 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -305,7 +305,7 @@ type Collector struct {
305305
debugSNMP bool
306306
}
307307

308-
func New(ctx context.Context, target, authName, snmpContext string, snmpEngineID string, auth *config.Auth, modules []*NamedModule, logger *slog.Logger, metrics Metrics, conc int, debugSNMP bool) *Collector {
308+
func New(ctx context.Context, target, authName, snmpContext, snmpEngineID string, auth *config.Auth, modules []*NamedModule, logger *slog.Logger, metrics Metrics, conc int, debugSNMP bool) *Collector {
309309
return &Collector{
310310
ctx: ctx,
311311
target: target,
@@ -809,36 +809,36 @@ func splitOid(oid []int, count int) ([]int, []int) {
809809

810810
// This mirrors decodeValue in gosnmp's helper.go.
811811
func pduValueAsString(pdu *gosnmp.SnmpPDU, typ string, metrics Metrics) string {
812-
switch pdu.Value.(type) {
812+
switch v := pdu.Value.(type) {
813813
case int:
814-
return strconv.Itoa(pdu.Value.(int))
814+
return strconv.Itoa(v)
815815
case uint:
816-
return strconv.FormatUint(uint64(pdu.Value.(uint)), 10)
816+
return strconv.FormatUint(uint64(v), 10)
817817
case uint64:
818-
return strconv.FormatUint(pdu.Value.(uint64), 10)
818+
return strconv.FormatUint(v, 10)
819819
case float32:
820-
return strconv.FormatFloat(float64(pdu.Value.(float32)), 'f', -1, 32)
820+
return strconv.FormatFloat(float64(v), 'f', -1, 32)
821821
case float64:
822-
return strconv.FormatFloat(pdu.Value.(float64), 'f', -1, 64)
822+
return strconv.FormatFloat(v, 'f', -1, 64)
823823
case string:
824824
if pdu.Type == gosnmp.ObjectIdentifier {
825825
// Trim leading period.
826-
return pdu.Value.(string)[1:]
826+
return v[1:]
827827
}
828828
// DisplayString.
829-
return strings.ToValidUTF8(pdu.Value.(string), "�")
829+
return strings.ToValidUTF8(v, "�")
830830
case []byte:
831831
if typ == "" || typ == "Bits" {
832832
typ = "OctetString"
833833
}
834834
// Reuse the OID index parsing code.
835-
parts := make([]int, len(pdu.Value.([]byte)))
836-
for i, o := range pdu.Value.([]byte) {
835+
parts := make([]int, len(v))
836+
for i, o := range v {
837837
parts[i] = int(o)
838838
}
839839
if typ == "OctetString" || typ == "DisplayString" {
840840
// Prepend the length, as it is explicit in an index.
841-
parts = append([]int{len(pdu.Value.([]byte))}, parts...)
841+
parts = append([]int{len(v)}, parts...)
842842
}
843843
str, _, _ := indexOidsAsString(parts, typ, 0, false, nil)
844844
return strings.ToValidUTF8(str, "�")

‎collector/collector_test.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -186,7 +186,7 @@ func TestPduToSample(t *testing.T) {
186186
"Template": []config.RegexpExtract{
187187
{
188188
Regex: config.Regexp{
189-
regexp.MustCompile("([0-9].[0-9]+)"),
189+
regexp.MustCompile(`(\d.\d+)`),
190190
},
191191
Value: "$1",
192192
},

‎config/config.go‎

Lines changed: 13 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,18 @@ import (
2626
"gopkg.in/yaml.v2"
2727
)
2828

29+
type Auth struct {
30+
Community Secret `yaml:"community,omitempty"`
31+
SecurityLevel string `yaml:"security_level,omitempty"`
32+
Username string `yaml:"username,omitempty"`
33+
Password Secret `yaml:"password,omitempty"`
34+
AuthProtocol string `yaml:"auth_protocol,omitempty"`
35+
PrivProtocol string `yaml:"priv_protocol,omitempty"`
36+
PrivPassword Secret `yaml:"priv_password,omitempty"`
37+
ContextName string `yaml:"context_name,omitempty"`
38+
Version int `yaml:"version,omitempty"`
39+
}
40+
2941
func LoadFile(logger *slog.Logger, paths []string, expandEnvVars bool) (*Config, error) {
3042
cfg := &Config{}
3143
for _, p := range paths {
@@ -271,18 +283,6 @@ func (s Secret) MarshalYAML() (interface{}, error) {
271283
return nil, nil
272284
}
273285

274-
type Auth struct {
275-
Community Secret `yaml:"community,omitempty"`
276-
SecurityLevel string `yaml:"security_level,omitempty"`
277-
Username string `yaml:"username,omitempty"`
278-
Password Secret `yaml:"password,omitempty"`
279-
AuthProtocol string `yaml:"auth_protocol,omitempty"`
280-
PrivProtocol string `yaml:"priv_protocol,omitempty"`
281-
PrivPassword Secret `yaml:"priv_password,omitempty"`
282-
ContextName string `yaml:"context_name,omitempty"`
283-
Version int `yaml:"version,omitempty"`
284-
}
285-
286286
func (c *Auth) UnmarshalYAML(unmarshal func(interface{}) error) error {
287287
*c = DefaultAuth
288288
type plain Auth
@@ -361,9 +361,7 @@ func (re *Regexp) UnmarshalYAML(unmarshal func(interface{}) error) error {
361361
}
362362

363363
func substituteEnvVariables(value string) (string, error) {
364-
result := os.Expand(value, func(s string) string {
365-
return os.Getenv(s)
366-
})
364+
result := os.Expand(value, os.Getenv)
367365
if result == "" {
368366
return "", errors.New(value + " environment variable not found")
369367
}

‎config_test.go‎

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -31,9 +31,9 @@ func TestHideConfigSecrets(t *testing.T) {
3131
}
3232

3333
// String method must not reveal authentication credentials.
34-
sc.RLock()
34+
sc.mu.RLock()
3535
c, err := yaml.Marshal(sc.C)
36-
sc.RUnlock()
36+
sc.mu.RUnlock()
3737
if err != nil {
3838
t.Errorf("Error marshaling config: %v", err)
3939
}
@@ -48,9 +48,9 @@ func TestLoadConfigWithOverrides(t *testing.T) {
4848
if err != nil {
4949
t.Errorf("Error loading config %v: %v", "testdata/snmp-with-overrides.yml", err)
5050
}
51-
sc.RLock()
51+
sc.mu.RLock()
5252
_, err = yaml.Marshal(sc.C)
53-
sc.RUnlock()
53+
sc.mu.RUnlock()
5454
if err != nil {
5555
t.Errorf("Error marshaling config: %v", err)
5656
}
@@ -63,9 +63,9 @@ func TestLoadMultipleConfigs(t *testing.T) {
6363
if err != nil {
6464
t.Errorf("Error loading configs %v: %v", configs, err)
6565
}
66-
sc.RLock()
66+
sc.mu.RLock()
6767
_, err = yaml.Marshal(sc.C)
68-
sc.RUnlock()
68+
sc.mu.RUnlock()
6969
if err != nil {
7070
t.Errorf("Error marshaling config: %v", err)
7171
}
@@ -84,9 +84,9 @@ func TestEnvSecrets(t *testing.T) {
8484
}
8585

8686
// String method must not reveal authentication credentials.
87-
sc.RLock()
87+
sc.mu.RLock()
8888
c, err := yaml.Marshal(sc.C)
89-
sc.RUnlock()
89+
sc.mu.RUnlock()
9090
if err != nil {
9191
t.Errorf("Error marshaling config: %v", err)
9292
}

‎generator/main.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -170,7 +170,7 @@ func main() {
170170
func scanParseOutput(logger *slog.Logger, output string) []string {
171171
var parseOutput []string
172172
output = strings.TrimSpace(strings.ToValidUTF8(output, "�"))
173-
if len(output) > 0 {
173+
if output != "" {
174174
parseOutput = strings.Split(output, "\n")
175175
}
176176
parseErrors := len(parseOutput)

‎generator/tree.go‎

Lines changed: 34 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -107,8 +107,7 @@ func prepareTree(nodes *Node, logger *slog.Logger) map[string]*Node {
107107
walkNode(nodes, func(n *Node) {
108108
// Set type on MAC addresses and strings.
109109
// RFC 2579
110-
switch n.Hint {
111-
case "1x:":
110+
if n.Hint == "1x:" {
112111
n.Type = "PhysAddress48"
113112
}
114113
if displayStringRe.MatchString(n.Hint) {
@@ -395,11 +394,12 @@ func generateConfigModule(cfg *ModuleConfig, node *Node, nameToNode map[string]*
395394

396395
// Convert (InetAddressType,InetAddress) to (InetAddress)
397396
if subtype, ok := combinedTypes[index.Type]; ok {
398-
if prevType == subtype {
397+
switch subtype {
398+
case prevType:
399399
metric.Indexes = metric.Indexes[:len(metric.Indexes)-1]
400-
} else if prev2Type == subtype {
400+
case prev2Type:
401401
metric.Indexes = metric.Indexes[:len(metric.Indexes)-2]
402-
} else {
402+
default:
403403
logger.Warn("Can't handle index type on node, missing preceding", "node", n.Label, "type", index.Type, "missing", subtype)
404404
return
405405
}
@@ -513,41 +513,43 @@ func generateConfigModule(cfg *ModuleConfig, node *Node, nameToNode map[string]*
513513
// Check that the object before an InetAddress is an InetAddressType.
514514
// If not, change it to an OctetString.
515515
for _, metric := range out.Metrics {
516-
if metric.Type == "InetAddress" || metric.Type == "InetAddressMissingSize" {
517-
// Get previous oid.
518-
oids := strings.Split(metric.Oid, ".")
519-
i, _ := strconv.Atoi(oids[len(oids)-1])
520-
oids[len(oids)-1] = strconv.Itoa(i - 1)
521-
prevOid := strings.Join(oids, ".")
522-
if prevObj, ok := nameToNode[prevOid]; !ok || prevObj.TextualConvention != "InetAddressType" {
523-
metric.Type = "OctetString"
524-
} else {
525-
// Make sure the InetAddressType is included.
526-
if len(tableInstances[metric.Oid]) > 0 {
527-
for _, index := range tableInstances[metric.Oid] {
528-
needToWalk[prevOid+index+"."] = struct{}{}
529-
}
530-
} else {
531-
needToWalk[prevOid] = struct{}{}
516+
if metric.Type != "InetAddress" && metric.Type != "InetAddressMissingSize" {
517+
continue
518+
}
519+
// Get previous oid.
520+
oids := strings.Split(metric.Oid, ".")
521+
i, _ := strconv.Atoi(oids[len(oids)-1])
522+
oids[len(oids)-1] = strconv.Itoa(i - 1)
523+
prevOid := strings.Join(oids, ".")
524+
if prevObj, ok := nameToNode[prevOid]; !ok || prevObj.TextualConvention != "InetAddressType" {
525+
metric.Type = "OctetString"
526+
} else {
527+
// Make sure the InetAddressType is included.
528+
if len(tableInstances[metric.Oid]) > 0 {
529+
for _, index := range tableInstances[metric.Oid] {
530+
needToWalk[prevOid+index+"."] = struct{}{}
532531
}
532+
} else {
533+
needToWalk[prevOid] = struct{}{}
533534
}
534535
}
535536
}
536537

537538
// Apply module config overrides to their corresponding metrics.
538539
for name, params := range cfg.Overrides {
539540
for _, metric := range out.Metrics {
540-
if name == metric.Name || name == metric.Oid {
541-
metric.RegexpExtracts = params.RegexpExtracts
542-
metric.DateTimePattern = params.DateTimePattern
543-
metric.Offset = params.Offset
544-
metric.Scale = params.Scale
545-
if params.Help != "" {
546-
metric.Help = params.Help
547-
}
548-
if params.Name != "" {
549-
metric.Name = params.Name
550-
}
541+
if name != metric.Name && name != metric.Oid {
542+
continue
543+
}
544+
metric.RegexpExtracts = params.RegexpExtracts
545+
metric.DateTimePattern = params.DateTimePattern
546+
metric.Offset = params.Offset
547+
metric.Scale = params.Scale
548+
if params.Help != "" {
549+
metric.Help = params.Help
550+
}
551+
if params.Name != "" {
552+
metric.Name = params.Name
551553
}
552554
}
553555
}

‎generator/tree_test.go‎

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

180180
func TestGenerateConfigModule(t *testing.T) {
181181
var regexpFooBar config.Regexp
182-
regexpFooBar.Regexp, _ = regexp.Compile(".*")
182+
regexpFooBar.Regexp = regexp.MustCompile(".*")
183183

184184
strMetrics := make(map[string][]config.RegexpExtract)
185185
strMetrics["Status"] = []config.RegexpExtract{

‎main.go‎

Lines changed: 10 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -142,10 +142,10 @@ func handler(w http.ResponseWriter, r *http.Request, logger *slog.Logger, export
142142
}
143143
}
144144
}
145-
sc.RLock()
145+
sc.mu.RLock()
146146
auth, authOk := sc.C.Auths[authName]
147147
if !authOk {
148-
sc.RUnlock()
148+
sc.mu.RUnlock()
149149
http.Error(w, fmt.Sprintf("Unknown auth '%s'", authName), http.StatusBadRequest)
150150
snmpRequestErrors.Inc()
151151
return
@@ -154,14 +154,14 @@ func handler(w http.ResponseWriter, r *http.Request, logger *slog.Logger, export
154154
for _, m := range modules {
155155
module, moduleOk := sc.C.Modules[m]
156156
if !moduleOk {
157-
sc.RUnlock()
157+
sc.mu.RUnlock()
158158
http.Error(w, fmt.Sprintf("Unknown module '%s'", m), http.StatusBadRequest)
159159
snmpRequestErrors.Inc()
160160
return
161161
}
162162
nmodules = append(nmodules, collector.NewNamedModule(m, module))
163163
}
164-
sc.RUnlock()
164+
sc.mu.RUnlock()
165165
logger = logger.With("auth", authName, "target", target)
166166
registry := prometheus.NewRegistry()
167167
c := collector.New(r.Context(), target, authName, snmpContext, snmpEngineID, auth, nmodules, logger, exporterMetrics, *concurrency, debug)
@@ -185,22 +185,22 @@ func updateConfiguration(w http.ResponseWriter, r *http.Request) {
185185
}
186186

187187
type SafeConfig struct {
188-
sync.RWMutex
189-
C *config.Config
188+
mu sync.RWMutex
189+
C *config.Config
190190
}
191191

192192
func (sc *SafeConfig) ReloadConfig(logger *slog.Logger, configFile []string, expandEnvVars bool) (err error) {
193193
conf, err := config.LoadFile(logger, configFile, expandEnvVars)
194194
if err != nil {
195195
return err
196196
}
197-
sc.Lock()
197+
sc.mu.Lock()
198198
sc.C = conf
199199
// Initialize metrics.
200200
for module := range sc.C.Modules {
201201
snmpCollectionDuration.WithLabelValues(module)
202202
}
203-
sc.Unlock()
203+
sc.mu.Unlock()
204204
return nil
205205
}
206206

@@ -373,9 +373,9 @@ func main() {
373373
}
374374

375375
http.HandleFunc(configPath, func(w http.ResponseWriter, r *http.Request) {
376-
sc.RLock()
376+
sc.mu.RLock()
377377
c, err := yaml.Marshal(sc.C)
378-
sc.RUnlock()
378+
sc.mu.RUnlock()
379379
if err != nil {
380380
logger.Error("Error marshaling configuration", "err", err)
381381
http.Error(w, err.Error(), http.StatusInternalServerError)

0 commit comments

Comments
 (0)