Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

chore: lint issues #2093

Merged
merged 1 commit into from
Sep 19, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
140 changes: 70 additions & 70 deletions apps/agent/.golangci.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -213,79 +213,78 @@ linters-settings:
linters:
disable-all: true
enable:
## enabled by default
- errcheck # checking for unchecked errors, these unchecked errors can be critical bugs in some cases
- gosimple # specializes in simplifying a code
- govet # reports suspicious constructs, such as Printf calls whose arguments do not align with the format string
- ineffassign # detects when assignments to existing variables are not used
- staticcheck # is a go vet on steroids, applying a ton of static analysis checks
- typecheck # like the front-end of a Go compiler, parses and type-checks Go code
- unused # checks for unused constants, variables, functions and types
## disabled by default
- asasalint # checks for pass []any as any in variadic func(...any)
- asciicheck # checks that your code does not contain non-ASCII identifiers
- bidichk # checks for dangerous unicode character sequences
- bodyclose # checks whether HTTP response body is closed successfully
- canonicalheader # checks whether net/http.Header uses canonical header
- copyloopvar # detects places where loop variables are copied
- cyclop # checks function and package cyclomatic complexity
- dupl # tool for code clone detection
- durationcheck # checks for two durations multiplied together
- errname # checks that sentinel errors are prefixed with the Err and error types are suffixed with the Error
- errorlint # finds code that will cause problems with the error wrapping scheme introduced in Go 1.13
- exhaustive # checks exhaustiveness of enum switch statements
- exportloopref # checks for pointers to enclosing loop variables
- fatcontext # detects nested contexts in loops
# - forbidigo # forbids identifiers
- funlen # tool for detection of long functions
- gocheckcompilerdirectives # validates go compiler directive comments (//go:)
- gochecknoglobals # checks that no global variables exist
- gochecknoinits # checks that no init functions are present in Go code
- gochecksumtype # checks exhaustiveness on Go "sum types"
- gocognit # computes and checks the cognitive complexity of functions
- goconst # finds repeated strings that could be replaced by a constant
- gocritic # provides diagnostics that check for bugs, performance and style issues
- gocyclo # computes and checks the cyclomatic complexity of functions
- godot # checks if comments end in a period
- goimports # in addition to fixing imports, goimports also formats your code in the same style as gofmt
- gomoddirectives # manages the use of 'replace', 'retract', and 'excludes' directives in go.mod
- gomodguard # allow and block lists linter for direct Go module dependencies. This is different from depguard where there are different block types for example version constraints and module recommendations
- goprintffuncname # checks that printf-like functions are named with f at the end
- gosec # inspects source code for security problems
- intrange # finds places where for loops could make use of an integer range
- lll # reports long lines
- loggercheck # checks key value pairs for common logger libraries (kitlog,klog,logr,zap)
- makezero # finds slice declarations with non-zero initial length
- mirror # reports wrong mirror patterns of bytes/strings usage
- mnd # detects magic numbers
- musttag # enforces field tags in (un)marshaled structs
- nakedret # finds naked returns in functions greater than a specified function length
- nestif # reports deeply nested if statements
- nilerr # finds the code that returns nil even if it checks that the error is not nil
- nilnil # checks that there is no simultaneous return of nil error and an invalid value
- noctx # finds sending http request without context.Context
- nolintlint # reports ill-formed or insufficient nolint directives
- nosprintfhostport # checks for misuse of Sprintf to construct a host with port in a URL
- perfsprint # checks that fmt.Sprintf can be replaced with a faster alternative
- predeclared # finds code that shadows one of Go's predeclared identifiers
- promlinter # checks Prometheus metrics naming via promlint
- protogetter # reports direct reads from proto message fields when getters should be used
- reassign # checks that package variables are not reassigned
- revive # fast, configurable, extensible, flexible, and beautiful linter for Go, drop-in replacement of golint
- rowserrcheck # checks whether Err of rows is checked successfully
- sloglint # ensure consistent code style when using log/slog
- spancheck # checks for mistakes with OpenTelemetry/Census spans
- sqlclosecheck # checks that sql.Rows and sql.Stmt are closed
- stylecheck # is a replacement for golint
- tenv # detects using os.Setenv instead of t.Setenv since Go1.17
- testableexamples # checks if examples are testable (have an expected output)
- testifylint # checks usage of github.com/stretchr/testify
- tparallel # detects inappropriate usage of t.Parallel() method in your Go test codes
- unconvert # removes unnecessary type conversions
- unparam # reports unused function parameters
- usestdlibvars # detects the possibility to use variables/constants from the Go standard library
- wastedassign # finds wasted assignment statements
- whitespace # detects leading and trailing whitespace
# - ineffassign # detects when assignments to existing variables are not used
# - staticcheck # is a go vet on steroids, applying a ton of static analysis checks
# - typecheck # like the front-end of a Go compiler, parses and type-checks Go code
# - unused # checks for unused constants, variables, functions and types
# - bodyclose # checks whether HTTP response body is closed successfully
#
# - asasalint # checks for pass []any as any in variadic func(...any)
# - asciicheck # checks that your code does not contain non-ASCII identifiers
# - bidichk # checks for dangerous unicode character sequences
# - canonicalheader # checks whether net/http.Header uses canonical header
# - copyloopvar # detects places where loop variables are copied
# - cyclop # checks function and package cyclomatic complexity
# - dupl # tool for code clone detection
# - durationcheck # checks for two durations multiplied together
# - errname # checks that sentinel errors are prefixed with the Err and error types are suffixed with the Error
# - errorlint # finds code that will cause problems with the error wrapping scheme introduced in Go 1.13
# - exhaustive # checks exhaustiveness of enum switch statements
# - exportloopref # checks for pointers to enclosing loop variables
# - fatcontext # detects nested contexts in loops
# # - forbidigo # forbids identifiers
# - funlen # tool for detection of long functions
# - gocheckcompilerdirectives # validates go compiler directive comments (//go:)
# - gochecknoglobals # checks that no global variables exist
# # - gochecknoinits # checks that no init functions are present in Go code
# - gochecksumtype # checks exhaustiveness on Go "sum types"
# - gocognit # computes and checks the cognitive complexity of functions
# - goconst # finds repeated strings that could be replaced by a constant
# - gocritic # provides diagnostics that check for bugs, performance and style issues
# - gocyclo # computes and checks the cyclomatic complexity of functions
# - godot # checks if comments end in a period
# - goimports # in addition to fixing imports, goimports also formats your code in the same style as gofmt
# - gomoddirectives # manages the use of 'replace', 'retract', and 'excludes' directives in go.mod
# - gomodguard # allow and block lists linter for direct Go module dependencies. This is different from depguard where there are different block types for example version constraints and module recommendations
# - goprintffuncname # checks that printf-like functions are named with f at the end
# - gosec # inspects source code for security problems
# - intrange # finds places where for loops could make use of an integer range
# - lll # reports long lines
# - loggercheck # checks key value pairs for common logger libraries (kitlog,klog,logr,zap)
# - makezero # finds slice declarations with non-zero initial length
# - mirror # reports wrong mirror patterns of bytes/strings usage
# - mnd # detects magic numbers
# - musttag # enforces field tags in (un)marshaled structs
# - nakedret # finds naked returns in functions greater than a specified function length
# - nestif # reports deeply nested if statements
# - nilerr # finds the code that returns nil even if it checks that the error is not nil
# - nilnil # checks that there is no simultaneous return of nil error and an invalid value
# - noctx # finds sending http request without context.Context
# - nolintlint # reports ill-formed or insufficient nolint directives
# - nosprintfhostport # checks for misuse of Sprintf to construct a host with port in a URL
# - perfsprint # checks that fmt.Sprintf can be replaced with a faster alternative
# - predeclared # finds code that shadows one of Go's predeclared identifiers
# - promlinter # checks Prometheus metrics naming via promlint
# - protogetter # reports direct reads from proto message fields when getters should be used
# - reassign # checks that package variables are not reassigned
# - revive # fast, configurable, extensible, flexible, and beautiful linter for Go, drop-in replacement of golint
# - rowserrcheck # checks whether Err of rows is checked successfully
# - sloglint # ensure consistent code style when using log/slog
# - spancheck # checks for mistakes with OpenTelemetry/Census spans
# - sqlclosecheck # checks that sql.Rows and sql.Stmt are closed
# - stylecheck # is a replacement for golint
# - tenv # detects using os.Setenv instead of t.Setenv since Go1.17
# - testableexamples # checks if examples are testable (have an expected output)
# - testifylint # checks usage of github.com/stretchr/testify
# - tparallel # detects inappropriate usage of t.Parallel() method in your Go test codes
# - unconvert # removes unnecessary type conversions
# - unparam # reports unused function parameters
# - usestdlibvars # detects the possibility to use variables/constants from the Go standard library
# - wastedassign # finds wasted assignment statements
# - whitespace # detects leading and trailing whitespace

## you may want to enable
#- decorder # checks declaration order and count of types, constants, variables and functions
Expand Down Expand Up @@ -347,3 +346,4 @@ issues:
- gosec
- noctx
- wrapcheck
- errcheck
2 changes: 1 addition & 1 deletion apps/agent/Taskfile.yml
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ tasks:
fmt:
cmds:
- go fmt ./...
- go vet ./...
- task: lint
test:
cmds:
- go test -cover -json -failfast ./... | tparse -all -progress
Expand Down
36 changes: 17 additions & 19 deletions apps/agent/cmd/agent/agent.go
Original file line number Diff line number Diff line change
Expand Up @@ -87,7 +87,8 @@ func run(c *cli.Context) error {

{
if cfg.Tracing != nil && cfg.Tracing.Axiom != nil {
closeTracer, err := tracing.Init(context.Background(), tracing.Config{
var closeTracer tracing.Closer
closeTracer, err = tracing.Init(context.Background(), tracing.Config{
Dataset: cfg.Tracing.Axiom.Dataset,
Application: "agent",
Version: "1.0.0",
Expand All @@ -108,7 +109,7 @@ func run(c *cli.Context) error {

m := metrics.NewNoop()
if cfg.Metrics != nil && cfg.Metrics.Axiom != nil {
realMetrics, err := metrics.New(metrics.Config{
m, err = metrics.New(metrics.Config{
Token: cfg.Metrics.Axiom.Token,
Dataset: cfg.Metrics.Axiom.Dataset,
Logger: logger.With().Str("pkg", "metrics").Logger(),
Expand All @@ -118,7 +119,6 @@ func run(c *cli.Context) error {
if err != nil {
logger.Fatal().Err(err).Msg("unable to start metrics")
}
m = realMetrics

}
defer m.Close()
Expand Down Expand Up @@ -167,21 +167,21 @@ func run(c *cli.Context) error {

if cfg.Cluster != nil {

memb, err := membership.New(membership.Config{
memb, membershipErr := membership.New(membership.Config{
NodeId: cfg.NodeId,
RpcAddr: cfg.Cluster.RpcAddr,
SerfAddr: cfg.Cluster.SerfAddr,
Logger: logger,
})
if err != nil {
return fmt.Errorf("failed to create membership: %w", err)
if membershipErr != nil {
return fmt.Errorf("failed to create membership: %w", membershipErr)
}

var join []string
if cfg.Cluster.Join.Dns != nil {
addrs, err := net.LookupHost(cfg.Cluster.Join.Dns.AAAA)
if err != nil {
return fmt.Errorf("failed to lookup dns: %w", err)
addrs, lookupErr := net.LookupHost(cfg.Cluster.Join.Dns.AAAA)
if lookupErr != nil {
return fmt.Errorf("failed to lookup dns: %w", lookupErr)
}
logger.Info().Strs("addrs", addrs).Msg("found dns records")
join = addrs
Expand Down Expand Up @@ -214,9 +214,9 @@ func run(c *cli.Context) error {
return fmt.Errorf("failed to create cluster: %w", err)
}
defer func() {
err := clus.Shutdown()
if err != nil {
logger.Error().Err(err).Msg("failed to shutdown cluster")
shutdownErr := clus.Shutdown()
if shutdownErr != nil {
logger.Error().Err(shutdownErr).Msg("failed to shutdown cluster")
}
}()

Expand Down Expand Up @@ -245,7 +245,8 @@ func run(c *cli.Context) error {
}

if cfg.Services.EventRouter != nil {
er, err := eventrouter.New(eventrouter.Config{
var er *eventrouter.Service
er, err = eventrouter.New(eventrouter.Config{
Logger: logger,
Metrics: m,
BatchSize: cfg.Services.EventRouter.Tinybird.BatchSize,
Expand All @@ -259,10 +260,7 @@ func run(c *cli.Context) error {
return err
}
srv.WithEventRouter(er)
if err != nil {
return fmt.Errorf("failed to add event router service: %w", err)

}
}

connectSrv, err := connect.New(connect.Config{Logger: logger, Image: cfg.Image, Metrics: m})
Expand All @@ -282,23 +280,23 @@ func run(c *cli.Context) error {
logger.Info().Msg("started ratelimit service")

go func() {
err := connectSrv.Listen(fmt.Sprintf(":%s", cfg.RpcPort))
err = connectSrv.Listen(fmt.Sprintf(":%s", cfg.RpcPort))
if err != nil {
logger.Fatal().Err(err).Msg("failed to start connect service")
}
}()

go func() {
logger.Info().Msgf("listening on port %s", cfg.Port)
err := srv.Listen(fmt.Sprintf(":%s", cfg.Port))
err = srv.Listen(fmt.Sprintf(":%s", cfg.Port))
if err != nil {
logger.Fatal().Err(err).Msg("failed to start service")
}
}()

if cfg.Prometheus != nil {
go func() {
err := prometheus.Listen(cfg.Prometheus.Path, cfg.Prometheus.Port)
err = prometheus.Listen(cfg.Prometheus.Path, cfg.Prometheus.Port)
if err != nil {
logger.Fatal().Err(err).Msg("failed to start prometheus")
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -41,13 +41,13 @@ func TestUpdateAutomaticallyCreatedIdentityWithManyKeys(t *testing.T) {
require.NoError(t, err)

t.Cleanup(func() {
_, err := sdk.Apis.DeleteAPI(ctx, operations.DeleteAPIRequestBody{
_, err = sdk.Apis.DeleteAPI(ctx, operations.DeleteAPIRequestBody{
APIID: api.Object.APIID,
})
require.NoError(t, err)
})

externalId := uid.New("testuser")
externalID := uid.New("testuser")

keys := make([]*operations.CreateKeyResponseBody, 1000)
concurrency := make(chan struct{}, 32)
Expand All @@ -58,11 +58,11 @@ func TestUpdateAutomaticallyCreatedIdentityWithManyKeys(t *testing.T) {

concurrency <- struct{}{}

key, err := sdk.Keys.CreateKey(ctx, operations.CreateKeyRequestBody{
key, createErr := sdk.Keys.CreateKey(ctx, operations.CreateKeyRequestBody{
APIID: api.Object.APIID,
OwnerID: unkey.String(externalId),
OwnerID: unkey.String(externalID),
})
require.NoError(t, err)
require.NoError(t, createErr)

keys[i] = key.Object

Expand Down Expand Up @@ -95,7 +95,7 @@ func TestUpdateAutomaticallyCreatedIdentityWithManyKeys(t *testing.T) {
})

_, err = sdk.Identities.UpdateIdentity(ctx, operations.UpdateIdentityRequestBody{
ExternalID: unkey.String(externalId),
ExternalID: unkey.String(externalID),
Meta: map[string]any{
"hello": "world",
},
Expand All @@ -116,7 +116,7 @@ func TestUpdateAutomaticallyCreatedIdentityWithManyKeys(t *testing.T) {

require.True(t, verifyRes.V1KeysVerifyKeyResponse.Valid)
require.NotNil(t, verifyRes.V1KeysVerifyKeyResponse.Identity)
require.Equal(t, externalId, verifyRes.V1KeysVerifyKeyResponse.Identity.ExternalID)
require.Equal(t, externalID, verifyRes.V1KeysVerifyKeyResponse.Identity.ExternalID)

meta, err := json.Marshal(verifyRes.V1KeysVerifyKeyResponse.Identity.Meta)
require.NoError(t, err)
Expand Down
15 changes: 12 additions & 3 deletions apps/agent/pkg/api/agent_auth.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,21 +17,30 @@ func newBearerAuthMiddleware(secret string) routes.Middeware {
authorizationHeader := r.Header.Get("Authorization")
if authorizationHeader == "" {
w.WriteHeader(401)
w.Write([]byte("Authorization header is required"))
_, err := w.Write([]byte("Authorization header is required"))
if err != nil {
http.Error(w, err.Error(), http.StatusInternalServerError)
}
return
}

token := strings.TrimPrefix(authorizationHeader, "Bearer ")
if token == "" {
w.WriteHeader(401)
w.Write([]byte("Bearer token is required"))
_, err := w.Write([]byte("Bearer token is required"))
if err != nil {
http.Error(w, err.Error(), http.StatusInternalServerError)
}
return

}

if subtle.ConstantTimeCompare([]byte(token), secretB) != 1 {
w.WriteHeader(401)
w.Write([]byte("Bearer token is invalid"))
_, err := w.Write([]byte("Bearer token is invalid"))
if err != nil {
http.Error(w, err.Error(), http.StatusInternalServerError)
}
return
}

Expand Down
6 changes: 4 additions & 2 deletions apps/agent/pkg/api/ctxutil/context.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,12 +2,14 @@ package ctxutil

import "context"

type contextKey string

const (
request_id string = "request_id"
request_id contextKey = "request_id"
)

// getValue returns the value for the given key from the context or its zero value if it doesn't exist.
func getValue[T any](ctx context.Context, key string) T {
func getValue[T any](ctx context.Context, key contextKey) T {
val, ok := ctx.Value(key).(T)
if !ok {
var t T
Expand Down
6 changes: 4 additions & 2 deletions apps/agent/pkg/api/routes/openapi/handler.go
Original file line number Diff line number Diff line change
Expand Up @@ -14,8 +14,10 @@ func New(svc routes.Services) *routes.Route {

w.WriteHeader(200)
w.Header().Set("Content-Type", "application/json")
w.Write(openapi.Spec)

_, err := w.Write(openapi.Spec)
if err != nil {
http.Error(w, "failed to write response", http.StatusInternalServerError)
}
},
)
}
Loading
Loading