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

tfsdk: Allow GetAttribute To Return Unknown Values Instead Of Null When Missing Due To Parent Being Unknown #186

Open
bflad opened this issue Sep 29, 2021 · 3 comments
Labels
breaking-change This PR introduces a breaking change or the resolution of this issue may require a breaking change. enhancement New feature or request

Comments

@bflad
Copy link
Contributor

bflad commented Sep 29, 2021

Module version

v0.4.2

Use-cases

As part of fixing #150, the GetAttribute() method on Config, Plan, and State will skip tftypes.ErrInvalidStep errors and instead return null values when valid, but missing. A related case of this behavior that might be useful for functionality such as plan modifiers and validation is knowing whether the value is instead unknown, which can happen if the value is missing from Config, Plan, or State because a parent path value is marked as unknown.

Attempted Solutions

Providers can verify if any parent paths are unknown, when a null value is returned. For example in attribute plan modifiers, the entire Config, Plan, or State value is available.

Proposal

Have the GetAttribute() method on Config, Plan, and State automatically perform the parent path checks for unknown. If found, return unknown value for type, otherwise continue returning null value for type.

References

@paddycarver
Copy link
Contributor

I think we may also just want to complicate the errors returned by tftypes.WalkAttributePath to be more explicit about why a step is invalid.

@bflad
Copy link
Contributor Author

bflad commented Oct 4, 2021

Created hashicorp/terraform-plugin-go#113 upstream to noodle on that.

@bflad bflad added this to the v1.0.0 milestone Mar 16, 2022
@bflad bflad added the breaking-change This PR introduces a breaking change or the resolution of this issue may require a breaking change. label Nov 9, 2022
@bflad bflad removed this from the v1.0.0 milestone Nov 9, 2022
bflad added a commit that referenced this issue Nov 10, 2022
Reference: #66
Reference: #186

Documenting existing intentions and behaviors with `tfsdk.Config`, `tfsdk.Plan`, and `tfsdk.State` type `GetAttribute()` and `SetAttribute()` methods.
bflad added a commit that referenced this issue Nov 10, 2022
Reference: #66
Reference: #186

Documenting existing intentions and behaviors with `tfsdk.Config`, `tfsdk.Plan`, and `tfsdk.State` type `GetAttribute()` and `SetAttribute()` methods.
@bflad
Copy link
Contributor Author

bflad commented Dec 16, 2022

Another option here is limiting GetAttribute() and SetAttribute() to only allow a single AtName() path step for the path. That removes the possibility of getting an incorrect value state, since Terraform should always ensure that top-level attributes are always set correctly. This also would remove the current framework implementation complexity associated with switching to/from the terraform-plugin-go path system and transform data manipulation beyond that first "level".

Whether or not that means making that name parameter a string, path.PathStepAttributeName (confusing or verbose for provider code, I'm sure), or leaving it as path.Path is certainly debatable. We would not be able to change the existing tfsdk.Config, tfsdk.Plan, or tfsdk.State parameter implementation or limit its existing implementation due to 1.x compatibility promises, but it may be something to consider when thinking about #590 should new types be introduced.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
breaking-change This PR introduces a breaking change or the resolution of this issue may require a breaking change. enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

2 participants