Fixes Controller Port Mapping Recovery

[c-kruse]

Fixes flaw in port mapping recovery that prevented recovery from functioning. Also handles recent changes to tcpListener naming conventions for config entity name prefixing and multikeylisteners.

Closes #2406

Summary by CodeRabbit

• Tests

  • Added comprehensive unit tests for port mapping recovery to validate that port mappings are correctly preserved across multiple configured listener endpoints.

• Refactor

  • Enhanced port mapping key derivation logic during recovery to ensure more reliable and deterministic listener endpoint associations.

[coderabbitai]

📝 Walkthrough

Walkthrough

Adds a portMappingKey helper in internal/qdr/port_mapping.go that normalizes TcpEndpoint names by handling TcpListenerNamePrefix, multiAddress/, and @ patterns. RecoverPortMapping is updated to use this helper instead of the raw map iteration key. Two new test files verify the fix: a unit test for RecoverPortMapping directly, and an integration test through ExtendedBindings.

Changes

Port mapping recovery fix and tests

Layer / File(s)	Summary
portMappingKey helper and RecoverPortMapping update internal/qdr/port_mapping.go	Adds strings import and introduces portMappingKey(listener TcpEndpoint) to normalize TCP listener names by stripping TcpListenerNamePrefix, replacing multiAddress/ with multiaddress-, and handling @-with-address cases. RecoverPortMapping now calls portMappingKey(listener) instead of using the map iteration key.
RecoverPortMapping unit test internal/qdr/port_mapping_test.go	Adds TestRecoverPortMappingPreservesListenerPorts constructing an InitialConfig with frontend, backend, per-target, and multi-address listeners, then asserting GetPortForKey returns the correct integer port for each normalized key via a local assertPortForKey helper.
ExtendedBindings round-trip integration test internal/kube/site/bindings_test.go	Adds log/slog and strconv imports and TestRecoveredPortMappingMatchesExtendedBindingKeys, which generates bridge config via ExtendedBindings, recovers the mapping with qdr.RecoverPortMapping, and verifies recovered TargetPort values match expected TCP listener ports for the main listener, per-pod entries, and multi-key entries.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title accurately summarizes the main objective of fixing the port mapping recovery mechanism in the controller.
Linked Issues check	✅ Passed	The code changes directly address the core issue: removing the self-defeating if/Release/delete block from the recovered() function [#2406], fixing the portMappingKey logic for tcpListener naming conventions, and adding comprehensive test coverage.
Out of Scope Changes check	✅ Passed	All changes are directly scoped to fixing port mapping recovery: the recovered() function fix in port_mapping.go, the portMappingKey helper for naming conventions, and test coverage in port_mapping_test.go and bindings_test.go.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

---

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

🧹 Nitpick comments (1)

internal/qdr/port_mapping_test.go (1)

35-43: ⚡ Quick win

Assert the recovered map without allocating on misses.

GetPortForKey mutates state by allocating a fresh port when key is absent, so this helper can hide a failed recovery if the free-port allocator happens to return the expected port. Since this test is in package qdr, check mapping.mappings directly.

Proposed test helper change

 func assertPortForKey(t *testing.T, mapping *PortMapping, key string, expected int) {
 	t.Helper()
-	actual, err := mapping.GetPortForKey(key)
-	if err != nil {
-		t.Fatalf("GetPortForKey(%q) returned error: %v", key, err)
+	actual, ok := mapping.mappings[key]
+	if !ok {
+		t.Fatalf("RecoverPortMapping() did not recover key %q", key)
 	}
 	if actual != expected {
-		t.Fatalf("GetPortForKey(%q) = %d, want %d", key, actual, expected)
+		t.Fatalf("recovered port for key %q = %d, want %d", key, actual, expected)
 	}
 }

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 40a70e92-7d7b-4bef-8beb-3e5a6f5ee45e

📥 Commits

Reviewing files that changed from the base of the PR and between 6f4f2b3 and a5d2997.

📒 Files selected for processing (3)

• internal/kube/site/bindings_test.go
• internal/qdr/port_mapping.go
• internal/qdr/port_mapping_test.go

[nluaces]

Tested in local:

• created listeners and checked their ports
• deleted one of them and restarted controller
• recreated the missing listener
• checked that the remaining listeners have the same ports