Add vtctldclient TLS support to vtadmin GRPC

go/cmd/vtadmin/main.go

@@ -36,6 +36,7 @@ import (
 	vtadminhttp "vitess.io/vitess/go/vt/vtadmin/http"
 	"vitess.io/vitess/go/vt/vtadmin/http/debug"
 	"vitess.io/vitess/go/vt/vtadmin/rbac"
+	"vitess.io/vitess/go/vt/vtctl/grpcclientcommon"
 	"vitess.io/vitess/go/vt/vtenv"
 )
 
@@ -160,7 +161,7 @@ func run(cmd *cobra.Command, args []string) {
 	}
 }
 
-func main() {
+func registerFlags() {
 	// Common flags
 	rootCmd.Flags().StringVar(&opts.Addr, "addr", ":15000", "address to serve on")
 	rootCmd.Flags().DurationVar(&opts.CMuxReadTimeout, "lmux-read-timeout", time.Second, "how long to spend connection muxing")
@@ -218,6 +219,14 @@ func main() {
 
 	servenv.RegisterMySQLServerFlags(rootCmd.Flags())
 
+	// Register TLS flags for gRPC connections to vtctld
+	grpcclientcommon.RegisterFlags(rootCmd.Flags())
+
+}
+
+func main() {
+	registerFlags()
+
 	if err := rootCmd.Execute(); err != nil {
 		log.Fatal(err)
 	}

go/cmd/vtadmin/main_test.go

@@ -0,0 +1,52 @@
+/*
+Copyright 2025 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 main
+
+import (
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+)
+
+func TestMainFlagRegistration(t *testing.T) {
+	registerFlags()
+
+	// Test that the TLS flags are properly registered
+	t.Run("grpc tls flags are registered", func(t *testing.T) {
+		certFlag := rootCmd.Flags().Lookup("vtctld-grpc-cert")
+		require.NotNil(t, certFlag, "vtctld-grpc-cert flag should be registered")
+		assert.Equal(t, "", certFlag.DefValue)
+		assert.Equal(t, "the cert to use to connect", certFlag.Usage)
+
+		keyFlag := rootCmd.Flags().Lookup("vtctld-grpc-key")
+		require.NotNil(t, keyFlag, "vtctld-grpc-key flag should be registered")
+		assert.Equal(t, "", keyFlag.DefValue)
+
+		caFlag := rootCmd.Flags().Lookup("vtctld-grpc-ca")
+		require.NotNil(t, caFlag, "vtctld-grpc-ca flag should be registered")
+		assert.Equal(t, "", caFlag.DefValue)
+
+		crlFlag := rootCmd.Flags().Lookup("vtctld-grpc-crl")
+		require.NotNil(t, crlFlag, "vtctld-grpc-crl flag should be registered")
+		assert.Equal(t, "", crlFlag.DefValue)
+
+		serverNameFlag := rootCmd.Flags().Lookup("vtctld-grpc-server-name")
+		require.NotNil(t, serverNameFlag, "vtctld-grpc-server-name flag should be registered")
+		assert.Equal(t, "", serverNameFlag.DefValue)
+	})
+}

go/vt/vtadmin/vtctldclient/proxy.go

@@ -21,8 +21,6 @@ import (
 	"sync"
 	"time"
 
-	"google.golang.org/grpc/credentials/insecure"
-
 	"google.golang.org/grpc"
 	grpcresolver "google.golang.org/grpc/resolver"
 
@@ -32,6 +30,7 @@ import (
 	"vitess.io/vitess/go/vt/vtadmin/cluster/resolver"
 	"vitess.io/vitess/go/vt/vtadmin/debug"
 	"vitess.io/vitess/go/vt/vtadmin/vtadminproto"
+	"vitess.io/vitess/go/vt/vtctl/grpcclientcommon"
 	"vitess.io/vitess/go/vt/vtctl/grpcvtctldclient"
 	"vitess.io/vitess/go/vt/vtctl/vtctldclient"
 
@@ -111,13 +110,14 @@ func (vtctld *ClientProxy) dial(ctx context.Context) error {
 	vtadminproto.AnnotateClusterSpan(vtctld.cluster, span)
 	span.Annotate("is_using_credentials", vtctld.creds != nil)
 
-	opts := []grpc.DialOption{
-		// TODO: make configurable. right now, omitting this and attempting
-		// to not use TLS results in:
-		//		grpc: no transport security set (use grpc.WithInsecure() explicitly or set credentials)
-		grpc.WithTransportCredentials(insecure.NewCredentials()),
+	// Get TLS configuration from command-line flags
+	tlsOpt, err := grpcclientcommon.SecureDialOption()
+	if err != nil {
+		return err
 	}
 
+	opts := []grpc.DialOption{tlsOpt}
+
 	if vtctld.creds != nil {
 		opts = append(opts, grpc.WithPerRPCCredentials(vtctld.creds))
 	}

go/vt/vtadmin/vtctldclient/proxy_test.go

@@ -19,16 +19,19 @@ package vtctldclient
 import (
 	"context"
 	"net"
+	"os"
 	"sync"
 	"testing"
 	"time"
 
+	"github.com/spf13/pflag"
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
 	"google.golang.org/grpc"
 
 	"vitess.io/vitess/go/vt/vtadmin/cluster/discovery/fakediscovery"
 	"vitess.io/vitess/go/vt/vtadmin/cluster/resolver"
+	"vitess.io/vitess/go/vt/vtctl/grpcclientcommon"
 
 	vtadminpb "vitess.io/vitess/go/vt/proto/vtadmin"
 	vtctldatapb "vitess.io/vitess/go/vt/proto/vtctldata"
@@ -228,3 +231,72 @@ func TestRedial(t *testing.T) {
 	require.NoError(t, err)
 	assert.Equal(t, nextAddr, resp.Keyspace.Name)
 }
+
+func TestDialSecureDialOptionError(t *testing.T) {
+	// Test that grpcclientcommon.SecureDialOption() returning an error in proxy.dial()
+
+	// Create temporary files with invalid content that will cause TLS parsing to fail
+	tmpCert, err := os.CreateTemp("", "invalid-cert-*.pem")
+	require.NoError(t, err)
+	defer os.Remove(tmpCert.Name())
+
+	// Write invalid certificate content
+	_, err = tmpCert.WriteString("invalid certificate data")
+	require.NoError(t, err)
+	tmpCert.Close()
+
+	tmpKey, err := os.CreateTemp("", "invalid-key-*.pem")
+	require.NoError(t, err)
+	defer os.Remove(tmpKey.Name())
+
+	// Write invalid key content
+	_, err = tmpKey.WriteString("invalid key data")
+	require.NoError(t, err)
+	tmpKey.Close()
+
+	// Now test by using command line arguments to trigger the same error in proxy.dial()
+	origArgs := os.Args
+	defer func() { os.Args = origArgs }()
+
+	// Set args that would cause SecureDialOption to fail
+	os.Args = []string{
+		"vtadmin",
+		"--vtctld-grpc-cert=" + tmpCert.Name(),
+		"--vtctld-grpc-key=" + tmpKey.Name(),
+	}
+
+	// Create a custom flag set and parse it to set the grpcclientcommon variables
+	fs := pflag.NewFlagSet("test", pflag.ContinueOnError)
+	grpcclientcommon.RegisterFlags(fs)
+
+	// Parse the flags which will set the cert and key variables in grpcclientcommon
+	err = fs.Parse(os.Args[1:])
+	require.NoError(t, err)
+
+	// Now when we create a proxy, it should fail at line 116
+	disco := fakediscovery.New()
+	disco.AddTaggedVtctlds(nil, &vtadminpb.Vtctld{
+		Hostname: "localhost:15999",
+	})
+
+	cfg := &Config{
+		Cluster: &vtadminpb.Cluster{
+			Id:   "test",
+			Name: "testcluster",
+		},
+		ResolverOptions: &resolver.Options{
+			Discovery:        disco,
+			DiscoveryTimeout: 50 * time.Millisecond,
+		},
+	}
+
+	// This should fail during New() -> dial() -> grpcclientcommon.SecureDialOption() -> line 116
+	proxy, err := New(context.Background(), cfg)
+
+	// Verify that the error comes from TLS configuration
+	assert.Error(t, err, "New should fail with invalid TLS files")
+	assert.Nil(t, proxy, "proxy should be nil when New fails")
+
+	// The error should be from certificate parsing
+	assert.Contains(t, err.Error(), "tls")
+}

go/vt/vtctl/grpcclientcommon/dial_option.go

@@ -34,6 +34,7 @@ func init() {
 	servenv.OnParseFor("vttestserver", RegisterFlags)
 	servenv.OnParseFor("vtctlclient", RegisterFlags)
 	servenv.OnParseFor("vtctldclient", RegisterFlags)
+	servenv.OnParseFor("vtadmin", RegisterFlags)
 }
 
 func RegisterFlags(fs *pflag.FlagSet) {