From e0951c44629d82b01c4a0f6c1e2aa062db6c1135 Mon Sep 17 00:00:00 2001
From: "vitess-bot[bot]" <108069721+vitess-bot[bot]@users.noreply.github.com>
Date: Tue, 17 Oct 2023 12:12:48 -0400
Subject: [PATCH] VReplication: error on vtctldclient commands w/o tablet types
 (#14294)

Signed-off-by: Matt Lord <mattalord@gmail.com>
---
 .github/CODEOWNERS                            |  3 +-
 .../command/vreplication/common/utils.go      | 15 +++-
 .../command/vreplication/common/utils_test.go | 82 +++++++++++++++++++
 .../command/vreplication/workflow/update.go   |  3 +
 4 files changed, 99 insertions(+), 4 deletions(-)
 create mode 100644 go/cmd/vtctldclient/command/vreplication/common/utils_test.go

diff --git a/.github/CODEOWNERS b/.github/CODEOWNERS
index b541376f4de..755ca395b68 100644
--- a/.github/CODEOWNERS
+++ b/.github/CODEOWNERS
@@ -17,7 +17,8 @@ go.sum @ajm188 @deepthi @harshit-gangal @mattlord @rohit-nayak-ps @systay @froui
 /go/cache @vmg
 /go/cmd @ajm188 @deepthi
 /go/cmd/vtadmin @ajm188 @notfelineit
-/go/cmd/vtctldclient @ajm188 @notfelineit
+/go/cmd/vtctldclient @ajm188 @mattlord
+/go/cmd/vtctldclient/command/vreplication @mattlord @rohit-nayak-ps
 /go/internal/flag @ajm188 @rohit-nayak-ps
 /go/mysql @harshit-gangal @systay @mattlord
 /go/pools @deepthi @harshit-gangal
diff --git a/go/cmd/vtctldclient/command/vreplication/common/utils.go b/go/cmd/vtctldclient/command/vreplication/common/utils.go
index 9f908e33c39..be3e52f9ab9 100644
--- a/go/cmd/vtctldclient/command/vreplication/common/utils.go
+++ b/go/cmd/vtctldclient/command/vreplication/common/utils.go
@@ -106,10 +106,17 @@ func ParseCells(cmd *cobra.Command) {
 	}
 }
 
-func ParseTabletTypes(cmd *cobra.Command) {
-	if !cmd.Flags().Lookup("tablet-types").Changed {
+func ParseTabletTypes(cmd *cobra.Command) error {
+	ttf := cmd.Flags().Lookup("tablet-types")
+	if ttf == nil {
+		return fmt.Errorf("no tablet-types flag found")
+	}
+	if !ttf.Changed {
 		CreateOptions.TabletTypes = tabletTypesDefault
+	} else if strings.TrimSpace(ttf.Value.String()) == "" {
+		return fmt.Errorf("invalid tablet-types value, at least one valid tablet type must be specified")
 	}
+	return nil
 }
 
 func validateOnDDL(cmd *cobra.Command) error {
@@ -124,7 +131,9 @@ func ParseAndValidateCreateOptions(cmd *cobra.Command) error {
 		return err
 	}
 	ParseCells(cmd)
-	ParseTabletTypes(cmd)
+	if err := ParseTabletTypes(cmd); err != nil {
+		return err
+	}
 	return nil
 }
 
diff --git a/go/cmd/vtctldclient/command/vreplication/common/utils_test.go b/go/cmd/vtctldclient/command/vreplication/common/utils_test.go
new file mode 100644
index 00000000000..77672d82350
--- /dev/null
+++ b/go/cmd/vtctldclient/command/vreplication/common/utils_test.go
@@ -0,0 +1,82 @@
+/*
+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 common
+
+import (
+	"testing"
+
+	"github.com/spf13/cobra"
+)
+
+func TestParseAndValidateCreateOptions(t *testing.T) {
+	tests := []struct {
+		name    string
+		setFunc func(*cobra.Command) error
+		wantErr bool
+	}{
+		{
+			name: "invalid tablet type",
+			setFunc: func(cmd *cobra.Command) error {
+				tabletTypesFlag := cmd.Flags().Lookup("tablet-types")
+				err := tabletTypesFlag.Value.Set("invalid")
+				tabletTypesFlag.Changed = true
+				return err
+			},
+			wantErr: true,
+		},
+		{
+			name: "no tablet types",
+			setFunc: func(cmd *cobra.Command) error {
+				tabletTypesFlag := cmd.Flags().Lookup("tablet-types")
+				err := tabletTypesFlag.Value.Set("")
+				tabletTypesFlag.Changed = true
+				return err
+			},
+			wantErr: true,
+		},
+		{
+			name: "valid tablet types",
+			setFunc: func(cmd *cobra.Command) error {
+				tabletTypesFlag := cmd.Flags().Lookup("tablet-types")
+				err := tabletTypesFlag.Value.Set("rdonly,replica")
+				tabletTypesFlag.Changed = true
+				return err
+			},
+			wantErr: false,
+		},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			cmd := &cobra.Command{}
+			AddCommonCreateFlags(cmd)
+			test := func() error {
+				if tt.setFunc != nil {
+					if err := tt.setFunc(cmd); err != nil {
+						return err
+					}
+				}
+				if err := ParseAndValidateCreateOptions(cmd); err != nil {
+					return err
+				}
+				return nil
+			}
+			if err := test(); (err != nil) != tt.wantErr {
+				t.Errorf("ParseAndValidateCreateOptions() error = %v, wantErr %t", err, tt.wantErr)
+			}
+		})
+	}
+}
diff --git a/go/cmd/vtctldclient/command/vreplication/workflow/update.go b/go/cmd/vtctldclient/command/vreplication/workflow/update.go
index 404900ddf8e..466d81e8be4 100644
--- a/go/cmd/vtctldclient/command/vreplication/workflow/update.go
+++ b/go/cmd/vtctldclient/command/vreplication/workflow/update.go
@@ -60,6 +60,9 @@ var (
 				updateOptions.Cells = textutil.SimulatedNullStringSlice
 			}
 			if cmd.Flags().Lookup("tablet-types").Changed {
+				if err := common.ParseTabletTypes(cmd); err != nil {
+					return err
+				}
 				changes = true
 			} else {
 				updateOptions.TabletTypes = []topodatapb.TabletType{topodatapb.TabletType(textutil.SimulatedNullInt)}