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

Add Logging for Differences Between Proposed New State and Prior State During Planning #627

Closed
bflad opened this issue Jan 19, 2023 · 1 comment · Fixed by #630
Closed
Labels
enhancement New feature or request

Comments

@bflad
Copy link
Member

bflad commented Jan 19, 2023

Module version

v1.1.1

Use-cases

During the PlanResourceChange RPC, the framework currently performs the following logic:

  • If there are differences between the proposed new state and prior state, automatically mark any Computed attributes without a configuration value as unknown.
  • Run schema-based plan modification
  • Run resource-based plan modification

The first piece of logic there is to prevent a large class of Terraform inconsistent result after apply errors for practitioners where Terraform requires that an applied state matches the planned state. The tradeoff here is that it is up to provider developers to "enhance" the plan to be more accurate by converting those unknown values if they are not expected to change (e.g. UseStateForUnknown()).

The proposed new state and prior state come from Terraform across the protocol. (Aside: We currently do nothing to this data except normalize empty collection block values to null, however this is done for all data coming across so it should never introduce unintentional differences between them.) Essentially, we are trusting that Terraform is correctly calculating the proposed new state data, but we have found a few cases where Terraform was not doing that perfectly given the framework's more robust type system as compared to the prior SDK.

Provider developers have raised a few issues around this. The rub here is that they are seeing the symptoms of these issues where Terraform is showing unexpected plans with these unknown values because of the framework automatically modifying the plan instead of the root causes that triggered the framework's automatic plan modification. Sometimes this can lead to wasted development time looking at the attributes marked as unknown, rather than the real issue.

Attempted Solutions

Troubleshooting these situations is more difficult than it should be, especially in the case of provider acceptance testing. Current options (if you know exactly for what you are looking for) right now are:

  • Using the TF_LOG_SDK_PROTO_DATA_DIR environment variable to inspect the PlanResourceChange RPC proposed new state and prior state data.
  • Inspecting the JSON plan
  • (Sometimes) Comparing states after re-applies

Proposal

When the framework is detecting a difference between the proposed new state and prior state, it could start logging the attribute paths of which root attributes/blocks are unequal. For example:

TIMESTAMP [DEBUG] sdk.framework: Detected value change between proposed new state and prior state: tf_attribute_path=attr1 ... other existing fields ...

For very safe cases, such as null <=> "" (hashicorp/terraform#31887), we could consider promoting that to WARN as its a known issue and potentially tailor the message further. We should avoid showing known values outside of Go zero-values in this logging though, as those known values may contain sensitive data which would be very complex to filter/mask.

References

@bflad bflad added the enhancement New feature or request label Jan 19, 2023
bflad added a commit that referenced this issue Jan 20, 2023
… marking

Reference: #627

This logging only contains the attribute paths to prevent showing sensitive or large values. This also promotes the marking log level from TRACE to DEBUG since the behavior may be unexpected by provider developers.

Example logging output from a provider with detected differences:

```
2023-01-19T20:42:44.529-0500 [DEBUG] sdk.framework: Detected value change between proposed new state and prior state: tf_resource_type=jupiterone_rule tf_req_id=fe77fac5-bdee-7fae-435e-0669566f5d6e tf_provider_addr=registry.terraform.io/hashicorp/jupiterone tf_rpc=PlanResourceChange tf_attribute_path=operations
2023-01-19T20:42:44.529-0500 [DEBUG] sdk.framework: Marking Computed attributes with null configuration values as unknown (known after apply) in the plan to prevent potential Terraform errors: tf_resource_type=jupiterone_rule tf_req_id=fe77fac5-bdee-7fae-435e-0669566f5d6e tf_provider_addr=registry.terraform.io/hashicorp/jupiterone tf_rpc=PlanResourceChange
```
@bflad bflad closed this as completed in #630 Feb 2, 2023
bflad added a commit that referenced this issue Feb 2, 2023
… marking (#630)

Reference: #627

This logging only contains the attribute paths to prevent showing sensitive or large values. This also promotes the marking log level from TRACE to DEBUG since the behavior may be unexpected by provider developers.

Example logging output from a provider with detected differences:

```
2023-01-19T20:42:44.529-0500 [DEBUG] sdk.framework: Detected value change between proposed new state and prior state: tf_resource_type=jupiterone_rule tf_req_id=fe77fac5-bdee-7fae-435e-0669566f5d6e tf_provider_addr=registry.terraform.io/hashicorp/jupiterone tf_rpc=PlanResourceChange tf_attribute_path=operations
2023-01-19T20:42:44.529-0500 [DEBUG] sdk.framework: Marking Computed attributes with null configuration values as unknown (known after apply) in the plan to prevent potential Terraform errors: tf_resource_type=jupiterone_rule tf_req_id=fe77fac5-bdee-7fae-435e-0669566f5d6e tf_provider_addr=registry.terraform.io/hashicorp/jupiterone tf_rpc=PlanResourceChange
```
@github-actions
Copy link

github-actions bot commented Mar 5, 2023

I'm going to lock this issue because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active issues.
If you have found a problem that seems similar to this, 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 Mar 5, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
enhancement New feature or request
Projects
None yet
1 participant