From e061a623eccd5eb90c8060c12e0f2fd2c983efac Mon Sep 17 00:00:00 2001
From: Casey Davenport <davenport.cas@gmail.com>
Date: Wed, 29 Jul 2026 11:45:20 -0700
Subject: [PATCH] Default perf ports when the Perf block is partially specified

The ports were only defaulted when Perf was absent entirely, so a config that
set direct/service but left the ports out reached the dataplane with port 0 and
the apiserver rejected the test policy. Move the range checks out of the
external-only branch too, so a bad port fails at config load rather than 20
seconds into a run.
---
 pkg/config/config.go      | 30 ++++++++++++++------
 pkg/config/config_test.go | 59 +++++++++++++++++++++++++++++++++++++++
 2 files changed, 80 insertions(+), 9 deletions(-)

diff --git a/pkg/config/config.go b/pkg/config/config.go
index 867123e..3967707 100644
--- a/pkg/config/config.go
+++ b/pkg/config/config.go
@@ -287,21 +287,33 @@ func defaultAndValidate(cfg *Config) error {
 				if tcfg.Perf.ExternalIPOrFQDN == "" {
 					return fmt.Errorf("ExternalIPOrFQDN is required for an external thruput-latency test")
 				}
-				if tcfg.TestKind == "thruput-latency" {
-					if tcfg.Perf.ControlPort == 0 {
-						return fmt.Errorf("ControlPort is required for an external thruput-latency test")
-					}
-					if tcfg.Perf.ControlPort > 65535 || tcfg.Perf.ControlPort < 1 {
-						return fmt.Errorf("ControlPort must be between 1 and 65535")
-					}
+
+				// External ports can't be defaulted. The test tools don't work through NAT, so
+				// these have to match the ports the service is actually exposed on.
+				if tcfg.TestKind == "thruput-latency" && tcfg.Perf.ControlPort == 0 {
+					return fmt.Errorf("ControlPort is required for an external thruput-latency test")
 				}
 				if tcfg.Perf.TestPort == 0 {
 					return fmt.Errorf("TestPort is required for an external thruput-latency test")
 				}
-				if tcfg.Perf.TestPort > 65535 || tcfg.Perf.TestPort < 1 {
-					return fmt.Errorf("TestPort must be between 1 and 65535")
+			} else {
+				// Default the ports per-field rather than only when Perf is absent entirely. A
+				// config that sets some of Perf but leaves the ports out otherwise reaches the
+				// dataplane with port 0, and the apiserver rejects the test policy.
+				if tcfg.Perf.ControlPort == 0 {
+					tcfg.Perf.ControlPort = 32000
+				}
+				if tcfg.Perf.TestPort == 0 {
+					tcfg.Perf.TestPort = 32001
 				}
 			}
+
+			if tcfg.TestKind == "thruput-latency" && (tcfg.Perf.ControlPort > 65535 || tcfg.Perf.ControlPort < 1) {
+				return fmt.Errorf("ControlPort must be between 1 and 65535")
+			}
+			if tcfg.Perf.TestPort > 65535 || tcfg.Perf.TestPort < 1 {
+				return fmt.Errorf("TestPort must be between 1 and 65535")
+			}
 		}
 	}
 	return nil
diff --git a/pkg/config/config_test.go b/pkg/config/config_test.go
index 3fff307..7508057 100644
--- a/pkg/config/config_test.go
+++ b/pkg/config/config_test.go
@@ -604,3 +604,62 @@ func TestTargetURLCustomValue(t *testing.T) {
 	require.NoError(t, err)
 	assert.Equal(t, "https://custom.example.org", cfg.TestConfigs[0].DNSPerf.TargetURL)
 }
+
+func TestPerfPortsDefaultedWhenPerfPartiallySpecified(t *testing.T) {
+	fileContent := `
+- testKind: thruput-latency
+  Perf:
+    direct: true
+    service: true
+    external: false
+`
+	filePath := "/tmp/test_configs.yaml"
+	err := os.WriteFile(filePath, []byte(fileContent), 0o644)
+	require.NoError(t, err)
+	defer os.Remove(filePath)
+
+	var cfg Config
+	cfg.TestConfigFile = filePath
+	err = loadTestConfigsFromFile(&cfg)
+	require.NoError(t, err)
+	assert.Equal(t, 32000, cfg.TestConfigs[0].Perf.ControlPort)
+	assert.Equal(t, 32001, cfg.TestConfigs[0].Perf.TestPort)
+}
+
+func TestPerfPortsStillRequiredForExternalTest(t *testing.T) {
+	fileContent := `
+- testKind: thruput-latency
+  Perf:
+    external: true
+    ExternalIPOrFQDN: perf.example.com
+`
+	filePath := "/tmp/test_configs.yaml"
+	err := os.WriteFile(filePath, []byte(fileContent), 0o644)
+	require.NoError(t, err)
+	defer os.Remove(filePath)
+
+	var cfg Config
+	cfg.TestConfigFile = filePath
+	err = loadTestConfigsFromFile(&cfg)
+	require.Error(t, err)
+	assert.Contains(t, err.Error(), "ControlPort is required")
+}
+
+func TestPerfPortOutOfRangeRejectedForNonExternalTest(t *testing.T) {
+	fileContent := `
+- testKind: thruput-latency
+  Perf:
+    external: false
+    TestPort: 99999
+`
+	filePath := "/tmp/test_configs.yaml"
+	err := os.WriteFile(filePath, []byte(fileContent), 0o644)
+	require.NoError(t, err)
+	defer os.Remove(filePath)
+
+	var cfg Config
+	cfg.TestConfigFile = filePath
+	err = loadTestConfigsFromFile(&cfg)
+	require.Error(t, err)
+	assert.Contains(t, err.Error(), "TestPort must be between 1 and 65535")
+}