From 96ae1d45e00838e1bddcfc4ec69786bd6d1d6b86 Mon Sep 17 00:00:00 2001 From: Lukasz Raczylo Date: Tue, 13 Jan 2026 09:37:45 +0000 Subject: [PATCH] style: Extract UI constants and refactor main view rendering (#30) - [x] Add golangci-lint configuration with gocritic ifElseChain disabled - [x] Rename error variables to avoid shadowing (createErr, watcherErr, watchErr, etc.) - [x] Replace `interface{}` with `any` type alias throughout codebase - [x] Add package-level documentation comments to all internal packages - [x] Reorder struct fields alphabetically for consistency - [x] Extract UI constants (terminal dimensions, column widths, colors) to constants.go - [x] Refactor BubbleTeaUI main view rendering into smaller helper functions - [x] Simplify nested conditionals and improve code clarity - [x] Add `isForwardDisabled()` helper method to BubbleTeaUI - [x] Update file permissions from 0644 to 0600 in config tests - [x] Add `#nosec` comments and error suppression where appropriate - [x] Improve test table struct field ordering for readability - [x] Fix resource parsing in AddForward using strings.SplitN - [x] Add comprehensive tests for new UI helper functions and constants --- .golangci.yml | 29 ++ cmd/kportal/main.go | 60 +-- internal/benchmark/results.go | 23 +- internal/benchmark/runner.go | 18 +- internal/benchmark/runner_test.go | 6 +- internal/config/config.go | 57 ++- internal/config/config_getters_test.go | 24 +- internal/config/config_test.go | 18 +- internal/config/mutator_test.go | 2 +- internal/config/validator.go | 18 +- internal/config/validator_test.go | 22 +- internal/config/watcher.go | 12 +- internal/config/watcher_test.go | 38 +- internal/converter/kftray.go | 19 +- internal/events/bus.go | 22 +- internal/forward/manager.go | 30 +- internal/forward/manager_test.go | 2 +- internal/forward/portcheck.go | 8 +- internal/forward/portcheck_test.go | 14 +- internal/forward/watchdog.go | 20 +- internal/forward/worker.go | 44 +- internal/forward/worker_unit_test.go | 10 +- internal/healthcheck/checker.go | 49 ++- internal/healthcheck/checker_test.go | 11 +- internal/httplog/logger.go | 28 +- internal/httplog/logger_test.go | 30 +- internal/httplog/proxy.go | 13 +- internal/httplog/proxy_test.go | 4 +- internal/k8s/client.go | 23 +- internal/k8s/client_test.go | 4 +- internal/k8s/discovery.go | 10 +- internal/k8s/discovery_test.go | 6 +- internal/k8s/portforward.go | 16 +- internal/k8s/resolver.go | 10 +- internal/logger/klog_bridge_test.go | 6 +- internal/logger/klog_logr_test.go | 14 +- internal/logger/logger.go | 42 +- internal/logger/logger_test.go | 38 +- internal/mdns/publisher.go | 21 +- internal/retry/backoff.go | 20 +- internal/ui/bubbletea_ui.go | 585 +++++++++++++++---------- internal/ui/bubbletea_ui_test.go | 255 ++++++++++- internal/ui/commands_test.go | 9 +- internal/ui/constants.go | 45 ++ internal/ui/handlers_test.go | 6 +- internal/ui/httplog_state_test.go | 10 +- internal/ui/mocks_test.go | 86 ++-- internal/ui/table.go | 12 +- internal/ui/wizard_commands.go | 27 +- internal/ui/wizard_exclusion_test.go | 4 +- internal/ui/wizard_state.go | 148 +++---- internal/ui/wizard_state_test.go | 4 +- internal/ui/wizard_styles.go | 3 +- internal/version/checker.go | 14 +- 54 files changed, 1319 insertions(+), 730 deletions(-) create mode 100644 .golangci.yml create mode 100644 internal/ui/constants.go diff --git a/.golangci.yml b/.golangci.yml new file mode 100644 index 0000000..287764e --- /dev/null +++ b/.golangci.yml @@ -0,0 +1,29 @@ +# golangci-lint configuration +# https://golangci-lint.run/usage/configuration/ + +run: + timeout: 5m + tests: true + +linters: + enable: + - errcheck + - gosimple + - govet + - ineffassign + - staticcheck + - unused + - gosec + - gocritic + - gofmt + +linters-settings: + govet: + enable: + - fieldalignment + gosec: + excludes: + - G304 # File path provided as taint input - handled with #nosec comments where needed + gocritic: + disabled-checks: + - ifElseChain # Complex conditionals are clearer as if-else than switch true diff --git a/cmd/kportal/main.go b/cmd/kportal/main.go index 1e6d07e..db51371 100644 --- a/cmd/kportal/main.go +++ b/cmd/kportal/main.go @@ -199,8 +199,8 @@ func main() { os.Exit(0) } // Create empty config file - if err := config.CreateEmptyConfigFile(*configFile); err != nil { - fmt.Fprintf(os.Stderr, "Error creating config file: %v\n", err) + if createErr := config.CreateEmptyConfigFile(*configFile); createErr != nil { + fmt.Fprintf(os.Stderr, "Error creating config file: %v\n", createErr) os.Exit(1) } fmt.Printf("Created %s\n", *configFile) @@ -309,7 +309,7 @@ func main() { bubbleTeaUI.SetHTTPLogSubscriber(func(forwardID string, callback func(entry ui.HTTPLogEntry)) func() { worker := manager.GetWorker(forwardID) if worker == nil { - logger.Debug("HTTP log subscription failed: worker not found", map[string]interface{}{ + logger.Debug("HTTP log subscription failed: worker not found", map[string]any{ "forward_id": forwardID, }) return func() {} // No-op cleanup @@ -318,7 +318,7 @@ func main() { proxy := worker.GetHTTPProxy() if proxy == nil { // This is expected for forwards without httpLog enabled - not an error - logger.Debug("HTTP log subscription skipped: proxy not enabled", map[string]interface{}{ + logger.Debug("HTTP log subscription skipped: proxy not enabled", map[string]any{ "forward_id": forwardID, }) return func() {} // HTTP logging not enabled for this forward @@ -326,7 +326,7 @@ func main() { proxyLogger := proxy.GetLogger() if proxyLogger == nil { - logger.Debug("HTTP log subscription failed: logger not available", map[string]interface{}{ + logger.Debug("HTTP log subscription failed: logger not available", map[string]any{ "forward_id": forwardID, }) return func() {} @@ -379,8 +379,8 @@ func main() { } // Start forwards - if err := manager.Start(cfg); err != nil { - fmt.Fprintf(os.Stderr, "Error starting forwards: %v\n", err) + if startErr := manager.Start(cfg); startErr != nil { + fmt.Fprintf(os.Stderr, "Error starting forwards: %v\n", startErr) os.Exit(1) } @@ -391,17 +391,18 @@ func main() { signal.Notify(sigChan, os.Interrupt, syscall.SIGTERM, syscall.SIGHUP) // Setup config watcher for hot-reload - watcher, err := config.NewWatcher(*configFile, func(newCfg *config.Config) error { + watcher, watcherErr := config.NewWatcher(*configFile, func(newCfg *config.Config) error { return manager.Reload(newCfg) }, *verbose) - if err != nil { + watcherStarted := false + if watcherErr != nil { if *verbose { - log.Printf("Warning: Failed to setup config watcher: %v", err) + log.Printf("Warning: Failed to setup config watcher: %v", watcherErr) log.Printf("Hot-reload will not be available") } } else { watcher.Start() - defer watcher.Stop() + watcherStarted = true } if *verbose { @@ -416,10 +417,10 @@ func main() { if *verbose { log.Printf("Received SIGHUP, reloading configuration...") } - newCfg, err := config.LoadConfig(*configFile) - if err != nil { + newCfg, loadErr := config.LoadConfig(*configFile) + if loadErr != nil { if *verbose { - log.Printf("Failed to reload config: %v", err) + log.Printf("Failed to reload config: %v", loadErr) } continue } @@ -432,9 +433,9 @@ func main() { continue } - if err := manager.Reload(newCfg); err != nil { + if reloadErr := manager.Reload(newCfg); reloadErr != nil { if *verbose { - log.Printf("Failed to reload: %v", err) + log.Printf("Failed to reload: %v", reloadErr) } } @@ -464,6 +465,10 @@ func main() { log.Printf("Received second signal (%v), forcing exit...", sig) } } + // Stop the watcher before exiting (defers won't run after os.Exit) + if watcherStarted { + watcher.Stop() + } os.Exit(0) } } @@ -485,15 +490,16 @@ func main() { }() // Setup config watcher for hot-reload - watcher, err := config.NewWatcher(*configFile, func(newCfg *config.Config) error { + watcher, watchErr := config.NewWatcher(*configFile, func(newCfg *config.Config) error { return manager.Reload(newCfg) }, *verbose) - if err != nil { - log.Printf("Warning: Failed to setup config watcher: %v", err) + watcherActive := false + if watchErr != nil { + log.Printf("Warning: Failed to setup config watcher: %v", watchErr) log.Printf("Hot-reload will not be available") } else { watcher.Start() - defer watcher.Stop() + watcherActive = true } log.Printf("Press Ctrl+C to stop") @@ -504,9 +510,9 @@ func main() { switch sig { case syscall.SIGHUP: log.Printf("Received SIGHUP, reloading configuration...") - newCfg, err := config.LoadConfig(*configFile) - if err != nil { - log.Printf("Failed to reload config: %v", err) + newCfg, loadErr := config.LoadConfig(*configFile) + if loadErr != nil { + log.Printf("Failed to reload config: %v", loadErr) continue } @@ -516,8 +522,8 @@ func main() { continue } - if err := manager.Reload(newCfg); err != nil { - log.Printf("Failed to reload: %v", err) + if reloadErr := manager.Reload(newCfg); reloadErr != nil { + log.Printf("Failed to reload: %v", reloadErr) } case os.Interrupt, syscall.SIGTERM: @@ -539,6 +545,10 @@ func main() { // Second signal received - force exit immediately log.Printf("Received second signal (%v), forcing exit...", sig) } + // Stop the watcher before exiting (defers won't run after os.Exit) + if watcherActive { + watcher.Stop() + } os.Exit(0) } } diff --git a/internal/benchmark/results.go b/internal/benchmark/results.go index 9e0d170..cbf73ff 100644 --- a/internal/benchmark/results.go +++ b/internal/benchmark/results.go @@ -1,3 +1,14 @@ +// Package benchmark provides HTTP benchmarking capabilities for port forwards. +// It measures latency, throughput, and reliability of forwarded connections. +// +// The benchmark runner sends configurable numbers of concurrent requests +// and collects statistics including: +// - Latency percentiles (P50, P95, P99) +// - Request success/failure rates +// - Throughput (requests/second) +// - Status code distribution +// +// Results can be displayed in the UI or exported for analysis. package benchmark import ( @@ -7,17 +18,17 @@ import ( // Results holds the aggregated results of a benchmark run type Results struct { - ForwardID string `json:"forward_id"` - URL string `json:"url"` - Method string `json:"method"` StartTime time.Time `json:"start_time"` EndTime time.Time `json:"end_time"` + StatusCodes map[int]int `json:"status_codes"` + Errors map[string]int `json:"errors,omitempty"` + Method string `json:"method"` + URL string `json:"url"` + ForwardID string `json:"forward_id"` + Latencies []time.Duration `json:"-"` TotalRequests int `json:"total_requests"` Successful int `json:"successful"` Failed int `json:"failed"` - Latencies []time.Duration `json:"-"` // Raw latencies for percentile calculation - StatusCodes map[int]int `json:"status_codes"` - Errors map[string]int `json:"errors,omitempty"` BytesRead int64 `json:"bytes_read"` BytesWritten int64 `json:"bytes_written"` } diff --git a/internal/benchmark/runner.go b/internal/benchmark/runner.go index 780224b..b71803c 100644 --- a/internal/benchmark/runner.go +++ b/internal/benchmark/runner.go @@ -16,15 +16,15 @@ type ProgressCallback func(completed, total int) // Config holds the benchmark configuration type Config struct { - URL string // Target URL - Method string // HTTP method - Headers map[string]string // Custom headers - Body []byte // Request body - Concurrency int // Number of concurrent workers - Requests int // Total number of requests (0 = use duration) - Duration time.Duration // Duration to run (0 = use requests) - Timeout time.Duration // Request timeout - ProgressCallback ProgressCallback // Optional callback for progress updates + Headers map[string]string + ProgressCallback ProgressCallback + URL string + Method string + Body []byte + Concurrency int + Requests int + Duration time.Duration + Timeout time.Duration } // Runner executes HTTP benchmarks diff --git a/internal/benchmark/runner_test.go b/internal/benchmark/runner_test.go index dff401c..0742402 100644 --- a/internal/benchmark/runner_test.go +++ b/internal/benchmark/runner_test.go @@ -106,7 +106,7 @@ func TestRunner(t *testing.T) { server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { time.Sleep(5 * time.Millisecond) // Simulate some latency w.WriteHeader(http.StatusOK) - w.Write([]byte(`{"status":"ok"}`)) + _, _ = w.Write([]byte(`{"status":"ok"}`)) })) defer server.Close() @@ -132,7 +132,7 @@ func TestRunner(t *testing.T) { func TestRunnerWithDuration(t *testing.T) { server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { w.WriteHeader(http.StatusOK) - w.Write([]byte(`ok`)) + _, _ = w.Write([]byte(`ok`)) })) defer server.Close() @@ -210,7 +210,7 @@ func TestRunnerWithProgressCallback(t *testing.T) { server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { time.Sleep(10 * time.Millisecond) // Add small delay so progress ticker can fire w.WriteHeader(http.StatusOK) - w.Write([]byte(`ok`)) + _, _ = w.Write([]byte(`ok`)) })) defer server.Close() diff --git a/internal/config/config.go b/internal/config/config.go index 5c8c7b5..1b26244 100644 --- a/internal/config/config.go +++ b/internal/config/config.go @@ -1,3 +1,24 @@ +// Package config provides configuration loading, validation, watching, and +// mutation for kportal. It handles parsing the .kportal.yaml configuration +// file and provides hot-reload support via file watching. +// +// The configuration structure supports multiple Kubernetes contexts, each +// with namespaces containing port-forward definitions. Additional settings +// for health checks, reliability, and mDNS hostname publishing are also +// supported. +// +// Basic usage: +// +// cfg, err := config.Load("~/.kportal.yaml") +// if err != nil { +// log.Fatal(err) +// } +// +// For hot-reload support, use the ConfigWatcher: +// +// watcher, err := config.NewConfigWatcher(path, func(cfg *config.Config) { +// // Handle configuration changes +// }) package config import ( @@ -36,10 +57,10 @@ const ( // Config represents the root configuration structure from .kportal.yaml type Config struct { - Contexts []Context `yaml:"contexts"` HealthCheck *HealthCheckSpec `yaml:"healthCheck,omitempty"` Reliability *ReliabilitySpec `yaml:"reliability,omitempty"` MDNS *MDNSSpec `yaml:"mdns,omitempty"` + Contexts []Context `yaml:"contexts"` } // MDNSSpec configures mDNS (multicast DNS) hostname publishing @@ -59,10 +80,10 @@ type HealthCheckSpec struct { // ReliabilitySpec configures connection reliability features type ReliabilitySpec struct { - TCPKeepalive string `yaml:"tcpKeepalive,omitempty"` // e.g., "30s" - OS-level keepalive - DialTimeout string `yaml:"dialTimeout,omitempty"` // e.g., "30s" - connection dial timeout - RetryOnStale bool `yaml:"retryOnStale,omitempty"` // Auto-reconnect on stale detection - WatchdogPeriod string `yaml:"watchdogPeriod,omitempty"` // e.g., "30s" - goroutine watchdog interval + TCPKeepalive string `yaml:"tcpKeepalive,omitempty"` + DialTimeout string `yaml:"dialTimeout,omitempty"` + WatchdogPeriod string `yaml:"watchdogPeriod,omitempty"` + RetryOnStale bool `yaml:"retryOnStale,omitempty"` } // parseDurationOrDefault parses a duration string and returns the default if empty or invalid. @@ -167,11 +188,11 @@ type Namespace struct { // HTTPLogSpec configures HTTP traffic logging for a forward type HTTPLogSpec struct { - Enabled bool `yaml:"enabled"` // Enable HTTP logging - LogFile string `yaml:"logFile,omitempty"` // Output file (empty = stdout) - MaxBodySize int `yaml:"maxBodySize,omitempty"` // Max body size to log (default 1MB) - IncludeHeaders bool `yaml:"includeHeaders,omitempty"` // Include headers in log - FilterPath string `yaml:"filterPath,omitempty"` // Optional glob filter for paths + LogFile string `yaml:"logFile,omitempty"` + FilterPath string `yaml:"filterPath,omitempty"` + MaxBodySize int `yaml:"maxBodySize,omitempty"` + Enabled bool `yaml:"enabled"` + IncludeHeaders bool `yaml:"includeHeaders,omitempty"` } // UnmarshalYAML implements custom unmarshaling to support both bool and struct formats @@ -196,17 +217,15 @@ func (h *HTTPLogSpec) UnmarshalYAML(unmarshal func(interface{}) error) error { // Forward represents a single port-forward configuration type Forward struct { - Resource string `yaml:"resource"` // e.g., "pod/my-app", "service/postgres", "pod" - Selector string `yaml:"selector"` // Label selector for pod resolution (e.g., "app=nginx,env=prod") - Protocol string `yaml:"protocol"` // tcp or udp - Port int `yaml:"port"` // Remote port - LocalPort int `yaml:"localPort"` // Local port - Alias string `yaml:"alias,omitempty"` // Optional human-readable alias for this forward - HTTPLog *HTTPLogSpec `yaml:"httpLog,omitempty"` // Optional HTTP traffic logging - - // Runtime fields (not in YAML) + HTTPLog *HTTPLogSpec `yaml:"httpLog,omitempty"` + Resource string `yaml:"resource"` + Selector string `yaml:"selector"` + Protocol string `yaml:"protocol"` + Alias string `yaml:"alias,omitempty"` contextName string namespaceName string + Port int `yaml:"port"` + LocalPort int `yaml:"localPort"` } // ID returns a unique identifier for this forward configuration. diff --git a/internal/config/config_getters_test.go b/internal/config/config_getters_test.go index 57d2d59..001e1d1 100644 --- a/internal/config/config_getters_test.go +++ b/internal/config/config_getters_test.go @@ -40,8 +40,8 @@ func TestParseDurationOrDefault(t *testing.T) { // TestConfig_GetHealthCheckIntervalOrDefault tests health check interval getter func TestConfig_GetHealthCheckIntervalOrDefault(t *testing.T) { tests := []struct { - name string config *Config + name string expected time.Duration }{ { @@ -83,8 +83,8 @@ func TestConfig_GetHealthCheckIntervalOrDefault(t *testing.T) { // TestConfig_GetHealthCheckTimeoutOrDefault tests health check timeout getter func TestConfig_GetHealthCheckTimeoutOrDefault(t *testing.T) { tests := []struct { - name string config *Config + name string expected time.Duration }{ { @@ -162,8 +162,8 @@ func TestConfig_GetHealthCheckMethod(t *testing.T) { // TestConfig_GetMaxConnectionAge tests max connection age getter func TestConfig_GetMaxConnectionAge(t *testing.T) { tests := []struct { - name string config *Config + name string expected time.Duration }{ { @@ -198,8 +198,8 @@ func TestConfig_GetMaxConnectionAge(t *testing.T) { // TestConfig_GetMaxIdleTime tests max idle time getter func TestConfig_GetMaxIdleTime(t *testing.T) { tests := []struct { - name string config *Config + name string expected time.Duration }{ { @@ -234,8 +234,8 @@ func TestConfig_GetMaxIdleTime(t *testing.T) { // TestConfig_GetTCPKeepalive tests TCP keepalive getter func TestConfig_GetTCPKeepalive(t *testing.T) { tests := []struct { - name string config *Config + name string expected time.Duration }{ { @@ -270,8 +270,8 @@ func TestConfig_GetTCPKeepalive(t *testing.T) { // TestConfig_GetRetryOnStale tests retry on stale getter func TestConfig_GetRetryOnStale(t *testing.T) { tests := []struct { - name string config *Config + name string expected bool }{ { @@ -306,8 +306,8 @@ func TestConfig_GetRetryOnStale(t *testing.T) { // TestConfig_GetWatchdogPeriod tests watchdog period getter func TestConfig_GetWatchdogPeriod(t *testing.T) { tests := []struct { - name string config *Config + name string expected time.Duration }{ { @@ -342,8 +342,8 @@ func TestConfig_GetWatchdogPeriod(t *testing.T) { // TestConfig_GetDialTimeout tests dial timeout getter func TestConfig_GetDialTimeout(t *testing.T) { tests := []struct { - name string config *Config + name string expected time.Duration }{ { @@ -378,8 +378,8 @@ func TestConfig_GetDialTimeout(t *testing.T) { // TestConfig_IsMDNSEnabled tests mDNS enabled getter func TestConfig_IsMDNSEnabled(t *testing.T) { tests := []struct { - name string config *Config + name string expected bool }{ { @@ -509,8 +509,8 @@ func TestForward_GetHTTPLogMaxBodySize(t *testing.T) { func TestForward_GetMDNSAlias(t *testing.T) { tests := []struct { name string - forward Forward expected string + forward Forward }{ { name: "explicit alias", @@ -591,7 +591,7 @@ func TestLoadConfig_FileTooLarge(t *testing.T) { largeData[i] = 'a' } - err := os.WriteFile(configPath, largeData, 0644) + err := os.WriteFile(configPath, largeData, 0600) require.NoError(t, err) cfg, err := LoadConfig(configPath) @@ -628,7 +628,7 @@ mdns: enabled: true ` - err := os.WriteFile(configPath, []byte(yaml), 0644) + err := os.WriteFile(configPath, []byte(yaml), 0600) require.NoError(t, err) cfg, err := LoadConfig(configPath) diff --git a/internal/config/config_test.go b/internal/config/config_test.go index d4d0d36..ab489b2 100644 --- a/internal/config/config_test.go +++ b/internal/config/config_test.go @@ -39,7 +39,7 @@ func TestLoadConfig_ValidYAML(t *testing.T) { localPort: 8081 ` - err := os.WriteFile(configPath, []byte(validYAML), 0644) + err := os.WriteFile(configPath, []byte(validYAML), 0600) assert.NoError(t, err, "should write temp config file") // Load the config @@ -82,7 +82,7 @@ func TestLoadConfig_InvalidYAML(t *testing.T) { forwards: [this is invalid yaml syntax ` - err := os.WriteFile(configPath, []byte(invalidYAML), 0644) + err := os.WriteFile(configPath, []byte(invalidYAML), 0600) assert.NoError(t, err, "should write temp config file") // Load the config @@ -103,8 +103,8 @@ func TestLoadConfig_FileNotFound(t *testing.T) { func TestForward_ID(t *testing.T) { tests := []struct { name string - forward Forward expectedID string + forward Forward }{ { name: "pod with explicit name", @@ -165,8 +165,8 @@ func TestForward_ID(t *testing.T) { func TestForward_String(t *testing.T) { tests := []struct { name string - forward Forward expectedString string + forward Forward }{ { name: "pod without selector", @@ -389,10 +389,8 @@ func TestHTTPLogSpec_UnmarshalYAML(t *testing.T) { if tt.expected { assert.NotNil(t, fwd.HTTPLog, "HTTPLog should not be nil") assert.True(t, fwd.HTTPLog.Enabled, "HTTPLog.Enabled should be true") - } else { - if fwd.HTTPLog != nil { - assert.False(t, fwd.HTTPLog.Enabled, "HTTPLog.Enabled should be false") - } + } else if fwd.HTTPLog != nil { + assert.False(t, fwd.HTTPLog.Enabled, "HTTPLog.Enabled should be false") } }) } @@ -407,8 +405,8 @@ func TestNewEmptyConfig(t *testing.T) { func TestConfig_IsEmpty(t *testing.T) { tests := []struct { - name string config *Config + name string expected bool }{ { @@ -505,7 +503,7 @@ func TestCreateEmptyConfigFile_AlreadyExists(t *testing.T) { configPath := filepath.Join(tmpDir, ".kportal.yaml") // Create existing file - err := os.WriteFile(configPath, []byte("existing content"), 0644) + err := os.WriteFile(configPath, []byte("existing content"), 0600) assert.NoError(t, err) // Try to create config file - should fail diff --git a/internal/config/mutator_test.go b/internal/config/mutator_test.go index f886704..8265d82 100644 --- a/internal/config/mutator_test.go +++ b/internal/config/mutator_test.go @@ -648,7 +648,7 @@ func TestMutator_Concurrent(t *testing.T) { } // Some will succeed, some will fail due to validation // The important thing is no race condition - mutator.AddForward("dev", "default", fwd) + _ = mutator.AddForward("dev", "default", fwd) }(i) } diff --git a/internal/config/validator.go b/internal/config/validator.go index 32bb6cf..2fbd942 100644 --- a/internal/config/validator.go +++ b/internal/config/validator.go @@ -17,9 +17,9 @@ func IsValidPort(port int) bool { // ValidationError represents a configuration validation error with context. type ValidationError struct { - Field string // The field that failed validation - Message string // Error message - Context map[string]string // Additional context information + Context map[string]string + Field string + Message string } // Validator validates configuration files. @@ -199,14 +199,12 @@ func (v *Validator) validateResource(fwd *Forward) []ValidationError { Message: fmt.Sprintf("Pod name cannot be empty for forward %s", fwd.ID()), }) } - } else { + } else if fwd.Selector == "" { // pod (no name) - must have selector - if fwd.Selector == "" { - errs = append(errs, ValidationError{ - Field: "selector", - Message: fmt.Sprintf("Forward %s uses generic 'pod' resource and must have a selector", fwd.ID()), - }) - } + errs = append(errs, ValidationError{ + Field: "selector", + Message: fmt.Sprintf("Forward %s uses generic 'pod' resource and must have a selector", fwd.ID()), + }) } } diff --git a/internal/config/validator_test.go b/internal/config/validator_test.go index 82edbd7..4bb4918 100644 --- a/internal/config/validator_test.go +++ b/internal/config/validator_test.go @@ -11,10 +11,10 @@ func TestValidator_ValidateConfig(t *testing.T) { validator := NewValidator() tests := []struct { - name string config *Config - expectErrors bool + name string errorContains []string + expectErrors bool }{ { name: "valid config", @@ -227,9 +227,9 @@ func TestValidator_ValidateResourceFormat(t *testing.T) { tests := []struct { name string + errorContains []string forward Forward expectErrors bool - errorContains []string }{ { name: "valid pod with name", @@ -370,10 +370,10 @@ func TestValidator_CheckDuplicatePorts(t *testing.T) { validator := NewValidator() tests := []struct { - name string config *Config - expectErrors bool + name string errorContains []string + expectErrors bool }{ { name: "no duplicate ports", @@ -552,8 +552,8 @@ func TestFormatValidationErrors(t *testing.T) { tests := []struct { name string errors []ValidationError - expectEmpty bool expectContains []string + expectEmpty bool }{ { name: "no errors", @@ -625,10 +625,10 @@ func TestValidator_ValidateStructure(t *testing.T) { validator := NewValidator() tests := []struct { - name string config *Config - expectErrors bool + name string errorContains []string + expectErrors bool }{ { name: "empty context name", @@ -697,10 +697,10 @@ func TestValidator_ValidateMDNS(t *testing.T) { validator := NewValidator() tests := []struct { - name string config *Config - expectErrors bool + name string errorContains []string + expectErrors bool }{ { name: "mDNS disabled - no validation", @@ -968,8 +968,8 @@ func TestValidator_ValidateConfigWithOptions(t *testing.T) { validator := NewValidator() tests := []struct { - name string config *Config + name string allowEmpty bool expectErrors bool }{ diff --git a/internal/config/watcher.go b/internal/config/watcher.go index 4b29f88..2554ea8 100644 --- a/internal/config/watcher.go +++ b/internal/config/watcher.go @@ -16,13 +16,13 @@ type ReloadCallback func(*Config) error // Watcher watches a configuration file for changes and triggers hot-reload. type Watcher struct { - configPath string callback ReloadCallback watcher *fsnotify.Watcher done chan struct{} + configPath string + wg sync.WaitGroup + stopOnce sync.Once verbose bool - wg sync.WaitGroup // Ensures watch goroutine exits before Stop returns - stopOnce sync.Once // Ensures Stop is safe to call multiple times } // NewWatcher creates a new file watcher for the given config file. @@ -34,7 +34,7 @@ func NewWatcher(configPath string, callback ReloadCallback, verbose bool) (*Watc absPath, err := filepath.Abs(configPath) if err != nil { - _ = watcher.Close() + _ = watcher.Close() // Cleanup on error path; already returning error return nil, fmt.Errorf("failed to resolve absolute path: %w", err) } @@ -42,7 +42,7 @@ func NewWatcher(configPath string, callback ReloadCallback, verbose bool) (*Watc // (many editors delete and recreate files on save) dir := filepath.Dir(absPath) if err := watcher.Add(dir); err != nil { - _ = watcher.Close() + _ = watcher.Close() // Cleanup on error path; already returning error return nil, fmt.Errorf("failed to watch directory %s: %w", dir, err) } @@ -66,7 +66,7 @@ func (w *Watcher) Start() { func (w *Watcher) Stop() { w.stopOnce.Do(func() { close(w.done) - _ = w.watcher.Close() + _ = w.watcher.Close() // Best-effort cleanup during shutdown }) w.wg.Wait() // Wait for watch goroutine to exit } diff --git a/internal/config/watcher_test.go b/internal/config/watcher_test.go index e68d92d..384b458 100644 --- a/internal/config/watcher_test.go +++ b/internal/config/watcher_test.go @@ -27,7 +27,7 @@ func TestNewWatcher(t *testing.T) { port: 8080 localPort: 8080 ` - err := os.WriteFile(configPath, []byte(initial), 0644) + err := os.WriteFile(configPath, []byte(initial), 0600) require.NoError(t, err) callback := func(cfg *Config) error { return nil } @@ -57,7 +57,7 @@ func TestNewWatcher_Verbose(t *testing.T) { port: 8080 localPort: 8080 ` - err := os.WriteFile(configPath, []byte(initial), 0644) + err := os.WriteFile(configPath, []byte(initial), 0600) require.NoError(t, err) callback := func(cfg *Config) error { return nil } @@ -85,13 +85,15 @@ func TestNewWatcher_RelativePath(t *testing.T) { port: 8080 localPort: 8080 ` - err := os.WriteFile(configPath, []byte(initial), 0644) + err := os.WriteFile(configPath, []byte(initial), 0600) require.NoError(t, err) // Change to tmpDir and use relative path - originalDir, _ := os.Getwd() - defer os.Chdir(originalDir) - os.Chdir(tmpDir) + originalDir, err := os.Getwd() + require.NoError(t, err) + defer func() { _ = os.Chdir(originalDir) }() + err = os.Chdir(tmpDir) + require.NoError(t, err) callback := func(cfg *Config) error { return nil } @@ -119,7 +121,7 @@ func TestWatcher_StartStop(t *testing.T) { port: 8080 localPort: 8080 ` - err := os.WriteFile(configPath, []byte(initial), 0644) + err := os.WriteFile(configPath, []byte(initial), 0600) require.NoError(t, err) callback := func(cfg *Config) error { return nil } @@ -161,7 +163,7 @@ func TestWatcher_DetectsFileChange(t *testing.T) { port: 8080 localPort: 8080 ` - err := os.WriteFile(configPath, []byte(initial), 0644) + err := os.WriteFile(configPath, []byte(initial), 0600) require.NoError(t, err) var mu sync.Mutex @@ -199,7 +201,7 @@ func TestWatcher_DetectsFileChange(t *testing.T) { port: 9090 localPort: 9090 ` - err = os.WriteFile(configPath, []byte(updated), 0644) + err = os.WriteFile(configPath, []byte(updated), 0600) require.NoError(t, err) // Wait for callback with timeout @@ -239,7 +241,7 @@ func TestWatcher_IgnoresInvalidConfig(t *testing.T) { port: 8080 localPort: 8080 ` - err := os.WriteFile(configPath, []byte(initial), 0644) + err := os.WriteFile(configPath, []byte(initial), 0600) require.NoError(t, err) callbackCount := 0 @@ -267,7 +269,7 @@ func TestWatcher_IgnoresInvalidConfig(t *testing.T) { - name: default forwards: [this is invalid ` - err = os.WriteFile(configPath, []byte(invalid), 0644) + err = os.WriteFile(configPath, []byte(invalid), 0600) require.NoError(t, err) // Wait a bit @@ -294,7 +296,7 @@ func TestWatcher_IgnoresValidationErrors(t *testing.T) { port: 8080 localPort: 8080 ` - err := os.WriteFile(configPath, []byte(initial), 0644) + err := os.WriteFile(configPath, []byte(initial), 0600) require.NoError(t, err) callbackCount := 0 @@ -328,7 +330,7 @@ func TestWatcher_IgnoresValidationErrors(t *testing.T) { port: 9090 localPort: 8080 ` - err = os.WriteFile(configPath, []byte(invalid), 0644) + err = os.WriteFile(configPath, []byte(invalid), 0600) require.NoError(t, err) // Wait a bit @@ -356,7 +358,7 @@ func TestWatcher_IgnoresOtherFiles(t *testing.T) { port: 8080 localPort: 8080 ` - err := os.WriteFile(configPath, []byte(initial), 0644) + err := os.WriteFile(configPath, []byte(initial), 0600) require.NoError(t, err) callbackCount := 0 @@ -378,7 +380,7 @@ func TestWatcher_IgnoresOtherFiles(t *testing.T) { time.Sleep(100 * time.Millisecond) // Write to a different file - err = os.WriteFile(otherPath, []byte("some content"), 0644) + err = os.WriteFile(otherPath, []byte("some content"), 0600) require.NoError(t, err) // Wait a bit @@ -405,7 +407,7 @@ func TestWatcher_HandleReload_LoadError(t *testing.T) { port: 8080 localPort: 8080 ` - err := os.WriteFile(configPath, []byte(initial), 0644) + err := os.WriteFile(configPath, []byte(initial), 0600) require.NoError(t, err) callbackCalled := false @@ -445,7 +447,7 @@ func TestWatcher_DoubleStop(t *testing.T) { port: 8080 localPort: 8080 ` - err := os.WriteFile(configPath, []byte(initial), 0644) + err := os.WriteFile(configPath, []byte(initial), 0600) require.NoError(t, err) callback := func(cfg *Config) error { return nil } @@ -479,7 +481,7 @@ func TestWatcher_StopWithoutStart(t *testing.T) { port: 8080 localPort: 8080 ` - err := os.WriteFile(configPath, []byte(initial), 0644) + err := os.WriteFile(configPath, []byte(initial), 0600) require.NoError(t, err) callback := func(cfg *Config) error { return nil } diff --git a/internal/converter/kftray.go b/internal/converter/kftray.go index 9c35c94..2fd395a 100644 --- a/internal/converter/kftray.go +++ b/internal/converter/kftray.go @@ -1,3 +1,12 @@ +// Package converter provides configuration migration from other port-forwarding +// tools to kportal's YAML format. Currently supports kftray JSON format. +// +// Basic usage: +// +// err := converter.ConvertKFTrayToKPortal("kftray.json", ".kportal.yaml") +// if err != nil { +// log.Fatal(err) +// } package converter import ( @@ -14,12 +23,12 @@ import ( type KFTrayConfig struct { Service string `json:"service"` Namespace string `json:"namespace"` - LocalPort int `json:"local_port"` - RemotePort int `json:"remote_port"` Context string `json:"context"` WorkloadType string `json:"workload_type"` Protocol string `json:"protocol"` Alias string `json:"alias"` + LocalPort int `json:"local_port"` + RemotePort int `json:"remote_port"` } // ConvertKFTrayToKPortal converts kftray JSON configuration to kportal YAML format @@ -32,8 +41,8 @@ func ConvertKFTrayToKPortal(inputFile, outputFile string) error { } var kftrayConfigs []KFTrayConfig - if err := json.Unmarshal(data, &kftrayConfigs); err != nil { - return fmt.Errorf("failed to parse JSON: %w", err) + if unmarshalErr := json.Unmarshal(data, &kftrayConfigs); unmarshalErr != nil { + return fmt.Errorf("failed to parse JSON: %w", unmarshalErr) } // Convert to kportal format @@ -169,9 +178,9 @@ type namespaceEntry struct { type forwardEntry struct { Resource string `yaml:"resource"` Protocol string `yaml:"protocol"` + Alias string `yaml:"alias,omitempty"` Port int `yaml:"port"` LocalPort int `yaml:"localPort"` - Alias string `yaml:"alias,omitempty"` } // Convert internal types to config package types diff --git a/internal/events/bus.go b/internal/events/bus.go index 04b2174..bfcaee3 100644 --- a/internal/events/bus.go +++ b/internal/events/bus.go @@ -1,3 +1,21 @@ +// Package events provides a publish-subscribe event bus for decoupled +// communication between kportal components. Events are typed and carry +// contextual data about forward lifecycle, health status, and configuration +// changes. +// +// Event types include: +// - Forward lifecycle: starting, connected, disconnected, reconnecting, stopped, error +// - Health: status_changed, stale +// - Watchdog: worker_hung +// - Config: reloaded +// +// Basic usage: +// +// bus := events.NewBus() +// bus.Subscribe(events.EventForwardConnected, func(e events.Event) { +// fmt.Printf("Forward %s connected\n", e.ForwardID) +// }) +// bus.Publish(events.Event{Type: events.EventForwardConnected, ForwardID: "..."}) package events import ( @@ -29,9 +47,9 @@ const ( // Event represents a system event type Event struct { + Data map[string]interface{} Type EventType ForwardID string - Data map[string]interface{} } // Handler is a function that handles events @@ -39,8 +57,8 @@ type Handler func(event Event) // Bus is a simple event bus for decoupled communication between components type Bus struct { - mu sync.RWMutex handlers map[EventType][]Handler + mu sync.RWMutex closed bool } diff --git a/internal/forward/manager.go b/internal/forward/manager.go index b2a8a29..1ae9bf0 100644 --- a/internal/forward/manager.go +++ b/internal/forward/manager.go @@ -1,3 +1,17 @@ +// Package forward provides the core port-forwarding orchestration for kportal. +// It manages the lifecycle of port-forward workers, handles hot-reload of +// configuration changes, and coordinates with the health checker and watchdog. +// +// The Manager is the central orchestrator that: +// - Creates and manages ForwardWorker instances for each configured forward +// - Handles graceful startup, shutdown, and reconfiguration +// - Coordinates with the HealthChecker for connection monitoring +// - Integrates with mDNS for hostname publishing +// +// ForwardWorker handles individual port-forward connections with: +// - Automatic retry with exponential backoff (1s → 2s → 4s → 8s → 10s max) +// - Pod restart detection and re-resolution +// - Graceful shutdown support package forward import ( @@ -24,19 +38,19 @@ type StatusUpdater interface { // Manager orchestrates all port-forward workers. // It handles starting, stopping, and hot-reloading forwards. type Manager struct { - workers map[string]*ForwardWorker // key: forward.ID() - workersMu sync.RWMutex + statusUI StatusUpdater + healthChecker *healthcheck.Checker clientPool *k8s.ClientPool resolver *k8s.ResourceResolver portForwarder *k8s.PortForwarder portChecker *PortChecker - healthChecker *healthcheck.Checker + workers map[string]*ForwardWorker watchdog *Watchdog mdnsPublisher *mdns.Publisher - eventBus *events.Bus // Event bus for decoupled communication - verbose bool + eventBus *events.Bus currentConfig *config.Config - statusUI StatusUpdater + workersMu sync.RWMutex + verbose bool } // NewManager creates a new forward Manager. @@ -414,11 +428,11 @@ func (m *Manager) startWorker(fwd config.Forward) error { // Find and notify the worker to reconnect m.workersMu.RLock() - worker, exists := m.workers[forwardID] + staleWorker, exists := m.workers[forwardID] m.workersMu.RUnlock() if exists { - worker.TriggerReconnect("stale connection") + staleWorker.TriggerReconnect("stale connection") } } }) diff --git a/internal/forward/manager_test.go b/internal/forward/manager_test.go index 0ee1b78..15379a8 100644 --- a/internal/forward/manager_test.go +++ b/internal/forward/manager_test.go @@ -185,8 +185,8 @@ type StatusUpdate struct { } type ForwardAdd struct { - ID string Fwd *config.Forward + ID string } type ErrorSet struct { diff --git a/internal/forward/portcheck.go b/internal/forward/portcheck.go index 1b154b1..5959adf 100644 --- a/internal/forward/portcheck.go +++ b/internal/forward/portcheck.go @@ -99,9 +99,9 @@ func getProcessNameByPIDWindows(pid string) string { // PortConflict represents a local port that is already in use. type PortConflict struct { - Port int // The conflicting port number - Resource string // The forward resource that needs this port - UsedBy string // Process information (PID, command) using the port + Resource string + UsedBy string + Port int } // PortChecker checks port availability on the local system. @@ -146,7 +146,7 @@ func (pc *PortChecker) isPortAvailable(port int) bool { if err != nil { return false } - _ = listener.Close() + _ = listener.Close() // Best-effort cleanup; port check succeeded, Close error is non-critical return true } diff --git a/internal/forward/portcheck_test.go b/internal/forward/portcheck_test.go index 6a6dc05..12774e6 100644 --- a/internal/forward/portcheck_test.go +++ b/internal/forward/portcheck_test.go @@ -40,8 +40,8 @@ func TestIsValidPID(t *testing.T) { func TestFormatProcessInfo(t *testing.T) { tests := []struct { name string - info processInfo expected string + info processInfo }{ { name: "invalid process", @@ -72,8 +72,8 @@ func TestFormatProcessInfo(t *testing.T) { func TestFormatProcessList(t *testing.T) { tests := []struct { name string - processes []processInfo expected string + processes []processInfo }{ { name: "empty list", @@ -206,7 +206,8 @@ func TestPortChecker_CheckAvailability_EmptyPorts(t *testing.T) { func TestPortChecker_CheckAvailability_ExcludeMap(t *testing.T) { pc := NewPortChecker() - // Create a listener to occupy a port + // Create a listener to occupy a port on all interfaces (matching production behavior) + // #nosec G102 -- test intentionally binds to all interfaces to match production port checking listener, err := net.Listen("tcp", ":0") assert.NoError(t, err, "should create listener") defer listener.Close() @@ -231,11 +232,13 @@ func TestPortChecker_CheckAvailability_ExcludeMap(t *testing.T) { func TestPortChecker_CheckAvailability_MultipleSkipPorts(t *testing.T) { pc := NewPortChecker() - // Create multiple listeners + // Create multiple listeners on all interfaces (matching production behavior) + // #nosec G102 -- test intentionally binds to all interfaces to match production port checking listener1, err := net.Listen("tcp", ":0") assert.NoError(t, err) defer listener1.Close() + // #nosec G102 -- test intentionally binds to all interfaces to match production port checking listener2, err := net.Listen("tcp", ":0") assert.NoError(t, err) defer listener2.Close() @@ -353,7 +356,8 @@ func TestNewPortChecker(t *testing.T) { func TestPortChecker_PortAvailability_Integration(t *testing.T) { pc := NewPortChecker() - // Create a listener to occupy a port + // Create a listener to occupy a port on all interfaces (matching production behavior) + // #nosec G102 -- test intentionally binds to all interfaces to match production port checking listener, err := net.Listen("tcp", ":0") assert.NoError(t, err, "should create listener") defer listener.Close() diff --git a/internal/forward/watchdog.go b/internal/forward/watchdog.go index b81fe9e..e31c5ee 100644 --- a/internal/forward/watchdog.go +++ b/internal/forward/watchdog.go @@ -19,25 +19,25 @@ const ( // the watchdog polls workers periodically. This reduces goroutine count and // simplifies worker implementation. type Watchdog struct { - mu sync.RWMutex - workers map[string]*workerState // key: forward ID - checkInterval time.Duration - hangThreshold time.Duration // How long without heartbeat before considered hung - heartbeatInterval time.Duration // How often to poll workers for heartbeat ctx context.Context + workers map[string]*workerState cancel context.CancelFunc + eventBus *events.Bus wg sync.WaitGroup - eventBus *events.Bus // Optional event bus for decoupled communication + checkInterval time.Duration + hangThreshold time.Duration + heartbeatInterval time.Duration + mu sync.RWMutex } // workerState tracks the health of a single worker type workerState struct { - forwardID string lastHeartbeat time.Time + worker HeartbeatResponder + onHungCallback func(forwardID string) + forwardID string heartbeatCount uint64 isHung bool - onHungCallback func(forwardID string) - worker HeartbeatResponder // Reference to worker for heartbeat polling } // HeartbeatResponder is an interface for workers that can respond to heartbeat checks @@ -204,8 +204,8 @@ func (w *Watchdog) pollHeartbeats() { // hungWorkerInfo stores information about a hung worker for deferred callback execution type hungWorkerInfo struct { - forwardID string callback func(string) + forwardID string } // checkWorkers checks all registered workers for hung state diff --git a/internal/forward/worker.go b/internal/forward/worker.go index 52a7fab..d181d05 100644 --- a/internal/forward/worker.go +++ b/internal/forward/worker.go @@ -23,23 +23,23 @@ const ( // ForwardWorker manages a single port-forward connection with automatic retry. type ForwardWorker struct { - forward config.Forward - portForwarder *k8s.PortForwarder - ctx context.Context - cancel context.CancelFunc - stopChan chan struct{} - doneChan chan struct{} - reconnectChan chan string // Channel to trigger reconnection - successChan chan struct{} // Channel to signal successful connection (for backoff reset) - verbose bool - lastPod string // Track the last pod we connected to + startTime time.Time statusUI StatusUpdater - healthChecker *healthcheck.Checker + ctx context.Context + reconnectChan chan string + httpProxy *httplog.Proxy watchdog *Watchdog - startTime time.Time // Track when the worker started - forwardCancel context.CancelFunc // Cancel function for current forward attempt - forwardCancelMu sync.Mutex // Protects forwardCancel - httpProxy *httplog.Proxy // HTTP logging proxy (nil if not enabled) + cancel context.CancelFunc + doneChan chan struct{} + portForwarder *k8s.PortForwarder + successChan chan struct{} + healthChecker *healthcheck.Checker + forwardCancel context.CancelFunc + stopChan chan struct{} + lastPod string + forward config.Forward + forwardCancelMu sync.Mutex + verbose bool } // NewForwardWorker creates a new ForwardWorker for a single forward configuration. @@ -142,7 +142,7 @@ func (w *ForwardWorker) run() { // Start HTTP logging proxy if enabled if err := w.startHTTPProxy(); err != nil { - logger.Error("Failed to start HTTP logging proxy", map[string]interface{}{ + logger.Error("Failed to start HTTP logging proxy", map[string]any{ "forward_id": w.forward.ID(), "error": err.Error(), }) @@ -175,7 +175,7 @@ func (w *ForwardWorker) run() { ) if err != nil { - logger.Error("Failed to resolve resource", map[string]interface{}{ + logger.Error("Failed to resolve resource", map[string]any{ "forward_id": w.forward.ID(), "context": w.forward.GetContext(), "namespace": w.forward.GetNamespace(), @@ -191,7 +191,7 @@ func (w *ForwardWorker) run() { if w.healthChecker != nil { w.healthChecker.MarkReconnecting(w.forward.ID()) } - logger.Info("Pod restart detected, switching to new pod", map[string]interface{}{ + logger.Info("Pod restart detected, switching to new pod", map[string]any{ "forward_id": w.forward.ID(), "old_pod": w.lastPod, "new_pod": podName, @@ -199,7 +199,7 @@ func (w *ForwardWorker) run() { "namespace": w.forward.GetNamespace(), }) } else if w.lastPod == "" { - logger.Info("Starting port forward", map[string]interface{}{ + logger.Info("Starting port forward", map[string]any{ "forward_id": w.forward.ID(), "target": w.forward.String(), "local_port": w.forward.LocalPort, @@ -228,7 +228,7 @@ func (w *ForwardWorker) run() { } // Log the error - logger.Warn("Port-forward connection failed, will retry", map[string]interface{}{ + logger.Warn("Port-forward connection failed, will retry", map[string]any{ "forward_id": w.forward.ID(), "context": w.forward.GetContext(), "namespace": w.forward.GetNamespace(), @@ -433,7 +433,7 @@ func (w *ForwardWorker) startHTTPProxy() error { w.httpProxy = proxy - logger.Info("HTTP logging proxy started", map[string]interface{}{ + logger.Info("HTTP logging proxy started", map[string]any{ "forward_id": w.forward.ID(), "local_port": w.forward.LocalPort, "target_port": targetPort, @@ -446,7 +446,7 @@ func (w *ForwardWorker) startHTTPProxy() error { func (w *ForwardWorker) stopHTTPProxy() { if w.httpProxy != nil { if err := w.httpProxy.Stop(); err != nil { - logger.Warn("Failed to stop HTTP proxy", map[string]interface{}{ + logger.Warn("Failed to stop HTTP proxy", map[string]any{ "forward_id": w.forward.ID(), "error": err.Error(), }) diff --git a/internal/forward/worker_unit_test.go b/internal/forward/worker_unit_test.go index 9b076de..8944631 100644 --- a/internal/forward/worker_unit_test.go +++ b/internal/forward/worker_unit_test.go @@ -55,8 +55,8 @@ func TestLogWriter_Write(t *testing.T) { func TestForwardWorker_GetForward(t *testing.T) { tests := []struct { name string - forward config.Forward description string + forward config.Forward }{ { name: "get pod forward", @@ -141,9 +141,9 @@ func TestForwardWorker_IsRunning(t *testing.T) { func TestForwardID(t *testing.T) { tests := []struct { name string + description string forward config.Forward expectUnique bool - description string }{ { name: "unique IDs for different forwards", @@ -183,9 +183,9 @@ func TestForwardID(t *testing.T) { func TestForwardString(t *testing.T) { tests := []struct { name string - forward config.Forward - expectedContains []string description string + expectedContains []string + forward config.Forward }{ { name: "pod forward string", @@ -259,8 +259,8 @@ func TestSleepWithBackoffConcept(t *testing.T) { func TestWorkerVerboseMode(t *testing.T) { tests := []struct { name string - verbose bool description string + verbose bool }{ { name: "verbose mode enabled", diff --git a/internal/healthcheck/checker.go b/internal/healthcheck/checker.go index 3fd39f0..0e05f24 100644 --- a/internal/healthcheck/checker.go +++ b/internal/healthcheck/checker.go @@ -1,3 +1,17 @@ +// Package healthcheck provides connection health monitoring for port-forwards. +// It detects stale, hung, or broken connections and triggers reconnection. +// +// The Checker supports two health check methods: +// - tcp-dial: Simple TCP connection test (fast but less reliable) +// - data-transfer: Attempts to read data from the connection (more reliable) +// +// Stale connection detection prevents issues during long-running operations +// like database dumps by monitoring: +// - Connection age (default: 25 minutes, before k8s 30-minute timeout) +// - Idle time (default: 10 minutes, detects hung tunnels) +// +// The package uses a sync.Pool for buffer reuse to minimize GC pressure +// during frequent health checks. package healthcheck import ( @@ -47,13 +61,13 @@ const ( // PortHealth represents the health status of a single port type PortHealth struct { - Port int LastCheck time.Time + RegisteredAt time.Time + ConnectionTime time.Time + LastActivity time.Time Status Status ErrorMessage string - RegisteredAt time.Time // When this port was registered - ConnectionTime time.Time // When current connection was established - LastActivity time.Time // Last time data was transferred + Port int } // StatusCallback is called when a port's health status changes @@ -63,26 +77,26 @@ type StatusCallback func(forwardID string, status Status, errorMsg string) // Uses a single goroutine to check all registered ports, reducing overhead // compared to one goroutine per port. type Checker struct { - mu sync.RWMutex - ports map[string]*PortHealth // key: forward ID - callbacks map[string]StatusCallback - interval time.Duration - timeout time.Duration - method CheckMethod - maxConnectionAge time.Duration - maxIdleTime time.Duration ctx context.Context + ports map[string]*PortHealth + callbacks map[string]StatusCallback + eventBus *events.Bus cancel context.CancelFunc + method CheckMethod wg sync.WaitGroup + interval time.Duration + maxIdleTime time.Duration + maxConnectionAge time.Duration + timeout time.Duration + mu sync.RWMutex started bool - eventBus *events.Bus // Optional event bus for decoupled communication } // CheckerOptions configures the health checker type CheckerOptions struct { + Method CheckMethod Interval time.Duration Timeout time.Duration - Method CheckMethod MaxConnectionAge time.Duration MaxIdleTime time.Duration } @@ -339,7 +353,10 @@ func (c *Checker) checkPort(forwardID string) { connectionAge.Round(time.Second), c.maxConnectionAge, idleTime.Round(time.Second)) } else if c.maxIdleTime > 0 && idleTime > c.maxIdleTime { newStatus = StatusStale - errorMsg = fmt.Sprintf("idle time %v exceeds max %v", idleTime.Round(time.Second), c.maxIdleTime) + // Round up to next second to ensure displayed time is always > max + // (avoids confusing "10m0s exceeds max 10m0s" when actual is 10m0.1s) + displayIdle := idleTime.Truncate(time.Second) + time.Second + errorMsg = fmt.Sprintf("idle time %v exceeds max %v", displayIdle, c.maxIdleTime) } else { // Perform connectivity check var checkErr error @@ -408,7 +425,7 @@ func (c *Checker) checkTCPDial(port int) error { if err != nil { return err } - _ = conn.Close() + _ = conn.Close() // Best-effort cleanup; health check succeeded return nil } diff --git a/internal/healthcheck/checker_test.go b/internal/healthcheck/checker_test.go index fc34499..ba273a8 100644 --- a/internal/healthcheck/checker_test.go +++ b/internal/healthcheck/checker_test.go @@ -88,9 +88,9 @@ func (s *HealthCheckTestSuite) TestRegisterAndUnregister() { func (s *HealthCheckTestSuite) TestTCPDialMethod() { tests := []struct { name string - setupPort bool expectedStatus Status description string + setupPort bool }{ { name: "port available - healthy", @@ -109,10 +109,9 @@ func (s *HealthCheckTestSuite) TestTCPDialMethod() { for _, tt := range tests { s.Run(tt.name, func() { var testPort int - var testListener net.Listener if tt.setupPort { - // Use the existing listener + // Use the existing listener from suite setup testPort = s.port } else { // Use a port that's not listening @@ -143,10 +142,6 @@ func (s *HealthCheckTestSuite) TestTCPDialMethod() { status, exists := checker.GetStatus("test-forward") assert.True(s.T(), exists) assert.Equal(s.T(), tt.expectedStatus, status, tt.description) - - if testListener != nil { - testListener.Close() - } }) } } @@ -201,7 +196,7 @@ func (s *HealthCheckTestSuite) TestDataTransferMethod() { } switch tt.serverBehavior { case "banner": - conn.Write([]byte("220 Welcome\r\n")) + _, _ = conn.Write([]byte("220 Welcome\r\n")) time.Sleep(50 * time.Millisecond) conn.Close() case "close": diff --git a/internal/httplog/logger.go b/internal/httplog/logger.go index 7250c6d..820bd2b 100644 --- a/internal/httplog/logger.go +++ b/internal/httplog/logger.go @@ -1,3 +1,15 @@ +// Package httplog provides HTTP request/response logging for port forwards. +// It captures HTTP traffic passing through the forward proxy and stores +// entries for viewing in the UI. +// +// The logger supports: +// - Request and response capture with headers and bodies +// - Configurable body size limits to prevent memory issues +// - Callback-based notifications for real-time log viewing +// - Thread-safe operation for concurrent forwards +// +// Bodies are truncated if they exceed the configured maximum size +// (default: 1MB) and marked as truncated in the log entry. package httplog import ( @@ -11,17 +23,17 @@ import ( // Entry represents a single HTTP log entry type Entry struct { Timestamp time.Time `json:"timestamp"` + Headers map[string]string `json:"headers,omitempty"` ForwardID string `json:"forward_id"` RequestID string `json:"request_id"` - Direction string `json:"direction"` // "request" or "response" + Direction string `json:"direction"` Method string `json:"method,omitempty"` Path string `json:"path,omitempty"` - StatusCode int `json:"status_code,omitempty"` - Headers map[string]string `json:"headers,omitempty"` - BodySize int `json:"body_size"` Body string `json:"body,omitempty"` - LatencyMs int64 `json:"latency_ms,omitempty"` Error string `json:"error,omitempty"` + StatusCode int `json:"status_code,omitempty"` + BodySize int `json:"body_size"` + LatencyMs int64 `json:"latency_ms,omitempty"` } // LogCallback is a function that receives log entries @@ -29,12 +41,12 @@ type LogCallback func(entry Entry) // Logger writes HTTP log entries to an output stream type Logger struct { - mu sync.Mutex output io.Writer - file *os.File // Only set if we opened the file ourselves + file *os.File forwardID string - maxBodyLen int callbacks []LogCallback + maxBodyLen int + mu sync.Mutex } // NewLogger creates a new HTTP logger diff --git a/internal/httplog/logger_test.go b/internal/httplog/logger_test.go index 0e32011..9ea2deb 100644 --- a/internal/httplog/logger_test.go +++ b/internal/httplog/logger_test.go @@ -166,15 +166,15 @@ func TestLogger_Log_Error(t *testing.T) { func TestLogger_BodyTruncation(t *testing.T) { tests := []struct { name string - maxBodyLen int body string + maxBodyLen int expectTrunc bool }{ - {"body under limit", 100, "short", false}, - {"body at limit", 5, "exact", false}, - {"body over limit", 5, "this is too long", true}, - {"empty body", 100, "", false}, - {"zero max", 0, "any", true}, + {name: "body under limit", maxBodyLen: 100, body: "short", expectTrunc: false}, + {name: "body at limit", maxBodyLen: 5, body: "exact", expectTrunc: false}, + {name: "body over limit", maxBodyLen: 5, body: "this is too long", expectTrunc: true}, + {name: "empty body", maxBodyLen: 100, body: "", expectTrunc: false}, + {name: "zero max", maxBodyLen: 0, body: "any", expectTrunc: true}, } for _, tt := range tests { @@ -186,10 +186,10 @@ func TestLogger_BodyTruncation(t *testing.T) { output: &buf, } - l.Log(Entry{Body: tt.body}) + _ = l.Log(Entry{Body: tt.body}) var entry Entry - json.Unmarshal(buf.Bytes(), &entry) + _ = json.Unmarshal(buf.Bytes(), &entry) if tt.expectTrunc { assert.Contains(t, entry.Body, "...(truncated)") @@ -219,9 +219,9 @@ func TestLogger_Callbacks(t *testing.T) { }) // Log entries - l.Log(Entry{Direction: "request", Path: "/api/1"}) - l.Log(Entry{Direction: "response", Path: "/api/1"}) - l.Log(Entry{Direction: "request", Path: "/api/2"}) + _ = l.Log(Entry{Direction: "request", Path: "/api/1"}) + _ = l.Log(Entry{Direction: "response", Path: "/api/1"}) + _ = l.Log(Entry{Direction: "request", Path: "/api/2"}) mu.Lock() assert.Len(t, received, 3) @@ -244,7 +244,7 @@ func TestLogger_MultipleCallbacks(t *testing.T) { l.AddCallback(func(entry Entry) { count1++ }) l.AddCallback(func(entry Entry) { count2++ }) - l.Log(Entry{}) + _ = l.Log(Entry{}) assert.Equal(t, 1, count1) assert.Equal(t, 1, count2) @@ -261,12 +261,12 @@ func TestLogger_ClearCallbacks(t *testing.T) { count := 0 l.AddCallback(func(entry Entry) { count++ }) - l.Log(Entry{}) + _ = l.Log(Entry{}) assert.Equal(t, 1, count) l.ClearCallbacks() - l.Log(Entry{}) + _ = l.Log(Entry{}) assert.Equal(t, 1, count) // Still 1 - callback was cleared } @@ -321,7 +321,7 @@ func TestLogger_Concurrent(t *testing.T) { wg.Add(1) go func(n int) { defer wg.Done() - l.Log(Entry{ + _ = l.Log(Entry{ Direction: "request", Path: "/api/" + string(rune('a'+n%26)), }) diff --git a/internal/httplog/proxy.go b/internal/httplog/proxy.go index 66fddc2..5367a4f 100644 --- a/internal/httplog/proxy.go +++ b/internal/httplog/proxy.go @@ -15,20 +15,21 @@ import ( "time" "github.com/nvm/kportal/internal/config" + "github.com/nvm/kportal/internal/logger" ) // Proxy is an HTTP reverse proxy with logging capabilities type Proxy struct { - localPort int // Port to listen on (user-facing) - targetPort int // Port to forward to (k8s tunnel) + listener net.Listener logger *Logger server *http.Server forwardID string - filterPath string // Glob pattern for path filtering - includeHdrs bool - listener net.Listener + filterPath string + localPort int + targetPort int requestCount uint64 mu sync.Mutex + includeHdrs bool running bool } @@ -100,7 +101,7 @@ func (p *Proxy) Start() error { // Start serving (blocking) go func() { if err := p.server.Serve(ln); err != nil && err != http.ErrServerClosed { - // Log error but don't crash - proxy will be replaced on reconnect + logger.Debug("HTTP proxy serve error (will be replaced on reconnect)", map[string]any{"error": err.Error()}) } }() diff --git a/internal/httplog/proxy_test.go b/internal/httplog/proxy_test.go index 544a002..3703414 100644 --- a/internal/httplog/proxy_test.go +++ b/internal/httplog/proxy_test.go @@ -331,7 +331,7 @@ func TestProxy_Start_PortInUse(t *testing.T) { } err := proxy1.Start() require.NoError(t, err) - defer proxy1.Stop() + defer func() { _ = proxy1.Stop() }() // Get the actual port addr := proxy1.listener.Addr().(*net.TCPAddr) @@ -353,9 +353,9 @@ func TestProxy_Start_PortInUse(t *testing.T) { // TestFlattenHeaders_EdgeCases tests header flattening edge cases func TestFlattenHeaders_EdgeCases(t *testing.T) { tests := []struct { - name string headers http.Header expected map[string]string + name string }{ { name: "empty headers", diff --git a/internal/k8s/client.go b/internal/k8s/client.go index 57e483b..a487822 100644 --- a/internal/k8s/client.go +++ b/internal/k8s/client.go @@ -1,3 +1,14 @@ +// Package k8s provides Kubernetes client management, resource resolution, +// and port-forwarding capabilities for kportal. +// +// Key components: +// - ClientPool: Thread-safe management of Kubernetes clients per context +// - ResourceResolver: Resolves pod/service/selector targets to actual pods +// - PortForwarder: Establishes and manages port-forward connections +// - Discovery: Provides resource discovery for the UI wizards +// +// The package handles automatic pod restart detection through re-resolution, +// caching with 30-second TTL, and graceful connection management. package k8s import ( @@ -12,10 +23,10 @@ import ( // ClientPool manages Kubernetes clients per context with thread-safe access. type ClientPool struct { - mu sync.RWMutex + loader clientcmd.ClientConfig clients map[string]*kubernetes.Clientset configs map[string]*rest.Config - loader clientcmd.ClientConfig + mu sync.RWMutex } // NewClientPool creates a new ClientPool instance. @@ -51,8 +62,8 @@ func (p *ClientPool) GetClient(contextName string) (*kubernetes.Clientset, error defer p.mu.Unlock() // Double-check in case another goroutine created it while we waited - if client, exists := p.clients[contextName]; exists { - return client, nil + if cachedClient, ok := p.clients[contextName]; ok { + return cachedClient, nil } // Create new client @@ -91,8 +102,8 @@ func (p *ClientPool) GetRestConfig(contextName string) (*rest.Config, error) { defer p.mu.Unlock() // Double-check in case another goroutine created it while we waited - if config, exists := p.configs[contextName]; exists { - return config, nil + if cachedConfig, ok := p.configs[contextName]; ok { + return cachedConfig, nil } // Create new config diff --git a/internal/k8s/client_test.go b/internal/k8s/client_test.go index 9c8b7d5..dc98011 100644 --- a/internal/k8s/client_test.go +++ b/internal/k8s/client_test.go @@ -146,8 +146,8 @@ func TestClientPool_ThreadSafety(t *testing.T) { go func() { pool.ClearCache() pool.RemoveContext("test-context") - pool.GetCurrentContext() - pool.ListContexts() + _, _ = pool.GetCurrentContext() + _, _ = pool.ListContexts() done <- true }() } diff --git a/internal/k8s/discovery.go b/internal/k8s/discovery.go index 5a30677..e26336d 100644 --- a/internal/k8s/discovery.go +++ b/internal/k8s/discovery.go @@ -28,11 +28,11 @@ func NewDiscovery(pool *ClientPool) *Discovery { // PodInfo contains information about a pod relevant for port forwarding. type PodInfo struct { + Created metav1.Time Name string Namespace string - Containers []ContainerInfo Status string - Created metav1.Time + Containers []ContainerInfo } // ContainerInfo contains information about a container within a pod. @@ -44,17 +44,17 @@ type ContainerInfo struct { // PortInfo describes a port exposed by a container or service. type PortInfo struct { Name string - Port int32 - TargetPort int32 // For services: the actual pod port to forward to Protocol string + Port int32 + TargetPort int32 } // ServiceInfo contains information about a service. type ServiceInfo struct { Name string Namespace string - Ports []PortInfo Type string + Ports []PortInfo } // ListContexts returns all available Kubernetes contexts from kubeconfig. diff --git a/internal/k8s/discovery_test.go b/internal/k8s/discovery_test.go index d202705..9073f61 100644 --- a/internal/k8s/discovery_test.go +++ b/internal/k8s/discovery_test.go @@ -14,12 +14,12 @@ import ( func TestResolveTargetPort(t *testing.T) { tests := []struct { - name string - servicePort corev1.ServicePort service *corev1.Service + name string + description string + servicePort corev1.ServicePort pods []corev1.Pod expectedPort int32 - description string }{ { name: "numeric targetPort", diff --git a/internal/k8s/portforward.go b/internal/k8s/portforward.go index 5ffd9d3..c0659f1 100644 --- a/internal/k8s/portforward.go +++ b/internal/k8s/portforward.go @@ -49,16 +49,16 @@ func (pf *PortForwarder) SetDialTimeout(timeout time.Duration) { // ForwardRequest contains the parameters for a port-forward request. type ForwardRequest struct { - ContextName string // Kubernetes context name - Namespace string // Namespace - Resource string // Resource (pod/name or service/name) - Selector string // Label selector (for pod resolution) - LocalPort int // Local port - RemotePort int // Remote port + Out io.Writer + ErrOut io.Writer StopChan chan struct{} ReadyChan chan struct{} - Out io.Writer // Output writer for logs - ErrOut io.Writer // Error output writer + ContextName string + Namespace string + Resource string + Selector string + LocalPort int + RemotePort int } // Forward establishes a port-forward connection to a Kubernetes resource. diff --git a/internal/k8s/resolver.go b/internal/k8s/resolver.go index fc2f6aa..bbc8fe0 100644 --- a/internal/k8s/resolver.go +++ b/internal/k8s/resolver.go @@ -19,15 +19,15 @@ const ( // ResolvedResource represents a resolved Kubernetes resource. type ResolvedResource struct { - Name string // The resolved pod or service name - Namespace string // The namespace - Timestamp time.Time // When this was resolved + Timestamp time.Time + Name string + Namespace string } // cacheEntry stores a cached resolution result with expiry. type cacheEntry struct { - resource ResolvedResource expiresAt time.Time + resource ResolvedResource } // ResourceResolver resolves Kubernetes resources with caching. @@ -188,7 +188,7 @@ func (r *ResourceResolver) getFromCache(key string) string { // Upgrade to write lock and delete expired entry r.cacheMu.Lock() // Double-check entry still exists and is still expired (may have been updated) - if entry, exists := r.cache[key]; exists && time.Now().After(entry.expiresAt) { + if expiredEntry, ok := r.cache[key]; ok && time.Now().After(expiredEntry.expiresAt) { delete(r.cache, key) } r.cacheMu.Unlock() diff --git a/internal/logger/klog_bridge_test.go b/internal/logger/klog_bridge_test.go index 29f05c5..ba9f69b 100644 --- a/internal/logger/klog_bridge_test.go +++ b/internal/logger/klog_bridge_test.go @@ -17,10 +17,10 @@ func TestKlogWriter(t *testing.T) { input string expectedLevel string expectedMsg string + description string loggerLevel Level loggerFormat Format shouldLog bool - description string }{ { name: "info level log", @@ -162,9 +162,9 @@ func TestKlogWriter(t *testing.T) { func TestKlogWriterBuffering(t *testing.T) { tests := []struct { name string + description string writes []string expectCount int - description string }{ { name: "single complete line", @@ -264,7 +264,7 @@ func TestKlogWriterConcurrency(t *testing.T) { go func(id int) { for j := 0; j < numWrites; j++ { msg := fmt.Sprintf("I1124 12:34:56.789012 12345 test.go:123] Message from goroutine %d iteration %d\n", id, j) - klogWriter.Write([]byte(msg)) + _, _ = klogWriter.Write([]byte(msg)) } done <- true }(i) diff --git a/internal/logger/klog_logr_test.go b/internal/logger/klog_logr_test.go index e3cedd9..f0fa016 100644 --- a/internal/logger/klog_logr_test.go +++ b/internal/logger/klog_logr_test.go @@ -14,12 +14,12 @@ import ( func TestLogrAdapter_Info(t *testing.T) { tests := []struct { name string - loggerLevel Level - logrLevel int message string keysAndValues []interface{} - expectOutput bool expectContains []string + loggerLevel Level + logrLevel int + expectOutput bool }{ { name: "info log v0 with debug logger", @@ -109,13 +109,13 @@ func TestLogrAdapter_Info(t *testing.T) { func TestLogrAdapter_Error(t *testing.T) { tests := []struct { - name string - loggerLevel Level err error + name string message string keysAndValues []interface{} - expectOutput bool expectContains []string + loggerLevel Level + expectOutput bool }{ { name: "error with error object", @@ -179,9 +179,9 @@ func TestLogrAdapter_Error(t *testing.T) { func TestLogrAdapter_WithName(t *testing.T) { tests := []struct { name string - loggerNames []string message string expectContains string + loggerNames []string }{ { name: "single logger name", diff --git a/internal/logger/logger.go b/internal/logger/logger.go index 917f022..6669725 100644 --- a/internal/logger/logger.go +++ b/internal/logger/logger.go @@ -1,3 +1,19 @@ +// Package logger provides structured logging with support for text and JSON +// output formats. It intercepts Kubernetes client-go logs and routes them +// through the structured logger. +// +// The package provides both instance-based and global logging: +// +// // Instance-based logging +// log := logger.New(logger.LevelInfo, logger.FormatJSON, os.Stderr) +// log.Info("message", "key", "value") +// +// // Global logging (after Init) +// logger.Init(logger.LevelInfo, logger.FormatText, os.Stderr) +// logger.Info("message", "key", "value") +// +// Log levels: DEBUG < INFO < WARN < ERROR +// Output formats: FormatText (human-readable), FormatJSON (structured) package logger import ( @@ -9,36 +25,50 @@ import ( "time" ) +// Level represents the logging level. +// Higher levels include all lower levels (e.g., LevelInfo includes WARN and ERROR). type Level int const ( + // LevelDebug is for detailed troubleshooting information. LevelDebug Level = iota + // LevelInfo is for general operational information. LevelInfo + // LevelWarn is for unexpected but handled situations. LevelWarn + // LevelError is for failures that require attention. LevelError ) +// Format represents the output format for log entries. type Format int const ( + // FormatText outputs human-readable log lines. FormatText Format = iota + // FormatJSON outputs structured JSON log entries. FormatJSON ) +// Logger is a structured logger with configurable level and format. +// It is safe for concurrent use. type Logger struct { + output io.Writer level Level format Format - output io.Writer - mu sync.Mutex // Protects concurrent writes to output + mu sync.Mutex } +// logEntry represents a single log entry for JSON output. type logEntry struct { - Time string `json:"time"` - Level string `json:"level"` - Message string `json:"message"` - Fields map[string]interface{} `json:"fields,omitempty"` + Fields map[string]any `json:"fields,omitempty"` + Time string `json:"time"` + Level string `json:"level"` + Message string `json:"message"` } +// New creates a new Logger with the specified level, format, and output writer. +// If output is nil, os.Stderr is used. func New(level Level, format Format, output io.Writer) *Logger { if output == nil { output = os.Stderr diff --git a/internal/logger/logger_test.go b/internal/logger/logger_test.go index 714ff4b..c507fd0 100644 --- a/internal/logger/logger_test.go +++ b/internal/logger/logger_test.go @@ -13,13 +13,13 @@ import ( func TestLoggerTextFormat(t *testing.T) { tests := []struct { + fields map[string]interface{} name string + message string + expectContains []string level Level logLevel Level - message string - fields map[string]interface{} expectOutput bool - expectContains []string }{ { name: "info logged at info level", @@ -138,13 +138,13 @@ func TestLoggerTextFormat(t *testing.T) { func TestLoggerJSONFormat(t *testing.T) { tests := []struct { + fields map[string]interface{} name string + message string + expectLevel string level Level logLevel Level - message string - fields map[string]interface{} expectOutput bool - expectLevel string }{ { name: "info logged at info level", @@ -268,12 +268,12 @@ func TestLoggerJSONFormat(t *testing.T) { func TestGlobalLogger(t *testing.T) { tests := []struct { - name string - initLevel Level - initFormat Format logFunc func(string, ...map[string]interface{}) + name string message string expectContains string + initLevel Level + initFormat Format }{ { name: "global info logger text", @@ -321,9 +321,9 @@ func TestGlobalLogger(t *testing.T) { func TestLogLevelsFiltering(t *testing.T) { tests := []struct { name string - loggerLevel Level logAtLevels []Level expectOutputs []bool + loggerLevel Level }{ { name: "debug level logs everything", @@ -387,14 +387,14 @@ func TestLoggerNilOutput(t *testing.T) { func TestLevelToString(t *testing.T) { tests := []struct { - level Level expected string + level Level }{ - {LevelDebug, "DEBUG"}, - {LevelInfo, "INFO"}, - {LevelWarn, "WARN"}, - {LevelError, "ERROR"}, - {Level(999), "UNKNOWN"}, + {level: LevelDebug, expected: "DEBUG"}, + {level: LevelInfo, expected: "INFO"}, + {level: LevelWarn, expected: "WARN"}, + {level: LevelError, expected: "ERROR"}, + {level: Level(999), expected: "UNKNOWN"}, } for _, tt := range tests { @@ -407,8 +407,8 @@ func TestLevelToString(t *testing.T) { func TestJSONFieldTypes(t *testing.T) { tests := []struct { - name string fields map[string]interface{} + name string }{ { name: "string fields", @@ -467,10 +467,10 @@ func TestJSONFieldTypes(t *testing.T) { func TestInitWithCustomOutput(t *testing.T) { tests := []struct { - name string output io.Writer - expectDiscard bool + name string description string + expectDiscard bool }{ { name: "init with custom buffer", diff --git a/internal/mdns/publisher.go b/internal/mdns/publisher.go index 26c8a98..53954ea 100644 --- a/internal/mdns/publisher.go +++ b/internal/mdns/publisher.go @@ -1,3 +1,16 @@ +// Package mdns provides multicast DNS (mDNS/Bonjour) hostname publishing +// for port forwards. When enabled, forwards with aliases can be accessed +// via .local hostnames on the local network. +// +// The Publisher manages mDNS service registrations using zeroconf: +// - Registers hostnames when forwards become active +// - Unregisters hostnames when forwards are stopped +// - Provides service discovery via the _kportal._tcp service type +// +// mDNS discovery commands: +// +// dns-sd -B _kportal._tcp local # macOS +// avahi-browse -t _kportal._tcp # Linux package mdns import ( @@ -23,11 +36,11 @@ const ( // Publisher manages mDNS hostname registrations for port forwards. // It allows forwards with aliases to be accessible via .local hostnames. type Publisher struct { - mu sync.RWMutex - servers map[string]*zeroconf.Server // forwardID -> server - aliases map[string]string // forwardID -> alias (for logging) - enabled bool + servers map[string]*zeroconf.Server + aliases map[string]string localIPs []string + mu sync.RWMutex + enabled bool } // NewPublisher creates a new mDNS Publisher. diff --git a/internal/retry/backoff.go b/internal/retry/backoff.go index 65dac49..881864d 100644 --- a/internal/retry/backoff.go +++ b/internal/retry/backoff.go @@ -1,3 +1,19 @@ +// Package retry provides exponential backoff with jitter for retry logic. +// It implements a backoff sequence of 1s → 2s → 4s → 8s → 10s (max), +// with 10% random jitter to prevent thundering herd problems. +// +// Basic usage: +// +// backoff := retry.NewBackoff() +// for { +// err := doSomething() +// if err == nil { +// backoff.Reset() +// break +// } +// delay := backoff.Next() +// time.Sleep(delay) +// } package retry import ( @@ -19,8 +35,8 @@ const ( // Backoff implements exponential backoff with jitter for retry logic. // The backoff sequence is: 1s → 2s → 4s → 8s → 10s (max, then stays at 10s). type Backoff struct { - attempt int rng *rand.Rand + attempt int } // NewBackoff creates a new Backoff instance with a seeded random number generator. @@ -53,7 +69,7 @@ func (b *Backoff) Next() time.Duration { // Add jitter (±10%) jitter := b.calculateJitter(delay) - delay = delay + jitter + delay += jitter b.attempt++ return delay diff --git a/internal/ui/bubbletea_ui.go b/internal/ui/bubbletea_ui.go index 46c4013..4097f59 100644 --- a/internal/ui/bubbletea_ui.go +++ b/internal/ui/bubbletea_ui.go @@ -1,3 +1,22 @@ +// Package ui provides the terminal user interface for kportal using bubbletea. +// It displays port-forward status in an interactive table and provides wizards +// for adding, editing, and removing forwards. +// +// The main components are: +// - BubbleTeaUI: The interactive TUI with table display and modal dialogs +// - TableUI: A simpler non-interactive status display for verbose mode +// - Wizards: Step-by-step interfaces for configuration changes +// - Controller: Coordinates UI with the forward manager +// +// Key bindings in the main view: +// - ↑↓/jk: Navigate forwards +// - Space: Toggle forward enabled/disabled +// - n: New forward wizard +// - e: Edit forward wizard +// - d: Delete forward +// - b: Benchmark forward +// - l: View HTTP logs +// - q: Quit package ui import ( @@ -35,8 +54,8 @@ type ForwardErrorMsg struct { // ForwardAddMsg is sent when a new forward is added type ForwardAddMsg struct { - ID string Forward *ForwardStatus + ID string } // ForwardRemoveMsg is sent when a forward is removed @@ -50,48 +69,32 @@ type HTTPLogSubscriber func(forwardID string, callback func(entry HTTPLogEntry)) // BubbleTeaUI is a bubbletea-based terminal UI type BubbleTeaUI struct { - mu sync.RWMutex - program *tea.Program - forwards map[string]*ForwardStatus - forwardOrder []string - selectedIndex int - disabledMap map[string]bool - toggleCallback func(id string, enable bool) - version string - errors map[string]string // Track error messages by forward ID - - // Update notification - updateAvailable bool - updateVersion string - updateURL string - - // Modal wizard state - viewMode ViewMode - addWizard *AddWizardState - removeWizard *RemoveWizardState - - // Delete confirmation state - deleteConfirming bool + discovery *k8s.Discovery + program *tea.Program + forwards map[string]*ForwardStatus + benchmarkState *BenchmarkState + httpLogSubscriber HTTPLogSubscriber + disabledMap map[string]bool + toggleCallback func(id string, enable bool) + httpLogCleanup func() + httpLogState *HTTPLogState + errors map[string]string + mutator *config.Mutator + removeWizard *RemoveWizardState + addWizard *AddWizardState + updateVersion string + updateURL string + configPath string deleteConfirmID string deleteConfirmAlias string - deleteConfirmCursor int // 0 = Yes, 1 = No - - // Benchmark state - benchmarkState *BenchmarkState - - // HTTP log viewing state - httpLogState *HTTPLogState - - // Log callback cleanup function - httpLogCleanup func() - - // Dependencies for wizards - discovery *k8s.Discovery - mutator *config.Mutator - configPath string - - // Manager for accessing workers - httpLogSubscriber HTTPLogSubscriber + version string + forwardOrder []string + viewMode ViewMode + deleteConfirmCursor int + selectedIndex int + mu sync.RWMutex + deleteConfirming bool + updateAvailable bool } // bubbletea model @@ -168,6 +171,8 @@ func (ui *BubbleTeaUI) AddForward(id string, fwd *config.Forward) { if existing, ok := ui.forwards[id]; ok { existing.Status = "Starting" ui.disabledMap[id] = false + // Clear any previous error when re-enabling + delete(ui.errors, id) ui.mu.Unlock() if ui.program != nil { @@ -176,15 +181,12 @@ func (ui *BubbleTeaUI) AddForward(id string, fwd *config.Forward) { return } - // Parse resource + // Parse resource (e.g., "pod/my-app" -> type="pod", name="my-app") resourceType := "pod" resourceName := fwd.Resource - for idx := 0; idx < len(fwd.Resource); idx++ { - if fwd.Resource[idx] == '/' { - resourceType = fwd.Resource[:idx] - resourceName = fwd.Resource[idx+1:] - break - } + if parts := strings.SplitN(fwd.Resource, "/", 2); len(parts) == 2 { + resourceType = parts[0] + resourceName = parts[1] } alias := fwd.Alias @@ -380,10 +382,10 @@ func (m model) View() string { // Fallback to reasonable defaults if dimensions not yet received if termWidth == 0 { - termWidth = 120 + termWidth = DefaultTermWidth } if termHeight == 0 { - termHeight = 40 + termHeight = DefaultTermHeight } // Overlay delete confirmation if active @@ -411,28 +413,98 @@ func (m model) View() string { } } +// mainViewColors holds the color palette for the main view +type mainViewColors struct { + header lipgloss.Color + active lipgloss.Color + warning lipgloss.Color + errorColor lipgloss.Color + muted lipgloss.Color + selectedBg lipgloss.Color + selectedFg lipgloss.Color +} + +// defaultMainViewColors returns the default color palette +func defaultMainViewColors() mainViewColors { + return mainViewColors{ + header: lipgloss.Color("220"), // Yellow + active: lipgloss.Color("46"), // Green + warning: lipgloss.Color("220"), // Yellow + errorColor: lipgloss.Color("196"), // Red + muted: lipgloss.Color("240"), // Gray + selectedBg: lipgloss.Color("240"), // Gray background + selectedFg: lipgloss.Color("230"), // Light foreground + } +} + +// keyBinding represents a keyboard shortcut and its description +type keyBinding struct { + key string + desc string +} + +// mainViewKeyBindings returns the key bindings for the main view +func mainViewKeyBindings() []keyBinding { + return []keyBinding{ + {"↑↓/jk", "Navigate"}, + {"Space", "Toggle"}, + {"n", "New"}, + {"e", "Edit"}, + {"d", "Delete"}, + {"b", "Bench"}, + {"l", "Logs"}, + {"q", "Quit"}, + } +} + func (m model) renderMainView() string { m.ui.mu.RLock() defer m.ui.mu.RUnlock() var b strings.Builder + colors := defaultMainViewColors() // Get terminal dimensions for proper sizing - termHeight := m.termHeight - if termHeight == 0 { - termHeight = 40 // Fallback + termWidth, termHeight := m.getTermDimensions() + + // Render title header + b.WriteString(m.renderTitle(colors.header)) + + // Render forwards table or empty message + if len(m.ui.forwardOrder) == 0 { + b.WriteString(m.renderEmptyMessage(colors.muted)) + } else { + b.WriteString(m.renderForwardsTable(colors)) } - // Color palette - headerColor := lipgloss.Color("220") // Yellow - activeColor := lipgloss.Color("46") // Green - warningColor := lipgloss.Color("220") // Yellow - errorColor := lipgloss.Color("196") // Red - mutedColor := lipgloss.Color("240") // Gray - selectedBg := lipgloss.Color("240") // Gray background - selectedFg := lipgloss.Color("230") // Light foreground + // Render error section if any errors exist + if len(m.ui.errors) > 0 { + b.WriteString(m.renderErrorSection()) + } + + // Render footer with proper spacing + b.WriteString(m.renderFooterWithSpacing(termWidth, termHeight, &b)) + + return b.String() +} + +// getTermDimensions returns terminal dimensions with fallback defaults +func (m model) getTermDimensions() (width, height int) { + width = m.termWidth + height = m.termHeight + if width == 0 { + width = DefaultTermWidth + } + if height == 0 { + height = DefaultTermHeight + } + return +} + +// renderTitle renders the title bar with version and optional update notification +func (m model) renderTitle(headerColor lipgloss.Color) string { + var b strings.Builder - // Title with version titleStyle := lipgloss.NewStyle(). Bold(true). Foreground(headerColor). @@ -451,180 +523,222 @@ func (m model) renderMainView() string { } b.WriteString("\n\n") - // No forwards - if len(m.ui.forwardOrder) == 0 { - disabledStyle := lipgloss.NewStyle().Foreground(mutedColor) - b.WriteString(disabledStyle.Render("No forwards configured\n")) - } else { - // Build table rows - var rows [][]string - for _, id := range m.ui.forwardOrder { + return b.String() +} + +// renderEmptyMessage renders the message shown when no forwards are configured +func (m model) renderEmptyMessage(mutedColor lipgloss.Color) string { + disabledStyle := lipgloss.NewStyle().Foreground(mutedColor) + return disabledStyle.Render("No forwards configured\n") +} + +// renderForwardsTable renders the forwards table with all styling +func (m model) renderForwardsTable(colors mainViewColors) string { + var b strings.Builder + + // Build table rows + rows := m.buildTableRows() + + // Create table with styling (no borders for cleaner look) + t := table.New(). + Border(lipgloss.HiddenBorder()). + Headers("CONTEXT", "NAMESPACE", "ALIAS", "TYPE", "RESOURCE", "REMOTE", "LOCAL", "STATUS"). + Rows(rows...). + StyleFunc(m.createTableStyleFunc(colors)) + + b.WriteString(t.Render()) + b.WriteString("\n") + + return b.String() +} + +// buildTableRows builds the data rows for the forwards table +func (m model) buildTableRows() [][]string { + var rows [][]string + + for _, id := range m.ui.forwardOrder { + fwd, ok := m.ui.forwards[id] + if !ok { + continue + } + + statusIcon, statusText := m.getStatusIconAndText(id, fwd) + + rows = append(rows, []string{ + truncate(fwd.Context, ColumnWidthContext), + truncate(fwd.Namespace, ColumnWidthNamespace), + truncate(fwd.Alias, ColumnWidthAlias), + truncate(fwd.Type, ColumnWidthType), + truncate(fwd.Resource, ColumnWidthResource), + fmt.Sprintf("%d", fwd.RemotePort), + fmt.Sprintf("%d", fwd.LocalPort), + statusIcon + " " + statusText, + }) + } + + return rows +} + +// getStatusIconAndText returns the appropriate status icon and text for a forward +func (m model) getStatusIconAndText(id string, fwd *ForwardStatus) (icon, text string) { + icon = "●" + text = fwd.Status + + if m.ui.isForwardDisabled(id) { + return "○", "Disabled" + } + + switch fwd.Status { + case "Starting": + icon = "○" + case "Reconnecting": + icon = "◐" + case "Error": + icon = "✗" + } + + return icon, text +} + +// createTableStyleFunc creates the style function for the forwards table +func (m model) createTableStyleFunc(colors mainViewColors) func(row, col int) lipgloss.Style { + return func(row, col int) lipgloss.Style { + // Header row + if row == table.HeaderRow { + return lipgloss.NewStyle(). + Bold(true). + Foreground(colors.header). + Padding(0, 1) + } + + baseStyle := lipgloss.NewStyle().Padding(0, 1) + + if row >= 0 && row < len(m.ui.forwardOrder) { + id := m.ui.forwardOrder[row] fwd, ok := m.ui.forwards[id] - if !ok { - continue + isSelected := row == m.ui.selectedIndex + isDisabled := m.ui.isForwardDisabled(id) + + // Selected row gets background highlight + if isSelected { + return baseStyle. + Background(colors.selectedBg). + Foreground(colors.selectedFg) } - isDisabled := m.ui.disabledMap[id] || fwd.Status == "Disabled" - - // Status icon and text - statusIcon := "●" - statusText := fwd.Status - + // Disabled rows are muted if isDisabled { - statusIcon = "○" - statusText = "Disabled" - } else { + return baseStyle.Foreground(colors.muted) + } + + // Status column gets colored based on status + if col == ColumnStatus && ok { switch fwd.Status { - case "Starting": - statusIcon = "○" - case "Reconnecting": - statusIcon = "◐" + case "Active": + return baseStyle.Foreground(colors.active) + case "Starting", "Reconnecting": + return baseStyle.Foreground(colors.warning) case "Error": - statusIcon = "✗" - } - } - - rows = append(rows, []string{ - truncate(fwd.Context, 14), - truncate(fwd.Namespace, 16), - truncate(fwd.Alias, 18), - truncate(fwd.Type, 8), - truncate(fwd.Resource, 20), - fmt.Sprintf("%d", fwd.RemotePort), - fmt.Sprintf("%d", fwd.LocalPort), - statusIcon + " " + statusText, - }) - } - - // Create table with styling (no borders for cleaner look) - t := table.New(). - Border(lipgloss.HiddenBorder()). - Headers("CONTEXT", "NAMESPACE", "ALIAS", "TYPE", "RESOURCE", "REMOTE", "LOCAL", "STATUS"). - Rows(rows...). - StyleFunc(func(row, col int) lipgloss.Style { - // Header row - if row == table.HeaderRow { - return lipgloss.NewStyle(). - Bold(true). - Foreground(headerColor). - Padding(0, 1) - } - - // Get the forward for this row to check its status - baseStyle := lipgloss.NewStyle().Padding(0, 1) - - if row >= 0 && row < len(m.ui.forwardOrder) { - id := m.ui.forwardOrder[row] - fwd, ok := m.ui.forwards[id] - isSelected := row == m.ui.selectedIndex - isDisabled := m.ui.disabledMap[id] || (ok && fwd.Status == "Disabled") - - // Selected row gets background highlight - if isSelected { - return baseStyle. - Background(selectedBg). - Foreground(selectedFg) - } - - // Disabled rows are muted - if isDisabled { - return baseStyle.Foreground(mutedColor) - } - - // Status column gets colored based on status - if col == 7 && ok { // STATUS column - switch fwd.Status { - case "Active": - return baseStyle.Foreground(activeColor) - case "Starting", "Reconnecting": - return baseStyle.Foreground(warningColor) - case "Error": - return baseStyle.Foreground(errorColor) - } - } - } - - return baseStyle - }) - - b.WriteString(t.Render()) - b.WriteString("\n") - } - - // Display errors if any (before footer) - if len(m.ui.errors) > 0 { - b.WriteString("\n\n") - errorHeaderStyle := lipgloss.NewStyle(). - Bold(true). - Foreground(lipgloss.Color("196")) - - b.WriteString(errorHeaderStyle.Render("Errors:")) - b.WriteString("\n") - - errorLineStyle := lipgloss.NewStyle(). - Foreground(lipgloss.Color("196")). - Width(118). // Slightly less than table width (120) for padding - MaxWidth(118) - - for id, errMsg := range m.ui.errors { - // Find the forward to display its alias - if fwd, ok := m.ui.forwards[id]; ok { - // Format: " • alias: error message" - prefix := fmt.Sprintf(" • %s: ", fwd.Alias) - - // Wrap the error message if it's too long - // Max line length is 118, subtract prefix length - maxErrLen := 118 - len(prefix) - wrappedMsg := wrapText(errMsg, maxErrLen) - - // Render first line with prefix - lines := strings.Split(wrappedMsg, "\n") - if len(lines) > 0 { - b.WriteString(errorLineStyle.Render(prefix + lines[0])) - b.WriteString("\n") - - // Render subsequent lines with indentation - indent := strings.Repeat(" ", len(prefix)) - for i := 1; i < len(lines); i++ { - b.WriteString(errorLineStyle.Render(indent + lines[i])) - b.WriteString("\n") - } + return baseStyle.Foreground(colors.errorColor) } } } + + return baseStyle } +} + +// renderErrorSection renders the error display section +func (m model) renderErrorSection() string { + var b strings.Builder + + b.WriteString("\n\n") + errorHeaderStyle := lipgloss.NewStyle(). + Bold(true). + Foreground(lipgloss.Color("196")) + + b.WriteString(errorHeaderStyle.Render("Errors:")) + b.WriteString("\n") + + errorLineStyle := lipgloss.NewStyle(). + Foreground(lipgloss.Color("196")). + Width(ErrorDisplayWidth). + MaxWidth(ErrorDisplayWidth) + + for id, errMsg := range m.ui.errors { + // Find the forward to display its alias + if fwd, ok := m.ui.forwards[id]; ok { + b.WriteString(m.renderErrorLine(fwd.Alias, errMsg, errorLineStyle)) + } + } + + return b.String() +} + +// renderErrorLine renders a single error line with proper wrapping +func (m model) renderErrorLine(alias, errMsg string, style lipgloss.Style) string { + var b strings.Builder + + // Format: " • alias: error message" + prefix := fmt.Sprintf(" • %s: ", alias) + + // Wrap the error message if it's too long + maxErrLen := ErrorDisplayWidth - len(prefix) + wrappedMsg := wrapText(errMsg, maxErrLen) + + // Render first line with prefix + lines := strings.Split(wrappedMsg, "\n") + if len(lines) > 0 { + b.WriteString(style.Render(prefix + lines[0])) + b.WriteString("\n") + + // Render subsequent lines with indentation + indent := strings.Repeat(" ", len(prefix)) + for i := 1; i < len(lines); i++ { + b.WriteString(style.Render(indent + lines[i])) + b.WriteString("\n") + } + } + + return b.String() +} + +// renderFooterWithSpacing renders the footer with proper vertical spacing +func (m model) renderFooterWithSpacing(termWidth, termHeight int, content *strings.Builder) string { + var b strings.Builder // Calculate current content height - currentContent := b.String() + currentContent := content.String() currentLines := strings.Count(currentContent, "\n") + 1 - // Footer styles + // Build footer content + footerLines := m.buildFooterLines(termWidth) + + // Calculate footer height and add spacing + footerHeight := len(footerLines) + 1 // +1 for the blank line before footer + remainingLines := termHeight - currentLines - footerHeight + if remainingLines > 0 { + b.WriteString(strings.Repeat("\n", remainingLines)) + } + + // Add footer at bottom footerStyle := lipgloss.NewStyle().Foreground(lipgloss.Color("240")) + b.WriteString("\n") + for i, line := range footerLines { + if i > 0 { + b.WriteString("\n") + } + b.WriteString(footerStyle.Render(line)) + } + + return b.String() +} + +// buildFooterLines builds the footer lines that fit within terminal width +func (m model) buildFooterLines(termWidth int) []string { keyStyle := lipgloss.NewStyle().Foreground(lipgloss.Color("220")) + bindings := mainViewKeyBindings() - // Get terminal width for footer wrapping - termWidth := m.termWidth - if termWidth == 0 { - termWidth = 120 - } - - // Define key bindings as structured data for flexible rendering - type keyBinding struct { - key string - desc string - } - bindings := []keyBinding{ - {"↑↓/jk", "Navigate"}, - {"Space", "Toggle"}, - {"n", "New"}, - {"e", "Edit"}, - {"d", "Delete"}, - {"b", "Bench"}, - {"l", "Logs"}, - {"q", "Quit"}, - } - - // Build footer lines that fit within terminal width var footerLines []string var currentLine strings.Builder currentLineVisualLen := 0 @@ -676,23 +790,7 @@ func (m model) renderMainView() string { currentLine.WriteString(totalSuffix) footerLines = append(footerLines, currentLine.String()) - // Calculate footer height - footerHeight := len(footerLines) + 1 // +1 for the blank line before footer - remainingLines := termHeight - currentLines - footerHeight - if remainingLines > 0 { - b.WriteString(strings.Repeat("\n", remainingLines)) - } - - // Add footer at bottom - b.WriteString("\n") - for i, line := range footerLines { - if i > 0 { - b.WriteString("\n") - } - b.WriteString(footerStyle.Render(line)) - } - - return b.String() + return footerLines } // wrapText wraps text to the specified width, breaking at word boundaries @@ -835,3 +933,18 @@ func (ui *BubbleTeaUI) toggleSelected() { go ui.toggleCallback(selectedID, !newState) // enable is inverse of disabled } } + +// isForwardDisabled checks if a forward is disabled. +// A forward is considered disabled if either: +// 1. The user has disabled it via the UI (tracked in disabledMap) +// 2. The forward's status is "Disabled" (from the manager) +// Caller must hold ui.mu.RLock or ui.mu.Lock. +func (ui *BubbleTeaUI) isForwardDisabled(id string) bool { + if ui.disabledMap[id] { + return true + } + if fwd, ok := ui.forwards[id]; ok && fwd.Status == "Disabled" { + return true + } + return false +} diff --git a/internal/ui/bubbletea_ui_test.go b/internal/ui/bubbletea_ui_test.go index 8152037..ee2e4a8 100644 --- a/internal/ui/bubbletea_ui_test.go +++ b/internal/ui/bubbletea_ui_test.go @@ -243,9 +243,9 @@ func TestBubbleTeaUI_Remove_ClearsErrors(t *testing.T) { func TestBubbleTeaUI_Remove_AdjustsSelectedIndex(t *testing.T) { tests := []struct { name string + removeID string forwards []string selectedIndex int - removeID string expectedIndex int expectedRemaining int }{ @@ -527,3 +527,256 @@ func TestBubbleTeaUI_ResetDeleteConfirmation(t *testing.T) { assert.Empty(t, ui.deleteConfirmAlias) assert.Equal(t, 0, ui.deleteConfirmCursor) } + +// TestBubbleTeaUI_IsForwardDisabled tests the disabled state helper +func TestBubbleTeaUI_IsForwardDisabled(t *testing.T) { + tests := []struct { + name string + forwardStatus string + disabledMap bool + expectedResult bool + }{ + { + name: "not disabled in map, Active status", + disabledMap: false, + forwardStatus: "Active", + expectedResult: false, + }, + { + name: "disabled in map, Active status", + disabledMap: true, + forwardStatus: "Active", + expectedResult: true, + }, + { + name: "not disabled in map, Disabled status", + disabledMap: false, + forwardStatus: "Disabled", + expectedResult: true, + }, + { + name: "both disabled in map and Disabled status", + disabledMap: true, + forwardStatus: "Disabled", + expectedResult: true, + }, + { + name: "not disabled in map, Error status", + disabledMap: false, + forwardStatus: "Error", + expectedResult: false, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + ui := NewBubbleTeaUI(nil, "1.0.0") + + fwd := &config.Forward{ + Resource: "pod/my-app", + Port: 8080, + LocalPort: 8080, + } + ui.AddForward("test-id", fwd) + + ui.mu.Lock() + ui.disabledMap["test-id"] = tt.disabledMap + ui.forwards["test-id"].Status = tt.forwardStatus + ui.mu.Unlock() + + ui.mu.RLock() + result := ui.isForwardDisabled("test-id") + ui.mu.RUnlock() + + assert.Equal(t, tt.expectedResult, result) + }) + } +} + +// TestBubbleTeaUI_IsForwardDisabled_NonExistent tests disabled check for non-existent forward +func TestBubbleTeaUI_IsForwardDisabled_NonExistent(t *testing.T) { + ui := NewBubbleTeaUI(nil, "1.0.0") + + ui.mu.RLock() + result := ui.isForwardDisabled("non-existent") + ui.mu.RUnlock() + + assert.False(t, result, "Non-existent forward should not be disabled") +} + +// TestBubbleTeaUI_AddForward_ReEnableClearsError tests that re-enabling clears previous errors +func TestBubbleTeaUI_AddForward_ReEnableClearsError(t *testing.T) { + ui := NewBubbleTeaUI(nil, "1.0.0") + + fwd := &config.Forward{ + Resource: "pod/my-app", + Port: 8080, + LocalPort: 8080, + } + + // Add forward + ui.AddForward("test-id", fwd) + + // Set error and disable + ui.SetError("test-id", "connection refused") + ui.mu.Lock() + ui.disabledMap["test-id"] = true + ui.forwards["test-id"].Status = "Disabled" + ui.mu.Unlock() + + // Verify error exists + ui.mu.RLock() + _, hasError := ui.errors["test-id"] + ui.mu.RUnlock() + assert.True(t, hasError, "Error should exist before re-enable") + + // Re-enable (re-add) + ui.AddForward("test-id", fwd) + + // Verify error is cleared + ui.mu.RLock() + _, hasError = ui.errors["test-id"] + ui.mu.RUnlock() + assert.False(t, hasError, "Error should be cleared after re-enable") +} + +// TestWrapText tests the text wrapping function +func TestWrapText(t *testing.T) { + tests := []struct { + name string + text string + expected string + width int + }{ + { + name: "short text fits", + text: "hello world", + width: 20, + expected: "hello world", + }, + { + name: "single long word", + text: "superlongwordthatexceedswidth", + width: 10, + expected: "superlongwordthatexceedswidth", + }, + { + name: "wraps at word boundary", + text: "hello world this is a test", + width: 15, + expected: "hello world\nthis is a test", + }, + { + name: "multiple wraps", + text: "one two three four five six", + width: 10, + expected: "one two\nthree four\nfive six", + }, + { + name: "empty string", + text: "", + width: 10, + expected: "", + }, + { + name: "single word", + text: "hello", + width: 10, + expected: "hello", + }, + { + name: "exact width", + text: "hello wor", + width: 9, + expected: "hello wor", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + result := wrapText(tt.text, tt.width) + assert.Equal(t, tt.expected, result) + }) + } +} + +// TestBubbleTeaUI_AddForward_ResourceParsing tests various resource format parsing +func TestBubbleTeaUI_AddForward_ResourceParsing(t *testing.T) { + tests := []struct { + name string + resource string + expectedType string + expectedName string + }{ + { + name: "pod with prefix", + resource: "pod/my-app", + expectedType: "pod", + expectedName: "my-app", + }, + { + name: "service resource", + resource: "service/postgres", + expectedType: "service", + expectedName: "postgres", + }, + { + name: "deployment resource", + resource: "deployment/api-server", + expectedType: "deployment", + expectedName: "api-server", + }, + { + name: "no type prefix (pod default)", + resource: "my-pod", + expectedType: "pod", + expectedName: "my-pod", + }, + { + name: "resource with multiple slashes", + resource: "custom/type/resource", + expectedType: "custom", + expectedName: "type/resource", + }, + { + name: "empty resource", + resource: "", + expectedType: "pod", + expectedName: "", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + ui := NewBubbleTeaUI(nil, "1.0.0") + + fwd := &config.Forward{ + Resource: tt.resource, + Port: 8080, + LocalPort: 8080, + } + ui.AddForward("test-id", fwd) + + ui.mu.RLock() + status := ui.forwards["test-id"] + ui.mu.RUnlock() + + assert.Equal(t, tt.expectedType, status.Type) + assert.Equal(t, tt.expectedName, status.Resource) + }) + } +} + +// TestConstants tests that UI constants are properly defined +func TestConstants(t *testing.T) { + assert.Equal(t, 120, DefaultTermWidth) + assert.Equal(t, 40, DefaultTermHeight) + assert.Equal(t, 7, ColumnStatus) + assert.Equal(t, 14, ColumnWidthContext) + assert.Equal(t, 16, ColumnWidthNamespace) + assert.Equal(t, 18, ColumnWidthAlias) + assert.Equal(t, 8, ColumnWidthType) + assert.Equal(t, 20, ColumnWidthResource) + assert.Equal(t, 118, ErrorDisplayWidth) + assert.Equal(t, 20, ViewportHeight) +} diff --git a/internal/ui/commands_test.go b/internal/ui/commands_test.go index a0d5d56..a0b4435 100644 --- a/internal/ui/commands_test.go +++ b/internal/ui/commands_test.go @@ -82,13 +82,16 @@ func TestMessageTypes(t *testing.T) { } assert.Equal(t, 8080, availableMsg.port) assert.True(t, availableMsg.available) + assert.Equal(t, "Port 8080 available", availableMsg.message) unavailableMsg := PortCheckedMsg{ port: 8080, available: false, message: "Port 8080 in use by process", } + assert.Equal(t, 8080, unavailableMsg.port) assert.False(t, unavailableMsg.available) + assert.Equal(t, "Port 8080 in use by process", unavailableMsg.message) }) t.Run("ForwardSavedMsg", func(t *testing.T) { @@ -117,10 +120,10 @@ func TestMessageTypes(t *testing.T) { t.Run("BenchmarkCompleteMsg", func(t *testing.T) { msg := BenchmarkCompleteMsg{ ForwardID: "fwd-123", - Results: nil, - Error: nil, } assert.Equal(t, "fwd-123", msg.ForwardID) + assert.Nil(t, msg.Results) + assert.Nil(t, msg.Error) }) t.Run("BenchmarkProgressMsg", func(t *testing.T) { @@ -256,7 +259,7 @@ func TestRunBenchmarkCmd_Cancellation(t *testing.T) { // Run with timeout to prevent hanging done := make(chan bool, 1) - var msg interface{} + var msg any go func() { msg = cmd() done <- true diff --git a/internal/ui/constants.go b/internal/ui/constants.go new file mode 100644 index 0000000..0121779 --- /dev/null +++ b/internal/ui/constants.go @@ -0,0 +1,45 @@ +package ui + +// Terminal dimension constants +const ( + // DefaultTermWidth is the fallback terminal width when not detected + DefaultTermWidth = 120 + + // DefaultTermHeight is the fallback terminal height when not detected + DefaultTermHeight = 40 +) + +// Table column constants +const ( + // Column indices in the forwards table + ColumnContext = 0 + ColumnNamespace = 1 + ColumnAlias = 2 + ColumnType = 3 + ColumnResource = 4 + ColumnRemote = 5 + ColumnLocal = 6 + ColumnStatus = 7 + + // Column widths for truncation + ColumnWidthContext = 14 + ColumnWidthNamespace = 16 + ColumnWidthAlias = 18 + ColumnWidthType = 8 + ColumnWidthResource = 20 + + // Error display widths + ErrorDisplayWidth = 118 // Slightly less than table width (120) for padding +) + +// Viewport constants +const ( + // ViewportHeight is the number of items visible in list views + ViewportHeight = 20 +) + +// Path display constants +const ( + // MaxPathWidth is the maximum width for displaying file paths + MaxPathWidth = 48 +) diff --git a/internal/ui/handlers_test.go b/internal/ui/handlers_test.go index 32c4ae5..a24abf1 100644 --- a/internal/ui/handlers_test.go +++ b/internal/ui/handlers_test.go @@ -695,12 +695,12 @@ func TestHandleSelectorValidated(t *testing.T) { func TestHandlePortChecked(t *testing.T) { tests := []struct { name string - available bool expectStep AddWizardStep + available bool expectError bool }{ - {"port available", true, StepConfirmation, false}, - {"port in use", false, StepEnterLocalPort, true}, + {name: "port available", available: true, expectStep: StepConfirmation, expectError: false}, + {name: "port in use", available: false, expectStep: StepEnterLocalPort, expectError: true}, } for _, tt := range tests { diff --git a/internal/ui/httplog_state_test.go b/internal/ui/httplog_state_test.go index a7ebf6d..8232542 100644 --- a/internal/ui/httplog_state_test.go +++ b/internal/ui/httplog_state_test.go @@ -180,13 +180,13 @@ func TestHTTPLogState_GetFilterModeLabel(t *testing.T) { state := newHTTPLogState("fwd", "alias") tests := []struct { - mode HTTPLogFilterMode expected string + mode HTTPLogFilterMode }{ - {HTTPLogFilterNone, "All"}, - {HTTPLogFilterText, "Text"}, - {HTTPLogFilterNon200, "Non-2xx"}, - {HTTPLogFilterErrors, "Errors (4xx/5xx)"}, + {mode: HTTPLogFilterNone, expected: "All"}, + {mode: HTTPLogFilterText, expected: "Text"}, + {mode: HTTPLogFilterNon200, expected: "Non-2xx"}, + {mode: HTTPLogFilterErrors, expected: "Errors (4xx/5xx)"}, } for _, tt := range tests { diff --git a/internal/ui/mocks_test.go b/internal/ui/mocks_test.go index 7a39929..8db2e90 100644 --- a/internal/ui/mocks_test.go +++ b/internal/ui/mocks_test.go @@ -10,36 +10,28 @@ import ( // MockDiscovery is a mock implementation of DiscoveryInterface for testing type MockDiscovery struct { - mu sync.Mutex - - // Return values - Contexts []string - CurrentContext string - Namespaces []string - Pods []k8s.PodInfo - PodsWithSelector []k8s.PodInfo - Services []k8s.ServiceInfo - - // Errors to return - ListContextsErr error - GetCurrentContextErr error - ListNamespacesErr error - ListPodsErr error - ListPodsWithSelectorErr error - ListServicesErr error - - // Call tracking + ListPodsErr error + ListServicesErr error + ListPodsWithSelectorErr error + ListContextsErr error + GetCurrentContextErr error + ListNamespacesErr error + LastSelector string + CurrentContext string + LastNamespace string + LastContextName string + PodsWithSelector []k8s.PodInfo + Services []k8s.ServiceInfo + Pods []k8s.PodInfo + Namespaces []string + Contexts []string ListContextsCalls int GetCurrentContextCalls int ListNamespacesCalls int ListPodsCalls int ListPodsWithSelectorCalls int ListServicesCalls int - - // Captured arguments - LastContextName string - LastNamespace string - LastSelector string + mu sync.Mutex } func NewMockDiscovery() *MockDiscovery { @@ -104,34 +96,26 @@ func (m *MockDiscovery) ListServices(ctx context.Context, contextName, namespace // MockMutator is a mock implementation of MutatorInterface for testing type MockMutator struct { - mu sync.Mutex - - // Errors to return - AddForwardErr error - RemoveForwardsErr error RemoveForwardByIDErr error UpdateForwardErr error - - // Call tracking - AddForwardCalls int - RemoveForwardsCalls int - RemoveForwardByIDCalls int - UpdateForwardCalls int - - // Captured arguments - LastContextName string - LastNamespaceName string - LastForward config.Forward - LastOldID string - LastRemovedID string - LastPredicate func(ctx, ns string, fwd config.Forward) bool - - // Storage for testing - Forwards []struct { + AddForwardErr error + RemoveForwardsErr error + LastPredicate func(ctx, ns string, fwd config.Forward) bool + LastContextName string + LastOldID string + LastNamespaceName string + LastRemovedID string + Forwards []struct { Context string Namespace string Forward config.Forward } + LastForward config.Forward + RemoveForwardByIDCalls int + UpdateForwardCalls int + RemoveForwardsCalls int + AddForwardCalls int + mu sync.Mutex } func NewMockMutator() *MockMutator { @@ -186,14 +170,10 @@ func (m *MockMutator) UpdateForward(oldID, newContextName, newNamespaceName stri // MockHTTPLogSubscriber is a mock for HTTP log subscription type MockHTTPLogSubscriber struct { - mu sync.Mutex - - // Subscription tracking Subscriptions map[string]func(HTTPLogEntry) CleanupCalls int - - // Control - ShouldFail bool + mu sync.Mutex + ShouldFail bool } func NewMockHTTPLogSubscriber() *MockHTTPLogSubscriber { @@ -237,11 +217,11 @@ func (m *MockHTTPLogSubscriber) GetSubscriberFunc() HTTPLogSubscriber { // MockToggleCallback tracks toggle callback invocations type MockToggleCallback struct { - mu sync.Mutex Calls []struct { ID string Enable bool } + mu sync.Mutex } func NewMockToggleCallback() *MockToggleCallback { diff --git a/internal/ui/table.go b/internal/ui/table.go index c7a1ee1..9a64ae7 100644 --- a/internal/ui/table.go +++ b/internal/ui/table.go @@ -14,17 +14,17 @@ type ForwardStatus struct { Context string Namespace string Alias string - Type string // "service", "pod", etc. - Resource string // name without type prefix + Type string + Resource string + Status string RemotePort int LocalPort int - Status string // "Starting", "Active", "Reconnecting", "Error" } // TableUI manages the terminal table display type TableUI struct { + forwards map[string]*ForwardStatus mu sync.RWMutex - forwards map[string]*ForwardStatus // key is forward ID verbose bool } @@ -101,12 +101,12 @@ func (t *TableUI) Render() { // Sort forwards by local port for consistent display type sortEntry struct { - id string fwd *ForwardStatus + id string } var entries []sortEntry for id, fwd := range t.forwards { - entries = append(entries, sortEntry{id, fwd}) + entries = append(entries, sortEntry{fwd: fwd, id: id}) } // Simple sort by local port diff --git a/internal/ui/wizard_commands.go b/internal/ui/wizard_commands.go index 68ed282..685f92e 100644 --- a/internal/ui/wizard_commands.go +++ b/internal/ui/wizard_commands.go @@ -9,6 +9,7 @@ import ( "github.com/nvm/kportal/internal/benchmark" "github.com/nvm/kportal/internal/config" "github.com/nvm/kportal/internal/k8s" + "github.com/nvm/kportal/internal/logger" ) const ( @@ -19,53 +20,53 @@ const ( // ContextsLoadedMsg is sent when contexts have been loaded type ContextsLoadedMsg struct { - contexts []string err error + contexts []string } // NamespacesLoadedMsg is sent when namespaces have been loaded type NamespacesLoadedMsg struct { - namespaces []string err error + namespaces []string } // PodsLoadedMsg is sent when pods have been loaded type PodsLoadedMsg struct { - pods []k8s.PodInfo err error + pods []k8s.PodInfo } // ServicesLoadedMsg is sent when services have been loaded type ServicesLoadedMsg struct { - services []k8s.ServiceInfo err error + services []k8s.ServiceInfo } // SelectorValidatedMsg is sent when a selector has been validated type SelectorValidatedMsg struct { - valid bool - pods []k8s.PodInfo err error + pods []k8s.PodInfo + valid bool } // PortCheckedMsg is sent when a port's availability has been checked type PortCheckedMsg struct { + message string port int available bool - message string } // ForwardSavedMsg is sent when a forward has been saved to config type ForwardSavedMsg struct { - success bool err error + success bool } // ForwardsRemovedMsg is sent when forwards have been removed from config type ForwardsRemovedMsg struct { - success bool - count int err error + count int + success bool } // WizardCompleteMsg signals that the wizard has completed @@ -241,9 +242,9 @@ func removeForwardByIDCmd(mutator *config.Mutator, id string) tea.Cmd { // BenchmarkCompleteMsg is sent when a benchmark run completes type BenchmarkCompleteMsg struct { - ForwardID string - Results *benchmark.Results Error error + Results *benchmark.Results + ForwardID string } // BenchmarkProgressMsg is sent periodically during benchmark execution @@ -291,7 +292,7 @@ func runBenchmarkCmd(ctx context.Context, forwardID string, localPort int, urlPa // Recover from panics in the callback defer func() { if r := recover(); r != nil { - // Silently recover - progress callback failure shouldn't crash the benchmark + logger.Debug("recovered from panic in progress callback", map[string]any{"panic": r}) } }() // Non-blocking send to progress channel diff --git a/internal/ui/wizard_exclusion_test.go b/internal/ui/wizard_exclusion_test.go index 653d9ba..1db6c5f 100644 --- a/internal/ui/wizard_exclusion_test.go +++ b/internal/ui/wizard_exclusion_test.go @@ -86,10 +86,10 @@ func TestWizardMutualExclusion_HTTPLogBlocksOthers(t *testing.T) { // TestWizardMutualExclusion_CheckActiveModal tests the modal activity check logic func TestWizardMutualExclusion_CheckActiveModal(t *testing.T) { tests := []struct { - name string setupFunc func(*BubbleTeaUI) - expectActive bool + name string activeModalStr string + expectActive bool }{ { name: "no modal active", diff --git a/internal/ui/wizard_state.go b/internal/ui/wizard_state.go index 298493d..e1b3d3e 100644 --- a/internal/ui/wizard_state.go +++ b/internal/ui/wizard_state.go @@ -109,45 +109,33 @@ func (r ResourceType) Description() string { // AddWizardState maintains the state for the add port forward wizard type AddWizardState struct { - step AddWizardStep - inputMode InputMode - cursor int - scrollOffset int // For scrolling long lists - textInput string - searchFilter string // For filtering lists (contexts, namespaces, services) - loading bool - error error - - // Selections made by user + error error + resourceValue string + originalID string + portCheckMsg string + alias string + textInput string + searchFilter string + selector string selectedContext string selectedNamespace string + pods []k8s.PodInfo + contexts []string + detectedPorts []k8s.PortInfo + matchingPods []k8s.PodInfo + services []k8s.ServiceInfo + namespaces []string + scrollOffset int selectedResourceType ResourceType - resourceValue string // pod prefix or service name - selector string // for pod selector type - remotePort int + step AddWizardStep localPort int - alias string - - // Available options (loaded asynchronously from k8s) - contexts []string - namespaces []string - pods []k8s.PodInfo - services []k8s.ServiceInfo - - // Validation state - portAvailable bool - portCheckMsg string - matchingPods []k8s.PodInfo - - // Edit mode - isEditing bool - originalID string // ID of the forward being edited - - // Detected ports from resources - detectedPorts []k8s.PortInfo - - // Confirmation focus (alias field vs buttons) - confirmationFocus ConfirmationFocus + cursor int + remotePort int + inputMode InputMode + confirmationFocus ConfirmationFocus + portAvailable bool + isEditing bool + loading bool } // newAddWizardState creates a new add wizard state initialized to the first step @@ -239,11 +227,11 @@ func (w *AddWizardState) clearTextInput() { // RemoveWizardState maintains the state for the remove port forward wizard type RemoveWizardState struct { + selected map[int]bool forwards []RemovableForward cursor int - selected map[int]bool + confirmCursor int confirming bool - confirmCursor int // 0 = Yes, 1 = No } // RemovableForward represents a forward that can be removed @@ -387,45 +375,39 @@ const ( // BenchmarkState maintains the state for the benchmark wizard type BenchmarkState struct { - step BenchmarkStep + error error + results *BenchmarkResults + cancelFunc func() + progressCh chan BenchmarkProgressMsg + textInput string forwardID string forwardAlias string + urlPath string + method string + cursor int + progress int + total int + step BenchmarkStep + requests int + concurrency int localPort int - - // Configuration - urlPath string - method string - concurrency int - requests int - cursor int // Current field being edited - textInput string - - // Running state - running bool - progress int - total int - progressCh chan BenchmarkProgressMsg // Channel for progress updates - cancelFunc func() // Function to cancel the running benchmark - - // Results - results *BenchmarkResults - error error + running bool } // BenchmarkResults holds benchmark results for display type BenchmarkResults struct { + StatusCodes map[int]int TotalRequests int Successful int Failed int - MinLatency float64 // milliseconds + MinLatency float64 MaxLatency float64 AvgLatency float64 P50Latency float64 P95Latency float64 P99Latency float64 - Throughput float64 // requests per second + Throughput float64 BytesRead int64 - StatusCodes map[int]int } // newBenchmarkState creates a new benchmark state for a forward @@ -455,41 +437,35 @@ const ( // HTTPLogState maintains the state for HTTP log viewing type HTTPLogState struct { - forwardID string - forwardAlias string - entries []HTTPLogEntry - cursor int - scrollOffset int - autoScroll bool - - // Filtering - filterMode HTTPLogFilterMode - filterText string - filterActive bool // true when typing in filter input - - // Detail view - showingDetail bool // true when viewing full entry details - detailScroll int // scroll position in detail view - copyMessage string // temporary message after copying (e.g., "Copied!") + forwardID string + forwardAlias string + filterText string + copyMessage string + entries []HTTPLogEntry + cursor int + scrollOffset int + filterMode HTTPLogFilterMode + detailScroll int + autoScroll bool + filterActive bool + showingDetail bool } // HTTPLogEntry represents a single HTTP log entry for display type HTTPLogEntry struct { - RequestID string // Used to match request/response pairs - Timestamp string - Direction string - Method string - Path string - StatusCode int - LatencyMs int64 - BodySize int - - // Detail fields - for viewing full request/response RequestHeaders map[string]string ResponseHeaders map[string]string + Method string + RequestID string + Path string + Direction string + Timestamp string RequestBody string ResponseBody string Error string + StatusCode int + LatencyMs int64 + BodySize int } // newHTTPLogState creates a new HTTP log viewing state diff --git a/internal/ui/wizard_state_test.go b/internal/ui/wizard_state_test.go index 745654b..18f9451 100644 --- a/internal/ui/wizard_state_test.go +++ b/internal/ui/wizard_state_test.go @@ -285,10 +285,10 @@ func TestClearSearchFilter(t *testing.T) { func TestMoveCursorWithFilteredLists(t *testing.T) { tests := []struct { name string - step AddWizardStep + searchFilter string contexts []string namespaces []string - searchFilter string + step AddWizardStep initialCursor int delta int expectedCursor int diff --git a/internal/ui/wizard_styles.go b/internal/ui/wizard_styles.go index 4954fdd..f2448db 100644 --- a/internal/ui/wizard_styles.go +++ b/internal/ui/wizard_styles.go @@ -143,7 +143,6 @@ func renderBreadcrumb(parts ...string) string { func renderList(items []string, cursor int, prefix string, scrollOffset int) string { var b strings.Builder - const viewportHeight = 20 totalItems := len(items) // Show scroll up indicator if there are items above the viewport @@ -153,7 +152,7 @@ func renderList(items []string, cursor int, prefix string, scrollOffset int) str // Calculate visible range start := scrollOffset - end := scrollOffset + viewportHeight + end := scrollOffset + ViewportHeight if end > totalItems { end = totalItems } diff --git a/internal/version/checker.go b/internal/version/checker.go index 7da0f83..54d7878 100644 --- a/internal/version/checker.go +++ b/internal/version/checker.go @@ -1,3 +1,15 @@ +// Package version provides version checking against GitHub releases. +// It queries the GitHub API to check for newer versions of kportal +// and provides update notifications. +// +// Basic usage: +// +// info, err := version.CheckForUpdate(ctx, "owner", "repo", "v1.0.0") +// if err != nil { +// log.Printf("Version check failed: %v", err) +// } else if info.UpdateAvailable { +// fmt.Printf("Update available: %s -> %s\n", info.CurrentVersion, info.LatestVersion) +// } package version import ( @@ -33,10 +45,10 @@ type UpdateInfo struct { // Checker checks for new versions on GitHub type Checker struct { + client *http.Client owner string repo string current string - client *http.Client } // NewChecker creates a new version checker