Optional + Computed TypeMap produces inconsistent result

appilon commented 2 years ago

Module version

0.4.2

Relevant provider source code

"permissions": {
                Type:     types.MapType{ElemType: types.BoolType},
                Optional: true,
                Computed: true,
            },

This is a very unique situation, the upstream API has a set of permissions that are dynamic based on the license of the resource. The entries in the map are dynamic and the default value for them are ALSO dynamic, hence the need for computed here. I am giving the user the ability to concretely set the permissions so it's also optional. I may be asking too much of the Framework/Terraform.

Terraform Configuration Files

data "sample_user_license" "standard" {
  license_definition_key = "standard"
}

resource "sample_profile" "test" {
  name            = "test"
  user_license_id = data.sample_user_license.standard.id
  description     = "test"
  permissions = {
    email_single = true
    edit_task = true
  }
}

Expected Behavior

Successful apply, with the dynamic set of permissions set in state

Actual Behavior

╷
│ Error: Provider produced inconsistent result after apply
│
│ When applying changes to sample_profile.test, provider "provider[\"registry.terraform.io/hashicorp/sample\"]" produced an unexpected new value: .permissions: new element "view_roles" has appeared.
│
│ This is a bug in the provider, which should be reported in the provider's own issue tracker.
╵
... this error repeats for the many many entries in the map set by the upstream API

Oddly enough the state does seem to have been set properly, so it's almost as if everything is fine but Terraform is just making a stink

➜ terraform state show sample_profile.test
# sample_profile.test: (tainted)
resource "sample_profile" "test" {
    description     = "test"
    id              = "00eB0000000zMDzIAM"
    name            = "test"
    permissions     = {
        "access_cmc"                         = false
        "access_content_builder"             = false
        "account_switcher_user"              = false
        "activate_contract"                  = false
        "activate_order"                     = false
        "activities_access"                  = true
        "add_analytics_remote_connections"   = false
        ...

Steps to Reproduce

References

bflad commented 2 years ago

Hey @appilon 👋 Thank you for raising this and sorry you ran into trouble here.

To a certain degree, this might be expected behavior with Terraform CLI, where an attribute must either be wholly known (at least all map keys, potentially with unknown values) or marked as wholly unknown. This is markedly one of the biggest differences between Terraform Plugin SDK and any other frameworks, such as this one, where there are no special rules to allow any changes between the proposed plan and returned state during update except unknown values.

There are potentially a few paths forward:

If the API in question has a lookup mechanism to retrieve those key-values, an attribute plan modifier could be created that performs that API call and merges those other key-values into the plan (or at least mark those keys as unknown).
Using update logic that filters the API response based on the given configuration, only setting configured keys into state. Caveat: No drift detection for other keys.
Using update logic that sets the configuration into state. Caveat: No drift detection for other keys.

You may also be able to find some inspiration with the Kubernetes provider (at least the parts based off the lower level terraform-plugin-go) since Kubernetes annotations have similar dynamic behaviors based on other runtime and configuration of the system. Hope this helps.

paddycarver commented 2 years ago

Generally, the way I've seen this handled is to have two attributes: one, an optional attribute, is the "managed keys" attribute, which contains only the keys/values the user defines in their configuration. The other, a computed attribute, is the "all keys" attribute, which reflects all the keys and their values regardless of whether they're in the configuration or not.

As Brian noted, you'll lose some drift detection here, because any keys added out of band are (by design) not managed until the user puts them in the "managed keys" attribute.

appilon commented 2 years ago

@paddycarver @bflad Thank you for the information, yes specifically Paddy I was leaning towards what you describe as the endgame solution, for now I'm going to just allow for the "managed keys" on the assumption that interpolating all the other keys is not valuable at this time, and upgrade to an additional Computed map for the ability to "read the rest" if desired. Should I close this issue as it seems like this is just a known limitation of what Terraform can do?

paddycarver commented 2 years ago

Works for me, if you're okay with that resolution. :)

github-actions[bot] commented 2 years ago

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.

hashicorp / terraform-plugin-framework