Repository: eko/pihole-exporter

Issue:
command line options missing in version 1.1.0
I'm using pihole_exporter usally as binary with Commandline Options like

/usr/local/bin/pihole-exporter -pihole_hostname=pihole.lan.u-v.de -pihole_port 443 -pihole_protocol https -pihole_password $PASSWORD  -port 9617

this is alse mentioned in the current README.md

Staring with version 1.1.0 I got as result:

flag provided but not defined: -pihole_hostname
Usage of /usr/local/bin/pihole-exporter:
  -debug
        enable debug logging

I'm aware that I can pas the configurtion setting in the environment, but I' not sure if the bove behavior is intended. If it is, the README.md is wrong currently.

Discussion:
@x-stp: 👀

Solution patch:
diff --git a/config/configuration.go b/config/configuration.go
index b2d8678..e7e16e9 100644
--- a/config/configuration.go
+++ b/config/configuration.go
@@ -18,13 +18,10 @@ import (
 
 // Config is the exporter CLI configuration.
 type Config struct {
-	PIHoleProtocol      string `config:"pihole_protocol"`
-	PIHoleHostname      string `config:"pihole_hostname"`
-	PIHolePort          uint16 `config:"pihole_port"`
-	PIHolePassword      string `config:"pihole_password"`
-	BindAddr            string `config:"bind_addr"`
-	Port                uint16 `config:"port"`
-	SkipTLSVerification bool   `config:"skip_tls_verification"`
+	PIHoleProtocol string `config:"pihole_protocol"`
+	PIHoleHostname string `config:"pihole_hostname"`
+	PIHolePort     uint16 `config:"pihole_port"`
+	PIHolePassword string `config:"pihole_password"`
 }
 
 type EnvConfig struct {
@@ -36,6 +33,7 @@ type EnvConfig struct {
 	Port                uint16        `config:"port"`
 	Timeout             time.Duration `config:"timeout"`
 	SkipTLSVerification bool          `config:"skip_tls_verification"`
+	Debug               bool          `config:"debug"`
 }
 
 const (
@@ -50,8 +48,9 @@ func getDefaultEnvConfig() *EnvConfig {
 		PIHolePassword:      []string{},
 		BindAddr:            "0.0.0.0",
 		Port:                9617,
-		Timeout:            DefaultTimeout,
+		Timeout:             DefaultTimeout,
 		SkipTLSVerification: false,
+		Debug:               false,
 	}
 }
 
@@ -82,7 +81,7 @@ func Load() (*EnvConfig, []Config, error) {
 // String implements fmt.Stringer with a modern strings.Builder implementation.
 func (c *Config) String() string {
 	var b strings.Builder
-	b.WriteString(fmt.Sprintf("<Config@%X ", c))
+	b.WriteString(fmt.Sprintf("<Config@%p ", c))
 	ref := reflect.ValueOf(c).Elem()
 	for i := 0; i < ref.NumField(); i++ {
 		tf := ref.Type().Field(i)
diff --git a/internal/pihole/client.go b/internal/pihole/client.go
index 8564b3b..5b8ed87 100644
--- a/internal/pihole/client.go
+++ b/internal/pihole/client.go
@@ -61,11 +61,11 @@ func NewClient(config *config.Config, envConfig *config.EnvConfig) *Client {
 		log.Fatalf("err: couldn't validate passed Config: %v", err)
 	}
 
-	log.Debugf("Creating client with config %+v", config)
+	log.Debugf("Creating client for host %s with protocol %s and port %d", config.PIHoleHostname, config.PIHoleProtocol, config.PIHolePort)
 
 	return &Client{
 		config:    config,
-		apiClient: *NewAPIClient(fmt.Sprintf("%s://%s:%d", config.PIHoleProtocol, config.PIHoleHostname, config.PIHolePort), config.PIHolePassword, envConfig.Timeout, config.SkipTLSVerification),
+		apiClient: *NewAPIClient(fmt.Sprintf("%s://%s:%d", config.PIHoleProtocol, config.PIHoleHostname, config.PIHolePort), config.PIHolePassword, envConfig.Timeout, envConfig.SkipTLSVerification),
 		Status:    make(chan *ClientChannel, 1),
 	}
 }
diff --git a/main.go b/main.go
index 86d753b..8fb8d67 100644
--- a/main.go
+++ b/main.go
@@ -2,10 +2,7 @@ package main
 
 import (
 	"errors"
-	"flag"
 	"net/http"
-	"os"
-	"strings"
 
 	log "github.com/sirupsen/logrus"
 
@@ -16,22 +13,15 @@ import (
 	"github.com/xonvanetta/shutdown/pkg/shutdown"
 )
 
-var (
-	debugFlag = flag.Bool("debug", false, "enable debug logging")
-)
-
 func main() {
-	flag.Parse()
-
-	debugEnv := strings.ToLower(os.Getenv("DEBUG"))
-	debug := *debugFlag || debugEnv == "true" || debugEnv == "1"
-
-	configureLogger(debug)
-
+	log.SetFormatter(&log.TextFormatter{FullTimestamp: true})
 	envConf, clientConfigs, err := config.Load()
 	if err != nil {
 		log.Fatalf("failed to load configuration: %v", err)
 	}
+	if envConf.Debug {
+		log.SetLevel(log.DebugLevel)
+	}
 
 	log.Infof("starting pihole-exporter")
 
@@ -60,16 +50,6 @@ func main() {
 	log.Info("pihole-exporter HTTP server stopped")
 }
 
-// configureLogger sets the global logrus level and formatter.
-func configureLogger(debug bool) {
-	if debug {
-		log.SetLevel(log.DebugLevel)
-	} else {
-		log.SetLevel(log.InfoLevel)
-	}
-	log.SetFormatter(&log.TextFormatter{FullTimestamp: true})
-}
-
 // buildClients constructs a slice of Pi‑hole API clients from configuration.
 func buildClients(clientConfigs []config.Config, envConfig *config.EnvConfig) []*pihole.Client {
 	clients := make([]*pihole.Client, 0, len(clientConfigs))



Test patch:
diff --git a/config/configuration_test.go b/config/configuration_test.go
index 95599b6..5594462 100644
--- a/config/configuration_test.go
+++ b/config/configuration_test.go
@@ -2,7 +2,11 @@ package config
 
 import (
 	"fmt"
+	"os"
+	"os/exec"
+	"reflect"
 	"testing"
+	"time"
 
 	"github.com/stretchr/testify/assert"
 )
@@ -124,3 +128,308 @@ func TestSplitMultipleHostWithMultipleConfigs(t *testing.T) {
 		})
 	}
 }
+
+// Helper function to safely set os.Args for the duration of a test
+func withArgs(args []string, f func()) {
+	originalArgs := os.Args
+	defer func() { os.Args = originalArgs }()
+	os.Args = args
+	f()
+}
+
+// TestZLoadConfigWithFlags tests only the flag-based configuration once
+// This is separate to avoid flag redefinition errors
+// The "Z" prefix ensures this test runs last in alphabetical order
+func TestZLoadConfigWithFlags(t *testing.T) {
+	// Skip unless specifically running just this test
+	if os.Getenv("TEST_SINGLE") != "TestZLoadConfigWithFlags" {
+		t.Skip("Skipping flag-based test unless run in isolation")
+	}
+
+	// Note: The Go flag package only allows flags to be registered once per process.
+	// We can only run one test case with flags per test binary execution.
+	test := struct {
+		name              string
+		args              []string
+		envVars           map[string]string
+		expectedEnvConfig *EnvConfig
+		expectedNumClient int
+		expectedClients   []Config
+		expectError       bool
+	}{
+		name: "Flags only - single host",
+		args: []string{"pihole-exporter",
+			"-pihole_protocol=https",
+			"-pihole_hostname=my.pi.hole",
+			"-pihole_port=443",
+			"-pihole_password=secret",
+			"-bind_addr=127.0.0.1",
+			"-port=9000",
+			"-timeout=10s",
+			"-skip_tls_verification=true",
+			"-debug=true",
+		},
+		envVars: map[string]string{},
+		expectedEnvConfig: &EnvConfig{
+			PIHoleProtocol:      []string{"https"},
+			PIHoleHostname:      []string{"my.pi.hole"},
+			PIHolePort:          []uint16{443},
+			PIHolePassword:      []string{"secret"},
+			BindAddr:            "127.0.0.1",
+			Port:                9000,
+			Timeout:             10 * time.Second,
+			SkipTLSVerification: true,
+			Debug:               true,
+		},
+		expectedNumClient: 1,
+		expectedClients: []Config{
+			{PIHoleProtocol: "https", PIHoleHostname: "my.pi.hole", PIHolePort: 443, PIHolePassword: "secret"},
+		},
+	}
+
+	// Set environment variables
+	for k, v := range test.envVars {
+		t.Setenv(k, v)
+	}
+
+	var loadedEnvConfig *EnvConfig
+	var loadedClientsConfig []Config
+	var err error
+
+	withArgs(test.args, func() {
+		loadedEnvConfig, loadedClientsConfig, err = Load()
+	})
+
+	if test.expectError {
+		if err == nil {
+			t.Errorf("Expected an error, but got nil")
+		}
+		return // Don't proceed with other checks if error was expected
+	}
+	if err != nil {
+		t.Fatalf("Load() returned an unexpected error: %v", err)
+	}
+
+	// Compare EnvConfig
+	if !reflect.DeepEqual(loadedEnvConfig, test.expectedEnvConfig) {
+		t.Errorf("Loaded EnvConfig mismatch:\nGot:  %+v\nWant: %+v", loadedEnvConfig, test.expectedEnvConfig)
+	}
+
+	// Compare Client Configs
+	if len(loadedClientsConfig) != test.expectedNumClient {
+		t.Errorf("Expected %d client configs, got %d", test.expectedNumClient, len(loadedClientsConfig))
+	} else {
+		if !reflect.DeepEqual(loadedClientsConfig, test.expectedClients) {
+			t.Errorf("Loaded client configs mismatch:\nGot:  %+v\nWant: %+v", loadedClientsConfig, test.expectedClients)
+		}
+	}
+}
+
+func TestLoadConfigEnvVars(t *testing.T) {
+	// Skip this test when running the default flag testing
+	if os.Getenv("TEST_FLAGS") != "" {
+		t.Skip("Skipping env vars test when flag tests are running")
+	}
+
+	// This test only uses environment variables, not command-line flags
+	// Set environment variables
+	t.Setenv("PIHOLE_PROTOCOL", "https")
+	t.Setenv("PIHOLE_HOSTNAME", "env.pi.hole")
+	t.Setenv("PIHOLE_PORT", "8443")
+	t.Setenv("PIHOLE_PASSWORD", "env_secret")
+	t.Setenv("BIND_ADDR", "0.0.0.0")
+	t.Setenv("PORT", "9001")
+	t.Setenv("TIMEOUT", "15s")
+	t.Setenv("SKIP_TLS_VERIFICATION", "true")
+	t.Setenv("DEBUG", "true")
+
+	expectedEnvConfig := &EnvConfig{
+		PIHoleProtocol:      []string{"https"},
+		PIHoleHostname:      []string{"env.pi.hole"},
+		PIHolePort:          []uint16{8443},
+		PIHolePassword:      []string{"env_secret"},
+		BindAddr:            "0.0.0.0",
+		Port:                9001,
+		Timeout:             15 * time.Second,
+		SkipTLSVerification: true,
+		Debug:               true,
+	}
+	expectedClients := []Config{
+		{PIHoleProtocol: "https", PIHoleHostname: "env.pi.hole", PIHolePort: 8443, PIHolePassword: "env_secret"},
+	}
+
+	withArgs([]string{"pihole-exporter"}, func() {
+		loadedEnvConfig, loadedClientsConfig, err := Load()
+		if err != nil {
+			t.Fatalf("Load() returned an unexpected error: %v", err)
+		}
+
+		if !reflect.DeepEqual(loadedEnvConfig, expectedEnvConfig) {
+			t.Errorf("Loaded EnvConfig mismatch:\nGot:  %+v\nWant: %+v", loadedEnvConfig, expectedEnvConfig)
+		}
+
+		if len(loadedClientsConfig) != 1 {
+			t.Errorf("Expected 1 client config, got %d", len(loadedClientsConfig))
+		} else if !reflect.DeepEqual(loadedClientsConfig, expectedClients) {
+			t.Errorf("Loaded client configs mismatch:\nGot:  %+v\nWant: %+v", loadedClientsConfig, expectedClients)
+		}
+	})
+}
+
+func TestLoadConfigDefaults(t *testing.T) {
+	// Skip this test when running the default flag testing
+	if os.Getenv("TEST_FLAGS") != "" {
+		t.Skip("Skipping defaults test when flag tests are running")
+	}
+
+	// This test checks the default values when no environment variables or flags are set
+	withArgs([]string{"pihole-exporter"}, func() {
+		// Clear all relevant environment variables
+		os.Unsetenv("PIHOLE_PROTOCOL")
+		os.Unsetenv("PIHOLE_HOSTNAME")
+		os.Unsetenv("PIHOLE_PORT")
+		os.Unsetenv("PIHOLE_PASSWORD")
+		os.Unsetenv("BIND_ADDR")
+		os.Unsetenv("PORT")
+		os.Unsetenv("TIMEOUT")
+		os.Unsetenv("SKIP_TLS_VERIFICATION")
+		os.Unsetenv("DEBUG")
+
+		loadedEnvConfig, loadedClientsConfig, err := Load()
+		if err != nil {
+			t.Fatalf("Load() returned an unexpected error: %v", err)
+		}
+
+		expectedEnvConfig := &EnvConfig{
+			PIHoleProtocol:      []string{"http"},
+			PIHoleHostname:      []string{"127.0.0.1"},
+			PIHolePort:          []uint16{80},
+			PIHolePassword:      []string{},
+			BindAddr:            "0.0.0.0",
+			Port:                9617,
+			Timeout:             5 * time.Second,
+			SkipTLSVerification: false,
+			Debug:               false,
+		}
+
+		expectedClients := []Config{
+			{PIHoleProtocol: "http", PIHoleHostname: "127.0.0.1", PIHolePort: 80, PIHolePassword: ""},
+		}
+
+		if !reflect.DeepEqual(loadedEnvConfig, expectedEnvConfig) {
+			t.Errorf("Loaded EnvConfig mismatch:\nGot:  %+v\nWant: %+v", loadedEnvConfig, expectedEnvConfig)
+		}
+
+		if len(loadedClientsConfig) != 1 {
+			t.Errorf("Expected 1 client config, got %d", len(loadedClientsConfig))
+		} else if !reflect.DeepEqual(loadedClientsConfig, expectedClients) {
+			t.Errorf("Loaded client configs mismatch:\nGot:  %+v\nWant: %+v", loadedClientsConfig, expectedClients)
+		}
+	})
+}
+
+// Remove the old test function that combined everything
+func TestLoadConfig(t *testing.T) {
+	t.Skip("This test uses flags which can cause flag redefinition errors. Use separate test functions instead.")
+}
+
+// TestLoadConfigSingleCase tests the specific flag case from issue #276
+func TestLoadConfigSingleCase(t *testing.T) {
+	// Skip unless specifically running just this test
+	if os.Getenv("TEST_SINGLE") != "TestLoadConfigSingleCase" {
+		t.Skip("Skipping flag-based test unless run in isolation")
+	}
+
+	// This test focuses on the specific flag combination from issue #276
+
+	args := []string{"pihole-exporter",
+		"-pihole_hostname=pi.hole",
+		"-pihole_port=80",
+	}
+
+	// Clear environment variables to ensure we're only testing flag behavior
+	os.Unsetenv("PIHOLE_PROTOCOL")
+	os.Unsetenv("PIHOLE_HOSTNAME")
+	os.Unsetenv("PIHOLE_PORT")
+	os.Unsetenv("PIHOLE_PASSWORD")
+
+	var loadedEnvConfig *EnvConfig
+	var loadedClientsConfig []Config
+	var err error
+
+	withArgs(args, func() {
+		loadedEnvConfig, loadedClientsConfig, err = Load()
+	})
+
+	if err != nil {
+		t.Fatalf("Load() returned an unexpected error: %v", err)
+	}
+
+	// Verify hostname and port were properly loaded from flags
+	if len(loadedEnvConfig.PIHoleHostname) != 1 || loadedEnvConfig.PIHoleHostname[0] != "pi.hole" {
+		t.Errorf("PIHoleHostname not properly loaded from flags, got: %v", loadedEnvConfig.PIHoleHostname)
+	}
+
+	if len(loadedEnvConfig.PIHolePort) != 1 || loadedEnvConfig.PIHolePort[0] != 80 {
+		t.Errorf("PIHolePort not properly loaded from flags, got: %v", loadedEnvConfig.PIHolePort)
+	}
+
+	// Verify client config
+	expected := Config{
+		PIHoleProtocol: "http",
+		PIHoleHostname: "pi.hole",
+		PIHolePort:     80,
+		PIHolePassword: "",
+	}
+
+	if len(loadedClientsConfig) != 1 || !reflect.DeepEqual(loadedClientsConfig[0], expected) {
+		t.Errorf("Client config not properly generated, got: %+v", loadedClientsConfig)
+	}
+}
+
+// TestMain controls the execution of all tests in this package
+func TestMain(m *testing.M) {
+	// The issue with flag tests in Go is that once a flag is defined,
+	// it can't be redefined in the same process. This makes testing
+	// code that uses flag.Parse() challenging.
+
+	// First, check if we're being run to test a single specific test
+	if name := os.Getenv("TEST_SINGLE"); name != "" {
+		if os.Getenv("TEST_FLAGS") != "" {
+			// Run a specific flag test
+			if name == "TestLoadConfigSingleCase" {
+				// Just run the single case
+				result := m.Run()
+				os.Exit(result)
+			}
+		}
+
+		// For other cases, follow normal test flow
+	}
+
+	// By default, run just the non-flag tests
+	_, isNotTestRun := os.LookupEnv("RUNNING_ALL_TESTS")
+	if !isNotTestRun {
+		// We're in the main test process
+		os.Setenv("RUNNING_ALL_TESTS", "1")
+
+		// Run specific flag tests in isolation
+		fmt.Println("\nRunning flag test for issue #276 in isolation")
+		cmd := exec.Command(os.Args[0], "-test.run=TestLoadConfigSingleCase")
+		cmd.Env = append(os.Environ(), "TEST_SINGLE=TestLoadConfigSingleCase", "TEST_FLAGS=1")
+		output, err := cmd.CombinedOutput()
+		fmt.Println(string(output))
+		if err != nil {
+			fmt.Println("Flag test failed:", err)
+			os.Exit(1)
+		}
+
+		fmt.Println("\nAll tests completed successfully!")
+		os.Exit(0)
+	} else {
+		// This is a regular test run, skip the flag tests
+		fmt.Println("Running non-flag tests")
+		result := m.Run()
+		os.Exit(result)
+	}
+}