From 96880bc38bc8290817e30fe4966cca593c79c71a Mon Sep 17 00:00:00 2001
From: Jay Rauchenstein <rauchenstein@cockroachlabs.com>
Date: Mon, 10 May 2021 15:17:04 -0400
Subject: [PATCH] default "--join" flag port to 26257

This behavior existed for "start --join" but not for "connect --join".
(Our code didn't set a default, so it defaulted to port 443 for https.)
The change is implemented at the level of flag parsing, so it is now
uniform for all commands using the join flag.

Fixes #61620.

Release note (bug fix): Hosts listed with the "connect --join" command
line flag now default to port 26257 (was 443).  This matches the
existing behavior of "start --join".
---
 pkg/base/store_spec.go      |  4 +++
 pkg/base/store_spec_test.go | 16 ++++++------
 pkg/cli/flags_test.go       | 51 +++++++++++++++++++++++++++++++++++++
 3 files changed, 63 insertions(+), 8 deletions(-)

diff --git a/pkg/base/store_spec.go b/pkg/base/store_spec.go
index ab9f2ac897da..052026ded334 100644
--- a/pkg/base/store_spec.go
+++ b/pkg/base/store_spec.go
@@ -533,6 +533,10 @@ func (jls *JoinListType) Set(value string) error {
 		if err != nil {
 			return err
 		}
+		// Default the port if unspecified.
+		if len(port) == 0 {
+			port = DefaultPort
+		}
 		// Re-join the parts. This guarantees an address that
 		// will be valid for net.SplitHostPort().
 		*jls = append(*jls, net.JoinHostPort(addr, port))
diff --git a/pkg/base/store_spec_test.go b/pkg/base/store_spec_test.go
index 41203b7f926e..8ad799f06575 100644
--- a/pkg/base/store_spec_test.go
+++ b/pkg/base/store_spec_test.go
@@ -225,14 +225,14 @@ func TestJoinListType(t *testing.T) {
 		err  string
 	}{
 		{"", "", "no address specified in --join"},
-		{":", "--join=:", ""},
-		{"a", "--join=a:", ""},
-		{"a,b", "--join=a: --join=b:", ""},
-		{"a,,b", "--join=a: --join=b:", ""},
-		{",a", "--join=a:", ""},
-		{"a,", "--join=a:", ""},
-		{"a:123,b", "--join=a:123 --join=b:", ""},
-		{"[::1]:123,b", "--join=[::1]:123 --join=b:", ""},
+		{":", "--join=:" + base.DefaultPort, ""},
+		{"a", "--join=a:" + base.DefaultPort, ""},
+		{"a,b", "--join=a:" + base.DefaultPort + " --join=b:" + base.DefaultPort, ""},
+		{"a,,b", "--join=a:" + base.DefaultPort + " --join=b:" + base.DefaultPort, ""},
+		{",a", "--join=a:" + base.DefaultPort, ""},
+		{"a,", "--join=a:" + base.DefaultPort, ""},
+		{"a:123,b", "--join=a:123 --join=b:" + base.DefaultPort, ""},
+		{"[::1]:123,b", "--join=[::1]:123 --join=b:" + base.DefaultPort, ""},
 		{"[::1,b", "", `address \[::1: missing ']' in address`},
 	}
 
diff --git a/pkg/cli/flags_test.go b/pkg/cli/flags_test.go
index 04403253541d..ef1cfbaed278 100644
--- a/pkg/cli/flags_test.go
+++ b/pkg/cli/flags_test.go
@@ -894,6 +894,57 @@ func TestServerJoinSettings(t *testing.T) {
 	}
 }
 
+func TestConnectJoinSettings(t *testing.T) {
+	defer leaktest.AfterTest(t)()
+	defer log.Scope(t).Close(t)
+
+	// Avoid leaking configuration changes after the tests end.
+	defer initCLIDefaults()
+
+	f := connectInitCmd.Flags()
+	testData := []struct {
+		args         []string
+		expectedJoin []string
+	}{
+		{[]string{"connect", "init", "--join=a"},
+			[]string{"a:" + base.DefaultPort}},
+		{[]string{"connect", "init", "--join=a,b,c"},
+			[]string{"a:" + base.DefaultPort, "b:" + base.DefaultPort, "c:" + base.DefaultPort}},
+		{[]string{"connect", "init", "--join=a", "--join=b"},
+			[]string{"a:" + base.DefaultPort, "b:" + base.DefaultPort}},
+		{[]string{"connect", "init", "--join=127.0.0.1"},
+			[]string{"127.0.0.1:" + base.DefaultPort}},
+		{[]string{"connect", "init", "--join=127.0.0.1:"},
+			[]string{"127.0.0.1:" + base.DefaultPort}},
+		{[]string{"connect", "init", "--join=127.0.0.1,abc"},
+			[]string{"127.0.0.1:" + base.DefaultPort, "abc:" + base.DefaultPort}},
+		{[]string{"connect", "init", "--join=[::1],[::2]"},
+			[]string{"[::1]:" + base.DefaultPort, "[::2]:" + base.DefaultPort}},
+		{[]string{"connect", "init", "--join=[::1]:123,[::2]"},
+			[]string{"[::1]:123", "[::2]:" + base.DefaultPort}},
+		{[]string{"connect", "init", "--join=[::1],127.0.0.1"},
+			[]string{"[::1]:" + base.DefaultPort, "127.0.0.1:" + base.DefaultPort}},
+		{[]string{"connect", "init", "--join=[::1]:123", "--join=[::2]"},
+			[]string{"[::1]:123", "[::2]:" + base.DefaultPort}},
+	}
+
+	for i, td := range testData {
+		initCLIDefaults()
+		if err := f.Parse(td.args); err != nil {
+			t.Fatalf("Parse(%#v) got unexpected error: %v", td.args, err)
+		}
+
+		if err := extraClientFlagInit(); err != nil {
+			t.Fatal(err)
+		}
+
+		if !reflect.DeepEqual(td.expectedJoin, []string(serverCfg.JoinList)) {
+			t.Errorf("%d. serverCfg.JoinList expected %#v, but got %#v. td.args was '%#v'.",
+				i, td.expectedJoin, serverCfg.JoinList, td.args)
+		}
+	}
+}
+
 func TestClientConnSettings(t *testing.T) {
 	defer leaktest.AfterTest(t)()
 	defer log.Scope(t).Close(t)