[PR #2849] [CLOSED] Add TestPingAllByIPRandomClientPort to validate direct connections with randomize_client_port #2892

Closed
opened 2025-12-29 04:19:32 +01:00 by adam · 0 comments
Owner

📋 Pull Request Information

Original PR: https://github.com/juanfont/headscale/pull/2849
Author: @Copilot
Created: 11/1/2025
Status: Closed

Base: mainHead: copilot/debug-issue-2846


📝 Commits (4)

  • c09556d Initial plan
  • 2ac534d Add TestPingAllByIPDirectConnections test to validate direct connections with randomize_client_port
  • aafe727 Refine direct connection validation logic in TestPingAllByIPDirectConnections
  • 5fd393c Rename test to TestPingAllByIPRandomClientPort and add to GitHub workflow

📊 Changes

2 files changed (+103 additions, -0 deletions)

View changed files

📝 .github/workflows/test-integration.yaml (+1 -0)
📝 integration/general_test.go (+102 -0)

📄 Description

This PR adds a new integration test TestPingAllByIPRandomClientPort to validate that nodes can establish direct peer-to-peer connections when the randomize_client_port feature is enabled, helping to debug issue #2846.

Changes Made

  • New Test: Added TestPingAllByIPRandomClientPort in integration/general_test.go
    • Based on the existing TestPingAllByIP test structure
    • Enables randomize_client_port via HEADSCALE_RANDOMIZE_CLIENT_PORT=true configuration
    • Uses embedded DERP server with TLS and random IP allocation
    • Validates that nodes establish direct peer-to-peer connections (not relayed through DERP)
  • CI Integration: Added test to .github/workflows/test-integration.yaml to run in the CI pipeline

Direct Connection Validation

The test validates direct connections by checking the PeerStatus.CurAddr field for each peer:

  • Direct connections have CurAddr set to an actual IP:port address
  • DERP-relayed connections either have no CurAddr or contain the DERP magic IP (127.3.3.40)
  • Asserts that each client has at least one direct connection
  • Provides detailed logging showing which connections are direct vs relayed

Test Configuration

  • 2 users with multiple nodes per user (based on MustTestVersions)
  • Embedded DERP server only
  • TLS enabled
  • Random IP allocation strategy
  • randomize_client_port enabled (key requirement for issue #2846)

This test will help validate that the randomize_client_port feature works correctly and that nodes can establish direct connections as intended.

Original prompt

To debug https://github.com/juanfont/headscale/issues/2846,


💬 We'd love your input! Share your thoughts on Copilot coding agent in our 2 minute survey.


🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.

## 📋 Pull Request Information **Original PR:** https://github.com/juanfont/headscale/pull/2849 **Author:** [@Copilot](https://github.com/apps/copilot-swe-agent) **Created:** 11/1/2025 **Status:** ❌ Closed **Base:** `main` ← **Head:** `copilot/debug-issue-2846` --- ### 📝 Commits (4) - [`c09556d`](https://github.com/juanfont/headscale/commit/c09556dd22ff3ef31b63c4ff26b6d30b382da47c) Initial plan - [`2ac534d`](https://github.com/juanfont/headscale/commit/2ac534dd302a36b47ab44d5f1e39b4a776fcbea7) Add TestPingAllByIPDirectConnections test to validate direct connections with randomize_client_port - [`aafe727`](https://github.com/juanfont/headscale/commit/aafe727cb909ab09fc646742005ff23cce4e0364) Refine direct connection validation logic in TestPingAllByIPDirectConnections - [`5fd393c`](https://github.com/juanfont/headscale/commit/5fd393c50789c31eddb9502c57c327d8caba159b) Rename test to TestPingAllByIPRandomClientPort and add to GitHub workflow ### 📊 Changes **2 files changed** (+103 additions, -0 deletions) <details> <summary>View changed files</summary> 📝 `.github/workflows/test-integration.yaml` (+1 -0) 📝 `integration/general_test.go` (+102 -0) </details> ### 📄 Description This PR adds a new integration test `TestPingAllByIPRandomClientPort` to validate that nodes can establish direct peer-to-peer connections when the `randomize_client_port` feature is enabled, helping to debug issue #2846. ## Changes Made - **New Test**: Added `TestPingAllByIPRandomClientPort` in `integration/general_test.go` - Based on the existing `TestPingAllByIP` test structure - Enables `randomize_client_port` via `HEADSCALE_RANDOMIZE_CLIENT_PORT=true` configuration - Uses embedded DERP server with TLS and random IP allocation - Validates that nodes establish **direct** peer-to-peer connections (not relayed through DERP) - **CI Integration**: Added test to `.github/workflows/test-integration.yaml` to run in the CI pipeline ## Direct Connection Validation The test validates direct connections by checking the `PeerStatus.CurAddr` field for each peer: - Direct connections have `CurAddr` set to an actual IP:port address - DERP-relayed connections either have no `CurAddr` or contain the DERP magic IP (`127.3.3.40`) - Asserts that each client has at least one direct connection - Provides detailed logging showing which connections are direct vs relayed ## Test Configuration - 2 users with multiple nodes per user (based on `MustTestVersions`) - Embedded DERP server only - TLS enabled - Random IP allocation strategy - **randomize_client_port enabled** (key requirement for issue #2846) This test will help validate that the `randomize_client_port` feature works correctly and that nodes can establish direct connections as intended. <!-- START COPILOT CODING AGENT SUFFIX --> <details> <summary>Original prompt</summary> > To debug https://github.com/juanfont/headscale/issues/2846, </details> <!-- START COPILOT CODING AGENT TIPS --> --- 💬 We'd love your input! Share your thoughts on Copilot coding agent in our [2 minute survey](https://gh.io/copilot-coding-agent-survey). --- <sub>🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.</sub>
adam added the pull-request label 2025-12-29 04:19:32 +01:00
adam closed this issue 2025-12-29 04:19:32 +01:00
Sign in to join this conversation.
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: starred/headscale#2892