Add configuration wizard. (#2)

* Add configuration wizard.

internal/forward/manager.go

@@ -9,6 +9,12 @@ import (
 	"github.com/nvm/kportal/internal/config"
 	"github.com/nvm/kportal/internal/healthcheck"
 	"github.com/nvm/kportal/internal/k8s"
+	"github.com/nvm/kportal/internal/logger"
+)
+
+const (
+	healthCheckInterval = 5 * time.Second
+	healthCheckTimeout  = 2 * time.Second
 )
 
 // StatusUpdater is an interface for updating forward status
@@ -34,17 +40,17 @@ type Manager struct {
 }
 
 // NewManager creates a new forward Manager.
-func NewManager(verbose bool) *Manager {
+func NewManager(verbose bool) (*Manager, error) {
 	clientPool, err := k8s.NewClientPool()
 	if err != nil {
-		log.Fatalf("Failed to create client pool: %v", err)
+		return nil, fmt.Errorf("failed to create client pool: %w", err)
 	}
 
 	resolver := k8s.NewResourceResolver(clientPool)
 	portForwarder := k8s.NewPortForwarder(clientPool, resolver)
 
 	// Create health checker: check every 5 seconds with 2 second timeout
-	healthChecker := healthcheck.NewChecker(5*time.Second, 2*time.Second)
+	healthChecker := healthcheck.NewChecker(healthCheckInterval, healthCheckTimeout)
 
 	return &Manager{
 		workers:       make(map[string]*ForwardWorker),
@@ -54,7 +60,7 @@ func NewManager(verbose bool) *Manager {
 		portChecker:   NewPortChecker(),
 		healthChecker: healthChecker,
 		verbose:       verbose,
-	}
+	}, nil
 }
 
 // SetStatusUI sets the status updater for the manager
@@ -93,7 +99,14 @@ func (m *Manager) Start(cfg *config.Config) error {
 
 	for _, fwd := range forwards {
 		if err := m.startWorker(fwd); err != nil {
-			log.Printf("Failed to start worker for %s: %v", fwd.ID(), err)
+			logger.Error("Failed to start worker", map[string]interface{}{
+				"forward_id": fwd.ID(),
+				"context":    fwd.GetContext(),
+				"namespace":  fwd.GetNamespace(),
+				"resource":   fwd.Resource,
+				"local_port": fwd.LocalPort,
+				"error":      err.Error(),
+			})
 			// Continue with other workers
 		}
 	}
@@ -146,7 +159,9 @@ func (m *Manager) Reload(newCfg *config.Config) error {
 		return fmt.Errorf("new configuration is nil")
 	}
 
-	log.Printf("Reloading configuration...")
+	logger.Info("Reloading configuration", map[string]interface{}{
+		"new_forwards_count": len(newCfg.GetAllForwards()),
+	})
 
 	// Get all forwards from new config
 	newForwards := newCfg.GetAllForwards()
@@ -295,8 +310,10 @@ func (m *Manager) stopWorker(id string) error {
 	// Unregister from health checker
 	m.healthChecker.Unregister(id)
 
-	// Note: We DON'T call Remove() here anymore - keep it in the UI
-	// The UI will show it as disabled instead
+	// Notify UI to remove the forward
+	if m.statusUI != nil {
+		m.statusUI.Remove(id)
+	}
 
 	// Stop the worker
 	worker.Stop()

internal/forward/portcheck.go

@@ -8,6 +8,19 @@ import (
 	"strings"
 )
 
+// isValidPID validates that a PID string contains only digits
+func isValidPID(pid string) bool {
+	if len(pid) == 0 || len(pid) > 9 {
+		return false
+	}
+	for _, c := range pid {
+		if c < '0' || c > '9' {
+			return false
+		}
+	}
+	return true
+}
+
 // PortConflict represents a local port that is already in use.
 type PortConflict struct {
 	Port     int    // The conflicting port number
@@ -93,6 +106,10 @@ func (pc *PortChecker) getProcessUsingPortUnix(port int) string {
 	pids := strings.Split(pidStr, "\n")
 	pid := pids[0]
 
+	if !isValidPID(pid) {
+		return "unknown"
+	}
+
 	// Get process name using ps
 	cmd = exec.Command("ps", "-p", pid, "-o", "comm=")
 	output, err = cmd.Output()
@@ -140,6 +157,10 @@ func (pc *PortChecker) getProcessUsingPortWindows(port int) string {
 
 		pid := fields[len(fields)-1]
 
+		if !isValidPID(pid) {
+			return "unknown"
+		}
+
 		// Get process name using tasklist
 		cmd = exec.Command("tasklist", "/FI", fmt.Sprintf("PID eq %s", pid), "/FO", "CSV", "/NH")
 		output, err = cmd.Output()
@@ -188,16 +209,3 @@ func FormatConflicts(conflicts []PortConflict) string {
 
 	return sb.String()
 }
-
-// GetPortsFromForwards extracts all local ports from a list of forward configurations.
-func GetPortsFromForwards(forwards []interface{}) []int {
-	ports := make([]int, 0, len(forwards))
-	for _, fwd := range forwards {
-		// This function expects a generic interface to work with different forward types
-		// The actual implementation should use the Forward struct from config package
-		if f, ok := fwd.(interface{ GetLocalPort() int }); ok {
-			ports = append(ports, f.GetLocalPort())
-		}
-	}
-	return ports
-}

internal/forward/worker.go

@@ -10,9 +10,14 @@ import (
 	"github.com/nvm/kportal/internal/config"
 	"github.com/nvm/kportal/internal/healthcheck"
 	"github.com/nvm/kportal/internal/k8s"
+	"github.com/nvm/kportal/internal/logger"
 	"github.com/nvm/kportal/internal/retry"
 )
 
+const (
+	portForwardReadyTimeout = 30 * time.Second
+)
+
 // ForwardWorker manages a single port-forward connection with automatic retry.
 type ForwardWorker struct {
 	forward       config.Forward
@@ -86,7 +91,13 @@ func (w *ForwardWorker) run() {
 		)
 
 		if err != nil {
-			log.Printf("[%s] Failed to resolve resource: %v", w.forward.ID(), err)
+			logger.Error("Failed to resolve resource", map[string]interface{}{
+				"forward_id": w.forward.ID(),
+				"context":    w.forward.GetContext(),
+				"namespace":  w.forward.GetNamespace(),
+				"resource":   w.forward.Resource,
+				"error":      err.Error(),
+			})
 			w.sleepWithBackoff(backoff)
 			continue
 		}
@@ -96,10 +107,20 @@ func (w *ForwardWorker) run() {
 			if w.healthChecker != nil {
 				w.healthChecker.MarkReconnecting(w.forward.ID())
 			}
-			log.Printf("[%s] Switched to new pod: %s → %s", w.forward.ID(), w.lastPod, podName)
+			logger.Info("Pod restart detected, switching to new pod", map[string]interface{}{
+				"forward_id": w.forward.ID(),
+				"old_pod":    w.lastPod,
+				"new_pod":    podName,
+				"context":    w.forward.GetContext(),
+				"namespace":  w.forward.GetNamespace(),
+			})
 		} else if w.lastPod == "" {
-			log.Printf("[%s] Forwarding %s → localhost:%d",
-				w.forward.ID(), w.forward.String(), w.forward.LocalPort)
+			logger.Info("Starting port forward", map[string]interface{}{
+				"forward_id": w.forward.ID(),
+				"target":     w.forward.String(),
+				"local_port": w.forward.LocalPort,
+				"pod":        podName,
+			})
 			if w.healthChecker != nil {
 				w.healthChecker.MarkStarting(w.forward.ID())
 			}
@@ -123,7 +144,14 @@ func (w *ForwardWorker) run() {
 			}
 
 			// Log the error
-			log.Printf("[%s] Port-forward connection failed: %v", w.forward.ID(), err)
+			logger.Warn("Port-forward connection failed, will retry", map[string]interface{}{
+				"forward_id": w.forward.ID(),
+				"context":    w.forward.GetContext(),
+				"namespace":  w.forward.GetNamespace(),
+				"resource":   w.forward.Resource,
+				"local_port": w.forward.LocalPort,
+				"error":      err.Error(),
+			})
 
 			// Clear last pod so we re-resolve on next attempt
 			w.lastPod = ""
@@ -206,7 +234,7 @@ func (w *ForwardWorker) establishForward(podName string) error {
 		return fmt.Errorf("failed to establish forward: %w", err)
 	case <-w.ctx.Done():
 		return nil
-	case <-time.After(30 * time.Second):
+	case <-time.After(portForwardReadyTimeout):
 		return fmt.Errorf("timeout waiting for port-forward to become ready")
 	}
 

internal/forward/worker_unit_test.go

@@ -0,0 +1,286 @@
+package forward
+
+import (
+	"testing"
+
+	"github.com/nvm/kportal/internal/config"
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+)
+
+func TestLogWriter_Write(t *testing.T) {
+	tests := []struct {
+		name          string
+		prefix        string
+		input         string
+		expectedInLog string
+		description   string
+	}{
+		{
+			name:          "write simple message",
+			prefix:        "[worker] ",
+			input:         "test message",
+			expectedInLog: "[worker] test message",
+			description:   "Should write message with prefix to log",
+		},
+		{
+			name:          "write empty message",
+			prefix:        "[test] ",
+			input:         "",
+			expectedInLog: "[test] ",
+			description:   "Should handle empty message",
+		},
+		{
+			name:          "write multiline message",
+			prefix:        "[fwd] ",
+			input:         "line1\nline2",
+			expectedInLog: "[fwd] line1\nline2",
+			description:   "Should handle multiline messages",
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			// Test logWriter
+			originalWriter := &logWriter{prefix: tt.prefix}
+
+			n, err := originalWriter.Write([]byte(tt.input))
+
+			require.NoError(t, err, "Write should not return error")
+			assert.Equal(t, len(tt.input), n, "Write should return number of bytes written")
+		})
+	}
+}
+
+func TestForwardWorker_GetForward(t *testing.T) {
+	tests := []struct {
+		name        string
+		forward     config.Forward
+		description string
+	}{
+		{
+			name: "get pod forward",
+			forward: config.Forward{
+				Resource:  "pod/my-app",
+				LocalPort: 8080,
+				Port:      80,
+				Protocol:  "tcp",
+			},
+			description: "Should return the forward configuration",
+		},
+		{
+			name: "get service forward",
+			forward: config.Forward{
+				Resource:  "service/postgres",
+				LocalPort: 5432,
+				Port:      5432,
+				Protocol:  "tcp",
+			},
+			description: "Should return service forward configuration",
+		},
+		{
+			name: "get forward with selector",
+			forward: config.Forward{
+				Resource:  "pod",
+				Selector:  "app=nginx,env=prod",
+				LocalPort: 8080,
+				Port:      80,
+				Protocol:  "tcp",
+			},
+			description: "Should return forward with label selector",
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			// Note: We can't easily test the full worker lifecycle without mocks,
+			// but we can test the constructor and simple getters
+
+			// This test would require proper mocking setup
+			// For now, we'll test the Forward struct directly
+
+			id := tt.forward.ID()
+			assert.NotEmpty(t, id, "Forward should have an ID")
+
+			forwardStr := tt.forward.String()
+			assert.NotEmpty(t, forwardStr, "Forward should have a string representation")
+			assert.Contains(t, forwardStr, tt.forward.Resource, "String should contain resource")
+		})
+	}
+}
+
+func TestForwardWorker_IsRunning(t *testing.T) {
+	// This is a basic test of the goroutine state tracking
+	// Full integration tests would require mock dependencies
+
+	t.Run("worker state tracking", func(t *testing.T) {
+		// Test the concept of the done channel
+		doneChan := make(chan struct{})
+
+		// Initially, channel is open (worker would be running)
+		select {
+		case <-doneChan:
+			t.Fatal("doneChan should be open initially")
+		default:
+			// Expected: channel is open
+		}
+
+		// Close the channel (simulating worker done)
+		close(doneChan)
+
+		// Now channel should be closed
+		select {
+		case <-doneChan:
+			// Expected: channel is closed
+		default:
+			t.Fatal("doneChan should be closed after close")
+		}
+	})
+}
+
+func TestForwardID(t *testing.T) {
+	tests := []struct {
+		name         string
+		forward      config.Forward
+		expectUnique bool
+		description  string
+	}{
+		{
+			name: "unique IDs for different forwards",
+			forward: config.Forward{
+				Resource:  "pod/app1",
+				LocalPort: 8080,
+				Port:      80,
+			},
+			expectUnique: true,
+			description:  "Different forwards should have different IDs",
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			id1 := tt.forward.ID()
+
+			// Create a different forward
+			fwd2 := config.Forward{
+				Resource:  "pod/app2",
+				LocalPort: 8081,
+				Port:      80,
+			}
+			id2 := fwd2.ID()
+
+			if tt.expectUnique {
+				assert.NotEqual(t, id1, id2, "Different forwards should have different IDs")
+			}
+
+			// Same forward should produce same ID
+			id3 := tt.forward.ID()
+			assert.Equal(t, id1, id3, "Same forward should produce same ID")
+		})
+	}
+}
+
+func TestForwardString(t *testing.T) {
+	tests := []struct {
+		name             string
+		forward          config.Forward
+		expectedContains []string
+		description      string
+	}{
+		{
+			name: "pod forward string",
+			forward: config.Forward{
+				Resource:  "pod/my-app",
+				LocalPort: 8080,
+				Port:      80,
+			},
+			expectedContains: []string{"pod/my-app", "8080", "80"},
+			description:      "Should contain resource and ports",
+		},
+		{
+			name: "service forward string",
+			forward: config.Forward{
+				Resource:  "service/postgres",
+				LocalPort: 5432,
+				Port:      5432,
+			},
+			expectedContains: []string{"service/postgres", "5432"},
+			description:      "Should contain service and port",
+		},
+		{
+			name: "selector forward string",
+			forward: config.Forward{
+				Resource:  "pod",
+				Selector:  "app=nginx",
+				LocalPort: 8080,
+				Port:      80,
+			},
+			expectedContains: []string{"app=nginx", "8080", "80"},
+			description:      "Should contain selector and ports",
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			result := tt.forward.String()
+
+			assert.NotEmpty(t, result, "String representation should not be empty")
+
+			for _, expected := range tt.expectedContains {
+				assert.Contains(t, result, expected,
+					"String should contain %s", expected)
+			}
+		})
+	}
+}
+
+func TestSleepWithBackoffConcept(t *testing.T) {
+	// Test the backoff concept without actually running a worker
+	t.Run("backoff delay increases", func(t *testing.T) {
+		// This tests the retry backoff behavior conceptually
+		delays := []int{1, 2, 4, 8, 10, 10, 10}
+
+		for i, expected := range delays {
+			// Simulate backoff calculation
+			delay := 1
+			for j := 0; j < i; j++ {
+				delay *= 2
+				if delay > 10 {
+					delay = 10
+				}
+			}
+
+			assert.Equal(t, expected, delay,
+				"Backoff at attempt %d should be %d", i, expected)
+		}
+	})
+}
+
+func TestWorkerVerboseMode(t *testing.T) {
+	tests := []struct {
+		name        string
+		verbose     bool
+		description string
+	}{
+		{
+			name:        "verbose mode enabled",
+			verbose:     true,
+			description: "Worker should respect verbose flag",
+		},
+		{
+			name:        "verbose mode disabled",
+			verbose:     false,
+			description: "Worker should respect non-verbose flag",
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			// Test that verbose flag is a boolean
+			assert.IsType(t, bool(true), tt.verbose)
+
+			// In a real worker, this would control logging
+			// For now, we just verify the type
+		})
+	}
+}