From d16ffd11db768f438a80faa8e2c42b4a19f36f27 Mon Sep 17 00:00:00 2001 From: Peter Mattis Date: Mon, 13 Apr 2020 16:44:33 -0400 Subject: [PATCH] cli: add --cert-principal-map to client commands Add support for the `--cert-principal-map` flag to the certs and client commands. Anywhere we were accepting the `--certs-dir` flag, we now also accept the `--cert-principal-map` flag. Fixes #47300 Fixes #47754 Fixes #48116 Release note (cli change): Support the `--cert-principal-map` flag in the `cert *` and "client" commands such as `sql`, `init`, and `quit`. --- pkg/cli/cert.go | 3 --- pkg/cli/context.go | 12 ++++------- pkg/cli/flags.go | 20 +++++++++++++------ pkg/cli/flags_test.go | 8 ++++++-- .../test_cert_advisory_validation.tcl | 18 +++++++++++++++++ 5 files changed, 42 insertions(+), 19 deletions(-) diff --git a/pkg/cli/cert.go b/pkg/cli/cert.go index 771385a8a153..e10b919f40ee 100644 --- a/pkg/cli/cert.go +++ b/pkg/cli/cert.go @@ -203,9 +203,6 @@ List certificates and keys found in the certificate directory. // runListCerts loads and lists all certs. func runListCerts(cmd *cobra.Command, args []string) error { - if err := security.SetCertPrincipalMap(certCtx.certPrincipalMap); err != nil { - return err - } cm, err := security.NewCertificateManager(baseCfg.SSLCertsDir) if err != nil { return errors.Wrap(err, "cannot load certificates") diff --git a/pkg/cli/context.go b/pkg/cli/context.go index 49b9fc27cdb8..d111dc3f929b 100644 --- a/pkg/cli/context.go +++ b/pkg/cli/context.go @@ -77,6 +77,7 @@ func initCLIDefaults() { cliCtx.cmdTimeout = 0 // no timeout cliCtx.clientConnHost = "" cliCtx.clientConnPort = base.DefaultPort + cliCtx.certPrincipalMap = nil cliCtx.sqlConnURL = "" cliCtx.sqlConnUser = "" cliCtx.sqlConnPasswd = "" @@ -173,8 +174,6 @@ func initCLIDefaults() { authCtx.validityPeriod = 1 * time.Hour - certCtx.certPrincipalMap = nil - initPreFlagsDefaults() // Clear the "Changed" state of all the registered command-line flags. @@ -217,6 +216,9 @@ type cliContext struct { // clientConnPort is the port name/number to use to connect to a server. clientConnPort string + // certPrincipalMap is the cert-principal:db-principal map. + certPrincipalMap []string + // for CLI commands that use the SQL interface, these parameters // determine how to connect to the server. sqlConnURL, sqlConnUser, sqlConnDBName string @@ -405,9 +407,3 @@ var demoCtx struct { insecure bool geoLibsDir string } - -// certCtx captures the command-line parameters of the `cert` command. -// Defaults set by InitCLIDefaults() above. -var certCtx struct { - certPrincipalMap []string -} diff --git a/pkg/cli/flags.go b/pkg/cli/flags.go index 5538d8037b23..f84c09db155e 100644 --- a/pkg/cli/flags.go +++ b/pkg/cli/flags.go @@ -240,7 +240,9 @@ func init() { // Every command but start will inherit the following setting. AddPersistentPreRunE(cockroachCmd, func(cmd *cobra.Command, _ []string) error { - extraClientFlagInit() + if err := extraClientFlagInit(); err != nil { + return err + } return setDefaultStderrVerbosity(cmd, log.Severity_WARNING) }) @@ -441,12 +443,11 @@ func init() { f := cmd.Flags() // All certs commands need the certificate directory. StringFlag(f, &baseCfg.SSLCertsDir, cliflags.CertsDir, baseCfg.SSLCertsDir) + // All certs commands get the certificate principal map. + StringSlice(f, &cliCtx.certPrincipalMap, + cliflags.CertPrincipalMap, cliCtx.certPrincipalMap) } - // The list certs command needs the certificate principal map. - StringSlice(listCertsCmd.Flags(), &certCtx.certPrincipalMap, - cliflags.CertPrincipalMap, certCtx.certPrincipalMap) - for _, cmd := range []*cobra.Command{createCACertCmd, createClientCACertCmd} { f := cmd.Flags() // CA certificates have a longer expiration time. @@ -495,6 +496,9 @@ func init() { // Certificate flags. StringFlag(f, &baseCfg.SSLCertsDir, cliflags.CertsDir, baseCfg.SSLCertsDir) + // Certificate principal map. + StringSlice(f, &cliCtx.certPrincipalMap, + cliflags.CertPrincipalMap, cliCtx.certPrincipalMap) } // Auth commands. @@ -878,7 +882,10 @@ func extraServerFlagInit(cmd *cobra.Command) error { return nil } -func extraClientFlagInit() { +func extraClientFlagInit() error { + if err := security.SetCertPrincipalMap(cliCtx.certPrincipalMap); err != nil { + return err + } serverCfg.Addr = net.JoinHostPort(cliCtx.clientConnHost, cliCtx.clientConnPort) serverCfg.AdvertiseAddr = serverCfg.Addr serverCfg.SQLAddr = net.JoinHostPort(cliCtx.clientConnHost, cliCtx.clientConnPort) @@ -894,6 +901,7 @@ func extraClientFlagInit() { if sqlCtx.debugMode { sqlCtx.echo = true } + return nil } func setDefaultStderrVerbosity(cmd *cobra.Command, defaultSeverity log.Severity) error { diff --git a/pkg/cli/flags_test.go b/pkg/cli/flags_test.go index 1a0890940d2e..f2c0323586c7 100644 --- a/pkg/cli/flags_test.go +++ b/pkg/cli/flags_test.go @@ -799,7 +799,9 @@ func TestServerJoinSettings(t *testing.T) { t.Fatalf("Parse(%#v) got unexpected error: %v", td.args, err) } - extraClientFlagInit() + if err := extraClientFlagInit(); err != nil { + t.Fatal(err) + } var actual []string myHostname, _ := os.Hostname() @@ -861,7 +863,9 @@ func TestClientConnSettings(t *testing.T) { t.Fatalf("Parse(%#v) got unexpected error: %v", td.args, err) } - extraClientFlagInit() + if err := extraClientFlagInit(); err != nil { + t.Fatal(err) + } if td.expectedAddr != serverCfg.Addr { t.Errorf("%d. serverCfg.Addr expected '%s', but got '%s'. td.args was '%#v'.", i, td.expectedAddr, serverCfg.Addr, td.args) diff --git a/pkg/cli/interactive_tests/test_cert_advisory_validation.tcl b/pkg/cli/interactive_tests/test_cert_advisory_validation.tcl index de08050f2d35..97f85110c4bb 100644 --- a/pkg/cli/interactive_tests/test_cert_advisory_validation.tcl +++ b/pkg/cli/interactive_tests/test_cert_advisory_validation.tcl @@ -77,3 +77,21 @@ send "$argv cert list --certs-dir=$certs_dir --cert-principal-map=foo.bar:node\r eexpect "Certificate directory:" expect $prompt end_test + +start_test "Check that 'cert create-client' can utilize cert principal map." +send "$argv cert create-client root --certs-dir=$certs_dir --ca-key=$certs_dir/ca.key --cert-principal-map=foo.bar:node\r" +eexpect $prompt +end_test + +start_test "Check that the client commands can use cert principal map." +system "$argv start-single-node --certs-dir=$certs_dir --cert-principal-map=foo.bar:node --advertise-addr=localhost --background >>expect-cmd.log 2>&1" +send "$argv sql --certs-dir=$certs_dir --cert-principal-map=foo.bar:node -e \"select 'hello'\"\r" +eexpect "hello" +expect $prompt +send "$argv node ls --certs-dir=$certs_dir --cert-principal-map=foo.bar:node\r" +eexpect "1 row" +expect $prompt +send "$argv quit --certs-dir=$certs_dir --cert-principal-map=foo.bar:node\r" +eexpect "ok" +expect $prompt +end_test