Skip to content

Commit b252dd8

Browse files
agelostsalagelostsal
committed
AM-390 Introduce staticcheck
1 parent 48f29ce commit b252dd8

53 files changed

Lines changed: 847 additions & 1530 deletions

Some content is hidden

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

‎.github/workflows/staticcheck.yml‎

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,31 @@
1+
name: Staticcheck
2+
3+
on:
4+
pull_request:
5+
push:
6+
branches: [ main ]
7+
8+
jobs:
9+
staticcheck:
10+
runs-on: ubuntu-latest
11+
12+
steps:
13+
- name: Checkout code
14+
uses: actions/checkout@v4
15+
with:
16+
fetch-depth: 1
17+
18+
- name: Set up Go
19+
uses: actions/setup-go@v5
20+
with:
21+
go-version-file: go.mod
22+
cache: true
23+
24+
- name: Download dependencies
25+
run: go mod download
26+
27+
- name: Run Staticcheck
28+
uses: dominikh/staticcheck-action@v1
29+
with:
30+
version: "latest"
31+
install-go: false

‎.staticcheck.conf‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
checks = ["all", "-ST1000"]

‎auth/acl.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -58,7 +58,7 @@ func AppendToACL(ctx context.Context, projectUUID string, resourceType string, r
5858
return store.AppendToACL(ctx, projectUUID, resourceType, resourceName, userUUIDs)
5959
}
6060

61-
// AppendToACL is used to remove users from a topic's or sub's acl
61+
// RemoveFromACL is used to remove users from a topic's or sub's acl
6262
func RemoveFromACL(ctx context.Context, projectUUID string, resourceType string, resourceName string, acl []string, store stores.Store) error {
6363

6464
// Transform user name to user uuid

‎auth/auth_test.go‎

Lines changed: 8 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -538,6 +538,7 @@ func (suite *AuthTestSuite) TestAuth() {
538538
qUsers1 = append(qUsers1, User{"uuid0", []ProjectRoles{{"ARGO", []string{"consumer", "publisher"}, []string{}, []string{}}}, "Test", "", "", "", "", "S3CR3T", "Test@test.com", []string{}, created, modified, ""})
539539
// return all users
540540
pu1, e1 := PaginatedFindUsers(suite.ctx, "", 0, "", true, true, store2)
541+
suite.NoError(e1)
541542

542543
var qUsers2 []User
543544
qUsers2 = append(qUsers2, User{"uuid8", []ProjectRoles{{"ARGO2", []string{"consumer", "publisher"}, []string{}, []string{}}}, "UserZ", "", "", "", "", "S3CR3T1", "foo-email", []string{}, created, modified, ""})
@@ -556,6 +557,7 @@ func (suite *AuthTestSuite) TestAuth() {
556557

557558
// return the first page with 2 users
558559
pu2, e2 := PaginatedFindUsers(suite.ctx, "", 3, "", true, true, store2)
560+
suite.NoError(e2)
559561

560562
var qUsers3 []User
561563
qUsers3 = append(qUsers3, User{"uuid4", []ProjectRoles{{"ARGO", []string{"publisher", "consumer"}, []string{"topic2"}, []string{"sub3", "sub4"}}}, "UserZ", "", "", "", "", "S3CR3T4", "foo-email", []string{}, created, modified, "UserA"})
@@ -571,10 +573,6 @@ func (suite *AuthTestSuite) TestAuth() {
571573
// invalid id
572574
_, e5 := PaginatedFindUsers(suite.ctx, "invalid", 0, "", true, true, store2)
573575

574-
// check user list by project
575-
var qUsersB []User
576-
qUsersB = append(qUsersB, User{"uuid8", []ProjectRoles{{"ARGO2", []string{"consumer", "publisher"}, []string{}, []string{}}}, "UserZ", "", "", "", "", "S3CR3T1", "foo-email", []string{}, created, modified, ""})
577-
578576
// check user list by project and with unprivileged mode (token redacted)
579577
var qUsersC []User
580578
qUsersC = append(qUsersC, User{"uuid8", []ProjectRoles{{"ARGO2", []string{"consumer", "publisher"}, []string{}, []string{}}}, "UserZ", "", "", "", "", "", "foo-email", []string{}, created, modified, ""})
@@ -793,13 +791,13 @@ func (suite *AuthTestSuite) TestModACL() {
793791
e1 := ModACL(suite.ctx, "argo_uuid", "topics", "topic1", []string{"UserX", "UserZ"}, store)
794792
suite.Nil(e1)
795793

796-
tACL1, _ := store.TopicsACL["topic1"]
794+
tACL1 := store.TopicsACL["topic1"]
797795
suite.Equal([]string{"uuid3", "uuid4"}, tACL1.ACL)
798796

799797
e2 := ModACL(suite.ctx, "argo_uuid", "subscriptions", "sub1", []string{"UserX", "UserZ"}, store)
800798
suite.Nil(e2)
801799

802-
sACL1, _ := store.SubsACL["sub1"]
800+
sACL1 := store.SubsACL["sub1"]
803801
suite.Equal([]string{"uuid3", "uuid4"}, sACL1.ACL)
804802

805803
e3 := ModACL(suite.ctx, "argo_uuid", "mistype", "sub1", []string{"UserX", "UserZ"}, store)
@@ -813,13 +811,13 @@ func (suite *AuthTestSuite) TestAppendToACL() {
813811
e1 := AppendToACL(suite.ctx, "argo_uuid", "topics", "topic1", []string{"UserX", "UserZ", "UserZ"}, store)
814812
suite.Nil(e1)
815813

816-
tACL1, _ := store.TopicsACL["topic1"]
814+
tACL1 := store.TopicsACL["topic1"]
817815
suite.Equal([]string{"uuid1", "uuid2", "uuid3", "uuid4"}, tACL1.ACL)
818816

819817
e2 := AppendToACL(suite.ctx, "argo_uuid", "subscriptions", "sub1", []string{"UserX", "UserZ", "UserZ"}, store)
820818
suite.Nil(e2)
821819

822-
sACL1, _ := store.SubsACL["sub1"]
820+
sACL1 := store.SubsACL["sub1"]
823821
suite.Equal([]string{"uuid1", "uuid2", "uuid3", "uuid4"}, sACL1.ACL)
824822

825823
e3 := AppendToACL(suite.ctx, "argo_uuid", "mistype", "sub1", []string{"UserX", "UserZ"}, store)
@@ -833,13 +831,13 @@ func (suite *AuthTestSuite) TestRemoveFromACL() {
833831
e1 := RemoveFromACL(suite.ctx, "argo_uuid", "topics", "topic1", []string{"UserA", "UserK"}, store)
834832
suite.Nil(e1)
835833

836-
tACL1, _ := store.TopicsACL["topic1"]
834+
tACL1 := store.TopicsACL["topic1"]
837835
suite.Equal([]string{"uuid2"}, tACL1.ACL)
838836

839837
e2 := RemoveFromACL(suite.ctx, "argo_uuid", "subscriptions", "sub1", []string{"UserA", "UserK"}, store)
840838
suite.Nil(e2)
841839

842-
sACL1, _ := store.SubsACL["sub1"]
840+
sACL1 := store.SubsACL["sub1"]
843841
suite.Equal([]string{"uuid2"}, sACL1.ACL)
844842

845843
e3 := RemoveFromACL(suite.ctx, "argo_uuid", "mistype", "sub1", []string{"UserX", "UserZ"}, store)

‎auth/users.go‎

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -109,7 +109,7 @@ func (us *Users) Empty() bool {
109109

110110
// One returns the first user if a user list is not empty
111111
func (us *Users) One() User {
112-
if us.Empty() == false {
112+
if !us.Empty() {
113113
return us.List[0]
114114
}
115115
return User{}
@@ -670,7 +670,7 @@ func UpdateUser(ctx context.Context, uuid, firstName, lastName, organization, de
670670
// Check roles
671671

672672
for _, roleItem := range item.Roles {
673-
if IsRoleValid(roleItem, validRoles) == false {
673+
if !IsRoleValid(roleItem, validRoles) {
674674
return User{}, errors.New("invalid role: " + roleItem)
675675
}
676676
}
@@ -681,9 +681,9 @@ func UpdateUser(ctx context.Context, uuid, firstName, lastName, organization, de
681681
prList = nil
682682
}
683683

684-
if serviceRoles != nil && len(serviceRoles) > 0 {
684+
if len(serviceRoles) > 0 {
685685
for _, roleItem := range serviceRoles {
686-
if IsRoleValid(roleItem, validRoles) == false {
686+
if !IsRoleValid(roleItem, validRoles) {
687687
return User{}, errors.New("invalid role: " + roleItem)
688688
}
689689
}
@@ -740,16 +740,16 @@ func CreateUser(ctx context.Context, uuid string, name string, fname string, lna
740740

741741
// Check roles
742742
for _, roleItem := range item.Roles {
743-
if IsRoleValid(roleItem, validRoles) == false {
743+
if !IsRoleValid(roleItem, validRoles) {
744744
return User{}, errors.New("invalid role: " + roleItem)
745745
}
746746
}
747747
prList = append(prList, stores.QProjectRoles{ProjectUUID: prUUID, Roles: item.Roles})
748748
}
749749

750-
if serviceRoles != nil && len(serviceRoles) > 0 {
750+
if len(serviceRoles) > 0 {
751751
for _, roleItem := range serviceRoles {
752-
if IsRoleValid(roleItem, validRoles) == false {
752+
if !IsRoleValid(roleItem, validRoles) {
753753
return User{}, errors.New("invalid role: " + roleItem)
754754
}
755755
}

‎brokers/broker.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -21,4 +21,4 @@ type Broker interface {
2121
TimeToOffset(ctx context.Context, topic string, time time.Time) (int64, error)
2222
}
2323

24-
var ErrOffsetOff = errors.New("Offset is off")
24+
var ErrOffsetOff = errors.New("offset is off")

‎brokers/kafka.go‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -33,11 +33,11 @@ type KafkaBroker struct {
3333
func (b *KafkaBroker) lockForTopic(topic string) {
3434
// Check if lock for topic exists
3535
_, present := b.consumeLock[topic]
36-
if present == false {
36+
if !present {
3737
// TopicLock is not in list so add it
3838
b.createTopicLock.Lock()
3939
_, nowPresent := b.consumeLock[topic]
40-
if nowPresent == false {
40+
if !nowPresent {
4141
b.consumeLock[topic] = &topicLock{}
4242
b.consumeLock[topic].Lock()
4343
}
@@ -50,7 +50,7 @@ func (b *KafkaBroker) lockForTopic(topic string) {
5050
func (b *KafkaBroker) unlockForTopic(topic string) {
5151
// Check if lock for topic exists
5252
_, present := b.consumeLock[topic]
53-
if present == false {
53+
if !present {
5454
return
5555
}
5656

‎brokers/mock.go‎

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -6,9 +6,10 @@ import (
66

77
"errors"
88
"fmt"
9-
"github.com/ARGOeu/argo-messaging/messages"
109
"strings"
1110
"time"
11+
12+
"github.com/ARGOeu/argo-messaging/messages"
1213
)
1314

1415
// MockBroker struct
@@ -103,12 +104,12 @@ func (b *MockBroker) Publish(ctx context.Context, topic string, msg messages.Mes
103104
return msgID, fmt.Sprintf("%s.%s", s[0], s[1]), 0, int64(len(b.MsgList)), nil
104105
}
105106

106-
// GetOffset returns a current topic's offset
107+
// GetMaxOffset returns a current topic's offset
107108
func (b *MockBroker) GetMaxOffset(ctx context.Context, topic string) int64 {
108109
return int64(len(b.MsgList) + 1)
109110
}
110111

111-
// GetOffset returns a current topic's offset
112+
// GetMinOffset returns a current topic's offset
112113
func (b *MockBroker) GetMinOffset(ctx context.Context, topic string) int64 {
113114
return int64(len(b.MsgList))
114115
}
@@ -118,7 +119,7 @@ func (b *MockBroker) Consume(ctx context.Context, topic string, offset int64, im
118119
return b.MsgList, nil
119120
}
120121

121-
// Delete topic from the broker
122+
// DeleteTopic remove the topic from the broker
122123
func (b *MockBroker) DeleteTopic(ctx context.Context, topic string) error {
123124

124125
_, ok := b.Topics[topic]

‎config/config.go‎

Lines changed: 23 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -11,14 +11,15 @@ import (
1111
log "github.com/sirupsen/logrus"
1212

1313
"crypto/x509"
14-
"github.com/samuel/go-zookeeper/zk"
15-
lSyslog "github.com/sirupsen/logrus/hooks/syslog"
16-
"github.com/spf13/pflag"
17-
"github.com/spf13/viper"
1814
"log/syslog"
1915
"os"
2016
"path/filepath"
2117
"strings"
18+
19+
"github.com/samuel/go-zookeeper/zk"
20+
lSyslog "github.com/sirupsen/logrus/hooks/syslog"
21+
"github.com/spf13/pflag"
22+
"github.com/spf13/viper"
2223
)
2324

2425
// AuthOption defines how the service will handle authentication/authorization
@@ -28,7 +29,7 @@ type AuthOption int
2829
const (
2930
// the api key can reside in the url parameter 'key'
3031
// maps to config value 'key'
31-
UrlKey = iota + 1
32+
URLKey = iota + 1
3233
// the api key can reside in the header 'x-api-key'
3334
// maps to config value 'header'
3435
HeaderKey
@@ -62,8 +63,8 @@ type APICfg struct {
6263
ProxyHostname string
6364

6465
PushEnabled bool
65-
// Whether or not it should communicate over tls with the push server
66-
PushTlsEnabled bool
66+
// Whether it should communicate over tls with the push server
67+
PushTLSEnabled bool
6768
// Push server endpoint
6869
PushServerHost string
6970
// Push server port
@@ -147,6 +148,10 @@ func (cfg *APICfg) GetZooList() ([]string, error) {
147148
).Info("Trying to connect to Zookeeper")
148149

149150
zConn, _, err := zk.Connect(cfg.ZooHosts, time.Second)
151+
if err != nil {
152+
return peerList, err
153+
}
154+
150155
// Check if indeed connected and can read
151156
_, _, _, err = zConn.ChildrenW("/")
152157
if err != nil {
@@ -225,7 +230,7 @@ func (cfg *APICfg) LoadCAs() (roots *x509.CertPool) {
225230
}
226231

227232
if ok = roots.AppendCertsFromPEM(bytes); !ok {
228-
return fmt.Errorf("Could not append cert to CA: %v ", filepath.Join(cfg.CertificateAuthoritiesDir, info.Name()))
233+
return fmt.Errorf("could not append cert to CA: %v ", filepath.Join(cfg.CertificateAuthoritiesDir, info.Name()))
229234
}
230235
}
231236

@@ -249,19 +254,14 @@ func setLogLevel(logLvl string) {
249254
switch logLvl {
250255
case "DEBUG":
251256
log.SetLevel(log.DebugLevel)
252-
break
253257
case "INFO":
254258
log.SetLevel(log.InfoLevel)
255-
break
256259
case "WARNING":
257260
log.SetLevel(log.WarnLevel)
258-
break
259261
case "ERROR":
260262
log.SetLevel(log.ErrorLevel)
261-
break
262263
case "FATAL":
263264
log.SetLevel(log.FatalLevel)
264-
break
265265
default:
266266
log.SetLevel(log.InfoLevel)
267267
}
@@ -304,12 +304,10 @@ func (cfg *APICfg) setAuthOption(authOpt string) {
304304
switch strings.ToLower(authOpt) {
305305
case "both":
306306
cfg.authOption = URLKeyAndHeaderKey
307-
break
308307
case "header":
309308
cfg.authOption = HeaderKey
310-
break
311309
default:
312-
cfg.authOption = UrlKey
310+
cfg.authOption = URLKey
313311
}
314312
}
315313

@@ -328,7 +326,7 @@ func (cfg *APICfg) LoadTest() {
328326
// Find and read the configuration file
329327
err := viper.ReadInConfig()
330328
if err != nil {
331-
panic(fmt.Errorf("Errod trying to read the configuration file: %s \n", err))
329+
panic(fmt.Errorf("error trying to read the configuration file: %s", err))
332330
}
333331

334332
// Load Kafka configuration
@@ -463,12 +461,12 @@ func (cfg *APICfg) LoadTest() {
463461
).Infof("Parameter Loaded - push_enabled: %v", cfg.PushEnabled)
464462

465463
// push TLS enabled true or false
466-
cfg.PushTlsEnabled = viper.GetBool("push_tls_enabled")
464+
cfg.PushTLSEnabled = viper.GetBool("push_tls_enabled")
467465
log.WithFields(
468466
log.Fields{
469467
"type": "service_log",
470468
},
471-
).Infof("Parameter Loaded - push_tls_enabled: %v", cfg.PushTlsEnabled)
469+
).Infof("Parameter Loaded - push_tls_enabled: %v", cfg.PushTLSEnabled)
472470

473471
// push server host
474472
cfg.PushServerHost = viper.GetString("push_server_host")
@@ -508,7 +506,7 @@ func (cfg *APICfg) Load() {
508506
// Set Flags
509507
var configPath *string
510508

511-
if pflag.Parsed() == false {
509+
if !pflag.Parsed() {
512510

513511
pflag.String("log-level", "INFO", "set the desired log level")
514512
viper.BindPFlag("log_level", pflag.Lookup("log-level"))
@@ -589,7 +587,7 @@ func (cfg *APICfg) Load() {
589587
// Find and read the configuration file
590588
err := viper.ReadInConfig()
591589
if err != nil {
592-
panic(fmt.Errorf("Errod trying to read the configuration file: %s \n", err))
590+
panic(fmt.Errorf("error trying to read the configuration file: %s", err))
593591
}
594592

595593
// First check log level parameter and set logger
@@ -723,12 +721,12 @@ func (cfg *APICfg) Load() {
723721
).Infof("Parameter Loaded - push_enabled: %v", cfg.PushEnabled)
724722

725723
// push TLS enabled true or false
726-
cfg.PushTlsEnabled = viper.GetBool("push_tls_enabled")
724+
cfg.PushTLSEnabled = viper.GetBool("push_tls_enabled")
727725
log.WithFields(
728726
log.Fields{
729727
"type": "service_log",
730728
},
731-
).Infof("Parameter Loaded - push_tls_enabled: %v", cfg.PushTlsEnabled)
729+
).Infof("Parameter Loaded - push_tls_enabled: %v", cfg.PushTLSEnabled)
732730

733731
// push server host
734732
cfg.PushServerHost = viper.GetString("push_server_host")
@@ -874,12 +872,12 @@ func (cfg *APICfg) LoadStrJSON(input string) {
874872
).Infof("Parameter Loaded - push_enabled: %v", cfg.PushEnabled)
875873

876874
// push TLS enabled true or false
877-
cfg.PushTlsEnabled = viper.GetBool("push_tls_enabled")
875+
cfg.PushTLSEnabled = viper.GetBool("push_tls_enabled")
878876
log.WithFields(
879877
log.Fields{
880878
"type": "service_log",
881879
},
882-
).Infof("Parameter Loaded - push_tls_enabled: %v", cfg.PushTlsEnabled)
880+
).Infof("Parameter Loaded - push_tls_enabled: %v", cfg.PushTLSEnabled)
883881

884882
// push server host
885883
cfg.PushServerHost = viper.GetString("push_server_host")

0 commit comments

Comments
 (0)