Fix command.RunCustom(...) correctly (#18904)

* Revert "Remove t.Parallel() due to initialization race (#18751)"

This reverts commit ebcd65310221aff1dfcb94a571d70e38944006df.

We're going to fix this properly, running initCommands exactly once.

Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com>

* Prevent parallel testing racing in initCommands(...)

When running initCommands(...) from multiple tests, they can potentially
race, causing a panic. Test callers needing to set formatting
information must use RunCustom(...) instead of directly invoking the
test backend directly. When using t.Parallel(...) in these top-level
tests, we thus could race.

This removes the Commands global variable, making it a local variable
instead as nothing else appears to use it. We'll update Enterprise to
add in the Enterprise-specific commands to the existing list.

Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com>

---------

Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com>
This commit is contained in:
Alexander Scheel 2023-01-31 08:18:21 -05:00 committed by GitHub
parent 119e2274fc
commit 9352e30d50
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
3 changed files with 19 additions and 9 deletions

View File

@ -219,13 +219,10 @@ var (
"kubernetes": ksr.NewServiceRegistration,
}
initCommandsEnt = func(ui, serverCmdUi cli.Ui, runOpts *RunOptions) {}
initCommandsEnt = func(ui, serverCmdUi cli.Ui, runOpts *RunOptions, commands map[string]cli.CommandFactory) {}
)
// Commands is the mapping of all the available commands.
var Commands map[string]cli.CommandFactory
func initCommands(ui, serverCmdUi cli.Ui, runOpts *RunOptions) {
func initCommands(ui, serverCmdUi cli.Ui, runOpts *RunOptions) map[string]cli.CommandFactory {
loginHandlers := map[string]LoginHandler{
"alicloud": &credAliCloud.CLIHandler{},
"aws": &credAws.CLIHandler{},
@ -258,7 +255,7 @@ func initCommands(ui, serverCmdUi cli.Ui, runOpts *RunOptions) {
}
}
Commands = map[string]cli.CommandFactory{
commands := map[string]cli.CommandFactory{
"agent": func() (cli.Command, error) {
return &AgentCommand{
BaseCommand: &BaseCommand{
@ -841,7 +838,8 @@ func initCommands(ui, serverCmdUi cli.Ui, runOpts *RunOptions) {
},
}
initCommandsEnt(ui, serverCmdUi, runOpts)
initCommandsEnt(ui, serverCmdUi, runOpts, commands)
return commands
}
// MakeShutdownCh returns a channel that can be used for shutdown

View File

@ -217,14 +217,14 @@ func RunCustom(args []string, runOpts *RunOptions) int {
return 1
}
initCommands(ui, serverCmdUi, runOpts)
commands := initCommands(ui, serverCmdUi, runOpts)
hiddenCommands := []string{"version"}
cli := &cli.CLI{
Name: "vault",
Args: args,
Commands: Commands,
Commands: commands,
HelpFunc: groupedHelpFunc(
cli.BasicHelpFunc("vault"),
),

View File

@ -16,6 +16,8 @@ import (
)
func TestPKIHC_AllGood(t *testing.T) {
t.Parallel()
client, closer := testVaultServer(t)
defer closer()
@ -70,6 +72,8 @@ func TestPKIHC_AllGood(t *testing.T) {
}
func TestPKIHC_AllBad(t *testing.T) {
t.Parallel()
client, closer := testVaultServer(t)
defer closer()
@ -130,6 +134,8 @@ func TestPKIHC_AllBad(t *testing.T) {
}
func TestPKIHC_OnlyIssuer(t *testing.T) {
t.Parallel()
client, closer := testVaultServer(t)
defer closer()
@ -152,6 +158,8 @@ func TestPKIHC_OnlyIssuer(t *testing.T) {
}
func TestPKIHC_NoMount(t *testing.T) {
t.Parallel()
client, closer := testVaultServer(t)
defer closer()
@ -166,6 +174,8 @@ func TestPKIHC_NoMount(t *testing.T) {
}
func TestPKIHC_ExpectedEmptyMount(t *testing.T) {
t.Parallel()
client, closer := testVaultServer(t)
defer closer()
@ -186,6 +196,8 @@ func TestPKIHC_ExpectedEmptyMount(t *testing.T) {
}
func TestPKIHC_NoPerm(t *testing.T) {
t.Parallel()
client, closer := testVaultServer(t)
defer closer()