feat: full CRUD for load balancer with P0/P1 hardening by larkly · Pull Request #110 · larkly/lazystack

larkly · 2026-03-30T14:59:03Z

Summary

Full CRUD for all LB sub-resources: listeners, pools, members, health monitors
Create LB from list view with inline subnet picker
Edit (Enter) for all resources with pre-filled forms
Health monitor standalone lifecycle: create, edit, delete via Ctrl+H on pools pane
Admin state toggle (o key) for LB, listeners, pools, members
PENDING state protection — blocks mutations, shows warning
Error sanitization — wraps API errors with user-friendly messages
Adaptive polling — fast during PENDING states, normal otherwise
SOURCE_IP_PORT LB method for OVN networks
IP address validation, member count on pool rows, cascade delete with child counts

P0/P1 issues addressed (#97)

Health monitor lifecycle (create/edit/delete independent of pool)
PENDING state protection (disable actions, show messaging)
Admin state toggle (o key)
Stronger cascade delete confirmation (shows child counts)
Error message sanitization (no API endpoint leakage)
IP address validation on member create
Listener edit warning for non-updatable fields
Member count on pool rows
Adaptive polling

Test plan

Copilot

Pull request overview

This PR expands the load balancer TUI to support full CRUD across LB sub-resources and adds several “production hardening” improvements (pending-state protections, safer deletes, admin-state toggles, and sanitized error messaging).

Changes:

Add standalone health monitor create/edit/delete flows (including pool-pane shortcuts) and corresponding loadbalancer API helpers.
Add admin-state (enable/disable) toggles for LB/listeners/pools/members and display disabled state in the UI.
Harden UX: sanitize API errors, block mutations during PENDING_*, add IP validation for members, show pool member counts, and enhance cascade delete confirmation text.

Reviewed changes

Copilot reviewed 13 out of 13 changed files in this pull request and generated 5 comments.

Show a summary per file

File	Description
src/internal/ui/lbpoolcreate/lbpoolcreate.go	Sanitize errors; update pool update call signature.
src/internal/ui/lbmonitorcreate/lbmonitorcreate.go	New health monitor create/edit modal and validation/UI interactions.
src/internal/ui/lbmembercreate/lbmembercreate.go	Sanitize errors; add IP validation; update member update call signature.
src/internal/ui/lblistenercreate/lblistenercreate.go	Sanitize errors; warn about non-editable fields; update listener update call signature.
src/internal/ui/lbdetail/lbdetail.go	Expose data for confirmations; show admin/disabled state; member counts; pending action bar; monitor actions in pools.
src/internal/ui/lbcreate/lbcreate.go	Sanitize errors; update LB update call signature.
src/internal/ui/help/help.go	Document new shortcuts (`ctrl+h`, `o`).
src/internal/shared/errors.go	Introduce `SanitizeAPIError` to prevent endpoint leakage and improve messages.
src/internal/loadbalancer/lb.go	Add `AdminStateUp` fields; extend Update* APIs to support admin toggle; add health monitor CRUD helpers.
src/internal/app/render.go	Render the new health monitor modal when active.
src/internal/app/app.go	Add modal routing; pending-state mutation protection; route `ctrl+h` and `o` to new actions.
src/internal/app/actions_server.go	Implement “delete health monitor” confirm action execution.
src/internal/app/actions_resource.go	Enrich LB delete confirmation with child counts; add monitor create/edit/delete openers; add admin-state toggle actions.

Comments suppressed due to low confidence (1)

src/internal/ui/lbdetail/lbdetail.go:1104

The Members pane action bar doesn’t include the o admin-state toggle even though it’s advertised in Hints() and handled in app.go. Adding an o Enable/Disable action when a member is selected would align the action bar with the available commands.

	case FocusMembers:
		if m.SelectedPoolID() != "" {
			buttons = append(buttons, btn{"^n", "Add Member"})
		}
		if m.SelectedMemberID() != "" {
			buttons = append(buttons, btn{"enter", "Edit"})
			buttons = append(buttons, btn{"^d", "Delete Member"})
		}

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Rebased all P0/P1 hardening features from feat/lb-p0-p1-fixes onto current main, preserving the advanced member settings (server picker, backup, monitor address/port, tags) that were added after the PR diverged. New features: - Health monitor standalone CRUD (ctrl+h on pools pane) - Admin state toggle (o key) for LB, listeners, pools, members - PENDING state protection blocks all mutations during provisioning - Cascade delete confirmation shows child resource counts - Error sanitization via SanitizeAPIError for all LB forms - Member count [N] on pool rows - DISABLED status display for admin-down members Reviewer comments addressed: - Health monitor edit no longer sends HTTP fields for TCP/PING types - PENDING guard blocks all mutation keys including o and ctrl+h - Action bar shows o Enable/Disable button on all four panes

Copilot

Pull request overview

Copilot reviewed 13 out of 13 changed files in this pull request and generated 9 comments.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-03-30T20:24:23Z

+		memberCount := len(m.members[p.ID])
+		countStr := lipgloss.NewStyle().Foreground(shared.ColorMuted).Render(fmt.Sprintf(" [%d]", memberCount))
+
 		line := fmt.Sprintf("%s%-*s  %-*s  %s",
 			prefix, nameW, name, methodW, method, health)

 		if selected {
 			line = selectedRowStyle.Render(line)
 		}
+		line += countStr
 		lines = append(lines, line)


In the pools list, the member count is appended after applying selectedRowStyle, so the highlighted background doesn't cover the count and the count can also push the row past maxWidth (the truncation math above doesn’t account for the added [n]). Consider including the count in the formatted line before styling/truncation (or recomputing maxHW/row width to include the count) so selected rows and alignment stay consistent.

Copilot · 2026-03-30T20:24:24Z

+// TotalMemberCount returns the total number of members across all pools.
+func (m Model) TotalMemberCount() int {
+	total := 0
+	for _, mems := range m.members {
+		total += len(mems)
+	}
+	return total
+}


TotalMemberCount() sums len(m.members[poolID]), but fetchDetail() only populates m.members for pools whose ListMembers call succeeds (errors are currently ignored). This can under-count and show misleading cascade-delete totals (and per-pool counts). Consider tracking member-load failures separately and surfacing an “unknown” indicator in the UI/confirm text when some pools’ member counts couldn’t be fetched.

Copilot · 2026-03-30T20:24:24Z

+		if !mem.AdminStateUp {
+			displayStatus = "DISABLED"
+		}
+		status := shared.StatusIcon(displayStatus) + displayStatus


displayStatus introduces a new synthetic status value "DISABLED", but shared.StatusIcon() has no mapping for it, so disabled members will render without an icon prefix while other statuses include one. Consider adding "DISABLED" (and possibly other admin-down markers) to shared.statusIconMap for consistent status rendering.

Suggested change

if !mem.AdminStateUp {

displayStatus = "DISABLED"

}

status := shared.StatusIcon(displayStatus) + displayStatus

statusIcon := shared.StatusIcon(displayStatus)

if !mem.AdminStateUp {

displayStatus = "DISABLED"

// Use a dedicated icon for disabled members since shared.StatusIcon

// does not handle the synthetic "DISABLED" status value.

statusIcon = "⏻ "

}

status := statusIcon + displayStatus

Copilot · 2026-03-30T20:24:24Z

+		body += fmt.Sprintf("\nThis will cascade-delete %d listeners, %d pools, and %d members.", lc, pc, mc)
+	} else {
+		body += "\nThis will cascade-delete all listeners, pools and members."


The cascade-delete confirmation now shows counts for listeners/pools/members when invoked from the detail view, but it omits health monitors (which are also deleted via pool/LB cascade). Consider including a monitor count (or explicitly mentioning monitors in the text) so the confirmation accurately reflects what will be removed.

Suggested change

body += fmt.Sprintf("\nThis will cascade-delete %d listeners, %d pools, and %d members.", lc, pc, mc)

} else {

body += "\nThis will cascade-delete all listeners, pools and members."

body += fmt.Sprintf("\nThis will cascade-delete %d listeners, %d pools, %d members, and any associated health monitors.", lc, pc, mc)

} else {

body += "\nThis will cascade-delete all listeners, pools, members, and health monitors."

Copilot · 2026-03-30T20:24:24Z

+			err := loadbalancer.DeleteHealthMonitor(context.Background(), lbClient, id)
+			if err != nil {
+				shared.Debugf("[action] delete health monitor from %s failed: %s", name, err)
+				return shared.ResourceActionErrMsg{Action: "Delete monitor", Name: name, Err: err}


This new health monitor delete action forwards the raw API error (err) into ResourceActionErrMsg, which is rendered by the error modal via err.Error() (currently unsanitized). Since this PR introduces shared.SanitizeAPIError to prevent endpoint leakage, consider sanitizing the error before returning it here (or, preferably, centralizing sanitization in modal.NewError / the ResourceActionErrMsg handler) so monitor deletion failures don’t expose internal endpoints.

Suggested change

return shared.ResourceActionErrMsg{Action: "Delete monitor", Name: name, Err: err}

return shared.ResourceActionErrMsg{Action: "Delete monitor", Name: name, Err: shared.SanitizeAPIError(err)}

Copilot · 2026-03-30T20:24:25Z

+// SanitizeAPIError wraps raw gophercloud errors with user-friendly messages.
+func SanitizeAPIError(err error) string {
+	if gophercloud.ResponseCodeIs(err, http.StatusConflict) {
+		return "Resource is busy (provisioning in progress). Try again shortly."
+	}
+	if gophercloud.ResponseCodeIs(err, http.StatusForbidden) {
+		return "Permission denied. Check your project role."
+	}
+	if gophercloud.ResponseCodeIs(err, http.StatusNotFound) {
+		return "Resource not found. It may have been deleted."
+	}
+	if gophercloud.ResponseCodeIs(err, http.StatusBadRequest) {
+		return "Invalid request. Check your input values."
+	}
+	if gophercloud.ResponseCodeIs(err, http.StatusRequestEntityTooLarge) {
+		return "Quota exceeded. Check your resource limits."
+	}
+
+	return urlPattern.ReplaceAllString(err.Error(), "[endpoint]")
+}


SanitizeAPIError is security-sensitive behavior and is now used across several UI flows. There are existing Go tests in src/internal/shared (e.g., styles_test.go), but there are no unit tests here to lock in the mapping behavior (HTTP status → message) and URL redaction. Adding table-driven tests for the handled status codes and for URL replacement would help prevent regressions.

Copilot · 2026-03-30T20:24:25Z

+
+// Hints returns key hints.
+func (m Model) Hints() string {
+	return "tab/\u2191\u2193 navigate \u2022 \u2190\u2192 pick type \u2022 ctrl+s submit \u2022 esc cancel"


Hints() always says "�[0m�[0mtab/↑↓ navigate • ←→ pick type …", but in edit mode the monitor type isn’t editable (and ←→ is also used for HTTP method / submit-cancel navigation). Consider making the hint string conditional on editMode and/or including HTTP method navigation so the on-screen hints match actual behavior.

Suggested change

return "tab/\u2191\u2193 navigate \u2022 \u2190\u2192 pick type \u2022 ctrl+s submit \u2022 esc cancel"

// In create mode, the user can pick the monitor type with left/right.

if !m.editMode {

return "tab/\u2191\u2193 navigate \u2022 \u2190\u2192 pick type \u2022 ctrl+s submit \u2022 esc cancel"

}

// In edit mode, the monitor type is fixed; left/right is used for other fields

// such as HTTP method or submit/cancel navigation, so adjust the hint.

return "tab/\u2191\u2193 navigate \u2022 \u2190\u2192 change HTTP method/submit-cancel \u2022 ctrl+s submit \u2022 esc cancel"

Copilot · 2026-03-30T20:24:25Z

+func (m *Model) advanceFocus(dir int) {
+	for {
+		m.focusField = (m.focusField + dir + numFields) % numFields
+		if m.editMode && m.focusField == fieldType {
+			continue
+		}
+		if !m.isHTTPType() && (m.focusField == fieldURLPath || m.focusField == fieldCodes || m.focusField == fieldHTTPMethod) {
+			continue
+		}
+		break
+	}
+	m.updateFocusInputs()
+}


This new modal contains substantial focus-navigation and conditional-field logic (e.g., advanceFocus() skipping HTTP-only fields and the edit-mode submit path only sending HTTP fields for HTTP/HTTPS). There are existing UI unit tests for similar form navigation (e.g., lbmembercreate_test.go), but this modal currently has none; adding a few table-driven tests around focus advancement and the edit-mode submit field selection would help prevent regressions.

Copilot · 2026-03-30T20:24:25Z

+	urlPath := strings.TrimSpace(m.urlPathInput.Value())
+	codes := strings.TrimSpace(m.codesInput.Value())
+	httpMethod := httpMethodOpts[m.selectedHTTPMethod]
+
+	m.submitting = true
+	m.err = ""
+	client := m.client
+
+	if m.editMode {
+		id := m.monitorID
+		// Reviewer fix #1: only send HTTP-specific fields for HTTP/HTTPS monitors
+		var urlPathPtr, codesPtr, httpMethodPtr *string
+		if m.isHTTPType() {
+			urlPathPtr = &urlPath
+			codesPtr = &codes
+			httpMethodPtr = &httpMethod
+		}
+		return m, tea.Batch(m.spinner.Tick, func() tea.Msg {


In create mode you normalize HTTP/HTTPS monitors to defaults (URL path "/" and expected codes "200" when left blank), but in edit mode the same fields are sent as-is. This means an edited HTTP/HTTPS monitor can submit empty URLPath / ExpectedCodes (and possibly fail validation server-side or unintentionally clear the values). Consider applying the same defaulting/validation in the edit-mode path before setting the pointers.

Enter was hijacked to open monitor edit when a pool had a health monitor attached, making pool settings (name, LB method) inaccessible. Now Ctrl+H consistently handles all monitor operations (add/edit), Ctrl+D handles monitor delete (or pool delete if no monitor), and Enter always opens pool edit.

Rebased all P0/P1 hardening features from feat/lb-p0-p1-fixes onto current main, preserving the advanced member settings (server picker, backup, monitor address/port, tags) that were added after the PR diverged. New features: - Health monitor standalone CRUD (ctrl+h on pools pane) - Admin state toggle (o key) for LB, listeners, pools, members - PENDING state protection blocks all mutations during provisioning - Cascade delete confirmation shows child resource counts - Error sanitization via SanitizeAPIError for all LB forms - Member count [N] on pool rows - DISABLED status display for admin-down members Reviewer comments addressed: - Health monitor edit no longer sends HTTP fields for TCP/PING types - PENDING guard blocks all mutation keys including o and ctrl+h - Action bar shows o Enable/Disable button on all four panes

larkly requested a review from Copilot March 30, 2026 15:09

Copilot started reviewing on behalf of larkly March 30, 2026 15:09 View session

Copilot AI reviewed Mar 30, 2026

View reviewed changes

Comment thread src/internal/ui/lbmonitorcreate/lbmonitorcreate.go

Comment thread src/internal/app/app.go Outdated

Comment thread src/internal/app/app.go Outdated

Comment thread src/internal/ui/lbdetail/lbdetail.go

Comment thread src/internal/ui/lbdetail/lbdetail.go

larkly force-pushed the feat/lb-p0-p1-fixes branch from 9098696 to c37319a Compare March 30, 2026 20:11

larkly requested a review from Copilot March 30, 2026 20:16

Copilot started reviewing on behalf of larkly March 30, 2026 20:17 View session

larkly self-assigned this Mar 30, 2026

Copilot AI reviewed Mar 30, 2026

View reviewed changes

larkly merged commit ad3dbe7 into main Mar 31, 2026
3 checks passed

larkly deleted the feat/lb-p0-p1-fixes branch March 31, 2026 00:06

	return shared.ResourceActionErrMsg{Action: "Delete monitor", Name: name, Err: err}
	return shared.ResourceActionErrMsg{Action: "Delete monitor", Name: name, Err: shared.SanitizeAPIError(err)}

-	return "tab/\u2191\u2193 navigate \u2022 \u2190\u2192 pick type \u2022 ctrl+s submit \u2022 esc cancel"
+	// In create mode, the user can pick the monitor type with left/right.
+	if !m.editMode {
+		return "tab/\u2191\u2193 navigate \u2022 \u2190\u2192 pick type \u2022 ctrl+s submit \u2022 esc cancel"
+	}
+	// In edit mode, the monitor type is fixed; left/right is used for other fields
+	// such as HTTP method or submit/cancel navigation, so adjust the hint.
+	return "tab/\u2191\u2193 navigate \u2022 \u2190\u2192 change HTTP method/submit-cancel \u2022 ctrl+s submit \u2022 esc cancel"

Conversation

larkly commented Mar 30, 2026

Summary

P0/P1 issues addressed (#97)

Test plan

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI Mar 30, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 30, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 30, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 30, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 30, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 30, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 30, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 30, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 30, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants