From bdaae2064eb3bc02b452add3ed38b7e1d4564476 Mon Sep 17 00:00:00 2001 From: e_forbes Date: Tue, 9 Dec 2025 13:41:26 +0000 Subject: [PATCH 1/4] Migrates gitlab-sshd main file to labkit v2 --- cmd/gitlab-sshd/main.go | 61 +++++++++++++++++++++++++------------ go.mod | 4 +-- go.sum | 4 +++ internal/command/command.go | 6 ++++ 4 files changed, 54 insertions(+), 21 deletions(-) diff --git a/cmd/gitlab-sshd/main.go b/cmd/gitlab-sshd/main.go index aeae48d6..769f1cf7 100644 --- a/cmd/gitlab-sshd/main.go +++ b/cmd/gitlab-sshd/main.go @@ -4,6 +4,7 @@ package main import ( "context" "flag" + "log/slog" "os" "os/signal" "syscall" @@ -11,11 +12,11 @@ import ( "gitlab.com/gitlab-org/gitlab-shell/v14/internal/command" "gitlab.com/gitlab-org/gitlab-shell/v14/internal/config" - "gitlab.com/gitlab-org/gitlab-shell/v14/internal/logger" "gitlab.com/gitlab-org/gitlab-shell/v14/internal/sshd" - "gitlab.com/gitlab-org/labkit/log" + "gitlab.com/gitlab-org/labkit/fields" "gitlab.com/gitlab-org/labkit/monitoring" + "gitlab.com/gitlab-org/labkit/v2/log" ) var ( @@ -43,8 +44,9 @@ func overrideConfigFromEnvironment(cfg *config.Config) { } func main() { + ctx := context.Background() + logger := log.New() command.CheckForVersionFlag(os.Args, Version, BuildTime) - flag.Parse() cfg := new(config.Config) @@ -52,27 +54,30 @@ func main() { var err error cfg, err = config.NewFromDir(*configDir) if err != nil { - log.WithError(err).Fatal("failed to load configuration from specified directory") + logger.ErrorContext( + ctx, + "failed to load configuration from specified directory", + slog.String(fields.ErrorMessage, err.Error()), + ) + return } } overrideConfigFromEnvironment(cfg) if err := cfg.IsSane(); err != nil { + ctx = log.WithFields(context.Background(), slog.String( + fields.ErrorMessage, err.Error(), + )) if *configDir == "" { - log.WithError(err).Fatal("no config-dir provided, using only environment variables") + logger.ErrorContext(ctx, "no config-dir provided, using only environment variables") } else { - log.WithError(err).Fatal("configuration error") + logger.ErrorContext(ctx, "configuration error") } + return } cfg.ApplyGlobalState() - logCloser := logger.ConfigureStandalone(cfg) - defer func() { - if err := logCloser.Close(); err != nil { - log.WithError(err).Fatal("Error closing logCloser") - } - }() ctx, finished := command.Setup("gitlab-sshd", cfg) defer finished() @@ -80,7 +85,10 @@ func main() { server, err := sshd.NewServer(cfg) if err != nil { - log.WithError(err).Fatal("Failed to start GitLab built-in sshd") + logger.ErrorContext(ctx, "Failed to start GitLab built-in sshd", + slog.String(fields.ErrorMessage, err.Error()), + ) + return } // Startup monitoring endpoint. @@ -97,20 +105,33 @@ func main() { gracefulShutdown(ctx, done, cfg, server, cancel) if err := server.ListenAndServe(ctx); err != nil { - log.WithError(err).Fatal("GitLab built-in sshd failed to listen for new connections") + logger.ErrorContext(ctx, "GitLab built-in sshd failed to listen for new connections", + slog.String(fields.ErrorMessage, err.Error())) + return } } -func gracefulShutdown(ctx context.Context, done chan os.Signal, cfg *config.Config, server *sshd.Server, cancel context.CancelFunc) { +func gracefulShutdown( + ctx context.Context, + done chan os.Signal, + cfg *config.Config, + server *sshd.Server, + cancel context.CancelFunc, +) { go func() { sig := <-done signal.Reset(syscall.SIGINT, syscall.SIGTERM) + logger := log.New() gracePeriod := time.Duration(cfg.Server.GracePeriod) - log.WithContextFields(ctx, log.Fields{"shutdown_timeout_s": gracePeriod.Seconds(), "signal": sig.String()}).Info("Shutdown initiated") + logger.InfoContext(ctx, "Shutdown initiated", + slog.Float64("shutdown_timeout_s", gracePeriod.Seconds()), + slog.String("signal", sig.String()), + ) if err := server.Shutdown(); err != nil { - log.WithError(err).Fatal("Error shutting down the server") + logger.ErrorContext(ctx, "Error shutting down the server", slog.String(fields.ErrorMessage, err.Error())) + return } <-time.After(gracePeriod) @@ -126,7 +147,9 @@ func startupMonitoringEndpoint(cfg *config.Config, server *sshd.Server) { monitoring.WithBuildInformation(Version, BuildTime), monitoring.WithServeMux(server.MonitoringServeMux()), ) - - log.WithError(err).Fatal("monitoring service raised an error") + logger := log.New() + logger.Error("monitoring service raised an error", slog.String( + fields.ErrorMessage, err.Error(), + )) }() } diff --git a/go.mod b/go.mod index 005c18a9..2ae6e60c 100644 --- a/go.mod +++ b/go.mod @@ -23,7 +23,7 @@ require ( // Please do not override. Once v16.11.1 is released, this comment // can be removed. gitlab.com/gitlab-org/gitaly/v16 v16.11.0-rc1.0.20250408053233-c6d43513e93c - gitlab.com/gitlab-org/labkit v1.27.1 + gitlab.com/gitlab-org/labkit v1.34.0 golang.org/x/crypto v0.41.0 golang.org/x/sync v0.16.0 google.golang.org/grpc v1.72.0 @@ -65,7 +65,7 @@ require ( github.com/googleapis/enterprise-certificate-proxy v0.3.4 // indirect github.com/googleapis/gax-go/v2 v2.13.0 // indirect github.com/grpc-ecosystem/go-grpc-middleware v1.4.0 // indirect - github.com/grpc-ecosystem/go-grpc-middleware/v2 v2.3.1 // indirect + github.com/grpc-ecosystem/go-grpc-middleware/v2 v2.3.2 // indirect github.com/hashicorp/go-cleanhttp v0.5.2 // indirect github.com/hashicorp/yamux v0.1.2-0.20220728231024-8f49b6f63f18 // indirect github.com/jmespath/go-jmespath v0.4.0 // indirect diff --git a/go.sum b/go.sum index f068899f..97d792c7 100644 --- a/go.sum +++ b/go.sum @@ -350,6 +350,8 @@ github.com/grpc-ecosystem/go-grpc-middleware v1.4.0 h1:UH//fgunKIs4JdUbpDl1VZCDa github.com/grpc-ecosystem/go-grpc-middleware v1.4.0/go.mod h1:g5qyo/la0ALbONm6Vbp88Yd8NsDy6rZz+RcrMPxvld8= github.com/grpc-ecosystem/go-grpc-middleware/v2 v2.3.1 h1:KcFzXwzM/kGhIRHvc8jdixfIJjVzuUJdnv+5xsPutog= github.com/grpc-ecosystem/go-grpc-middleware/v2 v2.3.1/go.mod h1:qOchhhIlmRcqk/O9uCo/puJlyo07YINaIqdZfZG3Jkc= +github.com/grpc-ecosystem/go-grpc-middleware/v2 v2.3.2 h1:sGm2vDRFUrQJO/Veii4h4zG2vvqG6uWNkBHSTqXOZk0= +github.com/grpc-ecosystem/go-grpc-middleware/v2 v2.3.2/go.mod h1:wd1YpapPLivG6nQgbf7ZkG1hhSOXDhhn4MLTknx2aAc= github.com/grpc-ecosystem/go-grpc-prometheus v1.2.0 h1:Ovs26xHkKqVztRpIrF/92BcuyuQ/YW4NSIpoGtfXNho= github.com/grpc-ecosystem/go-grpc-prometheus v1.2.0/go.mod h1:8NvIoxWQoOIhqOTXgfV/d3M/q6VIi02HzZEHgUlZvzk= github.com/grpc-ecosystem/grpc-gateway v1.16.0/go.mod h1:BDjrQk3hbvj6Nolgz8mAMFbcEtjT1g+wF4CSlocrBnw= @@ -556,6 +558,8 @@ gitlab.com/gitlab-org/go/reopen v1.0.0 h1:6BujZ0lkkjGIejTUJdNO1w56mN1SI10qcVQyQl gitlab.com/gitlab-org/go/reopen v1.0.0/go.mod h1:D6OID8YJDzEVZNYW02R/Pkj0v8gYFSIhXFTArAsBQw8= gitlab.com/gitlab-org/labkit v1.27.1 h1:c4gL4qfHPMZwetbFZO5HDam98MOS1Ul/CC8QTPon5/c= gitlab.com/gitlab-org/labkit v1.27.1/go.mod h1:ZHOQIOVQKeOEKvQ/GhGBjUNbV3zWsx8nty6D/SRCyd4= +gitlab.com/gitlab-org/labkit v1.34.0 h1:wJrUZdfxbZrA9+qvnpk5gYJtceW+p7ceSJSg1BPZWTs= +gitlab.com/gitlab-org/labkit v1.34.0/go.mod h1:JqQLdgjV/KKAZJ6gvNodaLStmWeTT9mxgJPIEi66VHI= go.etcd.io/raft/v3 v3.6.0 h1:5NtvbDVYpnfZWcIHgGRk9DyzkBIXOi8j+DDp1IcnUWQ= go.etcd.io/raft/v3 v3.6.0/go.mod h1:nLvLevg6+xrVtHUmVaTcTz603gQPHfh7kUAwV6YpfGo= go.opencensus.io v0.21.0/go.mod h1:mSImk1erAIZhrmZN+AvHh14ztQfjbGwt4TtuofqLduU= diff --git a/internal/command/command.go b/internal/command/command.go index eedbe7fd..51d18673 100644 --- a/internal/command/command.go +++ b/internal/command/command.go @@ -3,13 +3,16 @@ package command import ( "context" "fmt" + "log/slog" "os" "path" "strings" "gitlab.com/gitlab-org/gitlab-shell/v14/internal/config" "gitlab.com/gitlab-org/labkit/correlation" + "gitlab.com/gitlab-org/labkit/fields" "gitlab.com/gitlab-org/labkit/tracing" + "gitlab.com/gitlab-org/labkit/v2/log" ) type Command interface { @@ -73,6 +76,9 @@ func Setup(serviceName string, config *config.Config) (context.Context, func()) if correlationID == "" { correlationID := correlation.SafeRandomID() ctx = correlation.ContextWithCorrelation(ctx, correlationID) + ctx = log.WithFields(ctx, slog.String( + fields.CorrelationID, correlationID, + )) } return ctx, func() { -- GitLab From 3caaf9e1cf0788fa74b908fd10e676b53da28f77 Mon Sep 17 00:00:00 2001 From: e_forbes Date: Tue, 9 Dec 2025 13:50:46 +0000 Subject: [PATCH 2/4] go mod tidy --- go.sum | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/go.sum b/go.sum index 97d792c7..0cdfbe57 100644 --- a/go.sum +++ b/go.sum @@ -1,5 +1,5 @@ -cel.dev/expr v0.20.0 h1:OunBvVCfvpWlt4dN7zg3FM6TDkzOePe1+foGJ9AXeeI= -cel.dev/expr v0.20.0/go.mod h1:MrpN08Q+lEBs+bGYdLxxHkZoUSsCp0nSKTs0nTymJgw= +cel.dev/expr v0.23.1 h1:K4KOtPCJQjVggkARsjG9RWXP6O4R73aHeJMa/dmCQQg= +cel.dev/expr v0.23.1/go.mod h1:hLPLo1W4QUmuYdA72RBX06QTs6MXw941piREPl3Yfiw= cloud.google.com/go v0.26.0/go.mod h1:aQUYkXzVsufM+DwF1aE+0xfcU+56JwCaLick0ClmMTw= cloud.google.com/go v0.34.0/go.mod h1:aQUYkXzVsufM+DwF1aE+0xfcU+56JwCaLick0ClmMTw= cloud.google.com/go v0.38.0/go.mod h1:990N+gfupTy94rShfmMCWGDn0LpTmnzTp2qbd1dvSRU= @@ -348,8 +348,6 @@ github.com/googleapis/gax-go/v2 v2.13.0 h1:yitjD5f7jQHhyDsnhKEBU52NdvvdSeGzlAnDP github.com/googleapis/gax-go/v2 v2.13.0/go.mod h1:Z/fvTZXF8/uw7Xu5GuslPw+bplx6SS338j1Is2S+B7A= github.com/grpc-ecosystem/go-grpc-middleware v1.4.0 h1:UH//fgunKIs4JdUbpDl1VZCDaL56wXCB/5+wF6uHfaI= github.com/grpc-ecosystem/go-grpc-middleware v1.4.0/go.mod h1:g5qyo/la0ALbONm6Vbp88Yd8NsDy6rZz+RcrMPxvld8= -github.com/grpc-ecosystem/go-grpc-middleware/v2 v2.3.1 h1:KcFzXwzM/kGhIRHvc8jdixfIJjVzuUJdnv+5xsPutog= -github.com/grpc-ecosystem/go-grpc-middleware/v2 v2.3.1/go.mod h1:qOchhhIlmRcqk/O9uCo/puJlyo07YINaIqdZfZG3Jkc= github.com/grpc-ecosystem/go-grpc-middleware/v2 v2.3.2 h1:sGm2vDRFUrQJO/Veii4h4zG2vvqG6uWNkBHSTqXOZk0= github.com/grpc-ecosystem/go-grpc-middleware/v2 v2.3.2/go.mod h1:wd1YpapPLivG6nQgbf7ZkG1hhSOXDhhn4MLTknx2aAc= github.com/grpc-ecosystem/go-grpc-prometheus v1.2.0 h1:Ovs26xHkKqVztRpIrF/92BcuyuQ/YW4NSIpoGtfXNho= @@ -556,8 +554,6 @@ gitlab.com/gitlab-org/gitaly/v16 v16.11.0-rc1.0.20250408053233-c6d43513e93c h1:x gitlab.com/gitlab-org/gitaly/v16 v16.11.0-rc1.0.20250408053233-c6d43513e93c/go.mod h1:/rkj6992VsNymUeG6N3VnLZ8Pvb1Y9ZUo00Yy35t8WQ= gitlab.com/gitlab-org/go/reopen v1.0.0 h1:6BujZ0lkkjGIejTUJdNO1w56mN1SI10qcVQyQlOPM+8= gitlab.com/gitlab-org/go/reopen v1.0.0/go.mod h1:D6OID8YJDzEVZNYW02R/Pkj0v8gYFSIhXFTArAsBQw8= -gitlab.com/gitlab-org/labkit v1.27.1 h1:c4gL4qfHPMZwetbFZO5HDam98MOS1Ul/CC8QTPon5/c= -gitlab.com/gitlab-org/labkit v1.27.1/go.mod h1:ZHOQIOVQKeOEKvQ/GhGBjUNbV3zWsx8nty6D/SRCyd4= gitlab.com/gitlab-org/labkit v1.34.0 h1:wJrUZdfxbZrA9+qvnpk5gYJtceW+p7ceSJSg1BPZWTs= gitlab.com/gitlab-org/labkit v1.34.0/go.mod h1:JqQLdgjV/KKAZJ6gvNodaLStmWeTT9mxgJPIEi66VHI= go.etcd.io/raft/v3 v3.6.0 h1:5NtvbDVYpnfZWcIHgGRk9DyzkBIXOi8j+DDp1IcnUWQ= -- GitLab From c1b113491c0f5fe3b350d3ab954bf0b96729ab31 Mon Sep 17 00:00:00 2001 From: e_forbes Date: Tue, 9 Dec 2025 13:51:21 +0000 Subject: [PATCH 3/4] Unused --- internal/sshd/sshd.go | 14 -------------- 1 file changed, 14 deletions(-) diff --git a/internal/sshd/sshd.go b/internal/sshd/sshd.go index e83b78ef..478c63b9 100644 --- a/internal/sshd/sshd.go +++ b/internal/sshd/sshd.go @@ -265,20 +265,6 @@ func (s *Server) proxyPolicy() (proxyproto.PolicyFunc, error) { } } -func extractDataFromContext(ctx context.Context) command.LogData { - logData := command.LogData{} - - if ctx == nil { - return logData - } - - if ctx.Value(command.LogDataKey) != nil { - logData = ctx.Value(command.LogDataKey).(command.LogData) - } - - return logData -} - func extractLogDataFromContext(ctx context.Context) command.LogData { logData := command.LogData{} -- GitLab From 397f2162fa3b3d78edd88a05938fb72ceae0293b Mon Sep 17 00:00:00 2001 From: e_forbes Date: Tue, 9 Dec 2025 14:06:24 +0000 Subject: [PATCH 4/4] Lint fix --- support/lint_last_known_acceptable.txt | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/support/lint_last_known_acceptable.txt b/support/lint_last_known_acceptable.txt index 1f68e46d..196c7968 100644 --- a/support/lint_last_known_acceptable.txt +++ b/support/lint_last_known_acceptable.txt @@ -36,15 +36,16 @@ cmd/gitlab-shell/main.go:48:15: Error return value of `fmt.Fprintln` is not chec cmd/gitlab-shell/main.go:53:23: Error return value of `logCloser.Close` is not checked (errcheck) cmd/gitlab-shell/main.go:60:14: Error return value of `fmt.Fprintf` is not checked (errcheck) cmd/gitlab-shell/main.go:61:3: exitAfterDefer: os.Exit will exit, and `defer logCloser.Close()` will not run (gocritic) +cmd/gitlab-sshd/main.go:46:6: Function 'main' is too long (64 > 60) (funlen) internal/command/authorizedkeys/authorized_keys.go:29:4: internal/command/authorizedkeys/authorized_keys.go:29: Line contains TODO/BUG/FIXME/NOTE/OPTIMIZE/HACK: "TODO: Log this event once we have a cons..." (godox) internal/command/command.go:1:1: package-comments: should have a package comment (revive) -internal/command/command.go:15:6: exported: exported type Command should have comment or be unexported (revive) -internal/command/command.go:19:6: exported: exported type LogMetadata should have comment or be unexported (revive) -internal/command/command.go:26:6: exported: exported type LogData should have comment or be unexported (revive) -internal/command/command.go:37:1: exported: exported function CheckForVersionFlag should have comment or be unexported (revive) -internal/command/command.go:48:1: exported: comment on exported function Setup should be of the form "Setup ..." (revive) -internal/command/command.go:80:15: Error return value of `closer.Close` is not checked (errcheck) -internal/command/command.go:84:1: exported: exported function NewLogData should have comment or be unexported (revive) +internal/command/command.go:18:6: exported: exported type Command should have comment or be unexported (revive) +internal/command/command.go:22:6: exported: exported type LogMetadata should have comment or be unexported (revive) +internal/command/command.go:29:6: exported: exported type LogData should have comment or be unexported (revive) +internal/command/command.go:40:1: exported: exported function CheckForVersionFlag should have comment or be unexported (revive) +internal/command/command.go:51:1: exported: comment on exported function Setup should be of the form "Setup ..." (revive) +internal/command/command.go:86:15: Error return value of `closer.Close` is not checked (errcheck) +internal/command/command.go:90:1: exported: exported function NewLogData should have comment or be unexported (revive) internal/command/githttp/pull.go:48: 48-65 lines are duplicate of `internal/command/githttp/push.go:45-62` (dupl) internal/command/githttp/push.go:45: 45-62 lines are duplicate of `internal/command/githttp/pull.go:48-65` (dupl) internal/command/lfsauthenticate/lfsauthenticate.go:87:13: Error return value of `fmt.Fprintf` is not checked (errcheck) @@ -90,4 +91,3 @@ internal/gitlabnet/healthcheck/client_test.go:19:41: unused-parameter: parameter internal/gitlabnet/lfstransfer/client.go:137:3: internal/gitlabnet/lfstransfer/client.go:137: Line contains TODO/BUG/FIXME/NOTE/OPTIMIZE/HACK: "FIXME: This causes tests to fail" (godox) internal/sshd/server_config.go:130:19: SA1019: ssh.KeyAlgoDSA is deprecated: DSA is only supported at insecure key sizes, and was removed from major implementations. (staticcheck) internal/sshd/server_config_test.go:5:2: SA1019: "crypto/dsa" has been deprecated since Go 1.16 because it shouldn't be used: DSA is a legacy algorithm, and modern alternatives such as Ed25519 (implemented by package crypto/ed25519) should be used instead. Keys with 1024-bit moduli (L1024N160 parameters) are cryptographically weak, while bigger keys are not widely supported. Note that FIPS 186-5 no longer approves DSA for signature generation. (staticcheck) -internal/sshd/sshd.go:268:6: func `extractDataFromContext` is unused (unused) -- GitLab