[Backport] Update topo {Get,Create}Keyspace to prevent invalid keyspace names (#12732)

changelog/16.0/16.0.1/summary.md

@@ -7,3 +7,9 @@ Below is a summary of this Go patch release. You can learn more [here](https://g
 
 > go1.20.2 (released 2023-03-07) includes a security fix to the crypto/elliptic package, as well as bug fixes to the compiler, the covdata command, the linker, the runtime, and the crypto/ecdh, crypto/rsa, crypto/x509, os, and syscall packages.
 
+### Keyspace name validation in TopoServer
+
+Prior to v16.0.1, it was possible to create a keyspace with invalid characters, which would then be inaccessible to various cluster management operations.
+
+ Keyspace names may no longer contain the forward slash ("/") character, and TopoServer's `GetKeyspace` and `CreateKeyspace` methods return an error if given such a name.
+

go/vt/topo/keyspace.go

@@ -18,6 +18,7 @@ package topo
 
 import (
 	"path"
+	"strings"
 
 	"google.golang.org/protobuf/proto"
 
@@ -54,6 +55,20 @@ func (ki *KeyspaceInfo) SetKeyspaceName(name string) {
 	ki.keyspace = name
 }
 
+var invalidKeyspaceNameChars = "/"
+
+// ValidateKeyspaceName checks if the provided name is a valid name for a
+// keyspace.
+//
+// As of v16.0.1, "all invalid characters" is just the forward slash ("/").
+func ValidateKeyspaceName(name string) error {
+	if strings.ContainsAny(name, invalidKeyspaceNameChars) {
+		return vterrors.Errorf(vtrpcpb.Code_INVALID_ARGUMENT, "keyspace name %s contains invalid characters; may not contain any of the following: %+v", name, strings.Split(invalidKeyspaceNameChars, ""))
+	}
+
+	return nil
+}
+
 // GetServedFrom returns a Keyspace_ServedFrom record if it exists.
 func (ki *KeyspaceInfo) GetServedFrom(tabletType topodatapb.TabletType) *topodatapb.Keyspace_ServedFrom {
 	for _, ksf := range ki.ServedFroms {
@@ -161,6 +176,10 @@ func (ki *KeyspaceInfo) ComputeCellServedFrom(cell string) []*topodatapb.SrvKeys
 // CreateKeyspace wraps the underlying Conn.Create
 // and dispatches the event.
 func (ts *Server) CreateKeyspace(ctx context.Context, keyspace string, value *topodatapb.Keyspace) error {
+	if err := ValidateKeyspaceName(keyspace); err != nil {
+		return vterrors.Wrapf(err, "CreateKeyspace: %s", err)
+	}
+
 	data, err := proto.Marshal(value)
 	if err != nil {
 		return err
@@ -181,6 +200,10 @@ func (ts *Server) CreateKeyspace(ctx context.Context, keyspace string, value *to
 
 // GetKeyspace reads the given keyspace and returns it
 func (ts *Server) GetKeyspace(ctx context.Context, keyspace string) (*KeyspaceInfo, error) {
+	if err := ValidateKeyspaceName(keyspace); err != nil {
+		return nil, vterrors.Wrapf(err, "GetKeyspace: %s", err)
+	}
+
 	keyspacePath := path.Join(KeyspacesPath, keyspace, KeyspaceFile)
 	data, version, err := ts.globalCell.Get(ctx, keyspacePath)
 	if err != nil {

go/vt/topo/topotests/keyspace_test.go

@@ -0,0 +1,72 @@
+/*
+Copyright 2023 The Vitess Authors.
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+package topotests
+
+import (
+	"context"
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+
+	"vitess.io/vitess/go/vt/topo/memorytopo"
+	"vitess.io/vitess/go/vt/vterrors"
+
+	topodatapb "vitess.io/vitess/go/vt/proto/topodata"
+	"vitess.io/vitess/go/vt/proto/vtrpc"
+)
+
+func TestCreateKeyspace(t *testing.T) {
+	ts := memorytopo.NewServer("zone1")
+	ctx := context.Background()
+
+	t.Run("valid name", func(t *testing.T) {
+		err := ts.CreateKeyspace(ctx, "ks", &topodatapb.Keyspace{})
+		require.NoError(t, err)
+	})
+	t.Run("invalid name", func(t *testing.T) {
+		err := ts.CreateKeyspace(ctx, "no/slashes/allowed", &topodatapb.Keyspace{})
+		assert.Error(t, err)
+		assert.Equal(t, vtrpc.Code_INVALID_ARGUMENT, vterrors.Code(err), "%+v", err)
+	})
+}
+
+func TestGetKeyspace(t *testing.T) {
+	ts := memorytopo.NewServer("zone1")
+	ctx := context.Background()
+
+	t.Run("valid name", func(t *testing.T) {
+		// First, create the keyspace.
+		err := ts.CreateKeyspace(ctx, "ks", &topodatapb.Keyspace{})
+		require.NoError(t, err)
+
+		// Now, get it.
+		ks, err := ts.GetKeyspace(ctx, "ks")
+		require.NoError(t, err)
+		assert.NotNil(t, ks)
+	})
+
+	t.Run("invalid name", func(t *testing.T) {
+		// We can't create the keyspace (because we can't create a keyspace
+		// with an invalid name), so we'll validate the error we get is *not*
+		// NOT_FOUND.
+		ks, err := ts.GetKeyspace(ctx, "no/slashes/allowed")
+		assert.Error(t, err)
+		assert.Equal(t, vtrpc.Code_INVALID_ARGUMENT, vterrors.Code(err), "%+v", err)
+		assert.Nil(t, ks)
+	})
+}

go/vt/vtorc/inst/keyspace_dao.go

@@ -30,6 +30,10 @@ var ErrKeyspaceNotFound = errors.New("keyspace not found")
 
 // ReadKeyspace reads the vitess keyspace record.
 func ReadKeyspace(keyspaceName string) (*topo.KeyspaceInfo, error) {
+	if err := topo.ValidateKeyspaceName(keyspaceName); err != nil {
+		return nil, err
+	}
+
 	query := `
 		select
 			keyspace_type,