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

BUGFIX: use ListType of ObjectType for Computed attributes instead of Blocks #246

Conversation

detro
Copy link
Contributor

@detro detro commented Jul 25, 2022

Closes #244

Blocks, on a protocol-level, don't support expressing the attribute of being Computed: terraform-plugin-framework doesn't support it (yet?).

But in our case it's possible to just do away with Blocks for the tls_certificate.certificates, and instead define an attribute of type ListType, that carries an ObjectType. By using an actual Attribute, we can then express the fact that this is a Computed attribute of the data source, and has such it's not expected to have a value until the APPLY-PHASE of Terraform. That will make the error encountered by practitioners in #224 go away.

NOTE: We CANNOT use Attribute.NestAttributes for this, as that would break the compatibility with Protocol 5, i.e with TF >= 0.12.

Ivan De Marino added 2 commits July 25, 2022 14:46
…f `Object`, instead of a blocks' list

This is necessary. so that we can express to Terraform that the attribute is indeed `Computed` and it can't be expected to be populated, until the data source is read.

This was creating an issue (see #244), as Terraform protocol doesn't support expressing that a Block is Computed: only attributes can be.

This approach avoids the use of `NestedAttributes`, and as such is compatible with Protocol v5 (i.e. TF >= 0.12).
@detro detro requested a review from a team as a code owner July 25, 2022 14:13
…a scenario where "computed certificates are unknown until applied"
Copy link
Contributor

@bflad bflad left a comment

Choose a reason for hiding this comment

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

Looks good to me, just left some suggestions.

CHANGELOG.md Outdated Show resolved Hide resolved
internal/provider/data_source_certificate.go Outdated Show resolved Hide resolved
@detro detro merged commit 648f27e into main Jul 25, 2022
@detro detro deleted the detro/BUGFIX_224-use_list_of_objects_for_computed_attributes_instead_of_blocks branch July 25, 2022 15:42
Copy link

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 May 27, 2024
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.

data.tls_certificate.this.certificates is empty list of object
2 participants