feat: add EKS cluster and node group browser#183
Conversation
![amazon-q-developer[bot]](https://avatars.githubusercontent.com/in/1220912?s=60&v=4){kind=small}
There was a problem hiding this comment.
Review Summary
This PR successfully adds EKS cluster and managed node group browsing capabilities to the TUI. The implementation includes comprehensive features for viewing cluster details, node group scaling configurations, and health status.
Critical Changes Required
Closure Variable Capture Issue: Both loadEKSClusters() and loadEKSNodeGroups() in internal/app/screen_eks.go capture variables outside their closure scope that should be captured inside. Move the cfg and cluster variable assignments inside the closure functions to prevent potential race conditions where the Model's state changes between closure creation and execution.
Strengths
- Clean implementation following existing TUI patterns
- Proper pagination handling for EKS API calls
- Good test coverage with mock clients
- Appropriate error handling throughout
- Well-structured UI components with filtering and navigation
You can now have the agent implement changes and create commits directly on your pull request's source branch. Simply comment with /q followed by your request in natural language to ask the agent to make changes.
| } | ||
|
|
||
| func (m Model) loadEKSClusters() tea.Cmd { | ||
| cfg := m.cfg |
There was a problem hiding this comment.
Reassigning the closure variable inside the loop causes both loadEKSClusters() and loadEKSNodeGroups() to capture the same cfg reference. If the Model's cfg field changes between the creation of the closure and its execution, both functions will use the modified cfg value, potentially causing unexpected behavior or accessing incorrect AWS credentials.
| cfg := m.cfg | |
| return func() tea.Msg { | |
| cfg := m.cfg | |
| ctx, cancel := context.WithTimeout(context.Background(), eksAPITimeout) |
| cfg := m.cfg | ||
| cluster := m.selectedEKSCluster |
There was a problem hiding this comment.
Reassigning the closure variable inside the loop causes both loadEKSClusters() and loadEKSNodeGroups() to capture the same cfg reference. If the Model's cfg field changes between the creation of the closure and its execution, both functions will use the modified cfg value, potentially causing unexpected behavior or accessing incorrect AWS credentials.
| cfg := m.cfg | |
| cluster := m.selectedEKSCluster | |
| return func() tea.Msg { | |
| cfg := m.cfg | |
| cluster := m.selectedEKSCluster | |
| if cluster == nil { | |
| return errMsg{fmt.Errorf("no EKS cluster selected")} | |
| } | |
| ctx, cancel := context.WithTimeout(context.Background(), eksAPITimeout) |
2 participants
Summary
Testing
Closes #166