diff --git a/api/cliconfig/config_test.go b/api/cliconfig/config_test.go
index 695ae16b22..67cf86ab82 100644
--- a/api/cliconfig/config_test.go
+++ b/api/cliconfig/config_test.go
@@ -61,17 +61,43 @@ nope = "true"
}
}
+// TestParseConfig_HclDuplicateKey tests the parsing of HCL files with duplicate keys.
+// TODO (HCL_DUP_KEYS_DEPRECATION): on full removal change this test to ensure that duplicate attributes cannot be parsed
+// under any circumstances.
func TestParseConfig_HclDuplicateKey(t *testing.T) {
- _, duplicate, err := parseConfig(`
+ t.Run("fail parsing without env var", func(t *testing.T) {
+ _, _, err := parseConfig(`
token_helper = "/token"
token_helper = "/token"
`)
- // TODO (HCL_DUP_KEYS_DEPRECATION): change this to expect an error once support for duplicate keys is fully removed
- if err != nil {
- t.Fatal("expected no error")
- }
+ if err == nil {
+ t.Fatal("expected error")
+ }
+ })
- if !duplicate {
- t.Fatal("expected duplicate")
- }
+ t.Run("fail parsing with env var set to false", func(t *testing.T) {
+ t.Setenv(allowHclDuplicatesEnvVar, "false")
+ _, _, err := parseConfig(`
+token_helper = "/token"
+token_helper = "/token"
+`)
+ if err == nil {
+ t.Fatal("expected error")
+ }
+ })
+
+ t.Run("succeed parsing with env var set to true", func(t *testing.T) {
+ t.Setenv(allowHclDuplicatesEnvVar, "true")
+ _, duplicate, err := parseConfig(`
+token_helper = "/token"
+token_helper = "/token"
+`)
+ if err != nil {
+ t.Fatal("expected no error")
+ }
+
+ if !duplicate {
+ t.Fatal("expected duplicate")
+ }
+ })
}
diff --git a/api/cliconfig/hcl_dup_attr_deprecation.go b/api/cliconfig/hcl_dup_attr_deprecation.go
index 66bcca3c07..f971c0cd0b 100644
--- a/api/cliconfig/hcl_dup_attr_deprecation.go
+++ b/api/cliconfig/hcl_dup_attr_deprecation.go
@@ -4,6 +4,9 @@
package cliconfig
import (
+ "fmt"
+ "os"
+ "strconv"
"strings"
"github.com/hashicorp/hcl"
@@ -11,13 +14,32 @@ import (
hclParser "github.com/hashicorp/hcl/hcl/parser"
)
+// allowHclDuplicatesEnvVar is an environment variable that allows Vault to revert back to accepting HCL files with
+// duplicate attributes. It's temporary until we finish the deprecation process, at which point this will be removed
+const allowHclDuplicatesEnvVar = "VAULT_ALLOW_PENDING_REMOVAL_DUPLICATE_HCL_ATTRIBUTES"
+
// parseAndCheckForDuplicateHclAttributes parses the input JSON/HCL file and if it is HCL it also checks
-// for duplicate keys in the HCL file, allowing callers to handle the issue accordingly. In a future release we'll
-// change the behavior to treat duplicate keys as an error and eventually remove this helper altogether.
-// TODO (HCL_DUP_KEYS_DEPRECATION): remove once not used anymore
+// for duplicate keys in the HCL file, allowing callers to handle the issue accordingly. It now only accepts duplicate
+// // keys if the environment variable VAULT_ALLOW_PENDING_REMOVAL_DUPLICATE_HCL_ATTRIBUTES is set to true. In a future
+// // release we'll remove this function entirely and there will be no way to parse HCL files with duplicate keys.
+// // TODO (HCL_DUP_KEYS_DEPRECATION): remove once not used anymore
func parseAndCheckForDuplicateHclAttributes(input string) (res *ast.File, duplicate bool, err error) {
res, err = hcl.Parse(input)
if err != nil && strings.Contains(err.Error(), "Each argument can only be defined once") {
+ allowHclDuplicatesRaw := os.Getenv(allowHclDuplicatesEnvVar)
+ if allowHclDuplicatesRaw == "" {
+ // default is to not allow duplicates
+ return nil, false, err
+ }
+ allowHclDuplicates, envParseErr := strconv.ParseBool(allowHclDuplicatesRaw)
+ if envParseErr != nil {
+ return nil, false, fmt.Errorf("error parsing %q environment variable: %w", allowHclDuplicatesEnvVar, err)
+ }
+ if !allowHclDuplicates {
+ return nil, false, err
+ }
+
+ // if allowed by the environment variable, parse again without failing on duplicate attributes
duplicate = true
res, err = hclParser.ParseDontErrorOnDuplicateKeys([]byte(input))
}
diff --git a/api/hcl_dup_attr_deprecation.go b/api/hcl_dup_attr_deprecation.go
index 49fb203f2a..8780ff651d 100644
--- a/api/hcl_dup_attr_deprecation.go
+++ b/api/hcl_dup_attr_deprecation.go
@@ -4,6 +4,9 @@
package api
import (
+ "fmt"
+ "os"
+ "strconv"
"strings"
"github.com/hashicorp/hcl"
@@ -11,13 +14,32 @@ import (
hclParser "github.com/hashicorp/hcl/hcl/parser"
)
+// allowHclDuplicatesEnvVar is an environment variable that allows Vault to revert back to accepting HCL files with
+// duplicate attributes. It's temporary until we finish the deprecation process, at which point this will be removed
+const allowHclDuplicatesEnvVar = "VAULT_ALLOW_PENDING_REMOVAL_DUPLICATE_HCL_ATTRIBUTES"
+
// parseAndCheckForDuplicateHclAttributes parses the input JSON/HCL file and if it is HCL it also checks
-// for duplicate keys in the HCL file, allowing callers to handle the issue accordingly. In a future release we'll
-// change the behavior to treat duplicate keys as an error and eventually remove this helper altogether.
+// for duplicate keys in the HCL file, allowing callers to handle the issue accordingly. It now only accepts duplicate
+// keys if the environment variable VAULT_ALLOW_PENDING_REMOVAL_DUPLICATE_HCL_ATTRIBUTES is set to true. In a future
+// release we'll remove this function entirely and there will be no way to parse HCL files with duplicate keys.
// TODO (HCL_DUP_KEYS_DEPRECATION): remove once not used anymore
func parseAndCheckForDuplicateHclAttributes(input string) (res *ast.File, duplicate bool, err error) {
res, err = hcl.Parse(input)
if err != nil && strings.Contains(err.Error(), "Each argument can only be defined once") {
+ allowHclDuplicatesRaw := os.Getenv(allowHclDuplicatesEnvVar)
+ if allowHclDuplicatesRaw == "" {
+ // default is to not allow duplicates
+ return nil, false, err
+ }
+ allowHclDuplicates, envParseErr := strconv.ParseBool(allowHclDuplicatesRaw)
+ if envParseErr != nil {
+ return nil, false, fmt.Errorf("error parsing %q environment variable: %w", allowHclDuplicatesEnvVar, err)
+ }
+ if !allowHclDuplicates {
+ return nil, false, err
+ }
+
+ // if allowed by the environment variable, parse again without failing on duplicate attributes
duplicate = true
res, err = hclParser.ParseDontErrorOnDuplicateKeys([]byte(input))
}
diff --git a/api/ssh_agent_test.go b/api/ssh_agent_test.go
index 8c34242d5f..360b0df907 100644
--- a/api/ssh_agent_test.go
+++ b/api/ssh_agent_test.go
@@ -14,12 +14,25 @@ import (
// TestSSH_CanLoadDuplicateKeys verifies that during the deprecation process of duplicate HCL attributes this function
// will still allow them.
+// TODO (HCL_DUP_KEYS_DEPRECATION): on full removal change this test to ensure that duplicate attributes cannot be parsed
+// under any circumstances.
func TestSSH_CanLoadDuplicateKeys(t *testing.T) {
- _, err := LoadSSHHelperConfig("./test-fixtures/agent_config_duplicate_keys.hcl")
- require.NoError(t, err)
- // TODO (HCL_DUP_KEYS_DEPRECATION): Change test to expect an error
- // require.Error(t, err)
- // require.Contains(t, err.Error(), "Each argument can only be defined once")
+ t.Run("fail parsing without env var", func(t *testing.T) {
+ _, err := LoadSSHHelperConfig("./test-fixtures/agent_config_duplicate_keys.hcl")
+ require.Error(t, err)
+ require.Contains(t, err.Error(), "Each argument can only be defined once")
+ })
+ t.Run("fail parsing with env var set to false", func(t *testing.T) {
+ t.Setenv(allowHclDuplicatesEnvVar, "false")
+ _, err := LoadSSHHelperConfig("./test-fixtures/agent_config_duplicate_keys.hcl")
+ require.Error(t, err)
+ require.Contains(t, err.Error(), "Each argument can only be defined once")
+ })
+ t.Run("succeed parsing with env var set to true", func(t *testing.T) {
+ t.Setenv(allowHclDuplicatesEnvVar, "true")
+ _, err := LoadSSHHelperConfig("./test-fixtures/agent_config_duplicate_keys.hcl")
+ require.NoError(t, err)
+ })
}
func TestSSH_CreateTLSClient(t *testing.T) {
diff --git a/changelog/31215.txt b/changelog/31215.txt
new file mode 100644
index 0000000000..e1d0b07822
--- /dev/null
+++ b/changelog/31215.txt
@@ -0,0 +1,5 @@
+```release-note:deprecation
+core: disallow usage of duplicate attributes in HCL configuration files and policy definitions, which were already
+deprecated. For now those errors can be suppressed back to warnings by setting the environment variable
+VAULT_ALLOW_PENDING_REMOVAL_DUPLICATE_HCL_ATTRIBUTES.
+```
\ No newline at end of file
diff --git a/command/agent_test.go b/command/agent_test.go
index c347d7b377..5cd3be1799 100644
--- a/command/agent_test.go
+++ b/command/agent_test.go
@@ -30,6 +30,7 @@ import (
credAppRole "github.com/hashicorp/vault/builtin/credential/approle"
"github.com/hashicorp/vault/command/agent"
agentConfig "github.com/hashicorp/vault/command/agent/config"
+ "github.com/hashicorp/vault/helper/random"
"github.com/hashicorp/vault/helper/testhelpers/minimal"
"github.com/hashicorp/vault/helper/useragent"
vaulthttp "github.com/hashicorp/vault/http"
@@ -363,6 +364,7 @@ listener "tcp" {
)
configPath := makeTempFile(t, "config.hcl", config)
+ t.Setenv(random.AllowHclDuplicatesEnvVar, "true")
// Start the agent
ui, cmd := testAgentCommand(t, logger)
cmd.client = serverClient
@@ -400,7 +402,7 @@ listener "tcp" {
//----------------------------------------------------
// TODO (HCL_DUP_KEYS_DEPRECATION): Eventually remove this check together with the duplicate attribute in this
- // test's configuration, create separate test ensuring such a config is not valid
+ // test's configuration
require.Contains(t, ui.ErrorWriter.String(),
"WARNING: Duplicate keys found")
@@ -3115,16 +3117,37 @@ func TestAgent_Config_ReloadTls(t *testing.T) {
}
// TestAgent_Config_HclDuplicateKey checks that a log warning is printed when the agent config has duplicate attributes
+// TODO (HCL_DUP_KEYS_DEPRECATION): always expect error once deprecation is done
func TestAgent_Config_HclDuplicateKey(t *testing.T) {
- configFile := populateTempFile(t, "agent-config.hcl", `
+ t.Run("duplicate error with env unset", func(t *testing.T) {
+ configFile := populateTempFile(t, "agent-config.hcl", `
log_level = "trace"
log_level = "debug"
`)
- _, duplicate, err := agentConfig.LoadConfigFileCheckDuplicates(configFile.Name())
- // TODO (HCL_DUP_KEYS_DEPRECATION): expect error on duplicates once deprecation is done
- require.NoError(t, err)
- require.True(t, duplicate)
- // require.Contains(t, err.Error(), "Each argument can only be defined once")
+ _, _, err := agentConfig.LoadConfigFileCheckDuplicates(configFile.Name())
+ require.Error(t, err)
+ require.Contains(t, err.Error(), "Each argument can only be defined once")
+ })
+ t.Run("duplicate error with env set to false", func(t *testing.T) {
+ configFile := populateTempFile(t, "agent-config.hcl", `
+log_level = "trace"
+log_level = "debug"
+`)
+ t.Setenv(random.AllowHclDuplicatesEnvVar, "false")
+ _, _, err := agentConfig.LoadConfigFileCheckDuplicates(configFile.Name())
+ require.Error(t, err)
+ require.Contains(t, err.Error(), "Each argument can only be defined once")
+ })
+ t.Run("duplicate warning with env set to true", func(t *testing.T) {
+ configFile := populateTempFile(t, "agent-config.hcl", `
+log_level = "trace"
+log_level = "debug"
+`)
+ t.Setenv(random.AllowHclDuplicatesEnvVar, "true")
+ _, duplicate, err := agentConfig.LoadConfigFileCheckDuplicates(configFile.Name())
+ require.NoError(t, err)
+ require.True(t, duplicate)
+ })
}
// TestAgent_NonTLSListener_SIGHUP tests giving a SIGHUP signal to a listener
diff --git a/command/login_test.go b/command/login_test.go
index 52622edd96..a8204f61d3 100644
--- a/command/login_test.go
+++ b/command/login_test.go
@@ -16,6 +16,7 @@ import (
credToken "github.com/hashicorp/vault/builtin/credential/token"
credUserpass "github.com/hashicorp/vault/builtin/credential/userpass"
"github.com/hashicorp/vault/command/token"
+ "github.com/hashicorp/vault/helper/random"
"github.com/hashicorp/vault/helper/testhelpers"
"github.com/hashicorp/vault/vault"
"github.com/stretchr/testify/require"
@@ -639,18 +640,33 @@ token_helper = ""
}
token := secret.Auth.ClientToken
- ui, cmd := testLoginCommand(t)
- cmd.tokenHelper = nil // cause default one to be used
- cmd.client = client
+ t.Run("fail if duplicates are not allowed", func(t *testing.T) {
+ _, cmd := testLoginCommand(t)
+ cmd.tokenHelper = nil // cause default one to be used
+ cmd.client = client
- code := cmd.Run([]string{
- token,
+ code := cmd.Run([]string{
+ token,
+ })
+ if exp := 1; code != exp {
+ t.Errorf("expected %d to be %d", code, exp)
+ }
})
- if exp := 0; code != exp {
- t.Errorf("expected %d to be %d", code, exp)
- }
- // TODO (HCL_DUP_KEYS_DEPRECATION): Instead ensure that login command fails if the config file contains duplicate keys
- require.Contains(t, ui.ErrorWriter.String(),
- "WARNING: Duplicate keys found in the Vault token helper configuration file, duplicate keys in HCL files are deprecated and will be forbidden in a future release.")
+ t.Run("succeed if duplicates are allowed", func(t *testing.T) {
+ t.Setenv(random.AllowHclDuplicatesEnvVar, "true")
+ ui, cmd := testLoginCommand(t)
+ cmd.tokenHelper = nil // cause default one to be used
+ cmd.client = client
+
+ code := cmd.Run([]string{
+ token,
+ })
+ if exp := 0; code != exp {
+ t.Errorf("expected %d to be %d", code, exp)
+ }
+
+ require.Contains(t, ui.ErrorWriter.String(),
+ "WARNING: Duplicate keys found in the Vault token helper configuration file, duplicate keys in HCL files are deprecated and will be forbidden in a future release.")
+ })
}
diff --git a/command/operator_migrate_test.go b/command/operator_migrate_test.go
index 16b70a2c92..b2a92d3396 100644
--- a/command/operator_migrate_test.go
+++ b/command/operator_migrate_test.go
@@ -22,6 +22,7 @@ import (
log "github.com/hashicorp/go-hclog"
"github.com/hashicorp/go-secure-stdlib/base62"
"github.com/hashicorp/vault/command/server"
+ "github.com/hashicorp/vault/helper/random"
"github.com/hashicorp/vault/sdk/physical"
"github.com/hashicorp/vault/vault"
)
@@ -219,6 +220,7 @@ storage_destination "raft" {
},
},
}
+ t.Setenv(random.AllowHclDuplicatesEnvVar, "true")
cfg, err := cmd.loadMigratorConfig(cfgName)
if err != nil {
t.Fatal(cfg)
@@ -226,9 +228,9 @@ storage_destination "raft" {
if diff := deep.Equal(cfg, expCfg); diff != nil {
t.Fatal(diff)
}
- // TODO (HCL_DUP_KEYS_DEPRECATION): Remove warning and instead add one of these "verifyBad" tests down below
- // to ensure that duplicate attributes fail to parse.
+ // TODO (HCL_DUP_KEYS_DEPRECATION): Remove warning and leave only this "verifyBad" test down below
strings.Contains(ui.ErrorWriter.String(), "WARNING: Duplicate keys found in migration configuration file, duplicate keys in HCL files are deprecated and will be forbidden in a future release.")
+ t.Setenv(random.AllowHclDuplicatesEnvVar, "false")
verifyBad := func(cfg string) {
os.WriteFile(cfgName, []byte(cfg), 0o644)
@@ -276,6 +278,16 @@ storage_destination "raft" {
storage_destination "consul" {
path = "dest_path"
+}`)
+ // duplicate hcl attribute
+ verifyBad(`
+storage_source "consul" {
+ path = "src_path"
+}
+
+storage_destination "raft" {
+ path = "dest_path"
+ path = "dest_path"
}`)
})
diff --git a/command/policy_fmt_test.go b/command/policy_fmt_test.go
index e2044b7235..a581f3873b 100644
--- a/command/policy_fmt_test.go
+++ b/command/policy_fmt_test.go
@@ -9,6 +9,7 @@ import (
"testing"
"github.com/hashicorp/cli"
+ "github.com/hashicorp/vault/helper/random"
"github.com/stretchr/testify/require"
)
@@ -24,10 +25,9 @@ func testPolicyFmtCommand(tb testing.TB) (*cli.MockUi, *PolicyFmtCommand) {
}
func TestPolicyFmtCommand_Run(t *testing.T) {
- t.Parallel()
-
testCases := map[string]struct {
args []string
+ envVars map[string]string
policyArg string
out string
expected string
@@ -71,16 +71,38 @@ path "secret" {
out: "failed to parse policy",
code: 1,
},
- // TODO (HCL_DUP_KEYS_DEPRECATION): change this test case to expect a specific error when deprecation is done
- "hcl_duplicate_key": {
+ // TODO (HCL_DUP_KEYS_DEPRECATION): remove this test once deprecation is fully done
+ "hcl_duplicate_key_env_set": {
policyArg: `
path "secret" {
capabilities = ["create", "update", "delete"]
capabilities = ["create"]
}
`,
- code: 0,
- out: "WARNING: Duplicate keys found in the provided policy, duplicate keys in HCL files are deprecated and will be forbidden in a future release.",
+ envVars: map[string]string{random.AllowHclDuplicatesEnvVar: "true"},
+ code: 0,
+ out: "WARNING: Duplicate keys found in the provided policy, duplicate keys in HCL files are deprecated and will be forbidden in a future release.",
+ },
+ "hcl_duplicate_key_env_not_set": {
+ policyArg: `
+path "secret" {
+ capabilities = ["create", "update", "delete"]
+ capabilities = ["create"]
+}
+`,
+ code: 1,
+ out: "failed to parse policy: The argument \"capabilities\" at 4:3 was already set. Each argument can only be defined once",
+ },
+ "hcl_duplicate_key_env_set_to_false": {
+ policyArg: `
+path "secret" {
+ capabilities = ["create", "update", "delete"]
+ capabilities = ["create"]
+}
+`,
+ envVars: map[string]string{random.AllowHclDuplicatesEnvVar: "false"},
+ code: 1,
+ out: "failed to parse policy: The argument \"capabilities\" at 4:3 was already set. Each argument can only be defined once",
},
}
@@ -90,7 +112,10 @@ path "secret" {
for name, tc := range testCases {
t.Run(name, func(t *testing.T) {
r := require.New(t)
- t.Parallel()
+
+ for k, v := range tc.envVars {
+ t.Setenv(k, v)
+ }
args := tc.args
if tc.policyArg != "" {
diff --git a/command/proxy_test.go b/command/proxy_test.go
index 96c94c4877..636dd1f74c 100644
--- a/command/proxy_test.go
+++ b/command/proxy_test.go
@@ -24,6 +24,7 @@ import (
credAppRole "github.com/hashicorp/vault/builtin/credential/approle"
"github.com/hashicorp/vault/command/agent"
proxyConfig "github.com/hashicorp/vault/command/proxy/config"
+ "github.com/hashicorp/vault/helper/random"
"github.com/hashicorp/vault/helper/testhelpers/minimal"
"github.com/hashicorp/vault/helper/useragent"
vaulthttp "github.com/hashicorp/vault/http"
@@ -313,6 +314,7 @@ auto_auth {
wg := &sync.WaitGroup{}
wg.Add(1)
go func() {
+ t.Setenv(random.AllowHclDuplicatesEnvVar, "true")
cmd.Run([]string{"-config", configPath})
wg.Done()
}()
@@ -324,7 +326,7 @@ auto_auth {
}
// TODO (HCL_DUP_KEYS_DEPRECATION): Eventually remove this check together with the duplicate attribute in this
- // test's configuration, create separate test ensuring such a config is not valid
+ // test's configuration
require.Contains(t, ui.ErrorWriter.String(),
"WARNING: Duplicate keys found")
diff --git a/command/server/config.go b/command/server/config.go
index 12f09df0dd..c91df5aef9 100644
--- a/command/server/config.go
+++ b/command/server/config.go
@@ -653,6 +653,14 @@ func ParseConfigCheckDuplicate(d, source string) (cfg *Config, duplicate bool, e
// Parse!
obj, duplicate, err := random.ParseAndCheckForDuplicateHclAttributes(d)
if err != nil {
+ if strings.Contains(err.Error(), "was already set. Each argument can only be defined once") {
+ knownPossibleAttributeDupErrors := []string{"retry_join", "transform", "listener"}
+ for _, s := range knownPossibleAttributeDupErrors {
+ if strings.Contains(err.Error(), fmt.Sprintf("The argument %q at", s)) {
+ return nil, duplicate, fmt.Errorf("%w (if using the attribute syntax %s = [...], change it to the block syntax %s { ... })", err, s, s)
+ }
+ }
+ }
return nil, duplicate, err
}
diff --git a/command/server/config_test_helpers.go b/command/server/config_test_helpers.go
index dc00be5e61..e6e7066f22 100644
--- a/command/server/config_test_helpers.go
+++ b/command/server/config_test_helpers.go
@@ -16,6 +16,7 @@ import (
"github.com/hashicorp/hcl"
"github.com/hashicorp/hcl/hcl/ast"
"github.com/hashicorp/hcl/hcl/token"
+ "github.com/hashicorp/vault/helper/random"
"github.com/hashicorp/vault/internalshared/configutil"
"github.com/stretchr/testify/require"
)
@@ -32,8 +33,6 @@ func boolPointer(x bool) *bool {
// testConfigRaftRetryJoin decodes and normalizes retry_join stanzas.
func testConfigRaftRetryJoin(t *testing.T) {
- t.Parallel()
-
retryJoinExpected := []map[string]string{
// NOTE: Normalization handles IPv6 addresses and returns auto_join with
// sorted stable keys.
@@ -47,15 +46,49 @@ func testConfigRaftRetryJoin(t *testing.T) {
{"auto_join": "provider=k8s label_selector=\"app.kubernetes.io/name=vault, component=server\" namespace=vault"},
{"auto_join": "provider=k8s label_selector=\"app.kubernetes.io/name=vault1,component=server\" namespace=vault1"},
}
- for _, cfg := range []string{
- "attr",
- "block",
- "mixed",
- } {
- t.Run(cfg, func(t *testing.T) {
- t.Parallel()
+ testCases := map[string]struct {
+ configFile string
+ envVars map[string]string
+ errorContains string
+ }{
+ "attributes_duplicate_error": {
+ configFile: "./test-fixtures/raft_retry_join_attr.hcl",
+ errorContains: "The argument \"retry_join\" at 11:3 was already set. Each argument can only be defined once (if using the attribute syntax retry_join = [...], change it to the block syntax retry_join { ... })",
+ },
+ "attributes_allowed_with_env_var": {
+ configFile: "./test-fixtures/raft_retry_join_attr.hcl",
+ envVars: map[string]string{
+ random.AllowHclDuplicatesEnvVar: "true",
+ },
+ },
+ "blocks": {
+ configFile: "./test-fixtures/raft_retry_join_block.hcl",
+ },
+ "mixed_duplicate_error": {
+ configFile: "./test-fixtures/raft_retry_join_mixed.hcl",
+ errorContains: "The argument \"retry_join\" at 14:3 was already set. Each argument can only be defined once (if using the attribute syntax retry_join = [...], change it to the block syntax retry_join { ... })",
+ },
+ "mixed_allowed_with_env_var": {
+ configFile: "./test-fixtures/raft_retry_join_mixed.hcl",
+ envVars: map[string]string{
+ random.AllowHclDuplicatesEnvVar: "true",
+ },
+ },
+ }
+
+ for name, tc := range testCases {
+ t.Run(name, func(t *testing.T) {
+ for k, v := range tc.envVars {
+ t.Setenv(k, v)
+ }
+
+ config, err := LoadConfigFile(tc.configFile)
+ if tc.errorContains != "" {
+ require.Error(t, err)
+ require.Contains(t, err.Error(), tc.errorContains)
+ return
+ }
- config, err := LoadConfigFile(fmt.Sprintf("./test-fixtures/raft_retry_join_%s.hcl", cfg))
require.NoError(t, err)
retryJoinJSON, err := json.Marshal(retryJoinExpected)
require.NoError(t, err)
@@ -607,11 +640,25 @@ func testUnknownFieldValidationHcl(t *testing.T) {
}
}
+// TODO (HCL_DUP_KEYS_DEPRECATION): remove warning test once deprecation is completed
func testDuplicateKeyValidationHcl(t *testing.T) {
- _, duplicate, err := LoadConfigFileCheckDuplicate("./test-fixtures/invalid_config_duplicate_key.hcl")
- // TODO (HCL_DUP_KEYS_DEPRECATION): require error once deprecation is done
- require.NoError(t, err)
- require.True(t, duplicate)
+ t.Run("env unset", func(t *testing.T) {
+ _, _, err := LoadConfigFileCheckDuplicate("./test-fixtures/invalid_config_duplicate_key.hcl")
+ require.Error(t, err)
+ })
+
+ t.Run("env set to false", func(t *testing.T) {
+ t.Setenv(random.AllowHclDuplicatesEnvVar, "false")
+ _, _, err := LoadConfigFileCheckDuplicate("./test-fixtures/invalid_config_duplicate_key.hcl")
+ require.Error(t, err)
+ })
+
+ t.Run("env set to true", func(t *testing.T) {
+ t.Setenv(random.AllowHclDuplicatesEnvVar, "true")
+ _, duplicate, err := LoadConfigFileCheckDuplicate("./test-fixtures/invalid_config_duplicate_key.hcl")
+ require.NoError(t, err)
+ require.True(t, duplicate)
+ })
}
// testConfigWithAdministrativeNamespaceJson tests that a config with a valid administrative namespace path is correctly validated and loaded.
diff --git a/helper/random/parser.go b/helper/random/parser.go
index 307470f5d7..7f151b4816 100644
--- a/helper/random/parser.go
+++ b/helper/random/parser.go
@@ -5,7 +5,9 @@ package random
import (
"fmt"
+ "os"
"reflect"
+ "strconv"
"strings"
"unicode/utf8"
@@ -15,14 +17,31 @@ import (
"github.com/mitchellh/mapstructure"
)
+// AllowHclDuplicatesEnvVar is an environment variable that allows Vault to revert back to accepting HCL files with
+// duplicate attributes. It's temporary until we finish the deprecation process, at which point this will be removed
+const AllowHclDuplicatesEnvVar = "VAULT_ALLOW_PENDING_REMOVAL_DUPLICATE_HCL_ATTRIBUTES"
+
// ParseAndCheckForDuplicateHclAttributes parses the input JSON/HCL file and if it is HCL it also checks
// for duplicate keys in the HCL file, allowing callers to handle the issue accordingly. In a future release we'll
// change the behavior to treat duplicate keys as an error and eventually remove this helper altogether.
// TODO (HCL_DUP_KEYS_DEPRECATION): remove once not used anymore
func ParseAndCheckForDuplicateHclAttributes(input string) (res *ast.File, duplicate bool, err error) {
res, err = hcl.Parse(input)
- // TODO (HCL_DUP_KEYS_DEPRECATION): on the "pending removal stage" check for env var before allowing the "warn only" behavior
if err != nil && strings.Contains(err.Error(), "Each argument can only be defined once") {
+ allowHclDuplicatesRaw := os.Getenv(AllowHclDuplicatesEnvVar)
+ if allowHclDuplicatesRaw == "" {
+ // default is to not allow duplicates
+ return nil, false, err
+ }
+ allowHclDuplicates, envParseErr := strconv.ParseBool(allowHclDuplicatesRaw)
+ if envParseErr != nil {
+ return nil, false, fmt.Errorf("error parsing %q environment variable: %w", AllowHclDuplicatesEnvVar, err)
+ }
+ if !allowHclDuplicates {
+ return nil, false, err
+ }
+
+ // if allowed by the environment variable, parse again without failing on duplicate attributes
duplicate = true
res, err = hclParser.ParseDontErrorOnDuplicateKeys([]byte(input))
}
diff --git a/vault/logical_system_test.go b/vault/logical_system_test.go
index 11180a0e02..c10bae903a 100644
--- a/vault/logical_system_test.go
+++ b/vault/logical_system_test.go
@@ -2813,21 +2813,32 @@ func TestSystemBackend_policyCRUD(t *testing.T) {
// TestSystemBackend_writeHCLDuplicateAttributes checks that trying to create a policy with duplicate HCL attributes
// results in a warning being returned by the API
func TestSystemBackend_writeHCLDuplicateAttributes(t *testing.T) {
- b := testSystemBackend(t)
-
// policy with duplicate attribute
rules := `path "foo/" { policy = "read" policy = "read" }`
req := logical.TestRequest(t, logical.UpdateOperation, "policy/foo")
req.Data["policy"] = rules
- resp, err := b.HandleRequest(namespace.RootContext(nil), req)
- // TODO (HCL_DUP_KEYS_DEPRECATION): change this test to expect an error when creating a policy with duplicate attributes
- if err != nil {
- t.Fatalf("err: %v %#v", err, resp)
- }
- if resp != nil && (resp.IsError() || len(resp.Data) > 0) {
- t.Fatalf("bad: %#v", resp)
- }
- require.Contains(t, resp.Warnings, "policy contains duplicate attributes, which will no longer be supported in a future version")
+
+ t.Run("fails with env unset", func(t *testing.T) {
+ b := testSystemBackend(t)
+ resp, err := b.HandleRequest(namespace.RootContext(nil), req)
+ require.Error(t, err)
+ require.Error(t, resp.Error())
+ require.EqualError(t, resp.Error(), "failed to parse policy: The argument \"policy\" at 1:31 was already set. Each argument can only be defined once")
+ })
+
+ // TODO (HCL_DUP_KEYS_DEPRECATION): leave only test above once deprecation is done
+ t.Run("warning with env set", func(t *testing.T) {
+ t.Setenv(random.AllowHclDuplicatesEnvVar, "true")
+ b := testSystemBackend(t)
+ resp, err := b.HandleRequest(namespace.RootContext(nil), req)
+ if err != nil {
+ t.Fatalf("err: %v %#v", err, resp)
+ }
+ if resp != nil && (resp.IsError() || len(resp.Data) > 0) {
+ t.Fatalf("bad: %#v", resp)
+ }
+ require.Contains(t, resp.Warnings, "policy contains duplicate attributes, which will no longer be supported in a future version")
+ })
}
func TestSystemBackend_enableAudit(t *testing.T) {
diff --git a/vault/policy_store_test.go b/vault/policy_store_test.go
index 6fc73b5a31..4b3f1173f6 100644
--- a/vault/policy_store_test.go
+++ b/vault/policy_store_test.go
@@ -12,6 +12,7 @@ import (
log "github.com/hashicorp/go-hclog"
"github.com/hashicorp/vault/helper/namespace"
+ "github.com/hashicorp/vault/helper/random"
"github.com/hashicorp/vault/sdk/logical"
"github.com/stretchr/testify/require"
)
@@ -458,11 +459,15 @@ func TestPolicyStore_DuplicateAttributes(t *testing.T) {
ps := core.policyStore
dupAttrPolicy := aclPolicy + `
path "foo" {
- capabilities = ["deny"]
- capabilities = ["deny"]
+ capabilities = ["list"]
+ capabilities = ["read"]
}
`
+ t.Setenv(random.AllowHclDuplicatesEnvVar, "true")
policy, err := ParseACLPolicy(namespace.RootNamespace, dupAttrPolicy)
+ require.NoError(t, err)
+ // check that "list" and "read" get concatenated
+ require.Len(t, policy.Paths[len(policy.Paths)-1].Capabilities, 2)
policy.Templated = true
require.NoError(t, err)
ctx := namespace.RootContext(context.Background())
@@ -480,4 +485,13 @@ path "foo" {
require.NotNil(t, p)
require.NoError(t, err)
require.Contains(t, logOut.String(), "HCL policy contains duplicate attributes, which will no longer be supported in a future version")
+
+ t.Setenv(random.AllowHclDuplicatesEnvVar, "false")
+ _, err = ps.ACL(ctx, nil, map[string][]string{namespace.RootNamespace.ID: {"dev", "ops"}})
+ require.Error(t, err)
+ require.Contains(t, err.Error(), "error parsing templated policy \"dev\": failed to parse policy: The argument \"capabilities\" at 61:2 was already set. Each argument can only be defined once")
+ ps.tokenPoliciesLRU.Purge()
+ _, err = ps.GetPolicy(ctx, "dev", PolicyTypeACL)
+ require.Error(t, err)
+ require.Contains(t, err.Error(), "failed to parse policy: failed to parse policy: The argument \"capabilities\" at 61:2 was already set. Each argument can only be defined once")
}
diff --git a/website/content/docs/updates/deprecation.mdx b/website/content/docs/updates/deprecation.mdx
index f512a6d000..0890c38578 100644
--- a/website/content/docs/updates/deprecation.mdx
+++ b/website/content/docs/updates/deprecation.mdx
@@ -41,6 +41,8 @@ or raise a ticket with your support team.
@include 'deprecation/snowflake-password-auth.mdx'
+@include 'deprecation/duplicate-hcl-attributes.mdx'
+
@@ -50,8 +52,6 @@ or raise a ticket with your support team.
@include 'deprecation/centrify-auth-method.mdx'
-@include 'deprecation/duplicate-hcl-attributes.mdx'
-
diff --git a/website/content/partials/deprecation/duplicate-hcl-attributes.mdx b/website/content/partials/deprecation/duplicate-hcl-attributes.mdx
index f40a834490..765917a32e 100644
--- a/website/content/partials/deprecation/duplicate-hcl-attributes.mdx
+++ b/website/content/partials/deprecation/duplicate-hcl-attributes.mdx
@@ -4,7 +4,8 @@
| :-------: | :---------------------: | :--------------: |
| JUN 2025 | OCT 2025 | N/A
-The ability to duplicate attributes in HCL configuration files and policy definitions is deprecated.
+In Vault v1.20.x and v1.19.6+, the ability to duplicate attributes in HCL configuration
+files and policy definitions is deprecated.
To find affected policies, look for "policy contains duplicate attributes"
warnings in the server logs. Once we remove support for duplicate attributes,