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

Update HCL to handle type mismatches when applying defaults #32775

Merged
merged 5 commits into from
Mar 9, 2023

Conversation

liamcervante
Copy link
Member

@liamcervante liamcervante commented Mar 3, 2023

This PR includes a test that reproduces the crash in the linked issue, updates HCL and go-cty to fix the linked issue, and updates some tests broken as a result of the refinement features added in the latest go-cty release.

Fixes #32752

Target Release

1.4.1

Draft CHANGELOG entry

BUG FIXES

  • hcl: Fix a crash in Terraform when attempting to apply defaults into an incompatible type

@liamcervante liamcervante requested a review from a team March 9, 2023 09:39
@liamcervante liamcervante marked this pull request as ready for review March 9, 2023 09:39
@liamcervante liamcervante added the 1.4-backport If you add this label to a PR before merging, backport-assistant will open a new PR once merged label Mar 9, 2023
@liamcervante liamcervante merged commit 32f151f into main Mar 9, 2023
@liamcervante liamcervante deleted the liamcervante/32752 branch March 9, 2023 13:30
@github-actions
Copy link

github-actions bot commented Mar 9, 2023

Reminder for the merging maintainer: if this is a user-visible change, please update the changelog on the appropriate release branch.

apparentlymart added a commit that referenced this pull request Mar 15, 2023
We inadvertently incorporated the new minor release of cty into the 1.4
branch, and that's introduced some more refined handling of unknown values
that is too much of a change to introduce in a patch release.

Therefore this reverts back to the previous minor release for the v1.4
series, and then we'll separately get the main branch ready to work
correctly with the new cty before Terraform v1.5.

This reverts just the upgrade and the corresponding test changes from
#32775, while retaining the HCL upgrade and the new test case it
introduced for that bug it was trying to fix. That new test is still
passing so it seems that the cty upgrade is not crucial to that fix.
apparentlymart added a commit that referenced this pull request Mar 15, 2023
We inadvertently incorporated the new minor release of cty into the 1.4
branch, and that's introduced some more refined handling of unknown values
that is too much of a change to introduce in a patch release.

Therefore this reverts back to the previous minor release for the v1.4
series, and then we'll separately get the main branch ready to work
correctly with the new cty before Terraform v1.5.

This reverts just the upgrade and the corresponding test changes from
PR #32775, while retaining the HCL upgrade and the new test case it
introduced for that bug it was trying to fix. That new test is still
passing so it seems that the cty upgrade is not crucial to that fix.
apparentlymart added a commit that referenced this pull request Mar 15, 2023
We inadvertently incorporated the new minor release of cty into the 1.4
branch, and that's introduced some more refined handling of unknown values
that is too much of a change to introduce in a patch release.

Therefore this reverts back to the previous minor release for the v1.4
series, and then we'll separately get the main branch ready to work
correctly with the new cty before Terraform v1.5.

This reverts just the upgrade and the corresponding test changes from
#32775, while retaining the HCL upgrade and the new test case it
introduced for that bug it was trying to fix. That new test is still
passing so it seems that the cty upgrade is not crucial to that fix.
apparentlymart added a commit that referenced this pull request Mar 15, 2023
We inadvertently incorporated the new minor release of cty into the 1.4
branch, and that's introduced some more refined handling of unknown values
that is too much of a change to introduce in a patch release.

Therefore this reverts back to the previous minor release for the v1.4
series, and then we'll separately get the main branch ready to work
correctly with the new cty before Terraform v1.5.

This reverts just the upgrade and the corresponding test changes from
#32775, while retaining the HCL upgrade and the new test case it
introduced for that bug it was trying to fix. That new test is still
passing so it seems that the cty upgrade is not crucial to that fix.
kmoe pushed a commit that referenced this pull request Mar 16, 2023
We inadvertently incorporated the new minor release of cty into the 1.4
branch, and that's introduced some more refined handling of unknown values
that is too much of a change to introduce in a patch release.

Therefore this reverts back to the previous minor release for the v1.4
series, and then we'll separately get the main branch ready to work
correctly with the new cty before Terraform v1.5.

This reverts just the upgrade and the corresponding test changes from
#32775, while retaining the HCL upgrade and the new test case it
introduced for that bug it was trying to fix. That new test is still
passing so it seems that the cty upgrade is not crucial to that fix.
alisdair pushed a commit that referenced this pull request Mar 16, 2023
We inadvertently incorporated the new minor release of cty into the 1.4
branch, and that's introduced some more refined handling of unknown values
that is too much of a change to introduce in a patch release.

Therefore this reverts back to the previous minor release for the v1.4
series, and then we'll separately get the main branch ready to work
correctly with the new cty before Terraform v1.5.

This reverts just the upgrade and the corresponding test changes from
#32775, while retaining the HCL upgrade and the new test case it
introduced for that bug it was trying to fix. That new test is still
passing so it seems that the cty upgrade is not crucial to that fix.
genx7up pushed a commit to aweps/terraform that referenced this pull request Mar 17, 2023
…p#32775)

* add test reproducing terraform crash

* pull latest hcl

* add missing go.sum file

* also fix tests broken by go-cty update
genx7up pushed a commit to aweps/terraform that referenced this pull request Mar 17, 2023
We inadvertently incorporated the new minor release of cty into the 1.4
branch, and that's introduced some more refined handling of unknown values
that is too much of a change to introduce in a patch release.

Therefore this reverts back to the previous minor release for the v1.4
series, and then we'll separately get the main branch ready to work
correctly with the new cty before Terraform v1.5.

This reverts just the upgrade and the corresponding test changes from
hashicorp#32775, while retaining the HCL upgrade and the new test case it
introduced for that bug it was trying to fix. That new test is still
passing so it seems that the cty upgrade is not crucial to that fix.
genx7up pushed a commit to aweps/terraform that referenced this pull request Mar 17, 2023
…p#32775)

* add test reproducing terraform crash

* pull latest hcl

* add missing go.sum file

* also fix tests broken by go-cty update
genx7up pushed a commit to aweps/terraform that referenced this pull request Mar 17, 2023
We inadvertently incorporated the new minor release of cty into the 1.4
branch, and that's introduced some more refined handling of unknown values
that is too much of a change to introduce in a patch release.

Therefore this reverts back to the previous minor release for the v1.4
series, and then we'll separately get the main branch ready to work
correctly with the new cty before Terraform v1.5.

This reverts just the upgrade and the corresponding test changes from
hashicorp#32775, while retaining the HCL upgrade and the new test case it
introduced for that bug it was trying to fix. That new test is still
passing so it seems that the cty upgrade is not crucial to that fix.
@github-actions
Copy link

github-actions bot commented Apr 9, 2023

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 contributions.
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 Apr 9, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
1.4-backport If you add this label to a PR before merging, backport-assistant will open a new PR once merged
Projects
None yet
Development

Successfully merging this pull request may close these issues.

crash when passing invalid value to object attribute with defaults
2 participants