hashicorp / terraform-plugin-codegen-framework

Terraform Provider Code Generation Specification to Framework
Mozilla Public License 2.0
39 stars 17 forks source link

Error: Provider produced invalid plan - planned value `cty.ListValEmpty(cty.String)` does not match config value `cty.UnknownVal(cty.List(cty.String))` #160

Closed oscarhermoso closed 2 months ago

oscarhermoso commented 2 months ago

Module version

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

Relevant provider source code

func ServerFirewallRulesResourceSchema(ctx context.Context) schema.Schema {
    return schema.Schema{
        Attributes: map[string]schema.Attribute{
            "firewall_rules": schema.ListNestedAttribute{
                NestedObject: schema.NestedAttributeObject{
                    Attributes: map[string]schema.Attribute{
                        # ...
                        "destination_addresses": schema.ListAttribute{
                            ElementType:         types.StringType,
                            Required:            true,
                            Description:         "The destination addresses to match for this rule. Each address may be an individual IPv4 address or a range in IPv4 CIDR notation.",
                            MarkdownDescription: "The destination addresses to match for this rule. Each address may be an individual IPv4 address or a range in IPv4 CIDR notation.",
                            Validators: []validator.List{
                                listvalidator.SizeAtLeast(1),
                            },
                        },
                        # ...
                    },
                    CustomType: FirewallRulesType{
                        ObjectType: types.ObjectType{
                            AttrTypes: FirewallRulesValue{}.AttributeTypes(ctx),
                        },
                    },
                },
                Required:            true,
                Description:         "A list of rules for the server. NB: that any existing rules that are not included will be removed. Submit an empty list to clear all rules.",
                MarkdownDescription: "A list of rules for the server. NB: that any existing rules that are not included will be removed. Submit an empty list to clear all rules.",
            },
            # ...
        },
    }
}

Terraform Configuration Files

resource "binarylane_server" "server" {
  count = 1

  name              = "${local.cluster_id}-server-${count.index + 1}"
  region            = "per"
  image             = "ubuntu-24.04"
  size              = "std-min"
  password          = random_password.binarylane.result
  ssh_keys          = [binarylane_ssh_key.example.id]
  vpc_id            = binarylane_vpc.example.id
  public_ipv4_count = 1
  user_data         = sensitive(data.cloudinit_config.server.rendered)
  wait_for_create   = 60 # Must wait for the server to be ready before creating firewall rules
}

resource "binarylane_server_firewall_rules" "example" {
  for_each = { for server in concat(binarylane_server.server, binarylane_server.agent) : server.name => server }

  server_id = each.value.id

  firewall_rules = [
    {
      description           = "K3s supervisor and Kubernetes API Server"
      protocol              = "tcp"
      source_addresses      = ["0.0.0.0/0"]
      destination_addresses = local.server_ips # <-- blows up here
      destination_ports     = ["6443"]
      action                = "accept"
    },
    # ...
  ]
}

Debug Output

https://gist.github.com/oscarhermoso/53849e90d66a97258ca2cb72906fe848

Expected Behavior

Terraform should plan succeed, with binarylane_server_firewall_rules.example["tf-example-k8s-agent-2"].firewall_rules[0].destination_addresses and similar planned as unknown values

i.e.

GIVEN a resource schema with a list of objects attribute AND one of the object properties is a list of strings WHEN the list of strings property is planned with unknown length with unknown values THEN the plan output should also have unknown length with unknown values

Actual Behavior

│ Error: Provider produced invalid plan │ │ Provider "registry.terraform.io/oscarhermoso/binarylane" planned an invalid value for │ binarylane_server_firewall_rules.example["tf-example-k8s-agent-2"].firewall_rules[0].destination_addresses: │ planned value cty.ListValEmpty(cty.String) does not match config value │ cty.UnknownVal(cty.List(cty.String)). │ │ This is a bug in the provider, which should be reported in the provider's own issue tracker.

Steps to Reproduce

  1. Clone https://github.com/oscarhermoso/terraform-provider-binarylane
  2. cd examples/k3s
  3. terraform init
  4. Uncomment block in binarylane_server_firewall_rules.example:
    # TODO: Due to a bug, this only works if you uncomment AFTER creating servers
    # {
    #   description           = "K3s supervisor and Kubernetes API Server"
    #   protocol              = "tcp"
    #   source_addresses      = ["0.0.0.0/0"]
    #   destination_addresses = local.server_ips
    #   destination_ports     = ["6443"]
    #   action                = "accept"
    # },
  5. Run BINARYLANE_API_TOKEN=anything TF_ACC=1 tf apply

Additional Context

Failing in https://github.com/oscarhermoso/terraform-provider-binarylane/blob/main/internal/provider/server_firewall_rules_resource.go

After adding logging in a ModifyPlan method, I can see that the raw Config has type "destination_addresses":tftypes.List[tftypes.String]<unknown>, whereas the raw Plan has type "destination_addresses":tftypes.List[tftypes.String].

Logging ``` │ Warning: req.config │ │ with binarylane_server_firewall_rules.example["tf-example-k8s-agent-1"], │ on main.tf line 131, in resource "binarylane_server_firewall_rules" "example": │ 131: resource "binarylane_server_firewall_rules" "example" { │ │ tftypes.Object["firewall_rules":tftypes.List[tftypes.Object["action":tftypes.String, "description":tftypes.String, │ "destination_addresses":tftypes.List[tftypes.String], "destination_ports":tftypes.List[tftypes.String], │ "protocol":tftypes.String, "source_addresses":tftypes.List[tftypes.String]]], │ "server_id":tftypes.Number]<"firewall_rules":tftypes.List[tftypes.Object["action":tftypes.String, │ "description":tftypes.String, "destination_addresses":tftypes.List[tftypes.String], │ "destination_ports":tftypes.List[tftypes.String], "protocol":tftypes.String, │ "source_addresses":tftypes.List[tftypes.String]]], │ "description":tftypes.String<"K3s supervisor and Kubernetes API Server">, │ "destination_addresses":tftypes.List[tftypes.String], │ "destination_ports":tftypes.List[tftypes.String]>, "protocol":tftypes.String<"tcp">, │ "source_addresses":tftypes.List[tftypes.String]>>>, "server_id":tftypes.Number> │ │ (and 2 more similar warnings elsewhere) ╵ ╷ │ Warning: req.plan │ │ with binarylane_server_firewall_rules.example["tf-example-k8s-agent-1"], │ on main.tf line 131, in resource "binarylane_server_firewall_rules" "example": │ 131: resource "binarylane_server_firewall_rules" "example" { │ │ tftypes.Object["firewall_rules":tftypes.List[tftypes.Object["action":tftypes.String, "description":tftypes.String, │ "destination_addresses":tftypes.List[tftypes.String], "destination_ports":tftypes.List[tftypes.String], │ "protocol":tftypes.String, "source_addresses":tftypes.List[tftypes.String]]], │ "server_id":tftypes.Number]<"firewall_rules":tftypes.List[tftypes.Object["action":tftypes.String, │ "description":tftypes.String, "destination_addresses":tftypes.List[tftypes.String], │ "destination_ports":tftypes.List[tftypes.String], "protocol":tftypes.String, │ "source_addresses":tftypes.List[tftypes.String]]], │ "description":tftypes.String<"K3s supervisor and Kubernetes API Server">, │ "destination_addresses":tftypes.List[tftypes.String]<>, │ "destination_ports":tftypes.List[tftypes.String]>, "protocol":tftypes.String<"tcp">, │ "source_addresses":tftypes.List[tftypes.String]>>>, "server_id":tftypes.Number> ```

Considering the terraform-provider-binarylane priovider does not contain any code convert req.Raw.Config to req.Raw.Plan- my understanding is that either this is a bug with Terraform, or I need to add some code in a ModifyPlan method or similar to handle unknown values.

Either way, would appreciate any assistance with investigating the issue. Thanks in advance.

References

austinvalle commented 2 months ago

Hey there @oscarhermoso 👋🏻 , thanks for reporting the issue and sorry you're running into trouble here.

Appreciate the detailed example, this bug is actually in the custom type code (FirewallRulesValue).ToObjectValue (which I believe from looking at your repo, is coming from our code generator terraform-plugin-codegen-framework).

https://github.com/oscarhermoso/terraform-provider-binarylane/blob/44772aa55dcd6bfcc39c04498283cb5d4742dcad/internal/resources/server_firewall_rules_resource_gen.go#L624-L643

When creating a new object, it doesn't check if the nested value is null or unknown before attempting to create a known value, by default it gets zero elements, and mistakenly creates a known empty list, rather than an unknown list.

This object creation happens during plan modification automatically, so it's silently modifying the plan to an empty list here: https://github.com/hashicorp/terraform-plugin-framework/blob/91cc7808c4760d079d46dee1e7e36d00b89d10e0/internal/fwserver/attribute_plan_modification.go#L194


I believe the correct implementation should look something like:

func (v FirewallRulesValue) ToObjectValue(ctx context.Context) (basetypes.ObjectValue, diag.Diagnostics) {
    var diags diag.Diagnostics

    // I'd expect the generated code to look something like
    var destinationAddressesVal basetypes.ListValue
    var d diag.Diagnostics
    switch {
    case v.DestinationAddresses.IsUnknown():
        destinationAddressesVal = types.ListUnknown(types.StringType)
    case v.DestinationAddresses.IsNull():
        destinationAddressesVal = types.ListNull(types.StringType)
    default:
        destinationAddressesVal, d = types.ListValue(types.StringType, v.DestinationAddresses.Elements())
    }

    diags.Append(d...)

    // .. the rest of the ToObjectValue function ..

This bug will need to be fixed in the code generator itself as well so that all the other nested values receive the same treatment, so I'm going to transfer this issue over to that repository.

oscarhermoso commented 2 months ago

Thanks @austinvalle :heart: