In-place update for VPC firewall rules (#432)

karencfv · sudomateo · web-flow · commit 747832902149 · 2025-06-02T16:30:49.000-07:00
Fixes: #431 --------- Co-authored-by: Matthew Sanabria <matthew.sanabria@oxide.computer>
diff --git a/.changelog/0.10.0.toml b/.changelog/0.10.0.toml
@@ -1,18 +1,18 @@
 [[breaking]]
-title = "Minimum Terraform version required"
-description = "`oxide_silo` [#425](https://github.com/oxidecomputer/terraform-provider-oxide/pull/425). Breaking change due to `tls_certificates` attribute being a
-[write-only attribute](https://developer.hashicorp.com/terraform/plugin/framework/resources/write-only-arguments)."
+title = "Minimum Terraform version v1.11 required"
+description = "Due to the introduction of [write-only attributes](https://developer.hashicorp.com/terraform/plugin/framework/resources/write-only-arguments) in the new `oxide_silo` resoucre, the minimum Terraform version is now v1.11 [#425](https://github.com/oxidecomputer/terraform-provider-oxide/pull/425)."
 
 [[features]]
 title = "New resource"
 description = "`oxide_silo` [#425](https://github.com/oxidecomputer/terraform-provider-oxide/pull/425)."
 
+[[features]]
 title = "New data resource"
 description = "`oxide_vpc_router_route` [#423](https://github.com/oxidecomputer/terraform-provider-oxide/pull/423)."
 
 [[enhancements]]
-title = ""
-description = ""
+title = "VPC firewall rules resource"
+description = "In place updates are now supported [#432](https://github.com/oxidecomputer/terraform-provider-oxide/pull/432)"
 
 [[bugs]]
 title = ""
diff --git a/.github/workflows/build-test.yml b/.github/workflows/build-test.yml
@@ -28,4 +28,4 @@ jobs:
       - name: test
         run: make test
       - name: lint
-        run: sudo make lint
+        run: make lint
diff --git a/Makefile b/Makefile
@@ -113,11 +113,11 @@ sdk-version:
 # This way linting tools don't need to be downloaded/installed every time you
 # want to run the linters.
 VERSION_DIR:=$(GOBIN)/versions
-VERSION_GOLANGCILINT:=v1.61.0
-VERSION_TFPROVIDERDOCS:=v0.9.1
-VERSION_TERRAFMT:=v0.5.2
-VERSION_TFPROVIDERLINT:=v0.30.0
-VERSION_WHATSIT:=7fd2b385f
+VERSION_GOLANGCILINT:=v1.64.8
+VERSION_TFPROVIDERDOCS:=v0.12.1
+VERSION_TERRAFMT:=v0.5.4
+VERSION_TFPROVIDERLINT:=v0.31.0
+VERSION_WHATSIT:=053446d
 
 tools: $(GOBIN)/golangci-lint $(GOBIN)/tfproviderdocs $(GOBIN)/terrafmt $(GOBIN)/tfproviderlint 
 
diff --git a/README.md b/README.md
@@ -21,7 +21,7 @@ To generate a token, follow these steps:
 
 ```hcl
 terraform {
-  required_version = ">= 1.0"
+  required_version = ">= 1.11"
 
   required_providers {
     oxide = {
diff --git a/docs/index.md b/docs/index.md
@@ -24,7 +24,7 @@ Note: Cannot use `profile` with `host` and `token` arguments and vice versa.
 
 ```hcl
 terraform {
-  required_version = ">= 1.0"
+  required_version = ">= 1.11"
 
   required_providers {
     oxide = {
diff --git a/docs/resources/oxide_vpc_firewall_rules.md b/docs/resources/oxide_vpc_firewall_rules.md
@@ -9,6 +9,10 @@ This resource manages VPC firewall rules.
 !> Firewall rules defined by this resource are considered exhaustive and will
 overwrite any other firewall rules for the VPC once applied.
 
+!> Setting the `rules` attribute to `[]` will delete all firewall rules for the
+VPC which may cause undesired network traffic. Please double check the firewall
+rules when updating this resource.
+
 ## Example Usage
 
 ```hcl
@@ -48,7 +52,7 @@ resource "oxide_vpc_firewall_rules" "example" {
 ### Required
 
 - `vpc_id` (String) ID of the VPC that will have the firewall rules applied to.
-- `rules` (Set) Associated firewall rules. Updates require replacement. (see [below for nested schema](#nestedatt--rules))
+- `rules` (Set) Associated firewall rules. Set to `[]` to delete all firewall rules. (see [below for nested schema](#nestedatt--rules))
 
 ### Optional
 
diff --git a/examples/demo/demo.tf b/examples/demo/demo.tf
@@ -1,5 +1,5 @@
 terraform {
-  required_version = ">= 1.0"
+  required_version = ">= 1.11"
 
   required_providers {
     oxide = {
diff --git a/examples/disk_resource/disk.tf b/examples/disk_resource/disk.tf
@@ -1,5 +1,5 @@
 terraform {
-  required_version = ">= 1.0"
+  required_version = ">= 1.11"
 
   required_providers {
     oxide = {
diff --git a/examples/instance_resource/instance.tf b/examples/instance_resource/instance.tf
@@ -1,5 +1,5 @@
 terraform {
-  required_version = ">= 1.0"
+  required_version = ">= 1.11"
 
   required_providers {
     oxide = {
diff --git a/examples/vpc_resource/vpc.tf b/examples/vpc_resource/vpc.tf
@@ -1,5 +1,5 @@
 terraform {
-  required_version = ">= 1.0"
+  required_version = ">= 1.11"
 
   required_providers {
     oxide = {
diff --git a/go.mod b/go.mod
@@ -1,6 +1,6 @@
 module github.com/oxidecomputer/terraform-provider-oxide
 
-go 1.23.0
+go 1.24.3
 
 require (
 	github.com/google/uuid v1.6.0
@@ -11,7 +11,7 @@ require (
 	github.com/hashicorp/terraform-plugin-log v0.9.0
 	github.com/hashicorp/terraform-plugin-sdk/v2 v2.37.0
 	github.com/hashicorp/terraform-plugin-testing v1.13.1
-	github.com/oxidecomputer/oxide.go v0.4.1-0.20250423011427-65b1d0f6b391
+	github.com/oxidecomputer/oxide.go v0.4.1-0.20250530023940-ecfa72d833e0
 	github.com/stretchr/testify v1.10.0
 )
 
diff --git a/go.sum b/go.sum
@@ -136,8 +136,8 @@ github.com/mitchellh/reflectwalk v1.0.2 h1:G2LzWKi524PWgd3mLHV8Y5k7s6XUvT0Gef6zx
 github.com/mitchellh/reflectwalk v1.0.2/go.mod h1:mSTlrgnPZtwu0c4WaC2kGObEpuNDbx0jmZXqmk4esnw=
 github.com/oklog/run v1.0.0 h1:Ru7dDtJNOyC66gQ5dQmaCa0qIsAUFY3sFpK1Xk8igrw=
 github.com/oklog/run v1.0.0/go.mod h1:dlhp/R75TPv97u0XWUtDeV/lRKWPKSdTuV0TZvrmrQA=
-github.com/oxidecomputer/oxide.go v0.4.1-0.20250423011427-65b1d0f6b391 h1:q/ebxcSiqQrrdmDsd+w7gwqZUcGhpHmY9Bu2uYKNGL8=
-github.com/oxidecomputer/oxide.go v0.4.1-0.20250423011427-65b1d0f6b391/go.mod h1:yNLdQdroM42/yDIFlCsLAR9PawAdeJZDgHdAx+wcywg=
+github.com/oxidecomputer/oxide.go v0.4.1-0.20250530023940-ecfa72d833e0 h1:P/pU75YLSgM9vIbdTyAc92H9k+yuXYWLTzaMk71QubA=
+github.com/oxidecomputer/oxide.go v0.4.1-0.20250530023940-ecfa72d833e0/go.mod h1:4gfHlxdBQLs/34UbChPvINd+pGNAnGlASRGEd4xIz1Y=
 github.com/pelletier/go-toml v1.9.5 h1:4yBQzkHv+7BHq2PQUZF3Mx0IYxG7LsP222s7Agd3ve8=
 github.com/pelletier/go-toml v1.9.5/go.mod h1:u1nR/EPcESfeI/szUZKdtJ0xRNbUoANCkoOuaOx1Y+c=
 github.com/pjbgf/sha1cd v0.3.2 h1:a9wb0bp1oC2TGwStyn0Umc/IGKQnEgF0vVaZ8QF8eo4=
diff --git a/internal/provider/resource_vpc_firewall_rules.go b/internal/provider/resource_vpc_firewall_rules.go
@@ -19,7 +19,6 @@ import (
 	"github.com/hashicorp/terraform-plugin-framework/resource"
 	"github.com/hashicorp/terraform-plugin-framework/resource/schema"
 	"github.com/hashicorp/terraform-plugin-framework/resource/schema/planmodifier"
-	"github.com/hashicorp/terraform-plugin-framework/resource/schema/setplanmodifier"
 	"github.com/hashicorp/terraform-plugin-framework/resource/schema/stringplanmodifier"
 	"github.com/hashicorp/terraform-plugin-framework/schema/validator"
 	"github.com/hashicorp/terraform-plugin-framework/types"
@@ -114,9 +113,6 @@ func (r *vpcFirewallRulesResource) Schema(ctx context.Context, _ resource.Schema
 			"rules": schema.SetNestedAttribute{
 				Required:    true,
 				Description: "Associated firewall rules.",
-				PlanModifiers: []planmodifier.Set{
-					setplanmodifier.RequiresReplace(),
-				},
 				NestedObject: schema.NestedAttributeObject{
 					Attributes: map[string]schema.Attribute{
 						"action": schema.StringAttribute{
@@ -495,14 +491,18 @@ func (r *vpcFirewallRulesResource) Delete(ctx context.Context, req resource.Dele
 	tflog.Trace(ctx, fmt.Sprintf("deleted firewall rules for VPC with ID: %v", state.VPCID.ValueString()), map[string]any{"success": true})
 }
 
+// newVPCFirewallRulesUpdateBody builds the parameters required by the Oxide
+// vpc_firewall_rules_update API using the specified rules.
 func newVPCFirewallRulesUpdateBody(rules []vpcFirewallRulesResourceRuleModel) *oxide.VpcFirewallRuleUpdateParams {
-	var updateRules []oxide.VpcFirewallRuleUpdate
+	// The make builtin is used to explicitly get an empty slice rather than a zero
+	// value slice for the use case of removing all the firewall rules from a VPC.
+	//
+	// This is necessary because of the following.
+	// * The vpc_firewall_rules_update API requires `{"rules": []}` to remove all rules.
+	// * [oxide.VpcFirewallRuleUpdateParams] uses `omitzero` on its Rules field.
+	updateRules := make([]oxide.VpcFirewallRuleUpdate, 0)
 	body := new(oxide.VpcFirewallRuleUpdateParams)
 
-	if rules == nil {
-		return body
-	}
-
 	for _, rule := range rules {
 		r := oxide.VpcFirewallRuleUpdate{
 			Action:      oxide.VpcFirewallRuleAction(rule.Action.ValueString()),
@@ -523,8 +523,13 @@ func newVPCFirewallRulesUpdateBody(rules []vpcFirewallRulesResourceRuleModel) *o
 	return body
 }
 
+// newVPCFirewallRulesModel translates a slice of [oxide.VpcFirewallRule] into a
+// slice of [vpcFirewallRulesResourceRuleModel].
 func newVPCFirewallRulesModel(rules []oxide.VpcFirewallRule) ([]vpcFirewallRulesResourceRuleModel, diag.Diagnostics) {
-	var model []vpcFirewallRulesResourceRuleModel
+	// The make builtin is used to explicitly get an empty slice rather than a zero
+	// value slice for the use case of removing all the firewall rules from a VPC.
+	// See the comment within [newVPCFirewallRulesUpdateBody] for more information.
+	model := make([]vpcFirewallRulesResourceRuleModel, 0)
 
 	for _, rule := range rules {
 		m := vpcFirewallRulesResourceRuleModel{
diff --git a/internal/provider/resource_vpc_firewall_rules_test.go b/internal/provider/resource_vpc_firewall_rules_test.go
@@ -173,6 +173,24 @@ resource "oxide_vpc_firewall_rules" "{{.BlockName}}" {
 }
 `
 
+var resourceFirewallRulesUpdateConfigTpl3 = `
+data "oxide_project" "{{.SupportBlockName}}" {
+	name = "tf-acc-test"
+}
+
+resource "oxide_vpc" "{{.SupportBlockName2}}" {
+	project_id        = data.oxide_project.{{.SupportBlockName}}.id
+	description       = "a test vpc"
+	name              = "{{.VPCName}}"
+	dns_name          = "my-vpc-dns"
+}
+
+resource "oxide_vpc_firewall_rules" "{{.BlockName}}" {
+	vpc_id = oxide_vpc.{{.SupportBlockName2}}.id
+	rules = []
+}
+`
+
 func TestAccCloudResourceFirewallRules_full(t *testing.T) {
 	blockName := newBlockName("firewall_rules")
 	supportBlockName := newBlockName("support")
@@ -202,7 +220,7 @@ func TestAccCloudResourceFirewallRules_full(t *testing.T) {
 		resourceFirewallRulesUpdateConfigTpl,
 	)
 	if err != nil {
-		t.Errorf("error parsing config template data: %e", err)
+		t.Errorf("error parsing update config template data: %e", err)
 	}
 
 	configUpdate2, err := parsedAccConfig(
@@ -215,7 +233,20 @@ func TestAccCloudResourceFirewallRules_full(t *testing.T) {
 		resourceFirewallRulesUpdateConfigTpl2,
 	)
 	if err != nil {
-		t.Errorf("error parsing config template data: %e", err)
+		t.Errorf("error parsing update config 2 template data: %e", err)
+	}
+
+	configUpdate3, err := parsedAccConfig(
+		resourceFirewallRulesConfig{
+			BlockName:         blockName,
+			SupportBlockName:  supportBlockName,
+			SupportBlockName2: supportBlockName2,
+			VPCName:           vpcName,
+		},
+		resourceFirewallRulesUpdateConfigTpl3,
+	)
+	if err != nil {
+		t.Errorf("error parsing update config 3 template data: %e", err)
 	}
 
 	resource.ParallelTest(t, resource.TestCase{
@@ -235,6 +266,10 @@ func TestAccCloudResourceFirewallRules_full(t *testing.T) {
 				Config: configUpdate2,
 				Check:  checkResourceFirewallRulesUpdate2(resourceName, vpcName),
 			},
+			{
+				Config: configUpdate3,
+				Check:  checkResourceFirewallRulesUpdate3(resourceName),
+			},
 		},
 	})
 }
@@ -321,6 +356,14 @@ func checkResourceFirewallRulesUpdate2(resourceName, vpcName string) resource.Te
 	}...)
 }
 
+func checkResourceFirewallRulesUpdate3(resourceName string) resource.TestCheckFunc {
+	return resource.ComposeAggregateTestCheckFunc([]resource.TestCheckFunc{
+		resource.TestCheckResourceAttrSet(resourceName, "id"),
+		resource.TestCheckResourceAttrSet(resourceName, "vpc_id"),
+		resource.TestCheckResourceAttr(resourceName, "rules.#", "0"),
+	}...)
+}
+
 func testAccFirewallRulesDestroy(s *terraform.State) error {
 	client, err := newTestClient()
 	if err != nil {
