From 398f1a7022cd03486502bc2eca9da0629af82d54 Mon Sep 17 00:00:00 2001 From: Conrad Hoffmann Date: Wed, 1 Oct 2025 12:08:54 +0200 Subject: [PATCH] Refactor config loading and server initialization As is, LoadConfig() does some things that are not strictly related to the configuration, such as parsing command line arguments. This has led to a proliferation of different ways to load the config based on various needs and also prevents tools that need a config but are not services to use custom command line arguments. This commit aims to decouple config loading from everything else and do nothing but loading the config files. On a high level, this commit: - renames server.NewServer() to server.New() - moves config.Debug and config.Addr into the server package - moves crypto.InitCrypto() call into server.New() - moves command line parsing into server.New(), using passed-in values rather than os.Args The only changes required for services would be changing cfg := config.LoadConfig(":5100") server := server.NewServer("meta.sr.ht", cfg) to cfg := config.LoadConfig() server := server.New("meta.sr.ht", ":5100", cfg, os.Args) All other tools will be switched to just LoadConfig() and, optionally, a call to crypto.InitCrypto(). I managed to completely remove some global state (addr) and at least make the rest private, so that users are forced to use the designated functions. The config module gained support for custom FS implementation, mainly for testing. --- auth/middleware_test.go | 20 ++++++-------- cmd/token/main.go | 2 +- config/config.go | 61 ++++++++++++++++------------------------- server/email.go | 6 ++-- server/server.go | 39 +++++++++++++++++++++----- 5 files changed, 68 insertions(+), 60 deletions(-) diff --git a/auth/middleware_test.go b/auth/middleware_test.go index 2289ee9a1b4737c0b63125f9e4d7c7cc191fedde..d07a912bb2fccd660b947cb87a9d44fd6ce947da 100644 --- a/auth/middleware_test.go +++ b/auth/middleware_test.go @@ -3,10 +3,10 @@ package auth import ( "context" "encoding/json" - "net" "net/http" "strings" "testing" + "testing/fstest" "time" "github.com/DATA-DOG/go-sqlmock" @@ -154,24 +154,20 @@ func TestInternal(t *testing.T) { var conf ini.File func init() { - var err error - conf, err = ini.Load(strings.NewReader(` + mapfs := fstest.MapFS{ + "config.ini": { + Data: []byte(` [webhooks] private-key=ebzsjPaN6E13ln/FeNWly1C92q6bVMVdOnDo1HPl5fc= [sr.ht] network-key=tbuG-7Vh44vrDq1L_HKWkHnWrDOtJhEkPKPiauaLeuk= -internal-ipnet=127.0.0.1/24,::1/64`)) - if err != nil { - panic(err) +internal-ipnet=127.0.0.1/24,::1/64`), + }, } - crypto.InitCrypto(conf) - // This doesn't get populated because we can't use config.LoadFile - _, local4, _ := net.ParseCIDR("127.0.0.1/8") - _, local6, _ := net.ParseCIDR("::1/64") - config.InternalIPNet = append(config.InternalIPNet, *local4) - config.InternalIPNet = append(config.InternalIPNet, *local6) + config.FS = mapfs + conf = config.LoadConfig() } func middleware() (http.HandlerFunc, *context.Context, *bool) { diff --git a/cmd/token/main.go b/cmd/token/main.go index e84a73b6b63abfa0ed7c2e7b7d393ffc58affde3..c1500153220d5c57b9835a12c93689ec2c6df933 100644 --- a/cmd/token/main.go +++ b/cmd/token/main.go @@ -10,7 +10,7 @@ import ( ) func main() { - conf := config.LoadConfig(":1111") + conf := config.LoadConfig() crypto.InitCrypto(conf) tok := auth.DecodeBearerToken(os.Args[1]) fmt.Printf("%+v\n", tok) diff --git a/config/config.go b/config/config.go index 495213bf09e6b6843a077c4a61e5a104a37af60e..00beea948b46f710bd103205b32d35516645a032 100644 --- a/config/config.go +++ b/config/config.go @@ -2,6 +2,8 @@ package config import ( "fmt" + "io" + "io/fs" "log" "net" "os" @@ -9,20 +11,24 @@ import ( "strconv" "strings" - "git.sr.ht/~sircmpwn/getopt" "github.com/vaughan0/go-ini" - - "git.sr.ht/~sircmpwn/core-go/crypto" ) +// osFS implement the fs.FS interface for the host OS filesystem. +type osFS struct{} + +func (osFS) Open(name string) (fs.File, error) { return os.Open(filepath.FromSlash(name)) } + +func (osFS) Glob(pattern string) ([]string, error) { return filepath.Glob(filepath.FromSlash(pattern)) } + var ( - Debug bool - Addr string - InternalIPNet []net.IPNet + // FS is the filesystem to operate on. Can be exchanged for testing or embedding. + FS fs.GlobFS = osFS{} + internalIPNet []net.IPNet ) -// Just loads the config files -func LoadFiles() ini.File { +// Loads the application configuration. +func LoadConfig() ini.File { var ( config ini.File err error @@ -35,15 +41,20 @@ func LoadFiles() ini.File { "/etc/sr.ht/config.ini", "/etc/sr.ht/*.ini", } { - matches, err_ := filepath.Glob(path) + matches, err_ := FS.Glob(path) if err_ != nil { panic(err) // only happens on bad input } for _, f := range matches { + var r io.Reader + r, err = FS.Open(f) + if err != nil { + break + } if config == nil { - config, err = ini.LoadFile(f) + config, err = ini.Load(r) } else { - err = config.LoadFile(f) + err = config.Load(r) } if err != nil { break @@ -56,30 +67,6 @@ func LoadFiles() ini.File { if err != nil { log.Fatalf("Failed to load config file: %v", err) } - return config -} - -// Loads the application configuration, reads options from the command line, -// and initializes some internals based on these results. -func LoadConfig(defaultAddr string) ini.File { - Addr = defaultAddr - - opts, _, err := getopt.Getopts(os.Args, "b:d") - if err != nil { - panic(err) - } - - for _, opt := range opts { - switch opt.Option { - case 'b': - Addr = opt.Value - case 'd': - Debug = true - } - } - - config := LoadFiles() - crypto.InitCrypto(config) nets, ok := config.Get("sr.ht", "internal-ipnet") if !ok { @@ -92,7 +79,7 @@ func LoadConfig(defaultAddr string) ini.File { if err != nil { panic(fmt.Errorf("[sr.ht]internal-ipnet: %w", err)) } - InternalIPNet = append(InternalIPNet, *net) + internalIPNet = append(internalIPNet, *net) } return config @@ -185,7 +172,7 @@ func GetBool(conf ini.File, section, name string, defValue bool) bool { // Returns true if the given IP address is part of the internal networks as // per the configuration of [sr.ht]internal-ipnet. func IsInternalIP(ip net.IP) bool { - for _, net := range InternalIPNet { + for _, net := range internalIPNet { if net.Contains(ip) { return true } diff --git a/server/email.go b/server/email.go index 4f4ebf5efb71ac06d0b6550af5c785805c5cb2ee..67d2c4a17563364da2bd42c5bff4e8522cdcdc10 100644 --- a/server/email.go +++ b/server/email.go @@ -7,7 +7,7 @@ import ( "fmt" "io" "log" - "runtime/debug" + rtdebug "runtime/debug" "strings" "github.com/99designs/gqlgen/graphql" @@ -41,9 +41,9 @@ func EmailRecover(ctx context.Context, _origErr interface{}) error { return origErr } - stack := string(debug.Stack()) + stack := string(rtdebug.Stack()) log.Println(stack) - if config.Debug { + if debug { return fmt.Errorf("internal system error") } diff --git a/server/server.go b/server/server.go index 4b80d302f0ead74d1a0add3ba0aec203b766cfef..f92d70b92abcfdd820c59977ee06ef5b2c04baa2 100644 --- a/server/server.go +++ b/server/server.go @@ -16,6 +16,7 @@ import ( "time" work "git.sr.ht/~sircmpwn/dowork" + "git.sr.ht/~sircmpwn/getopt" "github.com/99designs/gqlgen/graphql" "github.com/99designs/gqlgen/graphql/handler" "github.com/99designs/gqlgen/graphql/handler/extension" @@ -42,6 +43,8 @@ import ( ) var ( + debug bool + requestsProcessed = promauto.NewCounter(prometheus.CounterOpts{ Name: "api_requests_processed_total", Help: "Total number of API requests processed", @@ -56,6 +59,7 @@ var ( type Server struct { Schema graphql.ExecutableSchema + addr string conf ini.File db *sql.DB redis goRedis.UniversalClient @@ -68,10 +72,31 @@ type Server struct { MaxComplexity int } -// Creates a new common server context for a SourceHut GraphQL daemon. -func NewServer(service string, conf ini.File) *Server { +// New creates a new common server context for a SourceHut GraphQL daemon. It +// parses command line arguments and uses the provided configuration for setting +// up the server and initializing the [crypto] subsystem. +func New(service, defaultAddr string, conf ini.File, args []string) *Server { + addr := defaultAddr + + opts, _, err := getopt.Getopts(args, "b:d") + if err != nil { + panic(err) + } + + for _, opt := range opts { + switch opt.Option { + case 'b': + addr = opt.Value + case 'd': + debug = true + } + } + + crypto.InitCrypto(conf) + root := chi.NewRouter() server := &Server{ + addr: addr, conf: conf, root: root, router: root.Group(func(_ chi.Router) {}), @@ -121,7 +146,7 @@ func (server *Server) WithSchema( MaxUploadSize: 1073741824, // 1 GiB (TODO: configurable?) }) - if config.Debug { + if debug { srv.AroundOperations(func(ctx context.Context, next graphql.OperationHandler) graphql.ResponseHandler { oc := graphql.GetOperationContext(ctx) log.Printf("%s %s", oc.OperationName, oc.RawQuery) @@ -135,7 +160,7 @@ func (server *Server) WithSchema( server.root.Group(func(r chi.Router) { r.Use(middleware.RealIP) - if config.Debug { + if debug { r.Use(middleware.Logger) play := playground.Handler("GraphQL playground", "/query") @@ -239,7 +264,7 @@ func (server *Server) WithDefaultMiddleware() *Server { server.router.Use(redis.Middleware(rc)) server.router.Use(auth.Middleware(server.conf, apiconf)) server.router.Use(middleware.RealIP) - if config.Debug { + if debug { server.router.Use(middleware.Logger) } server.router.Use(middleware.Timeout(timeout)) @@ -308,11 +333,11 @@ func (server *Server) WithQueues(queues ...*work.Queue) *Server { // Run the server. Blocks until SIGINT is received. func (server *Server) Run() { - qlisten, err := reuseport.Listen("tcp", config.Addr) + qlisten, err := reuseport.Listen("tcp", server.addr) if err != nil { panic(err) } - log.Printf("Running on %s", config.Addr) + log.Printf("Running on %s", server.addr) qserver := &http.Server{Handler: server.root} go qserver.Serve(qlisten)