Skip to content

Commit 8b4b14f

Browse files
committed
vpc_firewall_rules: fix provider produced invalid plan
Added acceptance tests to surface the issue reported in #453. Updated the schema for `rules` from `schema.SetNestedAttribute` to `schema.ListNestedAttribute` to better work with how the Oxide API treats VPC firewall rules. See below for the reason why this was required. In #432 we removed the requires replace directive from the `vpc_firewall_rules` resource to support in-place modification. The intent was to prevent unnecessary network connectivity issues while VPC firewall rules are deleted and created anew (e.g., when the delete succeeds and the create fails). With this change, `Update` is now called rather than `Delete` followed by `Create`. However, this change was also a bit of a workaround since the Oxide API does not support in-place updates of VPC firewall rules, at least not in the manner than Terraform expects. Instead, when you replace VPC firewall rules using the Oxide API each rule will be given a unique ID. ``` > curl --silent --request PUT --header "Authorization: Bearer $OXIDE_TOKEN" --header "Content-Type: application/json" $OXIDE_HOST/v1/vpc-firewall-rules?project=matthewsanabria&vpc=example --data @/tmp/rules.json {"rules":[{"id":"0852c8c8-e1d4-4dd4-84a7-19946eae0313","name":"allow-https","description":"Allow HTTPS.","time_created":"2025-06-20T19:40:22.514499Z","time_modified":"2025-06-20T19:40:22.514499Z","status":"enabled","direction":"inbound","targets":[{"type":"subnet","value":"example"}],"filters":{"hosts":[{"type":"vpc","value":"example"}],"protocols":["TCP"],"ports":["443"]},"action":"allow","priority":50,"vpc_id":"b7b7b444-1c64-4cb5-82d0-f1e4404a100c"},{"id":"bac0f44f-1835-4943-9944-08e6c328bbea","name":"allow-ssh","description":"Allow SSH (new).","time_created":"2025-06-20T19:40:22.514501Z","time_modified":"2025-06-20T19:40:22.514501Z","status":"enabled","direction":"inbound","targets":[{"type":"subnet","value":"example"}],"filters":{"hosts":[{"type":"vpc","value":"example"}],"protocols":["TCP"],"ports":["22"]},"action":"allow","priority":50,"vpc_id":"b7b7b444-1c64-4cb5-82d0-f1e4404a100c"}]} > curl --silent --request GET --header "Authorization: Bearer $OXIDE_TOKEN" --header "Content-Type: application/json" $OXIDE_HOST/v1/vpc-firewall-rules?project=matthewsanabria&vpc=example | jq -r '.rules[] | pick(.id, .name)' { "id": "0852c8c8-e1d4-4dd4-84a7-19946eae0313", "name": "allow-https" } { "id": "bac0f44f-1835-4943-9944-08e6c328bbea", "name": "allow-ssh" } ``` Even if you issue another `PUT` request with the exact same request body Oxide will still generate new IDs for the rules. ``` > curl --silent --request PUT --header "Authorization: Bearer $OXIDE_TOKEN" --header "Content-Type: application/json" $OXIDE_HOST/v1/vpc-firewall-rules?project=matthewsanabria&vpc=example --data @/tmp/rules.json {"rules":[{"id":"74658b3c-f3f9-4e36-985c-f9259c81c98a","name":"allow-https","description":"Allow HTTPS.","time_created":"2025-06-20T19:40:47.171866Z","time_modified":"2025-06-20T19:40:47.171866Z","status":"enabled","direction":"inbound","targets":[{"type":"subnet","value":"example"}],"filters":{"hosts":[{"type":"vpc","value":"example"}],"protocols":["TCP"],"ports":["443"]},"action":"allow","priority":50,"vpc_id":"b7b7b444-1c64-4cb5-82d0-f1e4404a100c"},{"id":"91b353f1-8025-4c89-9d28-760c2c5719dd","name":"allow-ssh","description":"Allow SSH (new).","time_created":"2025-06-20T19:40:47.171868Z","time_modified":"2025-06-20T19:40:47.171868Z","status":"enabled","direction":"inbound","targets":[{"type":"subnet","value":"example"}],"filters":{"hosts":[{"type":"vpc","value":"example"}],"protocols":["TCP"],"ports":["22"]},"action":"allow","priority":50,"vpc_id":"b7b7b444-1c64-4cb5-82d0-f1e4404a100c"}]} > curl --silent --request GET --header "Authorization: Bearer $OXIDE_TOKEN" --header "Content-Type: application/json" $OXIDE_HOST/v1/vpc-firewall-rules?project=matthewsanabria&vpc=example | jq -r '.rules[] | pick(.id, .name)' { "id": "74658b3c-f3f9-4e36-985c-f9259c81c98a", "name": "allow-https" } { "id": "91b353f1-8025-4c89-9d28-760c2c5719dd", "name": "allow-ssh" } ``` Let's walk through why this is a problem for Terraform. I used Terraform to create two VPC firewall rules as shown below. ``` > jq -r '.resources[1].instances[].attributes.rules[] | pick(.id, .name)' terraform.tfstate { "id": "12bbb9ca-3360-4f9f-ad5a-ada3cbb736c4", "name": "allow-https" } { "id": "5d83771f-76fd-421f-a785-3726b6310389", "name": "allow-ssh" } ``` I then updated the port for the `allow-ssh` rule from 22 to 23. The Terraform plan shows that the resource will be updated in place by deleting the `allow-ssh` rule with ID `5d83771f-76fd-421f-a785-3726b6310389` and creating a new one. It will not touch the `allow-https` rule at all. ``` > terraform apply oxide_vpc.example: Refreshing state... [id=b7b7b444-1c64-4cb5-82d0-f1e4404a100c] oxide_vpc_firewall_rules.example: Refreshing state... [id=c75d1e61-9737-4abd-be16-8837920e2344] Terraform used the selected providers to generate the following execution plan. Resource actions are indicated with the following symbols: ~ update in-place Terraform will perform the following actions: # oxide_vpc_firewall_rules.example will be updated in-place ~ resource "oxide_vpc_firewall_rules" "example" { ~ id = "c75d1e61-9737-4abd-be16-8837920e2344" -> (known after apply) ~ rules = [ - { - action = "allow" -> null - description = "Allow SSH." -> null - direction = "inbound" -> null - filters = { - hosts = [ - { - type = "vpc" -> null - value = "example" -> null }, ] -> null - ports = [ - "22", ] -> null - protocols = [ - "TCP", ] -> null } -> null - id = "5d83771f-76fd-421f-a785-3726b6310389" -> null - name = "allow-ssh" -> null - priority = 50 -> null - status = "enabled" -> null - targets = [ - { - type = "subnet" -> null - value = "example" -> null }, ] -> null - time_created = "2025-06-20 20:30:10.992084 +0000 UTC" -> null - time_modified = "2025-06-20 20:30:10.992084 +0000 UTC" -> null }, + { + action = "allow" + description = "Allow SSH." + direction = "inbound" + filters = { + hosts = [ + { + type = "vpc" + value = "example" }, ] + ports = [ + "23", ] + protocols = [ + "TCP", ] } + id = (known after apply) + name = "allow-ssh" + priority = 50 + status = "enabled" + targets = [ + { + type = "subnet" + value = "example" }, ] + time_created = (known after apply) + time_modified = (known after apply) }, # (1 unchanged element hidden) ] # (1 unchanged attribute hidden) } Plan: 0 to add, 1 to change, 0 to destroy. ``` Applying this plan causes the error noted in this issue. Looking closer at the error text we see Terraform is referring to the `allow-https` rule with ID `12bbb9ca-3360-4f9f-ad5a-ada3cbb736c4`, not the `allow-ssh` rule. ``` Do you want to perform these actions? Terraform will perform the actions described above. Only 'yes' will be accepted to approve. Enter a value: yes oxide_vpc_firewall_rules.example: Modifying... [id=c75d1e61-9737-4abd-be16-8837920e2344] ╷ │ Error: Provider produced inconsistent result after apply │ │ When applying changes to oxide_vpc_firewall_rules.example, provider "provider[\"registry.terraform.io/oxidecomputer/oxide\"]" │ produced an unexpected new value: .rules: planned set element │ cty.ObjectVal(map[string]cty.Value{"action":cty.StringVal("allow"), "description":cty.StringVal("Allow HTTPS."), │ "direction":cty.StringVal("inbound"), │ "filters":cty.ObjectVal(map[string]cty.Value{"hosts":cty.SetVal([]cty.Value{cty.ObjectVal(map[string]cty.Value{"type":cty.StringVal("vpc"), │ "value":cty.StringVal("example")})}), "ports":cty.SetVal([]cty.Value{cty.StringVal("443")}), │ "protocols":cty.SetVal([]cty.Value{cty.StringVal("TCP")})}), "id":cty.StringVal("12bbb9ca-3360-4f9f-ad5a-ada3cbb736c4"), │ "name":cty.StringVal("allow-https"), "priority":cty.NumberIntVal(50), "status":cty.StringVal("enabled"), │ "targets":cty.SetVal([]cty.Value{cty.ObjectVal(map[string]cty.Value{"type":cty.StringVal("subnet"), │ "value":cty.StringVal("example")})}), "time_created":cty.StringVal("2025-06-20 20:07:00.207241 +0000 UTC"), │ "time_modified":cty.StringVal("2025-06-20 20:07:00.207241 +0000 UTC")}) does not correlate with any element in actual. │ │ This is a bug in the provider, which should be reported in the provider's own issue tracker. ``` Why does Terraform care about the `allow-https` rule here at all if the plan was never meant to touch it? It's because before the plan Terraform had the following in its state. ``` > jq -r '.resources[1].instances[].attributes.rules[] | pick(.id, .name)' terraform.tfstate { "id": "12bbb9ca-3360-4f9f-ad5a-ada3cbb736c4", "name": "allow-https" } { "id": "9c723c19-16ce-489e-9c6d-23daf2ebeb1c", "name": "allow-ssh" } ``` Terraform expected after the plan was applied the state would look like this. The `allow-https` rule would remain untouched and the `allow-ssh` rule would be completely new with a new ID. ``` > jq -r '.resources[1].instances[].attributes.rules[] | pick(.id, .name)' terraform.tfstate { "id": "12bbb9ca-3360-4f9f-ad5a-ada3cbb736c4", "name": "allow-https" } { "id": "935e19c0-9036-419b-b615-d68665aed048", "name": "allow-ssh" } ``` Instead, the Oxide API generated new IDs for all the rules, so Terraform got back the following state. ``` > jq -r '.resources[1].instances[].attributes.rules[] | pick(.id, .name)' terraform.tfstate { "id": "32f2e026-f258-4512-bd68-79b613a8b5de", "name": "allow-https" } { "id": "935e19c0-9036-419b-b615-d68665aed048", "name": "allow-ssh" } ``` Now Terraform was left wondering what happened to the original `allow-https` rule with ID `12bbb9ca-3360-4f9f-ad5a-ada3cbb736c4`. Since Terraform couldn't find that original rule from the plan in the new state it returned an error.
1 parent 26cfef7 commit 8b4b14f

File tree

2 files changed

+194
-146
lines changed

2 files changed

+194
-146
lines changed

internal/provider/resource_vpc_firewall_rules.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -110,7 +110,7 @@ func (r *vpcFirewallRulesResource) Schema(ctx context.Context, _ resource.Schema
110110
stringplanmodifier.RequiresReplace(),
111111
},
112112
},
113-
"rules": schema.SetNestedAttribute{
113+
"rules": schema.ListNestedAttribute{
114114
Required: true,
115115
Description: "Associated firewall rules.",
116116
NestedObject: schema.NestedAttributeObject{

0 commit comments

Comments
 (0)