mirror of
https://github.com/hashicorp/vault.git
synced 2025-08-05 22:27:03 +02:00
VAULT-35838: advance deprecation of duplicate HCL attributes to pending removal stage (#31215)
* HCL dup attr deprecation: pending removal * correct docs * add changelog * better error message for possible common errors * Update website/content/partials/deprecation/duplicate-hcl-attributes.mdx Co-authored-by: Yoko Hyakuna <yoko@hashicorp.com> * Update website/content/partials/deprecation/duplicate-hcl-attributes.mdx Co-authored-by: Luis (LT) Carbonell <lt.carbonell@hashicorp.com> --------- Co-authored-by: Yoko Hyakuna <yoko@hashicorp.com> Co-authored-by: Luis (LT) Carbonell <lt.carbonell@hashicorp.com>
This commit is contained in:
parent
857e66b3e2
commit
194241e1d1
@ -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")
|
||||
}
|
||||
})
|
||||
}
|
||||
|
@ -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))
|
||||
}
|
||||
|
@ -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))
|
||||
}
|
||||
|
@ -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) {
|
||||
|
5
changelog/31215.txt
Normal file
5
changelog/31215.txt
Normal file
@ -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.
|
||||
```
|
@ -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
|
||||
|
@ -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.")
|
||||
})
|
||||
}
|
||||
|
@ -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"
|
||||
}`)
|
||||
})
|
||||
|
||||
|
@ -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 != "" {
|
||||
|
@ -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")
|
||||
|
||||
|
@ -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
|
||||
}
|
||||
|
||||
|
@ -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.
|
||||
|
@ -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))
|
||||
}
|
||||
|
@ -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) {
|
||||
|
@ -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")
|
||||
}
|
||||
|
@ -41,6 +41,8 @@ or raise a ticket with your support team.
|
||||
|
||||
@include 'deprecation/snowflake-password-auth.mdx'
|
||||
|
||||
@include 'deprecation/duplicate-hcl-attributes.mdx'
|
||||
|
||||
</Tab>
|
||||
<Tab heading="PENDING REMOVAL">
|
||||
|
||||
@ -50,8 +52,6 @@ or raise a ticket with your support team.
|
||||
|
||||
@include 'deprecation/centrify-auth-method.mdx'
|
||||
|
||||
@include 'deprecation/duplicate-hcl-attributes.mdx'
|
||||
|
||||
</Tab>
|
||||
<Tab heading="REMOVED">
|
||||
|
||||
|
@ -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,
|
||||
|
Loading…
Reference in New Issue
Block a user