From 8882274093f7d52aec39d15cb872ff70f70b73f3 Mon Sep 17 00:00:00 2001 From: Kevin Franklin Kim Date: Tue, 16 Apr 2024 09:38:28 +0200 Subject: [PATCH] feat: update & fix lints --- .golangci.yml | 247 ++++++++++++++---- config/remote.go | 2 +- config/watch.go | 4 +- errors/wrappederror_test.go | 7 +- go.mod | 2 - jwt/jwtkey.go | 6 +- log/with.go | 2 +- net/gotsrpc/errors.go | 2 +- net/http/middleware/jwt.go | 8 +- net/http/roundtripware/circuitbreaker.go | 2 +- net/http/roundtripware/circuitbreaker_test.go | 30 +-- persistence/mongo/collection.go | 4 +- persistence/mongo/utils.go | 4 +- server.go | 10 +- server_test.go | 13 +- service/httpreadme_test.go | 2 +- serviceenabler.go | 12 +- telemetry/meter.go | 3 +- 18 files changed, 248 insertions(+), 112 deletions(-) diff --git a/.golangci.yml b/.golangci.yml index a2052d7..c149ab6 100644 --- a/.golangci.yml +++ b/.golangci.yml @@ -1,90 +1,229 @@ run: timeout: 5m - skip-dirs: - - tmp + +issues: + exclude-dirs: + - 'bin' + - 'tmp' + - 'vendor' + exclude-rules: + - path: _test\.go + linters: + - forcetypeassert + - gocheckcompilerdirectives linters-settings: - # https://golangci-lint.run/usage/linters/#revive - revive: - rules: - - name: indent-error-flow - disabled: true - - name: exported - disabled: true + # https://golangci-lint.run/usage/linters/#misspell + misspell: + mode: restricted + # https://golangci-lint.run/usage/linters/#asasalint + asasalint: + ignore-test: true + # https://golangci-lint.run/usage/linters/#exhaustive + exhaustive: + default-signifies-exhaustive: true # https://golangci-lint.run/usage/linters/#gocritic gocritic: - enabled-tags: - - diagnostic - - performance - - style - disabled-tags: - - experimental - - opinionated disabled-checks: + - assignOp - ifElseChain - settings: - hugeParam: - sizeThreshold: 512 + # https://golangci-lint.run/usage/linters/#testifylint + testifylint: + disable: + - float-compare # https://golangci-lint.run/usage/linters/#gosec gosec: - config: - G306: "0700" - excludes: - - G101 # Potential hardcoded credentials - - G102 # Bind to all interfaces - - G112 # Potential slowloris attack - - G401 # Detect the usage of DES, RC4, MD5 or SHA1 - - G402 # Look for bad TLS connection settings - - G404 # Insecure random number source (rand) - - G501 # Import blocklist: crypto/md5 - - G505 # Import blocklist: crypto/sha1 + confidence: medium + # https://golangci-lint.run/usage/linters/#revive + revive: + enable-all-rules: true + ignore-generated-header: true + rules: + - name: line-length-limit + disabled: true + #- name: if-return + # disabled: true + #- name: bare-return + # disabled: true + - name: cognitive-complexity + disabled: true + - name: unused-parameter + disabled: true + - name: add-constant + disabled: true + - name: indent-error-flow + disabled: true + - name: cyclomatic + disabled: true + - name: function-length + disabled: true + - name: early-return + disabled: true + #- name: nested-structs + # disabled: true + #- name: var-naming + # disabled: true + - name: use-any + disabled: true + - name: max-public-structs + disabled: true + #- name: function-result-limit + # disabled: true + - name: flag-parameter + disabled: true + - name: unused-receiver + disabled: true + #- name: argument-limit + # disabled: true + #- name: empty-lines + # disabled: true + - name: confusing-naming + disabled: true + #- name: import-alias-naming + # disabled: true + - name: empty-block + disabled: true + #- name: import-shadowing + # disabled: true + - name: unhandled-error + arguments: + - "fmt.Printf" + - "fmt.Println" + #- name: max-control-nesting + # disabled: true + - name: exported + disabled: true + #- name: unchecked-type-assertion + # disabled: true + #- name: unnecessary-stmt + # disabled: true + #- name: get-return + # disabled: true + #- name: context-keys-type + # disabled: true + #- name: comment-spacings + # disabled: true + #- name: struct-tag + # disabled: true + #- name: confusing-results + # disabled: true + - name: superfluous-else + disabled: true + - name: unexported-return + disabled: true + #- name: error-return + # disabled: true + #- name: redefines-builtin-id + # disabled: true linters: + disable-all: true enable: # Enabled by default linters: - - errcheck - - gosimple - - govet - - ineffassign - - staticcheck + - errcheck # errcheck is a program for checking for unchecked errors in Go code. These unchecked errors can be critical bugs in some cases [fast: false, auto-fix: false] + - gosimple # (megacheck) Linter for Go source code that specializes in simplifying code [fast: false, auto-fix: false] + - govet # (vet, vetshadow) Vet examines Go source code and reports suspicious constructs. It is roughly the same as 'go vet' and uses its passes. [fast: false, auto-fix: false] + - ineffassign # Detects when assignments to existing variables are not used [fast: true, auto-fix: false] + - staticcheck # (megacheck) It's a set of rules from staticcheck. It's not the same thing as the staticcheck binary. The author of staticcheck doesn't support or approve the use of staticcheck as a library inside golangci-lint. [fast: false, auto-fix: false] + - unused # (megacheck) Checks Go code for unused constants, variables, functions and types [fast: false, auto-fix: false] - # Disabled by default linters: - - asciicheck # Simple linter to check that your code does not contain non-ASCII identifiers [fast: true, auto-fix: false] + # Disabled by your configuration linters: + - asasalint # check for pass []any as any in variadic func(...any) [fast: false, auto-fix: false] + - asciicheck # checks that all code identifiers does not have non-ASCII symbols in the name [fast: true, auto-fix: false] - bidichk # Checks for dangerous unicode character sequences [fast: true, auto-fix: false] + - bodyclose # checks whether HTTP response body is closed successfully [fast: false, auto-fix: false] + #- containedctx # containedctx is a linter that detects struct contained context.Context field [fast: false, auto-fix: false] + - contextcheck # check whether the function uses a non-inherited context [fast: false, auto-fix: false] + #- copyloopvar # (go >= 1.22) copyloopvar is a linter detects places where loop variables are copied [fast: true, auto-fix: false] + #- cyclop # checks function and package cyclomatic complexity [fast: false, auto-fix: false] + - decorder # check declaration order and count of types, constants, variables and functions [fast: true, auto-fix: false] + #- depguard # Go linter that checks if package imports are in a list of acceptable packages [fast: true, auto-fix: false] + #- dogsled # Checks assignments with too many blank identifiers (e.g. x, _, _, _, := f()) [fast: true, auto-fix: false] #- dupl # Tool for code clone detection [fast: true, auto-fix: false] + #- dupword # checks for duplicate words in the source code [fast: true, auto-fix: false] + - durationcheck # check for two durations multiplied together [fast: false, auto-fix: false] + - errchkjson # Checks types passed to the json encoding functions. Reports unsupported types and reports occations, where the check for the returned error can be omitted. [fast: false, auto-fix: false] + - errname # Checks that sentinel errors are prefixed with the `Err` and error types are suffixed with the `Error`. [fast: false, auto-fix: false] + - errorlint # errorlint is a linter for that can be used to find code that will cause problems with the error wrapping scheme introduced in Go 1.13. [fast: false, auto-fix: false] + - execinquery # execinquery is a linter about query string checker in Query function which reads your Go src files and warning it finds [fast: false, auto-fix: false] + - exhaustive # check exhaustiveness of enum switch statements [fast: false, auto-fix: false] + #- exhaustruct # Checks if all structure fields are initialized [fast: false, auto-fix: false] + - exportloopref # checks for pointers to enclosing loop variables [fast: false, auto-fix: false] + #- forbidigo # Forbids identifiers [fast: false, auto-fix: false] - forcetypeassert # finds forced type assertions [fast: true, auto-fix: false] + #- funlen # Tool for detection of long functions [fast: true, auto-fix: false] + #- gci # Gci controls Go package import order and makes it always deterministic. [fast: true, auto-fix: true] + #- ginkgolinter # enforces standards of using ginkgo and gomega [fast: false, auto-fix: false] + - gocheckcompilerdirectives # Checks that go compiler directive comments (//go:) are valid. [fast: true, auto-fix: false] + #- gochecknoglobals # Check that no global variables exist. [fast: false, auto-fix: false] #- gochecknoinits # Checks that no init functions are present in Go code [fast: true, auto-fix: false] - - goconst # Finds repeated strings that could be replaced by a constant [fast: true, auto-fix: false] - - gocritic # Provides diagnostics that check for bugs, performance and style issues. [fast: false, auto-fix: false] - - goimports # In addition to fixing imports, goimports also formats your code in the same style as gofmt. [fast: true, auto-fix: true] - #- gomoddirectives # Manage the use of 'replace', 'retract', and 'excludes' directives in go.mod. [fast: true, auto-fix: false] + - gochecksumtype # Run exhaustiveness checks on Go "sum types" [fast: false, auto-fix: false] + #- gocognit # Computes and checks the cognitive complexity of functions [fast: true, auto-fix: false] + #- goconst # Finds repeated strings that could be replaced by a constant [fast: true, auto-fix: false] + - gocritic # Provides diagnostics that check for bugs, performance and style issues. [fast: false, auto-fix: true] + #- gocyclo # Computes and checks the cyclomatic complexity of functions [fast: true, auto-fix: false] + #- godot # Check if comments end in a period [fast: true, auto-fix: true] + #- godox # Tool for detection of FIXME, TODO and other comment keywords [fast: true, auto-fix: false] + #- goerr113 # Go linter to check the errors handling expressions [fast: false, auto-fix: false] + - gofmt # Gofmt checks whether code was gofmt-ed. By default this tool runs with -s option to check for code simplification [fast: true, auto-fix: true] + #- gofumpt # Gofumpt checks whether code was gofumpt-ed. [fast: true, auto-fix: true] + - goheader # Checks is file header matches to pattern [fast: true, auto-fix: true] + - goimports # Check import statements are formatted according to the 'goimport' command. Reformat imports in autofix mode. [fast: true, auto-fix: true] + #- gomnd # An analyzer to detect magic numbers. [fast: true, auto-fix: false] + - gomoddirectives # Manage the use of 'replace', 'retract', and 'excludes' directives in go.mod. [fast: true, auto-fix: false] - gomodguard # Allow and block list linter for direct Go module dependencies. This is different from depguard where there are different block types for example version constraints and module recommendations. [fast: true, auto-fix: false] - - goprintffuncname # Checks that printf-like functions are named with `f` at the end [fast: true, auto-fix: false] - - gosec # (gas): Inspects source code for security problems [fast: false, auto-fix: false] - - grouper # An analyzer to analyze expression groups. [fast: true, auto-fix: false] + - goprintffuncname # Checks that printf-like functions are named with `f` at the end. [fast: true, auto-fix: false] + - gosec # (gas) Inspects source code for security problems [fast: false, auto-fix: false] + #- gosmopolitan # Report certain i18n/l10n anti-patterns in your Go codebase [fast: false, auto-fix: false] + - grouper # Analyze expression groups. [fast: true, auto-fix: false] - importas # Enforces consistent import aliases [fast: false, auto-fix: false] + #- inamedparam # reports interfaces with unnamed method parameters [fast: true, auto-fix: false] + #- interfacebloat # A linter that checks the number of methods inside an interface. [fast: true, auto-fix: false] + #- intrange # (go >= 1.22) intrange is a linter to find places where for loops could make use of an integer range. [fast: true, auto-fix: false] + #- ireturn # Accept Interfaces, Return Concrete Types [fast: false, auto-fix: false] + #- lll # Reports long lines [fast: true, auto-fix: false] + #- loggercheck # (logrlint) Checks key value pairs for common logger libraries (kitlog,klog,logr,zap). [fast: false, auto-fix: false] #- maintidx # maintidx measures the maintainability index of each function. [fast: true, auto-fix: false] - makezero # Finds slice declarations with non-zero initial length [fast: false, auto-fix: false] - - misspell # Finds commonly misspelled English words in comments [fast: true, auto-fix: true] - - nakedret # Finds naked returns in functions greater than a specified function length [fast: true, auto-fix: false] + - misspell # Finds commonly misspelled English words [fast: true, auto-fix: true] + - mirror # reports wrong mirror patterns of bytes/strings usage [fast: false, auto-fix: true] + - musttag # enforce field tags in (un)marshaled structs [fast: false, auto-fix: false] + #- nakedret # Checks that functions with naked returns are not longer than a maximum size (can be zero). [fast: true, auto-fix: false] #- nestif # Reports deeply nested if statements [fast: true, auto-fix: false] - nilerr # Finds the code that returns nil even if it checks that the error is not nil. [fast: false, auto-fix: false] - nilnil # Checks that there is no simultaneous return of `nil` error and an invalid value. [fast: false, auto-fix: false] - - noctx # noctx finds sending http request without context.Context [fast: false, auto-fix: false] - - nolintlint # Reports ill-formed or insufficient nolint directives [fast: true, auto-fix: false] + #- nlreturn # nlreturn checks for a new line before return and branch statements to increase code clarity [fast: true, auto-fix: false] + - noctx # Finds sending http request without context.Context [fast: false, auto-fix: false] + - nolintlint # Reports ill-formed or insufficient nolint directives [fast: true, auto-fix: true] - nonamedreturns # Reports all named returns [fast: false, auto-fix: false] - nosprintfhostport # Checks for misuse of Sprintf to construct a host with port in a URL. [fast: true, auto-fix: false] + #- paralleltest # Detects missing usage of t.Parallel() method in your Go test [fast: false, auto-fix: false] + #- perfsprint # Checks that fmt.Sprintf can be replaced with a faster alternative. [fast: false, auto-fix: false] - prealloc # Finds slice declarations that could potentially be pre-allocated [fast: true, auto-fix: false] - predeclared # find code that shadows one of Go's predeclared identifiers [fast: true, auto-fix: false] - promlinter # Check Prometheus metrics naming via promlint [fast: true, auto-fix: false] + #- protogetter # Reports direct reads from proto message fields when getters should be used [fast: false, auto-fix: true] + - reassign # Checks that package variables are not reassigned [fast: false, auto-fix: false] - revive # Fast, configurable, extensible, flexible, and beautiful linter for Go. Drop-in replacement of golint. [fast: false, auto-fix: false] + #- rowserrcheck # checks whether Rows.Err of rows is checked successfully [fast: false, auto-fix: false] + #- sloglint # ensure consistent code style when using log/slog [fast: false, auto-fix: false] + #- spancheck # Checks for mistakes with OpenTelemetry/Census spans. [fast: false, auto-fix: false] + #- sqlclosecheck # Checks that sql.Rows, sql.Stmt, sqlx.NamedStmt, pgx.Query are closed. [fast: false, auto-fix: false] + - stylecheck # Stylecheck is a replacement for golint [fast: false, auto-fix: false] + #- tagalign # check that struct tags are well aligned [fast: true, auto-fix: true] - tagliatelle # Checks the struct tags. [fast: true, auto-fix: false] + - tenv # tenv is analyzer that detects using os.Setenv instead of t.Setenv since Go1.17 [fast: false, auto-fix: false] + - testableexamples # linter checks if examples are testable (have an expected output) [fast: true, auto-fix: false] + - testifylint # Checks usage of github.com/stretchr/testify. [fast: false, auto-fix: false] - testpackage # linter that makes you use a separate _test package [fast: true, auto-fix: false] - - thelper # thelper detects golang test helpers without t.Helper() call and checks the consistency of test helpers [fast: false, auto-fix: false] + - thelper # thelper detects tests helpers which is not start with t.Helper() method. [fast: false, auto-fix: false] + #- tparallel # tparallel detects inappropriate usage of t.Parallel() method in your Go test codes. [fast: false, auto-fix: false] - unconvert # Remove unnecessary type conversions [fast: false, auto-fix: false] - - unparam # Reports unused function parameters [fast: false, auto-fix: false] + #- unparam # Reports unused function parameters [fast: false, auto-fix: false] - usestdlibvars # A linter that detect the possibility to use variables/constants from the Go standard library. [fast: true, auto-fix: false] - - wastedassign # wastedassign finds wasted assignment statements. [fast: false, auto-fix: false] - - whitespace # Tool for detection of leading and trailing whitespace [fast: true, auto-fix: true] - disable: - - unused + #- varnamelen # checks that the length of a variable's name matches its scope [fast: false, auto-fix: false] + - wastedassign # Finds wasted assignment statements [fast: false, auto-fix: false] + - whitespace # Whitespace is a linter that checks for unnecessary newlines at the start and end of functions, if, for, etc. [fast: true, auto-fix: true] + #- wrapcheck # Checks that errors returned from external packages are wrapped [fast: false, auto-fix: false] + #- wsl # add or remove empty lines [fast: true, auto-fix: false] + #- zerologlint # Detects the wrong usage of `zerolog` that a user forgets to dispatch with `Send` or `Msg` [fast: false, auto-fix: false] diff --git a/config/remote.go b/config/remote.go index ab85c67..ae54652 100644 --- a/config/remote.go +++ b/config/remote.go @@ -3,7 +3,7 @@ package config import ( "github.com/pkg/errors" "github.com/spf13/viper" - _ "github.com/spf13/viper/remote" + _ "github.com/spf13/viper/remote" // required import ) var remotes []struct { diff --git a/config/watch.go b/config/watch.go index 9713e2e..870b771 100644 --- a/config/watch.go +++ b/config/watch.go @@ -20,7 +20,7 @@ func WatchBool(ctx context.Context, fn func() bool, callback func(bool)) { func WatchTime(ctx context.Context, fn func() time.Time, callback func(time.Time)) { current := fn() watch(ctx, func() { - if value := fn(); value != current { + if value := fn(); !value.Equal(current) { current = value callback(current) } @@ -102,7 +102,7 @@ func WatchBoolChan(ctx context.Context, fn func() bool, ch chan bool) { func WatchTimeChan(ctx context.Context, fn func() time.Time, ch chan time.Time) { current := fn() watch(ctx, func() { - if value := fn(); value != current { + if value := fn(); !value.Equal(current) { current = value ch <- current } diff --git a/errors/wrappederror_test.go b/errors/wrappederror_test.go index 05f06b0..da12b14 100644 --- a/errors/wrappederror_test.go +++ b/errors/wrappederror_test.go @@ -7,6 +7,7 @@ import ( keelerrors "github.com/foomo/keel/errors" "github.com/pkg/errors" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" ) func ExampleNewWrappedError() { @@ -63,13 +64,13 @@ func Test_wrappedError_Error(t *testing.T) { parentErr := errors.New("parent") childErr := errors.New("child") wrappedErr := keelerrors.NewWrappedError(parentErr, childErr) - assert.Equal(t, wrappedErr.Error(), "parent: child") + assert.Equal(t, "parent: child", wrappedErr.Error()) } func Test_wrappedError_Is(t *testing.T) { parentErr := errors.New("parent") childErr := errors.New("child") wrappedErr := keelerrors.NewWrappedError(parentErr, childErr) - assert.ErrorIs(t, wrappedErr, parentErr) - assert.ErrorIs(t, wrappedErr, childErr) + require.ErrorIs(t, wrappedErr, parentErr) + require.ErrorIs(t, wrappedErr, childErr) } diff --git a/go.mod b/go.mod index 5e5a2a4..eb91f01 100644 --- a/go.mod +++ b/go.mod @@ -152,5 +152,3 @@ require ( gopkg.in/yaml.v2 v2.4.0 // indirect gopkg.in/yaml.v3 v3.0.1 // indirect ) - -replace go.opentelemetry.io/contrib/instrumentation/net/http/otelhttp v0.32.0 => github.com/foomo/opentelemetry-go-contrib/instrumentation/net/http/otelhttp v0.32.1-0.20220517120905-10e2553b9bac diff --git a/jwt/jwtkey.go b/jwt/jwtkey.go index b55d872..b7fc16a 100644 --- a/jwt/jwtkey.go +++ b/jwt/jwtkey.go @@ -38,9 +38,9 @@ func NewKeyFromFilenames(publicKeyPemFilename, privateKeyPemFilename string) (Ke // load private key if privateKeyPemFilename != "" { - if bytes, err := os.ReadFile(privateKeyPemFilename); err != nil { + if value, err := os.ReadFile(privateKeyPemFilename); err != nil { return Key{}, errors.Wrap(err, "failed to read private key: "+privateKeyPemFilename) - } else if key, err := jwt.ParseRSAPrivateKeyFromPEM([]byte(strings.ReplaceAll(string(bytes), `\n`, "\n"))); err != nil { + } else if key, err := jwt.ParseRSAPrivateKeyFromPEM([]byte(strings.ReplaceAll(string(value), `\n`, "\n"))); err != nil { return Key{}, errors.Wrap(err, "failed to parse private key: "+privateKeyPemFilename) } else { private = key @@ -54,7 +54,7 @@ func NewKeyFromFilenames(publicKeyPemFilename, privateKeyPemFilename string) (Ke return Key{}, errors.Wrap(err, "failed to parse public key: "+publicKeyPemFilename) } else { hasher := sha256.New() - hasher.Write(bytes.TrimSpace(v)) + _, _ = hasher.Write(bytes.TrimSpace(v)) id = hex.EncodeToString(hasher.Sum(nil)) public = key } diff --git a/log/with.go b/log/with.go index cf23441..e8d7640 100644 --- a/log/with.go +++ b/log/with.go @@ -40,7 +40,7 @@ func WithServiceName(l *zap.Logger, name string) *zap.Logger { return With(l, FServiceName(name)) } -func WithTraceID(l *zap.Logger, ctx context.Context) *zap.Logger { +func WithTraceID(l *zap.Logger, ctx context.Context) *zap.Logger { //nolint:revive if spanCtx := trace.SpanContextFromContext(ctx); spanCtx.IsValid() && spanCtx.IsSampled() { l = With(l, FTraceID(spanCtx.TraceID().String()), FSpanID(spanCtx.SpanID().String())) } diff --git a/net/gotsrpc/errors.go b/net/gotsrpc/errors.go index 6105d64..46219ce 100644 --- a/net/gotsrpc/errors.go +++ b/net/gotsrpc/errors.go @@ -19,7 +19,7 @@ func NewError(e Error) *Error { func (e *Error) Is(err error) bool { if e == nil || err == nil { return false - } else if v, ok := err.(*Error); ok && v != nil { //nolint:errorlint + } else if v, ok := err.(*Error); ok && v != nil { return e.Error() == v.Error() } return false diff --git a/net/http/middleware/jwt.go b/net/http/middleware/jwt.go index d165ecf..2971f70 100644 --- a/net/http/middleware/jwt.go +++ b/net/http/middleware/jwt.go @@ -125,18 +125,18 @@ func JWTWithSetContext(v bool) JWTOption { } // JWT middleware -func JWT(jwt *jwt.JWT, contextKey interface{}, opts ...JWTOption) Middleware { +func JWT(v *jwt.JWT, contextKey interface{}, opts ...JWTOption) Middleware { options := GetDefaultJWTOptions() for _, opt := range opts { if opt != nil { opt(&options) } } - return JWTWithOptions(jwt, contextKey, options) + return JWTWithOptions(v, contextKey, options) } // JWTWithOptions middleware -func JWTWithOptions(jwt *jwt.JWT, contextKey interface{}, opts JWTOptions) Middleware { +func JWTWithOptions(v *jwt.JWT, contextKey interface{}, opts JWTOptions) Middleware { return func(l *zap.Logger, name string, next http.Handler) http.Handler { return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { claims := opts.ClaimsProvider() @@ -174,7 +174,7 @@ func JWTWithOptions(jwt *jwt.JWT, contextKey interface{}, opts JWTOptions) Middl } // handle existing token - jwtToken, err := jwt.ParseWithClaims(token, claims) + jwtToken, err := v.ParseWithClaims(token, claims) if err != nil { if resume := opts.ErrorHandler(l, w, r, err); resume { next.ServeHTTP(w, r) diff --git a/net/http/roundtripware/circuitbreaker.go b/net/http/roundtripware/circuitbreaker.go index 824afd9..407bcfa 100644 --- a/net/http/roundtripware/circuitbreaker.go +++ b/net/http/roundtripware/circuitbreaker.go @@ -191,7 +191,7 @@ func CircuitBreaker(set *CircuitBreakerSettings, opts ...CircuitBreakerOption) R } // continue with the middleware chain - resp, err = next(r) //nolint:bodyclose + resp, err = next(r) var respCopy *http.Response if resp != nil { diff --git a/net/http/roundtripware/circuitbreaker_test.go b/net/http/roundtripware/circuitbreaker_test.go index 13b4a0f..79ce7ec 100644 --- a/net/http/roundtripware/circuitbreaker_test.go +++ b/net/http/roundtripware/circuitbreaker_test.go @@ -41,7 +41,7 @@ var cbSettings = &roundtripware.CircuitBreakerSettings{ return counts.ConsecutiveFailures > 3 }, OnStateChange: func(name string, from gobreaker.State, to gobreaker.State) { - fmt.Printf("\n\nstate changed from %s to %s\n\n", from, to) + _, _ = fmt.Printf("\n\nstate changed from %s to %s\n\n", from, to) }, } @@ -101,7 +101,7 @@ func TestCircuitBreaker(t *testing.T) { require.NoError(t, err) resp, err := client.Do(req) if resp != nil { - defer resp.Body.Close() + _ = resp.Body.Close() } require.NotErrorIs(t, err, roundtripware.ErrCircuitBreaker) } @@ -112,7 +112,7 @@ func TestCircuitBreaker(t *testing.T) { require.NoError(t, err) resp, err := client.Do(req) if err == nil { - defer resp.Body.Close() + _ = resp.Body.Close() } require.ErrorIs(t, err, roundtripware.ErrCircuitBreaker) @@ -123,7 +123,7 @@ func TestCircuitBreaker(t *testing.T) { require.NoError(t, err) resp, err = client.Do(req) if resp != nil { - defer resp.Body.Close() + _ = resp.Body.Close() } require.NoError(t, err) } @@ -161,7 +161,7 @@ func TestCircuitBreakerCopyBodies(t *testing.T) { require.NoError(t, errRead) // also try to close one of the bodies (should also be handled by the RoundTripware) - req.Body.Close() + _ = req.Body.Close() return err }, true, true, @@ -175,7 +175,7 @@ func TestCircuitBreakerCopyBodies(t *testing.T) { require.NoError(t, err) resp, err := client.Do(req) if resp != nil { - defer resp.Body.Close() + _ = resp.Body.Close() } require.NoError(t, err) // make sure the correct data is returned @@ -227,7 +227,7 @@ func TestCircuitBreakerReadFromNotCopiedBodies(t *testing.T) { require.NoError(t, err) resp, err := client.Do(req) if resp != nil { - defer resp.Body.Close() + _ = resp.Body.Close() } require.Error(t, err) require.ErrorIs(t, err, roundtripware.ErrReadFromActualBody) @@ -256,7 +256,7 @@ func TestCircuitBreakerReadFromNotCopiedBodies(t *testing.T) { require.NoError(t, err) resp, err = client.Do(req) if resp != nil { - defer resp.Body.Close() + _ = resp.Body.Close() } require.Error(t, err) require.ErrorIs(t, err, roundtripware.ErrReadFromActualBody) @@ -303,7 +303,7 @@ func TestCircuitBreakerInterval(t *testing.T) { require.NoError(t, err) resp, err := client.Do(req) if resp != nil { - defer resp.Body.Close() + _ = resp.Body.Close() } require.NotErrorIs(t, err, roundtripware.ErrCircuitBreaker) } @@ -318,7 +318,7 @@ func TestCircuitBreakerInterval(t *testing.T) { require.NoError(t, err) resp, err := client.Do(req) if resp != nil { - defer resp.Body.Close() + _ = resp.Body.Close() } require.NotErrorIs(t, err, roundtripware.ErrCircuitBreaker) } @@ -328,7 +328,7 @@ func TestCircuitBreakerInterval(t *testing.T) { require.NoError(t, err) resp, err := client.Do(req) if resp != nil { - defer resp.Body.Close() + _ = resp.Body.Close() } require.ErrorIs(t, err, roundtripware.ErrCircuitBreaker) } @@ -370,7 +370,7 @@ func TestCircuitBreakerIgnore(t *testing.T) { require.NoError(t, err) resp, err := client.Do(req) if resp != nil { - defer resp.Body.Close() + _ = resp.Body.Close() } require.NotErrorIs(t, err, roundtripware.ErrCircuitBreaker) require.NoError(t, err) @@ -399,16 +399,16 @@ func TestCircuitBreakerTimeout(t *testing.T) { // -> circuit breaker should change to open state for i := 0; i < 4; i++ { ctx, cancel := context.WithTimeout(context.Background(), 200*time.Millisecond) - defer cancel() req, err := http.NewRequestWithContext(ctx, http.MethodGet, svr.URL, nil) require.NoError(t, err) resp, err := client.Do(req) if resp != nil { - defer resp.Body.Close() + _ = resp.Body.Close() } require.NotErrorIs(t, err, roundtripware.ErrCircuitBreaker) require.ErrorIs(t, err, context.DeadlineExceeded) + cancel() } // send another request with a bigger timeout @@ -419,7 +419,7 @@ func TestCircuitBreakerTimeout(t *testing.T) { require.NoError(t, err) resp, err := client.Do(req) if resp != nil { - defer resp.Body.Close() + _ = resp.Body.Close() } require.ErrorIs(t, err, roundtripware.ErrCircuitBreaker) } diff --git a/persistence/mongo/collection.go b/persistence/mongo/collection.go index 89ae5f0..29425b7 100644 --- a/persistence/mongo/collection.go +++ b/persistence/mongo/collection.go @@ -394,7 +394,7 @@ func (c *Collection) FindIterate(ctx context.Context, filter interface{}, handle return err } - defer CloseCursor(cursor) + defer CloseCursor(context.WithoutCancel(ctx), cursor) for cursor.Next(ctx) { if err := handler(cursor.Decode); err != nil { @@ -424,7 +424,7 @@ func (c *Collection) AggregateIterate(ctx context.Context, pipeline mongo.Pipeli return err } - defer CloseCursor(cursor) + defer CloseCursor(context.WithoutCancel(ctx), cursor) for cursor.Next(ctx) { if err := handler(cursor.Decode); err != nil { diff --git a/persistence/mongo/utils.go b/persistence/mongo/utils.go index 9843ab5..7427447 100644 --- a/persistence/mongo/utils.go +++ b/persistence/mongo/utils.go @@ -8,8 +8,8 @@ import ( ) // CloseCursor with defer -func CloseCursor(cursor *mongo.Cursor) { - if err := cursor.Close(context.Background()); err != nil { +func CloseCursor(ctx context.Context, cursor *mongo.Cursor) { + if err := cursor.Close(ctx); err != nil { log.WithError(nil, err).Error("failed to close cursor") } } diff --git a/server.go b/server.go index 738d7fc..551ee0c 100644 --- a/server.go +++ b/server.go @@ -233,11 +233,11 @@ func (s *Server) ShutdownCancel() context.CancelFunc { } // AddService add a single service -func (s *Server) AddService(service Service) { - if !slices.Contains(s.services, service) { - s.services = append(s.services, service) - s.AddAlwaysHealthzers(service) - s.AddCloser(service) +func (s *Server) AddService(v Service) { + if !slices.Contains(s.services, v) { + s.services = append(s.services, v) + s.AddAlwaysHealthzers(v) + s.AddCloser(v) } } diff --git a/server_test.go b/server_test.go index d90ff9c..d03a2ca 100644 --- a/server_test.go +++ b/server_test.go @@ -11,7 +11,6 @@ import ( "time" "github.com/foomo/keel/service" - "github.com/stretchr/testify/assert" "github.com/stretchr/testify/suite" "go.uber.org/goleak" "go.uber.org/zap" @@ -102,7 +101,7 @@ func (s *KeelTestSuite) TestServiceHTTPZap() { s.Run("default", func() { if statusCode, body, err := s.httpGet("http://localhost:9100/log"); s.NoError(err) { s.Equal(http.StatusOK, statusCode) - s.Equal(body, `{"level":"info","disableCaller":true,"disableStacktrace":true}`) + s.Equal(`{"level":"info","disableCaller":true,"disableStacktrace":true}`, body) } if statusCode, _, err := s.httpGet("http://localhost:55000/log/info"); s.NoError(err) { s.Equal(http.StatusOK, statusCode) @@ -115,7 +114,7 @@ func (s *KeelTestSuite) TestServiceHTTPZap() { s.Run("set debug level", func() { if statusCode, body, err := s.httpPut("http://localhost:9100/log", `{"level":"debug"}`); s.NoError(err) { s.Equal(http.StatusOK, statusCode) - s.Equal(body, `{"level":"debug","disableCaller":true,"disableStacktrace":true}`) + s.Equal(`{"level":"debug","disableCaller":true,"disableStacktrace":true}`, body) } if statusCode, _, err := s.httpGet("http://localhost:55000/log/info"); s.NoError(err) { s.Equal(http.StatusOK, statusCode) @@ -128,7 +127,7 @@ func (s *KeelTestSuite) TestServiceHTTPZap() { s.Run("enable caller", func() { if statusCode, body, err := s.httpPut("http://localhost:9100/log", `{"disableCaller":false}`); s.NoError(err) { s.Equal(http.StatusOK, statusCode) - s.Equal(body, `{"level":"debug","disableCaller":false,"disableStacktrace":true}`) + s.Equal(`{"level":"debug","disableCaller":false,"disableStacktrace":true}`, body) } if statusCode, _, err := s.httpGet("http://localhost:55000/log/error"); s.NoError(err) { s.Equal(http.StatusOK, statusCode) @@ -138,7 +137,7 @@ func (s *KeelTestSuite) TestServiceHTTPZap() { s.Run("enable stacktrace", func() { if statusCode, body, err := s.httpPut("http://localhost:9100/log", `{"disableStacktrace":false}`); s.NoError(err) { s.Equal(http.StatusOK, statusCode) - s.Equal(body, `{"level":"debug","disableCaller":false,"disableStacktrace":false}`) + s.Equal(`{"level":"debug","disableCaller":false,"disableStacktrace":false}`, body) } if statusCode, _, err := s.httpGet("http://localhost:55000/log/error"); s.NoError(err) { s.Equal(http.StatusOK, statusCode) @@ -179,7 +178,7 @@ func (s *KeelTestSuite) TestGraceful() { go func(waitChan chan string) { waitChan <- "ok" time.Sleep(time.Second) - if assert.NoError(s.T(), syscall.Kill(syscall.Getpid(), syscall.SIGINT)) { + if s.NoError(syscall.Kill(syscall.Getpid(), syscall.SIGINT)) { s.l.Info("killed myself") } }(waitChan) @@ -190,7 +189,7 @@ func (s *KeelTestSuite) TestGraceful() { { // check that server is down _, _, err := s.httpGet("http://localhost:55000/ok") - s.Error(err) + s.Require().Error(err) } s.l.Info("done") diff --git a/service/httpreadme_test.go b/service/httpreadme_test.go index 45b388d..a9cd65a 100644 --- a/service/httpreadme_test.go +++ b/service/httpreadme_test.go @@ -18,7 +18,7 @@ import ( "go.uber.org/zap" ) -func _ExampleNewHTTPReadme() { +func _ExampleNewHTTPReadme() { //nolint:unused // define vars so it does not panic _ = os.Setenv("EXAMPLE_REQUIRED_BOOL", "true") _ = os.Setenv("EXAMPLE_REQUIRED_STRING", "foo") diff --git a/serviceenabler.go b/serviceenabler.go index bfcdccc..19f38c3 100644 --- a/serviceenabler.go +++ b/serviceenabler.go @@ -40,10 +40,10 @@ func (w *ServiceEnabler) Name() string { } func (w *ServiceEnabler) Start(ctx context.Context) error { - w.watch() + w.watch(w.ctx) //nolint:contextcheck w.ctx = ctx if w.enabled() { - if err := w.enable(w.ctx); err != nil { + if err := w.enable(w.ctx); err != nil { //nolint:contextcheck return err } } else { @@ -56,7 +56,7 @@ func (w *ServiceEnabler) Close(ctx context.Context) error { l := log.WithServiceName(w.l, w.Name()) w.setClosed(true) if w.enabled() { - if err := w.disable(w.ctx); err != nil { + if err := w.disable(w.ctx); err != nil { //nolint:contextcheck return err } } else { @@ -102,7 +102,7 @@ func (w *ServiceEnabler) disable(ctx context.Context) error { return w.service.Close(ctx) } -func (w *ServiceEnabler) watch() { +func (w *ServiceEnabler) watch(ctx context.Context) { go func() { for { if w.closed() { @@ -112,12 +112,12 @@ func (w *ServiceEnabler) watch() { if value := w.enabledFn(); value != w.enabled() { if value { go func() { - if err := w.enable(w.ctx); err != nil { + if err := w.enable(ctx); err != nil { w.l.Fatal("failed to dynamically start service", log.FError(err)) } }() } else { - if err := w.disable(context.TODO()); err != nil { + if err := w.disable(context.TODO()); err != nil { //nolint:contextcheck w.l.Fatal("failed to dynamically close service", log.FError(err)) } } diff --git a/telemetry/meter.go b/telemetry/meter.go index 287dd9c..31eb9e8 100644 --- a/telemetry/meter.go +++ b/telemetry/meter.go @@ -3,7 +3,6 @@ package telemetry import ( "context" "encoding/json" - "log" "os" "github.com/foomo/keel/env" @@ -44,7 +43,7 @@ func NewStdOutMeterProvider(ctx context.Context, opts ...stdoutmetric.Option) (m exporter, err := stdoutmetric.New(opts...) if err != nil { - log.Fatalf("creating stdoutmetric exporter: %v", err) + return nil, err } resource := otelresource.NewSchemaless(