From dbc78305461ac163a6fd83e245c55d941f9d86c1 Mon Sep 17 00:00:00 2001 From: Lukasz Raczylo Date: Wed, 6 May 2026 12:45:35 +0100 Subject: [PATCH] fix(ui): edit-mode wizard allows keeping the same local port MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Previously, editing a forward and keeping its local port unchanged failed the wizard's port-availability check: the in-config scan found the forward's own entry and reported '✗ Port N already assigned to '. Users had to pick a different port, edit, then change back. checkPortCmd now accepts an excludeID. The wizard passes wizard.originalID when isEditing so the forward being edited is ignored during the in-config conflict scan. The OS-level port check is unchanged (still catches actual port collisions). New regression test: TestCheckPortCmd_ExcludeID_AllowsKeepingOwnPort. --- internal/ui/commands_test.go | 42 +++++++++++++++++++++++++++++++--- internal/ui/wizard_commands.go | 23 ++++++++++++------- internal/ui/wizard_handlers.go | 6 ++++- 3 files changed, 59 insertions(+), 12 deletions(-) diff --git a/internal/ui/commands_test.go b/internal/ui/commands_test.go index 8e0f241..910eced 100644 --- a/internal/ui/commands_test.go +++ b/internal/ui/commands_test.go @@ -162,7 +162,7 @@ func TestCheckPortCmd_PortAvailability(t *testing.T) { require.NoError(t, err) // Test checking a random high port that should be available - cmd := checkPortCmd(59999, configPath) + cmd := checkPortCmd(59999, configPath, "") msg := cmd() portMsg, ok := msg.(PortCheckedMsg) @@ -192,7 +192,7 @@ func TestCheckPortCmd_ConfigConflict(t *testing.T) { require.NoError(t, err) // Test checking port that's already in config - cmd := checkPortCmd(8080, configPath) + cmd := checkPortCmd(8080, configPath, "") msg := cmd() portMsg, ok := msg.(PortCheckedMsg) @@ -202,10 +202,46 @@ func TestCheckPortCmd_ConfigConflict(t *testing.T) { assert.Contains(t, portMsg.message, "already assigned") } +// TestCheckPortCmd_ExcludeID_AllowsKeepingOwnPort verifies that in edit mode +// (excludeID set to the forward's own ID), the wizard does not falsely report +// the same local port as already in use by the forward being edited. +func TestCheckPortCmd_ExcludeID_AllowsKeepingOwnPort(t *testing.T) { + tmpDir := t.TempDir() + configPath := filepath.Join(tmpDir, ".kportal.yaml") + + configContent := `contexts: + - name: test-ctx + namespaces: + - name: default + forwards: + - resource: pod/my-app + port: 80 + localPort: 8080 +` + err := os.WriteFile(configPath, []byte(configContent), 0600) + require.NoError(t, err) + + // The forward's ID format is "//:". + excludeID := "test-ctx/default/pod/my-app:8080" + + cmd := checkPortCmd(8080, configPath, excludeID) + msg := cmd() + + portMsg, ok := msg.(PortCheckedMsg) + require.True(t, ok, "Expected PortCheckedMsg") + assert.Equal(t, 8080, portMsg.port) + // The config-conflict path must skip the excluded ID. The OS-level port + // availability check still runs, so the result depends on whether 8080 is + // in use by some other process — the relevant assertion is that the + // message does NOT mention "already assigned" (which is the config check). + assert.NotContains(t, portMsg.message, "already assigned", + "excludeID should suppress the config self-conflict, but got %q", portMsg.message) +} + // TestCheckPortCmd_InvalidConfig tests behavior with invalid config file func TestCheckPortCmd_InvalidConfig(t *testing.T) { // Use a non-existent config path - cmd := checkPortCmd(59998, "/nonexistent/path/.kportal.yaml") + cmd := checkPortCmd(59998, "/nonexistent/path/.kportal.yaml", "") msg := cmd() portMsg, ok := msg.(PortCheckedMsg) diff --git a/internal/ui/wizard_commands.go b/internal/ui/wizard_commands.go index 427e5b5..8ad46c6 100644 --- a/internal/ui/wizard_commands.go +++ b/internal/ui/wizard_commands.go @@ -145,8 +145,11 @@ func validateSelectorCmd(discovery *k8s.Discovery, contextName, namespace, selec } } -// checkPortCmd checks if a local port is available -func checkPortCmd(port int, configPath string) tea.Cmd { +// checkPortCmd checks if a local port is available. +// excludeID, when non-empty, is the ID of a forward to ignore during the +// in-config conflict scan. Used in edit mode so the wizard does not flag the +// forward being edited as conflicting with itself. +func checkPortCmd(port int, configPath, excludeID string) tea.Cmd { return func() tea.Msg { // First check if port is already in the configuration cfg, err := config.LoadConfig(configPath) @@ -154,12 +157,16 @@ func checkPortCmd(port int, configPath string) tea.Cmd { // Check all forwards in config for this port allForwards := cfg.GetAllForwards() for _, fwd := range allForwards { - if fwd.LocalPort == port { - return PortCheckedMsg{ - port: port, - available: false, - message: fmt.Sprintf("✗ Port %d already assigned to %s", port, fwd.ID()), - } + if fwd.LocalPort != port { + continue + } + if excludeID != "" && fwd.ID() == excludeID { + continue + } + return PortCheckedMsg{ + port: port, + available: false, + message: fmt.Sprintf("✗ Port %d already assigned to %s", port, fwd.ID()), } } } diff --git a/internal/ui/wizard_handlers.go b/internal/ui/wizard_handlers.go index 261f877..3e2b150 100644 --- a/internal/ui/wizard_handlers.go +++ b/internal/ui/wizard_handlers.go @@ -656,7 +656,11 @@ func (m model) handleAddWizardEnter() (tea.Model, tea.Cmd) { wizard.localPort = port wizard.loading = true wizard.error = nil - return m, checkPortCmd(port, m.ui.configPath) + excludeID := "" + if wizard.isEditing { + excludeID = wizard.originalID + } + return m, checkPortCmd(port, m.ui.configPath, excludeID) } case StepConfirmation: