Skip to content

VAULT-35838: advance deprecation of duplicate HCL attributes to pending removal stage#31215

Merged
bosouza merged 6 commits into
mainfrom
bosouza-hcl-dup-attr-2
Jul 24, 2025
Merged

VAULT-35838: advance deprecation of duplicate HCL attributes to pending removal stage#31215
bosouza merged 6 commits into
mainfrom
bosouza-hcl-dup-attr-2

Conversation

@bosouza
Copy link
Copy Markdown
Contributor

@bosouza bosouza commented Jul 4, 2025

Description

This is a follow up to #30386, which went into Vault v1.20 (and backported to v1.1.6) and started to print warnings whenever Vault parsed any HCL with duplicate attributes, as well as returning some API warnings when creating policies with those duplicate attributes in their HCL definition.

Continuing to follow our deprecation process, this PR changes those warnings to be actual errors, preventing the parsing of HCL files containing duplicate attributes in Vault v1.21.x. The previous "log-only" behavior of v1.20.x can be restored in v1.21.x by setting the VAULT_ALLOW_PENDING_REMOVAL_DUPLICATE_HCL_ATTRIBUTES to true.

I'm also rectifying the deprecation notice for duplicate attributes, which was incorrectly added directly to the "pending removal" stage in #30512. As for updating it to the proper "pending removal" on v1.21.x, that will have to wait until October when we cut a branch for v1.21.x.

Also, something important to highlight is that HCL allows defining multiple blocks of the same type, putting each definition into its own object in an array with the type name, like how we expect retry_join to be used. However, the HCL library without the patch preventing duplicate attributes would also allow that same behavior to concatenate explicit list assignment. For example, a similar example to what I used in #30386

path "secret" {
  capabilities = ["read"]
  capabilities = ["write"]
}

would previously result in capabilities = ["read", "write"] (unlike what I previously thought, the "overwrite" behavior doesn't apply to lists), while with the patch this is forbidden. I think this outcome is still in line with the goal of reducing the potential for confusion, so I'm keeping this restriction on "implicit list concatenation via attribute assignments", and also added some improved error messages for cases where I think customers could run into this issue, instructing them to use the block syntax instead, like we already have in our docs for retry_join.

Jira: VAULT-35838
ADR: VLT-006: Deprecate and remove duplicate attributes in HCL files in Vault

TODO only if you're a HashiCorp employee

  • [-] Backport Labels: If this fix needs to be backported, use the appropriate backport/ label that matches the desired release branch. Note that in the CE repo, the latest release branch will look like backport/x.x.x, but older release branches will be backport/ent/x.x.x+ent.
    • [-] LTS: If this fixes a critical security vulnerability or severity 1 bug, it will also need to be backported to the current LTS versions of Vault. To ensure this, use all available enterprise labels.
  • [-] ENT Breakage: If this PR either 1) removes a public function OR 2) changes the signature
    of a public function, even if that change is in a CE file, double check that
    applying the patch for this PR to the ENT repo and running tests doesn't
    break any tests. Sometimes ENT only tests rely on public functions in CE
    files.
  • Jira: If this change has an associated Jira, it's referenced either
    in the PR description, commit message, or branch name.
  • RFC: If this change has an associated RFC, please link it in the description.
  • [-] ENT PR: If this change has an associated ENT PR, please link it in the
    description. Also, make sure the changelog is in this PR, not in your ENT PR.

PCI review checklist

  • I have documented a clear reason for, and description of, the change I am making.
  • [-] If applicable, I've documented a plan to revert these changes if they require more than reverting the pull request.
  • [-] If applicable, I've documented the impact of any changes to security controls.

Examples of changes to security controls include using new access control methods, adding or removing logging pipelines, etc.

@github-actions github-actions Bot added the hashicorp-contributed-pr If the PR is HashiCorp (i.e. not-community) contributed label Jul 4, 2025
@github-actions
Copy link
Copy Markdown

github-actions Bot commented Jul 4, 2025

CI Results:
All Go tests succeeded! ✅

@bosouza bosouza added this to the 1.21.0-rc1 milestone Jul 7, 2025
@bosouza bosouza requested a review from kubawi July 8, 2025 13:37
@bosouza bosouza marked this pull request as ready for review July 8, 2025 13:49
@bosouza bosouza requested review from a team as code owners July 8, 2025 13:49
@github-actions
Copy link
Copy Markdown

github-actions Bot commented Jul 8, 2025

Build Results:
All builds succeeded! ✅

Comment thread website/content/partials/deprecation/duplicate-hcl-attributes.mdx Outdated
Co-authored-by: Yoko Hyakuna <yoko@hashicorp.com>
miagilepner
miagilepner previously approved these changes Jul 16, 2025
Copy link
Copy Markdown
Collaborator

@miagilepner miagilepner left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!


// 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"
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: can this code be shared so it isn't repeated here and in api/cliconfig/hcl_dup_attr_depreciation.go?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I didn't want to make this an exported function in the api package, as we offer backwards-compatibility guarantees on those functions, that's why this same function is repeated twice in api (and ofc because we can't share unexported functions between packages like api and api/cliconfig, even tho they are part of the same api module). The third copy of this function in the vault module is needed for the same reason, and I also didn't want to introduce a dependency from api to vault.

@bosouza bosouza requested a review from yhyakuna July 22, 2025 15:50
stevendpclark
stevendpclark previously approved these changes Jul 23, 2025
Copy link
Copy Markdown
Contributor

@brewgator brewgator left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Super nit

Comment thread website/content/partials/deprecation/duplicate-hcl-attributes.mdx Outdated
Co-authored-by: Luis (LT) Carbonell <lt.carbonell@hashicorp.com>
@bosouza bosouza dismissed stale reviews from stevendpclark and miagilepner via 02b5a78 July 23, 2025 19:51
@bosouza bosouza requested a review from brewgator July 23, 2025 19:51
@bosouza bosouza merged commit 194241e into main Jul 24, 2025
92 checks passed
@bosouza bosouza deleted the bosouza-hcl-dup-attr-2 branch July 24, 2025 18:17
Erfankam pushed a commit to Erfankam/vault that referenced this pull request Sep 1, 2025
…ng removal stage (hashicorp#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>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

hashicorp-contributed-pr If the PR is HashiCorp (i.e. not-community) contributed

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants