hashicorp / terraform-plugin-framework

A next-generation framework for building Terraform providers.
https://developer.hashicorp.com/terraform/plugin/framework
Mozilla Public License 2.0
306 stars 93 forks source link

Set Nested Block Can Receive Unknown Value from a Dynamic Block in a Module #1025

Closed zliang-akamai closed 3 months ago

zliang-akamai commented 3 months ago

We originally received the bug report from: https://github.com/linode/terraform-provider-linode/issues/1528

Module version

github.com/hashicorp/terraform-plugin-framework v1.11.0

Relevant provider source code

Our schema and model design is like:

Blocks: map[string]schema.Block{
    "bucket_access": schema.SetNestedBlock{
        Description: "A list of permissions to grant this limited access key.",
        NestedObject: schema.NestedBlockObject{
            Attributes: map[string]schema.Attribute{
                "bucket_name": schema.StringAttribute{
                    Description: "The unique label of the bucket to which the key will grant limited access.",
                    Required:    true,
                },
                "cluster": schema.StringAttribute{
                    Description: "The Object Storage cluster where the bucket resides. " +
                        "Deprecated in favor of `region`",
                    Optional: true,
                    Computed: true,
                    DeprecationMessage: "The `cluster` attribute in a `bucket_access` block has " +
                        "been deprecated in favor of `region` attribute. A cluster value can be " +
                        "converted to a region value by removing -x at the end, for example, a " +
                        "cluster value `us-mia-1` can be converted to region value `us-mia`",
                    Validators: []validator.String{
                        stringvalidator.ExactlyOneOf(
                            path.MatchRelative().AtParent().AtName("region"),
                        ),
                    },
                    PlanModifiers: []planmodifier.String{
                        stringplanmodifier.UseStateForUnknown(),
                    },
                },
                "region": schema.StringAttribute{
                    Description: "The region where the bucket resides.",
                    Optional:    true,
                    Computed:    true,
                    Validators: []validator.String{
                        stringvalidator.ExactlyOneOf(
                            path.MatchRelative().AtParent().AtName("cluster"),
                        ),
                    },
                    PlanModifiers: []planmodifier.String{
                        stringplanmodifier.UseStateForUnknown(),
                    },
                },
                "permissions": schema.StringAttribute{
                    Description: "This Limited Access Key's permissions for the selected bucket.",
                    Required:    true,
                    // TODO: validate perms
                },
            },
        },
        PlanModifiers: []planmodifier.Set{
            setplanmodifier.RequiresReplace(),
            setplanmodifier.UseStateForUnknown(),
        },
    },
},
type ResourceModel struct {
    // other fields...
    BucketAccess []BucketAccessModelEntry `tfsdk:"bucket_access"`
}

type BucketAccessModelEntry struct {
    BucketName  types.String `tfsdk:"bucket_name"`
    Cluster     types.String `tfsdk:"cluster"`
    Permissions types.String `tfsdk:"permissions"`
    Region      types.String `tfsdk:"region"`
}

And we always use resp.Diagnostics.Append(req.Plan.Get(ctx, &plan)...) to get plan values into a model.

Terraform Configuration Files

In root directory:

terraform {
  required_providers {
    linode = {
      source  = "linode/linode"
      version = "2.25.0"
    }
  }
}

provider "linode" {
}

module "test" {
  source = "./access-key"

  shards_count = 6
}

In module directory:

terraform {
  required_providers {
    linode = {
      source = "linode/linode"
    }
  }
}

variable "shards_count" {
  type = number
}

resource "linode_object_storage_key" "this" {

  label = "test"
  dynamic "bucket_access" {

    for_each = range(var.shards_count)

    content {
      bucket_name = "test"
      region      = "us-iad-1"
      permissions = "read_only"
    }
  }
}

Weirdly, it works fine if the resource is in the root module..

Expected Behavior

Plan doesn't contain Unknown value and conversion works well.

by the time a resource is expected to be created, read, updated, or deleted, only its computed attributes can be unknown. The rest are guaranteed to have known values (or be null).

https://developer.hashicorp.com/terraform/plugin/framework/handling-data/terraform-concepts#unknown-values

Actual Behavior

╷
│ Error: Value Conversion Error
│ 
│   with module.test.linode_object_storage_key.this,
│   on access-key/main.tf line 13, in resource "linode_object_storage_key" "this":
│   13: resource "linode_object_storage_key" "this" {
│ 
│ An unexpected error was encountered trying to build a value. This is always an error in the provider. Please report the
│ following to the provider developer:
│ 
│ Received unknown value, however the target type cannot handle unknown values. Use the corresponding `types` package type or a
│ custom type that handles unknown values.
│ 
│ Path: bucket_access
│ Target Type: []objkey.BucketAccessModelEntry
│ Suggested Type: basetypes.SetValue

Steps to Reproduce

  1. terraform init
  2. terraform apply

References

https://github.com/linode/terraform-provider-linode/issues/1528

chrismarget commented 3 months ago

Your citation says: "by the time a resource is expected to be created, read, updated, or deleted, only its computed attributes can be unknown"

That "by the time" bit is doing some heavy lifting and is probably the source of some confusion.

The error is probably happening during validation here long before we reach "the time" when attributes are guaranteed to be not unknown.

This recent documentation update is probably relevant. The upshot is that validators run early and often, so they need to be able to handle Unknown values. Your model, because it contains a Go slice, cannot handle Unknown values.

Changing this line to the following will probably resolve the problem, but you'll have to take extra steps when unpacking the data:

BucketAccess types.Set `tfsdk:"bucket_access"`
zliang-akamai commented 3 months ago

@chrismarget thank you so much for the answer, and you are right! I will fix the validate function in our provider accordingly.