Consider Method for Disabling Legacy SDK Data Consistency Warning Log Demotion

[bflad]

Terraform Version

Terraform v1.5.4
on darwin_arm64

Use Cases

Terraform provider resources written with the legacy terraform-plugin-sdk Go module have a special flag enabled in the protocol which demotes certain cases of data consistency errors into warning logs:

TIMESTAMP [WARN]  Provider "TYPE" produced an invalid plan for ADDRESS, but we are tolerating it because it is using the legacy plugin SDK.
    The following problems may be the cause of any confusing errors from downstream operations:
      - .ATTRIBUTE: planned value cty.False for a non-computed attribute

If the problematic attribute value that raises the warning log is referenced elsewhere in a configuration, Terraform will raise an error diagnostic in the receiving location, which can help with finding these cases, but has a relatively low hit rate as compared to every attribute value that has this issue. This was a pragmatic choice for the existing provider ecosystem since at the time there was not a updated provider development SDK to match Terraform 0.12 and later concepts. It would have been extremely difficult for practitioners to upgrade Terraform if a part of doing so would have been to force all providers to fix all instances of these data inconsistency errors.

Fast forward to the present, the newer terraform-plugin-framework Go module is available, but it intentionally does not enable the protocol flag since a goal for moving the provider ecosystem forward is to not support errant provider implementations with respect to Terraform's intended data handling rules. Provider developers are embarking on the journey of migrating their terraform-plugin-sdk based providers to terraform-plugin-framework, and potentially encountering these data consistency errors for the first time either during initial acceptance testing after migrating their code or in worst case scenarios, after its been released after the migration.

At scale with larger providers, such as a major cloud provider, it is difficult to know about these underlying issues since they only exist in logging. Most practitioners/environments are not running with Terraform's logging capabilities enabled (e.g. setting the TF_LOG environment variable), and even for environments where this is enabled, it is difficult for developers to discover and remediate these issues since there are no native provider acceptance testing capabilities to detect this situation short of writing acceptance tests which explicitly check every attribute value in every resource.

Attempted Solutions

Rely on practitioner bug reports of the downstream resource error (when triggered) or provider developers enabling logging in acceptance testing environments and searching those logs for the relevant log entries.

Proposal

Introduce an environment variable so provider developers can have these errors raised during provider acceptance testing. In the core logic that checks for the legacy type system flag, conditionalize the warning log demotion on also not having the environment variable set. Normal practitioners should never encounter the newer behavior. Provider developers have the option of setting this environment variable in their provider acceptance testing environments. The terraform-plugin-framework migration guide can explicitly mention this environment variable as a preparation step.

References

No response

[bflad]

Another potential option we have would be doing something similar using an environment variable within terraform-plugin-sdk itself to not set the legacy type system protocol flag at all. I'm not sure if there would be additional considerations on the core side of doing this though.

[jbardin]

If the SDK were to just not set the flag, I think you would get the same behavior you are proposing, and I think it would be the easiest solution to conditionally switching the behavior.

One thing to consider though is that many of these errors are unavoidable with the legacy SDK, so enabling the strict protocol handling will make many resources non-functional. This will raise the error to the developer of course, but there's nothing that can be done about it without proactively moving to the plugin-framework. Since it requires proactive involvement regardless, are you sure it's more useful than directly looking for the warnings in the logs?

[apparentlymart]

I agree with @jbardin that I'd prefer a solution that's entirely within the SDKv2 codebase so that provider developers can enable it independently of what version of Terraform Core they are using.

If it were in the SDK then it also opens some additional options for how to enable it, instead of (or in addition to) environment variables:

• A special build tag or linker-set variable at build time.
• Something a provider developer can activate using an extra function call in their provider's init code.
• The previous item but on a per-resource-type basis rather than globally. e.g. a new method on ResourceData that means "I'm intending not to need the legacy SDK exception for this response".

The difference here is admittedly quite marginal, but it would avoid any situations where end-users might intentionally or unintentionally enable this setting on released providers and then open confusing bug reports about it.

With that said, I also share @jbardin's concern that this would probably just raise unavoidable errors where the only path forward would be to switch to the framework anyway. I wonder if we can instead try to document better how to find and understand these warning messages as a preparation step while planning a migration to the framework.

[bflad]

One thing to consider though is that many of these errors are unavoidable with the legacy SDK, so enabling the strict protocol handling will make many resources non-functional.

With that said, I also share @jbardin's concern that this would probably just raise unavoidable errors where the only path forward would be to switch to the framework anyway.

I was hoping that we might be able to tailor the error handling on core's side to avoid raising errors for unavoidable legacy SDK behaviors, such as providers returning unexpected values without Computed being set, since enabling that in the legacy SDK brings other potentially undesirable behaviors of copying prior state, etc.

---

The real crux here is that there is no intention of a team spending a time to properly design something for this situation. The goal was to loosely help provider developers (this feature request came from a major cloud partner) find out about data consistency issues separately from migrating their resources to the new framework when dealing with hundreds of resources. I will transfer this issue to the terraform-plugin-sdk repository for future consideration though.

[bflad]

Raised #1229 to add helper/schema.Resource level flags for this and some website documentation around the issues.

[apparentlymart]

I missed the reply above before and I don't think what I have to say here really changes what you implemented but just for posterity in case someone finds this later and wonders where that dangling thread leads... 😀

The flag in the provider protocol that this is now allowing provider developers to choose to switch off was at the time the most granular kind of detection we could imagine, based both on how the provider schema works and on how Terraform Core deals with provider configurations internally.

While it's possible that with fresh eyes we could imagine a more granular form of it that e.g. allows disregarding only a subset of the possible errors based on additional details in the schema, I don't think any of us is still familiar enough with the details of that tradeoff -- which itself arose from carefully navigating conflicting constraints both on the Terraform Core and the SDK sides -- to be able to design such a thing without some non-trivial research effort first, which is hard to justify for a legacy escape hatch that we're trying to move away from.

What you implemented here seems like a nice compromise, which is hopefully granular enough to make it useful as a migration aid without burdening provider developers with all of the legacy SDK's quirks all at once.

[github-actions]

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.