Skip to content

Commit

Permalink
feat: update & fix lints
Browse files Browse the repository at this point in the history
  • Loading branch information
franklinkim committed Apr 16, 2024
1 parent 686ef11 commit 8882274
Show file tree
Hide file tree
Showing 18 changed files with 247 additions and 111 deletions.
245 changes: 192 additions & 53 deletions .golangci.yml

Large diffs are not rendered by default.

2 changes: 1 addition & 1 deletion config/remote.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
4 changes: 2 additions & 2 deletions config/watch.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
Expand Down Expand Up @@ -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
}
Expand Down
7 changes: 4 additions & 3 deletions errors/wrappederror_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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() {
Expand Down Expand Up @@ -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)
}
2 changes: 0 additions & 2 deletions go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -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
6 changes: 3 additions & 3 deletions jwt/jwtkey.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
}
Expand Down
2 changes: 1 addition & 1 deletion log/with.go
Original file line number Diff line number Diff line change
Expand Up @@ -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()))
}
Expand Down
2 changes: 1 addition & 1 deletion net/gotsrpc/errors.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
8 changes: 4 additions & 4 deletions net/http/middleware/jwt.go
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand Down Expand Up @@ -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)
Expand Down
2 changes: 1 addition & 1 deletion net/http/roundtripware/circuitbreaker.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
30 changes: 15 additions & 15 deletions net/http/roundtripware/circuitbreaker_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
},
}

Expand Down Expand Up @@ -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)
}
Expand All @@ -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)

Expand All @@ -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)
}
Expand Down Expand Up @@ -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,
Expand All @@ -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
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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)
}
Expand All @@ -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)
}
Expand All @@ -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)
}
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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
Expand All @@ -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)
}
4 changes: 2 additions & 2 deletions persistence/mongo/collection.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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 {
Expand Down
4 changes: 2 additions & 2 deletions persistence/mongo/utils.go
Original file line number Diff line number Diff line change
Expand Up @@ -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")
}
}
10 changes: 5 additions & 5 deletions server.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
}

Expand Down
13 changes: 6 additions & 7 deletions server_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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)
Expand All @@ -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)
Expand All @@ -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)
Expand All @@ -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)
Expand Down Expand Up @@ -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)
Expand All @@ -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")
Expand Down
2 changes: 1 addition & 1 deletion service/httpreadme_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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")
Expand Down
Loading

0 comments on commit 8882274

Please sign in to comment.