Skip to content

Commit 1351ad1

Browse files
authored
TUI Browse Modal & Tests (#358)
* Prevent log spam in ServiceFWD by tracking "no pods" warnings - Added `noPodsLogged` flag to avoid repeatedly logging "no pods" warnings for a service. - Reset flag when pods become available, allowing future warnings if pods disappear again. * Add a TUI browse modal for discovering and forwarding namespaces and services * Improve namespace manager integration for clientSet retrieval in TUI pod logs streamer * Ensure `clientSet` is correctly initialized to prevent nil interface issues in TUI pod logs streamer * Add hint style and update header with browse hint, enable "left" key for navigation in browse modal * Add extensive unit tests for RootModel and event handling logic - Added tests for various RootModel functionalities, including focus cycling, resizing, and key/mouse interactions. - Implemented event handler tests for pod lifecycle events, service removals, and metrics updates. - Added TUI component tests for header, services, and detail models. * Add unit tests for BrowseModel navigation, visibility, loading states, and error handling - Introduced comprehensive tests for BrowseModel initialization, navigation across contexts, namespaces, and services, and visibility toggling. - Verified loading state behavior and ensured error state handling clears on user interaction. - Improved test coverage for edge scenarios like zero visible items and invalid user inputs. * Pin `codeql-action/upload-sarif` version in Scorecard workflow for dependency stability
1 parent 269f550 commit 1351ad1

File tree

13 files changed

+4227
-52
lines changed

13 files changed

+4227
-52
lines changed

.github/workflows/scorecard.yml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -73,6 +73,6 @@ jobs:
7373
# Upload the results to GitHub's code scanning dashboard (optional).
7474
# Commenting out will disable upload of results to your repo's Code Scanning dashboard
7575
- name: "Upload to code-scanning"
76-
uses: github/codeql-action/upload-sarif@v3
76+
uses: github/codeql-action/upload-sarif@45c373516f557556c15d420e3f5e0aa3d64366bc # v3
7777
with:
7878
sarif_file: results.sarif

cmd/kubefwd/services/services.go

Lines changed: 46 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -336,17 +336,36 @@ Try:
336336
clientSets := make(map[string]*kubernetes.Clientset)
337337
var clientSetsMu sync.RWMutex
338338

339+
// Declare nsManager here so it can be captured by closures below
340+
// It will be assigned later after config validation
341+
var nsManager *fwdns.NamespaceManager
342+
339343
if fwdtui.IsEnabled() {
340344
tuiManager = fwdtui.Init(stopListenCh, triggerShutdown)
341345

342346
// Set up pod logs streamer
343347
tuiManager.SetPodLogsStreamer(func(ctx context.Context, namespace, podName, containerName, k8sContext string, tailLines int64) (io.ReadCloser, error) {
348+
// First try the local clientSets map (populated during namespace watcher startup)
344349
clientSetsMu.RLock()
345-
clientSet, ok := clientSets[k8sContext]
350+
cs, ok := clientSets[k8sContext]
346351
clientSetsMu.RUnlock()
347352

348-
if !ok {
349-
return nil, fmt.Errorf("no clientset for context: %s", k8sContext)
353+
// Use kubernetes.Interface to support both *Clientset and Interface types
354+
// Note: Must check ok && cs != nil because assigning nil pointer to interface
355+
// creates non-nil interface with nil concrete value (Go nil interface gotcha)
356+
var clientSet kubernetes.Interface
357+
if ok && cs != nil {
358+
clientSet = cs
359+
}
360+
361+
if clientSet == nil {
362+
// Fall back to NamespaceManager's cache for individually-added services
363+
if nsManager != nil {
364+
clientSet = nsManager.GetClientSet(k8sContext)
365+
}
366+
if clientSet == nil {
367+
return nil, fmt.Errorf("no clientset for context: %s", k8sContext)
368+
}
350369
}
351370

352371
opts := &v1.PodLogOptions{
@@ -470,7 +489,7 @@ Try:
470489
}
471490

472491
// Create the namespace manager for dynamic watcher management
473-
nsManager := fwdns.NewManager(fwdns.ManagerConfig{
492+
nsManager = fwdns.NewManager(fwdns.ManagerConfig{
474493
HostFile: hostFileWithLock,
475494
ConfigPath: cfgFilePath,
476495
Domain: domain,
@@ -486,26 +505,36 @@ Try:
486505
GlobalStopCh: stopListenCh,
487506
})
488507

508+
// Set up adapters for Kubernetes discovery and service CRUD
509+
// These are used by both API and TUI for browse/forward operations
510+
getNsManager := func() *fwdns.NamespaceManager { return nsManager }
511+
k8sDiscovery := fwdapi.NewKubernetesDiscoveryAdapter(getNsManager, cfgFilePath)
512+
serviceCRUD := fwdapi.NewServiceCRUDAdapter(fwdtui.GetStore, getNsManager, cfgFilePath)
513+
nsController := fwdapi.NewNamespaceManagerAdapter(getNsManager)
514+
489515
// Register namespace manager with API if enabled
490516
if apiManager != nil {
491517
apiManager.SetNamespaceManager(nsManager)
492-
493-
// Set up Kubernetes discovery adapter
494-
k8sDiscovery := fwdapi.NewKubernetesDiscoveryAdapter(
495-
apiManager.GetNamespaceManager,
496-
cfgFilePath,
497-
)
498518
apiManager.SetKubernetesDiscovery(k8sDiscovery)
499-
500-
// Set up ServiceCRUD adapter for add/remove operations
501-
serviceCRUD := fwdapi.NewServiceCRUDAdapter(
502-
fwdtui.GetStore,
503-
apiManager.GetNamespaceManager,
504-
cfgFilePath,
505-
)
506519
apiManager.SetServiceCRUD(serviceCRUD)
507520
}
508521

522+
// Wire up TUI browse modal adapters
523+
if tuiManager != nil {
524+
tuiManager.SetBrowseDiscovery(k8sDiscovery)
525+
tuiManager.SetBrowseServiceCRUD(serviceCRUD)
526+
tuiManager.SetBrowseNamespaceController(nsController)
527+
tuiManager.SetRemoveForwardCallback(func(key string) error {
528+
fwdsvcregistry.RemoveByName(key)
529+
return nil
530+
})
531+
532+
// Set initial context in header
533+
if len(contexts) > 0 {
534+
tuiManager.SetHeaderContext(contexts[0])
535+
}
536+
}
537+
509538
// Start watchers for each context/namespace combination (skip in idle mode)
510539
if !idleMode {
511540
for _, ctx := range contexts {

pkg/fwdapi/adapters.go

Lines changed: 11 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -633,8 +633,10 @@ func (a *KubernetesDiscoveryAdapter) ListNamespaces(ctx string) ([]types.K8sName
633633
}
634634
}
635635

636-
// List namespaces from the cluster
637-
nsList, err := clientSet.CoreV1().Namespaces().List(context.Background(), metav1.ListOptions{})
636+
// List namespaces from the cluster with timeout
637+
ctx2, cancel := context.WithTimeout(context.Background(), 10*time.Second)
638+
defer cancel()
639+
nsList, err := clientSet.CoreV1().Namespaces().List(ctx2, metav1.ListOptions{})
638640
if err != nil {
639641
return nil, fmt.Errorf("failed to list namespaces: %w", err)
640642
}
@@ -688,8 +690,10 @@ func (a *KubernetesDiscoveryAdapter) ListServices(ctx, namespace string) ([]type
688690
}
689691
}
690692

691-
// List services from the cluster
692-
svcList, err := clientSet.CoreV1().Services(namespace).List(context.Background(), metav1.ListOptions{})
693+
// List services from the cluster with timeout
694+
ctx2, cancel := context.WithTimeout(context.Background(), 10*time.Second)
695+
defer cancel()
696+
svcList, err := clientSet.CoreV1().Services(namespace).List(ctx2, metav1.ListOptions{})
693697
if err != nil {
694698
return nil, fmt.Errorf("failed to list services: %w", err)
695699
}
@@ -750,7 +754,9 @@ func (a *KubernetesDiscoveryAdapter) GetService(ctx, namespace, name string) (*t
750754
}
751755
}
752756

753-
svc, err := clientSet.CoreV1().Services(namespace).Get(context.Background(), name, metav1.GetOptions{})
757+
ctx2, cancel := context.WithTimeout(context.Background(), 10*time.Second)
758+
defer cancel()
759+
svc, err := clientSet.CoreV1().Services(namespace).Get(ctx2, name, metav1.GetOptions{})
754760
if err != nil {
755761
return nil, fmt.Errorf("service not found: %w", err)
756762
}

pkg/fwdapi/manager.go

Lines changed: 90 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -181,6 +181,18 @@ type namespaceManagerAdapter struct {
181181
mgr *fwdns.NamespaceManager
182182
}
183183

184+
// NewNamespaceManagerAdapter creates a NamespaceController adapter from a NamespaceManager getter.
185+
// This allows the TUI and API to interact with the namespace manager through the NamespaceController interface.
186+
func NewNamespaceManagerAdapter(getMgr func() *fwdns.NamespaceManager) types.NamespaceController {
187+
return &lazyNamespaceManagerAdapter{getMgr: getMgr}
188+
}
189+
190+
// lazyNamespaceManagerAdapter is a lazy-loading adapter that fetches the manager on each call.
191+
// This is useful when the manager may not be available at adapter creation time.
192+
type lazyNamespaceManagerAdapter struct {
193+
getMgr func() *fwdns.NamespaceManager
194+
}
195+
184196
func (a *namespaceManagerAdapter) AddNamespace(ctx, namespace string, opts types.AddNamespaceOpts) (*types.NamespaceInfoResponse, error) {
185197
info, err := a.mgr.StartWatcher(ctx, namespace, fwdns.WatcherOpts{
186198
LabelSelector: opts.LabelSelector,
@@ -241,6 +253,84 @@ func (a *namespaceManagerAdapter) GetNamespace(ctx, namespace string) (*types.Na
241253
}, nil
242254
}
243255

256+
// lazyNamespaceManagerAdapter methods - these delegate to the manager obtained from getMgr()
257+
258+
func (a *lazyNamespaceManagerAdapter) AddNamespace(ctx, namespace string, opts types.AddNamespaceOpts) (*types.NamespaceInfoResponse, error) {
259+
mgr := a.getMgr()
260+
if mgr == nil {
261+
return nil, fmt.Errorf("namespace manager not available")
262+
}
263+
info, err := mgr.StartWatcher(ctx, namespace, fwdns.WatcherOpts{
264+
LabelSelector: opts.LabelSelector,
265+
FieldSelector: opts.FieldSelector,
266+
})
267+
if err != nil {
268+
return nil, err
269+
}
270+
return &types.NamespaceInfoResponse{
271+
Key: info.Key,
272+
Namespace: info.Namespace,
273+
Context: info.Context,
274+
ServiceCount: info.ServiceCount,
275+
ActiveCount: info.ActiveCount,
276+
ErrorCount: info.ErrorCount,
277+
}, nil
278+
}
279+
280+
func (a *lazyNamespaceManagerAdapter) RemoveNamespace(ctx, namespace string) error {
281+
mgr := a.getMgr()
282+
if mgr == nil {
283+
return fmt.Errorf("namespace manager not available")
284+
}
285+
return mgr.StopWatcher(ctx, namespace)
286+
}
287+
288+
func (a *lazyNamespaceManagerAdapter) ListNamespaces() []types.NamespaceInfoResponse {
289+
mgr := a.getMgr()
290+
if mgr == nil {
291+
return nil
292+
}
293+
watchers := mgr.ListWatchers()
294+
result := make([]types.NamespaceInfoResponse, len(watchers))
295+
for i, w := range watchers {
296+
result[i] = types.NamespaceInfoResponse{
297+
Key: w.Key,
298+
Namespace: w.Namespace,
299+
Context: w.Context,
300+
ServiceCount: w.ServiceCount,
301+
ActiveCount: w.ActiveCount,
302+
ErrorCount: w.ErrorCount,
303+
Running: w.Running,
304+
LabelSelector: w.LabelSelector,
305+
FieldSelector: w.FieldSelector,
306+
}
307+
}
308+
return result
309+
}
310+
311+
func (a *lazyNamespaceManagerAdapter) GetNamespace(ctx, namespace string) (*types.NamespaceInfoResponse, error) {
312+
mgr := a.getMgr()
313+
if mgr == nil {
314+
return nil, fmt.Errorf("namespace manager not available")
315+
}
316+
w := mgr.GetWatcher(ctx, namespace)
317+
if w == nil {
318+
return nil, fmt.Errorf("namespace %s.%s not found", namespace, ctx)
319+
}
320+
info := w.Info()
321+
return &types.NamespaceInfoResponse{
322+
Key: info.Key,
323+
Namespace: info.Namespace,
324+
Context: info.Context,
325+
ServiceCount: info.ServiceCount,
326+
ActiveCount: info.ActiveCount,
327+
ErrorCount: info.ErrorCount,
328+
Running: info.Running,
329+
LabelSelector: info.LabelSelector,
330+
FieldSelector: info.FieldSelector,
331+
}, nil
332+
}
333+
244334
// SetNamespaces sets the namespaces being forwarded (for info endpoint)
245335
func (m *Manager) SetNamespaces(namespaces []string) {
246336
m.namespaces = namespaces

pkg/fwdservice/fwdservice.go

Lines changed: 22 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -102,6 +102,10 @@ type ServiceFWD struct {
102102

103103
// reconnecting prevents multiple simultaneous reconnect attempts
104104
reconnecting bool
105+
106+
// noPodsLogged tracks if we've already logged "no pods" warning for this service
107+
// to avoid spamming logs with repeated warnings
108+
noPodsLogged bool
105109
}
106110

107111
type PortMap struct {
@@ -320,13 +324,22 @@ func (svcFwd *ServiceFWD) GetPodsForService() []v1.Pod {
320324
// that are no longer returned by k8s, should these not be correctly deleted.
321325
func (svcFwd *ServiceFWD) SyncPodForwards(force bool) {
322326
doSync := func() {
323-
log.Infof("SyncPodForwards starting for service %s (force=%v, currentForwards=%d)", svcFwd, force, len(svcFwd.PortForwards))
327+
// Only log sync details if we haven't already logged "no pods" (avoid log spam)
328+
if !svcFwd.noPodsLogged {
329+
log.Infof("SyncPodForwards starting for service %s (force=%v, currentForwards=%d)", svcFwd, force, len(svcFwd.PortForwards))
330+
}
324331
k8sPods := svcFwd.GetPodsForService()
325-
log.Infof("SyncPodForwards: Found %d eligible pods for service %s", len(k8sPods), svcFwd)
332+
if !svcFwd.noPodsLogged || len(k8sPods) > 0 {
333+
log.Infof("SyncPodForwards: Found %d eligible pods for service %s", len(k8sPods), svcFwd)
334+
}
326335

327336
// If no pods are found currently, schedule a retry if configured.
328337
if len(k8sPods) == 0 {
329-
log.Warnf("WARNING: No Running Pods returned for service %s", svcFwd)
338+
// Only log warning once to avoid spamming logs
339+
if !svcFwd.noPodsLogged {
340+
log.Warnf("WARNING: No Running Pods returned for service %s", svcFwd)
341+
svcFwd.noPodsLogged = true
342+
}
330343
// Schedule retry - don't update LastSyncedAt so we retry sooner
331344
if svcFwd.RetryInterval > 0 {
332345
go func() {
@@ -341,6 +354,12 @@ func (svcFwd *ServiceFWD) SyncPodForwards(force bool) {
341354
return
342355
}
343356

357+
// Pods found - reset the no-pods log flag so we log again if they disappear
358+
if svcFwd.noPodsLogged {
359+
log.Infof("Pods now available for service %s", svcFwd)
360+
svcFwd.noPodsLogged = false
361+
}
362+
344363
// Only update LastSyncedAt after successful pod discovery
345364
defer func() { svcFwd.LastSyncedAt = time.Now() }()
346365

0 commit comments

Comments
 (0)