Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

prevent duplicate local block creation #11534

Merged
merged 5 commits into from
Jan 28, 2022
Merged

Conversation

azr
Copy link
Contributor

@azr azr commented Jan 27, 2022

After #11527

A bug was introduced in #10509 where a local block could be parsed and added to the list n * n times. ( n being the number of local blocks defined per file ).

This makes sure these are only parsed once by:

  • first only getting all local and locals blocks
  • then validating that there are no dupes
  • then evaluating them

@azr azr requested a review from a team as a code owner January 27, 2022 13:43
@azr azr marked this pull request as draft January 27, 2022 13:43
func (c *PackerConfig) parseLocalVariables(f *hcl.File) ([]*LocalBlock, hcl.Diagnostics) {
// parseLocalVariableBlocks looks in the AST for 'local' and 'locals' blocks and
// returns them all.
func parseLocalVariableBlocks(f *hcl.File) ([]*LocalBlock, hcl.Diagnostics) {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I removed the PackerConfig, so that this function is only an extraction function.

Copy link
Member

Choose a reason for hiding this comment

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

Thanks for the call out here.

@azr azr marked this pull request as ready for review January 28, 2022 13:50

// divide by 2 so that you don't get duplicate locals
// appear to have double locals in LocalBlock, not sure if intentional
for i := 0; i < len(locals)/2; i++ {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This was not always correct

Copy link
Member

@nywilken nywilken left a comment

Choose a reason for hiding this comment

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

Nice catch! This looks good. Test results below

Example HCL

~>  cat /tmp/source.pkr.hcl
locals {
  timestamp = formatdate("YYYY-MM-DD", timestamp())
  other_local = "test-${local.timestamp}"
}

locals {
  other_local = "testagain-${local.timestamp}"
}

With Packer 1.7.9

~>  ~/Downloads/packer179 validate /tmp/source.pkr.hcl
The configuration is valid.

With Dev Build

~>  packer validate /tmp/source.pkr.hcl
Error: Duplicate local definition

  on /tmp/source.pkr.hcl line 7:
  (source code not available)

Duplicate other_local definition found.

@nywilken nywilken merged commit 5d17d7f into master Jan 28, 2022
@nywilken nywilken deleted the azr/fix-duplicate-local-block branch January 28, 2022 18:21
@nywilken nywilken added the bug label Feb 2, 2022
@azr azr mentioned this pull request Feb 14, 2022
7 tasks
@github-actions
Copy link

github-actions bot commented Mar 5, 2022

I'm going to lock this pull request because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active issues.
If you have found a problem that seems related to this change, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Mar 5, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants