refactor: move entrypoint to cmd/pop and convert root to library package

.gitignore

@@ -33,3 +33,6 @@ version.go
 
 # Ignore output dir
 out
+
+# Local Go build cache used by sandboxed verification
+.gocache/

LIBRARY-REFACTOR.md

@@ -0,0 +1,54 @@
+# Refactor `dnstapir-pop` Into `cmd/pop` + Root `pop` Package
+
+## Summary
+
+Move the executable entrypoint to `cmd/pop` and convert the repo root into package `pop`. The root package exposes a library-style `Run` API, while `cmd/pop/main.go` owns process concerns such as signals, stderr reporting, and exit codes.
+
+## Public API And Layout
+
+- Keep functionality in the repo root as `package pop`; do not create a `/pop` subdirectory.
+- Add:
+  ```go
+  type RunOptions struct {
+      Name, Version, Commit string
+      Args []string
+      Stdout, Stderr io.Writer
+      Reload <-chan struct{}
+  }
+
+  func Run(ctx context.Context, opts RunOptions) error
+  ```
+- `cmd/pop/main.go` defines the ldflag-backed `name`, `version`, and `commit` vars, installs SIGINT/SIGTERM context cancellation, converts SIGHUP into `RunOptions.Reload`, calls `pop.Run`, and uses `os.Exit(1)` only on returned errors.
+- Remove root globals used only by the old executable path: `Gconfig`, `mqttclientid`, and `POPExiter`.
+
+## Implementation Changes
+
+- Rename all root Go files from `package main` to `package pop`; move the old startup logic out of `main()` into `Run`.
+- Update `Makefile` so `make build` runs `go build ... -o out/dnstapir-pop ./cmd/pop`; keep package/install artifacts and binary name unchanged.
+- Replace process exits in `pop` with returned errors:
+  - `SetupLogging`, config validation/loading, MQTT setup/start, source parsing, output parsing, bootstrap setup, and policy parsing return contextual errors.
+  - `log.Fatal`, `os.Exit`, `panic`, `log.Panicf`, and `POPExiter` disappear from package `pop` for normal failure paths.
+- Convert long-running workers to context/error style:
+  - `DnsEngine(ctx, *Config) error`
+  - `APIhandler(ctx, *Config) error`
+  - `ConfigUpdater(ctx, *Config) error`
+  - `StatusUpdater(ctx, *Config) error`
+  - `RefreshEngine(ctx, *Config) error`
+- `Run` starts workers, watches worker errors, context cancellation, API stop requests, and reload events; it saves the RPZ serial before returning.
+- Remove `MqttEngine.SetupInterruptHandler()` from the library path; shutdown should flow through `Run` and `StopEngine`.
+- Update policy/RPZ helper signatures where needed so invalid list/zone formats return errors instead of exiting, and propagate those errors through RPZ generation.
+- Fix the current vet-blocking format string issues encountered in `configupdater.go`, `refreshengine.go`, and `statusupdater.go` as part of making `go test ./...` meaningful after the refactor.
+
+## Test Plan
+
+- Run `gofmt` on touched Go files.
+- Run `go list ./...` and confirm packages include `dnstapir-pop` and `dnstapir-pop/cmd/pop`.
+- Run `go test ./...`; expected target is green after the pre-existing vet issues are fixed.
+- Run `make build` and confirm `out/dnstapir-pop` is produced with ldflag metadata wired through `cmd/pop`.
+- Verify with search that `POPExiter`, root-package `os.Exit`, `log.Fatal`, and normal-error `panic` usages are gone.
+
+## Assumptions
+
+- The official library entrypoint is `pop.Run`; existing exported helper names can remain, but signatures may gain `context.Context` or `error` to satisfy the strict library API.
+- Config file paths, service names, package install paths, and runtime behavior stay the same unless needed to remove process exits.
+- Only `cmd/pop` may terminate the process; package `pop` reports failures to its caller.

Makefile

@@ -8,7 +8,8 @@ OUT:=$$(pwd)/out
 COMMIT:=$$(cat COMMIT 2> /dev/null || git describe --dirty=+WiP --always 2> /dev/null)
 GOFLAGS:=-v -ldflags "-X 'main.version=$(VERSION)' -X 'main.commit=$(COMMIT)' -X 'main.name=$(PROG)'"
 GOOS ?= $(shell uname -s | tr A-Z a-z)
-GO:=GOOS=$(GOOS) GOARCH=$(GOARCH) CGO_ENABLED=0 go
+GOCACHE ?= $(OUT)/.gocache
+GO:=GOOS=$(GOOS) GOARCH=$(GOARCH) CGO_ENABLED=0 GOCACHE=$(GOCACHE) go
 INSTALL:=install -b -c -s -p -m 0755
 
 # For version snapshots of packages
@@ -26,7 +27,7 @@ default: $(PROG)
 $(PROG): build
 
 build: outdir
-	$(GO) build $(GOFLAGS) -o $(OUT)/$(PROG)
+	$(GO) build $(GOFLAGS) -o $(OUT)/$(PROG) ./cmd/pop
 
 outdir:
 	@mkdir -p $(OUT)

apihandler.go

@@ -2,12 +2,14 @@
  * Copyright (c) 2024 Johan Stenstam, johan.stenstam@internetstiftelsen.se
  */
 
-package main
+package pop
 
 import (
+	"context"
 	"crypto/tls"
 	"encoding/gob"
 	"encoding/json"
+	"errors"
 	"fmt"
 	"log"
 	"net/http"
@@ -246,9 +248,9 @@ func APIbootstrap(conf *Config) func(w http.ResponseWriter, r *http.Request) {
 		defer func() {
 			w.Header().Set("Content-Type", "application/json")
 			me := conf.PopData.MqttEngine
-            me.DataMu.Lock() /* Lock because resp.TopicData needs to be accessed safely */
+			me.DataMu.Lock() /* Lock because resp.TopicData needs to be accessed safely */
 			err := json.NewEncoder(w).Encode(resp)
-            me.DataMu.Unlock()
+			me.DataMu.Unlock()
 			if err != nil {
 				log.Printf("Error from json encoder: %v", err)
 				log.Printf("resp: %v", resp)
@@ -270,9 +272,9 @@ func APIbootstrap(conf *Config) func(w http.ResponseWriter, r *http.Request) {
 		switch bp.Command {
 		case "doubtlist-status":
 			me := conf.PopData.MqttEngine
-            me.DataMu.Lock()
+			me.DataMu.Lock()
 			stats := me.Stats()
-            me.DataMu.Unlock()
+			me.DataMu.Unlock()
 			// resp.MsgCounters = stats.MsgCounters
 			// resp.MsgTimeStamps = stats.MsgTimeStamps
 			resp.TopicData = stats
@@ -544,7 +546,7 @@ func SetupBootstrapRouter(conf *Config) *mux.Router {
 	return r
 }
 
-func walkRoutes(router *mux.Router, address string) {
+func walkRoutes(router *mux.Router, address string) error {
 	log.Printf("Defined API endpoints for router on: %s\n", address)
 
 	walker := func(route *mux.Route, router *mux.Router, ancestors []*mux.Route) error {
@@ -556,18 +558,20 @@ func walkRoutes(router *mux.Router, address string) {
 		return nil
 	}
 	if err := router.Walk(walker); err != nil {
-		log.Panicf("Logging err: %s\n", err.Error())
+		return fmt.Errorf("walking routes: %w", err)
 	}
-	//	return nil
+	return nil
 }
 
 // In practice APIdispatcher doesn't need a termination signal, as it will
 // just sit inside http.ListenAndServe, but we keep it for symmetry.
-func APIhandler(conf *Config, done <-chan struct{}) {
+func APIhandler(ctx context.Context, conf *Config) error {
 	gob.Register(tapir.WBGlist{}) // Must register the type for gob encoding
 	router := SetupRouter(conf)
 
-	walkRoutes(router, viper.GetString("apiserver.address"))
+	if err := walkRoutes(router, viper.GetString("apiserver.address")); err != nil {
+		return err
+	}
 	log.Println("")
 
 	addresses := viper.GetStringSlice("apiserver.addresses")
@@ -606,47 +610,70 @@ func APIhandler(conf *Config, done <-chan struct{}) {
 	// tls.RequireAnyClientCert, tls.RequestClientCert, tls.NoClientCert
 
 	if err != nil {
-		POPExiter("Error creating API server tls config: %v\n", err)
+		return fmt.Errorf("error creating API server tls config: %w", err)
 	}
 
 	var wg sync.WaitGroup
+	var serveWG sync.WaitGroup
+	serverErrCh := make(chan error, len(addresses)+len(tlsaddresses)+len(bootstrapaddresses)+len(bootstraptlsaddresses))
+	var servers []*http.Server
+
+	startServer := func(label string, server *http.Server, serve func() error) {
+		servers = append(servers, server)
+		wg.Add(1)
+		serveWG.Add(1)
+		go func() {
+			defer serveWG.Done()
+			log.Printf("*** API: Starting %s. Listening on %s", label, server.Addr)
+			wg.Done()
+			if err := serve(); err != nil && !errors.Is(err, http.ErrServerClosed) {
+				select {
+				case serverErrCh <- fmt.Errorf("%s on %s: %w", label, server.Addr, err):
+				case <-ctx.Done():
+				}
+			}
+		}()
+	}
+
+	shutdownServers := func() {
+		shutdownCtx, cancel := context.WithTimeout(context.Background(), 5*time.Second)
+		defer cancel()
+		for _, server := range servers {
+			if err := server.Shutdown(shutdownCtx); err != nil {
+				log.Printf("APIhandler: error shutting down server on %s: %v", server.Addr, err)
+			}
+		}
+		serveWG.Wait()
+	}
+	defer shutdownServers()
 
 	// log.Println("*** API: Starting API dispatcher #1. Listening on", address)
 
 	if len(addresses) > 0 {
 		for idx, address := range addresses {
-			wg.Add(1)
-			go func(wg *sync.WaitGroup) {
-				apiServer := &http.Server{
-					Addr:         address,
-					Handler:      router,
-					ReadTimeout:  10 * time.Second,
-					WriteTimeout: 10 * time.Second,
-				}
-
-				log.Printf("*** API: Starting API dispatcher #%d. Listening on %s", idx+1, address)
-				wg.Done()
-				POPExiter(apiServer.ListenAndServe())
-			}(&wg)
+			apiServer := &http.Server{
+				Addr:         address,
+				Handler:      router,
+				ReadTimeout:  10 * time.Second,
+				WriteTimeout: 10 * time.Second,
+			}
+			startServer(fmt.Sprintf("API dispatcher #%d", idx+1), apiServer, apiServer.ListenAndServe)
 		}
 	}
 
 	if len(tlsaddresses) > 0 {
 		if tlspossible {
 			for idx, tlsaddress := range tlsaddresses {
-				wg.Add(1)
-				go func(wg *sync.WaitGroup) {
-					tlsServer := &http.Server{
-						Addr:         tlsaddress,
-						Handler:      router,
-						TLSConfig:    tlsConfig,
-						ReadTimeout:  10 * time.Second,
-						WriteTimeout: 10 * time.Second,
-					}
-					log.Printf("*** API: Starting TLS API dispatcher #%d. Listening on %s", idx+1, tlsaddress)
-					wg.Done()
-					POPExiter(tlsServer.ListenAndServeTLS(certfile, keyfile))
-				}(&wg)
+				tlsServer := &http.Server{
+					Addr:         tlsaddress,
+					Handler:      router,
+					TLSConfig:    tlsConfig,
+					ReadTimeout:  10 * time.Second,
+					WriteTimeout: 10 * time.Second,
+				}
+				startServer(fmt.Sprintf("TLS API dispatcher #%d", idx+1), tlsServer, func() error {
+					return tlsServer.ListenAndServeTLS(certfile, keyfile)
+				})
 			}
 		} else {
 			log.Printf("*** API: APIdispatcher: Error: Cannot provide TLS service without cert and key files.\n")
@@ -655,18 +682,13 @@ func APIhandler(conf *Config, done <-chan struct{}) {
 
 	if len(bootstrapaddresses) > 0 {
 		for idx, address := range bootstrapaddresses {
-			wg.Add(1)
-			go func(wg *sync.WaitGroup) {
-				apiServer := &http.Server{
-					Addr:         address,
-					Handler:      bootstraprouter,
-					ReadTimeout:  10 * time.Second,
-					WriteTimeout: 10 * time.Second,
-				}
-				log.Printf("*** API: Starting Bootstrap API dispatcher #%d. Listening on %s", idx+1, address)
-				wg.Done()
-				POPExiter(apiServer.ListenAndServe())
-			}(&wg)
+			apiServer := &http.Server{
+				Addr:         address,
+				Handler:      bootstraprouter,
+				ReadTimeout:  10 * time.Second,
+				WriteTimeout: 10 * time.Second,
+			}
+			startServer(fmt.Sprintf("Bootstrap API dispatcher #%d", idx+1), apiServer, apiServer.ListenAndServe)
 		}
 	} else {
 		log.Println("*** API: No bootstrap address specified")
@@ -675,20 +697,16 @@ func APIhandler(conf *Config, done <-chan struct{}) {
 	if len(bootstraptlsaddresses) > 0 {
 		if tlspossible {
 			for idx, address := range bootstraptlsaddresses {
-				wg.Add(1)
-				go func(wg *sync.WaitGroup) {
-					bootstrapTlsServer := &http.Server{
-						Addr:         address,
-						Handler:      bootstraprouter,
-						TLSConfig:    tlsConfig,
-						ReadTimeout:  10 * time.Second,
-						WriteTimeout: 10 * time.Second,
-					}
-
-					log.Printf("*** API: Starting Bootstrap TLS API dispatcher #%d. Listening on %s", idx+1, address)
-					wg.Done()
-					POPExiter(bootstrapTlsServer.ListenAndServeTLS(certfile, keyfile))
-				}(&wg)
+				bootstrapTlsServer := &http.Server{
+					Addr:         address,
+					Handler:      bootstraprouter,
+					TLSConfig:    tlsConfig,
+					ReadTimeout:  10 * time.Second,
+					WriteTimeout: 10 * time.Second,
+				}
+				startServer(fmt.Sprintf("Bootstrap TLS API dispatcher #%d", idx+1), bootstrapTlsServer, func() error {
+					return bootstrapTlsServer.ListenAndServeTLS(certfile, keyfile)
+				})
 			}
 		} else {
 			log.Printf("*** API: APIdispatcher: Error: Cannot provide Bootstrap TLS service without cert and key files.\n")
@@ -698,7 +716,18 @@ func APIhandler(conf *Config, done <-chan struct{}) {
 	}
 
 	wg.Wait()
-	log.Println("API dispatcher: unclear how to stop the http server nicely.")
+	if len(servers) == 0 {
+		log.Println("API dispatcher: no API servers configured")
+		return nil
+	}
+
+	select {
+	case <-ctx.Done():
+		log.Println("API dispatcher: stopping")
+		return nil
+	case err := <-serverErrCh:
+		return err
+	}
 }
 
 func BumpSerial(conf *Config, zone string) (string, error) {

bootstrap.go

@@ -2,7 +2,7 @@
  * Copyright (c) Johan Stenstam, johan.stenstam@internetstiftelsen.se
  */
 
-package main
+package pop
 
 import (
 	"bytes"
@@ -25,22 +25,17 @@ func (td *PopData) BootstrapMqttSource(src SourceConf) (*tapir.WBGlist, error) {
 		AuthMethod: "X-API-Key",
 	}
 
-	cd := viper.GetString("certs.certdir")
-	if cd == "" {
-		POPExiter("BootstrapMqttSource error: missing config key: certs.certdir")
-	}
-	// cert := cd + "/" + certname
-    key := viper.GetString("certs.tapir-pop.key")
-    cert := viper.GetString("certs.tapir-pop.cert")
+	key := viper.GetString("certs.tapir-pop.key")
+	cert := viper.GetString("certs.tapir-pop.cert")
 	tlsConfig, err := tapir.NewClientConfig(viper.GetString("certs.cacertfile"), key, cert)
 	if err != nil {
-		POPExiter("BootstrapMqttSource: Error: Could not set up TLS: %v", err)
+		return nil, fmt.Errorf("could not set up TLS: %w", err)
 	}
 	// XXX: Need to verify that the server cert is valid for the bootstrap server
 	tlsConfig.InsecureSkipVerify = true
 	err = api.SetupTLS(tlsConfig)
 	if err != nil {
-		POPExiter("BootstrapMqttSource: error setting up TLS for the API client: %v", err)
+		return nil, fmt.Errorf("error setting up TLS for the API client: %w", err)
 	}
 
 	bootstrapaddrs := viper.GetStringSlice("bootstrapserver.addresses")