Merge #63485 #63506

63485: roachprod: normalize usernames r=rail a=rail

Previously, we used account names as is for AWS, and the prefix before
the "@" character for GCE and Azure.

This approach doesn't work in case when an account name contains special
characters, such as dot, because those characters are not DNS safe.

It is possible to override the user name by passing `--username`, but
it's very inconvenient.

This patch:
 * uses lower case for all characters
 * drops all characters except a-zA-Z

Fixes #38771

Release note: None

63506: sql: fix multi-region test knob r=otan a=arulajmani

There was an error in the `RunBeforeMultiRegionUpdates` knob such that
if it were set, we would short circuit. The intention, instead, was to
only short circuit in the error case.

Release note: None

Co-authored-by: Rail Aliiev <[email protected]>
Co-authored-by: arulajmani <[email protected]>

pkg/cmd/roachprod/main.go

@@ -231,7 +231,11 @@ func verifyClusterName(clusterName string) (string, error) {
 		for _, account := range active {
 			if !seenAccounts[account] {
 				seenAccounts[account] = true
-				accounts = append(accounts, account)
+				cleanAccount := vm.DNSSafeAccount(account)
+				if cleanAccount != account {
+					log.Printf("WARN: using `%s' as username instead of `%s'", cleanAccount, account)
+				}
+				accounts = append(accounts, cleanAccount)
 			}
 		}
 	}

pkg/cmd/roachprod/vm/vm.go

@@ -17,6 +17,7 @@ import (
 	"strconv"
 	"strings"
 	"time"
+	"unicode"
 
 	"github.com/cockroachdb/cockroach/pkg/cmd/roachprod/config"
 	"github.com/cockroachdb/errors"
@@ -352,3 +353,20 @@ func ExpandZonesFlag(zoneFlag []string) (zones []string, err error) {
 	}
 	return zones, nil
 }
+
+// DNSSafeAccount takes a string and returns a cleaned version of the string that can be used in DNS entries.
+// Unsafe characters are dropped. No length check is performed.
+func DNSSafeAccount(account string) string {
+	safe := func(r rune) rune {
+		switch {
+		case r >= 'a' && r <= 'z':
+			return r
+		case r >= 'A' && r <= 'Z':
+			return unicode.ToLower(r)
+		default:
+			// Negative value tells strings.Map to drop the rune.
+			return -1
+		}
+	}
+	return strings.Map(safe, account)
+}

pkg/cmd/roachprod/vm/vm_test.go

@@ -108,3 +108,34 @@ func TestVM_ZoneEntry(t *testing.T) {
 		})
 	}
 }
+
+func TestDNSSafeAccount(t *testing.T) {
+
+	cases := []struct {
+		description, input, expected string
+	}{
+		{
+			"regular", "username", "username",
+		},
+		{
+			"mixed case", "UserName", "username",
+		},
+		{
+			"dot", "user.name", "username",
+		},
+		{
+			"underscore", "user_name", "username",
+		},
+		{
+			"dot and underscore", "u.ser_n.a_me", "username",
+		},
+		{
+			"Unicode and other characters", "~/❦u.ser_ऄn.a_meλ", "username",
+		},
+	}
+	for _, c := range cases {
+		t.Run(c.description, func(t *testing.T) {
+			assert.EqualValues(t, DNSSafeAccount(c.input), c.expected)
+		})
+	}
+}

pkg/sql/type_change.go

@@ -370,7 +370,9 @@ func (t *typeSchemaChanger) exec(ctx context.Context) error {
 					return err
 				}
 				if fn := t.execCfg.TypeSchemaChangerTestingKnobs.RunBeforeMultiRegionUpdates; fn != nil {
-					return fn()
+					if err := fn(); err != nil {
+						return err
+					}
 				}
 				repartitionedTables, err = performMultiRegionFinalization(
 					ctx,