feat: fix parsing of keyranges and return an error if invalid

Signed-off-by: Manan Gupta <[email protected]>

go/vt/vtorc/logic/keyspace_shard_discovery_test.go

@@ -93,7 +93,8 @@ func TestRefreshAllKeyspaces(t *testing.T) {
 	// Set clusters to watch to only watch ks1 and ks3
 	onlyKs1and3 := []string{"ks1/-80", "ks3/-80", "ks3/80-"}
 	clustersToWatch = onlyKs1and3
-	initializeShardsToWatch()
+	err := initializeShardsToWatch()
+	require.NoError(t, err)
 	require.NoError(t, RefreshAllKeyspacesAndShards(context.Background()))
 
 	// Verify that we only have ks1 and ks3 in vtorc's db.
@@ -107,7 +108,8 @@ func TestRefreshAllKeyspaces(t *testing.T) {
 
 	// Set clusters to watch to watch all keyspaces
 	clustersToWatch = nil
-	initializeShardsToWatch()
+	err = initializeShardsToWatch()
+	require.NoError(t, err)
 	// Change the durability policy of ks1
 	reparenttestutil.SetKeyspaceDurability(ctx, t, ts, "ks1", policy.DurabilitySemiSync)
 	require.NoError(t, RefreshAllKeyspacesAndShards(context.Background()))

go/vt/vtorc/logic/tablet_discovery.go

@@ -66,10 +66,10 @@ func RegisterFlags(fs *pflag.FlagSet) {
 
 // initializeShardsToWatch parses the --clusters_to_watch flag-value
 // into a map of keyspace/shards.
-func initializeShardsToWatch() {
+func initializeShardsToWatch() error {
 	shardsToWatch = make(map[string][]*topodatapb.KeyRange)
 	if len(clustersToWatch) == 0 {
-		return
+		return nil
 	}
 
 	for _, ks := range clustersToWatch {
@@ -80,10 +80,13 @@ func initializeShardsToWatch() {
 				log.Errorf("Could not parse keyspace/shard %q: %+v", ks, err)
 				continue
 			}
+			if !key.IsValidKeyRange(s) {
+				return fmt.Errorf("Invalid key range %q while parsing clusters to watch", s)
+			}
 			// Parse the shard name into key range value.
 			_, keyRange, err := topo.ValidateShardName(s)
 			if err != nil {
-				log.Errorf("Could not parse shard name %q: %+v", s, err)
+				return fmt.Errorf("Could not parse shard name %q: %+v", s, err)
 			}
 			// If the key range is nil, then the user is not using RangeBased Sharding.
 			// So we want to watch all the shards of the keyspace.
@@ -102,6 +105,7 @@ func initializeShardsToWatch() {
 	if len(shardsToWatch) == 0 {
 		log.Error("No keyspace/shards to watch, watching all keyspaces")
 	}
+	return nil
 }
 
 // tabletPartOfWatch checks if the given tablet is part of the watch list.
@@ -137,7 +141,10 @@ func OpenTabletDiscovery() <-chan time.Time {
 		log.Error(err)
 	}
 	// Parse --clusters_to_watch into a filter.
-	initializeShardsToWatch()
+	err := initializeShardsToWatch()
+	if err != nil {
+		log.Fatalf("Error parsing clusters to watch: %v", err)
+	}
 	// We refresh all information from the topo once before we start the ticks to do
 	// it on a timer.
 	ctx, cancel := context.WithTimeout(context.Background(), topo.RemoteOperationTimeout)

go/vt/vtorc/logic/tablet_discovery_test.go

@@ -216,7 +216,8 @@ func TestTabletsPartOfWatch(t *testing.T) {
 	for _, tt := range testCases {
 		t.Run(fmt.Sprintf("%v-Tablet-%v-%v", strings.Join(tt.in, ","), tt.tablet.GetKeyspace(), tt.tablet.GetShard()), func(t *testing.T) {
 			clustersToWatch = tt.in
-			initializeShardsToWatch()
+			err := initializeShardsToWatch()
+			require.NoError(t, err)
 			assert.Equal(t, tt.expectedPartOfWatch, tabletPartOfWatch(tt.tablet))
 		})
 	}
@@ -232,8 +233,9 @@ func TestInitializeShardsToWatch(t *testing.T) {
 	}()
 
 	testCases := []struct {
-		in       []string
-		expected map[string][]*topodatapb.KeyRange
+		in          []string
+		expected    map[string][]*topodatapb.KeyRange
+		expectedErr string
 	}{
 		{
 			in:       []string{},
@@ -255,6 +257,18 @@ func TestInitializeShardsToWatch(t *testing.T) {
 				},
 			},
 		},
+		{
+			in:          []string{"test/324"},
+			expectedErr: `Invalid key range "324" while parsing clusters to watch`,
+		},
+		{
+			in: []string{"test/0"},
+			expected: map[string][]*topodatapb.KeyRange{
+				"test": {
+					key.NewCompleteKeyRange(),
+				},
+			},
+		},
 		{
 			in: []string{"test/-", "test2/-80", "test2/80-"},
 			expected: map[string][]*topodatapb.KeyRange{
@@ -293,7 +307,12 @@ func TestInitializeShardsToWatch(t *testing.T) {
 				shardsToWatch = make(map[string][]*topodatapb.KeyRange, 0)
 			}()
 			clustersToWatch = testCase.in
-			initializeShardsToWatch()
+			err := initializeShardsToWatch()
+			if testCase.expectedErr != "" {
+				require.EqualError(t, err, testCase.expectedErr)
+				return
+			}
+			require.NoError(t, err)
 			require.Equal(t, testCase.expected, shardsToWatch)
 		})
 	}