From ed5a77e0d8f8e98347c207b0dc45a21b55b8810c Mon Sep 17 00:00:00 2001 From: e_forbes Date: Tue, 9 Dec 2025 14:00:29 +0000 Subject: [PATCH 1/3] Wires in the v2 labkit log library to valide parity This commit wires in duplicate log messages that will help us determine if the new log library is functionally comparable to the old logging setup. This will allow us to do safe validation in production before doing the final switch over. --- cmd/gitlab-sshd/acceptance_test.go | 1 - cmd/gitlab-sshd/main.go | 28 +++++++++++++++++++++++++- go.mod | 4 ++-- go.sum | 12 +++++------ support/lint_last_known_acceptable.txt | 1 + 5 files changed, 36 insertions(+), 10 deletions(-) diff --git a/cmd/gitlab-sshd/acceptance_test.go b/cmd/gitlab-sshd/acceptance_test.go index aa7f3ec8..ddb4bd25 100644 --- a/cmd/gitlab-sshd/acceptance_test.go +++ b/cmd/gitlab-sshd/acceptance_test.go @@ -52,7 +52,6 @@ var ( const ( testRepo = "test-gitlab-shell/gitlab-test.git" - testRepoNamespace = "test-gitlab-shell" testRepoImportURL = "https://gitlab.com/gitlab-org/gitlab-test.git" ) diff --git a/cmd/gitlab-sshd/main.go b/cmd/gitlab-sshd/main.go index aeae48d6..e78bb702 100644 --- a/cmd/gitlab-sshd/main.go +++ b/cmd/gitlab-sshd/main.go @@ -4,6 +4,8 @@ package main import ( "context" "flag" + "fmt" + "log/slog" "os" "os/signal" "syscall" @@ -14,8 +16,10 @@ import ( "gitlab.com/gitlab-org/gitlab-shell/v14/internal/logger" "gitlab.com/gitlab-org/gitlab-shell/v14/internal/sshd" + "gitlab.com/gitlab-org/labkit/fields" "gitlab.com/gitlab-org/labkit/log" "gitlab.com/gitlab-org/labkit/monitoring" + v2log "gitlab.com/gitlab-org/labkit/v2/log" ) var ( @@ -43,6 +47,9 @@ func overrideConfigFromEnvironment(cfg *config.Config) { } func main() { + ctx := context.Background() + v2Logger := v2log.New() + v2Logger.InfoContext(ctx, "v2log: gitlab-sshd starting up...") command.CheckForVersionFlag(os.Args, Version, BuildTime) flag.Parse() @@ -52,15 +59,23 @@ func main() { var err error cfg, err = config.NewFromDir(*configDir) if err != nil { + v2Logger.ErrorContext(ctx, "v2log: failed to load configuration from specified directory", slog.String( + fields.ErrorMessage, err.Error(), + )) log.WithError(err).Fatal("failed to load configuration from specified directory") } } overrideConfigFromEnvironment(cfg) if err := cfg.IsSane(); err != nil { + ctx = v2log.WithFields(ctx, + slog.String(fields.ErrorMessage, err.Error()), + ) if *configDir == "" { + v2Logger.ErrorContext(ctx, "v2log: no config-dir provided, using only environment variables") log.WithError(err).Fatal("no config-dir provided, using only environment variables") } else { + v2Logger.ErrorContext(ctx, "v2log: configuration error") log.WithError(err).Fatal("configuration error") } } @@ -70,6 +85,9 @@ func main() { logCloser := logger.ConfigureStandalone(cfg) defer func() { if err := logCloser.Close(); err != nil { + v2Logger.ErrorContext(ctx, "v2log: Error closing logCloser", slog.String( + fields.ErrorMessage, err.Error(), + )) log.WithError(err).Fatal("Error closing logCloser") } }() @@ -80,6 +98,9 @@ func main() { server, err := sshd.NewServer(cfg) if err != nil { + v2Logger.ErrorContext(ctx, "v2log: Failed to start Gitlab built-in sshd", slog.String( + fields.ErrorMessage, err.Error(), + )) log.WithError(err).Fatal("Failed to start GitLab built-in sshd") } @@ -97,22 +118,27 @@ func main() { gracefulShutdown(ctx, done, cfg, server, cancel) if err := server.ListenAndServe(ctx); err != nil { + v2Logger.ErrorContext(ctx, "v2log: GitLab built-in sshd failed to listen for new connections") log.WithError(err).Fatal("GitLab built-in sshd failed to listen for new connections") } } func gracefulShutdown(ctx context.Context, done chan os.Signal, cfg *config.Config, server *sshd.Server, cancel context.CancelFunc) { go func() { + v2Logger := v2log.New() sig := <-done signal.Reset(syscall.SIGINT, syscall.SIGTERM) gracePeriod := time.Duration(cfg.Server.GracePeriod) log.WithContextFields(ctx, log.Fields{"shutdown_timeout_s": gracePeriod.Seconds(), "signal": sig.String()}).Info("Shutdown initiated") + v2Logger.InfoContext(ctx, fmt.Sprintf("v2log: Shutdown initiated with grace period: %f", gracePeriod.Seconds()), + slog.String("signal", sig.String())) if err := server.Shutdown(); err != nil { log.WithError(err).Fatal("Error shutting down the server") + v2Logger.ErrorContext(ctx, "v2log: Error shutting down the server", slog.String( + fields.ErrorMessage, err.Error())) } - <-time.After(gracePeriod) cancel() 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..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,8 @@ 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= 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= @@ -554,8 +554,8 @@ 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= 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/support/lint_last_known_acceptable.txt b/support/lint_last_known_acceptable.txt index 1f68e46d..7bbd331e 100644 --- a/support/lint_last_known_acceptable.txt +++ b/support/lint_last_known_acceptable.txt @@ -36,6 +36,7 @@ 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:49:6: Function 'main' has too many statements (41 > 40) (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) -- GitLab From 60cc689df001fba915cc79128f73cedacf9a4e9e Mon Sep 17 00:00:00 2001 From: e_forbes Date: Wed, 10 Dec 2025 10:52:33 +0000 Subject: [PATCH 2/3] Configures UTC time on gitlab-sshd startup --- cmd/gitlab-sshd/main.go | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/cmd/gitlab-sshd/main.go b/cmd/gitlab-sshd/main.go index e78bb702..e5f1efca 100644 --- a/cmd/gitlab-sshd/main.go +++ b/cmd/gitlab-sshd/main.go @@ -46,8 +46,17 @@ func overrideConfigFromEnvironment(cfg *config.Config) { } } +func configureUTC() { + loc, err := time.LoadLocation("UTC") + if err != nil { + return + } + time.Local = loc +} + func main() { ctx := context.Background() + configureUTC() v2Logger := v2log.New() v2Logger.InfoContext(ctx, "v2log: gitlab-sshd starting up...") command.CheckForVersionFlag(os.Args, Version, BuildTime) -- GitLab From 2757d8f5cb45767364ab8dd7d65e400d007eeee9 Mon Sep 17 00:00:00 2001 From: e_forbes Date: Wed, 10 Dec 2025 11:04:31 +0000 Subject: [PATCH 3/3] reverting --- cmd/gitlab-sshd/main.go | 9 --------- 1 file changed, 9 deletions(-) diff --git a/cmd/gitlab-sshd/main.go b/cmd/gitlab-sshd/main.go index e5f1efca..e78bb702 100644 --- a/cmd/gitlab-sshd/main.go +++ b/cmd/gitlab-sshd/main.go @@ -46,17 +46,8 @@ func overrideConfigFromEnvironment(cfg *config.Config) { } } -func configureUTC() { - loc, err := time.LoadLocation("UTC") - if err != nil { - return - } - time.Local = loc -} - func main() { ctx := context.Background() - configureUTC() v2Logger := v2log.New() v2Logger.InfoContext(ctx, "v2log: gitlab-sshd starting up...") command.CheckForVersionFlag(os.Args, Version, BuildTime) -- GitLab