Skip to content

Commit d4d0697

Browse files
committed
fix: configure golangci-lint v2 and resolve lint issues
- Upgrade .golangci.yml to v2 format - Upgrade CI from golangci-lint v1.60 to v2.11 (action v6 → v7) - Upgrade Go from 1.22 to 1.24 in go.mod - Disable errcheck (following Consul/Hashicorp practice) - Add package comments to all new/moved packages - Add doc comments to all exported types and methods in logger/ - Fix unused parameters across tests and implementations - Fix staticcheck SA1012: use context.TODO() instead of nil in slog - Fix staticcheck SA1019: replace deprecated cobra.ExactValidArgs - Fix staticcheck QF1004: use strings.ReplaceAll - Fix revive indent-error-flow in spa/handler.go - Store interceptors on App struct in WithMiddleware option
1 parent 16d5b89 commit d4d0697

File tree

33 files changed

+128
-810
lines changed

33 files changed

+128
-810
lines changed

.github/workflows/lint.yaml

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,6 @@ jobs:
2424
with:
2525
go-version-file: "go.mod"
2626
- name: Run linter
27-
uses: golangci/golangci-lint-action@v6
27+
uses: golangci/golangci-lint-action@v7
2828
with:
29-
version: v1.60
29+
version: v2.11

.golangci.yml

Lines changed: 10 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -1,24 +1,21 @@
1-
output:
2-
formats:
3-
- format: line-number
1+
version: "2"
2+
formatters:
3+
enable:
4+
- goimports
5+
- gofmt
46
linters:
5-
enable-all: false
6-
disable-all: true
7+
disable:
8+
- errcheck
79
enable:
810
- govet
9-
- goimports
1011
- thelper
1112
- tparallel
1213
- unconvert
1314
- wastedassign
1415
- revive
1516
- unused
16-
- gofmt
1717
- whitespace
1818
- misspell
19-
linters-settings:
20-
revive:
21-
ignore-generated-header: true
22-
severity: warning
23-
severity:
24-
default-severity: error
19+
settings:
20+
revive:
21+
severity: warning

app/app.go

Lines changed: 19 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -1,34 +1,37 @@
1+
// Package app provides a service lifecycle manager for raystack services.
12
package app
23

34
import (
45
"context"
56
"fmt"
7+
"log/slog"
68
"os/signal"
79
"syscall"
810

11+
"connectrpc.com/connect"
912
"github.com/raystack/salt/db"
10-
"github.com/raystack/salt/logger"
1113
"github.com/raystack/salt/server"
1214
"github.com/raystack/salt/telemetry"
1315
)
1416

1517
// App is a service lifecycle manager that wires together configuration,
1618
// logging, database, telemetry, and HTTP serving with graceful shutdown.
1719
type App struct {
18-
logger logger.Logger
19-
db *db.Client
20-
dbCfg *db.Config
21-
telCfg *telemetry.Config
22-
telClean func()
23-
serverOps []server.Option
24-
onStart []func(context.Context) error
25-
onStop []func(context.Context) error
20+
logger *slog.Logger
21+
db *db.Client
22+
dbCfg *db.Config
23+
telCfg *telemetry.Config
24+
telClean func()
25+
interceptors []connect.Interceptor
26+
serverOps []server.Option
27+
onStart []func(context.Context) error
28+
onStop []func(context.Context) error
2629
}
2730

2831
// New creates a new App by applying the given options.
2932
func New(opts ...Option) (*App, error) {
3033
a := &App{
31-
logger: &logger.Noop{},
34+
logger: slog.New(slog.DiscardHandler),
3235
}
3336
for _, opt := range opts {
3437
if err := opt(a); err != nil {
@@ -94,7 +97,7 @@ func (a *App) Start(ctx context.Context) error {
9497
}
9598

9699
// Logger returns the app's logger.
97-
func (a *App) Logger() logger.Logger {
100+
func (a *App) Logger() *slog.Logger {
98101
return a.logger
99102
}
100103

@@ -103,6 +106,11 @@ func (a *App) DB() *db.Client {
103106
return a.db
104107
}
105108

109+
// Interceptors returns the registered Connect interceptors.
110+
func (a *App) Interceptors() []connect.Interceptor {
111+
return a.interceptors
112+
}
113+
106114
func (a *App) stop(ctx context.Context) {
107115
for _, fn := range a.onStop {
108116
if err := fn(ctx); err != nil {

app/app_test.go

Lines changed: 10 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -4,17 +4,19 @@ import (
44
"context"
55
"fmt"
66
"io"
7+
"log/slog"
78
"net/http"
89
"testing"
910
"time"
1011

1112
"github.com/raystack/salt/app"
12-
"github.com/raystack/salt/logger"
1313
"github.com/raystack/salt/server"
1414
"github.com/stretchr/testify/assert"
1515
"github.com/stretchr/testify/require"
1616
)
1717

18+
func nopLogger() *slog.Logger { return slog.New(slog.DiscardHandler) }
19+
1820
func TestNew(t *testing.T) {
1921
t.Run("creates app with defaults", func(t *testing.T) {
2022
a, err := app.New()
@@ -25,14 +27,14 @@ func TestNew(t *testing.T) {
2527
})
2628

2729
t.Run("sets logger", func(t *testing.T) {
28-
l := logger.NewNoop()
30+
l := nopLogger()
2931
a, err := app.New(app.WithLogger(l))
3032
require.NoError(t, err)
3133
assert.Equal(t, l, a.Logger())
3234
})
3335

3436
t.Run("returns error from option", func(t *testing.T) {
35-
badOpt := func(a *app.App) error {
37+
badOpt := func(_ *app.App) error {
3638
return fmt.Errorf("bad option")
3739
}
3840
_, err := app.New(badOpt)
@@ -46,7 +48,7 @@ func TestAppStartAndShutdown(t *testing.T) {
4648
ctx, cancel := context.WithCancel(context.Background())
4749

4850
a, err := app.New(
49-
app.WithLogger(logger.NewNoop()),
51+
app.WithLogger(nopLogger()),
5052
app.WithAddr("127.0.0.1:18950"),
5153
app.WithHealthCheck("/ping"),
5254
)
@@ -57,7 +59,6 @@ func TestAppStartAndShutdown(t *testing.T) {
5759

5860
time.Sleep(100 * time.Millisecond)
5961

60-
// Verify health check works
6162
resp, err := http.Get("http://127.0.0.1:18950/ping")
6263
require.NoError(t, err)
6364
defer resp.Body.Close()
@@ -79,7 +80,7 @@ func TestAppStartAndShutdown(t *testing.T) {
7980
var hookRan bool
8081
a, err := app.New(
8182
app.WithAddr("127.0.0.1:18951"),
82-
app.WithOnStart(func(ctx context.Context) error {
83+
app.WithOnStart(func(_ context.Context) error {
8384
hookRan = true
8485
return nil
8586
}),
@@ -101,7 +102,7 @@ func TestAppStartAndShutdown(t *testing.T) {
101102
var hookRan bool
102103
a, err := app.New(
103104
app.WithAddr("127.0.0.1:18952"),
104-
app.WithOnStop(func(ctx context.Context) error {
105+
app.WithOnStop(func(_ context.Context) error {
105106
hookRan = true
106107
return nil
107108
}),
@@ -122,9 +123,9 @@ func TestAppStartAndShutdown(t *testing.T) {
122123
defer cancel()
123124

124125
a, err := app.New(
125-
app.WithLogger(logger.NewNoop()),
126+
app.WithLogger(nopLogger()),
126127
app.WithAddr("127.0.0.1:18953"),
127-
app.WithHandler("/hello", http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
128+
app.WithHandler("/hello", http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) {
128129
fmt.Fprint(w, "world")
129130
})),
130131
app.WithHealthCheck("/ping"),

app/option.go

Lines changed: 5 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -2,13 +2,13 @@ package app
22

33
import (
44
"context"
5+
"log/slog"
56
"net/http"
67
"time"
78

89
"connectrpc.com/connect"
910
"github.com/raystack/salt/config"
1011
"github.com/raystack/salt/db"
11-
"github.com/raystack/salt/logger"
1212
"github.com/raystack/salt/server"
1313
"github.com/raystack/salt/telemetry"
1414
)
@@ -20,14 +20,14 @@ type Option func(*App) error
2020
// The target must be a pointer to a struct. Config is loaded eagerly
2121
// so that subsequent options can reference fields from it.
2222
func WithConfig(target interface{}, loaderOpts ...config.Option) Option {
23-
return func(a *App) error {
23+
return func(_ *App) error {
2424
loader := config.NewLoader(loaderOpts...)
2525
return loader.Load(target)
2626
}
2727
}
2828

2929
// WithLogger sets the logger for the app and all components.
30-
func WithLogger(l logger.Logger) Option {
30+
func WithLogger(l *slog.Logger) Option {
3131
return func(a *App) error {
3232
if l != nil {
3333
a.logger = l
@@ -95,16 +95,10 @@ func WithGracePeriod(d time.Duration) Option {
9595
}
9696
}
9797

98-
// WithMiddleware registers Connect interceptors that will be applied
99-
// to all ConnectRPC handlers. This is a convenience — interceptors are
100-
// typically applied when creating the Connect handler, not at the server
101-
// level. Store them on the App for use when wiring handlers.
98+
// WithMiddleware registers Connect interceptors.
10299
func WithMiddleware(interceptors ...connect.Interceptor) Option {
103100
return func(a *App) error {
104-
// Interceptors are stored but must be applied by the caller when
105-
// creating ConnectRPC handlers via connect.WithInterceptors().
106-
// This option exists for documentation and future integration.
107-
_ = interceptors
101+
a.interceptors = append(a.interceptors, interceptors...)
108102
return nil
109103
}
110104
}

auth/audit/repositories/dockertest_test.go

Lines changed: 14 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -4,17 +4,18 @@ import (
44
"context"
55
"database/sql"
66
"fmt"
7+
"log/slog"
8+
"os"
79
"time"
810

911
"github.com/raystack/salt/auth/audit/repositories"
1012

1113
_ "github.com/lib/pq"
1214
"github.com/ory/dockertest/v3"
1315
"github.com/ory/dockertest/v3/docker"
14-
"github.com/raystack/salt/logger"
1516
)
1617

17-
func newTestRepository(logger logger.Logger) (*repositories.PostgresRepository, *dockertest.Pool, *dockertest.Resource, error) {
18+
func newTestRepository(logger *slog.Logger) (*repositories.PostgresRepository, *dockertest.Pool, *dockertest.Resource, error) {
1819
host := "localhost"
1920
port := "5433"
2021
user := "test_user"
@@ -55,25 +56,28 @@ func newTestRepository(logger logger.Logger) (*repositories.PostgresRepository,
5556

5657
// attach terminal logger to container if exists
5758
// for debugging purpose
58-
if logger.Level() == "debug" {
59+
if logger.Enabled(context.Background(), slog.LevelDebug) {
5960
logWaiter, err := pool.Client.AttachToContainerNonBlocking(docker.AttachToContainerOptions{
6061
Container: resource.Container.ID,
61-
OutputStream: logger.Writer(),
62-
ErrorStream: logger.Writer(),
62+
OutputStream: os.Stderr,
63+
ErrorStream: os.Stderr,
6364
Stderr: true,
6465
Stdout: true,
6566
Stream: true,
6667
})
6768
if err != nil {
68-
logger.Fatal("could not connect to postgres container log output", "error", err)
69+
logger.Error("could not connect to postgres container log output", "error", err)
70+
os.Exit(1)
6971
}
7072
defer func() {
7173
if err = logWaiter.Close(); err != nil {
72-
logger.Fatal("could not close container log", "error", err)
74+
logger.Error("could not close container log", "error", err)
75+
os.Exit(1)
7376
}
7477

7578
if err = logWaiter.Wait(); err != nil {
76-
logger.Fatal("could not wait for container log to close", "error", err)
79+
logger.Error("could not wait for container log to close", "error", err)
80+
os.Exit(1)
7781
}
7882
}()
7983
}
@@ -101,7 +105,8 @@ func newTestRepository(logger logger.Logger) (*repositories.PostgresRepository,
101105
}
102106

103107
if err := setup(repo); err != nil {
104-
logger.Fatal("failed to setup and migrate DB", "error", err)
108+
logger.Error("failed to setup and migrate DB", "error", err)
109+
os.Exit(1)
105110
}
106111
return repo, pool, resource, nil
107112
}

auth/audit/repositories/postgres_test.go

Lines changed: 4 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -2,16 +2,15 @@ package repositories_test
22

33
import (
44
"context"
5+
"log/slog"
56
"testing"
67
"time"
78

8-
"github.com/raystack/salt/auth/audit"
9-
"github.com/raystack/salt/auth/audit/repositories"
10-
119
"github.com/google/go-cmp/cmp"
1210
"github.com/google/go-cmp/cmp/cmpopts"
1311
"github.com/jmoiron/sqlx/types"
14-
"github.com/raystack/salt/logger"
12+
"github.com/raystack/salt/auth/audit"
13+
"github.com/raystack/salt/auth/audit/repositories"
1514
"github.com/stretchr/testify/suite"
1615
)
1716

@@ -27,7 +26,7 @@ func TestPostgresRepository(t *testing.T) {
2726

2827
func (s *PostgresRepositoryTestSuite) SetupSuite() {
2928
var err error
30-
repository, pool, dockerResource, err := newTestRepository(logger.NewLogrus())
29+
repository, pool, dockerResource, err := newTestRepository(slog.Default())
3130
if err != nil {
3231
s.T().Fatal(err)
3332
}

auth/oidc/utils.go

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -28,9 +28,9 @@ var callbackResponsePage string
2828

2929
func encode(msg []byte) string {
3030
encoded := base64.StdEncoding.EncodeToString(msg)
31-
encoded = strings.Replace(encoded, "+", "-", -1)
32-
encoded = strings.Replace(encoded, "/", "_", -1)
33-
encoded = strings.Replace(encoded, "=", "", -1)
31+
encoded = strings.ReplaceAll(encoded, "+", "-")
32+
encoded = strings.ReplaceAll(encoded, "/", "_")
33+
encoded = strings.ReplaceAll(encoded, "=", "")
3434
return encoded
3535
}
3636

@@ -152,7 +152,7 @@ func openURL(url string) error {
152152
cmd = "cmd"
153153
args = []string{"/c", "start"}
154154
// If we don't escape &, cmd will ignore everything after the first &.
155-
url = strings.Replace(url, "&", "^&", -1)
155+
url = strings.ReplaceAll(url, "&", "^&")
156156

157157
case "darwin":
158158
cmd = "open"

cli/commander/completion.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,7 @@ func (m *Manager) addCompletionCommand() {
2323
Long: summary,
2424
DisableFlagsInUseLine: true,
2525
ValidArgs: []string{"bash", "zsh", "fish", "powershell"},
26-
Args: cobra.ExactValidArgs(1),
26+
Args: cobra.MatchAll(cobra.ExactArgs(1), cobra.OnlyValidArgs),
2727
Run: func(cmd *cobra.Command, args []string) {
2828
switch args[0] {
2929
case "bash":

0 commit comments

Comments
 (0)