refactor: dependency injection instead of package globals (#57)

* refactor: export Settings type, Setup returns value * refactor: resolver.Setup takes explicit Settings struct * refactor: GetHeadersWithoutTrustedHeaders takes explicit header params * refactor: server constructors take narrow config * refactor: Router struct with handler methods, remove geoSvc global * refactor: wire DI through main, remove setting.App references * refactor: remove App global, use returned Settings * refactor: update router tests for DI * chore: fix lint issues — rename ServerTimeouts to Timeouts, fix shadowed variables
2026-07-23 22:45:46 +00:00 · 2026-07-21 18:37:21 +02:00
parent 3874f6af3f
commit 2e32a20f60
19 changed files with 298 additions and 253 deletions
@@ -7,7 +7,6 @@ import (
 	"sort"
 	"strings"

-	"github.com/dcarrillo/whatismyip/internal/setting"
 	"github.com/gin-gonic/gin"
 )

@@ -30,10 +29,10 @@ func HeadersToSortedString(headers http.Header) string {
 }

 // GetHeadersWithoutTrustedHeaders returns a copy of the request headers with the trusted headers removed
-func GetHeadersWithoutTrustedHeaders(ctx *gin.Context) http.Header {
+func GetHeadersWithoutTrustedHeaders(ctx *gin.Context, trustedHeader, trustedPortHeader string) http.Header {
 	h := ctx.Request.Header.Clone()

-	for _, k := range []string{setting.App.TrustedHeader, setting.App.TrustedPortHeader} {
+	for _, k := range []string{trustedHeader, trustedPortHeader} {
 		delete(h, textproto.CanonicalMIMEHeaderKey(k))
 	}

@@ -29,7 +29,7 @@ type resolver struct {
 	Ipv6            []string `yaml:"ipv6,omitempty"`
 }

-type settings struct {
+type Settings struct {
 	GeodbPath           geodbConf
 	TemplatePath        string
 	BindAddress         string
@@ -51,75 +51,72 @@ const defaultAddress = ":8080"

 var ErrVersion = errors.New("setting: version requested")

-var App = settings{
-	// hard-coded for the time being
-	Server: serverSettings{
-		ReadTimeout:  10 * time.Second,
-		WriteTimeout: 10 * time.Second,
-	},
-}
-
-func Setup(args []string) (output string, err error) {
+func Setup(args []string) (cfg Settings, output string, err error) {
 	flags := flag.NewFlagSet("whatismyip", flag.ContinueOnError)
 	var buf bytes.Buffer
 	var resolverConf string
 	flags.SetOutput(&buf)

-	flags.StringVar(&App.GeodbPath.City, "geoip2-city", "", "Path to GeoIP2 city database. Enables geo information (--geoip2-asn becomes mandatory)")
-	flags.StringVar(&App.GeodbPath.ASN, "geoip2-asn", "", "Path to GeoIP2 ASN database. Enables ASN information. (--geoip2-city becomes mandatory)")
-	flags.StringVar(&App.TemplatePath, "template", "", "Path to the template file")
+	cfg.Server = serverSettings{
+		ReadTimeout:  10 * time.Second,
+		WriteTimeout: 10 * time.Second,
+	}
+
+	flags.StringVar(&cfg.GeodbPath.City, "geoip2-city", "", "Path to GeoIP2 city database. Enables geo information (--geoip2-asn becomes mandatory)")
+	flags.StringVar(&cfg.GeodbPath.ASN, "geoip2-asn", "", "Path to GeoIP2 ASN database. Enables ASN information. (--geoip2-city becomes mandatory)")
+	flags.StringVar(&cfg.TemplatePath, "template", "", "Path to the template file")
 	flags.StringVar(
 		&resolverConf,
 		"resolver",
 		"",
 		"Path to the resolver configuration. It actually enables the resolver for DNS client discovery.")
 	flags.StringVar(
-		&App.BindAddress,
+		&cfg.BindAddress,
 		"bind",
 		defaultAddress,
 		"Listening address (see https://pkg.go.dev/net?#Listen)",
 	)
 	flags.StringVar(
-		&App.TLSAddress,
+		&cfg.TLSAddress,
 		"tls-bind",
 		"",
 		"Listening address for TLS (see https://pkg.go.dev/net?#Listen)",
 	)
-	flags.StringVar(&App.TLSCrtPath, "tls-crt", "", "When using TLS, path to certificate file")
-	flags.StringVar(&App.TLSKeyPath, "tls-key", "", "When using TLS, path to private key file")
+	flags.StringVar(&cfg.TLSCrtPath, "tls-crt", "", "When using TLS, path to certificate file")
+	flags.StringVar(&cfg.TLSKeyPath, "tls-key", "", "When using TLS, path to private key file")
 	flags.StringVar(
-		&App.PrometheusAddress,
+		&cfg.PrometheusAddress,
 		"metrics-bind",
 		"",
 		"Listening address for Prometheus metrics endpoint (see https://pkg.go.dev/net?#Listen). It enables the metrics available at the given address/port via the /metrics endpoint.",
 	)
 	flags.StringVar(
-		&App.TrustedHeader,
+		&cfg.TrustedHeader,
 		"trusted-header",
 		"",
 		"Trusted request header for remote IP (e.g. X-Real-IP). When using this feature if -trusted-port-header is not set the client port is shown as 'unknown'",
 	)
 	flags.StringVar(
-		&App.TrustedPortHeader,
+		&cfg.TrustedPortHeader,
 		"trusted-port-header",
 		"",
 		"Trusted request header for remote client port (e.g. X-Real-Port). When this parameter is set -trusted-header becomes mandatory",
 	)
-	flags.BoolVar(&App.version, "version", false, "Output version information and exit")
+	flags.BoolVar(&cfg.version, "version", false, "Output version information and exit")
 	flags.BoolVar(
-		&App.EnableSecureHeaders,
+		&cfg.EnableSecureHeaders,
 		"enable-secure-headers",
 		false,
 		"Add sane security-related headers to every response",
 	)
 	flags.BoolVar(
-		&App.EnableHTTP3,
+		&cfg.EnableHTTP3,
 		"enable-http3",
 		false,
 		"Enable HTTP/3 protocol. HTTP/3 requires --tls-bind set, as HTTP/3 starts as a TLS connection that then gets upgraded to UDP. The UDP port is the same as the one used for the TLS server.",
 	)
 	flags.BoolVar(
-		&App.DisableTCPScan,
+		&cfg.DisableTCPScan,
 		"disable-scan",
 		false,
 		"Disable TCP port scanning functionality",
@@ -127,48 +124,48 @@ func Setup(args []string) (output string, err error) {

 	err = flags.Parse(args)
 	if err != nil {
-		return buf.String(), err
+		return cfg, buf.String(), err
 	}

-	if App.version {
-		return fmt.Sprintf("whatismyip version %s", core.Version), ErrVersion
+	if cfg.version {
+		return cfg, fmt.Sprintf("whatismyip version %s", core.Version), ErrVersion
 	}

-	if (App.GeodbPath.City != "" && App.GeodbPath.ASN == "") || (App.GeodbPath.City == "" && App.GeodbPath.ASN != "") {
-		return "", errors.New("both --geoip2-city and --geoip2-asn are mandatory to enable geo information")
+	if (cfg.GeodbPath.City != "" && cfg.GeodbPath.ASN == "") || (cfg.GeodbPath.City == "" && cfg.GeodbPath.ASN != "") {
+		return cfg, "", errors.New("both --geoip2-city and --geoip2-asn are mandatory to enable geo information")
 	}

-	if App.TrustedPortHeader != "" && App.TrustedHeader == "" {
-		return "", errors.New("trusted-header is mandatory when trusted-port-header is set")
+	if cfg.TrustedPortHeader != "" && cfg.TrustedHeader == "" {
+		return cfg, "", errors.New("trusted-header is mandatory when trusted-port-header is set")
 	}

-	if (App.TLSAddress != "") && (App.TLSCrtPath == "" || App.TLSKeyPath == "") {
-		return "", errors.New("in order to use TLS, the -tls-crt and -tls-key flags are mandatory")
+	if (cfg.TLSAddress != "") && (cfg.TLSCrtPath == "" || cfg.TLSKeyPath == "") {
+		return cfg, "", errors.New("in order to use TLS, the -tls-crt and -tls-key flags are mandatory")
 	}

-	if App.EnableHTTP3 && App.TLSAddress == "" {
-		return "", errors.New("in order to use HTTP3, the -tls-bind is mandatory")
+	if cfg.EnableHTTP3 && cfg.TLSAddress == "" {
+		return cfg, "", errors.New("in order to use HTTP3, the -tls-bind is mandatory")
 	}

-	if App.TemplatePath != "" {
-		info, err := os.Stat(App.TemplatePath)
+	if cfg.TemplatePath != "" {
+		info, err := os.Stat(cfg.TemplatePath)
 		if err != nil {
-			return "", fmt.Errorf("template path: %w", err)
+			return cfg, "", fmt.Errorf("template path: %w", err)
 		}
 		if info.IsDir() {
-			return "", fmt.Errorf("%s must be a file", App.TemplatePath)
+			return cfg, "", fmt.Errorf("%s must be a file", cfg.TemplatePath)
 		}
 	}

 	if resolverConf != "" {
 		var err error
-		App.Resolver, err = readYAML(resolverConf)
+		cfg.Resolver, err = readYAML(resolverConf)
 		if err != nil {
-			return "", fmt.Errorf("reading resolver configuration: %w", err)
+			return cfg, "", fmt.Errorf("reading resolver configuration: %w", err)
 		}
 	}

-	return buf.String(), nil
+	return cfg, buf.String(), nil
 }

 func readYAML(path string) (resolver resolver, err error) {
@@ -55,7 +55,7 @@ func TestParseMandatoryFlags(t *testing.T) {

 	for _, tt := range mandatoryFlags {
 		t.Run(strings.Join(tt.args, " "), func(t *testing.T) {
-			_, err := Setup(tt.args)
+			_, _, err := Setup(tt.args)
 			require.Error(t, err)
 			assert.Contains(t, err.Error(), "mandatory")
 		})
@@ -65,11 +65,11 @@ func TestParseMandatoryFlags(t *testing.T) {
 func TestParseFlags(t *testing.T) {
 	flags := []struct {
 		args []string
-		conf settings
+		conf Settings
 	}{
 		{
 			[]string{},
-			settings{
+			Settings{
 				BindAddress: ":8080",
 				Server: serverSettings{
 					ReadTimeout:  10 * time.Second,
@@ -79,7 +79,7 @@ func TestParseFlags(t *testing.T) {
 		},
 		{
 			[]string{"-disable-scan"},
-			settings{
+			Settings{
 				BindAddress: ":8080",
 				Server: serverSettings{
 					ReadTimeout:  10 * time.Second,
@@ -90,7 +90,7 @@ func TestParseFlags(t *testing.T) {
 		},
 		{
 			[]string{"-bind", ":8001", "-geoip2-city", "/city-path", "-geoip2-asn", "/asn-path"},
-			settings{
+			Settings{
 				GeodbPath: geodbConf{
 					City: "/city-path",
 					ASN:  "/asn-path",
@@ -107,7 +107,7 @@ func TestParseFlags(t *testing.T) {
 				"-geoip2-city", "/city-path", "-geoip2-asn", "/asn-path", "-tls-bind", ":9000",
 				"-tls-crt", "/crt-path", "-tls-key", "/key-path",
 			},
-			settings{
+			Settings{
 				GeodbPath: geodbConf{
 					City: "/city-path",
 					ASN:  "/asn-path",
@@ -127,7 +127,7 @@ func TestParseFlags(t *testing.T) {
 				"-geoip2-city", "/city-path", "-geoip2-asn", "/asn-path",
 				"-trusted-header", "header", "-trusted-port-header", "port-header",
 			},
-			settings{
+			Settings{
 				GeodbPath: geodbConf{
 					City: "/city-path",
 					ASN:  "/asn-path",
@@ -146,7 +146,7 @@ func TestParseFlags(t *testing.T) {
 				"-geoip2-city", "/city-path", "-geoip2-asn", "/asn-path",
 				"-trusted-header", "header", "-enable-secure-headers",
 			},
-			settings{
+			Settings{
 				GeodbPath: geodbConf{
 					City: "/city-path",
 					ASN:  "/asn-path",
@@ -164,9 +164,9 @@ func TestParseFlags(t *testing.T) {

 	for _, tt := range flags {
 		t.Run(strings.Join(tt.args, " "), func(t *testing.T) {
-			_, err := Setup(tt.args)
+			cfg, _, err := Setup(tt.args)
 			require.Nil(t, err)
-			assert.True(t, reflect.DeepEqual(App, tt.conf))
+			assert.True(t, reflect.DeepEqual(cfg, tt.conf))
 		})
 	}
 }
@@ -176,7 +176,7 @@ func TestParseFlagsUsage(t *testing.T) {

 	for _, arg := range usageArgs {
 		t.Run(arg, func(t *testing.T) {
-			output, err := Setup([]string{arg})
+			_, output, err := Setup([]string{arg})
 			assert.ErrorIs(t, err, flag.ErrHelp)
 			assert.Contains(t, output, "Usage of")
 		})
@@ -184,7 +184,7 @@ func TestParseFlagsUsage(t *testing.T) {
 }

 func TestParseFlagVersion(t *testing.T) {
-	output, err := Setup([]string{"-version"})
+	_, output, err := Setup([]string{"-version"})
 	assert.ErrorIs(t, err, ErrVersion)
 	assert.Contains(t, output, "whatismyip version")
 }
@@ -209,7 +209,7 @@ func TestParseFlagTemplate(t *testing.T) {

 	for _, tc := range testCases {
 		t.Run(tc.name, func(t *testing.T) {
-			_, err := Setup(tc.flags)
+			_, _, err := Setup(tc.flags)
 			require.Error(t, err)
 			assert.Contains(t, err.Error(), tc.errMsg)
 		})