diff --git a/client/cmd/debug.go b/client/cmd/debug.go index 2a8cdc88..02a742b2 100644 --- a/client/cmd/debug.go +++ b/client/cmd/debug.go @@ -19,6 +19,7 @@ import ( "github.com/netbirdio/netbird/client/server" mgmProto "github.com/netbirdio/netbird/shared/management/proto" "github.com/netbirdio/netbird/upload-server/types" + "github.com/netbirdio/netbird/version" ) const errCloseConnection = "Failed to close connection: %v" @@ -100,6 +101,7 @@ func debugBundle(cmd *cobra.Command, _ []string) error { Anonymize: anonymizeFlag, SystemInfo: systemInfoFlag, LogFileCount: logFileCount, + CliVersion: version.NetbirdVersion(), } if uploadBundleFlag { request.UploadURL = uploadBundleURLFlag @@ -298,6 +300,7 @@ func runForDuration(cmd *cobra.Command, args []string) error { Anonymize: anonymizeFlag, SystemInfo: systemInfoFlag, LogFileCount: logFileCount, + CliVersion: version.NetbirdVersion(), } if uploadBundleFlag { request.UploadURL = uploadBundleURLFlag @@ -432,6 +435,7 @@ func generateDebugBundle(config *profilemanager.Config, recorder *peer.Status, c SyncResponse: syncResponse, LogPath: logFilePath, CPUProfile: nil, + DaemonVersion: version.NetbirdVersion(), // acting as daemon }, debug.BundleConfig{ IncludeSystemInfo: true, diff --git a/client/cmd/service_controller.go b/client/cmd/service_controller.go index 88121c06..8de14794 100644 --- a/client/cmd/service_controller.go +++ b/client/cmd/service_controller.go @@ -102,7 +102,7 @@ func (p *program) Stop(srv service.Service) error { } // Common setup for service control commands -func setupServiceControlCommand(cmd *cobra.Command, ctx context.Context, cancel context.CancelFunc) (service.Service, error) { +func setupServiceControlCommand(cmd *cobra.Command, ctx context.Context, cancel context.CancelFunc, consoleLog bool) (service.Service, error) { // rootCmd env vars are already applied by PersistentPreRunE. SetFlagsFromEnvVars(serviceCmd) @@ -112,8 +112,14 @@ func setupServiceControlCommand(cmd *cobra.Command, ctx context.Context, cancel return nil, err } - if err := util.InitLog(logLevel, logFiles...); err != nil { - return nil, fmt.Errorf("init log: %w", err) + if consoleLog { + if err := util.InitLog(logLevel, util.LogConsole); err != nil { + return nil, fmt.Errorf("init log: %w", err) + } + } else { + if err := util.InitLog(logLevel, logFiles...); err != nil { + return nil, fmt.Errorf("init log: %w", err) + } } cfg, err := newSVCConfig() @@ -138,7 +144,7 @@ var runCmd = &cobra.Command{ SetupCloseHandler(ctx, cancel) SetupDebugHandler(ctx, nil, nil, nil, util.FindFirstLogPath(logFiles)) - s, err := setupServiceControlCommand(cmd, ctx, cancel) + s, err := setupServiceControlCommand(cmd, ctx, cancel, false) if err != nil { return err } @@ -152,7 +158,7 @@ var startCmd = &cobra.Command{ Short: "starts NetBird service", RunE: func(cmd *cobra.Command, args []string) error { ctx, cancel := context.WithCancel(cmd.Context()) - s, err := setupServiceControlCommand(cmd, ctx, cancel) + s, err := setupServiceControlCommand(cmd, ctx, cancel, false) if err != nil { return err } @@ -170,7 +176,7 @@ var stopCmd = &cobra.Command{ Short: "stops NetBird service", RunE: func(cmd *cobra.Command, args []string) error { ctx, cancel := context.WithCancel(cmd.Context()) - s, err := setupServiceControlCommand(cmd, ctx, cancel) + s, err := setupServiceControlCommand(cmd, ctx, cancel, false) if err != nil { return err } @@ -188,7 +194,7 @@ var restartCmd = &cobra.Command{ Short: "restarts NetBird service", RunE: func(cmd *cobra.Command, args []string) error { ctx, cancel := context.WithCancel(cmd.Context()) - s, err := setupServiceControlCommand(cmd, ctx, cancel) + s, err := setupServiceControlCommand(cmd, ctx, cancel, false) if err != nil { return err } @@ -206,7 +212,7 @@ var svcStatusCmd = &cobra.Command{ Short: "shows NetBird service status", RunE: func(cmd *cobra.Command, args []string) error { ctx, cancel := context.WithCancel(cmd.Context()) - s, err := setupServiceControlCommand(cmd, ctx, cancel) + s, err := setupServiceControlCommand(cmd, ctx, cancel, true) if err != nil { return err } diff --git a/client/internal/debug/debug.go b/client/internal/debug/debug.go index ebaf71b2..5176c17d 100644 --- a/client/internal/debug/debug.go +++ b/client/internal/debug/debug.go @@ -254,6 +254,8 @@ type BundleGenerator struct { capturePath string refreshStatus func() // Optional callback to refresh status before bundle generation clientMetrics MetricsExporter + daemonVersion string + cliVersion string anonymize bool includeSystemInfo bool @@ -278,6 +280,8 @@ type GeneratorDependencies struct { CapturePath string RefreshStatus func() ClientMetrics MetricsExporter + DaemonVersion string + CliVersion string } func NewBundleGenerator(deps GeneratorDependencies, cfg BundleConfig) *BundleGenerator { @@ -299,6 +303,8 @@ func NewBundleGenerator(deps GeneratorDependencies, cfg BundleConfig) *BundleGen capturePath: deps.CapturePath, refreshStatus: deps.RefreshStatus, clientMetrics: deps.ClientMetrics, + daemonVersion: deps.DaemonVersion, + cliVersion: deps.CliVersion, anonymize: cfg.Anonymize, includeSystemInfo: cfg.IncludeSystemInfo, @@ -459,9 +465,11 @@ func (g *BundleGenerator) addStatus() error { protoFullStatus := nbstatus.ToProtoFullStatus(fullStatus) protoFullStatus.Events = g.statusRecorder.GetEventHistory() overview := nbstatus.ConvertToStatusOutputOverview(protoFullStatus, nbstatus.ConvertOptions{ - Anonymize: g.anonymize, - ProfileName: profName, + Anonymize: g.anonymize, + ProfileName: profName, + DaemonVersion: g.daemonVersion, }) + overview.CliVersion = g.cliVersion statusOutput := overview.FullDetailSummary() statusReader := strings.NewReader(statusOutput) @@ -1039,7 +1047,8 @@ func (g *BundleGenerator) addRotatedLogFiles(logDir string) { return } - pattern := filepath.Join(logDir, "client-*.log.gz") + // This regex will match both logs rotated by us and logrotate on linux + pattern := filepath.Join(logDir, "client*.log.*") files, err := filepath.Glob(pattern) if err != nil { log.Warnf("failed to glob rotated logs: %v", err) @@ -1072,7 +1081,12 @@ func (g *BundleGenerator) addRotatedLogFiles(logDir string) { for i := 0; i < maxFiles; i++ { name := filepath.Base(files[i]) - if err := g.addSingleLogFileGz(files[i], name); err != nil { + if strings.HasSuffix(name, ".gz") { + err = g.addSingleLogFileGz(files[i], name) + } else { + err = g.addSingleLogfile(files[i], name) + } + if err != nil { log.Warnf("failed to add rotated log %s: %v", name, err) } } diff --git a/client/internal/debug/debug_logfiles_test.go b/client/internal/debug/debug_logfiles_test.go new file mode 100644 index 00000000..f6473f97 --- /dev/null +++ b/client/internal/debug/debug_logfiles_test.go @@ -0,0 +1,103 @@ +package debug + +import ( + "archive/zip" + "bytes" + "compress/gzip" + "io" + "os" + "path/filepath" + "testing" + "time" + + "github.com/stretchr/testify/require" +) + +// TestAddRotatedLogFiles_PicksUpAllVariants asserts that the rotated-log +// glob picks up logs rotated by timberjack (gzipped) and by logrotate (plain +// and gzipped), and skips unrelated files. +func TestAddRotatedLogFiles_PicksUpAllVariants(t *testing.T) { + dir := t.TempDir() + + writeFile(t, filepath.Join(dir, "client.log"), "active log\n") + writeFile(t, filepath.Join(dir, "other.log"), "unrelated\n") + + timberjackRotated := "client-2026-05-21T10-30-45.000.log.gz" + writeGzFile(t, filepath.Join(dir, timberjackRotated), "timberjack rotated content\n") + + logrotatePlain := "client.log.1" + writeFile(t, filepath.Join(dir, logrotatePlain), "logrotate plain content\n") + + logrotateGz := "client.log.2.gz" + writeGzFile(t, filepath.Join(dir, logrotateGz), "logrotate gz content\n") + + names := runAddRotatedLogFiles(t, dir, 10) + + require.Contains(t, names, timberjackRotated, "timberjack rotated file should be in bundle") + require.Contains(t, names, logrotatePlain, "logrotate plain rotated file should be in bundle") + require.Contains(t, names, logrotateGz, "logrotate gzipped rotated file should be in bundle") + require.NotContains(t, names, "client.log", "active log should not be added by addRotatedLogFiles") + require.NotContains(t, names, "other.log", "unrelated files should not be in bundle") +} + +// TestAddRotatedLogFiles_RespectsLogFileCount asserts that only the newest +// logFileCount rotated files are bundled, ordered by mtime. +func TestAddRotatedLogFiles_RespectsLogFileCount(t *testing.T) { + dir := t.TempDir() + + oldest := filepath.Join(dir, "client.log.3") + middle := filepath.Join(dir, "client.log.2") + newest := filepath.Join(dir, "client.log.1") + writeFile(t, oldest, "old\n") + writeFile(t, middle, "mid\n") + writeFile(t, newest, "new\n") + + now := time.Now() + require.NoError(t, os.Chtimes(oldest, now.Add(-2*time.Hour), now.Add(-2*time.Hour))) + require.NoError(t, os.Chtimes(middle, now.Add(-1*time.Hour), now.Add(-1*time.Hour))) + require.NoError(t, os.Chtimes(newest, now, now)) + + names := runAddRotatedLogFiles(t, dir, 2) + + require.Contains(t, names, "client.log.1") + require.Contains(t, names, "client.log.2") + require.NotContains(t, names, "client.log.3", "oldest file should be dropped when logFileCount=2") +} + +// runAddRotatedLogFiles calls addRotatedLogFiles against a fresh in-memory +// zip writer and returns the set of entry names that ended up in the archive. +func runAddRotatedLogFiles(t *testing.T, dir string, logFileCount uint32) map[string]struct{} { + t.Helper() + + var buf bytes.Buffer + g := &BundleGenerator{ + archive: zip.NewWriter(&buf), + logFileCount: logFileCount, + } + g.addRotatedLogFiles(dir) + require.NoError(t, g.archive.Close()) + + zr, err := zip.NewReader(bytes.NewReader(buf.Bytes()), int64(buf.Len())) + require.NoError(t, err) + + names := make(map[string]struct{}, len(zr.File)) + for _, f := range zr.File { + names[f.Name] = struct{}{} + } + return names +} + +func writeFile(t *testing.T, path, content string) { + t.Helper() + require.NoError(t, os.WriteFile(path, []byte(content), 0o644)) +} + +func writeGzFile(t *testing.T, path, content string) { + t.Helper() + var buf bytes.Buffer + gw := gzip.NewWriter(&buf) + _, err := io.WriteString(gw, content) + require.NoError(t, err) + require.NoError(t, gw.Close()) + require.NoError(t, os.WriteFile(path, buf.Bytes(), 0o644)) +} diff --git a/client/internal/engine.go b/client/internal/engine.go index 1de7164a..98032672 100644 --- a/client/internal/engine.go +++ b/client/internal/engine.go @@ -72,6 +72,7 @@ import ( sProto "github.com/netbirdio/netbird/shared/signal/proto" "github.com/netbirdio/netbird/util" "github.com/netbirdio/netbird/util/capture" + "github.com/netbirdio/netbird/version" ) // PeerConnectionTimeoutMax is a timeout of an initial connection attempt to a remote peer. @@ -1151,6 +1152,7 @@ func (e *Engine) handleBundle(params *mgmProto.BundleParameters) (*mgmProto.JobR LogPath: e.config.LogPath, TempDir: e.config.TempDir, ClientMetrics: e.clientMetrics, + DaemonVersion: version.NetbirdVersion(), RefreshStatus: func() { e.RunHealthProbes(true) }, diff --git a/client/proto/daemon.pb.go b/client/proto/daemon.pb.go index 91a4ec10..79fa1418 100644 --- a/client/proto/daemon.pb.go +++ b/client/proto/daemon.pb.go @@ -2717,6 +2717,7 @@ type DebugBundleRequest struct { SystemInfo bool `protobuf:"varint,3,opt,name=systemInfo,proto3" json:"systemInfo,omitempty"` UploadURL string `protobuf:"bytes,4,opt,name=uploadURL,proto3" json:"uploadURL,omitempty"` LogFileCount uint32 `protobuf:"varint,5,opt,name=logFileCount,proto3" json:"logFileCount,omitempty"` + CliVersion string `protobuf:"bytes,6,opt,name=cliVersion,proto3" json:"cliVersion,omitempty"` unknownFields protoimpl.UnknownFields sizeCache protoimpl.SizeCache } @@ -2779,6 +2780,13 @@ func (x *DebugBundleRequest) GetLogFileCount() uint32 { return 0 } +func (x *DebugBundleRequest) GetCliVersion() string { + if x != nil { + return x.CliVersion + } + return "" +} + type DebugBundleResponse struct { state protoimpl.MessageState `protogen:"open.v1"` Path string `protobuf:"bytes,1,opt,name=path,proto3" json:"path,omitempty"` @@ -6484,14 +6492,17 @@ const file_daemon_proto_rawDesc = "" + "\x12translatedHostname\x18\x04 \x01(\tR\x12translatedHostname\x128\n" + "\x0etranslatedPort\x18\x05 \x01(\v2\x10.daemon.PortInfoR\x0etranslatedPort\"G\n" + "\x17ForwardingRulesResponse\x12,\n" + - "\x05rules\x18\x01 \x03(\v2\x16.daemon.ForwardingRuleR\x05rules\"\x94\x01\n" + + "\x05rules\x18\x01 \x03(\v2\x16.daemon.ForwardingRuleR\x05rules\"\xb4\x01\n" + "\x12DebugBundleRequest\x12\x1c\n" + "\tanonymize\x18\x01 \x01(\bR\tanonymize\x12\x1e\n" + "\n" + "systemInfo\x18\x03 \x01(\bR\n" + "systemInfo\x12\x1c\n" + "\tuploadURL\x18\x04 \x01(\tR\tuploadURL\x12\"\n" + - "\flogFileCount\x18\x05 \x01(\rR\flogFileCount\"}\n" + + "\flogFileCount\x18\x05 \x01(\rR\flogFileCount\x12\x1e\n" + + "\n" + + "cliVersion\x18\x06 \x01(\tR\n" + + "cliVersion\"}\n" + "\x13DebugBundleResponse\x12\x12\n" + "\x04path\x18\x01 \x01(\tR\x04path\x12 \n" + "\vuploadedKey\x18\x02 \x01(\tR\vuploadedKey\x120\n" + diff --git a/client/proto/daemon.proto b/client/proto/daemon.proto index 95260faa..6982e4a1 100644 --- a/client/proto/daemon.proto +++ b/client/proto/daemon.proto @@ -472,6 +472,7 @@ message DebugBundleRequest { bool systemInfo = 3; string uploadURL = 4; uint32 logFileCount = 5; + string cliVersion = 6; } message DebugBundleResponse { diff --git a/client/proto/generate.sh b/client/proto/generate.sh index 21e020ae..1ae55e38 100755 --- a/client/proto/generate.sh +++ b/client/proto/generate.sh @@ -8,7 +8,7 @@ if ! which realpath >/dev/null 2>&1; then fi old_pwd=$(pwd) -script_path=$(dirname $(realpath "$0")) +script_path=$(dirname "$(realpath "$0")") cd "$script_path" go install google.golang.org/protobuf/cmd/protoc-gen-go@v1.36.6 go install google.golang.org/grpc/cmd/protoc-gen-go-grpc@v1.6.1 diff --git a/client/server/debug.go b/client/server/debug.go index 33247db5..14dcaba3 100644 --- a/client/server/debug.go +++ b/client/server/debug.go @@ -14,6 +14,7 @@ import ( "github.com/netbirdio/netbird/client/internal/debug" "github.com/netbirdio/netbird/client/proto" mgmProto "github.com/netbirdio/netbird/shared/management/proto" + "github.com/netbirdio/netbird/version" ) // DebugBundle creates a debug bundle and returns the location. @@ -67,6 +68,8 @@ func (s *Server) DebugBundle(_ context.Context, req *proto.DebugBundleRequest) ( CapturePath: capturePath, RefreshStatus: refreshStatus, ClientMetrics: clientMetrics, + DaemonVersion: version.NetbirdVersion(), + CliVersion: req.CliVersion, }, debug.BundleConfig{ Anonymize: req.GetAnonymize(), diff --git a/client/status/status.go b/client/status/status.go index b9bb86a6..e7e8ee11 100644 --- a/client/status/status.go +++ b/client/status/status.go @@ -549,6 +549,16 @@ func (o *OutputOverview) GeneralSummary(showURL bool, showRelays bool, showNameS goarm = fmt.Sprintf(" (ARMv%s)", os.Getenv("GOARM")) } + daemonVersion := "N/A" + if o.DaemonVersion != "" { + daemonVersion = o.DaemonVersion + } + + cliVersion := version.NetbirdVersion() + if o.CliVersion != "" { + cliVersion = o.CliVersion + } + wgPortString := "N/A" if o.WgPort > 0 { wgPortString = fmt.Sprintf("%d", o.WgPort) @@ -575,8 +585,8 @@ func (o *OutputOverview) GeneralSummary(showURL bool, showRelays bool, showNameS "%s"+ "Peers count: %s\n", fmt.Sprintf("%s/%s%s", goos, goarch, goarm), - o.DaemonVersion, - version.NetbirdVersion(), + daemonVersion, + cliVersion, o.ProfileName, managementConnString, signalConnString, diff --git a/client/ui/debug.go b/client/ui/debug.go index cf5ac1a7..d3d4fa4f 100644 --- a/client/ui/debug.go +++ b/client/ui/debug.go @@ -21,6 +21,7 @@ import ( "github.com/netbirdio/netbird/client/internal" "github.com/netbirdio/netbird/client/proto" uptypes "github.com/netbirdio/netbird/upload-server/types" + "github.com/netbirdio/netbird/version" ) // Initial state for the debug collection @@ -462,6 +463,7 @@ func (s *serviceClient) createDebugBundleFromCollection( request := &proto.DebugBundleRequest{ Anonymize: params.anonymize, SystemInfo: params.systemInfo, + CliVersion: version.NetbirdVersion(), } if params.upload { @@ -593,6 +595,7 @@ func (s *serviceClient) createDebugBundle(anonymize bool, systemInfo bool, uploa request := &proto.DebugBundleRequest{ Anonymize: anonymize, SystemInfo: systemInfo, + CliVersion: version.NetbirdVersion(), } if uploadURL != "" { diff --git a/go.mod b/go.mod index caf9cb68..bafdeaf8 100644 --- a/go.mod +++ b/go.mod @@ -24,13 +24,13 @@ require ( golang.zx2c4.com/wireguard/windows v0.5.3 google.golang.org/grpc v1.80.0 google.golang.org/protobuf v1.36.11 - gopkg.in/natefinch/lumberjack.v2 v2.2.1 ) require ( fyne.io/fyne/v2 v2.7.0 fyne.io/systray v1.12.1-0.20260116214250-81f8e1a496f9 git.sr.ht/~jackmordaunt/go-toast/v2 v2.0.3 + github.com/DeRuina/timberjack v1.4.2 github.com/awnumar/memguard v0.23.0 github.com/aws/aws-sdk-go-v2 v1.38.3 github.com/aws/aws-sdk-go-v2/config v1.31.6 diff --git a/go.sum b/go.sum index 7f008142..2f42f96b 100644 --- a/go.sum +++ b/go.sum @@ -29,6 +29,8 @@ github.com/Azure/go-ntlmssp v0.1.0 h1:DjFo6YtWzNqNvQdrwEyr/e4nhU3vRiwenz5QX7sFz+ github.com/Azure/go-ntlmssp v0.1.0/go.mod h1:NYqdhxd/8aAct/s4qSYZEerdPuH1liG2/X9DiVTbhpk= github.com/BurntSushi/toml v1.5.0 h1:W5quZX/G/csjUnuI8SUYlsHs9M38FC7znL0lIO+DvMg= github.com/BurntSushi/toml v1.5.0/go.mod h1:ukJfTF/6rtPPRCnwkur4qwRxa8vTRFBF0uk2lLoLwho= +github.com/DeRuina/timberjack v1.4.2 h1:4bKlzhKdsR+2oNkgef9mqb4n11ICow8VK88RfzJPzN8= +github.com/DeRuina/timberjack v1.4.2/go.mod h1:RLoeQrwrCGIEF8gO5nV5b/gMD0QIy7bzQhBUgpp1EqE= github.com/Masterminds/goutils v1.1.1 h1:5nUrii3FMTL5diU80unEVvNevw1nH4+ZV4DSLVJLSYI= github.com/Masterminds/goutils v1.1.1/go.mod h1:8cTjp+g8YejhMuvIA5y2vz3BpJxksy863GQaJW2MFNU= github.com/Masterminds/semver/v3 v3.3.0 h1:B8LGeaivUe71a5qox1ICM/JLl0NqZSW5CHyL+hmvYS0= @@ -940,8 +942,6 @@ gopkg.in/go-playground/assert.v1 v1.2.1/go.mod h1:9RXL0bg/zibRAgZUYszZSwO/z8Y/a8 gopkg.in/go-playground/validator.v9 v9.29.1/go.mod h1:+c9/zcJMFNgbLvly1L1V+PpxWdVbfP1avr/N00E2vyQ= gopkg.in/ini.v1 v1.67.0 h1:Dgnx+6+nfE+IfzjUEISNeydPJh9AXNNsWbGP9KzCsOA= gopkg.in/ini.v1 v1.67.0/go.mod h1:pNLf8WUiyNEtQjuu5G5vTm06TEv9tsIgeAvK8hOrP4k= -gopkg.in/natefinch/lumberjack.v2 v2.2.1 h1:bBRl1b0OH9s/DuPhuXpNl+VtCaJXFZ5/uEFST95x9zc= -gopkg.in/natefinch/lumberjack.v2 v2.2.1/go.mod h1:YD8tP3GAjkrDg1eZH7EGmyESg/lsYskCTPBJVb9jqSc= gopkg.in/square/go-jose.v2 v2.6.0 h1:NGk74WTnPKBNUhNzQX7PYcTLUjoq7mzKk2OKbvwk2iI= gopkg.in/square/go-jose.v2 v2.6.0/go.mod h1:M9dMgbHiYLoDGQrXy7OpJDJWiKiU//h+vD76mk0e1AI= gopkg.in/tomb.v1 v1.0.0-20141024135613-dd632973f1e7 h1:uRGJdciOHaEIrze2W8Q3AKkepLTh2hOroT7a+7czfdQ= diff --git a/util/log.go b/util/log.go index b1de2d99..3896ff6b 100644 --- a/util/log.go +++ b/util/log.go @@ -1,15 +1,16 @@ package util import ( + "fmt" "io" "os" "path/filepath" "slices" "strconv" + "github.com/DeRuina/timberjack" log "github.com/sirupsen/logrus" "google.golang.org/grpc/grpclog" - "gopkg.in/natefinch/lumberjack.v2" "github.com/netbirdio/netbird/formatter" ) @@ -37,8 +38,7 @@ func InitLog(logLevel string, logs ...string) error { func InitLogger(logger *log.Logger, logLevel string, logs ...string) error { level, err := log.ParseLevel(logLevel) if err != nil { - logger.Errorf("Failed parsing log-level %s: %s", logLevel, err) - return err + return fmt.Errorf("failed parsing log-level %s: %w", logLevel, err) } var writers []io.Writer logFmt := os.Getenv("NB_LOG_FORMAT") @@ -59,7 +59,11 @@ func InitLogger(logger *log.Logger, logLevel string, logs ...string) error { case "": logger.Warnf("empty log path received: %#v", logPath) default: - writers = append(writers, newRotatedOutput(logPath)) + writer, err := setupLogFile(logPath, isRotationDisabled(logger)) + if err != nil { + return fmt.Errorf("failed setting up log file: %s, %w", logPath, err) + } + writers = append(writers, writer) } } @@ -94,17 +98,43 @@ func FindFirstLogPath(logs []string) string { return "" } +func isRotationDisabled(logger *log.Logger) bool { + v, _ := os.LookupEnv("NB_LOG_DISABLE_ROTATION") + disabled, _ := strconv.ParseBool(v) + if disabled { + logger.Warnf("log rotation is disabled by env flag") + return true + } + conflict, configPath := FindFirstLogrotateConflict() + if conflict { + logger.Warnf("log rotation conflict detected in: %#v, rotation is disabled", configPath) + return true + } + return false +} + +func setupLogFile(logPath string, disableRotation bool) (io.Writer, error) { + if disableRotation { + file, err := os.OpenFile(logPath, os.O_WRONLY|os.O_APPEND|os.O_CREATE, 0600) + if err != nil { + return nil, err + } + return file, nil + } + return newRotatedOutput(logPath), nil +} + func newRotatedOutput(logPath string) io.Writer { maxLogSize := getLogMaxSize() - lumberjackLogger := &lumberjack.Logger{ + timberjackLogger := &timberjack.Logger{ // Log file absolute path, os agnostic - Filename: filepath.ToSlash(logPath), - MaxSize: maxLogSize, // MB - MaxBackups: 10, - MaxAge: 30, // days - Compress: true, + Filename: filepath.ToSlash(logPath), + MaxSize: maxLogSize, // MB + MaxBackups: 10, + MaxAge: 30, // days + Compression: "gzip", } - return lumberjackLogger + return timberjackLogger } func setGRPCLibLogger(logger *log.Logger) { @@ -127,7 +157,7 @@ func getLogMaxSize() int { if sizeVar, ok := os.LookupEnv("NB_LOG_MAX_SIZE_MB"); ok { size, err := strconv.ParseInt(sizeVar, 10, 64) if err != nil { - log.Errorf("Failed parsing log-size %s: %s. Should be just an integer", sizeVar, err) + log.Errorf("failed parsing log-size %s: %s. Should be just an integer", sizeVar, err) return defaultLogSize } diff --git a/util/log_test.go b/util/log_test.go new file mode 100644 index 00000000..e9933e47 --- /dev/null +++ b/util/log_test.go @@ -0,0 +1,96 @@ +package util + +import ( + "io" + "os" + "path/filepath" + "strings" + "testing" + "time" + + log "github.com/sirupsen/logrus" + "github.com/stretchr/testify/require" +) + +// TestSetupLogFile_RotatesOnSize drives >MaxSize bytes through the writer +// returned by setupLogFile and asserts a backup file appears. +func TestSetupLogFile_RotatesOnSize(t *testing.T) { + t.Setenv("NB_LOG_MAX_SIZE_MB", "1") + + dir := t.TempDir() + logPath := filepath.Join(dir, "netbird.log") + + w, err := setupLogFile(logPath, false) + require.NoError(t, err) + t.Cleanup(func() { + if c, ok := w.(io.Closer); ok { + _ = c.Close() + } + }) + + chunk := []byte(strings.Repeat("x", 64*1024) + "\n") + for range 20 { + _, err := w.Write(chunk) + require.NoError(t, err) + } + + info, err := os.Stat(logPath) + require.NoError(t, err) + require.Less(t, info.Size(), int64(1<<20), + "active log should be < 1 MB after rotation, got %d", info.Size()) + + require.Eventually(t, func() bool { + entries, _ := os.ReadDir(dir) + for _, e := range entries { + name := e.Name() + if name == filepath.Base(logPath) { + continue + } + if strings.HasPrefix(name, "netbird-") && strings.HasSuffix(name, ".log.gz") { + return true + } + } + return false + }, 5*time.Second, 50*time.Millisecond, "expected a rotated backup file in %s", dir) +} + +// TestSetupLogFile_RotationDisabled verifies that with rotation off, the file +// grows past MaxSize and no backups are created. +func TestSetupLogFile_RotationDisabled(t *testing.T) { + t.Setenv("NB_LOG_MAX_SIZE_MB", "1") + + dir := t.TempDir() + logPath := filepath.Join(dir, "netbird.log") + + w, err := setupLogFile(logPath, true) + require.NoError(t, err) + + f, ok := w.(*os.File) + require.True(t, ok, "expected plain *os.File when rotation is disabled, got %T", w) + t.Cleanup(func() { _ = f.Close() }) + + chunk := []byte(strings.Repeat("y", 64*1024) + "\n") + for range 20 { + _, err := w.Write(chunk) + require.NoError(t, err) + } + + info, err := os.Stat(logPath) + require.NoError(t, err) + require.GreaterOrEqual(t, info.Size(), int64(1<<20), + "file should exceed MaxSize when rotation is disabled, got %d", info.Size()) + + entries, err := os.ReadDir(dir) + require.NoError(t, err) + require.Len(t, entries, 1, "no backup files should exist when rotation is disabled, got %v", entries) +} + +// TestIsRotationDisabled_EnvFlag covers the NB_LOG_DISABLE_ROTATION env path. +// The logrotate-conflict branch is exercised separately on linux. +func TestIsRotationDisabled_EnvFlag(t *testing.T) { + logger := log.New() + logger.SetOutput(io.Discard) + + t.Setenv("NB_LOG_DISABLE_ROTATION", "true") + require.True(t, isRotationDisabled(logger)) +} diff --git a/util/logrotate_linux.go b/util/logrotate_linux.go new file mode 100644 index 00000000..7d2173ea --- /dev/null +++ b/util/logrotate_linux.go @@ -0,0 +1,93 @@ +//go:build linux + +package util + +import ( + "bufio" + "errors" + "io/fs" + "os" + "path/filepath" + "strings" + + log "github.com/sirupsen/logrus" +) + +const ( + defaultLogrotateConfPath = "/etc/logrotate.conf" + defaultLogrotateConfDir = "/etc/logrotate.d" + netbirdString = "netbird" +) + +// FindLogrotateConflicts scans the standard logrotate locations for +// indications of conflict with netbird. It returns true and the config file +// path if a conflict was found. +func FindFirstLogrotateConflict() (bool, string) { + return findFirstLogrotateConflictIn(defaultLogrotateConfPath, defaultLogrotateConfDir) +} + +func findFirstLogrotateConflictIn(confPath, confDir string) (bool, string) { + for _, f := range listLogrotateConfigs(confPath, confDir) { + present, err := scanLogrotateFile(f, netbirdString) + if err != nil { + if !errors.Is(err, fs.ErrNotExist) { + log.Debugf("scan %s: %v", f, err) + } + continue + } + if present { + return present, f + } + } + return false, "" +} + +// listLogrotateConfigs returns all config files for logrotate. +func listLogrotateConfigs(confPath, confDir string) []string { + files := []string{confPath} + entries, err := os.ReadDir(confDir) + if err != nil { + return files + } + for _, e := range entries { + if e.IsDir() { + continue + } + files = append(files, filepath.Join(confDir, e.Name())) + } + return files +} + +// scanLogrotateFile reads a config and reports if a non-comment line +// contains the given substring. +func scanLogrotateFile(path string, substring string) (bool, error) { + f, err := os.Open(path) + if err != nil { + return false, err + } + defer func() { + if err := f.Close(); err != nil { + log.Debugf("close %s: %v", path, err) + } + }() + + scanner := bufio.NewScanner(f) + for scanner.Scan() { + line := strings.TrimSpace(stripLogrotateComment(scanner.Text())) + if line == "" { + continue + } + if strings.Contains(line, substring) { + return true, nil + } + } + if err := scanner.Err(); err != nil { + return false, err + } + return false, nil +} + +func stripLogrotateComment(line string) string { + before, _, _ := strings.Cut(line, "#") + return before +} diff --git a/util/logrotate_linux_test.go b/util/logrotate_linux_test.go new file mode 100644 index 00000000..92d7e410 --- /dev/null +++ b/util/logrotate_linux_test.go @@ -0,0 +1,95 @@ +//go:build linux + +package util + +import ( + "os" + "path/filepath" + "testing" + + "github.com/stretchr/testify/require" +) + +func TestFindFirstLogrotateConflict(t *testing.T) { + t.Run("conflict in confDir", func(t *testing.T) { + confPath, confDir := newLogrotateLayout(t) + conflictPath := filepath.Join(confDir, "netbird") + writeLogrotateConfig(t, conflictPath, `/var/log/netbird/*.log { + daily + rotate 7 +}`) + writeLogrotateConfig(t, filepath.Join(confDir, "nginx"), `/var/log/nginx/*.log { daily }`) + + got, path := findFirstLogrotateConflictIn(confPath, confDir) + require.True(t, got) + require.Equal(t, conflictPath, path) + }) + + t.Run("conflict in main conf file", func(t *testing.T) { + confPath, confDir := newLogrotateLayout(t) + writeLogrotateConfig(t, confPath, `weekly +rotate 4 +include /etc/logrotate.d +/var/log/netbird/client.log { rotate 5 }`) + + got, path := findFirstLogrotateConflictIn(confPath, confDir) + require.True(t, got) + require.Equal(t, confPath, path) + }) + + t.Run("no conflict when netbird is absent", func(t *testing.T) { + confPath, confDir := newLogrotateLayout(t) + writeLogrotateConfig(t, filepath.Join(confDir, "nginx"), `/var/log/nginx/*.log { daily }`) + writeLogrotateConfig(t, filepath.Join(confDir, "syslog"), `/var/log/syslog { weekly }`) + + got, path := findFirstLogrotateConflictIn(confPath, confDir) + require.False(t, got) + require.Empty(t, path) + }) + + t.Run("commented-out netbird line is ignored", func(t *testing.T) { + confPath, confDir := newLogrotateLayout(t) + writeLogrotateConfig(t, filepath.Join(confDir, "misc"), `# /var/log/netbird/*.log { daily } +/var/log/other.log { weekly }`) + + got, path := findFirstLogrotateConflictIn(confPath, confDir) + require.False(t, got) + require.Empty(t, path) + }) + + t.Run("subdirectories in confDir are ignored", func(t *testing.T) { + confPath, confDir := newLogrotateLayout(t) + sub := filepath.Join(confDir, "nested") + require.NoError(t, os.MkdirAll(sub, 0o755)) + writeLogrotateConfig(t, filepath.Join(sub, "netbird"), `/var/log/netbird/*.log { daily }`) + + got, path := findFirstLogrotateConflictIn(confPath, confDir) + require.False(t, got) + require.Empty(t, path) + }) + + t.Run("missing paths return no conflict", func(t *testing.T) { + dir := t.TempDir() + got, path := findFirstLogrotateConflictIn( + filepath.Join(dir, "does-not-exist.conf"), + filepath.Join(dir, "does-not-exist.d"), + ) + require.False(t, got) + require.Empty(t, path) + }) +} + +// newLogrotateLayout creates a temp logrotate.conf path and logrotate.d dir, +// returning their paths. The conf file itself is not created. +func newLogrotateLayout(t *testing.T) (confPath, confDir string) { + t.Helper() + root := t.TempDir() + confDir = filepath.Join(root, "logrotate.d") + require.NoError(t, os.MkdirAll(confDir, 0o755)) + return filepath.Join(root, "logrotate.conf"), confDir +} + +func writeLogrotateConfig(t *testing.T, path, body string) { + t.Helper() + require.NoError(t, os.WriteFile(path, []byte(body), 0o644)) +} diff --git a/util/logrotate_nonlinux.go b/util/logrotate_nonlinux.go new file mode 100644 index 00000000..0a188b86 --- /dev/null +++ b/util/logrotate_nonlinux.go @@ -0,0 +1,10 @@ +//go:build !linux + +package util + +// FindLogrotateConflicts scans the standard logrotate locations for +// indications of conflict with netbird. It will always return false for +// non-linux devices. +func FindFirstLogrotateConflict() (bool, string) { + return false, "" +}