Skip to content

Fix conditionals erroring with an unknown condition and marked branch.#630

Merged
apparentlymart merged 1 commit into
hashicorp:mainfrom
cube2222:main
Oct 6, 2023
Merged

Fix conditionals erroring with an unknown condition and marked branch.#630
apparentlymart merged 1 commit into
hashicorp:mainfrom
cube2222:main

Conversation

@cube2222

@cube2222 cube2222 commented Oct 5, 2023

Copy link
Copy Markdown
Contributor

Fixes #629 and transitively also opentofu/opentofu#655 and hashicorp/terraform#33977.

Solution inspired by other implementations in this very file, as well as primitive functions in the go-cty repository itself.

@cube2222
Fix error of conditionals with an unknown condition and marked branch.
0a7a05d 
Signed-off-by: Jakub Martin <kubam@spacelift.io>
@cube2222 cube2222 mentioned this pull request Oct 5, 2023
Closed
@apparentlymart

apparentlymart commented Oct 6, 2023 •
edited
Loading

Copy link
Copy Markdown
Contributor

Thanks for digging into this, @cube2222!

This fix seems right to me. As you've seen, the Value.Range API can't safely accept marked values because not all of the methods of ValueRange can propagate marks to their results, and so applications/libraries that use marks must always unmark their values first.

FWIW, I've just released a new version v1.14.1 of my cty library, which is what's providing this "refinements" concept to HCL and Terraform, so that it'll produce a clearer panic message in response to caller bugs like this one. The new message will be "Value.Range on marked value; must be unmarked first", and so in any other situations where that appears I expect the solution will be a change similar to the one made in this PR.

With my cty maintainer hat on, (instead of my HashiCorp employee hat), I'd recommend callers of HCL upgrade to the latest cty along with upgrading HCL to get better feedback on other instances of this genre of bug, if any.

@apparentlymart apparentlymart merged commit 5b881c6 into hashicorp:main Oct 6, 2023
@Yantrio Yantrio mentioned this pull request Oct 6, 2023
Closed
@apparentlymart apparentlymart mentioned this pull request Oct 6, 2023
Merged
@crazy-max crazy-max mentioned this pull request Oct 19, 2023
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Conditionals with unknown conditions and marked branches are erroring.

2 participants

@cube2222 @apparentlymart