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

remove max items one defaults hacks #2049

Merged
merged 4 commits into from
May 31, 2024

Conversation

VenelinMartinov
Copy link
Contributor

@VenelinMartinov VenelinMartinov commented May 31, 2024

This is a pure refactor.

Reverts the options added in #1725 as these were the wrong way to do the change.

#1971 should be the proper fix to the issues encountered there, now backed by cross-tests.

This should simplify the code around makeTerraformInputs a bit.

@VenelinMartinov VenelinMartinov marked this pull request as ready for review May 31, 2024 17:38
Copy link

codecov bot commented May 31, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 63.21%. Comparing base (1f9028f) to head (38e62fc).

Current head 38e62fc differs from pull request most recent head 6b6de90

Please upload reports for the commit 6b6de90 to get more accurate results.

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #2049      +/-   ##
==========================================
+ Coverage   61.39%   63.21%   +1.82%     
==========================================
  Files         334      333       -1     
  Lines       44950    37382    -7568     
==========================================
- Hits        27595    23631    -3964     
+ Misses      15829    12224    -3605     
- Partials     1526     1527       +1     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@VenelinMartinov VenelinMartinov merged commit 3c1421d into master May 31, 2024
9 checks passed
@VenelinMartinov VenelinMartinov deleted the vvm/remove_max_items_one_default_hacks branch May 31, 2024 18:16
VenelinMartinov added a commit that referenced this pull request Jun 5, 2024
part of #1785

This change adds a normalisation step for collections when recovering
cty values to pass to terraform. This ensures we represent them
similarly to terraform.

In practice this means that all block collections need to be passed to
TF providers as an empty collection instead of nil. This should get rid
of quite a few subtle discrepancies in the data we pass to the TF
provider code. These sometimes result in panics since we pass unexpected
nils.

This gets rid of all known input discrepancies discovered so far through
cross-testing.

The full rules for what is a block are
[here](https://github.com/hashicorp/terraform-plugin-sdk/blob/1f499688ebd9420768f501d4ed622a51b2135ced/helper/schema/core_schema.go#L60).
It is essentially properties with schema: typeList or typeSet with a
Resource Elem.

fixes #1970
fixes #1915
fixes #1964
fixes #1767 
fixes #1762 

TODO: [DONE] remove the MaxItemsOne default hacks introduced in
#1725 (opened
#2049)

---------

Co-authored-by: Anton Tayanovskyy <[email protected]>
Co-authored-by: Ian Wahbe <[email protected]>
VenelinMartinov added a commit that referenced this pull request Jul 22, 2024
This was missed in
#2049

The code was never used and instead the problem was fixed in
#1971 at the root.

Simplify schema.go a bit as it is complicated enough already.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants