From 7df161aee08276e7e7a7ee49c86e24b08f4406c4 Mon Sep 17 00:00:00 2001 From: Lukasz Raczylo Date: Mon, 24 Nov 2025 11:09:23 +0000 Subject: [PATCH] bugfixes nov2025 (#3) * Fix enter misbehaving. * Cleanup after previous tui implementation. * Fix race condition and improve logging * Add filtering of the namespaces by text input in the wizard UI --- cmd/kportal/main.go | 26 ++- internal/config/config.go | 35 --- internal/config/config_test.go | 66 ------ internal/forward/manager.go | 15 +- internal/k8s/discovery.go | 15 -- internal/k8s/resolver.go | 26 --- internal/logger/klog_logr.go | 105 +++++++++ internal/logger/klog_logr_test.go | 367 ++++++++++++++++++++++++++++++ internal/logger/logger.go | 5 + internal/retry/backoff.go | 5 - internal/retry/backoff_test.go | 8 +- internal/ui/bubbletea_ui.go | 2 +- internal/ui/interactive.go | 181 --------------- internal/ui/table.go | 55 +---- internal/ui/wizard_handlers.go | 88 +++++-- internal/ui/wizard_state.go | 69 +++++- internal/ui/wizard_state_test.go | 350 ++++++++++++++++++++++++++++ internal/ui/wizard_views.go | 127 +++++++---- 18 files changed, 1098 insertions(+), 447 deletions(-) create mode 100644 internal/logger/klog_logr.go create mode 100644 internal/logger/klog_logr_test.go delete mode 100644 internal/ui/interactive.go create mode 100644 internal/ui/wizard_state_test.go diff --git a/cmd/kportal/main.go b/cmd/kportal/main.go index 9c63e12..3361a3f 100644 --- a/cmd/kportal/main.go +++ b/cmd/kportal/main.go @@ -12,6 +12,7 @@ import ( "syscall" "time" + "github.com/go-logr/logr" "github.com/nvm/kportal/internal/config" "github.com/nvm/kportal/internal/converter" "github.com/nvm/kportal/internal/forward" @@ -91,16 +92,35 @@ func main() { // Configure klog (used by kubernetes client-go) to route through our logger // This prevents k8s logs from interfering with the UI + // + // klog v2 uses multiple output mechanisms: + // 1. SetOutput() - for basic text output + // 2. SetLogger() - for structured/error logs (logr interface) + // + // We must configure BOTH to capture all logs including error messages + // that would otherwise bypass SetOutput() and write directly to stderr. + klog.LogToStderr(false) // Disable direct stderr writes if *verbose { - // In verbose mode, route klog through our structured logger at DEBUG level + // In verbose mode, route all klog through our structured logger at DEBUG level klogLogger := logger.New(logger.LevelDebug, logFmt, os.Stderr) + + // Configure text output routing klogWriter := logger.NewKlogWriter(klogLogger) klog.SetOutput(klogWriter) + + // Configure structured/error log routing via logr interface + // This captures "Unhandled Error" and other structured logs that bypass SetOutput + logrSink := logger.NewLogrAdapter(klogLogger) + klog.SetLogger(logr.New(logrSink)) } else { - // In non-verbose mode, completely silence klog + // In non-verbose mode, completely silence ALL klog output klog.SetOutput(io.Discard) + + // Also silence structured/error logs via a discard logger + silentLogger := logger.New(logger.LevelError+1, logFmt, io.Discard) // Level above ERROR = silence all + logrSink := logger.NewLogrAdapter(silentLogger) + klog.SetLogger(logr.New(logrSink)) } - klog.LogToStderr(false) // Handle conversion mode if *convertInput != "" { diff --git a/internal/config/config.go b/internal/config/config.go index 0a9c818..5e28f38 100644 --- a/internal/config/config.go +++ b/internal/config/config.go @@ -136,38 +136,3 @@ func (c *Config) GetAllForwards() []Forward { return forwards } - -// GetForwardsByContext returns all forwards for a specific context. -func (c *Config) GetForwardsByContext(contextName string) []Forward { - var forwards []Forward - - for _, ctx := range c.Contexts { - if ctx.Name == contextName { - for _, ns := range ctx.Namespaces { - forwards = append(forwards, ns.Forwards...) - } - break - } - } - - return forwards -} - -// GetForwardsByNamespace returns all forwards for a specific context and namespace. -func (c *Config) GetForwardsByNamespace(contextName, namespaceName string) []Forward { - var forwards []Forward - - for _, ctx := range c.Contexts { - if ctx.Name == contextName { - for _, ns := range ctx.Namespaces { - if ns.Name == namespaceName { - forwards = append(forwards, ns.Forwards...) - break - } - } - break - } - } - - return forwards -} diff --git a/internal/config/config_test.go b/internal/config/config_test.go index 1a6498a..6d2d807 100644 --- a/internal/config/config_test.go +++ b/internal/config/config_test.go @@ -298,72 +298,6 @@ func TestConfig_GetAllForwards(t *testing.T) { assert.Len(t, forwards, 4, "should return all forwards from all contexts and namespaces") } -func TestConfig_GetForwardsByContext(t *testing.T) { - yamlData := []byte(`contexts: - - name: cluster1 - namespaces: - - name: ns1 - forwards: - - resource: pod/app1 - port: 8080 - localPort: 8080 - - resource: pod/app2 - port: 8081 - localPort: 8081 - - name: cluster2 - namespaces: - - name: ns2 - forwards: - - resource: pod/app3 - port: 9090 - localPort: 9090 -`) - - cfg, err := ParseConfig(yamlData) - assert.NoError(t, err) - - forwards := cfg.GetForwardsByContext("cluster1") - assert.Len(t, forwards, 2, "should return forwards only from cluster1") - - forwards2 := cfg.GetForwardsByContext("cluster2") - assert.Len(t, forwards2, 1, "should return forwards only from cluster2") - - forwards3 := cfg.GetForwardsByContext("non-existent") - assert.Len(t, forwards3, 0, "should return empty slice for non-existent context") -} - -func TestConfig_GetForwardsByNamespace(t *testing.T) { - yamlData := []byte(`contexts: - - name: cluster1 - namespaces: - - name: ns1 - forwards: - - resource: pod/app1 - port: 8080 - localPort: 8080 - - resource: pod/app2 - port: 8081 - localPort: 8081 - - name: ns2 - forwards: - - resource: pod/app3 - port: 9090 - localPort: 9090 -`) - - cfg, err := ParseConfig(yamlData) - assert.NoError(t, err) - - forwards := cfg.GetForwardsByNamespace("cluster1", "ns1") - assert.Len(t, forwards, 2, "should return forwards only from cluster1/ns1") - - forwards2 := cfg.GetForwardsByNamespace("cluster1", "ns2") - assert.Len(t, forwards2, 1, "should return forwards only from cluster1/ns2") - - forwards3 := cfg.GetForwardsByNamespace("cluster1", "non-existent") - assert.Len(t, forwards3, 0, "should return empty slice for non-existent namespace") -} - func TestForward_SetContext(t *testing.T) { fwd := Forward{ Resource: "pod/my-app", diff --git a/internal/forward/manager.go b/internal/forward/manager.go index d8bf112..4de682e 100644 --- a/internal/forward/manager.go +++ b/internal/forward/manager.go @@ -298,6 +298,11 @@ func (m *Manager) startWorker(fwd config.Forward) error { // stopWorker stops and removes a forward worker. func (m *Manager) stopWorker(id string) error { + return m.stopWorkerInternal(id, true) +} + +// stopWorkerInternal stops a worker with option to remove from UI or just update status +func (m *Manager) stopWorkerInternal(id string, removeFromUI bool) error { m.workersMu.Lock() worker, exists := m.workers[id] if !exists { @@ -310,9 +315,13 @@ func (m *Manager) stopWorker(id string) error { // Unregister from health checker m.healthChecker.Unregister(id) - // Notify UI to remove the forward + // Notify UI - either remove or update to disabled status if m.statusUI != nil { - m.statusUI.Remove(id) + if removeFromUI { + m.statusUI.Remove(id) + } else { + m.statusUI.UpdateStatus(id, "Disabled") + } } // Stop the worker @@ -363,7 +372,7 @@ func (m *Manager) getResourceForPort(forwards []config.Forward, port int) string // DisableForward temporarily stops a forward by ID func (m *Manager) DisableForward(id string) error { - if err := m.stopWorker(id); err != nil { + if err := m.stopWorkerInternal(id, false); err != nil { return err } log.Printf("Disabled: %s", id) diff --git a/internal/k8s/discovery.go b/internal/k8s/discovery.go index 0beb27f..9f677d0 100644 --- a/internal/k8s/discovery.go +++ b/internal/k8s/discovery.go @@ -5,7 +5,6 @@ import ( "fmt" "net" "sort" - "strconv" "strings" corev1 "k8s.io/api/core/v1" @@ -305,17 +304,3 @@ func CheckPortAvailability(port int) (bool, string, error) { listener.Close() return true, "", nil } - -// ValidatePort checks if a port number is valid. -func ValidatePort(portStr string) (int, error) { - port, err := strconv.Atoi(portStr) - if err != nil { - return 0, fmt.Errorf("invalid port number: %s", portStr) - } - - if port < 1 || port > 65535 { - return 0, fmt.Errorf("port must be between 1 and 65535, got %d", port) - } - - return port, nil -} diff --git a/internal/k8s/resolver.go b/internal/k8s/resolver.go index 7fb09f6..cbae42c 100644 --- a/internal/k8s/resolver.go +++ b/internal/k8s/resolver.go @@ -228,29 +228,3 @@ func (r *ResourceResolver) InvalidateCache(contextName, namespace, resource stri } } } - -// GetPodList returns a list of pods matching the given criteria. -// This is useful for debugging and testing. -func (r *ResourceResolver) GetPodList(ctx context.Context, contextName, namespace, selector string) ([]*corev1.Pod, error) { - client, err := r.clientPool.GetClient(contextName) - if err != nil { - return nil, fmt.Errorf("failed to get client: %w", err) - } - - listOptions := metav1.ListOptions{} - if selector != "" { - listOptions.LabelSelector = selector - } - - pods, err := client.CoreV1().Pods(namespace).List(ctx, listOptions) - if err != nil { - return nil, fmt.Errorf("failed to list pods: %w", err) - } - - result := make([]*corev1.Pod, len(pods.Items)) - for i := range pods.Items { - result[i] = &pods.Items[i] - } - - return result, nil -} diff --git a/internal/logger/klog_logr.go b/internal/logger/klog_logr.go new file mode 100644 index 0000000..ce6529e --- /dev/null +++ b/internal/logger/klog_logr.go @@ -0,0 +1,105 @@ +package logger + +import ( + "github.com/go-logr/logr" +) + +// LogrAdapter implements the logr.LogSink interface to route klog v2 logs +// through our structured logger. This captures ALL klog output including +// error logs, structured logs, and named logger output. +type LogrAdapter struct { + logger *Logger + name string + level int +} + +// NewLogrAdapter creates a new logr.LogSink that routes all klog v2 logs +// through our structured logger. +func NewLogrAdapter(logger *Logger) logr.LogSink { + return &LogrAdapter{ + logger: logger, + name: "", + level: 0, + } +} + +// Init initializes the logger with runtime info (not used in our implementation). +func (l *LogrAdapter) Init(info logr.RuntimeInfo) { + // No-op: we don't need runtime info +} + +// Enabled tests whether this LogSink is enabled at the specified V-level. +// We route all logs through our logger's level filtering. +func (l *LogrAdapter) Enabled(level int) bool { + // Map logr V-levels to our levels: + // V(0) = Info level (always enabled if logger level <= Info) + // V(1+) = Debug level (enabled if logger level <= Debug) + if level == 0 { + return l.logger.level <= LevelInfo + } + return l.logger.level <= LevelDebug +} + +// Info logs a non-error message with the given key/value pairs. +func (l *LogrAdapter) Info(level int, msg string, keysAndValues ...interface{}) { + fields := l.kvToMap(keysAndValues) + if l.name != "" { + fields["logger"] = l.name + } + + // Map logr V-levels to our levels: + // V(0) = Info, V(1+) = Debug + if level == 0 { + l.logger.Info(msg, fields) + } else { + l.logger.Debug(msg, fields) + } +} + +// Error logs an error message with the given key/value pairs. +func (l *LogrAdapter) Error(err error, msg string, keysAndValues ...interface{}) { + fields := l.kvToMap(keysAndValues) + if l.name != "" { + fields["logger"] = l.name + } + if err != nil { + fields["error"] = err.Error() + } + + l.logger.Error(msg, fields) +} + +// WithValues returns a new LogSink with additional key/value pairs. +func (l *LogrAdapter) WithValues(keysAndValues ...interface{}) logr.LogSink { + // For simplicity, we don't implement value accumulation + // Each log call receives all its keysAndValues directly + return l +} + +// WithName returns a new LogSink with the specified name appended. +func (l *LogrAdapter) WithName(name string) logr.LogSink { + newLogger := *l + if l.name == "" { + newLogger.name = name + } else { + newLogger.name = l.name + "." + name + } + return &newLogger +} + +// kvToMap converts a slice of alternating keys and values to a map. +func (l *LogrAdapter) kvToMap(keysAndValues []interface{}) map[string]interface{} { + fields := make(map[string]interface{}) + fields["source"] = "k8s-client" + + for i := 0; i < len(keysAndValues); i += 2 { + if i+1 < len(keysAndValues) { + key, ok := keysAndValues[i].(string) + if ok { + fields[key] = keysAndValues[i+1] + } + } + } + + return fields +} diff --git a/internal/logger/klog_logr_test.go b/internal/logger/klog_logr_test.go new file mode 100644 index 0000000..e3cedd9 --- /dev/null +++ b/internal/logger/klog_logr_test.go @@ -0,0 +1,367 @@ +package logger + +import ( + "bytes" + "encoding/json" + "errors" + "testing" + + "github.com/go-logr/logr" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func TestLogrAdapter_Info(t *testing.T) { + tests := []struct { + name string + loggerLevel Level + logrLevel int + message string + keysAndValues []interface{} + expectOutput bool + expectContains []string + }{ + { + name: "info log v0 with debug logger", + loggerLevel: LevelDebug, + logrLevel: 0, + message: "Connection established", + keysAndValues: []interface{}{"pod", "my-app-123", "port", 8080}, + expectOutput: true, + expectContains: []string{"[INFO]", "Connection established", "pod", "my-app-123"}, + }, + { + name: "info log v0 with info logger", + loggerLevel: LevelInfo, + logrLevel: 0, + message: "Port forward ready", + keysAndValues: []interface{}{}, + expectOutput: true, + expectContains: []string{"[INFO]", "Port forward ready"}, + }, + { + name: "info log v0 silenced with warn logger", + loggerLevel: LevelWarn, + logrLevel: 0, + message: "This should not appear", + keysAndValues: []interface{}{}, + expectOutput: false, + expectContains: []string{}, + }, + { + name: "debug log v1 with debug logger", + loggerLevel: LevelDebug, + logrLevel: 1, + message: "Detailed connection info", + keysAndValues: []interface{}{"details", "some-value"}, + expectOutput: true, + expectContains: []string{"[DEBUG]", "Detailed connection info", "details"}, + }, + { + name: "debug log v1 silenced with info logger", + loggerLevel: LevelInfo, + logrLevel: 1, + message: "This debug should not appear", + keysAndValues: []interface{}{}, + expectOutput: false, + expectContains: []string{}, + }, + { + name: "info with odd number of kvs (incomplete pair)", + loggerLevel: LevelInfo, + logrLevel: 0, + message: "Message with incomplete kv", + keysAndValues: []interface{}{"key1", "value1", "key2"}, // key2 has no value + expectOutput: true, + expectContains: []string{"[INFO]", "Message with incomplete kv", "key1", "value1"}, + }, + { + name: "info with source field added automatically", + loggerLevel: LevelInfo, + logrLevel: 0, + message: "Test source field", + keysAndValues: []interface{}{}, + expectOutput: true, + expectContains: []string{"[INFO]", "Test source field", "source:k8s-client"}, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + buf := &bytes.Buffer{} + logger := New(tt.loggerLevel, FormatText, buf) + sink := NewLogrAdapter(logger) + logrLogger := logr.New(sink) + + logrLogger.V(tt.logrLevel).Info(tt.message, tt.keysAndValues...) + + output := buf.String() + if tt.expectOutput { + for _, expected := range tt.expectContains { + assert.Contains(t, output, expected, "Output should contain: %s", expected) + } + } else { + assert.Empty(t, output, "No output expected for this log level") + } + }) + } +} + +func TestLogrAdapter_Error(t *testing.T) { + tests := []struct { + name string + loggerLevel Level + err error + message string + keysAndValues []interface{} + expectOutput bool + expectContains []string + }{ + { + name: "error with error object", + loggerLevel: LevelError, + err: errors.New("connection failed"), + message: "Port forward failed", + keysAndValues: []interface{}{"pod", "my-app-123"}, + expectOutput: true, + expectContains: []string{"[ERROR]", "Port forward failed", "connection failed", "pod", "my-app-123"}, + }, + { + name: "error without error object", + loggerLevel: LevelError, + err: nil, + message: "Generic error message", + keysAndValues: []interface{}{}, + expectOutput: true, + expectContains: []string{"[ERROR]", "Generic error message"}, + }, + { + name: "error silenced with level above error", + loggerLevel: LevelError + 1, + err: errors.New("should not appear"), + message: "This error should not appear", + keysAndValues: []interface{}{}, + expectOutput: false, + expectContains: []string{}, + }, + { + name: "error with multiple kvs", + loggerLevel: LevelError, + err: errors.New("sandbox not found"), + message: "Unhandled Error", + keysAndValues: []interface{}{"pod", "test-pod", "uid", "abc123", "port", 8080}, + expectOutput: true, + expectContains: []string{"[ERROR]", "Unhandled Error", "sandbox not found", "pod", "test-pod", "uid", "abc123"}, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + buf := &bytes.Buffer{} + logger := New(tt.loggerLevel, FormatText, buf) + sink := NewLogrAdapter(logger) + logrLogger := logr.New(sink) + + logrLogger.Error(tt.err, tt.message, tt.keysAndValues...) + + output := buf.String() + if tt.expectOutput { + for _, expected := range tt.expectContains { + assert.Contains(t, output, expected, "Output should contain: %s", expected) + } + } else { + assert.Empty(t, output, "No output expected for this log level") + } + }) + } +} + +func TestLogrAdapter_WithName(t *testing.T) { + tests := []struct { + name string + loggerNames []string + message string + expectContains string + }{ + { + name: "single logger name", + loggerNames: []string{"portforward"}, + message: "Test message", + expectContains: "logger:portforward", + }, + { + name: "nested logger names", + loggerNames: []string{"controller", "worker", "healthcheck"}, + message: "Nested message", + expectContains: "logger:controller.worker.healthcheck", + }, + { + name: "no logger name", + loggerNames: []string{}, + message: "No name message", + expectContains: "source:k8s-client", // Should still have source but no logger field + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + buf := &bytes.Buffer{} + logger := New(LevelInfo, FormatText, buf) + sink := NewLogrAdapter(logger) + logrLogger := logr.New(sink) + + // Apply WithName calls + for _, name := range tt.loggerNames { + logrLogger = logrLogger.WithName(name) + } + + logrLogger.Info(tt.message) + + output := buf.String() + assert.Contains(t, output, tt.expectContains) + }) + } +} + +func TestLogrAdapter_Enabled(t *testing.T) { + tests := []struct { + name string + loggerLevel Level + logrLevel int + expectEnabled bool + }{ + { + name: "v0 enabled with debug logger", + loggerLevel: LevelDebug, + logrLevel: 0, + expectEnabled: true, + }, + { + name: "v0 enabled with info logger", + loggerLevel: LevelInfo, + logrLevel: 0, + expectEnabled: true, + }, + { + name: "v0 disabled with warn logger", + loggerLevel: LevelWarn, + logrLevel: 0, + expectEnabled: false, + }, + { + name: "v1 enabled with debug logger", + loggerLevel: LevelDebug, + logrLevel: 1, + expectEnabled: true, + }, + { + name: "v1 disabled with info logger", + loggerLevel: LevelInfo, + logrLevel: 1, + expectEnabled: false, + }, + { + name: "v2 enabled with debug logger", + loggerLevel: LevelDebug, + logrLevel: 2, + expectEnabled: true, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + logger := New(tt.loggerLevel, FormatText, &bytes.Buffer{}) + sink := NewLogrAdapter(logger) + + enabled := sink.Enabled(tt.logrLevel) + assert.Equal(t, tt.expectEnabled, enabled) + }) + } +} + +func TestLogrAdapter_JSONFormat(t *testing.T) { + buf := &bytes.Buffer{} + logger := New(LevelInfo, FormatJSON, buf) + sink := NewLogrAdapter(logger) + logrLogger := logr.New(sink).WithName("test-component") + + logrLogger.Info("Test JSON message", "key1", "value1", "key2", 123) + + // Parse JSON output + var entry logEntry + err := json.Unmarshal(buf.Bytes(), &entry) + require.NoError(t, err) + + assert.Equal(t, "INFO", entry.Level) + assert.Equal(t, "Test JSON message", entry.Message) + assert.Equal(t, "k8s-client", entry.Fields["source"]) + assert.Equal(t, "test-component", entry.Fields["logger"]) + assert.Equal(t, "value1", entry.Fields["key1"]) + assert.Equal(t, float64(123), entry.Fields["key2"]) // JSON numbers decode as float64 +} + +func TestLogrAdapter_ConcurrentWrites(t *testing.T) { + // Note: bytes.Buffer is not thread-safe for writes, so this test verifies + // that our LogrAdapter doesn't panic under concurrent load, but we don't + // verify exact output (since logger uses fmt.Fprintf which is also not thread-safe) + buf := &bytes.Buffer{} + logger := New(LevelDebug, FormatText, buf) + sink := NewLogrAdapter(logger) + logrLogger := logr.New(sink) + + // Spawn multiple goroutines writing concurrently + done := make(chan bool) + for i := 0; i < 10; i++ { + go func(id int) { + for j := 0; j < 100; j++ { + logrLogger.Info("Concurrent message", "goroutine", id, "iteration", j) + } + done <- true + }(i) + } + + // Wait for all goroutines + for i := 0; i < 10; i++ { + <-done + } + + output := buf.String() + + // Verify we got substantial output (not checking exact count due to buffer race) + // The main goal is to ensure no panics occur during concurrent writes + assert.NotEmpty(t, output, "Should have some log output") + assert.Contains(t, output, "Concurrent message") +} + +func TestLogrAdapter_RealWorldKlogError(t *testing.T) { + // Simulate the exact error message from the screenshot + buf := &bytes.Buffer{} + logger := New(LevelError, FormatText, buf) + sink := NewLogrAdapter(logger) + logrLogger := logr.New(sink).WithName("UnhandledError") + + err := errors.New("an error occurred forwarding 8401 -> 8401: error forwarding port 8401 to pod 4e1e861c28e3b25a88b082e79788169b5d8a7a117904b7bb8c7cd59285cf1d308, uid : failed to find sandbox '4e1e861c28e3b25a88b082e79788169b5d8a7a117904b7bb8c7cd59285cf1d308' in store: not found") + logrLogger.Error(err, "Unhandled Error") + + output := buf.String() + assert.Contains(t, output, "[ERROR]") + assert.Contains(t, output, "Unhandled Error") + assert.Contains(t, output, "failed to find sandbox") + assert.Contains(t, output, "logger:UnhandledError") +} + +func TestLogrAdapter_SilenceMode(t *testing.T) { + // Test that logs are completely silenced when logger level is above error + buf := &bytes.Buffer{} + logger := New(LevelError+1, FormatText, buf) + sink := NewLogrAdapter(logger) + logrLogger := logr.New(sink) + + // Try all log levels + logrLogger.V(0).Info("Info message should not appear") + logrLogger.V(1).Info("Debug message should not appear") + logrLogger.Error(errors.New("error object"), "Error message should not appear") + + output := buf.String() + assert.Empty(t, output, "All logs should be silenced") +} diff --git a/internal/logger/logger.go b/internal/logger/logger.go index 26f0d9f..917f022 100644 --- a/internal/logger/logger.go +++ b/internal/logger/logger.go @@ -5,6 +5,7 @@ import ( "fmt" "io" "os" + "sync" "time" ) @@ -28,6 +29,7 @@ type Logger struct { level Level format Format output io.Writer + mu sync.Mutex // Protects concurrent writes to output } type logEntry struct { @@ -55,6 +57,9 @@ func (l *Logger) log(level Level, msg string, fields map[string]interface{}) { levelStr := levelToString(level) + l.mu.Lock() + defer l.mu.Unlock() + if l.format == FormatJSON { entry := logEntry{ Time: time.Now().Format(time.RFC3339), diff --git a/internal/retry/backoff.go b/internal/retry/backoff.go index 23fc7a7..ad3f1de 100644 --- a/internal/retry/backoff.go +++ b/internal/retry/backoff.go @@ -67,8 +67,3 @@ func (b *Backoff) calculateJitter(delay time.Duration) time.Duration { jitter := (b.rng.Float64()*2 - 1) * maxJitter return time.Duration(jitter) } - -// Sleep waits for the next backoff duration. -func (b *Backoff) Sleep() { - time.Sleep(b.Next()) -} diff --git a/internal/retry/backoff_test.go b/internal/retry/backoff_test.go index b93798c..1ee1dac 100644 --- a/internal/retry/backoff_test.go +++ b/internal/retry/backoff_test.go @@ -158,10 +158,12 @@ func TestBackoff_ExponentialProgression(t *testing.T) { // We allow for jitter by checking a range for i := 1; i < len(delays)-1; i++ { // Each delay should be roughly double the previous (accounting for jitter) - // With 10% jitter on each value, worst case: (2.0 * 1.1) / 0.9 = 2.44 - // We use 1.7x to 2.5x as a reasonable range with 10% jitter on each + // With 10% jitter on each value: + // Lower bound: (2.0 * 0.9) / 1.1 ≈ 1.636 + // Upper bound: (2.0 * 1.1) / 0.9 ≈ 2.444 + // We use 1.6x to 2.5x as a reasonable range to account for jitter variance ratio := float64(delays[i]) / float64(delays[i-1]) - assert.GreaterOrEqual(t, ratio, 1.7, "exponential growth should be ~2x") + assert.GreaterOrEqual(t, ratio, 1.6, "exponential growth should be ~2x") assert.LessOrEqual(t, ratio, 2.5, "exponential growth should be ~2x") } } diff --git a/internal/ui/bubbletea_ui.go b/internal/ui/bubbletea_ui.go index dac36a2..c060d0e 100644 --- a/internal/ui/bubbletea_ui.go +++ b/internal/ui/bubbletea_ui.go @@ -378,7 +378,7 @@ func (m model) renderMainView() string { } isSelected := (idx == m.ui.selectedIndex) - isDisabled := m.ui.disabledMap[id] + isDisabled := m.ui.disabledMap[id] || fwd.Status == "Disabled" // Selection indicator indicator := " " diff --git a/internal/ui/interactive.go b/internal/ui/interactive.go deleted file mode 100644 index 33a7595..0000000 --- a/internal/ui/interactive.go +++ /dev/null @@ -1,181 +0,0 @@ -package ui - -import ( - "fmt" - "os" - "sync" - - "golang.org/x/term" -) - -// InteractiveController handles keyboard input and selection state -type InteractiveController struct { - mu sync.RWMutex - selectedIndex int - forwardIDs []string // Ordered list of forward IDs - disabledMap map[string]bool // Tracks which forwards are disabled - toggleCallback func(id string, enable bool) - enabled bool - oldTermState *term.State -} - -// NewInteractiveController creates a new interactive controller -func NewInteractiveController(toggleCallback func(id string, enable bool)) *InteractiveController { - return &InteractiveController{ - selectedIndex: 0, - forwardIDs: make([]string, 0), - disabledMap: make(map[string]bool), - toggleCallback: toggleCallback, - enabled: false, - } -} - -// Enable puts the terminal in raw mode for keyboard input -func (ic *InteractiveController) Enable() error { - ic.mu.Lock() - defer ic.mu.Unlock() - - if ic.enabled { - return nil - } - - // Save current terminal state - oldState, err := term.MakeRaw(int(os.Stdin.Fd())) - if err != nil { - return fmt.Errorf("failed to enable raw mode: %w", err) - } - - ic.oldTermState = oldState - ic.enabled = true - return nil -} - -// Disable restores the terminal to normal mode -func (ic *InteractiveController) Disable() error { - ic.mu.Lock() - defer ic.mu.Unlock() - - if !ic.enabled { - return nil - } - - if ic.oldTermState != nil { - if err := term.Restore(int(os.Stdin.Fd()), ic.oldTermState); err != nil { - return fmt.Errorf("failed to restore terminal: %w", err) - } - } - - ic.enabled = false - return nil -} - -// UpdateForwardsList updates the list of forwards for navigation -func (ic *InteractiveController) UpdateForwardsList(ids []string) { - ic.mu.Lock() - defer ic.mu.Unlock() - - ic.forwardIDs = ids - - // Ensure selected index is valid - if ic.selectedIndex >= len(ic.forwardIDs) { - ic.selectedIndex = len(ic.forwardIDs) - 1 - } - if ic.selectedIndex < 0 && len(ic.forwardIDs) > 0 { - ic.selectedIndex = 0 - } -} - -// MoveUp moves selection up -func (ic *InteractiveController) MoveUp() { - ic.mu.Lock() - defer ic.mu.Unlock() - - if ic.selectedIndex > 0 { - ic.selectedIndex-- - } -} - -// MoveDown moves selection down -func (ic *InteractiveController) MoveDown() { - ic.mu.Lock() - defer ic.mu.Unlock() - - if ic.selectedIndex < len(ic.forwardIDs)-1 { - ic.selectedIndex++ - } -} - -// ToggleSelected toggles the enable/disable state of the selected forward -func (ic *InteractiveController) ToggleSelected() { - ic.mu.Lock() - if ic.selectedIndex < 0 || ic.selectedIndex >= len(ic.forwardIDs) { - ic.mu.Unlock() - return - } - - selectedID := ic.forwardIDs[ic.selectedIndex] - currentlyDisabled := ic.disabledMap[selectedID] - newState := !currentlyDisabled - ic.disabledMap[selectedID] = newState - ic.mu.Unlock() - - // Call the toggle callback - if ic.toggleCallback != nil { - ic.toggleCallback(selectedID, !newState) // enable is inverse of disabled - } -} - -// GetSelectedIndex returns the current selection index -func (ic *InteractiveController) GetSelectedIndex() int { - ic.mu.RLock() - defer ic.mu.RUnlock() - return ic.selectedIndex -} - -// IsDisabled returns whether a forward is disabled -func (ic *InteractiveController) IsDisabled(id string) bool { - ic.mu.RLock() - defer ic.mu.RUnlock() - return ic.disabledMap[id] -} - -// GetSelectedID returns the ID of the currently selected forward -func (ic *InteractiveController) GetSelectedID() string { - ic.mu.RLock() - defer ic.mu.RUnlock() - - if ic.selectedIndex < 0 || ic.selectedIndex >= len(ic.forwardIDs) { - return "" - } - return ic.forwardIDs[ic.selectedIndex] -} - -// HandleKey processes keyboard input and returns true if should continue -func (ic *InteractiveController) HandleKey(b []byte) bool { - if len(b) == 0 { - return true - } - - // Handle single byte keys - if len(b) == 1 { - switch b[0] { - case 'q', 'Q', 3: // q, Q, or Ctrl+C - return false - case ' ', '\r': // Space or Enter to toggle - ic.ToggleSelected() - return true - } - } - - // Handle escape sequences (arrow keys) - if len(b) == 3 && b[0] == 27 && b[1] == 91 { - switch b[2] { - case 65: // Up arrow - ic.MoveUp() - case 66: // Down arrow - ic.MoveDown() - } - } - - return true -} diff --git a/internal/ui/table.go b/internal/ui/table.go index a9b09d4..c7a1ee1 100644 --- a/internal/ui/table.go +++ b/internal/ui/table.go @@ -23,10 +23,9 @@ type ForwardStatus struct { // TableUI manages the terminal table display type TableUI struct { - mu sync.RWMutex - forwards map[string]*ForwardStatus // key is forward ID - verbose bool - interactive *InteractiveController + mu sync.RWMutex + forwards map[string]*ForwardStatus // key is forward ID + verbose bool } // NewTableUI creates a new table UI manager @@ -37,13 +36,6 @@ func NewTableUI(verbose bool) *TableUI { } } -// SetInteractiveController sets the interactive controller -func (t *TableUI) SetInteractiveController(ic *InteractiveController) { - t.mu.Lock() - defer t.mu.Unlock() - t.interactive = ic -} - // AddForward registers a new forward for display func (t *TableUI) AddForward(id string, fwd *config.Forward) { t.mu.Lock() @@ -126,27 +118,10 @@ func (t *TableUI) Render() { } } - // Update interactive controller with current forward IDs (in display order) - if t.interactive != nil { - ids := make([]string, len(entries)) - for i, entry := range entries { - ids[i] = entry.id - } - t.interactive.UpdateForwardsList(ids) - } - // Print each forward - for i, entry := range entries { + for _, entry := range entries { fwd := entry.fwd - // Check if this row is selected - isSelected := false - isDisabled := false - if t.interactive != nil { - isSelected = (i == t.interactive.GetSelectedIndex()) - isDisabled = t.interactive.IsDisabled(entry.id) - } - // Truncate long names alias := truncate(fwd.Alias, 25) resource := truncate(fwd.Resource, 25) @@ -154,8 +129,8 @@ func (t *TableUI) Render() { // Color code status with indicator statusStr := formatStatusWithIndicator(fwd.Status) - // Build the row content - rowContent := fmt.Sprintf(" %-15s %-18s %-25s %-10s %-25s %-12d %-12d %s", + // Print the row + fmt.Printf(" %-15s %-18s %-25s %-10s %-25s %-12d %-12d %s\n", fwd.Context, fwd.Namespace, alias, @@ -164,26 +139,10 @@ func (t *TableUI) Render() { fwd.RemotePort, fwd.LocalPort, statusStr) - - // Apply selection highlighting or disabled styling - if isSelected { - // Replace leading spaces with arrow, then apply reverse video to entire line - rowContent = "\033[7m> " + rowContent[2:] + "\033[0m" - } else if isDisabled { - // Apply dimmed styling to entire line - rowContent = "\033[2m" + rowContent + "\033[0m" - } - - fmt.Println(rowContent) } fmt.Println(strings.Repeat("=", 130)) - helpText := "Total forwards: %d | ↑↓: Navigate | Space: Toggle | q: Quit" - if !t.verbose { - fmt.Printf(helpText+"\n", len(t.forwards)) - } else { - fmt.Printf("Total forwards: %d | Press Ctrl+C to stop\n", len(t.forwards)) - } + fmt.Printf("Total forwards: %d | Press Ctrl+C to stop\n", len(t.forwards)) // In verbose mode, add a newline to separate from logs if t.verbose { diff --git a/internal/ui/wizard_handlers.go b/internal/ui/wizard_handlers.go index ff7a7ec..2e3d9f6 100644 --- a/internal/ui/wizard_handlers.go +++ b/internal/ui/wizard_handlers.go @@ -10,6 +10,19 @@ import ( "github.com/nvm/kportal/internal/k8s" ) +// isFilterableStep returns true if the step supports search/filter +func isFilterableStep(step AddWizardStep) bool { + switch step { + case StepSelectContext, StepSelectNamespace: + return true + case StepEnterResource: + // Only service selection is filterable (pod prefix and selector are text input) + return true // We'll check resource type in the handler + default: + return false + } +} + // handleMainViewKeys handles keyboard input in the main view func (m model) handleMainViewKeys(msg tea.KeyMsg) (tea.Model, tea.Cmd) { // If delete confirmation is showing, handle it separately @@ -224,6 +237,12 @@ func (m model) handleAddWizardKeys(msg tea.KeyMsg) (tea.Model, tea.Cmd) { return m, tea.ClearScreen case "esc": + // If there's an active search filter, clear it instead of going back + if wizard.searchFilter != "" && isFilterableStep(wizard.step) { + wizard.clearSearchFilter() + return m, nil + } + // In edit mode, Esc always cancels (don't navigate back through skipped steps) if wizard.isEditing { m.ui.viewMode = ViewModeMain @@ -242,6 +261,7 @@ func (m model) handleAddWizardKeys(msg tea.KeyMsg) (tea.Model, tea.Cmd) { wizard.step-- wizard.cursor = 0 wizard.clearTextInput() + wizard.clearSearchFilter() wizard.error = nil // Reset input mode based on the step we're going back to @@ -300,26 +320,48 @@ func (m model) handleAddWizardKeys(msg tea.KeyMsg) (tea.Model, tea.Cmd) { return m.handleAddWizardEnter() case "backspace": - // Allow backspace in text input mode OR when focused on alias in confirmation + // Allow backspace in text input mode OR when focused on alias in confirmation OR when filtering canBackspace := wizard.inputMode == InputModeText || - (wizard.step == StepConfirmation && wizard.confirmationFocus == FocusAlias) - if canBackspace && len(wizard.textInput) > 0 { - wizard.textInput = wizard.textInput[:len(wizard.textInput)-1] + (wizard.step == StepConfirmation && wizard.confirmationFocus == FocusAlias) || + (wizard.inputMode == InputModeList && isFilterableStep(wizard.step) && len(wizard.searchFilter) > 0) + + if canBackspace { + if isFilterableStep(wizard.step) && wizard.inputMode == InputModeList && len(wizard.searchFilter) > 0 { + // Backspace in search filter + wizard.searchFilter = wizard.searchFilter[:len(wizard.searchFilter)-1] + wizard.cursor = 0 + wizard.scrollOffset = 0 + } else if len(wizard.textInput) > 0 { + wizard.textInput = wizard.textInput[:len(wizard.textInput)-1] + } } default: // Handle text input canTypeText := wizard.inputMode == InputModeText || - (wizard.step == StepConfirmation && wizard.confirmationFocus == FocusAlias) - if canTypeText && len(msg.String()) == 1 { - wizard.handleTextInput(rune(msg.String()[0])) + (wizard.step == StepConfirmation && wizard.confirmationFocus == FocusAlias) || + (wizard.inputMode == InputModeList && isFilterableStep(wizard.step)) - // Trigger validation for selector - if wizard.step == StepEnterResource && wizard.selectedResourceType == ResourceTypePodSelector { - if len(wizard.textInput) > 0 { - wizard.loading = true - wizard.error = nil - return m, validateSelectorCmd(m.ui.discovery, wizard.selectedContext, wizard.selectedNamespace, wizard.textInput) + if canTypeText && len(msg.String()) == 1 { + // If in list mode on filterable step, add to search filter instead of textInput + if wizard.inputMode == InputModeList && isFilterableStep(wizard.step) { + char := rune(msg.String()[0]) + // Only allow printable characters + if char >= 32 && char < 127 { + wizard.searchFilter += string(char) + wizard.cursor = 0 + wizard.scrollOffset = 0 + } + } else { + wizard.handleTextInput(rune(msg.String()[0])) + + // Trigger validation for selector + if wizard.step == StepEnterResource && wizard.selectedResourceType == ResourceTypePodSelector { + if len(wizard.textInput) > 0 { + wizard.loading = true + wizard.error = nil + return m, validateSelectorCmd(m.ui.discovery, wizard.selectedContext, wizard.selectedNamespace, wizard.textInput) + } } } } @@ -334,19 +376,23 @@ func (m model) handleAddWizardEnter() (tea.Model, tea.Cmd) { switch wizard.step { case StepSelectContext: - if wizard.cursor >= 0 && wizard.cursor < len(wizard.contexts) { - wizard.selectedContext = wizard.contexts[wizard.cursor] + filteredContexts := wizard.getFilteredContexts() + if wizard.cursor >= 0 && wizard.cursor < len(filteredContexts) { + wizard.selectedContext = filteredContexts[wizard.cursor] wizard.step = StepSelectNamespace wizard.cursor = 0 + wizard.clearSearchFilter() wizard.loading = true return m, loadNamespacesCmd(m.ui.discovery, wizard.selectedContext) } case StepSelectNamespace: - if wizard.cursor >= 0 && wizard.cursor < len(wizard.namespaces) { - wizard.selectedNamespace = wizard.namespaces[wizard.cursor] + filteredNamespaces := wizard.getFilteredNamespaces() + if wizard.cursor >= 0 && wizard.cursor < len(filteredNamespaces) { + wizard.selectedNamespace = filteredNamespaces[wizard.cursor] wizard.step = StepSelectResourceType wizard.cursor = 0 + wizard.clearSearchFilter() wizard.inputMode = InputModeList } @@ -403,13 +449,15 @@ func (m model) handleAddWizardEnter() (tea.Model, tea.Cmd) { } case ResourceTypeService: - if wizard.cursor >= 0 && wizard.cursor < len(wizard.services) { - wizard.resourceValue = wizard.services[wizard.cursor].Name + filteredServices := wizard.getFilteredServices() + if wizard.cursor >= 0 && wizard.cursor < len(filteredServices) { + wizard.resourceValue = filteredServices[wizard.cursor].Name wizard.step = StepEnterRemotePort wizard.clearTextInput() + wizard.clearSearchFilter() // Get ports from selected service - wizard.detectedPorts = wizard.services[wizard.cursor].Ports + wizard.detectedPorts = filteredServices[wizard.cursor].Ports if len(wizard.detectedPorts) > 0 { wizard.inputMode = InputModeList wizard.cursor = 0 diff --git a/internal/ui/wizard_state.go b/internal/ui/wizard_state.go index 3ce1870..6473a2f 100644 --- a/internal/ui/wizard_state.go +++ b/internal/ui/wizard_state.go @@ -1,9 +1,34 @@ package ui import ( + "strings" + "github.com/nvm/kportal/internal/k8s" ) +// filterStrings filters a slice of strings by a search filter (case-insensitive substring match) +func filterStrings(items []string, filter string) []string { + if filter == "" { + return items + } + filtered := []string{} + filterLower := strings.ToLower(filter) + for _, item := range items { + if strings.Contains(strings.ToLower(item), filterLower) { + filtered = append(filtered, item) + } + } + return filtered +} + +// matchesFilter checks if a string matches the filter (case-insensitive substring match) +func matchesFilter(item, filter string) bool { + if filter == "" { + return true + } + return strings.Contains(strings.ToLower(item), strings.ToLower(filter)) +} + // ViewMode represents the current view state of the UI type ViewMode int @@ -87,6 +112,7 @@ type AddWizardState struct { cursor int scrollOffset int // For scrolling long lists textInput string + searchFilter string // For filtering lists (contexts, namespaces, services) loading bool error error @@ -142,14 +168,14 @@ func (w *AddWizardState) moveCursor(delta int) { switch w.step { case StepSelectContext: - maxItems = len(w.contexts) + maxItems = len(w.getFilteredContexts()) case StepSelectNamespace: - maxItems = len(w.namespaces) + maxItems = len(w.getFilteredNamespaces()) case StepSelectResourceType: maxItems = 3 // Three resource types case StepEnterResource: if w.selectedResourceType == ResourceTypeService { - maxItems = len(w.services) + maxItems = len(w.getFilteredServices()) } case StepEnterRemotePort: if len(w.detectedPorts) > 0 { @@ -300,3 +326,40 @@ func (w *RemoveWizardState) getSelectedForwards() []RemovableForward { } return selected } + +// getFilteredContexts returns contexts filtered by search string +func (w *AddWizardState) getFilteredContexts() []string { + if w.searchFilter == "" { + return w.contexts + } + return filterStrings(w.contexts, w.searchFilter) +} + +// getFilteredNamespaces returns namespaces filtered by search string +func (w *AddWizardState) getFilteredNamespaces() []string { + if w.searchFilter == "" { + return w.namespaces + } + return filterStrings(w.namespaces, w.searchFilter) +} + +// getFilteredServices returns services filtered by search string +func (w *AddWizardState) getFilteredServices() []k8s.ServiceInfo { + if w.searchFilter == "" { + return w.services + } + filtered := []k8s.ServiceInfo{} + for _, svc := range w.services { + if matchesFilter(svc.Name, w.searchFilter) { + filtered = append(filtered, svc) + } + } + return filtered +} + +// clearSearchFilter clears the search filter and resets cursor/scroll +func (w *AddWizardState) clearSearchFilter() { + w.searchFilter = "" + w.cursor = 0 + w.scrollOffset = 0 +} diff --git a/internal/ui/wizard_state_test.go b/internal/ui/wizard_state_test.go new file mode 100644 index 0000000..745654b --- /dev/null +++ b/internal/ui/wizard_state_test.go @@ -0,0 +1,350 @@ +package ui + +import ( + "testing" + + "github.com/nvm/kportal/internal/k8s" + "github.com/stretchr/testify/assert" +) + +func TestFilterStrings(t *testing.T) { + tests := []struct { + name string + items []string + filter string + expected []string + }{ + { + name: "empty filter returns all items", + items: []string{"namespace-1", "namespace-2", "namespace-3"}, + filter: "", + expected: []string{"namespace-1", "namespace-2", "namespace-3"}, + }, + { + name: "filter matches multiple items", + items: []string{"prod-api", "prod-db", "staging-api", "dev-api"}, + filter: "prod", + expected: []string{"prod-api", "prod-db"}, + }, + { + name: "filter matches single item", + items: []string{"namespace-1", "namespace-2", "namespace-3"}, + filter: "2", + expected: []string{"namespace-2"}, + }, + { + name: "filter matches no items", + items: []string{"namespace-1", "namespace-2", "namespace-3"}, + filter: "xyz", + expected: []string{}, + }, + { + name: "case insensitive matching", + items: []string{"Production", "Staging", "Development"}, + filter: "prod", + expected: []string{"Production"}, + }, + { + name: "partial string matching", + items: []string{"my-app-frontend", "my-app-backend", "other-service"}, + filter: "app", + expected: []string{"my-app-frontend", "my-app-backend"}, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + result := filterStrings(tt.items, tt.filter) + assert.Equal(t, tt.expected, result) + }) + } +} + +func TestMatchesFilter(t *testing.T) { + tests := []struct { + name string + item string + filter string + expected bool + }{ + { + name: "empty filter matches everything", + item: "namespace-1", + filter: "", + expected: true, + }, + { + name: "exact match", + item: "namespace-1", + filter: "namespace-1", + expected: true, + }, + { + name: "partial match", + item: "production-api", + filter: "prod", + expected: true, + }, + { + name: "no match", + item: "namespace-1", + filter: "xyz", + expected: false, + }, + { + name: "case insensitive match", + item: "Production", + filter: "prod", + expected: true, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + result := matchesFilter(tt.item, tt.filter) + assert.Equal(t, tt.expected, result) + }) + } +} + +func TestGetFilteredContexts(t *testing.T) { + wizard := &AddWizardState{ + contexts: []string{"prod-cluster", "staging-cluster", "dev-cluster", "test-cluster"}, + } + + tests := []struct { + name string + filter string + expected []string + }{ + { + name: "no filter returns all", + filter: "", + expected: []string{"prod-cluster", "staging-cluster", "dev-cluster", "test-cluster"}, + }, + { + name: "filter by 'prod'", + filter: "prod", + expected: []string{"prod-cluster"}, + }, + { + name: "filter by 'cluster'", + filter: "cluster", + expected: []string{"prod-cluster", "staging-cluster", "dev-cluster", "test-cluster"}, + }, + { + name: "filter by 'staging'", + filter: "staging", + expected: []string{"staging-cluster"}, + }, + { + name: "filter with no matches", + filter: "xyz", + expected: []string{}, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + wizard.searchFilter = tt.filter + result := wizard.getFilteredContexts() + assert.Equal(t, tt.expected, result) + }) + } +} + +func TestGetFilteredNamespaces(t *testing.T) { + wizard := &AddWizardState{ + namespaces: []string{ + "kube-system", "kube-public", "default", + "prod-api", "prod-db", "staging-api", "staging-db", + "monitoring", "logging", + }, + } + + tests := []struct { + name string + filter string + expected []string + }{ + { + name: "no filter returns all", + filter: "", + expected: []string{ + "kube-system", "kube-public", "default", + "prod-api", "prod-db", "staging-api", "staging-db", + "monitoring", "logging", + }, + }, + { + name: "filter by 'prod'", + filter: "prod", + expected: []string{"prod-api", "prod-db"}, + }, + { + name: "filter by 'kube'", + filter: "kube", + expected: []string{"kube-system", "kube-public"}, + }, + { + name: "filter by 'api'", + filter: "api", + expected: []string{"prod-api", "staging-api"}, + }, + { + name: "filter by 'ing' (partial match)", + filter: "ing", + expected: []string{"staging-api", "staging-db", "monitoring", "logging"}, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + wizard.searchFilter = tt.filter + result := wizard.getFilteredNamespaces() + assert.Equal(t, tt.expected, result) + }) + } +} + +func TestGetFilteredServices(t *testing.T) { + wizard := &AddWizardState{ + services: []k8s.ServiceInfo{ + {Name: "api-gateway"}, + {Name: "api-backend"}, + {Name: "database"}, + {Name: "redis-cache"}, + {Name: "postgres-db"}, + }, + } + + tests := []struct { + name string + filter string + expected []string + }{ + { + name: "no filter returns all", + filter: "", + expected: []string{"api-gateway", "api-backend", "database", "redis-cache", "postgres-db"}, + }, + { + name: "filter by 'api'", + filter: "api", + expected: []string{"api-gateway", "api-backend"}, + }, + { + name: "filter by 'db'", + filter: "db", + expected: []string{"postgres-db"}, + }, + { + name: "filter by 'base'", + filter: "base", + expected: []string{"database"}, + }, + { + name: "filter by 'redis'", + filter: "redis", + expected: []string{"redis-cache"}, + }, + { + name: "filter with no matches", + filter: "xyz", + expected: []string{}, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + wizard.searchFilter = tt.filter + result := wizard.getFilteredServices() + resultNames := make([]string, len(result)) + for i, svc := range result { + resultNames[i] = svc.Name + } + assert.Equal(t, tt.expected, resultNames) + }) + } +} + +func TestClearSearchFilter(t *testing.T) { + wizard := &AddWizardState{ + searchFilter: "test", + cursor: 5, + scrollOffset: 10, + } + + wizard.clearSearchFilter() + + assert.Equal(t, "", wizard.searchFilter, "searchFilter should be cleared") + assert.Equal(t, 0, wizard.cursor, "cursor should be reset to 0") + assert.Equal(t, 0, wizard.scrollOffset, "scrollOffset should be reset to 0") +} + +func TestMoveCursorWithFilteredLists(t *testing.T) { + tests := []struct { + name string + step AddWizardStep + contexts []string + namespaces []string + searchFilter string + initialCursor int + delta int + expectedCursor int + }{ + { + name: "move down in filtered contexts", + step: StepSelectContext, + contexts: []string{"prod-1", "prod-2", "staging-1", "dev-1"}, + searchFilter: "prod", + initialCursor: 0, + delta: 1, + expectedCursor: 1, + }, + { + name: "cannot move beyond filtered list", + step: StepSelectContext, + contexts: []string{"prod-1", "prod-2", "staging-1", "dev-1"}, + searchFilter: "prod", + initialCursor: 1, + delta: 1, + expectedCursor: 1, // Should stay at 1 (last item in filtered list) + }, + { + name: "move up in filtered list", + step: StepSelectNamespace, + namespaces: []string{"ns-1", "ns-2", "ns-3", "other"}, + searchFilter: "ns", + initialCursor: 2, + delta: -1, + expectedCursor: 1, + }, + { + name: "cannot move above 0", + step: StepSelectNamespace, + namespaces: []string{"ns-1", "ns-2", "ns-3"}, + searchFilter: "ns", + initialCursor: 0, + delta: -1, + expectedCursor: 0, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + wizard := &AddWizardState{ + step: tt.step, + inputMode: InputModeList, + cursor: tt.initialCursor, + contexts: tt.contexts, + namespaces: tt.namespaces, + searchFilter: tt.searchFilter, + } + + wizard.moveCursor(tt.delta) + + assert.Equal(t, tt.expectedCursor, wizard.cursor) + }) + } +} diff --git a/internal/ui/wizard_views.go b/internal/ui/wizard_views.go index bc62d3d..43d927b 100644 --- a/internal/ui/wizard_views.go +++ b/internal/ui/wizard_views.go @@ -45,6 +45,12 @@ func (m model) renderSelectContext() string { b.WriteString(renderHeader("Add Port Forward", renderProgress(1, 7))) b.WriteString("Select Kubernetes Context:\n\n") + // Show search input if there's a filter active + if wizard.searchFilter != "" { + b.WriteString(renderTextInput("Filter: ", wizard.searchFilter, true)) + b.WriteString("\n\n") + } + if wizard.loading { b.WriteString(spinnerStyle.Render("⣾ Loading contexts...")) } else if wizard.error != nil { @@ -52,46 +58,56 @@ func (m model) renderSelectContext() string { } else if len(wizard.contexts) == 0 { b.WriteString(mutedStyle.Render("No contexts found in kubeconfig")) } else { - const viewportHeight = 20 - totalItems := len(wizard.contexts) + filteredContexts := wizard.getFilteredContexts() + if len(filteredContexts) == 0 { + b.WriteString(mutedStyle.Render("No matching contexts")) + } else { + const viewportHeight = 20 + totalItems := len(filteredContexts) - // Show scroll up indicator if there are items above the viewport - if wizard.scrollOffset > 0 { - b.WriteString(mutedStyle.Render(" ↑ More above ↑") + "\n") - } - - // Calculate visible range - start := wizard.scrollOffset - end := wizard.scrollOffset + viewportHeight - if end > totalItems { - end = totalItems - } - - // Render visible contexts with (current) marker on first one - for i := start; i < end; i++ { - prefix := " " - text := wizard.contexts[i] - if i == 0 { - text += mutedStyle.Render(" (current)") + // Show scroll up indicator if there are items above the viewport + if wizard.scrollOffset > 0 { + b.WriteString(mutedStyle.Render(" ↑ More above ↑") + "\n") } - if i == wizard.cursor { - prefix = "▸ " - b.WriteString(selectedStyle.Render(prefix + text)) - } else { - b.WriteString(prefix + text) + // Calculate visible range + start := wizard.scrollOffset + end := wizard.scrollOffset + viewportHeight + if end > totalItems { + end = totalItems } - b.WriteString("\n") - } - // Show scroll down indicator if there are items below the viewport - if end < totalItems { - b.WriteString(mutedStyle.Render(" ↓ More below ↓") + "\n") + // Render visible contexts with (current) marker on first one (only if not filtered) + for i := start; i < end; i++ { + prefix := " " + text := filteredContexts[i] + // Only show (current) marker if no filter and this is the first item in original list + if wizard.searchFilter == "" && i == 0 { + text += mutedStyle.Render(" (current)") + } + + if i == wizard.cursor { + prefix = "▸ " + b.WriteString(selectedStyle.Render(prefix + text)) + } else { + b.WriteString(prefix + text) + } + b.WriteString("\n") + } + + // Show scroll down indicator if there are items below the viewport + if end < totalItems { + b.WriteString(mutedStyle.Render(" ↓ More below ↓") + "\n") + } } } b.WriteString("\n") - b.WriteString(helpStyle.Render("↑/↓: Navigate Enter: Select Esc/Ctrl+C: Cancel")) + if wizard.searchFilter != "" { + b.WriteString(helpStyle.Render(fmt.Sprintf("↑/↓: Navigate Enter: Select Backspace: Clear filter (%d/%d) Esc: Cancel", len(wizard.getFilteredContexts()), len(wizard.contexts)))) + } else { + b.WriteString(helpStyle.Render("Type to filter ↑/↓: Navigate Enter: Select Esc/Ctrl+C: Cancel")) + } return b.String() } @@ -105,6 +121,12 @@ func (m model) renderSelectNamespace() string { b.WriteString("Select Namespace:\n\n") + // Show search input if there's a filter active + if wizard.searchFilter != "" { + b.WriteString(renderTextInput("Filter: ", wizard.searchFilter, true)) + b.WriteString("\n\n") + } + if wizard.loading { b.WriteString(spinnerStyle.Render("⣾ Loading namespaces...")) } else if wizard.error != nil { @@ -113,11 +135,20 @@ func (m model) renderSelectNamespace() string { } else if len(wizard.namespaces) == 0 { b.WriteString(mutedStyle.Render("No namespaces found")) } else { - b.WriteString(renderList(wizard.namespaces, wizard.cursor, " ", wizard.scrollOffset)) + filteredNamespaces := wizard.getFilteredNamespaces() + if len(filteredNamespaces) == 0 { + b.WriteString(mutedStyle.Render("No matching namespaces")) + } else { + b.WriteString(renderList(filteredNamespaces, wizard.cursor, " ", wizard.scrollOffset)) + } } b.WriteString("\n") - b.WriteString(helpStyle.Render("↑/↓: Navigate Enter: Select Esc: Back Ctrl+C: Cancel")) + if wizard.searchFilter != "" { + b.WriteString(helpStyle.Render(fmt.Sprintf("↑/↓: Navigate Enter: Select Backspace: Clear filter (%d/%d) Esc: Back", len(wizard.getFilteredNamespaces()), len(wizard.namespaces)))) + } else { + b.WriteString(helpStyle.Render("Type to filter ↑/↓: Navigate Enter: Select Esc: Back Ctrl+C: Cancel")) + } return b.String() } @@ -242,21 +273,41 @@ func (m model) renderEnterResource() string { case ResourceTypeService: b.WriteString("Select service:\n\n") + // Show search input if there's a filter active + if wizard.searchFilter != "" { + b.WriteString(renderTextInput("Filter: ", wizard.searchFilter, true)) + b.WriteString("\n\n") + } + if wizard.loading { b.WriteString(spinnerStyle.Render("⣾ Loading services...")) } else if len(wizard.services) == 0 { b.WriteString(mutedStyle.Render("No services found")) } else { - serviceNames := make([]string, len(wizard.services)) - for i, svc := range wizard.services { - serviceNames[i] = svc.Name + filteredServices := wizard.getFilteredServices() + if len(filteredServices) == 0 { + b.WriteString(mutedStyle.Render("No matching services")) + } else { + serviceNames := make([]string, len(filteredServices)) + for i, svc := range filteredServices { + serviceNames[i] = svc.Name + } + b.WriteString(renderList(serviceNames, wizard.cursor, " ", wizard.scrollOffset)) } - b.WriteString(renderList(serviceNames, wizard.cursor, " ", wizard.scrollOffset)) } } b.WriteString("\n") - b.WriteString(helpStyle.Render("Enter: Continue Esc: Back Ctrl+C: Cancel")) + // Show appropriate help text based on resource type and filter state + if wizard.selectedResourceType == ResourceTypeService { + if wizard.searchFilter != "" { + b.WriteString(helpStyle.Render(fmt.Sprintf("↑/↓: Navigate Enter: Select Backspace: Clear filter (%d/%d) Esc: Back", len(wizard.getFilteredServices()), len(wizard.services)))) + } else { + b.WriteString(helpStyle.Render("Type to filter ↑/↓: Navigate Enter: Select Esc: Back Ctrl+C: Cancel")) + } + } else { + b.WriteString(helpStyle.Render("Enter: Continue Esc: Back Ctrl+C: Cancel")) + } return b.String() }