[v13] update database and kube name validation

api/types/database.go

@@ -19,6 +19,7 @@ package types
 import (
 	"encoding/json"
 	"fmt"
+	"regexp"
 	"strings"
 	"time"
 
@@ -565,6 +566,24 @@ func (d *DatabaseV3) setStaticFields() {
 	d.Version = V3
 }
 
+// validDatabaseNameRegexp filters the allowed characters in database names.
+// This is the (almost) the same regexp used to check for valid DNS 1035 labels,
+// except we allow uppercase chars.
+var validDatabaseNameRegexp = regexp.MustCompile(`^[a-zA-Z]([-a-zA-Z0-9]*[a-zA-Z0-9])?$`)
+
+// ValidateDatabaseName returns an error if a given string is not a valid
+// Database name.
+// Unlike application access proxy, database name doesn't necessarily
+// need to be a valid subdomain but use the same validation logic for the
+// simplicity and consistency, except two differences: don't restrict names to
+// 63 chars in length and allow upper case chars.
+// This was added in v14 and backported, except that it's intentionally called
+// in lib/services:ValidateDatabase instead of within CheckAndSetDefaults below
+// for backwards compatibility.
+func ValidateDatabaseName(name string) error {
+	return ValidateResourceName(validDatabaseNameRegexp, name)
+}
+
 // CheckAndSetDefaults checks and sets default values for any missing fields.
 func (d *DatabaseV3) CheckAndSetDefaults() error {
 	d.setStaticFields()

api/types/database_test.go

@@ -17,6 +17,7 @@ limitations under the License.
 package types
 
 import (
+	"strings"
 	"testing"
 
 	"github.com/google/go-cmp/cmp"
@@ -890,3 +891,40 @@ func TestAWSIsEmpty(t *testing.T) {
 		})
 	}
 }
+
+func TestValidateDatabaseName(t *testing.T) {
+	t.Parallel()
+
+	tests := []struct {
+		name              string
+		dbName            string
+		expectErrContains string
+	}{
+		{
+			name:   "valid long name and uppercase chars",
+			dbName: strings.Repeat("aA", 100),
+		},
+		{
+			name:              "invalid trailing hyphen",
+			dbName:            "invalid-database-name-",
+			expectErrContains: `"invalid-database-name-" does not match regex`,
+		},
+		{
+			name:              "invalid first character",
+			dbName:            "1-invalid-database-name",
+			expectErrContains: `"1-invalid-database-name" does not match regex`,
+		},
+	}
+
+	for _, test := range tests {
+		t.Run(test.name, func(t *testing.T) {
+			err := ValidateDatabaseName(test.dbName)
+			if test.expectErrContains != "" {
+				require.Error(t, err)
+				require.ErrorContains(t, err, test.expectErrContains)
+				return
+			}
+			require.NoError(t, err)
+		})
+	}
+}

api/types/databaseserver_test.go

@@ -108,31 +108,6 @@ func TestDatabaseServerSorter(t *testing.T) {
 		},
 	}
 
-	makeServers := func(testVals []string, testField string) []DatabaseServer {
-		servers := make([]DatabaseServer, len(testVals))
-		for i := 0; i < len(testVals); i++ {
-			testVal := testVals[i]
-			dbSpec := dbSpecs[i%len(dbSpecs)]
-			var err error
-
-			servers[i], err = NewDatabaseServerV3(Metadata{
-				Name: "_",
-			}, DatabaseServerSpecV3{
-				HostID:   "_",
-				Hostname: "_",
-				Database: &DatabaseV3{
-					Metadata: Metadata{
-						Name:        getTestVal(testField == ResourceMetadataName, testVal),
-						Description: getTestVal(testField == ResourceSpecDescription, testVal),
-					},
-					Spec: dbSpec,
-				},
-			})
-			require.NoError(t, err)
-		}
-		return servers
-	}
-
 	cases := []struct {
 		name      string
 		wantErr   bool
@@ -156,7 +131,7 @@ func TestDatabaseServerSorter(t *testing.T) {
 		c := c
 		t.Run(fmt.Sprintf("%s desc", c.name), func(t *testing.T) {
 			sortBy := SortBy{Field: c.fieldName, IsDesc: true}
-			servers := DatabaseServers(makeServers(testValsUnordered, c.fieldName))
+			servers := DatabaseServers(makeServers(t, testValsUnordered, dbSpecs, c.fieldName))
 			require.NoError(t, servers.SortByCustom(sortBy))
 			targetVals, err := servers.GetFieldVals(c.fieldName)
 			require.NoError(t, err)
@@ -165,7 +140,7 @@ func TestDatabaseServerSorter(t *testing.T) {
 
 		t.Run(fmt.Sprintf("%s asc", c.name), func(t *testing.T) {
 			sortBy := SortBy{Field: c.fieldName}
-			servers := DatabaseServers(makeServers(testValsUnordered, c.fieldName))
+			servers := DatabaseServers(makeServers(t, testValsUnordered, dbSpecs, c.fieldName))
 			require.NoError(t, servers.SortByCustom(sortBy))
 			targetVals, err := servers.GetFieldVals(c.fieldName)
 			require.NoError(t, err)
@@ -175,6 +150,32 @@ func TestDatabaseServerSorter(t *testing.T) {
 
 	// Test error.
 	sortBy := SortBy{Field: "unsupported"}
-	servers := makeServers(testValsUnordered, "does-not-matter")
+	servers := makeServers(t, testValsUnordered, dbSpecs, "does-not-matter")
 	require.True(t, trace.IsNotImplemented(DatabaseServers(servers).SortByCustom(sortBy)))
 }
+
+func makeServers(t *testing.T, testVals []string, dbSpecs []DatabaseSpecV3, testField string) []DatabaseServer {
+	t.Helper()
+	servers := make([]DatabaseServer, len(testVals))
+	for i := 0; i < len(testVals); i++ {
+		testVal := testVals[i]
+		dbSpec := dbSpecs[i%len(dbSpecs)]
+		var err error
+
+		servers[i], err = NewDatabaseServerV3(Metadata{
+			Name: "foo",
+		}, DatabaseServerSpecV3{
+			HostID:   "_",
+			Hostname: "_",
+			Database: &DatabaseV3{
+				Metadata: Metadata{
+					Name:        getTestVal(testField == ResourceMetadataName, testVal),
+					Description: getTestVal(testField == ResourceSpecDescription, testVal),
+				},
+				Spec: dbSpec,
+			},
+		})
+		require.NoError(t, err)
+	}
+	return servers
+}

api/types/kubernetes.go

@@ -334,6 +334,12 @@ func (k *KubernetesClusterV3) setStaticFields() {
 // sneaky cluster names being used for client directory traversal and exploits.
 var validKubeClusterName = regexp.MustCompile(`^[a-zA-Z0-9._-]+$`)
 
+// ValidateKubeClusterName returns an error if a given string is not a valid
+// KubeCluster name.
+func ValidateKubeClusterName(name string) error {
+	return ValidateResourceName(validKubeClusterName, name)
+}
+
 // CheckAndSetDefaults checks and sets default values for any missing fields.
 func (k *KubernetesClusterV3) CheckAndSetDefaults() error {
 	k.setStaticFields()
@@ -346,8 +352,8 @@ func (k *KubernetesClusterV3) CheckAndSetDefaults() error {
 		}
 	}
 
-	if !validKubeClusterName.MatchString(k.Metadata.Name) {
-		return trace.BadParameter("invalid kubernetes cluster name: %q", k.Metadata.Name)
+	if err := ValidateKubeClusterName(k.Metadata.Name); err != nil {
+		return trace.Wrap(err, "invalid kubernetes cluster name")
 	}
 
 	if err := k.Spec.Azure.CheckAndSetDefaults(); err != nil && k.IsAzure() {

api/types/resource.go

@@ -524,3 +524,14 @@ type ListResourcesResponse struct {
 	// TotalCount is the total number of resources available as a whole.
 	TotalCount int
 }
+
+// ValidateResourceName validates a resource name using a given regexp.
+func ValidateResourceName(validationRegex *regexp.Regexp, name string) error {
+	if validationRegex.MatchString(name) {
+		return nil
+	}
+	return trace.BadParameter(
+		"%q does not match regex used for validation %q",
+		name, validationRegex.String(),
+	)
+}

api/types/resource_test.go

@@ -128,12 +128,12 @@ func TestMatchSearch_ResourceSpecific(t *testing.T) {
 		// searchNotDefined refers to resources where the searcheable field values are not defined.
 		searchNotDefined   bool
 		matchingSearchVals []string
-		newResource        func() ResourceWithLabels
+		newResource        func(*testing.T) ResourceWithLabels
 	}{
 		{
 			name:               "node",
 			matchingSearchVals: []string{"foo", "bar", "prod", "os"},
-			newResource: func() ResourceWithLabels {
+			newResource: func(t *testing.T) ResourceWithLabels {
 				server, err := NewServerWithLabels("_", KindNode, ServerSpecV2{
 					Hostname: "foo",
 					Addr:     "bar",
@@ -146,7 +146,7 @@ func TestMatchSearch_ResourceSpecific(t *testing.T) {
 		{
 			name:               "node using tunnel",
 			matchingSearchVals: []string{"tunnel"},
-			newResource: func() ResourceWithLabels {
+			newResource: func(t *testing.T) ResourceWithLabels {
 				server, err := NewServer("_", KindNode, ServerSpecV2{
 					UseTunnel: true,
 				})
@@ -158,7 +158,7 @@ func TestMatchSearch_ResourceSpecific(t *testing.T) {
 		{
 			name:               "windows desktop",
 			matchingSearchVals: []string{"foo", "bar", "env", "prod", "os"},
-			newResource: func() ResourceWithLabels {
+			newResource: func(t *testing.T) ResourceWithLabels {
 				desktop, err := NewWindowsDesktopV3("foo", labels, WindowsDesktopSpecV3{
 					Addr: "bar",
 				})
@@ -170,7 +170,7 @@ func TestMatchSearch_ResourceSpecific(t *testing.T) {
 		{
 			name:               "application",
 			matchingSearchVals: []string{"foo", "bar", "baz", "mac"},
-			newResource: func() ResourceWithLabels {
+			newResource: func(t *testing.T) ResourceWithLabels {
 				app, err := NewAppV3(Metadata{
 					Name:        "foo",
 					Description: "bar",
@@ -187,7 +187,7 @@ func TestMatchSearch_ResourceSpecific(t *testing.T) {
 		{
 			name:               "kube cluster",
 			matchingSearchVals: []string{"foo", "prod", "env"},
-			newResource: func() ResourceWithLabels {
+			newResource: func(t *testing.T) ResourceWithLabels {
 				kc, err := NewKubernetesClusterV3FromLegacyCluster("_", &KubernetesCluster{
 					Name:         "foo",
 					StaticLabels: labels,
@@ -200,7 +200,7 @@ func TestMatchSearch_ResourceSpecific(t *testing.T) {
 		{
 			name:               "database",
 			matchingSearchVals: []string{"foo", "bar", "baz", "prod", DatabaseTypeRedshift},
-			newResource: func() ResourceWithLabels {
+			newResource: func(t *testing.T) ResourceWithLabels {
 				db, err := NewDatabaseV3(Metadata{
 					Name:        "foo",
 					Description: "bar",
@@ -222,9 +222,9 @@ func TestMatchSearch_ResourceSpecific(t *testing.T) {
 		{
 			name:               "database with gcp keywords",
 			matchingSearchVals: []string{"cloud", "cloud sql"},
-			newResource: func() ResourceWithLabels {
+			newResource: func(t *testing.T) ResourceWithLabels {
 				db, err := NewDatabaseV3(Metadata{
-					Name:   "_",
+					Name:   "foo",
 					Labels: labels,
 				}, DatabaseSpecV3{
 					Protocol: "_",
@@ -242,9 +242,9 @@ func TestMatchSearch_ResourceSpecific(t *testing.T) {
 		{
 			name:             "app server",
 			searchNotDefined: true,
-			newResource: func() ResourceWithLabels {
+			newResource: func(t *testing.T) ResourceWithLabels {
 				appServer, err := NewAppServerV3(Metadata{
-					Name: "_",
+					Name: "foo",
 				}, AppServerSpecV3{
 					HostID: "_",
 					App:    &AppV3{Metadata: Metadata{Name: "_"}, Spec: AppSpecV3{URI: "_"}},
@@ -257,9 +257,9 @@ func TestMatchSearch_ResourceSpecific(t *testing.T) {
 		{
 			name:             "db server",
 			searchNotDefined: true,
-			newResource: func() ResourceWithLabels {
+			newResource: func(t *testing.T) ResourceWithLabels {
 				dbServer, err := NewDatabaseServerV3(Metadata{
-					Name: "_",
+					Name: "foo",
 				}, DatabaseServerSpecV3{
 					HostID:   "_",
 					Hostname: "_",
@@ -272,10 +272,10 @@ func TestMatchSearch_ResourceSpecific(t *testing.T) {
 		{
 			name:             "kube server",
 			searchNotDefined: true,
-			newResource: func() ResourceWithLabels {
+			newResource: func(t *testing.T) ResourceWithLabels {
 				kubeServer, err := NewKubernetesServerV3(
 					Metadata{
-						Name: "_",
+						Name: "foo",
 					}, KubernetesServerSpecV3{
 						HostID:   "_",
 						Hostname: "_",
@@ -293,7 +293,7 @@ func TestMatchSearch_ResourceSpecific(t *testing.T) {
 		{
 			name:             "desktop service",
 			searchNotDefined: true,
-			newResource: func() ResourceWithLabels {
+			newResource: func(t *testing.T) ResourceWithLabels {
 				desktopService, err := NewWindowsDesktopServiceV3(Metadata{
 					Name: "foo",
 				}, WindowsDesktopServiceSpecV3{
@@ -312,7 +312,7 @@ func TestMatchSearch_ResourceSpecific(t *testing.T) {
 		t.Run(tc.name, func(t *testing.T) {
 			t.Parallel()
 
-			resource := tc.newResource()
+			resource := tc.newResource(t)
 
 			// Nil search values, should always return true
 			match := resource.MatchSearch(nil)

api/types/server_test.go

@@ -31,7 +31,7 @@ func getTestVal(isTestField bool, testVal string) string {
 		return testVal
 	}
 
-	return "_"
+	return "foo"
 }
 
 func TestServerSorter(t *testing.T) {

lib/integrations/awsoidc/listdatabases_test.go

@@ -24,7 +24,6 @@ import (
 
 	"github.com/aws/aws-sdk-go-v2/service/rds"
 	rdsTypes "github.com/aws/aws-sdk-go-v2/service/rds/types"
-	"github.com/google/uuid"
 	"github.com/gravitational/trace"
 	"github.com/stretchr/testify/require"
 
@@ -99,7 +98,7 @@ func TestListDatabases(t *testing.T) {
 		for i := 0; i < totalDBs; i++ {
 			allInstances = append(allInstances, rdsTypes.DBInstance{
 				DBInstanceStatus:     stringPointer("available"),
-				DBInstanceIdentifier: stringPointer(uuid.NewString()),
+				DBInstanceIdentifier: stringPointer(fmt.Sprintf("db-%v", i)),
 				DbiResourceId:        stringPointer("db-123"),
 				DBInstanceArn:        stringPointer("arn:aws:iam::123456789012:role/MyARN"),
 				Engine:               stringPointer("postgres"),

lib/services/database.go

@@ -44,7 +44,6 @@ import (
 	"go.mongodb.org/mongo-driver/mongo/readpref"
 	"go.mongodb.org/mongo-driver/x/mongo/driver/connstring"
 	"golang.org/x/exp/slices"
-	"k8s.io/apimachinery/pkg/util/validation"
 
 	"github.com/gravitational/teleport/api/types"
 	apiawsutils "github.com/gravitational/teleport/api/utils/aws"
@@ -147,11 +146,11 @@ func ValidateDatabase(db types.Database) error {
 		return trace.Wrap(err)
 	}
 
-	// Unlike application access proxy, database proxy name doesn't necessarily
-	// need to be a valid subdomain but use the same validation logic for the
-	// simplicity and consistency.
-	if errs := validation.IsDNS1035Label(db.GetName()); len(errs) > 0 {
-		return trace.BadParameter("invalid database %q name: %v", db.GetName(), errs)
+	// This was added in v14 and backported, except that it's intentionally
+	// called here instead of in CheckAndSetDefaults for backwards
+	// compatibility.
+	if err := types.ValidateDatabaseName(db.GetName()); err != nil {
+		return trace.Wrap(err, "invalid database name")
 	}
 
 	if !slices.Contains(defaults.DatabaseProtocols, db.GetProtocol()) {