From bdd174da3c78767a8c80299ea7397238d97394c4 Mon Sep 17 00:00:00 2001 From: Ayman Bagabas Date: Wed, 12 Jul 2023 15:18:18 -0400 Subject: [PATCH] fix,feat: support custom log path and fix logging leak in hooks --- cmd/soft/hook.go | 17 ----------------- cmd/soft/root.go | 26 +++++++++++++++++++++----- cmd/soft/serve.go | 26 ++++++++++++++++++++++++++ server/config/config.go | 4 ++++ server/config/file.go | 2 ++ server/daemon/daemon.go | 1 + server/ssh/ssh.go | 1 + server/web/git.go | 7 ++++++- 8 files changed, 61 insertions(+), 23 deletions(-) diff --git a/cmd/soft/hook.go b/cmd/soft/hook.go index f610065e1..05a3e2786 100644 --- a/cmd/soft/hook.go +++ b/cmd/soft/hook.go @@ -11,7 +11,6 @@ import ( "path/filepath" "strings" - "github.com/charmbracelet/log" "github.com/charmbracelet/soft-serve/server/backend" "github.com/charmbracelet/soft-serve/server/config" "github.com/charmbracelet/soft-serve/server/db" @@ -37,17 +36,6 @@ var ( } ctx = config.WithContext(ctx, cfg) - - logPath := filepath.Join(cfg.DataPath, "log", "hooks.log") - f, err := os.OpenFile(logPath, os.O_APPEND|os.O_CREATE|os.O_WRONLY, 0644) - if err != nil { - return fmt.Errorf("opening file: %w", err) - } - - ctx = context.WithValue(ctx, logFileCtxKey, f) - logger := log.FromContext(ctx) - logger.SetOutput(f) - ctx = log.WithContext(ctx, logger) cmd.SetContext(ctx) db, err := db.Open(ctx, cfg.DB.Driver, cfg.DB.DataSource) if err != nil { @@ -55,17 +43,12 @@ var ( } // Set up the backend - // TODO: support other backends sb := backend.New(ctx, cfg, db) ctx = backend.WithContext(ctx, sb) cmd.SetContext(ctx) return nil }, - PersistentPostRunE: func(cmd *cobra.Command, _ []string) error { - f := cmd.Context().Value(logFileCtxKey).(*os.File) - return f.Close() - }, } hooksRunE = func(cmd *cobra.Command, args []string) error { diff --git a/cmd/soft/root.go b/cmd/soft/root.go index c5facde02..391d09e61 100644 --- a/cmd/soft/root.go +++ b/cmd/soft/root.go @@ -57,20 +57,27 @@ func init() { } func main() { - logger := newDefaultLogger() + logger, f, err := newDefaultLogger() + if err != nil { + log.Errorf("failed to create logger: %v", err) + } + + if f != nil { + defer f.Close() // nolint: errcheck + } // Set global logger log.SetDefault(logger) var opts []maxprocs.Option if config.IsVerbose() { - opts = append(opts, maxprocs.Logger(logger.Debugf)) + opts = append(opts, maxprocs.Logger(log.Debugf)) } // Set the max number of processes to the number of CPUs // This is useful when running soft serve in a container if _, err := maxprocs.Set(opts...); err != nil { - logger.Warn("couldn't set automaxprocs", "error", err) + log.Warn("couldn't set automaxprocs", "error", err) } ctx := log.WithContext(context.Background(), logger) @@ -80,7 +87,7 @@ func main() { } // newDefaultLogger returns a new logger with default settings. -func newDefaultLogger() *log.Logger { +func newDefaultLogger() (*log.Logger, *os.File, error) { dp := config.DataPath() cfg, err := config.ParseConfig(filepath.Join(dp, "config.yaml")) if err != nil { @@ -111,5 +118,14 @@ func newDefaultLogger() *log.Logger { logger.SetFormatter(log.TextFormatter) } - return logger + var f *os.File + if cfg.Log.Path != "" { + f, err = os.OpenFile(cfg.Log.Path, os.O_APPEND|os.O_CREATE|os.O_WRONLY, 0644) + if err != nil { + return nil, nil, err + } + logger.SetOutput(f) + } + + return logger, f, nil } diff --git a/cmd/soft/serve.go b/cmd/soft/serve.go index 050b6f5be..d592f92ac 100644 --- a/cmd/soft/serve.go +++ b/cmd/soft/serve.go @@ -10,15 +10,18 @@ import ( "time" "github.com/charmbracelet/soft-serve/server" + "github.com/charmbracelet/soft-serve/server/backend" "github.com/charmbracelet/soft-serve/server/config" "github.com/charmbracelet/soft-serve/server/db" "github.com/charmbracelet/soft-serve/server/db/migrate" + "github.com/charmbracelet/soft-serve/server/hooks" "github.com/spf13/cobra" ) var ( autoMigrate bool rollback bool + initHooks bool serveCmd = &cobra.Command{ Use: "serve", @@ -69,6 +72,13 @@ var ( return fmt.Errorf("start server: %w", err) } + if initHooks { + be := backend.New(ctx, cfg, db) + if err := initializeHooks(ctx, cfg, be); err != nil { + return fmt.Errorf("initialize hooks: %w", err) + } + } + done := make(chan os.Signal, 1) lch := make(chan error, 1) go func() { @@ -95,5 +105,21 @@ var ( func init() { serveCmd.Flags().BoolVarP(&autoMigrate, "auto-migrate", "", false, "automatically run database migrations") serveCmd.Flags().BoolVarP(&rollback, "rollback", "", false, "rollback the last database migration") + serveCmd.Flags().BoolVarP(&initHooks, "init-hooks", "", false, "initialize the hooks directory and update hooks for all repositories") rootCmd.AddCommand(serveCmd) } + +func initializeHooks(ctx context.Context, cfg *config.Config, be *backend.Backend) error { + repos, err := be.Repositories(ctx) + if err != nil { + return err + } + + for _, repo := range repos { + if err := hooks.GenerateHooks(ctx, cfg, repo.Name()); err != nil { + return err + } + } + + return nil +} diff --git a/server/config/config.go b/server/config/config.go index 4411acfe4..8af97c45e 100644 --- a/server/config/config.go +++ b/server/config/config.go @@ -83,6 +83,10 @@ type LogConfig struct { // Time format for the log `ts` field. // Format must be described in Golang's time format. TimeFormat string `env:"TIME_FORMAT" yaml:"time_format"` + + // Path to a file to write logs to. + // If not set, logs will be written to stderr. + Path string `env:"PATH" yaml:"path"` } // DBConfig is the database connection configuration. diff --git a/server/config/file.go b/server/config/file.go index 6e67d3714..a3b78c016 100644 --- a/server/config/file.go +++ b/server/config/file.go @@ -18,6 +18,8 @@ log: # Time format for the log "timestamp" field. # Should be described in Golang's time format. time_format: "{{ .Log.TimeFormat }}" + # Path to the log file. Leave empty to write to stderr. + #path: "{{ .Log.Path }}" # The SSH server configuration. ssh: diff --git a/server/daemon/daemon.go b/server/daemon/daemon.go index fa0f4db9a..62e395739 100644 --- a/server/daemon/daemon.go +++ b/server/daemon/daemon.go @@ -264,6 +264,7 @@ func (d *GitDaemon) handleClient(conn net.Conn) { "SOFT_SERVE_REPO_NAME=" + name, "SOFT_SERVE_REPO_PATH=" + filepath.Join(reposDir, repo), "SOFT_SERVE_HOST=" + host, + "SOFT_SERVE_LOG_PATH=" + filepath.Join(d.cfg.DataPath, "log", "hooks.log"), } // Add git protocol environment variable. diff --git a/server/ssh/ssh.go b/server/ssh/ssh.go index 3f7ef39e6..c08a19520 100644 --- a/server/ssh/ssh.go +++ b/server/ssh/ssh.go @@ -244,6 +244,7 @@ func (ss *SSHServer) Middleware(cfg *config.Config) wish.Middleware { "SOFT_SERVE_REPO_PATH=" + filepath.Join(reposDir, repo), "SOFT_SERVE_PUBLIC_KEY=" + ak, "SOFT_SERVE_USERNAME=" + s.User(), + "SOFT_SERVE_LOG_PATH=" + filepath.Join(cfg.DataPath, "log", "hooks.log"), } // Add ssh session & config environ diff --git a/server/web/git.go b/server/web/git.go index b975f759e..41171ffa6 100644 --- a/server/web/git.go +++ b/server/web/git.go @@ -224,6 +224,7 @@ func withAccess(fn http.HandlerFunc) http.HandlerFunc { func serviceRpc(w http.ResponseWriter, r *http.Request) { ctx := r.Context() + cfg := config.FromContext(ctx) logger := log.FromContext(ctx) service, dir, repo := git.Service(pat.Param(r, "service")), pat.Param(r, "dir"), pat.Param(r, "repo") @@ -252,7 +253,11 @@ func serviceRpc(w http.ResponseWriter, r *http.Request) { } if len(version) != 0 { - cmd.Env = append(cmd.Env, fmt.Sprintf("GIT_PROTOCOL=%s", version)) + cmd.Env = append(cmd.Env, []string{ + // TODO: add the rest of env vars when we support pushing using http + "SOFT_SERVE_LOG_PATH=" + filepath.Join(cfg.DataPath, "log", "hooks.log"), + fmt.Sprintf("GIT_PROTOCOL=%s", version), + }...) } // Handle gzip encoding