config: fix loading Vault token from env var (#19349)

The `defaultVault` variable is a pointer to the Vault configuration
named `default`. Initially, this variable points to the Vault
configuration that is used to load CLI flag values, but after those are
merged with the default and config file values the pointer reference
must be updated before mutating the config with environment variable
values.

.changelog/19349.txt

@@ -0,0 +1,3 @@
+```release-note:bug
+cli: Fixed a bug that caused the `nomad agent` command to ignore the `VAULT_TOKEN` and `VAULT_NAMESPACE` environment variables
+```

command/agent/command.go

@@ -293,6 +293,10 @@ func (c *Command) readConfig() *Config {
 		return nil
 	}
 
+	// Read Vault configuration for the default cluster again after all
+	// configuration sources have been merged.
+	defaultVault = config.defaultVault()
+
 	// Check to see if we should read the Vault token from the environment
 	if defaultVault.Token == "" {
 		defaultVault.Token = os.Getenv("VAULT_TOKEN")

command/agent/command_test.go

@@ -6,6 +6,7 @@ package agent
 import (
 	"math"
 	"os"
+	"path"
 	"path/filepath"
 	"strings"
 	"testing"
@@ -13,6 +14,7 @@ import (
 	"github.com/hashicorp/nomad/ci"
 	"github.com/hashicorp/nomad/helper/pointer"
 	"github.com/mitchellh/cli"
+	"github.com/shoenig/test/must"
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
 
@@ -504,3 +506,122 @@ func TestIsValidConfig(t *testing.T) {
 		})
 	}
 }
+
+func TestCommand_readConfig(t *testing.T) {
+	// Don't run in parallel since this test modifies environment variables.
+
+	configFiles := map[string]string{
+		"base.hcl": `
+data_dir = "/tmp/nomad"
+region   = "global"
+
+server {
+  enabled = true
+}
+
+client {
+  enabled = true
+}
+`,
+		"vault.hcl": `
+data_dir = "/tmp/nomad"
+region   = "global"
+
+server {
+  enabled = true
+}
+
+client {
+  enabled = true
+}
+
+vault {
+  token     = "token-from-config"
+  namespace = "ns-from-config"
+}
+`,
+	}
+
+	configDir := t.TempDir()
+	for k, v := range configFiles {
+		err := os.WriteFile(path.Join(configDir, k), []byte(v), 0644)
+		must.NoError(t, err)
+	}
+
+	testCases := []struct {
+		name    string
+		args    []string
+		env     map[string]string
+		checkFn func(*testing.T, *Config)
+	}{
+		{
+			name: "vault token and namespace from env var",
+			args: []string{
+				"-config", path.Join(configDir, "base.hcl"),
+			},
+			env: map[string]string{
+				"VAULT_TOKEN":     "token-from-env",
+				"VAULT_NAMESPACE": "ns-from-env",
+			},
+			checkFn: func(t *testing.T, c *Config) {
+				must.Eq(t, "token-from-env", c.Vaults[0].Token)
+				must.Eq(t, "ns-from-env", c.Vaults[0].Namespace)
+			},
+		},
+		{
+			name: "vault token and namespace from config takes precedence over env var",
+			args: []string{
+				"-config", path.Join(configDir, "vault.hcl"),
+			},
+			env: map[string]string{
+				"VAULT_TOKEN":     "token-from-env",
+				"VAULT_NAMESPACE": "ns-from-env",
+			},
+			checkFn: func(t *testing.T, c *Config) {
+				must.Eq(t, "token-from-config", c.Vaults[0].Token)
+				must.Eq(t, "ns-from-config", c.Vaults[0].Namespace)
+			},
+		},
+		{
+			name: "vault token and namespace from flag takes precedence over env var and config",
+			args: []string{
+				"-config", path.Join(configDir, "vault.hcl"),
+				"-vault-token", "secret-from-cli",
+				"-vault-namespace", "ns-from-cli",
+			},
+			env: map[string]string{
+				"VAULT_TOKEN":     "secret-from-env",
+				"VAULT_NAMESPACE": "ns-from-env",
+			},
+			checkFn: func(t *testing.T, c *Config) {
+				must.Eq(t, "secret-from-cli", c.Vaults[0].Token)
+				must.Eq(t, "ns-from-cli", c.Vaults[0].Namespace)
+			},
+		},
+	}
+
+	for _, tc := range testCases {
+		t.Run(tc.name, func(t *testing.T) {
+			ui := cli.NewMockUi()
+			defer func() {
+				// Print command stderr in case of a failed test case to help
+				// with debugging.
+				if t.Failed() {
+					t.Log(ui.ErrorWriter.String())
+				}
+			}()
+
+			cmd := &Command{
+				Ui:   ui,
+				args: tc.args,
+			}
+			for k, v := range tc.env {
+				t.Setenv(k, v)
+			}
+
+			got := cmd.readConfig()
+			must.NotNil(t, got)
+			tc.checkFn(t, got)
+		})
+	}
+}