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

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 opened this issue Sep 14, 2024 · 2 comments · Fixed by #161
Labels
bug Something isn't working

Comments

@oscarhermoso
Copy link

oscarhermoso commented Sep 14, 2024

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"
  # },
  1. 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]]]<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]]<"action":tftypes.String<"accept">,
│ "description":tftypes.String<"K3s supervisor and Kubernetes API Server">,
│ "destination_addresses":tftypes.List[tftypes.String]<unknown>,
│ "destination_ports":tftypes.List[tftypes.String]<tftypes.String<"6443">>, "protocol":tftypes.String<"tcp">,
│ "source_addresses":tftypes.List[tftypes.String]<tftypes.String<"0.0.0.0/0">>>>, "server_id":tftypes.Number<unknown>>
│ 
│ (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]]]<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]]<"action":tftypes.String<"accept">,
│ "description":tftypes.String<"K3s supervisor and Kubernetes API Server">,
│ "destination_addresses":tftypes.List[tftypes.String]<>,
│ "destination_ports":tftypes.List[tftypes.String]<tftypes.String<"6443">>, "protocol":tftypes.String<"tcp">,
│ "source_addresses":tftypes.List[tftypes.String]<tftypes.String<"0.0.0.0/0">>>>, "server_id":tftypes.Number<unknown>>

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
Copy link
Member

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
Copy link
Author

Thanks @austinvalle ❤️

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Oct 27, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug Something isn't working
Projects
None yet
2 participants